8000 fix: correctly check for chart directory conflict by foyerunix · Pull Request #30868 · helm/helm · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix: correctly check for chart directory conflict #30868

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
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: correctly check for chart directory conflict #30868

wants to merge 1 commit into from

Conversation

foyerunix
Copy link
@foyerunix foyerunix commented May 16, 2025 •
edited
Loading

Hello,

This PR fix an inconsistency when checking for a directory conflict while expanding a chart. Before we would check for a directory name composed of the chart name and the chart version. This would create a superfluous directory while also not really checking for a conflict because the chart will be expanded in a directory whose name would only be the chart name.

Example:

# bin/helm fetch external-dns --untar --version 8.7.12 --repo https://charts.bitnami.com/bitnami
# ls | grep external-dns
external-dns
external-dns:8.7.12

# bin/helm fetch external-dns --untar --version 8.7.11 --repo https://charts.bitnami.com/bitnami
# ls | grep external-dns
external-dns
external-dns:8.7.11
external-dns:8.7.12

We end up with a mix of the 8.7.11 and 8.7.12 charts.

Since we compute the final chartDir path from the chartName found in the Chart.yaml file in the Expand function, I think the directory conflict check is more reliable if implemented there.

Best Regards.

fix: correctly check for chart directory conflict
82b0bad 
This commit move the check for directory conflict
to the Expand function.

This let us check if we will overwrite the actual
chart directory. The previous implementation would
check if a directory suffixed with the chart version exist.

This would results in:
- Creating a spuerfluous directory with the chart name
suffixed by the chart version.
- Not preventing the overwrite of an existing chart with the same
name but with a different version.

Signed-off-by: foyerunix <foyerunix@foyer.lu>
@pull-request-size pull-request-size bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label May 16, 2025
@robertsirc robertsirc added the v4.x Issues and Pull Requests related to the major version v4 label Jun 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
size/S Denotes a PR that changes 10-29 lines, ignoring generated files. v4.x Issues and Pull Requests related to the major version v4
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@foyerunix @robertsirc
0