fix: Use the specified version of Helm when templating with Kustomize#5636
Conversation
Signed-off-by: Yuki Okushi <okushi@canary-inc.jp>
JohnTitor
requested review from
Warashi,
ffjlabo,
khanhtc1202 and
t-kikuc
as code owners
| - name: datadog | ||
| releaseName: datadog-agent | ||
| repo: https://helm.datadoghq.com | ||
| version: 3.102.0 |
There was a problem hiding this comment.
Perhaps we could come up with a minimal testcase and put it on local, would like to leave the decision to the maintainers.
Warashi
left a comment
Warashi
left a comment
There was a problem hiding this comment.
Thank you! Almost LGTM.
There is a lint error, so please fix it.
|
|
||
| kustomizePath, _, err := toolregistry.DefaultRegistry().Kustomize(ctx, "") | ||
| require.NoError(t, err) | ||
| helmPath, _, err := toolregistry.DefaultRegistry().Helm(ctx, "") |
There was a problem hiding this comment.
Please check the error is nil?
| helmPath, _, err := toolregistry.DefaultRegistry().Helm(ctx, "") | |
| helmPath, _, err := toolregistry.DefaultRegistry().Helm(ctx, "") | |
| require.NoError(t, err) |
Signed-off-by: Yuki Okushi <okushi@canary-inc.jp>
Warashi
left a comment
Warashi
left a comment
There was a problem hiding this comment.
Sorry, I overlooked the version comparison issue.
| // Pass the Helm command path to kustomize to use the specified version of Helm. | ||
| // Unconditionally adding this flag as it's unharmful when Helm is not used. | ||
| // Note: It's only available on Kustomize v4.1.0 and higher. | ||
| if c.version >= "4.1.0" && helm != nil { |
There was a problem hiding this comment.
I apologize for overlooking this version comparison issue.
We cannot compare semantic versions by simply comparing strings. For instance, versions greater than or equal to 10 are incorrectly evaluated as less than 9.
Could you please implement a feature to compare semantic versions and include corresponding tests?
There was a problem hiding this comment.
Ah, right.
Fixed and added tests in 84bfe4d (#5636)
Signed-off-by: Yuki Okushi <okushi@canary-inc.jp>
4 participants
What this PR does: Pass the
--helm-commandflag with the installed Helm path when kustomize-templating.Why we need it: Without this, Kustomize cannot find the specified version of Helm when templating.
Which issue(s) this PR fixes:
Fixes #5635
Does this PR introduce a user-facing change?: yes