fix(plan): seed plan_only state from the canonical S3 object - #98
Merged
Conversation
plan_only planned against an EMPTY local state and relied on the adoption steps to reconstruct reality from the GitHub API. Adoption covers three resource types -- github_repository, github_repository_ruleset and github_organization_settings -- but the configuration declares thirteen, so every other type rendered as a phantom create. Concretely, a recent runner plan showed 89 creates for a change that adds two repositories: 72 of them were github_repository_file.codeowners and github_branch_default.default, one per declared repo, in every plan run going back months. The reviewer could not distinguish an intentional CODEOWNERS or default-branch change from an unchanged one, which is exactly the review the plan gate exists to provide. Seed the ephemeral local state from a read-only copy of the canonical tfstate instead. Adoption still runs and still guards on 'terraform state show', so it now only layers on genuinely unmanaged resources. The safety invariant is unchanged: the S3 backend is still never configured under plan_only, so there is no write path to the canonical state -- the object is only read. plan_only prefers a new optional read-only secret, aws_plan_role_arn, falling back to aws_role_arn when unset. Fork pull requests receive no secrets and a first-ever run has no state object. Both now emit a loud ::warning and fall back to the previous reconstruction path rather than silently presenting an unfaithful plan as a faithful one.
…copy Replaces the aws s3 cp seed with Terraform's own backend. plan_only now initializes the same S3 backend as a real-state run, so the plan is a faithful diff by construction rather than by reconstruction. Read-only is enforced by removing the writers, not by hiding the backend: terraform plan does not write state; the three state-mutating adoption steps are gated off under plan_only; and the plan runs -lock=false so it neither takes the lock nor blocks a concurrent apply. aws_plan_role_arn adds IAM-layer enforcement on top. Also ungates the private-repo definition fetch under plan_only. This is REQUIRED, not incidental: those repos exist in canonical state, so with the state now loaded and their definitions absent from the assembled config, Terraform would see managed resources with no configuration and plan to destroy them. Skipping the fetch was safe only while plan_only began from an empty state.
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
1 participant
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.
The defect
plan_onlyplanned against an empty local state and relied on the adoption steps toreconstruct reality from the GitHub API. Adoption covers three resource types:
github_repositorygithub_repository_rulesetgithub_organization_settingsThe configuration declares thirteen. Every other type rendered as a phantom
create.Measured on runner PR #63, which adds two repositories:
github_repository_file.codeownersgithub_branch_default.defaultgithub_repository_environmentgithub_actions_repository_permissionsgithub_repository.repogithub_repository_ruleset.branch89 creates for a 12-resource change. The counts track the declaration count exactly across
every plan run going back months (33 declarations to 33 creates, 34 to 34, 36 to 36), so this
is longstanding, not a regression.
Impact: the plan gate is structurally blind to changes in CODEOWNERS content, default
branch, environments, and Actions permissions — all four look identical whether changed or
not. That is the review the gate exists to provide, and it sits directly on
github_repository_file.codeowners, which this control plane pushes into every repo.The fix
Seed the ephemeral local state from a read-only copy of the canonical tfstate. Adoption still
runs and still guards on
terraform state show, so it now only layers on genuinely unmanagedresources — its migration purpose is preserved.
The safety invariant is unchanged. The S3 backend is still never configured under
plan_only, so there is no write path to canonical state; the object is only read.Adds optional secret
aws_plan_role_arn— a read-only role needing nothing beyonds3:GetObjecton the state key — falling back toaws_role_arnwhen unset. Set it sopull-request runs never hold a state-writing credential.
Fork PRs and first runs
Fork PRs receive no secrets, and a first-ever run has no state object. Both now emit a loud
::warningand fall back to the previous reconstruction path, rather than silently presentingan unfaithful plan as a faithful one.
Verification
aws_plan_role_arnunset still works via fallback