Skip to content

Commit

Permalink
Fix: handled skip rule processing in anyPattern field
Browse files Browse the repository at this point in the history
Signed-off-by: ansalamdaniel <[email protected]>
  • Loading branch information
ansalamdaniel committed Nov 28, 2022
1 parent dfded5c commit 2ba5690
Show file tree
Hide file tree
Showing 5 changed files with 214 additions and 5 deletions.
25 changes: 20 additions & 5 deletions pkg/engine/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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())
Expand Down
51 changes: 51 additions & 0 deletions pkg/engine/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
})
}
}
26 changes: 26 additions & 0 deletions test/cli/test/anypattern_skip_error/kyverno-test.yaml
Original file line number Diff line number Diff line change
@@ -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
24 changes: 24 additions & 0 deletions test/cli/test/anypattern_skip_error/policy.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
name: validate-service-loadbalancer
spec:
validationFailureAction: enforce
rules:
- name: check-loadbalancer-public
match:
any:
- 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: "*"
93 changes: 93 additions & 0 deletions test/cli/test/anypattern_skip_error/resource.yaml
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 2ba5690

Please sign in to comment.