Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 23 additions & 2 deletions access/.tflint.d/policies/security.rego
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,29 @@ deny_boundary_required contains issue if {
)
}

# Warning, because the estate declares no security groups yet, so the rule has
# no live coverage to ratchet against.
# Warning, because the estate declares no security groups of its own, so the
# rule has no live coverage to ratchet against. The second clause reads the
# inline ingress blocks of an aws_security_group, which is the shape
# terraform plan -generate-config-out writes for an adopted group (lesson
# 15), so an adopted group with an open port gets the warning the rule was
# written for rather than slipping past it on shape alone.
warn_no_open_ingress contains issue if {
some r in terraform.resources("aws_security_group", {"ingress": {"cidr_blocks": "list(string)", "ipv6_cidr_blocks": "list(string)", "from_port": "number", "to_port": "number"}}, {"expand_mode": "none"})
some ing in r.config.ingress
some key in ["cidr_blocks", "ipv6_cidr_blocks"]
attr := ing.config[key]
not attr.unknown
some cidr in attr.value
cidr in open_cidrs
issue := tflint.issue(
sprintf(
"aws_security_group.%s opens %v to %s in an inline ingress block. Name the source security group, and declare the rule as its own aws_vpc_security_group_ingress_rule so it has an address a review can point at.",
[r.name, ing.config.from_port.value, cidr],
),
attr.range,
)
}

warn_no_open_ingress contains issue if {
some r in terraform.resources("aws_vpc_security_group_ingress_rule", {"cidr_ipv4": "string", "cidr_ipv6": "string"}, {"expand_mode": "none"})
some key in ["cidr_ipv4", "cidr_ipv6"]
Expand Down
62 changes: 60 additions & 2 deletions access/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ access/
modules/
persona/ the four archetypes a principal file instantiates
backends/ the two backend files, one of which is copied into an env
scripts/ backend, check, and the lesson 6 to 11 scripts below
scripts/ backend, check, and the lesson 6 to 15 scripts below
codeowners.map team name to GitHub handle, the one place the two meet
.tflint.d/
policies/ the rule pack, as Rego
Expand Down Expand Up @@ -149,7 +149,7 @@ are Rego like the rest rather than a side script.
| `no-iam-user-or-group` | error | any IAM user, group, access key or attachment to one |
| `tag-owner-required` | error | a role, policy, bucket, registry or permission set with no `owner` tag |
| `boundary-required` | error | a role, or a principal file that makes one, with no `permissions_boundary` |
| `no-open-ingress` | warning | an ingress rule naming `0.0.0.0/0` or `::/0` |
| `no-open-ingress` | warning | an ingress rule, or an inline `ingress` block on a group, naming `0.0.0.0/0` or `::/0` |
| `sg-reference-not-cidr` | warning | an ingress rule naming a raw CIDR instead of a source group |
| `trust-subject-pinned` | error | a federated trust with no subject condition, a subject matched by pattern, or a subject carrying a wildcard |
| `trust-audience-pinned` | error | a federated trust with no `StringEquals` audience, or an OIDC provider that lists no client id or is not https |
Expand Down Expand Up @@ -931,3 +931,61 @@ declared because Identity Center is read only live, folds in the rotation
check, and closes with what it did not see. `access-review.yml` runs it
quarterly against a Floci the job filled itself, which proves the shape and
not a real account, and uploads the artifact for four hundred days.

## Lesson 15, adopt in place

A resource that existed before the repo comes under management one file at
a time, and nothing about it changes except the estate's own tags.

```
access/
envs/prod/
provider.tf ec2 joins the endpoint overrides for the first security group
<type>.<label>.tf an import block and the reviewed resource, one file each, on a laptop and never here
.tflint.d/policies/
security.rego no-open-ingress reads inline ingress blocks too
scripts/
adopt-check every import in a root, tags only or fail
```

### The shape

An adopted resource's file holds its `import` block and its resource block
together. The resource block is `terraform plan -generate-config-out`
reviewed by hand, with `region`, `tags_all`, every null and every provider
default dropped, so the file says what the account holds and nothing the
provider invented. One resource at a time (decision 38).

### The one change

The provider's `default_tags` put `managed_by`, `repo` and `env` on
everything the estate owns, so the plan for an import is never empty here.
`adopt-check` reads the plan JSON for every importing resource and passes
one whose only differing attribute is `tags_all`, and fails one that would
change anything else, naming the attribute, because an apply would then edit
the account rather than adopt it (decision 62).

```sh
access/scripts/adopt-check
access/scripts/adopt-check satellites/waterpark-runner
```

### Adoption exempts nothing

An adopted role with no boundary and no owner tag fails `boundary-required`
and `tag-owner-required` like a written one, and an adopted group with an
open port gets `no-open-ingress`, which now reads the inline `ingress`
shape generated config writes. The failing check after an adoption is day
two's list.

### Backing out

A `removed` block with `lifecycle { destroy = false }` takes a resource
out of state and leaves it in the account. It is applied once and then
deleted from the repo.

### Never in this tree

The adopted files import resources that exist only where a hand made them,
so they live on the laptop that adopted them and the lesson's compare
excludes them. The reference tree carries the mechanism and no adoption.
1 change: 1 addition & 0 deletions access/envs/prod/provider.tf
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ provider "aws" {
iam = endpoints.value
sts = endpoints.value
s3 = endpoints.value
ec2 = endpoints.value
}
}

