Skip to content
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

diff is breaking for specific checks performed by bitnami/common #460

Open
morremeyer opened this issue May 26, 2023 · 32 comments
Open

diff is breaking for specific checks performed by bitnami/common #460

morremeyer opened this issue May 26, 2023 · 32 comments
Assignees

Comments

@morremeyer
Copy link

The plugin fails for a specific configuration (maybe a specific template function?) that is used in the bitnami/common chart.

It specifically leads to these bitnami charts displaying an error message on diff that certain credentials (e.g. for bitnami/mysql and bitnami/postgresql) need to be specified when upgrading, although they are set correctly.

This is evidenced by helm upgrade working with the same configuration without any issues (same goes for helmfile).

This was discovered in bitnami/charts#15073, but also mentioned by me in bitnami/charts#15175.

Specifically, the configuration mentioned in bitnami/charts#15175 (comment) shows the error mentioned there.

Tracing this down to its source, it seems to be happening when the following line is hit: https://github.com/bitnami/charts/blob/d8211ca955bae990f4b1af121419545cb90309eb/bitnami/common/templates/_secrets.tpl#L93 (see bitnami/charts#15175 (comment)).

I deduced this from the error message in line 111 being triggered: https://github.com/bitnami/charts/blob/d8211ca955bae990f4b1af121419545cb90309eb/bitnami/common/templates/_secrets.tpl#L111, which needs the template to fall into the else clause in https://github.com/bitnami/charts/blob/d8211ca955bae990f4b1af121419545cb90309eb/bitnami/common/templates/_secrets.tpl#L102.

This should not happen since https://github.com/bitnami/charts/blob/d8211ca955bae990f4b1af121419545cb90309eb/bitnami/common/templates/_secrets.tpl#L93 resolves to a secret being set and therefore the if in https://github.com/bitnami/charts/blob/d8211ca955bae990f4b1af121419545cb90309eb/bitnami/common/templates/_secrets.tpl#L94 should be entered.

I'm aware that this is a very unspecific report, please let me know if I should provide an easier reproduction scenario or similar.

@gldkru
Copy link

gldkru commented Jul 13, 2023

Can this problem be solved?

@morremeyer
Copy link
Author

@gldkru Is this a request to the maintainers to fix this or a question if it is technically possible to solve this?

@gldkru
Copy link

gldkru commented Jul 13, 2023

@morremeyer I want to solve this problem for myself and I found only this issue

@NfNitLoop
Copy link

I'm experiencing the same issue in bitnami/charts#17717. Our workaround has been to stop using helm diff. (or, more specifically, start using helmfile sync instead of helmfile apply.)

We'd like to be able to use helm-diff for these charts, though. If anyone knows of a fix in helm-diff or can help the bitnami folks make their charts compatible with helm-diff, we'd definitely prefer to use helm diff in our workflow! ❤️

@gz243
Copy link

gz243 commented Jul 21, 2023

We are encountering the same issue here. This is preventing us from using this plugin since we use quite a few Bitnami charts.

@jkroepke
Copy link
Contributor

What happens, if you pre-define the password on values and let them not generate through helm chart?

There are various additional issues, since helm diff uses helm template in background. And helm template does not support lookup by design.

The only way that I can see there is not using helm to generate a password.

@morremeyer
Copy link
Author

morremeyer commented Jul 22, 2023

@jkroepke I'm not generating the password values with the chart, I'm specifying them in a secret and referencing that.

As you noted, it looks like the lookup in that secret is breaking.

Do I understand your comment correctly that helm template should show the name failure mode here?

If that's the case, I assume we can reproduce it independently from helm diff.

@jkroepke
Copy link
Contributor

Yes. ArgoCD has the same issue, because ArgoCD depends on helm template, too.

helm/helm#8137

But a fix seems to be merged 2 days ago on helm directly. helm/helm#9426

If a new helm version released and helm diff includes the new helm version, it cloud resolve the issue here, too.

@morremeyer
Copy link
Author

ArgoCD having the same issue is interesting, I know of a case where the issue went away when migrating from helmfile to ArgoCD.

