fix: Avoid discovering dependencies if they are disabled #5119

yhakbar · 2025-11-18T14:35:08Z

Description

fixed disabled discovery detection
added regression test

Fixes #4977.

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.
Run the relevant tests successfully, including pre-commit checks.
Include release notes. If this PR is backward incompatible, include a migration guide.

Release Notes (draft)

Added / Removed / Updated [X].

Migration Guide

Summary by CodeRabbit

Release Notes

Bug Fixes
- Fixed an issue where disabled dependencies with empty configuration paths would incorrectly trigger cycle detection errors during discovery operations.

vercel · 2025-11-18T14:35:14Z

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

Project	Deployment	Preview	Comments	Updated (UTC)
terragrunt-docs	Ready	Preview	Comment	Nov 19, 2025 4:31pm

…scovery' into fix/fixing-cycle-detection-in-discovery

coderabbitai · 2025-11-19T16:22:15Z

📝 Walkthrough

Walkthrough

The changes add logic to skip disabled dependencies during path extraction in the discovery process. Test fixtures and integration tests validate that disabled dependencies with empty config paths do not trigger cycle detection errors while maintaining proper dependency discovery functionality.

Changes

Cohort / File(s)	Summary
Discovery Logic `internal/discovery/discovery.go`	Adds early continue statement in `extractDependencyPaths` to skip processing dependencies when `dependency.Enabled` is explicitly set to false.
Discovery Tests `internal/discovery/discovery_test.go`	Adds two new test cases: `TestDiscoveryDetectsCycle` validates cycle detection in mutual dependencies, and `TestDiscoveryDoesntDetectCycleWhenDisabled` verifies that disabled dependencies prevent cycle detection.
Integration Tests `test/integration_regressions_test.go`	Adds `TestDisabledDependencyEmptyConfigPath_NoCycleError` regression test verifying disabled dependencies with empty config paths do not trigger cycle errors.
Test Fixtures `test/fixtures/regressions/disabled-dependency-empty-config-path/root.hcl`, `unit-a/terragrunt.hcl`, `unit-b/terragrunt.hcl`	Terragrunt configuration files establishing test scenario with mutual dependencies where one is disabled.
Terraform Module Fixture `test/fixtures/regressions/disabled-dependency-empty-config-path/modules/id/main.tf`	Terraform module defining a random string generator with optional prefix/suffix formatting capabilities.

Sequence Diagram

sequenceDiagram
    participant Discovery as Discovery Process
    participant DepLoop as Dependency Loop
    participant DependencyA as Dependency (Enabled)
    participant DependencyB as Dependency (Enabled=false)
    participant PathList as Extracted Paths

    Discovery->>DepLoop: iterate cfg.TerragruntDependencies
    DepLoop->>DependencyA: check Enabled status
    alt Enabled = false
        DepLoop->>DepLoop: continue (skip processing)
    else Enabled = true or nil
        DepLoop->>DependencyA: validate ConfigPath
        DependencyA->>PathList: append to paths
    end
    DepLoop->>DependencyB: check Enabled status
    alt Enabled = false
        DepLoop->>DepLoop: continue (SKIP)
        Note over DepLoop,PathList: Disabled dependency not added to paths<br/>Prevents false cycle detection
    end
    DepLoop->>PathList: return extracted paths

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

internal/discovery/discovery.go: Simple conditional logic to skip disabled dependencies; verify correct placement within loop and no side effects on enabled dependencies.
Test fixtures and scenarios: Verify the test setup correctly represents the regression case (nested stacks with mutual disabled dependencies) and that cycle detection behavior is properly validated.
Integration test: Confirm test assertions align with expected discovery behavior and plan/run outputs.

Possibly related PRs

feat: Adding support for graph expressions in --filter #5049: Modifies discovery behavior and tests in internal/discovery around dependency extraction and cycle handling, directly related to cycle detection logic.
fix: Fixing usage of dependencies in discovery #4429: Modifies dependency iteration in internal/discovery/discovery.go, affecting which dependencies are processed during discovery.
chore: experiments tests improvements #4782: Modifies internal/discovery/discovery.go for dependency processing; the Enabled-field skip logic is directly related to overall dependency handling changes.

Suggested reviewers

ThisGuyCodes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix: Avoid discovering dependencies if they are disabled' directly describes the main change: skipping disabled dependencies in discovery to prevent cycle errors.
Description check	✅ Passed	The PR description includes issue reference, brief change summary, completed TODOs checklist, and release notes section; all key template sections are addressed.
Linked Issues check	✅ Passed	Code changes implement the fix for issue #4977 by preventing disabled dependencies from being included in discovery, which resolves the missing units in runner pool.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing cycle detection in dependency discovery when dependencies are disabled; test fixtures and regression tests support the core fix.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch fix/fixing-cycle-detection-in-discovery

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

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

