Initial changes for NDEV-19725

Signed-off-by: Anushka Mittal <[email protected]>

pkg/policy/generate/fake.go

@@ -17,7 +17,7 @@ type FakeGenerate struct {
 func NewFakeGenerate(rule kyvernov1.Generation) *FakeGenerate {
 	g := FakeGenerate{}
 	g.rule = rule
-	g.authCheck = fake.NewFakeAuth()
+	g.authChecker = fake.NewFakeAuth()
 	g.log = logging.GlobalLogger()
 	return &g
 }

pkg/policy/generate/validate.go

@@ -16,12 +16,10 @@ import (
 
 // Generate provides implementation to validate 'generate' rule
 type Generate struct {
-	// rule to hold 'generate' rule specifications
-	rule kyvernov1.Generation
-	// authCheck to check access for operations
-	authCheck Operations
-	// logger
-	log logr.Logger
+	user        string
+	rule        kyvernov1.Generation
+	authChecker auth.AuthChecks
+	log         logr.Logger
 }
 
 // NewGenerateFactory returns a new instance of Generate validation checker

pkg/policy/mutate/validate.go

@@ -8,77 +8,80 @@ import (
 	kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
 	"github.com/kyverno/kyverno/pkg/clients/dclient"
 	"github.com/kyverno/kyverno/pkg/engine/variables/regex"
-	"github.com/kyverno/kyverno/pkg/utils/api"
+	"github.com/kyverno/kyverno/pkg/logging"
+	"github.com/kyverno/kyverno/pkg/policy/generate/fake"
 	kubeutils "github.com/kyverno/kyverno/pkg/utils/kube"
 	"go.uber.org/multierr"
-	v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 )
 
 // Mutate provides implementation to validate 'mutate' rule
 type Mutate struct {
 	mutation    kyvernov1.Mutation
-	user        string
-	authChecker AuthChecker
+	authChecker auth.AuthChecks
 }
 
 // NewMutateFactory returns a new instance of Mutate validation checker
-func NewMutateFactory(m kyvernov1.Mutation, client dclient.Interface, user string) *Mutate {
+func NewMutateFactory(m kyvernov1.Mutation, client dclient.Interface, mock bool, backgroundSA string) *Mutate {
+	var authCheck auth.AuthChecks
+	if mock {
+		authCheck = fake.NewFakeAuth()
+	} else {
+		authCheck = auth.NewAuth(client, backgroundSA, logging.GlobalLogger())
+	}
 	return &Mutate{
 		mutation:    m,
-		user:        user,
-		authChecker: newAuthChecker(client, user),
+		authChecker: authCheck,
 	}
 }
 
 // Validate validates the 'mutate' rule
-func (m *Mutate) Validate(ctx context.Context) (string, error) {
+func (m *Mutate) Validate(ctx context.Context) (warnings []string, path string, err error) {
 	if m.hasForEach() {
 		if m.hasPatchStrategicMerge() || m.hasPatchesJSON6902() {
-			return "foreach", fmt.Errorf("only one of `foreach`, `patchStrategicMerge`, or `patchesJson6902` is allowed")
+			return nil, "foreach", fmt.Errorf("only one of `foreach`, `patchStrategicMerge`, or `patchesJson6902` is allowed")
 		}
 
 		return m.validateForEach("", m.mutation.ForEachMutation)
 	}
 
 	if m.hasPatchesJSON6902() && m.hasPatchStrategicMerge() {
-		return "foreach", fmt.Errorf("only one of `patchStrategicMerge` or `patchesJson6902` is allowed")
+		return nil, "foreach", fmt.Errorf("only one of `patchStrategicMerge` or `patchesJson6902` is allowed")
 	}
 
 	if m.mutation.Targets != nil {
 		if err := m.validateAuth(ctx, m.mutation.Targets); err != nil {
-			return "targets", fmt.Errorf("auth check fails, additional privileges are required for the service account '%s': %v", m.user, err)
+			return nil, "targets", fmt.Errorf("auth check fails, additional privileges are required for the service account '%s': %v", m.authChecker.User(), err)
 		}
 	}
-	return "", nil
+	return nil, "", nil
 }
 
-func (m *Mutate) validateForEach(tag string, foreach []kyvernov1.ForEachMutation) (string, error) {
+func (m *Mutate) validateForEach(tag string, foreach []kyvernov1.ForEachMutation) (warnings []string, path string, err error) {
 	for i, fe := range foreach {
 		tag = tag + fmt.Sprintf("foreach[%d]", i)
 		if fe.ForEachMutation != nil {
 			if fe.Context != nil || fe.AnyAllConditions != nil || fe.PatchesJSON6902 != "" || fe.RawPatchStrategicMerge != nil {
-				return tag, fmt.Errorf("a nested foreach cannot contain other declarations")
+				return nil, tag, fmt.Errorf("a nested foreach cannot contain other declarations")
 			}
 
 			return m.validateNestedForEach(tag, fe.ForEachMutation)
 		}
 
 		psm := fe.GetPatchStrategicMerge()
 		if (fe.PatchesJSON6902 == "" && psm == nil) || (fe.PatchesJSON6902 != "" && psm != nil) {
-			return tag, fmt.Errorf("only one of `patchStrategicMerge` or `patchesJson6902` is allowed")
+			return nil, tag, fmt.Errorf("only one of `patchStrategicMerge` or `patchesJson6902` is allowed")
 		}
 	}
 
-	return "", nil
+	return nil, "", nil
 }
 
-func (m *Mutate) validateNestedForEach(tag string, j *v1.JSON) (string, error) {
-	nestedForeach, err := api.DeserializeJSONArray[kyvernov1.ForEachMutation](j)
-	if err != nil {
-		return tag, fmt.Errorf("invalid foreach syntax: %w", err)
+func (m *Mutate) validateNestedForEach(tag string, j []kyvernov1.ForEachMutation) (warnings []string, path string, err error) {
+	if j != nil {
+		return m.validateForEach(tag, j)
 	}
 
-	return m.validateForEach(tag, nestedForeach)
+	return nil, "", nil
 }
 
 func (m *Mutate) hasForEach() bool {
@@ -96,24 +99,18 @@ func (m *Mutate) hasPatchesJSON6902() bool {
 func (m *Mutate) validateAuth(ctx context.Context, targets []kyvernov1.TargetResourceSpec) error {
 	var errs []error
 	for _, target := range targets {
-		if !regex.IsVariable(target.Kind) {
-			_, _, k, sub := kubeutils.ParseKindSelector(target.Kind)
-			srcKey := k
-			if sub != "" {
-				srcKey = srcKey + "/" + sub
-			}
-
-			if ok, err := m.authChecker.CanIUpdate(ctx, strings.Join([]string{target.APIVersion, k}, "/"), target.Namespace, sub); err != nil {
-				errs = append(errs, err)
-			} else if !ok {
-				errs = append(errs, fmt.Errorf("cannot %s/%s/%s in namespace %s", "update", target.APIVersion, srcKey, target.Namespace))
-			}
-
-			if ok, err := m.authChecker.CanIGet(ctx, strings.Join([]string{target.APIVersion, k}, "/"), target.Namespace, sub); err != nil {
-				errs = append(errs, err)
-			} else if !ok {
-				errs = append(errs, fmt.Errorf("cannot %s/%s/%s in namespace %s", "get", target.APIVersion, srcKey, target.Namespace))
-			}
+		if regex.IsVariable(target.Kind) {
+			continue
+		}
+		_, _, k, sub := kubeutils.ParseKindSelector(target.Kind)
+		gvk := strings.Join([]string{target.APIVersion, k}, "/")
+		verbs := []string{"get", "update"}
+		ok, msg, err := m.authChecker.CanI(ctx, verbs, gvk, target.Namespace, target.Name, sub)
+		if err != nil {
+			return err
+		}
+		if !ok {
+			errs = append(errs, fmt.Errorf(msg))
 		}
 	}
 	return multierr.Combine(errs...)

pkg/policy/validate/validate.go

@@ -5,47 +5,67 @@ import (
 	"fmt"
 
 	kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
+	"github.com/kyverno/kyverno/pkg/clients/dclient"
 	"github.com/kyverno/kyverno/pkg/engine/anchor"
+	"github.com/kyverno/kyverno/pkg/logging"
 	"github.com/kyverno/kyverno/pkg/policy/common"
+	"github.com/kyverno/kyverno/pkg/policy/generate/fake"
+	"github.com/kyverno/kyverno/pkg/utils/wildcard"
 )
 
 // Validate validates a 'validate' rule
 type Validate struct {
 	// rule to hold 'validate' rule specifications
-	rule *kyvernov1.Validation
+	rule           *kyvernov1.Rule
+	validationRule *kyvernov1.Validation
+	authChecker    auth.AuthChecks
 }
 
 // NewValidateFactory returns a new instance of Mutate validation checker
-func NewValidateFactory(rule *kyvernov1.Validation) *Validate {
-	m := Validate{
-		rule: rule,
+func NewValidateFactory(rule *kyvernov1.Rule, client dclient.Interface, mock bool, reportsSA string) *Validate {
+	var authChecker auth.AuthChecks
+	if mock {
+		authChecker = fake.NewFakeAuth()
+	} else {
+		authChecker = auth.NewAuth(client, reportsSA, logging.GlobalLogger())
+	}
+	return &Validate{
+		rule:           rule,
+		validationRule: &rule.Validation,
+		authChecker:    authChecker,
 	}
+}
 
-	return &m
+func NewMockValidateFactory(rule *kyvernov1.Rule) *Validate {
+	return &Validate{
+		rule:           rule,
+		validationRule: &rule.Validation,
+		authChecker:    fake.NewFakeAuth(),
+	}
 }
 
 // Validate validates the 'validate' rule
-func (v *Validate) Validate(ctx context.Context) (string, error) {
+func (v *Validate) Validate(ctx context.Context) (warnings []string, path string, err error) {
 	if err := v.validateElements(); err != nil {
-		return "", err
+		return nil, "", err
 	}
 
-	if target := v.rule.GetPattern(); target != nil {
+	if target := v.validationRule.GetPattern(); target != nil {
 		if path, err := common.ValidatePattern(target, "/", func(a anchor.Anchor) bool {
 			return anchor.IsCondition(a) ||
 				anchor.IsExistence(a) ||
 				anchor.IsEquality(a) ||
 				anchor.IsNegation(a) ||
 				anchor.IsGlobal(a)
 		}); err != nil {
-			return fmt.Sprintf("pattern.%s", path), err
+			return nil, fmt.Sprintf("pattern.%s", path), err
 		}
 	}
 
-	if target := v.rule.GetAnyPattern(); target != nil {
-		anyPattern, err := v.rule.DeserializeAnyPattern()
+	if target := v.validationRule.GetAnyPattern(); target != nil {
+		anyPattern, err := v.validationRule.DeserializeAnyPattern()
 		if err != nil {
-			return "anyPattern", fmt.Errorf("failed to deserialize anyPattern, expect array: %v", err)
+			return nil, "anyPattern", fmt.Errorf("failed to deserialize anyPattern, expect array: %v", err)
 		}
 		for i, pattern := range anyPattern {
 			if path, err := common.ValidatePattern(pattern, "/", func(a anchor.Anchor) bool {
@@ -55,24 +75,50 @@ func (v *Validate) Validate(ctx context.Context) (string, error) {
 					anchor.IsNegation(a) ||
 					anchor.IsGlobal(a)
 			}); err != nil {
-				return fmt.Sprintf("anyPattern[%d].%s", i, path), err
+				return nil, fmt.Sprintf("anyPattern[%d].%s", i, path), err
 			}
 		}
 	}
 
-	if v.rule.ForEachValidation != nil {
-		for _, foreach := range v.rule.ForEachValidation {
+	if v.validationRule.ForEachValidation != nil {
+		for _, foreach := range v.validationRule.ForEachValidation {
 			if err := v.validateForEach(foreach); err != nil {
-				return "", err
+				return nil, "", err
 			}
 		}
 	}
 
-	return "", nil
+	if w, err := v.validateAuth(ctx); err != nil {
+		return nil, "", err
+	} else if len(w) > 0 {
+		warnings = append(warnings, w...)
+	}
+
+	return warnings, "", nil
+}
+
+func (v *Validate) validateAuth(ctx context.Context) (warnings []string, err error) {
+	kinds := v.rule.MatchResources.GetKinds()
+	for _, k := range kinds {
+		if wildcard.ContainsWildcard(k) {
+			return nil, nil
+		}
+
+		verbs := []string{"get", "list", "watch"}
+		ok, msg, err := v.authChecker.CanI(ctx, verbs, k, "", "", "")
+		if err != nil {
+			return nil, err
+		}
+		if !ok {
+			return []string{msg}, nil
+		}
+	}
+
+	return nil, nil
 }
 
 func (v *Validate) validateElements() error {
-	count := validationElemCount(v.rule)
+	count := validationElemCount(v.validationRule)
 	if count == 0 {
 		return fmt.Errorf("one of pattern, anyPattern, deny, foreach must be specified")
 	}

pkg/validation/cleanuppolicy/validate.go

@@ -78,7 +78,7 @@ func validateAuth(ctx context.Context, client dclient.Interface, policy kyvernov
 			return err
 		}
 		if !allowedDeletion {
-			return fmt.Errorf("cleanup controller has no permission to delete kind %s", kind)
+			return fmt.Errorf("cleanup controller requires permissions to delete kind %s", kind)
 		}
 
 		checker = auth.NewCanI(client.Discovery(), client.GetKubeClient().AuthorizationV1().SubjectAccessReviews(), kind, namespace, "list", "", config.KyvernoUserName(config.KyvernoServiceAccountName()))

pkg/validation/policy/actions.go

@@ -22,15 +22,15 @@ type Validation interface {
 // - Mutate
 // - Validation
 // - Generate
-func validateActions(idx int, rule *kyvernov1.Rule, client dclient.Interface, mock bool, username string) error {
+func validateActions(idx int, rule *kyvernov1.Rule, client dclient.Interface, mock bool, backgroundSA, reportsSA string) error {
 	if rule == nil {
 		return nil
 	}
 
 	var checker Validation
 	// Mutate
 	if rule.HasMutate() {
-		checker = mutate.NewMutateFactory(rule.Mutation, client, username)
+		checker = mutate.NewMutateFactory(rule.Mutation, client, backgroundSA)
 		if path, err := checker.Validate(context.TODO()); err != nil {
 			return fmt.Errorf("path: spec.rules[%d].mutate.%s.: %v", idx, path, err)
 		}
@@ -55,7 +55,7 @@ func validateActions(idx int, rule *kyvernov1.Rule, client dclient.Interface, mo
 				return fmt.Errorf("path: spec.rules[%d].generate.%s.: %v", idx, path, err)
 			}
 		} else {
-			checker = generate.NewGenerateFactory(client, rule.Generation, username, logging.GlobalLogger())
+			checker = generate.NewGenerateFactory(client, rule.Generation, backgroundSA, logging.GlobalLogger())
 			if path, err := checker.Validate(context.TODO()); err != nil {
 				return fmt.Errorf("path: spec.rules[%d].generate.%s.: %v", idx, path, err)
 			}

pkg/validation/policy/validate.go

@@ -122,7 +122,7 @@ 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) {
+func Validate(policy, oldPolicy kyvernov1.PolicyInterface, client dclient.Interface, mock bool, openApiManager openapi.Manager, backgroundSA, reportsSA string) ([]string, error) {
 	var warnings []string
 	spec := policy.GetSpec()
 	background := spec.BackgroundProcessingEnabled()
@@ -278,7 +278,7 @@ func Validate(policy, oldPolicy kyvernov1.PolicyInterface, client dclient.Interf
 			}
 		}
 
-		if err := validateActions(i, &rules[i], client, mock, username); err != nil {
+		if err := validateActions(i, &rules[i], client, mock, backgroundSA, reportsSA); err != nil {
 			return warnings, err
 		}