Fix requeue when Kind is missing

Makefile

@@ -73,25 +73,8 @@ REGISTRY ?= quay.io/open-cluster-management
 TAG ?= latest
 IMAGE_NAME_AND_VERSION ?= $(REGISTRY)/$(IMG)
 
-# go-get-tool will 'go install' any package $1 and install it to LOCAL_BIN.
-define go-get-tool
-@set -e ;\
-echo "Checking installation of $(1)" ;\
-GOBIN=$(LOCAL_BIN) go install $(1)
-endef
-
 include build/common/Makefile.common.mk
 
-############################################################
-# work section
-############################################################
-
-$(GOBIN):
-	@mkdir -p $(GOBIN)
-
-$(LOCAL_BIN):
-	@mkdir -p $(LOCAL_BIN)
-
 ############################################################
 # clean section
 ############################################################
@@ -109,42 +92,15 @@ clean:
 # format section
 ############################################################
 
-.PHONY: fmt-dependencies
-fmt-dependencies:
-	$(call go-get-tool,github.com/daixiang0/gci@v0.10.1)
-	$(call go-get-tool,mvdan.cc/gofumpt@v0.5.0)
-
-# All available format: format-go format-protos format-python
-# Default value will run all formats, override these make target with your requirements:
-#    eg: fmt: format-go format-protos
 .PHONY: fmt
-fmt: fmt-dependencies
-	find . -not \( -path "./.go" -prune \) -name "*.go" | xargs gofmt -s -w
-	find . -not \( -path "./.go" -prune \) -name "*.go" | xargs gofumpt -l -w
-	find . -not \( -path "./.go" -prune \) -name "*.go" | xargs gci write -s standard -s default -s "prefix($(shell cat go.mod | head -1 | cut -d " " -f 2))"
-
-############################################################
-# check section
-############################################################
-
-.PHONY: check
-check: lint
-
-.PHONY: lint-dependencies
-lint-dependencies:
-	$(call go-get-tool,github.com/golangci/golangci-lint/cmd/golangci-lint@v1.52.2)
+fmt:
 
 .PHONY: lint
 lint:
 
 ############################################################
 # test section
 ############################################################
-GOSEC = $(LOCAL_BIN)/gosec
-KUBEBUILDER = $(LOCAL_BIN)/kubebuilder
-ENVTEST = $(LOCAL_BIN)/setup-envtest
-KBVERSION = 3.12.0
-ENVTEST_K8S_VERSION = 1.26.x
 
 .PHONY: test
 test: test-dependencies
@@ -157,26 +113,6 @@ test-coverage: test
 .PHONY: test-dependencies
 test-dependencies: envtest kubebuilder
 
-.PHONY: kubebuilder
-kubebuilder:
-	@if [ "$$($(KUBEBUILDER) version 2>/dev/null | grep -o KubeBuilderVersion:\"[0-9]*\.[0-9]\.[0-9]*\")" != "KubeBuilderVersion:\"$(KBVERSION)\"" ]; then \
-		echo "Installing Kubebuilder"; \
-		curl -L https://github.com/kubernetes-sigs/kubebuilder/releases/download/v$(KBVERSION)/kubebuilder_$(GOOS)_$(GOARCH) -o $(KUBEBUILDER); \
-		chmod +x $(KUBEBUILDER); \
-	fi
-
-.PHONY: envtest
-envtest:
-	$(call go-get-tool,sigs.k8s.io/controller-runtime/tools/setup-envtest@latest)
-
-.PHONY: gosec
-gosec:
-	$(call go-get-tool,github.com/securego/gosec/v2/cmd/gosec@v2.15.0)
-
-.PHONY: gosec-scan
-gosec-scan: gosec
-	$(GOSEC) -fmt sonarqube -out gosec.json -stdout -exclude-dir=.go -exclude-dir=test ./...
-
 ############################################################
 # build section
 ############################################################
