diff --git a/api/kyverno/v1alpha1/zz_generated.deepcopy.go b/api/kyverno/v1alpha1/zz_generated.deepcopy.go index d0f7dc74d1..fd5ab381cd 100644 --- a/api/kyverno/v1alpha1/zz_generated.deepcopy.go +++ b/api/kyverno/v1alpha1/zz_generated.deepcopy.go @@ -21,7 +21,7 @@ package v1alpha1 import ( policyreportv1alpha1 "github.com/kyverno/kyverno/api/policyreport/v1alpha1" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) diff --git a/api/kyverno/v1alpha2/zz_generated.deepcopy.go b/api/kyverno/v1alpha2/zz_generated.deepcopy.go index ead4450a5c..b9547fe49f 100644 --- a/api/kyverno/v1alpha2/zz_generated.deepcopy.go +++ b/api/kyverno/v1alpha2/zz_generated.deepcopy.go @@ -21,7 +21,7 @@ package v1alpha2 import ( policyreportv1alpha2 "github.com/kyverno/kyverno/api/policyreport/v1alpha2" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) diff --git a/api/policyreport/v1alpha1/zz_generated.deepcopy.go b/api/policyreport/v1alpha1/zz_generated.deepcopy.go index 9e65ab53f1..3d9d216ace 100644 --- a/api/policyreport/v1alpha1/zz_generated.deepcopy.go +++ b/api/policyreport/v1alpha1/zz_generated.deepcopy.go @@ -20,7 +20,7 @@ limitations under the License. package v1alpha1 import ( - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) diff --git a/api/policyreport/v1alpha2/zz_generated.deepcopy.go b/api/policyreport/v1alpha2/zz_generated.deepcopy.go index c83a6158bf..0e5c465423 100644 --- a/api/policyreport/v1alpha2/zz_generated.deepcopy.go +++ b/api/policyreport/v1alpha2/zz_generated.deepcopy.go @@ -20,7 +20,7 @@ limitations under the License. package v1alpha2 import ( - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) diff --git a/go.mod b/go.mod index 907c5f4a66..55e0ea0e06 100644 --- a/go.mod +++ b/go.mod @@ -56,6 +56,7 @@ require ( require ( github.com/aquilax/truncate v1.0.0 github.com/blang/semver/v4 v4.0.0 + github.com/nsf/jsondiff v0.0.0-20210926074059-1e845ec5d249 gopkg.in/inf.v0 v0.9.1 ) diff --git a/go.sum b/go.sum index b05a5da97d..ad3872f9f8 100644 --- a/go.sum +++ b/go.sum @@ -1268,6 +1268,8 @@ github.com/nats-io/nkeys v0.1.3/go.mod h1:xpnFELMwJABBLVhffcfd1MZx6VsNRFpEugbxzi github.com/nats-io/nuid v1.0.1/go.mod h1:19wcPz3Ph3q0Jbyiqsd0kePYG7A95tJPxeL+1OSON2c= github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno= github.com/nishanths/predeclared v0.0.0-20200524104333-86fad755b4d3/go.mod h1:nt3d53pc1VYcphSCIaYAJtnPYnr3Zyn8fMq2wvPGPso= +github.com/nsf/jsondiff v0.0.0-20210926074059-1e845ec5d249 h1:NHrXEjTNQY7P0Zfx1aMrNhpgxHmow66XQtm0aQLY0AE= +github.com/nsf/jsondiff v0.0.0-20210926074059-1e845ec5d249/go.mod h1:mpRZBD8SJ55OIICQ3iWH0Yz3cjzA61JdqMLoWXeB2+8= github.com/nxadm/tail v1.4.4/go.mod h1:kenIhsEOeOJmVchQTgglprH7qJGnHDVpk1VPCcaMI8A= github.com/nxadm/tail v1.4.8 h1:nPr65rt6Y5JFSKQO7qToXr7pePgD6Gwiw05lkbyAQTE= github.com/nxadm/tail v1.4.8/go.mod h1:+ncqLTQzXmGhMZNUePPaPqPvBxHAIsmXswZKocGu+AU= diff --git a/pkg/engine/anchor/common.go b/pkg/engine/anchor/common.go index 1cb4d6ecc1..df53c78fac 100644 --- a/pkg/engine/anchor/common.go +++ b/pkg/engine/anchor/common.go @@ -25,8 +25,9 @@ func IsGlobalAnchor(str string) bool { return false } - //TODO: trim spaces ? - return (str[:len(left)] == left && str[len(str)-len(right):] == right) + leftMatch := strings.TrimSpace(str[:len(left)]) == left + rightMatch := strings.TrimSpace(str[len(str)-len(right):]) == right + return leftMatch && rightMatch } //ContainsCondition returns true, if str is either condition anchor or @@ -46,8 +47,8 @@ func IsNegationAnchor(str string) bool { return (str[:len(left)] == left && str[len(str)-len(right):] == right) } -// IsAddingAnchor checks for addition anchor -func IsAddingAnchor(key string) bool { +// IsAddIfNotPresentAnchor checks for addition anchor +func IsAddIfNotPresentAnchor(key string) bool { const left = "+(" const right = ")" @@ -94,7 +95,7 @@ func RemoveAnchor(key string) (string, string) { return key[1 : len(key)-1], key[0:1] } - if IsExistenceAnchor(key) || IsAddingAnchor(key) || IsEqualityAnchor(key) || IsNegationAnchor(key) || IsGlobalAnchor(key) { + if IsExistenceAnchor(key) || IsAddIfNotPresentAnchor(key) || IsEqualityAnchor(key) || IsNegationAnchor(key) || IsGlobalAnchor(key) { return key[2 : len(key)-1], key[0:2] } diff --git a/pkg/engine/common/pattern.go b/pkg/engine/common/pattern.go index a4e0c282b3..3dfed0774f 100644 --- a/pkg/engine/common/pattern.go +++ b/pkg/engine/common/pattern.go @@ -151,6 +151,10 @@ func validateValueWithNilPattern(log logr.Logger, value interface{}) bool { // Handler for pattern values during validation process func validateValueWithStringPatterns(log logr.Logger, value interface{}, pattern string) bool { + if value == pattern { + return true + } + conditions := strings.Split(pattern, "|") for _, condition := range conditions { condition = strings.Trim(condition, " ") diff --git a/pkg/engine/mutate/patch/strategicMergePatch.go b/pkg/engine/mutate/patch/strategicMergePatch.go index e64b666e96..5756444474 100644 --- a/pkg/engine/mutate/patch/strategicMergePatch.go +++ b/pkg/engine/mutate/patch/strategicMergePatch.go @@ -97,6 +97,8 @@ func strategicMergePatch(logger logr.Logger, base, overlay string) ([]byte, erro } } + patchStr, _ := preprocessedYaml.String() + logger.V(3).Info("applying strategic merge patch", "patch", patchStr) f := patchstrategicmerge.Filter{ Patch: preprocessedYaml, } diff --git a/pkg/engine/mutate/patch/strategicMergePatch_test.go b/pkg/engine/mutate/patch/strategicMergePatch_test.go index 775d8a4e1a..cf498a4910 100644 --- a/pkg/engine/mutate/patch/strategicMergePatch_test.go +++ b/pkg/engine/mutate/patch/strategicMergePatch_test.go @@ -164,9 +164,7 @@ func TestMergePatch(t *testing.T) { t.Logf("Running test %d...", i+1) out, err := strategicMergePatch(log.Log, string(test.rawResource), string(test.rawPolicy)) assert.NilError(t, err) - - // has assertions inside - areEqualJSONs(t, test.expected, out) + assert.DeepEqual(t, toJSON(t, test.expected), toJSON(t, out)) } } diff --git a/pkg/engine/mutate/patch/strategicPreprocessing.go b/pkg/engine/mutate/patch/strategicPreprocessing.go index 9ceb71427c..e6d7b0ead7 100644 --- a/pkg/engine/mutate/patch/strategicPreprocessing.go +++ b/pkg/engine/mutate/patch/strategicPreprocessing.go @@ -4,6 +4,8 @@ import ( "encoding/json" "fmt" + "github.com/pkg/errors" + "github.com/go-logr/logr" "github.com/kyverno/kyverno/pkg/engine/anchor" "github.com/kyverno/kyverno/pkg/engine/validate" @@ -65,31 +67,26 @@ func preProcessRecursive(logger logr.Logger, pattern, resource *yaml.RNode) erro } func walkMap(logger logr.Logger, pattern, resource *yaml.RNode) error { - var err error - - err = validateConditions(logger, pattern, resource) - if err != nil { - return err + if _, err := handleAddIfNotPresentAnchor(pattern, resource); err != nil { + return errors.Wrap(err, "failed to process addIfNotPresent anchor") } - err = handleAddings(pattern, resource) - if err != nil { - return err + if err := validateConditions(logger, pattern, resource); err != nil { + return err // do not wrap condition errors } - nonAnchors, err := filterKeys(pattern, func(key string) bool { + isNotAnchor := func(key string) bool { return !hasAnchor(key) - }) + } + + nonAnchors, err := filterKeys(pattern, isNotAnchor) if err != nil { return err } - var resourceValue *yaml.RNode - for _, field := range nonAnchors { - if resource == nil || resource.Field(field) == nil { - resourceValue = nil - } else { + var resourceValue *yaml.RNode + if resource != nil && resource.Field(field) != nil { resourceValue = resource.Field(field).Value } @@ -139,57 +136,30 @@ func processListOfMaps(logger logr.Logger, pattern, resource *yaml.RNode) error hasAnyAnchor := hasAnchors(patternElement, hasAnchor) hasGlobalConditions := hasAnchors(patternElement, anchor.IsGlobalAnchor) if hasAnyAnchor { - anyGlobalConditionPassed := false var lastGlobalAnchorError error = nil for _, resourceElement := range resourceElements { - err := preProcessRecursive(logger, patternElement, resourceElement) - if err != nil { - switch err.(type) { - case ConditionError: - // Skip element, if condition has failed + if err := preProcessRecursive(logger, patternElement, resourceElement); err != nil { + logger.V(3).Info("anchor mismatch", "reason", err.Error()) + if isConditionError(err) { continue - case GlobalConditionError: + } + + if isGlobalConditionError(err) { lastGlobalAnchorError = err continue } return err + } + + if hasGlobalConditions { + // global anchor has passed, there is no need to return an error + anyGlobalConditionPassed = true } else { - if hasGlobalConditions { - // global anchor has passed, there is no need to return an error - anyGlobalConditionPassed = true - } - - // If condition is satisfied, create new pattern list element based on patternElement - // but related with current resource element by name. - // Resource element must have name. Without name kustomize won't be able to update this element. - // In case if element does not have name, skip it. - resourceElementName := resourceElement.Field("name") - if resourceElementName.IsNilOrEmpty() { - continue - } - - newNode := patternElement.Copy() - empty, err := deleteConditionsFromNestedMaps(newNode) - if err != nil { - return err - } - - // Do not add an empty element to the patch - if empty { - continue - } - - err = newNode.PipeE(yaml.SetField("name", resourceElementName.Value)) - if err != nil { - return err - } - - err = pattern.PipeE(yaml.Append(newNode.YNode())) - if err != nil { - return err + if err := handlePatternName(pattern, patternElement, resourceElement); err != nil { + return errors.Wrap(err, "failed to update name in pattern") } } } @@ -203,6 +173,50 @@ func processListOfMaps(logger logr.Logger, pattern, resource *yaml.RNode) error return nil } +func handlePatternName(pattern, patternElement, resourceElement *yaml.RNode) error { + // If condition is satisfied, create new pattern list element based on patternElement + // but related with current resource element by name. + // Resource element must have name. Without name kustomize won't be able to update this element. + // In case if element does not have name, skip it. + resourceElementName := resourceElement.Field("name") + if resourceElementName.IsNilOrEmpty() { + return nil + } + + newNode := patternElement.Copy() + empty, err := deleteAnchors(newNode, true, false) + if err != nil { + return err + } + + // Do not add an empty element to the patch + if empty { + return nil + } + + err = newNode.PipeE(yaml.SetField("name", resourceElementName.Value)) + if err != nil { + return err + } + + err = pattern.PipeE(yaml.Append(newNode.YNode())) + if err != nil { + return err + } + + return nil +} + +func isConditionError(err error) bool { + _, ok := err.(ConditionError) + return ok +} + +func isGlobalConditionError(err error) bool { + _, ok := err.(GlobalConditionError) + return ok +} + // validateConditions checks all conditions from current map. // If at least one condition fails, return error. // If caller handles list of maps and gets an error, it must skip element. @@ -223,35 +237,38 @@ func validateConditions(logger logr.Logger, pattern, resource *yaml.RNode) error return nil } -// handleAddings handles adding anchors. +// handleAddIfNotPresentAnchor handles adding anchors. // Remove anchor from pattern, if field already exists. // Remove anchor wrapping from key, if field does not exist in the resource. -func handleAddings(pattern, resource *yaml.RNode) error { - addings, err := filterKeys(pattern, anchor.IsAddingAnchor) +func handleAddIfNotPresentAnchor(pattern, resource *yaml.RNode) (int, error) { + anchors, err := filterKeys(pattern, anchor.IsAddIfNotPresentAnchor) if err != nil { - return err + return 0, err } - for _, adding := range addings { - key, _ := anchor.RemoveAnchor(adding) + for _, a := range anchors { + key, _ := anchor.RemoveAnchor(a) if resource != nil && resource.Field(key) != nil { // Resource already has this field. - // Delete the field with adding anchor from patch. - err = pattern.PipeE(yaml.Clear(adding)) + // Delete the field with addIfNotPresent anchor from patch. + err = pattern.PipeE(yaml.Clear(a)) if err != nil { - return err + return 0, err } - continue + } else { + // Remove anchor tags from patch field key. + renameField(a, key, pattern) } - - // Remove anchor wrap from patch field. - renameField(adding, key, pattern) } - return nil + return len(anchors), nil } func filterKeys(pattern *yaml.RNode, condition func(string) bool) ([]string, error) { + if !isMappingNode(pattern) { + return nil, nil + } + keys := make([]string, 0) fields, err := pattern.Fields() if err != nil { @@ -267,12 +284,23 @@ func filterKeys(pattern *yaml.RNode, condition func(string) bool) ([]string, err return keys, nil } +func isMappingNode(node *yaml.RNode) bool { + if err := yaml.ErrorIfInvalid(node, yaml.MappingNode); err != nil { + return false + } + + return true +} + func hasAnchor(key string) bool { - return anchor.ContainsCondition(key) || anchor.IsAddingAnchor(key) + return anchor.ContainsCondition(key) || anchor.IsAddIfNotPresentAnchor(key) } func hasAnchors(pattern *yaml.RNode, isAnchor func(key string) bool) bool { - if yaml.MappingNode == pattern.YNode().Kind { + ynode := pattern.YNode() + kind := ynode.Kind + + if kind == yaml.MappingNode { fields, err := pattern.Fields() if err != nil { return false @@ -290,6 +318,19 @@ func hasAnchors(pattern *yaml.RNode, isAnchor func(key string) bool) bool { } } } + } else if kind == yaml.ScalarNode { + v := ynode.Value + return anchor.ContainsCondition(v) + + } else if kind == yaml.SequenceNode { + elements, _ := pattern.Elements() + for _, e := range elements { + if hasAnchors(e, isAnchor) { + return true + } + } + + return false } return false @@ -343,52 +384,6 @@ func checkCondition(logger logr.Logger, pattern *yaml.RNode, resource *yaml.RNod return nil } -func deleteConditionsFromNestedMaps(pattern *yaml.RNode) (bool, error) { - if pattern.YNode().Kind != yaml.MappingNode { - return false, nil - } - - fields, err := pattern.Fields() - if err != nil { - return false, err - } - - for _, field := range fields { - if anchor.ContainsCondition(field) { - err = pattern.PipeE(yaml.Clear(field)) - if err != nil { - return false, err - } - } else { - child := pattern.Field(field).Value - if child != nil { - empty, err := deleteConditionsFromNestedMaps(child) - if err != nil { - return false, err - } - - if empty { - err = pattern.PipeE(yaml.Clear(field)) - if err != nil { - return false, err - } - } - } - } - } - - fields, err = pattern.Fields() - if err != nil { - return false, err - } - - if len(fields) == 0 { - return true, nil - } - - return false, nil -} - func deleteConditionElements(pattern *yaml.RNode) error { fields, err := pattern.Fields() if err != nil { @@ -396,11 +391,12 @@ func deleteConditionElements(pattern *yaml.RNode) error { } for _, field := range fields { - ok, err := deleteAnchors(pattern.Field(field).Value) + deleteScalar := anchor.ContainsCondition(field) + canDelete, err := deleteAnchors(pattern.Field(field).Value, deleteScalar, false) if err != nil { return err } - if ok { + if canDelete { err = pattern.PipeE(yaml.Clear(field)) if err != nil { return err @@ -414,32 +410,50 @@ func deleteConditionElements(pattern *yaml.RNode) error { // deleteAnchors deletes all the anchors and returns true, // if this node must be deleted from patch. // Node is considered to be deleted, if there were only -// anchors elemets. After anchors elements are removed, -// we have patch with nil values which could cause +// anchors elements. After anchors elements are removed, +// A patch with nil values which could cause // unnecessary resource elements deletion. -func deleteAnchors(node *yaml.RNode) (bool, error) { +func deleteAnchors(node *yaml.RNode, deleteScalar, traverseMappingNodes bool) (bool, error) { switch node.YNode().Kind { case yaml.MappingNode: - return deleteAnchorsInMap(node) + return deleteAnchorsInMap(node, traverseMappingNodes) case yaml.SequenceNode: - return deleteAnchorsInList(node) + return deleteAnchorsInList(node, traverseMappingNodes) + case yaml.ScalarNode: + return deleteScalar, nil } return false, nil } -func deleteAnchorsInMap(node *yaml.RNode) (bool, error) { +func deleteAnchorsInMap(node *yaml.RNode, traverseMappingNodes bool) (bool, error) { conditions, err := filterKeys(node, anchor.ContainsCondition) if err != nil { return false, err } - // Remove all conditions first. + // remove all conditional anchors with no child nodes first + anchorsExist := false for _, condition := range conditions { - err = node.PipeE(yaml.Clear(condition)) + field := node.Field(condition) + shouldDelete, err := deleteAnchors(field.Value, true, traverseMappingNodes) if err != nil { return false, err } + + if shouldDelete { + if err := node.PipeE(yaml.Clear(condition)); err != nil { + return false, err + } + } else { + anchorsExist = true + } + } + + if anchorsExist { + if err := stripAnchorsFromNode(node, ""); err != nil { + return false, errors.Wrap(err, "failed to remove anchor tags") + } } fields, err := node.Fields() @@ -448,15 +462,13 @@ func deleteAnchorsInMap(node *yaml.RNode) (bool, error) { } needToDelete := true - - // Go further through the map elements. for _, field := range fields { - ok, err := deleteAnchors(node.Field(field).Value) + canDelete, err := deleteAnchors(node.Field(field).Value, false, traverseMappingNodes) if err != nil { return false, err } - if ok { + if canDelete { err = node.PipeE(yaml.Clear(field)) if err != nil { return false, err @@ -471,26 +483,53 @@ func deleteAnchorsInMap(node *yaml.RNode) (bool, error) { return needToDelete, nil } -func deleteAnchorsInList(node *yaml.RNode) (bool, error) { +// stripAnchorFromNode strips one or more anchor fields from the node. +// If key is "" all anchor fields are stripped. Otherwise, only the matching +// field is stripped. +func stripAnchorsFromNode(node *yaml.RNode, key string) error { + anchors, err := filterKeys(node, anchor.ContainsCondition) + if err != nil { + return err + } + + for _, a := range anchors { + k, _ := anchor.RemoveAnchor(a) + if key == "" || k == key { + renameField(a, k, node) + } + } + + return nil +} + +func deleteAnchorsInList(node *yaml.RNode, traverseMappingNodes bool) (bool, error) { elements, err := node.Elements() if err != nil { return false, err } wasEmpty := len(elements) == 0 - for i, element := range elements { if hasAnchors(element, hasAnchor) { - deleteListElement(node, i) + shouldDelete := true + if traverseMappingNodes && isMappingNode(element) { + shouldDelete, err = deleteAnchors(element, true, traverseMappingNodes) + if err != nil { + return false, errors.Wrap(err, "failed to delete anchors") + } + } + + if shouldDelete { + deleteListElement(node, i) + } } else { // This element also could have some conditions // inside sub-arrays. Delete them too. - - ok, err := deleteAnchors(element) + canDelete, err := deleteAnchors(element, false, traverseMappingNodes) if err != nil { - return false, err + return false, errors.Wrap(err, "failed to delete anchors") } - if ok { + if canDelete { deleteListElement(node, i) } } @@ -524,8 +563,15 @@ func validateConditionsInternal(logger logr.Logger, pattern, resource *yaml.RNod return fmt.Errorf("could not found \"%s\" key in the resource", conditionKey) } - err = checkCondition(logger, pattern.Field(condition).Value, resource.Field(conditionKey).Value) - if err != nil { + patternValue := pattern.Field(condition).Value + resourceValue := resource.Field(conditionKey).Value + if count, err := handleAddIfNotPresentAnchor(patternValue, resourceValue); err != nil { + return err + } else if count > 0 { + continue + } + + if err := checkCondition(logger, patternValue, resourceValue); err != nil { return err } } diff --git a/pkg/engine/mutate/patch/strategicPreprocessing_test.go b/pkg/engine/mutate/patch/strategicPreprocessing_test.go index d96bfe2239..7e5d9b3a89 100644 --- a/pkg/engine/mutate/patch/strategicPreprocessing_test.go +++ b/pkg/engine/mutate/patch/strategicPreprocessing_test.go @@ -5,25 +5,11 @@ import ( "testing" "github.com/kyverno/kyverno/pkg/engine/anchor" - "gotest.tools/assert" "sigs.k8s.io/controller-runtime/pkg/log" yaml "sigs.k8s.io/kustomize/kyaml/yaml" ) -func areEqualJSONs(t *testing.T, s1, s2 []byte) { - var o1 interface{} - var o2 interface{} - - var err error - err = json.Unmarshal(s1, &o1) - assert.NilError(t, err) - - err = json.Unmarshal(s2, &o2) - assert.NilError(t, err) - assert.DeepEqual(t, o1, o2) -} - func Test_preProcessStrategicMergePatch_multipleAnchors(t *testing.T) { testCases := []struct { rawPolicy []byte @@ -341,7 +327,7 @@ func Test_preProcessStrategicMergePatch_multipleAnchors(t *testing.T) { "spec": { "volumes": [ { - "(hostPath)": { + "<(hostPath)": { "path": "*" } } @@ -400,7 +386,7 @@ func Test_preProcessStrategicMergePatch_multipleAnchors(t *testing.T) { "spec": { "volumes": [ { - "(hostPath)": { + "<(hostPath)": { "path": "*" } } @@ -855,19 +841,25 @@ func Test_preProcessStrategicMergePatch_multipleAnchors(t *testing.T) { } for i, test := range testCases { - - t.Logf("Running test %d...", i+1) + t.Logf("Running test %d...", i) preProcessedPolicy, err := preProcessStrategicMergePatch(log.Log, string(test.rawPolicy), string(test.rawResource)) assert.NilError(t, err) output, err := preProcessedPolicy.MarshalJSON() assert.NilError(t, err) - // has assertions inside - areEqualJSONs(t, test.expectedPatch, output) + assert.DeepEqual(t, toJSON(t, []byte(test.expectedPatch)), toJSON(t, output)) } } +func toJSON(t *testing.T, b []byte) interface{} { + var i interface{} + var err error + err = json.Unmarshal(b, &i) + assert.NilError(t, err) + return i +} + func Test_FilterKeys_NoConditions(t *testing.T) { patternRaw := []byte(`{ "key1": "value1", @@ -1056,7 +1048,7 @@ func Test_DeleteConditions(t *testing.T) { assert.NilError(t, err) assert.Equal(t, len(containers), 2) - _, err = deleteAnchors(pattern) + _, err = deleteAnchors(pattern, false, false) assert.NilError(t, err) containers, err = pattern.Field("spec").Value.Field("containers").Value.Elements() @@ -1150,7 +1142,20 @@ func Test_NonExistingKeyMustFailPreprocessing(t *testing.T) { pattern := yaml.MustParse(string(rawPattern)) resource := yaml.MustParse(string(rawResource)) - err := preProcessPattern(log.Log, pattern, resource) assert.Error(t, err, "condition failed: could not found \"key1\" key in the resource") } + +func Test_NestedConditionals(t *testing.T) { + rawPattern := `{"spec":{"template":{"spec":{"volumes":[{"(emptyDir)":{"+(sizeLimit)":"20Mi"},"name":"*"}]}}}}` + rawResource := `{"spec":{"template":{"spec":{"volumes":[{"emptyDir":{},"name":"vol02"}]}}}}` + expectedPattern := `{"spec":{"template":{"spec":{"volumes":[{"emptyDir":{"sizeLimit":"20Mi"},"name":"vol02"}]}}}}` + + pattern := yaml.MustParse(rawPattern) + resource := yaml.MustParse(rawResource) + err := preProcessPattern(log.Log, pattern, resource) + assert.NilError(t, err) + resultPattern, _ := pattern.String() + + assert.DeepEqual(t, toJSON(t, []byte(expectedPattern)), toJSON(t, []byte(resultPattern))) +} diff --git a/pkg/engine/mutate/utils.go b/pkg/engine/mutate/utils.go index c7b07651da..52c15cc4c4 100644 --- a/pkg/engine/mutate/utils.go +++ b/pkg/engine/mutate/utils.go @@ -11,7 +11,7 @@ func getAnchorAndElementsFromMap(anchorsMap map[string]interface{}) (map[string] for key, value := range anchorsMap { if commonAnchors.IsConditionAnchor(key) { anchors[key] = value - } else if !commonAnchors.IsAddingAnchor(key) { + } else if !commonAnchors.IsAddIfNotPresentAnchor(key) { elementsWithoutanchor[key] = value } } diff --git a/pkg/engine/validation.go b/pkg/engine/validation.go index 1d76d8f175..3d76de80ee 100644 --- a/pkg/engine/validation.go +++ b/pkg/engine/validation.go @@ -277,7 +277,7 @@ func (v *validator) validateElements(foreach *kyverno.ForEachValidation, element v.log.Info("skip rule", "reason", r.Message) continue } else if r.Status != response.RuleStatusPass { - msg := fmt.Sprintf("validation failed in foreach rule for %v", r.Message) + msg := fmt.Sprintf("validation failure: %v", r.Message) return ruleResponse(v.rule, utils.Validation, msg, r.Status), applyCount } diff --git a/pkg/kyverno/test/test_command.go b/pkg/kyverno/test/test_command.go index 05359a43d3..8e50ec6139 100644 --- a/pkg/kyverno/test/test_command.go +++ b/pkg/kyverno/test/test_command.go @@ -15,7 +15,6 @@ import ( "github.com/fatih/color" "github.com/go-git/go-billy/v5" "github.com/go-git/go-billy/v5/memfs" - "github.com/go-logr/logr" "github.com/kataras/tablewriter" report "github.com/kyverno/kyverno/api/policyreport/v1alpha2" client "github.com/kyverno/kyverno/pkg/dclient" @@ -391,11 +390,11 @@ func getLocalDirTestFiles(fs billy.Filesystem, path, fileName, valuesFile string return errors } -func buildPolicyResults(resps []*response.EngineResponse, testResults []TestResults, infos []policyreport.Info, policyResourcePath string, fs billy.Filesystem, isGit bool) (map[string]report.PolicyReportResult, []TestResults) { +func buildPolicyResults(engineResponses []*response.EngineResponse, testResults []TestResults, infos []policyreport.Info, policyResourcePath string, fs billy.Filesystem, isGit bool) (map[string]report.PolicyReportResult, []TestResults) { results := make(map[string]report.PolicyReportResult) now := metav1.Timestamp{Seconds: time.Now().Unix()} - for _, resp := range resps { + for _, resp := range engineResponses { policyName := resp.PolicyResponse.Policy.Name resourceName := resp.PolicyResponse.Resource.Name resourceKind := resp.PolicyResponse.Resource.Kind @@ -417,7 +416,7 @@ func buildPolicyResults(resps []*response.EngineResponse, testResults []TestResu Message: buildMessage(resp), } - var patcheResourcePath []string + var patchedResourcePath []string for i, test := range testResults { var userDefinedPolicyNamespace string var userDefinedPolicyName string @@ -457,7 +456,7 @@ func buildPolicyResults(resps []*response.EngineResponse, testResults []TestResu } } - patcheResourcePath = append(patcheResourcePath, test.PatchedResource) + patchedResourcePath = append(patchedResourcePath, test.PatchedResource) if _, ok := results[resultsKey]; !ok { results[resultsKey] = result } @@ -472,13 +471,12 @@ func buildPolicyResults(resps []*response.EngineResponse, testResults []TestResu var resultsKey []string var resultKey string - var result report.PolicyReportResult resultsKey = GetAllPossibleResultsKey(policyNamespace, policyName, rule.Name, resourceNamespace, resourceKind, resourceName) - for _, resultK := range resultsKey { - if val, ok := results[resultK]; ok { + for _, key := range resultsKey { + if val, ok := results[key]; ok { result = val - resultKey = resultK + resultKey = key } else { continue } @@ -491,7 +489,7 @@ func buildPolicyResults(resps []*response.EngineResponse, testResults []TestResu } else { var x string - for _, path := range patcheResourcePath { + for _, path := range patchedResourcePath { result.Result = report.StatusFail x = getAndComparePatchedResource(path, resp.PatchedResource, isGit, policyResourcePath, fs) if x == "pass" { @@ -538,12 +536,12 @@ func buildPolicyResults(resps []*response.EngineResponse, testResults []TestResu return results, testResults } -func GetAllPossibleResultsKey(policyNs, policy, rule, resourceNsnamespace, kind, resource string) []string { +func GetAllPossibleResultsKey(policyNamespace, policy, rule, resourceNamespace, kind, resource string) []string { var resultsKey []string resultKey1 := fmt.Sprintf("%s-%s-%s-%s", policy, rule, kind, resource) - resultKey2 := fmt.Sprintf("%s-%s-%s-%s-%s", policy, rule, resourceNsnamespace, kind, resource) - resultKey3 := fmt.Sprintf("%s-%s-%s-%s-%s", policyNs, policy, rule, kind, resource) - resultKey4 := fmt.Sprintf("%s-%s-%s-%s-%s-%s", policyNs, policy, rule, resourceNsnamespace, kind, resource) + resultKey2 := fmt.Sprintf("%s-%s-%s-%s-%s", policy, rule, resourceNamespace, kind, resource) + resultKey3 := fmt.Sprintf("%s-%s-%s-%s-%s", policyNamespace, policy, rule, kind, resource) + resultKey4 := fmt.Sprintf("%s-%s-%s-%s-%s-%s", policyNamespace, policy, rule, resourceNamespace, kind, resource) resultsKey = append(resultsKey, resultKey1, resultKey2, resultKey3, resultKey4) return resultsKey } @@ -584,12 +582,12 @@ func getAndComparePatchedResource(path string, enginePatchedResource unstructure if err != nil { os.Exit(1) } - var log logr.Logger - matched, err := generate.ValidateResourceWithPattern(log, enginePatchedResource.UnstructuredContent(), patchedResources.UnstructuredContent()) - + matched, err := generate.ValidateResourceWithPattern(log.Log, enginePatchedResource.UnstructuredContent(), patchedResources.UnstructuredContent()) if err != nil { + log.Log.Info("patched resource mismatch", "error", err.Error()) status = "fail" } + if matched == "" { status = "pass" } diff --git a/test/cli/test-mutate/foreach/addIfNotPresent/deploy-patched.yaml b/test/cli/test-mutate/foreach/addIfNotPresent/deploy-patched.yaml new file mode 100644 index 0000000000..fc352d4abd --- /dev/null +++ b/test/cli/test-mutate/foreach/addIfNotPresent/deploy-patched.yaml @@ -0,0 +1,35 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: svc-sizelimit-test + namespace: default +spec: + selector: + matchLabels: + test: svc-sizelimit-test + replicas: 1 + template: + metadata: + labels: + test: svc-sizelimit-test + spec: + securityContext: + runAsUser: 65000 + runAsGroup: 65000 + containers: + - name: pause + image: k8s.gcr.io/pause:3.3 + resources: + limits: + cpu: 10m + memory: 32Mi + requests: + cpu: 10m + memory: 32Mi + volumeMounts: + - name: vol02 + mountPath: /opt02 + volumes: + - name: vol02 + emptyDir: + sizeLimit: 20Mi \ No newline at end of file diff --git a/test/cli/test-mutate/foreach/addIfNotPresent/kyverno-test.yaml b/test/cli/test-mutate/foreach/addIfNotPresent/kyverno-test.yaml new file mode 100644 index 0000000000..d978d9c1ac --- /dev/null +++ b/test/cli/test-mutate/foreach/addIfNotPresent/kyverno-test.yaml @@ -0,0 +1,12 @@ +name: foreach-mutate +policies: + - policies.yaml +resources: + - resources.yaml +results: + - policy: mutate-emptydir + rule: setDefault + resource: svc-sizelimit-test + patchedResource: deploy-patched.yaml + kind: Deployment + result: pass diff --git a/test/cli/test-mutate/foreach/addIfNotPresent/policies.yaml b/test/cli/test-mutate/foreach/addIfNotPresent/policies.yaml new file mode 100644 index 0000000000..6c0849f32d --- /dev/null +++ b/test/cli/test-mutate/foreach/addIfNotPresent/policies.yaml @@ -0,0 +1,22 @@ +apiVersion: kyverno.io/v1 +kind: ClusterPolicy +metadata: + name: mutate-emptydir +spec: + rules: + - name: setDefault + match: + resources: + kinds: + - Deployment + mutate: + foreach: + - list: "request.object.spec.template.spec.volumes" + patchStrategicMerge: + spec: + template: + spec: + volumes: + - name: "{{ element.name }}" + (emptyDir): + +(sizeLimit): "20Mi" \ No newline at end of file diff --git a/test/cli/test-mutate/foreach/addIfNotPresent/resources.yaml b/test/cli/test-mutate/foreach/addIfNotPresent/resources.yaml new file mode 100644 index 0000000000..83c9f3c33f --- /dev/null +++ b/test/cli/test-mutate/foreach/addIfNotPresent/resources.yaml @@ -0,0 +1,35 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: svc-sizelimit-test + namespace: default +spec: + selector: + matchLabels: + test: svc-sizelimit-test + replicas: 1 + template: + metadata: + labels: + test: svc-sizelimit-test + spec: + securityContext: + runAsUser: 65000 + runAsGroup: 65000 + containers: + - name: pause + image: k8s.gcr.io/pause:3.3 + resources: + limits: + cpu: 10m + memory: 32Mi + requests: + cpu: 10m + memory: 32Mi + volumeMounts: + - name: vol02 + mountPath: /opt02 + volumes: + - name: vol02 + emptyDir: {} + diff --git a/test/cli/test-mutate/foreach/replaceRegistry/kyverno-test.yaml b/test/cli/test-mutate/foreach/replaceRegistry/kyverno-test.yaml new file mode 100644 index 0000000000..5f29762602 --- /dev/null +++ b/test/cli/test-mutate/foreach/replaceRegistry/kyverno-test.yaml @@ -0,0 +1,12 @@ +name: foreach-mutate +policies: + - policies.yaml +resources: + - resources.yaml +results: + - policy: replace-image-registry-containers + rule: set-default + resource: test-patched-image + patchedResource: pod-patched.yaml + kind: Pod + result: pass diff --git a/test/cli/test-mutate/foreach/replaceRegistry/pod-patched.yaml b/test/cli/test-mutate/foreach/replaceRegistry/pod-patched.yaml new file mode 100644 index 0000000000..7df4f8771a --- /dev/null +++ b/test/cli/test-mutate/foreach/replaceRegistry/pod-patched.yaml @@ -0,0 +1,27 @@ +apiVersion: v1 +kind: Pod +metadata: + namespace: default + name: test-patched-image +spec: + containers: + - args: + - --web.listen-address=127.0.0.1:9100 + - --path.procfs=/host/proc + - --path.sysfs=/host/sys + - --path.rootfs=/host/root + - --no-collector.wifi + - --no-collector.hwmon + - --collector.filesystem.ignored-mount-points=^/(dev|proc|sys|var/lib/docker/.+)($|/) + - --collector.filesystem.ignored-fs-types=^(autofs|binfmt_misc|cgroup|configfs|debugfs|devpts|devtmpfs|fusectl|hugetlbfs|mqueue|overlay|proc|procfs|pstore|rpc_pipefs|securityfs|sysfs|tracefs)$ + image: test/test3.2 + imagePullPolicy: IfNotPresent + name: node-exporter + - args: + - --logtostderr + - --secure-listen-address=[$(IP)]:9100 + - --tls-cipher-suites=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_RSA_WITH_AES_128_CBC_SHA256,TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256,TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 + - --upstream=http://127.0.0.1:9100/ + image: test/test3.2 + imagePullPolicy: IfNotPresent + name: kube-rbac-proxy diff --git a/test/cli/test-mutate/foreach/replaceRegistry/policies.yaml b/test/cli/test-mutate/foreach/replaceRegistry/policies.yaml new file mode 100644 index 0000000000..2c428babc1 --- /dev/null +++ b/test/cli/test-mutate/foreach/replaceRegistry/policies.yaml @@ -0,0 +1,22 @@ +apiVersion: kyverno.io/v1 +kind: ClusterPolicy +metadata: + name: replace-image-registry-containers + annotations: + pod-policies.kyverno.io/autogen-controllers: "none" +spec: + rules: + - name: set-default + match: + all: + - resources: + kinds: + - Pod + mutate: + foreach: + - list: "request.object.spec.containers" + patchStrategicMerge: + spec: + containers: + - (name): "{{ element.name }}" + image: test/test3.2 \ No newline at end of file diff --git a/test/cli/test-mutate/foreach/replaceRegistry/resources.yaml b/test/cli/test-mutate/foreach/replaceRegistry/resources.yaml new file mode 100644 index 0000000000..d9f2cf6567 --- /dev/null +++ b/test/cli/test-mutate/foreach/replaceRegistry/resources.yaml @@ -0,0 +1,27 @@ +apiVersion: v1 +kind: Pod +metadata: + namespace: default + name: test-patched-image +spec: + containers: + - args: + - --web.listen-address=127.0.0.1:9100 + - --path.procfs=/host/proc + - --path.sysfs=/host/sys + - --path.rootfs=/host/root + - --no-collector.wifi + - --no-collector.hwmon + - "--collector.filesystem.ignored-mount-points=^/(dev|proc|sys|var/lib/docker/.+)($|/)" + - "--collector.filesystem.ignored-fs-types=^(autofs|binfmt_misc|cgroup|configfs|debugfs|devpts|devtmpfs|fusectl|hugetlbfs|mqueue|overlay|proc|procfs|pstore|rpc_pipefs|securityfs|sysfs|tracefs)$" + image: docker.io/prom/node-exporter:v0.18.1 + imagePullPolicy: IfNotPresent + name: node-exporter + - args: + - --logtostderr + - --secure-listen-address=[$(IP)]:9100 + - --tls-cipher-suites=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_RSA_WITH_AES_128_CBC_SHA256,TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256,TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 + - --upstream=http://127.0.0.1:9100/ + image: kubesphere/kube-rbac-proxy:v0.8.0 + imagePullPolicy: IfNotPresent + name: kube-rbac-proxy \ No newline at end of file