HelmRelease: CRDs of disabled subcharts get installed anyway

[MBrouns]

Describe the bug

Assume a helm chart with the following structure

.
├── Chart.yaml
├── charts
│   └── subchart
│       ├── Chart.yaml
│       ├── crds
│       │   └── crd.yaml
│       ├── templates
│       │   └── subchartpod.yaml
│       └── values.yaml
├── templates
│   └── mainchartpod.yaml
└── values.yaml

where the mainchart's Chart.yaml says:

dependencies:
  - name: subchart
    condition: subchart.enabled

When I create a helm release with the following details (specifically note that the subchart is not enabled)

apiVersion: helm.toolkit.fluxcd.io/v2beta2
kind: HelmRelease
metadata:
  name: fluxcd-test
  namespace: default
spec:
  interval: 10m
  timeout: 5m
  chart:
    spec:
      chart: mainchart
      version: "0.1.0"
      sourceRef:
        kind: GitRepository
        name: fluxcd-subchart-crd-test
      interval: 5m
  releaseName: crd-test
  values:
    replicaCount: 1
    subchart:
      enabled: false

I still see the crd from the subchart created in the cluster. This is only the case for the crd in the subchart, not the resources defined in the subchartpod.yaml. This is not what I expect, and also not what happens when installing the helm chart directly into the cluster

I have tested this for all combinations of Kubernetes 1.26.8, 1.27.8 and 1.28.3 and flux 2.2.0 and 2.2.3

Running helm install test . --set 'subchart.enabled=false' directly does not install the subcharts crd

Steps to reproduce

1. install flux using the CLI
2. apply the following manifest to your cluster

apiVersion: source.toolkit.fluxcd.io/v1
kind: GitRepository
metadata:
  name: fluxcd-subchart-crd-test
  namespace: default
spec:
  interval: 5m0s
  url: https://github.com/mbrouns/fluxcd-subchart-crd-test
  ref:
    branch: main
---
apiVersion: helm.toolkit.fluxcd.io/v2beta2
kind: HelmRelease
metadata:
  name: fluxcd-test
  namespace: default
spec:
  interval: 10m
  timeout: 5m
  chart:
    spec:
      chart: mainchart
      version: "0.1.0"
      sourceRef:
        kind: GitRepository
        name: fluxcd-subchart-crd-test
      interval: 5m
  releaseName: crd-test
  values:
    replicaCount: 1
    subchart:
      enabled: false

3. run kubectl get crd and observe the crd called crontabs.stable.example.com being created

Expected behavior

I expect no crd to be created for disabled helm subcharts

Screenshots and recordings

No response

OS / Distro

MacOS running Minikube, but also on ranchers K8s

Flux version

v2.2.3

Flux check

► checking prerequisites
✔ Kubernetes 1.28.3 >=1.26.0-0
► checking version in cluster
✔ distribution: flux-v2.2.3
✔ bootstrapped: false
► checking controllers
✔ helm-controller: deployment ready
► ghcr.io/fluxcd/helm-controller:v0.37.4
✔ kustomize-controller: deployment ready
► ghcr.io/fluxcd/kustomize-controller:v1.2.2
✔ notification-controller: deployment ready
► ghcr.io/fluxcd/notification-controller:v1.2.4
✔ source-controller: deployment ready
► ghcr.io/fluxcd/source-controller:v1.2.4
► checking crds
✔ alerts.notification.toolkit.fluxcd.io/v1beta3
✔ buckets.source.toolkit.fluxcd.io/v1beta2
✔ gitrepositories.source.toolkit.fluxcd.io/v1
✔ helmcharts.source.toolkit.fluxcd.io/v1beta2
✔ helmreleases.helm.toolkit.fluxcd.io/v2beta2
✔ helmrepositories.source.toolkit.fluxcd.io/v1beta2
✔ kustomizations.kustomize.toolkit.fluxcd.io/v1
✔ ocirepositories.source.toolkit.fluxcd.io/v1beta2
✔ providers.notification.toolkit.fluxcd.io/v1beta3
✔ receivers.notification.toolkit.fluxcd.io/v1
✔ all checks passed

