feat: use cli-extension-iac [IAC-3213] #5730

sergiu-snyk · 2025-02-11T12:51:04Z

Pull Request Submission Checklist

Follows CONTRIBUTING guidelines
Includes detailed description of changes
Contains risk assessment (Low | Medium | High)
Highlights breaking API changes (if applicable)
Links to automated tests covering new functionality
Includes manual testing instructions (if necessary)
Updates relevant GitBook documentation (PR link: ___)

What does this PR do?

Enables the new IaC CLI extension that replaces snyk-iac-test.
A couple of acceptance tests exercising invalid flags for command snyk iac test had to be modified due to the default CLI invalid flag behavior for extensions.

A cleanup PR for the snyk-iac-test binary will follow after we release this and if no problems reported.

Where should the reviewer start?

How should this be manually tested?

Acceptance tests cover the IaC functionality. But to run manually use snyk iac test.

sergiu-snyk · 2025-03-24T08:24:52Z

cliv2/Makefile

@@ -151,7 +151,8 @@ $(BUILD_DIR)/$(V2_EXECUTABLE_NAME): $(BUILD_DIR) $(SRCS) generate-ls-protocol-me
 	@echo "$(LOG_PREFIX) Building ( $(BUILD_DIR)/$(V2_EXECUTABLE_NAME) )"
 	@echo "$(LOG_PREFIX)  LDFLAGS:     $(LDFLAGS)"
 	@echo "$(LOG_PREFIX)  GCFLAGS:     $(GCFLAGS)"
