cloudposse / cloudposse/atmos

refactor: consolidate terraform hook wrappers into cmd/terraform/shared

Open
#2,522 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
1.4k
Forks
175
Avg merge
2d 3h
Merged PRs (30d)
134

Description

### Describe the Refactor

The terraform-component hook wrappers in `cmd/terraform/utils.go` (`runHooks`, `runHooksWithOutput`, `runHooksOnError`, `runHooksOnErrorWithOutput`) and the new wrappers in `cmd/terraform/generate/hooks.go` (`runGeneratePlanfileHooks`, `runGeneratePlanfileErrorHook`, added in #2497 PR A) implement the same option-construction → `RunCIHooks` plumbing in two places.

Today the duplication exists because the import direction is one-way: `cmd/terraform` imports `cmd/terraform/generate` ([cmd/terraform/terraform.go#L64](https://github.com/cloudposse/atmos/blob/main/cmd/terraform/terraform.go#L64)), so the `generate` package cannot reuse the unexported wrappers in the parent package without creating a cycle.

### Proposed Solution

Move the hook wrappers into a new file `cmd/terraform/shared/hooks.go` and export them:

- `shared.RunHooks(event, cmd, args) error`
- `shared.RunHooksWithOutput(event, cmd, args, output) error`
- `shared.RunHooksOnError(event, cmd, args, cmdErr)`
- `shared.RunHooksOnErrorWithOutput(event, cmd, args, cmdErr, output)`

Then update all four call sites — plan, apply, deploy, generate-planfile — to use the shared functions. Delete the duplicated `runGeneratePlanfileHooks` / `runGeneratePlanfileErrorHook` in `cmd/terraform/generate/hooks.go`.

### Scope Considerations

- `runHooksWithOutput` in `cmd/terraform/utils.go` includes `resolveComponentPath` (only relevant for path-arg commands) and `--verify-plan` flag reading. The generate-planfile wrapper doesn't need either. The consolidated version should keep both call-sites correct — likely by making the path-resolution and verify-plan reads conditional on flag presence (already happens for verify-plan via `_ = cmd.Flags().GetBool("verify-plan")` returning false for commands that don't define it).
- `wirePerComponentHook` in `cmd/terraform/utils.go` does NOT need to move — only plan/apply/deploy use it and they all live in `cmd/terraform`. The generate package doesn't support multi-component flags today.
- The `wasMultiComponentExecution` sentinel and the `runHooksOnErrorWithOutput` package-level var (used for stubbing in deploy-defer-guard tests) need to migrate as well; tests in `cmd/terraform/utils_hooks_test.go` will need to reference the shared package.

### Why this is a follow-up, not part of #2497 PR A

PR A is scaffold for the hook lifecycle integration — small, reviewable, and contained. A cross-package refactor that touches plan / apply / deploy / generate-planfile would inflate the diff and obscure the actual feature being added. Separating the refactor lets it land independently with focused review.

### Acceptance Criteria

- [ ] `cmd/terraform/shared/hooks.go` defines the four exported wrappers.
- [ ] `cmd/terraform/utils.go` no longer defines `runHooks*` (or keeps them as thin shims that delegate to `shared`, marked deprecated).
- [ ] `cmd/terraform/generate/hooks.go` is deleted (its TODO marker is the trigger for this issue).
- [ ] `cmd/terraform/utils_hooks_test.go` tests still pass against the relocated implementations.
- [ ] No new behavior — pure refactor, identical CI semantics.

### References

- TODO marker added in #2497 PR A: `cmd/terraform/generate/hooks.go` lines 25-28.
- Existing wrappers: [`cmd/terraform/utils.go`](https://github.com/cloudposse/atmos/blob/main/cmd/terraform/utils.go#L36-L82).

Contributor guide

Open the contributing guide

Research direction

Start by comparing the wrappers in cmd/terraform/utils.go and cmd/terraform/generate/hooks.go, then inspect the plan, apply, deploy, and generate-planfile call sites. Run cmd/terraform/utils_hooks_test.go while tracking the sentinel and stubbed error wrapper; done means the shared wrappers serve all four paths and the tests pass with identical CI behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, terraform
Domain
cli, devops
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.