Git provider

GitHub

Container Registry provider

N/A

Additional context

If anyone's able to point me in the right direction I'd be happy to help fixing this

Code of Conduct

• I agree to follow this project's Code of Conduct

[stefanprodan]

Ok turns out this is a bug upstream in Helm and never fixed helm/helm#8508

[stefanprodan]

@MBrouns if your main chart contains no CRDs, you could skip all CRDs with:

apiVersion: helm.toolkit.fluxcd.io/v2beta2
kind: HelmRelease
spec:
  install:
    crds: Skip
  upgrade:
    crds: Skip

Besides this there is no way we can fix this issue in Flux. In Flux we use the Helm SDK to get the CRDs and the SDK returns all CRDs from all sub-charts before values are compiled.

[MBrouns]

Thanks for that lightning-quick response! Unfortunately we do enable other subcharts in the mainchart that have CRDs, so I don't think that'll fix it for us.

The weird thing is that if I do a helm install directly of the same manifests, the CRD is not installed. Only when I install the chart through flux it is. Are you suggesting there are differences between the helm cli and helm sdk?

[stefanprodan]

What version of the Helm CLI are you using?

[MBrouns]

version.BuildInfo{Version:"v3.13.3", GitCommit:"c8b948945e52abba22ff885446a1486cb5fd3474", GitTreeState:"clean", GoVersion:"go1.20.11"}

[stefanprodan]

I don't see how the Helm CLI can do anything different, both the controller and the CLI call this code where all CRDs are merged from all sub-charts and applied:

https://github.com/helm/helm/blob/14d0c13e9eefff5b4a1b511cf50643529692ec94/pkg/action/install.go#L254-L263

[MBrouns]

I agree that it's weird, but it does seem to be what's happening. Running helm install test . --set 'subchart.enabled=false' results in the CRD not being installed for the chart defined in https://github.com/MBrouns/fluxcd-subchart-crd-test/tree/main/mainchart

Let me preface this by saying I'm not very familiar with helm's codebase, but I've done some digging to understand the issue better. I can confirm that when I put a breakpoint here https://github.com/helm/helm/blob/14d0c13e9eefff5b4a1b511cf50643529692ec94/pkg/action/install.go#L256 the length of chrt.CRDObjects() is zero if the subchart is disabled and one if it is enabled.

The crd is initially in the chrt.CRDObjects() at the start of the RunWithContext but gets removed in the call to

if err := chartutil.ProcessDependenciesWithMerge(chrt, vals); err != nil {
    return nil, err
}

As far as I can tell though, Flux calls the same RunWithContext here:

helm-controller/internal/action/install.go

Line 62 in 9059e7f

return install.RunWithContext(ctx, chrt, vals.AsMap())

, but the lines above it (

helm-controller/internal/action/install.go

Lines 58 to 60 in 9059e7f

	if err := applyCRDs(config, policy, chrt, setOriginVisitor(v2.GroupVersion.Group, obj.Namespace, obj.Name)); err != nil {
	return nil, fmt.Errorf("failed to apply CustomResourceDefinitions: %w", err)
	}

) do mention something about installing CRDs anyway. Isn't that possibly where the issue is?

[MBrouns]

I looked into it a bit further and took the very blunt approach of calling helmchartutil.ProcessDependenciesWithMerge before applyCRDs is called and that does result in the crd of the subchart not being installed.

Can you confirm whether this makes sense? If so, I'll make the linked PR proper and add stuff like test cases

[artem-nefedov]

Got bitten by this as well with splunk-otel-collector chart.
@stefanprodan Helm cli does not reproduce this problem (can be tested with helm template --include-crds command), so this is purely a flux helm-controller issue.

A workaround with spec.install.crds=Skip helped at least.

[maximveksler]

This impacted us as well, victoria metrics operator has dual approach for CRD management where by default it attempts to install CRD's as templated resources owned by the chart. This caused the HelmRelease to fail since CRD objects already exist but not managed by Helm. A workaround is to turn on crds.plain = true for this specific controller however this problem seems to be general in Flux's handling of CRD on install & upgrade.