diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d76ce9..7c0e39e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,13 +1,27 @@ # astro changelog -## 0.4.2 (UNRELEASED, 2018) +## 0.5.0 (UNRELEASED, 2018) * Add Travis configuration, `make lint` and git precommit hook +* Fix `--help` displaying "pflag: help requested" (#1) * Fix issue with make not recompiling when source files changed * Fix issue with `make test` always returning true even when tests fail * Fix race condition that could cause failures due to astro downloading the same version of Terraform twice * Remove godep and move to Go modules (vgo) +* Change configuration syntax for remapping CLI flags to Terraform module + variables + +**Breaking changes:** + +* Before, there was a `flag:` option underneath module variables in the project + configuration that allowed you to modify the name of the flag on the CLI that + would represent that variable (e.g.: "--environment" could be remapped to + "--env"). + + This has been removed and there is now a completely new section in the YAML + called "flags". See the "Remapping flags" section of the README for more + information. ## 0.4.1 (October 3, 2018) diff --git a/README.md b/README.md index 01b8915..80fb145 100644 --- a/README.md +++ b/README.md @@ -228,3 +228,18 @@ database-dev-us-east-1: OK No changes (9s) app-dev-us-east-1: OK No changes (10s) > ``` + +#### Remapping CLI flags + +Astro is meant to be used every day by operators. If your Terraform variable names are long-winded to type at the CLI, you can remap them to something simpler. For example, instead of typing `--environment dev`, you may wish to shoren this to `--env dev`. + +You can specify a `flags:` block in your project configuration, like: + +``` +flags: + environment: + name: env + description: Environment to deploy to +``` + +This will remap the "environment" Terraform variable to `--env` on the astro command line. You can also specify a description that will show up in the `--help` text. diff --git a/astro/cli/astro/cmd/cmd_test.go b/astro/cli/astro/cmd/cmd_test.go index 7dfcd8b..a52b5c4 100644 --- a/astro/cli/astro/cmd/cmd_test.go +++ b/astro/cli/astro/cmd/cmd_test.go @@ -56,6 +56,8 @@ var ( terraformVersionRepo *tvm.VersionRepo ) +const VERSION_LATEST = "" + // compiles the astro binary and returns the path to it. func compileAstro() (string, error) { f, err := ioutil.TempFile("", "") @@ -109,6 +111,10 @@ type testResult struct { func runTest(t *testing.T, args []string, fixtureBasePath string, version string) *testResult { fixturePath := fixtureBasePath + if version == VERSION_LATEST { + version = terraformVersionsToTest[len(terraformVersionsToTest)-1] + } + // Determine if this version has a version-specific fixture. versionSpecificFixturePath := fmt.Sprintf("%s-%s", fixtureBasePath, version) if utils.FileExists(versionSpecificFixturePath) { @@ -184,6 +190,12 @@ func getSessionDirs(sessionBaseDir string) ([]string, error) { return sessionDirs, nil } +func TestHelpWorks(t *testing.T) { + result := runTest(t, []string{"--help"}, "", VERSION_LATEST) + assert.Contains(t, "A tool for managing multiple Terraform modules", result.Stderr.String()) + assert.NoError(t, result.Err) +} + func TestProjectApplyChangesSuccess(t *testing.T) { for _, version := range terraformVersionsToTest { t.Run(version, func(t *testing.T) { diff --git a/astro/cli/astro/cmd/cmds.go b/astro/cli/astro/cmd/cmds.go index 5bfb393..b2fc89f 100644 --- a/astro/cli/astro/cmd/cmds.go +++ b/astro/cli/astro/cmd/cmds.go @@ -40,7 +40,7 @@ var applyCmd = &cobra.Command{ return err } - vars := userVariables() + vars := flagsToUserVariables() var moduleNames []string if moduleNamesString != "" { @@ -57,7 +57,7 @@ var applyCmd = &cobra.Command{ }, ) if err != nil { - return fmt.Errorf("error running Terraform: %v", err) + return fmt.Errorf("ERROR: %v", processError(err)) } err = printExecStatus(status, results) @@ -81,7 +81,7 @@ var planCmd = &cobra.Command{ return err } - vars := userVariables() + vars := flagsToUserVariables() var moduleNames []string if moduleNamesString != "" { @@ -99,7 +99,7 @@ var planCmd = &cobra.Command{ }, ) if err != nil { - return fmt.Errorf("error running Terraform: %v", err) + return fmt.Errorf("ERROR: %v", processError(err)) } err = printExecStatus(status, results) @@ -113,25 +113,6 @@ var planCmd = &cobra.Command{ }, } -func userVariables() *astro.UserVariables { - values := make(map[string]string) - filters := make(map[string]bool) - - for _, flag := range _flags { - if flag.Value != "" { - values[flag.Variable] = flag.Value - if flag.IsFilter { - filters[flag.Variable] = true - } - } - } - - return &astro.UserVariables{ - Values: values, - Filters: filters, - } -} - func init() { applyCmd.PersistentFlags().StringVar(&moduleNamesString, "modules", "", "list of modules to apply") rootCmd.AddCommand(applyCmd) @@ -140,3 +121,15 @@ func init() { planCmd.PersistentFlags().StringVar(&moduleNamesString, "modules", "", "list of modules to plan") rootCmd.AddCommand(planCmd) } + +// processError interprets certain astro errors and embellishes them for +// display on the CLI. +func processError(err error) error { + switch e := err.(type) { + case astro.MissingRequiredVarsError: + // reverse map variables to CLI flags + return fmt.Errorf("missing required flags: %s", strings.Join(varsToFlagNames(e.MissingVars()), ", ")) + default: + return err + } +} diff --git a/astro/cli/astro/cmd/config.go b/astro/cli/astro/cmd/config.go index ccf55fa..56dd4ec 100644 --- a/astro/cli/astro/cmd/config.go +++ b/astro/cli/astro/cmd/config.go @@ -16,7 +16,14 @@ package cmd -import "github.com/uber/astro/astro/utils" +import ( + "errors" + "fmt" + + "github.com/uber/astro/astro" + "github.com/uber/astro/astro/conf" + "github.com/uber/astro/astro/utils" +) // configFileSearchPaths is the default list of paths the astro CLI // will attempt to find a config file at. @@ -27,6 +34,14 @@ var configFileSearchPaths = []string{ "terraform/astro.yml", } +var errCannotFindConfig = errors.New("unable to find config file") + +// Global cache +var ( + _conf *conf.Project + _project *astro.Project +) + // firstExistingFilePath takes a list of paths and returns the first one // where a file exists (or symlink to a file). func firstExistingFilePath(paths ...string) string { @@ -37,3 +52,54 @@ func firstExistingFilePath(paths ...string) string { } return "" } + +// configFile returns the path of the project config file. +func configFile() (string, error) { + // User provided config file path takes precedence + if userCfgFile != "" { + return userCfgFile, nil + } + + // Try to find the config file + if path := firstExistingFilePath(configFileSearchPaths...); path != "" { + return path, nil + } + + return "", errCannotFindConfig +} + +// currentConfig loads configuration or returns the previously loaded config. +func currentConfig() (*conf.Project, error) { + if _conf != nil { + return _conf, nil + } + + file, err := configFile() + if err != nil { + return nil, err + } + _conf, err = astro.NewConfigFromFile(file) + + return _conf, err +} + +// currentProject creates a new astro project or returns the previously created +// astro project. +func currentProject() (*astro.Project, error) { + if _project != nil { + return _project, nil + } + + config, err := currentConfig() + if err != nil { + return nil, err + } + c, err := astro.NewProject(*config) + if err != nil { + return nil, fmt.Errorf("unable to load module configuration: %v", err) + } + + _project = c + + return _project, nil +} diff --git a/astro/cli/astro/cmd/fixtures/flags/merge_values.yaml b/astro/cli/astro/cmd/fixtures/flags/merge_values.yaml index 8d56a87..1ade5ce 100644 --- a/astro/cli/astro/cmd/fixtures/flags/merge_values.yaml +++ b/astro/cli/astro/cmd/fixtures/flags/merge_values.yaml @@ -1,7 +1,7 @@ --- terraform: - version: 0.11.7 + path: ../../../../../fixtures/mock-terraform/success modules: - name: foo_mgmt diff --git a/astro/cli/astro/cmd/fixtures/flags/no_variables.yaml b/astro/cli/astro/cmd/fixtures/flags/no_variables.yaml index 05ccf9a..6063875 100644 --- a/astro/cli/astro/cmd/fixtures/flags/no_variables.yaml +++ b/astro/cli/astro/cmd/fixtures/flags/no_variables.yaml @@ -1,7 +1,7 @@ --- terraform: - version: 0.11.7 + path: ../../../../../fixtures/mock-terraform/success modules: - name: foo diff --git a/astro/cli/astro/cmd/fixtures/flags/simple_variables.yaml b/astro/cli/astro/cmd/fixtures/flags/simple_variables.yaml index 7b26f27..96b4a83 100644 --- a/astro/cli/astro/cmd/fixtures/flags/simple_variables.yaml +++ b/astro/cli/astro/cmd/fixtures/flags/simple_variables.yaml @@ -1,14 +1,18 @@ --- terraform: - version: 0.11.7 + path: ../../../../../fixtures/mock-terraform/success + +flags: + bar: + name: baz + description: Baz Description modules: - - name: foo + - name: fooModule path: . variables: - - name: no_flag - - name: with_flag - flag: flag_name - - name: with_values + - name: foo + - name: bar + - name: qux values: [dev, staging, prod] diff --git a/astro/cli/astro/cmd/flags.go b/astro/cli/astro/cmd/flags.go index eadda14..45e2b30 100644 --- a/astro/cli/astro/cmd/flags.go +++ b/astro/cli/astro/cmd/flags.go @@ -18,31 +18,52 @@ package cmd import ( "fmt" + "os" "sort" "strings" + "github.com/uber/astro/astro" "github.com/uber/astro/astro/conf" + + "github.com/spf13/cobra" + "github.com/spf13/pflag" ) -// Flag is populated with the custom variables read from command line flags -type Flag struct { - // Variable is the name of the Terraform variable set by the flag - Variable string - // Value is the value read from the command line, if present +// Text that will be appended to the --help output for plan/apply, showing +// user flags from the astro project config. +const userHelpTemplate = ` +User flags: +{{.ProjectFlagsHelp}}` + +// ProjectFlag is a CLI flag that represents a variable from the user's astro +// project config file. +type ProjectFlag struct { + // Name of the user flag at the command line + Name string + // Value is the string var the flag value will be put into Value string - // Flag is the name of the command line flag used to set the variable - Flag string - // IsRequired is true when thie flag is mandatory - IsRequired bool - // IsFilter is true when thie flag acts as a filter for the list of modules - IsFilter bool + // Description is what shows up next to the flag in --help + Description string + // Variable is the name of the user variable from the user's astro project + // config that this flag maps to. + Variable string // AllowedValues is the list of valid values for this flag AllowedValues []string } -// StringEnum implements pflag.Value interface, to check that the passed-in value is one of the strings in AllowedValues +// AddToFlagSet adds the flag to the specified flag set. +func (flag *ProjectFlag) AddToFlagSet(flags *pflag.FlagSet) { + if len(flag.AllowedValues) > 0 { + flags.Var(&StringEnum{Flag: flag}, flag.Name, flag.Description) + } else { + flags.StringVar(&flag.Value, flag.Name, "", flag.Description) + } +} + +// StringEnum implements pflag.Value interface, to check that the passed-in +// value is one of the strings in AllowedValues. type StringEnum struct { - Flag *Flag + Flag *ProjectFlag } // String returns the current value @@ -50,7 +71,8 @@ func (s *StringEnum) String() string { return s.Flag.Value } -// Set checks that the passed-in value is only of the allowd values, and returns an error if it is not +// Set checks that the passed-in value is only of the allowd values, and +// returns an error if it is not func (s *StringEnum) Set(value string) error { for _, allowedValue := range s.Flag.AllowedValues { if allowedValue == value { @@ -65,67 +87,153 @@ func (s *StringEnum) Type() string { return "string" } -// commandLineFlags returns a list of variables that can be set via command line -func commandLineFlags(conf *conf.Project) ([]*Flag, error) { - var err error - flags := make([]*Flag, 0) +// addProjectFlagsToCommands adds the user flags to the specified Cobra commands. +func addProjectFlagsToCommands(flags []*ProjectFlag, cmds ...*cobra.Command) { + ProjectFlagSet := flagsToFlagSet(flags) - for _, moduleConf := range conf.Modules { - for _, variableConf := range moduleConf.Variables { - if flags, err = addOrUpdateVariable(flags, variableConf); err != nil { - return nil, err - } + for _, cmd := range cmds { + for _, flag := range flags { + flag.AddToFlagSet(cmd.Flags()) + } + + // Update help text for the command to include the user flags + helpTmpl := cmd.HelpTemplate() + helpTmpl += "\nUser flags:\n" + helpTmpl += ProjectFlagSet.FlagUsages() + + cmd.SetHelpTemplate(helpTmpl) + + // Mark flag hidden so it doesn't appear in the normal help. We have to + // do this *after* calling FlagUsages above, otherwise the flags don't + // appear in the output. + for _, flag := range flags { + cmd.Flags().MarkHidden(flag.Name) + } + } +} + +// Load the astro configuration file and read flags from the project config. +func loadProjectFlagsFromConfig() ([]*ProjectFlag, error) { + findConfig := &cobra.Command{ + SilenceUsage: true, + SilenceErrors: true, + FParseErrWhitelist: cobra.FParseErrWhitelist{ + UnknownFlags: true, + }, + } + + // Strip the help options from os.Args so that the pre-loading of the + // config doesn't fail with pflag.ErrHelp + args := []string{} + for _, arg := range os.Args { + if arg == "-h" || arg == "--help" || arg == "-help" { + continue } + args = append(args, arg) } - for i := range flags { - flags[i].AllowedValues = uniqueStrings(flags[i].AllowedValues) + // Do an early first parse of the config flag before the main command, + findConfig.PersistentFlags().StringVar(&userCfgFile, "config", "", "config file") + if err := findConfig.ParseFlags(args); err != nil { + return nil, err } - return flags, nil + config, err := currentConfig() + if err != nil { + return nil, err + } + + return flagsFromConfig(config), nil } -func addOrUpdateVariable(flags []*Flag, variable conf.Variable) ([]*Flag, error) { - commandFlag := variable.CommandFlag() - found := false - for i := range flags { - if flags[i].Variable == variable.Name { - if err := checkVariableConsistency(flags[i], variable); err != nil { - return nil, err +// flagsFromConfig reads the astro config and returns a list of ProjectFlags that +// can be used to fill in astro variable values at runtime. +func flagsFromConfig(config *conf.Project) (flags []*ProjectFlag) { + flagMap := map[string]*ProjectFlag{} + + for _, moduleConf := range config.Modules { + for _, variableConf := range moduleConf.Variables { + var flagName string + var flagConf conf.Flag + + // Check for flag mapping in project configuration. This is a block + // in the configuration that allows users to remap variable names + // on the CLI and set a description for the --help message. + flagConf, flagConfExists := config.Flags[variableConf.Name] + if flagConfExists { + flagName = flagConf.Name + } else { + flagName = variableConf.Name + } + + if flag, ok := flagMap[flagName]; ok { + // aggregate values from all variables in the config + flag.AllowedValues = uniqueStrings(append(flag.AllowedValues, variableConf.Values...)) + } else { + flag := &ProjectFlag{ + Name: flagName, + Description: flagConf.Description, + Variable: variableConf.Name, + AllowedValues: variableConf.Values, + } + + flagMap[variableConf.Name] = flag } - flags[i].AllowedValues = append(flags[i].AllowedValues, variable.Values...) - found = true } - if flags[i].Flag == commandFlag { - if flags[i].Variable != variable.Name { - return nil, fmt.Errorf("Flag '%s' is mapped to conflicting flags '%s' and '%s'", commandFlag, variable.Name, flags[i].Variable) + } + + // return as list + for _, flag := range flagMap { + flags = append(flags, flag) + } + + return flags +} + +// Create an astro.UserVariables suitable for passing into ExecutionParameters +// from the user flags. +func flagsToUserVariables() *astro.UserVariables { + values := make(map[string]string) + filters := make(map[string]bool) + + for _, flag := range projectFlags { + if flag.Value != "" { + values[flag.Variable] = flag.Value + if len(flag.AllowedValues) > 0 { + filters[flag.Variable] = true } } } - if !found { - flags = append(flags, &Flag{ - Variable: variable.Name, - Flag: commandFlag, - IsRequired: variable.IsRequired(), - IsFilter: variable.IsFilter(), - AllowedValues: variable.Values, - }) + + return &astro.UserVariables{ + Values: values, + Filters: filters, } - return flags, nil } -func checkVariableConsistency(flag *Flag, variable conf.Variable) error { - commandFlag := variable.CommandFlag() - if flag.Flag != commandFlag { - return fmt.Errorf("variable '%s' is mapped to conflicting flags '%s' and '%s'", variable.Name, commandFlag, flag.Flag) +// Converts a list of ProjectFlags to a pflag.FlagSet. +func flagsToFlagSet(flags []*ProjectFlag) *pflag.FlagSet { + flagSet := pflag.NewFlagSet("ProjectFlags", pflag.ContinueOnError) + for _, flag := range flags { + flag.AddToFlagSet(flagSet) } - if flag.IsRequired != variable.IsRequired() { - return fmt.Errorf("variable '%s' is inconsistently marked as required", variable.Name) + return flagSet +} + +// flagName returns the flag name, given a variable name. +func flagName(variableName string) string { + if flag, ok := _conf.Flags[variableName]; ok { + return flag.Name } - if flag.IsFilter != variable.IsFilter() { - return fmt.Errorf("variable '%s' is inconsistently used as filter", variable.Name) + return variableName +} + +// varsToFlagNames converts a list of variable names to CLI flags. +func varsToFlagNames(variableNames []string) (flagNames []string) { + for _, v := range variableNames { + flagNames = append(flagNames, fmt.Sprintf("--%s", flagName(v))) } - return nil + return flagNames } func uniqueStrings(strings []string) []string { diff --git a/astro/cli/astro/cmd/flags_test.go b/astro/cli/astro/cmd/flags_test.go index 86c8388..1bec14a 100644 --- a/astro/cli/astro/cmd/flags_test.go +++ b/astro/cli/astro/cmd/flags_test.go @@ -14,75 +14,91 @@ * limitations under the License. */ -package cmd +package cmd_test import ( "testing" - "github.com/uber/astro/astro" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) -func TestNoVariables(t *testing.T) { - c, err := astro.NewConfigFromFile("fixtures/flags/no_variables.yaml") - require.NoError(t, err) - - flags, err := commandLineFlags(c) - require.NoError(t, err) - - assert.Equal(t, []*Flag{}, flags) +func TestHelpUserFlags(t *testing.T) { + result := runTest(t, []string{ + "--config=simple_variables.yaml", + "plan", + "--help", + }, "fixtures/flags", VERSION_LATEST) + assert.Contains(t, result.Stdout.String(), "User flags:") + assert.Contains(t, result.Stdout.String(), "--foo") + assert.Contains(t, result.Stdout.String(), "--baz") + assert.Contains(t, result.Stdout.String(), "Baz Description") + assert.Contains(t, result.Stdout.String(), "--qux") } -func TestSimpleVariables(t *testing.T) { - c, err := astro.NewConfigFromFile("fixtures/flags/simple_variables.yaml") - require.NoError(t, err) +func TestHelpNoUserFlags(t *testing.T) { + result := runTest(t, []string{ + "--config=no_variables.yaml", + "plan", + "--help", + }, "fixtures/flags", VERSION_LATEST) + assert.NotContains(t, result.Stdout.String(), "User flags:") +} - flags, err := commandLineFlags(c) - require.NoError(t, err) +func TestHelpShowsConfigLoadError(t *testing.T) { + result := runTest(t, []string{ + "--config=/nonexistent/path/to/config", + "plan", + "--help", + }, "fixtures/flags", VERSION_LATEST) + assert.Contains(t, result.Stderr.String(), "There was an error loading astro config") +} - assert.Equal(t, []*Flag{ - &Flag{ - Variable: "no_flag", - Flag: "no_flag", - IsRequired: true, - }, - &Flag{ - Variable: "with_flag", - Flag: "flag_name", - IsRequired: true, - }, - &Flag{ - Variable: "with_values", - Flag: "with_values", - IsFilter: true, - AllowedValues: []string{ - "dev", - "prod", - "staging", - }, - }, - }, flags) +func TestHelpDoesntAlwaysShowLoadingError(t *testing.T) { + result := runTest(t, []string{ + "--help", + }, "fixtures/flags", VERSION_LATEST) + assert.NotContains(t, result.Stderr.String(), "There was an error loading astro config") } -func TestMergeValues(t *testing.T) { - c, err := astro.NewConfigFromFile("fixtures/flags/merge_values.yaml") - require.NoError(t, err) +func TestPlanErrorOnMissingValues(t *testing.T) { + result := runTest(t, []string{ + "--config=simple_variables.yaml", + "plan", + }, "fixtures/flags", VERSION_LATEST) + assert.Error(t, result.Err) + assert.Contains(t, result.Stderr.String(), "missing required flags") + assert.Contains(t, result.Stderr.String(), "--foo") + assert.Contains(t, result.Stderr.String(), "--baz") +} - flags, err := commandLineFlags(c) - require.NoError(t, err) +func TestPlanAllowedValues(t *testing.T) { + tt := []string{ + "mgmt", + "dev", + "staging", + "prod", + } + for _, env := range tt { + t.Run(env, func(t *testing.T) { + result := runTest(t, []string{ + "--config=merge_values.yaml", + "plan", + "--environment", + env, + }, "fixtures/flags", VERSION_LATEST) + assert.NoError(t, result.Err) + }) + } +} - assert.Equal(t, []*Flag{ - &Flag{ - Variable: "environment", - Flag: "environment", - IsFilter: true, - AllowedValues: []string{ - "dev", - "mgmt", - "prod", - "staging", - }, - }, - }, flags) +func TestPlanFailOnNotAllowedValue(t *testing.T) { + result := runTest(t, []string{ + "--config=merge_values.yaml", + "plan", + "--environment", + "foo", + }, "fixtures/flags", VERSION_LATEST) + assert.Error(t, result.Err) + assert.Contains(t, result.Stderr.String(), "invalid argument") + assert.Contains(t, result.Stderr.String(), "allowed values") } diff --git a/astro/cli/astro/cmd/main.go b/astro/cli/astro/cmd/main.go index faff848..6cfe9a0 100644 --- a/astro/cli/astro/cmd/main.go +++ b/astro/cli/astro/cmd/main.go @@ -15,34 +15,27 @@ */ // Package cmd contains the source for the `astro` command line tool -// that operators use to interact with the project. The layout of files -// in this package is defined by Cobra, which is the library that powers -// the CLI tool. +// that operators use to interact with the project. package cmd import ( - "errors" "fmt" "io/ioutil" "log" "os" + "strings" - "github.com/uber/astro/astro" - "github.com/uber/astro/astro/conf" "github.com/uber/astro/astro/logger" "github.com/spf13/cobra" ) +// CLI flags var ( - userCfgFile string - trace bool - userVars map[string]string - verbose bool - - _flags []*Flag - _conf *conf.Project - _project *astro.Project + trace bool + userCfgFile string + projectFlags []*ProjectFlag + verbose bool ) var rootCmd = &cobra.Command{ @@ -52,15 +45,6 @@ var rootCmd = &cobra.Command{ SilenceErrors: true, } -// Execute adds all child commands to the root command and sets flags appropriately. -// This is called by main.main(). It only needs to happen once to the rootCmd. -func Execute() error { - if err := initUserFlagsFromConfig(); err != nil { - return err - } - return rootCmd.Execute() -} - func init() { rootCmd.PersistentFlags().BoolVarP(&verbose, "verbose", "v", false, "verbose output") rootCmd.PersistentFlags().BoolVarP(&trace, "trace", "", false, "trace output") @@ -77,90 +61,43 @@ func init() { }) } -func initUserFlagsFromConfig() error { - findConfig := &cobra.Command{ - SilenceUsage: true, - SilenceErrors: true, - FParseErrWhitelist: cobra.FParseErrWhitelist{ - UnknownFlags: true, - }, +// Main is the main entry point into the CLI program. +func Main() (exitCode int) { + var projectFlagsLoadErr error + + // Try to parse user flags from their astro config file. Reading the astro + // config could fail with an error, e.g. if there is no config file found, + // but this is not a hard failure. Save the error for later, so we can let + // the user know about the error in certain cases. + projectFlags, projectFlagsLoadErr = loadProjectFlagsFromConfig() + if projectFlags != nil && len(projectFlags) > 0 { + addProjectFlagsToCommands(projectFlags, applyCmd, planCmd) } - findConfig.PersistentFlags().StringVar(&userCfgFile, "config", "", "config file") + rc := 0 - if err := findConfig.ParseFlags(os.Args); err != nil { - return err - } - config, err := currentConfig() - if err != nil { - return err - } + if err := rootCmd.Execute(); err != nil { + fmt.Fprintln(os.Stderr, err.Error()) - _flags, err = commandLineFlags(config) - if err != nil { - return err - } - for _, flag := range _flags { - usage := fmt.Sprintf("Set \"%s\" Terraform variable", flag.Variable) - for _, command := range rootCmd.Commands() { - if len(flag.AllowedValues) > 0 { - command.Flags().Var(&StringEnum{Flag: flag}, flag.Flag, usage) - } else { - command.Flags().StringVar(&flag.Value, flag.Flag, "", usage) - } - if flag.IsRequired { - command.MarkFlagRequired(flag.Flag) - } + if projectFlagsLoadErr != nil && strings.Contains(err.Error(), "unknown flag") { + printConfigLoadingError(projectFlagsLoadErr) } - } - - return nil -} -// configFile returns the path of the project config file. -func configFile() (string, error) { - // User provided config file path takes precedence - if userCfgFile != "" { - return userCfgFile, nil + // exit with error + rc = 1 } - // Try to find the config file - if path := firstExistingFilePath(configFileSearchPaths...); path != "" { - return path, nil + // If there was an error when parsing the user's project config file, + // then display a message to let them know in case they're wondering + // why their CLI flags are not showing up. + if projectFlagsLoadErr != nil && projectFlagsLoadErr != errCannotFindConfig { + printConfigLoadingError(projectFlagsLoadErr) } - return "", errors.New("unable to find config file") + return rc } -func currentConfig() (*conf.Project, error) { - if _conf != nil { - return _conf, nil - } - - file, err := configFile() - if err != nil { - return nil, err - } - _conf, err = astro.NewConfigFromFile(file) - - return _conf, err -} - -func currentProject() (*astro.Project, error) { - if _project != nil { - return _project, nil - } - - config, err := currentConfig() - if err != nil { - return nil, err - } - c, err := astro.NewProject(*config) - if err != nil { - return nil, fmt.Errorf("unable to load module configuration: %v", err) - } - - _project = c - - return _project, nil +func printConfigLoadingError(e error) { + fmt.Fprintf(os.Stderr, "\nNOTE: There was an error loading astro config:\n") + fmt.Fprintln(os.Stderr, e.Error()) } diff --git a/astro/cli/astro/main.go b/astro/cli/astro/main.go index 9171794..f7d9ff7 100644 --- a/astro/cli/astro/main.go +++ b/astro/cli/astro/main.go @@ -17,15 +17,11 @@ package main import ( - "fmt" "os" "github.com/uber/astro/astro/cli/astro/cmd" ) func main() { - if err := cmd.Execute(); err != nil { - fmt.Fprintln(os.Stderr, err) - os.Exit(1) - } + os.Exit(cmd.Main()) } diff --git a/astro/conf/astro.go b/astro/conf/astro.go index 4137641..f026fe9 100644 --- a/astro/conf/astro.go +++ b/astro/conf/astro.go @@ -24,6 +24,10 @@ import ( // Project represents the structure of the YAML configuration for astro. type Project struct { + // Flags is a mapping of module variable names to user flags, e.g. for on + // the CLI. + Flags map[string]Flag + // Hooks contains configuration of hooks that can be invoked at various // stages of the CLI lifecycle. Hooks Hooks diff --git a/astro/conf/flags.go b/astro/conf/flags.go new file mode 100644 index 0000000..79535e1 --- /dev/null +++ b/astro/conf/flags.go @@ -0,0 +1,25 @@ +/* + * Copyright (c) 2018 Uber Technologies, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package conf + +// Flag is a user +type Flag struct { + // Name of the flag that is visible to the user, e.g. on the CLI. + Name string + // Description is an optional description to show to the user. + Description string +} diff --git a/astro/conf/variable.go b/astro/conf/variable.go index 4406802..114b123 100644 --- a/astro/conf/variable.go +++ b/astro/conf/variable.go @@ -21,28 +21,11 @@ package conf type Variable struct { // Name is the name/key of the variable. Name string - // Flag is the command-line flag of the variable - Flag string // Values is a list of possible values for the variable. A value of nil // means the possible values are unbound. Values []string } -// CommandFlag is name of the command line flag that can be used to set this variable -func (v *Variable) CommandFlag() string { - if v.Flag != "" { - return v.Flag - } - return v.Name -} - -// IsRequired returns true if the command-line parameter is mandatory -// -// The full behaviour is described in the README -func (v *Variable) IsRequired() bool { - return len(v.Values) == 0 -} - // IsFilter returns true if the command-line parameter acts as a filter // // The full behaviour is described in the README diff --git a/astro/execution.go b/astro/execution.go index 8d8360d..0c60a17 100644 --- a/astro/execution.go +++ b/astro/execution.go @@ -24,6 +24,30 @@ import ( "github.com/uber/astro/astro/conf" ) +// MissingRequiredVarsError is an error type that is returned from plan or +// apply when there are variables that need to be provided at run time that are +// missing. +type MissingRequiredVarsError struct { + missing []string +} + +func (e *MissingRequiredVarsError) plural() string { + if len(e.missing) > 0 { + return "s" + } + return "" +} + +// Error is the error message, so this satisfies the error interface. +func (e MissingRequiredVarsError) Error() string { + return fmt.Sprintf("missing required variable%s: %s", e.plural(), strings.Join(e.missing, ", ")) +} + +// MissingVars returns a list of the missing user variables. +func (e MissingRequiredVarsError) MissingVars() []string { + return e.missing +} + // terraformExecution is an interface that covers both bound and unbound // executions. type terraformExecution interface { @@ -115,14 +139,20 @@ type unboundExecution struct { func (e *unboundExecution) bind(userVars map[string]string) (*boundExecution, error) { boundVars := union(e.Variables(), userVars) + missingVars := []string{} + // Check that the user provided variables replace everything that // needs to be replaced. for _, val := range boundVars { if err := assertAllVarsReplaced(val); err != nil { - return nil, fmt.Errorf("unable to bind execution: %v; %v", e.ID(), err) + missingVars = append(missingVars, extractMissingVarNames(val)...) } } + if len(missingVars) > 0 { + return nil, MissingRequiredVarsError{missing: missingVars} + } + // Create a copy of the config and search attributes for placeholders // to replace with values from the bound vars. boundConfig := e.ModuleConfig() diff --git a/astro/templates.go b/astro/templates.go index 9577ea0..0472eb0 100644 --- a/astro/templates.go +++ b/astro/templates.go @@ -19,12 +19,31 @@ package astro import ( "bytes" "fmt" + "regexp" "strings" "text/template" ) +var ( + // matches "{fox}" in "the quick {fox}" + reVarPlaceholder = regexp.MustCompile(`\{(.*)\}`) +) + +// extractMissingVarNames takes an input string like "foo {bar} {baz}" and +// returns a list of the var names between {}, e.g. [bar, baz]. +func extractMissingVarNames(s string) (vars []string) { + matches := reVarPlaceholder.FindAllStringSubmatch(s, -1) + for _, match := range matches { + vars = append(vars, match[1]) + } + return vars +} + +// assertAllVarsReplaced asserts that all vars have been replaced in a string, +// i.e. that there are no values like "{baz}" in the string. It returns an +// error if there is. func assertAllVarsReplaced(s string) error { - if strings.ContainsAny(s, "{}") || strings.Contains(s, "<no value>") { + if strings.ContainsAny(s, "{}") { return fmt.Errorf("not all vars replaced in string: %v", s) } return nil diff --git a/go.mod b/go.mod index 32f6e40..8dfb765 100644 --- a/go.mod +++ b/go.mod @@ -25,7 +25,7 @@ require ( github.com/spf13/cast v1.2.0 // indirect github.com/spf13/cobra v0.0.3 github.com/spf13/jwalterweatherman v0.0.0-20180109140146-7c0cea34c8ec // indirect - github.com/spf13/pflag v1.0.1 // indirect + github.com/spf13/pflag v1.0.1 github.com/spf13/viper v1.0.2 github.com/stretchr/testify v1.2.1 golang.org/x/sys v0.0.0-20171222143536-83801418e1b5 // indirect