Fix: handled skip rule processing in anyPattern field

Signed-off-by: ansalamdaniel <ansalam.daniel@infracloud.io>
infracloudio · Oct 31, 2022 · a5ffaa6 · a5ffaa6
1 parent 8a59e24
commit a5ffaa6
Show file tree

Hide file tree

Showing 2 changed files with 72 additions and 6 deletions.
diff --git a/pkg/engine/validation.go b/pkg/engine/validation.go
@@ -631,6 +631,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)
@@ -647,19 +648,33 @@ func (v *validator) validatePatterns(resource unstructured.Unstructured) *respon
 			}
 
 			if pe, ok := err.(*validate.PatternError); ok {
-				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)
+				var patternErr error
+				v.log.V(3).Info("validation error", "anyPattern[%d]", idx, "path", pe.Path)
+
+				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
@@ -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())
+			}
+		})
+	}
+}