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.

Sign up for GitHub

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

Jump to bottom

Fix helm lint fails when global values are used in subcharts #8514

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

Fix helm lint fails when global values are used in subcharts #8514

wants to merge 1 commit into from

Conversation

wawa0210
Copy link
Contributor

@wawa0210 wawa0210 commented Jul 25, 2020 •
edited
Loading

Signed-off-by: wawa0210 xiaozhang0210@hotmail.com

Fix #8489

Introduced by #7288

What this PR does / why we need it:

When using helm lint --with-subcharts, the values.yaml data of the parent directory is not transferred to the subdirectory, resulting in missing global values, and helm lint failure

Special notes for your reviewer:

If applicable:

  • this PR contains documentation
  • this PR contains unit tests
  • this PR has been tested for backwards compatibility

@helm-bot helm-bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jul 25, 2020
@wawa0210 wawa0210 force-pushed the fix-8489 branch 2 times, most recently from ef0eb87 to 344838e Compare July 25, 2020 17:06
@wawa0210 wawa0210 closed this Jul 28, 2020
@helm-bot helm-bot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jul 28, 2020
@wawa0210 wawa0210 reopened this Jul 28, 2020
@helm-bot helm-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 28, 2020
@helm-bot helm-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jul 28, 2020
@wawa0210
fix helm lint --with-subcharts errot
5de95f8 
Signed-off-by: wawa0210 <xiaozhang0210@hotmail.com>
@wawa0210
Copy link
Contributor Author

@bacongobbler

Can you help, take some time to help reivew this pr? Looking forward to your comments

@jbilliau-rcd
Copy link

jbilliau-rcd commented Jun 21, 2021 •
edited
Loading

Is this being merged in and released soon @bacongobbler @wawa0210 ? Having the exact same issue: my helm lint commands all fail since I have a subchart being called even though i have a values.yaml file in my base chart folder setting that subchart to false by default.

dependencies:
  - name: external-ingress-template
    repository: "file://../charts/external-ingress-template"
    condition: global.externalIngress.enabled

~/git/ql-helm-charts$ helm lint charts/app-template/ --values charts/app-template/values.yaml --values helmatest/internal-api/values.yaml

[INFO] Chart.yaml: icon is recommended
[ERROR] templates/: template: app-template/charts/external-ingress-template/templates/ingress-gateway.yaml:15:22: executing "app-template/charts/external-ingress-template/templates/ingress-gateway.yaml" at <.Values.global.externalIngress.externalUrl>: wrong type for value; expected string; got interface {}

It shouldnt even be trying to do that since the values.yaml file in my base chart has this:

externalIngress:
    enabled: false

@oreissig
Copy link

Is there any help needed to get this PR over the finish line?
I'd love to see this being resolved, as helm lint is only of limited use for our use case right now.

@0x0dr1y
Copy link

0x0dr1y commented Sep 6, 2022

What's the current status of this?

@felipecrs felipecrs mentioned this pull request Dec 20, 2022
Closed
@rvarun11
Copy link

Any update on this? Would love see this past the finish line.

@wawa0210
Copy link
Contributor Author

wawa0210 commented Dec 22, 2022 •
edited
Loading

The community has not responded for too long, if this PR is meaningful, I can try to follow up again

@0x0dr1y
Copy link

0x0dr1y commented Feb 9, 2023

@wawa0210 In my eyes this i still meaningful

@celalsahin
Copy link

Any news on this?

joejulian
joejulian reviewed Aug 9, 2023
View reviewed changes
cmd/helm/testdata/output/lint-chart-with-bad-subcharts-with-subcharts.txt
Comment on lines -1 to -20
==> Linting testdata/testcharts/chart-with-bad-subcharts
[INFO] Chart.yaml: icon is recommended
[WARNING] templates/: directory not found
[ERROR] : unable to load chart
error unpacking bad-subchart in chart-with-bad-subcharts: validation: chart.metadata.name is required

==> Linting testdata/testcharts/chart-with-bad-subcharts/charts/bad-subchart
[ERROR] Chart.yaml: name is required
[ERROR] Chart.yaml: apiVersion is required. The value must be either "v1" or "v2"
[ERROR] Chart.yaml: version is required
[INFO] Chart.yaml: icon is recommended
[WARNING] templates/: directory not found
[ERROR] : unable to load chart
validation: chart.metadata.name is required

==> Linting testdata/testcharts/chart-with-bad-subcharts/charts/good-subchart
[INFO] Chart.yaml: icon is recommended
[WARNING] templates/: directory not found

Error: 3 chart(s) linted, 2 chart(s) failed
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like a lot of context is lost.

@joejulian
Copy link
Contributor

joejulian commented Aug 9, 2023 •
edited
Loading

@wawa0210 If you're still interested in this, it'll need a rebase.

@joejulian joejulian added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joejulian joejulian joejulian left review comments

Assignees
No one assigned
Labels
needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. Stale
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

helm lint fails when global values are used in subcharts
8 participants
@wawa0210 @jbilliau-rcd @oreissig @0x0dr1y @rvarun11 @celalsahin @joejulian @helm-bot