diff --git a/cliv2/cmd/cliv2/main.go b/cliv2/cmd/cliv2/main.go index 47611c4891..66930a0c9c 100644 --- a/cliv2/cmd/cliv2/main.go +++ b/cliv2/cmd/cliv2/main.go @@ -121,6 +121,7 @@ func initApplicationConfiguration(config configuration.Configuration) { config.AddAlternativeKeys(configuration.LOG_LEVEL, []string{debug_level_flag}) config.AddAlternativeKeys(configuration.INTEGRATION_NAME, []string{integrationNameFlag}) config.AddAlternativeKeys(middleware.ConfigurationKeyRequestAttempts, []string{"snyk_max_attempts", maxNetworkRequestAttempts}) + config.AddAlternativeKeys(cliv2.ConfigKeyRequestConcurrency, []string{"snyk_request_concurrency"}) } func getFullCommandString(cmd *cobra.Command) string { diff --git a/cliv2/internal/cliv2/cliv2.go b/cliv2/internal/cliv2/cliv2.go index bac899a594..cb62d2ebce 100644 --- a/cliv2/internal/cliv2/cliv2.go +++ b/cliv2/internal/cliv2/cliv2.go @@ -66,6 +66,13 @@ const ( const ( configKeyErrFile = "INTERNAL_ERR_FILE_PATH" ERROR_HAS_BEEN_DISPLAYED = "hasBeenDisplayed" + // ConfigKeyRequestConcurrency is the configuration key holding the + // resolved maximum number of concurrent in-flight Snyk dependency-test + // or dependency-monitor HTTP requests issued by the legacy CLI. The + // user-facing SNYK_REQUEST_CONCURRENCY env var feeds this key (registered + // in main.go via AddAlternativeKeys); the resolved value is forwarded to + // the legacy CLI process via constants.SNYK_INTERNAL_REQUEST_CONCURRENCY_ENV. + ConfigKeyRequestConcurrency = "internal_request_concurrency" ) var ( @@ -355,6 +362,7 @@ func PrepareV1EnvironmentVariables( constants.SNYK_NPM_ALL_PROXY, constants.SNYK_OPENSSL_CONF, constants.SNYK_INTERNAL_PREVIEW_FEATURES_ENABLED, + constants.SNYK_INTERNAL_REQUEST_CONCURRENCY_ENV, constants.DEBUG_CONST, } @@ -391,6 +399,16 @@ func fillEnvironmentFromConfig(inputAsMap map[string]string, config configuratio inputAsMap[constants.SNYK_INTERNAL_ERR_FILE] = config.GetString(configKeyErrFile) inputAsMap[constants.SNYK_TEMP_PATH] = config.GetString(configuration.TEMP_DIR_PATH) + // Forward the resolved request concurrency to the legacy CLI when the user + // set the value. We can't use config.IsSet here: in GAF, IsSet does not + // pre-bind env vars for alternative keys, so it returns false even when + // the SNYK_REQUEST_CONCURRENCY env var is set under WithSupportedEnvVarPrefixes + // (the production setup). GetString goes through GAF's get(), which binds + // the alt key before reading, so it returns the resolved value correctly. + if v := config.GetString(ConfigKeyRequestConcurrency); v != "" { + inputAsMap[constants.SNYK_INTERNAL_REQUEST_CONCURRENCY_ENV] = v + } + if config.GetBool(configuration.PREVIEW_FEATURES_ENABLED) { inputAsMap[constants.SNYK_INTERNAL_PREVIEW_FEATURES_ENABLED] = "1" } diff --git a/cliv2/internal/cliv2/cliv2_test.go b/cliv2/internal/cliv2/cliv2_test.go index 847f66f2da..54da22d7ae 100644 --- a/cliv2/internal/cliv2/cliv2_test.go +++ b/cliv2/internal/cliv2/cliv2_test.go @@ -270,6 +270,56 @@ func Test_PrepareV1EnvironmentVariables_OnlyExplicitlySetValues(t *testing.T) { }) } +func Test_PrepareV1EnvironmentVariables_RequestConcurrency(t *testing.T) { + // Mirror main.go's production configuration setup. Crucially, this uses + // WithSupportedEnvVarPrefixes (NOT WithAutomaticEnv): under that setup, + // GAF's IsSet does not pre-bind env vars for alternative keys, so any + // implementation that gates forwarding on IsSet would fail to forward + // the value. This test catches that regression. + newConfig := func() configuration.Configuration { + c := configuration.NewWithOpts( + configuration.WithSupportedEnvVarPrefixes("snyk_", "internal_", "test_"), + ) + c.AddAlternativeKeys(cliv2.ConfigKeyRequestConcurrency, []string{"snyk_request_concurrency"}) + return c + } + + t.Run("forwards resolved value to internal env when alt key is set via env", func(t *testing.T) { + t.Setenv("SNYK_REQUEST_CONCURRENCY", "17") + + actual, err := cliv2.PrepareV1EnvironmentVariables([]string{}, "foo", "bar", "proxy", "cacertlocation", newConfig(), []string{}) + + assert.Nil(t, err) + assert.Contains(t, actual, constants.SNYK_INTERNAL_REQUEST_CONCURRENCY_ENV+"=17") + }) + + t.Run("does not set internal env when alt key is unset", func(t *testing.T) { + // guard against a stray env var leaking into the test environment + t.Setenv("SNYK_REQUEST_CONCURRENCY", "") + _ = os.Unsetenv("SNYK_REQUEST_CONCURRENCY") + + actual, err := cliv2.PrepareV1EnvironmentVariables([]string{}, "foo", "bar", "proxy", "cacertlocation", newConfig(), []string{}) + + assert.Nil(t, err) + for _, kv := range actual { + assert.NotContains(t, kv, constants.SNYK_INTERNAL_REQUEST_CONCURRENCY_ENV+"=") + } + }) + + t.Run("user-set internal env is stripped before Go reapplies it", func(t *testing.T) { + t.Setenv("SNYK_REQUEST_CONCURRENCY", "9") + + // Simulate a user trying to bypass Go config by setting the internal var directly. + input := []string{constants.SNYK_INTERNAL_REQUEST_CONCURRENCY_ENV + "=999"} + + actual, err := cliv2.PrepareV1EnvironmentVariables(input, "foo", "bar", "proxy", "cacertlocation", newConfig(), []string{}) + + assert.Nil(t, err) + assert.Contains(t, actual, constants.SNYK_INTERNAL_REQUEST_CONCURRENCY_ENV+"=9") + assert.NotContains(t, actual, constants.SNYK_INTERNAL_REQUEST_CONCURRENCY_ENV+"=999") + }) +} + func Test_PrepareV1EnvironmentVariables_Fail_DontOverrideExisting(t *testing.T) { orgid := "orgid" testapi := "https://api.snyky.io" diff --git a/cliv2/internal/constants/constants.go b/cliv2/internal/constants/constants.go index 7b22c85e5d..ff082d0ea4 100644 --- a/cliv2/internal/constants/constants.go +++ b/cliv2/internal/constants/constants.go @@ -28,6 +28,7 @@ const DEBUG_CONST = "DEBUG" const SNYK_INTERNAL_ORGID_ENV = "SNYK_INTERNAL_ORGID" const SNYK_INTERNAL_ERR_FILE = "SNYK_ERR_FILE" const SNYK_INTERNAL_PREVIEW_FEATURES_ENABLED = "SNYK_INTERNAL_PREVIEW_FEATURES" +const SNYK_INTERNAL_REQUEST_CONCURRENCY_ENV = "SNYK_INTERNAL_REQUEST_CONCURRENCY" const SNYK_ENDPOINT_ENV = "SNYK_API" const SNYK_ORG_ENV = "SNYK_CFG_ORG" const SNYK_OPENSSL_CONF = "OPENSSL_CONF" diff --git a/src/lib/snyk-test/common.ts b/src/lib/snyk-test/common.ts index 8ebbbfe7d4..50c239c8b0 100644 --- a/src/lib/snyk-test/common.ts +++ b/src/lib/snyk-test/common.ts @@ -111,6 +111,30 @@ export type FailOn = 'all' | 'upgradable' | 'patchable'; export const RETRY_ATTEMPTS = 3; export const RETRY_DELAY = 500; +const DEFAULT_REQUEST_CONCURRENCY = 5; +const MIN_REQUEST_CONCURRENCY = 1; +const MAX_REQUEST_CONCURRENCY = 50; + +/** + * Returns the maximum number of in-flight Snyk dependency-test or + * dependency-monitor HTTP requests permitted at once. The wrapping Go CLI + * resolves the user-facing SNYK_REQUEST_CONCURRENCY env var (and any future + * config-file/flag sources) and forwards the resolved value via the internal + * SNYK_INTERNAL_REQUEST_CONCURRENCY env var read here. Values are clamped to + * [MIN_REQUEST_CONCURRENCY, MAX_REQUEST_CONCURRENCY]. + */ +export function getRequestConcurrency(): number { + const raw = process.env.SNYK_INTERNAL_REQUEST_CONCURRENCY; + if (!raw) { + return DEFAULT_REQUEST_CONCURRENCY; + } + const parsed = parseInt(raw, 10); + if (!Number.isFinite(parsed) || parsed < MIN_REQUEST_CONCURRENCY) { + return DEFAULT_REQUEST_CONCURRENCY; + } + return Math.min(parsed, MAX_REQUEST_CONCURRENCY); +} + /** * printDepGraph writes the given dep-graph and target name to the destination * stream as expected by the `depgraph` CLI workflow. diff --git a/src/lib/snyk-test/run-test.ts b/src/lib/snyk-test/run-test.ts index 8c50e4b844..89906cc347 100644 --- a/src/lib/snyk-test/run-test.ts +++ b/src/lib/snyk-test/run-test.ts @@ -39,6 +39,7 @@ import { isCI } from '../is-ci'; import { RETRY_ATTEMPTS, RETRY_DELAY, + getRequestConcurrency, printDepGraph, printDepGraphJsonl, printDepGraphError, @@ -94,9 +95,6 @@ import { ProblemError } from '@snyk/error-catalog-nodejs-public'; const debug = debugModule('snyk:run-test'); -// Controls the number of simultaneous test requests that can be in-flight. -const MAX_CONCURRENCY = 5; - function prepareResponseForParsing( payload: Payload, response: TestDependenciesResponse, @@ -297,7 +295,7 @@ async function sendAndParseResults( }; const responses = await pMap(payloads, sendRequest, { - concurrency: MAX_CONCURRENCY, + concurrency: getRequestConcurrency(), }); for (const { payload, originalPayload, response } of responses) { diff --git a/test/jest/unit/lib/snyk-test/common.spec.ts b/test/jest/unit/lib/snyk-test/common.spec.ts index 5afe8cad37..07a1d6ad75 100644 --- a/test/jest/unit/lib/snyk-test/common.spec.ts +++ b/test/jest/unit/lib/snyk-test/common.spec.ts @@ -1,7 +1,10 @@ import { CLI, ProblemError } from '@snyk/error-catalog-nodejs-public'; import { CustomError } from '../../../../../src/lib/errors'; import { FailedProjectScanError } from '../../../../../src/lib/plugins/get-multi-plugin-result'; -import { getOrCreateErrorCatalogError } from '../../../../../src/lib/snyk-test/common'; +import { + getOrCreateErrorCatalogError, + getRequestConcurrency, +} from '../../../../../src/lib/snyk-test/common'; describe('getOrCreateErrorCatalogError', () => { const defaultErrMessage = 'Default error message'; @@ -116,3 +119,60 @@ describe('getOrCreateErrorCatalogError', () => { expect(result.detail).toBe(defaultErrMessage); }); }); + +describe('getRequestConcurrency', () => { + const originalValue = process.env.SNYK_INTERNAL_REQUEST_CONCURRENCY; + + afterEach(() => { + if (originalValue === undefined) { + delete process.env.SNYK_INTERNAL_REQUEST_CONCURRENCY; + } else { + process.env.SNYK_INTERNAL_REQUEST_CONCURRENCY = originalValue; + } + }); + + it('returns the default of 5 when SNYK_INTERNAL_REQUEST_CONCURRENCY is unset', () => { + delete process.env.SNYK_INTERNAL_REQUEST_CONCURRENCY; + expect(getRequestConcurrency()).toBe(5); + }); + + it('returns the default of 5 when SNYK_INTERNAL_REQUEST_CONCURRENCY is empty', () => { + process.env.SNYK_INTERNAL_REQUEST_CONCURRENCY = ''; + expect(getRequestConcurrency()).toBe(5); + }); + + it('returns the parsed value when SNYK_INTERNAL_REQUEST_CONCURRENCY is a valid integer', () => { + process.env.SNYK_INTERNAL_REQUEST_CONCURRENCY = '15'; + expect(getRequestConcurrency()).toBe(15); + }); + + it('clamps to the maximum of 50 when SNYK_INTERNAL_REQUEST_CONCURRENCY exceeds the cap', () => { + process.env.SNYK_INTERNAL_REQUEST_CONCURRENCY = '500'; + expect(getRequestConcurrency()).toBe(50); + }); + + it('returns the default when SNYK_INTERNAL_REQUEST_CONCURRENCY is below the minimum', () => { + process.env.SNYK_INTERNAL_REQUEST_CONCURRENCY = '0'; + expect(getRequestConcurrency()).toBe(5); + }); + + it('returns the default when SNYK_INTERNAL_REQUEST_CONCURRENCY is negative', () => { + process.env.SNYK_INTERNAL_REQUEST_CONCURRENCY = '-5'; + expect(getRequestConcurrency()).toBe(5); + }); + + it('returns the default when SNYK_INTERNAL_REQUEST_CONCURRENCY is non-numeric', () => { + process.env.SNYK_INTERNAL_REQUEST_CONCURRENCY = 'abc'; + expect(getRequestConcurrency()).toBe(5); + }); + + it('honors the minimum boundary', () => { + process.env.SNYK_INTERNAL_REQUEST_CONCURRENCY = '1'; + expect(getRequestConcurrency()).toBe(1); + }); + + it('honors the maximum boundary', () => { + process.env.SNYK_INTERNAL_REQUEST_CONCURRENCY = '50'; + expect(getRequestConcurrency()).toBe(50); + }); +});