atmos icon indicating copy to clipboard operation
atmos copied to clipboard

refactor: Migrate terraform commands to registry pattern

Open osterman opened this issue 3 months ago • 12 comments

what

  • Migrate all terraform commands from monolithic cmd files to registry-based modular structure
  • Restructure terraform command handling into individual command providers under cmd/terraform/
  • Update flag handling to use StandardFlagParser pattern for consistent CLI argument processing
  • Fix I/O handling in terraform plan-diff to use data.Writeln() for proper output layering
  • Add comprehensive terraform registry migration PRD documentation

why

Improves code maintainability and extensibility by following the CommandProvider registry pattern established in the codebase. Breaks down monolithic terraform command files into focused, single-responsibility modules. Ensures proper I/O separation between data output and UI messages as per CLAUDE.md standards.

references

  • Implements docs/prd/command-registry-pattern.md
  • Follows I/O/UI standards in CLAUDE.md
  • Adds docs/prd/terraform-registry-migration.md with detailed migration architecture

Summary by CodeRabbit

  • New Features

    • Full Terraform command suite added (many terraform subcommands) and generate/varfile/planfile/backend helpers.
    • Interactive prompts to select component and stack when omitted (TTY).
  • Improvements

    • Compatibility/pass‑through flags to native Terraform exposed and documented; help output refreshed and condensed.
    • Tab completions improved (stack suggestions filtered by component).
    • terraform clean: --everything, --force/-f, --skip-lock-file and improved dry‑run; clearer --from-plan/plan handling.
  • Chores

    • Telemetry notice added; test/snapshot sanitization updated.

✏️ Tip: You can customize this high-level summary in your review settings.

osterman avatar Nov 23 '25 00:11 osterman

[!CAUTION]

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

Split Terraform into per-subcommand Cobra modules and typed executors, add pre-parsing to extract passthrough Terraform flags, introduce a compat-flag registry and separated storage, expose small cmd wrappers (CreateAuthManager, GetConfigAndStacksInfo), enhance flag/completion plumbing, and update help, prompts, tests, and snapshots.

Changes