Expand Down
1 change: 1 addition & 0 deletions access/scripts/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ jobs are thin wrappers around these rather than shell embedded in YAML.
| `expiring` | every dated grant, soonest first, read from the account | 11 |
| `offboard` | remove a principal and every reference, with a preview that reads the account | 11 |
| `access-review` | the quarterly artifact, every principal and what it can reach, read from the account | 11 |
| `adopt-check` | every import in a root, passing only the ones that change nothing but the estate's tags | 15 |

None of them holds a credential of its own. The ones that talk to an account
take the throwaway `test` key pair against Floci by default, `LIVE=true`
Expand Down
133 changes: 133 additions & 0 deletions access/scripts/adopt-check
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
#!/usr/bin/env bash
# Adopt in place, checked. Every resource an import block brings under
# management must arrive with nothing changed but the estate's own tags.
#
# access/scripts/adopt-check over envs/prod
# access/scripts/adopt-check <root> over another root
# access/scripts/adopt-check --json ... the same as JSON
#
# The mechanism is terraform plan -json, read for every resource change the
# plan marks as importing. An import that plans no change is the ideal. An
# import whose only change is tags_all is the estate's provider putting its
# default tags, managed_by, repo and env, on a resource it now owns, which
# is the one day-one change adoption makes here and the plan names it. Any
# other attribute in the change means the file does not say what the account
# holds, the review out of -generate-config-out missed something, and this
# script fails with the attribute named so the file is fixed before the
# apply rather than the account being edited by the apply (decision 62).
#
# Exit 0 when every import is clean or tags only. Exit 2 when one is not.
# Exit 1 when the plan itself failed.
set -uo pipefail

access_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
cd "$access_root" || exit 1

as_json=false
root="envs/prod"
while [ $# -gt 0 ]; do
case "$1" in
--json)
as_json=true
shift
;;
*)
root="$1"
shift
;;
esac
done
[ -d "$root" ] || {
echo "adopt-check: no such root: access/$root" >&2
exit 1
}

work="$(mktemp -d -t wp-adopt)"
trap 'rm -rf "$work"' EXIT

export AWS_ACCESS_KEY_ID="${AWS_ACCESS_KEY_ID:-test}"
export AWS_SECRET_ACCESS_KEY="${AWS_SECRET_ACCESS_KEY:-test}"
export AWS_DEFAULT_REGION="${AWS_DEFAULT_REGION:-us-east-1}"

terraform -chdir="$root" init -input=false -no-color >"$work/init.log" 2>&1 || {
echo "adopt-check: terraform init failed in access/$root" >&2
tail -5 "$work/init.log" >&2
exit 1
}
terraform -chdir="$root" plan -input=false -no-color -out="$work/tfplan" >"$work/plan.log" 2>&1 || {
echo "adopt-check: terraform plan failed in access/$root" >&2
tail -8 "$work/plan.log" >&2
exit 1
}
terraform -chdir="$root" show -json "$work/tfplan" >"$work/plan.json" 2>/dev/null

