-
Notifications
You must be signed in to change notification settings - Fork 182
feat: accept version flag for zarf tools download-init
#3568
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
Conversation
Signed-off-by: Josh Brewer <[email protected]>
✅ Deploy Preview for zarf-docs canceled.
|
Codecov ReportAttention: Patch coverage is
🚀 New features to boost your workflow:
|
Signed-off-by: Josh Brewer <[email protected]>
|
Signed-off-by: Josh Brewer <[email protected]>
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.
Adds a neat capability to the download-init
command that many may find useful.
Given the parameters remained the same - I could not find an attack vector that this change might enable.
Arbitrary strings are validated by the OCI logic - if the tag doesn't exist the logs indicate as such. That does not mean there isn't potential for misuse.
Anytime we introduce accepting input from the user - I would prefer we perform input validation. In src/pkg/zoci/utils.go
this might look something like:
// GetInitPackageURL returns the URL for the init package for the given version with strict validation.
func GetInitPackageURL(version string) (string, error) {
// Strict SemVer validation (vMAJOR.MINOR.PATCH with optional pre-release and metadata)
var validVersion = regexp.MustCompile(`^v\d+\.\d+\.\d+(-[a-zA-Z0-9._-]+)?(\+[a-zA-Z0-9._-]+)?$`)
if !validVersion.MatchString(version) {
return "", fmt.Errorf("invalid version: must follow SemVer and start with 'v' (e.g., v0.50.0)")
}
return fmt.Sprintf("ghcr.io/zarf-dev/packages/init:%s", version), nil
}
This would prevent any unnecessary network calls and provide more explicit errors.
Open to feedback from other maintainers as well.
@brandtkeller I like that suggestion, and I think it could be made even simpler by using the semver package we already use in Zarf ver, err := semver.NewVersion(config.CLIVersion)
if err != nil {
return fmt.Errorf("invalid version %s: %w", version, err)
} |
Signed-off-by: Josh Brewer <[email protected]>
zarf tools download-init
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.
LGTM
Description
Updates to allow downloading of versioned zarf init packages
...
Related Issue
Fixes #3366
Checklist before merging