feat: address PR feedback, add optional flag to emit events in involv…

…ed objects namespace

Signed-off-by: Craig Trought <[email protected]>

Makefile

@@ -67,6 +67,7 @@ MANAGER_IMAGE_PATCH := "apiVersion: apps/v1\
 \n        - --port=8443\
 \n        - --logtostderr\
 \n        - --emit-admission-events\
+\n        - --admission-events-involved-namespace\
 \n        - --exempt-namespace=${GATEKEEPER_NAMESPACE}\
 \n        - --operation=webhook\
 \n        - --operation=mutation-webhook\
@@ -87,6 +88,7 @@ MANAGER_IMAGE_PATCH := "apiVersion: apps/v1\
 \n        name: manager\
 \n        args:\
 \n        - --emit-audit-events\
+\n        - --audit-events-involved-namespace\
 \n        - --operation=audit\
 \n        - --operation=status\
 \n        - --operation=mutation-status\
@@ -190,6 +192,8 @@ e2e-helm-deploy: e2e-helm-install
 		--set postInstall.probeWebhook.enabled=true \
 		--set emitAdmissionEvents=true \
 		--set emitAuditEvents=true \
+		--set admissionEventsInvolvedNamespace=true \
+		--set auditEventsInvolvedNamespace=true \
 		--set disabledBuiltins={http.send} \
 		--set logMutations=true \
 		--set mutationAnnotations=true;\
@@ -201,6 +205,8 @@ e2e-helm-upgrade-init: e2e-helm-install
 		--debug --wait \
 		--set emitAdmissionEvents=true \
 		--set emitAuditEvents=true \
+		--set admissionEventsInvolvedNamespace=true \
+		--set auditEventsInvolvedNamespace=true \
 		--set postInstall.labelNamespace.enabled=true \
 		--set postInstall.probeWebhook.enabled=true \
 		--set disabledBuiltins={http.send} \
@@ -222,6 +228,8 @@ e2e-helm-upgrade:
 		--set postInstall.probeWebhook.enabled=true \
 		--set emitAdmissionEvents=true \
 		--set emitAuditEvents=true \
+		--set admissionEventsInvolvedNamespace=true \
+		--set auditEventsInvolvedNamespace=true \
 		--set disabledBuiltins={http.send} \
 		--set logMutations=true \
 		--set mutationAnnotations=true;\

cmd/build/helmify/kustomize-for-helm.yaml

@@ -76,6 +76,7 @@ spec:
             - --logtostderr
             - --log-denies={{ .Values.logDenies }}
             - --emit-admission-events={{ .Values.emitAdmissionEvents }}
+            - --admission-events-involved-namespace={{ .Values.admissionEventsInvolvedNamespace }}
             - --log-level={{ (.Values.controllerManager.logLevel | empty | not) | ternary .Values.controllerManager.logLevel .Values.logLevel }}
             - --exempt-namespace={{ .Release.Namespace }}
             - --operation=webhook
@@ -156,6 +157,7 @@ spec:
             - --audit-chunk-size={{ .Values.auditChunkSize }}
             - --audit-match-kind-only={{ .Values.auditMatchKindOnly }}
             - --emit-audit-events={{ .Values.emitAuditEvents }}
+            - --audit-events-involved-namespace={{ .Values.auditEventsInvolvedNamespace }}
             - --operation=audit
             - --operation=status
             - HELMSUBST_MUTATION_STATUS_ENABLED_ARG

cmd/build/helmify/static/README.md

