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: properly verify precondition in old object validation (#11644)

Browse source 
* fix: properly verify precondition in old object validation

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

* fix: tests

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

* fix: assert bug

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

* fix: properly update the values

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 2024-12-04 12:02:12 +05:30 committed by GitHub
parent 8746a8ffbb
commit 174534b518
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 184 additions and 41 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
pkg/engine/api/policycontext.go
View file

@ -17,9 +17,9 @@ type PolicyContext interface {
NewResource() unstructured.Unstructured
OldResource() unstructured.Unstructured
SetResources(oldResource, newResource unstructured.Unstructured) error
SetOperation(kyvernov1.AdmissionOperation) error
AdmissionInfo() kyvernov2.RequestInfo
Operation() kyvernov1.AdmissionOperation
SetOperation(op kyvernov1.AdmissionOperation) error
NamespaceLabels() map[string]string
RequestResource() metav1.GroupVersionResource
ResourceKind() (schema.GroupVersionKind, string)

12
pkg/engine/handlers/validation/utils.go
View file

@ -1,14 +1,24 @@
package validation
import (
"github.com/go-logr/logr"
kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
kyvernov2 "github.com/kyverno/kyverno/api/kyverno/v2"
enginecontext "github.com/kyverno/kyverno/pkg/engine/context"
"github.com/kyverno/kyverno/pkg/engine/internal"
engineutils "github.com/kyverno/kyverno/pkg/engine/utils"
authenticationv1 "k8s.io/api/authentication/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
)
func matchResource(resource unstructured.Unstructured, rule kyvernov1.Rule, namespaceLabels map[string]string, policyNamespace string, operation kyvernov1.AdmissionOperation) bool {
func matchResource(logger logr.Logger, resource unstructured.Unstructured, rule kyvernov1.Rule, namespaceLabels map[string]string, policyNamespace string, operation kyvernov1.AdmissionOperation, jsonContext enginecontext.Interface) bool {
if rule.RawAnyAllConditions != nil {
preconditionsPassed, _, err := internal.CheckPreconditions(logger, jsonContext, rule.RawAnyAllConditions)
if !preconditionsPassed || err != nil {
return false
}
}
// cannot use admission info from the current request as the user can be different, if the rule matches on old request user info, it should skip
admissionInfo := kyvernov2.RequestInfo{
Roles: []string{"kyverno:invalidrole"},

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

@ -16,6 +16,7 @@ import (
enginectx "github.com/kyverno/kyverno/pkg/engine/context"
"github.com/kyverno/kyverno/pkg/engine/handlers"
engineutils "github.com/kyverno/kyverno/pkg/engine/utils"
"github.com/pkg/errors"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/client-go/tools/cache"
@ -123,7 +124,7 @@ func (h validateAssertHandler) Process(
if action.Enforce() {
allowExisitingViolations := rule.HasValidateAllowExistingViolations()
if engineutils.IsUpdateRequest(policyContext) && allowExisitingViolations {
errs, err := validateOldObject(ctx, policyContext, rule, payload, bindings)
errs, err := validateOldObject(ctx, logger, 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")
@ -149,25 +150,37 @@ func (h validateAssertHandler) Process(
)
}
func validateOldObject(ctx context.Context, policyContext engineapi.PolicyContext, rule kyvernov1.Rule, payload map[string]any, bindings binding.Bindings) (field.ErrorList, error) {
func validateOldObject(ctx context.Context, logger logr.Logger, policyContext engineapi.PolicyContext, rule kyvernov1.Rule, payload map[string]any, bindings binding.Bindings) (errs field.ErrorList, err error) {
if policyContext.Operation() != kyvernov1.Update {
return nil, nil
}
oldResource := policyContext.OldResource()
if ok := matchResource(oldResource, rule, policyContext.NamespaceLabels(), policyContext.Policy().GetNamespace(), kyvernov1.Create); !ok {
return nil, nil
if err := policyContext.SetOperation(kyvernov1.Create); err != nil { // simulates the condition when old object was "created"
return nil, errors.Wrapf(err, "failed to set operation")
}
payload["object"] = policyContext.OldResource().Object
payload["oldObject"] = nil
payload["operation"] = kyvernov1.Create
asserttion := assert.Parse(ctx, rule.Validation.Assert.Value)
errs, err := assert.Assert(ctx, nil, asserttion, payload, bindings)
if err != nil {
return nil, fmt.Errorf("failed to apply assertion: %w", err)
defer func() {
if err = policyContext.SetOperation(kyvernov1.Update); err != nil {
logger.Error(errors.Wrapf(err, "failed to reset operation"), "")
}
payload["object"] = policyContext.NewResource().Object
payload["oldObject"] = policyContext.OldResource().Object
payload["operation"] = kyvernov1.Update
}()
if ok := matchResource(logger, oldResource, rule, policyContext.NamespaceLabels(), policyContext.Policy().GetNamespace(), kyvernov1.Create, policyContext.JSONContext()); !ok {
return
}
return errs, nil
assertion := assert.Parse(ctx, rule.Validation.Assert.Value)
errs, err = assert.Assert(ctx, nil, assertion, payload, bindings)
return
}

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

@ -172,7 +172,7 @@ func (h validatePssHandler) validateOldObject(
rule kyvernov1.Rule,
engineLoader engineapi.EngineContextLoader,
exceptions []*kyvernov2.PolicyException,
) (*engineapi.RuleResponse, error) {
) (resp *engineapi.RuleResponse, err error) {
if policyContext.Operation() != kyvernov1.Update {
return nil, nil
}
@ -181,27 +181,30 @@ func (h validatePssHandler) validateOldObject(
oldResource := policyContext.OldResource()
emptyResource := unstructured.Unstructured{}
if ok := matchResource(oldResource, rule, policyContext.NamespaceLabels(), policyContext.Policy().GetNamespace(), kyvernov1.Create); !ok {
return nil, nil
}
if err := policyContext.SetResources(emptyResource, oldResource); err != nil {
if err = policyContext.SetResources(emptyResource, oldResource); err != nil {
return nil, errors.Wrapf(err, "failed to set resources")
}
if err := policyContext.SetOperation(kyvernov1.Create); err != nil { // simulates the condition when old object was "created"
if err = policyContext.SetOperation(kyvernov1.Create); err != nil { // simulates the condition when old object was "created"
return nil, errors.Wrapf(err, "failed to set operation")
}
_, resp := h.validate(ctx, logger, policyContext, oldResource, rule, engineLoader, exceptions)
defer func() {
if err = policyContext.SetResources(oldResource, newResource); err != nil {
logger.Error(errors.Wrapf(err, "failed to reset resources"), "")
}
if err := policyContext.SetResources(oldResource, newResource); err != nil {
return nil, errors.Wrapf(err, "failed to reset resources")
if err = policyContext.SetOperation(kyvernov1.Update); err != nil {
logger.Error(errors.Wrapf(err, "failed to reset operations"), "")
}
}()
if ok := matchResource(logger, oldResource, rule, policyContext.NamespaceLabels(), policyContext.Policy().GetNamespace(), kyvernov1.Create, policyContext.JSONContext()); !ok {
return
}
if err := policyContext.SetOperation(kyvernov1.Update); err != nil {
return nil, errors.Wrapf(err, "failed to reset operation")
}
_, resp = h.validate(ctx, logger, policyContext, oldResource, rule, engineLoader, exceptions)
return resp, nil
return
}
func convertChecks(checks []pssutils.PSSCheckResult, kind string) (newChecks []pssutils.PSSCheckResult) {

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

@ -175,7 +175,7 @@ func (v *validator) validate(ctx context.Context) *engineapi.RuleResponse {
return ruleResponse
}
func (v *validator) validateOldObject(ctx context.Context) (*engineapi.RuleResponse, error) {
func (v *validator) validateOldObject(ctx context.Context) (resp *engineapi.RuleResponse, err error) {
if v.policyContext.Operation() != kyvernov1.Update {
return nil, errors.New("invalid operation")
}
@ -184,28 +184,32 @@ func (v *validator) validateOldObject(ctx context.Context) (*engineapi.RuleRespo
oldResource := v.policyContext.OldResource()
emptyResource := unstructured.Unstructured{}
if ok := matchResource(oldResource, v.rule, v.policyContext.NamespaceLabels(), v.policyContext.Policy().GetNamespace(), kyvernov1.Create); !ok {
return engineapi.RuleSkip(v.rule.Name, engineapi.Validation, "resource not matched", v.rule.ReportProperties), nil
}
if err := v.policyContext.SetResources(emptyResource, oldResource); err != nil {
if err = v.policyContext.SetResources(emptyResource, oldResource); err != nil {
return nil, errors.Wrapf(err, "failed to set resources")
}
if err := v.policyContext.SetOperation(kyvernov1.Create); err != nil { // simulates the condition when old object was "created"
if err = v.policyContext.SetOperation(kyvernov1.Create); err != nil { // simulates the condition when old object was "created"
return nil, errors.Wrapf(err, "failed to set operation")
}
resp := v.validate(ctx)
defer func() {
if err = v.policyContext.SetResources(oldResource, newResource); err != nil {
v.log.Error(errors.Wrapf(err, "failed to reset resources"), "")
}
if err := v.policyContext.SetResources(oldResource, newResource); err != nil {
return nil, errors.Wrapf(err, "failed to reset resources")
if err = v.policyContext.SetOperation(kyvernov1.Update); err != nil {
v.log.Error(errors.Wrapf(err, "failed to reset operation"), "")
}
}()
if ok := matchResource(v.log, oldResource, v.rule, v.policyContext.NamespaceLabels(), v.policyContext.Policy().GetNamespace(), kyvernov1.Create, v.policyContext.JSONContext()); !ok {
resp = engineapi.RuleSkip(v.rule.Name, engineapi.Validation, "resource not matched", nil)
return
}
if err := v.policyContext.SetOperation(kyvernov1.Update); err != nil {
return nil, errors.Wrapf(err, "failed to reset operation")
}
resp = v.validate(ctx)
return resp, nil
return
}
func (v *validator) validateForEach(ctx context.Context) *engineapi.RuleResponse {

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

@ -72,7 +72,7 @@ var (
"UPDATE"
],
"kinds": [
"Namespace"
"Pod"
]
}
}
@ -100,7 +100,7 @@ var (
"UPDATE"
],
"kinds": [
"Namespace"
"Pod"
]
}
}
@ -151,7 +151,7 @@ var (
"name": "check-image",
"validate": {
"failureAction": "Enforce",
"allowExistingViolations": true,
"allowExistingViolations": true,
"foreach": [
{
"deny": {

9
test/conformance/chainsaw/validate/clusterpolicy/standard/enforce/validate-existing-with-precondition/README.md Normal file
View file

@ -0,0 +1,9 @@
## Description
This test verifies that preconditions are respected when validating old object
## Expected Behavior
1. A policy is created that matches only update operations
2. An ingress is created
3. An update is sent to the ingress, since the policy did not match create operation in precondition the validation should not skip in this case because of skip existing violation behaviour.

33
test/conformance/chainsaw/validate/clusterpolicy/standard/enforce/validate-existing-with-precondition/chainsaw-test.yaml Executable file
View file

@ -0,0 +1,33 @@
apiVersion: chainsaw.kyverno.io/v1alpha1
kind: Test
metadata:
name: enforce-validate-existing
spec:
steps:
- name: create policy
use:
template: ../../../../../_step-templates/create-policy.yaml
with:
bindings:
- name: file
value: policy.yaml
- name: wait policy ready
use:
template: ../../../../../_step-templates/cluster-policy-ready.yaml
with:
bindings:
- name: name
value: prevent-ingress-annotation
- name: step-02
try:
- apply:
file: ingress.yaml
- assert:
file: ingress-ready.yaml
- name: step-03
try:
- apply:
expect:
- check:
($error != null): true
file: ingress-update.yaml

4
test/conformance/chainsaw/validate/clusterpolicy/standard/enforce/validate-existing-with-precondition/ingress-ready.yaml Normal file
View file

@ -0,0 +1,4 @@
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
name: minimal-ingress

18
test/conformance/chainsaw/validate/clusterpolicy/standard/enforce/validate-existing-with-precondition/ingress-update.yaml Normal file
View file

@ -0,0 +1,18 @@
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
name: minimal-ingress
annotations:
alb.ingress.kubernetes.io/scheme: test-update
spec:
ingressClassName: nginx-example
rules:
- http:
paths:
- path: /testpath
pathType: Prefix
backend:
service:
name: test
port:
number: 80

18
test/conformance/chainsaw/validate/clusterpolicy/standard/enforce/validate-existing-with-precondition/ingress.yaml Normal file
View file

@ -0,0 +1,18 @@
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
name: minimal-ingress
annotations:
alb.ingress.kubernetes.io/scheme: test
spec:
ingressClassName: nginx-example
rules:
- http:
paths:
- path: /testpath
pathType: Prefix
backend:
service:
name: test
port:
number: 80

31
test/conformance/chainsaw/validate/clusterpolicy/standard/enforce/validate-existing-with-precondition/policy.yaml Normal file
View file

@ -0,0 +1,31 @@
apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
name: prevent-ingress-annotation
spec:
validationFailureAction: Enforce
rules:
- name: prevent-ingress-annotation-subnet
match:
any:
- resources:
kinds:
- Ingress
preconditions:
any:
- key: "{{request.operation || 'BACKGROUND'}}"
operator: Equals
value: UPDATE
validate:
message: |
alb.ingress.kubernetes.io/scheme cannot be updated
pattern:
deny:
conditions:
any:
- key: "{{ request.object.metadata.annotations.\"alb.ingress.kubernetes.io/scheme\" || '' }}"
operator: Equals
value: ""
- key: "{{ request.object.metadata.annotations.\"alb.ingress.kubernetes.io/scheme\" }}"
operator: NotEquals
value: "{{ request.oldObject.metadata.annotations.\"alb.ingress.kubernetes.io/scheme\" }}"