search

fluxcd / helm-controller

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

Replace `_` with `+` when verifying the chart version matches the OCI artifact tag #1102

Closed baburciu closed 3 weeks ago

baburciu commented 3 weeks ago

Closes https://github.com/fluxcd/helm-controller/issues/1099

OCI tags are not semver compatible, with + character not allowed. Helm worked around this by converting + to _ before pushing to the OCI repo and vice versa when pulling from the repo.

This PR makes helm-controller adhere to that convention, by converting OCI artifact tag using _ to + replacement prior to any check or processing.

baburciu commented 3 weeks ago

I'm not sure what should be best returned as error

return "", fmt.Errorf("failed parsing artifact revision %s", tagD[0])

or

return "", fmt.Errorf("failed parsing artifact revision %s", tagConverted)

but the conversion needs to happen prior to calling semver.NewVersion

stefanprodan commented 3 weeks ago

I think we should return the one with a + as that's the string we try to validate as semver.

PS. This PR needs a test using a tag that has _.

baburciu commented 3 weeks ago

thank you, I've added a test for it, which is passing make test. I'm new to this, so if it's not that good, please inform