Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: override env variables instead of appending #3323

Merged
merged 1 commit into from
Jun 20, 2022

Conversation

PeterSchafer
Copy link
Collaborator

When creating the environment variables for CLIv1 invocation ensure to override existing ones and not only append environment variables.

Affected Environment Variables are:

  • HTTP_PROXY
  • HTTPS_PROXY
  • NODE_EXTRA_CA_CERTS

What does this PR do?

It ensures that environment variables are unique and that there is no possible conflict/uncertainty between externally defined and internally set environment variables when being passed to CLIv1.

How should this be manually tested?

Run CLIv2 with --debug and use the following environments for testing:

  • HTTP_PROXY:
    env: SNYK_HTTP_PROTOCOL_UPGRADE=0 SNYK_API=http://snyk.io/api/v1 HTTP_PROXY=http://127.0.0.1:3128
    expected: dial tcp 127.0.0.1:3128: connect: connection refused
  • HTTPS_PROXY:
    env: HTTPS_PROXY=https://127.0.0.1:312
    expected: dial tcp 127.0.0.1:3128: connect: connection refused
  • NODE_EXTRA_CA_CERTS:
    env: NODE_EXTRA_CA_CERTS=./here
    NOT expected: Warning: Ignoring extra certs from ./here, load failed: error:02001002:system library:fopen:No such file or directory

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>
@PeterSchafer PeterSchafer requested a review from a team as a code owner June 15, 2022 10:54
@PeterSchafer PeterSchafer merged commit 181ff05 into fix/v2_http_proxy Jun 20, 2022
@PeterSchafer PeterSchafer deleted the chore/v2_cleanup_cliv1_env branch June 20, 2022 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants