Skip to content

Commit

Permalink
Fix: handling unexpected global-anchor-variable for the apply command
Browse files Browse the repository at this point in the history
Signed-off-by: ansalamdaniel <[email protected]>
  • Loading branch information
ansalamdaniel committed Dec 6, 2022
1 parent 4d4ec16 commit b51fdac
Show file tree
Hide file tree
Showing 7 changed files with 192 additions and 1 deletion.
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

0 comments on commit b51fdac

Please sign in to comment.