Skip to content

Commit fb7ee2f

Browse files
stormqueen1990stormqueen1990
authored
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.
1 parent 45e57f0 commit fb7ee2f

File tree

6 files changed

+479
-162
lines changed

6 files changed

+479
-162
lines changed

‎kustomize/commands/edit/add/configmap.go

Lines changed: 61 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
package add
55

66
import (
7+
"fmt"
8+
79
"github.com/spf13/cobra"
810
"sigs.k8s.io/kustomize/api/ifc"
911
"sigs.k8s.io/kustomize/api/resource"
@@ -16,8 +18,9 @@ import (
1618
func newCmdAddConfigMap(
1719
fSys filesys.FileSystem,
1820
ldr ifc.KvLoader,
19-
rf *resource.Factory) *cobra.Command {
20-
var flags flagsAndArgs
21+
rf *resource.Factory,
22+
) *cobra.Command {
23+
var flags configmapSecretFlagsAndArgs
2124
cmd := &cobra.Command{
2225
Use: "configmap NAME [--behavior={create|merge|replace}] [--from-file=[key=]source] [--from-literal=key1=value1]",
2326
Short: "Adds a configmap to the kustomization file",
@@ -36,63 +39,35 @@ func newCmdAddConfigMap(
3639
kustomize edit add configmap my-configmap --behavior=merge --from-env-file=env/path.env
3740
`,
3841
RunE: func(_ *cobra.Command, args []string) error {
39-
err := flags.ExpandFileSource(fSys)
40-
if err != nil {
41-
return err
42-
}
43-
44-
err = flags.Validate(args)
45-
if err != nil {
46-
return err
47-
}
48-
49-
// Load the kustomization file.
50-
mf, err := kustfile.NewKustomizationFile(fSys)
51-
if err != nil {
52-
return err
53-
}
54-
55-
kustomization, err := mf.Read()
56-
if err != nil {
57-
return err
58-
}
59-
60-
// Add the flagsAndArgs map to the kustomization file.
61-
err = addConfigMap(ldr, kustomization, flags, rf)
62-
if err != nil {
63-
return err
64-
}
65-
66-
// Write out the kustomization file with added configmap.
67-
return mf.Write(kustomization)
42+
return runEditAddConfigMap(flags, fSys, args, ldr, rf)
6843
},
6944
}
7045

7146
cmd.Flags().StringSliceVar(
7247
&flags.FileSources,
73-
"from-file",
48+
fromFileFlag,
7449
[]string{},
7550
"Key file can be specified using its file path, in which case file basename will be used as configmap "+
7651
"key, or optionally with a key and file path, in which case the given key will be used. Specifying a "+
7752
"directory will iterate each named file in the directory whose basename is a valid configmap key.")
7853
cmd.Flags().StringArrayVar(
7954
&flags.LiteralSources,
80-
"from-literal",
55+
fromLiteralFlag,
8156
[]string{},
8257
"Specify a key and literal value to insert in configmap (i.e. mykey=somevalue)")
8358
cmd.Flags().StringVar(
8459
&flags.EnvFileSource,
85-
"from-env-file",
60+
fromEnvFileFlag,
8661
"",
8762
"Specify the path to a file to read lines of key=val pairs to create a configmap (i.e. a Docker .env file).")
8863
cmd.Flags().BoolVar(
8964
&flags.DisableNameSuffixHash,
90-
"disableNameSuffixHash",
65+
flagDisableNameSuffixHash,
9166
false,
9267
"Disable the name suffix for the configmap")
9368
cmd.Flags().StringVar(
9469
&flags.Behavior,
95-
"behavior",
70+
flagBehavior,
9671
"",
9772
"Specify the behavior for config map generation, i.e whether to create a new configmap (the default), "+
9873
"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(
10176
return cmd
10277
}
10378

79+
func runEditAddConfigMap(
80+
flags configmapSecretFlagsAndArgs,
81+
fSys filesys.FileSystem,
82+
args []string,
83+
ldr ifc.KvLoader,
84+
rf *resource.Factory,
85+
) error {
86+
err := flags.ExpandFileSource(fSys)
87+
if err != nil {
88+
return fmt.Errorf("failed to expand file source: %w", err)
89+
}
90+
91+
err = flags.Validate(args)
92+
if err != nil {
93+
return fmt.Errorf("failed to validate flags: %w", err)
94+
}
95+
96+
// Load the kustomization file.
97+
mf, err := kustfile.NewKustomizationFile(fSys)
98+
if err != nil {
99+
return fmt.Errorf("failed to load kustomization file: %w", err)
100+
}
101+
102+
kustomization, err := mf.Read()
103+
if err != nil {
104+
return fmt.Errorf("failed to read kustomization file: %w", err)
105+
}
106+
107+
// Add the configmapSecretFlagsAndArgs map to the kustomization file.
108+
err = addConfigMap(ldr, kustomization, flags, rf)
109+
if err != nil {
110+
return fmt.Errorf("failed to create configmap: %w", err)
111+
}
112+
113+
// Write out the kustomization file with added configmap.
114+
err = mf.Write(kustomization)
115+
if err != nil {
116+
return fmt.Errorf("failed to write kustomization file: %w", err)
117+
}
118+
119+
return nil
120+
}
121+
104122
// addConfigMap adds a configmap to a kustomization file.
105123
// Note: error may leave kustomization file in an undefined state.
106124
// Suggest passing a copy of kustomization file.
107125
func addConfigMap(
108126
ldr ifc.KvLoader,
109127
k *types.Kustomization,
110-
flags flagsAndArgs, rf *resource.Factory) error {
128+
flags configmapSecretFlagsAndArgs,
129+
rf *resource.Factory,
130+
) error {
111131
args := findOrMakeConfigMapArgs(k, flags.Name)
112-
mergeFlagsIntoCmArgs(args, flags)
132+
mergeFlagsIntoGeneratorArgs(&args.GeneratorArgs, flags)
113133
// Validate by trying to create corev1.configmap.
114134
args.Options = types.MergeGlobalOptionsIntoLocal(
115135
args.Options, k.GeneratorOptions)
@@ -124,30 +144,9 @@ func findOrMakeConfigMapArgs(m *types.Kustomization, name string) *types.ConfigM
124144
}
125145
}
126146
// config map not found, create new one and add it to the kustomization file.
127-
cm := &types.ConfigMapArgs{GeneratorArgs: types.GeneratorArgs{Name: name}}
147+
cm := &types.ConfigMapArgs{
148+
GeneratorArgs: types.GeneratorArgs{Name: name},
149+
}
128150
m.ConfigMapGenerator = append(m.ConfigMapGenerator, *cm)
129151
return &m.ConfigMapGenerator[len(m.ConfigMapGenerator)-1]
130152
}
131-
132-
func mergeFlagsIntoCmArgs(args *types.ConfigMapArgs, flags flagsAndArgs) {
133-
if len(flags.LiteralSources) > 0 {
134-
args.LiteralSources = append(
135-
args.LiteralSources, flags.LiteralSources...)
136-
}
137-
if len(flags.FileSources) > 0 {
138-
args.FileSources = append(
139-
args.FileSources, flags.FileSources...)
140-
}
141-
if flags.EnvFileSource != "" {
142-
args.EnvSources = append(
143-
args.EnvSources, flags.EnvFileSource)
144-
}
145-
if flags.DisableNameSuffixHash {
146-
args.Options = &types.GeneratorOptions{
147-
DisableNameSuffixHash: true,
148-
}
149-
}
150-
if flags.Behavior != "" {
151-
args.Behavior = flags.Behavior
152-
}
153-
}

‎kustomize/commands/edit/add/flagsandargs.go renamed to ‎kustomize/commands/edit/add/configmapSecretFlagsAndArgs.go

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,17 @@ import (
1313
"sigs.k8s.io/kustomize/kyaml/filesys"
1414
)
1515

16-
// flagsAndArgs encapsulates the options for add secret/configmap commands.
17-
type flagsAndArgs struct {
16+
const (
17+
fromFileFlag = "from-file"
18+
fromLiteralFlag = "from-literal"
19+
fromEnvFileFlag = "from-env-file"
20+
flagDisableNameSuffixHash = "disableNameSuffixHash"
21+
flagBehavior = "behavior"
22+
flagFormat = "--%s=%s"
23+
)
24+
25+
// configmapSecretFlagsAndArgs encapsulates the options for add secret/configmap commands.
26+
type configmapSecretFlagsAndArgs struct {
1827
// Name of configMap/Secret (required)
1928
Name string
2029
// FileSources to derive the configMap/Secret from (optional)
@@ -35,7 +44,7 @@ type flagsAndArgs struct {
3544
}
3645

3746
// Validate validates required fields are set to support structured generation.
38-
func (a *flagsAndArgs) Validate(args []string) error {
47+
func (a *configmapSecretFlagsAndArgs) Validate(args []string) error {
3948
if len(args) != 1 {
4049
return fmt.Errorf("name must be specified once")
4150
}
@@ -71,7 +80,7 @@ func (a *flagsAndArgs) Validate(args []string) error {
7180
// and the key, if missing, is the same as the value.
7281
// In the case where the key is explicitly declared,
7382
// the globbing, if present, must have exactly one match.
74-
func (a *flagsAndArgs) ExpandFileSource(fSys filesys.FileSystem) error {
83+
func (a *configmapSecretFlagsAndArgs) ExpandFileSource(fSys filesys.FileSystem) error {
7584
var results []string
7685
for _, pattern := range a.FileSources {
7786
var patterns []string
@@ -105,3 +114,26 @@ func (a *flagsAndArgs) ExpandFileSource(fSys filesys.FileSystem) error {
105114
a.FileSources = results
106115
return nil
107116
}
117+
118+
func mergeFlagsIntoGeneratorArgs(args *types.GeneratorArgs, flags configmapSecretFlagsAndArgs) {
119+
if len(flags.LiteralSources) > 0 {
120+
args.LiteralSources = append(
121+
args.LiteralSources, flags.LiteralSources...)
122+
}
123+
if len(flags.FileSources) > 0 {
124+
args.FileSources = append(
125+
args.FileSources, flags.FileSources...)
126+
}
127+
if flags.EnvFileSource != "" {
128+
args.EnvSources = append(
129+
args.EnvSources, flags.EnvFileSource)
130+
}
131+
if flags.DisableNameSuffixHash {
132+
args.Options = &types.GeneratorOptions{
133+
DisableNameSuffixHash: true,
134+
}
135+
}
136+
if flags.Behavior != "" {
137+
args.Behavior = flags.Behavior
138+
}
139+
}

‎kustomize/commands/edit/add/flagsandargs_test.go renamed to ‎kustomize/commands/edit/add/configmapSecretFlagsAndArgs_test.go

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7,68 +7,67 @@ import (
77
"reflect"
88
"testing"
99

10-
"github.com/stretchr/testify/assert"
1110
"github.com/stretchr/testify/require"
1211
"sigs.k8s.io/kustomize/kyaml/filesys"
1312
)
1413

1514
func TestDataValidation_NoName(t *testing.T) {
16-
fa := flagsAndArgs{}
17-
assert.Error(t, fa.Validate([]string{}))
15+
fa := configmapSecretFlagsAndArgs{}
16+
require.Error(t, fa.Validate([]string{}))
1817
}
1918

2019
func TestDataValidation_MoreThanOneName(t *testing.T) {
21-
fa := flagsAndArgs{}
20+
fa := configmapSecretFlagsAndArgs{}
2221

23-
assert.Error(t, fa.Validate([]string{"name", "othername"}))
22+
require.Error(t, fa.Validate([]string{"name", "othername"}))
2423
}
2524

2625
func TestDataConfigValidation_Flags(t *testing.T) {
2726
tests := []struct {
2827
name string
29-
fa flagsAndArgs
28+
fa configmapSecretFlagsAndArgs
3029
shouldFail bool
3130
}{
3231
{
3332
name: "env-file-source and literal are both set",
34-
fa: flagsAndArgs{
33+
fa: configmapSecretFlagsAndArgs{
3534
LiteralSources: []string{"one", "two"},
3635
EnvFileSource: "three",
3736
},
3837
shouldFail: true,
3938
},
4039
{
4140
name: "env-file-source and from-file are both set",
42-
fa: flagsAndArgs{
41+
fa: configmapSecretFlagsAndArgs{
4342
FileSources: []string{"one", "two"},
4443
EnvFileSource: "three",
4544
},
4645
shouldFail: true,
4746
},
4847
{
4948
name: "we don't have any option set",
50-
fa: flagsAndArgs{},
49+
fa: configmapSecretFlagsAndArgs{},
5150
shouldFail: true,
5251
},
5352
{
5453
name: "we have from-file and literal ",
55-
fa: flagsAndArgs{
54+
fa: configmapSecretFlagsAndArgs{
5655
LiteralSources: []string{"one", "two"},
5756
FileSources: []string{"three", "four"},
5857
},
5958
shouldFail: false,
6059
},
6160
{
6261
name: "correct behavior",
63-
fa: flagsAndArgs{
62+
fa: configmapSecretFlagsAndArgs{
6463
EnvFileSource: "foo",
6564
Behavior: "merge",
6665
},
6766
shouldFail: false,
6867
},
6968
{
7069
name: "incorrect behavior",
71-
fa: flagsAndArgs{
70+
fa: configmapSecretFlagsAndArgs{
7271
EnvFileSource: "foo",
7372
Behavior: "merge-unknown",
7473
},
@@ -79,9 +78,9 @@ func TestDataConfigValidation_Flags(t *testing.T) {
7978
for _, test := range tests {
8079
err := test.fa.Validate([]string{"name"})
8180
if test.shouldFail {
82-
assert.Error(t, err)
81+
require.Error(t, err)
8382
} else {
84-
assert.NoError(t, err)
83+
require.NoError(t, err)
8584
}
8685
}
8786
}
@@ -94,7 +93,7 @@ func TestExpandFileSource(t *testing.T) {
9493
require.NoError(t, err)
9594
_, err = fSys.Create("dir/readme")
9695
require.NoError(t, err)
97-
fa := flagsAndArgs{
96+
fa := configmapSecretFlagsAndArgs{
9897
FileSources: []string{"dir/fa*"},
9998
}
10099
err = fa.ExpandFileSource(fSys)
@@ -118,7 +117,7 @@ func TestExpandFileSourceWithKey(t *testing.T) {
118117
require.NoError(t, err)
119118
_, err = fSys.Create("dir/readme")
120119
require.NoError(t, err)
121-
fa := flagsAndArgs{
120+
fa := configmapSecretFlagsAndArgs{
122121
FileSources: []string{"foo-key=dir/fa*", "bar-key=dir/foobar", "dir/simplebar"},
123122
}
124123
err = fa.ExpandFileSource(fSys)
@@ -141,7 +140,7 @@ func TestExpandFileSourceWithKeyAndError(t *testing.T) {
141140
require.NoError(t, err)
142141
_, err = fSys.Create("dir/readme")
143142
require.NoError(t, err)
144-
fa := flagsAndArgs{
143+
fa := configmapSecretFlagsAndArgs{
145144
FileSources: []string{"foo-key=dir/fa*"},
146145
}
147146
err = fa.ExpandFileSource(fSys)

0 commit comments

Comments
 (0)