From 68fc271e05d3bd74b3795bbcd59367b5248fda90 Mon Sep 17 00:00:00 2001 From: Steve Ramage Date: Sat, 7 Oct 2023 14:07:44 -0700 Subject: [PATCH] Resolves #386 - Fixes help screen crash and improves start up performance by deferring runbook validation to actual execution --- README.md | 11 ++-- cmd/root.go | 54 +++++++++------- cmd/root_test.go | 16 +++++ cmd/runbooks.go | 66 +++++++++++++++++++ external/resources/resources.go | 4 ++ external/runbooks/hello-world.epcc.yml | 2 - external/runbooks/run-all-runbooks.sh | 3 + external/runbooks/runbook_validation.go | 4 +- external/runbooks/runbook_validation_test.go | 67 +++++++++++++------- external/runbooks/runbooks.go | 19 +++--- external/runbooks/runbooks_test.go | 23 +------ 11 files changed, 181 insertions(+), 88 deletions(-) create mode 100644 cmd/root_test.go diff --git a/README.md b/README.md index 8f7895d..436535d 100644 --- a/README.md +++ b/README.md @@ -53,11 +53,12 @@ The following is a summary of the main commands, in general you can type `epcc h | `epcc test-json [KEY] [VAL] [KEY] [VAL] ...` | Render a JSON document based on the supplied key and value pairs | #### Power User Commands -| Command | Description | -|-----------------------------------------|--------------------------------------------------------------| -| `epcc reset-store ` | Reset the store to an initial state (on a best effort basis) | -| `epcc runbooks show ` | Show a specific runbook (script) | -| `epcc runbooks run ` | Run a specific runbook (script) | +| Command | Description | +|-----------------------------------------|----------------------------------------------------------------------------| +| `epcc reset-store ` | Reset the store to an initial state (on a best effort basis) | +| `epcc runbooks show ` | Show a specific runbook (script) | +| `epcc runbooks validate` | Validates all runbooks (built in and user supplied, outputting any errors) | +| `epcc runbooks run ` | Run a specific runbook (script) | #### Tuning Runbooks diff --git a/cmd/root.go b/cmd/root.go index 71851cb..3a2dfdf 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -61,7 +61,7 @@ var profileNameFromCommandLine = "" func InitializeCmd() { os.Args = misc.AddImplicitDoubleDash(os.Args) - if os.Args[1] == "__complete" { + if len(os.Args) > 1 && os.Args[1] == "__complete" { DisableLongOutput = true DisableExampleOutput = true } @@ -98,6 +98,7 @@ func InitializeCmd() { Logs.AddCommand(LogsList, LogsShow, LogsClear) + testJson.ResetFlags() testJson.Flags().BoolVarP(&noWrapping, "no-wrapping", "", false, "if set, we won't wrap the output the json in a data tag") testJson.Flags().BoolVarP(&compliant, "compliant", "", false, "if set, we wrap most keys in an attributes tags automatically.") @@ -115,6 +116,7 @@ func InitializeCmd() { RootCmd.PersistentFlags().Uint16VarP(&statisticsFrequency, "statistics-frequency", "", 15, "How often to print runtime statistics (0 turns them off)") RootCmd.PersistentFlags().BoolVarP(&aliases.SkipAliasProcessing, "skip-alias-processing", "", false, "if set, we don't process the response for aliases") + ResetStore.ResetFlags() ResetStore.PersistentFlags().BoolVarP(&DeleteApplicationKeys, "delete-application-keys", "", false, "if set, we delete application keys as well") aliasesCmd.AddCommand(aliasListCmd, aliasClearCmd) @@ -170,10 +172,13 @@ func AddRootPreRunFunc(f func(cmd *cobra.Command, args []string) error) { persistentPreRunFuncs = append(persistentPreRunFuncs, f) } -var RootCmd = &cobra.Command{ - Use: os.Args[0], - Short: "A command line interface for interacting with the Elastic Path Commerce Cloud API", - Long: `The EPCC CLI tool provides a powerful command line interface for interacting with the Elastic Path Commerce Cloud API. +var RootCmd = GetRootCommand() + +func GetRootCommand() *cobra.Command { + return &cobra.Command{ + Use: os.Args[0], + Short: "A command line interface for interacting with the Elastic Path Commerce Cloud API", + Long: `The EPCC CLI tool provides a powerful command line interface for interacting with the Elastic Path Commerce Cloud API. The EPCC CLI tool uses environment variables for configuration and in particular a tool like https://direnv.net/ which auto populates your shell with environment variables when you switch directories. This allows you to store a context in a folder, @@ -189,30 +194,31 @@ Environment Variables - EPCC_PROFILE - The name of the profile we will use (isolates namespace, credentials, etc...) `, - PersistentPreRunE: func(cmd *cobra.Command, args []string) error { - log.SetLevel(logger.Loglevel) - - env := config.GetEnv() - if env.EPCC_RATE_LIMIT != 0 { - rateLimit = env.EPCC_RATE_LIMIT - } - log.Debugf("Rate limit set to %d request per second, printing statistics every %d seconds ", rateLimit, statisticsFrequency) - httpclient.Initialize(rateLimit, requestTimeout, int(statisticsFrequency)) + PersistentPreRunE: func(cmd *cobra.Command, args []string) error { + log.SetLevel(logger.Loglevel) - for _, runFunc := range persistentPreRunFuncs { - err := runFunc(cmd, args) - if err != nil { - return err + env := config.GetEnv() + if env.EPCC_RATE_LIMIT != 0 { + rateLimit = env.EPCC_RATE_LIMIT + } + log.Debugf("Rate limit set to %d request per second, printing statistics every %d seconds ", rateLimit, statisticsFrequency) + httpclient.Initialize(rateLimit, requestTimeout, int(statisticsFrequency)) + + for _, runFunc := range persistentPreRunFuncs { + err := runFunc(cmd, args) + if err != nil { + return err + } } - } - version.CheckVersionChangeAndLogWarning() + version.CheckVersionChangeAndLogWarning() - return nil - }, + return nil + }, - SilenceUsage: true, - Version: fmt.Sprintf("%s (Commit %s)", version.Version, version.Commit), + SilenceUsage: true, + Version: fmt.Sprintf("%s (Commit %s)", version.Version, version.Commit), + } } func Execute() { diff --git a/cmd/root_test.go b/cmd/root_test.go new file mode 100644 index 0000000..2865307 --- /dev/null +++ b/cmd/root_test.go @@ -0,0 +1,16 @@ +package cmd + +import ( + "github.com/elasticpath/epcc-cli/external/resources" + "github.com/elasticpath/epcc-cli/external/runbooks" + "testing" +) + +func BenchmarkRootCommandBootstrap(b *testing.B) { + for i := 0; i < b.N; i++ { + runbooks.Reset() + RootCmd = GetRootCommand() + resources.PublicInit() + InitializeCmd() + } +} diff --git a/cmd/runbooks.go b/cmd/runbooks.go index 935397a..586bd3e 100644 --- a/cmd/runbooks.go +++ b/cmd/runbooks.go @@ -33,10 +33,41 @@ func initRunbookCommands() { runbookGlobalCmd.AddCommand(initRunbookShowCommands()) runbookGlobalCmd.AddCommand(initRunbookRunCommands()) runbookGlobalCmd.AddCommand(initRunbookDevCommands()) + runbookGlobalCmd.AddCommand(initRunbookValidateCommands()) } var AbortRunbookExecution = atomic.Bool{} +func initRunbookValidateCommands() *cobra.Command { + // epcc runbook validate + runbookValidateCommand := &cobra.Command{ + Use: "validate", + Short: "Validates all runbooks", + RunE: func(cmd *cobra.Command, args []string) error { + errMsg := "" + + for _, runbook := range runbooks.GetRunbooks() { + log.Debugf("Validating runbook %s", runbook.Name) + err := runbooks.ValidateRunbook(&runbook) + + if err != nil { + newErr := fmt.Errorf("validation of runbook '%s' failed: %v", runbook.Name, err) + errMsg += newErr.Error() + "\n" + } + } + + if errMsg == "" { + log.Infof("All runbooks are valid!") + return nil + } else { + return fmt.Errorf("one or more runbooks failed validation\n:%s", errMsg) + } + }, + } + + return runbookValidateCommand +} + func initRunbookShowCommands() *cobra.Command { // epcc runbook show @@ -56,6 +87,24 @@ func initRunbookShowCommands() *cobra.Command { Long: runbook.Description.Long, Short: runbook.Description.Short, } + + runbookShowRunbookCmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { + err := RootCmd.PersistentPreRunE(RootCmd, args) + + if err != nil { + return err + } + + log.Debugf("Validating runbook %s", runbook.Name) + err = runbooks.ValidateRunbook(&runbook) + + if err != nil { + return fmt.Errorf("validation of runbook '%s' failed: %v", runbook.Name, err) + } + + return nil + } + runbookShowCommand.AddCommand(runbookShowRunbookCmd) for _, runbookAction := range runbook.RunbookActions { @@ -70,6 +119,7 @@ func initRunbookShowCommands() *cobra.Command { Long: runbookAction.Description.Long, Short: runbookAction.Description.Short, RunE: func(cmd *cobra.Command, args []string) error { + for stepIdx, cmd := range runbookAction.RawCommands { templateName := fmt.Sprintf("Runbook: %s Action: %s Step: %d", runbook.Name, runbookAction.Name, stepIdx) @@ -128,6 +178,22 @@ func initRunbookRunCommands() *cobra.Command { Short: runbook.Description.Short, } + runbookRunRunbookCmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { + err := RootCmd.PersistentPreRunE(RootCmd, args) + + if err != nil { + return err + } + + err = runbooks.ValidateRunbook(&runbook) + + if err != nil { + return fmt.Errorf("validation of runbook '%s' failed: %v", runbook.Name, err) + } + + return nil + } + runbookRunCommand.AddCommand(runbookRunRunbookCmd) for _, runbookAction := range runbook.RunbookActions { diff --git a/external/resources/resources.go b/external/resources/resources.go index eefd8df..33bfa2b 100644 --- a/external/resources/resources.go +++ b/external/resources/resources.go @@ -169,6 +169,10 @@ func AppendResourceData(newResources map[string]Resource) { } func init() { + PublicInit() +} + +func PublicInit() { resourceData, err := GenerateResourceMetadataFromYaml(resourceMetaData) diff --git a/external/runbooks/hello-world.epcc.yml b/external/runbooks/hello-world.epcc.yml index 5df4f0a..bdb0a24 100644 --- a/external/runbooks/hello-world.epcc.yml +++ b/external/runbooks/hello-world.epcc.yml @@ -63,5 +63,3 @@ actions: epcc delete customer email=kelly.burns@test.example epcc delete customer email=mohamed.love@test.example epcc delete customer email=lindsey.sexton@test.example - - diff --git a/external/runbooks/run-all-runbooks.sh b/external/runbooks/run-all-runbooks.sh index ebb0376..bb3cda0 100755 --- a/external/runbooks/run-all-runbooks.sh +++ b/external/runbooks/run-all-runbooks.sh @@ -5,6 +5,9 @@ SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd ) set -e set -x +#Let's test that epcc command works after an embarrassing bug that caused it to panic :( +epcc + epcc reset-store .+ echo "Starting Misc Runbook" diff --git a/external/runbooks/runbook_validation.go b/external/runbooks/runbook_validation.go index 9158066..5506483 100644 --- a/external/runbooks/runbook_validation.go +++ b/external/runbooks/runbook_validation.go @@ -7,7 +7,7 @@ import ( "strings" ) -func validateRunbook(runbook *Runbook) error { +func ValidateRunbook(runbook *Runbook) error { if len(runbook.Name) == 0 { return fmt.Errorf("Runbook has no name") @@ -19,7 +19,7 @@ func validateRunbook(runbook *Runbook) error { for _, runbookAction := range runbook.RunbookActions { if (len(runbookAction.RawCommands)) == 0 { - return fmt.Errorf("number of commands in action %s is zero", runbookAction.Name) + return fmt.Errorf("number of commands in action '%s' is zero", runbookAction.Name) } argumentsWithDefaults := CreateMapForRunbookArgumentPointers(runbookAction) diff --git a/external/runbooks/runbook_validation_test.go b/external/runbooks/runbook_validation_test.go index cfac93c..7a1f536 100644 --- a/external/runbooks/runbook_validation_test.go +++ b/external/runbooks/runbook_validation_test.go @@ -2,6 +2,7 @@ package runbooks import ( "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "testing" ) @@ -17,7 +18,7 @@ docs: http://localhost assert.NoError(t, err, "Error should be nil") // Execute SUT - err = validateRunbook(runbook) + err = ValidateRunbook(runbook) // Verification assert.ErrorContains(t, err, "Runbook has no name") @@ -36,7 +37,7 @@ docs: http://localhost assert.NoError(t, err, "Error should be nil") // Execute SUT - err = validateRunbook(runbook) + err = ValidateRunbook(runbook) // Verification assert.ErrorContains(t, err, "number of actions is zero") @@ -58,10 +59,10 @@ actions: assert.NoError(t, err, "Error should be nil") // Execute SUT - err = validateRunbook(runbook) + err = ValidateRunbook(runbook) // Verification - assert.ErrorContains(t, err, "number of commands in action test-action is zero") + assert.ErrorContains(t, err, "number of commands in action 'test-action' is zero") } func TestThatRunbookWithActionThatDoesNotStartWithEpccFails(t *testing.T) { @@ -81,7 +82,7 @@ actions: assert.NoError(t, err, "Error should be nil") // Execute SUT - err = validateRunbook(runbook) + err = ValidateRunbook(runbook) // Verification assert.ErrorContains(t, err, "Each command needs be a recognized command") @@ -104,7 +105,7 @@ actions: assert.NoError(t, err, "Error should be nil") // Execute SUT - err = validateRunbook(runbook) + err = ValidateRunbook(runbook) // Verification assert.ErrorContains(t, err, "Each epcc command should be followed by a verb") @@ -127,7 +128,7 @@ actions: assert.NoError(t, err, "Error should be nil") // Execute SUT - err = validateRunbook(runbook) + err = ValidateRunbook(runbook) // Verification assert.NoError(t, err) @@ -150,7 +151,7 @@ actions: assert.NoError(t, err, "Error should be nil") // Execute SUT - err = validateRunbook(runbook) + err = ValidateRunbook(runbook) // Verification assert.NoError(t, err) @@ -173,7 +174,7 @@ actions: assert.NoError(t, err, "Error should be nil") // Execute SUT - err = validateRunbook(runbook) + err = ValidateRunbook(runbook) // Verification assert.NoError(t, err) @@ -196,7 +197,7 @@ actions: assert.NoError(t, err, "Error should be nil") // Execute SUT - err = validateRunbook(runbook) + err = ValidateRunbook(runbook) // Verification assert.NoError(t, err) @@ -219,7 +220,7 @@ actions: assert.NoError(t, err, "Error should be nil") // Execute SUT - err = validateRunbook(runbook) + err = ValidateRunbook(runbook) // Verification assert.NoError(t, err) @@ -242,7 +243,7 @@ actions: assert.NoError(t, err, "Error should be nil") // Execute SUT - err = validateRunbook(runbook) + err = ValidateRunbook(runbook) // Verification assert.NoError(t, err) @@ -265,7 +266,7 @@ actions: assert.NoError(t, err, "Error should be nil") // Execute SUT - err = validateRunbook(runbook) + err = ValidateRunbook(runbook) // Verification assert.ErrorContains(t, err, "Invalid argument to sleep a long time") @@ -293,7 +294,7 @@ actions: assert.NoError(t, err, "Error should be nil") // Execute SUT - err = validateRunbook(runbook) + err = ValidateRunbook(runbook) // Verification assert.NoError(t, err) @@ -323,7 +324,7 @@ actions: assert.NoError(t, err, "Error should be nil") // Execute SUT - err = validateRunbook(runbook) + err = ValidateRunbook(runbook) // Verification assert.ErrorContains(t, err, "Each command needs be a recognized command") @@ -350,7 +351,7 @@ actions: assert.NoError(t, err, "Error should be nil") // Execute SUT - err = validateRunbook(runbook) + err = ValidateRunbook(runbook) // Verification assert.NoError(t, err) @@ -377,7 +378,7 @@ actions: assert.NoError(t, err, "Error should be nil") // Execute SUT - err = validateRunbook(runbook) + err = ValidateRunbook(runbook) // Verification assert.ErrorContains(t, err, "Each command needs to have a valid verb") @@ -400,7 +401,7 @@ actions: assert.NoError(t, err, "Error should be nil") // Execute SUT - err = validateRunbook(runbook) + err = ValidateRunbook(runbook) // Verification assert.ErrorContains(t, err, "error rendering template") @@ -423,7 +424,7 @@ actions: assert.NoError(t, err, "Error should be nil") // Execute SUT - err = validateRunbook(runbook) + err = ValidateRunbook(runbook) // Verification assert.ErrorContains(t, err, "error rendering template") @@ -450,7 +451,7 @@ actions: assert.NoError(t, err, "Error should be nil") // Execute SUT - err = validateRunbook(runbook) + err = ValidateRunbook(runbook) // Verification assert.ErrorContains(t, err, "Expected closing quote") @@ -477,7 +478,7 @@ actions: assert.NoError(t, err, "Error should be nil") // Execute SUT - err = validateRunbook(runbook) + err = ValidateRunbook(runbook) // Verification assert.ErrorContains(t, err, "error processing variable time, value make-faster is not an integer") @@ -504,8 +505,30 @@ actions: assert.NoError(t, err, "Error should be nil") // Execute SUT - err = validateRunbook(runbook) + err = ValidateRunbook(runbook) // Verification assert.ErrorContains(t, err, " error processing variable time, unknown type [magic] ") } + +func TestThatInitializeBuiltInRunbooksActuallyValidate(t *testing.T) { + + // Fixture Setup + + // language=yaml + Reset() + + // Execute SUT + InitializeBuiltInRunbooks() + + runbookNames := GetRunbookNames() + + for k, v := range runbooks { + t.Run(k, func(t *testing.T) { + assert.NoError(t, ValidateRunbook(&v)) + }) + } + + // Verification + require.GreaterOrEqual(t, len(runbookNames), 1, "Expected that some runbooks should be loaded.") +} diff --git a/external/runbooks/runbooks.go b/external/runbooks/runbooks.go index d089653..32c185a 100644 --- a/external/runbooks/runbooks.go +++ b/external/runbooks/runbooks.go @@ -53,6 +53,9 @@ type RunbookDescription struct { } func init() { + Reset() +} +func Reset() { runbooks = make(map[string]Runbook) } @@ -137,20 +140,14 @@ func AddRunbookFromYaml(yaml string) error { return err } - err = validateRunbook(runbook) - if err != nil { - return err - } else { - - log.Tracef("Loaded runbook: %s, %s", runbook.Name, yaml) + log.Tracef("Loaded runbook: %s, %s", runbook.Name, yaml) - if _, ok := runbooks[runbook.Name]; ok { - log.Warnf("Runbook name %s already loaded and is being overriden", runbook.Name) - } - - runbooks[runbook.Name] = *runbook + if _, ok := runbooks[runbook.Name]; ok { + log.Warnf("Runbook name %s already loaded and is being overriden", runbook.Name) } + runbooks[runbook.Name] = *runbook + return nil } diff --git a/external/runbooks/runbooks_test.go b/external/runbooks/runbooks_test.go index fc50638..1ce7d71 100644 --- a/external/runbooks/runbooks_test.go +++ b/external/runbooks/runbooks_test.go @@ -75,33 +75,12 @@ actions: require.Equal(t, []string{"test"}, runbookNames) } -func TestThatAddRunbookWithNameAndInvalidYamlDoesNotAddRunbook(t *testing.T) { - - // Fixture Setup - - // language=yaml - runbooks = map[string]Runbook{} - - invalidRunbook := ` -name: unit-test-runbook -docs: "http://localhost" -` - - // Execute SUT - err := AddRunbookFromYaml(invalidRunbook) - require.Errorf(t, err, "Should get an error when adding runbook") - runbookNames := GetRunbookNames() - - // Verification - require.Equal(t, []string{}, runbookNames) -} - func TestThatInitializeBuiltInRunbooksActuallyLoadsRunbooks(t *testing.T) { // Fixture Setup // language=yaml - runbooks = map[string]Runbook{} + Reset() // Execute SUT InitializeBuiltInRunbooks()