Cohort / File(s) Summary
Root & preprocessing
\cmd/root.go``, \pkg/flags/compat/separated.go``
Add preprocessCompatibilityFlags to pre-scan args, store/retrieve separated passthrough args via compat.SetSeparated/GetSeparated/ResetSeparated, and adjust RootCmd args before Cobra parsing.
Compat registry
cmd/internal/registry.go, \cmd/internal/registry_test.go``
Add in-memory compat flags registry with RegisterCommandCompatFlags, GetSubcommandCompatFlags, GetCompatFlagsForCommand and concurrency tests.
Terraform command provider & core
\cmd/terraform/terraform.go``, \cmd/terraform/flags.go``, \cmd/terraform/terraform_test.go``
Introduce TerraformCommandProvider, terraformParser, Terraform flag registries/builders, compat flag wiring, and early heatmap/perf detection.
Per-subcommand Cobra modules
\cmd/terraform/.go``, \cmd/terraform/generate/``, (deleted) \cmd/terraform_commands.go``
Replace monolithic command wiring with many per-subcommand files (plan, apply, deploy, clean, plan-diff, shell, generate/, varfile, etc.), each registering flags, completions, Viper binding, and delegating to typed Execute flows; remove legacy aggregate module.
Typed executors & internal reorg
\internal/exec/``, \pkg/terraform/``, \internal/exec/options.go``
Add typed option structs and Execute* functions (Clean, Shell, Varfile, Planfile, GenerateBackend, etc.), TF_DATA_DIR handling, helpers, deprecation wrappers, and compatibility re-exports.
Flag system & parser enhancements
\pkg/flags/{options.go,registry.go,standard.go,standard_parser.go,types.go,flag_parser.go}``
Remove old WithTerraformFlags, add WithFlagRegistry, expose Registry() on parsers, add SetCompletionFunc/GetCompletionFunc, propagate completions recursively, and refine Cobra parsing heuristics.
Completions, cmd utils & auth wrappers
\cmd/cmd_utils.go``, \cmd/auth_login.go``, \cmd/cmd_utils_test.go``, \cmd/terraform/shared/prompt.go``
Export FlagStack, add GetConfigAndStacksInfo and CreateAuthManager wrappers, expose StackFlagCompletion, update tests, and add shared prompt/completion helpers for component/stack selection.
Help rendering & snapshots
\cmd/help_template.go``, \tests/snapshots/*``
Add COMPATIBILITY FLAGS section to help (queries compat registry) and update many golden snapshots to reflect new help layout and passthrough flags.
Validation, errors & utilities
\cmd/internal/validation.go``, \errors/errors.go``, \pkg/utils/*`**
Add ValidateAtmosConfig, new sentinel errors (ErrUnknownSubcommand, ErrInteractiveNotAvailable, ErrDeprecatedCmdNotCallable, backend-related errors), and small message/format tweaks.
Interactive prompts & UI
\pkg/flags/interactive.go``, \cmd/terraform/shared/prompt.go``, \cmd/terraform/shared/prompt.go``
Replace selector UI with form selector (ESC/Ctrl+C abort), add PromptForComponent/PromptForStack and prompt error handling, and wire interactive flows for missing args.
Tests, harness & sanitization
\tests/``, \internal/exec/_test.go``, \cmd/terraform/*_test.go`**
Add/adjust tests for compat flags, completions, clean/shell/generate APIs, improve sanitizeOutput logic, and update numerous snapshots and fixtures.
Docs & guidance
\website/blog/``, \website/docs/cli/commands/terraform/.mdx``, \CLAUDE.md``
Add blog posts and CLI docs documenting the registry migration, interactive prompts, new flags (--everything, --force), and developer guidance updates.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    actor User
    participant CLI as atmos (Cobra)
    participant Preproc as Compat Preprocessor
    participant Registry as Compat Registry
    participant Cmd as Terraform Cobra Cmd
    participant Exec as Typed Executor (pkg/terraform / internal/exec)
    participant TF as Terraform/OpenTofu

    User->>CLI: atmos terraform ...flags...
    CLI->>Preproc: preprocessCompatibilityFlags(os.Args)
    Preproc-->>Registry: SetSeparated(separatedArgs)
    Preproc-->>CLI: atmosArgs (filtered)
    CLI->>Cmd: dispatch terraform command (RunE)
    Cmd->>Exec: build typed options, call ExecuteX(opts, atmosConfig)
    Exec->>Registry: GetSubcommandCompatFlags(provider, subcmd)
    Exec->>Preproc: GetSeparated() → separatedArgs
    Exec->>TF: invoke Terraform/OpenTofu with separatedArgs + generated files
    TF-->>Exec: output / exit
    Exec-->>Cmd: return result
    Cmd-->>User: stdout/stderr / exit

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

  • Areas needing extra attention:
    • cmd/root.go — preprocessCompatibilityFlags interactions with Cobra arg parsing and lifecycle of compat separated values.
    • cmd/internal/registry.go & tests — concurrency, overwrite semantics, provider/subcommand isolation.
    • cmd/terraform/* — Viper binding, completion registration, and correct handoff to typed Execute* APIs.
    • internal/exec/* and pkg/terraform/* — correctness of typed Execute* implementations (Clean, Shell, Varfile, Planfile, GenerateBackend), TF_DATA_DIR handling, and deprecated wrappers.
    • pkg/flags changes — completion plumbing (SetCompletionFunc/GetCompletionFunc), Registry() accessors and parse adjustments.
    • cmd/help_template.go & snapshots — formatting and accuracy of COMPATIBILITY FLAGS output; many updated golden files.

Possibly related PRs

  • cloudposse/atmos#1475 — closely related: exposes/auth-manager construction and auth wiring that align with the added CreateAuthManager wrapper.
  • cloudposse/atmos#1643 — overlaps the command-registry / CommandProvider migration and registry/provider changes introduced here.
  • cloudposse/atmos#727 — overlapping Terraform clean work (flags like --everything, --force) and related cleaning logic.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.55% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed Title clearly describes the main change: migrating Terraform commands to a registry pattern, which aligns with the substantial refactoring work across cmd/terraform/.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 399f135f4fab5b3bb2a364123534110e2b60ef36 and 75d0be2a3bd1f4b77cfe8d57aa0ce626907f36f9.

📒 Files selected for processing (10)
  • cmd/terraform/terraform.go (1 hunks)
  • tests/snapshots/TestCLICommands_atmos_terraform_-help_passthrough.stdout.golden (1 hunks)
  • tests/snapshots/TestCLICommands_atmos_terraform_-version_passthrough.stdout.golden (1 hunks)
  • tests/test-cases/help-and-usage.yaml (2 hunks)
  • website/docs/cli/commands/terraform/terraform-clean.mdx (2 hunks)
  • website/docs/cli/commands/terraform/terraform-deploy.mdx (1 hunks)
  • website/docs/cli/commands/terraform/terraform-destroy.mdx (1 hunks)
  • website/docs/cli/commands/terraform/terraform-output.mdx (1 hunks)
  • website/docs/cli/commands/terraform/terraform-shell.mdx (1 hunks)
  • website/docs/cli/commands/terraform/terraform-workspace.mdx (1 hunks)

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.

❤️ Share

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

coderabbitai[bot] avatar Nov 23 '25 00:11 coderabbitai[bot]

[!WARNING]

CLAUDE.md Too Large

The modified CLAUDE.md exceeds the 40000 byte size limit:

  • CLAUDE.md: 40792 bytes (over by 792 bytes, ~1%)

Action needed: Please refactor the oversized CLAUDE.md file. Consider:

  • Removing verbose explanations
  • Consolidating redundant examples
  • Keeping only essential requirements
  • Moving detailed guides to separate docs in docs/ or docs/prd/

All MANDATORY requirements must be preserved.

github-actions[bot] avatar Nov 23 '25 00:11 github-actions[bot]

[!WARNING]

This PR exceeds the recommended limit of 1,000 lines.

Large PRs are difficult to review and may be rejected due to their size.

Please verify that this PR does not address multiple issues. Consider refactoring it into smaller, more focused PRs to facilitate a smoother review process.

mergify[bot] avatar Nov 23 '25 00:11 mergify[bot]

Dependency Review

✅ No vulnerabilities or license issues found.

Scanned Files

None

github-actions[bot] avatar Nov 23 '25 00:11 github-actions[bot]

💥 This pull request now has conflicts. Could you fix it @osterman? 🙏

mergify[bot] avatar Nov 24 '25 23:11 mergify[bot]

[!WARNING]

Changelog Entry Required

This PR is labeled minor or major but doesn't include a changelog entry.

Action needed: Add a new blog post in website/blog/ to announce this change.

Example filename: website/blog/2025-12-16-feature-name.mdx

Alternatively: If this change doesn't require a changelog entry, remove the minor or major label.

github-actions[bot] avatar Nov 25 '25 00:11 github-actions[bot]

Codecov Report

:x: Patch coverage is 52.48780% with 974 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 72.93%. Comparing base (9ad6890) to head (75d0be2). :warning: Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
cmd/terraform/utils.go 31.55% 126 Missing and 15 partials :warning:
internal/exec/terraform_clean.go 36.07% 86 Missing and 15 partials :warning:
cmd/terraform/shared/prompt.go 29.80% 64 Missing and 9 partials :warning:
cmd/terraform/terraform.go 22.98% 66 Missing and 1 partial :warning:
cmd/help_template.go 0.00% 53 Missing :warning:
cmd/terraform/shell.go 19.67% 48 Missing and 1 partial :warning:
cmd/terraform/generate/planfile.go 30.50% 40 Missing and 1 partial :warning:
cmd/terraform/varfile.go 46.26% 34 Missing and 2 partials :warning:
cmd/terraform/clean.go 26.08% 33 Missing and 1 partial :warning:
internal/exec/terraform_shell.go 22.22% 28 Missing :warning:
... and 58 more
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1813      +/-   ##
==========================================
- Coverage   73.48%   72.93%   -0.56%     
==========================================
  Files         565      603      +38     
  Lines       54599    56024    +1425     
==========================================
+ Hits        40123    40860     +737     
- Misses      11603    12252     +649     
- Partials     2873     2912      +39     
Flag Coverage Δ
unittests 72.93% <52.48%> (-0.56%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
cmd/list/components.go 59.52% <100.00%> (+0.59%) :arrow_up:
cmd/list/metadata.go 21.66% <100.00%> (+1.66%) :arrow_up:
cmd/terraform/compat_flags.go 100.00% <100.00%> (ø)
cmd/terraform/completion.go 100.00% <100.00%> (ø)
cmd/terraform/flags.go 100.00% <100.00%> (ø)
cmd/terraform/options.go 100.00% <100.00%> (ø)
errors/errors.go 100.00% <ø> (ø)
internal/exec/terraform_utils.go 66.10% <100.00%> (-0.38%) :arrow_down:
pkg/flags/compat/compatibility_flags.go 91.72% <ø> (ø)
pkg/flags/global_registry.go 97.18% <100.00%> (+0.95%) :arrow_up:
... and 72 more

... and 4 files with indirect coverage changes

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Nov 30 '25 17:11 codecov[bot]

@coderabbitai full review

osterman avatar Dec 10 '25 23:12 osterman

✅ Actions performed

Full review triggered.

coderabbitai[bot] avatar Dec 10 '25 23:12 coderabbitai[bot]

💥 This pull request now has conflicts. Could you fix it @osterman? 🙏

mergify[bot] avatar Dec 13 '25 00:12 mergify[bot]

💥 This pull request now has conflicts. Could you fix it @osterman? 🙏

mergify[bot] avatar Dec 14 '25 02:12 mergify[bot]

[!IMPORTANT]

Cloud Posse Engineering Team Review Required

This pull request modifies files that require Cloud Posse's review. Please be patient, and a core maintainer will review your changes.

To expedite this process, reach out to us on Slack in the #pr-reviews channel.

mergify[bot] avatar Dec 15 '25 22:12 mergify[bot]

These changes were released in v1.202.0-rc.4.

github-actions[bot] avatar Dec 17 '25 01:12 github-actions[bot]

These changes were released in v1.203.0-test.1.

github-actions[bot] avatar Dec 19 '25 17:12 github-actions[bot]