Rework of CLI flag remapping (#21)

This reworks the flags feature and refactors a lot of the code under the hood
to change the design and fix some issues.

* Fix the --help issue.

  Previously, --help didn't work at all and resulted in an error: `pflag: help
  requested` (see #1). This fixes that issue and also improves the printing of
  user flags in the --help text. User flags are now shown at the bottom,
  separate from builtin and global astro flags. E.g.:

  ```
  $ astro plan --help
  Generate execution plans for modules

  Usage:
    astro plan [flags] [-- [Terraform argument]...]

  Flags:
        --detach           disconnect remote state before planning
    -h, --help             help for plan
        --modules string   list of modules to plan

  Global Flags:
        --config string   config file
        --trace           trace output
    -v, --verbose         verbose output

  User flags:
        --env string      Environment to deploy
        --region string   Region to deploy
  $
  ```

* Previously, Terraform variable names were remapped to a different flag name
  by specifying the `flag:` option in the variable config, e.g.:

  ```
  - name: app
    path: core/app
    variables:
    - name: aws_region
      flag: region
  ```

  This meant if you had many modules defined with the same variable, you had to
  type out the mapping correctly every time or else you would get an error.

  The mapping configuration has now moved to its own block in the project
  config, which now looks like:

  ```
  flags:
    aws_region:
      name: region
      description: Region to deploy
  ```

  Users can now also specify a description that will appear in the --help text.

  Note that the flags section is completely optional, and be default astro will
  just expose the full variable name as the CLI flag.

* Refactoring

  The implementation of user/project flags did a lot of the error checking in
  the cli/astro package itself, however, we want to move in a direction where
  those checks occur in the core astro package. The CLI should be a thin
  wrapper around the core package.

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)
 

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.

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) {

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
+	}
+}

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
+}

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

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

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]