feat: upgrade snyk-iac-test to v0.51.3

[jaspervdj-snyk]

Pull Request Submission

Please check the boxes once done.

The pull request must:

• Reviewer Documentation
  • follow CONTRIBUTING rules
  • be accompanied by a detailed description of the changes
  • contain a risk assessment of the change (Low | Medium | High) with regards to breaking existing functionality. A change e.g. of an underlying language plugin can completely break the functionality for that language, but appearing as only a version change in the dependencies.
  • highlight breaking API if applicable
  • contain a link to the automatic tests that cover the updated functionality.
  • contain testing instructions in case that the reviewer wants to manual verify as well, to add to the manual testing done by the author.
  • link to the link to the PR for the User-facing documentation
• User facing Documentation
  • update any relevant documentation in gitbook by submitting a gitbook PR, and including the PR link here
  • ensure that the message of the final single commit is descriptive and prefixed with either feat: or fix: , others might be used in rare occasions as well, if there is no need to document the changes in the release notes. The changes or fixes should be described in detail in the commit message for the changelog & release notes.
• Testing
  • Changes, removals and additions to functionality must be covered by acceptance / integration tests or smoke tests - either already existing ones, or new ones, created by the author of the PR.

Pull Request Approval

Once a pull request has been reviewed and all necessary revisions have been made, it is approved for merging into
the main codebase. The merging of the code PR is performed by the code owners, the merging of the documentation PR
by our content writers.

What does this PR do?

Upgrade snyk-iac-test to v0.51.3. Changes in the snyk-iac-test -> upgrades for policy-engine to version that have the following fixes:

• Print scan URL to debug output after uploading report.
• Fix panic on invalid arm input
• Fix panic on invalid tf input
• Fix tfplan policy-engine detection for resources defined with counts

Risk: LOW

The main change is related to showing an addition log line for when using IaC custom debug. The upgrade of this exec is hit only for when using IaC+ (docs) that is a product that is in Open Beta.

Where should the reviewer start?

snyk-iac-test changes: https://github.com/snyk/snyk-iac-test/compare/v0.50.4...v0.51.3
policy-engine changes: snyk/policy-engine@v0.30.5...v0.30.9

These are not adding any changes of the output we get in the CLI from snyk-iac-test.

How should this be manually tested?

See this: #5127 (comment)

Any background context you want to provide?

About automated tests: #5127 (comment)

What are the relevant tickets?

IA-156

Screenshots

Additional questions

[github-actions]

	Warnings
⚠️	You've modified files in src/ directory, but haven't updated anything in test folder. Is there something that could be tested?

Generated by 🚫 dangerJS against 0b781a9

[cmars]

Questions:

• Have you manually tested this e2e with a CLI build, where snyk iac test is driving the updated binary, and confirmed there are no regressions?
• Looking at the source changes, this new version appears to use a value derived from an environment variable SNYK_IAC_TEST_API_REST_URL when constructing a platform.SnykPlatformClient. Is that set by the CLI, or is there potential for regression here?

[andreeaneata]

Questions:

• Have you manually tested this e2e with a CLI build, where snyk iac test is driving the updated binary, and confirmed there are no regressions?
• Looking at the source changes, this new version appears to use a value derived from an environment variable SNYK_IAC_TEST_API_REST_URL when constructing a platform.SnykPlatformClient. Is that set by the CLI, or is there potential for regression here?

Hi @cmars!

Yes, this has been tested. There seem to be a lot of changes, but all of them are internal and the only change that will be reflected in here will be a log line.

The SNYK_IAC_TEST_API_REST_URL is something we already use in snyk-iac-test module and it gets in there like this:

cli/src/lib/iac/test/v2/scan/index.ts

