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

Improve local helm chart validation #2410

Merged
merged 1 commit into from
May 17, 2023

Conversation

L3n41c
Copy link
Contributor

@L3n41c L3n41c commented May 16, 2023

Proposed changes

The helm.NewRelease(…) function has some logic to look for a local chart before trying to contact a remote registry, even if a RepositoryOpts.Repo parameter has been explicitly provided.

This is an issue when the chart we want to create a release from has a name that conflicts with a local folder name.

This change improves the logic and considers a local directory to be a valid chart only if it contains a Chart.yaml file.

Related issues (optional)

@github-actions
Copy link

PR is now waiting for a maintainer to run the acceptance tests.
Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

@github-actions
Copy link

PR is now waiting for a maintainer to run the acceptance tests.
Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

@kpitzen
Copy link
Contributor

kpitzen commented May 16, 2023

/run-acceptance-tests

@pulumi-bot
Copy link
Contributor

@kpitzen
Copy link
Contributor

kpitzen commented May 16, 2023

Hi @L3n41c - thank you for opening this PR! I just saw the issue you opened on its behalf, and I appreciate the diligence you've gone through here. I've approved the tests to run, so will wait on their results for now.

@kpitzen
Copy link
Contributor

kpitzen commented May 16, 2023

Looks like tests are running successfully, and we have a good changelog entry! Just want to cc: @rquitales as a SME to verify this looks good as well.

@kpitzen kpitzen requested a review from rquitales May 16, 2023 19:10
@rquitales rquitales changed the title Fix Chart.yaml file is missing error Improve local helm chart validation May 16, 2023
Copy link
Contributor

@rquitales rquitales left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! I think there's still more work to be done to fully close #2409, but this is in the right step and should help solve the current issue you're facing. Approving this PR, and I'll follow-up in the GH Issue about potential next steps we might do to close out #2409.

I've added a comment in #2409 (comment) regarding what I think should also be done. That'll require slightly more work, especially to add more/new test cases. Feel free to add that into this PR if you have capacity - we're more than happy to help point you in the right direction for that.

@rquitales rquitales merged commit 4cb1007 into pulumi:master May 17, 2023
9 of 10 checks passed
@L3n41c L3n41c deleted the lenaic/fix_missing_chart branch May 19, 2023 11:51
@ffMathy
Copy link

ffMathy commented May 24, 2023

@L3n41c and @rquitales this has broken local charts from .tar.gz files.

See #2426.

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.

None yet

5 participants