From 30893b0184e959d0a731a35f8ce36a0f6f2c2555 Mon Sep 17 00:00:00 2001 From: Mauren Berti <698465+stormqueen1990@users.noreply.github.com> Date: Fri, 17 Nov 2023 13:14:53 -0500 Subject: [PATCH] feat: edit set configmap (#5391) * feat: add new command 'edit set configmap' * Add a new command 'edit set configmap' to allow editing the values of an already-existing configmap in a kustomization file. * Add tests to validate the new feature. * fix: add tests, minor refactoring to use constants * Include tests to validade the new function ValidateSet, included to do necessary validations when running the 'kustomize edit set configmap' command. * Minor refactorings to use the existing constants in the 'edit set configmap' command. * Add dashes before each item in the comment explaining how ExpandFileSource() works so IDEs don't try to reformat the list and remove the indentation in it. * Because this change mutates the list of literal sources, ensure that both add and set save the resulting list in a predictable order to make it easier to check when new items are added/removed and aid in testing. * Since literal sources are the only bit that's important in this test, verify that the literal sources in the actual result is equal to what we expected it to be. * fix: change format to print resource name Use '%q' formatter instead of '%s' to print resource name Co-authored-by: Varsha * fix: add changes from code review * Unexport constant that is used only in the scope of a single function. * Add extra validation to ensure format is correct with one single '=' per key-value pair. * Add extra set of tests to validate format. * Update test case to match new printed format in the error message. * fix: rollback sort for edit add/set configmap * chore: rename test package and unexport functions Rename the test package from set_test back to set and unexport functions that do not need to be exported anymore for testing purposes. * feat: handle empty and default namespace as equal Handle the empty and the default namespaces as equal. Add tests to validate this scenario. --------- Co-authored-by: Varsha --- go.work.sum | 33 ++- kustomize/commands/edit/add/configmap.go | 2 +- kustomize/commands/edit/add/secret.go | 2 +- kustomize/commands/edit/all.go | 3 +- kustomize/commands/edit/set/all.go | 9 +- kustomize/commands/edit/set/configmap.go | 151 +++++++++++ kustomize/commands/edit/set/configmap_test.go | 246 +++++++++++++++++ .../util/configmapSecretFlagsAndArgs.go | 111 ++++++-- .../util/configmapSecretFlagsAndArgs_test.go | 248 ++++++++++++++++-- kustomize/go.mod | 1 + kustomize/go.sum | 2 + 11 files changed, 764 insertions(+), 44 deletions(-) create mode 100644 kustomize/commands/edit/set/configmap.go create mode 100644 kustomize/commands/edit/set/configmap_test.go diff --git a/go.work.sum b/go.work.sum index 1af079419..1cd12ee35 100644 --- a/go.work.sum +++ b/go.work.sum @@ -201,12 +201,11 @@ go.uber.org/multierr v1.1.0 h1:HoEmRHQPVSqub6w2z2d2EOVs2fjyFRGyofhKuyDq0QI= go.uber.org/zap v1.10.0 h1:ORx85nbTijNz8ljznvCMR1ZBIPKFn3jQrag10X2AsuM= golang.org/x/crypto v0.0.0-20201002170205-7f63de1d35b0 h1:hb9wdF1z5waM+dSIICn1l0DkLVDT3hqhhQsDNUmHPRE= golang.org/x/crypto v0.13.0/go.mod h1:y6Z2r+Rw4iayiXXAIxJIDAJ1zMW4yaTpebo8fPOliYc= +golang.org/x/crypto v0.14.0 h1:wBqGXzWJW6m1XrIKlAH0Hs1JJ7+9KBwnIO8v66Q9cHc= golang.org/x/crypto v0.14.0/go.mod h1:MVFd36DqK4CsrnJYDkBA3VC4m2GkXAM0PvzMCn4JQf4= -golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6 h1:QE6XYQK6naiK1EPAe1g/ILLxN5RBoH5xkJk3CqlMI/Y= golang.org/x/image v0.0.0-20190802002840-cff245a6509b h1:+qEpEAPhDZ1o0x3tHzZTQDArnOixOzGD9HUJfcg0mb4= golang.org/x/lint v0.0.0-20200302205851-738671d3881b h1:Wh+f8QHJXR411sJR8/vRBTZ7YapZaRvUcLFFJhusH0k= golang.org/x/mobile v0.0.0-20190719004257-d2bd2a29d028 h1:4+4C/Iv2U4fMZBiMCc98MG1In4gJY5YRhtpDNeDeHWs= -golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= golang.org/x/net v0.4.0/go.mod h1:MBQ8lrhLObU/6UmLb4fmbmk5OcyYmqtbGd/9yIeKjEE= golang.org/x/net v0.8.0/go.mod h1:QVkue5JL9kW//ek3r6jTKnTFis1tRmNAW2P1shuFdJc= @@ -224,5 +223,35 @@ golang.org/x/text v0.13.0/go.mod h1:TvPlkZtksWOMsz7fbANvkp4WM8x/WCo/om8BMLbz+aE= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= golang.org/x/tools v0.7.0/go.mod h1:4pg6aUX35JBAogB10C9AtvVL+qowtN4pT3CGSQex14s= +golang.org/x/mod v0.13.0 h1:I/DsJXRlw/8l/0c24sM9yb0T4z9liZTduXvdAWYiysY= +golang.org/x/mod v0.13.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= +golang.org/x/net v0.16.0/go.mod h1:NxSsAGuq816PNPmqtQdLE42eU2Fs7NoRIZrHJAlaCOE= +golang.org/x/sync v0.4.0 h1:zxkM55ReGkDlKSM+Fu41A+zmbZuaPVbGMzvvdUPznYQ= +golang.org/x/sync v0.4.0/go.mod h1:FU7BRWz2tNW+3quACPkgCx/L+uEAv1htQ0V83Z9Rj+Y= +golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= +golang.org/x/tools v0.13.0/go.mod h1:HvlwmtVNQAhOuCjW7xxvovg8wbNq7LwfXh/k7wXUl58= +golang.org/x/tools v0.14.0 h1:jvNa2pY0M4r62jkRQ6RwEZZyPcymeL9XZMLBbV7U2nc= +golang.org/x/tools v0.14.0/go.mod h1:uYBEerGOWcJyEORxN+Ek8+TT266gXkNlHdJBwexUsBg= +golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2/go.mod h1:K8+ghG5WaK9qNqU5K3HdILfMLy1f3aNYFI/wnl100a8= +gomodules.xyz/jsonpatch/v2 v2.0.1 h1:xyiBuvkD2g5n7cYzx6u2sxQvsAy4QJsZFCzGVdzOXZ0= +gonum.org/v1/gonum v0.0.0-20190331200053-3d26580ed485 h1:OB/uP/Puiu5vS5QMRPrXCDWUPb+kt8f1KW8oQzFejQw= +gonum.org/v1/netlib v0.0.0-20190331212654-76723241ea4e h1:jRyg0XfpwWlhEV8mDfdNGBeSJM2fuyh9Yjrnd8kF2Ts= +google.golang.org/api v0.30.0 h1:yfrXXP61wVuLb0vBcG6qaOoIoqYEzOQS8jum51jkv2w= +google.golang.org/genproto v0.0.0-20220107163113-42d7afdf6368 h1:Et6SkiuvnBn+SgrSYXs/BrUpGB4mbdwt4R3vaPIlicA= +google.golang.org/grpc v1.40.0 h1:AGJ0Ih4mHjSeibYkFGh1dD9KJ/eOtZ93I6hoHhukQ5Q= +gopkg.in/alecthomas/kingpin.v2 v2.2.6 h1:jMFz6MfLP0/4fUyZle81rXUoxOBFi19VUFKVDOQfozc= +gopkg.in/cheggaaa/pb.v1 v1.0.25 h1:Ev7yu1/f6+d+b3pi5vPdRPc6nNtP1umSfcWiEfRqv6I= +gopkg.in/errgo.v2 v2.1.0 h1:0vLT13EuvQ0hNvakwLuFZ/jYrLp5F3kcWHXdRggjCE8= +gopkg.in/ini.v1 v1.51.0 h1:AQvPpx3LzTDM0AjnIRlVFwFFGC+npRopjZxLJj6gdno= +gopkg.in/natefinch/lumberjack.v2 v2.0.0 h1:1Lc07Kr7qY4U2YPouBjpCLxpiyxIVoxqXgkXLknAOE8= +gopkg.in/resty.v1 v1.12.0 h1:CuXP0Pjfw9rOuY6EP+UvtNvt5DSqHpIxILZKT/quCZI= +gopkg.in/square/go-jose.v2 v2.2.2 h1:orlkJ3myw8CN1nVQHBFfloD+L3egixIa4FvUP6RosSA= +gotest.tools v2.2.0+incompatible h1:VsBPFP1AI068pPrMxtb/S8Zkgf9xEmTLJjfM+P5UIEo= +honnef.co/go/tools v0.0.1-2020.1.4 h1:UoveltGrhghAA7ePc+e+QYDHXrBps2PqFZiHkGR/xK8= +k8s.io/apiserver v0.17.0 h1:XhUix+FKFDcBygWkQNp7wKKvZL030QUlH1o8vFeSgZA= +k8s.io/code-generator v0.17.0 h1:y+KWtDWNqlJzJu/kUy8goJZO0X71PGIpAHLX8a0JYk0= +k8s.io/component-base v0.17.0 h1:BnDFcmBDq+RPpxXjmuYnZXb59XNN9CaFrX8ba9+3xrA= +k8s.io/gengo v0.0.0-20210813121822-485abfe95c7c h1:GohjlNKauSai7gN4wsJkeZ3WAJx4Sh+oT/b5IYn5suA= k8s.io/klog v1.0.0 h1:Pt+yjF5aB1xDSVbau4VsWe+dQNzA0qv1LlXdC2dF6Q8= sigs.k8s.io/structured-merge-diff v1.0.1-0.20191108220359-b1b620dd3f06 h1:zD2IemQ4LmOcAumeiyDWXKUI2SO0NYDe3H6QGvPOVgU= diff --git a/kustomize/commands/edit/add/configmap.go b/kustomize/commands/edit/add/configmap.go index 0b44aa786..1566753bc 100644 --- a/kustomize/commands/edit/add/configmap.go +++ b/kustomize/commands/edit/add/configmap.go @@ -97,7 +97,7 @@ func runEditAddConfigMap( return fmt.Errorf("failed to expand file source: %w", err) } - err = flags.Validate(args) + err = flags.ValidateAdd(args) if err != nil { return fmt.Errorf("failed to validate flags: %w", err) } diff --git a/kustomize/commands/edit/add/secret.go b/kustomize/commands/edit/add/secret.go index f55005d0b..982a9c5c3 100644 --- a/kustomize/commands/edit/add/secret.go +++ b/kustomize/commands/edit/add/secret.go @@ -89,7 +89,7 @@ func runEditAddSecret( return fmt.Errorf("failed to expand file source: %w", err) } - err = flags.Validate(args) + err = flags.ValidateAdd(args) if err != nil { return fmt.Errorf("failed to validate flags: %w", err) } diff --git a/kustomize/commands/edit/all.go b/kustomize/commands/edit/all.go index ccd7f8e32..ee30dbfb1 100644 --- a/kustomize/commands/edit/all.go +++ b/kustomize/commands/edit/all.go @@ -48,7 +48,8 @@ func NewCmdEdit( set.NewCmdSet( fSys, kv.NewLoader(ldrhelper.NewFileLoaderAtCwd(fSys), v), - v), + v, + rf), fix.NewCmdFix(fSys, w), remove.NewCmdRemove(fSys, v), listbuiltin.NewCmdListBuiltinPlugin(), diff --git a/kustomize/commands/edit/set/all.go b/kustomize/commands/edit/set/all.go index 6979665af..0c716c91b 100644 --- a/kustomize/commands/edit/set/all.go +++ b/kustomize/commands/edit/set/all.go @@ -6,11 +6,17 @@ package set import ( "github.com/spf13/cobra" "sigs.k8s.io/kustomize/api/ifc" + "sigs.k8s.io/kustomize/api/resource" "sigs.k8s.io/kustomize/kyaml/filesys" ) // NewCmdSet returns an instance of 'set' subcommand. -func NewCmdSet(fSys filesys.FileSystem, ldr ifc.KvLoader, v ifc.Validator) *cobra.Command { +func NewCmdSet( + fSys filesys.FileSystem, + ldr ifc.KvLoader, + v ifc.Validator, + rf *resource.Factory, +) *cobra.Command { c := &cobra.Command{ Use: "set", Short: "Sets the value of different fields in kustomization file", @@ -26,6 +32,7 @@ func NewCmdSet(fSys filesys.FileSystem, ldr ifc.KvLoader, v ifc.Validator) *cobr } c.AddCommand( + newCmdSetConfigMap(fSys, ldr, rf), newCmdSetNamePrefix(fSys), newCmdSetNameSuffix(fSys), newCmdSetNamespace(fSys, v), diff --git a/kustomize/commands/edit/set/configmap.go b/kustomize/commands/edit/set/configmap.go new file mode 100644 index 000000000..71aae88da --- /dev/null +++ b/kustomize/commands/edit/set/configmap.go @@ -0,0 +1,151 @@ +// Copyright 2023 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package set + +import ( + "fmt" + + "github.com/spf13/cobra" + "golang.org/x/exp/slices" + "sigs.k8s.io/kustomize/api/ifc" + "sigs.k8s.io/kustomize/api/resource" + "sigs.k8s.io/kustomize/api/types" + "sigs.k8s.io/kustomize/kustomize/v5/commands/internal/kustfile" + "sigs.k8s.io/kustomize/kustomize/v5/commands/internal/util" + "sigs.k8s.io/kustomize/kyaml/filesys" +) + +func newCmdSetConfigMap( + fSys filesys.FileSystem, + ldr ifc.KvLoader, + rf *resource.Factory, +) *cobra.Command { + var flags util.ConfigMapSecretFlagsAndArgs + cmd := &cobra.Command{ + Use: "configmap NAME [--from-literal=key1=value1] [--namespace=namespace-name] [--new-namespace=new-namespace-name]", + Short: "Edits the value for an existing key for a configmap in the kustomization file", + Long: `Edits the value for an existing key in an existing configmap in the kustomization file. +Both configmap name and key name must exist for this command to succeed.`, + Example: ` + # Edits an existing configmap in the kustomization file, changing value of key1 to 2 + kustomize edit set configmap my-configmap --from-literal=key1=2 + + # Edits an existing configmap in the kustomization file, changing namespace to 'new-namespace' + kustomize edit set configmap my-configmap --namespace=current-namespace --new-namespace=new-namespace +`, + RunE: func(_ *cobra.Command, args []string) error { + return runEditSetConfigMap(flags, fSys, args, ldr, rf) + }, + } + + cmd.Flags().StringArrayVar( + &flags.LiteralSources, + util.FromLiteralFlag, + []string{}, + "Specify an existing key and a new value to update a ConfigMap (i.e. mykey=newvalue)") + cmd.Flags().StringVar( + &flags.Namespace, + util.NamespaceFlag, + "", + "Current namespace of the target ConfigMap") + cmd.Flags().StringVar( + &flags.NewNamespace, + util.NewNamespaceFlag, + "", + "New namespace value for the target ConfigMap") + + return cmd +} + +func runEditSetConfigMap( + flags util.ConfigMapSecretFlagsAndArgs, + fSys filesys.FileSystem, + args []string, + ldr ifc.KvLoader, + rf *resource.Factory, +) error { + err := flags.ExpandFileSource(fSys) + if err != nil { + return fmt.Errorf("failed to expand file source: %w", err) + } + + err = flags.ValidateSet(args) + if err != nil { + return fmt.Errorf("failed to validate flags: %w", err) + } + + // Load the kustomization file. + mf, err := kustfile.NewKustomizationFile(fSys) + if err != nil { + return fmt.Errorf("failed to load kustomization file: %w", err) + } + + kustomization, err := mf.Read() + if err != nil { + return fmt.Errorf("failed to read kustomization file: %w", err) + } + + // Updates the existing ConfigMap + err = setConfigMap(ldr, kustomization, flags, rf) + if err != nil { + return fmt.Errorf("failed to create configmap: %w", err) + } + + // Write out the kustomization file with added configmap. + err = mf.Write(kustomization) + if err != nil { + return fmt.Errorf("failed to write kustomization file: %w", err) + } + + return nil +} + +func setConfigMap( + ldr ifc.KvLoader, + k *types.Kustomization, + flags util.ConfigMapSecretFlagsAndArgs, + rf *resource.Factory, +) error { + args, err := findConfigMapArgs(k, flags.Name, flags.Namespace) + if err != nil { + return fmt.Errorf("could not set new ConfigMap value: %w", err) + } + + if len(flags.LiteralSources) > 0 { + err := util.UpdateLiteralSources(&args.GeneratorArgs, flags) + if err != nil { + return fmt.Errorf("failed to update literal sources: %w", err) + } + } + + // update namespace to new one + if flags.NewNamespace != "" { + args.Namespace = flags.NewNamespace + } + + // Validate by trying to create corev1.configmap. + args.Options = types.MergeGlobalOptionsIntoLocal( + args.Options, k.GeneratorOptions) + + _, err = rf.MakeConfigMap(ldr, args) + if err != nil { + return fmt.Errorf("failed to validate ConfigMap structure: %w", err) + } + + return nil +} + +// findConfigMapArgs finds the generator arguments corresponding to the specified +// ConfigMap name. ConfigMap must exist for this command to be successful. +func findConfigMapArgs(m *types.Kustomization, name, namespace string) (*types.ConfigMapArgs, error) { + cmIndex := slices.IndexFunc(m.ConfigMapGenerator, func(cmArgs types.ConfigMapArgs) bool { + return name == cmArgs.Name && util.NamespaceEqual(namespace, cmArgs.Namespace) + }) + + if cmIndex == -1 { + return nil, fmt.Errorf("unable to find ConfigMap with name '%q'", name) + } + + return &m.ConfigMapGenerator[cmIndex], nil +} diff --git a/kustomize/commands/edit/set/configmap_test.go b/kustomize/commands/edit/set/configmap_test.go new file mode 100644 index 000000000..e7f1939a0 --- /dev/null +++ b/kustomize/commands/edit/set/configmap_test.go @@ -0,0 +1,246 @@ +// Copyright 2023 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package set + +import ( + "testing" + + "github.com/stretchr/testify/require" + "sigs.k8s.io/kustomize/api/kv" + "sigs.k8s.io/kustomize/api/pkg/loader" + "sigs.k8s.io/kustomize/api/provider" + "sigs.k8s.io/kustomize/kustomize/v5/commands/internal/kustfile" + testutils_test "sigs.k8s.io/kustomize/kustomize/v5/commands/internal/testutils" + "sigs.k8s.io/kustomize/kyaml/filesys" +) + +func TestEditSetConfigMapError(t *testing.T) { + fSys := filesys.MakeFsInMemory() + pvd := provider.NewDefaultDepProvider() + + testCases := []struct { + name string + input string + args []string + expectedErrorMsg string + }{ + { + name: "fails to set a value because key doesn't exist", + input: `--- +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +configMapGenerator: +- literals: + - test=value + - key1=val1 + name: test-cm +`, + args: []string{"test-cm", "--from-literal=key3=val2"}, + expectedErrorMsg: "key 'key3' not found in resource", + }, + { + name: "fails to set a value because configmap doesn't exist", + input: `--- +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +configMapGenerator: +- literals: + - test=value + - key1=val1 + name: test-cm +`, + args: []string{"test-cm2", "--from-literal=key3=val2"}, + expectedErrorMsg: "unable to find ConfigMap with name '\"test-cm2\"'", + }, + { + name: "fails validation because no attributes are being changed", + input: `--- +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +configMapGenerator: +- literals: + - test=value + - key1=val1 + name: test-cm + namespace: test-ns +`, + args: []string{"test-cm", "--namespace=test-ns"}, + expectedErrorMsg: "at least one of [--from-literal, --new-namespace] must be specified", + }, + { + name: "fails when a literal source doesn't have a key", + input: `--- +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +configMapGenerator: +- literals: + - test=value + - key1=val1 + name: test-cm +`, + args: []string{"test-cm", "--from-literal=value"}, + expectedErrorMsg: "literal values must be specified in the key=value format", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + cmd := newCmdSetConfigMap( + fSys, + kv.NewLoader( + loader.NewFileLoaderAtCwd(fSys), + pvd.GetFieldValidator()), + pvd.GetResourceFactory(), + ) + + testutils_test.WriteTestKustomizationWith(fSys, []byte(tc.input)) + + cmd.SetArgs(tc.args) + err := cmd.Execute() + + require.Error(t, err) + require.Contains(t, err.Error(), tc.expectedErrorMsg) + }) + } +} + +func TestEditSetConfigMapSuccess(t *testing.T) { + fSys := filesys.MakeFsInMemory() + pvd := provider.NewDefaultDepProvider() + testCases := []struct { + name string + input string + args []string + expectedLiterals []string + expectedNamespace string + }{ + { + name: "set a value successfully", + input: `--- +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +configMapGenerator: +- literals: + - key1=val1 + - test=value + name: test-cm +`, + expectedLiterals: []string{"key1=val2", "test=value"}, + args: []string{"test-cm", "--from-literal=key1=val2"}, + }, + { + name: "successfully update namespace of target configmap", + input: `--- +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +configMapGenerator: +- literals: + - test=value + - key1=val1 + name: test-cm + namespace: test-ns +`, + args: []string{"test-cm", "--namespace=test-ns", "--new-namespace=test-new-ns"}, + expectedNamespace: "test-new-ns", + }, + { + name: "successfully update namespace of target configmap with empty namespace in file and namespace specified in command", + input: `--- +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +configMapGenerator: +- literals: + - test=value + - key1=val1 + name: test-cm +`, + args: []string{"test-cm", "--namespace=default", "--new-namespace=test-new-ns"}, + expectedNamespace: "test-new-ns", + }, + { + name: "successfully update namespace of target configmap with default namespace and no namespace specified in command", + input: `--- +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +configMapGenerator: +- literals: + - test=value + - key1=val1 + name: test-cm + namespace: default +`, + args: []string{"test-cm", "--new-namespace=test-new-ns"}, + expectedNamespace: "test-new-ns", + }, + { + name: "successfully update literal source of target configmap with empty namespace in file and namespace specified in command", + input: `--- +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +configMapGenerator: +- literals: + - test=value + - key1=val1 + name: test-cm +`, + args: []string{"test-cm", "--namespace=default", "--from-literal=key1=val2"}, + expectedLiterals: []string{"test=value", "key1=val2"}, + }, + { + name: "successfully update namespace of target configmap with default namespace and no namespace specified in command", + input: `--- +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +configMapGenerator: +- literals: + - test=value + - key1=val1 + name: test-cm + namespace: default +`, + args: []string{"test-cm", "--namespace=default", "--from-literal=key1=val2"}, + expectedLiterals: []string{"test=value", "key1=val2"}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + cmd := newCmdSetConfigMap( + fSys, + kv.NewLoader( + loader.NewFileLoaderAtCwd(fSys), + pvd.GetFieldValidator()), + pvd.GetResourceFactory(), + ) + + testutils_test.WriteTestKustomizationWith(fSys, []byte(tc.input)) + + cmd.SetArgs(tc.args) + err := cmd.Execute() + + require.NoError(t, err) + + _, err = testutils_test.ReadTestKustomization(fSys) + require.NoError(t, err) + + mf, err := kustfile.NewKustomizationFile(fSys) + require.NoError(t, err) + + kustomization, err := mf.Read() + require.NoError(t, err) + + require.NotNil(t, kustomization) + require.NotEmpty(t, kustomization.ConfigMapGenerator) + require.Greater(t, len(kustomization.ConfigMapGenerator), 0) + + if tc.expectedNamespace != "" { + require.Equal(t, tc.expectedNamespace, kustomization.ConfigMapGenerator[0].Namespace) + } + + if len(tc.expectedLiterals) > 0 { + require.ElementsMatch(t, tc.expectedLiterals, kustomization.ConfigMapGenerator[0].LiteralSources) + } + }) + } +} diff --git a/kustomize/commands/internal/util/configmapSecretFlagsAndArgs.go b/kustomize/commands/internal/util/configmapSecretFlagsAndArgs.go index ffc0a36bd..0a4ef5a63 100644 --- a/kustomize/commands/internal/util/configmapSecretFlagsAndArgs.go +++ b/kustomize/commands/internal/util/configmapSecretFlagsAndArgs.go @@ -18,36 +18,41 @@ const ( DisableNameSuffixHashFlag = "disableNameSuffixHash" BehaviorFlag = "behavior" NamespaceFlag = "namespace" + NewNamespaceFlag = "new-namespace" FlagFormat = "--%s=%s" ) // ConfigMapSecretFlagsAndArgs encapsulates the options for add secret/configmap commands. type ConfigMapSecretFlagsAndArgs struct { - // Name of configMap/Secret (required) + // Name of ConfigMap/Secret (required) Name string - // FileSources to derive the configMap/Secret from (optional) + // FileSources to derive the ConfigMap/Secret from (optional) FileSources []string - // LiteralSources to derive the configMap/Secret from (optional) + // LiteralSources to derive the ConfigMap/Secret from (optional) LiteralSources []string - // EnvFileSource to derive the configMap/Secret from (optional) + // EnvFileSource to derive the ConfigMap/Secret from (optional) // TODO: Rationalize this name with Generic.EnvSource EnvFileSource string // Resource generation behavior (optional) Behavior string // Type of secret to create Type string - // Namespace of secret + // Namespace of ConfigMap/Secret (optional) -- if unspecified, default is assumed Namespace string // Disable name suffix DisableNameSuffixHash bool + // NewNamespace for ConfigMap/Secret (optional) -- only for 'edit set' command + NewNamespace string } -// Validate validates required fields are set to support structured generation. -func (a *ConfigMapSecretFlagsAndArgs) Validate(args []string) error { +// ValidateAdd validates required fields are set to support structured generation for the +// edit add command. +func (a *ConfigMapSecretFlagsAndArgs) ValidateAdd(args []string) error { if len(args) != 1 { return fmt.Errorf("name must be specified once") } a.Name = args[0] + if len(a.EnvFileSource) == 0 && len(a.FileSources) == 0 && len(a.LiteralSources) == 0 { return fmt.Errorf("at least from-env-file, or from-file or from-literal must be set") } @@ -62,24 +67,41 @@ func (a *ConfigMapSecretFlagsAndArgs) Validate(args []string) error { return nil } +// ValidateSet validates required fields are set to support structured generation for the +// edit set command. +func (a *ConfigMapSecretFlagsAndArgs) ValidateSet(args []string) error { + if len(args) != 1 { + return fmt.Errorf("name must be specified once") + } + a.Name = args[0] + + if len(a.LiteralSources) == 0 && a.NewNamespace == "" { + return fmt.Errorf("at least one of [--from-literal, --new-namespace] must be specified") + } + + return nil +} + // ExpandFileSource normalizes a string list, possibly // containing globs, into a validated, globless list. // For example, this list: -// some/path -// some/dir/a* -// bfile=some/dir/b* +// - some/path +// - some/dir/a* +// - bfile=some/dir/b* // becomes: -// some/path -// some/dir/airplane -// some/dir/ant -// some/dir/apple -// bfile=some/dir/banana +// - some/path +// - some/dir/airplane +// - some/dir/ant +// - some/dir/apple +// - bfile=some/dir/banana // i.e. everything is converted to a key=value pair, // where the value is always a relative file path, // and the key, if missing, is the same as the value. // In the case where the key is explicitly declared, // the globbing, if present, must have exactly one match. func (a *ConfigMapSecretFlagsAndArgs) ExpandFileSource(fSys filesys.FileSystem) error { + const numberComponentsWithKey = 2 + var results []string for _, pattern := range a.FileSources { var patterns []string @@ -87,7 +109,7 @@ func (a *ConfigMapSecretFlagsAndArgs) ExpandFileSource(fSys filesys.FileSystem) // check if the pattern is in `--from-file=[key=]source` format // and if so split it to send only the file-pattern to glob function s := strings.Split(pattern, "=") - if len(s) == 2 { + if len(s) == numberComponentsWithKey { patterns = append(patterns, s[1]) key = s[0] } else { @@ -114,6 +136,63 @@ func (a *ConfigMapSecretFlagsAndArgs) ExpandFileSource(fSys filesys.FileSystem) return nil } +// UpdateLiteralSources looks for literal sources that already exist and tries +// to replace their values with new values. +// The key specified must exist in the target resource (ConfigMap or Secret). +func UpdateLiteralSources( + args *types.GeneratorArgs, + flags ConfigMapSecretFlagsAndArgs, +) error { + sources := make(map[string]any) + + for _, val := range args.LiteralSources { + key, value, err := validateAndExtractLiteralSource(val) + if err != nil { + return fmt.Errorf("failed to update resource: %w", err) + } + sources[key] = value + } + + for _, val := range flags.LiteralSources { + key, value, err := validateAndExtractLiteralSource(val) + if err != nil { + return fmt.Errorf("failed to update resource: %w", err) + } + + if _, ok := sources[key]; !ok { + return fmt.Errorf("key '%s' not found in resource", key) + } + + sources[key] = value + } + + // re-assemble key-pairs + newLiteralSources := make([]string, 0) + for key, val := range sources { + newLiteralSources = append(newLiteralSources, fmt.Sprintf("%s=%s", key, val)) + } + + args.LiteralSources = newLiteralSources + + return nil +} + +func validateAndExtractLiteralSource(val string) (string, string, error) { + // This is the separator to be used as a boundary for the key/value pair + const keyValueSeparator = "=" + // Maximum number of times the separator can appear in the string + const maximumSeparatorNumber = 1 + + count := strings.Count(val, keyValueSeparator) + if count <= 0 || count > maximumSeparatorNumber { + return "", "", fmt.Errorf("invalid format: literal values must be specified in the key=value format") + } + + key, value, _ := strings.Cut(val, keyValueSeparator) // we don't need the value of found because of prior validation + + return key, value, nil +} + func MergeFlagsIntoGeneratorArgs(args *types.GeneratorArgs, flags ConfigMapSecretFlagsAndArgs) { if len(flags.LiteralSources) > 0 { args.LiteralSources = append( diff --git a/kustomize/commands/internal/util/configmapSecretFlagsAndArgs_test.go b/kustomize/commands/internal/util/configmapSecretFlagsAndArgs_test.go index f27ae443b..95ce7fa80 100644 --- a/kustomize/commands/internal/util/configmapSecretFlagsAndArgs_test.go +++ b/kustomize/commands/internal/util/configmapSecretFlagsAndArgs_test.go @@ -8,26 +8,101 @@ import ( "testing" "github.com/stretchr/testify/require" + "sigs.k8s.io/kustomize/api/types" . "sigs.k8s.io/kustomize/kustomize/v5/commands/internal/util" "sigs.k8s.io/kustomize/kyaml/filesys" ) -func TestDataValidation_NoName(t *testing.T) { +func TestValidateAdd(t *testing.T) { fa := ConfigMapSecretFlagsAndArgs{} - require.Error(t, fa.Validate([]string{})) + + testCases := []struct { + name string + args []string + wantErr func(require.TestingT, error, ...interface{}) + }{ + { + "validateAdd with no arguments", + []string{}, + require.Error, + }, + { + "validateAdd with more than one name", + []string{"name", "othername"}, + require.Error, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + tc.wantErr(t, fa.ValidateAdd(tc.args)) + }) + } } -func TestDataValidation_MoreThanOneName(t *testing.T) { - fa := ConfigMapSecretFlagsAndArgs{} +func TestValidateSet(t *testing.T) { + testCases := []struct { + name string + args []string + fa ConfigMapSecretFlagsAndArgs + wantErr func(require.TestingT, error, ...interface{}) + }{ + { + // must have one single name + name: "fails with no arguments", + args: []string{}, + fa: ConfigMapSecretFlagsAndArgs{}, + wantErr: require.Error, + }, + { + // must have one single name + name: "fails with more than one name", + args: []string{"testname", "testname2"}, + fa: ConfigMapSecretFlagsAndArgs{}, + wantErr: require.Error, + }, + { + // must have at least --from-literal or --new-namespace + name: "succeeds with --from-literal", + args: []string{"test-configmap"}, + fa: ConfigMapSecretFlagsAndArgs{ + LiteralSources: []string{"key1=value1"}, + }, + wantErr: require.NoError, + }, + { + // must have at least --from-literal or --new-namespace + name: "succeeds with --new-namespace", + args: []string{"test-configmap"}, + fa: ConfigMapSecretFlagsAndArgs{ + NewNamespace: "new-namespace", + }, + wantErr: require.NoError, + }, + { + // must have at least --from-literal or --new-namespace + name: "succeeds with --new-namespace and --from-literal", + args: []string{"test-configmap"}, + fa: ConfigMapSecretFlagsAndArgs{ + LiteralSources: []string{"key1=value1"}, + NewNamespace: "new-namespace", + }, + wantErr: require.NoError, + }, + } - require.Error(t, fa.Validate([]string{"name", "othername"})) + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + tc.wantErr(t, tc.fa.ValidateSet(tc.args)) + }) + } } func TestDataConfigValidation_Flags(t *testing.T) { tests := []struct { - name string - fa ConfigMapSecretFlagsAndArgs - shouldFail bool + name string + fa ConfigMapSecretFlagsAndArgs + wantErr func(require.TestingT, error, ...interface{}) }{ { name: "env-file-source and literal are both set", @@ -35,7 +110,7 @@ func TestDataConfigValidation_Flags(t *testing.T) { LiteralSources: []string{"one", "two"}, EnvFileSource: "three", }, - shouldFail: true, + wantErr: require.Error, }, { name: "env-file-source and from-file are both set", @@ -43,12 +118,12 @@ func TestDataConfigValidation_Flags(t *testing.T) { FileSources: []string{"one", "two"}, EnvFileSource: "three", }, - shouldFail: true, + wantErr: require.Error, }, { - name: "we don't have any option set", - fa: ConfigMapSecretFlagsAndArgs{}, - shouldFail: true, + name: "we don't have any option set", + fa: ConfigMapSecretFlagsAndArgs{}, + wantErr: require.Error, }, { name: "we have from-file and literal ", @@ -56,7 +131,7 @@ func TestDataConfigValidation_Flags(t *testing.T) { LiteralSources: []string{"one", "two"}, FileSources: []string{"three", "four"}, }, - shouldFail: false, + wantErr: require.NoError, }, { name: "correct behavior", @@ -64,7 +139,7 @@ func TestDataConfigValidation_Flags(t *testing.T) { EnvFileSource: "foo", Behavior: "merge", }, - shouldFail: false, + wantErr: require.NoError, }, { name: "incorrect behavior", @@ -72,17 +147,14 @@ func TestDataConfigValidation_Flags(t *testing.T) { EnvFileSource: "foo", Behavior: "merge-unknown", }, - shouldFail: true, + wantErr: require.Error, }, } for _, test := range tests { - err := test.fa.Validate([]string{"name"}) - if test.shouldFail { - require.Error(t, err) - } else { - require.NoError(t, err) - } + t.Run(test.name, func(t *testing.T) { + test.wantErr(t, test.fa.ValidateAdd([]string{"name"})) + }) } } @@ -149,3 +221,135 @@ func TestExpandFileSourceWithKeyAndError(t *testing.T) { t.Fatalf("FileSources should not be correctly expanded: %v", fa.FileSources) } } + +func TestUpdateLiteralSources(t *testing.T) { + testCases := []struct { + name string + args *types.GeneratorArgs + flags ConfigMapSecretFlagsAndArgs + expectedArgs *types.GeneratorArgs + wantErr func(require.TestingT, error, ...interface{}) + }{ + { + name: "fails when key doesn't exist", + args: &types.GeneratorArgs{ + KvPairSources: types.KvPairSources{ + LiteralSources: []string{"key1=val1", "otherkey=value"}, + }, + }, + flags: ConfigMapSecretFlagsAndArgs{ + LiteralSources: []string{ + "key2=value", + }, + }, + expectedArgs: &types.GeneratorArgs{ + KvPairSources: types.KvPairSources{ + LiteralSources: []string{"key1=val1", "otherkey=value"}, + }, + }, + wantErr: require.Error, + }, + { + name: "updates correctly an existing key", + args: &types.GeneratorArgs{ + KvPairSources: types.KvPairSources{ + LiteralSources: []string{"key2=val1", "otherkey=value"}, + }, + }, + flags: ConfigMapSecretFlagsAndArgs{ + LiteralSources: []string{ + "key2=value", + }, + }, + expectedArgs: &types.GeneratorArgs{ + KvPairSources: types.KvPairSources{ + LiteralSources: []string{"key2=value", "otherkey=value"}, + }, + }, + wantErr: require.NoError, + }, + { + name: "fails when format for literal sources is incorrect in flags", + args: &types.GeneratorArgs{ + KvPairSources: types.KvPairSources{ + LiteralSources: []string{"key2=val1", "otherkey=value"}, + }, + }, + flags: ConfigMapSecretFlagsAndArgs{ + LiteralSources: []string{ + "key2", + }, + }, + expectedArgs: &types.GeneratorArgs{ + KvPairSources: types.KvPairSources{ + LiteralSources: []string{"key2=val1", "otherkey=value"}, + }, + }, + wantErr: require.Error, + }, + { // unlikely to happen + name: "fails when format for literal sources is incorrect in existing args", + args: &types.GeneratorArgs{ + KvPairSources: types.KvPairSources{ + LiteralSources: []string{"key2", "otherkey=value"}, + }, + }, + flags: ConfigMapSecretFlagsAndArgs{ + LiteralSources: []string{ + "key2=val2", + }, + }, + expectedArgs: &types.GeneratorArgs{ + KvPairSources: types.KvPairSources{ + LiteralSources: []string{"key2", "otherkey=value"}, + }, + }, + wantErr: require.Error, + }, + { + name: "fails when literal sources from flags contain more than one =", + args: &types.GeneratorArgs{ + KvPairSources: types.KvPairSources{ + LiteralSources: []string{"key2=val1", "otherkey=value"}, + }, + }, + flags: ConfigMapSecretFlagsAndArgs{ + LiteralSources: []string{ + "key2=val2=val3", + }, + }, + expectedArgs: &types.GeneratorArgs{ + KvPairSources: types.KvPairSources{ + LiteralSources: []string{"key2=val1", "otherkey=value"}, + }, + }, + wantErr: require.Error, + }, + { // unlikely to happen + name: "fails when literal sources contain more than one = in existing args", + args: &types.GeneratorArgs{ + KvPairSources: types.KvPairSources{ + LiteralSources: []string{"key2=val1=val3", "otherkey=value"}, + }, + }, + flags: ConfigMapSecretFlagsAndArgs{ + LiteralSources: []string{ + "key2=val2", + }, + }, + expectedArgs: &types.GeneratorArgs{ + KvPairSources: types.KvPairSources{ + LiteralSources: []string{"key2=val1=val3", "otherkey=value"}, + }, + }, + wantErr: require.Error, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + tc.wantErr(t, UpdateLiteralSources(tc.args, tc.flags)) + require.Equal(t, tc.expectedArgs.LiteralSources, tc.args.LiteralSources) + }) + } +} diff --git a/kustomize/go.mod b/kustomize/go.mod index 0da2375e8..829fc68b5 100644 --- a/kustomize/go.mod +++ b/kustomize/go.mod @@ -7,6 +7,7 @@ require ( github.com/spf13/cobra v1.7.0 github.com/spf13/pflag v1.0.5 github.com/stretchr/testify v1.8.1 + golang.org/x/exp v0.0.0-20231006140011-7918f672742d golang.org/x/text v0.13.0 sigs.k8s.io/kustomize/api v0.15.0 sigs.k8s.io/kustomize/cmd/config v0.12.0 diff --git a/kustomize/go.sum b/kustomize/go.sum index 377893fe6..ebeb93474 100644 --- a/kustomize/go.sum +++ b/kustomize/go.sum @@ -66,6 +66,8 @@ github.com/xlab/treeprint v1.2.0 h1:HzHnuAF1plUN2zGlAFHbSQP2qJ0ZAD3XF5XD7OesXRQ= github.com/xlab/treeprint v1.2.0/go.mod h1:gj5Gd3gPdKtR1ikdDK6fnFLdmIS0X30kTTuNd/WEJu0= go.starlark.net v0.0.0-20200306205701-8dd3e2ee1dd5 h1:+FNtrFTmVw0YZGpBGX56XDee331t6JAXeK2bcyhLOOc= go.starlark.net v0.0.0-20200306205701-8dd3e2ee1dd5/go.mod h1:nmDLcffg48OtT/PSW0Hg7FvpRQsQh5OSqIylirxKC7o= +golang.org/x/exp v0.0.0-20231006140011-7918f672742d h1:jtJma62tbqLibJ5sFQz8bKtEM8rJBtfilJ2qTU199MI= +golang.org/x/exp v0.0.0-20231006140011-7918f672742d/go.mod h1:ldy0pHrwJyGW56pPQzzkH36rKxoZW1tw7ZJpeKx+hdo= golang.org/x/sys v0.0.0-20191002063906-3421d5a6bb1c/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.13.0 h1:Af8nKPmuFypiUBjVoU9V20FiaFXOcuZI21p0ycVYYGE= golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=