OCI artifact semver alignment with Helm for tags with underscore

[baburciu]

HelmRelease referencing a chart directly from an OCIRepository fails if the OCIRepository.spec.ref.tag contains the underscore (_) character.

Example:

  apiVersion: helm.toolkit.fluxcd.io/v2
  kind: HelmRelease
  metadata:
    :
    name: monitoring-crd
    namespace: sylva-system    
    :
  spec:
    chartRef:
      kind: OCIRepository
      name: unit-monitoring-crd
    install:
      createNamespace: true
    interval: 15m
    :
  status:
    conditions:
    :
    - lastTransitionTime: "2024-10-25T22:07:45Z"
      message: artifact revision 104.1.1_up57.0.3 does not match chart version 104.1.1+up57.0.3
      observedGeneration: 2
      reason: ChartMutateError
      status: "False"
      type: Ready
---
  apiVersion: source.toolkit.fluxcd.io/v1beta2
  kind: OCIRepository
  metadata:
    :
    name: unit-monitoring-crd
    namespace: sylva-system
    :
  spec:
    interval: 60m0s
    provider: generic
    ref:
      tag: 104.1.1_up57.0.3
    timeout: 60s
    url: oci://registry.gitlab.com/sylva-projects/sylva-core/rancher-monitoring-crd
  status:
    artifact:
      :
      revision: 104.1.1_up57.0.3@sha256:85ea0013056258452ce27431392167ac036fd7fd1cb64df3a750b04851ac6d20

This happens due to https://github.com/fluxcd/helm-controller/blob/v1.0.1/internal/controller/helmrelease_controller.go#L917-L920 check, which expects the tag from the tag@digest formatted OCI artifact manifest digest (which ends up as the OCIRepository.status.artifact.revision), to be equal to the Chart.yaml-defined version.

We consider helm constraints and that "OCI registries don't support + as a tag character", as helm itself informs:

OCI artifact references (e.g. tags) do not support the plus sign (+). To support
storing semantic versions, Helm adopts the convention of changing plus (+) to
an underscore (_) in chart version tags when pushing to a registry and back to
a plus (+) when pulling from a registry.

that is why when we create OCI artifacts out of helm charts in Project Sylva we replace + with _ from chart's version to define the artifact tag (but Chart.yaml contents are untouched, e.g. OCI artifact oci://registry.gitlab.com/sylva-projects/sylva-core/rancher-monitoring-crd:104.1.1_up57.0.3 has version: 104.1.1+up57.0.3 in Chart.yaml).

It looks like FluxCD helm-controller doesn't do the reverse (replace _ with +) before doing the said validation.