However, it's amazing news that there is a fix in helm itself for this.
Thanks for tying all the loose ends together here, @jkroepke!

I'll check if this is still an issue with the next helm release, will update here once I find out.

@jkroepke
Copy link
Contributor

ArgoCD ref: argoproj/argo-cd#5202

@acascais
Copy link

acascais commented Oct 6, 2023

We're having the same problem with the bitnami postgres helm chart: helmfile diff fails, helmfile sync works fine.

Is there a fix for this?

@NeonSludge
Copy link

NeonSludge commented Mar 29, 2024

Still breaking with latest helm-diff and Helm 3.14.

Correction: still breaking for tools that do not use dry-run=server.

@philmtd
Copy link

philmtd commented May 23, 2024

We're also running into this issue with Helmfile and the Bitnami Charts. Sorry for tagging you, @yxxhero, but you're currently appearing as the most active maintainer here. Do you think you could take a look at this issue?

@yxxhero
Copy link
Collaborator

yxxhero commented May 23, 2024

@philmtd ok. I will work on this. could you give me an example on github. so I can debug it.

@jbierweasel
Copy link

jbierweasel commented May 23, 2024

Thanks for having a look at it!
Here is a minimal configuration for recreating this behaviour.

Values

Given my_values.yaml:

mongodb:
  enabled: true
  architecture: "replicaset"
  auth:
    enabled: true
    existingSecret: mongodb-credentials-test
    username: test
    database: test

  replicaCount: 3

  arbiter:
    enabled: false

Create Secret

kubectl create secret generic mongodb-credentials-test --from-literal=mongodb-password="mysupersecurepassword" --from-literal=mongodb-root-password="mysupersecurepassword" --from-literal=mongodb-replica-set-key="mysupersecurepassword"

Install - Success ✅

helm install test-db bitnami/mongodb -f my_values.yaml

Upgrade - Success ✅

helm upgrade test-db bitnami/mongodb -f my_values.yaml

Diff - Fails ❌

helm diff upgrade test-db bitnami/mongodb -f my_values.yaml

Error Message:

Error: Failed to render chart: exit status 1: Error: execution error at (mongodb/templates/secrets.yaml:26:21):
PASSWORDS ERROR: You must provide your current passwords when upgrading the release.
                 Note that even after reinstallation, old credentials may be needed as they may be kept in persistent volume claims.
                 Further information can be obtained at https://docs.bitnami.com/general/how-to/troubleshoot-helm-chart-issues/#credential-errors-while-upgrading-chart-releases

    'auth.rootPassword' must not be empty, please add '--set auth.rootPassword=$MONGODB_ROOT_PASSWORD' to the command. To get the current value:

        export MONGODB_ROOT_PASSWORD=$(kubectl get secret --namespace "default" test-db-mongodb -o jsonpath="{.data.mongodb-root-password}" | base64 -d)

@philmtd
Copy link

philmtd commented May 24, 2024

Thank you for looking into it, @yxxhero!

I just tested it with the latest version of Bitnami's MongoDB chart, and my configuration looks like this:

create secret

kubectl create secret generic mongodb-credentials-test --from-literal=mongodb-password="mysupersecurepassword" --from-literal=mongodb-root-password="mysupersecurepassword" --from-literal=mongodb-replica-set-key="mysupersecurepassword" --from-literal=mongodb-passwords="mysupersecretpassword"

create values.yaml

architecture: "replicaset"
auth:
  enabled: true
  existingSecret: mongodb-credentials-test
  username: test
  database: test

replicaCount: 3

arbiter:
  enabled: false

install mongodb

helm install test-db bitnami/mongodb -f values.yaml

Works ✅

upgrade mongodb

helm upgrade test-db bitnami/mongodb -f values.yaml

Works ✅

diff mongodb

helm diff upgrade test-db bitnami/mongodb -f values.yaml

