fix(helm): Allow empty version for localpath chart #3815
Conversation
Signed-off-by: Brandt Keller <[email protected]>
✅ Deploy Preview for zarf-docs canceled.
|
Codecov ReportAttention: Patch coverage is
🚀 New features to boost your workflow:
|
Signed-off-by: Brandt Keller <[email protected]>
Signed-off-by: Brandt Keller <[email protected]>
brandtkeller
changed the title
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| if chart.Version != "" && chart.Version != loadedChart.Metadata.Version { | ||
| // this is important as deploy will use teh chart version to find the chart tarball location | ||
| return fmt.Errorf("expected chart version %s does not match the actual chart metadata version %s", chart.Version, loadedChart.Metadata.Version) |
There was a problem hiding this comment.
++ erroring here makes sense to me
|
I agree with this change, however we must consider how this will effect packages created and deployed with different versions of Zarf. A package using localpath without a version, will not deploy on earlier versions of Zarf. The error will be: 2025-05-22 10:49:41 ERR failed to deploy package: unable to deploy component "demo-helm-charts": open /tmp/zarf-3011270750/components/demo-helm-charts/values/podinfo-local--0: no such file or directoryI don't think we have this written down anywhere (we should write it down), but a goal with Zarf has been to not assume people have the same Zarf version for create and deploy. This change is also proposed by a comment in #2245, which will happen with #3433 Adding this for context, open to other solutions besides waiting for the next schema version |
|
@AustinAbro321 superb callout and catch - I will get that note added to the draft principles document. Otherwise I fully agree that this should not be merged as-is. I will think about compatibility otherwise we may need to close this out. |
Description
chart version can be any value for
localPathhelm charts and this is used in the naming of the helm archive within the zarf package. This can result in confusion and inaccuracy of artifact versions were one to inspect/decompress a package.Given that a chart name in the
ZarfPackageConfigmust be unique (and is enforced during create) - then allowing for version being not present should not create any collisions.This PR makes the following assumptions:
localPathcharts should not require a version be specifiedThis PR helps decide on paths forward.
Related Issue
Fixes #3813
Checklist before merging