From 142360b0edfcc1bd0a5e1bc76f58db9cb12bddc8 Mon Sep 17 00:00:00 2001 From: Anna Song Date: Tue, 24 Jan 2023 17:10:20 -0800 Subject: [PATCH] Localize ConfigMapGenerator, SecretGenerator --- api/internal/localizer/builtinplugins.go | 81 +++++++++--- api/internal/localizer/localizer.go | 45 ++++--- api/internal/localizer/localizer_test.go | 152 ++++++++++++++++------- api/krusty/localizer/runner_test.go | 30 +++++ 4 files changed, 230 insertions(+), 78 deletions(-) diff --git a/api/internal/localizer/builtinplugins.go b/api/internal/localizer/builtinplugins.go index d933a6a71..8ee4ce39a 100644 --- a/api/internal/localizer/builtinplugins.go +++ b/api/internal/localizer/builtinplugins.go @@ -32,6 +32,22 @@ func (lbp *localizeBuiltinPlugins) Filter(plugins []*yaml.RNode) ([]*yaml.RNode, for _, singlePlugin := range plugins { err := singlePlugin.PipeE(fsslice.Filter{ FsSlice: types.FsSlice{ + types.FieldSpec{ + Gvk: resid.Gvk{Version: konfig.BuiltinPluginApiVersion, Kind: builtinhelpers.ConfigMapGenerator.String()}, + Path: "env", + }, + types.FieldSpec{ + Gvk: resid.Gvk{Version: konfig.BuiltinPluginApiVersion, Kind: builtinhelpers.ConfigMapGenerator.String()}, + Path: "envs", + }, + types.FieldSpec{ + Gvk: resid.Gvk{Version: konfig.BuiltinPluginApiVersion, Kind: builtinhelpers.SecretGenerator.String()}, + Path: "env", + }, + types.FieldSpec{ + Gvk: resid.Gvk{Version: konfig.BuiltinPluginApiVersion, Kind: builtinhelpers.SecretGenerator.String()}, + Path: "envs", + }, types.FieldSpec{ Gvk: resid.Gvk{Version: konfig.BuiltinPluginApiVersion, Kind: builtinhelpers.PatchTransformer.String()}, Path: "path", @@ -47,20 +63,36 @@ func (lbp *localizeBuiltinPlugins) Filter(plugins []*yaml.RNode) ([]*yaml.RNode, }, SetValue: func(node *yaml.RNode) error { lbp.locPathFn = lbp.lc.localizeFile - return lbp.localizeNode(node) + return lbp.localizeAll(node) }, - }, fieldspec.Filter{ - FieldSpec: types.FieldSpec{ - Gvk: resid.Gvk{Version: konfig.BuiltinPluginApiVersion, Kind: builtinhelpers.PatchStrategicMergeTransformer.String()}, - Path: "paths", + }, + fsslice.Filter{ + FsSlice: types.FsSlice{ + types.FieldSpec{ + Gvk: resid.Gvk{Version: konfig.BuiltinPluginApiVersion, Kind: builtinhelpers.ConfigMapGenerator.String()}, + Path: "files", + }, + types.FieldSpec{ + Gvk: resid.Gvk{Version: konfig.BuiltinPluginApiVersion, Kind: builtinhelpers.SecretGenerator.String()}, + Path: "files", + }, + }, + SetValue: func(node *yaml.RNode) error { + lbp.locPathFn = lbp.lc.localizeFileSource + return lbp.localizeAll(node) + }, }, - SetValue: func(node *yaml.RNode) error { - lbp.locPathFn = lbp.lc.localizeK8sResource - return errors.Wrap(node.VisitElements(lbp.localizeNode)) - }, - }) - // TODO(annasong): localize ConfigMapGenerator, SecretGenerator, - // HelmChartInflationGenerator + fieldspec.Filter{ + FieldSpec: types.FieldSpec{ + Gvk: resid.Gvk{Version: konfig.BuiltinPluginApiVersion, Kind: builtinhelpers.PatchStrategicMergeTransformer.String()}, + Path: "paths", + }, + SetValue: func(node *yaml.RNode) error { + lbp.locPathFn = lbp.lc.localizeK8sResource + return lbp.localizeAll(node) + }, + }) + // TODO(annasong): localize HelmChartInflationGenerator if err != nil { return nil, errors.Wrap(err) } @@ -68,14 +100,27 @@ func (lbp *localizeBuiltinPlugins) Filter(plugins []*yaml.RNode) ([]*yaml.RNode, return plugins, nil } -// localizeNode sets the scalar node to its value localized by locPathFn. -func (lbp *localizeBuiltinPlugins) localizeNode(node *yaml.RNode) error { +// localizeAll sets each entry in node to its value localized by locPathFn. +// Node is a sequence or scalar value. +func (lbp *localizeBuiltinPlugins) localizeAll(node *yaml.RNode) error { + // We rely on the build command to throw errors for nodes in + // built-in plugins that are sequences when expected to be scalar, + // and vice versa. + switch node.YNode().Kind { + case yaml.SequenceNode: + return errors.Wrap(node.VisitElements(lbp.localizeScalar)) + case yaml.ScalarNode: + return lbp.localizeScalar(node) + default: + return errors.Errorf("expected sequence or scalar node") + } +} + +// localizeScalar sets the scalar node to its value localized by locPathFn. +func (lbp *localizeBuiltinPlugins) localizeScalar(node *yaml.RNode) error { localizedPath, err := lbp.locPathFn(node.YNode().Value) if err != nil { return err } - if localizedPath != "" { - err = filtersutil.SetScalar(localizedPath)(node) - } - return err + return filtersutil.SetScalar(localizedPath)(node) } diff --git a/api/internal/localizer/localizer.go b/api/internal/localizer/localizer.go index f3b72dfa5..206c31a13 100644 --- a/api/internal/localizer/localizer.go +++ b/api/internal/localizer/localizer.go @@ -193,9 +193,7 @@ func (lc *localizer) localizeNativeFields(kust *types.Kustomization) error { if err != nil { return errors.WrapPrefixf(err, "unable to localize patchesStrategicMerge entry") } - if locPath != "" { - kust.PatchesStrategicMerge[i] = types.PatchStrategicMerge(locPath) - } + kust.PatchesStrategicMerge[i] = types.PatchStrategicMerge(locPath) } for i, replacement := range kust.Replacements { locPath, err := lc.localizeFile(replacement.Path) @@ -222,18 +220,10 @@ func (lc *localizer) localizeGenerator(generator *types.GeneratorArgs) error { } locFiles := make([]string, len(generator.FileSources)) for i, file := range generator.FileSources { - k, f, err := generators.ParseFileSource(file) + locFiles[i], err = lc.localizeFileSource(file) if err != nil { - return errors.WrapPrefixf(err, "unable to parse generator files entry %q", file) + return err } - newFile, err := lc.localizeFile(f) - if err != nil { - return errors.WrapPrefixf(err, "unable to localize generator files path in entry %q", file) - } - if f != file { - newFile = k + "=" + newFile - } - locFiles[i] = newFile } generator.EnvSource = locEnvSrc generator.EnvSources = locEnvs @@ -241,6 +231,26 @@ func (lc *localizer) localizeGenerator(generator *types.GeneratorArgs) error { return nil } +// localizeFileSource returns the localized file source found in configMap and +// secretGenerators. +func (lc *localizer) localizeFileSource(source string) (string, error) { + key, file, err := generators.ParseFileSource(source) + if err != nil { + return "", errors.Wrap(err) + } + locFile, err := lc.localizeFile(file) + if err != nil { + return "", errors.WrapPrefixf(err, "invalid file source %q", source) + } + var locSource string + if source == file { + locSource = locFile + } else { + locSource = key + "=" + locFile + } + return locSource, nil +} + // localizeHelmInflationGenerator localizes helmChartInflationGenerator on kust. // localizeHelmInflationGenerator localizes values files and copies local chart homes. func (lc *localizer) localizeHelmInflationGenerator(kust *types.Kustomization) error { @@ -559,10 +569,9 @@ func (lc *localizer) localizeBuiltinPlugins(kust *types.Kustomization) error { return nil } -// localizeK8sResource returns the localized file path if resourceEntry is a -// file containing a kubernetes resource. -// localizeK8sResource returns the empty string if resourceEntry is an inline -// resource. +// localizeK8sResource returns the localized resourceEntry if it is a file +// containing a kubernetes resource. +// localizeK8sResource returns resourceEntry if it is an inline resource. func (lc *localizer) localizeK8sResource(resourceEntry string) (string, error) { _, isFile, err := lc.loadK8sResource(resourceEntry) if err != nil { @@ -571,7 +580,7 @@ func (lc *localizer) localizeK8sResource(resourceEntry string) (string, error) { if isFile { return lc.localizeFile(resourceEntry) } - return "", nil + return resourceEntry, nil } // loadK8sResource tries to load resourceEntry as a file path or inline diff --git a/api/internal/localizer/localizer_test.go b/api/internal/localizer/localizer_test.go index 817ce9005..f1ed06c83 100644 --- a/api/internal/localizer/localizer_test.go +++ b/api/internal/localizer/localizer_test.go @@ -670,37 +670,61 @@ transformers: checkFSys(t, expected, actual) } -func TestLocalizeGenerators(t *testing.T) { - kustAndPlugins := map[string]string{ - "kustomization.yaml": `apiVersion: kustomize.config.k8s.io/v1beta1 -generators: -- plugin.yaml -- | - apiVersion: builtin - behavior: create - kind: ConfigMapGenerator - literals: - - APPLE=orange - metadata: - name: another-map - --- - apiVersion: builtin - kind: SecretGenerator - literals: - - APPLE=b3Jhbmdl - metadata: - name: secret - options: - disableNameSuffixHash: true -kind: Kustomization +func TestLocalizeGeneratorsConfigMap(t *testing.T) { + files := map[string]string{ + "kustomization.yaml": `generators: +- configMapGenerator `, - "plugin.yaml": `apiVersion: builtin + "configMapGenerator": `apiVersion: builtin +behavior: create +env: one.env +envs: +- two.env +- three.env +files: +- four.properties +- key=five.properties kind: ConfigMapGenerator metadata: - name: map + name: custom-generator +options: + disableNameSuffix: true `, + "one.env": "key1=value1", + "two.env": "key2=value2", + "three.env": "key3=value3", + "four.properties": "key4=value4", + "five.properties": "key5=value5", } - checkLocalizeInTargetSuccess(t, kustAndPlugins) + checkLocalizeInTargetSuccess(t, files) +} + +func TestLocalizeGeneratorsSecret(t *testing.T) { + files := map[string]string{ + "kustomization.yaml": `generators: +- secretGenerator +`, + "secretGenerator": `apiVersion: builtin +env: one.env +envs: +- two.env +- three.env +files: +- four.properties +- key=five.properties +kind: SecretGenerator +literals: +- key6=value6 +metadata: + name: custom-generator +`, + "one.env": "key1=value1", + "two.env": "key2=value2", + "three.env": "key3=value3", + "four.properties": "key4=value4", + "five.properties": "key5=value5", + } + checkLocalizeInTargetSuccess(t, files) } func TestLocalizeTransformersPatch(t *testing.T) { @@ -833,7 +857,23 @@ validators: checkLocalizeInTargetSuccess(t, kustAndPlugin) } -func TestLocalizeBuiltinPluginsNotResource(t *testing.T) { +func TestLocalizeBuiltinPlugins_SequenceScalarEquivalence(t *testing.T) { + kustomization := map[string]string{ + "kustomization.yaml": `transformers: +- | + apiVersion: builtin + kind: PatchTransformer + metadata: + name: path-should-be-scalar-but-accept-sequence + path: + - patchSM.yaml +`, + "patchSM.yaml": podConfiguration, + } + checkLocalizeInTargetSuccess(t, kustomization) +} + +func TestLocalizeBuiltinPlugins_NotResource(t *testing.T) { type testCase struct { name string files map[string]string @@ -895,24 +935,52 @@ when parsing as filepath received error: %s`, test.errPrefix, test.inlineErrMsg, } } -func TestLocalizeBuiltinPluginsFileError(t *testing.T) { - kustAndPatches := map[string]string{ - "kustomization.yaml": `transformers: -- patch.yaml +func TestLocalizeBuiltinPlugins_Errors(t *testing.T) { + for name, test := range map[string]struct { + files map[string]string + fieldSpecErr string + locErr string + }{ + "file_dne": { + files: map[string]string{ + "kustomization.yaml": `transformers: +- | + apiVersion: builtin + kind: PatchTransformer + metadata: + name: file-does-not-exist + path: patchSM.yaml `, - "patch.yaml": `apiVersion: builtin -kind: PatchTransformer -metadata: - name: my-patch -path: patchSM.yaml + }, + fieldSpecErr: "considering field 'path' of object PatchTransformer.builtin.[noGrp]/file-does-not-exist.[noNs]", + locErr: "invalid file reference: '/a/patchSM.yaml' doesn't exist", + }, + "not_sequence_or_scalar": { + files: map[string]string{ + "kustomization.yaml": `transformers: +- | + apiVersion: builtin + kind: PatchTransformer + metadata: + name: path-node-has-wrong-kind + path: + mappingNode: patchSM.yaml `, + "patchSM.yaml": podConfiguration, + }, + fieldSpecErr: "considering field 'path' of object PatchTransformer.builtin.[noGrp]/path-node-has-wrong-kind.[noNs]", + locErr: "expected sequence or scalar node", + }, + } { + t.Run(name, func(t *testing.T) { + expected, actual := makeFileSystems(t, "/a", test.files) + err := Run("/a", "", "/dst", actual) + const errPrefix = `unable to localize target "/a"` + require.EqualError(t, err, fmt.Sprintf( + "%s: %s: %s", errPrefix, test.fieldSpecErr, test.locErr)) + checkFSys(t, expected, actual) + }) } - _, actual := makeFileSystems(t, "/a", kustAndPatches) - - err := Run("/a", "", "/dst", actual) - require.EqualError(t, err, "unable to localize target \"/a\": "+ - "considering field 'path' of object PatchTransformer.builtin.[noGrp]/my-patch.[noNs]: "+ - "invalid file reference: '/a/patchSM.yaml' doesn't exist") } func TestLocalizeDirInTarget(t *testing.T) { diff --git a/api/krusty/localizer/runner_test.go b/api/krusty/localizer/runner_test.go index 950735c78..b4e4108c8 100644 --- a/api/krusty/localizer/runner_test.go +++ b/api/krusty/localizer/runner_test.go @@ -646,3 +646,33 @@ func TestHelmHomeEscapesScope(t *testing.T) { require.NoError(t, fsExpected.Mkdir(filepath.Join(dst, "home"))) CheckFs(t, dst, fsExpected, fsActual) } + +func TestSymlinkedFileSource(t *testing.T) { + // target (and scope) + // - kustomization + // - file + // - link to file + fsExpected, fsActual, target := PrepareFs(t, nil, map[string]string{ + "kustomization.yaml": `configMapGenerator: +- files: + - filename-used-as-key-in-configMap +`, + "different-key": "properties", + }) + link(t, target, map[string]string{ + "filename-used-as-key-in-configMap": "different-key", + }) + + dst := target.Join("dst") + err := localizer.Run(fsActual, target.String(), "", dst) + require.NoError(t, err) + + SetupDir(t, fsExpected, dst, map[string]string{ + "kustomization.yaml": `configMapGenerator: +- files: + - different-key +`, + "different-key": "properties", + }) + CheckFs(t, dst, fsExpected, fsActual) +}