Skip to content

fix: don't run Sveltos templating over Helm values on chart-identity paths - #1956

Draft
AmirAliSobhGol wants to merge 1 commit into
projectsveltos:mainfrom
AmirAliSobhGol:fix/helm-values-not-sveltos-templated-on-identity-paths
Draft

fix: don't run Sveltos templating over Helm values on chart-identity paths#1956
AmirAliSobhGol wants to merge 1 commit into
projectsveltos:mainfrom
AmirAliSobhGol:fix/helm-values-not-sveltos-templated-on-identity-paths

Conversation

@AmirAliSobhGol

@AmirAliSobhGol AmirAliSobhGol commented Sep 10, 2026

Copy link
Copy Markdown

What happens

A single ClusterProfile whose helmCharts[].values fail Sveltos template rendering — e.g. helm-style {{ .Values.x }} placeholders meant for the chart's own tpl — silently wedges helm lifecycle for the whole cluster:

  • the broken profile reports no failure (featureSummaries stays empty);
  • deleting the broken profile hangs forever (its ClusterSummary finalizer is never removed);
  • deleting any other profile on the same cluster leaves its helm release installed forever.

Steps to reproduce (v1.14.0, kind)

  1. kind create cluster --name mgmt, install Sveltos v1.14.0, register the management cluster itself (SveltosCluster mgmt/mgmt), and label it:

    kubectl label sveltoscluster -n mgmt mgmt env=repro
  2. Apply a healthy profile:

    apiVersion: config.projectsveltos.io/v1beta1
    kind: ClusterProfile
    metadata:
      name: prometheus
    spec:
      clusterSelector:
        matchLabels:
          env: repro
      helmCharts:
        - repositoryURL: https://prometheus-community.github.io/helm-charts
          repositoryName: prometheus-community
          chartName: prometheus-community/prometheus
          chartVersion: "27.x"
          releaseName: prometheus
          releaseNamespace: prometheus
          helmChartAction: Install
  3. Apply the broken profile — the values are valid for the chart but are not a valid Sveltos template (.Values does not exist in the Sveltos template context):

    apiVersion: config.projectsveltos.io/v1beta1
    kind: ClusterProfile
    metadata:
      name: fluent-bit
    spec:
      clusterSelector:
        matchLabels:
          env: repro
      helmCharts:
        - repositoryURL: https://fluent.github.io/helm-charts
          repositoryName: fluent
          chartName: fluent/fluent-bit
          chartVersion: "0.48.x"
          releaseName: fluent-bit
          releaseNamespace: fluent-bit
          helmChartAction: Install
          values: |
            config:
              service: |
                [SERVICE]
                    Flush {{ .Values.flush }}
                    Log_Level {{ .Values.logLevel }}
  4. Wait for the prometheus release to install. Note the fluent-bit ClusterSummary shows empty featureSummaries — no error is surfaced anywhere.

  5. kubectl delete clusterprofile fluent-bit
    Expected: deletion completes (or at least a visible per-chart failure).
    Actual: hangs indefinitely (30+ minutes in my run); the ClusterSummary finalizer is never removed.

  6. kubectl delete clusterprofile prometheus
    Expected: the prometheus release is uninstalled and the profile goes away.
    Actual: the release stays installed indefinitely (17+ minutes until I deployed the fix, at which point both deletions completed on their own); the addon-controller logs repeat ClusterProfile prometheus matches cluster mgmt/mgmt but ClusterSummary not fully processed yet.

Root cause

The render error (can't evaluate field Values in type *controllers.currentClusterObjects) comes from getInstantiatedChart, which runs instantiateStructFields over every field of the HelmChart, including Values. That single error wedges three paths:

  1. Registration: updateChartMapgetClusterSummaryWithInstantiatedChartsgetInstantiatedChart fails before RegisterClusterSummaryForCharts runs, and reconciliation of the ClusterSummary aborts there — featureSummaries stays empty, so there is no visible failure either.
  2. Undeploy: the undeploy path (uninstallHelmCharts, and undeployHelmChartResourcesgetClusterSummaryWithInstantiatedCharts) re-runs the same instantiation, retries the error indefinitely, and the ClusterSummary finalizer is never removed.
  3. Cross-profile blockage: because the broken profile never registers its charts, canUninstallHelmChartallMatchingProfilesProcessedisProfileFullyProcessed sees GetRegisteredChartsCount < len(HelmCharts) forever and returns WaitForProfileProcessingError on every retry — so any healthy profile deleted while the broken one exists keeps its release installed indefinitely.

But Values is not needed on any of these paths — chartManager registration and uninstall only need the chart identity (release name/namespace, repo, chart name/version). Meanwhile the paths that do consume values (install / upgrade / value-hash) already instantiate the raw Values string themselves via getHelmChartInstantiatedValues, so instantiating them in getInstantiatedChart also meant values were templated twice.

Fix

getInstantiatedChart sets Values aside before instantiateStructFields and restores the raw string afterward. Identity fields are still templated exactly as before (the existing test templating chartVersion from cluster labels still passes); values are templated exactly once, at the point of use, where a render error surfaces as a per-chart deploy failure (Helm Failed in featureSummaries) instead of wedging registration and undeploy.

Verification

  • New regression test: getInstantiatedChart leaves Values alone even when they are not a valid Sveltos template.
  • Full ./controllers/ envtest suite passes.
  • Live verification on a kind cluster running v1.14.0 with this patch applied: the two wedged profile deletions completed within minutes of rolling out the patched image, the healthy profile's release uninstalled cleanly, the broken profile reported a visible Helm Failed template error, and a full re-run of the scenario (deploy both → delete broken → delete healthy) completed without hangs.

🤖 Generated with Claude Code

…paths

getInstantiatedChart instantiates every field of a HelmChart as a Sveltos
Go template, including Values. Values that are not valid Sveltos templates
(e.g. helm-style {{ .Values.x }} placeholders meant for the chart's own tpl
rendering) made instantiation fail on paths that only need the chart
identity:

- updateChartMap: the ClusterSummary never registers its charts with the
  chartManager, so allMatchingProfilesProcessed never sees the profile as
  processed and canUninstallHelmChart returns
  WaitForProfileProcessingError forever: helm uninstall of every other
  profile on the cluster is blocked.
- uninstallHelmCharts / getClusterSummaryWithInstantiatedCharts on the
  undeploy path: undeploy retries the template error forever and the
  ClusterSummary finalizer is never removed, wedging profile deletion.

Values are already instantiated at the point of use (install/upgrade/hash)
by getHelmChartInstantiatedValues, so instantiating them here was also a
double templating. Leave Values untouched in getInstantiatedChart and let
the deploy path surface the template error as a per-chart failure instead.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@gianlucam76

Copy link
Copy Markdown
Member

Thanks. I see the error and the double instantiation of Values is also breaking this https://projectsveltos.io/main/template/additional_template_info/#embedding-go-templates-in-sveltos

If we proceed with this I suggest renaming the getInstantiatedChart to getInstantiatedChartIdentity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants