Skip to content

Commit 3936de7

Browse files
JustinKulimagic-mirror-bot[bot]
JustinKuli
authored and
magic-mirror-bot[bot]
committed
Handle SCC annotations in namespaces
Several annotations relating to SCCs are automatically populated on namespaces by OpenShift when they are empty. Previously, applying annotations to a namespace via a mustonlyhave policy was then causing those values to be removed and then be set again, interfering with workloads in that namespace. Now these annotations are specially handled to avoid this. SCC annotations set explicitly in the policy will still be checked correctly. Refs: - https://issues.redhat.com/browse/ACM-12507 Signed-off-by: Justin Kulikauskas <[email protected]> (cherry picked from commit 1027df9)
1 parent e428a4e commit 3936de7

File tree

4 files changed

+94
-48
lines changed

4 files changed

+94
-48
lines changed

‎controllers/configurationpolicy_controller.go

+7-2
Original file line numberDiff line numberDiff line change
@@ -2488,9 +2488,14 @@ func handleSingleKey(
24882488
}
24892489

24902490
if key == "metadata" {
2491-
// filter out autogenerated annotations that have caused compare issues in the past
2491+
isNamespace := desiredObj.GroupVersionKind() == namespaceGVK
2492+
2493+
// metadata has some special cases that need to be considered
24922494
mergedValue, existingValue = fmtMetadataForCompare(
2493-
mergedValue.(map[string]interface{}), existingValue.(map[string]interface{}))
2495+
mergedValue.(map[string]interface{}),
2496+
existingValue.(map[string]interface{}),
2497+
isNamespace,
2498+
)
24942499
}
24952500