internal/discovery/discovery_test.go (1)
930-1040: Cycle tests look good; consider fixing a misleading comment

The two tests correctly cover:

a real cycle (foo ↔ bar) being detected, and

the same shape with one edge disabled not producing a cycle.

Minor nit: in TestDiscoveryDoesntDetectCycleWhenDisabled the comment above CycleCheck still says “Verify that a cycle is detected”, which contradicts the assertion.

You could clarify with a tiny edit:
-	// Verify that a cycle is detected
+	// Verify that no cycle is detected

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bfcf295 and 434ae3f.

📒 Files selected for processing (7)

internal/discovery/discovery.go (1 hunks)
internal/discovery/discovery_test.go (1 hunks)
test/fixtures/regressions/disabled-dependency-empty-config-path/modules/id/main.tf (1 hunks)
test/fixtures/regressions/disabled-dependency-empty-config-path/root.hcl (1 hunks)
test/fixtures/regressions/disabled-dependency-empty-config-path/unit-a/terragrunt.hcl (1 hunks)
test/fixtures/regressions/disabled-dependency-empty-config-path/unit-b/terragrunt.hcl (1 hunks)
test/integration_regressions_test.go (2 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.go

⚙️ CodeRabbit configuration file

Review the Go code for quality and correctness. Make sure that the Go code follows best practices, is performant, and is easy to understand and maintain.

Files:

internal/discovery/discovery_test.go
internal/discovery/discovery.go
test/integration_regressions_test.go

🧠 Learnings (4)

📚 Learning: 2025-02-10T23:20:04.295Z

Learnt from: yhakbar
Repo: gruntwork-io/terragrunt PR: 3868
File: docs-starlight/patches/@astrojs%2Fstarlight@0.31.1.patch:33-33
Timestamp: 2025-02-10T23:20:04.295Z
Learning: In Terragrunt projects, all `.hcl` files can be assumed to be Terragrunt configurations by default, with specific exceptions like `.terraform.lock.hcl` that need explicit handling.

Applied to files:

test/fixtures/regressions/disabled-dependency-empty-config-path/unit-a/terragrunt.hcl

📚 Learning: 2025-11-03T04:40:01.000Z

Learnt from: ThisGuyCodes
Repo: gruntwork-io/terragrunt PR: 5041
File: test/fixtures/hclvalidate/valid/duplicate-attributes-in-required-providers/main.tf:2-7
Timestamp: 2025-11-03T04:40:01.000Z
Learning: In the terragrunt repository, test fixtures under test/fixtures/hclvalidate/valid/ are intentionally testing that INPUT validation succeeds even when Terraform code contains syntax errors or other issues unrelated to input validation (e.g., duplicate attributes, circular references, invalid locals). The "valid" designation means "valid for input validation purposes" not "syntactically valid Terraform/OpenTofu code".

Applied to files:

test/fixtures/regressions/disabled-dependency-empty-config-path/unit-a/terragrunt.hcl
test/fixtures/regressions/disabled-dependency-empty-config-path/unit-b/terragrunt.hcl

📚 Learning: 2025-08-19T16:05:54.723Z

Learnt from: Resonance1584
Repo: gruntwork-io/terragrunt PR: 4683
File: go.mod:86-90
Timestamp: 2025-08-19T16:05:54.723Z
Learning: When analyzing Go module dependencies for removal, always check for both direct imports and API usage across all Go files in the repository, not just a quick search. The github.com/mattn/go-zglob library is used for filesystem walking and glob expansion in multiple Terragrunt files including util/file.go, format commands, and AWS provider patch functionality.

Applied to files:

internal/discovery/discovery.go

📚 Learning: 2025-02-10T13:36:19.542Z

Learnt from: levkohimins
Repo: gruntwork-io/terragrunt PR: 3723
File: cli/commands/stack/action.go:160-160
Timestamp: 2025-02-10T13:36:19.542Z
Learning: The project uses a custom error package `github.com/gruntwork-io/terragrunt/internal/errors` which provides similar functionality to `fmt.Errorf` but includes stack traces. Prefer using this package's error functions (e.g., `errors.Errorf`, `errors.New`) over the standard library's error handling.

Applied to files:

internal/discovery/discovery.go

🧬 Code graph analysis (2)

internal/discovery/discovery_test.go (3)

options/options.go (1)

NewTerragruntOptions (380-382)

test/helpers/logger/logger.go (1)

CreateLogger (9-14)

internal/discovery/constructor.go (1)

NewDiscovery (153-172)

test/integration_regressions_test.go (2)

test/helpers/package.go (4)

CleanupTerraformFolder (882-889)

CopyEnvironment (89-105)

CreateGitRepo (1103-1108)

RunTerragruntCommandWithOutput (1007-1011)

util/file.go (1)

JoinPath (626-628)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

GitHub Check: integration_tests / Test (AWS Tofu)
GitHub Check: Pull Request has non-contributor approval

🔇 Additional comments (7)

test/fixtures/regressions/disabled-dependency-empty-config-path/root.hcl (1)

1-1: Minimal root fixture is fine

Single-line comment is enough for this regression fixture and keeps the root config intentionally empty.

internal/discovery/discovery.go (1)

1379-1395: Skipping disabled dependencies in path extraction matches desired semantics

Short‑circuiting when dependency.Enabled is explicitly false ensures disabled dependencies don’t contribute edges to the dependency graph or trigger config_path validation/empty‑path errors, which is exactly what the regression tests exercise.

test/fixtures/regressions/disabled-dependency-empty-config-path/unit-a/terragrunt.hcl (1)

1-9: Unit‑a fixture is clear and minimal

Including root.hcl, wiring terraform.source to ../modules/id, and leaving inputs empty keeps this unit focused and suitable as a simple peer for the regression scenario.

test/integration_regressions_test.go (2)

16-23: New regression fixture constant is consistent

The testFixtureDisabledDependencyEmptyConfigPath constant matches the new fixture path and aligns with the existing naming pattern for regression fixtures.

376-421: Integration test robustly captures the disabled‑dependency regression

TestDisabledDependencyEmptyConfigPath_NoCycleError exercises both a direct plan on unit-b and a run --all plan from the fixture root, asserting:

no error exit,

no “cycle”/“Cycle detected” messages, and

no “has empty config_path” message.

This is a solid end‑to‑end guard for the fix.

test/fixtures/regressions/disabled-dependency-empty-config-path/unit-b/terragrunt.hcl (1)

1-31: Fixture accurately encodes “disabled dependency with empty config_path”

Defining unit_a_path = "" and using config_path = try(local.unit_a_path, "") together with enabled = false is a precise repro for the regression, and guarding output access with try(dependency.unit_a.outputs.random_string, "") keeps evaluation safe when the dependency is ignored.

test/fixtures/regressions/disabled-dependency-empty-config-path/modules/id/main.tf (1)

1-32: ID helper module fixture is straightforward and reusable

The small random_string-based module with prefix/suffix/separator variables and an id output is clean and generic, appropriate as a shared building block for the regression fixtures.

fix: Avoid discovering dependencies if they are disabled

c5b3867

vercel bot deployed to Preview November 18, 2025 14:36 View deployment

denis256 added 3 commits November 19, 2025 16:20

chore: added regression tests for disabled dependencies

5a8d31a

Merge remote-tracking branch 'origin/fix/fixing-cycle-detection-in-di…

e2e2703

…scovery' into fix/fixing-cycle-detection-in-discovery

chore: added regression test

b869c49

vercel bot deployed to Preview November 19, 2025 16:23 View deployment

chore: regression tests

434ae3f

vercel bot deployed to Preview November 19, 2025 16:31 View deployment

denis256 marked this pull request as ready for review November 19, 2025 16:57

denis256 requested review from ThisGuyCodes and denis256 as code owners November 19, 2025 16:57

coderabbitai bot reviewed Nov 19, 2025

View reviewed changes

odgrim approved these changes Nov 20, 2025

View reviewed changes

denis256 merged commit 25150a6 into main Nov 20, 2025
50 checks passed

denis256 deleted the fix/fixing-cycle-detection-in-discovery branch November 20, 2025 15:15

BrewTestBot mentioned this pull request Nov 20, 2025

terragrunt 0.93.10 Homebrew/homebrew-core#255275

Merged

coderabbitai bot mentioned this pull request Nov 28, 2025

feat: improve disabled dependency management #5155

Open

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix: Avoid discovering dependencies if they are disabled #5119

fix: Avoid discovering dependencies if they are disabled #5119

Uh oh!

yhakbar commented Nov 18, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

vercel bot commented Nov 18, 2025 •

edited

Loading

Uh oh!

coderabbitai bot commented Nov 19, 2025 •

edited

Loading

Walkthrough

Changes

Sequence Diagram

Estimated code review effort

Possibly related PRs

Suggested reviewers

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

fix: Avoid discovering dependencies if they are disabled #5119

fix: Avoid discovering dependencies if they are disabled #5119

Uh oh!

Conversation

yhakbar commented Nov 18, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

TODOs

Release Notes (draft)

Migration Guide

Summary by CodeRabbit

Release Notes

Uh oh!

vercel bot commented Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai bot commented Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram

Estimated code review effort

Possibly related PRs

Suggested reviewers

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

yhakbar commented Nov 18, 2025 •

edited by coderabbitai bot

Loading

vercel bot commented Nov 18, 2025 •

edited

Loading

coderabbitai bot commented Nov 19, 2025 •

edited

Loading