From ec961f8c8a5af2e961599991bbd9a4a6751ab769 Mon Sep 17 00:00:00 2001 From: Yash Sethiya Date: Tue, 5 Sep 2023 20:18:17 +0530 Subject: [PATCH] Supporting regex in config path (#636) * started supporting regex in config path Signed-off-by: sethiyash * copying with calling apply recursively Signed-off-by: sethiyash * refactoring Signed-off-by: sethiyash * fixing linting issues Signed-off-by: sethiyash * stop overwriting object in copy mod and made changes in remove mod Signed-off-by: sethiyash * tweaked the logic to resolve issues Signed-off-by: sethiyash * added e2e testcase with regex in config Signed-off-by: sethiyash * made suggested changes on e2e testcases Signed-off-by: sethiyash * regex as seprate path part Signed-off-by: sethiyash * renamed regexObj to regex Signed-off-by: sethiyash * fixing testcase Signed-off-by: sethiyash * added nil check Signed-off-by: Yash Sethiya Signed-off-by: sethiyash * refactored logic to support regex with indexes Signed-off-by: sethiyash * adopted some nits Signed-off-by: sethiyash * Added comment in e2e testcases Signed-off-by: sethiyash --------- Signed-off-by: sethiyash --- pkg/kapp/resources/mod_field_copy.go | 105 ++++++-- pkg/kapp/resources/mod_field_remove.go | 17 ++ pkg/kapp/resources/mod_object_ref_set.go | 3 + pkg/kapp/resources/mod_path.go | 19 ++ pkg/kapp/resources/mod_string_map_append.go | 3 + test/e2e/config_test.go | 276 ++++++++++++++++++++ 6 files changed, 407 insertions(+), 16 deletions(-) diff --git a/pkg/kapp/resources/mod_field_copy.go b/pkg/kapp/resources/mod_field_copy.go index ea02eb433..b6ebe5876 100644 --- a/pkg/kapp/resources/mod_field_copy.go +++ b/pkg/kapp/resources/mod_field_copy.go @@ -5,6 +5,7 @@ package resources import ( "fmt" + "regexp" ) type FieldCopyModSource string @@ -30,29 +31,37 @@ func (t FieldCopyMod) IsResourceMatching(res Resource) bool { } func (t FieldCopyMod) ApplyFromMultiple(res Resource, srcs map[FieldCopyModSource]Resource) error { - // Make a copy of resource, to avoid modifications - // that may be done even in case when there is nothing to copy - updatedRes := res.DeepCopy() - - updated, err := t.apply(updatedRes.unstructured().Object, t.Path, Path{}, srcs) - if err != nil { - return fmt.Errorf("FieldCopyMod for path '%s' on resource '%s': %s", t.Path.AsString(), res.Description(), err) - } - - if updated { - res.setUnstructured(updatedRes.unstructured()) + for _, src := range t.Sources { + source, found := srcs[src] + if !found { + continue + } + // Make a copy of resource, to avoid modifications + // that may be done even in case when there is nothing to copy + updatedRes := res.DeepCopy() + updated, err := t.apply(updatedRes.unstructured().Object, source.unstructured().Object, t.Path, Path{}, srcs) + if err != nil { + return fmt.Errorf("FieldCopyMod for path '%s' on resource '%s': %s", t.Path.AsString(), res.Description(), err) + } + if updated { + res.setUnstructured(updatedRes.unstructured()) + } } return nil } -func (t FieldCopyMod) apply(obj interface{}, path Path, fullPath Path, srcs map[FieldCopyModSource]Resource) (bool, error) { +func (t FieldCopyMod) apply(obj interface{}, srcObj interface{}, path Path, fullPath Path, srcs map[FieldCopyModSource]Resource) (bool, error) { for i, part := range path { isLast := len(path) == i+1 fullPath = append(fullPath, part) switch { case part.MapKey != nil: + srcTypedObj, ok := srcObj.(map[string]interface{}) + if !ok { + return false, fmt.Errorf("Unexpected non-map found: %T", srcObj) + } typedObj, ok := obj.(map[string]interface{}) if !ok { return false, fmt.Errorf("Unexpected non-map found: %T", obj) @@ -62,13 +71,21 @@ func (t FieldCopyMod) apply(obj interface{}, path Path, fullPath Path, srcs map[ return t.copyIntoMap(typedObj, fullPath, srcs) } - var found bool + var ( + found bool + srcObjFound bool + ) + srcObj, srcObjFound = srcTypedObj[*part.MapKey] + if !srcObjFound || srcObj == nil { + return false, nil + } + obj, found = typedObj[*part.MapKey] // TODO check strictness? if !found || obj == nil { // create empty maps if there are no downstream array indexes; // if there are, we cannot make them anyway, so just exit - if path.ContainsNonMapKeys() { + if path.ContainsArrayIndex() { return false, nil } obj = map[string]interface{}{} @@ -87,6 +104,11 @@ func (t FieldCopyMod) apply(obj interface{}, path Path, fullPath Path, srcs map[ return false, fmt.Errorf("Unexpected non-array found: %T", obj) } + srcTypedObj, ok := srcObj.([]interface{}) + if !ok { + return false, fmt.Errorf("Unexpected non-array found: %T", srcObj) + } + var anyUpdated bool for objI, obj := range typedObj { @@ -95,7 +117,11 @@ func (t FieldCopyMod) apply(obj interface{}, path Path, fullPath Path, srcs map[ newFullPath := append([]*PathPart{}, fullPath...) newFullPath[len(newFullPath)-1] = &PathPart{ArrayIndex: &PathPartArrayIndex{Index: &objI}} - updated, err := t.apply(obj, path[i+1:], newFullPath, srcs) + var srcTypeObj map[string]interface{} + if objI < len(srcTypedObj) { + srcTypeObj = srcTypedObj[objI].(map[string]interface{}) + } + updated, err := t.apply(obj, srcTypeObj, path[i+1:], newFullPath, srcs) if err != nil { return false, err } @@ -112,9 +138,15 @@ func (t FieldCopyMod) apply(obj interface{}, path Path, fullPath Path, srcs map[ return false, fmt.Errorf("Unexpected non-array found: %T", obj) } + srcTypedObj, ok := srcObj.([]interface{}) + if !ok { + return false, fmt.Errorf("Unexpected non-array found: %T", srcObj) + } + if *part.ArrayIndex.Index < len(typedObj) { obj = typedObj[*part.ArrayIndex.Index] - return t.apply(obj, path[i+1:], fullPath, srcs) + srcObj = srcTypedObj[*part.ArrayIndex.Index] + return t.apply(obj, srcObj, path[i+1:], fullPath, srcs) } return false, nil // index not found, nothing to append to @@ -123,6 +155,29 @@ func (t FieldCopyMod) apply(obj interface{}, path Path, fullPath Path, srcs map[ panic(fmt.Sprintf("Unknown array index: %#v", part.ArrayIndex)) } + case part.Regex != nil: + if part.Regex.Regex == nil { + panic("Regex should be non nil") + } + matchedKeys, err := matchRegexWithSrcObj(*part.Regex.Regex, srcObj) + if err != nil { + return false, err + } + var anyUpdated bool + for _, key := range matchedKeys { + newPath := append(Path{&PathPart{MapKey: &key}}, path[i+1:]...) + newFullPath := fullPath[:len(fullPath)-1] + updated, err := t.apply(obj, srcObj, newPath, newFullPath, srcs) + if err != nil { + return false, err + } + if updated { + anyUpdated = true + } + } + + return anyUpdated, nil + default: panic(fmt.Sprintf("Unexpected path part: %#v", part)) } @@ -203,3 +258,21 @@ func (t FieldCopyMod) obtainValue(obj interface{}, path Path) (interface{}, bool return obj, true, nil } + +func matchRegexWithSrcObj(regexString string, srcObj interface{}) ([]string, error) { + var matchedKeys []string + regex, err := regexp.Compile(regexString) + if err != nil { + return matchedKeys, err + } + srcTypedObj, ok := srcObj.(map[string]interface{}) + if !ok && srcTypedObj != nil { + return matchedKeys, fmt.Errorf("Unexpected non-map found: %T", srcObj) + } + for key := range srcTypedObj { + if regex.MatchString(key) { + matchedKeys = append(matchedKeys, key) + } + } + return matchedKeys, nil +} diff --git a/pkg/kapp/resources/mod_field_remove.go b/pkg/kapp/resources/mod_field_remove.go index 3f7d0569e..513e8603b 100644 --- a/pkg/kapp/resources/mod_field_remove.go +++ b/pkg/kapp/resources/mod_field_remove.go @@ -96,6 +96,23 @@ func (t FieldRemoveMod) apply(obj interface{}, path Path) error { default: panic(fmt.Sprintf("Unknown array index: %#v", part.ArrayIndex)) } + case part.Regex != nil: + if part.Regex.Regex == nil { + panic("Regex should be non nil") + } + matchedKeys, err := matchRegexWithSrcObj(*part.Regex.Regex, obj) + if err != nil { + return err + } + for _, key := range matchedKeys { + newPath := append(Path{&PathPart{MapKey: &key}}, path[i+1:]...) + err := t.apply(obj, newPath) + if err != nil { + return err + } + } + + return nil default: panic(fmt.Sprintf("Unexpected path part: %#v", part)) diff --git a/pkg/kapp/resources/mod_object_ref_set.go b/pkg/kapp/resources/mod_object_ref_set.go index 40d3d46f7..9509247aa 100644 --- a/pkg/kapp/resources/mod_object_ref_set.go +++ b/pkg/kapp/resources/mod_object_ref_set.go @@ -77,6 +77,9 @@ func (t ObjectRefSetMod) apply(obj interface{}, path Path) error { panic(fmt.Sprintf("Unknown array index: %#v", part.ArrayIndex)) } + case part.Regex != nil: + panic("Regex in path part is only supported for rebaseRules.") + default: panic(fmt.Sprintf("Unexpected path part: %#v", part)) } diff --git a/pkg/kapp/resources/mod_path.go b/pkg/kapp/resources/mod_path.go index ca84392a1..8d6ef7d23 100644 --- a/pkg/kapp/resources/mod_path.go +++ b/pkg/kapp/resources/mod_path.go @@ -22,6 +22,7 @@ type Path []*PathPart type PathPart struct { MapKey *string + Regex *PathPartRegex ArrayIndex *PathPartArrayIndex } @@ -32,6 +33,10 @@ type PathPartArrayIndex struct { All *bool `json:"allIndexes"` } +type PathPartRegex struct { + Regex *string `json:"regex"` +} + func NewPathFromStrings(strs []string) Path { var path Path for _, str := range strs { @@ -83,6 +88,15 @@ func (p Path) ContainsNonMapKeys() bool { return false } +func (p Path) ContainsArrayIndex() bool { + for _, part := range p { + if part.ArrayIndex != nil { + return true + } + } + return false +} + func NewPathPartFromString(str string) *PathPart { return &PathPart{MapKey: &str} } @@ -104,6 +118,8 @@ func (p *PathPart) AsString() string { return fmt.Sprintf("%d", *p.ArrayIndex.Index) case p.ArrayIndex != nil && p.ArrayIndex.All != nil: return "(all)" + case p.Regex != nil && p.Regex.Regex != nil: + return *p.Regex.Regex default: panic("Unknown path part") } @@ -112,10 +128,13 @@ func (p *PathPart) AsString() string { func (p *PathPart) UnmarshalJSON(data []byte) error { var str string var idx PathPartArrayIndex + var regx PathPartRegex switch { case json.Unmarshal(data, &str) == nil: p.MapKey = &str + case json.Unmarshal(data, ®x) == nil && regx.Regex != nil: + p.Regex = ®x case json.Unmarshal(data, &idx) == nil: p.ArrayIndex = &idx default: diff --git a/pkg/kapp/resources/mod_string_map_append.go b/pkg/kapp/resources/mod_string_map_append.go index b4c8cbe57..638d41262 100644 --- a/pkg/kapp/resources/mod_string_map_append.go +++ b/pkg/kapp/resources/mod_string_map_append.go @@ -84,6 +84,9 @@ func (t StringMapAppendMod) apply(obj interface{}, path Path) error { panic(fmt.Sprintf("Unknown array index: %#v", part.ArrayIndex)) } + case part.Regex != nil: + panic("Regex in path part is only supported for rebaseRules.") + default: panic(fmt.Sprintf("Unexpected path part: %#v", part)) } diff --git a/test/e2e/config_test.go b/test/e2e/config_test.go index e56c9ea3c..430fa2522 100644 --- a/test/e2e/config_test.go +++ b/test/e2e/config_test.go @@ -4,6 +4,7 @@ package e2e import ( + "fmt" "math/rand" "strings" "testing" @@ -777,6 +778,281 @@ rules: }) } +func TestConfigHavingRegex(t *testing.T) { + configMapResYaml := ` +apiVersion: v1 +kind: ConfigMap +metadata: + name: game-demo + annotations: + foo1: bar1 + foo2: bar2 +data: + player_initial_lives: "3" +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: game-test + annotations: + foo2: bar2 +data: + player_initial_lives: "3" +` + + updatedConfigMapResYaml := ` +apiVersion: v1 +kind: ConfigMap +metadata: + name: game-demo +data: + player_initial_lives: "3" +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: game-test +data: + player_initial_lives: "3" +` + + configYaml := ` +--- +apiVersion: kapp.k14s.io/v1alpha1 +kind: Config +rebaseRules: + - path: [metadata, annotations, {regex: "^foo"}] + type: %s + sources: [new, existing] + resourceMatchers: + - apiVersionKindMatcher: {apiVersion: v1, kind: ConfigMap} +` + + faultyConfigYaml := ` +--- +apiVersion: kapp.k14s.io/v1alpha1 +kind: Config +rebaseRules: + - path: [metadata, annotations, {regex: }] + type: %s + sources: [new, existing] + resourceMatchers: + - apiVersionKindMatcher: {apiVersion: v1, kind: ConfigMap} +` + + deploymentResYaml := ` +apiVersion: apps/v1 +kind: Deployment +metadata: + namespace: default + name: simple-app +spec: + selector: + matchLabels: + simple-app: "" + template: + metadata: + labels: + simple-app: "" + spec: + containers: + - name: simple-app + image: docker.io/dkalinin/k8s-simple-app@sha256:4c8b96d4fffdfae29258d94a22ae4ad1fe36139d47288b8960d9958d1e63a9d0 + env: + - name: HELLO + value: strange + - name: HELLO_MSG + value: stranger +` + + updatedDeploymentResYaml := ` +apiVersion: apps/v1 +kind: Deployment +metadata: + namespace: default + name: simple-app +spec: + selector: + matchLabels: + simple-app: "" + template: + metadata: + labels: + simple-app: "" + spec: + containers: + - name: simple-app + image: docker.io/dkalinin/k8s-simple-app@sha256:4c8b96d4fffdfae29258d94a22ae4ad1fe36139d47288b8960d9958d1e63a9d0 + env: + - name: HELLO + - name: HELLO_MSG +` + + deploymentConfig := ` +--- +apiVersion: kapp.k14s.io/v1alpha1 +kind: Config + +rebaseRules: + - path: [spec, template, spec, containers, {allIndexes: true}, env, %s] + type: %s + sources: [new, existing] + resourceMatchers: + - apiVersionKindMatcher: {apiVersion: apps/v1, kind: Deployment} +` + + deploymentConfigIndex := ` +--- +apiVersion: kapp.k14s.io/v1alpha1 +kind: Config + +rebaseRules: + - path: [spec, template, spec, containers, {allIndexes: true}, env, {index: 0}, %s] + type: copy + sources: [new, existing] + resourceMatchers: + - apiVersionKindMatcher: {apiVersion: apps/v1, kind: Deployment} + - path: [spec, template, spec, containers, {allIndexes: true}, env, {index: 1}, %s] + type: copy + sources: [new, existing] + resourceMatchers: + - apiVersionKindMatcher: {apiVersion: apps/v1, kind: Deployment} +` + + env := BuildEnv(t) + logger := Logger{} + kapp := Kapp{t, env.Namespace, env.KappBinaryPath, logger} + fieldsExcludedInMatch := []string{"kapp.k14s.io/app", "creationTimestamp:", "resourceVersion:", "uid:", "selfLink:", "kapp.k14s.io/association"} + name := "test-config-path-regex" + cleanUp := func() { + kapp.Run([]string{"delete", "-a", name}) + } + + cleanUp() + defer cleanUp() + + logger.Section("deploy configmaps with annotations", func() { + _, _ = kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name}, + RunOpts{IntoNs: true, StdinReader: strings.NewReader(configMapResYaml)}) + }) + + logger.Section("deploy configmaps without annotations", func() { + _, err := kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name, "--diff-run", "--diff-exit-status"}, + RunOpts{IntoNs: true, AllowError: true, StdinReader: strings.NewReader(updatedConfigMapResYaml + fmt.Sprintf(configYaml, "copy"))}) + + require.Errorf(t, err, "Expected to receive error") + require.Containsf(t, err.Error(), "Exiting after diffing with no pending changes (exit status 2)", "Expected to find stderr output") + require.Containsf(t, err.Error(), "exit code: '2'", "Expected to find exit code") + }) + + logger.Section("passing faulty config", func() { + _, err := kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name, "--diff-run", "--diff-exit-status"}, + RunOpts{IntoNs: true, AllowError: true, StdinReader: strings.NewReader(updatedConfigMapResYaml + fmt.Sprintf(faultyConfigYaml, "copy"))}) + + require.Errorf(t, err, "Expected to receive error") + require.Containsf(t, err.Error(), "panic: Unknown path part", "Expected to panic") + }) + + logger.Section("Remove all the annotation with remove config", func() { + out, _ := kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name, "--diff-changes-yaml"}, + RunOpts{IntoNs: true, StdinReader: strings.NewReader(configMapResYaml + fmt.Sprintf(configYaml, "remove"))}) + + expectedOutput := ` +--- +# update: configmap/game-demo (v1) namespace: kapp-test +apiVersion: v1 +data: + player_initial_lives: "3" +kind: ConfigMap +metadata: + annotations: {} + labels: + name: game-demo + namespace: kapp-test +--- +# update: configmap/game-test (v1) namespace: kapp-test +apiVersion: v1 +data: + player_initial_lives: "3" +kind: ConfigMap +metadata: + annotations: {} + labels: + name: game-test + namespace: kapp-test +` + out = strings.TrimSpace(replaceTarget(replaceSpaces(replaceTs(out)))) + out = clearKeys(fieldsExcludedInMatch, out) + + expectedOutput = strings.TrimSpace(replaceSpaces(expectedOutput)) + require.Contains(t, out, expectedOutput, "output does not match") + }) + + logger.Section("Deployment resource", func() { + _, _ = kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name}, + RunOpts{IntoNs: true, StdinReader: strings.NewReader(deploymentResYaml)}) + }) + + logger.Section("Deployment resource with remove value field and copying with rebase rule", func() { + _, err := kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name, "--diff-run", "--diff-exit-status"}, + RunOpts{IntoNs: true, AllowError: true, StdinReader: strings.NewReader(updatedDeploymentResYaml + fmt.Sprintf(deploymentConfig, "{allIndexes: true}, value", "copy"))}) + + // no change as value field is copied again for all indexes in the updatedDeployment using config resource + require.Errorf(t, err, "Expected to receive error") + require.Containsf(t, err.Error(), "Exiting after diffing with no pending changes (exit status 2)", "Expected to find stderr output") + require.Containsf(t, err.Error(), "exit code: '2'", "Expected to find exit code") + }) + + logger.Section("Deployment resource with remove value field and copying with rebase rule using regex", func() { + _, err := kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name, "--diff-run", "--diff-exit-status"}, + RunOpts{IntoNs: true, AllowError: true, StdinReader: strings.NewReader(updatedDeploymentResYaml + fmt.Sprintf(deploymentConfig, "{allIndexes: true}, {regex: \"^val\"}", "copy"))}) + + // no change as value field is copied again using regex for all indexes in the updatedDeployment using config resource + require.Errorf(t, err, "Expected to receive error") + require.Containsf(t, err.Error(), "Exiting after diffing with no pending changes (exit status 2)", "Expected to find stderr output") + require.Containsf(t, err.Error(), "exit code: '2'", "Expected to find exit code") + }) + + logger.Section("Deployment resource with remove value field and copying with rebase rule using index and field", func() { + _, err := kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name, "--diff-run", "--diff-exit-status"}, + RunOpts{IntoNs: true, AllowError: true, StdinReader: strings.NewReader(updatedDeploymentResYaml + fmt.Sprintf(deploymentConfigIndex, "value", "value"))}) + + // no change as value field is copied again for both index of env 0 and 1 in the updatedDeployment using config resource + require.Errorf(t, err, "Expected to receive error") + require.Containsf(t, err.Error(), "Exiting after diffing with no pending changes (exit status 2)", "Expected to find stderr output") + require.Containsf(t, err.Error(), "exit code: '2'", "Expected to find exit code") + }) + + logger.Section("Deployment resource with remove value field and copying with rebase rule using index and regex", func() { + _, err := kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name, "--diff-run", "--diff-exit-status"}, + RunOpts{IntoNs: true, AllowError: true, StdinReader: strings.NewReader(updatedDeploymentResYaml + fmt.Sprintf(deploymentConfigIndex, "{regex: \"^val\"}", "{regex: \"^val\"}"))}) + + // no change as value field is copied again using regex for both index of env 0 and 1 in the updatedDeployment using config resource + require.Errorf(t, err, "Expected to receive error") + require.Containsf(t, err.Error(), "Exiting after diffing with no pending changes (exit status 2)", "Expected to find stderr output") + require.Containsf(t, err.Error(), "exit code: '2'", "Expected to find exit code") + }) + + logger.Section("Deployment resource with remove value field and unmatched regex", func() { + _, err := kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name, "--diff-run", "--diff-exit-status"}, + RunOpts{IntoNs: true, AllowError: true, StdinReader: strings.NewReader(updatedDeploymentResYaml + fmt.Sprintf(deploymentConfigIndex, "{regex: \"^tal\"}", "{regex: \"^tal\"}"))}) + + // change exists as no field is present as per given regex and hence it was unable to copy the field + require.Errorf(t, err, "Expected to receive error") + require.Containsf(t, err.Error(), "Exiting after diffing with pending changes (exit status 3)", "Expected to find stderr output") + }) + + logger.Section("Deployment resource with remove value field and unmatched regex and allIndex", func() { + _, err := kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name, "--diff-run", "--diff-exit-status"}, + RunOpts{IntoNs: true, AllowError: true, StdinReader: strings.NewReader(updatedDeploymentResYaml + fmt.Sprintf(deploymentConfig, "{allIndexes: true}, {regex: \"^tal\"}", "copy"))}) + + // change exists as no field is present as per given regex on all the indexes and hence it was unable to copy the field + require.Errorf(t, err, "Expected to receive error") + require.Containsf(t, err.Error(), "Exiting after diffing with pending changes (exit status 3)", "Expected to find stderr output") + }) + +} + func RandomString(n int) string { letters := []rune("abcdefghijklmnopqrstuvwxyz0123456789")