-
Notifications
You must be signed in to change notification settings - Fork 177
Add helm-v4 support #226
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
base: main
Are you sure you want to change the base?
Add helm-v4 support #226
Conversation
e952f82 to
8c7554d
Compare
|
Thank you for this PR I will take a look |
|
Thanks! I will have to rebase first |
|
Thanks @avoidik, added it to our fork. |
This commit adds support for Helm v4 while maintaining backward compatibility with Helm v3. Helm v2 support has been dropped. Key changes: - Added dual Helm v3/v4 API support using wrapper pattern - Runtime version detection by reading plugin manifest format - Separate plugin manifests for Helm v3 and v4 - Install hook automatically copies correct manifest - Updated acceptance tests to test both Helm 3 and 4 - Bumped dependencies to latest versions (Go 1.25, Helm v3.19.4, Helm v4.0.4) - Added arm64/aarch64 architecture support - Updated GoReleaser configuration (fixed deprecation warning) - Removed all Helm v2 code and references
|
hello @nerdeveloper I have rebased my PR, now it's ready for review |
…m to start in acceptance test environment
|
By default |
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.
Overall the Helm 4 support implementation looks solid with proper backward compatibility for Helm 3. Found a few minor issues:
1. Bug: Error handling in certPoolFromFile function
Files: cmd/helm-cm-push/main_test.go (line 37) and pkg/chartmuseum/upload_test.go (line 33)
if !caCertPool.AppendCertsFromPEM(caCert) {
return nil, err // BUG: err is nil here from previous successful ReadFile
}Fix: Return a descriptive error instead:
if !caCertPool.AppendCertsFromPEM(caCert) {
return nil, fmt.Errorf("failed to append CA certs from PEM file: %s", filename)
}2. Typo in test file
File: pkg/helm/chart_test.go (lines 36, 39)
expexted should be expected
ref. #225