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

Relative path for module prefix #2716

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
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
16 changes: 15 additions & 1 deletion configstack/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,21 @@ func resolveTerraformModule(terragruntConfigPath string, moduleMap map[string]*T
}

if opts.IncludeModulePrefix {
opts.OutputPrefix = fmt.Sprintf("[%v] ", modulePath)
prefix := modulePath
workingDirPath := terragruntOptions.WorkingDir

// try to convert working dir to absolute path
if path, err := filepath.Abs(workingDirPath); err == nil {
workingDirPath = path
}

if workingDirPath != modulePath {
// try to generate relative path to module to be used as prefix
if relativePrefix, err := filepath.Rel(workingDirPath, modulePath); err == nil {
prefix = relativePrefix
}
}
opts.OutputPrefix = fmt.Sprintf("[%v] ", prefix)
Comment on lines +334 to +348
Copy link
Contributor

@maunzCache maunzCache Oct 3, 2023

Choose a reason for hiding this comment

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

Edit: Just noticed this PR was in draft mode after spending an hour on it. Sorry if that comment came to early.

I was thinking about this part real hard because i thought there was another way and dug through some go implementation and came up with this:

Suggested change
prefix := modulePath
workingDirPath := terragruntOptions.WorkingDir
// try to convert working dir to absolute path
if path, err := filepath.Abs(workingDirPath); err == nil {
workingDirPath = path
}
if workingDirPath != modulePath {
// try to generate relative path to module to be used as prefix
if relativePrefix, err := filepath.Rel(workingDirPath, modulePath); err == nil {
prefix = relativePrefix
}
}
opts.OutputPrefix = fmt.Sprintf("[%v] ", prefix)
// try to convert working dir to absolute path
absoluteWorkingDirPath, err := filepath.Abs(terragruntOptions.WorkingDir); error != nil {
return _, err
}
modulePathPrefix := modulePath
if absoluteWorkingDirPath != modulePathPrefix {
// try to generate relative path to module to be used as prefix
modulePathPrefix, err = filepath.Rel(absoluteWorkingDirPath, modulePathPrefix); error != nil {
return _, err
}
}
opts.OutputPrefix = fmt.Sprintf("[%v] ", modulePathPrefix)

Let's go through this:

  1. The prefix var doesn't matter until later so move that down.
  2. If filepath.Abs or filepath.Rel errors, this happens on a system level or the input was garbage to begin with. Simply call it directly and pass the error
  3. prefix and modulePath should not differ during the runtime so pass it directly.

Also renamed the variables. Ignored it if its too verbose.

Note: I have not scrolled through the whole function so returning _ on error might be wrong.

So what do you think? I know it is a simple piece of code and i don't want to argue. Just exchanging thoughts.

}

return &TerraformModule{Path: modulePath, Config: *terragruntConfig, TerragruntOptions: opts}, nil
Expand Down
29 changes: 26 additions & 3 deletions test/integration_include_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,17 +141,40 @@ func TestTerragruntRunAllModulesWithPrefix(t *testing.T) {
stdoutLines := strings.Split(planOutput, "\n")
for _, line := range stdoutLines {
if strings.Contains(line, "alpha") {
assert.Contains(t, line, includeRunAllFixturePath+"a")
assert.Contains(t, line, "[a]")
}
if strings.Contains(line, "beta") {
assert.Contains(t, line, includeRunAllFixturePath+"b")
assert.Contains(t, line, "[b]")
}
if strings.Contains(line, "charlie") {
assert.Contains(t, line, includeRunAllFixturePath+"c")
assert.Contains(t, line, "[c]")
}
}
}

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

stdout := bytes.Buffer{}
stderr := bytes.Buffer{}
err := runTerragruntCommand(
t,
fmt.Sprintf(
"terragrunt run-all plan --terragrunt-non-interactive --terragrunt-include-module-prefix --terragrunt-working-dir %s --terragrunt-include-external-dependencies",
filepath.Join(TEST_FIXTURE_INCLUDE_PARENT, "app"),
),
&stdout,
&stderr,
)
require.NoError(t, err)
logBufferContentsLineByLine(t, stdout, "stdout")
logBufferContentsLineByLine(t, stderr, "stderr")

planOutput := stdout.String()
assert.Contains(t, planOutput, "[../dependency]")

}

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

Expand Down