Repository navigation
[codex] consolidate terraform bulk execution on scheduler - #2466
Merged
Merged
Conversation
Contributor
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request. 
|
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
Contributor
This was referenced May 21, 2026
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (77.15%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #2466 +/- ##
==========================================
+ Coverage 78.88% 78.89% +0.01%
==========================================
Files 1198 1200 +2
Lines 114797 115889 +1092
==========================================
+ Hits 90560 91435 +875
- Misses 19270 19397 +127
- Partials 4967 5057 +90
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Mikhail Shirkov (shirkevich)
force-pushed
the
codex/dag-scheduler-core
branch
from
May 25, 2026 07:46
cfd2b2a to
e3f69de
Compare
Contributor
|
💥 This pull request now has conflicts. Could you fix it Mikhail Shirkov (@shirkevich)? 🙏 |
Mikhail Shirkov (shirkevich)
force-pushed
the
codex/dag-terraform-graph-bulk-path
branch
from
May 25, 2026 07:53
1f0197e to
3d1c53c
Compare
Mikhail Shirkov (shirkevich)
marked this pull request as ready for review
May 26, 2026 18:07
Member
|
CodeRabbit (@coderabbitai) can you spot any regressions? |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Get list of files changed in PR `#2466`
gh pr view 2466 --repo cloudposse/atmos --json files --jq '.files[].path' 2>/dev/null | head -80Length of output: 399 🏁 Script executed: #!/bin/bash
# Get the PR diff
gh pr diff 2466 --repo cloudposse/atmos 2>/dev/null | head -2000Length of output: 32391 🏁 Script executed: #!/bin/bash
# Check for any remaining references to removed functions from terraform_all.go
rg -n "buildTerraformDependencyGraph|applyFiltersToGraph|executeInDependencyOrder|processTerraformComponent|walkTerraformComponents" --type goLength of output: 7726 🏁 Script executed: #!/bin/bash
# Check what remains in terraform_all.go after the diff
cat internal/exec/terraform_all.goLength of output: 10527 🏁 Script executed: #!/bin/bash
# Check what remains in terraform_query.go after the diff
cat internal/exec/terraform_query.goLength of output: 4079 🏁 Script executed: #!/bin/bash
# Check the full terraform_all.go on the PR branch for dead code / orphaned functions
gh api repos/cloudposse/atmos/contents/internal/exec/terraform_all.go?ref=codex/dag-scheduler-consolidate-bulk-terraform 2>/dev/null | jq -r '.content' | base64 -d 2>/dev/null || echo "Try different branch ref"Length of output: 356 🏁 Script executed: #!/bin/bash
# Get PR branch name
gh pr view 2466 --repo cloudposse/atmos --json headRefName --jq '.headRefName'Length of output: 192 🏁 Script executed: #!/bin/bash
# Check for destroy-order reversal in the scheduler
rg -n "destroy|reverse|SubCommand" --type go pkg/scheduler/Length of output: 858 🏁 Script executed: #!/bin/bash
# Check isComponentEnabled implementation
rg -n "func isComponentEnabled" --type go -A 20Length of output: 2994 🏁 Script executed: #!/bin/bash
# Check if executeInDependencyOrder or executeTerraformForNode are called anywhere else
rg -n "executeInDependencyOrder|executeTerraformForNode" --type goLength of output: 1353 🏁 Script executed: #!/bin/bash
# Check if applyFiltersToGraph is called from production code (not tests)
rg -n "applyFiltersToGraph" --type go -g '!*_test.go'Length of output: 449 🏁 Script executed: #!/bin/bash
# Check the terraform_affected_graph.go call to buildTerraformDependencyGraph for context
rg -n "buildTerraformDependencyGraph" --type go -C 5 internal/exec/terraform_affected_graph.goLength of output: 514 🏁 Script executed: #!/bin/bash
# Check the new adapter's shouldSkipComponent vs old shouldSkipComponentForGraph - specifically for metadata.enabled
rg -n "enabled" --type go internal/exec/terraform_all.go -C 3Length of output: 433
🔴 Regression 1 —
|
Mikhail Shirkov (shirkevich)
force-pushed
the
codex/dag-terraform-graph-bulk-path
branch
from
May 27, 2026 18:13
3d1c53c to
713f3ff
Compare
Mikhail Shirkov (shirkevich)
force-pushed
the
codex/dag-scheduler-core
branch
from
May 27, 2026 18:13
e3f69de to
1c61093
Compare
Collaborator
Author
|
CodeRabbit (@coderabbitai) review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
|
Wondering what really moved? Review this PR in Change Stack to inspect semantic changes, definitions, and references. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughRoutes multi-component Terraform execution through a scheduler adapter with context-aware entrypoints, adds CLI options for concurrency/failure/logging, implements graph build/filter/reverse and dispatch, centralizes per-node execution/output capture and hooks, and adds extensive tests, fixtures, and small utilities. ChangesTerraform Scheduler Adapter & Integration
Sequence Diagram(s)sequenceDiagram
participant CLI
participant Describe as ExecuteDescribeStacks
participant Adapter as scheduleradapters.ExecuteTerraform
participant Scheduler
participant Dispatcher as TerraformDispatcher
participant Executor as TerraformExecutor
CLI->>Describe: describe stacks (info.Stack)
CLI->>Adapter: ExecuteTerraform(ctx, TerraformOptions)
Adapter->>Scheduler: schedule filtered nodes
Scheduler->>Dispatcher: run node
Dispatcher->>Executor: invoke per-node executor with node-specific Info
Executor->>Dispatcher: return result / error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
# Conflicts: # pkg/terraform/output/environment.go
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
pkg/git/git.go (1)
317-324: ⚡ Quick winConsider adding a comment explaining string-based error detection.
String-based error inspection is fragile to upstream message changes. A brief comment explaining that go-git doesn't expose typed errors for this extension failure would help future maintainers understand why this approach is necessary.
Example:
// isUnsupportedWorktreeConfigError reports go-git failures caused by worktreeConfig. // Uses string matching because go-git does not export typed errors for extension failures. func isUnsupportedWorktreeConfigError(err error) bool {🤖 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 `@pkg/git/git.go` around lines 317 - 324, Add a brief explanatory comment above isUnsupportedWorktreeConfigError explaining why the function relies on string matching (e.g., go-git does not export typed errors for worktreeConfig/extension failures so we must inspect the error text), then keep the existing implementation; reference the function name isUnsupportedWorktreeConfigError and mention the specific strings "repositoryformatversion" and "worktreeconfig" to clarify why those substrings are used.cmd/terraform/apply.go (1)
124-125: ⚡ Quick winInconsistency: apply missing max-concurrency flag.
The
destroyandplancommands both register a--max-concurrencyflag (destroy.go:41, plan.go:128), butapplydoes not. This creates an inconsistent UX where users can control concurrency for plan/destroy but not apply when using--all,--affected, or--components.🔧 Suggested addition
flags.WithBoolFlag("all", "", false, "Apply all components in all stacks"), flags.WithStringFlag("failure-mode", "", terraformFailureModeFailFast, "Terraform apply failure handling mode. Supported values: fail-fast, keep-going"), + flags.WithIntFlag("max-concurrency", "", 1, "Maximum number of Terraform apply components to execute concurrently"), flags.WithBoolFlag("ci", "", false, "Enable CI mode for automated pipelines (writes job summary, outputs)"), flags.WithEnvVars("from-plan", "ATMOS_TERRAFORM_APPLY_FROM_PLAN"), flags.WithEnvVars("planfile", "ATMOS_TERRAFORM_APPLY_PLANFILE"), flags.WithEnvVars("failure-mode", "ATMOS_TERRAFORM_APPLY_FAILURE_MODE"), + flags.WithEnvVars("max-concurrency", "ATMOS_TERRAFORM_APPLY_MAX_CONCURRENCY"), flags.WithEnvVars("ci", "ATMOS_CI", "CI"),🤖 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 `@cmd/terraform/apply.go` around lines 124 - 125, The apply command is missing the same max-concurrency option that plan and destroy already expose, creating an inconsistent CLI surface for multi-component operations. Update the flag registration in the apply command setup to add the `--max-concurrency` option alongside the existing flags in the apply command’s flag list, using the same naming and behavior as the corresponding flag handling in the plan and destroy commands so `--all`, `--affected`, and `--components` can be controlled consistently.
🤖 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.
Inline comments:
In `@cmd/terraform/options.go`:
- Around line 78-80: The parsed values for FailureMode and PlanLogOrder must be
validated immediately after building the options struct (the block that sets
MaxConcurrency, FailureMode, PlanLogOrder in cmd/terraform/options.go) to fail
fast with a clear message; check FailureMode against the allowed set
{"fail-fast","keep-going"} and PlanLogOrder against {"stream","grouped"} and
return a formatted error listing valid options if either is invalid. Change the
options builder function to return (options, error) instead of only options and
propagate that error through all call sites mentioned (apply.go, destroy.go,
plan.go) so callers handle the validation error. Ensure error messages reference
the offending flag name and allowed values.
In `@pkg/git/git.go`:
- Around line 387-402: In gitRepositoryPaths, wrap the error returned by
exec.Command(...).Output() with the static sentinel from errors/errors.go
instead of returning the raw err; replace the current "return "", "", "", err"
with a wrapped error (e.g. fmt.Errorf("%w: %v", errors.<appropriateSentinel>,
err)) so the failure to execute "git rev-parse" is categorized, and keep the
existing use of errUtils.ErrUnexpectedGitRevParseOutput for the unexpected
output branch.
---
Nitpick comments:
In `@cmd/terraform/apply.go`:
- Around line 124-125: The apply command is missing the same max-concurrency
option that plan and destroy already expose, creating an inconsistent CLI
surface for multi-component operations. Update the flag registration in the
apply command setup to add the `--max-concurrency` option alongside the existing
flags in the apply command’s flag list, using the same naming and behavior as
the corresponding flag handling in the plan and destroy commands so `--all`,
`--affected`, and `--components` can be controlled consistently.
In `@pkg/git/git.go`:
- Around line 317-324: Add a brief explanatory comment above
isUnsupportedWorktreeConfigError explaining why the function relies on string
matching (e.g., go-git does not export typed errors for worktreeConfig/extension
failures so we must inspect the error text), then keep the existing
implementation; reference the function name isUnsupportedWorktreeConfigError and
mention the specific strings "repositoryformatversion" and "worktreeconfig" to
clarify why those substrings are used.
🪄 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: 6d2209e8-cff4-4ece-90dc-a3ed128fd179
📒 Files selected for processing (47)
cmd/terraform/apply.gocmd/terraform/destroy.gocmd/terraform/options.gocmd/terraform/options_test.gocmd/terraform/plan.gocmd/terraform/utils.goerrors/errors.gogo.modinternal/exec/terraform_affected.gointernal/exec/terraform_all.gointernal/exec/terraform_query.gointernal/exec/terraform_utils_test.gopkg/auth/credentials/store.gopkg/auth/credentials/store_test.gopkg/git/git.gopkg/git/git_test.gopkg/io/line_prefix_writer.gopkg/io/line_prefix_writer_test.gopkg/provisioner/workdir/fs.gopkg/provisioner/workdir/fs_test.gopkg/scheduler/adapters/terraform.gopkg/scheduler/adapters/terraform_test.gopkg/schema/schema.gopkg/terraform/output/environment.gotests/cli_auth_console_test.gotests/cli_auth_login_provider_test.gotests/cli_describe_identity_test.gotests/cli_double_hyphen_test.gotests/cli_identity_flag_test.gotests/cli_interactive_test.gotests/cli_plugin_cache_test.gotests/cli_profile_test.gotests/cli_skip_init_test.gotests/cli_terraform_test.gotests/cli_test.gotests/cli_workdir_test.gotests/fixtures/scenarios/terraform-floci-dag/atmos.yamltests/fixtures/scenarios/terraform-floci-dag/components/terraform/alias-shared/main.tftests/fixtures/scenarios/terraform-floci-dag/components/terraform/bucket-marker/main.tftests/fixtures/scenarios/terraform-floci-dag/components/terraform/final-marker/main.tftests/fixtures/scenarios/terraform-floci-dag/components/terraform/queue-marker/main.tftests/fixtures/scenarios/terraform-floci-dag/components/terraform/seed/main.tftests/fixtures/scenarios/terraform-floci-dag/components/terraform/topic-marker/main.tftests/fixtures/scenarios/terraform-floci-dag/stacks/deploy/local.yamltests/snapshots/TestCLICommands_atmos_terraform_apply_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_terraform_apply_help.stdout.goldentests/snapshots/TestCLICommands_config_alias_tp_--help_shows_terraform_plan_help.stdout.golden
💤 Files with no reviewable changes (31)
- tests/snapshots/TestCLICommands_atmos_terraform_apply_help.stdout.golden
- tests/fixtures/scenarios/terraform-floci-dag/components/terraform/bucket-marker/main.tf
- tests/snapshots/TestCLICommands_atmos_terraform_apply_--help.stdout.golden
- tests/cli_describe_identity_test.go
- tests/fixtures/scenarios/terraform-floci-dag/atmos.yaml
- pkg/io/line_prefix_writer.go
- tests/fixtures/scenarios/terraform-floci-dag/components/terraform/topic-marker/main.tf
- tests/fixtures/scenarios/terraform-floci-dag/components/terraform/seed/main.tf
- tests/cli_double_hyphen_test.go
- tests/fixtures/scenarios/terraform-floci-dag/components/terraform/final-marker/main.tf
- pkg/terraform/output/environment.go
- tests/cli_terraform_test.go
- tests/fixtures/scenarios/terraform-floci-dag/components/terraform/queue-marker/main.tf
- tests/cli_skip_init_test.go
- tests/fixtures/scenarios/terraform-floci-dag/stacks/deploy/local.yaml
- tests/cli_test.go
- tests/cli_identity_flag_test.go
- tests/cli_auth_login_provider_test.go
- tests/cli_interactive_test.go
- tests/fixtures/scenarios/terraform-floci-dag/components/terraform/alias-shared/main.tf
- pkg/provisioner/workdir/fs.go
- pkg/io/line_prefix_writer_test.go
- tests/cli_plugin_cache_test.go
- tests/snapshots/TestCLICommands_config_alias_tp_--help_shows_terraform_plan_help.stdout.golden
- tests/cli_auth_console_test.go
- tests/cli_profile_test.go
- pkg/provisioner/workdir/fs_test.go
- tests/cli_workdir_test.go
- pkg/schema/schema.go
- pkg/scheduler/adapters/terraform_test.go
- pkg/scheduler/adapters/terraform.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/auth/credentials/store.go
Member
|
Mikhail Shirkov (@shirkevich) please address the coderabbitai comments (and nitpick comments), thanks |
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/auth/credentials/store_test.go (1)
183-193:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSubtests missing env var cleanup - test is environment-dependent.
The "auth config overrides default" and "default remains system" subtests don't clear
ATMOS_KEYRING_TYPE. If this var is set externally (CI, dev machine), these tests become flaky or pass for the wrong reason.Proposed fix
t.Run("auth config overrides default", func(t *testing.T) { + t.Setenv("ATMOS_KEYRING_TYPE", "") // Clear any inherited value. authConfig := &schema.AuthConfig{ Keyring: schema.KeyringConfig{Type: "file"}, } assert.Equal(t, "file", resolveKeyringType(authConfig)) }) t.Run("default remains system", func(t *testing.T) { + t.Setenv("ATMOS_KEYRING_TYPE", "") // Clear any inherited value. assert.Equal(t, "system", resolveKeyringType(nil)) })Based on learnings: "tests should prefer t.Setenv for environment variable setup/teardown instead of os.Setenv/Unsetenv to ensure test-scoped isolation."
🤖 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 `@pkg/auth/credentials/store_test.go` around lines 183 - 193, The two subtests around resolveKeyringType are environment-dependent because they don't isolate ATMOS_KEYRING_TYPE; update both subtests to use t.Setenv("ATMOS_KEYRING_TYPE", "") (or set to a specific value) at the start so the env var is test-scoped and automatically restored, ensuring "auth config overrides default" and "default remains system" reliably exercise resolveKeyringType(authConfig) and resolveKeyringType(nil) respectively.
🧹 Nitpick comments (2)
internal/exec/terraform_all.go (2)
7-7: 💤 Low valueImport alias inconsistency.
This file imports
github.com/charmbracelet/logdirectly while the coding guidelines specify usingpkg/loggerwith thelogalias. Other files in this PR (e.g.,terraform_affected.go) uselog "github.com/cloudposse/atmos/pkg/logger".Suggested fix
- log "github.com/charmbracelet/log" + log "github.com/cloudposse/atmos/pkg/logger"🤖 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 `@internal/exec/terraform_all.go` at line 7, The import in terraform_all.go currently pulls github.com/charmbracelet/log; change it to use the project logger alias by replacing that import with log "github.com/cloudposse/atmos/pkg/logger" so it matches other files (e.g., terraform_affected.go) and coding guidelines; ensure any references to log in the file continue to compile with the new import alias.
89-119: ⚡ Quick winDead production code flagged in PR comments.
Per the PR discussion,
executeInDependencyOrderandapplyFiltersToGraphare no longer called by production paths and are only referenced from tests. The scheduler adapter now handles this logic. Consider removing or marking as deprecated to avoid maintenance burden.Also applies to: 208-240
🤖 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 `@internal/exec/terraform_all.go` around lines 89 - 119, The functions executeInDependencyOrder and applyFiltersToGraph are dead production code and should be removed or explicitly deprecated; either delete both functions (and clean up any now-unused imports) and update tests to exercise the scheduler adapter logic instead, or if tests still need them keep them but add a clear deprecation comment (// Deprecated: use scheduler adapter XYZ) and mark them unexported or gate them behind a test-only build tag, then run and fix any failing tests that referenced executeInDependencyOrder or applyFiltersToGraph so all usages point to the scheduler adapter methods.
🤖 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.
Outside diff comments:
In `@pkg/auth/credentials/store_test.go`:
- Around line 183-193: The two subtests around resolveKeyringType are
environment-dependent because they don't isolate ATMOS_KEYRING_TYPE; update both
subtests to use t.Setenv("ATMOS_KEYRING_TYPE", "") (or set to a specific value)
at the start so the env var is test-scoped and automatically restored, ensuring
"auth config overrides default" and "default remains system" reliably exercise
resolveKeyringType(authConfig) and resolveKeyringType(nil) respectively.
---
Nitpick comments:
In `@internal/exec/terraform_all.go`:
- Line 7: The import in terraform_all.go currently pulls
github.com/charmbracelet/log; change it to use the project logger alias by
replacing that import with log "github.com/cloudposse/atmos/pkg/logger" so it
matches other files (e.g., terraform_affected.go) and coding guidelines; ensure
any references to log in the file continue to compile with the new import alias.
- Around line 89-119: The functions executeInDependencyOrder and
applyFiltersToGraph are dead production code and should be removed or explicitly
deprecated; either delete both functions (and clean up any now-unused imports)
and update tests to exercise the scheduler adapter logic instead, or if tests
still need them keep them but add a clear deprecation comment (// Deprecated:
use scheduler adapter XYZ) and mark them unexported or gate them behind a
test-only build tag, then run and fix any failing tests that referenced
executeInDependencyOrder or applyFiltersToGraph so all usages point to the
scheduler adapter methods.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 126ca3cd-7b45-4fed-88eb-5795a375b1d9
📒 Files selected for processing (51)
cmd/terraform/apply.gocmd/terraform/deploy.gocmd/terraform/destroy.gocmd/terraform/init.gocmd/terraform/options.gocmd/terraform/options_test.gocmd/terraform/plan.gocmd/terraform/plan_diff.gocmd/terraform/subcommands_test.gocmd/terraform/utils.gocmd/terraform/workspace.goerrors/errors.gogo.modinternal/exec/terraform_affected.gointernal/exec/terraform_all.gointernal/exec/terraform_query.gointernal/exec/terraform_utils_test.gopkg/auth/credentials/store.gopkg/auth/credentials/store_test.gopkg/git/git.gopkg/git/git_test.gopkg/io/line_prefix_writer.gopkg/io/line_prefix_writer_test.gopkg/provisioner/workdir/fs.gopkg/provisioner/workdir/fs_test.gopkg/scheduler/adapters/terraform.gopkg/scheduler/adapters/terraform_test.gopkg/schema/schema.gopkg/terraform/output/environment.gotests/cli_auth_console_test.gotests/cli_auth_login_provider_test.gotests/cli_describe_identity_test.gotests/cli_double_hyphen_test.gotests/cli_identity_flag_test.gotests/cli_interactive_test.gotests/cli_plugin_cache_test.gotests/cli_profile_test.gotests/cli_skip_init_test.gotests/cli_terraform_test.gotests/cli_test.gotests/cli_workdir_test.gotests/fixtures/scenarios/terraform-floci-dag/atmos.yamltests/fixtures/scenarios/terraform-floci-dag/components/terraform/alias-shared/main.tftests/fixtures/scenarios/terraform-floci-dag/components/terraform/bucket-marker/main.tftests/fixtures/scenarios/terraform-floci-dag/components/terraform/final-marker/main.tftests/fixtures/scenarios/terraform-floci-dag/components/terraform/queue-marker/main.tftests/fixtures/scenarios/terraform-floci-dag/components/terraform/seed/main.tftests/fixtures/scenarios/terraform-floci-dag/components/terraform/topic-marker/main.tftests/fixtures/scenarios/terraform-floci-dag/stacks/deploy/local.yamltests/snapshots/TestCLICommands_atmos_terraform_apply_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_terraform_apply_help.stdout.golden
💤 Files with no reviewable changes (30)
- tests/fixtures/scenarios/terraform-floci-dag/atmos.yaml
- tests/fixtures/scenarios/terraform-floci-dag/components/terraform/topic-marker/main.tf
- tests/fixtures/scenarios/terraform-floci-dag/stacks/deploy/local.yaml
- tests/snapshots/TestCLICommands_atmos_terraform_apply_--help.stdout.golden
- tests/cli_auth_login_provider_test.go
- tests/snapshots/TestCLICommands_atmos_terraform_apply_help.stdout.golden
- tests/fixtures/scenarios/terraform-floci-dag/components/terraform/bucket-marker/main.tf
- pkg/provisioner/workdir/fs.go
- pkg/terraform/output/environment.go
- tests/fixtures/scenarios/terraform-floci-dag/components/terraform/queue-marker/main.tf
- tests/cli_identity_flag_test.go
- tests/cli_interactive_test.go
- tests/fixtures/scenarios/terraform-floci-dag/components/terraform/seed/main.tf
- tests/cli_terraform_test.go
- tests/fixtures/scenarios/terraform-floci-dag/components/terraform/final-marker/main.tf
- tests/cli_test.go
- pkg/provisioner/workdir/fs_test.go
- tests/fixtures/scenarios/terraform-floci-dag/components/terraform/alias-shared/main.tf
- tests/cli_auth_console_test.go
- tests/cli_profile_test.go
- tests/cli_double_hyphen_test.go
- pkg/io/line_prefix_writer.go
- tests/cli_skip_init_test.go
- pkg/io/line_prefix_writer_test.go
- tests/cli_workdir_test.go
- pkg/schema/schema.go
- tests/cli_plugin_cache_test.go
- pkg/scheduler/adapters/terraform.go
- pkg/scheduler/adapters/terraform_test.go
- tests/cli_describe_identity_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- go.mod
- pkg/auth/credentials/store.go
Andriy Knysh (aknysh)
approved these changes
Jun 6, 2026
Contributor
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request.
|
|
These changes were released in v1.221.0-rc.5. |
This was referenced Jul 13, 2026
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
--all,--components, and--querythrough the scheduler-backed Terraform adapterdependencies.componentsfirst, withsettings.depends_onfallback--identity terraformtesting works1for this PRStacking
This PR is stacked on PR 2 and targets
codex/dag-scheduler-core.PR 4 is #2468 and is stacked on this branch to introduce plan-only
--max-concurrencywiring.Supersedes the earlier fork-headed draft #2462 now that the stack branches exist in
cloudposse/atmos.Draft note
This branch is back to the intended PR 3 review shape: Terraform
--all,--components, and--queryshare the graph-backed scheduler path, but execution remains sequential.The temporary
ATMOS_EXPERIMENTAL_DAG_MAX_CONCURRENCYvalidation hook has been removed. User-visible plan concurrency now belongs to PR 4.This branch retains the narrow credential-store concurrency-safety prerequisite discovered during validation:
ATMOS_KEYRING_TYPEprecedenceValidation
go test ./pkg/scheduler ./pkg/scheduler/adapters ./internal/exec -run TestExecuteTerraformQuery|TestExecuteTerraformQueryNoMatches|TestBuildTerraformDependencyGraph|TestExecuteTerraformAllUsesGraphBackedSequentialOrder|TestExecuteTerraformComponentsUsesGraphBackedSequentialOrder|TestExecuteTerraformQueryUsesGraphBackedSequentialOrder|TestExecuteTerraformKeepsIndependentComponentsSequential|TestBuildTerraformGraphgo test ./pkg/auth/credentialsgo test -race ./pkg/auth/credentials -run TestNewCredentialStoreWithConfig_ConcurrentInitializationgo test ./pkg/auth ./internal/exec -run TestCreateAndAuthenticateManagerWithAtmosConfig|TestSetupTerraformAuth|TestProcessComponentConfig_PropagatesAuthManager|TestProcessComponentConfig_AuthManagerGuardBranchesbuild/atmosand live-tested against a downstream stack withterraform plan --alland an explicit identityValidation findings carried forward
viper.BindEnv, causingfatal error: concurrent map writes. This PR fixes that narrowly inpkg/auth/credentials.Follow-up discussion
The longer-term way to unlock true parallelism for aliases sharing one physical Terraform folder would be per-node isolated workdirs plus isolated
TF_DATA_DIRand generated files. That needs repo-owner discussion because it changes the operator debugging model: Atmos would need to decide whether and how to retain those per-node copies for inspection, howatmos terraform shellmaps to them, and how cleanup/debug artifacts are managed.Summary by CodeRabbit
New Features
Bug Fixes
Tests