From 7017086ab0fc7afc51628e81d579f6cdade59e28 Mon Sep 17 00:00:00 2001 From: ansalamdaniel Date: Thu, 27 Oct 2022 20:52:37 +0530 Subject: [PATCH] Fix: handled skip rule processing in anyPattern field Signed-off-by: ansalamdaniel --- pkg/engine/validation.go | 25 ++++- pkg/engine/validation_test.go | 51 ++++++++++ .../anypattern_skip_error/kyverno-test.yaml | 26 ++++++ .../test/anypattern_skip_error/policy.yaml | 23 +++++ .../test/anypattern_skip_error/resource.yaml | 93 +++++++++++++++++++ 5 files changed, 213 insertions(+), 5 deletions(-) create mode 100644 test/cli/test/anypattern_skip_error/kyverno-test.yaml create mode 100644 test/cli/test/anypattern_skip_error/policy.yaml create mode 100644 test/cli/test/anypattern_skip_error/resource.yaml diff --git a/pkg/engine/validation.go b/pkg/engine/validation.go index 699553943d36..c8ee51d51847 100644 --- a/pkg/engine/validation.go +++ b/pkg/engine/validation.go @@ -633,6 +633,7 @@ func (v *validator) validatePatterns(resource unstructured.Unstructured) *respon if v.anyPattern != nil { var failedAnyPatternsErrors []error + var skippedAnyPatternErrors []error var err error anyPatterns, err := deserializeAnyPattern(v.anyPattern) @@ -649,19 +650,33 @@ func (v *validator) validatePatterns(resource unstructured.Unstructured) *respon } if pe, ok := err.(*validate.PatternError); ok { + var patternErr error v.log.V(3).Info("validation rule failed", "anyPattern[%d]", idx, "path", pe.Path) - if pe.Path == "" { - patternErr := fmt.Errorf("rule %s[%d] failed: %s", v.rule.Name, idx, err.Error()) - failedAnyPatternsErrors = append(failedAnyPatternsErrors, patternErr) + + if pe.Skip { + patternErr = fmt.Errorf("rule %s[%d] skipped: %s", v.rule.Name, idx, err.Error()) + skippedAnyPatternErrors = append(skippedAnyPatternErrors, patternErr) } else { - patternErr := fmt.Errorf("rule %s[%d] failed at path %s", v.rule.Name, idx, pe.Path) + if pe.Path == "" { + patternErr = fmt.Errorf("rule %s[%d] failed: %s", v.rule.Name, idx, err.Error()) + } else { + patternErr = fmt.Errorf("rule %s[%d] failed at path %s", v.rule.Name, idx, pe.Path) + } failedAnyPatternsErrors = append(failedAnyPatternsErrors, patternErr) } } } // Any Pattern validation errors - if len(failedAnyPatternsErrors) > 0 { + if len(skippedAnyPatternErrors) > 0 && len(failedAnyPatternsErrors) == 0 { + var errorStr []string + for _, err := range skippedAnyPatternErrors { + errorStr = append(errorStr, err.Error()) + } + + v.log.V(4).Info(fmt.Sprintf("Validation rule '%s' skipped. %s", v.rule.Name, errorStr)) + return ruleResponse(*v.rule, response.Validation, strings.Join(errorStr, " "), response.RuleStatusSkip, nil) + } else if len(failedAnyPatternsErrors) > 0 { var errorStr []string for _, err := range failedAnyPatternsErrors { errorStr = append(errorStr, err.Error()) diff --git a/pkg/engine/validation_test.go b/pkg/engine/validation_test.go index d39e70325c8c..7a5e71d03969 100644 --- a/pkg/engine/validation_test.go +++ b/pkg/engine/validation_test.go @@ -3095,3 +3095,54 @@ func Test_block_bypass(t *testing.T) { executeTest(t, testcase) } } + +func Test_ValidatePattern_anyPattern(t *testing.T) { + var policy kyverno.ClusterPolicy + rawPolicy := []byte(`{"apiVersion":"kyverno.io\/v1","kind":"ClusterPolicy","metadata":{"name":"validate-service-loadbalancer"},"spec":{"validationFailureAction":"enforce","rules":[{"name":"check-loadbalancer-public","match":{"resources":{"kinds":["Service"]}},"validate":{"message":"Service of type 'LoadBalancer' is public and does not explicitly define network security. To use a public LB you must supply either spec[loadBalancerSourceRanges] or the 'service.beta.kubernetes.io\/aws-load-balancer-security-groups' annotation.","anyPattern":[{"spec":{"<(type)":"LoadBalancer"},"metadata":{"annotations":{"service.beta.kubernetes.io\/aws-load-balancer-security-groups":"?*"}}},{"spec":{"<(type)":"LoadBalancer","loadBalancerSourceRanges":"*"}}]}},{"name":"check-loadbalancer-internal","match":{"resources":{"kinds":["Service"]}},"validate":{"message":"Service of type 'LoadBalancer' is internal and does not explicitly define network security. To set the LB to internal, use annotation 'service.beta.kubernetes.io\/aws-load-balancer-internal' with value 'true' or '0.0.0.0\/0' ","pattern":{"spec":{"<(type)":"LoadBalancer"},"metadata":{"annotations":{"=(service.beta.kubernetes.io\/aws-load-balancer-internal)":"0.0.0.0\/0|true"}}}}}]}}`) + testCases := []struct { + description string + rawPolicy []byte + rawResource []byte + expectedFailed bool + expectedSkipped bool + expectedSuccess bool + }{ + { + description: "skip", + rawPolicy: rawPolicy, + rawResource: []byte(`{"apiVersion":"v1","kind":"Service","metadata":{"labels":{"app.kubernetes.io\/managed-by":"Helm"},"name":"service-clusterip-pass"},"spec":{"type":"ClusterIP","ports":[{"name":"http","port":80,"protocol":"TCP","targetPort":3000}]}}`), + expectedSkipped: true, + }, + { + description: "fail", + rawPolicy: rawPolicy, + rawResource: []byte(`{"apiVersion":"v1","kind":"Service","metadata":{"annotations":{"service.beta.kubernetes.io\/aws-load-balancer-internal":"anything"},"labels":{"app.kubernetes.io\/managed-by":"Helm"},"name":"service-internal-fail"},"spec":{"type":"LoadBalancer","clusterIP":"10.96.0.2","ports":[{"name":"http","nodePort":31207,"port":80,"protocol":"TCP","targetPort":3000}]}}`), + expectedFailed: true, + }, + { + description: "success", + rawPolicy: rawPolicy, + rawResource: []byte(`{"apiVersion":"v1","kind":"Service","metadata":{"annotations":{"service.beta.kubernetes.io\/aws-load-balancer-internal":"0.0.0.0\/0"},"labels":{"app.kubernetes.io\/managed-by":"Helm"},"name":"service-internal-pass"},"spec":{"type":"LoadBalancer","clusterIP":"100.69.148.11","loadBalancerSourceRanges":["3.224.166.65\/32","3.210.193.151\/32","3.226.136.65\/32","10.0.0.0\/8"]}}`), + expectedSuccess: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + err := json.Unmarshal(tc.rawPolicy, &policy) + assert.NilError(t, err) + + resourceUnstructured, err := utils.ConvertToUnstructured(tc.rawResource) + assert.NilError(t, err) + + er := Validate(&PolicyContext{Policy: &policy, NewResource: *resourceUnstructured, JSONContext: context.NewContext()}) + if tc.expectedFailed { + assert.Assert(t, er.IsFailed()) + } else if tc.expectedSkipped { + assert.Assert(t, er.IsSkipped()) + } else if tc.expectedSuccess { + assert.Assert(t, er.IsSuccessful()) + } + }) + } +} diff --git a/test/cli/test/anypattern_skip_error/kyverno-test.yaml b/test/cli/test/anypattern_skip_error/kyverno-test.yaml new file mode 100644 index 000000000000..0486aea6be01 --- /dev/null +++ b/test/cli/test/anypattern_skip_error/kyverno-test.yaml @@ -0,0 +1,26 @@ +name: validate-service-loadbalancer +policies: + - policy.yaml +resources: + - resource.yaml +results: + - policy: validate-service-loadbalancer + rule: check-loadbalancer-public + resource: service-public-pass + kind: Service + result: pass + - policy: validate-service-loadbalancer + rule: check-loadbalancer-public + resource: service-public-2-pass + kind: Service + result: pass + - policy: validate-service-loadbalancer + rule: check-loadbalancer-public + resource: service-public-fail + kind: Service + result: fail + - policy: validate-service-loadbalancer + rule: check-loadbalancer-public + resource: service-clusterip-skip + kind: Service + result: skip \ No newline at end of file diff --git a/test/cli/test/anypattern_skip_error/policy.yaml b/test/cli/test/anypattern_skip_error/policy.yaml new file mode 100644 index 000000000000..d787483c8229 --- /dev/null +++ b/test/cli/test/anypattern_skip_error/policy.yaml @@ -0,0 +1,23 @@ +apiVersion: kyverno.io/v1 +kind: ClusterPolicy +metadata: + name: validate-service-loadbalancer +spec: + validationFailureAction: enforce + rules: + - name: check-loadbalancer-public + match: + resources: + kinds: + - Service + validate: + message: "Service of type 'LoadBalancer' is public and does not explicitly define network security. To use a public LB you must supply either spec[loadBalancerSourceRanges] or the 'service.beta.kubernetes.io/aws-load-balancer-security-groups' annotation." + anyPattern: + - spec: + <(type): LoadBalancer + metadata: + annotations: + service.beta.kubernetes.io/aws-load-balancer-security-groups: "?*" + - spec: + <(type): LoadBalancer + loadBalancerSourceRanges: "*" \ No newline at end of file diff --git a/test/cli/test/anypattern_skip_error/resource.yaml b/test/cli/test/anypattern_skip_error/resource.yaml new file mode 100644 index 000000000000..f49c7feed85e --- /dev/null +++ b/test/cli/test/anypattern_skip_error/resource.yaml @@ -0,0 +1,93 @@ +apiVersion: v1 +kind: Service +metadata: + annotations: + meta.helm.sh/release-name: app-my-release + labels: + app.kubernetes.io/managed-by: Helm + name: service-public-pass +spec: + type: LoadBalancer + loadBalancerSourceRanges: + - 3.224.166.65/32 + - 3.210.193.151/32 + - 3.226.136.65/32 + ports: + - name: http + nodePort: 31207 + port: 80 + protocol: TCP + targetPort: 3000 + - name: https + nodePort: 30400 + port: 443 + protocol: TCP + targetPort: 3000 +--- +apiVersion: v1 +kind: Service +metadata: + annotations: + service.beta.kubernetes.io/aws-load-balancer-cross-zone-load-balancing-enabled: "true" + service.beta.kubernetes.io/aws-load-balancer-security-groups: sg-01d2131dfgs45645 + labels: + app.kubernetes.io/instance: app-hello-node-test + name: service-public-2-pass +spec: + type: LoadBalancer + clusterIP: 100.67.234.84 +--- +apiVersion: v1 +kind: Service +metadata: + annotations: + external-dns.alpha.kubernetes.io/hostname: hello-node-public-test.[REDACTED] + meta.helm.sh/release-name: app-hello-node-test + meta.helm.sh/release-namespace: app-hello-node-test + service.beta.kubernetes.io/aws-load-balancer-additional-resource-tags: Environment=test,ops_environment=test,ops_costcenter=220,service=hello-node,kubernetesManager=true,region=us-east-1,elb_name=public + service.beta.kubernetes.io/aws-load-balancer-backend-protocol: http + service.beta.kubernetes.io/aws-load-balancer-connection-draining-enabled: "true" + service.beta.kubernetes.io/aws-load-balancer-connection-draining-timeout: "60" + service.beta.kubernetes.io/aws-load-balancer-cross-zone-load-balancing-enabled: "true" + service.beta.kubernetes.io/aws-load-balancer-healthcheck-healthy-threshold: "2" + service.beta.kubernetes.io/aws-load-balancer-healthcheck-interval: "5" + service.beta.kubernetes.io/aws-load-balancer-healthcheck-timeout: "2" + service.beta.kubernetes.io/aws-load-balancer-healthcheck-unhealthy-threshold: "2" + service.beta.kubernetes.io/aws-load-balancer-ssl-ports: "443" + labels: + app.kubernetes.io/managed-by: Helm + name: service-public-fail +spec: + type: LoadBalancer + clusterIP: 100.67.234.84 + clusterIPs: + - 100.67.234.84 + externalTrafficPolicy: Cluster + ipFamilies: + - IPv4 + ipFamilyPolicy: SingleStack + ports: + - name: http + nodePort: 31207 + port: 80 + protocol: TCP + targetPort: 3000 + - name: https + nodePort: 30400 + port: 443 + protocol: TCP + targetPort: 3000 +--- +apiVersion: v1 +kind: Service +metadata: + labels: + app.kubernetes.io/managed-by: Helm + name: service-clusterip-skip +spec: + type: ClusterIP + ports: + - name: http + port: 80 + protocol: TCP + targetPort: 3000