Lines 64 to 87 in 74c864e

	env['SNYK_IAC_TEST_API_REST_URL'] =
	process.env['SNYK_IAC_TEST_API_REST_URL'] || apiUrl;
	env['SNYK_IAC_TEST_API_REST_TOKEN'] =
	process.env['SNYK_IAC_TEST_API_REST_TOKEN'] || getApiToken();
	env['SNYK_IAC_TEST_API_REST_OAUTH_TOKEN'] =
	process.env['SNYK_IAC_TEST_API_REST_OAUTH_TOKEN'] || getOAuthToken();
	env['SNYK_IAC_TEST_API_V1_URL'] =
	process.env['SNYK_IAC_TEST_API_V1_URL'] || apiUrl;
	env['SNYK_IAC_TEST_API_V1_TOKEN'] =
	process.env['SNYK_IAC_TEST_API_V1_TOKEN'] || getApiToken();
	env['SNYK_IAC_TEST_API_V1_OAUTH_TOKEN'] =
	process.env['SNYK_IAC_TEST_API_V1_OAUTH_TOKEN'] || getOAuthToken();
	const args = processFlags(
	options,
	rulesBundlePath,
	rulesClientURL,
	outputPath,
	policyPath,
	);
	args.push(...options.paths);
	const child = await spawn(policyEnginePath, args, env);

How I've tested this:

1. The snyk-iac-test exec to used can be overwritten using this env var: SNYK_IAC_POLICY_ENGINE_PATH (I've build the snyk-iac-test on main)
2. I added the extra debug params to be able to see our added log line and the command needs to be with --report to see the changes: SNYK_IAC_POLICY_ENGINE_PATH=/Users/andreea-theodora.neata/dev/snyk-iac-test/snyk-iac-test DEBUG=snyk:iac:output snyk iac test --org=bcbea77c-d8a3-423a-917b-3e44d9d55b77 -d (the org had to have IaC+ enabled - snykCloud and snykCloudDataModel entitlements + iacIntegratedExperience FF)
3. Results:

And I've also tested: without IaC+ (the flow that is not using this executable), without --report, without the debug params etc -> everything seems right.

[andreeaneata]

@cmars the last upgrade I've added only contains a small fix in the engine we use in snyk-iac-test (0.51.1 -> 0.51.2)

[andreeaneata]

@cmars I've added a new minor upgrade -> contains a fix in the engine we use in snyk-iac-test (0.51.2 -> 0.51.3)

[PeterSchafer]

Hi @andreeaneata
thanks for your answers above. When you say -> everything seems right. I wonder if we have somwhere automated tests that verify this as well.
The main question that we are struggling to answer with such type of PR's is, will this introduce a regression for existing functionality and how do we ensure that the added features and fixes don't experience regression in the future. Manual tests are a good add-on on automated tests though.

The original PR template asks for links to tests that verify the changes. They can be in the CLI repo or in the related repo.

[PeterSchafer]

In addition, looking at the commit message and PR title, for our users and us, it is difficult to understand what they really get out of this. Please take a look at this on how to write these user facing messages.
Thank you!

[andreeaneata]

Hi @andreeaneata thanks for your answers above. When you say -> everything seems right. I wonder if we have somwhere automated tests that verify this as well. The main question that we are struggling to answer with such type of PR's is, will this introduce a regression for existing functionality and how do we ensure that the added features and fixes don't experience regression in the future. Manual tests are a good add-on on automated tests though.

The original PR template asks for links to tests that verify the changes. They can be in the CLI repo or in the related repo.

Hi @PeterSchafer!

We have some tests in this repo that run as unit tests and also as acceptance tests that verify regressions:

1. Acceptance: https://github.com/snyk/cli/tree/main/test/jest/acceptance/iac
2. Unit: https://github.com/snyk/cli/tree/main/test/jest/unit/iac

They run as part of the PR check and those passed (I also run them on my local).

After this PR will be merged we also have some E2E environment tests that will verify regressions for changes we add here: https://github.com/snyk/environment-testing/tree/main/test/cli/tests/iac

I understand your concerns and we will do better in the future with the PR descriptions.

[andreeaneata]

@PeterSchafer Our colleagues released fixes for our policies + new rules and this PR is holding that get to our customers since the latest rules bundle is downloaded automatically based on the needed policy-engine version that is hidden by the snyk-iac-test upgrade. Can we sync somehow to understand what more is needed to move this forward? 🙏

LE: I've updated the PR description

CC: @uogbonna @wayne-snyk to not get yet to the support tickets since this was not released yet - PE version before this - v0.30.5

[cmars]

LGTM. Thanks for the background & context on this change, and sorry for the delay in getting back to you!.