-	@GOEXPERIMENT=$(FIPS_CRYPTO_BACKEND) GOOS=$(_GO_OS) GOARCH=$(GOARCH) $(GOCMD) build -tags=application -ldflags="$(LDFLAGS) -X github.com/snyk/snyk-ls/application/config.Version=$(LS_COMMIT_HASH) -X github.com/snyk/snyk-ls/application/config.LsProtocolVersion=$(LS_PROTOCOL_VERSION)  -X main.internalOS=$(GOOS) -X github.com/snyk/cli/cliv2/internal/embedded/cliv1.snykCLIVersion=$(CLI_V1_VERSION_TAG)" $(GCFLAGS) -o $(BUILD_DIR)/$(V2_EXECUTABLE_NAME) $(WORKING_DIR)/cmd/cliv2/*.go
+	@echo "$(LOG_PREFIX)  IAC_RULES_URL:     $(IAC_RULES_URL)"
+	@GOEXPERIMENT=$(FIPS_CRYPTO_BACKEND) GOOS=$(_GO_OS) GOARCH=$(GOARCH) $(GOCMD) build -tags=application -ldflags="$(LDFLAGS) -X github.com/snyk/snyk-ls/application/config.Version=$(LS_COMMIT_HASH) -X github.com/snyk/snyk-ls/application/config.LsProtocolVersion=$(LS_PROTOCOL_VERSION)  -X main.internalOS=$(GOOS) -X github.com/snyk/cli/cliv2/internal/embedded/cliv1.snykCLIVersion=$(CLI_V1_VERSION_TAG) -X github.com/snyk/cli-extension-iac/internal/commands/iactest.internalRulesClientURL=$(IAC_RULES_URL)" $(GCFLAGS) -o $(BUILD_DIR)/$(V2_EXECUTABLE_NAME) $(WORKING_DIR)/cmd/cliv2/*.go


injected IAC_RULES_URL at build time as discussed with @PeterSchafer. This is for now an environment variable in the CLI project, but could be moved to a context if that's better.

Yes, please move it to a context. The usage won't change though.
Thank you!

sergiu-snyk · 2025-03-28T10:03:39Z

test/jest/acceptance/iac/cli-share-results.spec.ts

@@ -91,7 +91,7 @@ describe('CLI Share Results', () => {

    it('forwards project tags', async () => {
      const { exitCode } = await run(
-        'snyk iac test ./iac/arm/rule_test.json --report --tags=foo=bar',
+        'snyk iac test ./iac/arm/rule_test.json --report --project-tags=foo=bar',


--tags is an alias for --project-tags that should not be used, and is no longer supported. I don't consider this a breaking change, since it is undocumented in the command help, or any online documentation. In the very unlikely event anyone uses it, the command help should point them to use --project-tags

j-luong · 2025-04-01T10:24:09Z

src/lib/iac/test/v2/analytics/iac-cloud-context.ts

@@ -11,7 +11,7 @@ type IacCloudContext = Pick<
 >;

 export function getIacCloudContext(
-  testConfig: TestConfig,
+  testConfig: Partial<TestConfig>,


question: why not use snykCloudEnvironment?: string here instead of a partially applied interface? Makes it much clearer what the parameters are.

You're right. IDK why didn't consider it.

j-luong · 2025-04-01T10:32:37Z

src/cli/commands/test/iac/v2/index.ts

 import { getFlag } from '../index';
 import { IaCTestFlags } from '../local-execution/types';
 import { findAndLoadPolicy } from '../../../../../lib/policy';
 import { assertIacV2Options } from './assert-iac-options';
+import { addIacAnalytics } from '../../../../../lib/iac/test/v2/analytics';


note: the analytics here will be deprecated at some point in favour of instrumentation data in Go, the analytics payload constructed via addIacAnalytics can be preserved when we eventually migrate, but any dashboards/monitoring reliant on these analytics might be impacted. Just something to be aware of.

j-luong · 2025-04-01T10:39:12Z

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

+): Promise<TestOutput> {
+  const results = await readJson(outputFilePath);
+
+  return mapSnykIacTestOutputToTestOutput(results);


suggestion: now that we support Error Catalog errors, this is a good opportunity to update the errors thrown by mapSnykIacTestOutputToTestOutput() to more specific Error Catalog errors (if applicable)

That's something we considered, but for now we haven't identified good candidates for more specific errors (errors that would benefit from providing more context in form of external links), and we use some of the existing ones (those related to invalid flags). The error specific details for the generic IaC failure error should be good enough.

j-luong · 2025-04-01T11:20:06Z

question: how will the file reading/writing behave for multiple concurrent reads/writes? It doesn't look like it's tested?

PeterSchafer · 2025-04-01T12:30:16Z

cliv2/Makefile

@@ -151,7 +151,8 @@ $(BUILD_DIR)/$(V2_EXECUTABLE_NAME): $(BUILD_DIR) $(SRCS) generate-ls-protocol-me
 	@echo "$(LOG_PREFIX) Building ( $(BUILD_DIR)/$(V2_EXECUTABLE_NAME) )"
 	@echo "$(LOG_PREFIX)  LDFLAGS:     $(LDFLAGS)"
 	@echo "$(LOG_PREFIX)  GCFLAGS:     $(GCFLAGS)"
-	@GOEXPERIMENT=$(FIPS_CRYPTO_BACKEND) GOOS=$(_GO_OS) GOARCH=$(GOARCH) $(GOCMD) build -tags=application -ldflags="$(LDFLAGS) -X github.com/snyk/snyk-ls/application/config.Version=$(LS_COMMIT_HASH) -X github.com/snyk/snyk-ls/application/config.LsProtocolVersion=$(LS_PROTOCOL_VERSION)  -X main.internalOS=$(GOOS) -X github.com/snyk/cli/cliv2/internal/embedded/cliv1.snykCLIVersion=$(CLI_V1_VERSION_TAG)" $(GCFLAGS) -o $(BUILD_DIR)/$(V2_EXECUTABLE_NAME) $(WORKING_DIR)/cmd/cliv2/*.go
+	@echo "$(LOG_PREFIX)  IAC_RULES_URL:     $(IAC_RULES_URL)"


Question: What is the developer experience when this Environment Variable is not specified? Is it worth to explicitly warn when it is empty?

Nitpick: Maybe move this to where LS version: is printed 😄

This is the error I see, right now.

Can we make the test a bit more descriptive and actionable? I'm worried about all the asks we will be getting because someone is using a dev CLI without the URL set.

Yes, good points. I think we should actually fail the build if that bundle is not provided, wdyt?

Maybe we fail the command execution instead?!

It fails right now, doesn't it? Just that the message is not very actionable. like you said.

yes, it fails, but it seems to do a lot of things until it fails with the message right now. So a proposal could be to do a check for the variable and fail early and actionable. Does this make sense?

for example here

Gotcha! Makes sense!

sergiu-snyk · 2025-04-01T12:38:16Z

question: how will the file reading/writing behave for multiple concurrent reads/writes? It doesn't look like it's tested?

You mean the file provided via --iac-test-output-file? That's created by our extension in a temp dir based on the pid (go-application-framework provides it via TEMP_DIR_PATH), and we also put an uuid in the file name, just in case. There's only one such file per command execution, and two separate executions would use different files.
Also the read happens after the write, always.

It is essentially the same mechanism that the existing snyk-iac-test binary uses, nothing changed here except passing that file name via argument from the extension to the legacy CLI, instead of being generated by the snyk-iac-test process at the path provided to it by the legacy CLI via --output, and then read in the legacy CLI here

sergiu-snyk force-pushed the feat/IAC-3213/cli-extension-iac branch 4 times, most recently from 6ed4749 to 40b2247 Compare February 12, 2025 11:11

sergiu-snyk force-pushed the feat/IAC-3213/cli-extension-iac branch 3 times, most recently from 522620c to 5f07b9b Compare March 21, 2025 08:19

sergiu-snyk commented Mar 24, 2025

View reviewed changes

sergiu-snyk force-pushed the feat/IAC-3213/cli-extension-iac branch 2 times, most recently from 5a241da to 6a1e8f9 Compare March 25, 2025 17:52

snyk deleted a comment from github-actions bot Mar 25, 2025

feat: use cli-extension-iac [IAC-3213]

edda6f2

sergiu-snyk force-pushed the feat/IAC-3213/cli-extension-iac branch from 6a1e8f9 to edda6f2 Compare March 25, 2025 18:17

sergiu-snyk marked this pull request as ready for review March 27, 2025 08:57

sergiu-snyk requested a review from a team as a code owner March 27, 2025 08:57

sergiu-snyk commented Mar 28, 2025

View reviewed changes

j-luong reviewed Apr 1, 2025

View reviewed changes

PeterSchafer reviewed Apr 1, 2025

View reviewed changes

sergiu-snyk mentioned this pull request Apr 4, 2025

fix: early return if rule bundle URL not set [IAC-3123] snyk/cli-extension-iac#20

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: use cli-extension-iac [IAC-3213] #5730

feat: use cli-extension-iac [IAC-3213] #5730

sergiu-snyk commented Feb 11, 2025 •

edited

Loading

sergiu-snyk Mar 24, 2025 •

edited

Loading

PeterSchafer Apr 1, 2025

sergiu-snyk Mar 28, 2025

j-luong Apr 1, 2025

sergiu-snyk Apr 1, 2025

j-luong Apr 1, 2025

j-luong Apr 1, 2025

sergiu-snyk Apr 1, 2025

j-luong commented Apr 1, 2025

PeterSchafer Apr 1, 2025

PeterSchafer Apr 1, 2025

sergiu-snyk Apr 1, 2025

PeterSchafer Apr 1, 2025

sergiu-snyk Apr 1, 2025 •

edited

Loading

PeterSchafer Apr 1, 2025 •

edited

Loading

PeterSchafer Apr 1, 2025

sergiu-snyk Apr 1, 2025

sergiu-snyk commented Apr 1, 2025

feat: use cli-extension-iac [IAC-3213] #5730

Are you sure you want to change the base?

feat: use cli-extension-iac [IAC-3213] #5730

Conversation

sergiu-snyk commented Feb 11, 2025 • edited Loading

Pull Request Submission Checklist

What does this PR do?

Where should the reviewer start?

How should this be manually tested?

sergiu-snyk Mar 24, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

j-luong commented Apr 1, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sergiu-snyk Apr 1, 2025 • edited Loading

Choose a reason for hiding this comment

PeterSchafer Apr 1, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sergiu-snyk commented Apr 1, 2025

sergiu-snyk commented Feb 11, 2025 •

edited

Loading

sergiu-snyk Mar 24, 2025 •

edited

Loading

sergiu-snyk Apr 1, 2025 •

edited

Loading

PeterSchafer Apr 1, 2025 •

edited

Loading