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.

Sign up for GitHub

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

Jump to bottom

Relative path for module prefix #2716

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Relative path for module prefix #2716

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1c8bdd6
Add setting of relative path for modules / absolute path for app
denis256 Sep 14, 2023
2630fcd
Tests update
denis256 Sep 15, 2023
6759775
Add relative path to module output
denis256 Sep 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 15 additions & 1 deletion configstack/module.go
View file
Open in desktop
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 •
edited

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
View file
Open in desktop
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