Add support for the --log-mutations and --mutation-annotations flags

Relates: https://issues.redhat.com/browse/ACM-8749 Signed-off-by: mprahl <[email protected]>
stolostron · Dec 6, 2023 · 660bc20 · 660bc20
1 parent 86c813e
commit 660bc20
Show file tree

Hide file tree

Showing 8 changed files with 99 additions and 7 deletions.
diff --git a/.github/workflows/ci_tests.yaml b/.github/workflows/ci_tests.yaml
@@ -142,11 +142,6 @@ jobs:
         path: gatekeeper
         fetch-depth: 0 # Fetch all history for all tags and branches
 
-    # TODO: gatekeeper-operator does not --log-mutations nor --mutation-annotations
-    - name: Remove unhandled gatekeeper bats tests
-      run: |
-        sed -i -e '67,73d;82,85d' /home/runner/work/gatekeeper-operator/gatekeeper-operator/gatekeeper/test/bats/test.bats
-
     - name: Gatekeeper E2E Tests
       run: |
         make download-binaries

diff --git a/api/v1alpha1/gatekeeper_types.go b/api/v1alpha1/gatekeeper_types.go
@@ -146,6 +146,13 @@ type WebhookConfig struct {
 	Resources *corev1.ResourceRequirements `json:"resources,omitempty"`
 	// +optional
 	DisabledBuiltins []string `json:"disabledBuiltins,omitempty"`
+	// +optional
+	// Sets the --log-mutations flag which enables logging of mutation events and errors. This defaults to Disabled.
+	LogMutations *Mode `json:"logMutations,omitempty"`
+	// +optional
+	// Sets the --mutation-annotations flag which adds the gatekeeper.sh/mutation-id and gatekeeper.sh/mutations
+	// annotations on mutated objects. This defaults to Disabled.
+	MutationAnnotations *Mode `json:"mutationAnnotations,omitempty"`
 }
 
 // +kubebuilder:validation:Enum:=DEBUG;INFO;WARNING;ERROR

diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go
diff --git a/bundle/manifests/operator.gatekeeper.sh_gatekeepers.yaml b/bundle/manifests/operator.gatekeeper.sh_gatekeepers.yaml
@@ -1052,6 +1052,21 @@ spec:
                     - WARNING
                     - ERROR
                     type: string
+                  logMutations:
+                    description: Sets the --log-mutations flag which enables logging
+                      of mutation events and errors. This defaults to Disabled.
+                    enum:
+                    - Enabled
+                    - Disabled
+                    type: string
+                  mutationAnnotations:
+                    description: Sets the --mutation-annotations flag which adds the
+                      gatekeeper.sh/mutation-id and gatekeeper.sh/mutations annotations
+                      on mutated objects. This defaults to Disabled.
+                    enum:
+                    - Enabled
+                    - Disabled
+                    type: string
                   namespaceSelector:
                     description: A label selector is a label query over a set of resources.
                       The result of matchLabels and matchExpressions are ANDed. An

diff --git a/config/crd/bases/operator.gatekeeper.sh_gatekeepers.yaml b/config/crd/bases/operator.gatekeeper.sh_gatekeepers.yaml
@@ -1052,6 +1052,21 @@ spec:
                     - WARNING
                     - ERROR
                     type: string
+                  logMutations:
+                    description: Sets the --log-mutations flag which enables logging
+                      of mutation events and errors. This defaults to Disabled.
+                    enum:
+                    - Enabled
+                    - Disabled
+                    type: string
+                  mutationAnnotations:
+                    description: Sets the --mutation-annotations flag which adds the
+                      gatekeeper.sh/mutation-id and gatekeeper.sh/mutations annotations
+                      on mutated objects. This defaults to Disabled.
+                    enum:
+                    - Enabled
+                    - Disabled
+                    type: string
                   namespaceSelector:
                     description: A label selector is a label query over a set of resources.
                       The result of matchLabels and matchExpressions are ANDed. An

