From dd807d62c5b18b7c1c99fc3c37ee3f4315d8154f Mon Sep 17 00:00:00 2001 From: adamarnold-msm Date: Mon, 1 Nov 2021 10:45:53 +0000 Subject: [PATCH 1/3] Add -spec flag to support specification file consisting of multiple desired state files state files, add priority override --- docs/cmd_reference.md | 3 ++ examples/example-spec.yaml | 6 ++++ internal/app/cli.go | 62 +++++++++++++++++++++++++++++---- internal/app/cli_test.go | 10 +++--- internal/app/spec_state.go | 52 +++++++++++++++++++++++++++ internal/app/spec_state_test.go | 50 ++++++++++++++++++++++++++ tests/Invalid_example_spec.yaml | 4 +++ 7 files changed, 175 insertions(+), 12 deletions(-) create mode 100644 examples/example-spec.yaml create mode 100644 internal/app/spec_state.go create mode 100644 internal/app/spec_state_test.go create mode 100644 tests/Invalid_example_spec.yaml diff --git a/docs/cmd_reference.md b/docs/cmd_reference.md index 6660398c..e12a7e09 100644 --- a/docs/cmd_reference.md +++ b/docs/cmd_reference.md @@ -77,6 +77,9 @@ This lists available CMD options in Helmsman: `--no-ssm-subst` turn off SSM parameter substitution globally. + `-spec string` + specification file name, contains locations of desired state files to be merged + `--subst-ssm-values` turn on SSM parameter substitution in values files. diff --git a/examples/example-spec.yaml b/examples/example-spec.yaml new file mode 100644 index 00000000..d4590476 --- /dev/null +++ b/examples/example-spec.yaml @@ -0,0 +1,6 @@ +--- +stateFiles: + - path: examples/example.yaml + priority: -10 + - path: examples/minimal-example.yaml + - path: examples/minimal-example.toml diff --git a/internal/app/cli.go b/internal/app/cli.go index eb7cff84..a455ce45 100644 --- a/internal/app/cli.go +++ b/internal/app/cli.go @@ -24,6 +24,29 @@ const ( // Allow parsing of multiple string command line options into an array of strings type stringArray []string +type fileOptionArray []fileOption + +type fileOption struct { + name string + priority int +} + +func (f *fileOptionArray) String() string { + var a []string + for _, v := range *f { + a = append(a, v.name) + } + return strings.Join(a, " ") +} + +func (f *fileOptionArray) Set(value string) error { + var fo fileOption + + fo.name = value + *f = append(*f, fo) + return nil +} + func (i *stringArray) String() string { return strings.Join(*i, " ") } @@ -35,7 +58,8 @@ func (i *stringArray) Set(value string) error { type cli struct { debug bool - files stringArray + files fileOptionArray + spec string envFiles stringArray target stringArray group stringArray @@ -88,6 +112,7 @@ func (c *cli) parse() { flag.Var(&c.group, "group", "limit execution to specific group of apps.") flag.IntVar(&c.diffContext, "diff-context", -1, "number of lines of context to show around changes in helm diff output") flag.IntVar(&c.parallel, "p", 1, "max number of concurrent helm releases to run") + flag.StringVar(&c.spec, "spec", "", "specification file name, contains locations of desired state files to be merged") flag.StringVar(&c.kubeconfig, "kubeconfig", "", "path to the kubeconfig file to use for CLI requests") flag.StringVar(&c.nsOverride, "ns-override", "", "override defined namespaces with this one") flag.StringVar(&c.contextOverride, "context-override", "", "override releases context defined in release state with this one") @@ -147,6 +172,10 @@ func (c *cli) parse() { log.Fatal("--target and --group can't be used together.") } + if len(flags.files) > 0 && len(flags.spec) > 0 { + log.Fatal("-f and -spec can't be used together.") + } + if c.parallel < 1 { c.parallel = 1 } @@ -159,7 +188,7 @@ func (c *cli) parse() { kubectlVersion := getKubectlVersion() log.Verbose("kubectl client version: " + kubectlVersion) - if len(c.files) == 0 { + if len(c.files) == 0 && len(c.spec) == 0 { log.Info("No desired state files provided.") os.Exit(0) } @@ -218,32 +247,51 @@ func (c *cli) readState(s *state) error { os.RemoveAll(tempFilesDir) _ = os.MkdirAll(tempFilesDir, 0o755) + if len(c.spec) > 0 { + + sp := new(StateFiles) + sp.specFromYAML(c.spec) + + for _, val := range sp.StateFiles { + fo := fileOption{} + fo.name = val.Path + fo.priority = val.Priority + checkSpecValid(fo.name) + c.files = append(c.files, fo) + } + } + // read the TOML/YAML desired state file for _, f := range c.files { var fileState state - if err := fileState.fromFile(f); err != nil { + if err := fileState.fromFile(f.name); err != nil { return err } - log.Infof("Parsed [[ %s ]] successfully and found [ %d ] apps", f, len(fileState.Apps)) + + if f.priority != 0 { + fileState.patchPriority(f.priority) + } + + log.Infof("Parsed [[ %s ]] successfully and found [ %d ] apps", f.name, len(fileState.Apps)) // Merge Apps that already existed in the state for appName, app := range fileState.Apps { if _, ok := s.Apps[appName]; ok { if err := mergo.Merge(s.Apps[appName], app, mergo.WithAppendSlice, mergo.WithOverride); err != nil { - return fmt.Errorf("failed to merge %s from desired state file %s: %w", appName, f, err) + return fmt.Errorf("failed to merge %s from desired state file %s: %w", appName, f.name, err) } } } // Merge the remaining Apps if err := mergo.Merge(&s.Apps, &fileState.Apps); err != nil { - return fmt.Errorf("failed to merge desired state file %s: %w", f, err) + return fmt.Errorf("failed to merge desired state file %s: %w", f.name, err) } // All the apps are already merged, make fileState.Apps empty to avoid conflicts in the final merge fileState.Apps = make(map[string]*release) if err := mergo.Merge(s, &fileState, mergo.WithAppendSlice, mergo.WithOverride); err != nil { - return fmt.Errorf("failed to merge desired state file %s: %w", f, err) + return fmt.Errorf("failed to merge desired state file %s: %w", f.name, err) } } diff --git a/internal/app/cli_test.go b/internal/app/cli_test.go index 79680b1f..9b34399f 100644 --- a/internal/app/cli_test.go +++ b/internal/app/cli_test.go @@ -22,7 +22,7 @@ func Test_readState(t *testing.T) { { name: "yaml minimal example; no validation", flags: cli{ - files: stringArray([]string{"../../examples/minimal-example.yaml"}), + files: fileOptionArray([]fileOption{fileOption{"../../examples/minimal-example.yaml", 0}}), skipValidation: true, }, want: result{ @@ -35,7 +35,7 @@ func Test_readState(t *testing.T) { { name: "toml minimal example; no validation", flags: cli{ - files: stringArray([]string{"../../examples/minimal-example.toml"}), + files: fileOptionArray([]fileOption{fileOption{"../../examples/minimal-example.toml", 0}}), skipValidation: true, }, want: result{ @@ -49,7 +49,7 @@ func Test_readState(t *testing.T) { name: "yaml minimal example; no validation with bad target", flags: cli{ target: stringArray([]string{"foo"}), - files: stringArray([]string{"../../examples/minimal-example.yaml"}), + files: fileOptionArray([]fileOption{{"../../examples/minimal-example.yaml", 0}}), skipValidation: true, }, want: result{ @@ -63,7 +63,7 @@ func Test_readState(t *testing.T) { name: "yaml minimal example; no validation; target jenkins", flags: cli{ target: stringArray([]string{"jenkins"}), - files: stringArray([]string{"../../examples/minimal-example.yaml"}), + files: fileOptionArray([]fileOption{{"../../examples/minimal-example.yaml", 0}}), skipValidation: true, }, want: result{ @@ -76,7 +76,7 @@ func Test_readState(t *testing.T) { { name: "yaml and toml minimal examples merged; no validation", flags: cli{ - files: stringArray([]string{"../../examples/minimal-example.yaml", "../../examples/minimal-example.toml"}), + files: fileOptionArray([]fileOption{{"../../examples/minimal-example.yaml", 0}, {"../../examples/minimal-example.toml", 0}}), skipValidation: true, }, want: result{ diff --git a/internal/app/spec_state.go b/internal/app/spec_state.go new file mode 100644 index 00000000..d926c296 --- /dev/null +++ b/internal/app/spec_state.go @@ -0,0 +1,52 @@ +package app + +import ( + "io/ioutil" + + "gopkg.in/yaml.v2" +) + +type SpecConfig struct { + Path string `yaml:"path"` + Priority int `yaml:"priority"` +} + +type StateFiles struct { + StateFiles []SpecConfig `yaml:"stateFiles"` +} + +// fromYAML reads a yaml file and decodes it to a state type. +// parser which throws an error if the YAML file is not valid. +func (pc *StateFiles) specFromYAML(file string) error { + rawYamlFile, err := ioutil.ReadFile(file) + if err != nil { + log.Errorf("specFromYaml %v %v", file, err) + return err + } + + yamlFile := string(rawYamlFile) + + if err = yaml.UnmarshalStrict([]byte(yamlFile), pc); err != nil { + return err + } + + return nil +} + +func checkSpecValid(f string) error { + if err := isValidFile(f, []string{"yaml", "yml"}); err != nil { + // exit + return err + } + + return nil +} + +func (s *state) patchPriority(priority int) error { + for app, _ := range s.Apps { + if s.Apps[app].Priority > priority { + s.Apps[app].Priority = priority + } + } + return nil +} diff --git a/internal/app/spec_state_test.go b/internal/app/spec_state_test.go new file mode 100644 index 00000000..21a2df1a --- /dev/null +++ b/internal/app/spec_state_test.go @@ -0,0 +1,50 @@ +package app + +import ( + "testing" +) + +func Test_specFromYAML(t *testing.T) { + type args struct { + file string + s *StateFiles + } + tests := []struct { + name string + args args + want bool + }{ + { + name: "test case 1 -- Valid YAML", + args: args{ + file: "../../examples/example-spec.yaml", + s: new(StateFiles), + }, + want: true, + }, { + name: "test case 2 -- Invalid Yaml", + args: args{ + file: "../../tests/Invalid_example_spec.yaml", + s: new(StateFiles), + }, + want: false, + }, + } + + teardownTestCase := setupStateFileTestCase(t) + defer teardownTestCase(t) + for _, tt := range tests { + // os.Args = append(os.Args, "-f ../../examples/example.yaml") + t.Run(tt.name, func(t *testing.T) { + err := tt.args.s.specFromYAML(tt.args.file) + if err != nil { + t.Log(err) + } + + got := err == nil + if got != tt.want { + t.Errorf("specFromYaml() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/tests/Invalid_example_spec.yaml b/tests/Invalid_example_spec.yaml new file mode 100644 index 00000000..19ccd2c1 --- /dev/null +++ b/tests/Invalid_example_spec.yaml @@ -0,0 +1,4 @@ +--- +stateFiles: + name1: invalid/example.yaml + name2: invalid/minimal-example.yaml From e652370613298ce4993425921ea3d52ebaec8932 Mon Sep 17 00:00:00 2001 From: Mateusz Kubaczyk <12384536+mkubaczyk@users.noreply.github.com> Date: Mon, 6 Dec 2021 12:52:18 +0100 Subject: [PATCH 2/3] Add ordering of files in -spec file before patching the priorities of apps --- internal/app/cli.go | 14 ++++++++++- internal/app/spec_state.go | 9 -------- internal/app/spec_state_test.go | 41 +++++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 10 deletions(-) diff --git a/internal/app/cli.go b/internal/app/cli.go index a455ce45..7034b52e 100644 --- a/internal/app/cli.go +++ b/internal/app/cli.go @@ -4,6 +4,7 @@ import ( "flag" "fmt" "os" + "sort" "strings" "github.com/imdario/mergo" @@ -47,6 +48,14 @@ func (f *fileOptionArray) Set(value string) error { return nil } +func (f fileOptionArray) sort() { + log.Verbose("Sorting files listed in the -spec file based on their priorities... ") + + sort.SliceStable(f, func(i, j int) bool { + return (f)[i].priority < (f)[j].priority + }) +} + func (i *stringArray) String() string { return strings.Join(*i, " ") } @@ -256,9 +265,12 @@ func (c *cli) readState(s *state) error { fo := fileOption{} fo.name = val.Path fo.priority = val.Priority - checkSpecValid(fo.name) + if err := isValidFile(fo.name, validManifestFiles); err != nil { + return fmt.Errorf("invalid -spec file: %w", err) + } c.files = append(c.files, fo) } + c.files.sort() } // read the TOML/YAML desired state file diff --git a/internal/app/spec_state.go b/internal/app/spec_state.go index d926c296..d7dee36a 100644 --- a/internal/app/spec_state.go +++ b/internal/app/spec_state.go @@ -33,15 +33,6 @@ func (pc *StateFiles) specFromYAML(file string) error { return nil } -func checkSpecValid(f string) error { - if err := isValidFile(f, []string{"yaml", "yml"}); err != nil { - // exit - return err - } - - return nil -} - func (s *state) patchPriority(priority int) error { for app, _ := range s.Apps { if s.Apps[app].Priority > priority { diff --git a/internal/app/spec_state_test.go b/internal/app/spec_state_test.go index 21a2df1a..e3b5f66c 100644 --- a/internal/app/spec_state_test.go +++ b/internal/app/spec_state_test.go @@ -48,3 +48,44 @@ func Test_specFromYAML(t *testing.T) { }) } } + +func Test_specFileSort(t *testing.T) { + type args struct { + files fileOptionArray + } + tests := []struct { + name string + args args + want [3]int + }{ + { + name: "test case 1 -- Files sorted by priority", + args: args{ + files: fileOptionArray( + []fileOption{ + fileOption{"third.yaml", 0}, + fileOption{"first.yaml", -20}, + fileOption{"second.yaml", -10}, + }), + }, + want: [3]int{-20, -10, 0}, + }, + } + + teardownTestCase := setupStateFileTestCase(t) + defer teardownTestCase(t) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tt.args.files.sort() + + got := [3]int{} + for i, f := range tt.args.files { + got[i] = f.priority + } + if got != tt.want { + t.Errorf("files from spec file are not sorted by priority = %v, want %v", got, tt.want) + } + }) + } +} + From a9b0ff85271e44f4b3fba30fa3d63ab52a44ad3c Mon Sep 17 00:00:00 2001 From: adamarnold-msm Date: Wed, 8 Dec 2021 19:24:45 +0000 Subject: [PATCH 3/3] Remove patchPriority as now sorting on priority --- examples/example-spec.yaml | 1 - internal/app/cli.go | 5 ----- internal/app/spec_state.go | 16 +++------------- internal/app/spec_state_test.go | 3 +-- 4 files changed, 4 insertions(+), 21 deletions(-) diff --git a/examples/example-spec.yaml b/examples/example-spec.yaml index d4590476..14757ed1 100644 --- a/examples/example-spec.yaml +++ b/examples/example-spec.yaml @@ -1,6 +1,5 @@ --- stateFiles: - path: examples/example.yaml - priority: -10 - path: examples/minimal-example.yaml - path: examples/minimal-example.toml diff --git a/internal/app/cli.go b/internal/app/cli.go index 7034b52e..99a3f09c 100644 --- a/internal/app/cli.go +++ b/internal/app/cli.go @@ -264,7 +264,6 @@ func (c *cli) readState(s *state) error { for _, val := range sp.StateFiles { fo := fileOption{} fo.name = val.Path - fo.priority = val.Priority if err := isValidFile(fo.name, validManifestFiles); err != nil { return fmt.Errorf("invalid -spec file: %w", err) } @@ -281,10 +280,6 @@ func (c *cli) readState(s *state) error { return err } - if f.priority != 0 { - fileState.patchPriority(f.priority) - } - log.Infof("Parsed [[ %s ]] successfully and found [ %d ] apps", f.name, len(fileState.Apps)) // Merge Apps that already existed in the state for appName, app := range fileState.Apps { diff --git a/internal/app/spec_state.go b/internal/app/spec_state.go index d7dee36a..3a7b5144 100644 --- a/internal/app/spec_state.go +++ b/internal/app/spec_state.go @@ -6,13 +6,12 @@ import ( "gopkg.in/yaml.v2" ) -type SpecConfig struct { - Path string `yaml:"path"` - Priority int `yaml:"priority"` +type StatePath struct { + Path string `yaml:"path"` } type StateFiles struct { - StateFiles []SpecConfig `yaml:"stateFiles"` + StateFiles []StatePath `yaml:"stateFiles"` } // fromYAML reads a yaml file and decodes it to a state type. @@ -32,12 +31,3 @@ func (pc *StateFiles) specFromYAML(file string) error { return nil } - -func (s *state) patchPriority(priority int) error { - for app, _ := range s.Apps { - if s.Apps[app].Priority > priority { - s.Apps[app].Priority = priority - } - } - return nil -} diff --git a/internal/app/spec_state_test.go b/internal/app/spec_state_test.go index e3b5f66c..dc209414 100644 --- a/internal/app/spec_state_test.go +++ b/internal/app/spec_state_test.go @@ -51,7 +51,7 @@ func Test_specFromYAML(t *testing.T) { func Test_specFileSort(t *testing.T) { type args struct { - files fileOptionArray + files fileOptionArray } tests := []struct { name string @@ -88,4 +88,3 @@ func Test_specFileSort(t *testing.T) { }) } } -