The Production Plan Review

A Terraform PR is not a code change in the usual sense. The code change is the configuration; the code change is also the change request to the cloud. The reviewer is approving both. Most production Terraform incidents are not caused by code that does not work; they are caused by code that works but should not have shipped.

The right review of a Terraform PR is a rubric walk, not a diff walk. The reviewer reads the configuration, reads the plan, walks the rubric, and decides whether the change is safe to apply. The rubric is what catches the dangerous changes the diff walk misses. The diff walk tells you what the change is; the rubric tells you whether it should be.

The review rubric

Five questions, applied in order. The first four can be answered from the PR description and the plan file. The fifth is the human call.

1. What is the scope of the change?

Read the plan summary line: Plan: 4 to add, 2 to change, 0 to destroy.

A change of 4 to add and 2 to change is one kind of work. A single aws_db_instance that is being replaced is another kind of work entirely, and the summary line will not say so: Terraform counts only adds, changes, and destroys, and it folds a replacement into one add and one destroy. So the summary tells you the size of the change and nothing about its shape. Get the shape from the body of the plan:

# READ-ONLY: count the replacements the summary line hides.
# The trailing "resource " keeps the legend at the top of the
# plan out of the count.
terraform show -no-color tfplan | grep -cE '^\s*[-+]/[+-] resource '

Map the plan to the expected work:

Plan shape          Work type                   Review posture
-------             ---------                   --------------
0 changes           Configuration and state     Approve
                    agree; no drift             ("nothing to do")

1 to 5 add          Net new resource            Read every address
1 to 5 change       In-place updates            Read the diff
1 to 5 destroy      Dead resource removal       Confirm intent
Any -/+ or +/-      Forced replacements         Read the resource
                                                carefully

5+ changes          Substantial change          Read the PR
                                                description first

10+ to destroy      Mass removal                Stop and verify
                                                with author

Any change to       High-blast-radius           Read carefully
state, identity,
or import

The map is not a rule; it is a posture. A 1-resource change to an IAM role is more consequential than a 10-resource change to tags.

2. What is the blast radius?

For every resource being changed, name the consumers. A change to a security group affects every instance that references it. A change to an IAM role affects every service that assumes it. A change to a subnet affects every resource deployed there.

The reviewer names the blast radius in the PR comment even when it is small. A change to a tag has a blast radius of “the autoscaler that consumes the tag”; naming that is the discipline.

3. Is the change reversible?

Three categories of reversibility:

• Trivially reversible. A change to a tag, a change to a description, a change to a non-force-new attribute. The apply can be rolled back by reverting the change in the next PR.
• Reversible with cost. A change that creates new resources. The rollback is to destroy them. The cost is whatever the resources cost to run for the window between apply and rollback.
• Not reversible without data loss. A replacement, a destroy of a resource that owns data, a state operation. The change cannot be undone by another change in the configuration.

A PR that proposes a “not reversible” change requires a justification that is more than “the configuration says so”. The justification is reviewed by a senior engineer.

4. Does the change match the change ticket?

The PR is associated with a ticket. The ticket describes the work: a feature, a bug fix, a security request, a compliance review. The change should match the ticket.

Ticket:  Disable public S3 buckets; #SEC-1042
PR:      AWS017 ignores on every public ingress
Match?   No. The ticket says disable; the PR adds
         ignores. The reviewer rejects; the author
         reopens the ticket and re-proposes.

The first pass of the rubric is the alignment between the ticket and the PR. A PR that solves a different problem is a PR that does not belong in the queue.

5. Do you, the reviewer, trust this change with your

production account?

This is the call that the rubric cannot make for you. The first four questions produce the data; the fifth is the human judgement. If the data does not yet support the call, the right action is to ask for more information or to reject with a reason.

What to ignore

A Terraform PR carries a lot of noise. The discipline includes what to skip.

Skip whitespace and formatting-only changes. A PR that is purely terraform fmt produces thousands of lines of diff in a large module. The change is real, but the risk is low. A label on the PR (formatting-only) and an approval from one reviewer suffice.

Skip CI re-runs. A second push that triggers a re-plan, with no code change, produces a new comment that is functionally identical to the first. Read the diff between the two comments; if there is no diff, the re-run is noise.

Skip bot comments. Renovate, Dependabot, and the like produce PRs that bump provider versions, module versions, or GitHub Actions versions. These are noise in the queue but they are not noise for review. The PR is read with the same rubric as a human PR. What is ignored is the per-update diff churn; the PR substance is read.

Skip the second reviewer if the rubric is satisfied. The 2-eye principle means two reviewers, not three, not five. Once two competent reviewers have walked the rubric and the data matches, additional reviewers add process cost without information.

Comment discipline

The PR comment is a record. The comment is visible to future reviewers, audit, and the post-incident reviewer three months later. Write the comment as if it will be read in an incident review.

Do comment on:

• The blast radius in concrete terms.
• The reversibility category.
• The change-ticket alignment (or absence).
• Specific lines that are wrong.
• A question that the author should answer in the PR description, not in a long thread.

Do not comment on:

• Style preferences that are not in the style guide.
• The author’s choice of count vs for_each. Both are valid; the choice is theirs.
• The plan output (the plan is in the artifact store; link to it, do not paste it).
• Opinions on provider choice (use a different provider is a multi-PR conversation, not a PR comment).
• Personal remarks about the author.

A comment like “I would have done this differently” is a comment that adds nothing. A comment like “This changes the IAM role’s trust policy; what is the consumer of the new trust?” is a comment that asks the right question.

The CODEOWNERS setup

CODEOWNERS is a GitHub file (and equivalents on GitLab, Bitbucket) that maps file paths to required reviewers. The right CODEOWNERS for a Terraform repository routes the high-blast-radius paths to senior engineers automatically; the bulk paths to anyone with merge rights.

# Default: any engineer with merge rights
*                                       @runbook/iac-engineers

# Module surface area: senior review
/modules/**                             @runbook/senior-platform

# High-blast-radius resources: senior review
**/security_group*                      @runbook/security-reviewers
**/iam_*                               @runbook/security-reviewers
**/route_table*                        @runbook/network-reviewers
**/kms_*                               @runbook/security-reviewers

# State operations: state custodian
/state/**                              @runbook/state-custodian

# Provider version bumps: anyone
/versions.tf                           @runbook/iac-engineers

Three rules:

1. Specific paths before wildcards. The first matching rule wins. Put the specific rule above the general one.
2. Use teams, not individuals. @runbook/security-reviewers is a team. If the team has three members, the round-robin assigns the review; if one is on holiday, the others cover.
3. Keep the list short. A 50-line CODEOWNERS is a CODEOWNERS that nobody reads. Six to twelve entries is the production size.

CODEOWNERS enforces automatic assignment; it does not enforce automatic approval. The reviewer still applies the rubric.

The PR template

A PR template is a .github/pull_request_template.md file populated on every PR. The template is the cheap forcing function: every PR has the same fields; the reviewer can spot the missing field in one second.

## What does this PR do, and why?
<!-- One paragraph. The first sentence should be a
description of the change. The second should be the
ticket or incident reference. -->

## Change scope
<!-- Tick one or more.
- [ ] Net new resource(s)
- [ ] In-place update to existing resource(s)
- [ ] Destroy of resource(s)
- [ ] Forced replacement (destroy and create)
- [ ] State operation (mv, rm, import)
- [ ] Provider or module version bump only
-->

## Blast radius
<!-- Which resources, services, or consumers does this
change affect? -->

## Reversibility
<!-- Trivially reversible / Reversible with cost /
Not reversible without data loss -->

## Plan output
<!-- Link to the plan artifact. Do not paste the plan
output here; it is in the artifact store. -->

## How was this tested?
<!-- Staging apply? Plan against a throwaway account?
None yet? State that explicitly. -->

## Checklist
<!--
- [ ] terraform fmt -check
- [ ] terraform validate
- [ ] tflint clean
- [ ] tfsec / checkov / trivy clean (or documented
  suppressions with justifications)
- [ ] State backup taken (if state operation)
-->

Three properties of the right template:

1. The first field is the why. A change without a reason is a change that cannot be evaluated.
2. The blast radius and reversibility fields are required. The reviewer can spot their absence.
3. The checklist names the scans the pipeline runs. A checked box is the author’s attestation; the pipeline’s exit code is the system’s attestation. Both are present.

Validation: walk a sample PR

PR #1042: Tighten web tier SG to bastion only
File:        network/sg.tf
Plan:        0 to add, 2 to change, 0 to destroy
Author:      e.brandi
Reviewer:    k.osadi (CODEOWNERS auto-assign)

Walking the rubric:

1. Scope. 2 changes, 0 adds, 0 destroys, so nothing is being replaced. Limited scope. Read every line.
2. Blast radius. Security group rule changes affect every resource attached to the SG. List the resources: aws_instance.web[0..2], aws_lb.web. Verify by terraform state list | grep sg-.
3. Reversibility. Both rules are non-ForceNew; trivially reversible.
4. Change ticket. Ticket SEC-1042 calls for bastion- only SSH. The PR adds bastion SG ingress and removes 0.0.0.0/0. Matches.
5. Trust. The blast radius is the web tier; the bastion is documented in the network module; the change matches the ticket. Approve.

A clean walk takes a few minutes for a small PR and 15-30 minutes for a large one. A walk that finds an unknown takes longer; the unknown is the reason for the walk.

Production failure modes

1. The reviewer approved a 200-line plan because “it only touches tags”. The plan also included a ~ on an IAM role’s policy document that was not visible without scrolling. The fix is to scroll every plan fully; the rubric forces the walk.

2. The reviewer approved a state operation without reading the state backup. The terraform state rm removed the resource; the next apply recreated it empty; the database was lost. The fix is to require the state backup filename in the PR description and to verify the backup exists before approval.

3. The PR template was empty. The author submitted a PR with the default body. The reviewer had no context. The review was a code diff in a vacuum; the change passed. The fix is to require the template or reject the PR.

4. CODEOWNERS was bypassed with a direct push. An engineer pushed directly to main with --force. The CODEOWNERS check was bypassed. The merge was reverted. The fix is to enforce branch protection; direct pushes are not allowed.

5. The reviewer rubber-stamped the PR because they trusted the author. The PR introduced a public S3 bucket. The scanner flagged it; the reviewer ignored the scanner output. The fix is to fail the CI on the scanner and to require the reviewer to read the scanner output before approving.

6. The blast radius was not named. The reviewer approved a change to a route table that propagated to ten VPCs. The rollback required ten coordinated changes. The fix is the field in the PR template; the field forces the answer.

Security and performance

Security: the PR review is the human layer of the security pipeline. The scanners catch the patterns they recognise; the reviewer catches the patterns they recognise. The two are not the same. A security- critical change that does not match any scanner rule is caught by the reviewer, not by the gate. The gate is necessary; the reviewer is the supplement.

Performance: a good review takes 15 minutes for a small PR and up to an hour for a large PR. The cost is real and it is a budget item. Teams that under-budget the review end up with reviewers who skim, and skimming is the failure mode that the rubric addresses.

Production guidance

• One template, every PR. The template forces the right questions.
• CODEOWNERS routes the high-blast-radius paths automatically. Do not bypass CODEOWNERS with a manual assign.
• Comment on substance, not style. The PR comment is a record.
• Walk the rubric on every PR. The cost of the walk is smaller than the cost of the change you missed.
• Two reviewers is the production default. Three is an exception. Five is theatre.

What comes next

The next lesson is the network-change review: the diff to look for in security groups, NACLs, and route tables, the cost of a permissive change, and the right approval workflow.

Verification

Take a real Terraform PR (open in the team’s last week) and walk the rubric against it without checking the author’s PR description. Note where the rubric forces an answer that the description did not volunteer. Note where the description volunteered an answer the rubric did not ask for. The exercise is the discipline.