diff --git a/controllers/configurationpolicy_controller.go b/controllers/configurationpolicy_controller.go index f9cb0664..80c5096e 100644 --- a/controllers/configurationpolicy_controller.go +++ b/controllers/configurationpolicy_controller.go @@ -3553,17 +3553,34 @@ func handleKeys( continue } - // only look at labels and annotations for metadata - configurationPolicies do not update other metadata fields if key == "metadata" { - // if it's not the right type, the map will be empty - mdMap, _ := mergedObj.(map[string]interface{}) + if mdMap, ok := mergedObj.(map[string]any); ok { + // ConfigurationPolicies should not affect metadata fields other than labels and + // annotations, and if the desired state is invalid, a violation should be raised. + mergedAnnos, found, err := unstructured.NestedStringMap(mdMap, "annotations") + if err != nil { + // A test verifies that this error text is stable + errMsg, _ := strings.CutPrefix(err.Error(), ".annotations accessor error: ") + + return true, fmt.Sprintf("invalid annotation, error: %v", errMsg), true, statusMismatch, missingKey + } + + if found { + existingObj.SetAnnotations(mergedAnnos) + } - // if either isn't found, they'll just be empty - mergedAnnotations, _, _ := unstructured.NestedStringMap(mdMap, "annotations") - mergedLabels, _, _ := unstructured.NestedStringMap(mdMap, "labels") + mergedLabels, found, err := unstructured.NestedStringMap(mdMap, "labels") + if err != nil { + // A test verifies that this error text is stable + errMsg, _ := strings.CutPrefix(err.Error(), ".labels accessor error: ") + + return true, fmt.Sprintf("invalid label, error: %v", errMsg), true, statusMismatch, missingKey + } - existingObj.SetAnnotations(mergedAnnotations) - existingObj.SetLabels(mergedLabels) + if found { + existingObj.SetLabels(mergedLabels) + } + } } else { existingObj.UnstructuredContent()[key] = mergedObj } diff --git a/test/dryrun/kind_field/kind_field_test.go b/test/dryrun/kind_field/kind_field_test.go index 4974fe58..4191d90a 100644 --- a/test/dryrun/kind_field/kind_field_test.go +++ b/test/dryrun/kind_field/kind_field_test.go @@ -23,3 +23,17 @@ var podAnnotationMismatch embed.FS func TestPodAnnotationMisMatch(t *testing.T) { t.Run("Test pod annotation mismatch", dryrun.Run(podAnnotationMismatch)) } + +//go:embed pod_label_invalid/* +var podLabelInvalid embed.FS + +func TestPodLabelInvalid(t *testing.T) { + t.Run("Test invalid pod label in policy", dryrun.Run(podLabelInvalid)) +} + +//go:embed pod_annotation_invalid/* +var podAnnotationInvalid embed.FS + +func TestPodAnnotationInvalid(t *testing.T) { + t.Run("Test invalid pod annotation in policy", dryrun.Run(podAnnotationInvalid)) +} diff --git a/test/dryrun/kind_field/pod_annotation_invalid/input_1.yaml b/test/dryrun/kind_field/pod_annotation_invalid/input_1.yaml new file mode 100644 index 00000000..8c267b91 --- /dev/null +++ b/test/dryrun/kind_field/pod_annotation_invalid/input_1.yaml @@ -0,0 +1,15 @@ +apiVersion: v1 +kind: Pod +metadata: + name: nginx-pod-e2e-10 + namespace: managed + annotations: + test: e2e10 + labels: + test: e2e10 +spec: + containers: + - image: nginx:1.7.9 + name: nginx + ports: + - containerPort: 80 diff --git a/test/dryrun/kind_field/pod_annotation_invalid/input_ns.yaml b/test/dryrun/kind_field/pod_annotation_invalid/input_ns.yaml new file mode 100644 index 00000000..e32c3ced --- /dev/null +++ b/test/dryrun/kind_field/pod_annotation_invalid/input_ns.yaml @@ -0,0 +1,5 @@ +apiVersion: v1 +kind: Namespace +metadata: + name: managed +spec: {} diff --git a/test/dryrun/kind_field/pod_annotation_invalid/output.txt b/test/dryrun/kind_field/pod_annotation_invalid/output.txt new file mode 100644 index 00000000..4e1a6623 --- /dev/null +++ b/test/dryrun/kind_field/pod_annotation_invalid/output.txt @@ -0,0 +1,6 @@ +# Diffs: +v1 Pod managed/nginx-pod-e2e-10: + +# Compliance messages: +NonCompliant; violation - pods [nginx-pod-e2e-10] found but not as specified in namespace managed +NonCompliant; violation - invalid annotation, error: contains non-string value in the map under key "case": 10 is of the type int64, expected string diff --git a/test/dryrun/kind_field/pod_annotation_invalid/policy.yaml b/test/dryrun/kind_field/pod_annotation_invalid/policy.yaml new file mode 100644 index 00000000..8f8e38f1 --- /dev/null +++ b/test/dryrun/kind_field/pod_annotation_invalid/policy.yaml @@ -0,0 +1,18 @@ +apiVersion: policy.open-cluster-management.io/v1 +kind: ConfigurationPolicy +metadata: + name: policy-kind-labels-fail +spec: + remediationAction: inform # will be overridden by remediationAction in parent policy + severity: low + object-templates: + - complianceType: musthave + objectDefinition: + apiVersion: v1 + kind: Pod + metadata: + namespace: managed + name: nginx-pod-e2e-10 + annotations: + case: 10 + test: fail diff --git a/test/dryrun/kind_field/pod_label_invalid/input_1.yaml b/test/dryrun/kind_field/pod_label_invalid/input_1.yaml new file mode 100644 index 00000000..8c267b91 --- /dev/null +++ b/test/dryrun/kind_field/pod_label_invalid/input_1.yaml @@ -0,0 +1,15 @@ +apiVersion: v1 +kind: Pod +metadata: + name: nginx-pod-e2e-10 + namespace: managed + annotations: + test: e2e10 + labels: + test: e2e10 +spec: + containers: + - image: nginx:1.7.9 + name: nginx + ports: + - containerPort: 80 diff --git a/test/dryrun/kind_field/pod_label_invalid/input_ns.yaml b/test/dryrun/kind_field/pod_label_invalid/input_ns.yaml new file mode 100644 index 00000000..e32c3ced --- /dev/null +++ b/test/dryrun/kind_field/pod_label_invalid/input_ns.yaml @@ -0,0 +1,5 @@ +apiVersion: v1 +kind: Namespace +metadata: + name: managed +spec: {} diff --git a/test/dryrun/kind_field/pod_label_invalid/output.txt b/test/dryrun/kind_field/pod_label_invalid/output.txt new file mode 100644 index 00000000..c4ad8436 --- /dev/null +++ b/test/dryrun/kind_field/pod_label_invalid/output.txt @@ -0,0 +1,6 @@ +# Diffs: +v1 Pod managed/nginx-pod-e2e-10: + +# Compliance messages: +NonCompliant; violation - pods [nginx-pod-e2e-10] found but not as specified in namespace managed +NonCompliant; violation - invalid label, error: contains non-string value in the map under key "problem": true is of the type bool, expected string diff --git a/test/dryrun/kind_field/pod_label_invalid/policy.yaml b/test/dryrun/kind_field/pod_label_invalid/policy.yaml new file mode 100644 index 00000000..54d535d5 --- /dev/null +++ b/test/dryrun/kind_field/pod_label_invalid/policy.yaml @@ -0,0 +1,18 @@ +apiVersion: policy.open-cluster-management.io/v1 +kind: ConfigurationPolicy +metadata: + name: policy-kind-labels-fail +spec: + remediationAction: inform # will be overridden by remediationAction in parent policy + severity: low + object-templates: + - complianceType: musthave + objectDefinition: + apiVersion: v1 + kind: Pod + metadata: + namespace: managed + name: nginx-pod-e2e-10 + labels: + problem: yes + test: fail