1
0
Fork 0
mirror of https://github.com/kyverno/kyverno.git synced 2024-12-15 17:51:20 +00:00

fix: Delete downstream objects on precondition fail (#7496)

* fix: Delete downstream objects on precondition fail

When a rule fails the match in a generate rule, the downstream resource gets deleted. This will now also happen if the rule is skipped due to a precondition.

Signed-off-by: Mike Bryant <mike.bryant@mettle.co.uk>

* add debug command

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

* sync trigger updates to downstream

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

* fix bgscan fetching trigger

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

* fix: Move rbac change into tests for better isolation

Signed-off-by: Mike Bryant <mike.bryant@mettle.co.uk>

* fix unit test

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

---------

Signed-off-by: Mike Bryant <mike.bryant@mettle.co.uk>
Signed-off-by: ShutingZhao <shuting@nirmata.com>
Co-authored-by: shuting <shuting@nirmata.com>
This commit is contained in:
Mike Bryant 2023-06-15 16:32:19 +01:00 committed by GitHub
parent 8e86ad3bcf
commit 91021b65b6
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
46 changed files with 544 additions and 118 deletions

View file

@ -9,6 +9,7 @@ runs:
run: |
kubectl get mutatingwebhookconfigurations
kubectl get validatingwebhookconfigurations
kubectl auth can-i --list --as system:serviceaccount:kyverno:kyverno-background-controller
- shell: bash
run: |
kubectl -n kyverno get pod

View file

@ -69,7 +69,6 @@ func (e *engine) filterRule(
newResource := policyContext.NewResource()
oldResource := policyContext.OldResource()
admissionInfo := policyContext.AdmissionInfo()
ctx := policyContext.JSONContext()
namespaceLabels := policyContext.NamespaceLabels()
policy := policyContext.Policy()
gvk, subresource := policyContext.ResourceKind()
@ -102,13 +101,28 @@ func (e *engine) filterRule(
}
// evaluate pre-conditions
if val, msg, err := variables.EvaluateConditions(logger, ctx, copyConditions); err != nil {
pass, msg, err := variables.EvaluateConditions(logger, policyContext.JSONContext(), copyConditions)
if err != nil {
return engineapi.RuleError(rule.Name, ruleType, "failed to evaluate conditions", err)
} else if !val {
logger.V(4).Info("skip rule as preconditions are not met", "rule", rule.Name, "message", msg)
return engineapi.RuleSkip(rule.Name, ruleType, "")
}
// build rule Response
return engineapi.RulePass(rule.Name, ruleType, "")
if pass {
return engineapi.RulePass(rule.Name, ruleType, "")
}
if policyContext.OldResource().Object != nil {
if err = policyContext.JSONContext().AddResource(policyContext.OldResource().Object); err != nil {
return engineapi.RuleError(rule.Name, ruleType, "failed to update JSON context for old resource", err)
}
if val, msg, err := variables.EvaluateConditions(logger, policyContext.JSONContext(), copyConditions); err != nil {
return engineapi.RuleError(rule.Name, ruleType, "failed to evaluate conditions for old resource", err)
} else {
if val {
return engineapi.RuleFail(rule.Name, ruleType, msg)
}
}
}
logger.V(4).Info("skip rule as preconditions are not met", "rule", rule.Name, "message", msg)
return engineapi.RuleSkip(rule.Name, ruleType, "")
}

View file

@ -27,6 +27,7 @@ func (pc *policyController) handleGenerate(policyKey string, policy kyvernov1.Po
return nil
}
logger.V(4).Info("reconcile policy with generateExisting enabled")
if err := pc.handleGenerateForExisting(policy); err != nil {
logger.Error(err, "failed to create UR for generateExisting")
return err
@ -104,6 +105,7 @@ func (pc *policyController) syncDataRulechanges(policy kyvernov1.PolicyInterface
return err
}
pc.log.V(4).Info("sync data rule changes to downstream targets")
var errorList []error
for _, downstream := range downstreams.Items {
labels := downstream.GetLabels()

View file

@ -3,51 +3,21 @@ package policy
import (
kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/util/sets"
)
// check if all slice elements are same
func isMatchResourcesAllValid(rule kyvernov1.Rule) bool {
var kindlist []string
for _, all := range rule.MatchResources.All {
kindlist = append(kindlist, all.Kinds...)
}
if len(kindlist) == 0 {
return false
}
for i := 1; i < len(kindlist); i++ {
if kindlist[i] != kindlist[0] {
return false
}
}
return true
}
func fetchUniqueKinds(rule kyvernov1.Rule) []string {
var kindlist []string
kinds := sets.New(rule.MatchResources.Kinds...)
kindlist = append(kindlist, rule.MatchResources.Kinds...)
for _, all := range rule.MatchResources.Any {
kindlist = append(kindlist, all.Kinds...)
for _, any := range rule.MatchResources.Any {
kinds.Insert(any.Kinds...)
}
if isMatchResourcesAllValid(rule) {
for _, all := range rule.MatchResources.All {
kindlist = append(kindlist, all.Kinds...)
}
for _, all := range rule.MatchResources.All {
kinds.Insert(all.Kinds...)
}
inResult := make(map[string]bool)
var result []string
for _, kind := range kindlist {
if _, ok := inResult[kind]; !ok {
inResult[kind] = true
result = append(result, kind)
}
}
return result
return kinds.UnsortedList()
}
func convertlist(ulists []unstructured.Unstructured) []*unstructured.Unstructured {

View file

@ -1,84 +1,15 @@
package policy
import (
"fmt"
"testing"
kyverno "github.com/kyverno/kyverno/api/kyverno/v1"
kubeutils "github.com/kyverno/kyverno/pkg/utils/kube"
"gotest.tools/assert"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
)
func Test_isMatchResourcesAllValid(t *testing.T) {
tests := []struct {
name string
rule kyverno.Rule
want bool
}{
{
name: "All with no kinds are invalid",
rule: kyverno.Rule{
MatchResources: kyverno.MatchResources{
All: []kyverno.ResourceFilter{
{
ResourceDescription: kyverno.ResourceDescription{
Kinds: []string{},
},
},
},
},
},
want: false,
},
{
name: "All with same kind are valid",
rule: kyverno.Rule{
MatchResources: kyverno.MatchResources{
All: []kyverno.ResourceFilter{
{
ResourceDescription: kyverno.ResourceDescription{
Kinds: []string{"kind1"},
},
},
{
ResourceDescription: kyverno.ResourceDescription{
Kinds: []string{"kind1"},
},
},
},
},
},
want: true,
},
{
name: "All with different kinds are invalid",
rule: kyverno.Rule{
MatchResources: kyverno.MatchResources{
All: []kyverno.ResourceFilter{
{
ResourceDescription: kyverno.ResourceDescription{
Kinds: []string{"kind1"},
},
},
{
ResourceDescription: kyverno.ResourceDescription{
Kinds: []string{"kind2"},
},
},
},
},
},
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.DeepEqual(t, tt.want, isMatchResourcesAllValid(tt.rule))
})
}
}
func Test_fetchUniqueKinds(t *testing.T) {
tests := []struct {
@ -160,12 +91,17 @@ func Test_fetchUniqueKinds(t *testing.T) {
},
},
},
want: []string{"kind1", "kind2"},
want: []string{"kind1", "kind2", "kind4", "kind5"},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.DeepEqual(t, fetchUniqueKinds(tt.rule), tt.want)
kinds := fetchUniqueKinds(tt.rule)
for _, want := range tt.want {
if !kubeutils.ContainsKind(kinds, want) {
assert.Error(t, fmt.Errorf("%s fails, expected %s", tt.name, want), "")
}
}
})
}
}

View file

@ -176,7 +176,7 @@ func (h *generationHandler) applyGeneration(
}
// handleFailedRules sync changes of the trigger to the downstream
// it can be 1. trigger deletion; 2. trigger no longer matches, when a rule fails
// it can be 1. trigger deletion; 2. trigger no longer matches, when a rule fails or is skipped
func (h *generationHandler) syncTriggerAction(
ctx context.Context,
request admissionv1.AdmissionRequest,

View file

@ -0,0 +1,12 @@
apiVersion: v1
kind: Namespace
metadata:
name: cpol-clone-sync-existing-update-trigger-no-precondition-ns
---
apiVersion: v1
kind: ConfigMap
metadata:
labels:
create: "true"
name: test-org
namespace: cpol-clone-sync-existing-update-trigger-no-precondition-ns

View file

@ -0,0 +1,9 @@
apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
name: cpol-clone-sync-existing-update-trigger-no-precondition
status:
conditions:
- reason: Succeeded
status: "True"
type: Ready

View file

@ -0,0 +1,36 @@
apiVersion: v1
data:
foo: YmFy
kind: Secret
metadata:
name: source-secret
namespace: cpol-clone-sync-existing-update-trigger-no-precondition-ns
type: Opaque
---
apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
name: cpol-clone-sync-existing-update-trigger-no-precondition
spec:
generateExisting: true
rules:
- name: clone-secret
match:
any:
- resources:
kinds:
- ConfigMap
preconditions:
any:
- key: "{{ request.object.metadata.labels.create || '' }}"
operator: Equals
value: "true"
generate:
apiVersion: v1
kind: Secret
name: downstream-secret
namespace: "{{request.object.metadata.namespace}}"
synchronize: true
clone:
namespace: cpol-clone-sync-existing-update-trigger-no-precondition-ns
name: source-secret

View file

@ -0,0 +1,4 @@
apiVersion: kuttl.dev/v1beta1
kind: TestStep
commands:
- command: sleep 3

View file

@ -0,0 +1,4 @@
apiVersion: kuttl.dev/v1beta1
kind: TestStep
assert:
- downstream.yaml

View file

@ -0,0 +1,7 @@
apiVersion: v1
kind: ConfigMap
metadata:
labels:
create: "false"
name: test-org
namespace: cpol-clone-sync-existing-update-trigger-no-precondition-ns

View file

@ -0,0 +1,4 @@
apiVersion: kuttl.dev/v1beta1
kind: TestStep
commands:
- command: sleep 3

View file

@ -0,0 +1,4 @@
apiVersion: kuttl.dev/v1beta1
kind: TestStep
error:
- downstream.yaml

View file

@ -0,0 +1,11 @@
## Description
This test checks to ensure that updates to a trigger which cause it to no longer match a precondition of the rule, with a generate clone declaration and sync enabled, results in the downstream resource's deletion.
## Expected Behavior
If the downstream resource is deleted, the test passes. If it remains, the test fails.
## Reference Issue(s)
https://github.com/kyverno/kyverno/issues/7481

View file

@ -0,0 +1,8 @@
apiVersion: v1
data:
foo: YmFy
kind: Secret
metadata:
name: downstream-secret
namespace: cpol-clone-sync-existing-update-trigger-no-precondition-ns
type: Opaque

View file

@ -0,0 +1,9 @@
apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
name: cpol-clone-sync-no-existing-update-trigger-no-precondition
status:
conditions:
- reason: Succeeded
status: "True"
type: Ready

View file

@ -0,0 +1,40 @@
apiVersion: v1
kind: Namespace
metadata:
name: cpol-clone-sync-no-existing-update-trigger-no-precondition-ns
---
apiVersion: v1
data:
foo: YmFy
kind: Secret
metadata:
name: source-secret
namespace: cpol-clone-sync-no-existing-update-trigger-no-precondition-ns
type: Opaque
---
apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
name: cpol-clone-sync-no-existing-update-trigger-no-precondition
spec:
rules:
- name: clone-secret
match:
any:
- resources:
kinds:
- ConfigMap
preconditions:
any:
- key: "{{ request.object.metadata.labels.create || '' }}"
operator: Equals
value: "true"
generate:
apiVersion: v1
kind: Secret
name: downstream-secret
namespace: "{{request.object.metadata.namespace}}"
synchronize: true
clone:
namespace: cpol-clone-sync-no-existing-update-trigger-no-precondition-ns
name: source-secret

View file

@ -0,0 +1,7 @@
apiVersion: v1
kind: ConfigMap
metadata:
labels:
create: "true"
name: test-org
namespace: cpol-clone-sync-no-existing-update-trigger-no-precondition-ns

View file

@ -0,0 +1,4 @@
apiVersion: kuttl.dev/v1beta1
kind: TestStep
assert:
- downstream.yaml

View file

@ -0,0 +1,7 @@
apiVersion: v1
kind: ConfigMap
metadata:
labels:
create: "false"
name: test-org
namespace: cpol-clone-sync-no-existing-update-trigger-no-precondition-ns

View file

@ -0,0 +1,4 @@
apiVersion: kuttl.dev/v1beta1
kind: TestStep
commands:
- command: sleep 3

View file

@ -0,0 +1,4 @@
apiVersion: kuttl.dev/v1beta1
kind: TestStep
error:
- downstream.yaml

View file

@ -0,0 +1,11 @@
## Description
This test checks to ensure that updates to a trigger which cause it to no longer match a precondition of the rule, with a generate clone declaration and sync enabled, results in the downstream resource's deletion.
## Expected Behavior
If the downstream resource is deleted, the test passes. If it remains, the test fails.
## Reference Issue(s)
https://github.com/kyverno/kyverno/issues/7481

View file

@ -0,0 +1,8 @@
apiVersion: v1
data:
foo: YmFy
kind: Secret
metadata:
name: downstream-secret
namespace: cpol-clone-sync-no-existing-update-trigger-no-precondition-ns
type: Opaque

View file

@ -0,0 +1,20 @@
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
labels:
app.kubernetes.io/component: background-controller
app.kubernetes.io/instance: kyverno
app.kubernetes.io/part-of: kyverno
name: kyverno:background-controller:pdb
rules:
- apiGroups:
- '*'
resources:
- poddisruptionbudgets
verbs:
- create
- update
- patch
- delete
- get
- list

View file

@ -0,0 +1,27 @@
apiVersion: v1
kind: Namespace
metadata:
name: cpol-data-sync-existing-update-trigger-no-precondition-ns
---
apiVersion: apps/v1
kind: Deployment
metadata:
name: test
namespace: cpol-data-sync-existing-update-trigger-no-precondition-ns
spec:
selector:
matchLabels:
app.kubernetes.io/instance: test
app.kubernetes.io/name: nginx
replicas: 1
template:
metadata:
labels:
app.kubernetes.io/instance: test
app.kubernetes.io/name: nginx
spec:
containers:
- name: nginx
image: nginx:1.14.2
ports:
- containerPort: 80

View file

@ -0,0 +1,9 @@
apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
name: cpol-data-sync-existing-update-trigger-no-precondition
status:
conditions:
- reason: Succeeded
status: "True"
type: Ready

View file

@ -0,0 +1,31 @@
apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
name: cpol-data-sync-existing-update-trigger-no-precondition
spec:
generateExisting: true
rules:
- name: create-default-pdb
match:
all:
- resources:
kinds:
- Deployment
- StatefulSet
preconditions:
all:
- key: "{{ request.object.spec.replicas }}"
operator: GreaterThan
value: 1
generate:
synchronize: true
apiVersion: policy/v1
kind: PodDisruptionBudget
name: "{{request.object.metadata.name}}-default"
namespace: "{{request.object.metadata.namespace}}"
data:
spec:
minAvailable: 50%
selector:
matchLabels: >-
{{ not_null(request.object.spec.selector.matchLabels, request.object.spec.template.metadata.labels) }}

View file

@ -0,0 +1,23 @@
---
apiVersion: apps/v1
kind: Deployment
metadata:
name: test
namespace: cpol-data-sync-existing-update-trigger-no-precondition-ns
spec:
selector:
matchLabels:
app.kubernetes.io/instance: test
app.kubernetes.io/name: nginx
replicas: 2
template:
metadata:
labels:
app.kubernetes.io/instance: test
app.kubernetes.io/name: nginx
spec:
containers:
- name: nginx
image: nginx:1.14.2
ports:
- containerPort: 80

View file

@ -0,0 +1,4 @@
apiVersion: kuttl.dev/v1beta1
kind: TestStep
assert:
- downstream.yaml

View file

@ -0,0 +1,23 @@
---
apiVersion: apps/v1
kind: Deployment
metadata:
name: test
namespace: cpol-data-sync-existing-update-trigger-no-precondition-ns
spec:
selector:
matchLabels:
app.kubernetes.io/instance: test
app.kubernetes.io/name: nginx
replicas: 1
template:
metadata:
labels:
app.kubernetes.io/instance: test
app.kubernetes.io/name: nginx
spec:
containers:
- name: nginx
image: nginx:1.14.2
ports:
- containerPort: 80

View file

@ -0,0 +1,4 @@
apiVersion: kuttl.dev/v1beta1
kind: TestStep
commands:
- command: sleep 3

View file

@ -0,0 +1,4 @@
apiVersion: kuttl.dev/v1beta1
kind: TestStep
error:
- downstream.yaml

View file

@ -0,0 +1,11 @@
## Description
This test checks to ensure that updates to a trigger which cause it to no longer match a precondition of the rule, with a generate data declaration and sync enabled, results in the downstream resource's deletion.
## Expected Behavior
If the downstream resource is deleted, the test passes. If it remains, the test fails.
## Reference Issue(s)
https://github.com/kyverno/kyverno/issues/7481

View file

@ -0,0 +1,11 @@
apiVersion: policy/v1
kind: PodDisruptionBudget
metadata:
name: test-default
namespace: cpol-data-sync-existing-update-trigger-no-precondition-ns
spec:
minAvailable: 50%
selector:
matchLabels:
app.kubernetes.io/instance: test
app.kubernetes.io/name: nginx

View file

@ -0,0 +1,20 @@
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
labels:
app.kubernetes.io/component: background-controller
app.kubernetes.io/instance: kyverno
app.kubernetes.io/part-of: kyverno
name: kyverno:background-controller:pdb
rules:
- apiGroups:
- '*'
resources:
- poddisruptionbudgets
verbs:
- create
- update
- patch
- delete
- get
- list

View file

@ -0,0 +1,9 @@
apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
name: cpol-data-sync-no-existing-update-trigger-no-precondition
status:
conditions:
- reason: Succeeded
status: "True"
type: Ready

View file

@ -0,0 +1,30 @@
apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
name: cpol-data-sync-no-existing-update-trigger-no-precondition
spec:
rules:
- name: create-default-pdb
match:
all:
- resources:
kinds:
- Deployment
- StatefulSet
preconditions:
all:
- key: "{{ request.object.spec.replicas }}"
operator: GreaterThan
value: 1
generate:
synchronize: true
apiVersion: policy/v1
kind: PodDisruptionBudget
name: "{{request.object.metadata.name}}-default"
namespace: "{{request.object.metadata.namespace}}"
data:
spec:
minAvailable: 50%
selector:
matchLabels: >-
{{ not_null(request.object.spec.selector.matchLabels, request.object.spec.template.metadata.labels) }}

View file

@ -0,0 +1,27 @@
apiVersion: v1
kind: Namespace
metadata:
name: cpol-data-sync-no-existing-update-trigger-no-precondition-ns
---
apiVersion: apps/v1
kind: Deployment
metadata:
name: test
namespace: cpol-data-sync-no-existing-update-trigger-no-precondition-ns
spec:
selector:
matchLabels:
app.kubernetes.io/instance: test
app.kubernetes.io/name: nginx
replicas: 2
template:
metadata:
labels:
app.kubernetes.io/instance: test
app.kubernetes.io/name: nginx
spec:
containers:
- name: nginx
image: nginx:1.14.2
ports:
- containerPort: 80

View file

@ -0,0 +1,4 @@
apiVersion: kuttl.dev/v1beta1
kind: TestStep
assert:
- downstream.yaml

View file

@ -0,0 +1,23 @@
---
apiVersion: apps/v1
kind: Deployment
metadata:
name: test
namespace: cpol-data-sync-no-existing-update-trigger-no-precondition-ns
spec:
selector:
matchLabels:
app.kubernetes.io/instance: test
app.kubernetes.io/name: nginx
replicas: 1
template:
metadata:
labels:
app.kubernetes.io/instance: test
app.kubernetes.io/name: nginx
spec:
containers:
- name: nginx
image: nginx:1.14.2
ports:
- containerPort: 80

View file

@ -0,0 +1,4 @@
apiVersion: kuttl.dev/v1beta1
kind: TestStep
commands:
- command: sleep 3

View file

@ -0,0 +1,4 @@
apiVersion: kuttl.dev/v1beta1
kind: TestStep
error:
- downstream.yaml

View file

@ -0,0 +1,11 @@
## Description
This test checks to ensure that updates to a trigger which cause it to no longer match a precondition of the rule, with a generate data declaration and sync enabled, results in the downstream resource's deletion.
## Expected Behavior
If the downstream resource is deleted, the test passes. If it remains, the test fails.
## Reference Issue(s)
https://github.com/kyverno/kyverno/issues/7481

View file

@ -0,0 +1,11 @@
apiVersion: policy/v1
kind: PodDisruptionBudget
metadata:
name: test-default
namespace: cpol-data-sync-no-existing-update-trigger-no-precondition-ns
spec:
minAvailable: 50%
selector:
matchLabels:
app.kubernetes.io/instance: test
app.kubernetes.io/name: nginx