diff --git a/pkg/kapp/resources/mod_field_copy.go b/pkg/kapp/resources/mod_field_copy.go index 10900c0ec..fdf362d5f 100644 --- a/pkg/kapp/resources/mod_field_copy.go +++ b/pkg/kapp/resources/mod_field_copy.go @@ -28,29 +28,37 @@ func (t FieldCopyMod) ApplyFromMultiple(res Resource, srcs map[FieldCopyModSourc return nil } - // 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 { + // Make a copy of resource, to avoid modifications + // that may be done even in case when there is nothing to copy + updatedRes := res.DeepCopy() + source, found := srcs[src] + if !found { + continue + } + //updatedSrcRes := source.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) @@ -60,13 +68,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.ContainsNonMapKeysAndRegex() { + if path.ContainsArrayIndex() { return false, nil } obj = map[string]interface{}{} @@ -85,6 +101,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 { @@ -93,7 +114,7 @@ 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) + updated, err := t.apply(obj, srcTypedObj[objI], path[i+1:], newFullPath, srcs) if err != nil { return false, err } @@ -110,9 +131,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 @@ -122,7 +149,7 @@ func (t FieldCopyMod) apply(obj interface{}, path Path, fullPath Path, srcs map[ } case part.Regex != nil && part.Regex.Regex != nil: - matchedKeys, err := t.matchRegexWithSources(path, srcs) + matchedKeys, err := t.matchRegexWithSrcObj(*part.Regex.Regex, srcObj) if err != nil { return false, err } @@ -130,7 +157,7 @@ func (t FieldCopyMod) apply(obj interface{}, path Path, fullPath Path, srcs map[ for _, key := range matchedKeys { newPath := append(Path{&PathPart{MapKey: &key}}, path[i+1:]...) newFullPath := fullPath[:len(fullPath)-1] - updated, err := t.apply(obj, newPath, newFullPath, srcs) + updated, err := t.apply(obj, srcObj, newPath, newFullPath, srcs) if err != nil { return false, err } @@ -220,51 +247,19 @@ func (t FieldCopyMod) obtainValue(obj interface{}, path Path) (interface{}, bool return obj, true, nil } -func (t FieldCopyMod) matchRegexWithSources(path Path, srcs map[FieldCopyModSource]Resource) ([]string, error) { - for _, src := range t.Sources { - if srcRes, found := srcs[src]; found && srcRes != nil { - matchedKeys, err := t.obtainMatchingRegexKeys(srcRes.unstructured().Object, path) - if err == nil && len(matchedKeys) > 0 { - return matchedKeys, err - } - } - } - return []string{}, fmt.Errorf("Field value not present in mentioned sources %s", t.Sources) -} - -func (t FieldCopyMod) obtainMatchingRegexKeys(obj interface{}, path Path) ([]string, error) { +func (t FieldCopyMod) matchRegexWithSrcObj(regexString string, srcObj interface{}) ([]string, error) { var matchedKeys []string - for _, part := range path { - switch { - case part.MapKey != nil: - typedObj, ok := obj.(map[string]interface{}) - if !ok && typedObj != nil { - return matchedKeys, fmt.Errorf("Unexpected non-map found: %T", obj) - } - - var found bool - obj, found = typedObj[*part.MapKey] - if !found { - return matchedKeys, nil - } - - case part.Regex != nil && part.Regex.Regex != nil: - regex, err := regexp.Compile(*part.Regex.Regex) - if err != nil { - return matchedKeys, err - } - typedObj, ok := obj.(map[string]interface{}) - if !ok && typedObj != nil { - return matchedKeys, fmt.Errorf("Unexpected non-map found: %T", obj) - } - for key := range typedObj { - if regex.MatchString(key) { - matchedKeys = append(matchedKeys, key) - } - } - - default: - panic(fmt.Sprintf("Unexpected path part: %#v", part)) + 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 6b67681c8..0db7954c7 100644 --- a/pkg/kapp/resources/mod_field_remove.go +++ b/pkg/kapp/resources/mod_field_remove.go @@ -93,7 +93,7 @@ func (t FieldRemoveMod) apply(obj interface{}, path Path) error { panic(fmt.Sprintf("Unknown array index: %#v", part.ArrayIndex)) } case part.Regex != nil && part.Regex.Regex != nil: - matchedKeys, err := t.obtainMatchingRegexKeys(obj, part) + matchedKeys, err := t.obtainMatchingRegexKeys(obj, *part.Regex.Regex) if err != nil { return err } @@ -114,9 +114,9 @@ func (t FieldRemoveMod) apply(obj interface{}, path Path) error { panic("unreachable") } -func (t FieldRemoveMod) obtainMatchingRegexKeys(obj interface{}, part *PathPart) ([]string, error) { +func (t FieldRemoveMod) obtainMatchingRegexKeys(obj interface{}, regexString string) ([]string, error) { var matchedKeys []string - regex, err := regexp.Compile(*part.Regex.Regex) + regex, err := regexp.Compile(regexString) if err != nil { return matchedKeys, err } diff --git a/pkg/kapp/resources/mod_path.go b/pkg/kapp/resources/mod_path.go index 12bcf4bd2..b194dbedb 100644 --- a/pkg/kapp/resources/mod_path.go +++ b/pkg/kapp/resources/mod_path.go @@ -87,9 +87,9 @@ func (p Path) ContainsNonMapKeys() bool { return false } -func (p Path) ContainsNonMapKeysAndRegex() bool { +func (p Path) ContainsArrayIndex() bool { for _, part := range p { - if part.MapKey == nil && (part.Regex == nil || part.Regex.Regex == nil) { + if part.ArrayIndex != nil { return true } } diff --git a/test/e2e/config_test.go b/test/e2e/config_test.go index 8719f7ed9..f20978af9 100644 --- a/test/e2e/config_test.go +++ b/test/e2e/config_test.go @@ -691,7 +691,7 @@ rules: } func TestConfigHavingRegex(t *testing.T) { - yaml1 := ` + configMapResYaml := ` apiVersion: v1 kind: ConfigMap metadata: @@ -712,7 +712,7 @@ data: player_initial_lives: "3" ` - yaml2 := ` + updatedConfigMapResYaml := ` apiVersion: v1 kind: ConfigMap metadata: @@ -752,6 +752,85 @@ rebaseRules: - 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} @@ -766,12 +845,12 @@ rebaseRules: logger.Section("deploy configmaps with annotations", func() { _, _ = kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name}, - RunOpts{IntoNs: true, StdinReader: strings.NewReader(yaml1)}) + 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(yaml2 + fmt.Sprintf(configYaml, "copy"))}) + 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") @@ -780,7 +859,7 @@ rebaseRules: 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(yaml2 + fmt.Sprintf(faultyConfigYaml, "copy"))}) + 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") @@ -788,7 +867,7 @@ rebaseRules: 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(yaml1 + fmt.Sprintf(configYaml, "remove"))}) + RunOpts{IntoNs: true, StdinReader: strings.NewReader(configMapResYaml + fmt.Sprintf(configYaml, "remove"))}) expectedOutput := ` --- @@ -820,6 +899,63 @@ metadata: 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"))}) + 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"))}) + + 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"))}) + + 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\"}"))}) + + 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\"}"))}) + + 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"))}) + + 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 {