From 51eec73ba86ecd9c4abbc6092422f640b1712452 Mon Sep 17 00:00:00 2001 From: Will Kutler Date: Wed, 16 Nov 2022 14:31:37 -0500 Subject: [PATCH] emit pending on mapping err Signed-off-by: Will Kutler --- controllers/templatesync/template_sync.go | 20 ++++------- test/e2e/case12_ordering_test.go | 21 +++++++++++ .../case12-plc-invalid-dep.yaml | 36 +++++++++++++++++++ 3 files changed, 63 insertions(+), 14 deletions(-) create mode 100644 test/resources/case12_ordering/case12-plc-invalid-dep.yaml diff --git a/controllers/templatesync/template_sync.go b/controllers/templatesync/template_sync.go index 095441af..9a337fad 100644 --- a/controllers/templatesync/template_sync.go +++ b/controllers/templatesync/template_sync.go @@ -264,17 +264,7 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request reconcile.Requ } } - dependencyFailures, depMappingErr := r.processDependencies(ctx, dClient, rMapper, templateDeps, tLogger) - - // skip template if there is a dependency mapping error - if depMappingErr != nil { - resultError = err - errMsg := fmt.Sprintf("Mapping not found, please check if you have CRD deployed: %s", err) - - r.emitTemplateError(instance, tIndex, tName, errMsg) - - continue - } + dependencyFailures := r.processDependencies(ctx, dClient, rMapper, templateDeps, tLogger) // fetch resource res := dClient.Resource(rsrc).Namespace(instance.GetNamespace()) @@ -492,7 +482,7 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request reconcile.Requ // processDependencies iterates through all dependencies of a template and returns an array of any that are not met func (r *PolicyReconciler) processDependencies(ctx context.Context, dClient dynamic.Interface, rMapper meta.RESTMapper, templateDeps map[depclient.ObjectIdentifier]string, tLogger logr.Logger, -) ([]depclient.ObjectIdentifier, error) { +) []depclient.ObjectIdentifier { var dependencyFailures []depclient.ObjectIdentifier for dep := range templateDeps { @@ -515,7 +505,9 @@ func (r *PolicyReconciler) processDependencies(ctx context.Context, dClient dyna "kind", depGvk.Kind, ) - return nil, err + dependencyFailures = append(dependencyFailures, dep) + + continue } // set up namespace for replicated policy dependencies @@ -546,7 +538,7 @@ func (r *PolicyReconciler) processDependencies(ctx context.Context, dClient dyna } } - return dependencyFailures, nil + return dependencyFailures } // generatePendingErr formats the list of failed dependencies into a readable error diff --git a/test/e2e/case12_ordering_test.go b/test/e2e/case12_ordering_test.go index d03006cd..a69773ec 100644 --- a/test/e2e/case12_ordering_test.go +++ b/test/e2e/case12_ordering_test.go @@ -13,6 +13,8 @@ import ( const ( case12PolicyName string = "case12-test-policy" case12PolicyYaml string = "../resources/case12_ordering/case12-plc.yaml" + case12PolicyNameInvalid string = "case12-test-policy-invalid" + case12PolicyYamlInvalid string = "../resources/case12_ordering/case12-plc-invalid-dep.yaml" case12ExtraDepsPolicyName string = "case12-test-policy-multi" case12ExtraDepsPolicyYaml string = "../resources/case12_ordering/case12-plc-multiple-deps.yaml" case12Plc2TemplatesName string = "case12-test-policy-2-templates" @@ -207,6 +209,21 @@ var _ = Describe("Test dependency logic in template sync", Ordered, func() { By("Checking if policy status is compliant") Eventually(checkCompliance(case12PolicyName), defaultTimeoutSeconds, 1).Should(Equal("Compliant")) + By("Creating a policy with an invalid dep on hub cluster in ns:" + clusterNamespaceOnHub) + _, err = kubectlHub("apply", "-f", case12PolicyYamlInvalid, "-n", clusterNamespaceOnHub) + Expect(err).To(BeNil()) + hubPlc = utils.GetWithTimeout( + clientHubDynamic, + gvrPolicy, + case12PolicyNameInvalid, + clusterNamespaceOnHub, + true, + defaultTimeoutSeconds) + Expect(hubPlc).NotTo(BeNil()) + + By("Checking if policy status is pending") + Eventually(checkCompliance(case12PolicyNameInvalid), defaultTimeoutSeconds, 1).Should(Equal("Pending")) + By("Deleting dependency on hub cluster in ns:" + clusterNamespaceOnHub) _, err = kubectlHub("delete", "-f", case12DepYaml, "-n", clusterNamespaceOnHub) Expect(err).To(BeNil()) @@ -214,6 +231,10 @@ var _ = Describe("Test dependency logic in template sync", Ordered, func() { By("Deleting the policy on hub cluster in ns:" + clusterNamespaceOnHub) _, err = kubectlHub("delete", "-f", case12PolicyYaml, "-n", clusterNamespaceOnHub) Expect(err).To(BeNil()) + + By("Deleting invalid policy in ns:" + clusterNamespaceOnHub) + _, err = kubectlHub("delete", "-f", case12PolicyYamlInvalid, "-n", clusterNamespaceOnHub) + Expect(err).To(BeNil()) }) It("Should remove template if dependency changes", func() { By("Creating a dep on hub cluster in ns:" + clusterNamespaceOnHub) diff --git a/test/resources/case12_ordering/case12-plc-invalid-dep.yaml b/test/resources/case12_ordering/case12-plc-invalid-dep.yaml new file mode 100644 index 00000000..49f06bc8 --- /dev/null +++ b/test/resources/case12_ordering/case12-plc-invalid-dep.yaml @@ -0,0 +1,36 @@ +apiVersion: policy.open-cluster-management.io/v1 +kind: Policy +metadata: + name: case12-test-policy-invalid + labels: + policy.open-cluster-management.io/cluster-name: managed + policy.open-cluster-management.io/cluster-namespace: managed + policy.open-cluster-management.io/root-policy: case12-test-policy +spec: + remediationAction: inform + disabled: false + dependencies: + - apiVersion: invalid.api.group/v1 + kind: Policy + name: namespace-foo-setup-policy + namespace: "" + compliance: Compliant + policy-templates: + - objectDefinition: + apiVersion: policy.open-cluster-management.io/v1 + kind: ConfigurationPolicy + metadata: + name: case12-config-policy-invalid + spec: + remediationAction: inform + object-templates: + - complianceType: musthave + objectDefinition: + apiVersion: v1 + kind: Pod + metadata: + name: nginx-pod-e2e + namespace: default + spec: + containers: + - name: nginx