# One row per importing resource, with the attributes that differ between
# what the account holds and what the file says, tags_all set aside.
rows="$(jq -c '[
(.resource_changes // [])[]
| select(.change.importing != null)
| . as $c
| ($c.change.before // {}) as $b
| ($c.change.after // {}) as $a
| ([ (($b | keys? // []) + ($a | keys? // []) | unique)[]
| . as $k
| select(($b[$k] // null) != ($a[$k] // null))
]) as $diff
| { address: $c.address, type: $c.type, id: $c.change.importing.id,
actions: $c.change.actions,
tags_only: (($diff | map(select(. != "tags_all"))) | length == 0),
other: ($diff | map(select(. != "tags_all"))),
tags_added: (if ($diff | index("tags_all")) != null then (($a.tags_all // {}) | to_entries | map(select(($b.tags_all // {})[.key] == null) | .key)) else [] end) }
]' "$work/plan.json")"

# Import blocks whose resource is already in state make no importing change
# in the plan, so they are read off the files and reported as such, and a
# plan that would still change one of them is read the same way as above.
declared="$(grep -h -A1 '^import {' "$root"/*.tf 2>/dev/null | sed -n 's/^[[:space:]]*to[[:space:]]*=[[:space:]]*//p' | sort -u)"
while IFS= read -r addr; do
[ -n "$addr" ] || continue
if jq -e --arg a "$addr" '.[] | select(.address == $a)' <<<"$rows" >/dev/null; then
continue
fi
change="$(jq -c --arg a "$addr" '
(.resource_changes // [])[] | select(.address == $a)
| . as $c | ($c.change.before // {}) as $b | ($c.change.after // {}) as $a2
| { actions: $c.change.actions,
other: ([ (($b | keys? // []) + ($a2 | keys? // []) | unique)[] | . as $k
| select(($b[$k] // null) != ($a2[$k] // null)) ] | map(select(. != "tags_all"))) }' "$work/plan.json")"
if [ -z "$change" ] || [ "$(jq -r '.actions | join(",")' <<<"$change")" = "no-op" ]; then
rows="$(jq -c --arg a "$addr" '. + [{address: $a, type: "", id: "", actions: ["no-op"], tags_only: true, other: [], tags_added: [], in_state: true}]' <<<"$rows")"
else
rows="$(jq -c --arg a "$addr" --argjson ch "$change" '. + [{address: $a, type: "", id: "", actions: $ch.actions, tags_only: (($ch.other | length) == 0), other: $ch.other, tags_added: [], in_state: true}]' <<<"$rows")"
fi
done <<EOT
$declared
EOT
rows="$(jq -c 'sort_by(.address)' <<<"$rows")"

count="$(jq 'length' <<<"$rows")"
bad="$(jq '[.[] | select(.tags_only | not)] | length' <<<"$rows")"

if [ "$as_json" = true ]; then
jq -n --argjson r "$rows" --arg root "$root" '{root: $root, imports: $r}'
else
echo "adopt check, access/$root"
echo
if [ "$count" -eq 0 ]; then
echo " no import block in this root, so nothing is being adopted"
else
jq -r '.[] | " [" + (if .tags_only then "ok" else "FAIL" end) + "] " + .address + (if .id != "" then " from " + .id else " already in state" end)
+ (if (.actions | index("no-op")) != null then "\n nothing changes" else "" end)
+ (if (.tags_added | length) > 0 then "\n the estate adds its tags: " + (.tags_added | join(", ")) else "" end)
+ (if (.other | length) > 0 then "\n the file differs from the account on: " + (.other | join(", ")) + "\n fix the file, because an apply would edit the account" else "" end)' <<<"$rows"
fi
echo
if [ "$bad" -eq 0 ]; then
echo " $count adoption(s), and none changes anything but the estate's own tags."
else
echo " $bad of $count adoption(s) would change the account. Adopt in place means the file says what is there."
fi
fi

[ "$bad" -eq 0 ] && exit 0
exit 2
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# Fails no-open-ingress on the inline shape. This is what an adopted group
# looks like straight out of -generate-config-out, and the open port is the
# same open port whichever shape declares it.
resource "aws_security_group" "legacy_open" {
name = "legacy-open"
description = "ssh from anywhere"
vpc_id = "vpc-default-us-east-1"

ingress {
from_port = 22
to_port = 22
protocol = "tcp"
cidr_blocks = ["0.0.0.0/0"]
}

tags = {
owner = "platform"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Passes. An inline ingress from a private range is not the rule's concern,
# even if the sibling rule would still rather it named a source group.
resource "aws_security_group" "legacy_private" {
name = "legacy-private"
description = "ssh from the office"
vpc_id = "vpc-default-us-east-1"

ingress {
from_port = 22
to_port = 22
protocol = "tcp"
cidr_blocks = ["10.0.0.0/8"]
}

tags = {
owner = "platform"
}
}
Loading
Loading