From fb7ee2f4871d4ef054ecd9d2e1bc9b10cbfde4a9 Mon Sep 17 00:00:00 2001 From: Mauren Berti <698465+stormqueen1990@users.noreply.github.com> Date: Sun, 1 Oct 2023 22:32:45 -0400 Subject: [PATCH] refactor: change "add configmap/secret" commands to reuse code and improve tests (#5315) * feat: minor refactoring + test improvements * Refactor some bits of the edit add secret/configmap commands to reuse code. * Improve test coverage for both these commands by invoking the cobra command directly. * Add some reusable constants for both these commands and their tests. * fix: changes from code review * Update formatting as requested in code review. * Rename flagsAndArgs to configmapSecretFlagsAndArgs: change the file and struct names to reflect the real usage of this code. * Remove excessive newlines from the imports block. * Replace all usages of assert with require and fix newlines in imports. --- kustomize/commands/edit/add/configmap.go | 123 +++++++------ ...args.go => configmapSecretFlagsAndArgs.go} | 40 ++++- ...go => configmapSecretFlagsAndArgs_test.go} | 33 ++-- kustomize/commands/edit/add/configmap_test.go | 168 ++++++++++++++++-- kustomize/commands/edit/add/secret.go | 113 ++++++------ kustomize/commands/edit/add/secret_test.go | 164 ++++++++++++++++- 6 files changed, 479 insertions(+), 162 deletions(-) rename kustomize/commands/edit/add/{flagsandargs.go => configmapSecretFlagsAndArgs.go} (73%) rename kustomize/commands/edit/add/{flagsandargs_test.go => configmapSecretFlagsAndArgs_test.go} (83%) diff --git a/kustomize/commands/edit/add/configmap.go b/kustomize/commands/edit/add/configmap.go index 16c0cc3fe4..bb200f9e59 100644 --- a/kustomize/commands/edit/add/configmap.go +++ b/kustomize/commands/edit/add/configmap.go @@ -4,6 +4,8 @@ package add import ( + "fmt" + "github.com/spf13/cobra" "sigs.k8s.io/kustomize/api/ifc" "sigs.k8s.io/kustomize/api/resource" @@ -16,8 +18,9 @@ import ( func newCmdAddConfigMap( fSys filesys.FileSystem, ldr ifc.KvLoader, - rf *resource.Factory) *cobra.Command { - var flags flagsAndArgs + rf *resource.Factory, +) *cobra.Command { + var flags configmapSecretFlagsAndArgs cmd := &cobra.Command{ Use: "configmap NAME [--behavior={create|merge|replace}] [--from-file=[key=]source] [--from-literal=key1=value1]", Short: "Adds a configmap to the kustomization file", @@ -36,63 +39,35 @@ func newCmdAddConfigMap( kustomize edit add configmap my-configmap --behavior=merge --from-env-file=env/path.env `, RunE: func(_ *cobra.Command, args []string) error { - err := flags.ExpandFileSource(fSys) - if err != nil { - return err - } - - err = flags.Validate(args) - if err != nil { - return err - } - - // Load the kustomization file. - mf, err := kustfile.NewKustomizationFile(fSys) - if err != nil { - return err - } - - kustomization, err := mf.Read() - if err != nil { - return err - } - - // Add the flagsAndArgs map to the kustomization file. - err = addConfigMap(ldr, kustomization, flags, rf) - if err != nil { - return err - } - - // Write out the kustomization file with added configmap. - return mf.Write(kustomization) + return runEditAddConfigMap(flags, fSys, args, ldr, rf) }, } cmd.Flags().StringSliceVar( &flags.FileSources, - "from-file", + fromFileFlag, []string{}, "Key file can be specified using its file path, in which case file basename will be used as configmap "+ "key, or optionally with a key and file path, in which case the given key will be used. Specifying a "+ "directory will iterate each named file in the directory whose basename is a valid configmap key.") cmd.Flags().StringArrayVar( &flags.LiteralSources, - "from-literal", + fromLiteralFlag, []string{}, "Specify a key and literal value to insert in configmap (i.e. mykey=somevalue)") cmd.Flags().StringVar( &flags.EnvFileSource, - "from-env-file", + fromEnvFileFlag, "", "Specify the path to a file to read lines of key=val pairs to create a configmap (i.e. a Docker .env file).") cmd.Flags().BoolVar( &flags.DisableNameSuffixHash, - "disableNameSuffixHash", + flagDisableNameSuffixHash, false, "Disable the name suffix for the configmap") cmd.Flags().StringVar( &flags.Behavior, - "behavior", + flagBehavior, "", "Specify the behavior for config map generation, i.e whether to create a new configmap (the default), "+ "to merge with a previously defined one, or to replace an existing one. Merge and replace should be used only "+ @@ -101,15 +76,60 @@ func newCmdAddConfigMap( return cmd } +func runEditAddConfigMap( + flags 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.Validate(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) + } + + // Add the configmapSecretFlagsAndArgs map to the kustomization file. + err = addConfigMap(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 +} + // addConfigMap adds a configmap to a kustomization file. // Note: error may leave kustomization file in an undefined state. // Suggest passing a copy of kustomization file. func addConfigMap( ldr ifc.KvLoader, k *types.Kustomization, - flags flagsAndArgs, rf *resource.Factory) error { + flags configmapSecretFlagsAndArgs, + rf *resource.Factory, +) error { args := findOrMakeConfigMapArgs(k, flags.Name) - mergeFlagsIntoCmArgs(args, flags) + mergeFlagsIntoGeneratorArgs(&args.GeneratorArgs, flags) // Validate by trying to create corev1.configmap. args.Options = types.MergeGlobalOptionsIntoLocal( args.Options, k.GeneratorOptions) @@ -124,30 +144,9 @@ func findOrMakeConfigMapArgs(m *types.Kustomization, name string) *types.ConfigM } } // config map not found, create new one and add it to the kustomization file. - cm := &types.ConfigMapArgs{GeneratorArgs: types.GeneratorArgs{Name: name}} + cm := &types.ConfigMapArgs{ + GeneratorArgs: types.GeneratorArgs{Name: name}, + } m.ConfigMapGenerator = append(m.ConfigMapGenerator, *cm) return &m.ConfigMapGenerator[len(m.ConfigMapGenerator)-1] } - -func mergeFlagsIntoCmArgs(args *types.ConfigMapArgs, flags flagsAndArgs) { - if len(flags.LiteralSources) > 0 { - args.LiteralSources = append( - args.LiteralSources, flags.LiteralSources...) - } - if len(flags.FileSources) > 0 { - args.FileSources = append( - args.FileSources, flags.FileSources...) - } - if flags.EnvFileSource != "" { - args.EnvSources = append( - args.EnvSources, flags.EnvFileSource) - } - if flags.DisableNameSuffixHash { - args.Options = &types.GeneratorOptions{ - DisableNameSuffixHash: true, - } - } - if flags.Behavior != "" { - args.Behavior = flags.Behavior - } -} diff --git a/kustomize/commands/edit/add/flagsandargs.go b/kustomize/commands/edit/add/configmapSecretFlagsAndArgs.go similarity index 73% rename from kustomize/commands/edit/add/flagsandargs.go rename to kustomize/commands/edit/add/configmapSecretFlagsAndArgs.go index 03077e42c1..83c0e3b9d1 100644 --- a/kustomize/commands/edit/add/flagsandargs.go +++ b/kustomize/commands/edit/add/configmapSecretFlagsAndArgs.go @@ -13,8 +13,17 @@ import ( "sigs.k8s.io/kustomize/kyaml/filesys" ) -// flagsAndArgs encapsulates the options for add secret/configmap commands. -type flagsAndArgs struct { +const ( + fromFileFlag = "from-file" + fromLiteralFlag = "from-literal" + fromEnvFileFlag = "from-env-file" + flagDisableNameSuffixHash = "disableNameSuffixHash" + flagBehavior = "behavior" + flagFormat = "--%s=%s" +) + +// configmapSecretFlagsAndArgs encapsulates the options for add secret/configmap commands. +type configmapSecretFlagsAndArgs struct { // Name of configMap/Secret (required) Name string // FileSources to derive the configMap/Secret from (optional) @@ -35,7 +44,7 @@ type flagsAndArgs struct { } // Validate validates required fields are set to support structured generation. -func (a *flagsAndArgs) Validate(args []string) error { +func (a *configmapSecretFlagsAndArgs) Validate(args []string) error { if len(args) != 1 { return fmt.Errorf("name must be specified once") } @@ -71,7 +80,7 @@ func (a *flagsAndArgs) Validate(args []string) error { // 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 *flagsAndArgs) ExpandFileSource(fSys filesys.FileSystem) error { +func (a *configmapSecretFlagsAndArgs) ExpandFileSource(fSys filesys.FileSystem) error { var results []string for _, pattern := range a.FileSources { var patterns []string @@ -105,3 +114,26 @@ func (a *flagsAndArgs) ExpandFileSource(fSys filesys.FileSystem) error { a.FileSources = results return nil } + +func mergeFlagsIntoGeneratorArgs(args *types.GeneratorArgs, flags configmapSecretFlagsAndArgs) { + if len(flags.LiteralSources) > 0 { + args.LiteralSources = append( + args.LiteralSources, flags.LiteralSources...) + } + if len(flags.FileSources) > 0 { + args.FileSources = append( + args.FileSources, flags.FileSources...) + } + if flags.EnvFileSource != "" { + args.EnvSources = append( + args.EnvSources, flags.EnvFileSource) + } + if flags.DisableNameSuffixHash { + args.Options = &types.GeneratorOptions{ + DisableNameSuffixHash: true, + } + } + if flags.Behavior != "" { + args.Behavior = flags.Behavior + } +} diff --git a/kustomize/commands/edit/add/flagsandargs_test.go b/kustomize/commands/edit/add/configmapSecretFlagsAndArgs_test.go similarity index 83% rename from kustomize/commands/edit/add/flagsandargs_test.go rename to kustomize/commands/edit/add/configmapSecretFlagsAndArgs_test.go index c3bb9d0dbe..49abb37371 100644 --- a/kustomize/commands/edit/add/flagsandargs_test.go +++ b/kustomize/commands/edit/add/configmapSecretFlagsAndArgs_test.go @@ -7,31 +7,30 @@ import ( "reflect" "testing" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "sigs.k8s.io/kustomize/kyaml/filesys" ) func TestDataValidation_NoName(t *testing.T) { - fa := flagsAndArgs{} - assert.Error(t, fa.Validate([]string{})) + fa := configmapSecretFlagsAndArgs{} + require.Error(t, fa.Validate([]string{})) } func TestDataValidation_MoreThanOneName(t *testing.T) { - fa := flagsAndArgs{} + fa := configmapSecretFlagsAndArgs{} - assert.Error(t, fa.Validate([]string{"name", "othername"})) + require.Error(t, fa.Validate([]string{"name", "othername"})) } func TestDataConfigValidation_Flags(t *testing.T) { tests := []struct { name string - fa flagsAndArgs + fa configmapSecretFlagsAndArgs shouldFail bool }{ { name: "env-file-source and literal are both set", - fa: flagsAndArgs{ + fa: configmapSecretFlagsAndArgs{ LiteralSources: []string{"one", "two"}, EnvFileSource: "three", }, @@ -39,7 +38,7 @@ func TestDataConfigValidation_Flags(t *testing.T) { }, { name: "env-file-source and from-file are both set", - fa: flagsAndArgs{ + fa: configmapSecretFlagsAndArgs{ FileSources: []string{"one", "two"}, EnvFileSource: "three", }, @@ -47,12 +46,12 @@ func TestDataConfigValidation_Flags(t *testing.T) { }, { name: "we don't have any option set", - fa: flagsAndArgs{}, + fa: configmapSecretFlagsAndArgs{}, shouldFail: true, }, { name: "we have from-file and literal ", - fa: flagsAndArgs{ + fa: configmapSecretFlagsAndArgs{ LiteralSources: []string{"one", "two"}, FileSources: []string{"three", "four"}, }, @@ -60,7 +59,7 @@ func TestDataConfigValidation_Flags(t *testing.T) { }, { name: "correct behavior", - fa: flagsAndArgs{ + fa: configmapSecretFlagsAndArgs{ EnvFileSource: "foo", Behavior: "merge", }, @@ -68,7 +67,7 @@ func TestDataConfigValidation_Flags(t *testing.T) { }, { name: "incorrect behavior", - fa: flagsAndArgs{ + fa: configmapSecretFlagsAndArgs{ EnvFileSource: "foo", Behavior: "merge-unknown", }, @@ -79,9 +78,9 @@ func TestDataConfigValidation_Flags(t *testing.T) { for _, test := range tests { err := test.fa.Validate([]string{"name"}) if test.shouldFail { - assert.Error(t, err) + require.Error(t, err) } else { - assert.NoError(t, err) + require.NoError(t, err) } } } @@ -94,7 +93,7 @@ func TestExpandFileSource(t *testing.T) { require.NoError(t, err) _, err = fSys.Create("dir/readme") require.NoError(t, err) - fa := flagsAndArgs{ + fa := configmapSecretFlagsAndArgs{ FileSources: []string{"dir/fa*"}, } err = fa.ExpandFileSource(fSys) @@ -118,7 +117,7 @@ func TestExpandFileSourceWithKey(t *testing.T) { require.NoError(t, err) _, err = fSys.Create("dir/readme") require.NoError(t, err) - fa := flagsAndArgs{ + fa := configmapSecretFlagsAndArgs{ FileSources: []string{"foo-key=dir/fa*", "bar-key=dir/foobar", "dir/simplebar"}, } err = fa.ExpandFileSource(fSys) @@ -141,7 +140,7 @@ func TestExpandFileSourceWithKeyAndError(t *testing.T) { require.NoError(t, err) _, err = fSys.Create("dir/readme") require.NoError(t, err) - fa := flagsAndArgs{ + fa := configmapSecretFlagsAndArgs{ FileSources: []string{"foo-key=dir/fa*"}, } err = fa.ExpandFileSource(fSys) diff --git a/kustomize/commands/edit/add/configmap_test.go b/kustomize/commands/edit/add/configmap_test.go index a6323e6636..0b91ef5ccf 100644 --- a/kustomize/commands/edit/add/configmap_test.go +++ b/kustomize/commands/edit/add/configmap_test.go @@ -4,13 +4,18 @@ package add import ( + "fmt" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "sigs.k8s.io/kustomize/api/kv" - ldrhelper "sigs.k8s.io/kustomize/api/pkg/loader" + "sigs.k8s.io/kustomize/api/pkg/loader" + "sigs.k8s.io/kustomize/api/provider" valtest_test "sigs.k8s.io/kustomize/api/testutils/valtest" "sigs.k8s.io/kustomize/api/types" + "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" ) @@ -19,7 +24,7 @@ func TestNewAddConfigMapIsNotNil(t *testing.T) { assert.NotNil(t, newCmdAddConfigMap( fSys, kv.NewLoader( - ldrhelper.NewFileLoaderAtCwd(fSys), + loader.NewFileLoaderAtCwd(fSys), valtest_test.MakeFakeValidator()), nil)) } @@ -47,10 +52,10 @@ func TestMergeFlagsIntoConfigMapArgs_LiteralSources(t *testing.T) { args := findOrMakeConfigMapArgs(k, "foo") mergeFlagsIntoGeneratorArgs( &args.GeneratorArgs, - flagsAndArgs{LiteralSources: []string{"k1=v1"}}) + configmapSecretFlagsAndArgs{LiteralSources: []string{"k1=v1"}}) mergeFlagsIntoGeneratorArgs( &args.GeneratorArgs, - flagsAndArgs{LiteralSources: []string{"k2=v2"}}) + configmapSecretFlagsAndArgs{LiteralSources: []string{"k2=v2"}}) assert.Equal(t, "k1=v1", k.ConfigMapGenerator[0].LiteralSources[0]) assert.Equal(t, "k2=v2", k.ConfigMapGenerator[0].LiteralSources[1]) } @@ -60,10 +65,10 @@ func TestMergeFlagsIntoConfigMapArgs_FileSources(t *testing.T) { args := findOrMakeConfigMapArgs(k, "foo") mergeFlagsIntoGeneratorArgs( &args.GeneratorArgs, - flagsAndArgs{FileSources: []string{"file1"}}) + configmapSecretFlagsAndArgs{FileSources: []string{"file1"}}) mergeFlagsIntoGeneratorArgs( &args.GeneratorArgs, - flagsAndArgs{FileSources: []string{"file2"}}) + configmapSecretFlagsAndArgs{FileSources: []string{"file2"}}) assert.Equal(t, "file1", k.ConfigMapGenerator[0].FileSources[0]) assert.Equal(t, "file2", k.ConfigMapGenerator[0].FileSources[1]) } @@ -73,10 +78,10 @@ func TestMergeFlagsIntoConfigMapArgs_EnvSource(t *testing.T) { args := findOrMakeConfigMapArgs(k, "foo") mergeFlagsIntoGeneratorArgs( &args.GeneratorArgs, - flagsAndArgs{EnvFileSource: "env1"}) + configmapSecretFlagsAndArgs{EnvFileSource: "env1"}) mergeFlagsIntoGeneratorArgs( &args.GeneratorArgs, - flagsAndArgs{EnvFileSource: "env2"}) + configmapSecretFlagsAndArgs{EnvFileSource: "env2"}) assert.Equal(t, "env1", k.ConfigMapGenerator[0].EnvSources[0]) assert.Equal(t, "env2", k.ConfigMapGenerator[0].EnvSources[1]) } @@ -85,7 +90,7 @@ func TestMergeFlagsIntoConfigMapArgs_Behavior(t *testing.T) { k := &types.Kustomization{} args := findOrMakeConfigMapArgs(k, "foo") - createBehaviorFlags := flagsAndArgs{ + createBehaviorFlags := configmapSecretFlagsAndArgs{ Behavior: "create", EnvFileSource: "env1", } @@ -94,7 +99,7 @@ func TestMergeFlagsIntoConfigMapArgs_Behavior(t *testing.T) { createBehaviorFlags) assert.Equal(t, "create", k.ConfigMapGenerator[0].Behavior) - mergeBehaviorFlags := flagsAndArgs{ + mergeBehaviorFlags := configmapSecretFlagsAndArgs{ Behavior: "merge", EnvFileSource: "env1", } @@ -103,7 +108,7 @@ func TestMergeFlagsIntoConfigMapArgs_Behavior(t *testing.T) { mergeBehaviorFlags) assert.Equal(t, "merge", k.ConfigMapGenerator[0].Behavior) - replaceBehaviorFlags := flagsAndArgs{ + replaceBehaviorFlags := configmapSecretFlagsAndArgs{ Behavior: "replace", EnvFileSource: "env1", } @@ -112,3 +117,144 @@ func TestMergeFlagsIntoConfigMapArgs_Behavior(t *testing.T) { replaceBehaviorFlags) assert.Equal(t, "replace", k.ConfigMapGenerator[0].Behavior) } + +// TestEditAddConfigMapWithLiteralSource executes the same command flow as the CLI invocation +// with a --from-literal flag +func TestEditAddConfigMapWithLiteralSource(t *testing.T) { + const ( + configMapName = "test-kustomization" + literalSource = "test-key=test-value" + ) + + fSys := filesys.MakeEmptyDirInMemory() + testutils_test.WriteTestKustomization(fSys) + + pvd := provider.NewDefaultDepProvider() + ldr := kv.NewLoader(loader.NewFileLoaderAtCwd(fSys), pvd.GetFieldValidator()) + + args := []string{ + configMapName, + fmt.Sprintf(flagFormat, fromLiteralFlag, literalSource), + } + cmd := newCmdAddConfigMap(fSys, ldr, pvd.GetResourceFactory()) + cmd.SetArgs(args) + require.NoError(t, cmd.Execute()) + + _, 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.Equal(t, 1, len(kustomization.ConfigMapGenerator)) + + newCmGenerator := kustomization.ConfigMapGenerator[0] + require.NotNil(t, newCmGenerator) + require.Equal(t, configMapName, newCmGenerator.Name) + require.Contains(t, newCmGenerator.LiteralSources, literalSource) +} + +// TestEditAddConfigMapWithEnvSource executes the same command flow as the CLI invocation +// with a --from-env-file flag +func TestEditAddConfigMapWithEnvSource(t *testing.T) { + const ( + configMapName = "test-kustomization" + envSource = "test-env-source" + ) + + fSys := filesys.MakeEmptyDirInMemory() + testutils_test.WriteTestKustomization(fSys) + + pvd := provider.NewDefaultDepProvider() + ldr := kv.NewLoader(loader.NewFileLoaderAtCwd(fSys), pvd.GetFieldValidator()) + + envFileContent, err := fSys.Create("test-env-source") + require.NoError(t, err) + + _, err = envFileContent.Write([]byte("TEST=value")) + require.NoError(t, err) + + err = envFileContent.Close() + require.NoError(t, err) + + args := []string{ + configMapName, + fmt.Sprintf(flagFormat, fromEnvFileFlag, envSource), + } + cmd := newCmdAddConfigMap(fSys, ldr, pvd.GetResourceFactory()) + cmd.SetArgs(args) + require.NoError(t, cmd.Execute()) + + _, 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.Equal(t, 1, len(kustomization.ConfigMapGenerator)) + + newCmGenerator := kustomization.ConfigMapGenerator[0] + require.NotNil(t, newCmGenerator) + require.Equal(t, configMapName, newCmGenerator.Name) + require.Contains(t, newCmGenerator.EnvSources, envSource) +} + +// TestEditAddConfigMapWithFileSource executes the same command flow as the CLI invocation +// with a --from-file flag +func TestEditAddConfigMapWithFileSource(t *testing.T) { + const ( + configMapName = "test-kustomization" + fileSource = "test-file-source" + ) + + fSys := filesys.MakeEmptyDirInMemory() + testutils_test.WriteTestKustomization(fSys) + + pvd := provider.NewDefaultDepProvider() + ldr := kv.NewLoader(loader.NewFileLoaderAtCwd(fSys), pvd.GetFieldValidator()) + + fileContent, err := fSys.Create("test-file-source") + require.NoError(t, err) + + _, err = fileContent.Write([]byte("any content here")) + require.NoError(t, err) + + err = fileContent.Close() + require.NoError(t, err) + + args := []string{ + configMapName, + fmt.Sprintf(flagFormat, fromFileFlag, fileSource), + } + cmd := newCmdAddConfigMap(fSys, ldr, pvd.GetResourceFactory()) + cmd.SetArgs(args) + require.NoError(t, cmd.Execute()) + + _, 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.Equal(t, 1, len(kustomization.ConfigMapGenerator)) + + newCmGenerator := kustomization.ConfigMapGenerator[0] + require.NotNil(t, newCmGenerator) + require.Equal(t, configMapName, newCmGenerator.Name) + require.Contains(t, newCmGenerator.FileSources, fileSource) +} diff --git a/kustomize/commands/edit/add/secret.go b/kustomize/commands/edit/add/secret.go index 32172a8b60..5a6d7ab004 100644 --- a/kustomize/commands/edit/add/secret.go +++ b/kustomize/commands/edit/add/secret.go @@ -4,6 +4,8 @@ package add import ( + "fmt" + "github.com/spf13/cobra" "sigs.k8s.io/kustomize/api/ifc" "sigs.k8s.io/kustomize/api/resource" @@ -16,8 +18,9 @@ import ( func newCmdAddSecret( fSys filesys.FileSystem, ldr ifc.KvLoader, - rf *resource.Factory) *cobra.Command { - var flags flagsAndArgs + rf *resource.Factory, +) *cobra.Command { + var flags configmapSecretFlagsAndArgs cmd := &cobra.Command{ Use: "secret NAME [--from-file=[key=]source] [--from-literal=key1=value1] [--type=Opaque|kubernetes.io/tls]", Short: "Adds a secret to the kustomization file.", @@ -33,53 +36,25 @@ func newCmdAddSecret( kustomize edit add secret my-secret --from-env-file=env/path.env `, RunE: func(_ *cobra.Command, args []string) error { - err := flags.ExpandFileSource(fSys) - if err != nil { - return err - } - - err = flags.Validate(args) - if err != nil { - return err - } - - // Load the kustomization file. - mf, err := kustfile.NewKustomizationFile(fSys) - if err != nil { - return err - } - - kustomization, err := mf.Read() - if err != nil { - return err - } - - // Add the flagsAndArgs map to the kustomization file. - err = addSecret(ldr, kustomization, flags, rf) - if err != nil { - return err - } - - // Write out the kustomization file with added secret. - return mf.Write(kustomization) + return runEditAddSecret(flags, fSys, args, ldr, rf) }, } cmd.Flags().StringSliceVar( &flags.FileSources, - "from-file", + fromFileFlag, []string{}, "Key file can be specified using its file path, in which case file basename will be used as secret "+ "key, or optionally with a key and file path, in which case the given key will be used. Specifying a "+ "directory will iterate each named file in the directory whose basename is a valid secret key.") cmd.Flags().StringArrayVar( &flags.LiteralSources, - "from-literal", + fromLiteralFlag, []string{}, "Specify a key and literal value to insert in secret (i.e. mykey=somevalue)") cmd.Flags().StringVar( &flags.EnvFileSource, - "from-env-file", + fromEnvFileFlag, "", "Specify the path to a file to read lines of key=val pairs to create a secret (i.e. a Docker .env file).") cmd.Flags().StringVar( @@ -94,20 +69,63 @@ func newCmdAddSecret( "Specify the namespace of the secret") cmd.Flags().BoolVar( &flags.DisableNameSuffixHash, - "disableNameSuffixHash", + flagDisableNameSuffixHash, false, "Disable the name suffix for the secret") return cmd } +func runEditAddSecret( + flags 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.Validate(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) + } + + // Add the configmapSecretFlagsAndArgs map to the kustomization file. + err = addSecret(ldr, kustomization, flags, rf) + if err != nil { + return fmt.Errorf("failed to create secret: %w", err) + } + + // Write out the kustomization file with added secret. + err = mf.Write(kustomization) + if err != nil { + return fmt.Errorf("failed to write kustomization file: %w", err) + } + + return nil +} + // addSecret adds a secret to a kustomization file. // Note: error may leave kustomization file in an undefined state. // Suggest passing a copy of kustomization file. func addSecret( ldr ifc.KvLoader, k *types.Kustomization, - flags flagsAndArgs, rf *resource.Factory) error { + flags configmapSecretFlagsAndArgs, rf *resource.Factory) error { args := findOrMakeSecretArgs(k, flags.Name, flags.Namespace, flags.Type) mergeFlagsIntoGeneratorArgs(&args.GeneratorArgs, flags) // Validate by trying to create corev1.secret. @@ -131,26 +149,3 @@ func findOrMakeSecretArgs(m *types.Kustomization, name, namespace, secretType st m.SecretGenerator = append(m.SecretGenerator, *secret) return &m.SecretGenerator[len(m.SecretGenerator)-1] } - -func mergeFlagsIntoGeneratorArgs(args *types.GeneratorArgs, flags flagsAndArgs) { - if len(flags.LiteralSources) > 0 { - args.LiteralSources = append( - args.LiteralSources, flags.LiteralSources...) - } - if len(flags.FileSources) > 0 { - args.FileSources = append( - args.FileSources, flags.FileSources...) - } - if flags.EnvFileSource != "" { - args.EnvSources = append( - args.EnvSources, flags.EnvFileSource) - } - if flags.DisableNameSuffixHash { - args.Options = &types.GeneratorOptions{ - DisableNameSuffixHash: true, - } - } - if flags.Behavior != "" { - args.Behavior = flags.Behavior - } -} diff --git a/kustomize/commands/edit/add/secret_test.go b/kustomize/commands/edit/add/secret_test.go index d01afd0933..1622f75152 100644 --- a/kustomize/commands/edit/add/secret_test.go +++ b/kustomize/commands/edit/add/secret_test.go @@ -4,13 +4,18 @@ package add import ( + "fmt" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "sigs.k8s.io/kustomize/api/kv" - ldrhelper "sigs.k8s.io/kustomize/api/pkg/loader" + "sigs.k8s.io/kustomize/api/pkg/loader" + "sigs.k8s.io/kustomize/api/provider" valtest_test "sigs.k8s.io/kustomize/api/testutils/valtest" "sigs.k8s.io/kustomize/api/types" + "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" ) @@ -19,7 +24,7 @@ func TestNewCmdAddSecretIsNotNil(t *testing.T) { assert.NotNil(t, newCmdAddSecret( fSys, kv.NewLoader( - ldrhelper.NewFileLoaderAtCwd(fSys), + loader.NewFileLoaderAtCwd(fSys), valtest_test.MakeFakeValidator()), nil)) } @@ -48,10 +53,10 @@ func TestMergeFlagsIntoSecretArgs_LiteralSources(t *testing.T) { args := findOrMakeSecretArgs(k, "foo", "bar", "forbidden") mergeFlagsIntoGeneratorArgs( &args.GeneratorArgs, - flagsAndArgs{LiteralSources: []string{"k1=v1"}}) + configmapSecretFlagsAndArgs{LiteralSources: []string{"k1=v1"}}) mergeFlagsIntoGeneratorArgs( &args.GeneratorArgs, - flagsAndArgs{LiteralSources: []string{"k2=v2"}}) + configmapSecretFlagsAndArgs{LiteralSources: []string{"k2=v2"}}) assert.Equal(t, "k1=v1", k.SecretGenerator[0].LiteralSources[0]) assert.Equal(t, "k2=v2", k.SecretGenerator[0].LiteralSources[1]) } @@ -61,10 +66,10 @@ func TestMergeFlagsIntoSecretArgs_FileSources(t *testing.T) { args := findOrMakeSecretArgs(k, "foo", "bar", "forbidden") mergeFlagsIntoGeneratorArgs( &args.GeneratorArgs, - flagsAndArgs{FileSources: []string{"file1"}}) + configmapSecretFlagsAndArgs{FileSources: []string{"file1"}}) mergeFlagsIntoGeneratorArgs( &args.GeneratorArgs, - flagsAndArgs{FileSources: []string{"file2"}}) + configmapSecretFlagsAndArgs{FileSources: []string{"file2"}}) assert.Equal(t, "file1", k.SecretGenerator[0].FileSources[0]) assert.Equal(t, "file2", k.SecretGenerator[0].FileSources[1]) } @@ -74,10 +79,10 @@ func TestMergeFlagsIntoSecretArgs_EnvSource(t *testing.T) { args := findOrMakeSecretArgs(k, "foo", "bar", "forbidden") mergeFlagsIntoGeneratorArgs( &args.GeneratorArgs, - flagsAndArgs{EnvFileSource: "env1"}) + configmapSecretFlagsAndArgs{EnvFileSource: "env1"}) mergeFlagsIntoGeneratorArgs( &args.GeneratorArgs, - flagsAndArgs{EnvFileSource: "env2"}) + configmapSecretFlagsAndArgs{EnvFileSource: "env2"}) assert.Equal(t, "env1", k.SecretGenerator[0].EnvSources[0]) assert.Equal(t, "env2", k.SecretGenerator[0].EnvSources[1]) } @@ -87,6 +92,147 @@ func TestMergeFlagsIntoSecretArgs_DisableNameSuffixHash(t *testing.T) { args := findOrMakeSecretArgs(k, "foo", "bar", "forbidden") mergeFlagsIntoGeneratorArgs( &args.GeneratorArgs, - flagsAndArgs{DisableNameSuffixHash: true}) + configmapSecretFlagsAndArgs{DisableNameSuffixHash: true}) assert.True(t, k.SecretGenerator[0].Options.DisableNameSuffixHash) } + +// TestEditAddSecretWithLiteralSource executes the same command flow as the CLI invocation +// with a --from-literal flag +func TestEditAddSecretWithLiteralSource(t *testing.T) { + const ( + secretName = "test-kustomization" + literalSource = "test-key=test-value" + ) + + fSys := filesys.MakeEmptyDirInMemory() + testutils_test.WriteTestKustomization(fSys) + + pvd := provider.NewDefaultDepProvider() + ldr := kv.NewLoader(loader.NewFileLoaderAtCwd(fSys), pvd.GetFieldValidator()) + + args := []string{ + secretName, + fmt.Sprintf(flagFormat, fromLiteralFlag, literalSource), + } + cmd := newCmdAddSecret(fSys, ldr, pvd.GetResourceFactory()) + cmd.SetArgs(args) + require.NoError(t, cmd.Execute()) + + _, 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.SecretGenerator) + require.Equal(t, 1, len(kustomization.SecretGenerator)) + + newSecretGenerator := kustomization.SecretGenerator[0] + require.NotNil(t, newSecretGenerator) + require.Equal(t, secretName, newSecretGenerator.Name) + require.Contains(t, newSecretGenerator.LiteralSources, literalSource) +} + +// TestEditAddSecretWithEnvSource executes the same command flow as the CLI invocation +// with a --from-env-file flag +func TestEditAddSecretWithEnvSource(t *testing.T) { + const ( + secretName = "test-kustomization" + envSource = "test-env-source" + ) + + fSys := filesys.MakeEmptyDirInMemory() + testutils_test.WriteTestKustomization(fSys) + + pvd := provider.NewDefaultDepProvider() + ldr := kv.NewLoader(loader.NewFileLoaderAtCwd(fSys), pvd.GetFieldValidator()) + + envFileContent, err := fSys.Create("test-env-source") + require.NoError(t, err) + + _, err = envFileContent.Write([]byte("TEST=value")) + require.NoError(t, err) + + err = envFileContent.Close() + require.NoError(t, err) + + args := []string{ + secretName, + fmt.Sprintf(flagFormat, fromEnvFileFlag, envSource), + } + cmd := newCmdAddSecret(fSys, ldr, pvd.GetResourceFactory()) + cmd.SetArgs(args) + require.NoError(t, cmd.Execute()) + + _, 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.SecretGenerator) + require.Equal(t, 1, len(kustomization.SecretGenerator)) + + newSecretGenerator := kustomization.SecretGenerator[0] + require.NotNil(t, newSecretGenerator) + require.Equal(t, secretName, newSecretGenerator.Name) + require.Contains(t, newSecretGenerator.EnvSources, envSource) +} + +// TestEditAddSecretWithFileSource executes the same command flow as the CLI invocation +// with a --from-file flag +func TestEditAddSecretWithFileSource(t *testing.T) { + const ( + secretName = "test-kustomization" + fileSource = "test-file-source" + ) + + fSys := filesys.MakeEmptyDirInMemory() + testutils_test.WriteTestKustomization(fSys) + + pvd := provider.NewDefaultDepProvider() + ldr := kv.NewLoader(loader.NewFileLoaderAtCwd(fSys), pvd.GetFieldValidator()) + + fileContent, err := fSys.Create("test-file-source") + require.NoError(t, err) + + _, err = fileContent.Write([]byte("any content here")) + require.NoError(t, err) + + err = fileContent.Close() + require.NoError(t, err) + + args := []string{ + secretName, + fmt.Sprintf(flagFormat, fromFileFlag, fileSource), + } + cmd := newCmdAddSecret(fSys, ldr, pvd.GetResourceFactory()) + cmd.SetArgs(args) + require.NoError(t, cmd.Execute()) + + _, 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.SecretGenerator) + require.Equal(t, 1, len(kustomization.SecretGenerator)) + + newSecretGenerator := kustomization.SecretGenerator[0] + require.NotNil(t, newSecretGenerator) + require.Equal(t, secretName, newSecretGenerator.Name) + require.Contains(t, newSecretGenerator.FileSources, fileSource) +}