HelmRelease: CRDs of disabled subcharts get installed anyway #938

MBrouns · 2024-04-12T10:35:11Z

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

install flux using the CLI
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

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

The text was updated successfully, but these errors were encountered:

stefanprodan · 2024-04-12T11:03:45Z

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

stefanprodan · 2024-04-12T11:13:48Z

@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 · 2024-04-12T11:20:27Z

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 · 2024-04-12T11:25:11Z

What version of the Helm CLI are you using?

MBrouns · 2024-04-12T11:30:04Z

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

stefanprodan · 2024-04-12T12:21:38Z

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 · 2024-04-16T06:12:42Z

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 · 2024-04-16T16:14:44Z

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 · 2024-06-14T13:25:31Z

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 · 2024-11-20T19:42:18Z

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.

eric-carlsson · 2024-12-04T09:23:44Z

Hi,

I encountered the same problem when installing grafana/loki: CRDs from the grafana-agent-operator subchart are installed despite the corresponding condition monitoring.selfMonitoring.grafanaAgent.installOperator being set to false. For now the workaround described above, setting spec.install.crds: Skip, works, but it's not ideal.

I can also confirm that this is not an issue when I install the same chart using the same values with the Helm CLI (version.BuildInfo{Version:"v3.16.3", GitCommit:"v3.16.3", GitTreeState:"", GoVersion:"go1.23.3"}), it seems to be local to Flux.

stefanprodan transferred this issue from fluxcd/flux2 Apr 12, 2024

stefanprodan added the blocked/upstream Blocked by an upstream dependency or issue label Apr 12, 2024

MBrouns mentioned this issue Apr 16, 2024

WIP: don't install CRDs of disabled subcharts #941

Open

matheuscscp linked a pull request Dec 10, 2024 that will close this issue

Fix install and upgrade applying subchart CRDs when condition is false #1123

Open

matheuscscp self-assigned this Dec 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HelmRelease: CRDs of disabled subcharts get installed anyway #938

HelmRelease: CRDs of disabled subcharts get installed anyway #938

MBrouns commented Apr 12, 2024 •

edited

Loading

stefanprodan commented Apr 12, 2024 •

edited

Loading

stefanprodan commented Apr 12, 2024

MBrouns commented Apr 12, 2024

stefanprodan commented Apr 12, 2024

MBrouns commented Apr 12, 2024

stefanprodan commented Apr 12, 2024

MBrouns commented Apr 16, 2024 •

edited

Loading

MBrouns commented Apr 16, 2024

artem-nefedov commented Jun 14, 2024 •

edited

Loading

maximveksler commented Nov 20, 2024

eric-carlsson commented Dec 4, 2024 •

edited

Loading

HelmRelease: CRDs of disabled subcharts get installed anyway #938

HelmRelease: CRDs of disabled subcharts get installed anyway #938

Comments

MBrouns commented Apr 12, 2024 • edited Loading

Describe the bug

Steps to reproduce

Expected behavior

Screenshots and recordings

OS / Distro

Flux version

Flux check

Git provider

Container Registry provider

Additional context

Code of Conduct

stefanprodan commented Apr 12, 2024 • edited Loading

stefanprodan commented Apr 12, 2024

MBrouns commented Apr 12, 2024

stefanprodan commented Apr 12, 2024

MBrouns commented Apr 12, 2024

stefanprodan commented Apr 12, 2024

MBrouns commented Apr 16, 2024 • edited Loading

MBrouns commented Apr 16, 2024

artem-nefedov commented Jun 14, 2024 • edited Loading

maximveksler commented Nov 20, 2024

eric-carlsson commented Dec 4, 2024 • edited Loading

MBrouns commented Apr 12, 2024 •

edited

Loading

stefanprodan commented Apr 12, 2024 •

edited

Loading

MBrouns commented Apr 16, 2024 •

edited

Loading

artem-nefedov commented Jun 14, 2024 •

edited

Loading

eric-carlsson commented Dec 4, 2024 •

edited

Loading