search

fluxcd / helm-controller

The GitOps Toolkit Helm reconciler, for declarative Helming
https://fluxcd.io
Apache License 2.0
396 stars 153 forks source link

Ignore 'v' version prefix in OCI artifact and Helm chart #990

Closed makkes closed 1 month ago

makkes commented 1 month ago

Tools such as Bitnami's charts-syncer strip the v prefix from the chart version so that the OCI artifact version differs from the version defined in the chart's metadata. This leads to an error similar to this returned from h-c:

artifact revision 1.14.5 does not match chart version v1.14.5

This PR makes h-c ignore a leading v prefix in either the chart version of the OCI artifact tag.

stefanprodan commented 1 month ago

The v prefix is no valid for Helm charts, are you saying that you have a chart stored as vX.Y.Z in a Helm HTTP repo and charts-syncer removes it from Chart.yaml but not from the artifact tag?

makkes commented 1 month ago

The v prefix is no valid for Helm charts

I ran into this with cert-manager:

$ helm search repo jetstack/cert-manager                                                                                                                                                                                                                            
NAME                                    CHART VERSION   APP VERSION     DESCRIPTION                                                                                                                                                                                               
jetstack/cert-manager                   v1.14.5         v1.14.5         A Helm chart for cert-manager

They're using a v-prefixed version in their charts (that's also in the Chart.yaml).

are you saying that you have a chart stored as vX.Y.Z in a Helm HTTP repo and charts-syncer removes it from Chart.yaml but not from the artifact tag?

The other way around: charts-syncer stores the Helm artifact in OCI tagged as X.Y.Z but the Chart.yaml defines it as vX.Y.Z.

stefanprodan commented 1 month ago

Ok, we can backport this to v1.0.x but I would wait a week or two before doing a patch release.

stefanprodan commented 1 month ago

We should also allow the reverse situation, v prefix in the tag, but no v in Chart.yaml.

makkes commented 1 month ago

We should also allow the reverse situation, v prefix in the tag, but no v in Chart.yaml.

It catches both. I implemented a test for each case.

fluxcdbot commented 1 month ago

Successfully created backport PR for release/v1.0.x: