diff --git a/controllers/clustersummary_controller.go b/controllers/clustersummary_controller.go index bbc33e01..662de454 100644 --- a/controllers/clustersummary_controller.go +++ b/controllers/clustersummary_controller.go @@ -2249,7 +2249,7 @@ func getClusterSummaryWithInstantiatedCharts(ctx context.Context, cs *configv1be for i := range cs.Spec.ClusterProfileSpec.HelmCharts { helmChart := &cs.Spec.ClusterProfileSpec.HelmCharts[i] - instantiateHelmChart, err := getInstantiatedChart(ctx, innerDCtx, helmChart, logger) + instantiateHelmChart, err := getInstantiatedChartIdentity(ctx, innerDCtx, helmChart, logger) if err != nil { return nil, err } diff --git a/controllers/export_test.go b/controllers/export_test.go index 6adf2b97..2e828d3a 100644 --- a/controllers/export_test.go +++ b/controllers/export_test.go @@ -157,7 +157,7 @@ var ( UpdateValueHashOnHelmChartSummary = updateValueHashOnHelmChartSummary DesiredValuesAreSubset = desiredValuesAreSubset GetCredentialsAndCAFiles = getCredentialsAndCAFiles - GetInstantiatedChart = getInstantiatedChart + GetInstantiatedChartIdentity = getInstantiatedChartIdentity GetHelmChartValuesFrom = getHelmChartValuesFrom GetHelmChartInstantiatedValues = getHelmChartInstantiatedValues LocateChartWithTimeout = locateChartWithTimeout diff --git a/controllers/handlers_helm.go b/controllers/handlers_helm.go index b15a4cf2..faa83731 100644 --- a/controllers/handlers_helm.go +++ b/controllers/handlers_helm.go @@ -514,7 +514,7 @@ func walkAndUndeployHelmChartsInPullMode(ctx context.Context, c client.Client, c for i := range clusterSummary.Spec.ClusterProfileSpec.HelmCharts { currentChart := &clusterSummary.Spec.ClusterProfileSpec.HelmCharts[i] - instantiatedChart, err := getInstantiatedChart(ctx, dCtx, currentChart, logger) + instantiatedChart, err := getInstantiatedChartIdentity(ctx, dCtx, currentChart, logger) if err != nil { return err } @@ -966,7 +966,7 @@ func uninstallHelmCharts(ctx context.Context, c client.Client, clusterSummary *c for i := range clusterSummary.Spec.ClusterProfileSpec.HelmCharts { currentChart := &clusterSummary.Spec.ClusterProfileSpec.HelmCharts[i] - instantiatedChart, err := getInstantiatedChart(ctx, dCtx, currentChart, logger) + instantiatedChart, err := getInstantiatedChartIdentity(ctx, dCtx, currentChart, logger) if err != nil { return nil, err } @@ -1271,7 +1271,7 @@ func deploySingleChart(ctx context.Context, c client.Client, dCtx *deploymentCon currentChart *configv1beta1.HelmChart, kubeconfig string, isPullMode bool, logger logr.Logger) (*chartStepResult, error) { - instantiatedChart, err := getInstantiatedChart(ctx, dCtx, currentChart, logger) + instantiatedChart, err := getInstantiatedChartIdentity(ctx, dCtx, currentChart, logger) if err != nil { return nil, err } @@ -3219,7 +3219,7 @@ func buildReferencedHelmReleaseSummaries(ctx context.Context, c client.Client, for i := range currentClusterSummary.Spec.ClusterProfileSpec.HelmCharts { currentChart := ¤tClusterSummary.Spec.ClusterProfileSpec.HelmCharts[i] - instantiatedChart, err := getInstantiatedChart(ctx, dCtx, currentChart, logger) + instantiatedChart, err := getInstantiatedChartIdentity(ctx, dCtx, currentChart, logger) if err != nil { return nil, false, err } @@ -3349,7 +3349,7 @@ func updateStatusForNonReferencedHelmReleases(ctx context.Context, c client.Clie for i := range dCtx.clusterSummary.Spec.ClusterProfileSpec.HelmCharts { currentChart := dCtx.clusterSummary.Spec.ClusterProfileSpec.HelmCharts[i] - instantiatedChart, err := getInstantiatedChart(ctx, dCtx, ¤tChart, logger) + instantiatedChart, err := getInstantiatedChartIdentity(ctx, dCtx, ¤tChart, logger) if err != nil { return dCtx.clusterSummary, err } @@ -3593,7 +3593,7 @@ func collectResourcesFromManagedHelmChartsForDriftDetection(ctx context.Context, for i := range clusterSummary.Spec.ClusterProfileSpec.HelmCharts { currentChart := &clusterSummary.Spec.ClusterProfileSpec.HelmCharts[i] - instantiatedChart, err := getInstantiatedChart(ctx, innerDCtx, currentChart, logger) + instantiatedChart, err := getInstantiatedChartIdentity(ctx, innerDCtx, currentChart, logger) if err != nil { return nil, err } @@ -5423,12 +5423,22 @@ func generateReportForSameVersion(ctx context.Context, currentValues map[string] return report, nil } -func getInstantiatedChart(ctx context.Context, dCtx *deploymentContext, +func getInstantiatedChartIdentity(ctx context.Context, dCtx *deploymentContext, currentChart *configv1beta1.HelmChart, logger logr.Logger) (*configv1beta1.HelmChart, error) { // Create a deep copy of the chart to avoid modifying the original. instantiatedChart := currentChart.DeepCopy() + // Values are deliberately left out of the instantiation below: they are templated by + // getHelmChartInstantiatedValues at the point of use (install/upgrade/hash). This method + // is also called on paths that only need the chart identity (chartManager registration, + // uninstall): failing those paths on values that are not valid Sveltos templates (e.g. + // helm-style {{ .Values.x }} placeholders meant for the chart's own tpl rendering) would + // permanently wedge undeploy of every chart in the profile and, through + // allMatchingProfilesProcessed, block helm uninstalls of other profiles on the cluster. + values := instantiatedChart.Values + instantiatedChart.Values = "" + // Call the new recursive helper function to instantiate all fields. if err := instantiateStructFields(ctx, getManagementClusterConfig(), getManagementClusterClient(), instantiatedChart, dCtx.clusterSummary, dCtx.clusterObjects, dCtx.mgmtResources, logger); err != nil { @@ -5437,6 +5447,8 @@ func getInstantiatedChart(ctx context.Context, dCtx *deploymentContext, return nil, &configv1beta1.TemplateInstantiationError{Message: msg} } + instantiatedChart.Values = values + return instantiatedChart, nil } @@ -5927,7 +5939,7 @@ func getStaleReleases(ctx context.Context, c client.Client, clusterSummary *conf currentlyReferencedReleases := make(map[string]bool) for i := range clusterSummary.Spec.ClusterProfileSpec.HelmCharts { currentChart := &clusterSummary.Spec.ClusterProfileSpec.HelmCharts[i] - instantiatedChart, err := getInstantiatedChart(ctx, dCtx, currentChart, logger) + instantiatedChart, err := getInstantiatedChartIdentity(ctx, dCtx, currentChart, logger) if err != nil { return nil, err } diff --git a/controllers/handlers_helm_test.go b/controllers/handlers_helm_test.go index a56a54e6..a7af03f6 100644 --- a/controllers/handlers_helm_test.go +++ b/controllers/handlers_helm_test.go @@ -1236,7 +1236,7 @@ var _ = Describe("HandlersHelm", func() { Expect(report.ReleaseNamespace).To(Equal(helmChart.ReleaseNamespace)) }) - It("getInstantiatedChart returns instantiated HelmChart matching passed in chart", func() { + It("getInstantiatedChartIdentity returns instantiated HelmChart matching passed in chart", func() { helmChart := &configv1beta1.HelmChart{ ReleaseName: randomString(), ReleaseNamespace: randomString(), ChartName: randomString(), ChartVersion: randomString(), @@ -1265,7 +1265,7 @@ var _ = Describe("HandlersHelm", func() { textlogger.NewLogger(textlogger.NewConfig())) Expect(err).To(BeNil()) - instaniatedChart, err := controllers.GetInstantiatedChart(context.TODO(), + instaniatedChart, err := controllers.GetInstantiatedChartIdentity(context.TODO(), controllers.NewDeploymentContext(clusterSummary, clusterObjects, nil), helmChart, textlogger.NewLogger(textlogger.NewConfig())) Expect(err).To(BeNil()) @@ -1278,7 +1278,7 @@ var _ = Describe("HandlersHelm", func() { Expect(instaniatedChart.ChartVersion).To(Equal(helmChart.ChartVersion)) }) - It("getInstantiatedChart returns instantiated HelmChart", func() { + It("getInstantiatedChartIdentity returns instantiated HelmChart", func() { helmChart := &configv1beta1.HelmChart{ ReleaseName: randomString(), ReleaseNamespace: randomString(), ChartName: randomString(), RepositoryURL: randomString(), @@ -1316,7 +1316,7 @@ var _ = Describe("HandlersHelm", func() { textlogger.NewLogger(textlogger.NewConfig())) Expect(err).To(BeNil()) - instaniatedChart, err := controllers.GetInstantiatedChart(context.TODO(), + instaniatedChart, err := controllers.GetInstantiatedChartIdentity(context.TODO(), controllers.NewDeploymentContext(clusterSummary, clusterObjects, nil), helmChart, textlogger.NewLogger(textlogger.NewConfig())) Expect(err).To(BeNil()) @@ -1329,6 +1329,52 @@ var _ = Describe("HandlersHelm", func() { Expect(instaniatedChart.ChartVersion).To(Equal("25.0.2")) }) + It("getInstantiatedChartIdentity leaves Values alone even when they are not a valid Sveltos template", func() { + helmChart := &configv1beta1.HelmChart{ + ReleaseName: randomString(), ReleaseNamespace: randomString(), + ChartName: randomString(), ChartVersion: randomString(), + RepositoryURL: randomString(), RepositoryName: randomString(), + HelmChartAction: configv1beta1.HelmChartActionInstall, + // Helm-style placeholders meant for the chart's own tpl rendering. They are not + // valid Sveltos templates (.Values does not exist in the Sveltos template context): + // instantiating them here used to fail and wedge chart registration and uninstall. + Values: `config: + service: | + [SERVICE] + Flush {{ .Values.flush }} + Log_Level {{ .Values.logLevel }}`, + } + + clusterSummary.Namespace = defaultNamespace + clusterSummary.Spec.ClusterNamespace = defaultNamespace + + cluster := &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterSummary.Spec.ClusterName, + Namespace: clusterSummary.Spec.ClusterNamespace, + }, + } + + Expect(testEnv.Create(context.TODO(), cluster)).To(Succeed()) + Expect(waitForObject(context.TODO(), testEnv.Client, cluster)).To(Succeed()) + + Expect(testEnv.Create(context.TODO(), clusterSummary)).To(Succeed()) + Expect(waitForObject(context.TODO(), testEnv.Client, clusterSummary)).To(Succeed()) + + clusterObjects, err := controllers.FetchClusterObjects(context.TODO(), testEnv.Config, testEnv.Client, + clusterSummary.Spec.ClusterNamespace, clusterSummary.Spec.ClusterName, libsveltosv1beta1.ClusterTypeCapi, + textlogger.NewLogger(textlogger.NewConfig())) + Expect(err).To(BeNil()) + + instaniatedChart, err := controllers.GetInstantiatedChartIdentity(context.TODO(), + controllers.NewDeploymentContext(clusterSummary, clusterObjects, nil), helmChart, + textlogger.NewLogger(textlogger.NewConfig())) + Expect(err).To(BeNil()) + Expect(instaniatedChart.Values).To(Equal(helmChart.Values)) + Expect(instaniatedChart.ReleaseName).To(Equal(helmChart.ReleaseName)) + Expect(instaniatedChart.ReleaseNamespace).To(Equal(helmChart.ReleaseNamespace)) + }) + It("updateClusterReportWithHelmReports updates ClusterReports with HelmReports", func() { helmChart := &configv1beta1.HelmChart{ ReleaseName: randomString(), ReleaseNamespace: randomString(), diff --git a/controllers/init_container_work.go b/controllers/init_container_work.go index 9cc0553d..895d544f 100644 --- a/controllers/init_container_work.go +++ b/controllers/init_container_work.go @@ -141,7 +141,7 @@ func updateClusterSummaryHelmHashes(ctx context.Context, directClient client.Cli clusterObjects: clusterObjects, mgmtResources: mgmtResources, } - instantiatedChart, err := getInstantiatedChart(ctx, innerDCtx, helmChart, logger) + instantiatedChart, err := getInstantiatedChartIdentity(ctx, innerDCtx, helmChart, logger) if err != nil { logger.V(logs.LogInfo).Error(err, "failed to get instantiated chart") return err