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

fix: use proxy aware snyk-iac-test [INC-1647] #5557

Merged
merged 1 commit into from
Oct 29, 2024

Conversation

sergiu-snyk
Copy link
Contributor

@sergiu-snyk sergiu-snyk commented Oct 29, 2024

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?

snyk iac test command was broken for IaC+ (FF iacIntegratedExperience) when using snyk auth, because the snyk-iac-test binary used Oauth token from env var (which was removed at some point) instead of directing the API calls through the internal proxy of the CLI (which handles auth)

This PR upgrades to a new version of snyk-iac-test that uses the internal proxy for API calls.
See https://github.com/snyk/snyk-iac-test/pull/256 for the changes done on the snyk-iac-test side.

How should this be manually tested?

Run snyk iac test with an org that has iacIntegratedExperience FF enabled, with Oauth2 or with token via env var.
Run snyk iac test with an org that has iacIntegratedExperience FF disabled, with Oauth2 or with token via env var.

What are the relevant tickets?

INC-1647

slack thread

@sergiu-snyk sergiu-snyk requested a review from a team as a code owner October 29, 2024 08:37
Copy link
Contributor

github-actions bot commented Oct 29, 2024

Warnings
⚠️

You've modified files in src/ directory, but haven't updated anything in test folder. Is there something that could be tested?

Generated by 🚫 dangerJS against d5d1e2e

@sergiu-snyk sergiu-snyk force-pushed the fix/INC-1647/use-proxy-aware-snyk-iac-test branch from 2028433 to 537cfd8 Compare October 29, 2024 08:42
@@ -284,7 +283,6 @@ async function spawn(
env: Record<string, string | undefined>,
) {
return new Promise<SpawnResult>((resolve) => {
env = restoreEnvProxy(env);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

restoring proxy env vars before spawning snyk-iac-test is no longer necessary

@PeterSchafer
Copy link
Collaborator

@sergiu-snyk looks good to me! But please do npm run format to fix the linter issue

@sergiu-snyk sergiu-snyk force-pushed the fix/INC-1647/use-proxy-aware-snyk-iac-test branch from 0427c0d to d5d1e2e Compare October 29, 2024 14:22
@sergiu-snyk sergiu-snyk merged commit c4c78e7 into main Oct 29, 2024
7 checks passed
@sergiu-snyk sergiu-snyk deleted the fix/INC-1647/use-proxy-aware-snyk-iac-test branch October 29, 2024 15:19
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.

2 participants