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

Helm v3.13.0 and above breaks umbrella charts exposing empty values #12599

Open
jawadqur opened this issue Nov 28, 2023 · 11 comments
Open

Helm v3.13.0 and above breaks umbrella charts exposing empty values #12599

jawadqur opened this issue Nov 28, 2023 · 11 comments

Comments

@jawadqur
Copy link

jawadqur commented Nov 28, 2023

Title: Umbrella Chart Image Override Issue Since v3.13.0

Issue Description

Environment:

  • Helm Version: 3.13.0 and above
  • Chart Name: gen3 (Umbrella Chart)
  • Repository: gen3-helm

Description:
Since the release of Helm v3.13.0, we have been encountering a critical issue with our umbrella chart, named gen3, in the gen3-helm repository. This chart is designed to expose values that allow overriding images for the child charts. However, these values are initially empty in the gen3 chart's values.yaml file.

Expected Behavior:
In previous versions of Helm, these empty values were handled without any issues, allowing the umbrella chart to function correctly.

Current Problem:
Starting with Helm v3.13.0, when attempting to install the gen3 umbrella chart, the process fails with an "InvalidImageName" error. This suggests that the empty values from the gen3 values.yaml are being incorrectly passed to the child charts. This behavior marks a significant and undocumented change from how Helm previously handled such scenarios.

Steps to Reproduce:

  1. Use Helm v3.13.0 or any later version.
  2. Attempt to install the gen3 umbrella chart.
  3. Observe the "InvalidImageName" error during the installation process.

Impact:
This issue is causing significant disruption, as it prevents the proper deployment of our umbrella chart and its associated services.

Additional Context:
Using helm v3.12.3 I see the old behavior which I think is correct. I would like to not re-make the charts for this, but if this is expected behavior then it should be documented.

@jawadqur
Copy link
Author

jawadqur commented Nov 28, 2023

Could this be the commit that broke it? Although this is not included in 3.13.0, but in 3.13.1

3547a4b

@jawadqur jawadqur changed the title Helm v1.13.0 and above breaks umbrella charts exposing empty values Helm v3.13.0 and above breaks umbrella charts exposing empty values Nov 28, 2023
@gjenkins8
Copy link
Contributor

Use Helm v3.13.0 or any later version.

Can you confirm with v3.13.2 please. 3.13.0 and 3.13.1 had bugs with values handling.

Helm 3.13 series overall had fixes when it came to values handling, so their might be a regression. See:

#12162
#12480 (fix applied to 3.13.2)

@jawadqur
Copy link
Author

jawadqur commented Dec 6, 2023

I can confirm that issue is still present in 3.13.2

The presedence issue has been fixed probably, but nil/empty values are now taking presedence, where as the behaviour for those should be that nil values should be treated as nil, and not empty strings.

@jawadqur
Copy link
Author

jawadqur commented Dec 7, 2023

https://github.com/helm/helm/blob/main/pkg/chartutil/dependencies.go#L37-L39

I guess this is a new behavioral change in helm.

@felipecrs
Copy link

I can confirm this breaking change as well. I can easily reproduce by templating with 3.12.3 and then templating again with 3.13.2.

@jervds
Copy link

jervds commented Dec 11, 2023

Hello,

Can someone confirm that the behavior is expected or not?
Should we wait for a new version 3.13.3 to fix it, or should we adapt our templates?

Thanks a lot for the answer,
Jérôme

@intetinte
Copy link

My impression is that umbrella chart values takes precedence of child chart values. That implies the umbrella chart could potentially clear values in the child charts.

@rptaylor
Copy link

If you look at the linked PRs that George mentioned , parent chart values are higher priority than subcharts. I think the previous expected behaviour that you were relying on (null value in parent chart would not override a subchart) was a bug and the change is mentioned in the release notes: https://github.com/helm/helm/releases/tag/v3.13.0
"Values handling had numerous issues fixed and now consistently has a priority of (1) User specified values (e.g CLI), (2) parent chart values, (3) imported values, and (4) subchart values. Additionally, null can now consistently be used to remove values. Note, there is a regression around this in 3.13.0 that's fixed in 3.13.1."

But if you want to rely on two bugs that cancel each other out just use --values (#12488)

@felipecrs
Copy link

felipecrs commented Dec 23, 2023

Being a bug or not, true adherence to the semver spec would require bumping major version before changing or fixing it.

Or at least a breaking change notice should be added to the release notes, with more careful explanations as to how to handle them like migration examples.

Copy link

This issue has been marked as stale because it has been open for 90 days with no activity. This thread will be automatically closed in 30 days if no further activity occurs.

@github-actions github-actions bot added the Stale label Mar 23, 2024
@felipecrs
Copy link

This is still valid. If there is no plan to fix it, at least an official statement would be nice.

@github-actions github-actions bot removed the Stale label Mar 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants