validate target resource scope & namespace settings (#7098)

Signed-off-by: ShutingZhao <shuting@nirmata.com>

api/kyverno/v1/common_types.go

@@ -2,6 +2,7 @@ package v1
 
 import (
 	"encoding/json"
+	"fmt"
 
 	"github.com/kyverno/kyverno/pkg/engine/variables/regex"
 	"github.com/sigstore/k8s-manifest-sigstore/pkg/k8smanifest"
@@ -9,6 +10,8 @@ import (
 	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
 	apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/sets"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/pod-security-admission/api"
 )
 
@@ -563,10 +566,14 @@ type CloneList struct {
 	Selector *metav1.LabelSelector `json:"selector,omitempty" yaml:"selector,omitempty"`
 }
 
-func (g *Generation) Validate() error {
+func (g *Generation) Validate(path *field.Path, clusterResources sets.Set[string]) (errs field.ErrorList) {
+	if err := g.validateTargetsScope(clusterResources); err != nil {
+		errs = append(errs, field.Forbidden(path.Child("generate").Child("namespace"), fmt.Sprintf("target resource scope mismatched: %v ", err)))
+	}
+
 	generateType, _ := g.GetTypeAndSync()
 	if generateType == Data {
-		return nil
+		return errs
 	}
 
 	newGeneration := Generation{
@@ -578,7 +585,11 @@ func (g *Generation) Validate() error {
 		CloneList: g.CloneList,
 	}
 
-	return regex.ObjectHasVariables(newGeneration)
+	if err := regex.ObjectHasVariables(newGeneration); err != nil {
+		errs = append(errs, field.Forbidden(path.Child("generate").Child("clone/cloneList"), "Generation Rule Clone/CloneList should not have variables"))
+	}
+
+	return errs
 }
 
 func (g *Generation) GetData() apiextensions.JSON {
@@ -589,6 +600,21 @@ func (g *Generation) SetData(in apiextensions.JSON) {
 	g.RawData = ToJSON(in)
 }
 
+func (g *Generation) validateTargetsScope(clusterResources sets.Set[string]) error {
+	target := g.ResourceSpec
+	if clusterResources.Has(target.GetKind()) {
+		if target.GetNamespace() != "" {
+			return fmt.Errorf("the target namespace must not be set for cluster-wide resource: %v", target.GetKind())
+		}
+	} else {
+		if target.GetNamespace() == "" {
+			return fmt.Errorf("the target namespace must be set for namespaced resource: %v", target.GetKind())
+		}
+	}
+
+	return nil
+}
+
 type GenerateType string
 
 const (

api/kyverno/v1/rule_test.go

@@ -1133,7 +1133,7 @@ func Test_Validate_ClusterPolicy_Generate_Variables(t *testing.T) {
 		var rule *Rule
 		err := json.Unmarshal(testcase.rule, &rule)
 		assert.NilError(t, err, testcase.name)
-		errs := rule.ValidateGenerateVariables(path)
+		errs := rule.ValidateGenerate(path, nil)
 		assert.Equal(t, len(errs) != 0, testcase.shouldFail, testcase.name)
 	}
 }

api/kyverno/v1/rule_types.go

@@ -393,15 +393,12 @@ func (r *Rule) ValidatePSaControlNames(path *field.Path) (errs field.ErrorList)
 	return errs
 }
 
-func (r *Rule) ValidateGenerateVariables(path *field.Path) (errs field.ErrorList) {
+func (r *Rule) ValidateGenerate(path *field.Path, clusterResources sets.Set[string]) (errs field.ErrorList) {
 	if !r.HasGenerate() {
 		return nil
 	}
 
-	if err := r.Generation.Validate(); err != nil {
+	return r.Generation.Validate(path, clusterResources)
-		errs = append(errs, field.Forbidden(path.Child("generate").Child("clone/cloneList"), fmt.Sprintf("Generation Rule Clone/CloneList \"%s\" should not have variables", r.Name)))
-	}
-	return errs
 }
 
 // Validate implements programmatic validation
@@ -412,6 +409,6 @@ func (r *Rule) Validate(path *field.Path, namespaced bool, policyNamespace strin
 	errs = append(errs, r.ExcludeResources.Validate(path.Child("exclude"), namespaced, clusterResources)...)
 	errs = append(errs, r.ValidateMutationRuleTargetNamespace(path, namespaced, policyNamespace)...)
 	errs = append(errs, r.ValidatePSaControlNames(path)...)
-	errs = append(errs, r.ValidateGenerateVariables(path)...)
+	errs = append(errs, r.ValidateGenerate(path, clusterResources)...)
 	return errs
 }

api/kyverno/v2beta1/rule_test.go

@@ -534,7 +534,7 @@ func Test_Validate_ClusterPolicy_Generate_Variables(t *testing.T) {
 		var rule *Rule
 		err := json.Unmarshal(testcase.rule, &rule)
 		assert.NilError(t, err, testcase.name)
-		errs := rule.ValidateGenerateVariables(path)
+		errs := rule.ValidateGenerate(path, nil)
 		assert.Equal(t, len(errs) != 0, testcase.shouldFail, testcase.name)
 	}
 }

api/kyverno/v2beta1/rule_types.go

@@ -172,15 +172,12 @@ func (r *Rule) ValidateMatchExcludeConflict(path *field.Path) (errs field.ErrorL
 	return append(errs, field.Invalid(path, r, "Rule is matching an empty set"))
 }
 
-func (r *Rule) ValidateGenerateVariables(path *field.Path) (errs field.ErrorList) {
+func (r *Rule) ValidateGenerate(path *field.Path, clusterResources sets.Set[string]) (errs field.ErrorList) {
 	if !r.HasGenerate() {
 		return nil
 	}
 
-	if err := r.Generation.Validate(); err != nil {
+	return r.Generation.Validate(path, clusterResources)
-		errs = append(errs, field.Forbidden(path.Child("generate").Child("clone/cloneList"), fmt.Sprintf("Generation Rule Clone/CloneList \"%s\" should not have variables", r.Name)))
-	}
-	return errs
 }
 
 // Validate implements programmatic validation
@@ -189,6 +186,6 @@ func (r *Rule) Validate(path *field.Path, namespaced bool, clusterResources sets
 	errs = append(errs, r.ValidateMatchExcludeConflict(path)...)
 	errs = append(errs, r.MatchResources.Validate(path.Child("match"), namespaced, clusterResources)...)
 	errs = append(errs, r.ExcludeResources.Validate(path.Child("exclude"), namespaced, clusterResources)...)
-	errs = append(errs, r.ValidateGenerateVariables(path)...)
+	errs = append(errs, r.ValidateGenerate(path, clusterResources)...)
 	return errs
 }

pkg/validation/policy/validate.go

@@ -124,7 +124,6 @@ func checkValidationFailureAction(spec *kyvernov1.Spec) []string {
 // Validate checks the policy and rules declarations for required configurations
 func Validate(policy, oldPolicy kyvernov1.PolicyInterface, client dclient.Interface, mock bool, openApiManager openapi.Manager, username string) ([]string, error) {
 	var warnings []string
-	namespaced := policy.IsNamespaced()
 	spec := policy.GetSpec()
 	background := spec.BackgroundProcessingEnabled()
 	mutateExistingOnPolicyUpdate := spec.GetMutateExistingOnPolicyUpdate()
@@ -150,7 +149,7 @@ func Validate(policy, oldPolicy kyvernov1.PolicyInterface, client dclient.Interf
 
 	var res []*metav1.APIResourceList
 	clusterResources := sets.New[string]()
-	if !mock && namespaced {
+	if !mock {
 		// Get all the cluster type kind supported by cluster
 		res, err = discovery.ServerPreferredResources(client.Discovery().DiscoveryInterface())
 		if err != nil {
@@ -176,7 +175,7 @@ func Validate(policy, oldPolicy kyvernov1.PolicyInterface, client dclient.Interf
 		return warnings, errs.ToAggregate()
 	}
 
-	if !namespaced {
+	if !policy.IsNamespaced() {
 		err := validateNamespaces(spec, specPath.Child("validationFailureActionOverrides"))
 		if err != nil {
 			return warnings, err
@@ -284,7 +283,7 @@ func Validate(policy, oldPolicy kyvernov1.PolicyInterface, client dclient.Interf
 
 		// validate Cluster Resources in namespaced policy
 		// For namespaced policy, ClusterResource type field and values are not allowed in match and exclude
-		if namespaced {
+		if policy.IsNamespaced() {
 			if err := checkClusterResourceInMatchAndExclude(rule, clusterResources, policy.GetNamespace(), mock, res); err != nil {
 				return warnings, err
 			}

test/conformance/kuttl/generate/validation/clusterpolicy/target-namespace-scope/01-fail.yaml

@@ -0,0 +1,7 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+apply:
+- file: policy-namespaced-target.yaml
+  shouldFail: true
+- file: policy-cluster-target.yaml
+  shouldFail: true

test/conformance/kuttl/generate/validation/clusterpolicy/target-namespace-scope/README.md

@@ -0,0 +1,12 @@
+## Description
+
+This test ensures that the target namespace must be set for namespace-scoped target resource, and must not be set for cluster-wide target resources.
+
+## Expected Behavior
+
+The test fails if the policy creation is allowed, otherwise passes.
+
+
+## Reference Issue(s)
+
+https://github.com/kyverno/kyverno/issues/7038

test/conformance/kuttl/generate/validation/clusterpolicy/target-namespace-scope/policy-cluster-target.yaml

@@ -0,0 +1,19 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: cpol-cluster-target
+spec:
+  generateExisting: false
+  rules:
+  - name: cpol-cluster-target
+    match:
+      any:
+      - resources:
+          kinds:
+          - ConfigMap
+    generate:
+      synchronize: false
+      apiVersion: v1
+      kind: Namespace
+      namespace: fake-ns
+      name: cpol-cluster-target-ns

test/conformance/kuttl/generate/validation/clusterpolicy/target-namespace-scope/policy-namespaced-target.yaml

@@ -0,0 +1,34 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: cpol-data-nosync-modify-rule-policy
+spec:
+  generateExisting: false
+  rules:
+  - name: cpol-data-nosync-modify-rule-rule
+    match:
+      any:
+      - resources:
+          kinds:
+          - Namespace
+    exclude:
+      any:
+      - resources:
+          namespaces:
+          - kube-system
+          - default
+          - kube-public
+          - kyverno
+    generate:
+      synchronize: false
+      apiVersion: v1
+      kind: ConfigMap
+      name: zk-kafka-address
+      data:
+        kind: ConfigMap
+        metadata:
+          labels:
+            somekey: somevalue
+        data:
+          ZK_ADDRESS: "192.168.10.10:2181,192.168.10.11:2181,192.168.10.12:2181"
+          KAFKA_ADDRESS: "192.168.10.13:9092,192.168.10.14:9092,192.168.10.15:9092"

test/conformance/kuttl/generate/validation/policy/target-namespace-scope/01-fail.yaml

@@ -0,0 +1,5 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+apply:
+- file: policy-namespaced-target.yaml
+  shouldFail: true

test/conformance/kuttl/generate/validation/policy/target-namespace-scope/README.md

@@ -0,0 +1,12 @@
+## Description
+
+This test ensures that the target namespace must be set for the namespaced policy.
+
+## Expected Behavior
+
+The test fails if the policy creation is allowed, otherwise passes.
+
+
+## Reference Issue(s)
+
+https://github.com/kyverno/kyverno/issues/7038

test/conformance/kuttl/generate/validation/policy/target-namespace-scope/policy-namespaced-target.yaml

@@ -0,0 +1,18 @@
+apiVersion: kyverno.io/v1
+kind: Policy
+metadata:
+  name: pol-cluster-target
+spec:
+  generateExisting: false
+  rules:
+  - name: pol-cluster-target
+    match:
+      any:
+      - resources:
+          kinds:
+          - ConfigMap
+    generate:
+      synchronize: false
+      apiVersion: v1
+      kind: Secret
+      name: cpol-cluster-target-ns