Skip to content

feat: change downloadurl and add fallback on setup action#148

Merged
j-luong merged 3 commits intomasterfrom
CLI-427-update-cli-download-url
Aug 7, 2024
Merged

feat: change downloadurl and add fallback on setup action#148
j-luong merged 3 commits intomasterfrom
CLI-427-update-cli-download-url

Conversation

@j-luong
Copy link
Copy Markdown
Contributor

@j-luong j-luong commented Aug 2, 2024

This PR changes the default download url to https://downloads.snyk.io/, adds a backup URL that uses https://static.sn/ yk.io, for downloading the Snyk CLI

Changes

Testing
Tested the script to ensure:

  • Successful downloads from the main URL
  • Fallback to the backup URL when the main URL fails
  • Proper error handling if both URLs fail
  • Please review and test in your environment to ensure it meets our reliability standards.

@j-luong j-luong requested a review from a team as a code owner August 2, 2024 10:19
@j-luong j-luong force-pushed the CLI-427-update-cli-download-url branch from aa4a42a to 7643863 Compare August 2, 2024 10:23
j-luong and others added 2 commits August 7, 2024 09:40
Co-authored-by: PeterSchafer <[email protected]>
Co-authored-by: PeterSchafer <[email protected]>
Copy link
Copy Markdown
Contributor

@PeterSchafer PeterSchafer left a comment

Choose a reason for hiding this comment

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

LGTM

@j-luong j-luong merged commit ae94425 into master Aug 7, 2024
@j-luong j-luong deleted the CLI-427-update-cli-download-url branch August 7, 2024 08:53
j-luong added a commit that referenced this pull request Aug 7, 2024
@electrofelix
Copy link
Copy Markdown

@PeterSchafer @j-luong Have seen some checksum failed errors recently and it's not altogether clear why, which suggests that curl got an response from the remote server that indicated there was an error and saved it to the output file and returned zero as the exit code.

Should this script use --fail-with-body with curl to cause 4xx error codes to be considered failures as well?

The errors also highlighted than if there is an issue with the binary downloaded from downloads.snyk.io/cli due to the checksum, there is no fallback to static.snyk.io/cli. Only individual downloads will fallback, so could end up with a binary from https://downloads.snyk.io/cli and checksum from https://static.snyk.io/cli and not clear if that is a problem.

@pybalo-nex
Copy link
Copy Markdown

#150 Is a revert of this merged feature, for the record.

@raphabot-snyk
Copy link
Copy Markdown

In my opinion, if the binary is downloaded from one of the sources, than the checksum file should also be downloaded from the exact same source. I'm thinking of a scenario where both are out of sync, for some reason, and they the are downloaded from different sources.

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.

6 participants