From c8c631d4a7d1a11e65c2cdbedf8bc6b274434371 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?= Date: Mon, 21 Mar 2022 12:51:12 +0100 Subject: [PATCH] refactor: MatchResources validation (#3422) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor: ValidationFailureActionOverrides validation Signed-off-by: Charles-Edouard Brétéché * refactor: MatchResources validation Signed-off-by: Charles-Edouard Brétéché Co-authored-by: shuting --- api/kyverno/v1/clusterpolicy_test.go | 2 +- api/kyverno/v1/clusterpolicy_types.go | 7 +- api/kyverno/v1/common_types.go | 69 --------------- api/kyverno/v1/match_resources_test.go | 67 +++++++++++++++ api/kyverno/v1/match_resources_types.go | 52 ++++++++++++ api/kyverno/v1/policy_test.go | 2 +- api/kyverno/v1/policy_types.go | 5 +- api/kyverno/v1/resource_description_test.go | 39 +++++++++ api/kyverno/v1/resource_description_types.go | 70 +++++++++++++++ api/kyverno/v1/rule_test.go | 6 +- api/kyverno/v1/rule_types.go | 4 +- api/kyverno/v1/spec_test.go | 2 +- api/kyverno/v1/spec_types.go | 8 +- pkg/policy/validate.go | 89 ++++---------------- 14 files changed, 263 insertions(+), 159 deletions(-) create mode 100644 api/kyverno/v1/match_resources_test.go create mode 100644 api/kyverno/v1/match_resources_types.go create mode 100644 api/kyverno/v1/resource_description_test.go create mode 100644 api/kyverno/v1/resource_description_types.go diff --git a/api/kyverno/v1/clusterpolicy_test.go b/api/kyverno/v1/clusterpolicy_test.go index 895e523f96..dc6043a04b 100644 --- a/api/kyverno/v1/clusterpolicy_test.go +++ b/api/kyverno/v1/clusterpolicy_test.go @@ -14,7 +14,7 @@ func Test_ClusterPolicy_Name(t *testing.T) { Name: "this-is-a-way-too-long-policy-name-that-should-trigger-an-error-when-calling-the-policy-validation-method", }, } - errs := subject.Validate(false) + errs := subject.Validate(false, nil) assert.Assert(t, len(errs) == 1) assert.Equal(t, errs[0].Field, "name") assert.Equal(t, errs[0].Type, field.ErrorTypeTooLong) diff --git a/api/kyverno/v1/clusterpolicy_types.go b/api/kyverno/v1/clusterpolicy_types.go index f98640037c..437a513b54 100644 --- a/api/kyverno/v1/clusterpolicy_types.go +++ b/api/kyverno/v1/clusterpolicy_types.go @@ -4,6 +4,7 @@ import ( "strings" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation/field" ) @@ -85,13 +86,13 @@ func (p *ClusterPolicy) IsReady() bool { return p.Status.IsReady() } -// Validate implements programmatic validation. +// Validate implements programmatic validation // namespaced means that the policy is bound to a namespace and therefore // should not filter/generate cluster wide resources. -func (p *ClusterPolicy) Validate(namespaced bool) field.ErrorList { +func (p *ClusterPolicy) Validate(namespaced bool, clusterResources sets.String) field.ErrorList { var errs field.ErrorList errs = append(errs, ValidatePolicyName(field.NewPath("name"), p.Name)...) - errs = append(errs, p.Spec.Validate(field.NewPath("spec"), namespaced)...) + errs = append(errs, p.Spec.Validate(field.NewPath("spec"), namespaced, clusterResources)...) return errs } diff --git a/api/kyverno/v1/common_types.go b/api/kyverno/v1/common_types.go index 1eab384f99..3425d42a5a 100755 --- a/api/kyverno/v1/common_types.go +++ b/api/kyverno/v1/common_types.go @@ -180,31 +180,6 @@ var ConditionOperators = map[string]ConditionOperator{ "DurationLessThan": ConditionOperator("DurationLessThan"), } -// MatchResources is used to specify resource and admission review request data for -// which a policy rule is applicable. -type MatchResources struct { - // Any allows specifying resources which will be ORed - // +optional - Any ResourceFilters `json:"any,omitempty" yaml:"any,omitempty"` - - // All allows specifying resources which will be ANDed - // +optional - All ResourceFilters `json:"all,omitempty" yaml:"all,omitempty"` - - // UserInfo contains information about the user performing the operation. - // Specifying UserInfo directly under match is being deprecated. - // Please specify under "any" or "all" instead. - // +optional - UserInfo `json:",omitempty" yaml:",omitempty"` - - // ResourceDescription contains information about the resource being created or modified. - // Requires at least one tag to be specified when under MatchResources. - // Specifying ResourceDescription directly under match is being deprecated. - // Please specify under "any" or "all" instead. - // +optional - ResourceDescription `json:"resources,omitempty" yaml:"resources,omitempty"` -} - // ExcludeResources specifies resource and admission review request data for // which a policy rule is not applicable. type ExcludeResources struct { @@ -242,50 +217,6 @@ type ResourceFilter struct { ResourceDescription `json:"resources,omitempty" yaml:"resources,omitempty"` } -// ResourceDescription contains criteria used to match resources. -type ResourceDescription struct { - // Kinds is a list of resource kinds. - // +optional - Kinds []string `json:"kinds,omitempty" yaml:"kinds,omitempty"` - - // Name is the name of the resource. The name supports wildcard characters - // "*" (matches zero or many characters) and "?" (at least one character). - // +optional - Name string `json:"name,omitempty" yaml:"name,omitempty"` - - // Names are the names of the resources. Each name supports wildcard characters - // "*" (matches zero or many characters) and "?" (at least one character). - // NOTE: "Name" is being deprecated in favor of "Names". - // +optional - Names []string `json:"names,omitempty" yaml:"names,omitempty"` - - // Namespaces is a list of namespaces names. Each name supports wildcard characters - // "*" (matches zero or many characters) and "?" (at least one character). - // +optional - Namespaces []string `json:"namespaces,omitempty" yaml:"namespaces,omitempty"` - - // Annotations is a map of annotations (key-value pairs of type string). Annotation keys - // and values support the wildcard characters "*" (matches zero or many characters) and - // "?" (matches at least one character). - // +optional - Annotations map[string]string `json:"annotations,omitempty" yaml:"annotations,omitempty"` - - // Selector is a label selector. Label keys and values in `matchLabels` support the wildcard - // characters `*` (matches zero or many characters) and `?` (matches one character). - // Wildcards allows writing label selectors like ["storage.k8s.io/*": "*"]. Note that - // using ["*" : "*"] matches any key and value but does not match an empty label set. - // +optional - Selector *metav1.LabelSelector `json:"selector,omitempty" yaml:"selector,omitempty"` - - // NamespaceSelector is a label selector for the resource namespace. Label keys and values - // in `matchLabels` support the wildcard characters `*` (matches zero or many characters) - // and `?` (matches one character).Wildcards allows writing label selectors like - // ["storage.k8s.io/*": "*"]. Note that using ["*" : "*"] matches any key and value but - // does not match an empty label set. - // +optional - NamespaceSelector *metav1.LabelSelector `json:"namespaceSelector,omitempty" yaml:"namespaceSelector,omitempty"` -} - // Mutation defines how resource are modified. type Mutation struct { // PatchStrategicMerge is a strategic merge patch used to modify resources. diff --git a/api/kyverno/v1/match_resources_test.go b/api/kyverno/v1/match_resources_test.go new file mode 100644 index 0000000000..84052b645f --- /dev/null +++ b/api/kyverno/v1/match_resources_test.go @@ -0,0 +1,67 @@ +package v1 + +import ( + "testing" + + "gotest.tools/assert" + rbacv1 "k8s.io/api/rbac/v1" + "k8s.io/apimachinery/pkg/util/validation/field" +) + +func Test_MatchResources(t *testing.T) { + testCases := []struct { + name string + namespaced bool + subject MatchResources + errors []string + }{{ + name: "valid", + namespaced: true, + subject: MatchResources{ + Any: ResourceFilters{{ + UserInfo: UserInfo{ + Subjects: []rbacv1.Subject{{ + Kind: "ServiceAccount", + Namespace: "ns", + Name: "sa-1", + }}, + }, + }}, + }, + }, { + name: "any-all", + namespaced: true, + subject: MatchResources{ + Any: ResourceFilters{{ + UserInfo: UserInfo{ + Subjects: []rbacv1.Subject{{ + Kind: "ServiceAccount", + Namespace: "ns", + Name: "sa-1", + }}, + }, + }}, + All: ResourceFilters{{ + UserInfo: UserInfo{ + Subjects: []rbacv1.Subject{{ + Kind: "ServiceAccount", + Namespace: "ns", + Name: "sa-1", + }}, + }, + }}, + }, + errors: []string{ + `dummy: Invalid value: v1.MatchResources{Any:v1.ResourceFilters{v1.ResourceFilter{UserInfo:v1.UserInfo{Roles:[]string(nil), ClusterRoles:[]string(nil), Subjects:[]v1.Subject{v1.Subject{Kind:"ServiceAccount", APIGroup:"", Name:"sa-1", Namespace:"ns"}}}, ResourceDescription:v1.ResourceDescription{Kinds:[]string(nil), Name:"", Names:[]string(nil), Namespaces:[]string(nil), Annotations:map[string]string(nil), Selector:(*v1.LabelSelector)(nil), NamespaceSelector:(*v1.LabelSelector)(nil)}}}, All:v1.ResourceFilters{v1.ResourceFilter{UserInfo:v1.UserInfo{Roles:[]string(nil), ClusterRoles:[]string(nil), Subjects:[]v1.Subject{v1.Subject{Kind:"ServiceAccount", APIGroup:"", Name:"sa-1", Namespace:"ns"}}}, ResourceDescription:v1.ResourceDescription{Kinds:[]string(nil), Name:"", Names:[]string(nil), Namespaces:[]string(nil), Annotations:map[string]string(nil), Selector:(*v1.LabelSelector)(nil), NamespaceSelector:(*v1.LabelSelector)(nil)}}}, UserInfo:v1.UserInfo{Roles:[]string(nil), ClusterRoles:[]string(nil), Subjects:[]v1.Subject(nil)}, ResourceDescription:v1.ResourceDescription{Kinds:[]string(nil), Name:"", Names:[]string(nil), Namespaces:[]string(nil), Annotations:map[string]string(nil), Selector:(*v1.LabelSelector)(nil), NamespaceSelector:(*v1.LabelSelector)(nil)}}: Can't specify any and all together`, + }, + }} + + path := field.NewPath("dummy") + for _, testCase := range testCases { + errs := testCase.subject.Validate(path, testCase.namespaced, nil) + assert.Equal(t, len(errs), len(testCase.errors)) + for i, err := range errs { + assert.Equal(t, err.Error(), testCase.errors[i]) + } + } +} diff --git a/api/kyverno/v1/match_resources_types.go b/api/kyverno/v1/match_resources_types.go new file mode 100644 index 0000000000..23be5a7ccd --- /dev/null +++ b/api/kyverno/v1/match_resources_types.go @@ -0,0 +1,52 @@ +package v1 + +import ( + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apimachinery/pkg/util/validation/field" +) + +// MatchResources is used to specify resource and admission review request data for +// which a policy rule is applicable. +type MatchResources struct { + // Any allows specifying resources which will be ORed + // +optional + Any ResourceFilters `json:"any,omitempty" yaml:"any,omitempty"` + + // All allows specifying resources which will be ANDed + // +optional + All ResourceFilters `json:"all,omitempty" yaml:"all,omitempty"` + + // UserInfo contains information about the user performing the operation. + // Specifying UserInfo directly under match is being deprecated. + // Please specify under "any" or "all" instead. + // +optional + UserInfo `json:",omitempty" yaml:",omitempty"` + + // ResourceDescription contains information about the resource being created or modified. + // Requires at least one tag to be specified when under MatchResources. + // Specifying ResourceDescription directly under match is being deprecated. + // Please specify under "any" or "all" instead. + // +optional + ResourceDescription `json:"resources,omitempty" yaml:"resources,omitempty"` +} + +// Validate implements programmatic validation +func (m *MatchResources) Validate(path *field.Path, namespaced bool, clusterResources sets.String) field.ErrorList { + var errs field.ErrorList + if len(m.Any) > 0 && len(m.All) > 0 { + errs = append(errs, field.Invalid(path, m, "Can't specify any and all together")) + } + anyPath := path.Child("any") + for i, filter := range m.Any { + errs = append(errs, filter.UserInfo.Validate(anyPath.Index(i))...) + errs = append(errs, filter.ResourceDescription.Validate(anyPath.Index(i), namespaced, clusterResources)...) + } + allPath := path.Child("all") + for i, filter := range m.All { + errs = append(errs, filter.UserInfo.Validate(anyPath.Index(i))...) + errs = append(errs, filter.ResourceDescription.Validate(allPath.Index(i), namespaced, clusterResources)...) + } + errs = append(errs, m.UserInfo.Validate(path)...) + errs = append(errs, m.ResourceDescription.Validate(path, namespaced, clusterResources)...) + return errs +} diff --git a/api/kyverno/v1/policy_test.go b/api/kyverno/v1/policy_test.go index d71028c9d0..4bfc2db848 100644 --- a/api/kyverno/v1/policy_test.go +++ b/api/kyverno/v1/policy_test.go @@ -14,7 +14,7 @@ func Test_Policy_Name(t *testing.T) { Name: "this-is-a-way-too-long-policy-name-that-should-trigger-an-error-when-calling-the-policy-validation-method", }, } - errs := subject.Validate(true) + errs := subject.Validate(true, nil) assert.Assert(t, len(errs) == 1) assert.Equal(t, errs[0].Field, "name") assert.Equal(t, errs[0].Type, field.ErrorTypeTooLong) diff --git a/api/kyverno/v1/policy_types.go b/api/kyverno/v1/policy_types.go index 091f5ff8d8..05af3f69af 100755 --- a/api/kyverno/v1/policy_types.go +++ b/api/kyverno/v1/policy_types.go @@ -4,6 +4,7 @@ import ( "strings" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation/field" ) @@ -89,10 +90,10 @@ func (p *Policy) IsReady() bool { // Validate implements programmatic validation. // namespaced means that the policy is bound to a namespace and therefore // should not filter/generate cluster wide resources. -func (p *Policy) Validate(namespaced bool) field.ErrorList { +func (p *Policy) Validate(namespaced bool, clusterResources sets.String) field.ErrorList { var errs field.ErrorList errs = append(errs, ValidatePolicyName(field.NewPath("name"), p.Name)...) - errs = append(errs, p.Spec.Validate(field.NewPath("spec"), namespaced)...) + errs = append(errs, p.Spec.Validate(field.NewPath("spec"), namespaced, clusterResources)...) return errs } diff --git a/api/kyverno/v1/resource_description_test.go b/api/kyverno/v1/resource_description_test.go new file mode 100644 index 0000000000..4875681028 --- /dev/null +++ b/api/kyverno/v1/resource_description_test.go @@ -0,0 +1,39 @@ +package v1 + +import ( + "testing" + + "gotest.tools/assert" + "k8s.io/apimachinery/pkg/util/validation/field" +) + +func Test_ResourceDescription(t *testing.T) { + testCases := []struct { + name string + namespaced bool + subject ResourceDescription + errors []string + }{{ + name: "valid", + namespaced: true, + subject: ResourceDescription{}, + }, { + name: "namespaces", + namespaced: true, + subject: ResourceDescription{ + Namespaces: []string{"abc"}, + }, + errors: []string{ + "dummy.namespaces: Forbidden: Filtering namespaces not allowed in namespaced policies", + }, + }} + + path := field.NewPath("dummy") + for _, testCase := range testCases { + errs := testCase.subject.Validate(path, testCase.namespaced, nil) + assert.Equal(t, len(errs), len(testCase.errors)) + for i, err := range errs { + assert.Equal(t, err.Error(), testCase.errors[i]) + } + } +} diff --git a/api/kyverno/v1/resource_description_types.go b/api/kyverno/v1/resource_description_types.go new file mode 100644 index 0000000000..0752d38004 --- /dev/null +++ b/api/kyverno/v1/resource_description_types.go @@ -0,0 +1,70 @@ +package v1 + +import ( + "fmt" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apimachinery/pkg/util/validation/field" +) + +// ResourceDescription contains criteria used to match resources. +type ResourceDescription struct { + // Kinds is a list of resource kinds. + // +optional + Kinds []string `json:"kinds,omitempty" yaml:"kinds,omitempty"` + + // Name is the name of the resource. The name supports wildcard characters + // "*" (matches zero or many characters) and "?" (at least one character). + // +optional + Name string `json:"name,omitempty" yaml:"name,omitempty"` + + // Names are the names of the resources. Each name supports wildcard characters + // "*" (matches zero or many characters) and "?" (at least one character). + // NOTE: "Name" is being deprecated in favor of "Names". + // +optional + Names []string `json:"names,omitempty" yaml:"names,omitempty"` + + // Namespaces is a list of namespaces names. Each name supports wildcard characters + // "*" (matches zero or many characters) and "?" (at least one character). + // +optional + Namespaces []string `json:"namespaces,omitempty" yaml:"namespaces,omitempty"` + + // Annotations is a map of annotations (key-value pairs of type string). Annotation keys + // and values support the wildcard characters "*" (matches zero or many characters) and + // "?" (matches at least one character). + // +optional + Annotations map[string]string `json:"annotations,omitempty" yaml:"annotations,omitempty"` + + // Selector is a label selector. Label keys and values in `matchLabels` support the wildcard + // characters `*` (matches zero or many characters) and `?` (matches one character). + // Wildcards allows writing label selectors like ["storage.k8s.io/*": "*"]. Note that + // using ["*" : "*"] matches any key and value but does not match an empty label set. + // +optional + Selector *metav1.LabelSelector `json:"selector,omitempty" yaml:"selector,omitempty"` + + // NamespaceSelector is a label selector for the resource namespace. Label keys and values + // in `matchLabels` support the wildcard characters `*` (matches zero or many characters) + // and `?` (matches one character).Wildcards allows writing label selectors like + // ["storage.k8s.io/*": "*"]. Note that using ["*" : "*"] matches any key and value but + // does not match an empty label set. + // +optional + NamespaceSelector *metav1.LabelSelector `json:"namespaceSelector,omitempty" yaml:"namespaceSelector,omitempty"` +} + +// Validate implements programmatic validation +func (r *ResourceDescription) Validate(path *field.Path, namespaced bool, clusterResources sets.String) field.ErrorList { + var errs field.ErrorList + if namespaced { + if len(r.Namespaces) > 0 { + errs = append(errs, field.Forbidden(path.Child("namespaces"), "Filtering namespaces not allowed in namespaced policies")) + } + kindsChild := path.Child("kinds") + for i, kind := range r.Kinds { + if clusterResources.Has(kind) { + errs = append(errs, field.Forbidden(kindsChild.Index(i), fmt.Sprintf("Cluster wide resource '%s' not allowed in namespaced policy", kind))) + } + } + } + return errs +} diff --git a/api/kyverno/v1/rule_test.go b/api/kyverno/v1/rule_test.go index 920815eb40..71650991e0 100644 --- a/api/kyverno/v1/rule_test.go +++ b/api/kyverno/v1/rule_test.go @@ -13,7 +13,7 @@ func Test_Validate_RuleType_EmptyRule(t *testing.T) { Name: "validate-user-privilege", } path := field.NewPath("dummy") - errs := subject.Validate(path) + errs := subject.Validate(path, false, nil) assert.Equal(t, len(errs), 1) assert.Equal(t, errs[0].Field, "dummy") assert.Equal(t, errs[0].Type, field.ErrorTypeInvalid) @@ -90,7 +90,7 @@ func Test_Validate_RuleType_MultipleRule(t *testing.T) { assert.NilError(t, err) for _, rule := range policy.GetRules() { path := field.NewPath("dummy") - errs := rule.Validate(path) + errs := rule.Validate(path, false, nil) assert.Assert(t, len(errs) != 0) } } @@ -145,7 +145,7 @@ func Test_Validate_RuleType_SingleRule(t *testing.T) { assert.NilError(t, err) for _, rule := range policy.GetRules() { path := field.NewPath("dummy") - errs := rule.Validate(path) + errs := rule.Validate(path, false, nil) assert.Assert(t, len(errs) == 0) } } diff --git a/api/kyverno/v1/rule_types.go b/api/kyverno/v1/rule_types.go index f88c0a1749..41c61a6578 100644 --- a/api/kyverno/v1/rule_types.go +++ b/api/kyverno/v1/rule_types.go @@ -6,6 +6,7 @@ import ( "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation/field" ) @@ -130,8 +131,9 @@ func (r *Rule) ValidateRuleType(path *field.Path) field.ErrorList { } // Validate implements programmatic validation -func (r *Rule) Validate(path *field.Path) field.ErrorList { +func (r *Rule) Validate(path *field.Path, namespaced bool, clusterResources sets.String) field.ErrorList { var errs field.ErrorList errs = append(errs, r.ValidateRuleType(path)...) + errs = append(errs, r.MatchResources.Validate(path.Child("match"), namespaced, clusterResources)...) return errs } diff --git a/api/kyverno/v1/spec_test.go b/api/kyverno/v1/spec_test.go index 1eccd89a19..6ff74ee4e7 100644 --- a/api/kyverno/v1/spec_test.go +++ b/api/kyverno/v1/spec_test.go @@ -43,7 +43,7 @@ func Test_Validate_UniqueRuleName(t *testing.T) { }}, } path := field.NewPath("dummy") - errs := subject.Validate(path, false) + errs := subject.Validate(path, false, nil) assert.Equal(t, len(errs), 1) assert.Equal(t, errs[0].Field, "dummy.rules[1].name") assert.Equal(t, errs[0].Type, field.ErrorTypeInvalid) diff --git a/api/kyverno/v1/spec_types.go b/api/kyverno/v1/spec_types.go index 31401eef61..3de485c7ba 100644 --- a/api/kyverno/v1/spec_types.go +++ b/api/kyverno/v1/spec_types.go @@ -135,19 +135,19 @@ func (s *Spec) ValidateRuleNames(path *field.Path) field.ErrorList { } // ValidateRules implements programmatic validation of Rules -func (s *Spec) ValidateRules(path *field.Path) field.ErrorList { +func (s *Spec) ValidateRules(path *field.Path, namespaced bool, clusterResources sets.String) field.ErrorList { var errs field.ErrorList errs = append(errs, s.ValidateRuleNames(path)...) for i, rule := range s.Rules { - errs = append(errs, rule.Validate(path.Index(i))...) + errs = append(errs, rule.Validate(path.Index(i), namespaced, clusterResources)...) } return errs } // Validate implements programmatic validation -func (s *Spec) Validate(path *field.Path, namespaced bool) field.ErrorList { +func (s *Spec) Validate(path *field.Path, namespaced bool, clusterResources sets.String) field.ErrorList { var errs field.ErrorList - errs = append(errs, s.ValidateRules(path.Child("rules"))...) + errs = append(errs, s.ValidateRules(path.Child("rules"), namespaced, clusterResources)...) if namespaced && len(s.ValidationFailureActionOverrides) > 0 { errs = append(errs, field.Forbidden(path.Child("validationFailureActionOverrides"), "Use of validationFailureActionOverrides is supported only with ClusterPolicy")) } diff --git a/pkg/policy/validate.go b/pkg/policy/validate.go index 3f2c11ea78..063f1c1cb6 100644 --- a/pkg/policy/validate.go +++ b/pkg/policy/validate.go @@ -26,6 +26,7 @@ import ( v1beta1 "k8s.io/api/admission/v1beta1" "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apimachinery/pkg/util/yaml" "sigs.k8s.io/controller-runtime/pkg/log" @@ -85,21 +86,14 @@ func Validate(policy *kyverno.ClusterPolicy, client *dclient.Client, mock bool, var errs field.ErrorList specPath := field.NewPath("spec") - if errs := policy.Validate(namespaced); len(errs) != 0 { - return nil, errs.ToAggregate() - } - err := ValidateVariables(policy, background) if err != nil { return nil, err } var res []*metav1.APIResourceList - clusterResources := make([]string, 0) + clusterResources := sets.NewString() if !mock && namespaced { - var Empty struct{} - clusterResourcesMap := make(map[string]*struct{}) - // Get all the cluster type kind supported by cluster res, err := client.DiscoveryClient.DiscoveryCache().ServerPreferredResources() if err != nil { @@ -108,17 +102,16 @@ func Validate(policy *kyverno.ClusterPolicy, client *dclient.Client, mock bool, for _, resList := range res { for _, r := range resList.APIResources { if !r.Namespaced { - if _, ok := clusterResourcesMap[r.Kind]; !ok { - clusterResourcesMap[r.Kind] = &Empty - } + clusterResources.Insert(r.Kind) } } } - - for k := range clusterResourcesMap { - clusterResources = append(clusterResources, k) - } } + + if errs := policy.Validate(namespaced, clusterResources); len(errs) != 0 { + return nil, errs.ToAggregate() + } + rules := policy.GetRules() rulesPath := specPath.Child("rules") for i, rule := range rules { @@ -886,9 +879,6 @@ func ruleOnlyDealsWithResourceMetaData(rule kyverno.Rule) bool { func validateResources(path *field.Path, rule kyverno.Rule) (string, error) { // validate userInfo in match and exclude - if errs := rule.MatchResources.UserInfo.Validate(path.Child("match")); len(errs) != 0 { - return "match", errs.ToAggregate() - } if errs := rule.ExcludeResources.UserInfo.Validate(path.Child("exclude")); len(errs) != 0 { return "exclude", errs.ToAggregate() } @@ -901,10 +891,6 @@ func validateResources(path *field.Path, rule kyverno.Rule) (string, error) { return "exclude.", fmt.Errorf("can't specify any/all together with exclude resources") } - if len(rule.MatchResources.Any) > 0 && len(rule.MatchResources.All) > 0 { - return "match.", fmt.Errorf("can't specify any and all together") - } - if len(rule.ExcludeResources.Any) > 0 && len(rule.ExcludeResources.All) > 0 { return "match.", fmt.Errorf("can't specify any and all together") } @@ -1298,68 +1284,26 @@ func isWildcardPresent(v string) bool { // checkClusterResourceInMatchAndExclude returns false if namespaced ClusterPolicy contains cluster wide resources in // Match and Exclude block -func checkClusterResourceInMatchAndExclude(rule kyverno.Rule, clusterResources []string, mock bool, res []*metav1.APIResourceList) error { - // Contains Namespaces in Match->ResourceDescription - if len(rule.MatchResources.ResourceDescription.Namespaces) > 0 { - return fmt.Errorf("namespaced cluster policy : field namespaces not allowed in match.resources") - } +func checkClusterResourceInMatchAndExclude(rule kyverno.Rule, clusterResources sets.String, mock bool, res []*metav1.APIResourceList) error { // Contains Namespaces in Exclude->ResourceDescription if len(rule.ExcludeResources.ResourceDescription.Namespaces) > 0 { return fmt.Errorf("namespaced cluster policy : field namespaces not allowed in exclude.resources") } if !mock { - // Contains "Cluster Wide Resources" in Match->ResourceDescription->Kinds - for _, kind := range rule.MatchResources.ResourceDescription.Kinds { - for _, k := range clusterResources { - if kind == k { - return fmt.Errorf("namespaced policy : cluster-wide resource '%s' not allowed in match.resources.kinds", kind) - } - } - } - - // Contains "Cluster Wide Resources" in Match->All->ResourceFilter->ResourceDescription->Kinds - for _, allResourceFilter := range rule.MatchResources.All { - fmt.Println(allResourceFilter.ResourceDescription) - for _, kind := range allResourceFilter.ResourceDescription.Kinds { - for _, k := range clusterResources { - if kind == k { - return fmt.Errorf("namespaced policy : cluster-wide resource '%s' not allowed in match.resources.kinds", kind) - } - } - } - } - - // Contains "Cluster Wide Resources" in Match->Any->ResourceFilter->ResourceDescription->Kinds - for _, allResourceFilter := range rule.MatchResources.Any { - fmt.Println(allResourceFilter.ResourceDescription) - for _, kind := range allResourceFilter.ResourceDescription.Kinds { - for _, k := range clusterResources { - if kind == k { - return fmt.Errorf("namespaced policy : cluster-wide resource '%s' not allowed in match.resources.kinds", kind) - } - } - } - } - // Contains "Cluster Wide Resources" in Exclude->ResourceDescription->Kinds for _, kind := range rule.ExcludeResources.ResourceDescription.Kinds { - for _, k := range clusterResources { - if kind == k { - return fmt.Errorf("namespaced policy : cluster-wide resource '%s' not allowed in exclude.resources.kinds", kind) - } + if clusterResources.Has(kind) { + return fmt.Errorf("namespaced policy : cluster-wide resource '%s' not allowed in exclude.resources.kinds", kind) } - } // Contains "Cluster Wide Resources" in Exclude->All->ResourceFilter->ResourceDescription->Kinds for _, allResourceFilter := range rule.ExcludeResources.All { fmt.Println(allResourceFilter.ResourceDescription) for _, kind := range allResourceFilter.ResourceDescription.Kinds { - for _, k := range clusterResources { - if kind == k { - return fmt.Errorf("namespaced policy : cluster-wide resource '%s' not allowed in match.resources.kinds", kind) - } + if clusterResources.Has(kind) { + return fmt.Errorf("namespaced policy : cluster-wide resource '%s' not allowed in match.resources.kinds", kind) } } } @@ -1368,10 +1312,8 @@ func checkClusterResourceInMatchAndExclude(rule kyverno.Rule, clusterResources [ for _, allResourceFilter := range rule.ExcludeResources.Any { fmt.Println(allResourceFilter.ResourceDescription) for _, kind := range allResourceFilter.ResourceDescription.Kinds { - for _, k := range clusterResources { - if kind == k { - return fmt.Errorf("namespaced policy : cluster-wide resource '%s' not allowed in match.resources.kinds", kind) - } + if clusterResources.Has(kind) { + return fmt.Errorf("namespaced policy : cluster-wide resource '%s' not allowed in match.resources.kinds", kind) } } } @@ -1423,7 +1365,6 @@ func podControllerAutoGenExclusion(policy *kyverno.ClusterPolicy) bool { val, ok := annotations["pod-policies.kyverno.io/autogen-controllers"] reorderVal := strings.Split(strings.ToLower(val), ",") sort.Slice(reorderVal, func(i, j int) bool { return reorderVal[i] < reorderVal[j] }) - if ok && strings.ToLower(val) == "none" || reflect.DeepEqual(reorderVal, []string{"cronjob", "daemonset", "deployment", "job", "statefulset"}) == false { return true }