Skip to content

Removing vestigial documentation for dry-run env var #765

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jake-dog
Copy link

@jake-dog jake-dog commented Apr 9, 2025

As best I can tell the environment variable HELM_DIFF_USE_INSECURE_SERVER_SIDE_DRY_RUN referenced in documentation, originally added in #499, is not actually used anywhere in helm-diff or helm. I also did some testing with and without setting this environment variable and I couldn't see a difference in the helm-diff behavior.

If I'm mistaken then great; this environment variable seems like a good feature broadly speaking. If I'm correct however, then implementing this environment variable now could negatively impact users who have grown accustomed to the default behavior.

@yxxhero
Copy link
Collaborator

yxxhero commented Apr 19, 2025

@jake-dog maybe we should find out why that is not working?

@jake-dog
Copy link
Author

jake-dog commented Apr 20, 2025

Based on my review of commit logs, HELM_DIFF_USE_INSECURE_SERVER_SIDE_DRY_RUN was never implemented. If you prefer that the behavior is implemented as documented, I would be happy to do that. My only reservation is that it could negatively impact users who have grown accustomed to the default behavior.

For reference I ran helm diff upgrade under strace for an existing release using a chart that includes the lookup template function.

app version
helm v3.17.3
helm-diff 3.11.0
  1. Command: HELM_DIFF_USE_INSECURE_SERVER_SIDE_DRY_RUN=true helm diff upgrade my-release .
    Result: ["helm", "template", "my-release", ".", "--namespace", "helmdifftest", "--values", "/tmp/existing-values2247892059", "--validate", "--is-upgrade", "--dry-run=client"]
    lookup called? 🔴

  2. Command: helm diff upgrade my-release .
    Result: ["helm", "template", "my-release", ".", "--namespace", "helmdifftest", "--values", "/tmp/existing-values3611432982", "--validate", "--is-upgrade", "--dry-run=client"]
    lookup called? 🔴

  3. Command: HELM_DIFF_USE_INSECURE_SERVER_SIDE_DRY_RUN=true helm diff upgrade my-release --dry-run .
    Result: ["helm", "template", "my-release", ".", "--namespace", "helmdifftest", "--is-upgrade", "--dry-run=client"]
    lookup called? 🔴

  4. Command: helm diff upgrade my-release --dry-run .
    Result: ["helm", "template", "my-release", ".", "--namespace", "helmdifftest", "--is-upgrade", "--dry-run=client"]
    lookup called? 🔴

  5. Command: HELM_DIFF_USE_INSECURE_SERVER_SIDE_DRY_RUN=true helm diff upgrade my-release --dry-run=client
    Result: ["helm", "template", "my-release", ".", "--namespace", "helmdifftest", "--is-upgrade", "--dry-run=client"]
    lookup called? 🔴

  6. Command: helm diff upgrade my-release --dry-run=client .
    Result: ["helm", "template", "my-release", ".", "--namespace", "helmdifftest", "--is-upgrade", "--dry-run=client"]
    lookup called? 🔴

  7. Command: HELM_DIFF_USE_INSECURE_SERVER_SIDE_DRY_RUN=true helm diff upgrade my-release --dry-run=server .
    Result: ["helm", "template", "my-release", ".", "--namespace", "helmdifftest", "--values", "/tmp/existing-values3036357848", "--validate", "--is-upgrade", "--dry-run=server"]
    lookup called? 🟢

  8. Command: helm diff upgrade my-release --dry-run=server .
    Result: ["helm", "template", "my-release", ".", "--namespace", "helmdifftest", "--values", "/tmp/existing-values2789378336", "--validate", "--is-upgrade", "--dry-run=server"]
    lookup called? 🟢

  9. Command: HELM_DIFF_USE_INSECURE_SERVER_SIDE_DRY_RUN=false helm diff upgrade my-release --dry-run=server .
    Result: ["helm", "template", "my-release", ".", "--namespace", "helmdifftest", "--values", "/tmp/existing-values3326242558", "--validate", "--is-upgrade", "--dry-run=server"]
    lookup called? 🟢

If HELM_DIFF_USE_INSECURE_SERVER_SIDE_DRY_RUN is implemented as documented, it would mean that the behavior in test cases 8 & 9 would change to match the result of test cases 1 & 2.

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.

2 participants