Skip to content

Commit 217a12b

Browse files
committed
Convert non-string labels and annotations
Previously, if labels or annotations inside an objectTemplate used values that were not strings, the ConfigurationPolicy might try to remove all labels or annotations on that object. That behavior was potentially destructive, and it was easy for a policy author to accidentally trigger by getting some quotes wrong in their template, or by using `yes` as a value (which is interpreted by some parsers as a boolean `true`). Now, the ConfigurationPolicy will force all values in labels and annotations to be strings, which is most likely what the policy author intended. In the case of a `yes` string or something similar, there may still be a discrepancy, but the value in the ConfigurationPolicy will already be a boolean, so we can't detect or handle that case. Refs: - https://issues.redhat.com/browse/ACM-26186 Signed-off-by: Justin Kulikauskas <[email protected]>
1 parent e95b23e commit 217a12b

File tree

6 files changed

+55
-11
lines changed

6 files changed

+55
-11
lines changed

controllers/configurationpolicy_controller.go

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3553,17 +3553,33 @@ func handleKeys(
35533553
continue
35543554
}
35553555

3556-
// only look at labels and annotations for metadata - configurationPolicies do not update other metadata fields
35573556
if key == "metadata" {
3558-
// if it's not the right type, the map will be empty
3559-
mdMap, _ := mergedObj.(map[string]interface{})
3557+
if mdMap, ok := mergedObj.(map[string]any); ok {
3558+
// ConfigurationPolicies should not affect metadata fields other than labels and
3559+
// annotations, and they should only be updated if the merged object has the right
3560+
// format.
3561+
mergedAnnos, found, err := unstructured.NestedMap(mdMap, "annotations")
3562+
if found && err == nil {
3563+
// Force all annotation values to be strings
3564+
annos := make(map[string]string)
3565+
for k, v := range mergedAnnos {
3566+
annos[k] = fmt.Sprint(v)
3567+
}
3568+
3569+
existingObj.SetAnnotations(annos)
3570+
}
35603571

3561-
// if either isn't found, they'll just be empty
3562-
mergedAnnotations, _, _ := unstructured.NestedStringMap(mdMap, "annotations")
3563-
mergedLabels, _, _ := unstructured.NestedStringMap(mdMap, "labels")
3572+
mergedLabels, found, err := unstructured.NestedMap(mdMap, "labels")
3573+
if found && err == nil {
3574+
// Force all label values to be strings
3575+
labels := make(map[string]string)
3576+
for k, v := range mergedLabels {
3577+
labels[k] = fmt.Sprint(v)
3578+
}
35643579

3565-
existingObj.SetAnnotations(mergedAnnotations)
3566-
existingObj.SetLabels(mergedLabels)
3580+
existingObj.SetLabels(labels)
3581+
}
3582+
}
35673583
} else {
35683584
existingObj.UnstructuredContent()[key] = mergedObj
35693585
}

test/dryrun/kind_field/pod_annotation_match/input_1.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@ metadata:
55
namespace: managed
66
annotations:
77
test: e2e10
8+
foo: "true"
89
labels:
910
test: e2e10
11+
bar: "7"
1012
spec:
1113
containers:
1214
- image: nginx:1.7.9

test/dryrun/kind_field/pod_annotation_match/policy.yaml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,10 @@ spec:
1919
kind: Pod
2020
metadata:
2121
namespace: managed
22+
name: nginx-pod-e2e-10
2223
annotations:
2324
test: e2e10
25+
foo: yes
2426
labels:
2527
test: e2e10
28+
bar: 7

test/dryrun/kind_field/pod_annotation_mismatch/input_1.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ metadata:
77
test: e2e10
88
labels:
99
test: e2e10
10+
bar: fail
1011
spec:
1112
containers:
1213
- image: nginx:1.7.9
Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,24 @@
11
# Diffs:
2-
v1 Pod managed/-:
3-
2+
v1 Pod managed/nginx-pod-e2e-10:
3+
--- managed/nginx-pod-e2e-10 : existing
4+
+++ managed/nginx-pod-e2e-10 : updated
5+
@@ -1,13 +1,14 @@
6+
apiVersion: v1
7+
kind: Pod
8+
metadata:
9+
annotations:
10+
- test: e2e10
11+
+ foo: "true"
12+
+ test: fail
13+
labels:
14+
- bar: fail
15+
- test: e2e10
16+
+ bar: "7"
17+
+ test: fail
18+
name: nginx-pod-e2e-10
19+
namespace: managed
20+
spec:
21+
containers:
22+
- image: nginx:1.7.9
423
# Compliance messages:
5-
NonCompliant; violation - pods found but not as specified in namespace managed
24+
NonCompliant; violation - pods [nginx-pod-e2e-10] found but not as specified in namespace managed

test/dryrun/kind_field/pod_annotation_mismatch/policy.yaml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,10 @@ spec:
1212
kind: Pod
1313
metadata:
1414
namespace: managed
15+
name: nginx-pod-e2e-10
1516
annotations:
1617
test: fail
18+
foo: yes
1719
labels:
1820
test: fail
21+
bar: 7

0 commit comments

Comments
 (0)