Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cliv2/cmd/cliv2/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
18 changes: 18 additions & 0 deletions cliv2/internal/cliv2/cliv2.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,13 @@ const (
const (
configKeyErrFile = "INTERNAL_ERR_FILE_PATH"
ERROR_HAS_BEEN_DISPLAYED = "hasBeenDisplayed"
// ConfigKeyRequestConcurrency is the configuration key holding the
Comment thread
CatalinSnyk marked this conversation as resolved.
// 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 (
Expand Down Expand Up @@ -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,
}

Expand Down Expand Up @@ -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"
}
Expand Down
50 changes: 50 additions & 0 deletions cliv2/internal/cliv2/cliv2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
1 change: 1 addition & 0 deletions cliv2/internal/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
24 changes: 24 additions & 0 deletions src/lib/snyk-test/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Comment thread
PeterSchafer marked this conversation as resolved.
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.
Expand Down
6 changes: 2 additions & 4 deletions src/lib/snyk-test/run-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import { isCI } from '../is-ci';
import {
RETRY_ATTEMPTS,
RETRY_DELAY,
getRequestConcurrency,
printDepGraph,
printDepGraphJsonl,
printDepGraphError,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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) {
Expand Down
62 changes: 61 additions & 1 deletion test/jest/unit/lib/snyk-test/common.spec.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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);
});
});
Loading