From 081244a1027c052f30e24f465a225223689100dc Mon Sep 17 00:00:00 2001 From: shuting Date: Wed, 14 Apr 2021 13:09:00 -0700 Subject: [PATCH] Fix array variables substitution (#1800) * fix array variables substitution Signed-off-by: Shuting Zhao * Add ability to marshal complex vars to string Signed-off-by: Max Goncharenko * Added tests for variable substitution Signed-off-by: Max Goncharenko Co-authored-by: Max Goncharenko --- pkg/engine/validation_test.go | 68 ++++- pkg/engine/variables/variables_test.go | 23 +- pkg/engine/variables/vars.go | 38 ++- pkg/engine/variables/vars_test.go | 336 +++++++++++++++++++++++++ 4 files changed, 449 insertions(+), 16 deletions(-) diff --git a/pkg/engine/validation_test.go b/pkg/engine/validation_test.go index 33cc252c54..1b09f3aa85 100644 --- a/pkg/engine/validation_test.go +++ b/pkg/engine/validation_test.go @@ -1415,6 +1415,65 @@ func Test_VariableSubstitutionPathNotExistInAnyPattern_OnePatternStatisfiesButSu assert.Equal(t, er.PolicyResponse.Rules[0].Message, "variable substitution failed for rule test-path-not-exist: variable request.object.metadata.name1 not resolved at path /validate/anyPattern/0/spec/template/spec/containers/0/name") } +func Test_VariableSubstitution_NotOperatorWithStringVariable(t *testing.T) { + resourceRaw := []byte(`{ + "apiVersion": "v1", + "kind": "Deployment", + "metadata": { + "name": "test" + }, + "spec": { + "content": "sample text" + } + }`) + + policyraw := []byte(`{ + "apiVersion": "kyverno.io/v1", + "kind": "ClusterPolicy", + "metadata": { + "name": "substitute-variable" + }, + "spec": { + "rules": [ + { + "name": "not-operator-with-variable-should-alway-fail-validation", + "match": { + "resources": { + "kinds": [ + "Deployment" + ] + } + }, + "validate": { + "pattern": { + "spec": { + "content": "!{{ request.object.spec.content }}" + } + } + } + } + ] + } + }`) + + var policy kyverno.ClusterPolicy + assert.NilError(t, json.Unmarshal(policyraw, &policy)) + resourceUnstructured, err := utils.ConvertToUnstructured(resourceRaw) + assert.NilError(t, err) + + ctx := context.NewContext() + err = ctx.AddResource(resourceRaw) + assert.NilError(t, err) + + policyContext := &PolicyContext{ + Policy: policy, + JSONContext: ctx, + NewResource: *resourceUnstructured} + er := Validate(policyContext) + assert.Assert(t, !er.PolicyResponse.Rules[0].Success) + assert.Equal(t, er.PolicyResponse.Rules[0].Message, "validation error: rule not-operator-with-variable-should-alway-fail-validation failed at path /spec/content/") +} + func Test_VariableSubstitutionPathNotExistInAnyPattern_AllPathNotPresent(t *testing.T) { resourceRaw := []byte(`{ "apiVersion": "v1", @@ -1669,7 +1728,12 @@ func Test_VariableSubstitutionValidate_VariablesInMessageAreResolved(t *testing. { "key": "{{ request.object.metadata.labels.animal }}", "operator": "NotIn", - "value": "abcde" + "value": [ + "snake", + "bear", + "cat", + "dog" + ] } ] } @@ -1693,7 +1757,7 @@ func Test_VariableSubstitutionValidate_VariablesInMessageAreResolved(t *testing. JSONContext: ctx, NewResource: *resourceUnstructured} er := Validate(policyContext) - assert.Assert(t, er.PolicyResponse.Rules[0].Success) + assert.Assert(t, !er.PolicyResponse.Rules[0].Success) assert.Equal(t, er.PolicyResponse.Rules[0].Message, "The animal cow is not in the allowed list of animals.") } diff --git a/pkg/engine/variables/variables_test.go b/pkg/engine/variables/variables_test.go index a42a8ea062..73771bd653 100644 --- a/pkg/engine/variables/variables_test.go +++ b/pkg/engine/variables/variables_test.go @@ -6,6 +6,7 @@ import ( "testing" kyverno "github.com/kyverno/kyverno/pkg/api/kyverno/v1" + "gotest.tools/assert" authenticationv1 "k8s.io/api/authentication/v1" "sigs.k8s.io/controller-runtime/pkg/log" @@ -562,10 +563,26 @@ func Test_variableSubstitutionObjectOperatorNotEqualFail(t *testing.T) { t.Error(err) } - if patternCopy, err = SubstituteAll(log.Log, ctx, patternCopy); err == nil { - t.Error(err) - } + patternCopy, err = SubstituteAll(log.Log, ctx, patternCopy) + assert.NilError(t, err) + patternMapCopy, ok := patternCopy.(map[string]interface{}) + assert.Assert(t, ok) + + specInterface, ok := patternMapCopy["spec"] + assert.Assert(t, ok) + + specMap, ok := specInterface.(map[string]interface{}) + assert.Assert(t, ok) + + variableInterface, ok := specMap["variable"] + assert.Assert(t, ok) + + variableString, ok := variableInterface.(string) + assert.Assert(t, ok) + + expected := `!{"var1":"temp1","var2":"temp2","varNested":{"var1":"temp1"}}` + assert.Equal(t, expected, variableString) } func Test_variableSubstitutionMultipleObject(t *testing.T) { diff --git a/pkg/engine/variables/vars.go b/pkg/engine/variables/vars.go index 1173d314dc..c8a2d005b4 100644 --- a/pkg/engine/variables/vars.go +++ b/pkg/engine/variables/vars.go @@ -104,7 +104,7 @@ func validateBackgroundModeVars(log logr.Logger, ctx context.EvalInterface) json case context.InvalidVariableErr: return nil, err default: - return nil, fmt.Errorf("failed to resolve %v at path %s", variable, data.Path) + return nil, fmt.Errorf("failed to resolve %v at path %s: %v", variable, data.Path, err) } } } @@ -146,12 +146,12 @@ func substituteReferencesIfAny(log logr.Logger) jsonUtils.Action { case context.InvalidVariableErr: return nil, err default: - return nil, fmt.Errorf("failed to resolve %v at path %s", v, data.Path) + return nil, fmt.Errorf("failed to resolve %v at path %s: %v", v, data.Path, err) } } if resolvedReference == nil { - return data.Element, fmt.Errorf("failed to resolve %v at path %s", v, data.Path) + return data.Element, fmt.Errorf("failed to resolve %v at path %s: %v", v, data.Path, err) } log.V(3).Info("reference resolved", "reference", v, "value", resolvedReference, "path", data.Path) @@ -178,9 +178,10 @@ func substituteVariablesIfAny(log logr.Logger, ctx context.EvalInterface) jsonUt return data.Element, nil } - originalPattern := value vars := regexVariables.FindAllString(value, -1) for len(vars) > 0 { + originalPattern := value + for _, v := range vars { variable := replaceBracesAndTrimSpaces(v) @@ -195,23 +196,22 @@ func substituteVariablesIfAny(log logr.Logger, ctx context.EvalInterface) jsonUt case context.InvalidVariableErr: return nil, err default: - return nil, fmt.Errorf("failed to resolve %v at path %s", variable, data.Path) + return nil, fmt.Errorf("failed to resolve %v at path %s: %v", variable, data.Path, err) } } log.V(3).Info("variable substituted", "variable", v, "value", substitutedVar, "path", data.Path) - if val, ok := substitutedVar.(string); ok { - value = strings.Replace(value, v, val, -1) - continue - } - if substitutedVar != nil { if originalPattern == v { return substitutedVar, nil } - return nil, fmt.Errorf("failed to resolve %v at path %s", variable, data.Path) + if value, err = substituteVarInPattern(originalPattern, v, substitutedVar); err != nil { + return nil, fmt.Errorf("failed to resolve %v at path %s: %s", variable, data.Path, err.Error()) + } + + continue } return nil, NotFoundVariableErr{ @@ -228,6 +228,22 @@ func substituteVariablesIfAny(log logr.Logger, ctx context.EvalInterface) jsonUt }) } +func substituteVarInPattern(pattern, variable string, value interface{}) (string, error) { + var stringToSubstitute string + + if s, ok := value.(string); ok { + stringToSubstitute = s + } else { + buffer, err := json.Marshal(value) + if err != nil { + return "", fmt.Errorf("failed to marshal %T: %v", value, value) + } + stringToSubstitute = string(buffer) + } + + return strings.Replace(pattern, variable, stringToSubstitute, -1), nil +} + func replaceBracesAndTrimSpaces(v string) string { variable := strings.ReplaceAll(v, "{{", "") variable = strings.ReplaceAll(variable, "}}", "") diff --git a/pkg/engine/variables/vars_test.go b/pkg/engine/variables/vars_test.go index efb5e1a26a..6b17f98bef 100644 --- a/pkg/engine/variables/vars_test.go +++ b/pkg/engine/variables/vars_test.go @@ -6,6 +6,7 @@ import ( "strings" "testing" + v1 "github.com/kyverno/kyverno/pkg/api/kyverno/v1" "github.com/kyverno/kyverno/pkg/engine/context" ju "github.com/kyverno/kyverno/pkg/engine/json-utils" "gotest.tools/assert" @@ -334,6 +335,341 @@ func Test_policyContextValidation(t *testing.T) { assert.Assert(t, err != nil, err) } +func Test_variableSubstitution_array(t *testing.T) { + configmapRaw := []byte(` +{ + "animals": { + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": { + "name": "animals", + "namespace": "default" + }, + "data": { + "animals": "snake\nbear\ncat\ndog" + } + } +}`) + + ruleRaw := []byte(` +{ + "name": "validate-role-annotation", + "context": [ + { + "name": "animals", + "configMap": { + "name": "animals", + "namespace": "default" + } + } + ], + "match": { + "resources": { + "kinds": [ + "Deployment" + ] + } + }, + "validate": { + "message": "The animal {{ request.object.metadata.labels.animal }} is not in the allowed list of animals: {{ animals.data.animals }}.", + "deny": { + "conditions": [ + { + "key": "{{ request.object.metadata.labels.animal }}", + "operator": "NotIn", + "value": "{{ animals.data.animals }}" + } + ] + } + } +}`) + + resourceRaw := []byte(` +{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": { + "name": "busybox", + "labels": { + "app": "busybox", + "color": "red", + "animal": "cow", + "food": "pizza", + "car": "jeep", + "env": "qa" + } + } +} +`) + + var rule v1.Rule + err := json.Unmarshal(ruleRaw, &rule) + assert.NilError(t, err) + + ctx := context.NewContext("request.object", "animals") + ctx.AddJSON(configmapRaw) + ctx.AddResource(resourceRaw) + + vars, err := SubstituteAllInRule(log.Log, ctx, rule) + assert.NilError(t, err) + + assert.DeepEqual(t, vars.Validation.Message, "The animal cow is not in the allowed list of animals: snake\nbear\ncat\ndog.") +} + +var variableObject = []byte(` +{ + "complex_object_array": [ + "value1", + "value2", + "value3" + ], + "complex_object_map": { + "key1": "value1", + "key2": "value2", + "key3": "value3" + }, + "simple_object_bool": false, + "simple_object_int": 5, + "simple_object_float": -5.5, + "simple_object_string": "example", + "simple_object_null": null +} +`) + +func Test_SubstituteArray(t *testing.T) { + patternRaw := []byte(` + { + "spec": { + "content": "{{ request.object.complex_object_array }}" + } + } + `) + + var err error + var pattern, resource map[string]interface{} + err = json.Unmarshal(patternRaw, &pattern) + assert.NilError(t, err) + + err = json.Unmarshal(variableObject, &resource) + assert.NilError(t, err) + + ctx := context.NewContext() + ctx.AddResource(variableObject) + + resolved, err := SubstituteAll(log.Log, ctx, pattern) + assert.NilError(t, err) + + content := resolved.(map[string]interface{})["spec"].(map[string]interface{})["content"] + expected := resource["complex_object_array"] + + assert.DeepEqual(t, expected, content) +} + +func Test_SubstituteArrayInString(t *testing.T) { + patternRaw := []byte(` + { + "spec": { + "content": "content is {{ request.object.complex_object_map }}" + } + } + `) + + var err error + var pattern, resource map[string]interface{} + err = json.Unmarshal(patternRaw, &pattern) + assert.NilError(t, err) + + err = json.Unmarshal(variableObject, &resource) + assert.NilError(t, err) + + ctx := context.NewContext() + ctx.AddResource(variableObject) + + resolved, err := SubstituteAll(log.Log, ctx, pattern) + assert.NilError(t, err) + + content := resolved.(map[string]interface{})["spec"].(map[string]interface{})["content"] + expected := `content is {"key1":"value1","key2":"value2","key3":"value3"}` + + assert.DeepEqual(t, expected, content) +} + +func Test_SubstituteInt(t *testing.T) { + patternRaw := []byte(` + { + "spec": { + "content": "{{ request.object.simple_object_int }}" + } + } + `) + + var err error + var pattern, resource map[string]interface{} + err = json.Unmarshal(patternRaw, &pattern) + assert.NilError(t, err) + + err = json.Unmarshal(variableObject, &resource) + assert.NilError(t, err) + + ctx := context.NewContext() + ctx.AddResource(variableObject) + + resolved, err := SubstituteAll(log.Log, ctx, pattern) + assert.NilError(t, err) + + content := resolved.(map[string]interface{})["spec"].(map[string]interface{})["content"] + expected := resource["simple_object_int"] + + assert.DeepEqual(t, expected, content) +} + +func Test_SubstituteIntInString(t *testing.T) { + patternRaw := []byte(` + { + "spec": { + "content": "content = {{ request.object.simple_object_int }}" + } + } + `) + + var err error + var pattern, resource map[string]interface{} + err = json.Unmarshal(patternRaw, &pattern) + assert.NilError(t, err) + + err = json.Unmarshal(variableObject, &resource) + assert.NilError(t, err) + + ctx := context.NewContext() + ctx.AddResource(variableObject) + + resolved, err := SubstituteAll(log.Log, ctx, pattern) + assert.NilError(t, err) + + content := resolved.(map[string]interface{})["spec"].(map[string]interface{})["content"] + expected := "content = 5" + + assert.DeepEqual(t, expected, content) +} + +func Test_SubstituteBool(t *testing.T) { + patternRaw := []byte(` + { + "spec": { + "content": "{{ request.object.simple_object_bool }}" + } + } + `) + + var err error + var pattern, resource map[string]interface{} + err = json.Unmarshal(patternRaw, &pattern) + assert.NilError(t, err) + + err = json.Unmarshal(variableObject, &resource) + assert.NilError(t, err) + + ctx := context.NewContext() + ctx.AddResource(variableObject) + + resolved, err := SubstituteAll(log.Log, ctx, pattern) + assert.NilError(t, err) + + content := resolved.(map[string]interface{})["spec"].(map[string]interface{})["content"] + expected := false + + assert.DeepEqual(t, expected, content) +} + +func Test_SubstituteBoolInString(t *testing.T) { + patternRaw := []byte(` + { + "spec": { + "content": "content = {{ request.object.simple_object_bool }}" + } + } + `) + + var err error + var pattern, resource map[string]interface{} + err = json.Unmarshal(patternRaw, &pattern) + assert.NilError(t, err) + + err = json.Unmarshal(variableObject, &resource) + assert.NilError(t, err) + + ctx := context.NewContext() + ctx.AddResource(variableObject) + + resolved, err := SubstituteAll(log.Log, ctx, pattern) + assert.NilError(t, err) + + content := resolved.(map[string]interface{})["spec"].(map[string]interface{})["content"] + expected := "content = false" + + assert.DeepEqual(t, expected, content) +} + +func Test_SubstituteString(t *testing.T) { + patternRaw := []byte(` + { + "spec": { + "content": "{{ request.object.simple_object_string }}" + } + } + `) + + var err error + var pattern, resource map[string]interface{} + err = json.Unmarshal(patternRaw, &pattern) + assert.NilError(t, err) + + err = json.Unmarshal(variableObject, &resource) + assert.NilError(t, err) + + ctx := context.NewContext() + ctx.AddResource(variableObject) + + resolved, err := SubstituteAll(log.Log, ctx, pattern) + assert.NilError(t, err) + + content := resolved.(map[string]interface{})["spec"].(map[string]interface{})["content"] + var expected interface{} + expected = "example" + + assert.DeepEqual(t, expected, content) +} + +func Test_SubstituteStringInString(t *testing.T) { + patternRaw := []byte(` + { + "spec": { + "content": "content = {{ request.object.simple_object_string }}" + } + } + `) + + var err error + var pattern, resource map[string]interface{} + err = json.Unmarshal(patternRaw, &pattern) + assert.NilError(t, err) + + err = json.Unmarshal(variableObject, &resource) + assert.NilError(t, err) + + ctx := context.NewContext() + ctx.AddResource(variableObject) + + resolved, err := SubstituteAll(log.Log, ctx, pattern) + assert.NilError(t, err) + + content := resolved.(map[string]interface{})["spec"].(map[string]interface{})["content"] + var expected interface{} + expected = "content = example" + + assert.DeepEqual(t, expected, content) +} + func Test_ReferenceSubstitution(t *testing.T) { jsonRaw := []byte(` {