diff --git a/config/samples/gatekeeper_e2e_test.yaml b/config/samples/gatekeeper_e2e_test.yaml
@@ -15,3 +15,5 @@ spec:
     admissionEventsInvolvedNamespace: Enabled
     disabledBuiltins:
       - http.send
+    logMutations: Enabled
+    mutationAnnotations: Enabled
diff --git a/controllers/gatekeeper_controller.go b/controllers/gatekeeper_controller.go
@@ -88,6 +88,8 @@ const (
 	OperationMutationStatus             = "mutation-status"
 	OperationMutationWebhook            = "mutation-webhook"
 	DisabledBuiltinArg                  = "--disable-opa-builtin"
+	LogMutationsArg                     = "--log-mutations"
+	MutationAnnotationsArg              = "--mutation-annotations"
 )
 
 var (
@@ -659,6 +661,10 @@ func webhookOverrides(obj *unstructured.Unstructured, webhook *operatorv1alpha1.
 		if err := setDisabledBuiltins(obj, webhook.DisabledBuiltins); err != nil {
 			return err
 		}
+
+		if err := setMutationFlags(obj, webhook); err != nil {
+			return err
+		}
 	}
 
 	return nil
@@ -833,6 +839,27 @@ func setLogLevel(obj *unstructured.Unstructured, logLevel *operatorv1alpha1.LogL
 	return nil
 }
 
+func setMutationFlags(obj *unstructured.Unstructured, webhookConfig *operatorv1alpha1.WebhookConfig) error {
+	if webhookConfig == nil {
+		return nil
+	}
+
+	if webhookConfig.LogMutations != nil && webhookConfig.LogMutations.ToBool() {
+		err := setContainerArg(obj, managerContainer, LogMutationsArg, webhookConfig.LogMutations.ToBoolString(), false)
+		if err != nil {
+			return err
+		}
+	}
+
+	if webhookConfig.MutationAnnotations != nil && webhookConfig.MutationAnnotations.ToBool() {
+		return setContainerArg(
+			obj, managerContainer, MutationAnnotationsArg, webhookConfig.MutationAnnotations.ToBoolString(), false,
+		)
+	}
+
+	return nil
+}
+
 func setAuditInterval(obj *unstructured.Unstructured, auditInterval *metav1.Duration) error {
 	if auditInterval != nil {
 		return setContainerArg(obj, managerContainer, AuditIntervalArg, fmt.Sprint(auditInterval.Round(time.Second).Seconds()), false)

diff --git a/controllers/gatekeeper_controller_test.go b/controllers/gatekeeper_controller_test.go
@@ -1321,13 +1321,17 @@ func TestAllWebhookArgs(t *testing.T) {
 	expectObjContainerArgument(g, managerContainer, webhookObj).NotTo(HaveKey(AdmissionEventsInvolvedNamespaceArg))
 	expectObjContainerArgument(g, managerContainer, webhookObj).NotTo(HaveKey(LogLevelArg))
 	expectObjContainerArgument(g, managerContainer, webhookObj).NotTo(HaveKey(EnableMutationArg))
+	expectObjContainerArgument(g, managerContainer, webhookObj).NotTo(HaveKey(LogMutationsArg))
+	expectObjContainerArgument(g, managerContainer, webhookObj).NotTo(HaveKey(MutationAnnotationsArg))
 	// test nil
 	err = crOverrides(gatekeeper, WebhookFile, webhookObj, namespace, false, false)
 	g.Expect(err).ToNot(HaveOccurred())
 	expectObjContainerArgument(g, managerContainer, webhookObj).NotTo(HaveKey(EmitAdmissionEventsArg))
 	expectObjContainerArgument(g, managerContainer, webhookObj).NotTo(HaveKey(AdmissionEventsInvolvedNamespaceArg))
 	expectObjContainerArgument(g, managerContainer, webhookObj).NotTo(HaveKey(LogLevelArg))
 	expectObjContainerArgument(g, managerContainer, webhookObj).NotTo(HaveKey(EnableMutationArg))
+	expectObjContainerArgument(g, managerContainer, webhookObj).NotTo(HaveKey(LogMutationsArg))
+	expectObjContainerArgument(g, managerContainer, webhookObj).NotTo(HaveKey(MutationAnnotationsArg))
 	// test override without mutation
 	gatekeeper.Spec.Webhook = &webhookOverride
 	err = crOverrides(gatekeeper, WebhookFile, webhookObj, namespace, false, false)
@@ -1336,15 +1340,32 @@ func TestAllWebhookArgs(t *testing.T) {
 	expectObjContainerArgument(g, managerContainer, webhookObj).To(HaveKeyWithValue(AdmissionEventsInvolvedNamespaceArg, "true"))
 	expectObjContainerArgument(g, managerContainer, webhookObj).To(HaveKeyWithValue(LogLevelArg, "DEBUG"))
 	expectObjContainerArgument(g, managerContainer, webhookObj).NotTo(HaveKey(EnableMutationArg))
+	expectObjContainerArgument(g, managerContainer, webhookObj).NotTo(HaveKey(LogMutationsArg))
+	expectObjContainerArgument(g, managerContainer, webhookObj).NotTo(HaveKey(MutationAnnotationsArg))
 	// test override with mutation
-	mutatingWebhook := operatorv1alpha1.Enabled
-	gatekeeper.Spec.MutatingWebhook = &mutatingWebhook
+	enabled := operatorv1alpha1.Enabled
+	gatekeeper.Spec.MutatingWebhook = &enabled
+	err = crOverrides(gatekeeper, WebhookFile, webhookObj, namespace, false, false)
+	g.Expect(err).ToNot(HaveOccurred())
+	expectObjContainerArgument(g, managerContainer, webhookObj).To(HaveKeyWithValue(EmitAdmissionEventsArg, "true"))
+	expectObjContainerArgument(g, managerContainer, webhookObj).To(HaveKeyWithValue(AdmissionEventsInvolvedNamespaceArg, "true"))
+	expectObjContainerArgument(g, managerContainer, webhookObj).To(HaveKeyWithValue(LogLevelArg, "DEBUG"))
+	expectObjContainerArgument(g, managerContainer, webhookObj).To(HaveKeyWithValue(OperationArg, OperationMutationWebhook))
+	expectObjContainerArgument(g, managerContainer, webhookObj).NotTo(HaveKey(LogMutationsArg))
+	expectObjContainerArgument(g, managerContainer, webhookObj).NotTo(HaveKey(MutationAnnotationsArg))
+
+	// test override with mutation flags
+	gatekeeper.Spec.MutatingWebhook = &enabled
+	gatekeeper.Spec.Webhook.LogMutations = &enabled
+	gatekeeper.Spec.Webhook.MutationAnnotations = &enabled
 	err = crOverrides(gatekeeper, WebhookFile, webhookObj, namespace, false, false)
 	g.Expect(err).ToNot(HaveOccurred())
 	expectObjContainerArgument(g, managerContainer, webhookObj).To(HaveKeyWithValue(EmitAdmissionEventsArg, "true"))
 	expectObjContainerArgument(g, managerContainer, webhookObj).To(HaveKeyWithValue(AdmissionEventsInvolvedNamespaceArg, "true"))
 	expectObjContainerArgument(g, managerContainer, webhookObj).To(HaveKeyWithValue(LogLevelArg, "DEBUG"))
 	expectObjContainerArgument(g, managerContainer, webhookObj).To(HaveKeyWithValue(OperationArg, OperationMutationWebhook))
+	expectObjContainerArgument(g, managerContainer, webhookObj).To(HaveKeyWithValue(LogMutationsArg, "true"))
+	expectObjContainerArgument(g, managerContainer, webhookObj).To(HaveKeyWithValue(MutationAnnotationsArg, "true"))
 }
 
 func expectObjContainerArgument(g *WithT, containerName string, obj *unstructured.Unstructured) Assertion {