From 292d18983da64b4f15610887119521ba1bdc4132 Mon Sep 17 00:00:00 2001 From: Anna Song Date: Mon, 12 Dec 2022 13:41:33 -0500 Subject: [PATCH] Localize patchesJson6902, patchesStrategicMerge, replacements (#4904) * Localize patchesJson6902, patchesStrategicMerge, replacements * Address code review feedback * Improve readability * Remove deprecation warning check --- api/internal/localizer/localizer.go | 61 +++++++--- api/internal/localizer/localizer_test.go | 141 ++++++++++++++++++++--- 2 files changed, 174 insertions(+), 28 deletions(-) diff --git a/api/internal/localizer/localizer.go b/api/internal/localizer/localizer.go index d4d209094..137b6f0ed 100644 --- a/api/internal/localizer/localizer.go +++ b/api/internal/localizer/localizer.go @@ -120,15 +120,6 @@ func (lc *localizer) localizeNativeFields(kust *types.Kustomization) error { } kust.Components[i] = newPath } - for i := range kust.Patches { - if kust.Patches[i].Path != "" { - newPath, err := lc.localizeFile(kust.Patches[i].Path) - if err != nil { - return errors.WrapPrefixf(err, "unable to localize patches path %q", kust.Patches[i].Path) - } - kust.Patches[i].Path = newPath - } - } for i := range kust.ConfigMapGenerator { if err := lc.localizeGenerator(&kust.ConfigMapGenerator[i].GeneratorArgs); err != nil { @@ -140,9 +131,39 @@ func (lc *localizer) localizeNativeFields(kust *types.Kustomization) error { return errors.WrapPrefixf(err, "unable to localize secretGenerator") } } + if err := lc.localizePatches(kust.Patches); err != nil { + return errors.WrapPrefixf(err, "unable to localize patches") + } + // Allow use of deprecated field + //nolint:staticcheck + if err := lc.localizePatches(kust.PatchesJson6902); err != nil { + return errors.WrapPrefixf(err, "unable to localize patchesJson6902") + } + //nolint:staticcheck + for i, patch := range kust.PatchesStrategicMerge { + _, isFile, err := lc.loadResource(string(patch)) + if err != nil { + return errors.WrapPrefixf(err, "invalid patchesStrategicMerge entry") + } + if isFile { + newPath, err := lc.localizeFile(string(patch)) + if err != nil { + return errors.WrapPrefixf(err, "unable to localize patchesStrategicMerge entry") + } + kust.PatchesStrategicMerge[i] = types.PatchStrategicMerge(newPath) + } + } + for i, replacement := range kust.Replacements { + if replacement.Path != "" { + newPath, err := lc.localizeFile(replacement.Path) + if err != nil { + return errors.WrapPrefixf(err, "unable to localize replacements entry") + } + kust.Replacements[i].Path = newPath + } + } - // TODO(annasong): localize all other kustomization fields: resources, bases, crds, configurations, - // openapi, patchesJson6902, patchesStrategicMerge, replacements + // TODO(annasong): localize all other kustomization fields: resources, bases, crds, configurations, openapi return nil } @@ -176,6 +197,20 @@ func (lc *localizer) localizeGenerator(generator *types.GeneratorArgs) error { return nil } +// localizePatches localizes the file paths on patches if they are non-empty +func (lc *localizer) localizePatches(patches []types.Patch) error { + for i := range patches { + if patches[i].Path != "" { + newPath, err := lc.localizeFile(patches[i].Path) + if err != nil { + return err + } + patches[i].Path = newPath + } + } + return nil +} + // localizeFile localizes file path and returns the localized path func (lc *localizer) localizeFile(path string) (string, error) { content, err := lc.ldr.Load(path) @@ -303,9 +338,9 @@ func (lc *localizer) localizeBuiltinPlugins(kust *types.Kustomization) error { // loadResource tries to load resourceEntry as a file path or inline. // On success, loadResource returns the loaded resource map and whether resourceEntry is a file path. func (lc *localizer) loadResource(resourceEntry string) (resmap.ResMap, bool, error) { - var fileErr error rm, inlineErr := lc.rFactory.NewResMapFromBytes([]byte(resourceEntry)) if inlineErr != nil { + var fileErr error rm, fileErr = lc.rFactory.FromFile(lc.ldr, resourceEntry) if fileErr != nil { err := ResourceLoadError{ @@ -315,5 +350,5 @@ func (lc *localizer) loadResource(resourceEntry string) (resmap.ResMap, bool, er return nil, false, errors.WrapPrefixf(err, "unable to load resource entry %q", resourceEntry) } } - return rm, fileErr == nil, nil + return rm, inlineErr != nil, nil } diff --git a/api/internal/localizer/localizer_test.go b/api/internal/localizer/localizer_test.go index a4b9f3085..095287539 100644 --- a/api/internal/localizer/localizer_test.go +++ b/api/internal/localizer/localizer_test.go @@ -24,8 +24,7 @@ spec: - name: nginx image: nginx:1.14.2 ports: - - containerPort: 80 -` + - containerPort: 80` func makeMemoryFs(t *testing.T) filesys.FileSystem { t.Helper() @@ -290,6 +289,117 @@ spec: checkFSys(t, fSysExpected, fSys) } +func TestLocalizePatchesJson(t *testing.T) { + fSys := makeMemoryFs(t) + kustAndPatches := map[string]string{ + "kustomization.yaml": `apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +patchesJson6902: +- path: patch.yaml + target: + annotationSelector: zone=west + name: pod + version: v1 +- patch: '[{"op": "add", "path": "/new/path", "value": "value"}]' + target: + group: apps + kind: Pod +- path: patch.json + target: + namespace: my +`, + "patch.yaml": `- op: add + path: /some/new/path + value: value +- op: replace + path: /some/existing/path + value: new value`, + "patch.json": ` [ + {"op": "copy", "from": "/here", "path": "/there"}, + {"op": "remove", "path": "/some/existing/path"}, + ]`, + } + addFiles(t, fSys, "/alpha/beta", kustAndPatches) + + err := Run("/alpha/beta", "/", "/beta", fSys) + require.NoError(t, err) + + fSysExpected := makeMemoryFs(t) + addFiles(t, fSysExpected, "/alpha/beta", kustAndPatches) + addFiles(t, fSysExpected, "/beta/alpha/beta", kustAndPatches) + checkFSys(t, fSysExpected, fSys) +} + +func TestLocalizePatchesSM(t *testing.T) { + fSys := makeMemoryFs(t) + kustAndPatches := map[string]string{ + "kustomization.yaml": `apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +patchesStrategicMerge: +- |- + apiVersion: v1 + kind: ConfigMap + metadata: + name: map + data: + - APPLE: orange +- patch.yaml +`, + "patch.yaml": podConfiguration, + } + addFiles(t, fSys, "/a", kustAndPatches) + + err := Run("/a", "", "/dst", fSys) + require.NoError(t, err) + + fSysExpected := makeMemoryFs(t) + addFiles(t, fSysExpected, "/a", kustAndPatches) + addFiles(t, fSysExpected, "/dst", kustAndPatches) + checkFSys(t, fSysExpected, fSys) +} + +func TestLocalizeReplacements(t *testing.T) { + fSys := makeMemoryFs(t) + kustAndReplacement := map[string]string{ + "kustomization.yaml": `apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +replacements: +- path: replacement.yaml +- source: + fieldPath: path + name: map + targets: + - fieldPaths: + - path + select: + name: my-map +`, + "replacement.yaml": `source: + fieldPath: path.*.to.[some=field] + kind: Pod + options: + delimiter: / +targets: +- fieldPaths: + - config\.kubernetes\.io.annotations + - second.path + reject: + - group: apps + version: v2 + select: + namespace: my`, + } + addFiles(t, fSys, "/a", kustAndReplacement) + + err := Run("/a", "/", "/dst", fSys) + require.NoError(t, err) + + fSysExpected := makeMemoryFs(t) + addFiles(t, fSysExpected, "/a", kustAndReplacement) + addFiles(t, fSysExpected, "/dst/a", kustAndReplacement) + checkFSys(t, fSysExpected, fSys) +} + func TestLocalizeConfigMapGenerator(t *testing.T) { fSys := makeMemoryFs(t) kustAndData := map[string]string{ @@ -480,20 +590,20 @@ func TestLocalizeValidators(t *testing.T) { "kustomization.yaml": `apiVersion: kustomize.config.k8s.io/v1beta1 kind: Kustomization validators: -- |- +- | apiVersion: builtin kind: ReplacementTransformer metadata: name: replacement replacements: - source: - kind: ConfigMap - fieldPath: metadata.name + fieldPath: data.[field=value] + group: apps targets: - - select: - kind: ConfigMap - fieldPaths: - - metadata.name + - fieldPaths: + - spec.* + select: + kind: Pod - replacement.yaml `, "replacement.yaml": `apiVersion: builtin @@ -502,13 +612,14 @@ metadata: name: replacement-2 replacements: - source: - kind: Secret - fieldPath: data.USER_NAME + fieldPath: spec.containers.1.image + kind: Custom + namespace: test targets: - - select: - kind: Secret - fieldPaths: - - data.USER_NAME + - fieldPaths: + - path + select: + namespace: test `, } addFiles(t, fSys, "/", kustAndPlugin)