Skip to content

When specifying a helm chart via a localPath, the version field should not be required #3813

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
ktarplee opened this issue May 16, 2025 · 2 comments · May be fixed by #3815
Open

When specifying a helm chart via a localPath, the version field should not be required #3813

ktarplee opened this issue May 16, 2025 · 2 comments · May be fixed by #3815
Assignees
@brandtkeller

Comments

@ktarplee
Copy link

Right now components that use helm charts from a localPath are required to specify the version as well. This is redundant since the chart obviously has a chart version in the Chart.yaml file. What is worse is that the version specified below can be different than the chart which is at best misleading.

- name: splash
   localPath: ../charts/splash
   version: "0.0.1" # THIS SHOULD NOT BE REQUIRED WHEN localPath IS SPECIFIED
   namespace: default
   valuesFiles:
      - splash-values.yaml
@brandtkeller
Copy link
Member

Appreciate the issue @ktarplee

I agree that there is a disconnect between the requirement and the accuracy (of allowing any entry in that field).

I'll toss up a quick PR - I think using the required Chart.yaml version improves provenance here - the outlier being when the declared version in the ZarfPackageConfig does not match the actual version - we should probably warn.

@github-project-automation github-project-automation bot moved this to Triage in Zarf May 16, 2025
@brandtkeller brandtkeller self-assigned this May 16, 2025
@brandtkeller brandtkeller moved this from Triage to In progress in Zarf May 16, 2025
@brandtkeller brandtkeller linked a pull request May 16, 2025 that will close this issue
Open
2 tasks
@brandtkeller
Copy link
Member

As seen in the PR's first commit - attempted to use the loaded chart version as the version - but this creates an issue on deploy where deploy is looking for a specifically versioned chart tarball to load.

Chart names are required to be unique in the ZarfPackageConfig and therefor I believe we could drop the version when not supplied.

This then influences my stance to say we should error on create when version specified does not match the actual version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@brandtkeller brandtkeller

Labels
None yet
Projects
Zarf
Status: In progress
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(helm): Allow empty version for localpath chart zarf-dev/zarf
2 participants
@ktarplee @brandtkeller