1
0
Fork 0
mirror of https://github.com/kyverno/kyverno.git synced 2025-03-05 07:26:55 +00:00

fix: properly update policy context after preexisting resource in violation check (#9893)

* fix: properly update policy context after preexisting resource in violation check

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

* chore: remove all copy function usages

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

* chore: nit

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

* refactor context resource swap

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* feat: chainsaw tests

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

* fix: test:

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

* fix: logger panic

Signed-off-by: ShutingZhao <shuting@nirmata.com>

* fix: copy cover policycontext

Signed-off-by: ShutingZhao <shuting@nirmata.com>

---------

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>
Signed-off-by: Jim Bugwadia <jim@nirmata.com>
Signed-off-by: ShutingZhao <shuting@nirmata.com>
Co-authored-by: Jim Bugwadia <jim@nirmata.com>
Co-authored-by: ShutingZhao <shuting@nirmata.com>
This commit is contained in:
Vishal Choudhary 2024-03-13 21:54:53 +05:30 committed by GitHub
parent 60899905f7
commit f2833861f8
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
18 changed files with 384 additions and 77 deletions

View file

@ -77,7 +77,7 @@ func (c *GenerateController) handleNonPolicyChanges(policy kyvernov1.PolicyInter
failedDownstreams = append(failedDownstreams, spec)
errs = append(errs, err)
} else {
c.log.V(4).Info("downstream resource deleted", spec.String())
c.log.V(4).Info("downstream resource deleted", "spec", spec.String())
}
}
if len(errs) != 0 {

View file

@ -16,6 +16,7 @@ type PolicyContext interface {
Policy() kyvernov1.PolicyInterface
NewResource() unstructured.Unstructured
OldResource() unstructured.Unstructured
SetResources(oldResource, newResource unstructured.Unstructured) error
AdmissionInfo() kyvernov1beta1.RequestInfo
Operation() kyvernov1.AdmissionOperation
NamespaceLabels() map[string]string
@ -25,7 +26,5 @@ type PolicyContext interface {
Element() unstructured.Unstructured
SetElement(element unstructured.Unstructured)
OldPolicyContext() (PolicyContext, error)
JSONContext() enginecontext.Interface
Copy() PolicyContext
}

View file

@ -44,6 +44,8 @@ type EvalInterface interface {
// Interface to manage context operations
// TODO: move to contextapi to prevent circular dependencies
type Interface interface {
EvalInterface
// AddRequest marshals and adds the admission request to the context
AddRequest(request admissionv1.AdmissionRequest) error
@ -106,8 +108,6 @@ type Interface interface {
// Reset sets the internal state to the last checkpoint, but does not remove the checkpoint.
Reset()
EvalInterface
// AddJSON merges the json map with context
addJSON(dataMap map[string]interface{}) error
}

View file

@ -87,7 +87,7 @@ func (f *forEachMutator) mutateElements(ctx context.Context, foreach kyvernov1.F
index = len(elements) - 1 - index
}
f.policyContext.JSONContext().Reset()
policyContext := f.policyContext.Copy()
policyContext := f.policyContext
falseVar := false
if err := engineutils.AddElementToContext(policyContext, element, index, f.nesting, &falseVar); err != nil {

View file

@ -64,7 +64,7 @@ func (h mutateExistingHandler) Process(
if target.unstructured.Object == nil {
continue
}
policyContext := policyContext.Copy()
policyContext := policyContext
if err := policyContext.JSONContext().SetTargetResource(target.unstructured.Object); err != nil {
logger.Error(err, "failed to add target resource to the context")
continue

View file

@ -626,7 +626,7 @@ var (
)
func Test_VerifyManifest_SignedYAML(t *testing.T) {
policyContext := buildContext(t, test_policy, signed_resource, "")
policyContext := buildContext(t, kyvernov1.Create, test_policy, signed_resource, "")
var request v1.AdmissionRequest
_ = json.Unmarshal([]byte(signed_adreq), &request)
policyContext.JSONContext().AddRequest(request)
@ -648,7 +648,7 @@ func Test_VerifyManifest_SignedYAML(t *testing.T) {
}
func Test_VerifyManifest_UnsignedYAML(t *testing.T) {
policyContext := buildContext(t, test_policy, unsigned_resource, "")
policyContext := buildContext(t, kyvernov1.Create, test_policy, unsigned_resource, "")
var request v1.AdmissionRequest
_ = json.Unmarshal([]byte(unsigned_adreq), &request)
policyContext.JSONContext().AddRequest(request)
@ -670,7 +670,7 @@ func Test_VerifyManifest_UnsignedYAML(t *testing.T) {
}
func Test_VerifyManifest_InvalidYAML(t *testing.T) {
policyContext := buildContext(t, test_policy, invalid_resource, "")
policyContext := buildContext(t, kyvernov1.Create, test_policy, invalid_resource, "")
var request v1.AdmissionRequest
_ = json.Unmarshal([]byte(invalid_adreq), &request)
policyContext.JSONContext().AddRequest(request)
@ -692,7 +692,7 @@ func Test_VerifyManifest_InvalidYAML(t *testing.T) {
}
func Test_VerifyManifest_MustAll_InvalidYAML(t *testing.T) {
policyContext := buildContext(t, test_policy, multi_sig_resource, "")
policyContext := buildContext(t, kyvernov1.Create, test_policy, multi_sig_resource, "")
var request v1.AdmissionRequest
_ = json.Unmarshal([]byte(multi_sig_adreq), &request)
policyContext.JSONContext().AddRequest(request)
@ -720,7 +720,7 @@ func Test_VerifyManifest_MustAll_InvalidYAML(t *testing.T) {
}
func Test_VerifyManifest_MustAll_ValidYAML(t *testing.T) {
policyContext := buildContext(t, test_policy, multi_sig2_resource, "")
policyContext := buildContext(t, kyvernov1.Create, test_policy, multi_sig2_resource, "")
var request v1.AdmissionRequest
_ = json.Unmarshal([]byte(multi_sig2_adreq), &request)
policyContext.JSONContext().AddRequest(request)
@ -752,7 +752,7 @@ func Test_VerifyManifest_MustAll_ValidYAML(t *testing.T) {
}
func Test_VerifyManifest_AtLeastOne(t *testing.T) {
policyContext := buildContext(t, test_policy, multi_sig_resource, "")
policyContext := buildContext(t, kyvernov1.Create, test_policy, multi_sig_resource, "")
var request v1.AdmissionRequest
_ = json.Unmarshal([]byte(multi_sig_adreq), &request)
policyContext.JSONContext().AddRequest(request)
@ -780,7 +780,7 @@ func Test_VerifyManifest_AtLeastOne(t *testing.T) {
assert.Equal(t, verified, true)
}
func buildContext(t *testing.T, policy, resource string, oldResource string) engineapi.PolicyContext {
func buildContext(t *testing.T, operation kyvernov1.AdmissionOperation, policy, resource string, oldResource string) engineapi.PolicyContext {
var cpol kyvernov1.ClusterPolicy
err := json.Unmarshal([]byte(policy), &cpol)
assert.NilError(t, err)
@ -791,7 +791,7 @@ func buildContext(t *testing.T, policy, resource string, oldResource string) eng
policyContext, err := policycontext.NewPolicyContext(
jp,
*resourceUnstructured,
kyvernov1.Create,
operation,
nil,
cfg,
)

View file

@ -170,15 +170,23 @@ func (v *validator) validate(ctx context.Context) *engineapi.RuleResponse {
}
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")
if v.policyContext.Operation() != kyvernov1.Update {
return nil, errors.New("invalid operation")
}
newResource := v.policyContext.NewResource()
oldResource := v.policyContext.OldResource()
emptyResource := unstructured.Unstructured{}
if err := v.policyContext.SetResources(emptyResource, oldResource); err != nil {
return nil, errors.Wrapf(err, "failed to set resources")
}
v.policyContext = oldPc
resp := v.validate(ctx)
v.policyContext = pc
if err := v.policyContext.SetResources(oldResource, newResource); err != nil {
return nil, errors.Wrapf(err, "failed to reset resources")
}
return resp, nil
}
@ -214,7 +222,7 @@ func (v *validator) validateElements(ctx context.Context, foreach kyvernov1.ForE
}
v.policyContext.JSONContext().Reset()
policyContext := v.policyContext.Copy()
policyContext := v.policyContext
if err := engineutils.AddElementToContext(policyContext, element, index, v.nesting, elementScope); err != nil {
v.log.Error(err, "failed to add element to context")
return engineapi.RuleError(v.rule.Name, engineapi.Validation, "failed to process foreach", err), applyCount

View file

@ -0,0 +1,137 @@
package validation
import (
"context"
"testing"
"github.com/go-logr/logr"
kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
"github.com/kyverno/kyverno/pkg/engine/api"
enginecontext "github.com/kyverno/kyverno/pkg/engine/context"
"github.com/stretchr/testify/assert"
)
func Test_validateOldObject(t *testing.T) {
mockCL := func(ctx context.Context, contextEntries []kyvernov1.ContextEntry, jsonContext enginecontext.Interface) error {
return nil
}
policyContext := buildTestNamespaceLabelsContext(t)
rule := policyContext.Policy().GetSpec().Rules[0]
v := newValidator(logr.Discard(), mockCL, policyContext, rule)
ctx := context.TODO()
resp := v.validate(ctx)
assert.NotNil(t, resp)
assert.Equal(t, api.RuleStatusPass, resp.Status())
rule2 := policyContext.Policy().GetSpec().Rules[1]
v2 := newValidator(logr.Discard(), mockCL, policyContext, rule2)
resp2 := v2.validate(ctx)
assert.NotNil(t, resp2 != nil)
assert.Equal(t, api.RuleStatusFail, resp2.Status())
}
func buildTestNamespaceLabelsContext(t *testing.T) api.PolicyContext {
policy := `{
"apiVersion": "kyverno.io/v1",
"kind": "ClusterPolicy",
"metadata": {
"name": "block-label-changes"
},
"spec": {
"validationFailureAction": "Enforce",
"background": false,
"rules": [
{
"name": "require-labels",
"match": {
"all": [
{
"resources": {
"operations": [
"CREATE",
"UPDATE"
],
"kinds": [
"Namespace"
]
}
}
]
},
"validate": {
"message": "The label size is required",
"pattern": {
"metadata": {
"labels": {
"size": "small | medium | large"
}
}
}
}
},
{
"name": "check-mutable-labels",
"match": {
"all": [
{
"resources": {
"operations": [
"UPDATE"
],
"kinds": [
"Namespace"
]
}
}
]
},
"validate": {
"message": "The label size cannot be changed for a namespace",
"deny": {
"conditions": {
"all": [
{
"key": "{{ request.object.metadata.labels.size || '' }}",
"operator": "NotEquals",
"value": "{{ request.oldObject.metadata.labels.size }}"
}
]
}
}
}
}
]
}
}`
resource := `{
"apiVersion": "v1",
"kind": "Namespace",
"metadata": {
"annotations": {},
"labels": {
"kubernetes.io/metadata.name": "test",
"size": "large"
},
"name": "test"
},
"spec": {}
}`
oldResource := `{
"apiVersion": "v1",
"kind": "Namespace",
"metadata": {
"labels": {
"kubernetes.io/metadata.name": "test",
"size": "small"
},
"name": "test"
},
"spec": {}
}`
return buildContext(t, kyvernov1.Update, policy, resource, oldResource)
}

View file

@ -6,7 +6,6 @@ import (
kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
kyvernov1beta1 "github.com/kyverno/kyverno/api/kyverno/v1beta1"
"github.com/kyverno/kyverno/pkg/config"
engineapi "github.com/kyverno/kyverno/pkg/engine/api"
enginectx "github.com/kyverno/kyverno/pkg/engine/context"
"github.com/kyverno/kyverno/pkg/engine/jmespath"
admissionutils "github.com/kyverno/kyverno/pkg/utils/admission"
@ -62,26 +61,6 @@ 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
}
@ -90,6 +69,18 @@ func (c *PolicyContext) OldResource() unstructured.Unstructured {
return c.oldResource
}
func (c *PolicyContext) SetResources(oldResource, newResource unstructured.Unstructured) error {
c.newResource = newResource
if err := c.jsonContext.AddResource(c.newResource.Object); err != nil {
return errors.Wrapf(err, "failed to replace object in the JSON context")
}
c.oldResource = oldResource
if err := c.jsonContext.AddOldResource(c.oldResource.Object); err != nil {
return errors.Wrapf(err, "failed to replace old object in the JSON context")
}
return nil
}
func (c *PolicyContext) RequestResource() metav1.GroupVersionResource {
return c.requestResource
}
@ -134,53 +125,42 @@ func (c *PolicyContext) JSONContext() enginectx.Interface {
return c.jsonContext
}
func (c PolicyContext) Copy() engineapi.PolicyContext {
return c.copy()
}
// Mutators
func (c *PolicyContext) WithPolicy(policy kyvernov1.PolicyInterface) *PolicyContext {
copy := c.copy()
copy.policy = policy
return copy
c.policy = policy
return c
}
func (c *PolicyContext) WithNamespaceLabels(namespaceLabels map[string]string) *PolicyContext {
copy := c.copy()
copy.namespaceLabels = namespaceLabels
return copy
c.namespaceLabels = namespaceLabels
return c
}
func (c *PolicyContext) WithAdmissionInfo(admissionInfo kyvernov1beta1.RequestInfo) *PolicyContext {
copy := c.copy()
copy.admissionInfo = admissionInfo
return copy
c.admissionInfo = admissionInfo
return c
}
func (c *PolicyContext) WithNewResource(resource unstructured.Unstructured) *PolicyContext {
copy := c.copy()
copy.newResource = resource
return copy
c.newResource = resource
return c
}
func (c *PolicyContext) WithOldResource(resource unstructured.Unstructured) *PolicyContext {
copy := c.copy()
copy.oldResource = resource
return copy
c.oldResource = resource
return c
}
func (c *PolicyContext) WithResourceKind(gvk schema.GroupVersionKind, subresource string) *PolicyContext {
copy := c.copy()
copy.gvk = gvk
copy.subresource = subresource
return copy
c.gvk = gvk
c.subresource = subresource
return c
}
func (c *PolicyContext) WithRequestResource(gvr metav1.GroupVersionResource) *PolicyContext {
copy := c.copy()
copy.requestResource = gvr
return copy
c.requestResource = gvr
return c
}
func (c *PolicyContext) WithResources(newResource unstructured.Unstructured, oldResource unstructured.Unstructured) *PolicyContext {
@ -188,13 +168,8 @@ func (c *PolicyContext) WithResources(newResource unstructured.Unstructured, old
}
func (c *PolicyContext) WithAdmissionOperation(admissionOperation bool) *PolicyContext {
copy := c.copy()
copy.admissionOperation = admissionOperation
return copy
}
func (c PolicyContext) copy() *PolicyContext {
return &c
c.admissionOperation = admissionOperation
return c
}
// Constructors

View file

@ -0,0 +1,88 @@
package policycontext
import (
"testing"
kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
"github.com/kyverno/kyverno/pkg/config"
"github.com/kyverno/kyverno/pkg/engine/jmespath"
kubeutils "github.com/kyverno/kyverno/pkg/utils/kube"
"github.com/stretchr/testify/assert"
)
var (
cfg = config.NewDefaultConfiguration(false)
jp = jmespath.New(cfg)
)
func Test_setResources(t *testing.T) {
newResource, err := kubeutils.BytesToUnstructured([]byte(`{
"apiVersion": "v1",
"kind": "Namespace",
"metadata": {
"labels": {
"kubernetes.io/metadata.name": "test",
"size": "small"
},
"name": "namespace1"
},
"spec": {}
}`))
assert.Nil(t, err)
oldResource, err := kubeutils.BytesToUnstructured([]byte(`{
"apiVersion": "v1",
"kind": "Namespace",
"metadata": {
"labels": {
"kubernetes.io/metadata.name": "test",
"size": "small"
},
"name": "namespace2"
},
"spec": {}
}`))
assert.Nil(t, err)
pc, err := NewPolicyContext(jp, *newResource, kyvernov1.Update, nil, cfg)
assert.Nil(t, err)
pc = pc.WithOldResource(*oldResource)
n := pc.NewResource()
assert.Equal(t, "namespace1", n.GetName())
o := pc.OldResource()
assert.Equal(t, "namespace2", o.GetName())
// swap resources
pc.SetResources(*newResource, *oldResource)
n = pc.NewResource()
assert.Equal(t, "namespace2", n.GetName())
name, err := pc.JSONContext().Query("request.object.metadata.name")
assert.Nil(t, err)
assert.Equal(t, "namespace2", name)
o = pc.OldResource()
assert.Equal(t, "namespace1", o.GetName())
name, err = pc.JSONContext().Query("request.oldObject.metadata.name")
assert.Nil(t, err)
assert.Equal(t, "namespace1", name)
// swap back resources
pc.SetResources(*oldResource, *newResource)
n = pc.NewResource()
assert.Equal(t, "namespace1", n.GetName())
name, err = pc.JSONContext().Query("request.object.metadata.name")
assert.Nil(t, err)
assert.Equal(t, "namespace1", name)
o = pc.OldResource()
assert.Equal(t, "namespace2", o.GetName())
name, err = pc.JSONContext().Query("request.oldObject.metadata.name")
assert.Nil(t, err)
assert.Equal(t, "namespace2", name)
}

View file

@ -24,7 +24,9 @@ func (h *resourceHandlers) handleBackgroundApplies(ctx context.Context, logger l
h.handleGenerate(ctx, logger, request, generatePolicies, policyContext, ts)
}
func (h *resourceHandlers) handleMutateExisting(ctx context.Context, logger logr.Logger, request admissionv1.AdmissionRequest, policies []kyvernov1.PolicyInterface, policyContext *engine.PolicyContext, admissionRequestTimestamp time.Time) {
func (h *resourceHandlers) handleMutateExisting(ctx context.Context, logger logr.Logger, request admissionv1.AdmissionRequest, policies []kyvernov1.PolicyInterface, polCtx *engine.PolicyContext, admissionRequestTimestamp time.Time) {
policyContext := &engine.PolicyContext{}
*policyContext = *polCtx
if request.Operation == admissionv1.Delete {
policyContext = policyContext.WithNewResource(policyContext.OldResource())
}

View file

@ -0,0 +1,11 @@
## Description
This test ensures that request.oldObject is not null on UPDATE operations when there are multiple rules in a policy.
## Expected Behavior
The namespace update operation is allowed.
## Reference Issue(s)
https://github.com/kyverno/kyverno/issues/9885

View file

@ -0,0 +1,23 @@
apiVersion: chainsaw.kyverno.io/v1alpha1
kind: Test
metadata:
creationTimestamp: null
name: check-old-object
spec:
steps:
- name: step-01
try:
- apply:
file: policy.yaml
- assert:
file: policy-ready.yaml
- name: step-02
try:
- apply:
file: ns.yaml
- assert:
file: ns-ready.yaml
- name: step-03
try:
- apply:
file: ns-update.yaml

View file

@ -0,0 +1,7 @@
apiVersion: v1
kind: Namespace
metadata:
name: test
labels:
kubernetes.io/metadata.name: test
size: large

View file

@ -0,0 +1,7 @@
apiVersion: v1
kind: Namespace
metadata:
name: test
labels:
kubernetes.io/metadata.name: test
size: small

View file

@ -0,0 +1,7 @@
apiVersion: v1
kind: Namespace
metadata:
name: test
labels:
kubernetes.io/metadata.name: test
size: large

View file

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

View file

@ -0,0 +1,39 @@
apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
name: check-old-object
spec:
validationFailureAction: Enforce
background: false
rules:
- name: require-labels
match:
all:
- resources:
operations:
- CREATE
- UPDATE
kinds:
- Namespace
validate:
message: "The label `size` is required"
pattern:
metadata:
labels:
size: "small | medium | large"
- name: check-old-object
match:
all:
- resources:
operations:
- UPDATE
kinds:
- Namespace
validate:
message: "request.oldObject cannot be null for update requests"
deny:
conditions:
all:
- key: "{{ request.oldObject.metadata == null }}"
operator: Equals
value: true