Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: handling unexpected global-anchor-variable for the apply command #21

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion pkg/engine/mutate/patch/strategicPreprocessing.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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
}
Expand Down
37 changes: 37 additions & 0 deletions pkg/engine/mutate/patch/strategicPreprocessing_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
28 changes: 28 additions & 0 deletions test/cli/test-mutate/global-anchor/kyverno-test.yaml
Original file line number Diff line number Diff line change
@@ -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
14 changes: 14 additions & 0 deletions test/cli/test-mutate/global-anchor/patchedResource.yaml
Original file line number Diff line number Diff line change
@@ -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: {}
18 changes: 18 additions & 0 deletions test/cli/test-mutate/global-anchor/patchedResourceWithVolume.yaml
Original file line number Diff line number Diff line change
@@ -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
25 changes: 25 additions & 0 deletions test/cli/test-mutate/global-anchor/policy.yaml
Original file line number Diff line number Diff line change
@@ -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): {}
55 changes: 55 additions & 0 deletions test/cli/test-mutate/global-anchor/resources.yaml
Original file line number Diff line number Diff line change
@@ -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