Made changes following Yevgeniy’s recommendations

Added tests for both new interpolation function and extra parameter injection Note that I changed the order of functions in config_helpers to put the two newly created functions processMultipleInterpolationsInString and processSingleInterpolationInString bellow executeTerragruntHelperFunction which is an more important function in the file.
codelinx · May 23, 2017 · 027cebc · 027cebc
1 parent 528e3c7
commit 027cebc
Show file tree

Hide file tree

Showing 6 changed files with 99 additions and 55 deletions.
diff --git a/README.md b/README.md
@@ -1321,10 +1321,10 @@ commands = ["${get_terraform_commands_that_need_vars()}"]
 # which result in:
 commands = ["apply", "console", "destroy", "import", "plan", "push", "refresh"]
 
-# They shall not be used to do string composition like:
+# We do not recommend using them in string composition like:
 commands = "Some text ${get_terraform_commands_that_need_locking()}"
 
-# which result in:
+# which result in something useless like:
 commands = "Some text [apply destroy import init plan refresh taint untaint]"
 ```
 

diff --git a/cli/cli_app.go b/cli/cli_app.go
@@ -192,13 +192,14 @@ func runTerragrunt(terragruntOptions *options.TerragruntOptions) error {
 		if util.ListContainsElement(TERRAFORM_COMMANDS_WITH_SUBCOMMAND, terragruntOptions.TerraformCliArgs[0]) {
 			commandLength = 2
 		}
-		var args []string
-		args = append(args, terragruntOptions.TerraformCliArgs[:commandLength]...)
 
 		// Options must be inserted after command but before the other args
 		// command is either 1 word or 2 words
+		var args []string
+		args = append(args, terragruntOptions.TerraformCliArgs[:commandLength]...)
 		args = append(args, filterTerraformExtraArgs(terragruntOptions, conf)...)
-		terragruntOptions.TerraformCliArgs = append(args, terragruntOptions.TerraformCliArgs[commandLength:]...)
+		args = append(args, terragruntOptions.TerraformCliArgs[commandLength:]...)
+		terragruntOptions.TerraformCliArgs = args
 	}
 
 	if sourceUrl, hasSourceUrl := getTerraformSourceUrl(terragruntOptions, conf); hasSourceUrl {

diff --git a/config/config_helpers.go b/config/config_helpers.go
@@ -14,7 +14,7 @@ import (
 )
 
 var INTERPOLATION_SYNTAX_REGEX = regexp.MustCompile(`\$\{.*?\}`)
-var INTERPOLATION_SYNTAX_REGEX_SINGLE = regexp.MustCompile(`"\$\{.*?\}"`)
+var INTERPOLATION_SYNTAX_REGEX_SINGLE = regexp.MustCompile(fmt.Sprintf(`"(%s)"`, INTERPOLATION_SYNTAX_REGEX))
 var HELPER_FUNCTION_SYNTAX_REGEX = regexp.MustCompile(`\$\{(.*?)\((.*?)\)\}`)
 var HELPER_FUNCTION_GET_ENV_PARAMETERS_SYNTAX_REGEX = regexp.MustCompile(`\s*"(?P<env>[^=]+?)"\s*\,\s*"(?P<default>.*?)"\s*`)
 var MAX_PARENT_FOLDERS_TO_CHECK = 100
@@ -49,47 +49,83 @@ type EnvVar struct {
 
 // Given a string value from a Terragrunt configuration, parse the string, resolve any calls to helper functions using
 // the syntax ${...}, and return the final value.
-func ResolveTerragruntConfigString(terragruntConfigString string, include *IncludeConfig, terragruntOptions *options.TerragruntOptions) (resolved string, finalErr error) {
-	// The function we pass to ReplaceAllStringFunc cannot return an error, so we have to use named error
-	// parameters to capture such errors.
+func ResolveTerragruntConfigString(terragruntConfigString string, include *IncludeConfig, terragruntOptions *options.TerragruntOptions) (string, error) {
+	// First, we replace all single interpolation syntax (i.e. function directly enclosed within quotes "${function()}")
+	terragruntConfigString, err := processSingleInterpolationInString(terragruntConfigString, include, terragruntOptions)
+	if err != nil {
+		return terragruntConfigString, err
+	}
+	// Then, we replace all other interpolation functions (i.e. functions not directly enclosed within quotes)
+	return processMultipleInterpolationsInString(terragruntConfigString, include, terragruntOptions)
+}
+
+// Execute a single Terragrunt helper function and return its value as a string
+func executeTerragruntHelperFunction(functionName string, parameters string, include *IncludeConfig, terragruntOptions *options.TerragruntOptions) (interface{}, error) {
+	switch functionName {
+	case "find_in_parent_folders":
+		return findInParentFolders(terragruntOptions)
+	case "path_relative_to_include":
+		return pathRelativeToInclude(include, terragruntOptions)
+	case "path_relative_from_include":
+		return pathRelativeFromInclude(include, terragruntOptions)
+	case "get_env":
+		return getEnvironmentVariable(parameters, terragruntOptions)
+	case "get_tfvars_dir":
+		return getTfVarsDir(terragruntOptions)
+	case "get_parent_tfvars_dir":
+		return getParentTfVarsDir(include, terragruntOptions)
+	case "get_aws_account_id":
+		return getAWSAccountID()
+	case "get_terraform_commands_that_need_vars":
+		return TERRAFORM_COMMANDS_NEED_VARS, nil
+	case "get_terraform_commands_that_need_locking":
+		return TERRAFORM_COMMANDS_NEED_LOCKING, nil
+	default:
+		return "", errors.WithStackTrace(UnknownHelperFunction(functionName))
+	}
+}
 
+// For all interpolation functions that are called using the syntax "${function_name()}" (i.e. single interpolation function within string,
+// functions that return a non-string value we have to get rid of the surrounding quotes and convert the output to HCL syntax. For example,
+// for an array, we need to return "v1", "v2", "v3".
+func processSingleInterpolationInString(terragruntConfigString string, include *IncludeConfig, terragruntOptions *options.TerragruntOptions) (resolved string, finalErr error) {
+	// The function we pass to ReplaceAllStringFunc cannot return an error, so we have to use named error parameters to capture such errors.
 	resolved = INTERPOLATION_SYNTAX_REGEX_SINGLE.ReplaceAllStringFunc(terragruntConfigString, func(str string) string {
-		// We do a special treatment for function returning an array of values because they need
-		// to be declared like that "${function()" and the result must get rid of the surrounding quote
-		// to return a real array of values.
-		internalStr := str[1 : len(str)-1] // We get rid of the surrounding quote
-		out, err := resolveTerragruntInterpolation(internalStr, include, terragruntOptions)
+		matches := INTERPOLATION_SYNTAX_REGEX_SINGLE.FindStringSubmatch(terragruntConfigString)
+		out, err := resolveTerragruntInterpolation(matches[1], include, terragruntOptions)
 		if err != nil {
 			finalErr = err
 		}
 
 		switch out := out.(type) {
+		case string:
+			return fmt.Sprintf(`"%s"`, out)
 		case []string:
-			return util.ListToHCLArray(out)
+			return util.CommaSeparatedStrings(out)
+		default:
+			return fmt.Sprintf("%v", out)
 		}
-
-		// The function is not returning an array of string, so left the string unchanged
-		// Remaining functions will be processed by the next statement
-		return str
 	})
+	return
+}
 
-	resolved = INTERPOLATION_SYNTAX_REGEX.ReplaceAllStringFunc(resolved, func(str string) string {
+// For all interpolation functions that are called using the syntax "${function_a()}-${function_b()}" (i.e. multiple interpolation function
+// within the same string) or "Some text ${function_name()}" (i.e. string composition), we just replace the interpolation function call
+// by the string representation of its return.
+func processMultipleInterpolationsInString(terragruntConfigString string, include *IncludeConfig, terragruntOptions *options.TerragruntOptions) (resolved string, finalErr error) {
+	// The function we pass to ReplaceAllStringFunc cannot return an error, so we have to use named error parameters to capture such errors.
+	resolved = INTERPOLATION_SYNTAX_REGEX.ReplaceAllStringFunc(terragruntConfigString, func(str string) string {
 		out, err := resolveTerragruntInterpolation(str, include, terragruntOptions)
 		if err != nil {
 			finalErr = err
 		}
 
-		switch out := out.(type) {
-		case string:
-			return out
-		default:
-			return fmt.Sprintf("%v", out)
-		}
+		return fmt.Sprintf("%v", out)
 	})
-
 	return
 }
 
+// Given a string value from a Terragrunt configuration, parse the string, resolve any calls to helper functions using
 // Resolve a single call to an interpolation function of the format ${some_function()} in a Terragrunt configuration
 func resolveTerragruntInterpolation(str string, include *IncludeConfig, terragruntOptions *options.TerragruntOptions) (interface{}, error) {
 	matches := HELPER_FUNCTION_SYNTAX_REGEX.FindStringSubmatch(str)
@@ -100,32 +136,6 @@ func resolveTerragruntInterpolation(str string, include *IncludeConfig, terragru
 	}
 }
 
-// Execute a single Terragrunt helper function and return its value as a string
-func executeTerragruntHelperFunction(functionName string, parameters string, include *IncludeConfig, terragruntOptions *options.TerragruntOptions) (interface{}, error) {
-	switch functionName {
-	case "find_in_parent_folders":
-		return findInParentFolders(terragruntOptions)
-	case "path_relative_to_include":
-		return pathRelativeToInclude(include, terragruntOptions)
-	case "path_relative_from_include":
-		return pathRelativeFromInclude(include, terragruntOptions)
-	case "get_env":
-		return getEnvironmentVariable(parameters, terragruntOptions)
-	case "get_tfvars_dir":
-		return getTfVarsDir(terragruntOptions)
-	case "get_parent_tfvars_dir":
-		return getParentTfVarsDir(include, terragruntOptions)
-	case "get_aws_account_id":
-		return getAWSAccountID()
-	case "get_terraform_commands_that_need_vars":
-		return TERRAFORM_COMMANDS_NEED_VARS, nil
-	case "get_terraform_commands_that_need_locking":
-		return TERRAFORM_COMMANDS_NEED_LOCKING, nil
-	default:
-		return "", errors.WithStackTrace(UnknownHelperFunction(functionName))
-	}
-}
-
 // Return the directory where the Terragrunt configuration file lives
 func getTfVarsDir(terragruntOptions *options.TerragruntOptions) (string, error) {
 	terragruntConfigFileAbsPath, err := filepath.Abs(terragruntOptions.TerragruntConfigPath)

diff --git a/config/config_helpers_test.go b/config/config_helpers_test.go
@@ -5,6 +5,7 @@ import (
 	"github.com/gruntwork-io/terragrunt/errors"
 	"github.com/gruntwork-io/terragrunt/options"
 	"github.com/gruntwork-io/terragrunt/test/helpers"
+	"github.com/gruntwork-io/terragrunt/util"
 	"github.com/stretchr/testify/assert"
 	"os"
 	"path/filepath"
@@ -492,6 +493,27 @@ TERRAGRUNT_HIT","")}/bar`,
 			"foo/HIT/bar",
 			nil,
 		},