Building an image on top of fluxcd/helm-controller:v1.0.1 (we're currently using FluxCD v2.3.0) with below diff allows for the HelmRelease to be deployed.

--- a/internal/controller/helmrelease_controller.go
+++ b/internal/controller/helmrelease_controller.go
@@ -915,9 +915,12 @@ func mutateChartWithSourceRevision(chart *chart.Chart, source sourcev1.Source) (
        switch {
        case strings.Contains(revision, "@"):
                tagD := strings.Split(revision, "@")
-               if len(tagD) != 2 || tagD[0] != chart.Metadata.Version {
-                       return "", fmt.Errorf("artifact revision %s does not match chart version %s", tagD[0], chart.Metadata.Version)
-               }
+               // By convention: Change underscore (_) back to plus (+) to get valid SemVer
+               // see https://github.com/helm/helm/blob/v3.14.4/pkg/registry/client.go#L45-L50
+               convertedTag := strings.ReplaceAll(tagD[0], "_", "+")
+               if len(tagD) != 2 || convertedTag != chart.Metadata.Version {
+                       return "", fmt.Errorf("artifact revision %s does not match chart version %s", convertedTag, chart.Metadata.Version)
+                }
                // algotithm are sha256, sha384, sha512 with the canonical being sha256
                // So every digest starts with a sha algorithm and a colon
                sha, err := extractDigestSubString(tagD[1])

I could submit a PR if this makes sense

[stefanprodan]

We use the build tag to keep track of the OCI digests, this happens in the same function here:

helm-controller/internal/controller/helmrelease_controller.go

Line 928 in 6085b4f

*ver, err = ver.SetMetadata(sha)

So the build tag that you set at push is overwritten by Flux in Chart.yaml, doesn't this affect the sequential upgrades in your fork?

[baburciu]

I didn't understand well why the upgrade would be affected, I'll give it a try.
But if the point was regarding possibly duplicate use of + (one from the OCI tag itself post _ with + replacement and another from the leading 12 characters of the artifact manifest digest to the Chart.yaml-defined version to make sure mutable tags are detected), looks like the SetMetadata call would only return the latter - checked in Go playground. We see it in action below.
So I would think that for an upgrade from some OCIRepository.spec.ref.tag 104.1.2+up57.0.2 to 104.1.2+up57.0.3 (dummy values, rancher charts don't seem to have increments in that build metadata part), helm-controller would still see different values when replacing .Chart.Version, due to the 12 charts from different digests

hr & ocirepo status (click to expand)

  apiVersion: helm.toolkit.fluxcd.io/v2
  kind: HelmRelease
  metadata:
    :
    name: monitoring-crd
    namespace: sylva-system
  spec:
    chartRef:
      kind: OCIRepository
      name: unit-monitoring-crd
    interval: 15m
    releaseName: rancher-monitoring-crd
    storageNamespace: cattle-monitoring-system
    targetNamespace: cattle-monitoring-system
  status:
    conditions:
    - lastTransitionTime: "2024-10-31T09:17:22Z"
      message: Helm install succeeded for release cattle-monitoring-system/rancher-monitoring-crd.v1
        with chart [email protected]+efafa44b7261
      observedGeneration: 2
      reason: InstallSucceeded
      status: "True"
      type: Ready
---
  apiVersion: source.toolkit.fluxcd.io/v1beta2
  kind: OCIRepository
  metadata:
    :
    name: unit-monitoring-crd
    namespace: sylva-system
  spec:
    interval: 60m0s
    provider: generic
    ref:
      tag: 104.1.2_up57.0.3
    timeout: 60s
    url: oci://172.20.136.39/proxy_cache_registry.gitlab.com/sylva-projects/sylva-core/rancher-monitoring-crd
  status:
    artifact:
      digest: sha256:2b5e9fde0f5167956811364450270003013730d6fc06547e210712030626ed4b
      lastUpdateTime: "2024-10-31T09:15:49Z"
      metadata:
        catalog.cattle.io/certified: rancher
        catalog.cattle.io/hidden: "true"
        catalog.cattle.io/namespace: cattle-monitoring-system
        catalog.cattle.io/release-name: rancher-monitoring-crd
        org.opencontainers.image.created: "2024-10-21T21:11:07Z"
        org.opencontainers.image.description: Installs the CRDs for rancher-monitoring.
        org.opencontainers.image.title: rancher-monitoring-crd
        org.opencontainers.image.version: 104.1.2+up57.0.3
      path: ocirepository/sylva-system/unit-monitoring-crd/sha256:efafa44b7261a859aebfbabe54bb3e4a0b8a224a021c5fffdaf1bc916452d80d.tar.gz
      revision: 104.1.2_up57.0.3@sha256:efafa44b7261a859aebfbabe54bb3e4a0b8a224a021c5fffdaf1bc916452d80d
      size: 310847
      url: http://source-controller.flux-system.svc.cluster.local./ocirepository/sylva-system/unit-monitoring-crd/sha256:efafa44b7261a859aebfbabe54bb3e4a0b8a224a021c5fffdaf1bc916452d80d.tar.gz
    conditions:
    - lastTransitionTime: "2024-10-31T09:15:49Z"
      message: stored artifact for digest '104.1.2_up57.0.3@sha256:efafa44b7261a859aebfbabe54bb3e4a0b8a224a021c5fffdaf1bc916452d80d'
      observedGeneration: 1
      reason: Succeeded
      status: "True"
      type: Ready
    - lastTransitionTime: "2024-10-31T09:15:49Z"
      message: stored artifact for digest '104.1.2_up57.0.3@sha256:efafa44b7261a859aebfbabe54bb3e4a0b8a224a021c5fffdaf1bc916452d80d'
      observedGeneration: 1
      reason: Succeeded
      status: "True"
      type: ArtifactInStorage
    observedGeneration: 1
    url: http://source-controller.flux-system.svc.cluster.local./ocirepository/sylva-system/unit-monitoring-crd/latest.tar.gz

[stefanprodan]

Ah good point, the tag is overwritten not appended. Ok you can open a PR with this and we'll discuss it at the Flux dev meeting today.

[stefanprodan]

@baburciu can you also try flux reconcile hr to test if it doesn't do an upgrade due to the tag missmatch.

[baburciu]

I've tried flux reconcile hr and no helm upgrade was initiated

hr reconcile (click to expand)

$ kubectl get hr monitoring -w
NAME         AGE   READY   STATUS
monitoring   3s    False   OCIRepository 'sylva-system/unit-monitoring' is not ready: latest generation of object has not been reconciled
monitoring   5s    Unknown   Running 'install' action with timeout of 5m0s
monitoring   5s    Unknown   Running 'install' action with timeout of 5m0s
monitoring   81s   True      Helm install succeeded for release cattle-monitoring-system/rancher-monitoring.v1 with chart [email protected]+079cb44d8cca
monitoring   81s   True      Helm install succeeded for release cattle-monitoring-system/rancher-monitoring.v1 with chart [email protected]+079cb44d8cca
monitoring   81s   True      Helm install succeeded for release cattle-monitoring-system/rancher-monitoring.v1 with chart [email protected]+079cb44d8cca
monitoring   81s   True      Helm install succeeded for release cattle-monitoring-system/rancher-monitoring.v1 with chart [email protected]+079cb44d8cca
monitoring   2m50s   True      Helm install succeeded for release cattle-monitoring-system/rancher-monitoring.v1 with chart [email protected]+079cb44d8cca
monitoring   2m50s   True      Helm install succeeded for release cattle-monitoring-system/rancher-monitoring.v1 with chart [email protected]+079cb44d8cca
monitoring   2m50s   True      Helm install succeeded for release cattle-monitoring-system/rancher-monitoring.v1 with chart [email protected]+079cb44d8cca
monitoring   2m50s   True      Helm install succeeded for release cattle-monitoring-system/rancher-monitoring.v1 with chart [email protected]+079cb44d8cca


# in another session
$ flux reconcile hr monitoring
► annotating HelmRelease monitoring in sylva-system namespace
✔ HelmRelease annotated
◎ waiting for HelmRelease reconciliation
✔ applied revision 104.1.2+079cb44d8cca
$ kubectl get hr monitoring -o yaml
apiVersion: helm.toolkit.fluxcd.io/v2
kind: HelmRelease
metadata:
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"helm.toolkit.fluxcd.io/v2","kind":"HelmRelease","metadata":{"annotations":{},"name":"monitoring","namespace":"sylva-system"},"spec":{"chartRef":{"kind":"OCIRepository","name":"unit-monitoring"},"interval":"15m","releaseName":"rancher-monitoring","storageNamespace":"cattle-monitoring-system","targetNamespace":"cattle-monitoring-system"}}
    reconcile.fluxcd.io/requestedAt: "2024-10-31T14:42:52.500774125Z"
  creationTimestamp: "2024-10-31T14:40:02Z"
  finalizers:
  - finalizers.fluxcd.io
  generation: 1
  name: monitoring
  namespace: sylva-system
  resourceVersion: "27103"
  uid: fe6ec1ba-0511-4844-836c-7f43097d5b95
spec:
  chartRef:
    kind: OCIRepository
    name: unit-monitoring
  interval: 15m
  releaseName: rancher-monitoring
  storageNamespace: cattle-monitoring-system
  targetNamespace: cattle-monitoring-system
status:
  conditions:
  - lastTransitionTime: "2024-10-31T14:41:23Z"
    message: Helm install succeeded for release cattle-monitoring-system/rancher-monitoring.v1
      with chart [email protected]+079cb44d8cca
    observedGeneration: 1
    reason: InstallSucceeded
    status: "True"
    type: Ready
  - lastTransitionTime: "2024-10-31T14:41:23Z"
    message: Helm install succeeded for release cattle-monitoring-system/rancher-monitoring.v1
      with chart [email protected]+079cb44d8cca
    observedGeneration: 1
    reason: InstallSucceeded
    status: "True"
    type: Released
  history:
  - appVersion: v0.72.0
    chartName: rancher-monitoring
    chartVersion: 104.1.2+079cb44d8cca
    configDigest: sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855
    digest: sha256:599a01837285776b15bb5373bf0bfede282b7a80ca845de0e35d34d3bbbd609c
    firstDeployed: "2024-10-31T14:40:07Z"
    lastDeployed: "2024-10-31T14:40:07Z"
    name: rancher-monitoring
    namespace: cattle-monitoring-system
    ociDigest: sha256:079cb44d8ccac51138c5bfaaf3840c788fd7c4a2bfab110053e92b66bb3da2e4
    status: deployed
    version: 1
  lastAttemptedConfigDigest: sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855
  lastAttemptedGeneration: 1
  lastAttemptedReleaseAction: install
  lastAttemptedRevision: 104.1.2+079cb44d8cca
  lastAttemptedRevisionDigest: sha256:079cb44d8ccac51138c5bfaaf3840c788fd7c4a2bfab110053e92b66bb3da2e4
  lastHandledReconcileAt: "2024-10-31T14:42:52.500774125Z"
  observedGeneration: 1
  storageNamespace: cattle-monitoring-system
$