diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index 6ad5985f3..580a62d26 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -845,7 +845,7 @@ Spinifex both stores externally-issued certificates (`import-certificate`) and i ## RDS (PostgreSQL) -Each DB instance is one dedicated system-owned VM running the engine directly, launched from the `spinifex-rds-postgres` AMI, tagged `spinifex:managed-by=rds` and therefore hidden from the customer's EC2 API. The engine is reached over a customer-account ENI injected into a subnet of the DB subnet group, so **the endpoint is private — reachable from inside the VPC only**. `Endpoint.Address` is `{db-instance-identifier}.{account-id}.{region}.rds.{base-domain}` where northstar is configured, and the endpoint ENI's private IP where it is not; the IP is stable across VM replacement either way. Default port 5432. +Each DB instance is one dedicated system-owned VM running the engine directly, launched from the `spinifex-rds-postgres` AMI, tagged `spinifex:managed-by=rds` and therefore hidden from the customer's EC2 API. The engine is reached over a customer-account ENI injected into a subnet of the DB subnet group, so **the endpoint is private — reachable from inside the VPC only**, from any subnet of it rather than the endpoint ENI's own. `Endpoint.Address` is `{db-instance-identifier}.{account-id}.{region}.rds.{base-domain}` where northstar is configured, and the endpoint ENI's private IP where it is not; the IP is stable across VM replacement either way. Default port 5432. - **Engine:** `postgres` 18 only — `EngineVersion` accepts `18` or `18.x`, and there is no in-place upgrade. - **Instance classes:** `db.t3.{micro,small,medium,large}` and `db.m5.{large,xlarge}` — a naming facade over the platform's EC2 sizing table. Any other class is rejected at create. diff --git a/docs/terraform-workbooks/rds-quickstart/README.md b/docs/terraform-workbooks/rds-quickstart/README.md index a5f89e495..187b7cc8e 100644 --- a/docs/terraform-workbooks/rds-quickstart/README.md +++ b/docs/terraform-workbooks/rds-quickstart/README.md @@ -27,25 +27,27 @@ resources: # Terraform: RDS Quickstart -> The smallest end-to-end RDS example on Spinifex: a VPC, a DB subnet group, a parameter group, a PostgreSQL DB instance, and a client VM in the same subnet with `psql` already pointed at the endpoint. +> The smallest end-to-end RDS example on Spinifex: a VPC with a public client subnet and a private DB subnet, a DB subnet group, a parameter group, a PostgreSQL DB instance, and a client VM with `psql` already pointed at the endpoint. ## Overview A Spinifex DB instance runs in a VM you never see. What lands in your VPC is a single **endpoint ENI** in one of the DB subnet group's subnets, so the endpoint is **always private** — there is no `publicly_accessible` mode, and a request for one is rejected. That is why this workbook builds a client VM: it is the only place from which the database can be reached. -The layout is the one that shape implies: +The layout is the one that shape implies — the tier that talks to the database in its own subnet, and the database in a subnet with no route off the VPC: ``` IGW │ - 10.60.1.0/24 ──┬── client VM ──psql:5432──┐ - │ ▼ - └─────────────────── endpoint ENI ──▶ DB VM (platform-owned) + client subnet 10.60.1.0/24 ──── client VM + │ + psql:5432 + │ + db subnet 10.60.2.0/24 ──── endpoint ENI ──▶ DB VM (platform-owned) ``` -The client and the DB subnet group **share one subnet**, and in v1 they have to. The endpoint is reachable from clients on the endpoint ENI's own subnet only: the DB VM carries its default route on its platform-side interface, so a reply to a client in another subnet is never routed back. A subnet group spanning several subnets places the endpoint in one of them and leaves clients in the others timing out on connect. On AWS you would use two private subnets here instead. +The endpoint is reachable from **any subnet of the VPC**, so the client does not have to share the subnet the endpoint ENI landed in — which also means a subnet group spanning several subnets works wherever the endpoint is placed within it. -The shared subnet is "public" only in that it has an internet-gateway route, which the client needs to `apt-get` a psql. The endpoint ENI gets no public address and `publicly_accessible = true` is rejected, so sharing the subnet does not expose the database. +The client subnet is public in that it has an internet-gateway route, which the client needs to `apt-get` a psql. The DB subnet has no route table association, so it stays on the main route table `create-vpc` writes: intra-VPC routing and nothing else. The endpoint ENI gets no public address and `publicly_accessible = true` is rejected either way. The database security group admits `5432` from the client security group and nothing else, which compiles to an ACL on the endpoint ENI itself — the port is deny-by-default for everything else in the VPC. @@ -148,7 +150,7 @@ aws rds delete-db-instance --db-instance-identifier rds-quickstart \ - **`storage_encrypted = true` is set explicitly.** Storage is always encrypted, so the instance reports `true` whether or not you asked. The provider's attribute is optional but *not* computed, so leaving it out means the read-back carries a value your configuration does not — and every subsequent `plan` shows a change on an instance nothing has touched. - **`instance_class` and `engine_version` are literal.** `describe-db-engine-versions` and `describe-orderable-db-instance-options` are not implemented, so the `aws_rds_engine_version` and `aws_rds_orderable_db_instance` data sources are unavailable. `aws_db_instance` needs neither. -- **One subnet, shared.** AWS requires a DB subnet group to span two AZs. Spinifex is single-AZ — every subnet reports the same zone — so the group accepts any count, and one is what the v1 reachability rule above allows. +- **A DB subnet group over one subnet.** AWS requires a DB subnet group to span two AZs. Spinifex is single-AZ — every subnet reports the same zone — so the group accepts any count, and a second private subnet here would buy nothing but a second CIDR. - **The password is written to `~/.pgpass`, not to the environment.** A `PGPASSWORD` in `/etc/profile.d` is readable by every user on the client and leaks into `ps` output. ## Troubleshooting @@ -166,8 +168,8 @@ aws rds describe-events --source-type db-instance --source-identifier rds-quicks **`psql` hangs on the client.** Three causes, in the order worth checking: - cloud-init has not finished installing `postgresql-client` — check `cloud-init status --long`. -- the client is not in the endpoint ENI's subnet. Off-subnet clients cannot reach the endpoint in v1; put them in the DB subnet group's subnet, as this workbook does. - the security group is not letting you through. Nothing outside the client security group can open `5432` — that is the point of the rule — so a psql from your workstation will always time out. +- the client is outside the VPC. The endpoint exists only inside it: any subnet will do, but there is no path to it from anywhere else. **`InsufficientInstanceCapacity` on the DB instance.** The node admits a launch against live free memory. Free some, or drop to a smaller `db_instance_class`. diff --git a/docs/terraform-workbooks/rds-quickstart/main.tf b/docs/terraform-workbooks/rds-quickstart/main.tf index 9cf7f1070..6a888a950 100644 --- a/docs/terraform-workbooks/rds-quickstart/main.tf +++ b/docs/terraform-workbooks/rds-quickstart/main.tf @@ -10,13 +10,11 @@ # only way to reach the database is from inside the VPC, which is what the # client instance here is for. # -# The client and the DB subnet group deliberately share ONE subnet. In v1 the -# endpoint is reachable from clients on the endpoint ENI's own subnet only: the -# DB VM carries its default route on a platform-side interface and the customer -# ENI is pure ingress, so a reply to a client in another subnet is never routed -# back. A subnet group spanning several subnets places the endpoint in one of -# them and leaves clients in the others unable to connect. On AWS you would use -# two private subnets here instead. +# Two subnets: the client sits in a public one, because it needs an +# internet-gateway route to apt-get a psql, and the database in a private one +# with no route off the VPC at all. The endpoint is reachable from any subnet of +# the VPC, so the tier that talks to the database does not have to share the +# subnet the endpoint ENI landed in. # # Usage: # cd spinifex/docs/terraform-workbooks/rds-quickstart @@ -169,7 +167,7 @@ data "aws_ami" "ubuntu" { } # --------------------------------------------------------------------------- -# VPC — one subnet, shared by the client and the DB subnet group +# VPC — a public client subnet and a private DB subnet # --------------------------------------------------------------------------- resource "aws_vpc" "main" { @@ -190,18 +188,29 @@ resource "aws_internet_gateway" "igw" { } } -# Public only in the sense that it has an internet-gateway route, which the -# client needs to apt-get a psql. The endpoint ENI in it is given no public -# address, and PubliclyAccessible=true is rejected, so the database is not -# exposed by sharing the subnet. -resource "aws_subnet" "main" { +# Public: an internet-gateway route and a public address on launch, which is how +# the client reaches apt and how you SSH to it. +resource "aws_subnet" "client" { vpc_id = aws_vpc.main.id cidr_block = "10.60.1.0/24" availability_zone = data.aws_availability_zones.available.names[0] map_public_ip_on_launch = true tags = { - Name = "${var.name}-subnet" + Name = "${var.name}-client-subnet" + } +} + +# Private: no association, so it stays on the main route table create-vpc writes, +# which routes inside the VPC and nowhere else. The endpoint ENI lands here. +resource "aws_subnet" "db" { + vpc_id = aws_vpc.main.id + cidr_block = "10.60.2.0/24" + availability_zone = data.aws_availability_zones.available.names[0] + map_public_ip_on_launch = false + + tags = { + Name = "${var.name}-db-subnet" } } @@ -218,8 +227,8 @@ resource "aws_route_table" "public" { } } -resource "aws_route_table_association" "main" { - subnet_id = aws_subnet.main.id +resource "aws_route_table_association" "client" { + subnet_id = aws_subnet.client.id route_table_id = aws_route_table.public.id } @@ -288,7 +297,7 @@ resource "aws_security_group" "db" { resource "aws_db_subnet_group" "main" { name = "${var.name}-subnets" description = "RDS quickstart DB subnets" - subnet_ids = [aws_subnet.main.id] + subnet_ids = [aws_subnet.db.id] tags = { Name = "${var.name}-subnets" @@ -356,7 +365,8 @@ resource "aws_db_instance" "main" { } # --------------------------------------------------------------------------- -# Client VM — psql inside the VPC, which is the only place the endpoint exists +# Client VM — psql inside the VPC, which is the only place the endpoint exists, +# from the client subnet rather than the endpoint's own # --------------------------------------------------------------------------- resource "tls_private_key" "client" { @@ -377,7 +387,7 @@ resource "local_file" "client_pem" { resource "aws_instance" "client" { ami = data.aws_ami.ubuntu.id instance_type = var.instance_type - subnet_id = aws_subnet.main.id + subnet_id = aws_subnet.client.id vpc_security_group_ids = [aws_security_group.client.id] key_name = aws_key_pair.client.key_name diff --git a/scripts/images/common/mulga-mgmt-net.sh b/scripts/images/common/mulga-mgmt-net.sh index cf14e757b..c20f0e0fc 100644 --- a/scripts/images/common/mulga-mgmt-net.sh +++ b/scripts/images/common/mulga-mgmt-net.sh @@ -8,7 +8,9 @@ # - NIC_DHCP=1: the primary data ENI. OVN serves DHCP; we lease it (retrying # one-shots on a budget until the cross-host datapath is up) so the Ec2 IMDS # datasource can reach 169.254.169.254, and pin a /32 to it so a link-local -# 169.254.0.0/16 route on another NIC cannot hijack IMDS. +# 169.254.0.0/16 route on another NIC cannot hijack IMDS. A DHCP NIC with +# NIC_DEFAULT=0 (an RDS DB VM's customer ENI) instead loses its leased +# default route and gets a source-based policy table for its replies. # - NIC_CIDR: a static NIC (mgmt0 on br-mgmt, which has no DHCP). Applied # address-only; NIC_DEFAULT is 0, so mgmt0 is never the default route. # @@ -23,8 +25,9 @@ # cloud-init's network stage re-renders every ENI from IMDS and re-DHCPs # them, which puts back the default route the setup pass deleted from a # NIC_DEFAULT=0 interface — leaving the guest with two default routes and -# the customer ENI able to capture egress and IMDS. Nothing re-leases or -# re-addresses an interface in this pass. +# the customer ENI able to capture egress and IMDS. It also rewrites that +# NIC's policy table, whose routes went down with the bounced interface. +# Nothing re-leases or re-addresses an interface in this pass. set -eu NETCFG="${MULGA_NETCFG:-/sys/firmware/qemu_fw_cfg/by_name/opt/spinifex/netcfg/raw}" @@ -96,6 +99,65 @@ drop_default_route() { done } +# Network address of a leased CIDR, computed rather than read back from +# `ip route show`, so the policy table's contents do not depend on how a given +# `ip` renders a connected route. +cidr_network() { + echo "$1" | awk -F/ '{ + split($1, o, ".") + v = o[1] * 16777216 + o[2] * 65536 + o[3] * 256 + o[4] + blk = 2 ^ (32 - $2) + n = int(v / blk) * blk + printf "%d.%d.%d.%d/%d\n", int(n / 16777216) % 256, int(n / 65536) % 256, \ + int(n / 256) % 256, n % 256, $2 + }' +} + +# A NIC_DEFAULT=0 DHCP NIC (an RDS DB VM's customer ENI) has its leased +# default route deleted, so a reply to a client outside its own subnet leaves by +# the primary ENI with this NIC's source address and is dropped. Give it a +# private table and steer its replies at it by source address. +# +# Matching on source needs no knowledge of the customer VPC's CIDR: the engine +# binds its socket to this address, so every reply carries it whoever the client +# is. Every command reports its own failure — a `set -e` trip here is silent and +# would leave every later NIC in the blob unconfigured. +install_return_policy() { + iface="$1" + tid=$(( 100 + $2 )) + + addr=$(ip -4 addr show dev "$iface" 2>/dev/null | awk '$1 == "inet" { print $2; exit }') + if [ -z "$addr" ]; then + echo "[mulga-mgmt-net] ERROR: no address on $iface; no return-path policy" >&2 + return 0 + fi + ip4="${addr%%/*}" + net=$(cidr_network "$addr") + # The lease's gateway is OVN's per-subnet router IP, which reaches every + # subnet of the VPC. Read it before the default route is dropped from main. + gw=$(ip -4 route show default dev "$iface" 2>/dev/null | awk '{print $3; exit}') + + # Both passes rewrite the table, not just the rule: a rule survives an + # interface bounce but the routes go down with the device, and a rule whose + # table is empty falls through to main — the asymmetry this exists to fix. + ip route replace "$net" dev "$iface" scope link src "$ip4" table "$tid" || \ + echo "[mulga-mgmt-net] ERROR: failed to add $net to table $tid on $iface" >&2 + if [ -n "$gw" ]; then + ip route replace default via "$gw" dev "$iface" table "$tid" || \ + echo "[mulga-mgmt-net] ERROR: failed to add default via $gw to table $tid on $iface" >&2 + else + echo "[mulga-mgmt-net] no lease gateway on $iface; leaving table $tid's default as-is" + fi + + # A rule added twice is listed twice, so drop it by priority first. The + # delete is expected to fail on the first pass of a boot. + ip rule del pref "$tid" 2>/dev/null || true + ip rule add from "$ip4" table "$tid" pref "$tid" || \ + echo "[mulga-mgmt-net] ERROR: failed to add rule from $ip4 to table $tid" >&2 + + echo "[mulga-mgmt-net] return-path policy on $iface: from $ip4 lookup $tid" +} + for n in 0 1 2 3 4 5; do # ${VAR:-} keeps the unset NIC_* slots safe under `set -u`. eval "mac=\${NIC${n}_MAC:-}" @@ -138,6 +200,7 @@ for n in 0 1 2 3 4 5; do # its own default route and, left alone, would both blackhole egress and # steal IMDS from the primary ENI. if [ "$isdefault" != "1" ]; then + install_return_policy "$iface" "$n" drop_default_route "$iface" if [ "$MODE" = "enforce" ]; then echo "[mulga-mgmt-net] re-asserted no default route on $iface ($mac)" diff --git a/scripts/images/common/mulga-mgmt-net_test.sh b/scripts/images/common/mulga-mgmt-net_test.sh index 941ebf59b..cfa80b288 100644 --- a/scripts/images/common/mulga-mgmt-net_test.sh +++ b/scripts/images/common/mulga-mgmt-net_test.sh @@ -16,10 +16,12 @@ trap 'rm -rf "${WORK}"' EXIT STUBBIN="${WORK}/bin" mkdir -p "${STUBBIN}" -# ip stub: records every invocation and answers the three queries the script -# makes. Default routes live one per line in DEFAULTS as " ", so -# a `route del` really removes one and a re-query sees the result. The interface -# is the last argument in every form the script uses. +# ip stub: records every invocation and answers the queries the script makes. +# Default routes live one per line in DEFAULTS as " ", so a +# `route del` really removes one and a re-query sees the result; policy routes +# live in TABLES as " " and rules in RULES as +# "
", so idempotency is observable rather than inferred +# from the call log. The interface is the last argument in the dev-scoped forms. cat > "${STUBBIN}/ip" <<'EOF' #!/bin/sh echo "ip $*" >> "${IP_CALLS}" @@ -32,9 +34,39 @@ case " $* " in *" route del default dev "*) awk -v d="${iface}" '$1 != d' "${DEFAULTS}" > "${DEFAULTS}.tmp" mv "${DEFAULTS}.tmp" "${DEFAULTS}" ;; + *" route replace "*" table "*) + dst=""; tid=""; prev="" + for a in "$@"; do + [ "${prev}" = "replace" ] && dst="${a}" + [ "${prev}" = "table" ] && tid="${a}" + prev="${a}" + done + # `replace` overwrites the entry for that destination, as the real one does. + grep -v "^${tid} ${dst} " "${TABLES}" > "${TABLES}.tmp" 2>/dev/null + mv "${TABLES}.tmp" "${TABLES}" + echo "${tid} ${dst} $*" >> "${TABLES}" ;; + *" rule add "*) + src=""; tid=""; pref=""; prev="" + for a in "$@"; do + [ "${prev}" = "from" ] && src="${a}" + [ "${prev}" = "table" ] && tid="${a}" + [ "${prev}" = "pref" ] && pref="${a}" + prev="${a}" + done + # Appended unconditionally: a real `ip rule add` of a duplicate lists it twice. + echo "${pref} ${src} ${tid}" >> "${RULES}" ;; + *" rule del "*) + pref=""; prev="" + for a in "$@"; do + [ "${prev}" = "pref" ] && pref="${a}" + prev="${a}" + done + grep -q "^${pref} " "${RULES}" || exit 2 + grep -v "^${pref} " "${RULES}" > "${RULES}.tmp" + mv "${RULES}.tmp" "${RULES}" ;; *" addr show dev "*) if grep -q "^${iface}\$" "${LEASED}" 2>/dev/null; then - echo " inet 10.0.0.5/20 scope global" + awk -v d="${iface}" '$1 == d { print " inet " $2 " scope global" }' "${ADDRS}" fi ;; esac exit 0 @@ -55,8 +87,10 @@ for a in "$@"; do done [ -n "${iface}" ] || { for a in "$@"; do iface="${a}"; done; } echo "${iface}" >> "${LEASED}" -# What a real lease installs, and the reason the policy exists at all. -grep -q "^${iface} " "${DEFAULTS}" || echo "${iface} 172.31.0.1" >> "${DEFAULTS}" +# What a real lease installs, and the reason the policy exists at all: OVN serves +# the subnet's router IP as the gateway, which is what the return path needs. +gw=$(awk -v d="${iface}" '$1 == d { print $2; exit }' "${GWS}") +grep -q "^${iface} " "${DEFAULTS}" || echo "${iface} ${gw}" >> "${DEFAULTS}" exit 0 EOF cp "${STUBBIN}/dhcp-stub" "${STUBBIN}/udhcpc" @@ -98,7 +132,22 @@ eth1 02:aa:00:00:00:02 eth2 02:aa:00:00:00:03 EOF -export LINKS +# What each NIC leases: the system VPC on eth0, a customer subnet on eth1. The +# customer address has a non-zero host part, so an assertion on the connected +# route in the policy table exercises the script's mask arithmetic. +ADDRS="${WORK}/addrs" +cat > "${ADDRS}" <<'EOF' +eth0 10.251.185.42/24 +eth1 10.60.2.15/24 +EOF + +GWS="${WORK}/gws" +cat > "${GWS}" <<'EOF' +eth0 10.251.185.1 +eth1 10.60.2.1 +EOF + +export LINKS ADDRS GWS export MULGA_NETCFG="${NETCFG}" # The stub client is instant; a real budget would only slow a failing test. export MULGA_DHCP_BUDGET=5 @@ -106,12 +155,20 @@ export MULGA_DHCP_BUDGET=5 IP_CALLS="${WORK}/ip-calls"; export IP_CALLS DEFAULTS="${WORK}/defaults"; export DEFAULTS LEASED="${WORK}/leased"; export LEASED +TABLES="${WORK}/tables"; export TABLES +RULES="${WORK}/rules"; export RULES -reset_state() { : > "${IP_CALLS}"; : > "${DEFAULTS}"; : > "${LEASED}"; } +reset_state() { + : > "${IP_CALLS}"; : > "${DEFAULTS}"; : > "${LEASED}" + : > "${TABLES}"; : > "${RULES}" +} called() { grep -qF "$1" "${IP_CALLS}"; } has_default() { grep -q "^$1 " "${DEFAULTS}"; } leased() { grep -q "^$1\$" "${LEASED}"; } +# A route in policy table $1 for destination $2, and its full command in $3. +has_route() { grep -q "^$1 $2 .*$3" "${TABLES}"; } +rule_count() { grep -c "^$1 " "${RULES}"; } # --- The setup pass ------------------------------------------------------- @@ -128,7 +185,7 @@ want 'called "ip addr replace 10.15.8.10/24 dev eth2"' "setup addresses the stat want 'has_default eth0' "setup leaves the primary ENI's leased default route in place" wantnot 'has_default eth1' "setup deletes the default route the customer ENI leased" -want 'called "ip route replace 169.254.169.254/32 via 172.31.0.1 dev eth0"' \ +want 'called "ip route replace 169.254.169.254/32 via 10.251.185.1 dev eth0"' \ "setup pins IMDS to the primary ENI" # Every NIC in the blob has to be reached. A `set -e` trip part way through the @@ -137,13 +194,40 @@ want 'called "ip route replace 169.254.169.254/32 via 172.31.0.1 dev eth0"' \ # test escaped review. want 'grep -q "configured eth2" "${WORK}/setup.out"' "setup reaches every NIC in the blob" +# --- The customer ENI's return path --------------------------------------- + +# Deleting the customer ENI's default route is only half the policy: without a +# table of its own, a reply to a client in another subnet of the customer VPC +# matches nothing but main's default and leaves by the primary ENI. +want 'has_route 101 default "default via 10.60.2.1 dev eth1"' \ + "setup gives the customer ENI a default via its lease's gateway in its own table" +want 'has_route 101 10.60.2.0/24 "dev eth1 scope link src 10.60.2.15"' \ + "setup keeps the customer subnet on-link in its table, so the same-subnet case is unchanged" +want '[ "$(grep -c . "${RULES}")" -eq 1 ]' "setup adds exactly one rule" +want 'grep -q "^101 10.60.2.15 101\$" "${RULES}"' \ + "setup steers replies sourced from the endpoint address at that table" + +# The primary ENI keeps main's default and needs no policy of its own; the +# static mgmt NIC never leases one. +wantnot 'grep -q " 10.251.185.42 " "${RULES}"' "setup adds no rule for the primary ENI" +wantnot 'grep -qE "^(100|102) " "${TABLES}"' "setup adds no policy table for the other NICs" + +# A second setup pass is a stop/start re-attach. `ip rule add` appends, so a rule +# not deleted by priority first would be listed twice and the table would grow. +if ! sh "${SCRIPT}" > "${WORK}/setup2.out" 2>&1; then + fail "second setup pass exited non-zero" + cat "${WORK}/setup2.out" +fi +want '[ "$(rule_count 101)" -eq 1 ]' "a repeated setup pass leaves exactly one rule" +want '[ "$(grep -c "^101 " "${TABLES}")" -eq 2 ]' "a repeated setup pass leaves exactly two routes in the table" + # --- The enforce pass ----------------------------------------------------- # cloud-init's network stage re-renders the ENIs from IMDS and re-DHCPs them, # putting back the default route on the customer ENI. That is the state the # enforce pass exists to correct. reset_state -printf 'eth0 10.251.185.1\neth1 172.31.0.1\n' > "${DEFAULTS}" +printf 'eth0 10.251.185.1\neth1 10.60.2.1\n' > "${DEFAULTS}" printf 'eth0\neth1\n' > "${LEASED}" if ! sh "${SCRIPT}" --enforce-routes > "${WORK}/enforce.out" 2>&1; then @@ -160,12 +244,32 @@ wantnot 'called "ip addr replace"' "enforce does not re-address an interface" want 'called "ip route replace 169.254.169.254/32 via 10.251.185.1 dev eth0"' \ "enforce re-pins IMDS after a re-DHCP" +# --- The policy table after an interface bounce --------------------------- + +# cloud-init bouncing eth1 takes the table's routes down with the device, but +# not the rule — rules are not device-scoped. A rule whose table lookup finds +# nothing falls through to main, which is the bug, silently. So the enforce pass +# has to rewrite the routes, not just re-assert the rule. +: > "${TABLES}" +printf 'eth0 10.251.185.1\neth1 10.60.2.1\n' > "${DEFAULTS}" + +if ! sh "${SCRIPT}" --enforce-routes > "${WORK}/bounce.out" 2>&1; then + fail "enforce pass exited non-zero (after a bounce)" + cat "${WORK}/bounce.out" +fi + +want 'has_route 101 default "default via 10.60.2.1 dev eth1"' \ + "enforce reinstalls the table's default after the interface bounced" +want 'has_route 101 10.60.2.0/24 "dev eth1 scope link src 10.60.2.15"' \ + "enforce reinstalls the table's connected route after the interface bounced" +want '[ "$(rule_count 101)" -eq 1 ]' "enforce leaves exactly one rule, not a duplicate" + # --- Multiple default routes on one interface ----------------------------- # A re-DHCP can leave more than one, and deleting only the first would keep the # interface carrying a default route the policy says it must not have. reset_state -printf 'eth1 172.31.0.1\neth1 172.31.0.2\neth0 10.251.185.1\n' > "${DEFAULTS}" +printf 'eth1 10.60.2.1\neth1 10.60.2.2\neth0 10.251.185.1\n' > "${DEFAULTS}" printf 'eth0\neth1\n' > "${LEASED}" if ! sh "${SCRIPT}" --enforce-routes > "${WORK}/multi.out" 2>&1; then @@ -174,6 +278,23 @@ if ! sh "${SCRIPT}" --enforce-routes > "${WORK}/multi.out" 2>&1; then fi wantnot 'has_default eth1' "enforce clears every default route on the customer ENI, not just the first" +# --- A guest whose only NIC is the default one ---------------------------- + +# An eks-node carries no extra ENI. The policy is for a NIC that is barred from +# the default route, so a guest without one must come out of both passes with +# main untouched and no rules at all. +ONENIC="${WORK}/netcfg-one" +printf 'NIC0_MAC=02:aa:00:00:00:01\nNIC0_DHCP=1\nNIC0_DEFAULT=1\n' > "${ONENIC}" + +reset_state +if ! MULGA_NETCFG="${ONENIC}" sh "${SCRIPT}" > "${WORK}/onenic.out" 2>&1; then + fail "setup pass exited non-zero (single NIC)" + cat "${WORK}/onenic.out" +fi +want 'has_default eth0' "a single-NIC guest keeps its default route" +wantnot 'grep -q . "${RULES}"' "a single-NIC guest gets no rules" +wantnot 'grep -q . "${TABLES}"' "a single-NIC guest gets no policy tables" + # --- No blob -------------------------------------------------------------- reset_state diff --git a/tests/e2e/Makefile b/tests/e2e/Makefile index 8727acea7..e14ea83bd 100644 --- a/tests/e2e/Makefile +++ b/tests/e2e/Makefile @@ -57,7 +57,10 @@ e2e-gpu: ; $(MAKE) e2e PKG=./gpu/... e2e-natuplink: ; $(MAKE) e2e PKG=./natuplink/... e2e-storagegrowth: ; $(MAKE) e2e PKG=./storagegrowth/... e2e-partialblock: ; $(MAKE) e2e PKG=./partialblock/... -e2e-rds: ; $(MAKE) e2e PKG=./rds/... +# The only suite with its own budget: one instance's `available` wait is 23 +# minutes, so a single wedged create eats most of a 30m package timeout before +# the tests still running get to report. +e2e-rds: ; $(MAKE) e2e PKG=./rds/... TIMEOUT=45m e2e-all: ; $(MAKE) e2e e2e-build: diff --git a/tests/e2e/harness/rds_client.go b/tests/e2e/harness/rds_client.go index 842fa7759..ef1135040 100644 --- a/tests/e2e/harness/rds_client.go +++ b/tests/e2e/harness/rds_client.go @@ -61,14 +61,44 @@ apt-get install -y postgresql-client || exit 11 echo done > %[1]s `, rdsClientStatusFile, rdsClientExitCodeFile) +// RDSClientPlacement is where a client VM is put: the subnet it sits in and the +// security group it carries. Which subnet is the whole point for a caller +// proving an endpoint is reachable from somewhere other than its own. +type RDSClientPlacement struct { + SubnetID string + SGID string +} + // RDSClientVM returns an SSH target for a postgres client VM in the customer's -// default VPC — the one place a DB endpoint is reachable from. Memoized per -// fixture: the boot, the install and the CA push happen once per process. +// default VPC — the placement every test that only needs *a* client wants. +// Memoized per fixture: the boot, the install and the CA push happen once per +// process. // // The default VPC maps public IPs on launch and routes 0.0.0.0/0 at its IGW, so // the same VM is reachable from the runner for SSH and has the apt egress the // install needs. func RDSClientVM(t *testing.T, c *AWSClient, fx *Fixture, env *Env) SSHTarget { + t.Helper() + vpc := EnsureDefaultVPC(t, fx) + + // The default SG admits only same-SG members, so the runner's SSH and the + // client's connection to the DB endpoint both need opening explicitly. A + // caller supplying its own placement brings its own groups instead. + EnsureDefaultSGOpen(t, c) + AuthorizeTCPIngress(t, c, vpc.SGID, DBEnginePort) + + return RDSClientVMIn(t, c, fx, env, RDSClientPlacement{SubnetID: vpc.SubnetID, SGID: vpc.SGID}) +} + +// RDSClientVMIn is RDSClientVM in a placement the caller owns, for a topology +// the default VPC cannot express. The subnet must map public IPs on launch and +// reach an IGW, and the group must admit tcp/22 from the runner and let the +// client out to the endpoint: everything past that is the caller's own network. +// +// The memo is keyed through EnsureInstance, whose key already carries the subnet +// and group, so a second placement builds a second VM rather than returning the +// first. +func RDSClientVMIn(t *testing.T, c *AWSClient, fx *Fixture, env *Env, p RDSClientPlacement) SSHTarget { t.Helper() instType, arch := DiscoverNanoInstanceType(t, fx) ami := DiscoverUbuntuAMI(t, fx, arch) @@ -76,19 +106,13 @@ func RDSClientVM(t *testing.T, c *AWSClient, fx *Fixture, env *Env) SSHTarget { // outlives the test that first asked for the VM, and ArtifactDir prunes a // passing test's directory — taking the PEM the next caller needs with it. keyName, keyPath := EnsureKeyPair(t, fx, env.ArtifactDir) - vpc := EnsureDefaultVPC(t, fx) - - // The default SG admits only same-SG members, so the runner's SSH and the - // client's connection to the DB endpoint both need opening explicitly. - EnsureDefaultSGOpen(t, c) - AuthorizeTCPIngress(t, c, vpc.SGID, DBEnginePort) instanceID := EnsureInstance(t, fx, InstanceSpec{ AMIID: ami, InstanceType: instType, KeyName: keyName, - SubnetID: vpc.SubnetID, - SGID: vpc.SGID, + SubnetID: p.SubnetID, + SGID: p.SGID, UserData: base64.StdEncoding.EncodeToString([]byte(rdsClientUserData)), }) diff --git a/tests/e2e/rds/cross_subnet_test.go b/tests/e2e/rds/cross_subnet_test.go new file mode 100644 index 000000000..e23bccade --- /dev/null +++ b/tests/e2e/rds/cross_subnet_test.go @@ -0,0 +1,286 @@ +//go:build e2e + +package rds + +import ( + "fmt" + "strings" + "testing" + "time" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/aws/aws-sdk-go/service/rds" + "github.com/mulgadc/spinifex/tests/e2e/harness" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const ( + // The topology's own address space. A dedicated VPC rather than a second + // subnet in the default one: EnsureDefaultVPC and DiscoverDefaultVPC both + // take the first subnet DescribeSubnets happens to return, which is stable + // only while there is one, and TestSubnetAndParameterGroups builds a subnet + // group over every subnet the default VPC has. + crossSubnetVPCCIDR = "10.42.0.0/16" + crossSubnetClientCIDR = "10.42.1.0/24" + crossSubnetDBCIDR = "10.42.2.0/24" + + crossSubnetTable = "e2e_cross_subnet" + crossSubnetNote = "hello from the next subnet over" +) + +// crossSubnetNet is the topology the test owns: a client subnet with a public +// path for SSH and apt, and a DB subnet with no route table of its own, which +// leaves it on the main table CreateVpc writes — intra-VPC routing and nothing +// else, exactly what a private DB subnet is. +type crossSubnetNet struct { + VPCID string + ClientSubnetID string + DBSubnetID string + ClientSGID string + DBSGID string +} + +// TestCrossSubnetConnectivity is the suite's second client leg, and the only one +// that proves the endpoint is reachable from the VPC rather than from the +// endpoint ENI's own subnet. +// +// The distinction is invisible to every other test here: a client on the ENI's +// subnet is answered on-link, with no routing decision to get wrong, and the +// whole suite has only ever had one subnet in play. A client anywhere else needs +// the DB VM to route the reply back out the ENI it arrived on, which is the +// asymmetry a dual-NIC guest gets wrong by default. +func TestCrossSubnetConnectivity(t *testing.T) { + f := requireRDSFixture(t) + t.Parallel() + reserveDBVMs(t, dbClass) + + suffix := time.Now().Unix() + id := fmt.Sprintf("%s-xsubnet-%d", dbInstancePfx, suffix) + subnetGroup := fmt.Sprintf("%s-xsubnet-%d", dbInstancePfx, suffix) + + topo := setupCrossSubnetVPC(t, f) + + harness.Phase(t, "Creating DB subnet group %q over the DB subnet alone", subnetGroup) + createDBSubnetGroup(t, f, subnetGroup, topo.DBSubnetID) + + // Started before the client VM so the two boots overlap, as TestConnectivity + // does: the create returns immediately and the engine bootstraps while apt + // runs in the client. + harness.Phase(t, "Creating DB instance %q in the DB subnet", id) + createDBInstance(t, f, id, func(in *rds.CreateDBInstanceInput) { + in.DBSubnetGroupName = aws.String(subnetGroup) + in.VpcSecurityGroupIds = aws.StringSlice([]string{topo.DBSGID}) + }) + + // A fixture bound to this test, not the process one the shared default-VPC + // client hangs off: this VM has to be terminated before the cleanup below it + // deletes the subnet it is sitting in, and the process fixture drains after + // every test in the package has finished. + clientFx := harness.NewFixture(t, f.AWS) + client := harness.RDSClientVMIn(t, f.AWS, clientFx, f.Env, + harness.RDSClientPlacement{SubnetID: topo.ClientSubnetID, SGID: topo.ClientSGID}) + + instance := waitForAvailable(t, f, id) + endpoint := aws.StringValue(instance.Endpoint.Address) + require.NotEmpty(t, endpoint, "an available instance must publish an endpoint") + + eni := harness.DBEndpointENI(t, f.AWS, id) + privateIP := aws.StringValue(eni.PrivateIpAddress) + require.NotEmpty(t, privateIP, "the endpoint ENI must carry a private address") + + // Without this the test could pass for the wrong reason: a subnet group over + // one subnet is the only thing pinning the endpoint away from the client, and + // an endpoint that landed in the client's subnet would be the same + // same-subnet case TestConnectivity already covers. + require.Equal(t, topo.DBSubnetID, aws.StringValue(eni.SubnetId), + "the subnet group has one subnet, so the endpoint ENI must be in it") + + conn := harness.PSQLConn{ + Host: privateIP, Port: aws.Int64Value(instance.Endpoint.Port), + User: dbMasterUser, Password: dbMasterPassword, DBName: dbName, + } + + // The assertion the whole test exists for. It fails as a connect timeout, + // which is indistinguishable from a security-group denial at the client — so + // the group here admits the client subnet outright, leaving the return path + // as the only thing left to be wrong. + t.Run("AClientInAnotherSubnetRoundTripsARow", func(t *testing.T) { + harness.PSQL(t, client, conn, fmt.Sprintf( + "CREATE TABLE %s (id int primary key, note text); INSERT INTO %s VALUES (1, '%s');", + crossSubnetTable, crossSubnetTable, crossSubnetNote)) + + out := harness.PSQL(t, client, conn, fmt.Sprintf("SELECT note FROM %s WHERE id = 1;", crossSubnetTable)) + assert.Equal(t, crossSubnetNote, strings.TrimSpace(out), + "a row written from outside the endpoint's own subnet must read back") + }) + + // The address proves the datapath; the name is what an application tier in + // its own subnet is actually configured with. + t.Run("TheEndpointNameReachesTheSameDatabase", func(t *testing.T) { + requireEndpointName(t, endpoint) + + addrs := harness.ResolveInGuest(t, client, endpoint) + assert.Contains(t, addrs, privateIP, "the endpoint name must resolve to the customer ENI's address") + + byName := conn + byName.Host = endpoint + out := harness.PSQL(t, client, byName, fmt.Sprintf("SELECT note FROM %s WHERE id = 1;", crossSubnetTable)) + assert.Equal(t, crossSubnetNote, strings.TrimSpace(out), + "the name and the address must reach the same database across subnets") + }) +} + +// setupCrossSubnetVPC builds the two-subnet VPC and registers its teardown. No +// CreateWorkerEgress: only the client needs egress and it has a public IP, so +// the NAT gateway that helper also builds would be waited on for nothing. +func setupCrossSubnetVPC(t *testing.T, f *Fixture) crossSubnetNet { + t.Helper() + c := f.AWS + + harness.Phase(t, "Creating a VPC with a client subnet and a private DB subnet") + topo := crossSubnetNet{VPCID: harness.CreateVPC(t, c, crossSubnetVPCCIDR)} + t.Cleanup(func() { harness.DeleteVPC(t, c, topo.VPCID) }) + + topo.ClientSubnetID = harness.CreateSubnet(t, c, topo.VPCID, crossSubnetClientCIDR) + t.Cleanup(func() { harness.DeleteSubnet(t, c, topo.ClientSubnetID) }) + topo.DBSubnetID = harness.CreateSubnet(t, c, topo.VPCID, crossSubnetDBCIDR) + t.Cleanup(func() { harness.DeleteSubnet(t, c, topo.DBSubnetID) }) + + // The client needs a public IP for the runner's SSH and for apt. The DB + // subnet gets neither that nor a route table association: it keeps the main + // table, which routes inside the VPC and nowhere else. + _, err := c.EC2.ModifySubnetAttribute(&ec2.ModifySubnetAttributeInput{ + SubnetId: aws.String(topo.ClientSubnetID), + MapPublicIpOnLaunch: &ec2.AttributeBooleanValue{Value: aws.Bool(true)}, + }) + require.NoError(t, err, "modify-subnet-attribute MapPublicIpOnLaunch") + + igw, err := c.EC2.CreateInternetGateway(&ec2.CreateInternetGatewayInput{}) // e2e:allow-create — the client subnet's own path out. + require.NoError(t, err, "create-internet-gateway") + igwID := aws.StringValue(igw.InternetGateway.InternetGatewayId) + t.Cleanup(func() { + if _, derr := c.EC2.DetachInternetGateway(&ec2.DetachInternetGatewayInput{ + InternetGatewayId: aws.String(igwID), VpcId: aws.String(topo.VPCID), + }); derr != nil { + t.Logf("detach igw %s: %v", igwID, derr) + } + if _, derr := c.EC2.DeleteInternetGateway(&ec2.DeleteInternetGatewayInput{ + InternetGatewayId: aws.String(igwID), + }); derr != nil { + t.Logf("delete igw %s: %v", igwID, derr) + } + }) + _, err = c.EC2.AttachInternetGateway(&ec2.AttachInternetGatewayInput{ + InternetGatewayId: aws.String(igwID), VpcId: aws.String(topo.VPCID), + }) + require.NoError(t, err, "attach-internet-gateway") + + // The default route goes on a table associated with the client subnet, not on + // the main table: putting it on main would give the DB subnet a way out too, + // and a private DB subnet is the layout under test. + rt, err := c.EC2.CreateRouteTable(&ec2.CreateRouteTableInput{VpcId: aws.String(topo.VPCID)}) // e2e:allow-create — the client subnet's public table. + require.NoError(t, err, "create-route-table") + rtID := aws.StringValue(rt.RouteTable.RouteTableId) + t.Cleanup(func() { + if _, derr := c.EC2.DeleteRouteTable(&ec2.DeleteRouteTableInput{RouteTableId: aws.String(rtID)}); derr != nil { + t.Logf("delete route table %s: %v", rtID, derr) + } + }) + _, err = c.EC2.CreateRoute(&ec2.CreateRouteInput{ + RouteTableId: aws.String(rtID), + DestinationCidrBlock: aws.String("0.0.0.0/0"), + GatewayId: aws.String(igwID), + }) + require.NoError(t, err, "create-route 0.0.0.0/0 -> igw") + + assoc, err := c.EC2.AssociateRouteTable(&ec2.AssociateRouteTableInput{ + RouteTableId: aws.String(rtID), SubnetId: aws.String(topo.ClientSubnetID), + }) + require.NoError(t, err, "associate-route-table") + assocID := aws.StringValue(assoc.AssociationId) + t.Cleanup(func() { + if _, derr := c.EC2.DisassociateRouteTable(&ec2.DisassociateRouteTableInput{ + AssociationId: aws.String(assocID), + }); derr != nil { + t.Logf("disassociate route table %s: %v", assocID, derr) + } + }) + + topo.ClientSGID = createCrossSubnetSG(t, f, topo.VPCID, "client", crossSubnetSSHIngress()) + topo.DBSGID = createCrossSubnetSG(t, f, topo.VPCID, "db", crossSubnetDBIngress()) + + t.Logf("cross-subnet topology: vpc=%s client=%s (%s) db=%s (%s)", + topo.VPCID, topo.ClientSubnetID, crossSubnetClientCIDR, topo.DBSubnetID, crossSubnetDBCIDR) + return topo +} + +// The runner reaches the client over SSH from wherever CI happens to run it. +func crossSubnetSSHIngress() *ec2.IpPermission { + return &ec2.IpPermission{ + IpProtocol: aws.String("tcp"), + FromPort: aws.Int64(22), + ToPort: aws.Int64(22), + IpRanges: []*ec2.IpRange{{CidrIp: aws.String("0.0.0.0/0")}}, + } +} + +// The endpoint admits the client subnet by CIDR. A source group would be the +// same rule via a membership the control plane derives, and this test is about +// the return path: a denial and a missing route both present as a timeout, so +// the group is the one part left with nothing to get wrong. +func crossSubnetDBIngress() *ec2.IpPermission { + return &ec2.IpPermission{ + IpProtocol: aws.String("tcp"), + FromPort: aws.Int64(harness.DBEnginePort), + ToPort: aws.Int64(harness.DBEnginePort), + IpRanges: []*ec2.IpRange{{CidrIp: aws.String(crossSubnetClientCIDR)}}, + } +} + +func createCrossSubnetSG(t *testing.T, f *Fixture, vpcID, role string, ingress *ec2.IpPermission) string { + t.Helper() + name := fmt.Sprintf("%s-xsubnet-%s-%d", dbInstancePfx, role, time.Now().UnixNano()) + out, err := f.AWS.EC2.CreateSecurityGroup(&ec2.CreateSecurityGroupInput{ // e2e:allow-create — this test's own VPC, so no fixture group fits. + VpcId: aws.String(vpcID), + GroupName: aws.String(name), + Description: aws.String("rds e2e cross-subnet " + role), + }) + require.NoError(t, err, "create-security-group %s", name) + sgID := aws.StringValue(out.GroupId) + t.Cleanup(func() { + if _, derr := f.AWS.EC2.DeleteSecurityGroup(&ec2.DeleteSecurityGroupInput{ + GroupId: aws.String(sgID), + }); derr != nil { + t.Logf("delete sg %s: %v", sgID, derr) + } + }) + + _, err = f.AWS.EC2.AuthorizeSecurityGroupIngress(&ec2.AuthorizeSecurityGroupIngressInput{ + GroupId: aws.String(sgID), + IpPermissions: []*ec2.IpPermission{ingress}, + }) + require.NoError(t, err, "authorize-security-group-ingress on %s", sgID) + return sgID +} + +// A DB subnet group over exactly the subnets named, which is what places the +// endpoint ENI somewhere the client is not. +func createDBSubnetGroup(t *testing.T, f *Fixture, name string, subnetIDs ...string) { + t.Helper() + out, err := f.AWS.RDS.CreateDBSubnetGroup(&rds.CreateDBSubnetGroupInput{ + DBSubnetGroupName: aws.String(name), + DBSubnetGroupDescription: aws.String("rds e2e cross-subnet group"), + SubnetIds: aws.StringSlice(subnetIDs), + }) + require.NoError(t, err, "create-db-subnet-group %s", name) + require.NotNil(t, out.DBSubnetGroup) + t.Cleanup(func() { + if _, derr := f.AWS.RDS.DeleteDBSubnetGroup(&rds.DeleteDBSubnetGroupInput{ + DBSubnetGroupName: aws.String(name), + }); derr != nil && !harness.ErrorCodeIs(derr, "DBSubnetGroupNotFoundFault") { + t.Logf("cleanup: delete-db-subnet-group %s: %v", name, derr) + } + }) +}