24962501
if key == "stringData" && existingObj.GetKind() == "Secret" {

‎controllers/configurationpolicy_utils.go

+34-21
Original file line numberDiff line numberDiff line change
@@ -394,36 +394,49 @@ func formatMetadata(metadata map[string]interface{}) (formatted map[string]inter
394394
}
395395

396396
func fmtMetadataForCompare(
397-
metadataTemp, metadataExisting map[string]interface{},
398-
) (formatted, formattedExisting map[string]interface{}) {
399-
mdTemp := map[string]interface{}{}
400-
mdExisting := map[string]interface{}{}
397+
merged, existing map[string]interface{}, keepSCC bool,
398+
) (formattedMerged, formattedExisting map[string]interface{}) {
399+
formattedMerged = formatMetadata(merged)
400+
formattedExisting = formatMetadata(existing)
401401

402-
if labelsTemp, ok := metadataTemp["labels"]; ok {
403-
mdTemp["labels"] = labelsTemp
402+
if _, mergedHasLabels := formattedMerged["labels"]; !mergedHasLabels {
403+
delete(formattedExisting, "labels")
404+
}
404405

405-
if labelsExisting, ok := metadataExisting["labels"]; ok {
406-
mdExisting["labels"] = labelsExisting
407-
}
406+
if _, mergedHasAnnos := formattedMerged["annotations"]; !mergedHasAnnos {
407+
delete(formattedExisting, "annotations")
408+
409+
return formattedMerged, formattedExisting
408410
}
409411

410-
if annosTemp, ok := metadataTemp["annotations"]; ok {
411-
if annos, ok := annosTemp.(map[string]interface{}); ok {
412-
mdTemp["annotations"] = filterUnwantedAnnotations(annos)
413-
} else {
414-
mdTemp["annotations"] = annosTemp
412+
if !keepSCC {
413+
return formattedMerged, formattedExisting
414+
}
415+
416+
existingAnnos, ok := formattedExisting["annotations"].(map[string]interface{})
417+
if !ok {
418+
return formattedMerged, formattedExisting
419+
}
420+
421+
mergedAnnos, ok := formattedMerged["annotations"].(map[string]interface{})
422+
if !ok {
423+
return formattedMerged, formattedExisting
424+
}
425+
426+
// Copy existing SCC annotations to the merged metadata
427+
for key, val := range existingAnnos {
428+
if !strings.HasPrefix(key, "openshift.io/sa.scc.") {
429+
continue
415430
}
416431

417-
if annosExisting, ok := metadataExisting["annotations"]; ok {
418-
if annos, ok := annosExisting.(map[string]interface{}); ok {
419-
mdExisting["annotations"] = filterUnwantedAnnotations(annos)
420-
} else {
421-
mdExisting["annotations"] = annosExisting
422-
}
432+
if _, alreadyDefined := mergedAnnos[key]; !alreadyDefined {
433+
mergedAnnos[key] = val
423434
}
424435
}
425436

426-
return mdTemp, mdExisting
437+
formattedMerged["annotations"] = mergedAnnos
438+
439+
return formattedMerged, formattedExisting
427440
}
428441

429442
// Format name of resource with its namespace (if it has one)

‎test/e2e/case9_md_check_test.go

+49-25
Original file line numberDiff line numberDiff line change
@@ -10,32 +10,32 @@ import (
1010
"open-cluster-management.io/config-policy-controller/test/utils"
1111
)
1212

13-
const (
14-
case9ConfigPolicyNamePod string = "policy-pod-c9-create"
15-
case9ConfigPolicyNameAnno string = "policy-pod-anno"
16-
case9ConfigPolicyNameNoAnno string = "policy-pod-no-anno"
17-
case9ConfigPolicyNameLabelPatch string = "policy-label-patch"
18-
case9ConfigPolicyNameLabelCheck string = "policy-label-check"
19-
case9ConfigPolicyNameLabelAuto string = "policy-label-check-auto"
20-
case9ConfigPolicyNameNSCreate string = "policy-c9-create-ns"
21-
case9ConfigPolicyNameIgnoreLabels string = "policy-ignore-labels"
22-
case9MultiAnnoNSCreate string = "policy-create-ns-multiple-annotations"
23-
case9CheckNSMusthave string = "policy-check-ns-mdcomptype-mh"
24-
case9CheckNSMustonlyhave string = "policy-check-ns-mdcomptype-moh"
25-
case9PolicyYamlPod string = "../resources/case9_md_check/case9_pod_create.yaml"
26-
case9PolicyYamlAnno string = "../resources/case9_md_check/case9_annos.yaml"
27-
case9PolicyYamlNoAnno string = "../resources/case9_md_check/case9_no_annos.yaml"
28-
case9PolicyYamlLabelPatch string = "../resources/case9_md_check/case9_label_patch.yaml"
29-
case9PolicyYamlLabelCheck string = "../resources/case9_md_check/case9_label_check.yaml"
30-
case9PolicyYamlLabelAuto string = "../resources/case9_md_check/case9_label_check_auto.yaml"
31-
case9PolicyYamlIgnoreLabels string = "../resources/case9_md_check/case9_mustonlyhave_nolabels.yaml"
32-
case9PolicyYamlNSCreate string = "../resources/case9_md_check/case9_ns_create.yaml"
33-
case9PolicyYamlMultiAnnoNSCreate string = "../resources/case9_md_check/case9_multianno_ns_create.yaml"
34-
case9PolicyYamlCheckNSMusthave string = "../resources/case9_md_check/case9_checkns-md-mh.yaml"
35-
case9PolicyYamlCheckNSMustonlyhave string = "../resources/case9_md_check/case9_checkns-md-moh.yaml"
36-
)
37-
3813
var _ = Describe("Test pod obj template handling", func() {
14+
const (
15+
case9ConfigPolicyNamePod string = "policy-pod-c9-create"
16+
case9ConfigPolicyNameAnno string = "policy-pod-anno"
17+
case9ConfigPolicyNameNoAnno string = "policy-pod-no-anno"
18+
case9ConfigPolicyNameLabelPatch string = "policy-label-patch"
19+
case9ConfigPolicyNameLabelCheck string = "policy-label-check"
20+
case9ConfigPolicyNameLabelAuto string = "policy-label-check-auto"
21+
case9ConfigPolicyNameNSCreate string = "policy-c9-create-ns"
22+
case9ConfigPolicyNameIgnoreLabels string = "policy-ignore-labels"
23+
case9MultiAnnoNSCreate string = "policy-create-ns-multiple-annotations"
24+
case9CheckNSMusthave string = "policy-check-ns-mdcomptype-mh"
25+
case9CheckNSMustonlyhave string = "policy-check-ns-mdcomptype-moh"
26+
case9PolicyYamlPod string = "../resources/case9_md_check/case9_pod_create.yaml"
27+
case9PolicyYamlAnno string = "../resources/case9_md_check/case9_annos.yaml"
28+
case9PolicyYamlNoAnno string = "../resources/case9_md_check/case9_no_annos.yaml"
29+
case9PolicyYamlLabelPatch string = "../resources/case9_md_check/case9_label_patch.yaml"
30+
case9PolicyYamlLabelCheck string = "../resources/case9_md_check/case9_label_check.yaml"
31+
case9PolicyYamlLabelAuto string = "../resources/case9_md_check/case9_label_check_auto.yaml"
32+
case9PolicyYamlIgnoreLabels string = "../resources/case9_md_check/case9_mustonlyhave_nolabels.yaml"
33+
case9PolicyYamlNSCreate string = "../resources/case9_md_check/case9_ns_create.yaml"
34+
case9PolicyYamlMultiAnnoNSCreate string = "../resources/case9_md_check/case9_multianno_ns_create.yaml"
35+
case9PolicyYamlCheckNSMusthave string = "../resources/case9_md_check/case9_checkns-md-mh.yaml"
36+
case9PolicyYamlCheckNSMustonlyhave string = "../resources/case9_md_check/case9_checkns-md-moh.yaml"
37+
)
38+
3939
Describe("Create a pod policy on managed cluster in ns:"+testNamespace, Ordered, func() {
4040
It("should create a policy properly on the managed cluster", func() {
4141
By("Creating " + case9ConfigPolicyNamePod + " on managed")
@@ -128,6 +128,30 @@ var _ = Describe("Test pod obj template handling", func() {
128128
utils.CheckComplianceStatus(g, managedPlc, "Compliant")
129129
}, defaultTimeoutSeconds, 1).Should(Succeed())
130130
})
131+
It("should not remove scc namespace annotations even in mustonlyhave mode", func() {
132+
By("Checking the current annotations")
133+
obj := utils.GetWithTimeout(clientManagedDynamic, gvrNS,
134+
"case9-test", "", true, defaultTimeoutSeconds)
135+
Expect(obj.GetAnnotations()).To(HaveKeyWithValue("foo.bar/baz", "hello world"))
136+
Expect(obj.GetAnnotations()).To(HaveKeyWithValue("openshift.io/sa.scc.policy", "keep"))
137+
138+
By("Patching the annotations on the namespace")
139+
utils.Kubectl("patch", "namespace", "case9-test", "-o=yaml", "--type=merge",
140+
`-p={"metadata":{"annotations":{`+
141+
`"openshift.io/sa.scc.test": "example",`+
142+
`"openshift.io/sa.scc.policy": "example",`+
143+
`"foo.bar/baz": "incorrect"}}}`)
144+
145+
By("Verifying the annotations in the policy are updated, and the new SCC annotation is kept")
146+
Eventually(func(g Gomega) {
147+
utils.Kubectl("get", "namespace", "case9-test", "-o=yaml")
148+
obj := utils.GetWithTimeout(clientManagedDynamic, gvrNS,
149+
"case9-test", "", true, defaultTimeoutSeconds)
150+
g.Expect(obj.GetAnnotations()).To(HaveKeyWithValue("foo.bar/baz", "hello world"))
151+
g.Expect(obj.GetAnnotations()).To(HaveKeyWithValue("openshift.io/sa.scc.test", "example"))
152+
g.Expect(obj.GetAnnotations()).To(HaveKeyWithValue("openshift.io/sa.scc.policy", "keep"))
153+
}, defaultTimeoutSeconds, 1).Should(Succeed())
154+
})
131155
It("should ignore labels and annotations if none are specified in the template", func() {
132156
By("Creating " + case9ConfigPolicyNameIgnoreLabels + " on managed")
133157
utils.Kubectl("apply", "-f", case9PolicyYamlIgnoreLabels, "-n", testNamespace)

‎test/resources/case9_md_check/case9_ns_create.yaml

+4
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,12 @@ spec:
1515
- default
1616
object-templates:
1717
- complianceType: musthave
18+
metadataComplianceType: mustonlyhave
1819
objectDefinition:
1920
kind: Namespace
2021
apiVersion: v1
2122
metadata:
2223
name: case9-test
24+
annotations:
25+
foo.bar/baz: hello world
26+
openshift.io/sa.scc.policy: keep

0 commit comments

Comments
 (0)