From f608d4db180d4fcaa1d2e5f1f5ea586768db1aad Mon Sep 17 00:00:00 2001 From: shivkumar dudhani Date: Tue, 4 Feb 2020 12:13:41 -0800 Subject: [PATCH 01/12] variable substitution on copy and retry generate resource creation --- pkg/engine/generation.go | 5 +- pkg/engine/mutation.go | 4 +- pkg/engine/utils.go | 10 ++ pkg/engine/validate/common.go | 2 + pkg/engine/validate/validate.go | 24 ++++- pkg/engine/validation.go | 10 +- pkg/generate/generate.go | 163 +++++++++++++++++++++----------- 7 files changed, 157 insertions(+), 61 deletions(-) diff --git a/pkg/engine/generation.go b/pkg/engine/generation.go index 651943fc32..649db6d86e 100644 --- a/pkg/engine/generation.go +++ b/pkg/engine/generation.go @@ -35,9 +35,10 @@ func filterRule(rule kyverno.Rule, resource unstructured.Unstructured, admission if !MatchesResourceDescription(resource, rule) { return nil } - + // operate on the copy of the conditions, as we perform variable substitution + copyConditions := copyConditions(rule.Conditions) // evaluate pre-conditions - if !variables.EvaluateConditions(ctx, rule.Conditions) { + if !variables.EvaluateConditions(ctx, copyConditions) { glog.V(4).Infof("resource %s/%s does not satisfy the conditions for the rule ", resource.GetNamespace(), resource.GetName()) return nil } diff --git a/pkg/engine/mutation.go b/pkg/engine/mutation.go index 0764847838..0c1eb057ca 100644 --- a/pkg/engine/mutation.go +++ b/pkg/engine/mutation.go @@ -73,8 +73,10 @@ func Mutate(policyContext PolicyContext) (resp response.EngineResponse) { continue } + // operate on the copy of the conditions, as we perform variable substitution + copyConditions := copyConditions(rule.Conditions) // evaluate pre-conditions - if !variables.EvaluateConditions(ctx, rule.Conditions) { + if !variables.EvaluateConditions(ctx, copyConditions) { glog.V(4).Infof("resource %s/%s does not satisfy the conditions for the rule ", resource.GetNamespace(), resource.GetName()) continue } diff --git a/pkg/engine/utils.go b/pkg/engine/utils.go index 3d7384c846..a0680cdd97 100644 --- a/pkg/engine/utils.go +++ b/pkg/engine/utils.go @@ -257,3 +257,13 @@ func newPathNotPresentRuleResponse(rname, rtype, msg string) response.RuleRespon PathNotPresent: true, } } + +func copyConditions(original []kyverno.Condition) []kyverno.Condition { + var copy []kyverno.Condition + for _, condition := range original { + copyCondition := kyverno.Condition{} + condition.DeepCopyInto(©Condition) + copy = append(copy, copyCondition) + } + return copy +} diff --git a/pkg/engine/validate/common.go b/pkg/engine/validate/common.go index e45bd023ed..d577fe1053 100644 --- a/pkg/engine/validate/common.go +++ b/pkg/engine/validate/common.go @@ -13,6 +13,8 @@ const ( PathNotPresent ValidationFailureReason = iota // Rulefailure if the rule failed Rulefailure + // OtherError if there is any other type of error + OtherError ) // convertToString converts value to string diff --git a/pkg/engine/validate/validate.go b/pkg/engine/validate/validate.go index a0dab545ba..9c31ad2f4d 100644 --- a/pkg/engine/validate/validate.go +++ b/pkg/engine/validate/validate.go @@ -1,6 +1,7 @@ package validate import ( + "encoding/json" "errors" "fmt" "path/filepath" @@ -23,11 +24,16 @@ func ValidateResourceWithPattern(ctx context.EvalInterface, resource, pattern in return "", newValidatePatternError(PathNotPresent, invalidPaths) } + // Operate on copy of the value for variable substitution + patternCopy, err := copyInterface(pattern) + if err != nil { + return "", newValidatePatternError(OtherError, err.Error()) + } // first pass we substitute all the JMESPATH substitution for the variable // variable: {{}} // if a JMESPATH fails, we dont return error but variable is substitured with nil and error log - pattern = variables.SubstituteVariables(ctx, pattern) - path, err := validateResourceElement(resource, pattern, pattern, "/") + pattern = variables.SubstituteVariables(ctx, patternCopy) + path, err := validateResourceElement(resource, patternCopy, patternCopy, "/") if err != nil { return path, newValidatePatternError(Rulefailure, err.Error()) } @@ -35,6 +41,20 @@ func ValidateResourceWithPattern(ctx context.EvalInterface, resource, pattern in return "", ValidationError{} } +func copyInterface(original interface{}) (interface{}, error) { + tempData, err := json.Marshal(original) + if err != nil { + return nil, err + } + fmt.Println(string(tempData)) + var temp interface{} + err = json.Unmarshal(tempData, &temp) + if err != nil { + return nil, err + } + return temp, nil +} + // validateResourceElement detects the element type (map, array, nil, string, int, bool, float) // and calls corresponding handler // Pattern tree and resource tree can have different structure. In this case validation fails diff --git a/pkg/engine/validation.go b/pkg/engine/validation.go index 616629485b..87c9b04b91 100644 --- a/pkg/engine/validation.go +++ b/pkg/engine/validation.go @@ -118,8 +118,10 @@ func validateResource(ctx context.EvalInterface, policy kyverno.ClusterPolicy, r continue } + // operate on the copy of the conditions, as we perform variable substitution + copyConditions := copyConditions(rule.Conditions) // evaluate pre-conditions - if !variables.EvaluateConditions(ctx, rule.Conditions) { + if !variables.EvaluateConditions(ctx, copyConditions) { glog.V(4).Infof("resource %s/%s does not satisfy the conditions for the rule ", resource.GetNamespace(), resource.GetName()) continue } @@ -199,6 +201,12 @@ func validatePatterns(ctx context.EvalInterface, resource unstructured.Unstructu resp.Success = false resp.Message = fmt.Sprintf("Validation error: %s; Validation rule '%s' failed at path '%s'", rule.Validation.Message, rule.Name, path) + case validate.OtherError: + // other error + glog.V(4).Infof("Validation rule '%s' failed at '%s' for resource %s/%s/%s. %s: %v", rule.Name, path, resource.GetKind(), resource.GetNamespace(), resource.GetName(), rule.Validation.Message, err) + resp.Success = false + resp.Message = fmt.Sprintf("Validation error: %s; Validation rule '%s' failed at path '%s'", + rule.Validation.Message, rule.Name, path) } return resp } diff --git a/pkg/generate/generate.go b/pkg/generate/generate.go index 11ab1df346..47214a5220 100644 --- a/pkg/generate/generate.go +++ b/pkg/generate/generate.go @@ -1,6 +1,7 @@ package generate import ( + "encoding/json" "errors" "fmt" "reflect" @@ -104,7 +105,7 @@ func (c *Controller) applyGenerate(resource unstructured.Unstructured, gr kyvern } // Apply the generate rule on resource - return applyGeneratePolicy(c.client, policyContext, gr.Status.State) + return applyGeneratePolicy(c.client, policyContext) } func updateStatus(statusControl StatusControlInterface, gr kyverno.GenerateRequest, err error, genResources []kyverno.ResourceSpec) error { @@ -116,7 +117,7 @@ func updateStatus(statusControl StatusControlInterface, gr kyverno.GenerateReque return statusControl.Success(gr, genResources) } -func applyGeneratePolicy(client *dclient.Client, policyContext engine.PolicyContext, state kyverno.GenerateRequestState) ([]kyverno.ResourceSpec, error) { +func applyGeneratePolicy(client *dclient.Client, policyContext engine.PolicyContext) ([]kyverno.ResourceSpec, error) { // List of generatedResources var genResources []kyverno.ResourceSpec // Get the response as the actions to be performed on the resource @@ -135,7 +136,7 @@ func applyGeneratePolicy(client *dclient.Client, policyContext engine.PolicyCont if !rule.HasGenerate() { continue } - genResource, err := applyRule(client, rule, resource, ctx, state, processExisting) + genResource, err := applyRule(client, rule, resource, ctx, processExisting) if err != nil { return nil, err } @@ -145,9 +146,10 @@ func applyGeneratePolicy(client *dclient.Client, policyContext engine.PolicyCont return genResources, nil } -func applyRule(client *dclient.Client, rule kyverno.Rule, resource unstructured.Unstructured, ctx context.EvalInterface, state kyverno.GenerateRequestState, processExisting bool) (kyverno.ResourceSpec, error) { +func applyRule(client *dclient.Client, rule kyverno.Rule, resource unstructured.Unstructured, ctx context.EvalInterface, processExisting bool) (kyverno.ResourceSpec, error) { var rdata map[string]interface{} var err error + var mode ResourceMode var noGenResource kyverno.ResourceSpec if invalidPaths := variables.ValidateVariables(ctx, rule.Generation.ResourceSpec); len(invalidPaths) != 0 { @@ -169,11 +171,12 @@ func applyRule(client *dclient.Client, rule kyverno.Rule, resource unstructured. // DATA if gen.Data != nil { - if rdata, err = handleData(rule.Name, gen, client, resource, ctx, state); err != nil { + if rdata, mode, err = handleData(rule.Name, gen, client, resource, ctx); err != nil { glog.V(4).Info(err) switch e := err.(type) { case *ParseFailed, *NotFound, *ConfigNotFound: // handled errors + return noGenResource, e case *Violation: // create policy violation return noGenResource, e @@ -189,7 +192,7 @@ func applyRule(client *dclient.Client, rule kyverno.Rule, resource unstructured. } // CLONE if gen.Clone != (kyverno.CloneFrom{}) { - if rdata, err = handleClone(rule.Name, gen, client, resource, ctx, state); err != nil { + if rdata, mode, err = handleClone(rule.Name, gen, client, resource, ctx); err != nil { glog.V(4).Info(err) switch e := err.(type) { case *NotFound: @@ -211,21 +214,36 @@ func applyRule(client *dclient.Client, rule kyverno.Rule, resource unstructured. // we do not create new resource return noGenResource, err } - // Create the generate resource + + // build the resource template newResource := &unstructured.Unstructured{} newResource.SetUnstructuredContent(rdata) newResource.SetName(gen.Name) newResource.SetNamespace(gen.Namespace) - // Reset resource version - newResource.SetResourceVersion("") - glog.V(4).Infof("creating resource %v", newResource) - _, err = client.CreateResource(gen.Kind, gen.Namespace, newResource, false) - if err != nil { - glog.Info(err) - return noGenResource, err + if mode == Create { + // Reset resource version + newResource.SetResourceVersion("") + // Create the resource + glog.V(4).Infof("Creating new resource %s/%s/%s", gen.Kind, gen.Namespace, gen.Name) + _, err = client.CreateResource(gen.Kind, gen.Namespace, newResource, false) + if err != nil { + // Failed to create resource + return noGenResource, err + } + glog.V(4).Infof("Created new resource %s/%s/%s", gen.Kind, gen.Namespace, gen.Name) + + } else if mode == Update { + glog.V(4).Infof("Updating existing resource %s/%s/%s", gen.Kind, gen.Namespace, gen.Name) + // Update the resource + _, err := client.UpdateResource(gen.Kind, gen.Namespace, newResource, false) + if err != nil { + // Failed to update resource + return noGenResource, err + } + glog.V(4).Infof("Updated existing resource %s/%s/%s", gen.Kind, gen.Namespace, gen.Name) } - glog.V(4).Infof("created new resource %s %s %s ", gen.Kind, gen.Namespace, gen.Name) + return newGenResource, nil } @@ -261,81 +279,116 @@ func variableSubsitutionForAttributes(gen kyverno.Generation, ctx context.EvalIn return gen } -func handleData(ruleName string, generateRule kyverno.Generation, client *dclient.Client, resource unstructured.Unstructured, ctx context.EvalInterface, state kyverno.GenerateRequestState) (map[string]interface{}, error) { - if invalidPaths := variables.ValidateVariables(ctx, generateRule.Data); len(invalidPaths) != 0 { - return nil, NewViolation(ruleName, fmt.Errorf("path not present in generate data: %s", invalidPaths)) - } +// ResourceMode defines the mode for generated resource +type ResourceMode string - //work on copy - copyDataTemp := reflect.Indirect(reflect.ValueOf(generateRule.Data)) - copyData := copyDataTemp.Interface() - newData := variables.SubstituteVariables(ctx, copyData) +const ( + //Skip : failed to process rule, will not update the resource + Skip ResourceMode = "SKIP" + //Create : create a new resource + Create = "CREATE" + //Update : update/override the new resource + Update = "UPDATE" +) + +func copyInterface(original interface{}) (interface{}, error) { + tempData, err := json.Marshal(original) + if err != nil { + return nil, err + } + fmt.Println(string(tempData)) + var temp interface{} + err = json.Unmarshal(tempData, &temp) + if err != nil { + return nil, err + } + return temp, nil +} + +// manage the creation/update of resource to be generated using the spec defined in the policy +func handleData(ruleName string, generateRule kyverno.Generation, client *dclient.Client, resource unstructured.Unstructured, ctx context.EvalInterface) (map[string]interface{}, ResourceMode, error) { + //work on copy of the data + // as the type of data stoed in interface is not know, + // we marshall the data and unmarshal it into a new resource to create a copy + dataCopy, err := copyInterface(generateRule.Data) + if err != nil { + glog.V(4).Infof("failed to create a copy of the interface %v", generateRule.Data) + return nil, Skip, err + } + // replace variables with the corresponding values + newData := variables.SubstituteVariables(ctx, dataCopy) + // if any variable defined in the data is not avaialbe in the context + if invalidPaths := variables.ValidateVariables(ctx, newData); len(invalidPaths) != 0 { + return nil, Skip, NewViolation(ruleName, fmt.Errorf("path not present in generate data: %s", invalidPaths)) + } // check if resource exists obj, err := client.GetResource(generateRule.Kind, generateRule.Namespace, generateRule.Name) - glog.V(4).Info(err) if apierrors.IsNotFound(err) { - glog.V(4).Info(string(state)) // Resource does not exist - if state == "" { - // Processing the request first time - rdata, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&newData) - glog.V(4).Info(err) - if err != nil { - return nil, NewParseFailed(newData, err) - } - return rdata, nil + // Processing the request first time + rdata, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&newData) + if err != nil { + return nil, Skip, NewParseFailed(newData, err) } - glog.V(4).Info("Creating violation") - // State : Failed,Completed - // request has been processed before, so dont create the resource - // report Violation to notify the error - return nil, NewViolation(ruleName, NewNotFound(generateRule.Kind, generateRule.Namespace, generateRule.Name)) + glog.V(4).Infof("Resource %s/%s/%s does not exists, will try to create", generateRule.Kind, generateRule.Namespace, generateRule.Name) + return rdata, Create, nil } if err != nil { //something wrong while fetching resource - return nil, err + return nil, Skip, err } // Resource exists; verfiy the content of the resource ok, err := checkResource(ctx, newData, obj) if err != nil { - //something wrong with configuration - glog.V(4).Info(err) - return nil, err + // error while evaluating if the existing resource contains the required information + return nil, Skip, err } + if !ok { - return nil, NewConfigNotFound(newData, generateRule.Kind, generateRule.Namespace, generateRule.Name) + // existing resource does not contain the configuration mentioned in spec, will try to update + rdata, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&newData) + if err != nil { + return nil, Skip, NewParseFailed(newData, err) + } + + glog.V(4).Infof("Resource %s/%s/%s exists but missing required configuration, will try to update", generateRule.Kind, generateRule.Namespace, generateRule.Name) + return rdata, Update, nil } - // Existing resource does contain the required - return nil, nil + // Existing resource does contain the mentioned configuration in spec, skip processing the resource as it is already in expected state + return nil, Skip, nil } -func handleClone(ruleName string, generateRule kyverno.Generation, client *dclient.Client, resource unstructured.Unstructured, ctx context.EvalInterface, state kyverno.GenerateRequestState) (map[string]interface{}, error) { +// manage the creation/update based on the reference clone resource +func handleClone(ruleName string, generateRule kyverno.Generation, client *dclient.Client, resource unstructured.Unstructured, ctx context.EvalInterface) (map[string]interface{}, ResourceMode, error) { + // if any variable defined in the data is not avaialbe in the context if invalidPaths := variables.ValidateVariables(ctx, generateRule.Clone); len(invalidPaths) != 0 { - return nil, NewViolation(ruleName, fmt.Errorf("path not present in generate clone: %s", invalidPaths)) + return nil, Skip, NewViolation(ruleName, fmt.Errorf("path not present in generate clone: %s", invalidPaths)) } - // check if resource exists + // check if resource to be generated exists _, err := client.GetResource(generateRule.Kind, generateRule.Namespace, generateRule.Name) if err == nil { - // resource exists - return nil, nil + // resource does exists, not need to process further as it is already in expected state + return nil, Skip, nil } if !apierrors.IsNotFound(err) { //something wrong while fetching resource - return nil, err + return nil, Skip, err } - // get reference clone resource + // get clone resource reference in the rule obj, err := client.GetResource(generateRule.Kind, generateRule.Clone.Namespace, generateRule.Clone.Name) if apierrors.IsNotFound(err) { - return nil, NewNotFound(generateRule.Kind, generateRule.Clone.Namespace, generateRule.Clone.Name) + // reference resource does not exist, cant generate the resources + return nil, Skip, NewNotFound(generateRule.Kind, generateRule.Clone.Namespace, generateRule.Clone.Name) } if err != nil { //something wrong while fetching resource - return nil, err + return nil, Skip, err } - return obj.UnstructuredContent(), nil + // create the resource based on the reference clone + return obj.UnstructuredContent(), Create, nil } func checkResource(ctx context.EvalInterface, newResourceSpec interface{}, resource *unstructured.Unstructured) (bool, error) { From 69f65c713df0440ba2724cec9f828b57adaac3d4 Mon Sep 17 00:00:00 2001 From: shivkumar dudhani Date: Tue, 4 Feb 2020 14:35:10 -0800 Subject: [PATCH 02/12] temp --- pkg/generate/generate.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/pkg/generate/generate.go b/pkg/generate/generate.go index 47214a5220..dac8632f89 100644 --- a/pkg/generate/generate.go +++ b/pkg/generate/generate.go @@ -414,3 +414,26 @@ func generatePV(gr kyverno.GenerateRequest, resource unstructured.Unstructured, } return info } + +func addLabels(unstr *unstructured.Unstructured) { + // add managedBY label if not defined + labels := unstr.GetLabels() + if labels == nil { + labels = map[string]string{} + } + // ManagedBy label + key := "app.kubernetes.io/managed-by" + value := "kyverno" + val, ok := labels[key] + if ok { + if val != value { + glog.Infof("resource managed by %s, kyverno wont over-ride the label", val) + } + } + // we dont over-ride the key managed by + if !ok { + // add lable + labels[key] = value + } + +} From 34ad3a9a2ba2cb936804a184d1ccd64adc745f94 Mon Sep 17 00:00:00 2001 From: shivkumar dudhani Date: Mon, 10 Feb 2020 12:44:20 -0800 Subject: [PATCH 03/12] generate rule processing refactoring --- pkg/engine/context/context.go | 2 -- pkg/generate/cleanup/cleanup.go | 48 +++++++++++---------------- pkg/generate/generate.go | 30 ++++------------- pkg/generate/labels.go | 57 +++++++++++++++++++++++++++++++++ 4 files changed, 81 insertions(+), 56 deletions(-) create mode 100644 pkg/generate/labels.go diff --git a/pkg/engine/context/context.go b/pkg/engine/context/context.go index 81d174f2cd..09fcc30d86 100644 --- a/pkg/engine/context/context.go +++ b/pkg/engine/context/context.go @@ -122,7 +122,6 @@ func (ctx *Context) AddSA(userName string) error { saNamespace = groups[0] } - glog.V(4).Infof("Loading variable serviceAccountName with value: %s", saName) saNameObj := struct { SA string `json:"serviceAccountName"` }{ @@ -137,7 +136,6 @@ func (ctx *Context) AddSA(userName string) error { return err } - glog.V(4).Infof("Loading variable serviceAccountNamespace with value: %s", saNamespace) saNsObj := struct { SA string `json:"serviceAccountNamespace"` }{ diff --git a/pkg/generate/cleanup/cleanup.go b/pkg/generate/cleanup/cleanup.go index 7edc7a3cbc..878c4be589 100644 --- a/pkg/generate/cleanup/cleanup.go +++ b/pkg/generate/cleanup/cleanup.go @@ -1,42 +1,24 @@ package cleanup import ( - "time" - "github.com/golang/glog" kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1" dclient "github.com/nirmata/kyverno/pkg/dclient" - "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" ) -const timoutMins = 2 -const timeout = time.Minute * timoutMins // 2 minutes - func (c *Controller) processGR(gr kyverno.GenerateRequest) error { - // 1-Corresponding policy has been deleted - _, err := c.pLister.Get(gr.Spec.Policy) - if errors.IsNotFound(err) { - glog.V(4).Infof("delete GR %s", gr.Name) - return c.control.Delete(gr.Name) - } + // 1- Corresponding policy has been deleted + // then we dont delete the generated resources - // 2- Check for elapsed time since update - if gr.Status.State == kyverno.Completed { - glog.V(4).Infof("checking if owner exists for gr %s", gr.Name) - if !ownerResourceExists(c.client, gr) { - if err := deleteGeneratedResources(c.client, gr); err != nil { - return err - } - glog.V(4).Infof("delete GR %s", gr.Name) - return c.control.Delete(gr.Name) + // 2- The trigger resource is deleted, then delete the generated resources + if !ownerResourceExists(c.client, gr) { + if err := deleteGeneratedResources(c.client, gr); err != nil { + return err } - return nil - } - createTime := gr.GetCreationTimestamp() - if time.Since(createTime.UTC()) > timeout { - // the GR was in state ["",Failed] for more than timeout - glog.V(4).Infof("GR %s was not processed successfully in %d minutes", gr.Name, timoutMins) - glog.V(4).Infof("delete GR %s", gr.Name) + // - trigger-resource is delted + // - generated-resources are delted + // - > Now delete the GenerateRequest CR return c.control.Delete(gr.Name) } return nil @@ -44,16 +26,22 @@ func (c *Controller) processGR(gr kyverno.GenerateRequest) error { func ownerResourceExists(client *dclient.Client, gr kyverno.GenerateRequest) bool { _, err := client.GetResource(gr.Spec.Resource.Kind, gr.Spec.Resource.Namespace, gr.Spec.Resource.Name) - if err != nil { + // trigger resources has been deleted + if apierrors.IsNotFound(err) { return false } + if err != nil { + glog.V(4).Infof("Failed to get resource %s/%s/%s: error : %s", gr.Spec.Resource.Kind, gr.Spec.Resource.Namespace, gr.Spec.Resource.Name, err) + } + // if there was an error while querying the resources we dont delete the generated resources + // but expect the deletion in next reconciliation loop return true } func deleteGeneratedResources(client *dclient.Client, gr kyverno.GenerateRequest) error { for _, genResource := range gr.Status.GeneratedResources { err := client.DeleteResource(genResource.Kind, genResource.Namespace, genResource.Name, false) - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { glog.V(4).Infof("resource %s/%s/%s not found, will no delete", genResource.Kind, genResource.Namespace, genResource.Name) continue } diff --git a/pkg/generate/generate.go b/pkg/generate/generate.go index dac8632f89..9fc5de7835 100644 --- a/pkg/generate/generate.go +++ b/pkg/generate/generate.go @@ -221,6 +221,11 @@ func applyRule(client *dclient.Client, rule kyverno.Rule, resource unstructured. newResource.SetName(gen.Name) newResource.SetNamespace(gen.Namespace) + // manage labels + // - app.kubernetes.io/managed-by: kyverno + // - kyverno.io/generated-by: kind/namespace/name (trigger resource) + manageLabels(newResource, resource) + if mode == Create { // Reset resource version newResource.SetResourceVersion("") @@ -308,7 +313,7 @@ func copyInterface(original interface{}) (interface{}, error) { // manage the creation/update of resource to be generated using the spec defined in the policy func handleData(ruleName string, generateRule kyverno.Generation, client *dclient.Client, resource unstructured.Unstructured, ctx context.EvalInterface) (map[string]interface{}, ResourceMode, error) { //work on copy of the data - // as the type of data stoed in interface is not know, + // as the type of data stored in interface is not know, // we marshall the data and unmarshal it into a new resource to create a copy dataCopy, err := copyInterface(generateRule.Data) if err != nil { @@ -414,26 +419,3 @@ func generatePV(gr kyverno.GenerateRequest, resource unstructured.Unstructured, } return info } - -func addLabels(unstr *unstructured.Unstructured) { - // add managedBY label if not defined - labels := unstr.GetLabels() - if labels == nil { - labels = map[string]string{} - } - // ManagedBy label - key := "app.kubernetes.io/managed-by" - value := "kyverno" - val, ok := labels[key] - if ok { - if val != value { - glog.Infof("resource managed by %s, kyverno wont over-ride the label", val) - } - } - // we dont over-ride the key managed by - if !ok { - // add lable - labels[key] = value - } - -} diff --git a/pkg/generate/labels.go b/pkg/generate/labels.go new file mode 100644 index 0000000000..282caf55fa --- /dev/null +++ b/pkg/generate/labels.go @@ -0,0 +1,57 @@ +package generate + +import ( + "fmt" + + "github.com/golang/glog" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" +) + +func manageLabels(unstr *unstructured.Unstructured, triggerResource unstructured.Unstructured) { + // add managedBY label if not defined + labels := unstr.GetLabels() + if labels == nil { + labels = map[string]string{} + } + + // handle managedBy label + managedBy(labels) + // handle generatedBy label + generatedBy(labels, triggerResource) + + // update the labels + unstr.SetLabels(labels) +} + +func managedBy(labels map[string]string) { + // ManagedBy label + key := "app.kubernetes.io/managed-by" + value := "kyverno" + val, ok := labels[key] + if ok { + if val != value { + glog.Infof("resource managed by %s, kyverno wont over-ride the label", val) + return + } + } + if !ok { + // add label + labels[key] = value + } +} + +func generatedBy(labels map[string]string, triggerResource unstructured.Unstructured) { + key := "kyverno.io/generated-by" + value := fmt.Sprintf("%s-%s-%s", triggerResource.GetKind(), triggerResource.GetNamespace(), triggerResource.GetName()) + val, ok := labels[key] + if ok { + if val != value { + glog.Infof("resource generated by %s, kyverno wont over-ride the label", val) + return + } + } + if !ok { + // add label + labels[key] = value + } +} From f4c1973c36811aa6377eabf5860acf6cd7d55d01 Mon Sep 17 00:00:00 2001 From: shivkumar dudhani Date: Mon, 10 Feb 2020 13:01:40 -0800 Subject: [PATCH 04/12] fix CR comment --- pkg/engine/validate/validate.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/engine/validate/validate.go b/pkg/engine/validate/validate.go index 9c31ad2f4d..278e8f858d 100644 --- a/pkg/engine/validate/validate.go +++ b/pkg/engine/validate/validate.go @@ -32,7 +32,7 @@ func ValidateResourceWithPattern(ctx context.EvalInterface, resource, pattern in // first pass we substitute all the JMESPATH substitution for the variable // variable: {{}} // if a JMESPATH fails, we dont return error but variable is substitured with nil and error log - pattern = variables.SubstituteVariables(ctx, patternCopy) + patternCopy = variables.SubstituteVariables(ctx, patternCopy) path, err := validateResourceElement(resource, patternCopy, patternCopy, "/") if err != nil { return path, newValidatePatternError(Rulefailure, err.Error()) From d855247a98f2c7792c4458ed9d30466ed2e475a8 Mon Sep 17 00:00:00 2001 From: shivkumar dudhani Date: Thu, 13 Feb 2020 13:57:48 -0800 Subject: [PATCH 05/12] refactor generate to use variable substition at rule level --- pkg/api/kyverno/v1/types.go | 4 +- pkg/engine/validate/validate.go | 11 + pkg/engine/variables/vars.go | 167 ++++++++++++++ pkg/engine/variables/vars_test.go | 130 +++++++++++ pkg/generate/cleanup/cleanup.go | 4 +- pkg/generate/generate.go | 352 +++++++++++------------------- pkg/generate/report.go | 54 +---- 7 files changed, 443 insertions(+), 279 deletions(-) create mode 100644 pkg/engine/variables/vars.go create mode 100644 pkg/engine/variables/vars_test.go diff --git a/pkg/api/kyverno/v1/types.go b/pkg/api/kyverno/v1/types.go index b3ba284b7d..34a3273b18 100644 --- a/pkg/api/kyverno/v1/types.go +++ b/pkg/api/kyverno/v1/types.go @@ -216,8 +216,8 @@ type Validation struct { // Generation describes which resources will be created when other resource is created type Generation struct { ResourceSpec - Data interface{} `json:"data"` - Clone CloneFrom `json:"clone"` + Data interface{} `json:"data,omitempty"` + Clone CloneFrom `json:"clone,omitempty"` } // CloneFrom - location of the resource diff --git a/pkg/engine/validate/validate.go b/pkg/engine/validate/validate.go index 278e8f858d..eafdacb034 100644 --- a/pkg/engine/validate/validate.go +++ b/pkg/engine/validate/validate.go @@ -41,6 +41,17 @@ func ValidateResourceWithPattern(ctx context.EvalInterface, resource, pattern in return "", ValidationError{} } +// ValidateResourceWithPattern1 is a start of element-by-element validation process +// It assumes that validation is started from root, so "/" is passed +func ValidateResourceWithPattern1(resource, pattern interface{}) (string, error) { + path, err := validateResourceElement(resource, pattern, pattern, "/") + if err != nil { + return path, err + } + + return "", nil +} + func copyInterface(original interface{}) (interface{}, error) { tempData, err := json.Marshal(original) if err != nil { diff --git a/pkg/engine/variables/vars.go b/pkg/engine/variables/vars.go new file mode 100644 index 0000000000..36e1b3bf6f --- /dev/null +++ b/pkg/engine/variables/vars.go @@ -0,0 +1,167 @@ +package variables + +import ( + "fmt" + "regexp" + "strconv" + "strings" + + "github.com/golang/glog" + "github.com/nirmata/kyverno/pkg/engine/context" + "github.com/nirmata/kyverno/pkg/engine/operator" +) + +//SubstituteVars replaces the variables with the values defined in the context +// - if any variable is invaid or has nil value, it is considered as a failed varable substitution +func SubstituteVars(ctx context.EvalInterface, pattern interface{}) error { + errs := []error{} + subVars(ctx, pattern, "", &errs) + if len(errs) == 0 { + // no error while parsing the pattern + return nil + } + return fmt.Errorf("variable(s) not found or has nil values: %v", errs) +} + +func subVars(ctx context.EvalInterface, pattern interface{}, path string, errs *[]error) interface{} { + switch typedPattern := pattern.(type) { + case map[string]interface{}: + return subMap(ctx, typedPattern, path, errs) + case []interface{}: + return subArray(ctx, typedPattern, path, errs) + case string: + return subVal(ctx, typedPattern, path, errs) + default: + return pattern + } +} + +func subMap(ctx context.EvalInterface, patternMap map[string]interface{}, path string, errs *[]error) map[string]interface{} { + for key, patternElement := range patternMap { + curPath := path + "/" + key + value := subVars(ctx, patternElement, curPath, errs) + patternMap[key] = value + + } + return patternMap +} + +func subArray(ctx context.EvalInterface, patternList []interface{}, path string, errs *[]error) []interface{} { + for idx, patternElement := range patternList { + curPath := path + "/" + strconv.Itoa(idx) + value := subVars(ctx, patternElement, curPath, errs) + patternList[idx] = value + } + return patternList +} + +func subVal(ctx context.EvalInterface, valuePattern string, path string, errs *[]error) interface{} { + operatorVariable := getOp(valuePattern) + variable := valuePattern[len(operatorVariable):] + // substitute variable with value + value, failedVars := getValQuery(ctx, variable) + // if there are failedVars at this level + // capture as error and the path to the variables + for _, failedVar := range failedVars { + failedPath := path + "/" + failedVar + *errs = append(*errs, NewInvalidPath(failedPath)) + } + if operatorVariable == "" { + // default or operator.Equal + // equal + string value + // object variable + return value + } + // operator + string variable + switch value.(type) { + case string: + return string(operatorVariable) + value.(string) + default: + glog.Infof("cannot use operator with object variables. operator used %s in pattern %v", string(operatorVariable), valuePattern) + var emptyInterface interface{} + return emptyInterface + } + +} + +func getOp(pattern string) string { + operatorVariable := operator.GetOperatorFromStringPattern(pattern) + if operatorVariable == operator.Equal { + return "" + } + return string(operatorVariable) +} + +func getValQuery(ctx context.EvalInterface, valuePattern string) (interface{}, []string) { + var emptyInterface interface{} + validRegex := regexp.MustCompile(variableRegex) + groups := validRegex.FindAllStringSubmatch(valuePattern, -1) + // there can be multiple varialbes in a single value pattern + varMap, failedVars := getVal(ctx, groups) + if len(varMap) == 0 && len(failedVars) == 0 { + // no variables + // return original value + return valuePattern, nil + } + if isAllStrings(varMap) { + newVal := valuePattern + for key, value := range varMap { + if val, ok := value.(string); ok { + newVal = strings.Replace(newVal, key, val, -1) + } + } + return newVal, failedVars + } + // multiple substitution per statement for non-string types are not supported + for _, value := range varMap { + return value, failedVars + } + return emptyInterface, failedVars +} + +func getVal(ctx context.EvalInterface, groups [][]string) (map[string]interface{}, []string) { + substiutions := map[string]interface{}{} + var failedVars []string + for _, group := range groups { + // 0th is the string + varName := group[0] + varValue := group[1] + variable, err := ctx.Query(varValue) + // err !=nil -> invalid expression + // err == nil && variable == nil -> variable is empty or path is not present + // a variable with empty value is considered as a failed variable + if err != nil || (err == nil && variable == nil) { + // could not find the variable at the given path + failedVars = append(failedVars, varName) + continue + } + substiutions[varName] = variable + } + return substiutions, failedVars +} + +func isAllStrings(subVar map[string]interface{}) bool { + if len(subVar) == 0 { + return false + } + for _, value := range subVar { + if _, ok := value.(string); !ok { + return false + } + } + return true +} + +//InvalidPath stores the path to failed variable +type InvalidPath struct { + path string +} + +func (e *InvalidPath) Error() string { + return e.path +} + +//NewInvalidPath returns a new Invalid Path error +func NewInvalidPath(path string) *InvalidPath { + return &InvalidPath{path: path} +} diff --git a/pkg/engine/variables/vars_test.go b/pkg/engine/variables/vars_test.go new file mode 100644 index 0000000000..92ac0ac4a7 --- /dev/null +++ b/pkg/engine/variables/vars_test.go @@ -0,0 +1,130 @@ +package variables + +import ( + "encoding/json" + "testing" + + "github.com/nirmata/kyverno/pkg/engine/context" +) + +func Test_subVars_success(t *testing.T) { + patternMap := []byte(` + { + "kind": "{{request.object.metadata.name}}", + "name": "ns-owner-{{request.object.metadata.name}}", + "data": { + "rules": [ + { + "apiGroups": [ + "{{request.object.metadata.name}}" + ], + "resources": [ + "namespaces" + ], + "verbs": [ + "*" + ], + "resourceNames": [ + "{{request.object.metadata.name}}" + ] + } + ] + } + } + `) + + resourceRaw := []byte(` + { + "metadata": { + "name": "temp", + "namespace": "n1" + }, + "spec": { + "namespace": "n1", + "name": "temp1" + } + } + `) + + var pattern, resource interface{} + var err error + err = json.Unmarshal(patternMap, &pattern) + if err != nil { + t.Error(err) + } + err = json.Unmarshal(resourceRaw, &resource) + if err != nil { + t.Error(err) + } + // context + ctx := context.NewContext() + err = ctx.AddResource(resourceRaw) + if err != nil { + t.Error(err) + } + + if err := SubstituteVars(ctx, pattern); err != nil { + t.Error(err) + } +} + +func Test_subVars_failed(t *testing.T) { + patternMap := []byte(` + { + "kind": "{{request.object.metadata.name1}}", + "name": "ns-owner-{{request.object.metadata.name}}", + "data": { + "rules": [ + { + "apiGroups": [ + "{{request.object.metadata.name}}" + ], + "resources": [ + "namespaces" + ], + "verbs": [ + "*" + ], + "resourceNames": [ + "{{request.object.metadata.name1}}" + ] + } + ] + } + } + `) + + resourceRaw := []byte(` + { + "metadata": { + "name": "temp", + "namespace": "n1" + }, + "spec": { + "namespace": "n1", + "name": "temp1" + } + } + `) + + var pattern, resource interface{} + var err error + err = json.Unmarshal(patternMap, &pattern) + if err != nil { + t.Error(err) + } + err = json.Unmarshal(resourceRaw, &resource) + if err != nil { + t.Error(err) + } + // context + ctx := context.NewContext() + err = ctx.AddResource(resourceRaw) + if err != nil { + t.Error(err) + } + + if err := SubstituteVars(ctx, pattern); err == nil { + t.Error("error is expected") + } +} diff --git a/pkg/generate/cleanup/cleanup.go b/pkg/generate/cleanup/cleanup.go index 878c4be589..d25fddd44e 100644 --- a/pkg/generate/cleanup/cleanup.go +++ b/pkg/generate/cleanup/cleanup.go @@ -16,8 +16,8 @@ func (c *Controller) processGR(gr kyverno.GenerateRequest) error { if err := deleteGeneratedResources(c.client, gr); err != nil { return err } - // - trigger-resource is delted - // - generated-resources are delted + // - trigger-resource is deleted + // - generated-resources are deleted // - > Now delete the GenerateRequest CR return c.control.Delete(gr.Name) } diff --git a/pkg/generate/generate.go b/pkg/generate/generate.go index 9fc5de7835..1d2da296c0 100644 --- a/pkg/generate/generate.go +++ b/pkg/generate/generate.go @@ -2,9 +2,7 @@ package generate import ( "encoding/json" - "errors" "fmt" - "reflect" "github.com/golang/glog" kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1" @@ -13,10 +11,8 @@ import ( "github.com/nirmata/kyverno/pkg/engine/context" "github.com/nirmata/kyverno/pkg/engine/validate" "github.com/nirmata/kyverno/pkg/engine/variables" - "github.com/nirmata/kyverno/pkg/policyviolation" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime" ) func (c *Controller) processGR(gr *kyverno.GenerateRequest) error { @@ -30,24 +26,10 @@ func (c *Controller) processGR(gr *kyverno.GenerateRequest) error { glog.V(4).Infof("resource does not exist or is yet to be created, requeuing: %v", err) return err } - // 2 - Apply the generate policy on the resource genResources, err = c.applyGenerate(*resource, *gr) - switch e := err.(type) { - case *Violation: - // Generate event - // - resource -> rule failed and created PV - // - policy -> failed to apply of resource and created PV - c.pvGenerator.Add(generatePV(*gr, *resource, e)) - default: - // Generate event - // - resource -> rule failed - // - policy -> failed tp apply on resource - glog.V(4).Info(e) - } // 3 - Report Events reportEvents(err, c.eventGen, *gr, *resource) - // 4 - Update Status return updateStatus(c.statusControl, *gr, err, genResources) } @@ -97,13 +79,6 @@ func (c *Controller) applyGenerate(resource unstructured.Unstructured, gr kyvern return nil, fmt.Errorf("policy %s, dont not apply to resource %v", gr.Spec.Policy, gr.Spec.Resource) } - if pv := buildPathNotPresentPV(engineResponse); pv != nil { - c.pvGenerator.Add(pv...) - // variable substitiution fails in ruleInfo (match,exclude,condition) - // the overall policy should not apply to resource - return nil, fmt.Errorf("referenced path not present in generate policy %s", policy.Name) - } - // Apply the generate rule on resource return applyGeneratePolicy(c.client, policyContext) } @@ -152,61 +127,57 @@ func applyRule(client *dclient.Client, rule kyverno.Rule, resource unstructured. var mode ResourceMode var noGenResource kyverno.ResourceSpec - if invalidPaths := variables.ValidateVariables(ctx, rule.Generation.ResourceSpec); len(invalidPaths) != 0 { - return noGenResource, NewViolation(rule.Name, fmt.Errorf("path not present in generate resource spec: %s", invalidPaths)) + // convert to unstructured Resource + genUnst, err := getUnstrRule(rule.Generation.DeepCopy()) + if err != nil { + return noGenResource, err + } + glog.V(4).Info("applyRule1 %v", genUnst) + + // Variable substitutions + // format : {{ results in error and rule is not applied + // - valid variables are replaced with the values + if err := variables.SubstituteVars(ctx, genUnst.Object); err != nil { + return noGenResource, err + } + glog.V(4).Info("applyRule2 %v", genUnst.Object) + genKind, _, err := unstructured.NestedString(genUnst.Object, "kind") + if err != nil { + return noGenResource, err + } + genName, _, err := unstructured.NestedString(genUnst.Object, "name") + if err != nil { + return noGenResource, err + } + genNamespace, _, err := unstructured.NestedString(genUnst.Object, "namespace") + if err != nil { + return noGenResource, err } - // variable substitution - // - name - // - namespace - // - clone.name - // - clone.namespace - gen := variableSubsitutionForAttributes(rule.Generation, ctx) // Resource to be generated newGenResource := kyverno.ResourceSpec{ - Kind: gen.Kind, - Namespace: gen.Namespace, - Name: gen.Name, + Kind: genKind, + Namespace: genNamespace, + Name: genName, + } + genData, _, err := unstructured.NestedMap(genUnst.Object, "data") + if err != nil { + return noGenResource, err + } + genCopy, _, err := unstructured.NestedMap(genUnst.Object, "clone") + if err != nil { + return noGenResource, err } - // DATA - if gen.Data != nil { - if rdata, mode, err = handleData(rule.Name, gen, client, resource, ctx); err != nil { - glog.V(4).Info(err) - switch e := err.(type) { - case *ParseFailed, *NotFound, *ConfigNotFound: - // handled errors - return noGenResource, e - case *Violation: - // create policy violation - return noGenResource, e - default: - // errors that cant be handled - return noGenResource, e - } - } - if rdata == nil { - // existing resource contains the configuration - return newGenResource, nil - } + if genData != nil { + rdata, mode, err = manageData(genKind, genNamespace, genName, genData, client, resource) + } else { + rdata, mode, err = manageClone(genKind, genNamespace, genName, genCopy, client, resource) } - // CLONE - if gen.Clone != (kyverno.CloneFrom{}) { - if rdata, mode, err = handleClone(rule.Name, gen, client, resource, ctx); err != nil { - glog.V(4).Info(err) - switch e := err.(type) { - case *NotFound: - // handled errors - return noGenResource, e - default: - // errors that cant be handled - return noGenResource, e - } - } - if rdata == nil { - // resource already exists - return newGenResource, nil - } + if rdata == nil { + // existing resource contains the configuration + return newGenResource, nil } if processExisting { // handle existing resources @@ -218,8 +189,8 @@ func applyRule(client *dclient.Client, rule kyverno.Rule, resource unstructured. // build the resource template newResource := &unstructured.Unstructured{} newResource.SetUnstructuredContent(rdata) - newResource.SetName(gen.Name) - newResource.SetNamespace(gen.Namespace) + newResource.SetName(genName) + newResource.SetNamespace(genNamespace) // manage labels // - app.kubernetes.io/managed-by: kyverno @@ -230,58 +201,88 @@ func applyRule(client *dclient.Client, rule kyverno.Rule, resource unstructured. // Reset resource version newResource.SetResourceVersion("") // Create the resource - glog.V(4).Infof("Creating new resource %s/%s/%s", gen.Kind, gen.Namespace, gen.Name) - _, err = client.CreateResource(gen.Kind, gen.Namespace, newResource, false) + glog.V(4).Infof("Creating new resource %s/%s/%s", genKind, genNamespace, genName) + _, err = client.CreateResource(genKind, genNamespace, newResource, false) if err != nil { // Failed to create resource return noGenResource, err } - glog.V(4).Infof("Created new resource %s/%s/%s", gen.Kind, gen.Namespace, gen.Name) + glog.V(4).Infof("Created new resource %s/%s/%s", genKind, genNamespace, genName) } else if mode == Update { - glog.V(4).Infof("Updating existing resource %s/%s/%s", gen.Kind, gen.Namespace, gen.Name) + glog.V(4).Infof("Updating existing resource %s/%s/%s", genKind, genNamespace, genName) // Update the resource - _, err := client.UpdateResource(gen.Kind, gen.Namespace, newResource, false) + _, err := client.UpdateResource(genKind, genNamespace, newResource, false) if err != nil { // Failed to update resource return noGenResource, err } - glog.V(4).Infof("Updated existing resource %s/%s/%s", gen.Kind, gen.Namespace, gen.Name) + glog.V(4).Infof("Updated existing resource %s/%s/%s", genKind, genNamespace, genName) } return newGenResource, nil } -func variableSubsitutionForAttributes(gen kyverno.Generation, ctx context.EvalInterface) kyverno.Generation { - // Name - name := gen.Name - namespace := gen.Namespace - newNameVar := variables.SubstituteVariables(ctx, name) - - if newName, ok := newNameVar.(string); ok { - gen.Name = newName +func manageData(kind, namespace, name string, data map[string]interface{}, client *dclient.Client, resource unstructured.Unstructured) (map[string]interface{}, ResourceMode, error) { + // check if resource to be generated exists + obj, err := client.GetResource(kind, namespace, name) + if apierrors.IsNotFound(err) { + glog.V(4).Infof("Resource %s/%s/%s does not exists, will try to create", kind, namespace, name) + return data, Create, nil + } + if err != nil { + //something wrong while fetching resource + // client-errors + return nil, Skip, err + } + // Resource exists; verfiy the content of the resource + err = checkResource(data, obj) + if err == nil { + // Existing resource does contain the mentioned configuration in spec, skip processing the resource as it is already in expected state + return nil, Skip, nil } - newNamespaceVar := variables.SubstituteVariables(ctx, namespace) - if newNamespace, ok := newNamespaceVar.(string); ok { - gen.Namespace = newNamespace + glog.V(4).Infof("Resource %s/%s/%s exists but missing required configuration, will try to update", kind, namespace, name) + return data, Update, nil + +} + +func manageClone(kind, namespace, name string, clone map[string]interface{}, client *dclient.Client, resource unstructured.Unstructured) (map[string]interface{}, ResourceMode, error) { + // check if resource to be generated exists + _, err := client.GetResource(kind, namespace, name) + if err == nil { + // resource does exists, not need to process further as it is already in expected state + return nil, Skip, nil + } + //TODO: check this + if !apierrors.IsNotFound(err) { + //something wrong while fetching resource + return nil, Skip, err } - if gen.Clone != (kyverno.CloneFrom{}) { - // Clone - cloneName := gen.Clone.Name - cloneNamespace := gen.Clone.Namespace - - newcloneNameVar := variables.SubstituteVariables(ctx, cloneName) - if newcloneName, ok := newcloneNameVar.(string); ok { - gen.Clone.Name = newcloneName - } - newcloneNamespaceVar := variables.SubstituteVariables(ctx, cloneNamespace) - if newcloneNamespace, ok := newcloneNamespaceVar.(string); ok { - gen.Clone.Namespace = newcloneNamespace - } + newRNs, _, err := unstructured.NestedString(clone, "namespace") + if err != nil { + return nil, Skip, err } - return gen + newRName, _, err := unstructured.NestedString(clone, "name") + + if err != nil { + return nil, Skip, err + } + // Short-circuit if the resource to be generated and the clone is the same + if newRNs == namespace && newRName == name { + // attempting to clone it self, this will fail -> short-ciruit it + return nil, Skip, nil + } + + // check if the resource as reference in clone exists? + obj, err := client.GetResource(kind, newRNs, newRName) + if err != nil { + return nil, Skip, err + } + // create the resource based on the reference clone + return obj.UnstructuredContent(), Create, nil + } // ResourceMode defines the mode for generated resource @@ -292,130 +293,33 @@ const ( Skip ResourceMode = "SKIP" //Create : create a new resource Create = "CREATE" - //Update : update/override the new resource + //Update : update/overwrite the new resource Update = "UPDATE" ) -func copyInterface(original interface{}) (interface{}, error) { - tempData, err := json.Marshal(original) - if err != nil { - return nil, err - } - fmt.Println(string(tempData)) - var temp interface{} - err = json.Unmarshal(tempData, &temp) - if err != nil { - return nil, err - } - return temp, nil -} - -// manage the creation/update of resource to be generated using the spec defined in the policy -func handleData(ruleName string, generateRule kyverno.Generation, client *dclient.Client, resource unstructured.Unstructured, ctx context.EvalInterface) (map[string]interface{}, ResourceMode, error) { - //work on copy of the data - // as the type of data stored in interface is not know, - // we marshall the data and unmarshal it into a new resource to create a copy - dataCopy, err := copyInterface(generateRule.Data) - if err != nil { - glog.V(4).Infof("failed to create a copy of the interface %v", generateRule.Data) - return nil, Skip, err - } - // replace variables with the corresponding values - newData := variables.SubstituteVariables(ctx, dataCopy) - // if any variable defined in the data is not avaialbe in the context - if invalidPaths := variables.ValidateVariables(ctx, newData); len(invalidPaths) != 0 { - return nil, Skip, NewViolation(ruleName, fmt.Errorf("path not present in generate data: %s", invalidPaths)) - } - - // check if resource exists - obj, err := client.GetResource(generateRule.Kind, generateRule.Namespace, generateRule.Name) - if apierrors.IsNotFound(err) { - // Resource does not exist - // Processing the request first time - rdata, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&newData) - if err != nil { - return nil, Skip, NewParseFailed(newData, err) - } - glog.V(4).Infof("Resource %s/%s/%s does not exists, will try to create", generateRule.Kind, generateRule.Namespace, generateRule.Name) - return rdata, Create, nil - } - if err != nil { - //something wrong while fetching resource - return nil, Skip, err - } - // Resource exists; verfiy the content of the resource - ok, err := checkResource(ctx, newData, obj) - if err != nil { - // error while evaluating if the existing resource contains the required information - return nil, Skip, err - } - - if !ok { - // existing resource does not contain the configuration mentioned in spec, will try to update - rdata, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&newData) - if err != nil { - return nil, Skip, NewParseFailed(newData, err) - } - - glog.V(4).Infof("Resource %s/%s/%s exists but missing required configuration, will try to update", generateRule.Kind, generateRule.Namespace, generateRule.Name) - return rdata, Update, nil - } - // Existing resource does contain the mentioned configuration in spec, skip processing the resource as it is already in expected state - return nil, Skip, nil -} - -// manage the creation/update based on the reference clone resource -func handleClone(ruleName string, generateRule kyverno.Generation, client *dclient.Client, resource unstructured.Unstructured, ctx context.EvalInterface) (map[string]interface{}, ResourceMode, error) { - // if any variable defined in the data is not avaialbe in the context - if invalidPaths := variables.ValidateVariables(ctx, generateRule.Clone); len(invalidPaths) != 0 { - return nil, Skip, NewViolation(ruleName, fmt.Errorf("path not present in generate clone: %s", invalidPaths)) - } - - // check if resource to be generated exists - _, err := client.GetResource(generateRule.Kind, generateRule.Namespace, generateRule.Name) - if err == nil { - // resource does exists, not need to process further as it is already in expected state - return nil, Skip, nil - } - if !apierrors.IsNotFound(err) { - //something wrong while fetching resource - return nil, Skip, err - } - - // get clone resource reference in the rule - obj, err := client.GetResource(generateRule.Kind, generateRule.Clone.Namespace, generateRule.Clone.Name) - if apierrors.IsNotFound(err) { - // reference resource does not exist, cant generate the resources - return nil, Skip, NewNotFound(generateRule.Kind, generateRule.Clone.Namespace, generateRule.Clone.Name) - } - if err != nil { - //something wrong while fetching resource - return nil, Skip, err - } - // create the resource based on the reference clone - return obj.UnstructuredContent(), Create, nil -} - -func checkResource(ctx context.EvalInterface, newResourceSpec interface{}, resource *unstructured.Unstructured) (bool, error) { +func checkResource(newResourceSpec interface{}, resource *unstructured.Unstructured) error { // check if the resource spec if a subset of the resource - path, err := validate.ValidateResourceWithPattern(ctx, resource.Object, newResourceSpec) - if !reflect.DeepEqual(err, validate.ValidationError{}) { - glog.V(4).Infof("config not a subset of resource. failed at path %s: %v", path, err) - return false, errors.New(err.ErrorMsg) + if path, err := validate.ValidateResourceWithPattern1(resource.Object, newResourceSpec); err != nil { + glog.V(4).Info("Failed to match the resource at path %s: err", path, err) + return err } - return true, nil + return nil } -func generatePV(gr kyverno.GenerateRequest, resource unstructured.Unstructured, err *Violation) policyviolation.Info { - - info := policyviolation.Info{ - PolicyName: gr.Spec.Policy, - Resource: resource, - Rules: []kyverno.ViolatedRule{{ - Name: err.rule, - Type: "Generation", - Message: err.Error(), - }}, +func getUnstrRule(rule *kyverno.Generation) (*unstructured.Unstructured, error) { + ruleData, err := json.Marshal(rule) + if err != nil { + return nil, err } - return info + return ConvertToUnstructured(ruleData) +} + +//ConvertToUnstructured converts the resource to unstructured format +func ConvertToUnstructured(data []byte) (*unstructured.Unstructured, error) { + resource := &unstructured.Unstructured{} + err := resource.UnmarshalJSON(data) + if err != nil { + return nil, err + } + return resource, nil } diff --git a/pkg/generate/report.go b/pkg/generate/report.go index b2342e90cb..eaa5939e41 100644 --- a/pkg/generate/report.go +++ b/pkg/generate/report.go @@ -5,9 +5,7 @@ import ( "github.com/golang/glog" kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1" - "github.com/nirmata/kyverno/pkg/engine/response" "github.com/nirmata/kyverno/pkg/event" - "github.com/nirmata/kyverno/pkg/policyviolation" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) @@ -20,45 +18,9 @@ func reportEvents(err error, eventGen event.Interface, gr kyverno.GenerateReques eventGen.Add(events...) return } - switch e := err.(type) { - case *Violation: - // - resource -> rule failed and created PV - // - policy -> failed to apply of resource and created PV - glog.V(4).Infof("reporing events for %v", e) - events := failedEventsPV(err, gr, resource) - eventGen.Add(events...) - default: - // - resource -> rule failed - // - policy -> failed tp apply on resource - glog.V(4).Infof("reporing events for %v", e) - events := failedEvents(err, gr, resource) - eventGen.Add(events...) - } -} - -func failedEventsPV(err error, gr kyverno.GenerateRequest, resource unstructured.Unstructured) []event.Info { - var events []event.Info - // Cluster Policy - pe := event.Info{} - pe.Kind = "ClusterPolicy" - // cluserwide-resource - pe.Name = gr.Spec.Policy - pe.Reason = event.PolicyViolation.String() - pe.Source = event.GeneratePolicyController - pe.Message = fmt.Sprintf("policy failed to apply on resource %s/%s/%s creating violation: %v", resource.GetKind(), resource.GetNamespace(), resource.GetName(), err) - events = append(events, pe) - - // Resource - re := event.Info{} - re.Kind = resource.GetKind() - re.Namespace = resource.GetNamespace() - re.Name = resource.GetName() - re.Reason = event.PolicyViolation.String() - re.Source = event.GeneratePolicyController - re.Message = fmt.Sprintf("policy %s failed to apply created violation: %v", gr.Spec.Policy, err) - events = append(events, re) - - return events + glog.V(4).Infof("reporing events for %v", err) + events := failedEvents(err, gr, resource) + eventGen.Add(events...) } func failedEvents(err error, gr kyverno.GenerateRequest, resource unstructured.Unstructured) []event.Info { @@ -110,13 +72,3 @@ func successEvents(gr kyverno.GenerateRequest, resource unstructured.Unstructure return events } - -// buildPathNotPresentPV build violation info when referenced path not found -func buildPathNotPresentPV(er response.EngineResponse) []policyviolation.Info { - for _, rr := range er.PolicyResponse.Rules { - if rr.PathNotPresent { - return policyviolation.GeneratePVsFromEngineResponse([]response.EngineResponse{er}) - } - } - return nil -} From 5cee543755c457e941291b566507d52789904531 Mon Sep 17 00:00:00 2001 From: shivkumar dudhani Date: Fri, 14 Feb 2020 11:59:28 -0800 Subject: [PATCH 06/12] refactor variable substitution --- pkg/engine/context/context.go | 12 +- pkg/engine/context/evaluate.go | 14 ++ pkg/engine/generation.go | 11 +- pkg/engine/mutate/overlay.go | 30 +-- pkg/engine/mutation.go | 52 +++--- pkg/engine/mutation_test.go | 52 +----- pkg/engine/policy/background.go | 43 +++-- pkg/engine/policy/validate.go | 2 +- pkg/engine/policy/validate_test.go | 27 +-- pkg/engine/utils.go | 43 +---- pkg/engine/utils_test.go | 117 ------------ pkg/engine/validate/validate.go | 44 +---- pkg/engine/validation.go | 120 +++++------- pkg/engine/validation_test.go | 23 +-- pkg/engine/variables/evaluate.go | 2 +- pkg/engine/variables/operator/equal.go | 31 +++- pkg/engine/variables/operator/notequal.go | 30 ++- pkg/engine/variables/operator/operator.go | 2 +- pkg/engine/variables/validatevariables.go | 153 +++++++-------- .../variables/validatevariables_test.go | 175 ------------------ pkg/engine/variables/variables.go | 146 --------------- pkg/engine/variables/variables_check.go | 142 +++++++------- pkg/engine/variables/variables_test.go | 136 +++++++++----- pkg/engine/variables/vars.go | 28 ++- pkg/engine/variables/vars_test.go | 4 +- pkg/generate/generate.go | 15 +- 26 files changed, 456 insertions(+), 998 deletions(-) delete mode 100644 pkg/engine/variables/validatevariables_test.go delete mode 100644 pkg/engine/variables/variables.go diff --git a/pkg/engine/context/context.go b/pkg/engine/context/context.go index 09fcc30d86..05805a0a91 100644 --- a/pkg/engine/context/context.go +++ b/pkg/engine/context/context.go @@ -30,16 +30,18 @@ type EvalInterface interface { //Context stores the data resources as JSON type Context struct { - mu sync.RWMutex - // data map[string]interface{} - jsonRaw []byte + mu sync.RWMutex + jsonRaw []byte + whiteListVars []string } //NewContext returns a new context -func NewContext() *Context { +// pass the list of variables to be white-listed +func NewContext(whiteListVars ...string) *Context { ctx := Context{ // data: map[string]interface{}{}, - jsonRaw: []byte(`{}`), // empty json struct + jsonRaw: []byte(`{}`), // empty json struct + whiteListVars: whiteListVars, } return &ctx } diff --git a/pkg/engine/context/evaluate.go b/pkg/engine/context/evaluate.go index 70df8ba9b1..78f9643497 100644 --- a/pkg/engine/context/evaluate.go +++ b/pkg/engine/context/evaluate.go @@ -11,6 +11,11 @@ import ( //Query the JSON context with JMESPATH search path func (ctx *Context) Query(query string) (interface{}, error) { var emptyResult interface{} + // check for white-listed variables + if ctx.isWhiteListed(query) { + return emptyResult, fmt.Errorf("variable %s cannot be used", query) + } + // compile the query queryPath, err := jmespath.Compile(query) if err != nil { @@ -34,3 +39,12 @@ func (ctx *Context) Query(query string) (interface{}, error) { } return result, nil } + +func (ctx *Context) isWhiteListed(variable string) bool { + for _, wVar := range ctx.whiteListVars { + if wVar == variable { + return true + } + } + return false +} diff --git a/pkg/engine/generation.go b/pkg/engine/generation.go index 649db6d86e..5419c54e29 100644 --- a/pkg/engine/generation.go +++ b/pkg/engine/generation.go @@ -1,14 +1,11 @@ package engine import ( - "fmt" - "github.com/golang/glog" kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1" "github.com/nirmata/kyverno/pkg/engine/context" "github.com/nirmata/kyverno/pkg/engine/rbac" "github.com/nirmata/kyverno/pkg/engine/response" - "github.com/nirmata/kyverno/pkg/engine/utils" "github.com/nirmata/kyverno/pkg/engine/variables" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) @@ -37,6 +34,7 @@ func filterRule(rule kyverno.Rule, resource unstructured.Unstructured, admission } // operate on the copy of the conditions, as we perform variable substitution copyConditions := copyConditions(rule.Conditions) + // evaluate pre-conditions if !variables.EvaluateConditions(ctx, copyConditions) { glog.V(4).Infof("resource %s/%s does not satisfy the conditions for the rule ", resource.GetNamespace(), resource.GetName()) @@ -62,13 +60,6 @@ func filterRules(policy kyverno.ClusterPolicy, resource unstructured.Unstructure } for _, rule := range policy.Spec.Rules { - if paths := validateGeneralRuleInfoVariables(ctx, rule); len(paths) != 0 { - glog.Infof("referenced path not present in generate rule %s, resource %s/%s/%s, path: %s", rule.Name, resource.GetKind(), resource.GetNamespace(), resource.GetName(), paths) - resp.PolicyResponse.Rules = append(resp.PolicyResponse.Rules, - newPathNotPresentRuleResponse(rule.Name, utils.Mutation.String(), fmt.Sprintf("path not present: %s", paths))) - continue - } - if ruleResp := filterRule(rule, resource, admissionInfo, ctx); ruleResp != nil { resp.PolicyResponse.Rules = append(resp.PolicyResponse.Rules, *ruleResp) } diff --git a/pkg/engine/mutate/overlay.go b/pkg/engine/mutate/overlay.go index 0ffa924545..21e5e7b77e 100644 --- a/pkg/engine/mutate/overlay.go +++ b/pkg/engine/mutate/overlay.go @@ -14,40 +14,22 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" jsonpatch "github.com/evanphx/json-patch" - kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1" "github.com/nirmata/kyverno/pkg/engine/anchor" - "github.com/nirmata/kyverno/pkg/engine/context" "github.com/nirmata/kyverno/pkg/engine/response" "github.com/nirmata/kyverno/pkg/engine/utils" - "github.com/nirmata/kyverno/pkg/engine/variables" ) // ProcessOverlay processes mutation overlay on the resource -func ProcessOverlay(ctx context.EvalInterface, rule kyverno.Rule, resource unstructured.Unstructured) (resp response.RuleResponse, patchedResource unstructured.Unstructured) { +func ProcessOverlay(ruleName string, overlay interface{}, resource unstructured.Unstructured) (resp response.RuleResponse, patchedResource unstructured.Unstructured) { startTime := time.Now() - glog.V(4).Infof("started applying overlay rule %q (%v)", rule.Name, startTime) - resp.Name = rule.Name + glog.V(4).Infof("started applying overlay rule %q (%v)", ruleName, startTime) + resp.Name = ruleName resp.Type = utils.Mutation.String() defer func() { resp.RuleStats.ProcessingTime = time.Since(startTime) glog.V(4).Infof("finished applying overlay rule %q (%v)", resp.Name, resp.RuleStats.ProcessingTime) }() - // if referenced path not present, we skip processing the rule and report violation - if invalidPaths := variables.ValidateVariables(ctx, rule.Mutation.Overlay); len(invalidPaths) != 0 { - resp.Success = true - resp.PathNotPresent = true - resp.Message = fmt.Sprintf("referenced path not present: %s", invalidPaths) - glog.V(3).Infof("Skip applying rule '%s' on resource '%s/%s/%s': %s", rule.Name, resource.GetKind(), resource.GetNamespace(), resource.GetName(), resp.Message) - return resp, resource - } - - // substitute variables - // first pass we substitute all the JMESPATH substitution for the variable - // variable: {{}} - // if a JMESPATH fails, we dont return error but variable is substitured with nil and error log - overlay := variables.SubstituteVariables(ctx, rule.Mutation.Overlay) - patches, overlayerr := processOverlayPatches(resource.UnstructuredContent(), overlay) // resource does not satisfy the overlay pattern, we don't apply this rule if !reflect.DeepEqual(overlayerr, overlayError{}) { @@ -55,19 +37,19 @@ func ProcessOverlay(ctx context.EvalInterface, rule kyverno.Rule, resource unstr // condition key is not present in the resource, don't apply this rule // consider as success case conditionNotPresent: - glog.V(3).Infof("Skip applying rule '%s' on resource '%s/%s/%s': %s", rule.Name, resource.GetKind(), resource.GetNamespace(), resource.GetName(), overlayerr.ErrorMsg()) + glog.V(3).Infof("Skip applying rule '%s' on resource '%s/%s/%s': %s", ruleName, resource.GetKind(), resource.GetNamespace(), resource.GetName(), overlayerr.ErrorMsg()) resp.Success = true return resp, resource // conditions are not met, don't apply this rule case conditionFailure: - glog.V(3).Infof("Skip applying rule '%s' on resource '%s/%s/%s': %s", rule.Name, resource.GetKind(), resource.GetNamespace(), resource.GetName(), overlayerr.ErrorMsg()) + glog.V(3).Infof("Skip applying rule '%s' on resource '%s/%s/%s': %s", ruleName, resource.GetKind(), resource.GetNamespace(), resource.GetName(), overlayerr.ErrorMsg()) //TODO: send zero response and not consider this as applied? resp.Success = true resp.Message = overlayerr.ErrorMsg() return resp, resource // rule application failed case overlayFailure: - glog.Errorf("Resource %s/%s/%s: failed to process overlay: %v in the rule %s", resource.GetKind(), resource.GetNamespace(), resource.GetName(), overlayerr.ErrorMsg(), rule.Name) + glog.Errorf("Resource %s/%s/%s: failed to process overlay: %v in the rule %s", resource.GetKind(), resource.GetNamespace(), resource.GetName(), overlayerr.ErrorMsg(), ruleName) resp.Success = false resp.Message = fmt.Sprintf("failed to process overlay: %v", overlayerr.ErrorMsg()) return resp, resource diff --git a/pkg/engine/mutation.go b/pkg/engine/mutation.go index 0c1eb057ca..8fdf1cc33b 100644 --- a/pkg/engine/mutation.go +++ b/pkg/engine/mutation.go @@ -1,7 +1,6 @@ package engine import ( - "fmt" "reflect" "strings" "time" @@ -11,7 +10,6 @@ import ( "github.com/nirmata/kyverno/pkg/engine/mutate" "github.com/nirmata/kyverno/pkg/engine/rbac" "github.com/nirmata/kyverno/pkg/engine/response" - "github.com/nirmata/kyverno/pkg/engine/utils" "github.com/nirmata/kyverno/pkg/engine/variables" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) @@ -36,26 +34,13 @@ func Mutate(policyContext PolicyContext) (resp response.EngineResponse) { glog.V(4).Infof("started applying mutation rules of policy %q (%v)", policy.Name, startTime) defer endMutateResultResponse(&resp, startTime) - incrementAppliedRuleCount := func() { - // rules applied successfully count - resp.PolicyResponse.RulesAppliedCount++ - } - patchedResource := policyContext.NewResource - for _, rule := range policy.Spec.Rules { + var ruleResponse response.RuleResponse //TODO: to be checked before calling the resources as well if !rule.HasMutate() && !strings.Contains(PodControllers, resource.GetKind()) { continue } - - if paths := validateGeneralRuleInfoVariables(ctx, rule); len(paths) != 0 { - glog.Infof("referenced path not present in rule %s, resource %s/%s/%s, path: %s", rule.Name, resource.GetKind(), resource.GetNamespace(), resource.GetName(), paths) - resp.PolicyResponse.Rules = append(resp.PolicyResponse.Rules, - newPathNotPresentRuleResponse(rule.Name, utils.Mutation.String(), fmt.Sprintf("path not present in rule info: %s", paths))) - continue - } - startTime := time.Now() if !rbac.MatchAdmissionInfo(rule, policyContext.AdmissionInfo) { glog.V(3).Infof("rule '%s' cannot be applied on %s/%s/%s, admission permission: %v", @@ -76,34 +61,38 @@ func Mutate(policyContext PolicyContext) (resp response.EngineResponse) { // operate on the copy of the conditions, as we perform variable substitution copyConditions := copyConditions(rule.Conditions) // evaluate pre-conditions + // - handle variable subsitutions if !variables.EvaluateConditions(ctx, copyConditions) { glog.V(4).Infof("resource %s/%s does not satisfy the conditions for the rule ", resource.GetNamespace(), resource.GetName()) continue } + mutation := rule.Mutation.DeepCopy() // Process Overlay - if rule.Mutation.Overlay != nil { - var ruleResponse response.RuleResponse - ruleResponse, patchedResource = mutate.ProcessOverlay(ctx, rule, patchedResource) - if ruleResponse.Success { - // - variable substitution path is not present - if ruleResponse.PathNotPresent { - glog.V(4).Infof(ruleResponse.Message) - resp.PolicyResponse.Rules = append(resp.PolicyResponse.Rules, ruleResponse) - continue - } + if mutation.Overlay != nil { + overlay := mutation.Overlay + // subsiitue the variables + var err error + if overlay, err = variables.SubstituteVars(ctx, overlay); err != nil { + // variable subsitution failed + ruleResponse.Success = false + ruleResponse.Message = err.Error() + resp.PolicyResponse.Rules = append(resp.PolicyResponse.Rules, ruleResponse) + continue + } + ruleResponse, patchedResource = mutate.ProcessOverlay(rule.Name, overlay, patchedResource) + if ruleResponse.Success { // - overlay pattern does not match the resource conditions if ruleResponse.Patches == nil { glog.V(4).Infof(ruleResponse.Message) continue } - glog.Infof("Mutate overlay in rule '%s' successfully applied on %s/%s/%s", rule.Name, resource.GetKind(), resource.GetNamespace(), resource.GetName()) } resp.PolicyResponse.Rules = append(resp.PolicyResponse.Rules, ruleResponse) - incrementAppliedRuleCount() + incrementAppliedRuleCount(&resp) } // Process Patches @@ -112,7 +101,7 @@ func Mutate(policyContext PolicyContext) (resp response.EngineResponse) { ruleResponse, patchedResource = mutate.ProcessPatches(rule, patchedResource) glog.Infof("Mutate patches in rule '%s' successfully applied on %s/%s/%s", rule.Name, resource.GetKind(), resource.GetNamespace(), resource.GetName()) resp.PolicyResponse.Rules = append(resp.PolicyResponse.Rules, ruleResponse) - incrementAppliedRuleCount() + incrementAppliedRuleCount(&resp) } // insert annotation to podtemplate if resource is pod controller @@ -123,7 +112,7 @@ func Mutate(policyContext PolicyContext) (resp response.EngineResponse) { if strings.Contains(PodControllers, resource.GetKind()) { var ruleResponse response.RuleResponse - ruleResponse, patchedResource = mutate.ProcessOverlay(ctx, podTemplateRule, patchedResource) + ruleResponse, patchedResource = mutate.ProcessOverlay(rule.Name, podTemplateRule, patchedResource) if !ruleResponse.Success { glog.Errorf("Failed to insert annotation to podTemplate of %s/%s/%s: %s", resource.GetKind(), resource.GetNamespace(), resource.GetName(), ruleResponse.Message) continue @@ -139,6 +128,9 @@ func Mutate(policyContext PolicyContext) (resp response.EngineResponse) { resp.PatchedResource = patchedResource return resp } +func incrementAppliedRuleCount(resp *response.EngineResponse) { + resp.PolicyResponse.RulesAppliedCount++ +} func startMutateResultResponse(resp *response.EngineResponse, policy kyverno.ClusterPolicy, resource unstructured.Unstructured) { // set policy information diff --git a/pkg/engine/mutation_test.go b/pkg/engine/mutation_test.go index 30d4ee977f..1e7251cd16 100644 --- a/pkg/engine/mutation_test.go +++ b/pkg/engine/mutation_test.go @@ -3,7 +3,6 @@ package engine import ( "encoding/json" "reflect" - "strings" "testing" kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1" @@ -152,52 +151,7 @@ func Test_variableSubstitutionPathNotExist(t *testing.T) { Context: ctx, NewResource: *resourceUnstructured} er := Mutate(policyContext) - assert.Assert(t, er.PolicyResponse.Rules[0].PathNotPresent, true) -} - -func Test_variableSubstitutionPathNotExist_InRuleInfo(t *testing.T) { - resourceRaw := []byte(`{ - "apiVersion": "v1", - "kind": "Deployment", - "metadata": { - "name": "check-root-user" - } - }`) - - policyraw := []byte(`{ - "apiVersion": "kyverno.io/v1", - "kind": "ClusterPolicy", - "metadata": { - "name": "test-validate-variables" - }, - "spec": { - "rules": [ - { - "name": "test-match", - "match": { - "resources": { - "kinds": [ - "{{request.kind}}" - ] - } - } - } - ] - } - }`) - - var policy kyverno.ClusterPolicy - assert.NilError(t, json.Unmarshal(policyraw, &policy)) - resourceUnstructured, err := utils.ConvertToUnstructured(resourceRaw) - assert.NilError(t, err) - - ctx := context.NewContext() - ctx.AddResource(resourceRaw) - - policyContext := PolicyContext{ - Policy: policy, - Context: ctx, - NewResource: *resourceUnstructured} - er := Mutate(policyContext) - assert.Assert(t, strings.Contains(er.PolicyResponse.Rules[0].Message, "path not present in rule info")) + expectedErrorStr := "variable(s) not found or has nil values: [/spec/name/{{request.object.metadata.name1}}]" + t.Log(er.PolicyResponse.Rules[0].Message) + assert.Equal(t, er.PolicyResponse.Rules[0].Message, expectedErrorStr) } diff --git a/pkg/engine/policy/background.go b/pkg/engine/policy/background.go index e027d7c6c7..c66b920c37 100644 --- a/pkg/engine/policy/background.go +++ b/pkg/engine/policy/background.go @@ -4,19 +4,21 @@ import ( "fmt" kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1" + "github.com/nirmata/kyverno/pkg/engine/context" "github.com/nirmata/kyverno/pkg/engine/variables" ) //ContainsUserInfo returns error is userInfo is defined func ContainsUserInfo(policy kyverno.ClusterPolicy) error { + var err error // iterate of the policy rules to identify if userInfo is used for idx, rule := range policy.Spec.Rules { - if err := userInfoDefined(rule.MatchResources.UserInfo); err != nil { - return fmt.Errorf("path: spec/rules[%d]/match/%s", idx, err) + if path := userInfoDefined(rule.MatchResources.UserInfo); path != "" { + return fmt.Errorf("userInfo variable used at path: spec/rules[%d]/match/%s", idx, path) } - if err := userInfoDefined(rule.ExcludeResources.UserInfo); err != nil { - return fmt.Errorf("path: spec/rules[%d]/exclude/%s", idx, err) + if path := userInfoDefined(rule.ExcludeResources.UserInfo); path != "" { + return fmt.Errorf("userInfo variable used at path: spec/rules[%d]/exclude/%s", idx, path) } // variable defined with user information @@ -29,40 +31,43 @@ func ContainsUserInfo(policy kyverno.ClusterPolicy) error { // - request.userInfo* // - serviceAccountName // - serviceAccountNamespace + filterVars := []string{"request.userInfo*", "serviceAccountName", "serviceAccountNamespace"} + ctx := context.NewContext(filterVars...) for condIdx, condition := range rule.Conditions { - if err := variables.CheckVariables(condition.Key, filterVars, "/"); err != nil { - return fmt.Errorf("path: spec/rules[%d]/condition[%d]/key%s", idx, condIdx, err) + if condition.Key, err = variables.SubstituteVars(ctx, condition.Key); err != nil { + return fmt.Errorf("userInfo variable used at spec/rules[%d]/condition[%d]/key", idx, condIdx) } - if err := variables.CheckVariables(condition.Value, filterVars, "/"); err != nil { - return fmt.Errorf("path: spec/rules[%d]/condition[%d]/value%s", idx, condIdx, err) + + if condition.Value, err = variables.SubstituteVars(ctx, condition.Value); err != nil { + return fmt.Errorf("userInfo variable used at spec/rules[%d]/condition[%d]/value", idx, condIdx) } } - if err := variables.CheckVariables(rule.Mutation.Overlay, filterVars, "/"); err != nil { - return fmt.Errorf("path: spec/rules[%d]/mutate/overlay%s", idx, err) + if rule.Mutation.Overlay, err = variables.SubstituteVars(ctx, rule.Mutation.Overlay); err != nil { + return fmt.Errorf("userInfo variable used at spec/rules[%d]/mutate/overlay", idx) } - if err := variables.CheckVariables(rule.Validation.Pattern, filterVars, "/"); err != nil { - return fmt.Errorf("path: spec/rules[%d]/validate/pattern%s", idx, err) + if rule.Validation.Pattern, err = variables.SubstituteVars(ctx, rule.Validation.Pattern); err != nil { + return fmt.Errorf("userInfo variable used at spec/rules[%d]/validate/pattern", idx) } for idx2, pattern := range rule.Validation.AnyPattern { - if err := variables.CheckVariables(pattern, filterVars, "/"); err != nil { - return fmt.Errorf("path: spec/rules[%d]/validate/anyPattern[%d]%s", idx, idx2, err) + if pattern, err = variables.SubstituteVars(ctx, pattern); err != nil { + return fmt.Errorf("userInfo variable used at spec/rules[%d]/validate/anyPattern[%d]", idx, idx2) } } } return nil } -func userInfoDefined(ui kyverno.UserInfo) error { +func userInfoDefined(ui kyverno.UserInfo) string { if len(ui.Roles) > 0 { - return fmt.Errorf("roles") + return "roles" } if len(ui.ClusterRoles) > 0 { - return fmt.Errorf("clusterRoles") + return "clusterRoles" } if len(ui.Subjects) > 0 { - return fmt.Errorf("subjects") + return "subjects" } - return nil + return "" } diff --git a/pkg/engine/policy/validate.go b/pkg/engine/policy/validate.go index 24eb4e17b2..e6e6541a14 100644 --- a/pkg/engine/policy/validate.go +++ b/pkg/engine/policy/validate.go @@ -30,7 +30,7 @@ func Validate(p kyverno.ClusterPolicy) error { // policy.spec.background -> "true" // - cannot use variables with request.userInfo // - cannot define userInfo(roles, cluserRoles, subjects) for filtering (match & exclude) - return fmt.Errorf("userInfo not allowed in background policy mode. Failure path %s", err) + return fmt.Errorf("userInfo not allowed in background policy mode. %v", err) } } diff --git a/pkg/engine/policy/validate_test.go b/pkg/engine/policy/validate_test.go index e3bbbd718e..e9e08ab240 100644 --- a/pkg/engine/policy/validate_test.go +++ b/pkg/engine/policy/validate_test.go @@ -1190,10 +1190,7 @@ func Test_BackGroundUserInfo_match_roles(t *testing.T) { assert.NilError(t, err) err = ContainsUserInfo(*policy) - - if err.Error() != "path: spec/rules[0]/match/roles" { - t.Error("Incorrect Path") - } + assert.Equal(t, err.Error(), "userInfo variable used at path: spec/rules[0]/match/roles") } func Test_BackGroundUserInfo_match_clusterRoles(t *testing.T) { @@ -1226,10 +1223,7 @@ func Test_BackGroundUserInfo_match_clusterRoles(t *testing.T) { err = ContainsUserInfo(*policy) - if err.Error() != "path: spec/rules[0]/match/clusterRoles" { - t.Log(err) - t.Error("Incorrect Path") - } + assert.Equal(t, err.Error(), "userInfo variable used at path: spec/rules[0]/match/clusterRoles") } func Test_BackGroundUserInfo_match_subjects(t *testing.T) { @@ -1265,10 +1259,7 @@ func Test_BackGroundUserInfo_match_subjects(t *testing.T) { err = ContainsUserInfo(*policy) - if err.Error() != "path: spec/rules[0]/match/subjects" { - t.Log(err) - t.Error("Incorrect Path") - } + assert.Equal(t, err.Error(), "userInfo variable used at path: spec/rules[0]/match/subjects") } func Test_BackGroundUserInfo_mutate_overlay1(t *testing.T) { @@ -1300,7 +1291,7 @@ func Test_BackGroundUserInfo_mutate_overlay1(t *testing.T) { err = ContainsUserInfo(*policy) - if err.Error() != "path: spec/rules[0]/mutate/overlay/var1/{{request.userInfo}}" { + if err.Error() != "userInfo variable used at spec/rules[0]/mutate/overlay" { t.Log(err) t.Error("Incorrect Path") } @@ -1335,7 +1326,7 @@ func Test_BackGroundUserInfo_mutate_overlay2(t *testing.T) { err = ContainsUserInfo(*policy) - if err.Error() != "path: spec/rules[0]/mutate/overlay/var1/{{request.userInfo.userName}}" { + if err.Error() != "userInfo variable used at spec/rules[0]/mutate/overlay" { t.Log(err) t.Error("Incorrect Path") } @@ -1370,7 +1361,7 @@ func Test_BackGroundUserInfo_validate_pattern(t *testing.T) { err = ContainsUserInfo(*policy) - if err.Error() != "path: spec/rules[0]/validate/pattern/var1/{{request.userInfo}}" { + if err.Error() != "userInfo variable used at spec/rules[0]/validate/pattern" { t.Log(err) t.Error("Incorrect Path") } @@ -1409,7 +1400,7 @@ func Test_BackGroundUserInfo_validate_anyPattern(t *testing.T) { err = ContainsUserInfo(*policy) - if err.Error() != "path: spec/rules[0]/validate/anyPattern[1]/var1/{{request.userInfo}}" { + if err.Error() != "userInfo variable used at spec/rules[0]/validate/anyPattern[1]" { t.Log(err) t.Error("Incorrect Path") } @@ -1448,7 +1439,7 @@ func Test_BackGroundUserInfo_validate_anyPattern_multiple_var(t *testing.T) { err = ContainsUserInfo(*policy) - if err.Error() != "path: spec/rules[0]/validate/anyPattern[1]/var1/{{request.userInfo}}-{{temp}}" { + if err.Error() != "userInfo variable used at spec/rules[0]/validate/anyPattern[1]" { t.Log(err) t.Error("Incorrect Path") } @@ -1487,7 +1478,7 @@ func Test_BackGroundUserInfo_validate_anyPattern_serviceAccount(t *testing.T) { err = ContainsUserInfo(*policy) - if err.Error() != "path: spec/rules[0]/validate/anyPattern[1]/var1/{{serviceAccountName}}" { + if err.Error() != "userInfo variable used at spec/rules[0]/validate/anyPattern[1]" { t.Log(err) t.Error("Incorrect Path") } diff --git a/pkg/engine/utils.go b/pkg/engine/utils.go index 6a6a162af8..963a51012c 100644 --- a/pkg/engine/utils.go +++ b/pkg/engine/utils.go @@ -9,9 +9,6 @@ import ( "github.com/minio/minio/pkg/wildcard" kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1" - "github.com/nirmata/kyverno/pkg/engine/context" - "github.com/nirmata/kyverno/pkg/engine/response" - "github.com/nirmata/kyverno/pkg/engine/variables" "github.com/nirmata/kyverno/pkg/utils" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -224,48 +221,10 @@ func findKind(kinds []string, kindGVK string) bool { return false } -// validateGeneralRuleInfoVariables validate variable subtition defined in -// - MatchResources -// - ExcludeResources -// - Conditions -func validateGeneralRuleInfoVariables(ctx context.EvalInterface, rule kyverno.Rule) string { - var tempRule kyverno.Rule - var tempRulePattern interface{} - - tempRule.MatchResources = rule.MatchResources - tempRule.ExcludeResources = rule.ExcludeResources - tempRule.Conditions = rule.Conditions - - raw, err := json.Marshal(tempRule) - if err != nil { - glog.Infof("failed to serilize rule info while validating variable substitution: %v", err) - return "" - } - - if err := json.Unmarshal(raw, &tempRulePattern); err != nil { - glog.Infof("failed to serilize rule info while validating variable substitution: %v", err) - return "" - } - - return variables.ValidateVariables(ctx, tempRulePattern) -} - -func newPathNotPresentRuleResponse(rname, rtype, msg string) response.RuleResponse { - return response.RuleResponse{ - Name: rname, - Type: rtype, - Message: msg, - Success: true, - PathNotPresent: true, - } -} - func copyConditions(original []kyverno.Condition) []kyverno.Condition { var copy []kyverno.Condition for _, condition := range original { - copyCondition := kyverno.Condition{} - condition.DeepCopyInto(©Condition) - copy = append(copy, copyCondition) + copy = append(copy, *condition.DeepCopy()) } return copy } diff --git a/pkg/engine/utils_test.go b/pkg/engine/utils_test.go index c594904386..ced55be859 100644 --- a/pkg/engine/utils_test.go +++ b/pkg/engine/utils_test.go @@ -1,15 +1,11 @@ package engine import ( - "encoding/json" - "fmt" "testing" kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1" - context "github.com/nirmata/kyverno/pkg/engine/context" "github.com/nirmata/kyverno/pkg/engine/utils" "gotest.tools/assert" - authenticationv1 "k8s.io/api/authentication/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -396,116 +392,3 @@ func TestResourceDescriptionExclude_Label_Expression_Match(t *testing.T) { assert.Assert(t, !MatchesResourceDescription(*resource, rule)) } - -func Test_validateGeneralRuleInfoVariables(t *testing.T) { - rawResource := []byte(` - { - "apiVersion": "v1", - "kind": "Pod", - "metadata": { - "name": "image-with-hostpath", - "labels": { - "app.type": "prod", - "namespace": "my-namespace" - } - }, - "spec": { - "containers": [ - { - "name": "image-with-hostpath", - "image": "docker.io/nautiker/curl", - "volumeMounts": [ - { - "name": "var-lib-etcd", - "mountPath": "/var/lib" - } - ] - } - ], - "volumes": [ - { - "name": "var-lib-etcd", - "emptyDir": {} - } - ] - } - } - `) - - policyRaw := []byte(`{ - "apiVersion": "kyverno.io/v1", - "kind": "ClusterPolicy", - "metadata": { - "name": "test-validate-variables" - }, - "spec": { - "rules": [ - { - "name": "test-match", - "match": { - "Subjects": [ - { - "kind": "User", - "name": "{{request.userInfo.username1}}}" - } - ], - "resources": { - "kind": "{{request.object.kind}}" - } - } - }, - { - "name": "test-exclude", - "match": { - "resources": { - "namespaces": [ - "{{request.object.namespace}}" - ] - } - } - }, - { - "name": "test-condition", - "preconditions": [ - { - "key": "{{serviceAccountName}}", - "operator": "NotEqual", - "value": "testuser" - } - ] - } - ] - } - }`) - - userReqInfo := kyverno.RequestInfo{ - AdmissionUserInfo: authenticationv1.UserInfo{ - Username: "user1", - }, - } - - var policy kyverno.ClusterPolicy - assert.NilError(t, json.Unmarshal(policyRaw, &policy)) - - ctx := context.NewContext() - var err error - err = ctx.AddResource(rawResource) - if err != nil { - t.Error(err) - } - err = ctx.AddUserInfo(userReqInfo) - if err != nil { - t.Error(err) - } - err = ctx.AddSA("system:serviceaccount:test:testuser") - if err != nil { - t.Error(err) - } - - expectPaths := []string{"request.userInfo.username1", "request.object.namespace", ""} - - for i, rule := range policy.Spec.Rules { - invalidPaths := validateGeneralRuleInfoVariables(ctx, rule) - assert.Assert(t, invalidPaths == expectPaths[i], fmt.Sprintf("result not match, got invalidPaths %s", invalidPaths)) - } -} diff --git a/pkg/engine/validate/validate.go b/pkg/engine/validate/validate.go index eafdacb034..84bdd674bf 100644 --- a/pkg/engine/validate/validate.go +++ b/pkg/engine/validate/validate.go @@ -1,7 +1,6 @@ package validate import ( - "encoding/json" "errors" "fmt" "path/filepath" @@ -11,36 +10,9 @@ import ( "github.com/golang/glog" "github.com/nirmata/kyverno/pkg/engine/anchor" - "github.com/nirmata/kyverno/pkg/engine/context" "github.com/nirmata/kyverno/pkg/engine/operator" - "github.com/nirmata/kyverno/pkg/engine/variables" ) -// ValidateResourceWithPattern is a start of element-by-element validation process -// It assumes that validation is started from root, so "/" is passed -func ValidateResourceWithPattern(ctx context.EvalInterface, resource, pattern interface{}) (string, ValidationError) { - // if referenced path is not present, we skip processing the rule and report violation - if invalidPaths := variables.ValidateVariables(ctx, pattern); len(invalidPaths) != 0 { - return "", newValidatePatternError(PathNotPresent, invalidPaths) - } - - // Operate on copy of the value for variable substitution - patternCopy, err := copyInterface(pattern) - if err != nil { - return "", newValidatePatternError(OtherError, err.Error()) - } - // first pass we substitute all the JMESPATH substitution for the variable - // variable: {{}} - // if a JMESPATH fails, we dont return error but variable is substitured with nil and error log - patternCopy = variables.SubstituteVariables(ctx, patternCopy) - path, err := validateResourceElement(resource, patternCopy, patternCopy, "/") - if err != nil { - return path, newValidatePatternError(Rulefailure, err.Error()) - } - - return "", ValidationError{} -} - // ValidateResourceWithPattern1 is a start of element-by-element validation process // It assumes that validation is started from root, so "/" is passed func ValidateResourceWithPattern1(resource, pattern interface{}) (string, error) { @@ -52,20 +24,6 @@ func ValidateResourceWithPattern1(resource, pattern interface{}) (string, error) return "", nil } -func copyInterface(original interface{}) (interface{}, error) { - tempData, err := json.Marshal(original) - if err != nil { - return nil, err - } - fmt.Println(string(tempData)) - var temp interface{} - err = json.Unmarshal(tempData, &temp) - if err != nil { - return nil, err - } - return temp, nil -} - // validateResourceElement detects the element type (map, array, nil, string, int, bool, float) // and calls corresponding handler // Pattern tree and resource tree can have different structure. In this case validation fails @@ -102,7 +60,7 @@ func validateResourceElement(resourceElement, patternElement, originPattern inte } } if !ValidateValueWithPattern(resourceElement, patternElement) { - return path, fmt.Errorf("Validation rule failed at '%s' to validate value %v with pattern %v", path, resourceElement, patternElement) + return path, fmt.Errorf("Validation rule failed at '%s' to validate value '%v' with pattern '%v'", path, resourceElement, patternElement) } default: diff --git a/pkg/engine/validation.go b/pkg/engine/validation.go index 87c9b04b91..deb0f8e609 100644 --- a/pkg/engine/validation.go +++ b/pkg/engine/validation.go @@ -1,10 +1,8 @@ package engine import ( - "errors" "fmt" "reflect" - "strings" "time" "github.com/golang/glog" @@ -95,13 +93,6 @@ func validateResource(ctx context.EvalInterface, policy kyverno.ClusterPolicy, r } startTime := time.Now() - if paths := validateGeneralRuleInfoVariables(ctx, rule); len(paths) != 0 { - glog.Infof("referenced path not present in rule %s/, resource %s/%s/%s, path: %s", rule.Name, resource.GetKind(), resource.GetNamespace(), resource.GetName(), paths) - resp.PolicyResponse.Rules = append(resp.PolicyResponse.Rules, - newPathNotPresentRuleResponse(rule.Name, utils.Validation.String(), fmt.Sprintf("path not present: %s", paths))) - continue - } - if !rbac.MatchAdmissionInfo(rule, admissionInfo) { glog.V(3).Infof("rule '%s' cannot be applied on %s/%s/%s, admission permission: %v", rule.Name, resource.GetKind(), resource.GetNamespace(), resource.GetName(), admissionInfo) @@ -121,6 +112,7 @@ func validateResource(ctx context.EvalInterface, policy kyverno.ClusterPolicy, r // operate on the copy of the conditions, as we perform variable substitution copyConditions := copyConditions(rule.Conditions) // evaluate pre-conditions + // - handle variable subsitutions if !variables.EvaluateConditions(ctx, copyConditions) { glog.V(4).Infof("resource %s/%s does not satisfy the conditions for the rule ", resource.GetNamespace(), resource.GetName()) continue @@ -184,33 +176,29 @@ func validatePatterns(ctx context.EvalInterface, resource unstructured.Unstructu resp.RuleStats.ProcessingTime = time.Since(startTime) glog.V(4).Infof("finished applying validation rule %q (%v)", resp.Name, resp.RuleStats.ProcessingTime) }() + // work on a copy of validation rule + validationRule := rule.Validation.DeepCopy() // either pattern or anyPattern can be specified in Validation rule - if rule.Validation.Pattern != nil { - path, err := validate.ValidateResourceWithPattern(ctx, resource.Object, rule.Validation.Pattern) - if !reflect.DeepEqual(err, validate.ValidationError{}) { - switch err.StatusCode { - case validate.PathNotPresent: - resp.Success = true - resp.PathNotPresent = true - resp.Message = fmt.Sprintf("referenced path not present: %s", err.ErrorMsg) - glog.V(4).Infof("Skip applying rule '%s' on resource '%s/%s/%s': %s", rule.Name, resource.GetKind(), resource.GetNamespace(), resource.GetName(), resp.Message) - case validate.Rulefailure: - // rule application failed - glog.V(4).Infof("Validation rule '%s' failed at '%s' for resource %s/%s/%s. %s: %v", rule.Name, path, resource.GetKind(), resource.GetNamespace(), resource.GetName(), rule.Validation.Message, err) - resp.Success = false - resp.Message = fmt.Sprintf("Validation error: %s; Validation rule '%s' failed at path '%s'", - rule.Validation.Message, rule.Name, path) - case validate.OtherError: - // other error - glog.V(4).Infof("Validation rule '%s' failed at '%s' for resource %s/%s/%s. %s: %v", rule.Name, path, resource.GetKind(), resource.GetNamespace(), resource.GetName(), rule.Validation.Message, err) - resp.Success = false - resp.Message = fmt.Sprintf("Validation error: %s; Validation rule '%s' failed at path '%s'", - rule.Validation.Message, rule.Name, path) - } + if validationRule.Pattern != nil { + // substitute variables in the pattern + pattern := validationRule.Pattern + var err error + if pattern, err = variables.SubstituteVars(ctx, pattern); err != nil { + // variable subsitution failed + resp.Success = false + resp.Message = fmt.Sprintf("Validation error: %s; Validation rule '%s' failed. '%s'", + rule.Validation.Message, rule.Name, err) return resp } + if path, err := validate.ValidateResourceWithPattern1(resource.Object, pattern); err != nil { + // validation failed + resp.Success = false + resp.Message = fmt.Sprintf("Validation error: %s; Validation rule '%s' failed at path '%s'", + rule.Validation.Message, rule.Name, path) + return resp + } // rule application successful glog.V(4).Infof("rule %s pattern validated successfully on resource %s/%s/%s", rule.Name, resource.GetKind(), resource.GetNamespace(), resource.GetName()) resp.Success = true @@ -218,55 +206,45 @@ func validatePatterns(ctx context.EvalInterface, resource unstructured.Unstructu return resp } - // using anyPattern we can define multiple patterns and only one of them has to be successfully validated - // return directly if one pattern succeed - // if none succeed, report violation / policyerror(TODO) - if rule.Validation.AnyPattern != nil { - var ruleFailureErrs []error - var failedPaths, invalidPaths []string - for index, pattern := range rule.Validation.AnyPattern { - path, err := validate.ValidateResourceWithPattern(ctx, resource.Object, pattern) - // this pattern was successfully validated - if reflect.DeepEqual(err, validate.ValidationError{}) { - glog.V(4).Infof("anyPattern %v successfully validated on resource %s/%s/%s", pattern, resource.GetKind(), resource.GetNamespace(), resource.GetName()) + if validationRule.AnyPattern != nil { + var failedSubstitutionsErrors []error + var failedAnyPatternsErrors []error + var err error + for idx, pattern := range validationRule.AnyPattern { + if pattern, err = variables.SubstituteVars(ctx, pattern); err != nil { + // variable subsitution failed + failedSubstitutionsErrors = append(failedSubstitutionsErrors, err) + continue + } + _, err := validate.ValidateResourceWithPattern1(resource.Object, pattern) + if err == nil { resp.Success = true - resp.Message = fmt.Sprintf("Validation rule '%s' anyPattern[%d] succeeded.", rule.Name, index) + resp.Message = fmt.Sprintf("Validation rule '%s' anyPattern[%d] succeeded.", rule.Name, idx) return resp } - - switch err.StatusCode { - case validate.PathNotPresent: - invalidPaths = append(invalidPaths, err.ErrorMsg) - case validate.Rulefailure: - glog.V(4).Infof("Validation error: %s; Validation rule %s anyPattern[%d] failed at path %s for %s/%s/%s", - rule.Validation.Message, rule.Name, index, path, resource.GetKind(), resource.GetNamespace(), resource.GetName()) - ruleFailureErrs = append(ruleFailureErrs, errors.New(err.ErrorMsg)) - failedPaths = append(failedPaths, path) - } + glog.V(4).Infof("Validation error: %s; Validation rule %s anyPattern[%d] for %s/%s/%s", + rule.Validation.Message, rule.Name, idx, resource.GetKind(), resource.GetNamespace(), resource.GetName()) + patternErr := fmt.Errorf("anyPattern[%d] failed; %s", idx, err) + failedAnyPatternsErrors = append(failedAnyPatternsErrors, patternErr) } - // PathNotPresent - if len(invalidPaths) != 0 { - resp.Success = true - resp.PathNotPresent = true - resp.Message = fmt.Sprintf("referenced path not present: %s", strings.Join(invalidPaths, ";")) - glog.V(4).Infof("Skip applying rule '%s' on resource '%s/%s/%s': %s", rule.Name, resource.GetKind(), resource.GetNamespace(), resource.GetName(), resp.Message) + // Subsitution falures + if len(failedSubstitutionsErrors) > 0 { + resp.Success = false + resp.Message = fmt.Sprintf("Subsitutions failed at paths: %v", failedSubstitutionsErrors) return resp } - // none of the anyPatterns succeed: len(ruleFailureErrs) > 0 - glog.V(4).Infof("none of anyPattern comply with resource: %v", ruleFailureErrs) - resp.Success = false - var errorStr []string - for index, err := range ruleFailureErrs { - glog.V(4).Infof("anyPattern[%d] failed at path %s: %v", index, failedPaths[index], err) - str := fmt.Sprintf("Validation rule %s anyPattern[%d] failed at path %s.", rule.Name, index, failedPaths[index]) - errorStr = append(errorStr, str) + // Any Pattern validation errors + if len(failedAnyPatternsErrors) > 0 { + var errorStr []string + for _, err := range failedAnyPatternsErrors { + errorStr = append(errorStr, err.Error()) + } + resp.Success = false + resp.Message = fmt.Sprintf("Validation rule '%s' failed. %s", rule.Name, errorStr) + return resp } - - resp.Message = fmt.Sprintf("Validation error: %s; %s", rule.Validation.Message, strings.Join(errorStr, " ")) - return resp } - return response.RuleResponse{} } diff --git a/pkg/engine/validation_test.go b/pkg/engine/validation_test.go index 8a7ac06e23..0131a34511 100644 --- a/pkg/engine/validation_test.go +++ b/pkg/engine/validation_test.go @@ -294,7 +294,7 @@ func TestValidate_Fail_anyPattern(t *testing.T) { resourceUnstructured, err := utils.ConvertToUnstructured(rawResource) assert.NilError(t, err) er := Validate(PolicyContext{Policy: policy, NewResource: *resourceUnstructured}) - msgs := []string{"Validation error: A namespace is required; Validation rule check-default-namespace anyPattern[0] failed at path /metadata/namespace/. Validation rule check-default-namespace anyPattern[1] failed at path /metadata/namespace/."} + msgs := []string{"Validation rule 'check-default-namespace' failed. [anyPattern[0] failed; Validation rule failed at '/metadata/namespace/' to validate value '' with pattern '?*' anyPattern[1] failed; Validation rule failed at '/metadata/namespace/' to validate value '' with pattern '!default']"} for index, r := range er.PolicyResponse.Rules { assert.Equal(t, r.Message, msgs[index]) } @@ -1309,8 +1309,8 @@ func Test_VariableSubstitutionPathNotExistInPattern(t *testing.T) { Context: ctx, NewResource: *resourceUnstructured} er := Validate(policyContext) - assert.Assert(t, er.PolicyResponse.Rules[0].Success, true) - assert.Assert(t, er.PolicyResponse.Rules[0].PathNotPresent, true) + assert.Assert(t, !er.PolicyResponse.Rules[0].Success) + assert.Equal(t, er.PolicyResponse.Rules[0].Message, "Validation error: ; Validation rule 'test-path-not-exist' failed. 'variable(s) not found or has nil values: [/spec/containers/0/name/{{request.object.metadata.name1}}]'") } func Test_VariableSubstitutionPathNotExistInAnyPattern_OnePatternStatisfies(t *testing.T) { @@ -1399,10 +1399,8 @@ func Test_VariableSubstitutionPathNotExistInAnyPattern_OnePatternStatisfies(t *t Context: ctx, NewResource: *resourceUnstructured} er := Validate(policyContext) - assert.Assert(t, er.PolicyResponse.Rules[0].Success == true) - assert.Assert(t, er.PolicyResponse.Rules[0].PathNotPresent == false) - expectMsg := "Validation rule 'test-path-not-exist' anyPattern[1] succeeded." - assert.Assert(t, er.PolicyResponse.Rules[0].Message == expectMsg) + assert.Assert(t, er.PolicyResponse.Rules[0].Success) + assert.Equal(t, er.PolicyResponse.Rules[0].Message, "Validation rule 'test-path-not-exist' anyPattern[1] succeeded.") } func Test_VariableSubstitutionPathNotExistInAnyPattern_AllPathNotPresent(t *testing.T) { @@ -1491,8 +1489,8 @@ func Test_VariableSubstitutionPathNotExistInAnyPattern_AllPathNotPresent(t *test Context: ctx, NewResource: *resourceUnstructured} er := Validate(policyContext) - assert.Assert(t, er.PolicyResponse.Rules[0].Success, true) - assert.Assert(t, er.PolicyResponse.Rules[0].PathNotPresent, true) + assert.Assert(t, !er.PolicyResponse.Rules[0].Success) + assert.Equal(t, er.PolicyResponse.Rules[0].Message, "Subsitutions failed at paths: [variable(s) not found or has nil values: [/spec/template/spec/containers/0/name/{{request.object.metadata.name1}}] variable(s) not found or has nil values: [/spec/template/spec/containers/0/name/{{request.object.metadata.name2}}]]") } func Test_VariableSubstitutionPathNotExistInAnyPattern_AllPathPresent_NonePatternSatisfy(t *testing.T) { @@ -1582,8 +1580,7 @@ func Test_VariableSubstitutionPathNotExistInAnyPattern_AllPathPresent_NonePatter NewResource: *resourceUnstructured} er := Validate(policyContext) - expectedMsg := "Validation error: ; Validation rule test-path-not-exist anyPattern[0] failed at path /spec/template/spec/containers/0/name/. Validation rule test-path-not-exist anyPattern[1] failed at path /spec/template/spec/containers/0/name/." - assert.Assert(t, er.PolicyResponse.Rules[0].Success == false) - assert.Assert(t, er.PolicyResponse.Rules[0].PathNotPresent == false) - assert.Assert(t, er.PolicyResponse.Rules[0].Message == expectedMsg) + // expectedMsg := "Validation error: ; Validation rule test-path-not-exist anyPattern[0] failed at path /spec/template/spec/containers/0/name/. Validation rule test-path-not-exist anyPattern[1] failed at path /spec/template/spec/containers/0/name/." + assert.Assert(t, !er.PolicyResponse.Rules[0].Success) + assert.Equal(t, er.PolicyResponse.Rules[0].Message, "Validation rule 'test-path-not-exist' failed. [anyPattern[0] failed; Validation rule failed at '/spec/template/spec/containers/0/name/' to validate value 'pod-test-pod' with pattern 'test*' anyPattern[1] failed; Validation rule failed at '/spec/template/spec/containers/0/name/' to validate value 'pod-test-pod' with pattern 'test*']") } diff --git a/pkg/engine/variables/evaluate.go b/pkg/engine/variables/evaluate.go index 4a5c215ea0..519a909d01 100644 --- a/pkg/engine/variables/evaluate.go +++ b/pkg/engine/variables/evaluate.go @@ -10,7 +10,7 @@ import ( //Evaluate evaluates the condition func Evaluate(ctx context.EvalInterface, condition kyverno.Condition) bool { // get handler for the operator - handle := operator.CreateOperatorHandler(ctx, condition.Operator, SubstituteVariables) + handle := operator.CreateOperatorHandler(ctx, condition.Operator, SubstituteVars) if handle == nil { return false } diff --git a/pkg/engine/variables/operator/equal.go b/pkg/engine/variables/operator/equal.go index 8d83b78415..81ea80c621 100644 --- a/pkg/engine/variables/operator/equal.go +++ b/pkg/engine/variables/operator/equal.go @@ -25,25 +25,36 @@ type EqualHandler struct { //Evaluate evaluates expression with Equal Operator func (eh EqualHandler) Evaluate(key, value interface{}) bool { + var err error + //TODO: decouple variables from evaluation // substitute the variables - nKey := eh.subHandler(eh.ctx, key) - nValue := eh.subHandler(eh.ctx, value) + if key, err = eh.subHandler(eh.ctx, key); err != nil { + // Failed to resolve the variable + glog.Infof("Failed to resolve variables in key: %s: %v", key, err) + return false + } + if value, err = eh.subHandler(eh.ctx, value); err != nil { + // Failed to resolve the variable + glog.Infof("Failed to resolve variables in value: %s: %v", value, err) + return false + } + // key and value need to be of same type - switch typedKey := nKey.(type) { + switch typedKey := key.(type) { case bool: - return eh.validateValuewithBoolPattern(typedKey, nValue) + return eh.validateValuewithBoolPattern(typedKey, value) case int: - return eh.validateValuewithIntPattern(int64(typedKey), nValue) + return eh.validateValuewithIntPattern(int64(typedKey), value) case int64: - return eh.validateValuewithIntPattern(typedKey, nValue) + return eh.validateValuewithIntPattern(typedKey, value) case float64: - return eh.validateValuewithFloatPattern(typedKey, nValue) + return eh.validateValuewithFloatPattern(typedKey, value) case string: - return eh.validateValuewithStringPattern(typedKey, nValue) + return eh.validateValuewithStringPattern(typedKey, value) case map[string]interface{}: - return eh.validateValueWithMapPattern(typedKey, nValue) + return eh.validateValueWithMapPattern(typedKey, value) case []interface{}: - return eh.validateValueWithSlicePattern(typedKey, nValue) + return eh.validateValueWithSlicePattern(typedKey, value) default: glog.Errorf("Unsupported type %v", typedKey) return false diff --git a/pkg/engine/variables/operator/notequal.go b/pkg/engine/variables/operator/notequal.go index adc6afd27d..9af9e891f3 100644 --- a/pkg/engine/variables/operator/notequal.go +++ b/pkg/engine/variables/operator/notequal.go @@ -25,25 +25,35 @@ type NotEqualHandler struct { //Evaluate evaluates expression with NotEqual Operator func (neh NotEqualHandler) Evaluate(key, value interface{}) bool { + var err error + //TODO: decouple variables from evaluation // substitute the variables - nKey := neh.subHandler(neh.ctx, key) - nValue := neh.subHandler(neh.ctx, value) + if key, err = neh.subHandler(neh.ctx, key); err != nil { + // Failed to resolve the variable + glog.Infof("Failed to resolve variables in key: %s: %v", key, err) + return false + } + if value, err = neh.subHandler(neh.ctx, value); err != nil { + // Failed to resolve the variable + glog.Infof("Failed to resolve variables in value: %s: %v", value, err) + return false + } // key and value need to be of same type - switch typedKey := nKey.(type) { + switch typedKey := key.(type) { case bool: - return neh.validateValuewithBoolPattern(typedKey, nValue) + return neh.validateValuewithBoolPattern(typedKey, value) case int: - return neh.validateValuewithIntPattern(int64(typedKey), nValue) + return neh.validateValuewithIntPattern(int64(typedKey), value) case int64: - return neh.validateValuewithIntPattern(typedKey, nValue) + return neh.validateValuewithIntPattern(typedKey, value) case float64: - return neh.validateValuewithFloatPattern(typedKey, nValue) + return neh.validateValuewithFloatPattern(typedKey, value) case string: - return neh.validateValuewithStringPattern(typedKey, nValue) + return neh.validateValuewithStringPattern(typedKey, value) case map[string]interface{}: - return neh.validateValueWithMapPattern(typedKey, nValue) + return neh.validateValueWithMapPattern(typedKey, value) case []interface{}: - return neh.validateValueWithSlicePattern(typedKey, nValue) + return neh.validateValueWithSlicePattern(typedKey, value) default: glog.Error("Unsupported type %V", typedKey) return false diff --git a/pkg/engine/variables/operator/operator.go b/pkg/engine/variables/operator/operator.go index 17ad2d34e5..2b3f9ce7a1 100644 --- a/pkg/engine/variables/operator/operator.go +++ b/pkg/engine/variables/operator/operator.go @@ -17,7 +17,7 @@ type OperatorHandler interface { } //VariableSubstitutionHandler defines the handler function for variable substitution -type VariableSubstitutionHandler = func(ctx context.EvalInterface, pattern interface{}) interface{} +type VariableSubstitutionHandler = func(ctx context.EvalInterface, pattern interface{}) (interface{}, error) //CreateOperatorHandler returns the operator handler based on the operator used in condition func CreateOperatorHandler(ctx context.EvalInterface, op kyverno.ConditionOperator, subHandler VariableSubstitutionHandler) OperatorHandler { diff --git a/pkg/engine/variables/validatevariables.go b/pkg/engine/variables/validatevariables.go index f1c9ee7d3f..478564d708 100644 --- a/pkg/engine/variables/validatevariables.go +++ b/pkg/engine/variables/validatevariables.go @@ -1,91 +1,82 @@ package variables -import ( - "fmt" - "regexp" - "strings" +// // ValidateVariables validates if referenced path is present +// // return empty string if all paths are valid, otherwise return invalid path +// func ValidateVariables(ctx context.EvalInterface, pattern interface{}) string { +// var pathsNotPresent []string +// variableList := extractVariables(pattern) +// for _, variable := range variableList { +// if len(variable) == 2 { +// varName := variable[0] +// varValue := variable[1] +// glog.V(3).Infof("validating variable %s", varName) +// val, err := ctx.Query(varValue) +// if err == nil && val == nil { +// // path is not present, returns nil interface +// pathsNotPresent = append(pathsNotPresent, varValue) +// } +// } +// } - "github.com/golang/glog" - "github.com/nirmata/kyverno/pkg/engine/context" -) +// if len(pathsNotPresent) != 0 { +// return strings.Join(pathsNotPresent, ";") +// } +// return "" +// } -// ValidateVariables validates if referenced path is present -// return empty string if all paths are valid, otherwise return invalid path -func ValidateVariables(ctx context.EvalInterface, pattern interface{}) string { - var pathsNotPresent []string - variableList := extractVariables(pattern) - for _, variable := range variableList { - if len(variable) == 2 { - varName := variable[0] - varValue := variable[1] - glog.V(3).Infof("validating variable %s", varName) - val, err := ctx.Query(varValue) - if err == nil && val == nil { - // path is not present, returns nil interface - pathsNotPresent = append(pathsNotPresent, varValue) - } - } - } +// // extractVariables extracts variables in the pattern +// func extractVariables(pattern interface{}) [][]string { +// switch typedPattern := pattern.(type) { +// case map[string]interface{}: +// return extractMap(typedPattern) +// case []interface{}: +// return extractArray(typedPattern) +// case string: +// return extractValue(typedPattern) +// default: +// fmt.Printf("variable type %T", typedPattern) +// return nil +// } +// } - if len(pathsNotPresent) != 0 { - return strings.Join(pathsNotPresent, ";") - } - return "" -} +// func extractMap(patternMap map[string]interface{}) [][]string { +// var variableList [][]string -// extractVariables extracts variables in the pattern -func extractVariables(pattern interface{}) [][]string { - switch typedPattern := pattern.(type) { - case map[string]interface{}: - return extractMap(typedPattern) - case []interface{}: - return extractArray(typedPattern) - case string: - return extractValue(typedPattern) - default: - fmt.Printf("variable type %T", typedPattern) - return nil - } -} +// for _, patternElement := range patternMap { +// if vars := extractVariables(patternElement); vars != nil { +// variableList = append(variableList, vars...) +// } +// } +// return variableList +// } -func extractMap(patternMap map[string]interface{}) [][]string { - var variableList [][]string +// func extractArray(patternList []interface{}) [][]string { +// var variableList [][]string - for _, patternElement := range patternMap { - if vars := extractVariables(patternElement); vars != nil { - variableList = append(variableList, vars...) - } - } - return variableList -} +// for _, patternElement := range patternList { +// if vars := extractVariables(patternElement); vars != nil { +// variableList = append(variableList, vars...) +// } +// } +// return variableList +// } -func extractArray(patternList []interface{}) [][]string { - var variableList [][]string +// func extractValue(valuePattern string) [][]string { +// operatorVariable := getOperator(valuePattern) +// variable := valuePattern[len(operatorVariable):] +// return extractValueVariable(variable) +// } - for _, patternElement := range patternList { - if vars := extractVariables(patternElement); vars != nil { - variableList = append(variableList, vars...) - } - } - return variableList -} - -func extractValue(valuePattern string) [][]string { - operatorVariable := getOperator(valuePattern) - variable := valuePattern[len(operatorVariable):] - return extractValueVariable(variable) -} - -// returns multiple variable match groups -func extractValueVariable(valuePattern string) [][]string { - variableRegex := regexp.MustCompile(variableRegex) - groups := variableRegex.FindAllStringSubmatch(valuePattern, -1) - if len(groups) == 0 { - // no variables - return nil - } - // group[*] <- all the matches - // group[*][0] <- group match - // group[*][1] <- group capture value - return groups -} +// // returns multiple variable match groups +// func extractValueVariable(valuePattern string) [][]string { +// variableRegex := regexp.MustCompile(variableRegex) +// groups := variableRegex.FindAllStringSubmatch(valuePattern, -1) +// if len(groups) == 0 { +// // no variables +// return nil +// } +// // group[*] <- all the matches +// // group[*][0] <- group match +// // group[*][1] <- group capture value +// return groups +// } diff --git a/pkg/engine/variables/validatevariables_test.go b/pkg/engine/variables/validatevariables_test.go deleted file mode 100644 index e39e9cf486..0000000000 --- a/pkg/engine/variables/validatevariables_test.go +++ /dev/null @@ -1,175 +0,0 @@ -package variables - -import ( - "encoding/json" - "fmt" - "testing" - - kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1" - "github.com/nirmata/kyverno/pkg/engine/context" - "gotest.tools/assert" - authenticationv1 "k8s.io/api/authentication/v1" -) - -func Test_ExtractVariables(t *testing.T) { - patternRaw := []byte(` - { - "name": "ns-owner-{{request.userInfo.username}}", - "data": { - "rules": [ - { - "apiGroups": [ - "" - ], - "resources": [ - "namespaces" - ], - "verbs": [ - "*" - ], - "resourceNames": [ - "{{request.object.metadata.name}}" - ] - } - ] - } - } - `) - - var pattern interface{} - if err := json.Unmarshal(patternRaw, &pattern); err != nil { - t.Error(err) - } - - vars := extractVariables(pattern) - - result := [][]string{{"{{request.userInfo.username}}", "request.userInfo.username"}, - {"{{request.object.metadata.name}}", "request.object.metadata.name"}} - - assert.Assert(t, len(vars) == len(result), fmt.Sprintf("result does not match, var: %s", vars)) -} - -func Test_ValidateVariables_NoVariable(t *testing.T) { - patternRaw := []byte(` -{ - "name": "ns-owner", - "data": { - "rules": [ - { - "apiGroups": [ - "" - ], - "resources": [ - "namespaces" - ], - "verbs": [ - "*" - ], - "resourceNames": [ - "Pod" - ] - } - ] - } -} -`) - - resourceRaw := []byte(` - { - "metadata": { - "name": "temp", - "namespace": "n1" - }, - "spec": { - "namespace": "n1", - "name": "temp1" - } - } - `) - - // userInfo - userReqInfo := kyverno.RequestInfo{ - AdmissionUserInfo: authenticationv1.UserInfo{ - Username: "user1", - }, - } - var pattern, resource interface{} - assert.NilError(t, json.Unmarshal(patternRaw, &pattern)) - assert.NilError(t, json.Unmarshal(resourceRaw, &resource)) - - var err error - ctx := context.NewContext() - err = ctx.AddResource(resourceRaw) - if err != nil { - t.Error(err) - } - err = ctx.AddUserInfo(userReqInfo) - if err != nil { - t.Error(err) - } - invalidPaths := ValidateVariables(ctx, pattern) - assert.Assert(t, len(invalidPaths) == 0) -} - -func Test_ValidateVariables(t *testing.T) { - patternRaw := []byte(` -{ - "name": "ns-owner-{{request.userInfo.username}}", - "data": { - "rules": [ - { - "apiGroups": [ - "" - ], - "resources": [ - "namespaces" - ], - "verbs": [ - "*" - ], - "resourceNames": [ - "{{request.object.metadata.name1}}" - ] - } - ] - } -} -`) - - resourceRaw := []byte(` - { - "metadata": { - "name": "temp", - "namespace": "n1" - }, - "spec": { - "namespace": "n1", - "name": "temp1" - } - } - `) - - // userInfo - userReqInfo := kyverno.RequestInfo{ - AdmissionUserInfo: authenticationv1.UserInfo{ - Username: "user1", - }, - } - var pattern, resource interface{} - assert.NilError(t, json.Unmarshal(patternRaw, &pattern)) - assert.NilError(t, json.Unmarshal(resourceRaw, &resource)) - - ctx := context.NewContext() - var err error - err = ctx.AddResource(resourceRaw) - if err != nil { - t.Error(err) - } - err = ctx.AddUserInfo(userReqInfo) - if err != nil { - t.Error(err) - } - - invalidPaths := ValidateVariables(ctx, pattern) - assert.Assert(t, len(invalidPaths) > 0) -} diff --git a/pkg/engine/variables/variables.go b/pkg/engine/variables/variables.go deleted file mode 100644 index 71cda21df5..0000000000 --- a/pkg/engine/variables/variables.go +++ /dev/null @@ -1,146 +0,0 @@ -package variables - -import ( - "regexp" - "strings" - - "github.com/golang/glog" - "github.com/nirmata/kyverno/pkg/engine/context" - "github.com/nirmata/kyverno/pkg/engine/operator" -) - -const variableRegex = `\{\{([^{}]*)\}\}` - -//SubstituteVariables substitutes the JMESPATH with variable substitution -// supported substitutions -// - no operator + variable(string,object) -// unsupported substitutions -// - operator + variable(object) -> as we dont support operators with object types -func SubstituteVariables(ctx context.EvalInterface, pattern interface{}) interface{} { - // var err error - switch typedPattern := pattern.(type) { - case map[string]interface{}: - return substituteMap(ctx, typedPattern) - case []interface{}: - return substituteArray(ctx, typedPattern) - case string: - // variable substitution - return substituteValue(ctx, typedPattern) - default: - return pattern - } -} - -func substituteMap(ctx context.EvalInterface, patternMap map[string]interface{}) map[string]interface{} { - for key, patternElement := range patternMap { - value := SubstituteVariables(ctx, patternElement) - patternMap[key] = value - } - return patternMap -} - -func substituteArray(ctx context.EvalInterface, patternList []interface{}) []interface{} { - for idx, patternElement := range patternList { - value := SubstituteVariables(ctx, patternElement) - patternList[idx] = value - } - return patternList -} - -func substituteValue(ctx context.EvalInterface, valuePattern string) interface{} { - // patterns supported - // - operator + string - // operator + variable - operatorVariable := getOperator(valuePattern) - variable := valuePattern[len(operatorVariable):] - // substitute variable with value - value := getValueQuery(ctx, variable) - if operatorVariable == "" { - // default or operator.Equal - // equal + string variable - // object variable - return value - } - // operator + string variable - switch value.(type) { - case string: - return string(operatorVariable) + value.(string) - default: - glog.Infof("cannot use operator with object variables. operator used %s in pattern %v", string(operatorVariable), valuePattern) - var emptyInterface interface{} - return emptyInterface - } -} - -func getValueQuery(ctx context.EvalInterface, valuePattern string) interface{} { - var emptyInterface interface{} - // extract variable {{}} - validRegex := regexp.MustCompile(variableRegex) - groups := validRegex.FindAllStringSubmatch(valuePattern, -1) - // can have multiple variables in a single value pattern - // var Map - varMap := getValues(ctx, groups) - if len(varMap) == 0 { - // there are no varaiables - // return the original value - return valuePattern - } - // only substitute values if all the variable values are of type string - if isAllVarStrings(varMap) { - newVal := valuePattern - for key, value := range varMap { - if val, ok := value.(string); ok { - newVal = strings.Replace(newVal, key, val, -1) - } - } - return newVal - } - - // we do not support multiple substitution per statement for non-string types - for _, value := range varMap { - return value - } - return emptyInterface -} - -// returns map of variables as keys and variable values as values -func getValues(ctx context.EvalInterface, groups [][]string) map[string]interface{} { - var emptyInterface interface{} - subs := map[string]interface{}{} - for _, group := range groups { - if len(group) == 2 { - // 0th is string - varName := group[0] - varValue := group[1] - variable, err := ctx.Query(varValue) - if err != nil { - glog.V(4).Infof("variable substitution failed for query %s: %v", varName, err) - subs[varName] = emptyInterface - continue - } - if variable == nil { - subs[varName] = emptyInterface - } else { - subs[varName] = variable - } - } - } - return subs -} - -func isAllVarStrings(subVar map[string]interface{}) bool { - for _, value := range subVar { - if _, ok := value.(string); !ok { - return false - } - } - return true -} - -func getOperator(pattern string) string { - operatorVariable := operator.GetOperatorFromStringPattern(pattern) - if operatorVariable == operator.Equal { - return "" - } - return string(operatorVariable) -} diff --git a/pkg/engine/variables/variables_check.go b/pkg/engine/variables/variables_check.go index 19ee50172e..ba4f4a5c38 100644 --- a/pkg/engine/variables/variables_check.go +++ b/pkg/engine/variables/variables_check.go @@ -1,80 +1,80 @@ package variables -import ( - "fmt" - "regexp" - "strconv" -) +// import ( +// "fmt" +// "regexp" +// "strconv" +// ) -//CheckVariables checks if the variable regex has been used -func CheckVariables(pattern interface{}, variables []string, path string) error { - switch typedPattern := pattern.(type) { - case map[string]interface{}: - return checkMap(typedPattern, variables, path) - case []interface{}: - return checkArray(typedPattern, variables, path) - case string: - return checkValue(typedPattern, variables, path) - default: - return nil - } -} +// //CheckVariables checks if the variable regex has been used +// func CheckVariables(pattern interface{}, variables []string, path string) error { +// switch typedPattern := pattern.(type) { +// case map[string]interface{}: +// return checkMap(typedPattern, variables, path) +// case []interface{}: +// return checkArray(typedPattern, variables, path) +// case string: +// return checkValue(typedPattern, variables, path) +// default: +// return nil +// } +// } -func checkMap(patternMap map[string]interface{}, variables []string, path string) error { - for patternKey, patternElement := range patternMap { +// func checkMap(patternMap map[string]interface{}, variables []string, path string) error { +// for patternKey, patternElement := range patternMap { - if err := CheckVariables(patternElement, variables, path+patternKey+"/"); err != nil { - return err - } - } - return nil -} +// if err := CheckVariables(patternElement, variables, path+patternKey+"/"); err != nil { +// return err +// } +// } +// return nil +// } -func checkArray(patternList []interface{}, variables []string, path string) error { - for idx, patternElement := range patternList { - if err := CheckVariables(patternElement, variables, path+strconv.Itoa(idx)+"/"); err != nil { - return err - } - } - return nil -} +// func checkArray(patternList []interface{}, variables []string, path string) error { +// for idx, patternElement := range patternList { +// if err := CheckVariables(patternElement, variables, path+strconv.Itoa(idx)+"/"); err != nil { +// return err +// } +// } +// return nil +// } -func checkValue(valuePattern string, variables []string, path string) error { - operatorVariable := getOperator(valuePattern) - variable := valuePattern[len(operatorVariable):] - if checkValueVariable(variable, variables) { - return fmt.Errorf(path + valuePattern) - } - return nil -} +// func checkValue(valuePattern string, variables []string, path string) error { +// operatorVariable := getOperator(valuePattern) +// variable := valuePattern[len(operatorVariable):] +// if checkValueVariable(variable, variables) { +// return fmt.Errorf(path + valuePattern) +// } +// return nil +// } -func checkValueVariable(valuePattern string, variables []string) bool { - variableRegex := regexp.MustCompile(variableRegex) - groups := variableRegex.FindAllStringSubmatch(valuePattern, -1) - if len(groups) == 0 { - // no variables - return false - } - // if variables are defined, check against the list of variables to be filtered - for _, group := range groups { - if len(group) == 2 { - // group[0] -> {{variable}} - // group[1] -> variable - if variablePatternSearch(group[1], variables) { - return true - } - } - } - return false -} +// func checkValueVariable(valuePattern string, variables []string) bool { +// variableRegex := regexp.MustCompile(variableRegex) +// groups := variableRegex.FindAllStringSubmatch(valuePattern, -1) +// if len(groups) == 0 { +// // no variables +// return false +// } +// // if variables are defined, check against the list of variables to be filtered +// for _, group := range groups { +// if len(group) == 2 { +// // group[0] -> {{variable}} +// // group[1] -> variable +// if variablePatternSearch(group[1], variables) { +// return true +// } +// } +// } +// return false +// } -func variablePatternSearch(pattern string, regexs []string) bool { - for _, regex := range regexs { - varRegex := regexp.MustCompile(regex) - found := varRegex.FindString(pattern) - if found != "" { - return true - } - } - return true -} +// func variablePatternSearch(pattern string, regexs []string) bool { +// for _, regex := range regexs { +// varRegex := regexp.MustCompile(regex) +// found := varRegex.FindString(pattern) +// if found != "" { +// return true +// } +// } +// return true +// } diff --git a/pkg/engine/variables/variables_test.go b/pkg/engine/variables/variables_test.go index c7f125ed5a..ec85795dd7 100644 --- a/pkg/engine/variables/variables_test.go +++ b/pkg/engine/variables/variables_test.go @@ -58,12 +58,16 @@ func Test_variablesub1(t *testing.T) { resultMap := []byte(`{"data":{"rules":[{"apiGroups":[""],"resourceNames":["temp"],"resources":["namespaces"],"verbs":["*"]}]},"kind":"ClusterRole","name":"ns-owner-user1"}`) - var pattern, resource interface{} + var pattern, patternCopy, resource interface{} var err error err = json.Unmarshal(patternMap, &pattern) if err != nil { t.Error(err) } + err = json.Unmarshal(patternMap, &patternCopy) + if err != nil { + t.Error(err) + } err = json.Unmarshal(resourceRaw, &resource) if err != nil { t.Error(err) @@ -80,12 +84,15 @@ func Test_variablesub1(t *testing.T) { t.Error(err) } - value := SubstituteVariables(ctx, pattern) - resultRaw, err := json.Marshal(value) + if _, err := SubstituteVars(ctx, patternCopy); err != nil { + t.Error(err) + } + resultRaw, err := json.Marshal(patternCopy) if err != nil { t.Error(err) } - if !reflect.DeepEqual(resultMap, resultRaw) { + + if !reflect.DeepEqual(resultRaw, resultMap) { t.Log(string(resultMap)) t.Log(string(resultRaw)) t.Error("result does not match") @@ -139,12 +146,16 @@ func Test_variablesub_multiple(t *testing.T) { resultMap := []byte(`{"data":{"rules":[{"apiGroups":[""],"resourceNames":["temp"],"resources":["namespaces"],"verbs":["*"]}]},"kind":"ClusterRole","name":"ns-owner-n1-user1-bindings"}`) - var pattern, resource interface{} + var pattern, patternCopy, resource interface{} var err error err = json.Unmarshal(patternMap, &pattern) if err != nil { t.Error(err) } + err = json.Unmarshal(patternMap, &patternCopy) + if err != nil { + t.Error(err) + } err = json.Unmarshal(resourceRaw, &resource) if err != nil { @@ -163,11 +174,14 @@ func Test_variablesub_multiple(t *testing.T) { t.Error(err) } - value := SubstituteVariables(ctx, pattern) - resultRaw, err := json.Marshal(value) + if _, err := SubstituteVars(ctx, patternCopy); err != nil { + t.Error(err) + } + resultRaw, err := json.Marshal(patternCopy) if err != nil { t.Error(err) } + if !reflect.DeepEqual(resultMap, resultRaw) { t.Log(string(resultMap)) t.Log(string(resultRaw)) @@ -219,12 +233,16 @@ func Test_variablesubstitution(t *testing.T) { Username: "user1", }, } - var pattern, resource interface{} + var pattern, patternCopy, resource interface{} var err error err = json.Unmarshal(patternMap, &pattern) if err != nil { t.Error(err) } + err = json.Unmarshal(patternMap, &patternCopy) + if err != nil { + t.Error(err) + } err = json.Unmarshal(resourceRaw, &resource) if err != nil { @@ -243,8 +261,10 @@ func Test_variablesubstitution(t *testing.T) { t.Error(err) } - value := SubstituteVariables(ctx, pattern) - resultRaw, err := json.Marshal(value) + if _, err := SubstituteVars(ctx, patternCopy); err != nil { + t.Error(err) + } + resultRaw, err := json.Marshal(patternCopy) if err != nil { t.Error(err) } @@ -279,12 +299,16 @@ func Test_variableSubstitutionValue(t *testing.T) { resultMap := []byte(`{"spec":{"name":"temp"}}`) - var pattern, resource interface{} + var pattern, patternCopy, resource interface{} var err error err = json.Unmarshal(patternMap, &pattern) if err != nil { t.Error(err) } + err = json.Unmarshal(patternMap, &patternCopy) + if err != nil { + t.Error(err) + } err = json.Unmarshal(resourceRaw, &resource) if err != nil { @@ -298,8 +322,10 @@ func Test_variableSubstitutionValue(t *testing.T) { t.Error(err) } - value := SubstituteVariables(ctx, pattern) - resultRaw, err := json.Marshal(value) + if _, err := SubstituteVars(ctx, patternCopy); err != nil { + t.Error(err) + } + resultRaw, err := json.Marshal(patternCopy) if err != nil { t.Error(err) } @@ -331,12 +357,16 @@ func Test_variableSubstitutionValueOperatorNotEqual(t *testing.T) { `) resultMap := []byte(`{"spec":{"name":"!temp"}}`) - var pattern, resource interface{} + var pattern, patternCopy, resource interface{} var err error err = json.Unmarshal(patternMap, &pattern) if err != nil { t.Error(err) } + err = json.Unmarshal(patternMap, &patternCopy) + if err != nil { + t.Error(err) + } err = json.Unmarshal(resourceRaw, &resource) if err != nil { @@ -350,14 +380,16 @@ func Test_variableSubstitutionValueOperatorNotEqual(t *testing.T) { t.Error(err) } - value := SubstituteVariables(ctx, pattern) - resultRaw, err := json.Marshal(value) + if _, err := SubstituteVars(ctx, patternCopy); err != nil { + t.Error(err) + } + resultRaw, err := json.Marshal(patternCopy) if err != nil { t.Error(err) } - t.Log(string(resultRaw)) - t.Log(string(resultMap)) if !reflect.DeepEqual(resultMap, resultRaw) { + t.Log(string(resultRaw)) + t.Log(string(resultMap)) t.Error("result does not match") } } @@ -383,14 +415,17 @@ func Test_variableSubstitutionValueFail(t *testing.T) { } } `) - resultMap := []byte(`{"spec":{"name":null}}`) - var pattern, resource interface{} + var pattern, patternCopy, resource interface{} var err error err = json.Unmarshal(patternMap, &pattern) if err != nil { t.Error(err) } + err = json.Unmarshal(patternMap, &patternCopy) + if err != nil { + t.Error(err) + } err = json.Unmarshal(resourceRaw, &resource) if err != nil { @@ -404,16 +439,11 @@ func Test_variableSubstitutionValueFail(t *testing.T) { t.Error(err) } - value := SubstituteVariables(ctx, pattern) - resultRaw, err := json.Marshal(value) - if err != nil { - t.Error(err) - } - t.Log(string(resultRaw)) - t.Log(string(resultMap)) - if !reflect.DeepEqual(resultMap, resultRaw) { - t.Error("result does not match") + if _, err := SubstituteVars(ctx, patternCopy); err == nil { + t.Log("expected to fails") + t.Fail() } + } func Test_variableSubstitutionObject(t *testing.T) { @@ -444,12 +474,16 @@ func Test_variableSubstitutionObject(t *testing.T) { `) resultMap := []byte(`{"spec":{"variable":{"var1":"temp1","var2":"temp2","varNested":{"var1":"temp1"}}}}`) - var pattern, resource interface{} + var pattern, patternCopy, resource interface{} var err error err = json.Unmarshal(patternMap, &pattern) if err != nil { t.Error(err) } + err = json.Unmarshal(patternMap, &patternCopy) + if err != nil { + t.Error(err) + } err = json.Unmarshal(resourceRaw, &resource) if err != nil { @@ -463,14 +497,16 @@ func Test_variableSubstitutionObject(t *testing.T) { t.Error(err) } - value := SubstituteVariables(ctx, pattern) - resultRaw, err := json.Marshal(value) + if _, err := SubstituteVars(ctx, patternCopy); err != nil { + t.Error(err) + } + resultRaw, err := json.Marshal(patternCopy) if err != nil { t.Error(err) } - t.Log(string(resultRaw)) - t.Log(string(resultMap)) if !reflect.DeepEqual(resultMap, resultRaw) { + t.Log(string(resultRaw)) + t.Log(string(resultMap)) t.Error("result does not match") } } @@ -504,12 +540,16 @@ func Test_variableSubstitutionObjectOperatorNotEqualFail(t *testing.T) { resultMap := []byte(`{"spec":{"variable":null}}`) - var pattern, resource interface{} + var pattern, patternCopy, resource interface{} var err error err = json.Unmarshal(patternMap, &pattern) if err != nil { t.Error(err) } + err = json.Unmarshal(patternMap, &patternCopy) + if err != nil { + t.Error(err) + } err = json.Unmarshal(resourceRaw, &resource) if err != nil { @@ -523,14 +563,16 @@ func Test_variableSubstitutionObjectOperatorNotEqualFail(t *testing.T) { t.Error(err) } - value := SubstituteVariables(ctx, pattern) - resultRaw, err := json.Marshal(value) + if _, err := SubstituteVars(ctx, patternCopy); err != nil { + t.Error(err) + } + resultRaw, err := json.Marshal(patternCopy) if err != nil { t.Error(err) } - t.Log(string(resultRaw)) - t.Log(string(resultMap)) if !reflect.DeepEqual(resultMap, resultRaw) { + t.Log(string(resultRaw)) + t.Log(string(resultMap)) t.Error("result does not match") } } @@ -565,12 +607,16 @@ func Test_variableSubstitutionMultipleObject(t *testing.T) { resultMap := []byte(`{"spec":{"var":"temp1","variable":{"var1":"temp1","var2":"temp2","varNested":{"var1":"temp1"}}}}`) - var pattern, resource interface{} + var pattern, patternCopy, resource interface{} var err error err = json.Unmarshal(patternMap, &pattern) if err != nil { t.Error(err) } + err = json.Unmarshal(patternMap, &patternCopy) + if err != nil { + t.Error(err) + } err = json.Unmarshal(resourceRaw, &resource) if err != nil { @@ -584,14 +630,16 @@ func Test_variableSubstitutionMultipleObject(t *testing.T) { t.Error(err) } - value := SubstituteVariables(ctx, pattern) - resultRaw, err := json.Marshal(value) + if _, err := SubstituteVars(ctx, patternCopy); err != nil { + t.Error(err) + } + resultRaw, err := json.Marshal(patternCopy) if err != nil { t.Error(err) } - t.Log(string(resultRaw)) - t.Log(string(resultMap)) if !reflect.DeepEqual(resultMap, resultRaw) { + t.Log(string(resultRaw)) + t.Log(string(resultMap)) t.Error("result does not match") } } diff --git a/pkg/engine/variables/vars.go b/pkg/engine/variables/vars.go index 36e1b3bf6f..eb7d687e5a 100644 --- a/pkg/engine/variables/vars.go +++ b/pkg/engine/variables/vars.go @@ -11,16 +11,19 @@ import ( "github.com/nirmata/kyverno/pkg/engine/operator" ) +const variableRegex = `\{\{([^{}]*)\}\}` + //SubstituteVars replaces the variables with the values defined in the context // - if any variable is invaid or has nil value, it is considered as a failed varable substitution -func SubstituteVars(ctx context.EvalInterface, pattern interface{}) error { +func SubstituteVars(ctx context.EvalInterface, pattern interface{}) (interface{}, error) { + println(&pattern) errs := []error{} - subVars(ctx, pattern, "", &errs) + pattern = subVars(ctx, pattern, "", &errs) if len(errs) == 0 { // no error while parsing the pattern - return nil + return pattern, nil } - return fmt.Errorf("variable(s) not found or has nil values: %v", errs) + return pattern, fmt.Errorf("variable(s) not found or has nil values: %v", errs) } func subVars(ctx context.EvalInterface, pattern interface{}, path string, errs *[]error) interface{} { @@ -55,9 +58,16 @@ func subArray(ctx context.EvalInterface, patternList []interface{}, path string, return patternList } -func subVal(ctx context.EvalInterface, valuePattern string, path string, errs *[]error) interface{} { - operatorVariable := getOp(valuePattern) - variable := valuePattern[len(operatorVariable):] +func subVal(ctx context.EvalInterface, valuePattern interface{}, path string, errs *[]error) interface{} { + var emptyInterface interface{} + valueStr, ok := valuePattern.(string) + if !ok { + glog.Infof("failed to convert %v to string", valuePattern) + return emptyInterface + } + + operatorVariable := getOp(valueStr) + variable := valueStr[len(operatorVariable):] // substitute variable with value value, failedVars := getValQuery(ctx, variable) // if there are failedVars at this level @@ -70,15 +80,17 @@ func subVal(ctx context.EvalInterface, valuePattern string, path string, errs *[ // default or operator.Equal // equal + string value // object variable + valuePattern = value return value } // operator + string variable switch value.(type) { case string: + valuePattern = string(operatorVariable) + value.(string) return string(operatorVariable) + value.(string) default: glog.Infof("cannot use operator with object variables. operator used %s in pattern %v", string(operatorVariable), valuePattern) - var emptyInterface interface{} + valuePattern = emptyInterface return emptyInterface } diff --git a/pkg/engine/variables/vars_test.go b/pkg/engine/variables/vars_test.go index 92ac0ac4a7..2d8e1d92d2 100644 --- a/pkg/engine/variables/vars_test.go +++ b/pkg/engine/variables/vars_test.go @@ -63,7 +63,7 @@ func Test_subVars_success(t *testing.T) { t.Error(err) } - if err := SubstituteVars(ctx, pattern); err != nil { + if _, err := SubstituteVars(ctx, pattern); err != nil { t.Error(err) } } @@ -124,7 +124,7 @@ func Test_subVars_failed(t *testing.T) { t.Error(err) } - if err := SubstituteVars(ctx, pattern); err == nil { + if _, err := SubstituteVars(ctx, pattern); err == nil { t.Error("error is expected") } } diff --git a/pkg/generate/generate.go b/pkg/generate/generate.go index 1d2da296c0..06346cb042 100644 --- a/pkg/generate/generate.go +++ b/pkg/generate/generate.go @@ -126,22 +126,19 @@ func applyRule(client *dclient.Client, rule kyverno.Rule, resource unstructured. var err error var mode ResourceMode var noGenResource kyverno.ResourceSpec - // convert to unstructured Resource genUnst, err := getUnstrRule(rule.Generation.DeepCopy()) if err != nil { return noGenResource, err } - glog.V(4).Info("applyRule1 %v", genUnst) // Variable substitutions // format : {{ results in error and rule is not applied // - valid variables are replaced with the values - if err := variables.SubstituteVars(ctx, genUnst.Object); err != nil { + if _, err := variables.SubstituteVars(ctx, genUnst.Object); err != nil { return noGenResource, err } - glog.V(4).Info("applyRule2 %v", genUnst.Object) genKind, _, err := unstructured.NestedString(genUnst.Object, "kind") if err != nil { return noGenResource, err @@ -175,6 +172,10 @@ func applyRule(client *dclient.Client, rule kyverno.Rule, resource unstructured. } else { rdata, mode, err = manageClone(genKind, genNamespace, genName, genCopy, client, resource) } + if err != nil { + return noGenResource, err + } + if rdata == nil { // existing resource contains the configuration return newGenResource, nil @@ -265,7 +266,6 @@ func manageClone(kind, namespace, name string, clone map[string]interface{}, cli return nil, Skip, err } newRName, _, err := unstructured.NestedString(clone, "name") - if err != nil { return nil, Skip, err } @@ -275,10 +275,11 @@ func manageClone(kind, namespace, name string, clone map[string]interface{}, cli return nil, Skip, nil } + glog.V(4).Infof("check if resource %s/%s/%s exists", kind, newRNs, newRName) // check if the resource as reference in clone exists? obj, err := client.GetResource(kind, newRNs, newRName) if err != nil { - return nil, Skip, err + return nil, Skip, fmt.Errorf("reference clone resource %s/%s/%s not found. %v", kind, newRNs, newRName, err) } // create the resource based on the reference clone return obj.UnstructuredContent(), Create, nil @@ -300,7 +301,7 @@ const ( func checkResource(newResourceSpec interface{}, resource *unstructured.Unstructured) error { // check if the resource spec if a subset of the resource if path, err := validate.ValidateResourceWithPattern1(resource.Object, newResourceSpec); err != nil { - glog.V(4).Info("Failed to match the resource at path %s: err", path, err) + glog.V(4).Infof("Failed to match the resource at path %s: err %v", path, err) return err } return nil From 4f830acb6564156423affba368308984f965ebd3 Mon Sep 17 00:00:00 2001 From: shivkumar dudhani Date: Fri, 14 Feb 2020 12:05:13 -0800 Subject: [PATCH 07/12] refactor --- pkg/engine/validate/validate.go | 4 +- pkg/engine/validation.go | 4 +- pkg/engine/variables/validatevariables.go | 82 ----------------------- pkg/generate/generate.go | 2 +- 4 files changed, 5 insertions(+), 87 deletions(-) delete mode 100644 pkg/engine/variables/validatevariables.go diff --git a/pkg/engine/validate/validate.go b/pkg/engine/validate/validate.go index 84bdd674bf..9d94f07789 100644 --- a/pkg/engine/validate/validate.go +++ b/pkg/engine/validate/validate.go @@ -13,9 +13,9 @@ import ( "github.com/nirmata/kyverno/pkg/engine/operator" ) -// ValidateResourceWithPattern1 is a start of element-by-element validation process +// ValidateResourceWithPattern is a start of element-by-element validation process // It assumes that validation is started from root, so "/" is passed -func ValidateResourceWithPattern1(resource, pattern interface{}) (string, error) { +func ValidateResourceWithPattern(resource, pattern interface{}) (string, error) { path, err := validateResourceElement(resource, pattern, pattern, "/") if err != nil { return path, err diff --git a/pkg/engine/validation.go b/pkg/engine/validation.go index deb0f8e609..e8f6972790 100644 --- a/pkg/engine/validation.go +++ b/pkg/engine/validation.go @@ -192,7 +192,7 @@ func validatePatterns(ctx context.EvalInterface, resource unstructured.Unstructu return resp } - if path, err := validate.ValidateResourceWithPattern1(resource.Object, pattern); err != nil { + if path, err := validate.ValidateResourceWithPattern(resource.Object, pattern); err != nil { // validation failed resp.Success = false resp.Message = fmt.Sprintf("Validation error: %s; Validation rule '%s' failed at path '%s'", @@ -216,7 +216,7 @@ func validatePatterns(ctx context.EvalInterface, resource unstructured.Unstructu failedSubstitutionsErrors = append(failedSubstitutionsErrors, err) continue } - _, err := validate.ValidateResourceWithPattern1(resource.Object, pattern) + _, err := validate.ValidateResourceWithPattern(resource.Object, pattern) if err == nil { resp.Success = true resp.Message = fmt.Sprintf("Validation rule '%s' anyPattern[%d] succeeded.", rule.Name, idx) diff --git a/pkg/engine/variables/validatevariables.go b/pkg/engine/variables/validatevariables.go deleted file mode 100644 index 478564d708..0000000000 --- a/pkg/engine/variables/validatevariables.go +++ /dev/null @@ -1,82 +0,0 @@ -package variables - -// // ValidateVariables validates if referenced path is present -// // return empty string if all paths are valid, otherwise return invalid path -// func ValidateVariables(ctx context.EvalInterface, pattern interface{}) string { -// var pathsNotPresent []string -// variableList := extractVariables(pattern) -// for _, variable := range variableList { -// if len(variable) == 2 { -// varName := variable[0] -// varValue := variable[1] -// glog.V(3).Infof("validating variable %s", varName) -// val, err := ctx.Query(varValue) -// if err == nil && val == nil { -// // path is not present, returns nil interface -// pathsNotPresent = append(pathsNotPresent, varValue) -// } -// } -// } - -// if len(pathsNotPresent) != 0 { -// return strings.Join(pathsNotPresent, ";") -// } -// return "" -// } - -// // extractVariables extracts variables in the pattern -// func extractVariables(pattern interface{}) [][]string { -// switch typedPattern := pattern.(type) { -// case map[string]interface{}: -// return extractMap(typedPattern) -// case []interface{}: -// return extractArray(typedPattern) -// case string: -// return extractValue(typedPattern) -// default: -// fmt.Printf("variable type %T", typedPattern) -// return nil -// } -// } - -// func extractMap(patternMap map[string]interface{}) [][]string { -// var variableList [][]string - -// for _, patternElement := range patternMap { -// if vars := extractVariables(patternElement); vars != nil { -// variableList = append(variableList, vars...) -// } -// } -// return variableList -// } - -// func extractArray(patternList []interface{}) [][]string { -// var variableList [][]string - -// for _, patternElement := range patternList { -// if vars := extractVariables(patternElement); vars != nil { -// variableList = append(variableList, vars...) -// } -// } -// return variableList -// } - -// func extractValue(valuePattern string) [][]string { -// operatorVariable := getOperator(valuePattern) -// variable := valuePattern[len(operatorVariable):] -// return extractValueVariable(variable) -// } - -// // returns multiple variable match groups -// func extractValueVariable(valuePattern string) [][]string { -// variableRegex := regexp.MustCompile(variableRegex) -// groups := variableRegex.FindAllStringSubmatch(valuePattern, -1) -// if len(groups) == 0 { -// // no variables -// return nil -// } -// // group[*] <- all the matches -// // group[*][0] <- group match -// // group[*][1] <- group capture value -// return groups -// } diff --git a/pkg/generate/generate.go b/pkg/generate/generate.go index 06346cb042..cd9ab1b2fd 100644 --- a/pkg/generate/generate.go +++ b/pkg/generate/generate.go @@ -300,7 +300,7 @@ const ( func checkResource(newResourceSpec interface{}, resource *unstructured.Unstructured) error { // check if the resource spec if a subset of the resource - if path, err := validate.ValidateResourceWithPattern1(resource.Object, newResourceSpec); err != nil { + if path, err := validate.ValidateResourceWithPattern(resource.Object, newResourceSpec); err != nil { glog.V(4).Infof("Failed to match the resource at path %s: err %v", path, err) return err } From 3fbfa251adcbe69b658575f2c1bafd2075af8090 Mon Sep 17 00:00:00 2001 From: shivkumar dudhani Date: Fri, 14 Feb 2020 12:16:02 -0800 Subject: [PATCH 08/12] refactor --- pkg/engine/variables/variables_check.go | 80 ------------------------- 1 file changed, 80 deletions(-) delete mode 100644 pkg/engine/variables/variables_check.go diff --git a/pkg/engine/variables/variables_check.go b/pkg/engine/variables/variables_check.go deleted file mode 100644 index ba4f4a5c38..0000000000 --- a/pkg/engine/variables/variables_check.go +++ /dev/null @@ -1,80 +0,0 @@ -package variables - -// import ( -// "fmt" -// "regexp" -// "strconv" -// ) - -// //CheckVariables checks if the variable regex has been used -// func CheckVariables(pattern interface{}, variables []string, path string) error { -// switch typedPattern := pattern.(type) { -// case map[string]interface{}: -// return checkMap(typedPattern, variables, path) -// case []interface{}: -// return checkArray(typedPattern, variables, path) -// case string: -// return checkValue(typedPattern, variables, path) -// default: -// return nil -// } -// } - -// func checkMap(patternMap map[string]interface{}, variables []string, path string) error { -// for patternKey, patternElement := range patternMap { - -// if err := CheckVariables(patternElement, variables, path+patternKey+"/"); err != nil { -// return err -// } -// } -// return nil -// } - -// func checkArray(patternList []interface{}, variables []string, path string) error { -// for idx, patternElement := range patternList { -// if err := CheckVariables(patternElement, variables, path+strconv.Itoa(idx)+"/"); err != nil { -// return err -// } -// } -// return nil -// } - -// func checkValue(valuePattern string, variables []string, path string) error { -// operatorVariable := getOperator(valuePattern) -// variable := valuePattern[len(operatorVariable):] -// if checkValueVariable(variable, variables) { -// return fmt.Errorf(path + valuePattern) -// } -// return nil -// } - -// func checkValueVariable(valuePattern string, variables []string) bool { -// variableRegex := regexp.MustCompile(variableRegex) -// groups := variableRegex.FindAllStringSubmatch(valuePattern, -1) -// if len(groups) == 0 { -// // no variables -// return false -// } -// // if variables are defined, check against the list of variables to be filtered -// for _, group := range groups { -// if len(group) == 2 { -// // group[0] -> {{variable}} -// // group[1] -> variable -// if variablePatternSearch(group[1], variables) { -// return true -// } -// } -// } -// return false -// } - -// func variablePatternSearch(pattern string, regexs []string) bool { -// for _, regex := range regexs { -// varRegex := regexp.MustCompile(regex) -// found := varRegex.FindString(pattern) -// if found != "" { -// return true -// } -// } -// return true -// } From eec0decae7366919e8c0b270b1e86943be274394 Mon Sep 17 00:00:00 2001 From: shivkumar dudhani Date: Fri, 14 Feb 2020 12:25:13 -0800 Subject: [PATCH 09/12] CR fixes --- pkg/engine/variables/vars.go | 3 --- pkg/policy/background.go | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/pkg/engine/variables/vars.go b/pkg/engine/variables/vars.go index eb7d687e5a..a9fd45e8db 100644 --- a/pkg/engine/variables/vars.go +++ b/pkg/engine/variables/vars.go @@ -80,17 +80,14 @@ func subVal(ctx context.EvalInterface, valuePattern interface{}, path string, er // default or operator.Equal // equal + string value // object variable - valuePattern = value return value } // operator + string variable switch value.(type) { case string: - valuePattern = string(operatorVariable) + value.(string) return string(operatorVariable) + value.(string) default: glog.Infof("cannot use operator with object variables. operator used %s in pattern %v", string(operatorVariable), valuePattern) - valuePattern = emptyInterface return emptyInterface } diff --git a/pkg/policy/background.go b/pkg/policy/background.go index c66b920c37..423fc51c4d 100644 --- a/pkg/policy/background.go +++ b/pkg/policy/background.go @@ -51,7 +51,7 @@ func ContainsUserInfo(policy kyverno.ClusterPolicy) error { return fmt.Errorf("userInfo variable used at spec/rules[%d]/validate/pattern", idx) } for idx2, pattern := range rule.Validation.AnyPattern { - if pattern, err = variables.SubstituteVars(ctx, pattern); err != nil { + if rule.Validation.AnyPattern[idx2], err = variables.SubstituteVars(ctx, pattern); err != nil { return fmt.Errorf("userInfo variable used at spec/rules[%d]/validate/anyPattern[%d]", idx, idx2) } } From a8e6de2fab2140b7b23d816ee85f58228aa432a2 Mon Sep 17 00:00:00 2001 From: shivkumar dudhani Date: Fri, 14 Feb 2020 12:29:17 -0800 Subject: [PATCH 10/12] CR fixes --- pkg/engine/variables/vars.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/engine/variables/vars.go b/pkg/engine/variables/vars.go index a9fd45e8db..868ed8f672 100644 --- a/pkg/engine/variables/vars.go +++ b/pkg/engine/variables/vars.go @@ -83,9 +83,9 @@ func subVal(ctx context.EvalInterface, valuePattern interface{}, path string, er return value } // operator + string variable - switch value.(type) { + switch typedValue := value.(type) { case string: - return string(operatorVariable) + value.(string) + return typedValue + value.(string) default: glog.Infof("cannot use operator with object variables. operator used %s in pattern %v", string(operatorVariable), valuePattern) return emptyInterface From 37d50999fb070c1e5bd1fc5bb65807c415c72573 Mon Sep 17 00:00:00 2001 From: shivkumar dudhani Date: Fri, 14 Feb 2020 12:41:57 -0800 Subject: [PATCH 11/12] fix bug --- pkg/engine/variables/vars.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/engine/variables/vars.go b/pkg/engine/variables/vars.go index 868ed8f672..920fcb93c9 100644 --- a/pkg/engine/variables/vars.go +++ b/pkg/engine/variables/vars.go @@ -85,7 +85,7 @@ func subVal(ctx context.EvalInterface, valuePattern interface{}, path string, er // operator + string variable switch typedValue := value.(type) { case string: - return typedValue + value.(string) + return string(operatorVariable) + typedValue default: glog.Infof("cannot use operator with object variables. operator used %s in pattern %v", string(operatorVariable), valuePattern) return emptyInterface From a5ce084da4b0f91c63be3df0f596774ec9ba7410 Mon Sep 17 00:00:00 2001 From: shivkumar dudhani Date: Tue, 18 Feb 2020 18:25:41 -0800 Subject: [PATCH 12/12] CR comments --- pkg/engine/response/response.go | 12 ------------ pkg/engine/validate/common.go | 23 ----------------------- pkg/policyviolation/builder.go | 6 +++--- pkg/policyviolation/builder_test.go | 27 ++++++++++++--------------- 4 files changed, 15 insertions(+), 53 deletions(-) diff --git a/pkg/engine/response/response.go b/pkg/engine/response/response.go index 1231daeade..d060fc23c3 100644 --- a/pkg/engine/response/response.go +++ b/pkg/engine/response/response.go @@ -65,8 +65,6 @@ type RuleResponse struct { Success bool `json:"success"` // statistics RuleStats `json:",inline"` - // PathNotPresent indicates whether referenced path in variable substitution exist - PathNotPresent bool `json:"pathNotPresent"` } //ToString ... @@ -121,13 +119,3 @@ func (er EngineResponse) getRules(success bool) []string { } return rules } - -// IsPathNotPresent checks if the referenced path(in variable substitution) exist -func (er EngineResponse) IsPathNotPresent() bool { - for _, r := range er.PolicyResponse.Rules { - if r.PathNotPresent { - return true - } - } - return false -} diff --git a/pkg/engine/validate/common.go b/pkg/engine/validate/common.go index d577fe1053..9009da7c03 100644 --- a/pkg/engine/validate/common.go +++ b/pkg/engine/validate/common.go @@ -5,18 +5,6 @@ import ( "strconv" ) -//ValidationFailureReason defeins type for Validation Failure reason -type ValidationFailureReason int - -const ( - // PathNotPresent if path is not present - PathNotPresent ValidationFailureReason = iota - // Rulefailure if the rule failed - Rulefailure - // OtherError if there is any other type of error - OtherError -) - // convertToString converts value to string func convertToString(value interface{}) (string, error) { switch typed := value.(type) { @@ -46,14 +34,3 @@ func getRawKeyIfWrappedWithAttributes(str string) string { return str } } - -//ValidationError stores error for validation error -type ValidationError struct { - StatusCode ValidationFailureReason - ErrorMsg string -} - -// newValidatePatternError returns an validation error using the ValidationFailureReason and errorMsg -func newValidatePatternError(reason ValidationFailureReason, msg string) ValidationError { - return ValidationError{StatusCode: reason, ErrorMsg: msg} -} diff --git a/pkg/policyviolation/builder.go b/pkg/policyviolation/builder.go index d7367c35bd..1de366eb66 100644 --- a/pkg/policyviolation/builder.go +++ b/pkg/policyviolation/builder.go @@ -16,8 +16,8 @@ func GeneratePVsFromEngineResponse(ers []response.EngineResponse) (pvInfos []Inf glog.V(4).Infof("resource %v, has not been assigned a name, not creating a policy violation for it", er.PolicyResponse.Resource) continue } - // skip when response succeed AND referenced paths exist - if er.IsSuccesful() && !er.IsPathNotPresent() { + // skip when response succeed + if er.IsSuccesful() { continue } glog.V(4).Infof("Building policy violation for engine response %v", er) @@ -82,7 +82,7 @@ func buildPVInfo(er response.EngineResponse) Info { func buildViolatedRules(er response.EngineResponse) []kyverno.ViolatedRule { var violatedRules []kyverno.ViolatedRule for _, rule := range er.PolicyResponse.Rules { - if rule.Success && !rule.PathNotPresent { + if rule.Success { continue } vrule := kyverno.ViolatedRule{ diff --git a/pkg/policyviolation/builder_test.go b/pkg/policyviolation/builder_test.go index a445281844..bd3f8c97f0 100644 --- a/pkg/policyviolation/builder_test.go +++ b/pkg/policyviolation/builder_test.go @@ -19,17 +19,15 @@ func Test_GeneratePVsFromEngineResponse_PathNotExist(t *testing.T) { }, Rules: []response.RuleResponse{ { - Name: "test-path-not-exist", - Type: "Mutation", - Message: "referenced paths are not present: request.object.metadata.name1", - Success: true, - PathNotPresent: true, + Name: "test-path-not-exist", + Type: "Mutation", + Message: "referenced paths are not present: request.object.metadata.name1", + Success: false, }, { - Name: "test-path-exist", - Type: "Mutation", - Success: true, - PathNotPresent: false, + Name: "test-path-exist", + Type: "Mutation", + Success: true, }, }, }, @@ -44,11 +42,10 @@ func Test_GeneratePVsFromEngineResponse_PathNotExist(t *testing.T) { }, Rules: []response.RuleResponse{ { - Name: "test-path-not-exist-across-policy", - Type: "Mutation", - Message: "referenced paths are not present: request.object.metadata.name1", - Success: true, - PathNotPresent: true, + Name: "test-path-not-exist-across-policy", + Type: "Mutation", + Message: "referenced paths are not present: request.object.metadata.name1", + Success: true, }, }, }, @@ -56,5 +53,5 @@ func Test_GeneratePVsFromEngineResponse_PathNotExist(t *testing.T) { } pvInfos := GeneratePVsFromEngineResponse(ers) - assert.Assert(t, len(pvInfos) == 2) + assert.Assert(t, len(pvInfos) == 1) }