From 78d0602a2efc700084c5f05a3105c8124e0daf0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20Scha=CC=88fer?= <101886095+PeterSchafer@users.noreply.github.com> Date: Fri, 10 Jun 2022 16:17:21 +0200 Subject: [PATCH] fix: also add HTTP_PROXY environment variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Peter Schäfer <101886095+PeterSchafer@users.noreply.github.com> chore: skip tests that cannot be satisfied with cliv2 Signed-off-by: Peter Schäfer <101886095+PeterSchafer@users.noreply.github.com> Signed-off-by: Peter Schäfer <101886095+PeterSchafer@users.noreply.github.com> Signed-off-by: Peter Schäfer <101886095+PeterSchafer@users.noreply.github.com> chore: override env variables instead of appending When creating the environment variables for the CLIv1 invocation ensure to override existing and not only append environment variables. Signed-off-by: Peter Schäfer <101886095+PeterSchafer@users.noreply.github.com> --- cliv2/internal/cliv2/cliv2.go | 27 +++++---- cliv2/internal/cliv2/cliv2_test.go | 19 +++--- test/jest/acceptance/proxy-behavior.spec.ts | 65 +++++++++++---------- 3 files changed, 59 insertions(+), 52 deletions(-) diff --git a/cliv2/internal/cliv2/cliv2.go b/cliv2/internal/cliv2/cliv2.go index dd80115f4e..7138c0c10e 100644 --- a/cliv2/internal/cliv2/cliv2.go +++ b/cliv2/internal/cliv2/cliv2.go @@ -35,6 +35,9 @@ const SNYK_EXIT_CODE_ERROR = 2 const SNYK_INTEGRATION_NAME = "CLI_V1_PLUGIN" const SNYK_INTEGRATION_NAME_ENV = "SNYK_INTEGRATION_NAME" const SNYK_INTEGRATION_VERSION_ENV = "SNYK_INTEGRATION_VERSION" +const SNYK_HTTPS_PROXY_ENV = "HTTPS_PROXY" +const SNYK_HTTP_PROXY_ENV = "HTTP_PROXY" +const SNYK_CA_CERTIFICATE_LOCATION_ENV = "NODE_EXTRA_CA_CERTS" const ( V1_DEFAULT Handler = iota @@ -137,7 +140,7 @@ func determineHandler(passthroughArgs []string) Handler { return result } -func AddIntegrationEnvironment(input []string, name string, version string) (result []string, err error) { +func PrepareV1EnvironmentVariables(input []string, integrationName string, integrationVersion string, proxyAddress string, caCertificateLocation string) (result []string, err error) { inputAsMap := utils.ToKeyValueMap(input, "=") result = input @@ -146,27 +149,29 @@ func AddIntegrationEnvironment(input []string, name string, version string) (res _, integrationVersionExists := inputAsMap[SNYK_INTEGRATION_VERSION_ENV] if !integrationNameExists && !integrationVersionExists { - inputAsMap[SNYK_INTEGRATION_NAME_ENV] = name - inputAsMap[SNYK_INTEGRATION_VERSION_ENV] = version - result = utils.ToSlice(inputAsMap, "=") + inputAsMap[SNYK_INTEGRATION_NAME_ENV] = integrationName + inputAsMap[SNYK_INTEGRATION_VERSION_ENV] = integrationVersion } else if !(integrationNameExists && integrationVersionExists) { err = EnvironmentWarning{message: fmt.Sprintf("Partially defined environment, please ensure to provide both %s and %s together!", SNYK_INTEGRATION_NAME_ENV, SNYK_INTEGRATION_VERSION_ENV)} } + if err == nil { + inputAsMap[SNYK_HTTPS_PROXY_ENV] = proxyAddress + inputAsMap[SNYK_HTTP_PROXY_ENV] = proxyAddress + inputAsMap[SNYK_CA_CERTIFICATE_LOCATION_ENV] = caCertificateLocation + result = utils.ToSlice(inputAsMap, "=") + } + return result, err } func PrepareV1Command(cmd string, args []string, proxyPort int, caCertLocation string, integrationName string, integrationVersion string) (snykCmd *exec.Cmd, err error) { - snykCmd = exec.Command(cmd, args...) - snykCmd.Env = append(os.Environ(), - fmt.Sprintf("HTTPS_PROXY=http://127.0.0.1:%d", proxyPort), - fmt.Sprintf("NODE_EXTRA_CA_CERTS=%s", caCertLocation), - ) - - snykCmd.Env, err = AddIntegrationEnvironment(snykCmd.Env, integrationName, integrationVersion) + proxyAddress := fmt.Sprintf("http://127.0.0.1:%d", proxyPort) + snykCmd = exec.Command(cmd, args...) + snykCmd.Env, err = PrepareV1EnvironmentVariables(os.Environ(), integrationName, integrationVersion, proxyAddress, caCertLocation) snykCmd.Stdin = os.Stdin snykCmd.Stdout = os.Stdout snykCmd.Stderr = os.Stderr diff --git a/cliv2/internal/cliv2/cliv2_test.go b/cliv2/internal/cliv2/cliv2_test.go index c59a8f3b11..2a39600b08 100644 --- a/cliv2/internal/cliv2/cliv2_test.go +++ b/cliv2/internal/cliv2/cliv2_test.go @@ -1,22 +1,23 @@ package cliv2_test import ( - "github.com/snyk/cli/cliv2/internal/cliv2" "io/ioutil" "log" "os" "sort" "testing" + "github.com/snyk/cli/cliv2/internal/cliv2" + "github.com/stretchr/testify/assert" ) -func Test_addIntegrationEnvironment_Fill(t *testing.T) { +func Test_PrepareV1EnvironmentVariables_Fill(t *testing.T) { input := []string{"something=1", "in=2", "here=3=2"} - expected := []string{"something=1", "in=2", "here=3=2", "SNYK_INTEGRATION_NAME=foo", "SNYK_INTEGRATION_VERSION=bar"} + expected := []string{"something=1", "in=2", "here=3=2", "SNYK_INTEGRATION_NAME=foo", "SNYK_INTEGRATION_VERSION=bar", "HTTP_PROXY=proxy", "HTTPS_PROXY=proxy", "NODE_EXTRA_CA_CERTS=cacertlocation"} - actual, err := cliv2.AddIntegrationEnvironment(input, "foo", "bar") + actual, err := cliv2.PrepareV1EnvironmentVariables(input, "foo", "bar", "proxy", "cacertlocation") sort.Strings(expected) sort.Strings(actual) @@ -24,12 +25,12 @@ func Test_addIntegrationEnvironment_Fill(t *testing.T) { assert.Nil(t, err) } -func Test_addIntegrationEnvironment_DontOverrideExisting(t *testing.T) { +func Test_PrepareV1EnvironmentVariables_DontOverrideExisting(t *testing.T) { - input := []string{"something=1", "in=2", "here=3", "SNYK_INTEGRATION_NAME=exists", "SNYK_INTEGRATION_VERSION=already"} + input := []string{"something=1", "in=2", "here=3", "SNYK_INTEGRATION_NAME=exists", "SNYK_INTEGRATION_VERSION=already", "HTTP_PROXY=proxy", "HTTPS_PROXY=proxy", "NODE_EXTRA_CA_CERTS=cacertlocation"} expected := input - actual, err := cliv2.AddIntegrationEnvironment(input, "foo", "bar") + actual, err := cliv2.PrepareV1EnvironmentVariables(input, "foo", "bar", "proxy", "cacertlocation") sort.Strings(expected) sort.Strings(actual) @@ -37,12 +38,12 @@ func Test_addIntegrationEnvironment_DontOverrideExisting(t *testing.T) { assert.Nil(t, err) } -func Test_addIntegrationEnvironment_DontOverrideExisting2(t *testing.T) { +func Test_PrepareV1EnvironmentVariables_DontOverrideExisting2(t *testing.T) { input := []string{"something=1", "in=2", "here=3", "SNYK_INTEGRATION_NAME=exists"} expected := input - actual, err := cliv2.AddIntegrationEnvironment(input, "foo", "bar") + actual, err := cliv2.PrepareV1EnvironmentVariables(input, "foo", "bar", "unused", "unused") sort.Strings(expected) sort.Strings(actual) diff --git a/test/jest/acceptance/proxy-behavior.spec.ts b/test/jest/acceptance/proxy-behavior.spec.ts index 80721f1d09..122139c4e1 100644 --- a/test/jest/acceptance/proxy-behavior.spec.ts +++ b/test/jest/acceptance/proxy-behavior.spec.ts @@ -7,7 +7,6 @@ const SNYK_API_HTTP = 'http://snyk.io/api/v1'; const FAKE_HTTP_PROXY = `http://localhost:${fakeServerPort}`; jest.setTimeout(1000 * 60 * 1); - describe('Proxy configuration behavior', () => { describe('*_PROXY against HTTPS host', () => { it('tries to connect to the HTTPS_PROXY when HTTPS_PROXY is set', async () => { @@ -56,41 +55,43 @@ describe('Proxy configuration behavior', () => { }); describe('*_PROXY against HTTP host', () => { - it('tries to connect to the HTTP_PROXY when HTTP_PROXY is set', async () => { - const { code, stderr } = await runSnykCLI(`woof -d`, { - env: { - ...process.env, - HTTP_PROXY: FAKE_HTTP_PROXY, - SNYK_API: SNYK_API_HTTP, - SNYK_HTTP_PROTOCOL_UPGRADE: '0', - }, - }); + if (!isCLIV2()) { + it('tries to connect to the HTTP_PROXY when HTTP_PROXY is set', async () => { + const { code, stderr } = await runSnykCLI(`woof -d`, { + env: { + ...process.env, + HTTP_PROXY: FAKE_HTTP_PROXY, + SNYK_API: SNYK_API_HTTP, + SNYK_HTTP_PROTOCOL_UPGRADE: '0', + }, + }); - expect(code).toBe(0); + expect(code).toBe(0); - // It will *attempt* to connect to a FAKE_HTTP_PROXY (and fails, because it's not a real proxy server) - expect(stderr).toContain( - `Error: connect ECONNREFUSED 127.0.0.1:${fakeServerPort}`, - ); - }); - - it('does not try to connect to the HTTPS_PROXY when it is set', async () => { - const { code, stderr } = await runSnykCLI(`woof -d`, { - env: { - ...process.env, - HTTPS_PROXY: FAKE_HTTP_PROXY, - SNYK_API: SNYK_API_HTTP, - SNYK_HTTP_PROTOCOL_UPGRADE: '0', - }, + // It will *attempt* to connect to a FAKE_HTTP_PROXY (and fails, because it's not a real proxy server) + expect(stderr).toContain( + `Error: connect ECONNREFUSED 127.0.0.1:${fakeServerPort}`, + ); }); - expect(code).toBe(2); + it('does not try to connect to the HTTPS_PROXY when it is set', async () => { + const { code, stderr } = await runSnykCLI(`woof -d`, { + env: { + ...process.env, + HTTPS_PROXY: FAKE_HTTP_PROXY, + SNYK_API: SNYK_API_HTTP, + SNYK_HTTP_PROTOCOL_UPGRADE: '0', + }, + }); - // Incorrect behavior when Needle tries to upgrade connection after 301 http->https and the Agent option is set to a strict http/s protocol. - // See lines with `keepAlive` in request.ts for more details - expect(stderr).toContain( - 'TypeError [ERR_INVALID_PROTOCOL]: Protocol "https:" not supported. Expected "http:"', - ); - }); + expect(code).toBe(2); + + // Incorrect behavior when Needle tries to upgrade connection after 301 http->https and the Agent option is set to a strict http/s protocol. + // See lines with `keepAlive` in request.ts for more details + expect(stderr).toContain( + 'TypeError [ERR_INVALID_PROTOCOL]: Protocol "https:" not supported. Expected "http:"', + ); + }); + } }); });