@@ -145,6 +145,8 @@ _See [Exempting Namespaces](https://open-policy-agent.github.io/gatekeeper/websi
 | mutatingWebhookCustomRules                    | Custom rules for selecting which API resources trigger the webhook. NOTE: If you change this, ensure all your constraints are still being enforced.                                                                                                 | `{}`                                                                      |
 | emitAdmissionEvents                           | Emit K8s events in the involved namespace for admission violations (alpha feature)                                                                                                                                                                    | `false`                                                                   |
 | emitAuditEvents                               | Emit K8s events in the involved namespace for audit violations (alpha feature)                                                                                                                                                                        | `false`                                                                   |
+| auditEventsInvolvedNamespace                               | Emit admission events for each violation in the involved objects namespace, the default (false) generates events in the namespace gatekeeper is installed in                                                                                                                                                                        | `false`                                                                   |
+| admissionEventsInvolvedNamespace                               | Emit admission events for each violation in the involved objects namespace, the default (false) generates events in the namespace gatekeeper is installed in                                                                                                                                                                        | `false`                                                                   |
 | logDenies                                     | Log detailed info on each deny                                                                                                                                                                                                                      | `false`                                                                   |
 | logLevel                                      | Minimum log level                                                                                                                                                                                                                                   | `INFO`                                                                    |
 | image.pullPolicy                              | The image pull policy                                                                                                                                                                                                                               | `IfNotPresent`                                                            |

cmd/build/helmify/static/values.yaml

@@ -32,6 +32,8 @@ logDenies: false
 logMutations: false
 emitAdmissionEvents: false
 emitAuditEvents: false
+admissionEventsInvolvedNamespace: false
+auditEventsInvolvedNamespace: false
 resourceQuota: true
 postUpgrade:
   labelNamespace:

config/rbac/role.yaml

@@ -5,6 +5,13 @@ metadata:
   creationTimestamp: null
   name: manager-role
 rules:
+- apiGroups:
+  - ""
+  resources:
+  - events
+  verbs:
+  - create
+  - patch
 - apiGroups:
   - '*'
   resources:

manifest_staging/charts/gatekeeper/README.md

@@ -143,8 +143,10 @@ _See [Exempting Namespaces](https://open-policy-agent.github.io/gatekeeper/websi
 | mutatingWebhookObjectSelector                 | The label selector to further refine which namespaced resources will be selected by the webhook. Please note that an exemption label means users can circumvent Gatekeeper's mutation webhook unless measures are taken to control how exemption labels can be set.                                          | `{}`                                                                      |
 | mutatingWebhookTimeoutSeconds                 | The timeout for the mutating webhook in seconds                                                                                                                                                                                                     | `3`                                                                       |
 | mutatingWebhookCustomRules                    | Custom rules for selecting which API resources trigger the webhook. NOTE: If you change this, ensure all your constraints are still being enforced.                                                                                                 | `{}`                                                                      |
-| emitAdmissionEvents                           | Emit K8s events in gatekeeper namespace for admission violations (alpha feature)                                                                                                                                                                    | `false`                                                                   |
-| emitAuditEvents                               | Emit K8s events in gatekeeper namespace for audit violations (alpha feature)                                                                                                                                                                        | `false`                                                                   |
+| emitAdmissionEvents                           | Emit K8s events in the involved namespace for admission violations (alpha feature)                                                                                                                                                                    | `false`                                                                   |
+| emitAuditEvents                               | Emit K8s events in the involved namespace for audit violations (alpha feature)                                                                                                                                                                        | `false`                                                                   |
+| auditEventsInvolvedNamespace                               | Emit admission events for each violation in the involved objects namespace, the default (false) generates events in the namespace gatekeeper is installed in                                                                                                                                                                        | `false`                                                                   |
+| admissionEventsInvolvedNamespace                               | Emit admission events for each violation in the involved objects namespace, the default (false) generates events in the namespace gatekeeper is installed in                                                                                                                                                                        | `false`                                                                   |
 | logDenies                                     | Log detailed info on each deny                                                                                                                                                                                                                      | `false`                                                                   |
 | logLevel                                      | Minimum log level                                                                                                                                                                                                                                   | `INFO`                                                                    |
 | image.pullPolicy                              | The image pull policy                                                                                                                                                                                                                               | `IfNotPresent`                                                            |

manifest_staging/charts/gatekeeper/templates/gatekeeper-audit-deployment.yaml

@@ -55,6 +55,7 @@ spec:
         - --audit-chunk-size={{ .Values.auditChunkSize }}
         - --audit-match-kind-only={{ .Values.auditMatchKindOnly }}
         - --emit-audit-events={{ .Values.emitAuditEvents }}
+        - --audit-events-involved-namespace={{ .Values.auditEventsInvolvedNamespace }}
         - --operation=audit
         - --operation=status
         {{ if not .Values.disableMutation}}- --operation=mutation-status{{- end }}

manifest_staging/charts/gatekeeper/templates/gatekeeper-controller-manager-deployment.yaml

@@ -54,6 +54,7 @@ spec:
         - --logtostderr
         - --log-denies={{ .Values.logDenies }}
         - --emit-admission-events={{ .Values.emitAdmissionEvents }}
+        - --admission-events-involved-namespace={{ .Values.admissionEventsInvolvedNamespace }}
         - --log-level={{ (.Values.controllerManager.logLevel | empty | not) | ternary .Values.controllerManager.logLevel .Values.logLevel }}
         - --exempt-namespace={{ .Release.Namespace }}
         - --operation=webhook

manifest_staging/charts/gatekeeper/templates/gatekeeper-manager-role-clusterrole.yaml

@@ -11,6 +11,13 @@ metadata:
     release: '{{ .Release.Name }}'
   name: gatekeeper-manager-role
 rules:
+- apiGroups:
+  - ""
+  resources:
+  - events
+  verbs:
+  - create
+  - patch
 - apiGroups:
   - '*'
   resources:

manifest_staging/charts/gatekeeper/values.yaml

@@ -32,6 +32,8 @@ logDenies: false
 logMutations: false
 emitAdmissionEvents: false
 emitAuditEvents: false
+admissionEventsInvolvedNamespace: false
+auditEventsInvolvedNamespace: false
 resourceQuota: true
 postUpgrade:
   labelNamespace:

manifest_staging/deploy/gatekeeper.yaml

@@ -2930,6 +2930,13 @@ metadata:
     gatekeeper.sh/system: "yes"
   name: gatekeeper-manager-role
 rules:
+- apiGroups:
+  - ""
+  resources:
+  - events
+  verbs:
+  - create
+  - patch
 - apiGroups:
   - '*'
   resources:

pkg/audit/manager_test.go

@@ -9,6 +9,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/types"
 )
 
 func Test_newNSCache(t *testing.T) {
@@ -183,13 +184,11 @@ func Test_nsMapFromObjs(t *testing.T) {
 
 func Test_getViolationRef(t *testing.T) {
 	type args struct {
-		gkNamespace string
+		enamespace string
 		rkind       string
 		rname       string
-		rnamespace  string
-		ckind       string
-		cname       string
-		cnamespace  string
+		rrv 		string
+		ruid	    types.UID
 	}
 	tests := []struct {
 		name string
@@ -199,43 +198,41 @@ func Test_getViolationRef(t *testing.T) {
 		{
 			name: "Test case 1",
 			args: args{
-				gkNamespace: "default",
 				rkind:       "Pod",
 				rname:       "my-pod",
-				rnamespace:  "default",
-				ckind:       "LimitRange",
-				cname:       "my-limit-range",
-				cnamespace:  "default",
+				enamespace:  "default",
+				rrv:       	 "123456",
+				ruid:  		 "abcde-123456",
 			},
 			want: &corev1.ObjectReference{
-				Kind:      "Pod",
-				Name:      "my-pod",
-				UID:       "Pod/default/my-pod/LimitRange/default/my-limit-range",
-				Namespace: "default",
+				Kind:      		 "Pod",
+				Name:      		 "my-pod",
+				Namespace: 	 	 "default",
+				ResourceVersion: "123456",
+				UID:       		 "abcde-123456",
 			},
 		},
 		{
 			name: "Test case 2",
 			args: args{
-				gkNamespace: "kube-system",
 				rkind:       "Service",
+				enamespace:  "kube-system",
 				rname:       "my-service",
-				rnamespace:  "default",
-				ckind:       "PodSecurityPolicy",
-				cname:       "my-pod-security-policy",
-				cnamespace:  "kube-system",
+				rrv:       	 "123456",
+				ruid:  		 "abcde-123456",
 			},
 			want: &corev1.ObjectReference{
-				Kind:      "Service",
-				Name:      "my-service",
-				UID:       "Service/default/my-service/PodSecurityPolicy/kube-system/my-pod-security-policy",
-				Namespace: "kube-system",
+				Kind:      		 "Service",
+				Name:     		 "my-service",
+				Namespace: 		 "kube-system",
+				ResourceVersion: "123456",
+				UID:       		 "abcde-123456",
 			},
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if got := getViolationRef(tt.args.gkNamespace, tt.args.rkind, tt.args.rname, tt.args.rnamespace, tt.args.ckind, tt.args.cname, tt.args.cnamespace); !reflect.DeepEqual(got, tt.want) {
+			if got := getViolationRef(tt.args.enamespace, tt.args.rkind, tt.args.rname, tt.args.rrv, tt.args.ruid); !reflect.DeepEqual(got, tt.want) {
 				t.Errorf("getViolationRef() = %v, want %v", got, tt.want)
 			}
 		})

pkg/webhook/common.go

@@ -58,7 +58,8 @@ var (
 	deserializer                       = codecs.UniversalDeserializer()
 	disableEnforcementActionValidation = flag.Bool("disable-enforcementaction-validation", false, "disable validation of the enforcementAction field of a constraint")
 	logDenies                          = flag.Bool("log-denies", false, "log detailed info on each deny")
-	emitAdmissionEvents                = flag.Bool("emit-admission-events", false, "(alpha) emit Kubernetes events in the involved namespace for each admission violation")
+	emitAdmissionEvents                = flag.Bool("emit-admission-events", false, "(alpha) emit Kubernetes events for each admission violation")
+	admissionEventsInvolvedNamespace   = flag.Bool("admission-events-involved-namespace", false, "emit admission events for each violation in the involved objects namespace, the default (false) generates events in the namespace gatekeeper is installed in")
 	tlsMinVersion                      = flag.String("tls-min-version", "1.3", "minimum version of TLS supported")
 	serviceaccount                     = fmt.Sprintf("system:serviceaccount:%s:%s", util.GetNamespace(), serviceAccountName)
 	clientCAName                       = flag.String("client-ca-name", "", "name of the certificate authority bundle to authenticate the Kubernetes API server requests against")

pkg/webhook/policy.go

@@ -229,14 +229,17 @@ func (h *validationHandler) Handle(ctx context.Context, req admission.Request) a
 func (h *validationHandler) getValidationMessages(res []*rtypes.Result, req *admission.Request) ([]string, []string) {
 	var denyMsgs, warnMsgs []string
 	var resourceName string
+	obj := &unstructured.Unstructured{}
+
 	if len(res) > 0 && (*logDenies || *emitAdmissionEvents) {
 		resourceName = req.AdmissionRequest.Name
-		if len(resourceName) == 0 && req.AdmissionRequest.Object.Raw != nil {
-			// On a CREATE operation, the client may omit name and
-			// rely on the server to generate the name.
-			obj := &unstructured.Unstructured{}
+		if req.AdmissionRequest.Object.Raw != nil {
 			if _, _, err := deserializer.Decode(req.AdmissionRequest.Object.Raw, nil, obj); err == nil {
-				resourceName = obj.GetName()
+				// On a CREATE operation, the client may omit name and
+				// rely on the server to generate the name.
+				if len(resourceName) == 0 {
+					resourceName = obj.GetName()
+				}
 			}
 		}
 	}
@@ -289,23 +292,19 @@ func (h *validationHandler) getValidationMessages(res []*rtypes.Result, req *adm
 				eventMsg = "Admission webhook \"validation.gatekeeper.sh\" denied request"
 				reason = "FailedAdmission"
 			}
-			ref := getViolationRef(
-				req.AdmissionRequest.Kind.Kind,
-				resourceName,
-				req.AdmissionRequest.Namespace,
-				r.Constraint.GetKind(),
-				r.Constraint.GetName(),
-				r.Constraint.GetNamespace())
-			h.eventRecorder.AnnotatedEventf(
-				ref,
-				annotations,
-				corev1.EventTypeWarning,
-				reason,
-				"%s, Resource Namespace: %s, Constraint: %s, Message: %s",
-				eventMsg,
-				req.AdmissionRequest.Namespace,
-				r.Constraint.GetName(),
-				r.Msg)
+
+			enamespace := h.gkNamespace
+			if *admissionEventsInvolvedNamespace && len(req.AdmissionRequest.Namespace) > 0 {
+				enamespace = req.AdmissionRequest.Namespace
+			}
+
+			ref := getViolationRef(enamespace, req.AdmissionRequest.Kind.Kind, resourceName, obj.GetResourceVersion(), obj.GetUID())
+
+			if *admissionEventsInvolvedNamespace || len(req.AdmissionRequest.Namespace) == 0 {
+				h.eventRecorder.AnnotatedEventf(ref, annotations, corev1.EventTypeWarning, reason, "%s, Constraint: %s, Message: %s", eventMsg, r.Constraint.GetName(), r.Msg)
+			} else {
+				h.eventRecorder.AnnotatedEventf(ref, annotations, corev1.EventTypeWarning, reason, "%s, Resource Namespace: %s, Constraint: %s, Message: %s", eventMsg, req.AdmissionRequest.Namespace, r.Constraint.GetName(), r.Msg)
+			}
 		}
 
 		if r.EnforcementAction == string(util.Deny) {
@@ -603,13 +602,17 @@ func createReviewForResultant(obj *unstructured.Unstructured, ns *corev1.Namespa
 	}
 }
 
-func getViolationRef(rkind, rname, rnamespace, ckind, cname, cnamespace string) *corev1.ObjectReference {
-	return &corev1.ObjectReference{
+func getViolationRef(enamespace, rkind, rname, rrv string, ruid types.UID) *corev1.ObjectReference {
+	ref := &corev1.ObjectReference{
 		Kind:      rkind,
 		Name:      rname,
-		UID:       types.UID(rkind + "/" + rnamespace + "/" + rname + "/" + ckind + "/" + cnamespace + "/" + cname),
-		Namespace: rnamespace,
+		Namespace: enamespace,
+	}
+	if len(ruid) > 0 && len(rrv) > 0 {
+		ref.UID = ruid
+		ref.ResourceVersion = rrv
 	}
+	return ref
 }
 
 func AppendValidationWebhookIfEnabled(webhooks []rotator.WebhookInfo) []rotator.WebhookInfo {