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

Can't refer to aliased subcharts by alias names in master chart #2993

Closed
omkensey opened this issue Oct 1, 2017 · 22 comments
Closed

Can't refer to aliased subcharts by alias names in master chart #2993

omkensey opened this issue Oct 1, 2017 · 22 comments
Assignees
Labels
bug Categorizes issue or PR as related to a bug. Stale

Comments

@omkensey
Copy link

omkensey commented Oct 1, 2017

I have a chart with two subcharts in progress. The subcharts have hyphenated names, so to avoid working around hyphens in YAML keys, I aliased them to non-hyphenated names. However, when I try to use the alias names in the parent chart's templates, I get a render error if the values.yaml has the unaliased names for the subchart value keys; if the aliased names are used, then the subchart value keys are not actually computed into the subchart templates, leaving everything that isn't a global value blank.

@omkensey omkensey changed the title Aliasing subcharts confuses helm lint when using "required" Aliasing subcharts causes "required" to fail Oct 1, 2017
@omkensey omkensey changed the title Aliasing subcharts causes "required" to fail Can't refer to aliased subcharts by alias names in master chart Oct 2, 2017
@omkensey
Copy link
Author

omkensey commented Oct 2, 2017

I believe the attached file is a minimal reproducer for showing the issue -- from what I understand, it should work, but instead throws a render error.

testchart-0.2.0.tar.gz

@thomastaylor312
Copy link
Contributor

@omkensey Thanks for doing the due diligence to reproduce this one. I'll try to get around to fixing this. If it isn't a huge blocker, we may wait until 2.7.1 to fix

@thomastaylor312 thomastaylor312 self-assigned this Oct 7, 2017
@omkensey
Copy link
Author

omkensey commented Oct 7, 2017

I think I have a workaround I can use for now, and can PR it to something cleaner later.

@thomastaylor312
Copy link
Contributor

@omkensey That would be great! If you don't get around to it, let me know and I'll try to take it on

@josdotso
Copy link

josdotso commented Oct 24, 2017

I just hit this problem majorly. I'm writing a wrapper chart and the hyphens in the upstream chart name are killing me. Fixing this would logically fix the fact that range ops don't appear to work any better using "index".. Re: #2192

@josdotso
Copy link

This also seems to indicate that aliasing two of the same upstream chart for different purposes with unique alias-named values.yaml trees is impossible. I would go so far as to say the default at values.yaml should be the alias if alias exists. If alias DOES exist, then canonical name shouldn't work IMHO -- because collisions. (Unless we intelligently scope (and fall through) "global" usage of an aliased chart via its canonical name -- apart from alias config).

@omkensey
Copy link
Author

I would go so far as to say the default at values.yaml should be the alias if alias exists. If alias DOES exist, then canonical name shouldn't work IMHO -- because collisions.

As I understand it, that's how aliasing is supposed to work, actually -- see #2508.

The workaround I've been using is to set a variable at the top of the template, e.g.:

{{- $newname := index .Values "hyphenated-name" -}}

Then further down you can refer to $newname.whatever.

@thomastaylor312 that's the workaround I was referring to above; I've not got the coding chops to actually fix this in the helm code.

@omkensey
Copy link
Author

Over in my chart PR helm/charts#1719, unguiculus just said

[requirements.yaml] is used for dynamically linked dependencies only, whereas you added the dependencies directly to the charts folder. I guess that's the reason why you can't use aliases in this case.

So maybe that's the issue with what I (and @josdotso?) am doing -- but if that's the case, IMHO directly-added charts should be aliasable just like charts retrieved dynamically, so I'd argue there's still a bug here.

@josdotso
Copy link

josdotso commented Oct 25, 2017

Absolutely @omkensey !

Ranging on the index-based variable wasn't working for me last night when I tried it. Are you ranging over any such variables?

EDIT: also, my requirements.yaml deps are dynamic. My parent chart ships with no charts dir.

@omkensey
Copy link
Author

No, I'm not ranging anything in my chart.

@thomastaylor312
Copy link
Contributor

thomastaylor312 commented Oct 27, 2017

@omkensey I think that the comment from unguiculus was correct. The requirements file doesn't care what you have explicitly put in your charts directory and so that would be why you can't alias. So I don't think that is necessarily a bug in this case, but more of a feature add (I admit that is kind of splitting hairs). I would still like to fix it though if I can scrounge up the time

@josdotso So you are having this issue even with dynamic deps?

@josdotso
Copy link

josdotso commented Oct 27, 2017

@thomastaylor312, Yes -- I believe so. I'm using requirements.yaml and helm dependency build on a clone. I'll try to further confirm from scratch, but the dep build just pulls a tgz named for the canonical package with a directory in it named for the canonical package. It seems like the deps should be pulled to tgz of alias + canonical version, instead. However, there's no mutating of the folder inside the tgz, so maybe I just misunderstand the mechanism.

EDIT: That is, my Helm parent chart repo clone (one chart in a bare git repo) has no charts dir when I start.

@thomastaylor312
Copy link
Contributor

@josdotso It actually only does the "renaming" inside of helm when putting together the manifests to send to tiller

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

Prevent issues from auto-closing with an /lifecycle frozen comment.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or @fejta.
/lifecycle stale

@fejta-bot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten
/remove-lifecycle stale

@thomastaylor312
Copy link
Contributor

/remove-lifecycle rotten
/lifecycle frozen

@k8s-ci-robot k8s-ci-robot added bug Categorizes issue or PR as related to a bug. and removed bug labels Jun 5, 2018
@hstenzel
Copy link

I believe I just hit this bug, but with a twist.

In my scenario, I have a chart for my test harness. It needs to enable or disable various features of its subchart. However, the values being modified show up both in USER-SUPPLIED VALUES and COMPUTED VALUES, however the requirements.yaml condition is not being honored.

@MPV
Copy link

MPV commented Apr 17, 2019

Here's how I reproduce this bug:

1. Create a chart that uses the same sub chart twice (individually named/aliased to a and b):

$ tree parent-chart
parent-chart
├── Chart.yaml
├── charts
│   ├── a -> ../../another-chart
│   └── b -> ../../another-chart
├── templates
└── values.yaml

2. Set up different values for the different sub charts:

$ cat parent-chart/values.yaml
a:
  name: "A"
b:
  name: "B"

3. Double-check the default values of the sub chart:

$ cat another-chart/values.yaml
name: "default-name"

4. Check the computed values for the sub charts:

...where you can see that the COMPUTED VALUES aren't being set correctly for the aliased charts:

$ helm install --dry-run --debug parent-chart 2>&1
[...]
COMPUTED VALUES:
a:
  name: "A"
b:
  name: "B"
another-chart:
  name: "default-name"
[...]

...whereas I'm only expecting this:

[...]
COMPUTED VALUES:
a:
  name: "A"
b:
  name: "B"
[...]

...and in turn that the right a/b values would be applied to their respective aliased subchart. 😊

@MPV
Copy link

MPV commented Apr 23, 2019

@omkensey / @thomastaylor312 Were any of you able to take a stab at this?

@tomislater
Copy link

@MPV Yeah, I have found the same issue here: #7093 (comment)

@github-actions
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.

@admin-hobops
Copy link

admin-hobops commented Oct 23, 2021

Hi all! I'm willing to reward with 1000 USD to the first person who fixes this part of the code so aliases can be overridden from a parent chart. Is anyone with me?

We are discussing in this issue #7093

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Categorizes issue or PR as related to a bug. Stale
Projects
None yet
Development

No branches or pull requests

10 participants