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 install fails when empty file or folder of same name as chart present in working dir #13008
Comments
Marking this as a feature, as the behavior was designed this way (not saying it is the best way).
I think perhaps the behavior can be improved (as a non-breaking change). But not to the extent that a FS entry must be "valid" for it to be considered. e.g. If I downloaded a corrupt archive, I would expect attempting to install of that corrupt archive from the FS to fail. Not e.g. ignore and proceed to lookup a repository because the chart (archive) is not valid. So any introduced checks would need to be very specific and intuitive (for all scenarios: otherwise the next user hits a different use case which is confusing for them). And overall, while the behavior could be improved. It would need to be demonstrated it is worth it. To a certain extent, the "fail fast" behavior, and simple implementation of |
Output of
helm version
:Output of
kubectl version
:Cloud Provider/Platform (AKS, GKE, Minikube etc.):
kind v0.22.0 go1.21.7 darwin/arm64
Summary
Helm fails to install a chart when repository is specified on the command line with
--repo
and a file or directory of the same name as the chart is present in the local working directory.Steps to reproduce
touch hello
(ormkdir hello
)helm install
passing the repository via--repo
so that chart name does include have the[repo-name]/
prefixOR
Root cause analysis
The issue can be traced back to a check for the presence of a local filesystem entry of the same name as the chart (through
os.Stat()
) as a first step in locating the chart to be installed, infunc (c *ChartPathOptions) LocateChart(...)
:In the most common use-case, the user will invoke the
install
command with the chart name including the repository prefix, separated by a slash:which just happens to not be a valid name for a local filesystem entry, and thus avoids causing a false positive in
os.Stat()
. However, no explicit validation exists to confirm the presence of a local chart.When the CLI command includes the repository URL via the
--repo
argument, the chart name no longer includes the repo prefix and slash that was preventing it from matching a local FS entry. The conditions are thus created for the above mentioned installation error to take place.Possible solutions
This code was originally intended to detect the presence of a local copy of the chart to be installed. However, the
os.Stat()
check is too generic and will yield false positive matches even when a simple empty file or directory of the given name is present, which is obviously not the intended behaviour.This has been reported in various forms in the past which sparked conversations on wether a local chart should even be picked up when the command line explicitly requests one from a remote repository. It was pointed out that the current behaviour (of prioritising the local copy) needs to be preserved until the next major version of Helm.
However, here I am reporting an extreme manifestation of this issue which actually leads to install failure and should therefore be addressed as a bug within the confines of backward compatibility with the original intent.
A simple solution that would preserve the status quo while eliminating the error edge case would be to replace the
os.Stat()
with a more exhaustive check that determines with increased certainty if the local FS entry is indeed a valid chart, be it a directory or archive.PR #13009 implements this proposal.
The text was updated successfully, but these errors were encountered: