From ed4eb9666a2b51f4b9c44e88a45be97efda78217 Mon Sep 17 00:00:00 2001 From: Khaled Emara Date: Wed, 22 May 2024 12:04:14 +0300 Subject: [PATCH] fix(anchor): skip anchors don't have priority (#10206) * fix(anchor): give priority to skip anchors Signed-off-by: Khaled Emara * test(anchor): conditional anchor with a failing sibling Signed-off-by: Khaled Emara * test(anchor): conditional anchor mixed with other results Signed-off-by: Khaled Emara * fix(anchor): successful anchor with a skip anchor Signed-off-by: Khaled Emara --------- Signed-off-by: Khaled Emara Co-authored-by: shuting --- pkg/engine/validate/validate.go | 40 +++++++++++++--- pkg/engine/validate/validate_test.go | 72 ++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 6 deletions(-) diff --git a/pkg/engine/validate/validate.go b/pkg/engine/validate/validate.go index 5768de5934..989a01c423 100644 --- a/pkg/engine/validate/validate.go +++ b/pkg/engine/validate/validate.go @@ -145,17 +145,45 @@ func validateMap(log logr.Logger, resourceMap, patternMap map[string]interface{} continue } - return handlerPath, err + skipSiblingExists := false + for _, skipError := range skipErrors { + if _, ok := skipError.(*PatternError); !ok { + skipSiblingExists = true + break + } + } + if !skipSiblingExists { + return handlerPath, err + } else { + continue + } } applyCount++ } - if applyCount == 0 && len(skipErrors) > 0 { - return path, &PatternError{ - Err: multierr.Combine(skipErrors...), - Path: path, - Skip: true, + if len(skipErrors) > 0 { + if applyCount == 0 { + return path, &PatternError{ + Err: multierr.Combine(skipErrors...), + Path: path, + Skip: true, + } + } else { + skipSiblingExists := false + for _, skipError := range skipErrors { + if _, ok := skipError.(*PatternError); !ok { + skipSiblingExists = true + break + } + } + if skipSiblingExists { + return path, &PatternError{ + Err: multierr.Combine(skipErrors...), + Path: path, + Skip: true, + } + } } } diff --git a/pkg/engine/validate/validate_test.go b/pkg/engine/validate/validate_test.go index 165e0493a8..04d91744dd 100644 --- a/pkg/engine/validate/validate_test.go +++ b/pkg/engine/validate/validate_test.go @@ -1615,6 +1615,78 @@ func TestConditionalAnchorWithMultiplePatterns(t *testing.T) { resource: []byte(`{"metadata": {"labels": {"run": "nginx"},"name": "nginx"},"spec": {"containers": [{"image": "nginx","name": "nginx"}],"volumes": [{"hostPath": {"path": "/var/run/docker.sock"}}]}}`), status: engineapi.RuleStatusFail, }, + { + name: "test-43", + pattern: []byte(`{"spec": {"=(volumes)": [{"(name)": "!cache-volume","=(emptyDir)": {"sizeLimit": "?*"}}]}}`), + resource: []byte(`{"spec": {"volumes": [{"name": "cache-volume","emptyDir": {}}]}}`), + status: engineapi.RuleStatusSkip, + }, + { + name: "test-44", + pattern: []byte(`{"spec": {"=(initContainers)": [{"(name)": "!istio-init", "=(securityContext)": {"=(runAsUser)": ">0"}}], "=(containers)": [{"=(securityContext)": {"=(runAsUser)": ">0"}}]}}`), + resource: []byte(`{"spec": {"initContainers": [{"name": "nginx", "securityContext": {"runAsUser": 1000}}], "containers": [{"name": "nginx", "image": "nginx"}]}}`), + status: engineapi.RuleStatusPass, + }, + { + name: "test-45", + pattern: []byte(`{"spec": {"=(initContainers)": [{"(name)": "!istio-init", "=(securityContext)": {"=(runAsUser)": ">0"}}], "=(containers)": [{"=(securityContext)": {"=(runAsUser)": ">0"}}]}}`), + resource: []byte(`{"spec": {"initContainers": [{"name": "nginx", "securityContext": {"runAsUser": 0}}], "containers": [{"name": "nginx", "image": "nginx"}]}}`), + status: engineapi.RuleStatusFail, + }, + { + name: "test-46", + pattern: []byte(`{"spec": {"=(initContainers)": [{"(name)": "!istio-init", "=(securityContext)": {"=(runAsUser)": ">0"}}], "=(containers)": [{"=(securityContext)": {"=(runAsUser)": ">0"}}]}}`), + resource: []byte(`{"spec": {"initContainers": [{"name": "istio-init", "securityContext": {"runAsUser": 0}}], "containers": [{"securityContext": {"runAsUser": 1000}}]}}`), + status: engineapi.RuleStatusPass, + }, + { + name: "test-47", + pattern: []byte(`{"spec": {"=(initContainers)": [{"(name)": "!istio-init", "=(securityContext)": {"=(runAsUser)": ">0"}}], "=(containers)": [{"=(securityContext)": {"=(runAsUser)": ">0"}}]}}`), + resource: []byte(`{"spec": {"initContainers": [{"name": "istio-init", "securityContext": {"runAsUser": 1000}}], "containers": [{"securityContext": {"runAsUser": 0}}]}}`), + status: engineapi.RuleStatusFail, + }, + { + name: "test-48", + pattern: []byte(`{"spec": {"=(initContainers)": [{"(name)": "!istio-init", "=(securityContext)": {"=(runAsUser)": ">0"}}], "=(containers)": [{"=(securityContext)": {"=(runAsUser)": ">0"}}]}}`), + resource: []byte(`{"spec": {"containers": [{"securityContext": {"runAsUser": 1000}}]}}`), + status: engineapi.RuleStatusPass, + }, + { + name: "test-49", + pattern: []byte(`{"spec": {"=(initContainers)": [{"(name)": "!istio-init", "=(securityContext)": {"=(runAsUser)": ">0"}}], "=(containers)": [{"=(securityContext)": {"=(runAsUser)": ">0"}}]}}`), + resource: []byte(`{"spec": {"containers": [{"securityContext": {"runAsUser": 0}}]}}`), + status: engineapi.RuleStatusFail, + }, + { + name: "test-50", + pattern: []byte(`{"spec": {"=(initContainers)": [{"(name)": "!istio-init", "=(securityContext)": {"=(runAsUser)": ">0"}}], "=(containers)": [{"=(securityContext)": {"=(runAsUser)": ">0"}}]}}`), + resource: []byte(`{"spec": {"initContainers": [{"name": "istio-init", "securityContext": {"runAsUser": 0}}], "containers": [{"name": "nginx", "image": "nginx"}]}}`), + status: engineapi.RuleStatusPass, + }, + { + name: "test-51", + pattern: []byte(`{"spec": {"=(volumes)": [{"(name)": "!credential-socket&!istio-data&!istio-envoy&!workload-certs&!workload-socket","=(emptyDir)": {"sizeLimit": "?*"}}]}}`), + resource: []byte(`{"spec": {"volumes": [{"name": "credential-socket","emptyDir": {"sizeLimit": "1Gi"}}]}}`), + status: engineapi.RuleStatusSkip, + }, + { + name: "test-52", + pattern: []byte(`{"spec": {"=(volumes)": [{"(name)": "!credential-socket&!istio-data&!istio-envoy&!workload-certs&!workload-socket","=(emptyDir)": {"sizeLimit": "?*"}}]}}`), + resource: []byte(`{"spec": {"volumes": [{"name": "cache-volume","emptyDir": {"sizeLimit": "1Gi"}}]}}`), + status: engineapi.RuleStatusPass, + }, + { + name: "test-53", + pattern: []byte(`{"spec": {"=(volumes)": [{"(name)": "!credential-socket&!istio-data&!istio-envoy&!workload-certs&!workload-socket","=(emptyDir)": {"sizeLimit": "?*"}}]}}`), + resource: []byte(`{"spec": {"volumes": [{"name": "cache-volume"}]}}`), + status: engineapi.RuleStatusPass, + }, + { + name: "test-54", + pattern: []byte(`{"spec": {"=(volumes)": [{"(name)": "!credential-socket&!istio-data&!istio-envoy&!workload-certs&!workload-socket","=(emptyDir)": {"sizeLimit": "?*"}}]}}`), + resource: []byte(`{"spec": {"volumes": [{"name": "cache-volume","emptyDir": {}}]}}`), + status: engineapi.RuleStatusFail, + }, } for _, testCase := range testCases {