Skip to content

fix: Removing extra --auth-provider-cmd call - #6045

Merged
yhakbar merged 2 commits into
mainfrom
fix/removing-extra-auth-provider-cmd-call
May 6, 2026
Merged

fix: Removing extra --auth-provider-cmd call#6045
yhakbar merged 2 commits into
mainfrom
fix/removing-extra-auth-provider-cmd-call

Conversation

@yhakbar

@yhakbar yhakbar commented May 6, 2026

Copy link
Copy Markdown
Collaborator

Description

Removes additional call to the authentication provider command, preventing calls in .terragrunt-cache directories in favor of the ones that are called in unit directories beforehand for dependencies.

TODOs

Read the Gruntwork contribution guidelines.

  • I authored this code entirely myself
  • I am submitting code based on open source software (e.g. MIT, MPL-2.0, Apache)
  • I am adding or upgrading a dependency or adapted code and confirm it has a compatible open source license
  • Update the docs.
  • Update the changelog in the docs.
  • Run the relevant tests successfully, including pre-commit checks.
  • This change is backwards compatible.
  • If this change is not forwards compatible (e.g. a new feature), it is gated behind a feature flag.

Summary by CodeRabbit

  • Refactor
    • Centralized credential and IAM handling, replacing per-call credential plumbing with a merged IAM option and provider-based credential resolution for Terraform runs.
  • Tests
    • Added an integration test covering auth-provider invocation ordering and call-count under concurrent runs.
    • Added test fixtures to exercise auth-provider command behavior and dependency flows.
  • Chores
    • Updated test ignores to include generated JSONL logs.

@vercel

vercel Bot commented May 6, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
terragrunt-docs Ready Ready Preview, Comment May 6, 2026 7:43pm

Request Review

@coderabbitai

coderabbitai Bot commented May 6, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

Centralizes IAM credential handling in dependency output retrieval by merging IAM options and using an AWS provider-based credential setup, adds setupTFRunOptsForBareTerraform, removes per-call credsGetter parameters, and adds an integration test and fixtures exercising an auth-provider-cmd invocation counting scenario.

Changes

Dependency Output Credential Centralization

Layer / File(s) Summary
Data Shape / Signatures
pkg/config/dependency.go
Removed iamRoleOpts and credsGetter parameters from getTerragruntOutputJSONFromInitFolder signature.
Credential Merge / Provider Init
pkg/config/dependency.go (multiple blocks)
Rewrote credential setup to merge IAM options and obtain credentials via the AWS provider in both init-folder and remote-state dependency output flows.
TF Run Options Helper
pkg/config/dependency.go (lines ~1343–1357)
Added setupTFRunOptsForBareTerraform(pctx, workingDir) to build Terraform run options without explicit credential args; replaced prior per-call TF options construction with this helper.
Call Site Updates
pkg/config/dependency.go (lines ~1053–1220)
Updated init-folder and remote-state output retrieval paths to call the new helper and use merged IAM/provider-based credential resolution.
Tests / Fixtures (related runtime behavior)
test/...
No changes in this cohort; test fixtures added are handled in separate cohort below.

Auth-Provider Command Test Fixtures and Integration Test

