Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #1124 #1134

Merged
merged 2 commits into from
Apr 15, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 84 additions & 0 deletions config/config_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ package config
import (
"fmt"
"path/filepath"
"regexp"
"strings"

"github.com/hashicorp/go-getter"
"github.com/hashicorp/hcl/v2"
tflang "github.com/hashicorp/terraform/lang"
"github.com/zclconf/go-cty/cty"
Expand Down Expand Up @@ -404,6 +407,69 @@ func getCleanedTargetConfigPath(configPath string, workingPath string) string {
return util.CleanPath(targetConfig)
}

// If one of the xxx-all commands is called with the --terragrunt-source parameter, then for each module, we need to
// build its own --terragrunt-source parameter by doing the following:
//
// 1. Read the source URL from the Terragrunt configuration of each module
// 2. Extract the path from that URL (the part after a double-slash)
// 3. Append the path to the --terragrunt-source parameter
//
// Example:
//
// --terragrunt-source: /source/infrastructure-modules
// source param in module's terragrunt.hcl: git::[email protected]:acme/infrastructure-modules.git//networking/vpc?ref=v0.0.1
//
// This method will return: /source/infrastructure-modules//networking/vpc
//
func GetTerragruntSourceForModule(sourcePath string, modulePath string, moduleTerragruntConfig *TerragruntConfig) (string, error) {
if sourcePath == "" || moduleTerragruntConfig.Terraform == nil || moduleTerragruntConfig.Terraform.Source == nil || *moduleTerragruntConfig.Terraform.Source == "" {
return "", nil
}

// use go-getter to split the module source string into a valid URL and subdirectory (if // is present)
moduleUrl, moduleSubdir := getter.SourceDirSubdir(*moduleTerragruntConfig.Terraform.Source)

// if both URL and subdir are missing, something went terribly wrong
if moduleUrl == "" && moduleSubdir == "" {
return "", errors.WithStackTrace(InvalidSourceUrl{ModulePath: modulePath, ModuleSourceUrl: *moduleTerragruntConfig.Terraform.Source, TerragruntSource: sourcePath})
}
// if only subdir is missing, check if we can obtain a valid module name from the URL portion
if moduleUrl != "" && moduleSubdir == "" {
moduleSubdirFromUrl, err := getModulePathFromSourceUrl(moduleUrl)
if err != nil {
return moduleSubdirFromUrl, err
}
return util.JoinTerraformModulePath(sourcePath, moduleSubdirFromUrl), nil
}

return util.JoinTerraformModulePath(sourcePath, moduleSubdir), nil
}

// Parse sourceUrl not containing '//', and attempt to obtain a module path.
// Example:
//
// sourceUrl = "git::ssh://[email protected]/OurOrg/module-name.git"
// will return "module-name".

func getModulePathFromSourceUrl(sourceUrl string) (string, error) {

// Regexp for module name extraction. It assumes that the query string has already been stripped off.
// Then we simply capture anything after the last slash, and before `.` or end of string.
var moduleNameRegexp = regexp.MustCompile(`(?:.+/)(.+?)(?:\.|$)`)

// strip off the query string if present
sourceUrl = strings.Split(sourceUrl, "?")[0]

matches := moduleNameRegexp.FindStringSubmatch(sourceUrl)

// if regexp returns less/more than the full match + 1 capture group, then something went wrong with regex (invalid source string)
if len(matches) != 2 {
return "", errors.WithStackTrace(ErrorParsingModulePath{ModuleSourceUrl: sourceUrl})
}

return matches[1], nil
}

// Custom error types
type WrongNumberOfParams struct {
Func string
Expand Down Expand Up @@ -457,3 +523,21 @@ type TerragruntConfigNotFound struct {
func (err TerragruntConfigNotFound) Error() string {
return fmt.Sprintf("Terragrunt config %s not found", err.Path)
}

type InvalidSourceUrl struct {
ModulePath string
ModuleSourceUrl string
TerragruntSource string
}

func (err InvalidSourceUrl) Error() string {
return fmt.Sprintf("The --terragrunt-source parameter is set to '%s', but the source URL in the module at '%s' is invalid: '%s'. Note that the module URL must have a double-slash to separate the repo URL from the path within the repo!", err.TerragruntSource, err.ModulePath, err.ModuleSourceUrl)
}

type ErrorParsingModulePath struct {
ModuleSourceUrl string
}

func (err ErrorParsingModulePath) Error() string {
return fmt.Sprintf("Unable to obtain the module path from the source URL '%s'. Ensure that the URL is in a supported format.", err.ModuleSourceUrl)
}
38 changes: 38 additions & 0 deletions config/config_helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -795,6 +795,44 @@ func TestReadTerragruntConfigLocals(t *testing.T) {
assert.Equal(t, localsMap["number_expression"].(float64), float64(42))
}

func TestGetTerragruntSourceForModuleHappyPath(t *testing.T) {
t.Parallel()

testCases := []struct {
config *TerragruntConfig
source string
expected string
}{
{mockConfigWithSource(""), "", ""},
{mockConfigWithSource(""), "/source/modules", ""},
{mockConfigWithSource("git::[email protected]:acme/modules.git//foo/bar"), "/source/modules", "/source/modules//foo/bar"},
{mockConfigWithSource("git::[email protected]:acme/modules.git//foo/bar?ref=v0.0.1"), "/source/modules", "/source/modules//foo/bar"},
{mockConfigWithSource("git::[email protected]:acme/emr_cluster.git?ref=feature/fix_bugs"), "/source/modules", "/source/modules//emr_cluster"},
{mockConfigWithSource("git::ssh://[email protected]/OurOrg/some-module.git"), "/source/modules", "/source/modules//some-module"},
{mockConfigWithSource("github.com/hashicorp/example"), "/source/modules", "/source/modules//example"},
{mockConfigWithSource("github.com/hashicorp/example//subdir"), "/source/modules", "/source/modules//subdir"},
{mockConfigWithSource("[email protected]:hashicorp/example.git//subdir"), "/source/modules", "/source/modules//subdir"},
{mockConfigWithSource("./some/path//to/modulename"), "/source/modules", "/source/modules//to/modulename"},
}

for _, testCase := range testCases {
// The following is necessary to make sure testCase's values don't
// get updated due to concurrency within the scope of t.Run(..) below
testCase := testCase
t.Run(fmt.Sprintf("%v-%s", *testCase.config.Terraform.Source, testCase.source), func(t *testing.T) {
actual, err := GetTerragruntSourceForModule(testCase.source, "mock-for-test", testCase.config)
require.NoError(t, err)
assert.Equal(t, testCase.expected, actual)
})
}
}

func mockConfigWithSource(sourceUrl string) *TerragruntConfig {
cfg := TerragruntConfig{IsPartial: true}
cfg.Terraform = &TerraformConfig{Source: &sourceUrl}
return &cfg
}

// Return keys as a map so it is treated like a set, and order doesn't matter when comparing equivalence
func getKeys(valueMap map[string]cty.Value) map[string]bool {
keys := map[string]bool{}
Expand Down
25 changes: 25 additions & 0 deletions config/dependency.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"strings"
"sync"

"github.com/hashicorp/go-getter"
"github.com/hashicorp/hcl/v2"
"github.com/zclconf/go-cty/cty"
"github.com/zclconf/go-cty/cty/gocty"
Expand Down Expand Up @@ -353,6 +354,30 @@ func cloneTerragruntOptionsForDependencyOutput(terragruntOptions *options.Terrag
targetOptions.DownloadDir = downloadDir
}

// If the Source is set, then we need to recompute it in the context of the target config.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to make sure I understand here... If terragrunt.hcl has, say:

terraform {
  source = "github.com/org/modules.git//module-a"
}

dependency "b" {
  path = "../module-b"
}

And module-b/terragrunt.hcl contains:

terraform {
  source = "github.com/org/modules.git//module-b"
}

You're saying that if someone runs terragrunt apply --terragrunt-source /local/path/to/modules, then we are overriding the source parameter in both the terragrunt.hcl of module-a and of module-b?

If so, then this highlights two issues with the --terragrunt-source design:

  1. If you run terragrunt apply --terragrunt-source /foo/bar in folder yyy, as of now, the only thing it does is override the source URL in yyy/terragrunt.hcl to the exact value /foo/bar. However, if you run terragrunt apply-all --terragrunt-source /foo/bar in folder yyy, it overrides the source value of each child module within yyy/sub/path to something like /foo/bar/sub/path. The inconsistency between these two behaviors is being highlighted in this PR. Would it make sense to always use the latter behavior? That is --terragrunt-source should always be set to the root of your modules repo?
  2. This also highlights a limitation of --terragrunt-source: it assumes all your modules are coming from the same root repo. You won't be able to do an apply-all if you're pulling from multiple repos (e.g., your own infra modules, Terraform Registry, Gruntwork repos, etc).

Perhaps a better solution to this is to support some sort of mapping?

terragrunt apply --terragrunt-source-map source:dest

The --terragrunt-source-map param would replace any source URL that starts with source to instead start with dest.

For example:

terragrunt apply --terragrunt-source-map github.com/org/modules.git:/local/path/to/modules

The above would replace source = "github.com/org/modules.git//xxx" with /local/path/to/modules//xxx regardless of whether you were running apply, or apply-all, or using a dependency. This would give us a consistent behavior across all command and support mapping multiple repos by specifying the param multiple times:

terragrunt apply \
  --terragrunt-source-map github.com/org/modules.git:/local/path/to/modules \
  --terragrunt-source-map github.com/org/another-repo.git:/local/path/to/another-repo

We could also consider allowing users to specify this mapping in a file:

terragrunt apply --terragrunt-source-map mapping.hcl

Where mapping.hcl contains:

mappings = {
  "github.com/org/modules.git"      = "/local/path/to/modules"
  "github.com/org/another-repo.git" = "/local/path/to/another-repo"  
}

Note, I realize the above is a bigger change, so it could also be filed as an issue and done in a separate PR 😁

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed #1138

By the way, are you suggesting we close this PR in favor of solving for the mapping case, or are you ok with the band-aid fix here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed #1138

Thanks! I just updated it with the more detailed description above.

By the way, are you suggesting we close this PR in favor of solving for the mapping case, or are you ok with the band-aid fix here?

Your call! I'm OK with either approach. Fixing #1138 would be great, but definitely isn't a high priority, so a band-aid fix for now would be fine too. #1138 might make for a good trial project in the future, too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. +1 to #1138 being a good trial project. Thanks for the sanity check!

if terragruntOptions.Source != "" {
// We need the terraform source of the target config to compute the actual source to use
partialParseIncludedConfig, err := PartialParseConfigFile(
targetConfig,
targetOptions,
nil,
[]PartialDecodeSectionType{TerraformBlock},
)
if err != nil {
return nil, err
}
// Update the source value to be everything before "//" so that it can be recomputed
moduleUrl, _ := getter.SourceDirSubdir(terragruntOptions.Source)

// Finally, update the source to be the combined path between the terraform source in the target config, and the
// value before "//" in the original terragrunt options.
targetSource, err := GetTerragruntSourceForModule(moduleUrl, filepath.Dir(targetConfig), partialParseIncludedConfig)
if err != nil {
return nil, err
}
targetOptions.Source = targetSource
}

return targetOptions, nil
}

Expand Down
85 changes: 1 addition & 84 deletions configstack/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package configstack
import (
"fmt"
"path/filepath"
"regexp"
"sort"
"strings"

Expand All @@ -12,7 +11,6 @@ import (
"github.com/gruntwork-io/terragrunt/options"
"github.com/gruntwork-io/terragrunt/shell"
"github.com/gruntwork-io/terragrunt/util"
"github.com/hashicorp/go-getter"
zglob "github.com/mattn/go-zglob"
)

Expand Down Expand Up @@ -260,7 +258,7 @@ func resolveTerraformModule(terragruntConfigPath string, terragruntOptions *opti
return nil, errors.WithStackTrace(ErrorProcessingModule{UnderlyingError: err, HowThisModuleWasFound: howThisModuleWasFound, ModulePath: terragruntConfigPath})
}

terragruntSource, err := getTerragruntSourceForModule(modulePath, terragruntConfig, terragruntOptions)
terragruntSource, err := config.GetTerragruntSourceForModule(terragruntOptions.Source, modulePath, terragruntConfig)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -295,69 +293,6 @@ func resolveTerraformModule(terragruntConfigPath string, terragruntOptions *opti
return &TerraformModule{Path: modulePath, Config: *terragruntConfig, TerragruntOptions: opts}, nil
}

// If one of the xxx-all commands is called with the --terragrunt-source parameter, then for each module, we need to
// build its own --terragrunt-source parameter by doing the following:
//
// 1. Read the source URL from the Terragrunt configuration of each module
// 2. Extract the path from that URL (the part after a double-slash)
// 3. Append the path to the --terragrunt-source parameter
//
// Example:
//
// --terragrunt-source: /source/infrastructure-modules
// source param in module's terragrunt.hcl: git::[email protected]:acme/infrastructure-modules.git//networking/vpc?ref=v0.0.1
//
// This method will return: /source/infrastructure-modules//networking/vpc
//
func getTerragruntSourceForModule(modulePath string, moduleTerragruntConfig *config.TerragruntConfig, terragruntOptions *options.TerragruntOptions) (string, error) {
if terragruntOptions.Source == "" || moduleTerragruntConfig.Terraform == nil || moduleTerragruntConfig.Terraform.Source == nil || *moduleTerragruntConfig.Terraform.Source == "" {
return "", nil
}

// use go-getter to split the module source string into a valid URL and subdirectory (if // is present)
moduleUrl, moduleSubdir := getter.SourceDirSubdir(*moduleTerragruntConfig.Terraform.Source)

// if both URL and subdir are missing, something went terribly wrong
if moduleUrl == "" && moduleSubdir == "" {
return "", errors.WithStackTrace(InvalidSourceUrl{ModulePath: modulePath, ModuleSourceUrl: *moduleTerragruntConfig.Terraform.Source, TerragruntSource: terragruntOptions.Source})
}
// if only subdir is missing, check if we can obtain a valid module name from the URL portion
if moduleUrl != "" && moduleSubdir == "" {
moduleSubdirFromUrl, err := getModulePathFromSourceUrl(moduleUrl)
if err != nil {
return moduleSubdirFromUrl, err
}
return util.JoinTerraformModulePath(terragruntOptions.Source, moduleSubdirFromUrl), nil
}

return util.JoinTerraformModulePath(terragruntOptions.Source, moduleSubdir), nil
}

// Parse sourceUrl not containing '//', and attempt to obtain a module path.
// Example:
//
// sourceUrl = "git::ssh://[email protected]/OurOrg/module-name.git"
// will return "module-name".

func getModulePathFromSourceUrl(sourceUrl string) (string, error) {

// Regexp for module name extraction. It assumes that the query string has already been stripped off.
// Then we simply capture anything after the last slash, and before `.` or end of string.
var moduleNameRegexp = regexp.MustCompile(`(?:.+/)(.+?)(?:\.|$)`)

// strip off the query string if present
sourceUrl = strings.Split(sourceUrl, "?")[0]

matches := moduleNameRegexp.FindStringSubmatch(sourceUrl)

// if regexp returns less/more than the full match + 1 capture group, then something went wrong with regex (invalid source string)
if len(matches) != 2 {
return "", errors.WithStackTrace(ErrorParsingModulePath{ModuleSourceUrl: sourceUrl})
}

return matches[1], nil
}

// Look through the dependencies of the modules in the given map and resolve the "external" dependency paths listed in
// each modules config (i.e. those dependencies not in the given list of Terragrunt config canonical file paths).
// These external dependencies are outside of the current working directory, which means they may not be part of the
Expand Down Expand Up @@ -554,24 +489,6 @@ func (err ErrorProcessingModule) Error() string {
return fmt.Sprintf("Error processing module at '%s'. How this module was found: %s. Underlying error: %v", err.ModulePath, err.HowThisModuleWasFound, err.UnderlyingError)
}

type InvalidSourceUrl struct {
ModulePath string
ModuleSourceUrl string
TerragruntSource string
}

func (err InvalidSourceUrl) Error() string {
return fmt.Sprintf("The --terragrunt-source parameter is set to '%s', but the source URL in the module at '%s' is invalid: '%s'. Note that the module URL must have a double-slash to separate the repo URL from the path within the repo!", err.TerragruntSource, err.ModulePath, err.ModuleSourceUrl)
}

type ErrorParsingModulePath struct {
ModuleSourceUrl string
}

func (err ErrorParsingModulePath) Error() string {
return fmt.Sprintf("Unable to obtain the module path from the source URL '%s'. Ensure that the URL is in a supported format.", err.ModuleSourceUrl)
}

type InfiniteRecursion struct {
RecursionLevel int
Modules map[string]*TerraformModule
Expand Down
48 changes: 0 additions & 48 deletions configstack/module_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package configstack

import (
"fmt"
"os"
"testing"

Expand Down Expand Up @@ -771,53 +770,6 @@ func TestResolveTerraformModuleNoTerraformConfig(t *testing.T) {
assertModuleListsEqual(t, expected, actualModules)
}

func TestGetTerragruntSourceForModuleHappyPath(t *testing.T) {
t.Parallel()

testCases := []struct {
config *config.TerragruntConfig
opts *options.TerragruntOptions
expected string
}{
{mockConfigWithSource(""), mockOptionsWithSource(t, ""), ""},
{mockConfigWithSource(""), mockOptionsWithSource(t, "/source/modules"), ""},
{mockConfigWithSource("git::[email protected]:acme/modules.git//foo/bar"), mockOptionsWithSource(t, "/source/modules"), "/source/modules//foo/bar"},
{mockConfigWithSource("git::[email protected]:acme/modules.git//foo/bar?ref=v0.0.1"), mockOptionsWithSource(t, "/source/modules"), "/source/modules//foo/bar"},
{mockConfigWithSource("git::[email protected]:acme/emr_cluster.git?ref=feature/fix_bugs"), mockOptionsWithSource(t, "/source/modules"), "/source/modules//emr_cluster"},
{mockConfigWithSource("git::ssh://[email protected]/OurOrg/some-module.git"), mockOptionsWithSource(t, "/source/modules"), "/source/modules//some-module"},
{mockConfigWithSource("github.com/hashicorp/example"), mockOptionsWithSource(t, "/source/modules"), "/source/modules//example"},
{mockConfigWithSource("github.com/hashicorp/example//subdir"), mockOptionsWithSource(t, "/source/modules"), "/source/modules//subdir"},
{mockConfigWithSource("[email protected]:hashicorp/example.git//subdir"), mockOptionsWithSource(t, "/source/modules"), "/source/modules//subdir"},
{mockConfigWithSource("./some/path//to/modulename"), mockOptionsWithSource(t, "/source/modules"), "/source/modules//to/modulename"},
}

for _, testCase := range testCases {
// The following is necessary to make sure testCase's values don't
// get updated due to concurrency within the scope of t.Run(..) below
testCase := testCase
t.Run(fmt.Sprintf("%v-%s", *testCase.config.Terraform.Source, testCase.opts.Source), func(t *testing.T) {
actual, err := getTerragruntSourceForModule("mock-for-test", testCase.config, testCase.opts)
require.NoError(t, err)
assert.Equal(t, testCase.expected, actual)
})
}
}

func mockOptionsWithSource(t *testing.T, sourceUrl string) *options.TerragruntOptions {
opts, err := options.NewTerragruntOptionsForTest("mock-for-test.hcl")
if err != nil {
t.Fatalf("Error creating terragrunt options for test %v", err)
}
opts.Source = sourceUrl
return opts
}

func mockConfigWithSource(sourceUrl string) *config.TerragruntConfig {
cfg := config.TerragruntConfig{IsPartial: true}
cfg.Terraform = &config.TerraformConfig{Source: &sourceUrl}
return &cfg
}

func ptr(str string) *string {
return &str
}
11 changes: 11 additions & 0 deletions test/fixture-get-output/regression-1124/live/app/terragrunt.hcl
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
terraform {
source = "../../modules//app"
}

dependency "dep" {
config_path = "../dependency"
}

inputs = {
foo = dependency.dep.outputs.foo
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
terraform {
source = "../../modules//dependency"
}
2 changes: 2 additions & 0 deletions test/fixture-get-output/regression-1124/modules/app/main.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
variable "foo" {}
output "foo" { value = var.foo }
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
resource "random_string" "random" {
length = 16
}

output "foo" {
value = random_string.random.result
}
Loading