1
0
Fork 0
mirror of https://github.com/kyverno/kyverno.git synced 2025-03-28 02:18:15 +00:00

fix: allow changes to preexisting resource in violation of a policy in Enforce (#9027)

* fix: allow changes to preexisting resource in violation of a policy in Enforce

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>

* fix: missing error check

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>

* fix: tests

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>

* nit: cleanup

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>

* fix

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>

* fix: update old policy context

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>

* fix: preconditions always retured true

internal.CheckPreconditions always returned true when v.anyAllConditions, it should be populated with rule.RawAnyAllConditions when newValidator() is used to create a validator

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>

* fix: fix chainsaw test

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>

* fix: nit

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>

* debug

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>

* feat: update test

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>

* fix: add namespace

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>

* feat: add test for bad to good conversion

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>

* feat: add test step

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>

---------

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>
Co-authored-by: shuting <shuting@nirmata.com>
This commit is contained in:
Vishal Choudhary 2023-12-12 14:47:53 +05:30 committed by GitHub
parent 5cda6bf7d7
commit 1f4181645b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
20 changed files with 258 additions and 8 deletions

View file

@ -25,6 +25,7 @@ type PolicyContext interface {
Element() unstructured.Unstructured
SetElement(element unstructured.Unstructured)
OldPolicyContext() (PolicyContext, error)
JSONContext() enginecontext.Interface
Copy() PolicyContext
}

View file

@ -19,6 +19,7 @@ import (
"github.com/kyverno/kyverno/pkg/utils/api"
datautils "github.com/kyverno/kyverno/pkg/utils/data"
stringutils "github.com/kyverno/kyverno/pkg/utils/strings"
"github.com/pkg/errors"
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/client-go/tools/cache"
@ -72,15 +73,17 @@ type validator struct {
}
func newValidator(log logr.Logger, contextLoader engineapi.EngineContextLoader, ctx engineapi.PolicyContext, rule kyvernov1.Rule) *validator {
anyAllConditions, _ := datautils.ToMap(rule.RawAnyAllConditions)
return &validator{
log: log,
rule: rule,
policyContext: ctx,
contextLoader: contextLoader,
pattern: rule.Validation.GetPattern(),
anyPattern: rule.Validation.GetAnyPattern(),
deny: rule.Validation.Deny,
forEach: rule.Validation.ForEachValidation,
log: log,
rule: rule,
policyContext: ctx,
contextLoader: contextLoader,
pattern: rule.Validation.GetPattern(),
anyPattern: rule.Validation.GetAnyPattern(),
deny: rule.Validation.Deny,
anyAllConditions: anyAllConditions,
forEach: rule.Validation.ForEachValidation,
}
}
@ -138,6 +141,22 @@ func (v *validator) validate(ctx context.Context) *engineapi.RuleResponse {
}
ruleResponse := v.validateResourceWithRule()
if engineutils.IsUpdateRequest(v.policyContext) {
priorResp, err := v.validateOldObject(ctx)
if err != nil {
return engineapi.RuleError(v.rule.Name, engineapi.Validation, "failed to validate old object", err)
}
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
}
return engineapi.RuleSkip(v.rule.Name, engineapi.Validation, "skipping modified resource as validation results have not changed")
}
}
return ruleResponse
}
@ -150,6 +169,20 @@ func (v *validator) validate(ctx context.Context) *engineapi.RuleResponse {
return nil
}
func (v *validator) validateOldObject(ctx context.Context) (*engineapi.RuleResponse, error) {
pc := v.policyContext
oldPc, err := v.policyContext.OldPolicyContext()
if err != nil {
return nil, errors.Wrapf(err, "cannot get old policy context")
}
v.policyContext = oldPc
resp := v.validate(ctx)
v.policyContext = pc
return resp, nil
}
func (v *validator) validateForEach(ctx context.Context) *engineapi.RuleResponse {
applyCount := 0
for _, foreach := range v.forEach {

View file

@ -10,6 +10,7 @@ import (
enginectx "github.com/kyverno/kyverno/pkg/engine/context"
"github.com/kyverno/kyverno/pkg/engine/jmespath"
admissionutils "github.com/kyverno/kyverno/pkg/utils/admission"
"github.com/pkg/errors"
admissionv1 "k8s.io/api/admission/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@ -61,6 +62,26 @@ func (c *PolicyContext) Policy() kyvernov1.PolicyInterface {
return c.policy
}
func (c *PolicyContext) OldPolicyContext() (engineapi.PolicyContext, error) {
if c.Operation() != kyvernov1.Update {
return nil, errors.New("cannot create old policy context")
}
copy := c.copy()
oldJsonContext := copy.jsonContext
copy.oldResource = unstructured.Unstructured{}
copy.newResource = c.oldResource
if err := oldJsonContext.AddResource(nil); err != nil {
return nil, errors.Wrapf(err, "failed to replace object in the JSON context")
}
if err := oldJsonContext.AddOldResource(copy.OldResource().Object); err != nil {
return nil, errors.Wrapf(err, "failed to replace old object in the JSON context")
}
copy.jsonContext = oldJsonContext
return copy, nil
}
func (c *PolicyContext) NewResource() unstructured.Unstructured {
return c.newResource
}
@ -229,6 +250,7 @@ func NewPolicyContext(
if admissionInfo != nil {
policyContext = policyContext.WithAdmissionInfo(*admissionInfo)
}
return policyContext, nil
}
@ -257,6 +279,7 @@ func NewPolicyContextFromAdmissionRequest(
WithAdmissionOperation(true).
WithResourceKind(gvk, request.SubResource).
WithRequestResource(request.Resource)
return policyContext, nil
}

View file

@ -93,3 +93,19 @@ func TransformConditions(original apiextensions.JSON) (interface{}, error) {
}
return nil, fmt.Errorf("invalid preconditions")
}
func IsSameRuleResponse(r1 *engineapi.RuleResponse, r2 *engineapi.RuleResponse) bool {
if r1.Name() != r2.Name() ||
r1.RuleType() != r2.RuleType() ||
r1.Message() != r2.Message() ||
r1.Status() != r2.Status() {
return false
}
return true
}
func IsUpdateRequest(ctx engineapi.PolicyContext) bool {
// is the OldObject and NewObject are available, the request is an UPDATE
return ctx.OldResource().Object != nil && ctx.NewResource().Object != nil
}

View file

@ -0,0 +1,13 @@
---
apiVersion: chainsaw.kyverno.io/v1alpha1
kind: TestStep
metadata:
creationTimestamp: null
name: badpod
spec:
timeouts: {}
try:
- apply:
file: bad-pod.yaml
- assert:
file: bad-pod-ready.yaml

View file

@ -0,0 +1,13 @@
---
apiVersion: chainsaw.kyverno.io/v1alpha1
kind: TestStep
metadata:
creationTimestamp: null
name: policy
spec:
timeouts: {}
try:
- apply:
file: policy.yaml
- assert:
file: policy-ready.yaml

View file

@ -0,0 +1,13 @@
---
apiVersion: chainsaw.kyverno.io/v1alpha1
kind: TestStep
metadata:
creationTimestamp: null
name: goodpod
spec:
timeouts: {}
try:
- apply:
file: good-pod.yaml
- assert:
file: good-pod-ready.yaml

View file

@ -0,0 +1,13 @@
---
apiVersion: chainsaw.kyverno.io/v1alpha1
kind: TestStep
metadata:
creationTimestamp: null
name: badpod
spec:
timeouts: {}
try:
- script:
content: ./bad-pod-update-test.sh
timeout: 30s

View file

@ -0,0 +1,12 @@
---
apiVersion: chainsaw.kyverno.io/v1alpha1
kind: TestStep
metadata:
creationTimestamp: null
name: badpod
spec:
timeouts: {}
try:
- script:
content: ./good-pod-update-test.sh
timeout: 30s

View file

@ -0,0 +1,12 @@
---
apiVersion: chainsaw.kyverno.io/v1alpha1
kind: TestStep
metadata:
creationTimestamp: null
name: badtogoodpod
spec:
timeouts: {}
try:
- script:
content: ./update-bad-pod-to-comply.sh
timeout: 30s

View file

@ -0,0 +1,15 @@
## Description
This test mainly verifies that an enforce validate policy does not block changes in old objects that were present before policy was created
## Expected Behavior
1. A pod is created that violates the policy.
2. The policy is applied.
3. A pod is created that follows the policy.
4. Violating changes on bad pad does not cause error.
5. Violating changes in good pod causes error.
6. The bad pod once passed the policy, will be tracked by the policy and return error on bad changes.
## Reference Issue(s)
8837

View file

@ -0,0 +1,5 @@
apiVersion: v1
kind: Pod
metadata:
name: badpod
namespace: default

View file

@ -0,0 +1,8 @@
if kubectl label po badpod foo=bad1 --overwrite 2>&1 | grep -q "validation error: rule check-labels"
then
echo "Test failed, updating violating preexisting resource should not throw error"
exit 1
else
echo "Test succeed, updating violating preexisting resource does not throw error"
exit 0
fi

View file

@ -0,0 +1,14 @@
apiVersion: v1
kind: Pod
metadata:
name: badpod
namespace: default
labels:
foo: bad
spec:
containers:
- name: container01
image: busybox:1.35
args:
- sleep
- 1d

View file

@ -0,0 +1,5 @@
apiVersion: v1
kind: Pod
metadata:
name: goodpod
namespace: default

View file

@ -0,0 +1,8 @@
if kubectl label po goodpod foo=bad1 --overwrite 2>&1 | grep -q "validation error: rule check-labels"
then
echo "Test succeed, updating violating resource throws error"
exit 0
else
echo "Test failed, updating violating resource did not throw error"
exit 1
fi

View file

@ -0,0 +1,14 @@
apiVersion: v1
kind: Pod
metadata:
name: goodpod
namespace: default
labels:
foo: bar
spec:
containers:
- name: container01
image: busybox:1.35
args:
- sleep
- 1d

View file

@ -0,0 +1,4 @@
apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
name: check-labels

View file

@ -0,0 +1,19 @@
apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
name: check-labels
spec:
validationFailureAction: Enforce
background: true
rules:
- name: check-labels
match:
any:
- resources:
kinds:
- Pod
validate:
pattern:
metadata:
labels:
=(foo): "bar"

View file

@ -0,0 +1,9 @@
kubectl label po badpod foo=bar --overwrite
if kubectl label po badpod foo=bad1 --overwrite 2>&1 | grep -q "validation error: rule check-labels"
then
echo "Test succeed, updating violating resource throws error"
exit 0
else
echo "Test failed, updating violating resource did not throw error"
exit 1
fi