@@ -201,13 +137,10 @@ build-images:
 .PHONY: deploy
 deploy: generate-operator-yaml
 	kubectl apply -f deploy/operator.yaml -n $(KIND_NAMESPACE) --kubeconfig=$(MANAGED_CONFIG)_e2e
-
 
 ############################################################
 # Generate manifests
 ############################################################
-CONTROLLER_GEN = $(LOCAL_BIN)/controller-gen
-KUSTOMIZE = $(LOCAL_BIN)/kustomize
 
 .PHONY: manifests
 manifests: controller-gen
@@ -221,18 +154,9 @@ generate: controller-gen ## Generate code containing DeepCopy, DeepCopyInto, and
 generate-operator-yaml: kustomize manifests
 	$(KUSTOMIZE) build deploy/manager > deploy/operator.yaml
 
-.PHONY: controller-gen
-controller-gen: ## Download controller-gen locally if necessary.
-	$(call go-get-tool,sigs.k8s.io/controller-tools/cmd/controller-gen@v0.6.1)
-
-.PHONY: kustomize
-kustomize: ## Download kustomize locally if necessary.
-	$(call go-get-tool,sigs.k8s.io/kustomize/kustomize/v5@v5.0.1)
-
 ############################################################
 # e2e test section
 ############################################################
-GINKGO = $(LOCAL_BIN)/ginkgo
 
 .PHONY: kind-bootstrap-cluster
 kind-bootstrap-cluster: kind-bootstrap-cluster-dev kind-deploy-controller
@@ -293,10 +217,6 @@ install-resources:
 		kubectl apply -f https://raw.githubusercontent.com/open-policy-agent/gatekeeper/v3.11.0/deploy/gatekeeper.yaml  --kubeconfig=$(MANAGED_CONFIG)_e2e; \
 	fi
 
-.PHONY: e2e-dependencies
-e2e-dependencies:
-	$(call go-get-tool,github.com/onsi/ginkgo/v2/ginkgo@$(shell awk '/github.com\/onsi\/ginkgo\/v2/ {print $$2}' go.mod))
-
 .PHONY: e2e-test
 e2e-test: e2e-dependencies
 	$(GINKGO) -v --fail-fast $(E2E_TEST_ARGS) test/e2e
@@ -355,12 +275,8 @@ e2e-debug:
 ############################################################
 # test coverage
 ############################################################
-GOCOVMERGE = $(LOCAL_BIN)/gocovmerge
-.PHONY: coverage-dependencies
-coverage-dependencies:
-	$(call go-get-tool,github.com/wadey/gocovmerge@v0.0.0-20160331181800-b5bfa59ec0ad)
-
 COVERAGE_FILE = coverage.out
+
 .PHONY: coverage-merge
 coverage-merge: coverage-dependencies
 	@echo Merging the coverage reports into $(COVERAGE_FILE)

controllers/secretsync/secret_sync.go

@@ -109,10 +109,10 @@ func (r *SecretReconciler) Reconcile(ctx context.Context, request reconcile.Requ
 	}
 
 	managedEncryptionSecret := &corev1.Secret{}
+
 	err = r.ManagedClient.Get(
 		ctx, types.NamespacedName{Namespace: r.TargetNamespace, Name: request.Name}, managedEncryptionSecret,
 	)
