From 8a7250ffef58b3289eb64893f02343bf91a26384 Mon Sep 17 00:00:00 2001 From: Shuting Zhao Date: Fri, 27 Sep 2019 16:31:27 -0700 Subject: [PATCH] refactor policy validation, moved to pkg/api/kyverno --- pkg/api/kyverno/v1alpha1/utils.go | 103 ++++------------ pkg/api/kyverno/v1alpha1/validate.go | 176 +++++++++++++++++++++++++++ pkg/webhooks/policyvalidation.go | 86 +------------ 3 files changed, 202 insertions(+), 163 deletions(-) create mode 100644 pkg/api/kyverno/v1alpha1/validate.go diff --git a/pkg/api/kyverno/v1alpha1/utils.go b/pkg/api/kyverno/v1alpha1/utils.go index 0df80c05df..50b3b55832 100644 --- a/pkg/api/kyverno/v1alpha1/utils.go +++ b/pkg/api/kyverno/v1alpha1/utils.go @@ -3,86 +3,8 @@ package v1alpha1 import ( "errors" "fmt" - "reflect" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -func (r *Rule) HasMutate() bool { - return !reflect.DeepEqual(r.Mutation, Mutation{}) -} - -func (r *Rule) HasValidate() bool { - return !reflect.DeepEqual(r.Validation, Validation{}) -} - -func (r *Rule) HasGenerate() bool { - return !reflect.DeepEqual(r.Generation, Generation{}) -} - -// Validate checks if rule is not empty and all substructures are valid -func (r *Rule) Validate() error { - // check matches Resoource Description of match resource - err := r.MatchResources.ResourceDescription.Validate() - if err != nil { - return err - } - - return nil -} - -// Validate checks if all necesarry fields are present and have values. Also checks a Selector. -// Returns error if -// - kinds is not defined -func (pr *ResourceDescription) Validate() error { - if len(pr.Kinds) == 0 { - return errors.New("The Kind is not specified") - } - - if pr.Selector != nil { - selector, err := metav1.LabelSelectorAsSelector(pr.Selector) - if err != nil { - return err - } - requirements, _ := selector.Requirements() - if len(requirements) == 0 { - return errors.New("The requirements are not specified in selector") - } - } - - return nil -} - -// Validate if all mandatory PolicyPatch fields are set -func (pp *Patch) Validate() error { - if pp.Path == "" { - return errors.New("JSONPatch field 'path' is mandatory") - } - - if pp.Operation == "add" || pp.Operation == "replace" { - if pp.Value == nil { - return fmt.Errorf("JSONPatch field 'value' is mandatory for operation '%s'", pp.Operation) - } - - return nil - } else if pp.Operation == "remove" { - return nil - } - - return fmt.Errorf("Unsupported JSONPatch operation '%s'", pp.Operation) -} - -// Validate returns error if generator is configured incompletely -func (gen *Generation) Validate() error { - if gen.Data == nil && gen.Clone == (CloneFrom{}) { - return fmt.Errorf("Neither data nor clone (source) of %s is specified", gen.Kind) - } - if gen.Data != nil && gen.Clone != (CloneFrom{}) { - return fmt.Errorf("Both data nor clone (source) of %s are specified", gen.Kind) - } - return nil -} - // DeepCopyInto is declared because k8s:deepcopy-gen is // not able to generate this method for interface{} member func (in *Mutation) DeepCopyInto(out *Mutation) { @@ -122,3 +44,28 @@ func (rs ResourceSpec) ToKey() string { } return rs.Kind + "." + rs.Namespace + "." + rs.Name } + +// joinErrs joins the list of error into single error +// adds a new line between errors +func joinErrs(errs []error) error { + if len(errs) == 0 { + return nil + } + + res := "\n" + for _, err := range errs { + res = fmt.Sprintf(res + err.Error() + "\n") + } + + return errors.New(res) +} + +//Contains Check if strint is contained in a list of string +func containString(list []string, element string) bool { + for _, e := range list { + if e == element { + return true + } + } + return false +} diff --git a/pkg/api/kyverno/v1alpha1/validate.go b/pkg/api/kyverno/v1alpha1/validate.go new file mode 100644 index 0000000000..d58ecc68e7 --- /dev/null +++ b/pkg/api/kyverno/v1alpha1/validate.go @@ -0,0 +1,176 @@ +package v1alpha1 + +import ( + "errors" + "fmt" + "reflect" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func (p ClusterPolicy) Validate() error { + var errs []error + + for _, rule := range p.Spec.Rules { + err := rule.Validate() + errs = append(errs, err...) + } + + if err := p.ValidateUniqueRuleName(); err != nil { + errs = append(errs, err) + } + + return joinErrs(errs) +} + +// Validate checks if rule is not empty and all substructures are valid +func (r Rule) Validate() []error { + var errs []error + + // only one type of rule is allowed per rule + if err := r.ValidateRuleType(); err != nil { + errs = append(errs, err) + } + + // validate resource description block + if err := r.MatchResources.ResourceDescription.Validate(); err != nil { + errs = append(errs, err) + } + + if err := r.ExcludeResources.ResourceDescription.Validate(); err != nil { + errs = append(errs, err) + } + + // validate validation rule + if err := r.ValidateOverlayPattern(); err != nil { + errs = append(errs, err) + } + + return errs +} + +// validateOverlayPattern checks one of pattern/anyPattern must exist +func (r Rule) ValidateOverlayPattern() error { + if reflect.DeepEqual(r.Validation, Validation{}) { + return nil + } + + if r.Validation.Pattern == nil && len(r.Validation.AnyPattern) == 0 { + return fmt.Errorf("neither pattern nor anyPattern found in rule '%s'", r.Name) + } + + if r.Validation.Pattern != nil && len(r.Validation.AnyPattern) != 0 { + return fmt.Errorf("either pattern or anyPattern is allowed in rule '%s'", r.Name) + } + + return nil +} + +// ValidateExistingAnchor +// existing acnchor must define on array +func (r Rule) ValidateExistingAnchor() error { + + return nil +} + +// ValidateUniqueRuleName checks if the rule names are unique across a policy +func (p ClusterPolicy) ValidateUniqueRuleName() error { + var ruleNames []string + + for _, rule := range p.Spec.Rules { + if containString(ruleNames, rule.Name) { + return fmt.Errorf(`duplicate rule name: '%s'`, rule.Name) + } + ruleNames = append(ruleNames, rule.Name) + } + return nil +} + +// validateRuleType checks only one type of rule is defined per rule +func (r Rule) ValidateRuleType() error { + mutate := r.HasMutate() + validate := r.HasValidate() + generate := r.HasGenerate() + + if !mutate && !validate && !generate { + return fmt.Errorf("no rule defined in '%s'", r.Name) + } + + if (mutate && !validate && !generate) || + (!mutate && validate && !generate) || + (!mutate && !validate && generate) { + return nil + } + + return fmt.Errorf("multiple types of rule defined in rule '%s', only one type of rule is allowed per rule", r.Name) +} + +func (r Rule) HasMutate() bool { + return !reflect.DeepEqual(r.Mutation, Mutation{}) +} + +func (r Rule) HasValidate() bool { + return !reflect.DeepEqual(r.Validation, Validation{}) +} + +func (r Rule) HasGenerate() bool { + return !reflect.DeepEqual(r.Generation, Generation{}) +} + +// Validate checks if all necesarry fields are present and have values. Also checks a Selector. +// field type is checked through openapi +// Returns error if +// - kinds is empty array, i.e. kinds: [] +// - selector is invalid +func (rd ResourceDescription) Validate() error { + if reflect.DeepEqual(rd, ResourceDescription{}) { + return nil + } + + if len(rd.Kinds) == 0 { + return errors.New("field Kind is not specified") + } + + if rd.Selector != nil { + selector, err := metav1.LabelSelectorAsSelector(rd.Selector) + if err != nil { + return err + } + requirements, _ := selector.Requirements() + if len(requirements) == 0 { + return errors.New("the requirements are not specified in selector") + } + } + + return nil +} + +// Validate if all mandatory PolicyPatch fields are set +func (pp *Patch) Validate() error { + if pp.Path == "" { + return errors.New("JSONPatch field 'path' is mandatory") + } + + if pp.Operation == "add" || pp.Operation == "replace" { + if pp.Value == nil { + return fmt.Errorf("JSONPatch field 'value' is mandatory for operation '%s'", pp.Operation) + } + + return nil + } else if pp.Operation == "remove" { + return nil + } + + return fmt.Errorf("Unsupported JSONPatch operation '%s'", pp.Operation) +} + +// Validate returns error if generator is configured incompletely +func (gen *Generation) Validate() error { + if gen.Data == nil && gen.Clone == (CloneFrom{}) { + return fmt.Errorf("Neither data nor clone (source) of %s is specified", gen.Kind) + } + if gen.Data != nil && gen.Clone != (CloneFrom{}) { + return fmt.Errorf("Both data nor clone (source) of %s are specified", gen.Kind) + } + return nil +} diff --git a/pkg/webhooks/policyvalidation.go b/pkg/webhooks/policyvalidation.go index 6f65c35486..94494d62c7 100644 --- a/pkg/webhooks/policyvalidation.go +++ b/pkg/webhooks/policyvalidation.go @@ -2,13 +2,10 @@ package webhooks import ( "encoding/json" - "errors" "fmt" - "reflect" "github.com/golang/glog" kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1alpha1" - "github.com/nirmata/kyverno/pkg/utils" v1beta1 "k8s.io/api/admission/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -30,7 +27,7 @@ func (ws *WebhookServer) handlePolicyValidation(request *v1beta1.AdmissionReques }} } - if err := ws.validatePolicy(policy); err != nil { + if err := policy.Validate(); err != nil { admissionResp = &v1beta1.AdmissionResponse{ Allowed: false, Result: &metav1.Status{ @@ -60,87 +57,6 @@ func (ws *WebhookServer) handlePolicyValidation(request *v1beta1.AdmissionReques return admissionResp } -func (ws *WebhookServer) validatePolicy(policy *kyverno.ClusterPolicy) error { - // validate only one type of rule defined per rule - if err := validateRuleType(policy); err != nil { - return err - } - - // validate resource description block - - // validate ^() can only be used on array - - // check for uniqueness of rule names while CREATE/DELET - if err := validateUniqueRuleName(policy); err != nil { - return err - } - - if err := validateOverlayPattern(policy); err != nil { - return err - } - - return nil -} - -// validateRuleType checks only one type of rule is defined per rule -func validateRuleType(policy *kyverno.ClusterPolicy) error { - for _, rule := range policy.Spec.Rules { - mutate := rule.HasMutate() - validate := rule.HasValidate() - generate := rule.HasGenerate() - - if !mutate && !validate && !generate { - return fmt.Errorf("No rule defined in '%s'", rule.Name) - } - - if (mutate && !validate && !generate) || - (!mutate && validate && !generate) || - (!mutate && !validate && generate) { - return nil - } - - return fmt.Errorf("Multiple types of rule defined in rule '%s', only one type of rule is allowed per rule", rule.Name) - } - - return nil -} - -// Verify if the Rule names are unique within a policy -func validateUniqueRuleName(policy *kyverno.ClusterPolicy) error { - var ruleNames []string - - for _, rule := range policy.Spec.Rules { - if utils.ContainsString(ruleNames, rule.Name) { - msg := fmt.Sprintf(`The policy "%s" is invalid: duplicate rule name: "%s"`, policy.Name, rule.Name) - glog.Errorln(msg) - return errors.New(msg) - } - ruleNames = append(ruleNames, rule.Name) - } - - glog.V(4).Infof("Policy validation passed") - return nil -} - -// validateOverlayPattern checks one of pattern/anyPattern must exist -func validateOverlayPattern(policy *kyverno.ClusterPolicy) error { - for _, rule := range policy.Spec.Rules { - if reflect.DeepEqual(rule.Validation, kyverno.Validation{}) { - continue - } - - if rule.Validation.Pattern == nil && len(rule.Validation.AnyPattern) == 0 { - return errors.New(fmt.Sprintf("Invalid policy, neither pattern nor anyPattern found in validate rule %s", rule.Name)) - } - - if rule.Validation.Pattern != nil && len(rule.Validation.AnyPattern) != 0 { - return errors.New(fmt.Sprintf("Invalid policy, either pattern or anyPattern is allowed in validate rule %s", rule.Name)) - } - } - - return nil -} - func failResponseWithMsg(msg string) *v1beta1.AdmissionResponse { return &v1beta1.AdmissionResponse{ Allowed: false,