From 0b5c3cdcaae776c64a6e7297a329eec5075befc9 Mon Sep 17 00:00:00 2001 From: Vyankatesh Kudtarkar Date: Thu, 17 Nov 2022 17:40:47 +0530 Subject: [PATCH] Fix wildcard for any/all match/excude kinds --- pkg/policy/validate.go | 122 ++++++++++++++++++++++-------------- pkg/policy/validate_test.go | 102 ++++++++++++++++++++++++++++++ 2 files changed, 176 insertions(+), 48 deletions(-) diff --git a/pkg/policy/validate.go b/pkg/policy/validate.go index b40edab168..867e9366d2 100644 --- a/pkg/policy/validate.go +++ b/pkg/policy/validate.go @@ -250,54 +250,6 @@ func Validate(policy kyvernov1.PolicyInterface, client dclient.Interface, mock b return warnings, err } - if utils.ContainsString(rule.MatchResources.Kinds, "*") && spec.BackgroundProcessingEnabled() { - return warnings, fmt.Errorf("wildcard policy not allowed in background mode. Set spec.background=false to disable background mode for this policy rule ") - } - - if (utils.ContainsString(rule.MatchResources.Kinds, "*") && len(rule.MatchResources.Kinds) > 1) || (utils.ContainsString(rule.ExcludeResources.Kinds, "*") && len(rule.ExcludeResources.Kinds) > 1) { - return warnings, fmt.Errorf("wildard policy can not deal more than one kind") - } - - if utils.ContainsString(rule.MatchResources.Kinds, "*") || utils.ContainsString(rule.ExcludeResources.Kinds, "*") { - if rule.HasGenerate() || rule.HasVerifyImages() || rule.Validation.ForEachValidation != nil { - return warnings, fmt.Errorf("wildcard policy does not support rule type") - } - - if rule.HasValidate() { - if rule.Validation.GetPattern() != nil || rule.Validation.GetAnyPattern() != nil { - if !ruleOnlyDealsWithResourceMetaData(rule) { - return warnings, fmt.Errorf("policy can only deal with the metadata field of the resource if" + - " the rule does not match any kind") - } - } - - if rule.Validation.Deny != nil { - kyvernoConditions, _ := utils.ApiextensionsJsonToKyvernoConditions(rule.Validation.Deny.GetAnyAllConditions()) - switch typedConditions := kyvernoConditions.(type) { - case []kyvernov1.Condition: // backwards compatibility - for _, condition := range typedConditions { - key := condition.GetKey() - if !strings.Contains(key.(string), "request.object.metadata.") && (!wildCardAllowedVariables.MatchString(key.(string)) || strings.Contains(key.(string), "request.object.spec")) { - return warnings, fmt.Errorf("policy can only deal with the metadata field of the resource if" + - " the rule does not match any kind") - } - } - } - } - } - - if rule.HasMutate() { - if !ruleOnlyDealsWithResourceMetaData(rule) { - return warnings, fmt.Errorf("policy can only deal with the metadata field of the resource if" + - " the rule does not match any kind") - } - } - - if len(errs) != 0 { - return warnings, errs.ToAggregate() - } - } - if rule.HasVerifyImages() { verifyImagePath := rulePath.Child("verifyImages") for index, i := range rule.VerifyImages { @@ -321,6 +273,10 @@ func Validate(policy kyvernov1.PolicyInterface, client dclient.Interface, mock b match := rule.MatchResources exclude := rule.ExcludeResources for _, value := range match.Any { + wildcardErr := validateWildcard(value.ResourceDescription.Kinds, spec, rule) + if wildcardErr != nil { + return warnings, wildcardErr + } if !utils.ContainsString(value.ResourceDescription.Kinds, "*") { err := validateKinds(value.ResourceDescription.Kinds, mock, client, policy) if err != nil { @@ -329,6 +285,10 @@ func Validate(policy kyvernov1.PolicyInterface, client dclient.Interface, mock b } } for _, value := range match.All { + wildcardErr := validateWildcard(value.ResourceDescription.Kinds, spec, rule) + if wildcardErr != nil { + return warnings, wildcardErr + } if !utils.ContainsString(value.ResourceDescription.Kinds, "*") { err := validateKinds(value.ResourceDescription.Kinds, mock, client, policy) if err != nil { @@ -337,6 +297,10 @@ func Validate(policy kyvernov1.PolicyInterface, client dclient.Interface, mock b } } for _, value := range exclude.Any { + wildcardErr := validateWildcard(value.ResourceDescription.Kinds, spec, rule) + if wildcardErr != nil { + return warnings, wildcardErr + } if !utils.ContainsString(value.ResourceDescription.Kinds, "*") { err := validateKinds(value.ResourceDescription.Kinds, mock, client, policy) if err != nil { @@ -345,6 +309,10 @@ func Validate(policy kyvernov1.PolicyInterface, client dclient.Interface, mock b } } for _, value := range exclude.All { + wildcardErr := validateWildcard(value.ResourceDescription.Kinds, spec, rule) + if wildcardErr != nil { + return warnings, wildcardErr + } if !utils.ContainsString(value.ResourceDescription.Kinds, "*") { err := validateKinds(value.ResourceDescription.Kinds, mock, client, policy) if err != nil { @@ -352,6 +320,7 @@ func Validate(policy kyvernov1.PolicyInterface, client dclient.Interface, mock b } } } + if !utils.ContainsString(rule.MatchResources.Kinds, "*") { err := validateKinds(rule.MatchResources.Kinds, mock, client, policy) if err != nil { @@ -361,6 +330,15 @@ func Validate(policy kyvernov1.PolicyInterface, client dclient.Interface, mock b if err != nil { return warnings, errors.Wrapf(err, "exclude resource kind is invalid") } + } else { + wildcardErr := validateWildcard(rule.MatchResources.Kinds, spec, rule) + if wildcardErr != nil { + return warnings, wildcardErr + } + wildcardErr = validateWildcard(rule.ExcludeResources.Kinds, spec, rule) + if wildcardErr != nil { + return warnings, wildcardErr + } } // Validate string values in labels @@ -1163,6 +1141,54 @@ func podControllerAutoGenExclusion(policy kyvernov1.PolicyInterface) bool { return false } +// validateWildcard check for an Match/Exclude block contains "*" +func validateWildcard(kinds []string, spec *kyvernov1.Spec, rule kyvernov1.Rule) error { + + if utils.ContainsString(kinds, "*") && spec.BackgroundProcessingEnabled() { + return fmt.Errorf("wildcard policy not allowed in background mode. Set spec.background=false to disable background mode for this policy rule ") + } + if utils.ContainsString(kinds, "*") && len(kinds) > 1 { + return fmt.Errorf("wildard policy can not deal more than one kind") + } + if utils.ContainsString(kinds, "*") { + if rule.HasGenerate() || rule.HasVerifyImages() || rule.Validation.ForEachValidation != nil { + return fmt.Errorf("wildcard policy does not support rule type") + } + + if rule.HasValidate() { + if rule.Validation.GetPattern() != nil || rule.Validation.GetAnyPattern() != nil { + if !ruleOnlyDealsWithResourceMetaData(rule) { + return fmt.Errorf("policy can only deal with the metadata field of the resource if" + + " the rule does not match any kind") + } + } + + if rule.Validation.Deny != nil { + kyvernoConditions, _ := utils.ApiextensionsJsonToKyvernoConditions(rule.Validation.Deny.GetAnyAllConditions()) + switch typedConditions := kyvernoConditions.(type) { + case []kyvernov1.Condition: // backwards compatibility + for _, condition := range typedConditions { + key := condition.GetKey() + if !strings.Contains(key.(string), "request.object.metadata.") && (!wildCardAllowedVariables.MatchString(key.(string)) || strings.Contains(key.(string), "request.object.spec")) { + return fmt.Errorf("policy can only deal with the metadata field of the resource if" + + " the rule does not match any kind") + } + } + } + } + } + + if rule.HasMutate() { + if !ruleOnlyDealsWithResourceMetaData(rule) { + return fmt.Errorf("policy can only deal with the metadata field of the resource if" + + " the rule does not match any kind") + } + } + + } + return nil +} + // validateKinds verifies if an API resource that matches 'kind' is valid kind // and found in the cache, returns error if not found func validateKinds(kinds []string, mock bool, client dclient.Interface, p kyvernov1.PolicyInterface) error { diff --git a/pkg/policy/validate_test.go b/pkg/policy/validate_test.go index e6a35a0580..1b6ac9da13 100644 --- a/pkg/policy/validate_test.go +++ b/pkg/policy/validate_test.go @@ -1037,6 +1037,51 @@ func Test_Validate_ApiCall(t *testing.T) { } } +func Test_validate_Wildcard(t *testing.T) { + testCases := []struct { + resource kyverno.ContextEntry + expectedResult interface{} + }{ + { + resource: kyverno.ContextEntry{ + APICall: &kyverno.APICall{ + URLPath: "/apis/networking.k8s.io/v1/namespaces/{{request.namespace}}/networkpolicies", + JMESPath: "", + }, + }, + expectedResult: nil, + }, + { + resource: kyverno.ContextEntry{ + APICall: &kyverno.APICall{ + URLPath: "/apis/networking.k8s.io/v1/namespaces/{{request.namespace}}/networkpolicies", + JMESPath: "items[", + }, + }, + expectedResult: "failed to parse JMESPath items[: SyntaxError: Expected tStar, received: tEOF", + }, + { + resource: kyverno.ContextEntry{ + APICall: &kyverno.APICall{ + URLPath: "/apis/networking.k8s.io/v1/namespaces/{{request.namespace}}/networkpolicies", + JMESPath: "items[{{request.namespace}}", + }, + }, + expectedResult: nil, + }, + } + + for _, testCase := range testCases { + err := validateAPICall(testCase.resource) + + if err == nil { + assert.Equal(t, err, testCase.expectedResult) + } else { + assert.Equal(t, err.Error(), testCase.expectedResult) + } + } +} + func Test_Wildcards_Kind(t *testing.T) { rawPolicy := []byte(` { @@ -2222,3 +2267,60 @@ func testResourceList() []*metav1.APIResourceList { }, } } + +func Test_Any_wildcard_policy(t *testing.T) { + var err error + rawPolicy := []byte(`{ + "apiVersion": "kyverno.io/v1", + "kind": "ClusterPolicy", + "metadata": { + "name": "verify-image" + }, + "spec": { + "validationFailureAction": "enforce", + "background": false, + "rules": [ + { + "name": "verify-image", + "match": { + "any": [ + { + "resources": { + "kinds": [ + "*" + ] + } + } + ] + }, + "verifyImages": [ + { + "imageReferences": [ + "ghcr.io/kyverno/test-verify-image:*" + ], + "mutateDigest": true, + "attestors": [ + { + "entries": [ + { + "keys": { + "publicKeys": "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE8nXRh950IZbRj8Ra/N9sbqOPZrfM\n5/KAQN0/KjHcorm/J5yctVd7iEcnessRQjU917hmKO6JWVGHpDguIyakZA==\n-----END PUBLIC KEY----- \n" + } + } + ] + } + ] + } + ] + } + ] + } + }`) + var policy *kyverno.ClusterPolicy + err = json.Unmarshal(rawPolicy, &policy) + assert.NilError(t, err) + + openApiManager, _ := openapi.NewManager() + _, err = Validate(policy, nil, true, openApiManager) + assert.Assert(t, err != nil) +}