-
 	if err != nil {
 		if !errors.IsNotFound(err) {
 			reqLogger.Error(err, "Failed to get the replicated Secret. Requeueing the request.")

controllers/specsync/policy_spec_sync.go

@@ -116,8 +116,8 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request reconcile.Requ
 	}
 
 	managedPlc := &policiesv1.Policy{}
-	err = r.ManagedClient.Get(ctx, types.NamespacedName{Namespace: r.TargetNamespace, Name: request.Name}, managedPlc)
 
+	err = r.ManagedClient.Get(ctx, types.NamespacedName{Namespace: r.TargetNamespace, Name: request.Name}, managedPlc)
 	if err != nil {
 		if errors.IsNotFound(err) {
 			// not found on managed cluster, create it
@@ -132,8 +132,8 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request reconcile.Requ
 
 			managedPlc.SetOwnerReferences(nil)
 			managedPlc.SetResourceVersion("")
-			err = r.ManagedClient.Create(ctx, managedPlc)
 
+			err = r.ManagedClient.Create(ctx, managedPlc)
 			if err != nil {
 				reqLogger.Error(err, "Failed to create policy on managed...")
 

controllers/statussync/policy_status_sync.go

@@ -102,10 +102,10 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request reconcile.Requ
 			// The replicated policy on the managed cluster was deleted.
 			// check if it was deleted by user by checking if it still exists on hub
 			hubInstance := &policiesv1.Policy{}
+
 			err = r.HubClient.Get(
 				ctx, types.NamespacedName{Namespace: r.ClusterNamespaceOnHub, Name: request.Name}, hubInstance,
 			)
-
 			if err != nil {
 				if errors.IsNotFound(err) {
 					// confirmed deleted on hub, doing nothing
@@ -139,10 +139,11 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request reconcile.Requ
 
 		return reconcile.Result{}, err
 	}
+
 	// get hub policy
 	hubPlc := &policiesv1.Policy{}
-	err = r.HubClient.Get(ctx, types.NamespacedName{Namespace: r.ClusterNamespaceOnHub, Name: request.Name}, hubPlc)
 
+	err = r.HubClient.Get(ctx, types.NamespacedName{Namespace: r.ClusterNamespaceOnHub, Name: request.Name}, hubPlc)
 	if err != nil {
 		// hub policy not found, it has been deleted
 		if errors.IsNotFound(err) {
@@ -178,8 +179,8 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request reconcile.Requ
 
 	// plc matches hub plc, then get events
 	eventList := &corev1.EventList{}
-	err = r.ManagedClient.List(ctx, eventList, client.InNamespace(instance.GetNamespace()))
 
+	err = r.ManagedClient.List(ctx, eventList, client.InNamespace(instance.GetNamespace()))
 	if err != nil {
 		// there is an error to list events, requeue
 		reqLogger.Error(err, "Error listing events, will requeue the request")
@@ -395,7 +396,6 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request reconcile.Requ
 		reqLogger.Info("status mismatch on managed, update it")
 
 		err = r.ManagedClient.Status().Update(ctx, instance)
-
 		if err != nil {
 			reqLogger.Error(err, "Failed to get update policy status on managed")
 
@@ -420,8 +420,8 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request reconcile.Requ
 			reqLogger.Info("status not in sync, update the hub")
 
 			hubPlc.Status = instance.Status
-			err = r.HubClient.Status().Update(ctx, hubPlc)
 
+			err = r.HubClient.Status().Update(ctx, hubPlc)
 			if err != nil {
 				reqLogger.Error(err, "Failed to update policy status on hub")
 

controllers/templatesync/template_sync.go

@@ -280,7 +280,11 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request reconcile.Requ
 		// Gather raw object definition from the policy template
 		object, gvk, err := unstructured.UnstructuredJSONScheme.Decode(policyT.ObjectDefinition.Raw, nil, nil)
 		if err != nil {
-			resultError = err
+			// If it's missing the Kind, don't requeue since that requires an update to the Policy
+			if !runtime.IsMissingKind(err) {
+				resultError = err
+			}
+
 			errMsg := fmt.Sprintf("Failed to decode policy template with err: %s", err)
 
 			_ = r.emitTemplateError(ctx, instance, tIndex, fmt.Sprintf("template-%v", tIndex), false, errMsg)
@@ -484,10 +488,9 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request reconcile.Requ
 		}
 
 		res = dClient.Resource(rsrc).Namespace(resourceNs)
-
 		tObjectUnstructured := &unstructured.Unstructured{}
-		err = json.Unmarshal(policyT.ObjectDefinition.Raw, tObjectUnstructured)
 
+		err = json.Unmarshal(policyT.ObjectDefinition.Raw, tObjectUnstructured)
 		if err != nil {
 			resultError = err
 			errMsg := fmt.Sprintf("Failed to unmarshal the policy template: %s", err)

main.go

@@ -766,7 +766,6 @@ func addControllers(ctx context.Context, hubCfg *rest.Config, hubMgr manager.Man
 		hubClient, err = client.New(
 			hubCfg, client.Options{Scheme: scheme, Cache: &client.CacheOptions{Reader: hubCache}},
 		)
-
 		if err != nil {
 			log.Error(err, "Failed to generate a client to the hub cluster")
 			os.Exit(1)

test/e2e/case10_error_test.go

@@ -315,7 +315,7 @@ var _ = Describe("Test error handling", func() {
 		By("Creating the ConfigurationPolicy on the managed cluster directly")
 		_, err := kubectlManaged(
 			"apply",
-			"--filename=../resources/case10_template_sync_error_test/working-policy-configpol.yaml",
+			"--filename="+yamlBasePath+"working-policy-configpol.yaml",
 			"--namespace="+clusterNamespace,
 		)
 		Expect(err).ShouldNot(HaveOccurred())
@@ -353,6 +353,27 @@ var _ = Describe("Test error handling", func() {
 			1,
 		).Should(BeTrue())
 	})
+	It("should only generate one event for a missing kind", func() {
+		policyName := "case10-missing-kind"
+		hubApplyPolicy(policyName,
+			yamlBasePath+"missing-kind.yaml")
+
+		By("Checking for the error event and ensuring it only occurs once")
+		Eventually(
+			checkForEvent(
+				policyName, "Object 'Kind' is missing in",
+			),
+			defaultTimeoutSeconds,
+			1,
+		).Should(BeTrue())
+		Consistently(
+			getMatchingEvents(
+				policyName, "Object 'Kind' is missing in",
+			),
+			defaultTimeoutSeconds,
+			1,
+		).Should(HaveLen(2))
+	})
 })
 
 // Checks for an event on the managed cluster
@@ -381,3 +402,32 @@ func checkForEvent(policyName, msgSubStr string) func() bool {
 		return false
 	}
 }
+
+// Checks for an event on the managed cluster
+func getMatchingEvents(policyName, msgSubStr string) func() []string {
+	return func() []string {
+		eventInterface := clientManagedDynamic.Resource(gvrEvent).Namespace(clusterNamespace)
+
+		eventList, err := eventInterface.List(context.TODO(), metav1.ListOptions{
+			FieldSelector: "involvedObject.name=" + policyName,
+		})
+		if err != nil {
+			return []string{}
+		}
+
+		matchingEvents := []string{}
+
+		for _, event := range eventList.Items {
+			msg, found, err := unstructured.NestedString(event.Object, "message")
+			if !found || err != nil {
+				continue
+			}
+
+			if strings.Contains(msg, msgSubStr) {
+				matchingEvents = append(matchingEvents, msg)
+			}
+		}
+
+		return matchingEvents
+	}
+}

test/resources/case10_template_sync_error_test/missing-kind.yaml

@@ -0,0 +1,17 @@
+apiVersion: policy.open-cluster-management.io/v1
+kind: Policy
+metadata:
+  name: case10-missing-kind
+  labels:
+    policy.open-cluster-management.io/cluster-name: managed
+    policy.open-cluster-management.io/cluster-namespace: managed
+    policy.open-cluster-management.io/root-policy: case10-missing-kind
+spec:
+  remediationAction: inform
+  disabled: false
+  policy-templates:
+    - objectDefinition:
+        spec:
+          object-templates:
+            - metadata:
+                labels: {}