+		{
+			`"${get_terraform_commands_that_need_locking()}"`,
+			nil,
+			options.TerragruntOptions{TerragruntConfigPath: "/root/child/" + DefaultTerragruntConfigPath, NonInteractive: true},
+			util.CommaSeparatedStrings(TERRAFORM_COMMANDS_NEED_LOCKING),
+			nil,
+		},
+		{
+			`commands = ["${get_terraform_commands_that_need_vars()}"]`,
+			nil,
+			options.TerragruntOptions{TerragruntConfigPath: "/root/child/" + DefaultTerragruntConfigPath, NonInteractive: true},
+			fmt.Sprintf("commands = [%s]", util.CommaSeparatedStrings(TERRAFORM_COMMANDS_NEED_VARS)),
+			nil,
+		},
+		{
+			`commands = "test-${get_terraform_commands_that_need_vars()}"`,
+			nil,
+			options.TerragruntOptions{TerragruntConfigPath: "/root/child/" + DefaultTerragruntConfigPath, NonInteractive: true},
+			fmt.Sprintf(`commands = "test-%v"`, TERRAFORM_COMMANDS_NEED_VARS),
+			nil,
+		},
 	}
 
 	for _, testCase := range testCases {

diff --git a/test/integration_test.go b/test/integration_test.go
@@ -384,6 +384,17 @@ func TestExtraArgumentsWithRegion(t *testing.T) {
 	assert.Contains(t, out.String(), "Hello, World from Oregon!")
 }
 
+func TestPriorityOrderOfArgument(t *testing.T) {
+	// Do not use t.Parallel() on this test, it will infers with the other TestExtraArguments.* tests
+	out := new(bytes.Buffer)
+	injectedValue := "Injected-directly-by-argument"
+	runTerragruntRedirectOutput(t, fmt.Sprintf("terragrunt apply -var extra_var=%s --terragrunt-non-interactive --terragrunt-working-dir %s", injectedValue, TEST_FIXTURE_EXTRA_ARGS_PATH), out, os.Stderr)
+	t.Log(out.String())
+	// And the result value for test should be the injected variable since the injected arguments are injected before the suplied parameters,
+	// so our override of extra_var should be the last argument.
+	assert.Contains(t, out.String(), fmt.Sprintf("test = %s", injectedValue))
+}
+
 func cleanupTerraformFolder(t *testing.T, templatesPath string) {
 	removeFile(t, util.JoinPath(templatesPath, TERRAFORM_STATE))
 	removeFile(t, util.JoinPath(templatesPath, TERRAFORM_STATE_BACKUP))

diff --git a/util/collections.go b/util/collections.go
@@ -55,8 +55,8 @@ func removeDuplicatesFromList(list []string, keepLast bool) []string {
 	return out
 }
 
-// Returns an HCL compliant formated list of strings
-func ListToHCLArray(list []string) string {
+// CommaSeparatedStrings returns an HCL compliant formated list of strings (each string within double quote)
+func CommaSeparatedStrings(list []string) string {
 	values := make([]string, 0, len(list))
 	for _, value := range list {
 		values = append(values, fmt.Sprintf(`"%s"`, value))