diff --git a/pkg/engine/mutation_test.go b/pkg/engine/mutation_test.go index fa29ae043a..2faa6c1e4d 100644 --- a/pkg/engine/mutation_test.go +++ b/pkg/engine/mutation_test.go @@ -665,3 +665,107 @@ func Test_foreach(t *testing.T) { } } } + +func Test_foreach_element_mutation(t *testing.T) { + policyRaw := []byte(`{ + "apiVersion": "kyverno.io/v1", + "kind": "ClusterPolicy", + "metadata": { + "name": "mutate-privileged" + }, + "spec": { + "validationFailureAction": "audit", + "background": false, + "webhookTimeoutSeconds": 10, + "failurePolicy": "Fail", + "rules": [ + { + "name": "set-privileged", + "match": { + "resources": { + "kinds": [ + "Pod" + ] + } + }, + "mutate": { + "foreach": [ + { + "list": "request.object.spec.containers", + "patchStrategicMerge": { + "spec": { + "containers": [ + { + "(name)": "{{ element.name }}", + "securityContext": { + "privileged": false + } + } + ] + } + } + } + ] + } + } + ] + } +}`) + resourceRaw := []byte(`{ + "apiVersion": "v1", + "kind": "Pod", + "metadata": { + "name": "nginx" + }, + "spec": { + "containers": [ + { + "name": "nginx1", + "image": "nginx" + }, + { + "name": "nginx2", + "image": "nginx" + } + ] + } +}`) + var policy kyverno.ClusterPolicy + err := json.Unmarshal(policyRaw, &policy) + assert.NilError(t, err) + + resource, err := utils.ConvertToUnstructured(resourceRaw) + assert.NilError(t, err) + + ctx := context.NewContext() + err = ctx.AddResourceAsObject(resource.Object) + assert.NilError(t, err) + + policyContext := &PolicyContext{ + Policy: policy, + JSONContext: ctx, + NewResource: *resource, + } + + err = ctx.AddImageInfo(resource) + assert.NilError(t, err) + + err = context.MutateResourceWithImageInfo(resourceRaw, ctx) + assert.NilError(t, err) + + er := Mutate(policyContext) + + assert.Equal(t, len(er.PolicyResponse.Rules), 1) + assert.Equal(t, er.PolicyResponse.Rules[0].Status, response.RuleStatusPass) + + containers, _, err := unstructured.NestedSlice(er.PatchedResource.Object, "spec", "containers") + assert.NilError(t, err) + for _, c := range containers { + ctnr := c.(map[string]interface{}) + _securityContext, ok := ctnr["securityContext"] + assert.Assert(t, ok) + + securityContext := _securityContext.(map[string]interface{}) + assert.Equal(t, securityContext["privileged"], false) + } +} diff --git a/pkg/engine/validation_test.go b/pkg/engine/validation_test.go index 81159bbc05..e70fe4364c 100644 --- a/pkg/engine/validation_test.go +++ b/pkg/engine/validation_test.go @@ -2843,6 +2843,97 @@ func Test_foreach_context_preconditions_fail(t *testing.T) { testForEach(t, policyraw, resourceRaw, "", response.RuleStatusFail) } +func Test_foreach_element_validation(t *testing.T) { + + resourceRaw := []byte(`{ + "apiVersion": "v1", + "kind": "Pod", + "metadata": { + "name": "nginx" + }, + "spec": { + "containers": [ + { + "name": "nginx1", + "image": "nginx" + }, + { + "name": "nginx2", + "image": "nginx" + } + ] + } + }`) + + policyraw := []byte(`{ + "apiVersion": "kyverno.io/v1", + "kind": "ClusterPolicy", + "metadata": {"name": "check-container-names"}, + "spec": { + "validationFailureAction": "enforce", + "background": false, + "rules": [ + { + "name": "test", + "match": {"resources": { "kinds": [ "Pod" ] } }, + "validate": { + "message": "Invalid name", + "foreach": [ + { + "list": "request.object.spec.containers", + "pattern": { + "name": "{{ element.name }}" + } + } + ] + }}]}}`) + + testForEach(t, policyraw, resourceRaw, "", response.RuleStatusPass) +} + +func Test_outof_foreach_element_validation(t *testing.T) { + + resourceRaw := []byte(`{ + "apiVersion": "v1", + "kind": "Pod", + "metadata": { + "name": "nginx" + }, + "spec": { + "containers": [ + { + "name": "nginx1", + "image": "nginx" + }, + { + "name": "nginx2", + "image": "nginx" + } + ] + } + }`) + + policyraw := []byte(`{ + "apiVersion": "kyverno.io/v1", + "kind": "ClusterPolicy", + "metadata": {"name": "check-container-names"}, + "spec": { + "validationFailureAction": "enforce", + "background": false, + "rules": [ + { + "name": "test", + "match": {"resources": { "kinds": [ "Pod" ] } }, + "validate": { + "message": "Invalid name", + "pattern": { + "name": "{{ element.name }}" + } + }}]}}`) + + testForEach(t, policyraw, resourceRaw, "", response.RuleStatusError) +} + func testForEach(t *testing.T, policyraw []byte, resourceRaw []byte, msg string, status response.RuleStatus) { var policy kyverno.ClusterPolicy assert.NilError(t, json.Unmarshal(policyraw, &policy)) diff --git a/pkg/engine/variables/vars.go b/pkg/engine/variables/vars.go index 4e0414d418..ba2dd18840 100644 --- a/pkg/engine/variables/vars.go +++ b/pkg/engine/variables/vars.go @@ -203,6 +203,10 @@ func ValidateBackgroundModeVars(log logr.Logger, ctx context.EvalInterface, rule return jsonUtils.NewTraversal(rule, validateBackgroundModeVars(log, ctx)).TraverseJSON() } +func ValidateElementInForEach(log logr.Logger, rule interface{}) (interface{}, error) { + return jsonUtils.NewTraversal(rule, validateElementInForEach(log)).TraverseJSON() +} + func validateBackgroundModeVars(log logr.Logger, ctx context.EvalInterface) jsonUtils.Action { return jsonUtils.OnlyForLeafsAndKeys(func(data *jsonUtils.ActionData) (interface{}, error) { value, ok := data.Element.(string) @@ -229,6 +233,24 @@ func validateBackgroundModeVars(log logr.Logger, ctx context.EvalInterface) json }) } +func validateElementInForEach(log logr.Logger) jsonUtils.Action { + return jsonUtils.OnlyForLeafsAndKeys(func(data *jsonUtils.ActionData) (interface{}, error) { + value, ok := data.Element.(string) + if !ok { + return data.Element, nil + } + vars := RegexVariables.FindAllString(value, -1) + for _, v := range vars { + variable := replaceBracesAndTrimSpaces(v) + + if strings.HasPrefix(variable, "element") && !strings.Contains(data.Path, "/foreach/") { + return nil, fmt.Errorf("variable '%v' present outside of foreach at path %s", variable, data.Path) + } + } + return nil, nil + }) +} + // NotResolvedReferenceErr is returned when it is impossible to resolve the variable type NotResolvedReferenceErr struct { reference string diff --git a/pkg/kyverno/apply/apply_command.go b/pkg/kyverno/apply/apply_command.go index c122a6a9c6..66fa6c5dca 100644 --- a/pkg/kyverno/apply/apply_command.go +++ b/pkg/kyverno/apply/apply_command.go @@ -36,6 +36,11 @@ type Values struct { Policies []Policy `json:"policies"` } +type SkippedInvalidPolicies struct { + skipped []string + invalid []string +} + var applyHelp = ` To apply on a resource: kyverno apply /path/to/policy.yaml /path/to/folderOfPolicies --resource=/path/to/resource1 --resource=/path/to/resource2 @@ -117,12 +122,12 @@ func Command() *cobra.Command { } }() - rc, resources, skippedPolicies, pvInfos, err := applyCommandHelper(resourcePaths, cluster, policyReport, mutateLogPath, variablesString, valuesFile, namespace, policyPaths, stdin) + rc, resources, skipInvalidPolicies, pvInfos, err := applyCommandHelper(resourcePaths, cluster, policyReport, mutateLogPath, variablesString, valuesFile, namespace, policyPaths, stdin) if err != nil { return err } - printReportOrViolation(policyReport, rc, resourcePaths, len(resources), skippedPolicies, stdin, pvInfos) + printReportOrViolation(policyReport, rc, resourcePaths, len(resources), skipInvalidPolicies, stdin, pvInfos) return nil }, } @@ -140,47 +145,47 @@ func Command() *cobra.Command { } func applyCommandHelper(resourcePaths []string, cluster bool, policyReport bool, mutateLogPath string, - variablesString string, valuesFile string, namespace string, policyPaths []string, stdin bool) (rc *common.ResultCounts, resources []*unstructured.Unstructured, skippedPolicies []string, pvInfos []policyreport.Info, err error) { + variablesString string, valuesFile string, namespace string, policyPaths []string, stdin bool) (rc *common.ResultCounts, resources []*unstructured.Unstructured, skipInvalidPolicies SkippedInvalidPolicies, pvInfos []policyreport.Info, err error) { store.SetMock(true) kubernetesConfig := genericclioptions.NewConfigFlags(true) fs := memfs.New() if valuesFile != "" && variablesString != "" { - return rc, resources, skippedPolicies, pvInfos, sanitizederror.NewWithError("pass the values either using set flag or values_file flag", err) + return rc, resources, skipInvalidPolicies, pvInfos, sanitizederror.NewWithError("pass the values either using set flag or values_file flag", err) } variables, globalValMap, valuesMap, namespaceSelectorMap, err := common.GetVariable(variablesString, valuesFile, fs, false, "") if err != nil { if !sanitizederror.IsErrorSanitized(err) { - return rc, resources, skippedPolicies, pvInfos, sanitizederror.NewWithError("failed to decode yaml", err) + return rc, resources, skipInvalidPolicies, pvInfos, sanitizederror.NewWithError("failed to decode yaml", err) } - return rc, resources, skippedPolicies, pvInfos, err + return rc, resources, skipInvalidPolicies, pvInfos, err } openAPIController, err := openapi.NewOpenAPIController() if err != nil { - return rc, resources, skippedPolicies, pvInfos, sanitizederror.NewWithError("failed to initialize openAPIController", err) + return rc, resources, skipInvalidPolicies, pvInfos, sanitizederror.NewWithError("failed to initialize openAPIController", err) } var dClient *client.Client if cluster { restConfig, err := kubernetesConfig.ToRESTConfig() if err != nil { - return rc, resources, skippedPolicies, pvInfos, err + return rc, resources, skipInvalidPolicies, pvInfos, err } dClient, err = client.NewClient(restConfig, 15*time.Minute, make(chan struct{}), log.Log) if err != nil { - return rc, resources, skippedPolicies, pvInfos, err + return rc, resources, skipInvalidPolicies, pvInfos, err } } if len(policyPaths) == 0 { - return rc, resources, skippedPolicies, pvInfos, sanitizederror.NewWithError(fmt.Sprintf("require policy"), err) + return rc, resources, skipInvalidPolicies, pvInfos, sanitizederror.NewWithError("require policy", err) } if (len(policyPaths) > 0 && policyPaths[0] == "-") && len(resourcePaths) > 0 && resourcePaths[0] == "-" { - return rc, resources, skippedPolicies, pvInfos, sanitizederror.NewWithError("a stdin pipe can be used for either policies or resources, not both", err) + return rc, resources, skipInvalidPolicies, pvInfos, sanitizederror.NewWithError("a stdin pipe can be used for either policies or resources, not both", err) } policies, err := common.GetPoliciesFromPaths(fs, policyPaths, false, "") @@ -190,15 +195,15 @@ func applyCommandHelper(resourcePaths []string, cluster bool, policyReport bool, } if len(resourcePaths) == 0 && !cluster { - return rc, resources, skippedPolicies, pvInfos, sanitizederror.NewWithError(fmt.Sprintf("resource file(s) or cluster required"), err) + return rc, resources, skipInvalidPolicies, pvInfos, sanitizederror.NewWithError("resource file(s) or cluster required", err) } mutateLogPathIsDir, err := checkMutateLogPath(mutateLogPath) if err != nil { if !sanitizederror.IsErrorSanitized(err) { - return rc, resources, skippedPolicies, pvInfos, sanitizederror.NewWithError("failed to create file/folder", err) + return rc, resources, skipInvalidPolicies, pvInfos, sanitizederror.NewWithError("failed to create file/folder", err) } - return rc, resources, skippedPolicies, pvInfos, err + return rc, resources, skipInvalidPolicies, pvInfos, err } // empty the previous contents of the file just in case if the file already existed before with some content(so as to perform overwrites) @@ -210,22 +215,22 @@ func applyCommandHelper(resourcePaths []string, cluster bool, policyReport bool, if err != nil { if !sanitizederror.IsErrorSanitized(err) { - return rc, resources, skippedPolicies, pvInfos, sanitizederror.NewWithError("failed to truncate the existing file at "+mutateLogPath, err) + return rc, resources, skipInvalidPolicies, pvInfos, sanitizederror.NewWithError("failed to truncate the existing file at "+mutateLogPath, err) } - return rc, resources, skippedPolicies, pvInfos, err + return rc, resources, skipInvalidPolicies, pvInfos, err } } mutatedPolicies, err := common.MutatePolices(policies) if err != nil { if !sanitizederror.IsErrorSanitized(err) { - return rc, resources, skippedPolicies, pvInfos, sanitizederror.NewWithError("failed to mutate policy", err) + return rc, resources, skipInvalidPolicies, pvInfos, sanitizederror.NewWithError("failed to mutate policy", err) } } err = common.PrintMutatedPolicy(mutatedPolicies) if err != nil { - return rc, resources, skippedPolicies, pvInfos, sanitizederror.NewWithError("failed to marsal mutated policy", err) + return rc, resources, skipInvalidPolicies, pvInfos, sanitizederror.NewWithError("failed to marsal mutated policy", err) } resources, err = common.GetResourceAccordingToResourcePath(fs, resourcePaths, cluster, mutatedPolicies, dClient, namespace, policyReport, false, "") @@ -235,7 +240,7 @@ func applyCommandHelper(resourcePaths []string, cluster bool, policyReport bool, } if (len(resources) > 1 || len(mutatedPolicies) > 1) && variablesString != "" { - return rc, resources, skippedPolicies, pvInfos, sanitizederror.NewWithError("currently `set` flag supports variable for single policy applied on single resource ", nil) + return rc, resources, skipInvalidPolicies, pvInfos, sanitizederror.NewWithError("currently `set` flag supports variable for single policy applied on single resource ", nil) } if variablesString != "" { @@ -259,12 +264,19 @@ func applyCommandHelper(resourcePaths []string, cluster bool, policyReport bool, } rc = &common.ResultCounts{} - skippedPolicies = make([]string, 0) + skipInvalidPolicies.skipped = make([]string, 0) + skipInvalidPolicies.invalid = make([]string, 0) for _, policy := range mutatedPolicies { err := policy2.Validate(policy, nil, true, openAPIController) if err != nil { - skippedPolicies = append(skippedPolicies, policy.Name) + log.Log.V(4).Info(err.Error()) + + if strings.HasPrefix(err.Error(), "variable 'element.name'") { + skipInvalidPolicies.invalid = append(skipInvalidPolicies.invalid, policy.Name) + } else { + skipInvalidPolicies.skipped = append(skipInvalidPolicies.skipped, policy.Name) + } continue } @@ -274,7 +286,7 @@ func applyCommandHelper(resourcePaths []string, cluster bool, policyReport bool, if len(variables) == 0 { // check policy in variable file if valuesFile == "" || valuesMap[policy.Name] == nil { - skippedPolicies = append(skippedPolicies, policy.Name) + skipInvalidPolicies.skipped = append(skipInvalidPolicies.skipped, policy.Name) continue } } @@ -285,19 +297,19 @@ func applyCommandHelper(resourcePaths []string, cluster bool, policyReport bool, for _, resource := range resources { thisPolicyResourceValues, err := common.CheckVariableForPolicy(valuesMap, globalValMap, policy.GetName(), resource.GetName(), resource.GetKind(), variables, kindOnwhichPolicyIsApplied, variable) if err != nil { - return rc, resources, skippedPolicies, pvInfos, sanitizederror.NewWithError(fmt.Sprintf("policy `%s` have variables. pass the values for the variables for resource `%s` using set/values_file flag", policy.Name, resource.GetName()), err) + return rc, resources, skipInvalidPolicies, pvInfos, sanitizederror.NewWithError(fmt.Sprintf("policy `%s` have variables. pass the values for the variables for resource `%s` using set/values_file flag", policy.Name, resource.GetName()), err) } _, info, err := common.ApplyPolicyOnResource(policy, resource, mutateLogPath, mutateLogPathIsDir, thisPolicyResourceValues, policyReport, namespaceSelectorMap, stdin, rc, true) if err != nil { - return rc, resources, skippedPolicies, pvInfos, sanitizederror.NewWithError(fmt.Errorf("failed to apply policy %v on resource %v", policy.Name, resource.GetName()).Error(), err) + return rc, resources, skipInvalidPolicies, pvInfos, sanitizederror.NewWithError(fmt.Errorf("failed to apply policy %v on resource %v", policy.Name, resource.GetName()).Error(), err) } pvInfos = append(pvInfos, info) } } - return rc, resources, skippedPolicies, pvInfos, nil + return rc, resources, skipInvalidPolicies, pvInfos, nil } // checkMutateLogPath - checking path for printing mutated resource (-o flag) @@ -323,10 +335,17 @@ func checkMutateLogPath(mutateLogPath string) (mutateLogPathIsDir bool, err erro } // printReportOrViolation - printing policy report/violations -func printReportOrViolation(policyReport bool, rc *common.ResultCounts, resourcePaths []string, resourcesLen int, skippedPolicies []string, stdin bool, pvInfos []policyreport.Info) { - if len(skippedPolicies) > 0 { +func printReportOrViolation(policyReport bool, rc *common.ResultCounts, resourcePaths []string, resourcesLen int, skipInvalidPolicies SkippedInvalidPolicies, stdin bool, pvInfos []policyreport.Info) { + if len(skipInvalidPolicies.skipped) > 0 { fmt.Println("----------------------------------------------------------------------\nPolicies Skipped(as required variables are not provided by the users):") - for i, policyName := range skippedPolicies { + for i, policyName := range skipInvalidPolicies.skipped { + fmt.Println(i+1, ". ", policyName) + } + fmt.Println("----------------------------------------------------------------------") + } + if len(skipInvalidPolicies.invalid) > 0 { + fmt.Println("----------------------------------------------------------------------\nInvalid Policies:") + for i, policyName := range skipInvalidPolicies.invalid { fmt.Println(i+1, ". ", policyName) } fmt.Println("----------------------------------------------------------------------") @@ -372,7 +391,7 @@ func createFileOrFolder(mutateLogPath string, mutateLogPathIsDir bool) error { if os.IsNotExist(err) { errDir := os.MkdirAll(folderPath, 0750) if errDir != nil { - return sanitizederror.NewWithError(fmt.Sprintf("failed to create directory"), err) + return sanitizederror.NewWithError("failed to create directory", err) } } } @@ -382,23 +401,23 @@ func createFileOrFolder(mutateLogPath string, mutateLogPathIsDir bool) error { file, err := os.OpenFile(mutateLogPath, os.O_RDONLY|os.O_CREATE, 0600) // #nosec G304 if err != nil { - return sanitizederror.NewWithError(fmt.Sprintf("failed to create file"), err) + return sanitizederror.NewWithError("failed to create file", err) } err = file.Close() if err != nil { - return sanitizederror.NewWithError(fmt.Sprintf("failed to close file"), err) + return sanitizederror.NewWithError("failed to close file", err) } } else { errDir := os.MkdirAll(mutateLogPath, 0750) if errDir != nil { - return sanitizederror.NewWithError(fmt.Sprintf("failed to create directory"), err) + return sanitizederror.NewWithError("failed to create directory", err) } } } else { - return sanitizederror.NewWithError(fmt.Sprintf("failed to describe file"), err) + return sanitizederror.NewWithError("failed to describe file", err) } } diff --git a/pkg/kyverno/common/common.go b/pkg/kyverno/common/common.go index 7edbdb1226..b8474e17a3 100644 --- a/pkg/kyverno/common/common.go +++ b/pkg/kyverno/common/common.go @@ -388,7 +388,7 @@ func RemoveDuplicateAndObjectVariables(matches [][]string) string { for _, v := range m { foundVariable := strings.Contains(variableStr, v) if !foundVariable { - if !strings.Contains(v, "request.object") { + if !strings.Contains(v, "request.object") && !strings.Contains(v, "element") { variableStr = variableStr + " " + v } } diff --git a/pkg/kyverno/common/regex.go b/pkg/kyverno/common/regex.go index db7014cac8..e8e2bdd87e 100644 --- a/pkg/kyverno/common/regex.go +++ b/pkg/kyverno/common/regex.go @@ -8,7 +8,7 @@ import ( var RegexVariables = regexp.MustCompile(`\{\{[^{}]*\}\}`) // AllowedVariables represents regex for {{request.}}, {{serviceAccountName}}, {{serviceAccountNamespace}}, {{@}} and functions e.g. {{divide(,))}} -var AllowedVariables = regexp.MustCompile(`\{\{\s*(request\.|serviceAccountName|serviceAccountNamespace|@|([a-z_]+\())[^{}]*\}\}`) +var AllowedVariables = regexp.MustCompile(`\{\{\s*(request\.|serviceAccountName|serviceAccountNamespace|element\.|@|([a-z_]+\())[^{}]*\}\}`) // AllowedVariables represents regex for {{request.}}, {{serviceAccountName}}, {{serviceAccountNamespace}} var WildCardAllowedVariables = regexp.MustCompile(`\{\{\s*(request\.|serviceAccountName|serviceAccountNamespace)[^{}]*\}\}`) diff --git a/pkg/policy/background.go b/pkg/policy/background.go index aa25ca0f06..989ed5d237 100644 --- a/pkg/policy/background.go +++ b/pkg/policy/background.go @@ -57,7 +57,7 @@ func ContainsVariablesOtherThanObject(policy kyverno.ClusterPolicy) error { } } - filterVars := []string{"request.object", "request.namespace", "images"} + filterVars := []string{"request.object", "request.namespace", "images", "element"} ctx := context.NewContext(filterVars...) for _, contextEntry := range rule.Context { diff --git a/pkg/policy/background_test.go b/pkg/policy/background_test.go index 32a1dda100..b35fb0d31a 100644 --- a/pkg/policy/background_test.go +++ b/pkg/policy/background_test.go @@ -133,5 +133,5 @@ func Test_Validation_invalid_backgroundPolicy(t *testing.T) { err := json.Unmarshal(rawPolicy, &policy) assert.NilError(t, err) err = ContainsVariablesOtherThanObject(policy) - assert.Assert(t, strings.Contains(err.Error(), "variable serviceAccountName cannot be used, allowed variables: [request.object request.namespace images mycm]")) + assert.Assert(t, strings.Contains(err.Error(), "variable serviceAccountName cannot be used, allowed variables: [request.object request.namespace images element mycm]")) } diff --git a/pkg/policy/validate.go b/pkg/policy/validate.go index 1947375330..22c9689926 100644 --- a/pkg/policy/validate.go +++ b/pkg/policy/validate.go @@ -107,6 +107,11 @@ func Validate(policy *kyverno.ClusterPolicy, client *dclient.Client, mock bool, return fmt.Errorf("path: spec.rules[%d]: %v", i, err) } + err := validateElementInForEach(rule) + if err != nil { + return err + } + if err := validateRuleContext(rule); err != nil { return fmt.Errorf("path: spec.rules[%d]: %v", i, err) } @@ -338,6 +343,21 @@ func Validate(policy *kyverno.ClusterPolicy, client *dclient.Client, mock bool, return nil } +func validateElementInForEach(document apiextensions.JSON) error { + jsonByte, err := json.Marshal(document) + if err != nil { + return err + } + + var jsonInterface interface{} + err = json.Unmarshal(jsonByte, &jsonInterface) + if err != nil { + return err + } + _, err = variables.ValidateElementInForEach(log.Log, jsonInterface) + return err +} + func validateMatchKindHelper(rule kyverno.Rule) error { if !ruleOnlyDealsWithResourceMetaData(rule) { return fmt.Errorf("policy can only deal with the metadata field of the resource if" +