From b51fdacb35600ea75f61a2e359c1ea55c9c8d611 Mon Sep 17 00:00:00 2001 From: ansalamdaniel Date: Mon, 28 Nov 2022 17:20:04 +0530 Subject: [PATCH] Fix: handling unexpected global-anchor-variable for the apply command Signed-off-by: ansalamdaniel --- .../mutate/patch/strategicPreprocessing.go | 16 +++++- .../patch/strategicPreprocessing_test.go | 37 +++++++++++++ .../global-anchor/kyverno-test.yaml | 28 ++++++++++ .../global-anchor/patchedResource.yaml | 14 +++++ .../patchedResourceWithVolume.yaml | 18 ++++++ .../cli/test-mutate/global-anchor/policy.yaml | 25 +++++++++ .../test-mutate/global-anchor/resources.yaml | 55 +++++++++++++++++++ 7 files changed, 192 insertions(+), 1 deletion(-) create mode 100644 test/cli/test-mutate/global-anchor/kyverno-test.yaml create mode 100644 test/cli/test-mutate/global-anchor/patchedResource.yaml create mode 100644 test/cli/test-mutate/global-anchor/patchedResourceWithVolume.yaml create mode 100644 test/cli/test-mutate/global-anchor/policy.yaml create mode 100644 test/cli/test-mutate/global-anchor/resources.yaml diff --git a/pkg/engine/mutate/patch/strategicPreprocessing.go b/pkg/engine/mutate/patch/strategicPreprocessing.go index 3e268215ad7a..cdf473ce58d6 100644 --- a/pkg/engine/mutate/patch/strategicPreprocessing.go +++ b/pkg/engine/mutate/patch/strategicPreprocessing.go @@ -137,9 +137,9 @@ func processListOfMaps(logger logr.Logger, pattern, resource *yaml.RNode) error if hasAnyAnchor { anyGlobalConditionPassed := false var lastGlobalAnchorError error = nil + patternElementCopy := patternElement.Copy() for _, resourceElement := range resourceElements { - patternElementCopy := patternElement.Copy() if err := preProcessRecursive(logger, patternElementCopy, resourceElement); err != nil { logger.V(3).Info("anchor mismatch", "reason", err.Error()) if isConditionError(err) { @@ -163,7 +163,21 @@ func processListOfMaps(logger logr.Logger, pattern, resource *yaml.RNode) error } } } + if resource == nil { + if err := preProcessRecursive(logger, patternElementCopy, resource); err != nil { + logger.V(3).Info("anchor mismatch", "reason", err.Error()) + if isConditionError(err) { + continue + } + + return err + } + if hasGlobalConditions { + // global anchor has passed, there is no need to return an error + anyGlobalConditionPassed = true + } + } if !anyGlobalConditionPassed && lastGlobalAnchorError != nil { return lastGlobalAnchorError } diff --git a/pkg/engine/mutate/patch/strategicPreprocessing_test.go b/pkg/engine/mutate/patch/strategicPreprocessing_test.go index 3bebbb146c80..628bbea94ee0 100644 --- a/pkg/engine/mutate/patch/strategicPreprocessing_test.go +++ b/pkg/engine/mutate/patch/strategicPreprocessing_test.go @@ -1278,3 +1278,40 @@ func Test_NestedConditionals(t *testing.T) { assert.DeepEqual(t, toJSON(t, []byte(expectedPattern)), toJSON(t, []byte(resultPattern))) } + +func Test_GlobalCondition_Fail(t *testing.T) { + rawPattern := []byte(`{ + "metadata": { + "annotations": { + "+(cluster-autoscaler.kubernetes.io/safe-to-evict)": "true" + } + }, + "spec": { + "volumes": [ + { + "<(emptyDir)": {} + } + ] + } + }`) + rawResource := []byte(`{ + "apiVersion": "v1", + "kind": "Pod", + "metadata": { + "name": "pod-without-emptydir-hostpath" + }, + "spec": { + "containers": [ + { + "name": "nginx", + "image": "nginx" + } + ] + } + }`) + + pattern := yaml.MustParse(string(rawPattern)) + resource := yaml.MustParse(string(rawResource)) + err := preProcessPattern(logging.GlobalLogger(), pattern, resource) + assert.Error(t, err, "global condition failed: could not found \"emptyDir\" key in the resource") +} diff --git a/test/cli/test-mutate/global-anchor/kyverno-test.yaml b/test/cli/test-mutate/global-anchor/kyverno-test.yaml new file mode 100644 index 000000000000..923fcc861391 --- /dev/null +++ b/test/cli/test-mutate/global-anchor/kyverno-test.yaml @@ -0,0 +1,28 @@ +name: validate-service-loadbalancer +policies: + - policy.yaml +resources: + - resources.yaml +results: + - policy: add-safe-to-evict + rule: annotate-empty-dir + resource: pod-without-emptydir-hostpath + kind: Pod + result: skip + - policy: add-safe-to-evict + rule: annotate-empty-dir + resource: pod-with-emptydir-hostpath + patchedResource: patchedResource.yaml + kind: Pod + result: pass + - policy: add-safe-to-evict + rule: annotate-empty-dir + resource: pod-with-emptydir-hostpath-1 + patchedResource: patchedResourceWithVolume.yaml + kind: Pod + result: pass + - policy: add-safe-to-evict + rule: annotate-empty-dir + resource: pod-without-emptydir-hostpath-1 + kind: Pod + result: skip diff --git a/test/cli/test-mutate/global-anchor/patchedResource.yaml b/test/cli/test-mutate/global-anchor/patchedResource.yaml new file mode 100644 index 000000000000..46f0a328c381 --- /dev/null +++ b/test/cli/test-mutate/global-anchor/patchedResource.yaml @@ -0,0 +1,14 @@ +apiVersion: v1 +kind: Pod +metadata: + annotations: + cluster-autoscaler.kubernetes.io/safe-to-evict: "true" + name: pod-with-emptydir-hostpath + namespace: default +spec: + containers: + - image: nginx + name: nginx + volumes: + - name: demo-volume + emptyDir: {} diff --git a/test/cli/test-mutate/global-anchor/patchedResourceWithVolume.yaml b/test/cli/test-mutate/global-anchor/patchedResourceWithVolume.yaml new file mode 100644 index 000000000000..477a560930bb --- /dev/null +++ b/test/cli/test-mutate/global-anchor/patchedResourceWithVolume.yaml @@ -0,0 +1,18 @@ +apiVersion: v1 +kind: Pod +metadata: + annotations: + cluster-autoscaler.kubernetes.io/safe-to-evict: "true" + name: pod-with-emptydir-hostpath-1 + namespace: default +spec: + containers: + - image: nginx + name: nginx + volumeMounts: + - mountPath: /cache + name: cache-volume + volumes: + - name: cache-volume + emptyDir: + sizeLimit: 500Mi diff --git a/test/cli/test-mutate/global-anchor/policy.yaml b/test/cli/test-mutate/global-anchor/policy.yaml new file mode 100644 index 000000000000..88a057a4fce3 --- /dev/null +++ b/test/cli/test-mutate/global-anchor/policy.yaml @@ -0,0 +1,25 @@ +apiVersion: kyverno.io/v1 +kind: ClusterPolicy +metadata: + name: add-safe-to-evict + annotations: + policies.kyverno.io/category: Workload Management + policies.kyverno.io/description: The Kubernetes cluster autoscaler does not evict pods that + use hostPath or emptyDir volumes. To allow eviction of these pods, the annotation + cluster-autoscaler.kubernetes.io/safe-to-evict=true must be added to the pods. +spec: + rules: + - name: annotate-empty-dir + match: + any: + - resources: + kinds: + - Pod + mutate: + patchStrategicMerge: + metadata: + annotations: + +(cluster-autoscaler.kubernetes.io/safe-to-evict): "true" + spec: + volumes: + - <(emptyDir): {} diff --git a/test/cli/test-mutate/global-anchor/resources.yaml b/test/cli/test-mutate/global-anchor/resources.yaml new file mode 100644 index 000000000000..545246ab8849 --- /dev/null +++ b/test/cli/test-mutate/global-anchor/resources.yaml @@ -0,0 +1,55 @@ +apiVersion: v1 +kind: Pod +metadata: + name: pod-without-emptydir-hostpath +spec: + containers: + - name: nginx + image: nginx +--- +apiVersion: v1 +kind: Pod +metadata: + name: pod-with-emptydir-hostpath +spec: + containers: + - name: nginx + image: nginx + volumes: + - name: demo-volume + emptyDir: {} +--- +apiVersion: v1 +kind: Pod +metadata: + name: pod-with-emptydir-hostpath-1 +spec: + containers: + - name: nginx + image: nginx + volumeMounts: + - mountPath: /cache + name: cache-volume + volumes: + - name: cache-volume + emptyDir: + sizeLimit: 500Mi +--- +apiVersion: v1 +kind: Pod +metadata: + name: pod-without-emptydir-hostpath-1 +spec: + containers: + - name: nginx + image: nginx + volumeMounts: + - name: config-vol + mountPath: /etc/config + volumes: + - name: config-vol + configMap: + name: log-config + items: + - key: log_level + path: log_level