Error: Failed to render chart: exit status 1: Error: execution error at (mongodb/templates/secrets.yaml:26:21):
PASSWORDS ERROR: You must provide your current passwords when upgrading the release.
                 Note that even after reinstallation, old credentials may be needed as they may be kept in persistent volume claims.
                 Further information can be obtained at https://docs.bitnami.com/general/how-to/troubleshoot-helm-chart-issues/#credential-errors-while-upgrading-chart-releases

    'auth.rootPassword' must not be empty, please add '--set auth.rootPassword=$MONGODB_ROOT_PASSWORD' to the command. To get the current value:

        export MONGODB_ROOT_PASSWORD=$(kubectl get secret --namespace "sophora-latest" mongodb-credentials-test -o jsonpath="{.data.mongodb-root-password}" | base64 -d)


Use --debug flag to render out invalid YAML

Error: plugin "diff" exited with error

Does not work ⛔

@yxxhero
Copy link
Collaborator

yxxhero commented May 24, 2024

MONGODB_ROOT_PASSWORD=$(kubectl get secret --namespace "sophora-latest" mongodb-credentials-test -o jsonpath="{.data.mongodb-root-password}" | base64 -d)

update yuur values.yaml

architecture: "replicaset"
auth:
  enabled: true
  existingSecret: mongodb-credentials-test
  username: test
  database: test
  rootPassword: <vaule of var MONGODB_ROOT_PASSWORD>

replicaCount: 3

arbiter:
  enabled: false

take a try.

@philmtd
Copy link

philmtd commented May 24, 2024

Sure, that would work, but it should not be necessary. The idea of having the password in a secret is that we do not want to put it in the values. And a helm upgrade works with no issues. It's only helm diff upgrade resulting in this problem. Hence the issue here, as we suppose helm diff does something differently/wrong.

@yxxhero
Copy link
Collaborator

yxxhero commented May 24, 2024

Sure, that would work, but it should not be necessary. The idea of having the password in a secret is that we do not want to put it in the values. And a helm upgrade works with no issues. It's only helm diff upgrade resulting in this problem. Hence the issue here, as we suppose helm diff does something differently/wrong.

ok. I got you. let me see what happened.

@yxxhero yxxhero self-assigned this May 24, 2024
@yxxhero
Copy link
Collaborator

yxxhero commented May 24, 2024

HELM_DIFF_USE_UPGRADE_DRY_RUN=true, try to set the env and see the result. please. @philmtd

@philmtd
Copy link

philmtd commented May 24, 2024

That still leads to the same error, unfortunately. Also when passing dry-run as an argument (helm diff upgrade --dry-run test-db bitnami/mongodb -f values.yaml).

@yxxhero
Copy link
Collaborator

yxxhero commented May 25, 2024

--dry-run=server add this option. then please feedback. @philmtd

@yxxhero
Copy link
Collaborator

yxxhero commented May 25, 2024

and set HELM_DEBUG true. show the log.

@philmtd
Copy link

philmtd commented May 25, 2024

I just tried this and saw that I needed to update helm-diff. Now with the latest version installed --dry-run=server and also HELM_DIFF_USE_UPGRADE_DRY_RUN=true do work without errors! Using Helmfile (diff/apply) still breaks with the latest helm-diff version, though. Sorry for the confusion.

@yxxhero
Copy link
Collaborator

yxxhero commented May 25, 2024

Only set dryrun=server

@yxxhero
Copy link
Collaborator

yxxhero commented May 25, 2024

I am the mantainer of helmfile. Will see any need to updates.

@philmtd
Copy link

philmtd commented May 25, 2024

Only set dryrun=server

I did that. Tried this and the env var separately and both worked.

I am the mantainer of helmfile. Will see any need to updates.

I know :) Great! Thanks.

@yxxhero
Copy link
Collaborator

yxxhero commented May 25, 2024

I just tried this and saw that I needed to update helm-diff. Now with the latest version installed --dry-run=server and also HELM_DIFF_USE_UPGRADE_DRY_RUN=true do work without errors! Using Helmfile (diff/apply) still breaks with the latest helm-diff version, though. Sorry for the confusion.

could show the logs?

@philmtd
Copy link

philmtd commented May 25, 2024

Without dry run:

~/Desktop/m ❯ helm diff upgrade test-db bitnami/mongodb -f values.yaml               
Executing helm version
Executing helm get manifest test-db --namespace sophora-latest
Executing helm version
Executing helm template test-db bitnami/mongodb --namespace sophora-latest --values values.yaml --validate --is-upgrade --dry-run=client
Error: Failed to render chart: exit status 1: install.go:222: [debug] Original chart version: ""
install.go:239: [debug] CHART PATH: /Users/philmtd/Library/Caches/helm/repository/mongodb-15.5.2.tgz

Error: execution error at (mongodb/templates/secrets.yaml:26:21):
PASSWORDS ERROR: You must provide your current passwords when upgrading the release.
                 Note that even after reinstallation, old credentials may be needed as they may be kept in persistent volume claims.
                 Further information can be obtained at https://docs.bitnami.com/general/how-to/troubleshoot-helm-chart-issues/#credential-errors-while-upgrading-chart-releases

    'auth.rootPassword' must not be empty, please add '--set auth.rootPassword=$MONGODB_ROOT_PASSWORD' to the command. To get the current value:

        export MONGODB_ROOT_PASSWORD=$(kubectl get secret --namespace "sophora-latest" mongodb-credentials-test -o jsonpath="{.data.mongodb-root-password}" | base64 -d)

helm.go:84: [debug] execution error at (mongodb/templates/secrets.yaml:26:21):
PASSWORDS ERROR: You must provide your current passwords when upgrading the release.
                 Note that even after reinstallation, old credentials may be needed as they may be kept in persistent volume claims.
                 Further information can be obtained at https://docs.bitnami.com/general/how-to/troubleshoot-helm-chart-issues/#credential-errors-while-upgrading-chart-releases

    'auth.rootPassword' must not be empty, please add '--set auth.rootPassword=$MONGODB_ROOT_PASSWORD' to the command. To get the current value:

        export MONGODB_ROOT_PASSWORD=$(kubectl get secret --namespace "sophora-latest" mongodb-credentials-test -o jsonpath="{.data.mongodb-root-password}" | base64 -d)


Error: plugin "diff" exited with error
helm.go:84: [debug] plugin "diff" exited with error

With --dry-run=server:

~/Desktop/m ❯ helm diff upgrade --dry-run=server test-db bitnami/mongodb -f values.yaml
Executing helm version
Executing helm get manifest test-db --namespace sophora-latest
Executing helm version
Executing helm template test-db bitnami/mongodb --namespace sophora-latest --values values.yaml --validate --is-upgrade --dry-run=server
Executing helm get hooks test-db --namespace sophora-latest

With the env:

~/Desktop/m ❯ export HELM_DIFF_USE_UPGRADE_DRY_RUN=true
~/Desktop/m ❯ helm diff upgrade test-db bitnami/mongodb -f values.yaml
Executing helm version
Executing helm get manifest test-db --namespace sophora-latest
Executing helm version
Executing helm upgrade test-db bitnami/mongodb --namespace sophora-latest --values values.yaml --dry-run=server
Executing helm get hooks test-db --namespace sophora-latest

Using helmfile (helmfile diff or helmfile apply) without having HELM_DIFF_USE_UPGRADE_DRY_RUN set to true, it will result in the same error as above when doing it with helm diff alone.

Using helmfile with setting HELM_DIFF_USE_UPGRADE_DRY_RUN=true it will work without errors, same as with helm diff alone.

@yxxhero
Copy link
Collaborator

yxxhero commented May 25, 2024

see: helmfile/helmfile#1050
I will complete it.

@philmtd
Copy link

philmtd commented May 25, 2024

Thanks!

Just to be sure: I understand you will include the dry-run flag in helmfile to circumvent this issue. But isn't there a bug in helm-diff which causes this problem? I'd imagine it would be nicer to have a bugfix in helm diff rather than having to use an extra dry-run flag every time helmfile is used.

@yxxhero
Copy link
Collaborator

yxxhero commented May 25, 2024

@philmtd not bug. helmfile requires to support dry-run logic.

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

No branches or pull requests

10 participants