-
Notifications
You must be signed in to change notification settings - Fork 18
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(attestation): change default to NOT download cosign outside of setup flow #49
fix(attestation): change default to NOT download cosign outside of setup flow #49
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worked great for me, I'd probably ask you to clean up per my first comment there, but otherwise it's good to go and I don't need to look at it again.
@@ -54,12 +56,12 @@ func install_cosign() { | |||
return false | |||
} | |||
|
|||
func load_attestation_binary() { | |||
func load_attestation_binary(download_if_not_present: bool) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The : bool
there should be redundant; I'd expect it to be inferred from the body since the body is unambiguous, but let me know if it does break (it certainly is fine to be explicit about the type here, I don't
think this needs to change).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Per previous review.
Issue
Addressing issue raised in #27
Description
Chalk no longer downloads cosign by default outside of the
chalk setup
flow. Previouslyplugins/system.nim
's lookup of the location of the cosign binary caused cosign to be downloaded if it's path could not be determined. The default behaviour is now that lookups of cosign location will no longer initiate a download unless specifically requested. Thechalk setup
flow will still download cosign as part of its setup process if it cannot be found locally.Testing
Run any chalk operation with --trace enabled and see that no cosign download takes place
Run
chalk --trace setup
with no local cosign and see that a download takes place