Helm v4 support #872
Helm v4 support #872
Conversation
Helm v4 deprecation: Flag --validate has been deprecated, use '--dry-run=server' instead
we should use '--dry-run=server' when use helm v4, so we should check the helm verision and choice the right option to apply. |
I'm not exactly sure how to proceed here; I think this probably works already? From testing in Line 256 in 7b4b176 Is there some change in behavior that should be done here, or just some way to e.g. parametrize the tests in |
|
@jnoordsij we can fix the ci isssue firstly. |
Signed-off-by: yxxhero <[email protected]>
Signed-off-by: yxxhero <[email protected]>
Signed-off-by: yxxhero <[email protected]>
|
|
||
| // See https://github.com/databus23/helm-diff/issues/253 | ||
| diff.useUpgradeDryRun = os.Getenv("HELM_DIFF_USE_UPGRADE_DRY_RUN") == "true" | ||
| diff.useUpgradeDryRun = os.Getenv("HELM_DIFF_USE_UPGRADE_DRY_RUN") != "false" |
| args: []string{"test-release", "test/testdata/test-chart", "--values", "test/testdata/test-values.yaml", "--validate", "--is-upgrade"}, | ||
| cmd: []string{"upgrade"}, | ||
| args: []string{"test-release", "test/testdata/test-chart", "--values", "test/testdata/test-values.yaml", "--dry-run"}, | ||
| }, |
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for Helm v4 while attempting to maintain backward compatibility with Helm v3. The implementation updates the Helm SDK dependency from v3 to v4 and adapts to breaking API changes, most notably the deprecation of the --validate flag in favor of --dry-run=server.
Key Changes:
- Updates all Helm SDK imports from
helm.sh/helm/v3tohelm.sh/helm/v4 - Changes default behavior of
HELM_DIFF_USE_UPGRADE_DRY_RUNfrom opt-in to opt-out (now enabled by default) - Adds version detection to use appropriate flags for Helm v3 vs v4 (
--validatevs--dry-run=server) - Consolidates helm version detection logic from
cmd/helm3.gointocmd/helm.go - Updates CI test matrix to include both Helm v3.19.0 and v4.0.0
Reviewed Changes
Copilot reviewed 8 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| go.mod | Updates Helm dependency from v3.19.0 to v4.0.0, along with transitive dependency updates required by Helm v4 |
| go.sum | Comprehensive dependency checksum updates reflecting the Helm v4 upgrade and new transitive dependencies |
| manifest/generate.go | Updates Helm SDK imports from v3 to v4 |
| cmd/upgrade.go | Updates imports to v4, changes HELM_DIFF_USE_UPGRADE_DRY_RUN default to opt-out, removes log parameter from actionConfig.Init() call |
| cmd/helm3.go | File deleted - functionality moved to cmd/helm.go |
| cmd/helm.go | Consolidates helm3.go content, adds isHelmVersionGreaterThanEqual() function and Helm v4 version detection for conditional flag usage |
| cmd/helm_test.go | Adds unit tests for extractManifestFromHelmUpgradeDryRunOutput() function |
| main_test.go | Updates test stubs to reflect new default behavior (helm upgrade with --dry-run instead of helm template with --validate) |
| .golangci.yaml | Updates import whitelist from helm.sh/helm/v3 to helm.sh/helm/v4 |
| .github/workflows/ci.yaml | Updates test matrix to include Helm v4.0.0 alongside v3.19.0, adds consistent YAML formatting |
| if d.threeWayMerge || d.takeOwnership { | ||
| actionConfig = new(action.Configuration) | ||
| if err := actionConfig.Init(envSettings.RESTClientGetter(), envSettings.Namespace(), os.Getenv("HELM_DRIVER"), log.Printf); err != nil { | ||
| if err := actionConfig.Init(envSettings.RESTClientGetter(), envSettings.Namespace(), os.Getenv("HELM_DRIVER")); err != nil { |
There was a problem hiding this comment.
The actionConfig.Init() signature has changed - the fourth parameter (log function) has been removed. This is a breaking API change between Helm v3 and v4. While this change is necessary for Helm v4 support, we should verify this doesn't break backward compatibility with Helm v3.
The old Helm v3 signature was: Init(getter, namespace, helmDriver, logFunc)
The new Helm v4 signature is: Init(getter, namespace, helmDriver)
Consider checking the Helm version and calling the appropriate Init signature, or document that this plugin now requires Helm v4+.
|
|
||
| // See https://github.com/databus23/helm-diff/issues/253 | ||
| diff.useUpgradeDryRun = os.Getenv("HELM_DIFF_USE_UPGRADE_DRY_RUN") == "true" | ||
| diff.useUpgradeDryRun = os.Getenv("HELM_DIFF_USE_UPGRADE_DRY_RUN") != "false" |
There was a problem hiding this comment.
The default behavior for HELM_DIFF_USE_UPGRADE_DRY_RUN has changed from opt-in (== "true") to opt-out (!= "false"). This is a significant behavioral change that:
- Makes the plugin use
helm upgrade --dry-runby default instead ofhelm template - Changes the default from client-side to server-side rendering by default (with cluster access)
While this is mentioned in the PR description as the intended behavior for Helm v4 compatibility, this is a breaking change for users who rely on the default client-side behavior. Consider:
- Adding a clear migration notice in documentation
- Checking the Helm version and only enabling this by default for Helm v4+
- Or at minimum, ensure this breaking change is clearly documented in release notes
2 participants
With Helm v4 having its first rc (see https://github.com/helm/helm/releases/tag/v4.0.0-rc.1) I decided to check for plugin support, finding this plugin to be not entirely compatible yet. This is a rough attempt at updating to be compatible with Helm v4. I'm unsure about the full implications of support for Helm v3 by changes introduced in this PR. Any suggestions or help are welcome.
For reference, the only issue I found with running this with Helm v4 was the deprecation/error:
One can already circumvent this with
export HELM_DIFF_USE_UPGRADE_DRY_RUN=true; making this the default (as proposed by this PR) seems the way forward, but might have other technical implications.