fix: namespaceSelector for background policies (#6188)

Makefile

@@ -211,9 +211,9 @@ $(REPORTS_BIN): fmt vet
 	@echo Build reports controller binary... >&2
 	@CGO_ENABLED=$(CGO_ENABLED) GOOS=$(GOOS) go build -o ./$(REPORTS_BIN) -ldflags=$(LD_FLAGS) ./$(REPORTS_DIR)
 
-$(REPORTS_BIN): fmt vet
+$(BACKGROUND_BIN): fmt vet
 	@echo Build background controller binary... >&2
-	@CGO_ENABLED=$(CGO_ENABLED) GOOS=$(GOOS) go build -o $(BACKGROUND_BIN) -ldflags=$(LD_FLAGS) $(BACKGROUND_DIR)
+	@CGO_ENABLED=$(CGO_ENABLED) GOOS=$(GOOS) go build -o ./$(BACKGROUND_BIN) -ldflags=$(LD_FLAGS) ./$(BACKGROUND_DIR)
 
 .PHONY: build-kyverno-init
 build-kyverno-init: $(KYVERNOPRE_BIN) ## Build kyvernopre binary
@@ -344,7 +344,7 @@ ko-publish-reports-controller-dev: ko-login ## Build and publish reports control
 
 .PHONY: ko-publish-background-controller-dev
 ko-publish-background-controller-dev: ko-login ## Build and publish background controller dev image (with ko)
-	@LD_FLAGS=$(LD_FLAGS_DEV) KOCACHE=$(KOCACHE) KO_DOCKER_REPO=$(REPO_REPORTS) $(KO) build $(BACKGROUND_DIR) --bare --tags=$(KO_TAGS_DEV) --platform=$(PLATFORMS)
+	@LD_FLAGS=$(LD_FLAGS_DEV) KOCACHE=$(KOCACHE) KO_DOCKER_REPO=$(REPO_BACKGROUND) $(KO) build $(BACKGROUND_DIR) --bare --tags=$(KO_TAGS_DEV) --platform=$(PLATFORMS)
 
 
 .PHONY: ko-publish-all

pkg/background/mutate/mutate.go

@@ -16,10 +16,12 @@ import (
 	engineapi "github.com/kyverno/kyverno/pkg/engine/api"
 	"github.com/kyverno/kyverno/pkg/event"
 	"github.com/kyverno/kyverno/pkg/utils"
+	engineutils "github.com/kyverno/kyverno/pkg/utils/engine"
 	"go.uber.org/multierr"
 	yamlv2 "gopkg.in/yaml.v2"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	corev1listers "k8s.io/client-go/listers/core/v1"
 	"k8s.io/client-go/tools/cache"
 )
 
@@ -34,6 +36,7 @@ type MutateExistingController struct {
 	// listers
 	policyLister  kyvernov1listers.ClusterPolicyLister
 	npolicyLister kyvernov1listers.PolicyLister
+	nsLister      corev1listers.NamespaceLister
 
 	configuration          config.Configuration
 	informerCacheResolvers engineapi.ConfigmapResolver
@@ -49,6 +52,7 @@ func NewMutateExistingController(
 	contextLoader engine.ContextLoaderFactory,
 	policyLister kyvernov1listers.ClusterPolicyLister,
 	npolicyLister kyvernov1listers.PolicyLister,
+	nsLister corev1listers.NamespaceLister,
 	dynamicConfig config.Configuration,
 	informerCacheResolvers engineapi.ConfigmapResolver,
 	eventGen event.Interface,
@@ -60,6 +64,7 @@ func NewMutateExistingController(
 		contextLoader:          contextLoader,
 		policyLister:           policyLister,
 		npolicyLister:          npolicyLister,
+		nsLister:               nsLister,
 		configuration:          dynamicConfig,
 		informerCacheResolvers: informerCacheResolvers,
 		eventGen:               eventGen,
@@ -84,13 +89,14 @@ func (c *MutateExistingController) ProcessUR(ur *kyvernov1beta1.UpdateRequest) e
 		}
 
 		trigger, err := common.GetResource(c.client, ur.Spec, c.log)
-		if err != nil {
+		if err != nil || trigger == nil {
 			logger.WithName(rule.Name).Error(err, "failed to get trigger resource")
 			errs = append(errs, err)
 			continue
 		}
 
-		policyContext, _, err := common.NewBackgroundContext(c.client, ur, policy, trigger, c.configuration, c.informerCacheResolvers, nil, logger)
+		namespaceLabels := engineutils.GetNamespaceSelectorsFromNamespaceLister(trigger.GetKind(), trigger.GetNamespace(), c.nsLister, logger)
+		policyContext, _, err := common.NewBackgroundContext(c.client, ur, policy, trigger, c.configuration, c.informerCacheResolvers, namespaceLabels, logger)
 		if err != nil {
 			logger.WithName(rule.Name).Error(err, "failed to build policy context")
 			errs = append(errs, err)

pkg/background/update_request_controller.go

@@ -419,7 +419,7 @@ func (c *controller) processUR(ur *kyvernov1beta1.UpdateRequest) error {
 	statusControl := common.NewStatusControl(c.kyvernoClient, c.urLister)
 	switch ur.Spec.Type {
 	case kyvernov1beta1.Mutate:
-		ctrl := mutate.NewMutateExistingController(c.client, statusControl, c.contextLoader, c.cpolLister, c.polLister, c.configuration, c.informerCacheResolvers, c.eventGen, logger)
+		ctrl := mutate.NewMutateExistingController(c.client, statusControl, c.contextLoader, c.cpolLister, c.polLister, c.nsLister, c.configuration, c.informerCacheResolvers, c.eventGen, logger)
 		return ctrl.ProcessUR(ur)
 	case kyvernov1beta1.Generate:
 		ctrl := generate.NewGenerateController(c.client, c.kyvernoClient, statusControl, c.contextLoader, c.cpolLister, c.polLister, c.urLister, c.nsLister, c.configuration, c.informerCacheResolvers, c.eventGen, logger)

pkg/policy/policy_controller.go

@@ -25,6 +25,7 @@ import (
 	engineapi "github.com/kyverno/kyverno/pkg/engine/api"
 	"github.com/kyverno/kyverno/pkg/event"
 	"github.com/kyverno/kyverno/pkg/metrics"
+	engineutils "github.com/kyverno/kyverno/pkg/utils/engine"
 	kubeutils "github.com/kyverno/kyverno/pkg/utils/kube"
 	"go.uber.org/multierr"
 	corev1 "k8s.io/api/core/v1"
@@ -507,7 +508,8 @@ func (pc *PolicyController) updateUR(policyKey string, policy kyvernov1.PolicyIn
 }
 
 func (pc *PolicyController) handleUpdateRequest(ur *kyvernov1beta1.UpdateRequest, triggerResource *unstructured.Unstructured, rule kyvernov1.Rule, policy kyvernov1.PolicyInterface) (skip bool, err error) {
-	policyContext, _, err := backgroundcommon.NewBackgroundContext(pc.client, ur, policy, triggerResource, pc.configHandler, pc.informerCacheResolvers, nil, pc.log)
+	namespaceLabels := engineutils.GetNamespaceSelectorsFromNamespaceLister(triggerResource.GetKind(), triggerResource.GetNamespace(), pc.nsLister, pc.log)
+	policyContext, _, err := backgroundcommon.NewBackgroundContext(pc.client, ur, policy, triggerResource, pc.configHandler, pc.informerCacheResolvers, namespaceLabels, pc.log)
 	if err != nil {
 		return false, fmt.Errorf("failed to build policy context for rule %s: %w", rule.Name, err)
 	}

pkg/webhooks/resource/handlers.go

@@ -139,10 +139,10 @@ func (h *handlers) Validate(ctx context.Context, logger logr.Logger, request *ad
 	if request.Kind.Kind != "Namespace" && request.Namespace != "" {
 		namespaceLabels = engineutils.GetNamespaceSelectorsFromNamespaceLister(request.Kind.Kind, request.Namespace, h.nsLister, logger)
 	}
-
+	policyContext = policyContext.WithNamespaceLabels(namespaceLabels)
 	vh := validation.NewValidationHandler(logger, h.kyvernoClient, h.contextLoader, h.pCache, h.pcBuilder, h.eventGen, h.admissionReports, h.metricsConfig, h.configuration)
 
-	ok, msg, warnings := vh.HandleValidation(ctx, request, policies, policyContext, namespaceLabels, startTime)
+	ok, msg, warnings := vh.HandleValidation(ctx, request, policies, policyContext, startTime)
 	if !ok {
 		logger.Info("admission request denied")
 		return admissionutils.Response(request.UID, errors.New(msg), warnings...)

pkg/webhooks/resource/validation/validation.go

@@ -31,7 +31,7 @@ type ValidationHandler interface {
 	// HandleValidation handles validating webhook admission request
 	// If there are no errors in validating rule we apply generation rules
 	// patchedResource is the (resource + patches) after applying mutation rules
-	HandleValidation(context.Context, *admissionv1.AdmissionRequest, []kyvernov1.PolicyInterface, *engine.PolicyContext, map[string]string, time.Time) (bool, string, []string)
+	HandleValidation(context.Context, *admissionv1.AdmissionRequest, []kyvernov1.PolicyInterface, *engine.PolicyContext, time.Time) (bool, string, []string)
 }
 
 func NewValidationHandler(
@@ -75,15 +75,8 @@ func (v *validationHandler) HandleValidation(
 	request *admissionv1.AdmissionRequest,
 	policies []kyvernov1.PolicyInterface,
 	policyContext *engine.PolicyContext,
-	namespaceLabels map[string]string,
 	admissionRequestTimestamp time.Time,
 ) (bool, string, []string) {
-	if len(policies) == 0 {
-		// invoke handleAudit as we may have some policies in audit mode to consider
-		go v.handleAudit(ctx, policyContext.NewResource(), request, namespaceLabels)
-		return true, "", nil
-	}
-
 	resourceName := admissionutils.GetResourceName(request)
 	logger := v.log.WithValues("action", "validate", "resource", resourceName, "operation", request.Operation, "gvk", request.Kind)
 
@@ -108,7 +101,7 @@ func (v *validationHandler) HandleValidation(
 			"pkg/webhooks/resource/validate",
 			fmt.Sprintf("POLICY %s/%s", policy.GetNamespace(), policy.GetName()),
 			func(ctx context.Context, span trace.Span) {
-				policyContext := policyContext.WithPolicy(policy).WithNamespaceLabels(namespaceLabels)
+				policyContext := policyContext.WithPolicy(policy)
 				if policy.GetSpec().GetFailurePolicy() == kyvernov1.Fail {
 					failurePolicy = kyvernov1.Fail
 				}
@@ -147,7 +140,7 @@ func (v *validationHandler) HandleValidation(
 		return false, webhookutils.GetBlockedMessages(engineResponses), nil
 	}
 
-	go v.handleAudit(ctx, policyContext.NewResource(), request, namespaceLabels, engineResponses...)
+	go v.handleAudit(ctx, policyContext.NewResource(), request, engineResponses...)
 
 	warnings := webhookutils.GetWarningMessages(engineResponses)
 	return true, "", warnings
@@ -157,7 +150,6 @@ func (v *validationHandler) buildAuditResponses(
 	ctx context.Context,
 	resource unstructured.Unstructured,
 	request *admissionv1.AdmissionRequest,
-	namespaceLabels map[string]string,
 ) ([]*engineapi.EngineResponse, error) {
 	policies := v.pCache.GetPolicies(policycache.ValidateAudit, request.Kind.Kind, request.Namespace)
 	policyContext, err := v.pcBuilder.Build(request)
@@ -171,7 +163,7 @@ func (v *validationHandler) buildAuditResponses(
 			"pkg/webhooks/resource/validate",
 			fmt.Sprintf("POLICY %s/%s", policy.GetNamespace(), policy.GetName()),
 			func(ctx context.Context, span trace.Span) {
-				policyContext := policyContext.WithPolicy(policy).WithNamespaceLabels(namespaceLabels)
+				policyContext := policyContext.WithPolicy(policy)
 				responses = append(responses, engine.Validate(ctx, v.contextLoader, policyContext, v.cfg))
 			},
 		)
@@ -183,7 +175,6 @@ func (v *validationHandler) handleAudit(
 	ctx context.Context,
 	resource unstructured.Unstructured,
 	request *admissionv1.AdmissionRequest,
-	namespaceLabels map[string]string,
 	engineResponses ...*engineapi.EngineResponse,
 ) {
 	if !v.admissionReports {
@@ -205,7 +196,7 @@ func (v *validationHandler) handleAudit(
 		"",
 		fmt.Sprintf("AUDIT %s %s", request.Operation, request.Kind),
 		func(ctx context.Context, span trace.Span) {
-			responses, err := v.buildAuditResponses(ctx, resource, request, namespaceLabels)
+			responses, err := v.buildAuditResponses(ctx, resource, request)
 			if err != nil {
 				v.log.Error(err, "failed to build audit responses")
 			}

scripts/config/standard/kyverno.yaml

@@ -30,3 +30,4 @@ backgroundController:
         - namespaces
         - nodes
         - nodes/status
+        - pods

test/conformance/kuttl/mutate/clusterpolicy/standard/existing/basic-delete/01-manifests.yaml

@@ -1,58 +0,0 @@
-apiVersion: v1
-kind: Namespace
-metadata:
-  name: staging-2
-  labels:
-    app-type: corp
-  annotations:
-    cloud.platformzero.com/serviceClass: "xl2"
----
-apiVersion: v1
-data:
-  foo: bar
-kind: ConfigMap
-metadata:
-  name: dictionary-2
-  namespace: staging-2
----
-apiVersion: v1
-data:
-  foo: YmFy
-kind: Secret
-metadata:
-  name: test-secret-2
-  namespace: staging-2
-type: Opaque
----
-apiVersion: kyverno.io/v1
-kind: ClusterPolicy
-metadata:
-  name: test-post-mutation-delete-trigger
-spec:
-  mutateExistingOnPolicyUpdate: false
-  rules:
-    - name: mutate-secret-on-configmap-delete
-      match:
-        any:
-        - resources:
-            kinds:
-            - ConfigMap
-            names:
-            - dictionary-2
-            namespaces:
-            - staging-2
-      preconditions:
-        any:
-        - key: "{{ request.operation }}"
-          operator: Equals
-          value: DELETE
-      mutate:
-        targets:
-        - apiVersion: v1
-          kind: Secret
-          name: test-secret-2
-          namespace: "{{ request.object.metadata.namespace }}"
-        patchStrategicMerge:
-          metadata:
-            labels:
-              foo: "{{ request.object.metadata.name }}"

test/conformance/kuttl/mutate/clusterpolicy/standard/existing/basic-delete/02-delete-cm.yaml

@@ -1,7 +0,0 @@
-apiVersion: kuttl.dev/v1beta1
-kind: TestStep
-delete:
-- apiVersion: v1
-  kind: ConfigMap
-  name: dictionary-2
-  namespace: staging-2

test/conformance/kuttl/mutate/clusterpolicy/standard/existing/basic-delete/03-assert.yaml

@@ -1,7 +0,0 @@
-apiVersion: v1
-kind: Secret
-metadata:
-  name: test-secret-2
-  namespace: staging-2
-  labels:
-    foo: dictionary-2

test/conformance/kuttl/mutate/clusterpolicy/standard/existing/basic-delete/README.md

@@ -1,11 +0,0 @@
-## Description
-
-This is a basic test for the mutate existing capability which ensures that specifically deleting a triggering resource, via a precondition, results in the correct mutation of a different resource.
-
-## Expected Behavior
-
-When the `dictionary-2` ConfigMap is deleted, this should result in the mutation of the Secret named `test-secret-2` within the same Namespace to add the label `foo` with value set to the name or `dictionary-2` in this case. If the Secret is mutated so that the label `foo: dictionary-2` is present, the test passes. If not, the test fails.
-
-## Reference Issue(s)
-
-N/A

test/conformance/kuttl/mutate/clusterpolicy/standard/existing/basic-delete/cleanup.yaml

@@ -1,4 +0,0 @@
-apiVersion: kuttl.dev/v1beta1
-kind: TestStep
-commands:
-  - command: kubectl delete -f 01-manifests.yaml --force --wait=true --ignore-not-found=true

test/conformance/kuttl/mutate/clusterpolicy/standard/existing/namespaceselector/01-policy.yaml

@@ -0,0 +1,6 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+apply:
+- policy.yaml
+assert:
+- policy-assert.yaml

test/conformance/kuttl/mutate/clusterpolicy/standard/existing/namespaceselector/02-pod.yaml

@@ -0,0 +1,6 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+apply:
+- file: pod.yaml
+assert:
+- pod.yaml

test/conformance/kuttl/mutate/clusterpolicy/standard/existing/namespaceselector/03-configmap.yaml

@@ -0,0 +1,6 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+apply:
+- file: configmap.yaml
+assert:
+- configmap.yaml

test/conformance/kuttl/mutate/clusterpolicy/standard/existing/namespaceselector/04-assert.yaml

@@ -0,0 +1,7 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  annotations:
+    org: kyverno-test
+  name: test-org
+  namespace: test

test/conformance/kuttl/mutate/clusterpolicy/standard/existing/namespaceselector/README.md

@@ -0,0 +1,19 @@
+## Description
+
+The `namespaceSelector` should applies to mutateExisting policies upon admission reviews.
+
+## Expected Behavior
+The pod is mutated with annotation `org: kyverno-test`.
+
+## Steps
+
+### Test Steps
+
+1. Create a `ClusterPolicy` that mutates existing pod upon configmap operations in namespaces with label `org`.
+2. Create a pod in `test` namespace labeled by `org: kyverno-test`.
+3. Create a configmap in `test` namespace.
+4. The pod should be mutated with the annotation `org: kyverno-test`.
+
+## Reference Issue(s)
+
+https://github.com/kyverno/kyverno/issues/6176

test/conformance/kuttl/mutate/clusterpolicy/standard/existing/namespaceselector/configmap.yaml

@@ -0,0 +1,5 @@
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: test-org
+  namespace: test

test/conformance/kuttl/mutate/clusterpolicy/standard/existing/namespaceselector/pod.yaml

@@ -0,0 +1,9 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  name: test-org
+  namespace: test
+spec:
+  containers:
+  - image: nginx:latest
+    name: test-org

test/conformance/kuttl/mutate/clusterpolicy/standard/existing/namespaceselector/policy-assert.yaml

@@ -1,7 +1,7 @@
 apiVersion: kyverno.io/v1
 kind: ClusterPolicy
 metadata:
-  name: test-post-mutation-delete-trigger
+  name: org-label-inheritance-existing
 status:
   conditions:
   - reason: Succeeded

test/conformance/kuttl/mutate/clusterpolicy/standard/existing/namespaceselector/policy.yaml

@@ -0,0 +1,42 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: org-label-inheritance-existing
+  annotations:  
+    pod-policies.kyverno.io/autogen-controllers: none
+spec:
+  mutateExistingOnPolicyUpdate: false
+  validationFailureAction: enforce
+  rules:
+  - name: propagate org label from namespace
+    match:
+      any:
+      - resources:
+          kinds:
+          - ConfigMap
+          namespaceSelector:
+            matchExpressions:
+              - key: org
+                operator: Exists
+    context:
+    - name: org
+      apiCall:
+        urlPath: /api/v1/namespaces/{{ request.object.metadata.namespace }}
+        jmesPath: metadata.labels.org
+    mutate:
+      targets:
+      - apiVersion: v1
+        kind: Pod
+        namespace: "{{ request.object.metadata.namespace }}"
+        name: "{{ request.object.metadata.name }}"
+      patchStrategicMerge:
+        metadata:
+          annotations:
+            org: "{{ org }}"
+---
+apiVersion: v1
+kind: Namespace
+metadata:
+  labels:
+    org: kyverno-test
+  name: test

test/conformance/kuttl/mutate/clusterpolicy/standard/existing/onpolicyupdate/namespaceselector/01-assert.yaml

@@ -0,0 +1,22 @@
+apiVersion: v1
+kind: Namespace
+metadata:
+  labels:
+    org: kyverno-test
+  name: test
+---
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: test-org
+  namespace: test
+---
+apiVersion: v1
+kind: Pod
+metadata:
+  name: test-org
+  namespace: test
+spec:
+  containers:
+  - image: nginx:latest
+    name: test-org

test/conformance/kuttl/mutate/clusterpolicy/standard/existing/onpolicyupdate/namespaceselector/01-manifests.yaml

@@ -0,0 +1,22 @@
+apiVersion: v1
+kind: Namespace
+metadata:
+  labels:
+    org: kyverno-test
+  name: test
+---
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: test-org
+  namespace: test
+---
+apiVersion: v1
+kind: Pod
+metadata:
+  name: test-org
+  namespace: test
+spec:
+  containers:
+  - image: nginx:latest
+    name: test-org

test/conformance/kuttl/mutate/clusterpolicy/standard/existing/onpolicyupdate/namespaceselector/02-policy.yaml

@@ -0,0 +1,6 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+apply:
+- policy.yaml
+assert:
+- policy-assert.yaml

test/conformance/kuttl/mutate/clusterpolicy/standard/existing/onpolicyupdate/namespaceselector/03-assert.yaml

@@ -0,0 +1,7 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  annotations:
+    org: kyverno-test
+  name: test-org
+  namespace: test

test/conformance/kuttl/mutate/clusterpolicy/standard/existing/onpolicyupdate/namespaceselector/README.md

@@ -0,0 +1,18 @@
+## Description
+
+The `namespaceSelector` should applies to mutateExisting policies upon policy events.
+
+## Expected Behavior
+The pod is mutated with annotation `org: kyverno-test`.
+
+## Steps
+
+### Test Steps
+
+1. Create a pod and a configmap in `test` namespace labeled by `org: kyverno-test`.
+2. Create a `ClusterPolicy` that mutates existing pod.
+4. The pod should be mutated with the annotation `org: kyverno-test`.
+
+## Reference Issue(s)
+
+https://github.com/kyverno/kyverno/issues/6176

test/conformance/kuttl/mutate/clusterpolicy/standard/existing/onpolicyupdate/namespaceselector/policy-assert.yaml

@@ -0,0 +1,9 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: org-label-inheritance-existing
+status:
+  conditions:
+  - reason: Succeeded
+    status: "True"
+    type: Ready

test/conformance/kuttl/mutate/clusterpolicy/standard/existing/onpolicyupdate/namespaceselector/policy.yaml

@@ -0,0 +1,35 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: org-label-inheritance-existing
+  annotations:  
+    pod-policies.kyverno.io/autogen-controllers: none
+spec:
+  mutateExistingOnPolicyUpdate: true
+  validationFailureAction: enforce
+  rules:
+  - name: propagate org label from namespace
+    match:
+      any:
+      - resources:
+          kinds:
+          - ConfigMap
+          namespaceSelector:
+            matchExpressions:
+              - key: org
+                operator: Exists
+    context:
+    - name: org
+      apiCall:
+        urlPath: /api/v1/namespaces/{{ request.object.metadata.namespace }}
+        jmesPath: metadata.labels.org
+    mutate:
+      targets:
+      - apiVersion: v1
+        kind: Pod
+        namespace: "{{ request.object.metadata.namespace }}"
+        name: "{{ request.object.metadata.name }}"
+      patchStrategicMerge:
+        metadata:
+          annotations:
+            org: "{{ org }}"