Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 25 additions & 8 deletions controllers/configurationpolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
14 changes: 14 additions & 0 deletions test/dryrun/kind_field/kind_field_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
15 changes: 15 additions & 0 deletions test/dryrun/kind_field/pod_annotation_invalid/input_1.yaml
Original file line number Diff line number Diff line change
@@ -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
5 changes: 5 additions & 0 deletions test/dryrun/kind_field/pod_annotation_invalid/input_ns.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
apiVersion: v1
kind: Namespace
metadata:
name: managed
spec: {}
6 changes: 6 additions & 0 deletions test/dryrun/kind_field/pod_annotation_invalid/output.txt
Original file line number Diff line number Diff line change
@@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error is so clearly written it makes me want to cry 🥹

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is pretty nice :D

18 changes: 18 additions & 0 deletions test/dryrun/kind_field/pod_annotation_invalid/policy.yaml
Original file line number Diff line number Diff line change
@@ -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
15 changes: 15 additions & 0 deletions test/dryrun/kind_field/pod_label_invalid/input_1.yaml
Original file line number Diff line number Diff line change
@@ -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
5 changes: 5 additions & 0 deletions test/dryrun/kind_field/pod_label_invalid/input_ns.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
apiVersion: v1
kind: Namespace
metadata:
name: managed
spec: {}
6 changes: 6 additions & 0 deletions test/dryrun/kind_field/pod_label_invalid/output.txt
Original file line number Diff line number Diff line change
@@ -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
18 changes: 18 additions & 0 deletions test/dryrun/kind_field/pod_label_invalid/policy.yaml
Original file line number Diff line number Diff line change
@@ -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