Skip to content

Fix issue with Helm 4 where KubeVersion is not pulled in#175

Open
BojanOro wants to merge 1 commit into
mainfrom
bojanoro/prodeng-635-helm-4-does-not-pass-kube-version-when-using-helm-diff
Open

Fix issue with Helm 4 where KubeVersion is not pulled in#175
BojanOro wants to merge 1 commit into
mainfrom
bojanoro/prodeng-635-helm-4-does-not-pass-kube-version-when-using-helm-diff

Conversation

@BojanOro

Copy link
Copy Markdown
Contributor

With Helm 4, KubeVersion is not being pulled in properly when running helm diff. This results in Helm using the default value of 1.20.0, which does not meet the minimum version specified in many charts, causing the diff to fail.

Hypothetically this PR (databus23/helm-diff#872) should have fixed this, but it doesn't seem to work. Adding this flag makes it work for Helm 3 and Helm 4.

@BojanOro BojanOro requested a review from a team as a code owner November 18, 2025 15:46
@linear

linear Bot commented Nov 18, 2025

Copy link
Copy Markdown

Comment thread libsentrykube/helm.py
"--namespace",
helm_release.namespace,
"--install",
"--dry-run=server",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: helm diff upgrade with --dry-run=server fails on Helm v3.10.2, causing silent misinterpretation of flag incompatibility as "no diff".
Severity: CRITICAL | Confidence: 0.95

🔍 Detailed Analysis

The helm diff upgrade command, when executed with the --dry-run=server flag, will fail on systems using Helm v3.10.2, as specified in the project's Dockerfile. This flag was introduced in Helm v3.13. The _run_helm() function will raise a HelmException, which is then caught in diff() (lines 641-643). The error is silently swallowed, and an empty string is returned, leading to incorrect behavior where flag incompatibility errors are misinterpreted as "no diff detected".

💡 Suggested Fix

Update the Dockerfile to Helm v3.13+ or remove the --dry-run=server flag. Alternatively, add Helm version validation or refine error handling in diff() to distinguish between "no diff" and flag incompatibility.

🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: libsentrykube/helm.py#L624

Potential issue: The `helm diff upgrade` command, when executed with the
`--dry-run=server` flag, will fail on systems using Helm v3.10.2, as specified in the
project's Dockerfile. This flag was introduced in Helm v3.13. The `_run_helm()` function
will raise a `HelmException`, which is then caught in `diff()` (lines 641-643). The
error is silently swallowed, and an empty string is returned, leading to incorrect
behavior where flag incompatibility errors are misinterpreted as "no diff detected".

Did we get this right? 👍 / 👎 to inform future reviews.

Reference_id: 2772709

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BojanOro should this be addressed,
should this flag be conditioned by helm version?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might be able to use --dry-run=client instead? Cursor says that's compatible with older helm@3 versions.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although maybe --validate and --dry-run=server function differently by actually hitting k8s.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dmajere Can you reply to Michael's comment so we can close this out?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure what to answer, its @BojanOro's PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants