From 83f29ec318ef52c29520c77eb261ad98f99e3d08 Mon Sep 17 00:00:00 2001 From: Gianluca Mardente Date: Fri, 15 May 2026 09:15:18 +0200 Subject: [PATCH] (bug) Surface error on missing TemplateResourceRefs If a resource (non optional) referenced in TemplateResourceRefs section is missing, no error showed up in the ClusterSummary making it difficult to detect why the profile was not deployed. This PR fixes that by making sure the error is reported in the ClusterSummary Status An example ```yaml status: dependencies: no dependencies featureSummaries: - failureMessage: 'Referenced resource: Secret default/imported-secret does not exist: secrets "imported-secret" not found' featureID: Resources nextReconcileTime: "2026-05-15T07:17:30Z" ``` --- controllers/clustersummary_controller.go | 6 + controllers/export_test.go | 1 + controllers/templateresourcedef_utils.go | 7 + controllers/templateresourcedef_utils_test.go | 115 ++++++++++++ test/fv/missing_template_resource_ref_test.go | 173 ++++++++++++++++++ 5 files changed, 302 insertions(+) create mode 100644 test/fv/missing_template_resource_ref_test.go diff --git a/controllers/clustersummary_controller.go b/controllers/clustersummary_controller.go index 9816adcd..586f650b 100644 --- a/controllers/clustersummary_controller.go +++ b/controllers/clustersummary_controller.go @@ -503,6 +503,12 @@ func (r *ClusterSummaryReconciler) prepareForDeployment(ctx context.Context, err = r.updateChartMap(ctx, clusterSummaryScope, logger) if err != nil { + if apierrors.IsNotFound(err) { + // A required (non-optional) templateResourceRef is missing. Surface it as a + // failure so the operator can see why deployment is blocked. + r.setFailureMessage(clusterSummaryScope, err.Error()) + r.resetFeatureStatus(clusterSummaryScope, libsveltosv1beta1.FeatureStatusFailedNonRetriable) + } r.setNextReconcileTime(clusterSummaryScope, normalRequeueAfter) return reconcile.Result{RequeueAfter: normalRequeueAfter} } diff --git a/controllers/export_test.go b/controllers/export_test.go index d3698d7f..a9595c1c 100644 --- a/controllers/export_test.go +++ b/controllers/export_test.go @@ -225,6 +225,7 @@ var ( var ( GetTemplateResourceName = getTemplateResourceName GetTemplateResourceNamespace = getTemplateResourceNamespace + CollectTemplateResourceRefs = collectTemplateResourceRefs ) var ( diff --git a/controllers/templateresourcedef_utils.go b/controllers/templateresourcedef_utils.go index b94ef847..28868bdb 100644 --- a/controllers/templateresourcedef_utils.go +++ b/controllers/templateresourcedef_utils.go @@ -18,6 +18,7 @@ package controllers import ( "context" + "fmt" "strings" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -83,6 +84,12 @@ func collectTemplateResourceRefs(ctx context.Context, clusterSummary *configv1be if apierrors.IsNotFound(err) && ref.Optional { continue } + if apierrors.IsNotFound(err) { + // Wrap with a descriptive message so callers can surface a clear failure. + // Use %w so apierrors.IsNotFound still returns true for existing callers. + return nil, fmt.Errorf("referenced resource: %s %s/%s does not exist: %w", + ref.Resource.Kind, ref.Resource.Namespace, ref.Resource.Name, err) + } return nil, err } diff --git a/controllers/templateresourcedef_utils_test.go b/controllers/templateresourcedef_utils_test.go index 387c9912..7a8e47b7 100644 --- a/controllers/templateresourcedef_utils_test.go +++ b/controllers/templateresourcedef_utils_test.go @@ -24,6 +24,7 @@ import ( . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" @@ -240,6 +241,120 @@ var _ = Describe("TemplateResourceDef utils ", func() { }) }) +var _ = Describe("collectTemplateResourceRefs", func() { + var clusterSummary *configv1beta1.ClusterSummary + var cluster *clusterv1.Cluster + var ns *corev1.Namespace + var nsName string + + BeforeEach(func() { + var err error + scheme, err = setupScheme() + Expect(err).ToNot(HaveOccurred()) + + nsName = randomString() + ns = &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: nsName, + }, + } + Expect(testEnv.Create(context.TODO(), ns)).To(Succeed()) + Expect(waitForObject(context.TODO(), testEnv.Client, ns)).To(Succeed()) + + cluster = &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: upstreamClusterNamePrefix + randomString(), + Namespace: nsName, + }, + } + Expect(testEnv.Create(context.TODO(), cluster)).To(Succeed()) + Expect(waitForObject(context.TODO(), testEnv.Client, cluster)).To(Succeed()) + + clusterSummary = &configv1beta1.ClusterSummary{ + ObjectMeta: metav1.ObjectMeta{ + Name: randomString(), + Namespace: nsName, + }, + Spec: configv1beta1.ClusterSummarySpec{ + ClusterNamespace: nsName, + ClusterName: cluster.Name, + ClusterType: libsveltosv1beta1.ClusterTypeCapi, + }, + } + }) + + It("returns a descriptive NotFound error for a required missing resource", func() { + clusterSummary.Spec.ClusterProfileSpec.TemplateResourceRefs = []configv1beta1.TemplateResourceRef{ + { + Resource: corev1.ObjectReference{ + Kind: "ConfigMap", + APIVersion: "v1", + Namespace: nsName, + Name: "does-not-exist-" + randomString(), + }, + Identifier: "MissingResource", + Optional: false, + }, + } + + result, err := controllers.CollectTemplateResourceRefs(context.TODO(), clusterSummary) + Expect(result).To(BeNil()) + Expect(err).NotTo(BeNil()) + Expect(apierrors.IsNotFound(err)).To(BeTrue()) + Expect(err.Error()).To(ContainSubstring("referenced resource: ConfigMap")) + Expect(err.Error()).To(ContainSubstring("does not exist")) + }) + + It("skips an optional missing resource without returning an error", func() { + clusterSummary.Spec.ClusterProfileSpec.TemplateResourceRefs = []configv1beta1.TemplateResourceRef{ + { + Resource: corev1.ObjectReference{ + Kind: "ConfigMap", + APIVersion: "v1", + Namespace: nsName, + Name: "does-not-exist-" + randomString(), + }, + Identifier: "OptionalResource", + Optional: true, + }, + } + + result, err := controllers.CollectTemplateResourceRefs(context.TODO(), clusterSummary) + Expect(err).To(BeNil()) + Expect(result).To(BeEmpty()) + }) + + It("returns the resource when it exists", func() { + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: nsName, + Name: randomString(), + }, + Data: map[string]string{"key": "value"}, + } + Expect(testEnv.Create(context.TODO(), cm)).To(Succeed()) + Expect(waitForObject(context.TODO(), testEnv.Client, cm)).To(Succeed()) + + identifier := randomString() + clusterSummary.Spec.ClusterProfileSpec.TemplateResourceRefs = []configv1beta1.TemplateResourceRef{ + { + Resource: corev1.ObjectReference{ + Kind: "ConfigMap", + APIVersion: "v1", + Namespace: nsName, + Name: cm.Name, + }, + Identifier: identifier, + }, + } + + result, err := controllers.CollectTemplateResourceRefs(context.TODO(), clusterSummary) + Expect(err).To(BeNil()) + Expect(result).To(HaveKey(identifier)) + Expect(result[identifier].GetName()).To(Equal(cm.Name)) + }) +}) + var _ = Describe("extractWatchedFields", func() { It("returns only the listed top-level field", func() { u := &unstructured.Unstructured{Object: map[string]interface{}{ diff --git a/test/fv/missing_template_resource_ref_test.go b/test/fv/missing_template_resource_ref_test.go new file mode 100644 index 00000000..983458fe --- /dev/null +++ b/test/fv/missing_template_resource_ref_test.go @@ -0,0 +1,173 @@ +/* +Copyright 2026. projectsveltos.io. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package fv_test + +import ( + "context" + "fmt" + "strings" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/util/retry" + + configv1beta1 "github.com/projectsveltos/addon-controller/api/v1beta1" + "github.com/projectsveltos/addon-controller/lib/clusterops" + libsveltosv1beta1 "github.com/projectsveltos/libsveltos/api/v1beta1" +) + +var _ = Describe("Missing TemplateResourceRef", func() { + const ( + namePrefix = "missing-tmpl-ref-" + ) + + It("Reports failure when a required templateResourceRef is missing, recovers when the resource is created", + Label("FV", "PULLMODE", "EXTENDED"), func() { + + Byf("Create a ClusterProfile matching Cluster %s/%s", + kindWorkloadCluster.GetNamespace(), kindWorkloadCluster.GetName()) + clusterProfile := getClusterProfile(namePrefix, map[string]string{key: value}) + clusterProfile.Spec.SyncMode = configv1beta1.SyncModeContinuous + Expect(k8sClient.Create(context.TODO(), clusterProfile)).To(Succeed()) + + verifyClusterProfileMatches(clusterProfile) + + verifyClusterSummary(clusterops.ClusterProfileLabelName, + clusterProfile.Name, &clusterProfile.Spec, + kindWorkloadCluster.GetNamespace(), kindWorkloadCluster.GetName(), getClusterType()) + + // Create the ConfigMap that will be referenced by PolicyRefs (so FeatureResources is tracked) + configMapNs := randomString() + Byf("Create configMap namespace %s in the management cluster", configMapNs) + cmNsObj := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{Name: configMapNs}, + } + Expect(k8sClient.Create(context.TODO(), cmNsObj)).To(Succeed()) + + namespaceName := randomString() + configMap := createConfigMapWithPolicy(configMapNs, namePrefix+randomString(), + fmt.Sprintf(namespace, namespaceName)) + Byf("Creating ConfigMap %s/%s to be deployed via PolicyRefs", configMapNs, configMap.Name) + Expect(k8sClient.Create(context.TODO(), configMap)).To(Succeed()) + + // A Secret that will be referenced as a required templateResourceRef, but does not exist yet + secretNsName := randomString() + secretName := randomString() + + Byf("Updating ClusterProfile %s with templateResourceRefs pointing to missing Secret %s/%s", + clusterProfile.Name, secretNsName, secretName) + currentClusterProfile := &configv1beta1.ClusterProfile{} + err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + Expect(k8sClient.Get(context.TODO(), + types.NamespacedName{Name: clusterProfile.Name}, currentClusterProfile)).To(Succeed()) + currentClusterProfile.Spec.TemplateResourceRefs = []configv1beta1.TemplateResourceRef{ + { + Resource: corev1.ObjectReference{ + Kind: "Secret", + APIVersion: "v1", + Namespace: secretNsName, + Name: secretName, + }, + Identifier: "MySecret", + Optional: false, + }, + } + currentClusterProfile.Spec.PolicyRefs = []configv1beta1.PolicyRef{ + { + Kind: string(libsveltosv1beta1.ConfigMapReferencedResourceKind), + Namespace: configMap.Namespace, + Name: configMap.Name, + }, + } + return k8sClient.Update(context.TODO(), currentClusterProfile) + }) + Expect(err).To(BeNil()) + + Expect(k8sClient.Get(context.TODO(), + types.NamespacedName{Name: clusterProfile.Name}, currentClusterProfile)).To(Succeed()) + + clusterSummary := verifyClusterSummary(clusterops.ClusterProfileLabelName, + currentClusterProfile.Name, ¤tClusterProfile.Spec, + kindWorkloadCluster.GetNamespace(), kindWorkloadCluster.GetName(), getClusterType()) + + By("Verify ClusterSummary reports failure for missing required templateResourceRef") + Eventually(func() bool { + currentClusterSummary := &configv1beta1.ClusterSummary{} + err := k8sClient.Get(context.TODO(), + types.NamespacedName{Namespace: clusterSummary.Namespace, Name: clusterSummary.Name}, + currentClusterSummary) + if err != nil { + return false + } + for i := range currentClusterSummary.Status.FeatureSummaries { + fs := currentClusterSummary.Status.FeatureSummaries[i] + if fs.FeatureID == libsveltosv1beta1.FeatureResources && + fs.Status == libsveltosv1beta1.FeatureStatusFailedNonRetriable && + fs.FailureMessage != nil && + strings.Contains(*fs.FailureMessage, secretName) { + + return true + } + } + return false + }, timeout, pollingInterval).Should(BeTrue()) + + // Create the namespace and Secret so the templateResourceRef can be resolved + Byf("Creating namespace %s in the management cluster for the Secret", secretNsName) + secretNs := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{Name: secretNsName}, + } + Expect(k8sClient.Create(context.TODO(), secretNs)).To(Succeed()) + + Byf("Creating Secret %s/%s in the management cluster", secretNsName, secretName) + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: secretNsName, + Name: secretName, + }, + Data: map[string][]byte{"key": []byte("value")}, + } + Expect(k8sClient.Create(context.TODO(), secret)).To(Succeed()) + + Byf("Getting client to access the workload cluster") + workloadClient, err := getKindWorkloadClusterKubeconfig() + Expect(err).To(BeNil()) + Expect(workloadClient).ToNot(BeNil()) + + Byf("Verifying Namespace %s is created in the workload cluster after Secret is available", namespaceName) + Eventually(func() error { + currentNamespace := &corev1.Namespace{} + return workloadClient.Get(context.TODO(), types.NamespacedName{Name: namespaceName}, currentNamespace) + }, timeout, pollingInterval).Should(BeNil()) + + Byf("Verifying ClusterSummary %s status is set to Provisioned for Resources feature", clusterSummary.Name) + verifyFeatureStatusIsProvisioned(kindWorkloadCluster.GetNamespace(), clusterSummary.Name, libsveltosv1beta1.FeatureResources) + + deleteClusterProfile(clusterProfile) + + Byf("Cleaning up namespaces created in the management cluster") + currentNs := &corev1.Namespace{} + Expect(k8sClient.Get(context.TODO(), types.NamespacedName{Name: configMapNs}, currentNs)).To(Succeed()) + Expect(k8sClient.Delete(context.TODO(), currentNs)).To(Succeed()) + Expect(k8sClient.Get(context.TODO(), types.NamespacedName{Name: secretNsName}, currentNs)).To(Succeed()) + Expect(k8sClient.Delete(context.TODO(), currentNs)).To(Succeed()) + }) +})