Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughIntroduces a new experiment flag Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/config/stack.go`:
- Around line 320-344: In processUnitGenerateBlocks, add a defensive nil check
for cfg before iterating over cfg.GenerateConfigs: if cfg is nil, return nil (or
otherwise skip processing) to avoid a potential nil map access; locate the
ParseConfig call that assigns cfg and ensure you check "if cfg == nil { return
nil }" before the for name, genCfg := range cfg.GenerateConfigs loop so the
function safely handles a (nil, nil) return from ParseConfig.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e7f58264-08b9-4abc-8e6b-f871fe2a75fd
📒 Files selected for processing (11)
docs/src/content/docs/04-reference/04-experiments.mdinternal/experiment/experiment.gopkg/config/stack.gotest/fixtures/stacks/generate-if-disabled/live/terragrunt.stack.hcltest/fixtures/stacks/generate-if-disabled/units/first/alpha.tftest/fixtures/stacks/generate-if-disabled/units/first/beta.tftest/fixtures/stacks/generate-if-disabled/units/first/main.tftest/fixtures/stacks/generate-if-disabled/units/first/terragrunt.hcltest/fixtures/stacks/generate-if-disabled/units/second/main.tftest/fixtures/stacks/generate-if-disabled/units/second/terragrunt.hcltest/integration_stacks_test.go
| func processUnitGenerateBlocks(ctx context.Context, l log.Logger, pctx *ParsingContext, unitDir string) error { | ||
| configPath := filepath.Join(unitDir, DefaultTerragruntConfigPath) | ||
| if !util.FileExists(configPath) { | ||
| return nil | ||
| } | ||
|
|
||
| unitPctx := pctx.Clone() | ||
| unitPctx.TerragruntConfigPath = configPath | ||
| unitPctx.WorkingDir = unitDir | ||
| unitPctx.SkipOutputsResolution = true | ||
|
|
||
| cfg, err := ParseConfigFile(ctx, unitPctx, l, configPath, nil) | ||
| if err != nil { | ||
| l.Debugf("Could not parse generate blocks for %s during stack generation: %v", configPath, err) | ||
| return nil | ||
| } | ||
|
|
||
| for name, genCfg := range cfg.GenerateConfigs { | ||
| if err := codegen.WriteToFile(l, unitDir, &genCfg); err != nil { | ||
| return errors.Errorf("failed to process generate block %q (path %q) in %s: %w", name, genCfg.Path, unitDir, err) | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify if ParseConfigFile can return nil config without error
ast-grep --pattern $'func ParseConfigFile($$$) ($_, $_) {
$$$
}'Repository: gruntwork-io/terragrunt
Length of output: 4479
🏁 Script executed:
#!/bin/bash
# Find and examine the ParseConfig function
ast-grep --pattern $'func ParseConfig($$$) ($_, $_) {
$$$
}'Repository: gruntwork-io/terragrunt
Length of output: 8443
Add a nil check for cfg before iterating over GenerateConfigs.
The ParseConfig function uses a MultiError pattern that allows it to accumulate errors and continue processing. While unlikely in normal operation, if convertToTerragruntConfig returns nil and no other errors occur, ParseConfig could theoretically return (nil, nil). A defensive nil check here prevents potential panics on nil map access.
Proposed defensive nil check
cfg, err := ParseConfigFile(ctx, unitPctx, l, configPath, nil)
if err != nil {
l.Debugf("Could not parse generate blocks for %s during stack generation: %v", configPath, err)
return nil
}
+ if cfg == nil {
+ return nil
+ }
for name, genCfg := range cfg.GenerateConfigs {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func processUnitGenerateBlocks(ctx context.Context, l log.Logger, pctx *ParsingContext, unitDir string) error { | |
| configPath := filepath.Join(unitDir, DefaultTerragruntConfigPath) | |
| if !util.FileExists(configPath) { | |
| return nil | |
| } | |
| unitPctx := pctx.Clone() | |
| unitPctx.TerragruntConfigPath = configPath | |
| unitPctx.WorkingDir = unitDir | |
| unitPctx.SkipOutputsResolution = true | |
| cfg, err := ParseConfigFile(ctx, unitPctx, l, configPath, nil) | |
| if err != nil { | |
| l.Debugf("Could not parse generate blocks for %s during stack generation: %v", configPath, err) | |
| return nil | |
| } | |
| for name, genCfg := range cfg.GenerateConfigs { | |
| if err := codegen.WriteToFile(l, unitDir, &genCfg); err != nil { | |
| return errors.Errorf("failed to process generate block %q (path %q) in %s: %w", name, genCfg.Path, unitDir, err) | |
| } | |
| } | |
| return nil | |
| } | |
| func processUnitGenerateBlocks(ctx context.Context, l log.Logger, pctx *ParsingContext, unitDir string) error { | |
| configPath := filepath.Join(unitDir, DefaultTerragruntConfigPath) | |
| if !util.FileExists(configPath) { | |
| return nil | |
| } | |
| unitPctx := pctx.Clone() | |
| unitPctx.TerragruntConfigPath = configPath | |
| unitPctx.WorkingDir = unitDir | |
| unitPctx.SkipOutputsResolution = true | |
| cfg, err := ParseConfigFile(ctx, unitPctx, l, configPath, nil) | |
| if err != nil { | |
| l.Debugf("Could not parse generate blocks for %s during stack generation: %v", configPath, err) | |
| return nil | |
| } | |
| if cfg == nil { | |
| return nil | |
| } | |
| for name, genCfg := range cfg.GenerateConfigs { | |
| if err := codegen.WriteToFile(l, unitDir, &genCfg); err != nil { | |
| return errors.Errorf("failed to process generate block %q (path %q) in %s: %w", name, genCfg.Path, unitDir, err) | |
| } | |
| } | |
| return nil | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/config/stack.go` around lines 320 - 344, In processUnitGenerateBlocks,
add a defensive nil check for cfg before iterating over cfg.GenerateConfigs: if
cfg is nil, return nil (or otherwise skip processing) to avoid a potential nil
map access; locate the ParseConfig call that assigns cfg and ensure you check
"if cfg == nil { return nil }" before the for name, genCfg := range
cfg.GenerateConfigs loop so the function safely handles a (nil, nil) return from
ParseConfig.
|
|
||
| #### `stacks-generate-block` - What it does | ||
|
|
||
| When Terragrunt generates a stack (via `stack generate` or `stack run`), units may contain `generate` blocks with conditional logic such as `if_disabled = "remove"`. Without this experiment, those generate blocks are not processed during stack generation. This means that when a subsequent filtered run (`--filter`) triggers dependency resolution via `terraform output -json`, the generated unit directory may contain stale or conflicting files (e.g., duplicate `required_providers` blocks), causing Terraform to fail. |
There was a problem hiding this comment.
| When Terragrunt generates a stack (via `stack generate` or `stack run`), units may contain `generate` blocks with conditional logic such as `if_disabled = "remove"`. Without this experiment, those generate blocks are not processed during stack generation. This means that when a subsequent filtered run (`--filter`) triggers dependency resolution via `terraform output -json`, the generated unit directory may contain stale or conflicting files (e.g., duplicate `required_providers` blocks), causing Terraform to fail. | |
| When Terragrunt generates a stack (via `stack generate` or `stack run`), units may contain `generate` blocks with conditional logic such as `if_disabled = "remove"`. Without this experiment, those generate blocks are not processed during stack generation. This means that when a subsequent filtered run (`--filter`) triggers dependency resolution via `tofu/terraform output -json`, the generated unit directory may contain stale or conflicting files (e.g., duplicate `required_providers` blocks), causing an error in OpenTofu/Terraform. |
|
|
||
| #### `stacks-generate-block` - What it does | ||
|
|
||
| When Terragrunt generates a stack (via `stack generate` or `stack run`), units may contain `generate` blocks with conditional logic such as `if_disabled = "remove"`. Without this experiment, those generate blocks are not processed during stack generation. This means that when a subsequent filtered run (`--filter`) triggers dependency resolution via `terraform output -json`, the generated unit directory may contain stale or conflicting files (e.g., duplicate `required_providers` blocks), causing Terraform to fail. |
There was a problem hiding this comment.
I still don't get this. Why does tofu output -json get called from --filter or queue construction? It should only get called just in time before the dependent unit runs.
Description
Fixes #5702.
TODOs
Read the Gruntwork contribution guidelines.
Release Notes (draft)
Added / Removed / Updated [X].
Migration Guide
Summary by CodeRabbit
New Features
stacks-generate-blockexperiment that immediately processesgenerateblocks during stack unit generation, resolving issues with dependency resolution observing stale generated state.Documentation
Tests