Layer / File(s) Summary
Test Fixtures
test/fixtures/auth-provider-cmd/run-all-call-count/auth-provider.sh, .../.gitignore
Added auth-provider.sh that logs each invocation to calls.jsonl and writes an env marker; added *.jsonl to .gitignore.
Terraform Fixture Modules
test/fixtures/auth-provider-cmd/run-all-call-count/dep/*, .../dependent/*, .../root.hcl
Added simple Terraform configs and Terragrunt HCL for dep and dependent modules and a root.hcl remote_state block to exercise run-all behavior.
Integration Test
test/integration_test.go (new function)
Added TestReadTerragruntAuthProviderCmdRunAllCallCountWithRacing which runs --all apply, parses calls.jsonl, asserts exactly five auth-provider invocations, and verifies WorkingDir paths exclude the Terragrunt cache dir.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~40 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main objective of removing an extra authentication provider command call, directly matching the primary change across the codebase.
Description check ✅ Passed The description explains the fix and most checklist items are checked, though the 'authored entirely myself' item is unchecked, which is acceptable for open-source submissions.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/removing-extra-auth-provider-cmd-call

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 Trivy (0.69.3)

Failed to read Trivy output file: ENOENT: no such file or directory, open '/inmem/1273/nsjail-d9ceaa76-3ed6-436f-9d0c-a7bce399475e/merged/.trivy-output.json'


Comment @coderabbitai help to get the list of available commands and usage tips.

@yhakbar
yhakbar marked this pull request as ready for review May 6, 2026 18:55
@yhakbar
yhakbar requested a review from denis256 as a code owner May 6, 2026 18:55
denis256
denis256 previously approved these changes May 6, 2026
Comment thread pkg/config/dependency.go

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
test/integration_test.go (1)

3354-3367: ⚡ Quick win

Assert the per-module invocation pattern, not just the total count.

assert.Len(t, calls, 5) plus the cache-dir check will still pass if the extra auth-provider invocation moves to the wrong unit but the total stays at five. Since this regression is specifically about prefetching in unit directories for dependencies, please also assert the expected working-dir distribution (dep = 3, dependent = 2) or the exact documented sequence.

Proposed assertion tightening
 	assert.Len(t, calls, 5)
 
+	expectedCounts := map[string]int{
+		filepath.Join(rootPath, "dep"):       3,
+		filepath.Join(rootPath, "dependent"): 2,
+	}
+	actualCounts := map[string]int{}
+
 	for _, call := range calls {
 		assert.NotContains(t, call.WorkingDir, ".terragrunt-cache")
+		actualCounts[call.WorkingDir]++
 	}
+
+	assert.Equal(t, expectedCounts, actualCounts)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/integration_test.go` around lines 3354 - 3367, Tighten the test by
asserting the per-module invocation pattern instead of only total count: compute
counts for each call.WorkingDir from the calls slice and assert that the
working-dir corresponding to the "dep" module appears exactly 3 times and the
"dependent" module appears exactly 2 times (or alternatively assert the exact
documented sequence of WorkingDir values matches the expected 5-element order).
Update the assertions surrounding calls and the cache-dir check to verify these
per-module counts/sequence so the regression (extra auth-provider invocation
moving to the wrong unit) is caught.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@test/integration_test.go`:
- Around line 3354-3367: Tighten the test by asserting the per-module invocation
pattern instead of only total count: compute counts for each call.WorkingDir
from the calls slice and assert that the working-dir corresponding to the "dep"
module appears exactly 3 times and the "dependent" module appears exactly 2
times (or alternatively assert the exact documented sequence of WorkingDir
values matches the expected 5-element order). Update the assertions surrounding
calls and the cache-dir check to verify these per-module counts/sequence so the
regression (extra auth-provider invocation moving to the wrong unit) is caught.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: cca5019f-1d39-4ea8-b225-82acce4a5864

📥 Commits

Reviewing files that changed from the base of the PR and between edb722c and 8ba47f2.

📒 Files selected for processing (8)
  • test/fixtures/auth-provider-cmd/run-all-call-count/.gitignore
  • test/fixtures/auth-provider-cmd/run-all-call-count/auth-provider.sh
  • test/fixtures/auth-provider-cmd/run-all-call-count/dep/main.tf
  • test/fixtures/auth-provider-cmd/run-all-call-count/dep/terragrunt.hcl
  • test/fixtures/auth-provider-cmd/run-all-call-count/dependent/main.tf
  • test/fixtures/auth-provider-cmd/run-all-call-count/dependent/terragrunt.hcl
  • test/fixtures/auth-provider-cmd/run-all-call-count/root.hcl
  • test/integration_test.go

@yhakbar
yhakbar merged commit 9c49dd4 into main May 6, 2026
69 of 71 checks passed
@yhakbar
yhakbar deleted the fix/removing-extra-auth-provider-cmd-call branch May 6, 2026 21:36
Rahul-Kumar-prog pushed a commit to Rahul-Kumar-prog/terragrunt that referenced this pull request May 10, 2026
* fix: Removing extra `--auth-provider-cmd` call

* chore: Adding `TestReadTerragruntAuthProviderCmdRunAllCallCountWithRacing`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants