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

feat: add sha256sum check when downloading from hashicorp #7

Merged
merged 2 commits into from
Apr 18, 2022
Merged

Conversation

c4po
Copy link
Contributor

@c4po c4po commented Apr 14, 2022

  • get the release checksum files from terraform_${version}_SHA256SUMS
  • calculate the sha256sum on the terraform zip file
  • delete the zip file from the temporary folder

@c4po c4po requested a review from ericnorris April 16, 2022 00:00
}
sha256Sum := h.Sum(nil)

if checkSha256Sum != "" {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure when this would be the empty string - does HashiCorp occasionally not include checksums for certain builds?

break
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Related to the other comment I left - should we fail if we didn't find the checksum?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm worry about the string spit on double space " " may broken in some release. This should be replaced with a better regex match. I will fix it and add the error handling logic if cannot find the checksum.

Copy link
Contributor

@ericnorris ericnorris left a comment

Choose a reason for hiding this comment

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

left a minor comment about what we should do with an empty checksum, but otherwise LGTM!

@c4po
Copy link
Contributor Author

c4po commented Apr 18, 2022

left a minor comment about what we should do with an empty checksum, but otherwise LGTM!

thanks. I'll replace the find checksum logic with a regex match and add error handling logic in next PR.

@c4po c4po merged commit 9a1b0f2 into main Apr 18, 2022
@c4po c4po deleted the checksum branch April 18, 2022 20:05
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