Deleting a default key for helmrelease (setting to null)

[messiahUA]

Helm supports unsetting/deleting the default chart value: https://helm.sh/docs/chart_template_guide/values_files/#deleting-a-default-key

It works as expected when you set some value to null when HelmRelease is created the first time, for example you set cpu limit to null and the end result is:

apiVersion: helm.toolkit.fluxcd.io/v2beta1
kind: HelmRelease
[...]
spec:
  values:
    resources:
      limits:
        cpu: null
        memory: 128Mi

but if you update the HelmRelease and set some other value to null, for example memory limit, it ends up as:

apiVersion: helm.toolkit.fluxcd.io/v2beta1
kind: HelmRelease
[...]
spec:
  values:
    resources:
      limits:
        cpu: null

Thus, it doesn't propagate to helm and has no effect at all. I hope this bug is not inherent to the kubectl/k8s itself and can be fixed in kustomize-controller.

[stefanprodan]

I guess this is what kustomize build does too, in Flux we don’t manipulate yaml directly, we use the kustomize/api package to build the yamls in the same way the kustomize CLI does it.

[messiahUA]

@stefanprodan I've just tested by running kustomize build locally - it produces the value with null correctly and as I've mentioned it works as expected when the resource is created the first time. The problem occurs only on update.

Actually, I've just tested this manually by executing kubectl apply in the same order (result is the same), so this is related to kubectl and how it applies the change...

[stefanprodan]

so this is related to kubectl and how it applies the change

I don't see how we could do something about this, we exec to run kubectl apply and it's up to kubectl to do the 3-way-merge.

[messiahUA]

kubectl apply --server-side=true works correctly in my simple test, but it causes a conflict with helm-controller managed field.

[kingdonb]

Should this be moved to Helm Controller? I'm not sure it is a Kustomize Controller issue.

[messiahUA]

I think that this is indeed not a kustomize controller issue and not even a fluxcd issue, but if there are any ideas on the solution then can be moved to helm controller for sure.

In any case the good workaround is to just move the values to configmap and reference it via valuesFrom.

[acondrat]

I noticed that unsetting a default chart value does now work when using valuesFiles.

      valuesFiles:
        - charts/mychart/values.yaml
        - charts/mychart/overrides/test.yaml

A regular helm upgrade --install mychart . -f overrides/test.yaml deletes the key.

The doc says the following:

Values files are merged in the order of the list with the last file overriding the first. It is ignored when omitted. When values files are specified, the chart is fetched and packaged with the provided values.

Does it mean that Flux does not handle this special null value as Helm describes in deleting-a-default-key?

[gberche-orange]

Does it mean that Flux does not handle this special null value as Helm describes in deleting-a-default-key?

I'm reproducing the same problem with https://github.com/crossplane/crossplane/blob/969b8ebdf2aedc1849302b687f2eebeba94b55f3/cluster/charts/crossplane/values.yaml.tmpl#L65-L69

whose value.yml has

securityContextCrossplane:
  runAsUser: 65532
  runAsGroup: 65532
  allowPrivilegeEscalation: false
  readOnlyRootFilesystem: true

A strategic patch with the following

spec:
  values:
    #see https://github.com/jeremycaine/crossplane-with-openshift/blob/main/1.%20Installing%20Crossplane%20on%20Openshift/README.md#install-crossplane
    securityContextCrossplane:
      runAsUser: null
      runAsGroup: null
    securityContextRBACManager:
      runAsUser: null
      runAsGroup: null 

when trying to set a null runAsUser in the value gets ignored, and results into the HelmRelease value field as

    securityContextCrossplane: {}
    securityContextRBACManager: {}

As a result the default chart values are assigned by flux helm release

       securityContext:
          allowPrivilegeEscalation: false
          readOnlyRootFilesystem: true
          runAsGroup: 65532
          runAsUser: 65532

Trying to assign an empty map {} in the value, simply gets ignored by the strategic merge.

My workaround is to use helm post rendered to correct incorrect helm values in the resulting helm rendered templates, but this breaks the helm chart encapsulation.

[stgrace]

I ran into the same issue as @gberche-orange, with a very similar usecase. I noticed that when using flux as intended via git and using a kustomization, it does indeed delete the null value, but when applying an hr directly using kubectl apply, the null values are not deleted and helm install works as intended with the deletion of the default keys.

[stgrace]

Seems like it is a kustomize issue, not sure if there is much flux can do about it: kubernetes-sigs/kustomize#4628

[balonik]

I see this was fixed in Kustomize 5.x, but since Flux is still using the 4.x version the bug is still present.

[kingdonb]

The upgrade to Kustomize 5 includes permanent and final deprecation of patchesStrategicMerge and patchesJson6902 - there's a list of breaking changes in the release notes here, and a separate section for deprecations as well:

https://github.com/kubernetes-sigs/kustomize/releases/tag/kustomize%2Fv5.0.0

This should hopefully become normalized for the Flux 2.0.0 final release, I think it hasn't made it into any release candidate yet. It looks like these changes should harmonize to settle on patches that Flux has been promoting in our docs more for a while. I think many of those changes have already landed in Flux 2.0.0-rc.1, the API changes in particular deprecating other patch methods besides patches.

It's not clear from the release notes if Kustomize 5 is landing in 2.0.0, or if it may land yet in the next 2.0.0-rc.2. Let's stay tuned. I think it's good news, if Kustomize 5 fixes this 🎉 I think the breaking changes from upstream will likely come aligned with our own major bump, but I'm not the one doing the work so don't want to prognosticate too strongly about it.

[hiddeco]

Kustomize 5.x is planned before landing v2.0.0. Hopes this answers (part) of the question.

[amit-disc]

Is it already not upgraded in v1.0.0 according to the changelog https://github.com/fluxcd/kustomize-controller/blob/v1.0.0/CHANGELOG.md?

[kingdonb]

Yes, Kustomize v5 has been introduced to Flux since Flux 2.0.0 and Kustomize Controller 1.0.0, and we're up to Kustomize 5.2 as of Flux 2.2 (which comes with some more breaking fixes)

This issue should be resolved, if I understood the thread correctly reviewing it just now in summary? (Can anyone confirm to close it?)

[dulacp]

@kingdonb, it doesn't seem fixed to me, I'm still experiencing this issue on HelmRelease upgrades with these versions:

› flux version
flux: v2.2.3
distribution: flux-v2.2.3
helm-controller: v0.37.4
kustomize-controller: v1.2.2
notification-controller: v1.2.4
source-controller: v1.2.4

[NicoJDE]

same here

[tuxillo]

Is this still the case?