mirrors/kyverno
1
0
Fork 0
mirror of https://github.com/kyverno/kyverno.git synced 2024-12-14 11:57:48 +00:00
Code Activity

fix: skip processing the oldObject for audit policies (#10233)

Browse source 
* fix: skip processing the oldObject for audit policies

Signed-off-by: Mariam Fahmy <mariam.fahmy@nirmata.com>

* fix: modify error and skip messages

Signed-off-by: Mariam Fahmy <mariam.fahmy@nirmata.com>

* fix: modify the log level

Signed-off-by: Mariam Fahmy <mariam.fahmy@nirmata.com>

---------

Signed-off-by: Mariam Fahmy <mariam.fahmy@nirmata.com>
This commit is contained in:
Mariam Fahmy 2024-09-20 15:28:18 +03:00 committed by GitHub
parent 47235f07e4
commit 8eb081475e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 224 additions and 40 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

33
pkg/engine/handlers/validation/validate_assert.go
View file

@ -112,20 +112,31 @@ func (h validateAssertHandler) Process(
}
// compose a response
if len(errs) != 0 {
allowExisitingViolations := rule.HasValidateAllowExistingViolations()
if engineutils.IsUpdateRequest(policyContext) && allowExisitingViolations {
errs, err := validateOldObject(ctx, policyContext, rule, payload, bindings)
if err != nil {
logger.V(2).Info("warning: failed to validate old object, skipping the rule evaluation as pre-existing violations are allowed", "rule", rule.Name, "error", err.Error())
return resource, handlers.WithSkip(rule, engineapi.Validation, "failed to validate old object, skipping as preexisting violations are allowed")
}
var action kyvernov1.ValidationFailureAction
if rule.Validation.FailureAction != nil {
action = *rule.Validation.FailureAction
} else {
action = policyContext.Policy().GetSpec().ValidationFailureAction
}
logger.V(3).Info("old object verification", "errors", errs)
if len(errs) != 0 {
logger.V(3).Info("skipping modified resource as validation results have not changed")
return resource, handlers.WithSkip(rule, engineapi.Validation, "skipping modified resource as validation results have not changed")
// process the old object for UPDATE admission requests in case of enforce policies
if action == kyvernov1.Enforce {
allowExisitingViolations := rule.HasValidateAllowExistingViolations()
if engineutils.IsUpdateRequest(policyContext) && allowExisitingViolations {
errs, err := validateOldObject(ctx, policyContext, rule, payload, bindings)
if err != nil {
logger.V(4).Info("warning: failed to validate old object", "rule", rule.Name, "error", err.Error())
return resource, handlers.WithSkip(rule, engineapi.Validation, "failed to validate old object")
}
logger.V(3).Info("old object verification", "errors", errs)
if len(errs) != 0 {
logger.V(2).Info("warning: skipping the rule evaluation as pre-existing violations are allowed", "rule", rule.Name)
return resource, handlers.WithSkip(rule, engineapi.Validation, "skipping the rule evaluation as pre-existing violations are allowed")
}
}
}
var responses []*engineapi.RuleResponse
for _, err := range errs {
responses = append(responses, engineapi.RuleFail(rule.Name, engineapi.Validation, err.Error(), rule.ReportProperties))

36
pkg/engine/handlers/validation/validate_pss.go
View file

@ -133,23 +133,31 @@ func (h validatePssHandler) validate(
}
msg := fmt.Sprintf(`Validation rule '%s' failed. It violates PodSecurity "%s:%s": %s`, rule.Name, podSecurity.Level, podSecurity.Version, pss.FormatChecksPrint(pssChecks))
ruleResponse := engineapi.RuleFail(rule.Name, engineapi.Validation, msg, rule.ReportProperties).WithPodSecurityChecks(podSecurityChecks)
allowExisitingViolations := rule.HasValidateAllowExistingViolations()
if engineutils.IsUpdateRequest(policyContext) && allowExisitingViolations {
logger.V(4).Info("is update request")
priorResp, err := h.validateOldObject(ctx, logger, policyContext, resource, rule, engineLoader, exceptions)
if err != nil {
logger.V(2).Info("warning: failed to validate old object, skipping the rule evaluation as pre-existing violations are allowed", "rule", rule.Name, "error", err.Error())
return resource, engineapi.RuleSkip(rule.Name, engineapi.Validation, "failed to validate old object, skipping as preexisting violations are allowed", rule.ReportProperties)
}
var action kyvernov1.ValidationFailureAction
if rule.Validation.FailureAction != nil {
action = *rule.Validation.FailureAction
} else {
action = policyContext.Policy().GetSpec().ValidationFailureAction
}
if ruleResponse.Status() == priorResp.Status() {
logger.V(3).Info("skipping modified resource as validation results have not changed", "oldResp", priorResp, "newResp", ruleResponse)
if ruleResponse.Status() == engineapi.RuleStatusPass {
return resource, ruleResponse
// process the old object for UPDATE admission requests in case of enforce policies
if action == kyvernov1.Enforce {
allowExisitingViolations := rule.HasValidateAllowExistingViolations()
if engineutils.IsUpdateRequest(policyContext) && allowExisitingViolations {
priorResp, err := h.validateOldObject(ctx, logger, policyContext, resource, rule, engineLoader, exceptions)
if err != nil {
logger.V(4).Info("warning: failed to validate old object", "rule", rule.Name, "error", err.Error())
return resource, engineapi.RuleSkip(rule.Name, engineapi.Validation, "failed to validate old object", rule.ReportProperties)
}
if ruleResponse.Status() == priorResp.Status() {
logger.V(2).Info("warning: skipping the rule evaluation as pre-existing violations are allowed", "oldResp", priorResp, "newResp", ruleResponse)
if ruleResponse.Status() == engineapi.RuleStatusPass {
return resource, ruleResponse
}
return resource, engineapi.RuleSkip(rule.Name, engineapi.Validation, "skipping the rule evaluation as pre-existing violations are allowed", rule.ReportProperties)
}
return resource, engineapi.RuleSkip(rule.Name, engineapi.Validation, "skipping modified resource as validation results have not changed", rule.ReportProperties)
}
logger.V(4).Info("old object response is different", "oldResp", priorResp, "newResp", ruleResponse)
}
return resource, ruleResponse

34
pkg/engine/handlers/validation/validate_resource.go
View file

@ -146,20 +146,30 @@ func (v *validator) validate(ctx context.Context) *engineapi.RuleResponse {
v.log.V(2).Info("invalid validation rule: podSecurity, cel, patterns, or deny expected")
}
allowExisitingViolations := v.rule.HasValidateAllowExistingViolations()
if engineutils.IsUpdateRequest(v.policyContext) && allowExisitingViolations && v.nesting == 0 { // is update request and is the root level validate
priorResp, err := v.validateOldObject(ctx)
if err != nil {
v.log.V(2).Info("warning: failed to validate old object, skipping the rule evaluation as pre-existing violations are allowed", "rule", v.rule.Name, "error", err.Error())
return engineapi.RuleSkip(v.rule.Name, engineapi.Validation, "failed to validate old object, skipping as preexisting violations are allowed", ruleResponse.Properties())
}
var action kyvernov1.ValidationFailureAction
if v.rule.Validation.FailureAction != nil {
action = *v.rule.Validation.FailureAction
} else {
action = v.policyContext.Policy().GetSpec().ValidationFailureAction
}
if engineutils.IsSameRuleResponse(ruleResponse, priorResp) {
v.log.V(3).Info("skipping modified resource as validation results have not changed")
if ruleResponse.Status() == engineapi.RuleStatusPass {
return ruleResponse
// process the old object for UPDATE admission requests in case of enforce policies
if action == kyvernov1.Enforce {
allowExisitingViolations := v.rule.HasValidateAllowExistingViolations()
if engineutils.IsUpdateRequest(v.policyContext) && allowExisitingViolations && v.nesting == 0 { // is update request and is the root level validate
priorResp, err := v.validateOldObject(ctx)
if err != nil {
v.log.V(4).Info("warning: failed to validate old object", "rule", v.rule.Name, "error", err.Error())
return engineapi.RuleSkip(v.rule.Name, engineapi.Validation, "failed to validate old object", ruleResponse.Properties())
}
if engineutils.IsSameRuleResponse(ruleResponse, priorResp) {
v.log.V(2).Info("warning: skipping the rule evaluation as pre-existing violations are allowed")
if ruleResponse.Status() == engineapi.RuleStatusPass {
return ruleResponse
}
return engineapi.RuleSkip(v.rule.Name, engineapi.Validation, "skipping the rule evaluation as pre-existing violations are allowed", v.rule.ReportProperties)
}
return engineapi.RuleSkip(v.rule.Name, engineapi.Validation, "skipping modified resource as validation results have not changed", v.rule.ReportProperties)
}
}

6
pkg/engine/handlers/validation/validate_resource_test.go
View file

@ -150,9 +150,9 @@ var (
},
"name": "check-image",
"validate": {
"failureAction": "Audit",
"allowExistingViolations": true,
"foreach": [
"failureAction": "Enforce",
"allowExistingViolations": true,
"foreach": [
{
"deny": {
"conditions": {

15
test/conformance/chainsaw/reports/admission/update-deployment/README.md Normal file
View file

@ -0,0 +1,15 @@
## Description
This test verifies that policy report doesn't change when a resource is updated and the engine response is the same as before.
A policy in Audit mode is created.
A deployment is created, the deployment violates the policy and we assert the policy report contains a `warn` result.
The deployment is then updated but it still violates the policy.
## Expected result
When the resource is updated and it still violates the policy, the policy report should not change.
## Related issue(s)
- https://github.com/kyverno/kyverno/issues/10169

41
test/conformance/chainsaw/reports/admission/update-deployment/chainsaw-test.yaml Executable file
View file

@ -0,0 +1,41 @@
apiVersion: chainsaw.kyverno.io/v1alpha1
kind: Test
metadata:
creationTimestamp: null
name: update-deployment
spec:
steps:
- name: step-01
try:
- apply:
file: policy.yaml
- assert:
file: policy-assert.yaml
- name: step-02
try:
- apply:
file: deployment.yaml
- assert:
file: deployment.yaml
- name: step-03
try:
- sleep:
duration: 5s
- name: step-04
try:
- assert:
file: report-assert.yaml
- name: step-05
try:
- apply:
file: update-deployment.yaml
- assert:
file: update-deployment.yaml
- name: step-06
try:
- sleep:
duration: 5s
- name: step-07
try:
- assert:
file: report-assert.yaml

19
test/conformance/chainsaw/reports/admission/update-deployment/deployment.yaml Normal file
View file

@ -0,0 +1,19 @@
apiVersion: apps/v1
kind: Deployment
metadata:
labels:
app: nginx
name: nginx-test
spec:
replicas: 1
selector:
matchLabels:
app: nginx
template:
metadata:
labels:
app: nginx
spec:
containers:
- name: nginx
image: nginx:latest

9
test/conformance/chainsaw/reports/admission/update-deployment/policy-assert.yaml Normal file
View file

@ -0,0 +1,9 @@
apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
name: require-multiple-replicas
status:
conditions:
- reason: Succeeded
status: "True"
type: Ready

29
test/conformance/chainsaw/reports/admission/update-deployment/policy.yaml Normal file
View file

@ -0,0 +1,29 @@
apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
name: require-multiple-replicas
annotations:
policies.kyverno.io/category: Best Practises
policies.kyverno.io/minversion: 1.9.2
policies.kyverno.io/severity: low
policies.kyverno.io/subject: Deployment,StatefulSet
policies.kyverno.io/title: Require Multiple Replicas
policies.kyverno.io/scored: "false"
spec:
background: false
rules:
- name: require-multiple-replicas
match:
any:
- resources:
kinds:
- Deployment
- StatefulSet
operations:
- CREATE
- UPDATE
validate:
pattern:
spec:
replicas: ">1"
validationFailureAction: Audit

23
test/conformance/chainsaw/reports/admission/update-deployment/report-assert.yaml Normal file
View file

@ -0,0 +1,23 @@
apiVersion: wgpolicyk8s.io/v1alpha2
kind: PolicyReport
metadata:
ownerReferences:
- apiVersion: apps/v1
kind: Deployment
name: nginx-test
results:
- message: 'validation error: rule require-multiple-replicas failed at path /spec/replicas/'
policy: require-multiple-replicas
result: warn
rule: require-multiple-replicas
source: kyverno
scope:
apiVersion: apps/v1
kind: Deployment
name: nginx-test
summary:
error: 0
fail: 0
pass: 0
skip: 0
warn: 1

19
test/conformance/chainsaw/reports/admission/update-deployment/update-deployment.yaml Normal file
View file

@ -0,0 +1,19 @@
apiVersion: apps/v1
kind: Deployment
metadata:
labels:
app: nginx
name: nginx-test
spec:
replicas: 1
selector:
matchLabels:
app: nginx
template:
metadata:
labels:
app: nginx
spec:
containers:
- name: nginx-1
image: nginx:latest