diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index 28cceb62a..9caabd919 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -845,16 +845,35 @@ Spinifex both stores externally-issued certificates (`import-certificate`) and i --- -## RDS (PostgreSQL) +## RDS (PostgreSQL and MariaDB) -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. +Each DB instance is one dedicated system-owned VM running the engine directly, launched from the `spinifex-rds-postgres` or `spinifex-rds-mariadb` 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. -- **Engine:** `postgres` 18 only — `EngineVersion` accepts `18` or `18.x`, and there is no in-place upgrade. +Everything below applies to both engines unless it names one. `Engine` is fixed at create — there is no in-place engine change, no cross-engine snapshot restore and no migration between the two. + +| | `postgres` | `mariadb` | +|---|---|---| +| Version | 18 | 11.8 | +| `EngineVersion` accepted | `18`, or omitted | `11.8`, or omitted | +| Default port | 5432 | 3306 | +| Parameter-group family | `postgres18` | `mariadb11.8` | +| Implicit default group | `default.postgres18` | `default.mariadb11.8` | +| `DBName` maximum | 63 characters | 64 characters | +| `MasterUsername` maximum | 63 characters | 80 characters | +| Reserved usernames | `postgres`, `rdsadmin`, `rds_superuser`, and the `pg_` prefix | `root`, `mysql`, `mariadb.sys`, `rdsadmin`, `PUBLIC`, and the `mysql.` and `mariadb.` prefixes | +| Catalog size | 51 parameters | 54 parameters | + +**`mysql` is not an accepted engine and is not an alias for `mariadb`.** MariaDB is offered under its own AWS engine name, exactly as AWS RDS offers it. Naming `mysql` fails with `InvalidParameterValue`; a client — including Terraform's `aws_db_instance` — must set `engine = "mariadb"`. Aliasing would report an engine and a version the instance is not running, and the discrepancy would propagate into `DescribeDBInstances`, parameter-group families and snapshot metadata. + +- **Engine version:** pinned per engine, and an `EngineVersion` naming anything but the pin is rejected. A minor version such as `18.4` or `11.8.8` is rejected as well: the AMI does not promise any particular minor, so accepting one would be a promise the platform cannot keep. 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. - **Storage:** gp3 only, 20–65536 GiB, always encrypted with the cluster key. Grow-only, and a grow is **stop/start with downtime** — the volume cannot be resized while attached. -- **TLS:** offered, not enforced. The engine serves a per-instance certificate signed by the cluster CA (the `ca.pem` baked into AMIs) carrying both the ENI IP and the DNS name in its SAN set, so `sslmode=verify-full` works by name or by address. A deployment with no cluster CA starts the engine without TLS. -- **Master user:** administrative but **not a PostgreSQL superuser**, as on AWS. It gets `CREATEDB` and `CREATEROLE`, owns the initial database, and holds `rds_superuser` (`pg_monitor`, `pg_signal_backend`, `pg_checkpoint`) with `ADMIN OPTION`, so it can grant that set onward. It cannot `COPY ... FROM/TO PROGRAM`, touch server-side files, run `ALTER SYSTEM`, or install **untrusted** extensions — those need the cluster superuser, which no customer credential is. Trusted extensions install normally in a database it owns. `postgres`, `rdsadmin` and `rds_superuser` are rejected as `--master-username`. -- **Backups:** daily COW snapshots of the data volume inside `PreferredBackupWindow`. Retention defaults to 7 days and caps at 7; `0` disables automated backups. No point-in-time recovery. +- **`DBName`:** optional; an omitted name creates no initial database. When supplied it must begin with a letter and hold only letters, digits and underscores, within the per-engine limit above. The rule is narrower than either engine's own, because the guest interpolates the name into a `CREATE DATABASE`. +- **TLS:** offered, not enforced, on both engines. The engine serves a per-instance certificate signed by the cluster CA (the `ca.pem` baked into AMIs) carrying both the ENI IP and the DNS name in its SAN set, so PostgreSQL's `sslmode=verify-full` verifies by name or by address, and MariaDB's `--ssl-ca` plus `--ssl-verify-server-cert` verifies against the endpoint name. A deployment with no cluster CA starts the engine without TLS. +- **PostgreSQL master user:** administrative but **not a PostgreSQL superuser**, as on AWS. It gets `CREATEDB` and `CREATEROLE`, owns the initial database, and holds `rds_superuser` (`pg_monitor`, `pg_signal_backend`, `pg_checkpoint`) with `ADMIN OPTION`, so it can grant that set onward. It cannot `COPY ... FROM/TO PROGRAM`, touch server-side files, run `ALTER SYSTEM`, or install **untrusted** extensions — those need the cluster superuser, which no customer credential is. Trusted extensions install normally in a database it owns. +- **MariaDB master user:** created as `''@'%'` with an explicitly enumerated grant list `ON *.*` `WITH GRANT OPTION` — `SELECT`, `INSERT`, `UPDATE`, `DELETE`, `CREATE`, `DROP`, `RELOAD`, `PROCESS`, `REFERENCES`, `INDEX`, `ALTER`, `SHOW DATABASES`, `CREATE TEMPORARY TABLES`, `LOCK TABLES`, `EXECUTE`, `REPLICATION SLAVE`, `REPLICATION CLIENT`, `CREATE VIEW`, `SHOW VIEW`, `CREATE ROUTINE`, `ALTER ROUTINE`, `CREATE USER`, `EVENT`, `TRIGGER`. Deliberately withheld are `SUPER` and its 11.8 split-out successors, `FILE`, `SHUTDOWN` and everything scoped to `mysql.*`: `FILE` would give `SELECT ... INTO OUTFILE` and `LOAD_FILE()` as the OS user, and `INSERT` on `mysql.*` would allow `CREATE FUNCTION ... SONAME` to load native code into the server. +- **MariaDB storage engine:** `default_storage_engine` is platform-owned and pinned to **InnoDB**, and is absent from the parameter catalog — a deliberate divergence from AWS, which exposes it as modifiable. Only InnoDB keeps a redo log, so only InnoDB is covered by the snapshot guarantee. An explicit `ENGINE=Aria` or `ENGINE=MyISAM` on `CREATE TABLE` still works — the limitation is documented rather than prevented — but such a table has no redo log, so a snapshot taken without a successful quiesce can leave it inconsistent with no way back. Use InnoDB, as AWS also recommends. +- **Backups:** daily COW snapshots of the data volume inside `PreferredBackupWindow`. Retention defaults to 7 days and caps at 7; `0` disables automated backups. No point-in-time recovery, and MariaDB's binary log is off. - **Availability:** single-AZ. Engine crashes restart in-guest and VM crashes on a live host restart via `ec2-health-restart`, but an instance whose host is lost is reported `failed` with a reason in `StatusInfos` and needs operator recovery. Statuses: `creating`, `available`, `modifying`, `backing-up`, `rebooting`, `stopping`, `stopped`, `starting`, `deleting`, `failed`. (`recovering` is defined in the state machine but unreachable until auto-recovery lands.) @@ -863,7 +882,7 @@ Statuses: `creating`, `available`, `modifying`, `backing-up`, `rebooting`, `stop | Command | Implemented Flags | Missing Flags | Status | |---------|-------------------|---------------|--------| -| `create-db-instance` | `--db-instance-identifier`, `--engine` (postgres), `--engine-version` (18), `--db-instance-class`, `--allocated-storage` (20–65536 GiB), `--storage-type` (gp3), `--master-username`, `--master-user-password`, `--db-name` (letters, digits and underscores, beginning with a letter, max 63), `--port` (1150–65535), `--db-subnet-group-name` (unnamed → the account's default VPC subnet), `--vpc-security-group-ids`, `--db-parameter-group-name`, `--backup-retention-period` (0–7, default 7), `--preferred-backup-window`, `--preferred-maintenance-window` (unnamed → assigned from the identifier), `--deletion-protection`, `--tags`, `--storage-encrypted` (true only) | `--auto-minor-version-upgrade`, `--copy-tags-to-snapshot`, Performance Insights / Enhanced Monitoring flags (all accepted, no-op); see "Rejected Parameters" for those that fail loudly | **DONE** | +| `create-db-instance` | `--db-instance-identifier`, `--engine` (postgres, mariadb), `--engine-version` (the engine's pin, or omitted), `--db-instance-class`, `--allocated-storage` (20–65536 GiB), `--storage-type` (gp3), `--master-username`, `--master-user-password`, `--db-name` (letters, digits and underscores, beginning with a letter, max 63 on postgres and 64 on mariadb), `--port` (1150–65535), `--db-subnet-group-name` (unnamed → the account's default VPC subnet), `--vpc-security-group-ids`, `--db-parameter-group-name`, `--backup-retention-period` (0–7, default 7), `--preferred-backup-window`, `--preferred-maintenance-window` (unnamed → assigned from the identifier), `--deletion-protection`, `--tags`, `--storage-encrypted` (true only) | `--auto-minor-version-upgrade`, `--copy-tags-to-snapshot`, Performance Insights / Enhanced Monitoring flags (all accepted, no-op); see "Rejected Parameters" for those that fail loudly | **DONE** | | `describe-db-instances` | `--db-instance-identifier` | `--filters`, `--max-records`, `--marker` (parsed, not applied) | **DONE** | | `modify-db-instance` | `--db-instance-identifier`, `--master-user-password`, `--allocated-storage` (grow only, stop/start), `--db-instance-class` (VM replace), `--db-parameter-group-name`, `--vpc-security-group-ids`, `--deletion-protection`, `--backup-retention-period`, `--preferred-backup-window`, `--preferred-maintenance-window`, `--apply-immediately` | `--new-db-instance-identifier`, `--engine-version`, `--db-port-number`, `--db-subnet-group-name`, `--max-allocated-storage`, `--ca-certificate-identifier` (all rejected, not ignored) | **DONE** | | `delete-db-instance` | `--db-instance-identifier`, `--skip-final-snapshot`, `--final-db-snapshot-identifier` (exactly one is required) | `--delete-automated-backups` (no-op — automated snapshots are always purged with the instance) | **DONE** — `DeletionProtection` blocks the call; a final snapshot **retains** the data volume until that snapshot is deleted | @@ -875,7 +894,7 @@ Statuses: `creating`, `available`, `modifying`, `backing-up`, `rebooting`, `stop | Command | Implemented Flags | Missing Flags | Status | |---------|-------------------|---------------|--------| -| `create-db-snapshot` | `--db-snapshot-identifier`, `--db-instance-identifier`, `--tags` | — | **DONE** — the agent quiesces the engine first; a failed quiesce falls back to a crash-consistent snapshot and writes a `DescribeEvents` warning | +| `create-db-snapshot` | `--db-snapshot-identifier`, `--db-instance-identifier`, `--tags` | — | **DONE** — the agent quiesces the engine first (`pg_backup_start` on postgres, `BACKUP STAGE` on mariadb); a failed quiesce falls back to a crash-consistent snapshot and writes a `DescribeEvents` warning naming what that engine does and does not recover | | `describe-db-snapshots` | `--db-snapshot-identifier`, `--db-instance-identifier`, `--snapshot-type` (manual, automated) | `--filters`, `--dbi-resource-id`, `--include-shared`, `--include-public` (rejected), `--max-records`, `--marker` | **DONE** | | `delete-db-snapshot` | `--db-snapshot-identifier` | — | **DONE** — refused while a restored volume still references the snapshot; the last snapshot released reclaims a retained data volume | | `restore-db-instance-from-db-snapshot` | `--db-instance-identifier`, `--db-snapshot-identifier`, `--db-instance-class`, `--allocated-storage` (≥ the snapshot's), `--storage-type` (gp3), `--port`, `--db-subnet-group-name`, `--vpc-security-group-ids`, `--db-parameter-group-name`, `--deletion-protection`, `--tags`, `--engine` (must match the snapshot) | `--db-name` (rejected when it differs from the snapshot's), plus the create rejections | **DONE** — unnamed fields are inherited from the snapshot; the master password comes from the restored datadir | @@ -903,15 +922,17 @@ Statuses: `creating`, `available`, `modifying`, `backing-up`, `rebooting`, `stop | Command | Implemented Flags | Missing Flags | Status | |---------|-------------------|---------------|--------| -| `create-db-parameter-group` | `--db-parameter-group-name`, `--db-parameter-group-family` (postgres18), `--description`, `--tags` | — | **DONE** — names beginning with `default.` are reserved; the implicit group is `default.postgres18` | -| `describe-db-parameter-groups` | `--db-parameter-group-name` | `--filters`, `--max-records`, `--marker` | **DONE** — the implicit default group is always listed | +| `create-db-parameter-group` | `--db-parameter-group-name`, `--db-parameter-group-family` (`postgres18` or `mariadb11.8`), `--description`, `--tags` | — | **DONE** — names beginning with `default.` are reserved. **`--db-parameter-group-family` defaults to `postgres18` when omitted**, so a MariaDB group must name its family explicitly or the mismatch surfaces one call later, at `create-db-instance` | +| `describe-db-parameter-groups` | `--db-parameter-group-name` | `--filters`, `--max-records`, `--marker` | **DONE** — every engine's implicit default group is always listed, so both `default.postgres18` and `default.mariadb11.8` appear in every account | | `modify-db-parameter-group` | `--db-parameter-group-name`, `--parameters` (ParameterName, ParameterValue, ApplyMethod) | — | **DONE** — the whole batch is validated before anything is written and propagated to attached instances; `immediate` on a static parameter is rejected, as AWS does | -| `describe-db-parameters` | `--db-parameter-group-name`, `--source` (user, engine-default) | `--filters`, `--max-records`, `--marker` | **DONE** — 51-parameter PostgreSQL 18 catalog; memory defaults are computed per instance class and reported as literals | +| `describe-db-parameters` | `--db-parameter-group-name`, `--source` (user, engine-default) | `--filters`, `--max-records`, `--marker` | **DONE** — the catalog of the group's own family: 51 parameters on `postgres18`, 54 on `mariadb11.8`. Memory defaults are computed per instance class and reported as literals | | `delete-db-parameter-group` | `--db-parameter-group-name` | — | **DONE** — refused for a default group and while any instance references it | | `reset-db-parameter-group` | — | all | **NOT STARTED** (`InvalidAction`) | A group takes effect on an instance when `modify-db-instance --db-parameter-group-name` attaches it — immediately with `--apply-immediately`, otherwise at the next maintenance window. Later group edits propagate to every attached instance. Dynamic parameters are written into the engine's config and reloaded live; static ones are recorded `pending-reboot` and applied by `reboot-db-instance`. +**A group's family must match the instance's engine.** A `postgres18` group cannot be attached to a MariaDB instance and a `mariadb11.8` group cannot be attached to a PostgreSQL one, on create, on modify, on a deferred apply or on a snapshot restore; the mismatch is refused with `InvalidParameterCombination`. The catalogs are per-family too, so `modify-db-parameter-group` will not store one engine's parameter name into the other's group. Each catalog is a curated, validated subset rather than the engine's full variable set, and the platform-owned settings — the port, the datadir, the socket, the bind address, the `ssl_*` family, and on MariaDB `secure_file_priv`, `log_bin`, `default_storage_engine` and the InnoDB buffer-pool chunk settings — are absent from it rather than present-and-unmodifiable, because they are not the customer's to set. + ### RDS — Tags | Command | Implemented Flags | Missing Flags | Status | @@ -942,7 +963,8 @@ Policy: a parameter whose omission would create a false safety, security or avai | `DBSecurityGroups` | EC2-Classic security groups — use `VpcSecurityGroupIds` | | `DBClusterIdentifier`, `DBClusterSnapshotIdentifier` | Clustered engines are not offered | | `EnableCloudwatchLogsExports` | Log export is not implemented | -| `EngineVersion` other than 18, `Engine` on modify | No in-place engine or version change | +| `EngineVersion` other than the engine's pin, `Engine` on modify | No in-place engine or version change | +| `Engine=mysql` (and Aurora engines) | Oracle MySQL is not offered; `mariadb` is a distinct engine, not an alias for it | | `NewDBInstanceIdentifier` | The identifier is the DNS label and the KV key | | `DBPortNumber`, `DBSubnetGroupName` on modify | Both would move the endpoint | | `MaxAllocatedStorage` | Storage autoscaling is not implemented | @@ -961,14 +983,14 @@ Recognised actions below return `OperationNotSupported`, so a client sees "not o | Feature | Actions | Priority | Status | |---------|---------|----------|--------| -| MySQL engine | — (second AMI preset + `Engine` implementation) | High | **NOT STARTED** | -| Point-in-time recovery (WAL archiving to predastore) | `restore-db-instance-to-point-in-time` | High | **NOT STARTED** | +| Oracle MySQL engine | — | Low | **NOT PLANNED** — MariaDB is offered instead, under its own `mariadb` engine name; see the note above on why it is not aliased onto `mysql` | +| Point-in-time recovery (WAL archiving to predastore on postgres, binlog on mariadb) | `restore-db-instance-to-point-in-time` | High | **NOT STARTED** | | Auto-recovery from node loss (failure is detected, not repaired) | — | High | **NOT STARTED** | | Read replicas | `create-db-instance-read-replica`, `promote-read-replica` | Medium | **NOT STARTED** | | Aurora / DB clusters | `create-db-cluster`, `modify-db-cluster`, `delete-db-cluster`, `describe-db-clusters`, `failover-db-cluster` | Low | **NOT STARTED** | | Option groups | `create-option-group`, `modify-option-group`, `delete-option-group`, `describe-option-groups` | Low | **NOT STARTED** | | Engine-version / orderable-option data sources | `describe-db-engine-versions`, `describe-orderable-db-instance-options` | Low | **NOT STARTED** (`InvalidAction`) — not required by `aws_db_instance`; pin the class and version literally in Terraform | -| Multi-AZ standby, online (no-downtime) storage grow, storage autoscaling, IAM database auth, Performance Insights, Enhanced Monitoring, log exports, per-tenant private DNS zones, enforced TLS | — | — | **NOT STARTED** | +| Multi-AZ standby, online (no-downtime) storage grow, storage autoscaling, IAM database auth, Performance Insights, Enhanced Monitoring, log exports, per-tenant private DNS zones, enforced TLS, cross-engine snapshot restore, in-place migration between the two engines | — | — | **NOT STARTED** | IAM: `AmazonRDSFullAccess` and `AmazonRDSReadOnlyAccess` are available as managed policies. They grant `rds:` verb prefixes rather than `rds:*`, because `rds:*` would also appear to grant the internal agent actions the gateway reserves for a DB VM's own role. diff --git a/docs/terraform-workbooks/rds-quickstart/README.md b/docs/terraform-workbooks/rds-quickstart/README.md index 187b7cc8e..694b7512f 100644 --- a/docs/terraform-workbooks/rds-quickstart/README.md +++ b/docs/terraform-workbooks/rds-quickstart/README.md @@ -1,5 +1,5 @@ --- -title: "RDS Quickstart" +title: "RDS Quickstart (PostgreSQL)" description: "Stand up a managed PostgreSQL database on Spinifex with Terraform — a VPC, a DB subnet group, a parameter group, an aws_db_instance, and a client VM inside the VPC that connects to it with psql." category: "Terraform Workbooks" tags: @@ -16,7 +16,7 @@ sections: - troubleshooting resources: - title: "RDS CLI Reference" - url: "https://github.com/mulgadc/spinifex/blob/main/docs/COMMANDS.md#rds-postgresql" + url: "https://github.com/mulgadc/spinifex/blob/main/docs/COMMANDS.md#rds-postgresql-and-mariadb" - title: "Terraform AWS Provider — aws_db_instance" url: "https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/db_instance" - title: "Spinifex Repository" @@ -25,10 +25,12 @@ resources: url: "https://opentofu.org/" --- -# Terraform: RDS Quickstart +# Terraform: RDS Quickstart (PostgreSQL) > 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. +This workbook is **PostgreSQL throughout** — the engine, the `postgres18` parameter group, port `5432`, the client package and the `~/.pgpass` credential are all PostgreSQL's. Spinifex also serves MariaDB 11.8 under `engine = "mariadb"`; running this workbook against it means changing `engine`, `engine_version`, the parameter-group `family` to `mariadb11.8`, the security-group port to `3306` and the client to `mariadb-client`. See the [RDS command reference](https://github.com/mulgadc/spinifex/blob/main/docs/COMMANDS.md#rds-postgresql-and-mariadb) for both engines' pins, families and limits. Note that `mysql` is not an accepted engine and is not an alias for `mariadb`. + ## 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. @@ -127,7 +129,7 @@ psql "sslmode=verify-full sslrootcert=/home/ubuntu/ca.pem" \ | `spinifex_endpoint` | `https://127.0.0.1:9999` | Gateway as seen from the host running Terraform. | | `instance_type` | `t3.small` | EC2 type for the **client** VM. | | `db_instance_class` | `db.t3.micro` | DB class. One of the curated `db.*` subset. | -| `engine_version` | `18` | PostgreSQL major. `18` is the only version served. | +| `engine_version` | `18` | PostgreSQL major. `18` is the only version served, and a minor such as `18.4` is rejected. | | `db_name` | `appdb` | Database created at bootstrap. | | `db_username` | `appuser` | Master user. `postgres`, `rdsadmin`, `rds_superuser` and `pg_*` are reserved. | | `db_password` | `QuickstartS3cret1` | Master password — override it. No `/`, `"`, `@` or spaces. | diff --git a/tests/e2e/harness/rds_client.go b/tests/e2e/harness/rds_client.go index ef1135040..16262ba27 100644 --- a/tests/e2e/harness/rds_client.go +++ b/tests/e2e/harness/rds_client.go @@ -21,9 +21,9 @@ import ( // So the client that proves a customer can reach the database has to be a VM in // that subnet, driven over SSH — not psql on the machine running the test. // -// The client is an Ubuntu gold image with the postgres client installed at boot, -// not the rds-postgres engine AMI: that image carries neither sshd nor -// cloud-init, so a VM launched from it would come up unreachable. +// The client is an Ubuntu gold image with both engines' clients installed at +// boot, not an engine AMI: those images carry neither sshd nor cloud-init, so a +// VM launched from one would come up unreachable. const ( rdsClientUser = "ubuntu" @@ -37,17 +37,19 @@ const ( // trust root. The engine AMI ships one; an Ubuntu client does not. RDSClientCACertPath = "/home/ubuntu/spinifex-ca.pem" - // Postgres' assigned port, and the only one a DB instance publishes. - DBEnginePort = 5432 + // Each engine's assigned port, and the only one an instance of it publishes. + PostgresEnginePort = 5432 + MariaDBEnginePort = 3306 rdsClientSSHTimeout = 5 * time.Minute rdsClientSetupTimeout = 5 * time.Minute ) -// rdsClientUserData installs the postgres client at first boot. Status and +// rdsClientUserData installs both engines' clients at first boot. Status and // exit-code sentinels mirror the storagegrowth suite's protocol; the distinct -// exit codes separate "no apt egress" from "no such package", which otherwise -// both surface as a missing psql much later. +// exit codes separate "no apt egress" from "no such package", and one engine's +// client from the other's, which otherwise all surface as a missing binary much +// later. var rdsClientUserData = fmt.Sprintf(`#!/bin/bash set -u trap 'echo $? > %[2]s' EXIT @@ -57,6 +59,7 @@ echo running > %[1]s export DEBIAN_FRONTEND=noninteractive apt-get update -y || exit 10 apt-get install -y postgresql-client || exit 11 +apt-get install -y mariadb-client || exit 12 echo done > %[1]s `, rdsClientStatusFile, rdsClientExitCodeFile) @@ -83,9 +86,12 @@ func RDSClientVM(t *testing.T, c *AWSClient, fx *Fixture, env *Env) SSHTarget { // 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. + // caller supplying its own placement brings its own groups instead. Both + // engines' ports are opened here: the client VM is shared by every test that + // needs a connection, whichever engine it is connecting to. EnsureDefaultSGOpen(t, c) - AuthorizeTCPIngress(t, c, vpc.SGID, DBEnginePort) + AuthorizeTCPIngress(t, c, vpc.SGID, PostgresEnginePort) + AuthorizeTCPIngress(t, c, vpc.SGID, MariaDBEnginePort) return RDSClientVMIn(t, c, fx, env, RDSClientPlacement{SubnetID: vpc.SubnetID, SGID: vpc.SGID}) } @@ -141,9 +147,9 @@ func RDSClientVMIn(t *testing.T, c *AWSClient, fx *Fixture, env *Env, p RDSClien return tgt } -// waitForRDSClientTooling polls the boot-time install's sentinels until psql is -// installed. A non-zero exit code is reported with the cloud-init log tail, -// since the interesting failure is always inside apt. +// waitForRDSClientTooling polls the boot-time install's sentinels until both +// clients are installed. A non-zero exit code is reported with the cloud-init +// log tail, since the interesting failure is always inside apt. func waitForRDSClientTooling(t *testing.T, tgt SSHTarget) error { t.Helper() deadline := time.Now().Add(rdsClientSetupTimeout) @@ -163,13 +169,13 @@ func waitForRDSClientTooling(t *testing.T, tgt SSHTarget) error { if code != "" && code != "0" { log, _ := GuestExec(tgt, "tail -40 /var/log/cloud-init-output.log 2>/dev/null || true") - return fmt.Errorf("client VM postgres-client install exited %s (status=%q):\n%s", code, status, log) + return fmt.Errorf("client VM database-client install exited %s (status=%q):\n%s", code, status, log) } if code == "0" && status == "done" { return nil } if time.Now().After(deadline) { - return fmt.Errorf("client VM postgres-client install still %q after %s (last ssh error: %v)", + return fmt.Errorf("client VM database-client install still %q after %s (last ssh error: %v)", status, rdsClientSetupTimeout, lastErr) } time.Sleep(5 * time.Second) @@ -266,7 +272,7 @@ func TryPSQL(tgt SSHTarget, conn PSQLConn, sql string) (string, error) { func psqlCommand(conn PSQLConn, sql string) string { port := conn.Port if port == 0 { - port = DBEnginePort + port = PostgresEnginePort } env := []string{ "PGPASSWORD=" + shellQuote(conn.Password), diff --git a/tests/e2e/harness/rds_mariadb_client.go b/tests/e2e/harness/rds_mariadb_client.go new file mode 100644 index 000000000..e16c3170b --- /dev/null +++ b/tests/e2e/harness/rds_mariadb_client.go @@ -0,0 +1,110 @@ +//go:build e2e + +package harness + +import ( + "strconv" + "strings" + "testing" + "time" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/rds" +) + +// The MariaDB half of the client leg. The guest and its trust root are the +// shared ones RDSClientVM readies — only the wire protocol differs, so only the +// command line does. + +// MariaDBConn addresses one mariadb invocation from inside the client guest. +// The client is MariaDB's own rather than MySQL's: the server's authentication +// plugins and its TLS options are what MariaDB's client is written against. +type MariaDBConn struct { + Host string + Port int64 + User string + Password string + DBName string + // SSLRootCert is an in-guest path to the trust root, normally + // RDSClientCACertPath. Setting it is what turns TLS on: every option in the + // client's --ssl-* family implies an encrypted connection. --ssl itself is + // deliberately unused — the 11.x client deprecates it, and the deprecation + // notice would land in the same stream as the query's own output. + SSLRootCert string + // VerifyServerCert checks the serving certificate against the host being + // connected to, which is the other half of what verify-full means. + VerifyServerCert bool +} + +// MariaDBConnFor builds a connection from an available DB instance's own +// reported endpoint, so a test cannot accidentally assert against an address the +// control plane never published. +func MariaDBConnFor(t *testing.T, instance *rds.DBInstance, user, password, dbName string) MariaDBConn { + t.Helper() + if instance == nil || instance.Endpoint == nil { + t.Fatalf("MariaDBConnFor: instance publishes no endpoint") + } + return MariaDBConn{ + Host: aws.StringValue(instance.Endpoint.Address), + Port: aws.Int64Value(instance.Endpoint.Port), + User: user, + Password: password, + DBName: dbName, + } +} + +// One mariadb call, including connect. Sized as psqlTimeout is: generous enough +// for a cold connection to an engine that has just finished starting, tight +// enough to fail inside a subtest. +const mariadbTimeout = 90 * time.Second + +// MariaDB runs sql through the mariadb client in the client guest and returns +// its output. +// +// A failure fails the test, for the same reason PSQL's does: a green run that +// never opened a connection is the exact outcome this suite exists to prevent. +func MariaDB(t *testing.T, tgt SSHTarget, conn MariaDBConn, sql string) string { + t.Helper() + out, err := TryMariaDB(tgt, conn, sql) + if err != nil { + t.Fatalf("mariadb %s@%s:%d/%s: %v\n%s", conn.User, conn.Host, conn.Port, conn.DBName, err, out) + } + return out +} + +// TryMariaDB is MariaDB without the t.Fatal, for the cases where the connection +// is supposed to fail: a retired password, a security group that does not admit +// 3306, a certificate that must not verify. +func TryMariaDB(tgt SSHTarget, conn MariaDBConn, sql string) (string, error) { + return GuestExecTimeout(tgt, mariadbCommand(conn, sql), mariadbTimeout) +} + +// mariadbCommand renders the guest-side command line. The password goes through +// MYSQL_PWD rather than the client's argv, so it stays out of the guest's +// process list and out of any diagnostic that captures it. +// +// --batch makes the output tab-separated and unpadded, and in that mode the +// client aborts on the first failed statement instead of carrying on, which is +// what turns a broken statement into a non-zero exit rather than a warning. +func mariadbCommand(conn MariaDBConn, sql string) string { + port := conn.Port + if port == 0 { + port = MariaDBEnginePort + } + env := "MYSQL_PWD=" + shellQuote(conn.Password) + args := []string{ + "mariadb", "--batch", "--skip-column-names", "--connect-timeout=30", + "--host", shellQuote(conn.Host), + "--port", strconv.FormatInt(port, 10), + "--user", shellQuote(conn.User), + "--database", shellQuote(conn.DBName), + } + if conn.SSLRootCert != "" { + args = append(args, "--ssl-ca="+shellQuote(conn.SSLRootCert)) + } + if conn.VerifyServerCert { + args = append(args, "--ssl-verify-server-cert") + } + args = append(args, "--execute", shellQuote(sql)) + return env + " " + strings.Join(args, " ") +} diff --git a/tests/e2e/rds/connect_test.go b/tests/e2e/rds/connect_test.go index 52f44d4c7..c7727f203 100644 --- a/tests/e2e/rds/connect_test.go +++ b/tests/e2e/rds/connect_test.go @@ -138,7 +138,7 @@ func TestConnectivity(t *testing.T) { if _, err := harness.TryPSQL(client, byIP, "SELECT 1;"); err != nil { return nil } - return fmt.Errorf("%s still accepts connections through a group that does not admit %d", id, harness.DBEnginePort) + return fmt.Errorf("%s still accepts connections through a group that does not admit %d", id, harness.PostgresEnginePort) }, sgSettleTimeout, 5*time.Second) harness.Step(t, "Restoring the original groups on %q", id) diff --git a/tests/e2e/rds/cross_subnet_test.go b/tests/e2e/rds/cross_subnet_test.go index e23bccade..0b4051d54 100644 --- a/tests/e2e/rds/cross_subnet_test.go +++ b/tests/e2e/rds/cross_subnet_test.go @@ -233,8 +233,8 @@ func crossSubnetSSHIngress() *ec2.IpPermission { func crossSubnetDBIngress() *ec2.IpPermission { return &ec2.IpPermission{ IpProtocol: aws.String("tcp"), - FromPort: aws.Int64(harness.DBEnginePort), - ToPort: aws.Int64(harness.DBEnginePort), + FromPort: aws.Int64(harness.PostgresEnginePort), + ToPort: aws.Int64(harness.PostgresEnginePort), IpRanges: []*ec2.IpRange{{CidrIp: aws.String(crossSubnetClientCIDR)}}, } } diff --git a/tests/e2e/rds/main_test.go b/tests/e2e/rds/main_test.go index e4afcc8c7..23b4e993c 100644 --- a/tests/e2e/rds/main_test.go +++ b/tests/e2e/rds/main_test.go @@ -34,11 +34,12 @@ import ( // plane publishes is region-qualified. const defaultRegion = "ap-southeast-2" -// The instance spec every test in the suite creates from. The class is the -// floor and the storage is the API's own minimum: nothing here needs more, and -// each DB VM booted is charged against the phase's instance budget. Only a -// class-sensitive assertion names a bigger class, and only a grow names a -// bigger size. +// The PostgreSQL instance spec most of the suite creates from; the MariaDB +// cases carry their own engine, database name and credentials and share the +// class and storage. The class is the floor and the storage is the API's own +// minimum: nothing here needs more, and each DB VM booted is charged against the +// phase's instance budget. Only a class-sensitive assertion names a bigger +// class, and only a grow names a bigger size. const ( dbInstancePfx = "rds-e2e" dbEngine = "postgres" @@ -306,18 +307,27 @@ func validCreateInput(id string) *rds.CreateDBInstanceInput { // createDBInstance creates the suite's standard instance and returns the create // response's own view of it — status creating, no endpoint yet. +func createDBInstance(t *testing.T, f *Fixture, id string, opts ...func(*rds.CreateDBInstanceInput)) *rds.DBInstance { + t.Helper() + return createDBInstanceFrom(t, f, validCreateInput(id), opts...) +} + +// createDBInstanceFrom is createDBInstance for a request the caller assembled +// itself, which is what the MariaDB cases need: their engine, database name and +// credentials are not the suite's PostgreSQL defaults. // -// Every test that owns an instance goes through here, because the three things a +// Every test that owns an instance bottoms out here, because the three things a // test that boots a DB VM must not forget are registered here: the teardown, so // a failed run does not charge the next one, the failure-only diagnostic bundle, // without which "create timed out" names no owning phase, and the charge against // the test's DB-VM reservation. -func createDBInstance(t *testing.T, f *Fixture, id string, opts ...func(*rds.CreateDBInstanceInput)) *rds.DBInstance { +func createDBInstanceFrom(t *testing.T, f *Fixture, in *rds.CreateDBInstanceInput, + opts ...func(*rds.CreateDBInstanceInput)) *rds.DBInstance { t.Helper() - in := validCreateInput(id) for _, opt := range opts { opt(in) } + id := aws.StringValue(in.DBInstanceIdentifier) markDBCreateStarted(id) out, err := f.AWS.RDS.CreateDBInstance(in) //nolint:staticcheck // e2e:allow-create — the instance under test require.NoError(t, err, "create-db-instance %s", id) diff --git a/tests/e2e/rds/mariadb_test.go b/tests/e2e/rds/mariadb_test.go new file mode 100644 index 000000000..978d6675a --- /dev/null +++ b/tests/e2e/rds/mariadb_test.go @@ -0,0 +1,587 @@ +//go:build e2e + +package rds + +import ( + "fmt" + "strconv" + "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 family a MariaDB parameter group belongs to, which is the engine and + // its pinned series exactly as AWS RDS names it. + mariaFamily = mariaEngine + mariaVersion + mariaDefaultGroup = "default." + mariaFamily + mariaInstancePfx = dbInstancePfx + "-maria" + mariaDBName = "orders" + mariaMasterUser = "appuser" + mariaMasterPassword = "e2eM4ria'DB\\Secret1" + + // Both passwords carry the two characters ValidateMasterUserPassword permits + // and no MariaDB client can quote on our behalf, so rds-init's escaping and + // the agent's rotation are under test rather than assumed. + mariaRotatedPassword = "e2eR0tated\\Maria'2" + + // The table every MariaDB assertion is made against: a stop, a start or a VM + // replacement that loses it has not preserved the datadir. + mariaTable = "e2e_mariadb" + mariaNote = "hello from the client VM" + + // A dynamic parameter, adopted with SET GLOBAL, so the assertion does not + // have to wait out a restart. Neither this nor the static one below is a + // setting mysqld rewrites at startup, which would otherwise read as a value + // the customer set never reaching the engine. + mariaDynamicParameter = "wait_timeout" + mariaDynamicValue = "3600" + + // A static one — the engine adopts it only on a restart, which is what makes + // it the parameter that can be pending-reboot. + mariaStaticParameter = "innodb_read_io_threads" + mariaStaticValue = "8" +) + +// Versions that exist upstream but are not what the image runs: two other +// series, and a minor of the pinned one. Each has to be refused rather than +// served by an image running something else. +var mariaUnpinnedVersions = []string{"11.4", "10.11", "11.8.8"} + +// The MariaDB create request, valid as it stands so a caller mutates only the +// field it cares about. Same class and storage as the PostgreSQL spec: the +// engine is what is under test, not the sizing. +func validMariaDBCreateInput(id string) *rds.CreateDBInstanceInput { + return &rds.CreateDBInstanceInput{ + DBInstanceIdentifier: aws.String(id), + Engine: aws.String(mariaEngine), + DBInstanceClass: aws.String(dbClass), + AllocatedStorage: aws.Int64(dbStorageGiB), + DBName: aws.String(mariaDBName), + MasterUsername: aws.String(mariaMasterUser), + MasterUserPassword: aws.String(mariaMasterPassword), + } +} + +// TestMariaDBInstance is the second engine's own leg of the suite: one instance +// carried from create through a client connection, a password rotation, an +// attached parameter group, a stop/start and a VM replacement. +// +// It is one test rather than several because every assertion in it is about the +// same database, and a DB VM is the suite's scarcest resource. The instance is +// consumed: it ends on a larger class with a rotated password and both +// overrides in force. +func TestMariaDBInstance(t *testing.T) { + f := requireRDSFixture(t) + t.Parallel() + // One instance throughout — the class change replaces the VM behind it rather + // than adding a second — reserved at the class it ends on, since that is the + // peak the replacement has to find room for. + reserveDBVMs(t, grownClass) + + suffix := time.Now().Unix() + id := fmt.Sprintf("%s-%d", mariaInstancePfx, suffix) + paramGroup := fmt.Sprintf("%s-params-%d", mariaInstancePfx, suffix) + + // Created before the instance so its teardown is registered first and LIFO + // runs it last: a group delete issued while the instance still holds it is + // refused. + createMariaDBParameterGroup(t, f, paramGroup) + + harness.Phase(t, "Creating MariaDB instance %q", id) + created := createDBInstanceFrom(t, f, validMariaDBCreateInput(id), func(in *rds.CreateDBInstanceInput) { + in.DBParameterGroupName = aws.String(paramGroup) + }) + assert.Equal(t, harness.DBInstanceCreating, aws.StringValue(created.DBInstanceStatus)) + + client := rdsClient(t, f) + system := f.SystemAWS(t) + instance := waitForAvailable(t, f, id) + + // Reassigned by the rotation subtest, so every subtest after it connects with + // the credential the platform last installed rather than the one that created + // the instance. + conn := harness.MariaDBConnFor(t, instance, mariaMasterUser, mariaMasterPassword, mariaDBName) + endpoint := conn.Host + + // Captured while the instance is up: the VM carries no per-instance tag, so + // after a replacement it can only be recognised by an ID held from before. + vmID := aws.StringValue(harness.DBInstanceVM(t, system, id).InstanceId) + dataVolumeID := aws.StringValue(harness.DBInstanceDataVolume(t, system, id).VolumeId) + privateIP := aws.StringValue(harness.DBEndpointENI(t, f.AWS, id).PrivateIpAddress) + + t.Run("DescribesAsMariaDBOnItsOwnPort", func(t *testing.T) { + assert.Equal(t, mariaEngine, aws.StringValue(instance.Engine), + "the engine identifier is mariadb; mysql is not offered as an alias for it") + assert.Equal(t, mariaVersion, aws.StringValue(instance.EngineVersion)) + assert.Equal(t, dbClass, aws.StringValue(instance.DBInstanceClass)) + assert.Equal(t, int64(dbStorageGiB), aws.Int64Value(instance.AllocatedStorage)) + assert.Equal(t, mariaMasterUser, aws.StringValue(instance.MasterUsername)) + assert.True(t, aws.BoolValue(instance.StorageEncrypted)) + assert.False(t, aws.BoolValue(instance.MultiAZ)) + assert.False(t, aws.BoolValue(instance.PubliclyAccessible)) + assert.Equal(t, paramGroup, dbParameterGroupName(instance)) + + require.NotNil(t, instance.Endpoint, "an available instance must publish an endpoint") + assert.Equal(t, int64(harness.MariaDBEnginePort), aws.Int64Value(instance.Endpoint.Port), + "MariaDB's default port is its own, not the other engine's") + }) + + // The first assertion that proves an engine is actually serving: every status + // before this one is the control plane's own account of itself. + t.Run("AClientInTheVPCRoundTripsARow", func(t *testing.T) { + harness.MariaDB(t, client, conn, fmt.Sprintf( + "CREATE TABLE %s (id int primary key, note text); INSERT INTO %s VALUES (1, '%s');", + mariaTable, mariaTable, mariaNote)) + + assert.Equal(t, mariaNote, mariaValue(t, client, conn, + fmt.Sprintf("SELECT note FROM %s WHERE id = 1;", mariaTable)), + "the row written over the endpoint must read back") + + // default_storage_engine is platform-owned and pinned to InnoDB because + // the snapshot guarantee rests on it: a table created without an explicit + // ENGINE= clause has to land somewhere with a redo log. + assert.Equal(t, "InnoDB", mariaValue(t, client, conn, fmt.Sprintf( + "SELECT engine FROM information_schema.tables WHERE table_schema = '%s' AND table_name = '%s';", + mariaDBName, mariaTable)), + "a table created with no ENGINE clause must be InnoDB") + }) + + // TLS is offered rather than enforced, so what is asserted is that a client + // asking for it gets it. + t.Run("AClientCanConnectOverTLS", func(t *testing.T) { + encrypted := conn + encrypted.SSLRootCert = harness.RDSClientCACertPath + assert.NotEmpty(t, mariaSessionCipher(t, client, encrypted), + "a connection made against the cluster CA must report a negotiated cipher") + + // Verified by name only, in its own subtest because a cluster with no base + // domain has no name to verify against. The endpoint name and the ENI + // address are both in the certificate's SAN set, but the client's hostname + // check is written against the certificate's DNS names, so pinning the + // address here would assert something about the client's IP-SAN handling + // rather than about ours. + t.Run("AndTheCertificateVerifiesByName", func(t *testing.T) { + requireEndpointName(t, endpoint) + verified := encrypted + verified.VerifyServerCert = true + assert.NotEmpty(t, mariaSessionCipher(t, client, verified), + "the serving certificate must verify against the cluster CA under the endpoint name") + }) + }) + + // The system keeps no cleartext password anywhere, so the only proof a + // rotation reached the engine is that the new credential authenticates and + // the old one does not. + t.Run("TheMasterPasswordRotates", func(t *testing.T) { + _, err := f.AWS.RDS.ModifyDBInstance(&rds.ModifyDBInstanceInput{ + DBInstanceIdentifier: aws.String(id), + MasterUserPassword: aws.String(mariaRotatedPassword), + ApplyImmediately: aws.Bool(true), + }) + require.NoError(t, err, "modify-db-instance master password") + + rotated := conn + rotated.Password = mariaRotatedPassword + assert.Equal(t, mariaNote, mariaValue(t, client, rotated, + fmt.Sprintf("SELECT note FROM %s WHERE id = 1;", mariaTable)), + "the rotated password must reach the same database") + + out, err := harness.TryMariaDB(client, conn, "SELECT 1;") + require.Error(t, err, "the retired password must not connect: %s", out) + assert.Contains(t, out, "Access denied", + "the old password must be refused by the engine, not by the network") + + conn = rotated + }) + + // The family check is one funnel every binding path goes through, and modify + // is the one a customer reaches by hand. Non-disruptive: it is refused before + // anything is written, so the instance is left where it was found. + t.Run("APostgreSQLParameterGroupCannotBeAttached", func(t *testing.T) { + harness.ExpectError(t, "InvalidParameterCombination", func() error { + _, err := f.AWS.RDS.ModifyDBInstance(&rds.ModifyDBInstanceInput{ + DBInstanceIdentifier: aws.String(id), + DBParameterGroupName: aws.String("default." + dbParameterGroupFamily), + ApplyImmediately: aws.Bool(true), + }) + return err + }) + + current, err := harness.DescribeDBInstance(f.AWS, id) + require.NoError(t, err) + assert.Equal(t, paramGroup, dbParameterGroupName(current), + "a refused attachment must leave the instance on the group it already had") + }) + + // The only assertions that prove a resolved parameter reached the engine + // rather than merely being stored, one per apply method. + t.Run("AttachedGroupEditsReachTheRunningEngine", func(t *testing.T) { + require.NotEqual(t, mariaDynamicValue, mariaGlobal(t, client, conn, mariaDynamicParameter), + "the chosen dynamic value must differ from the catalog default") + + setMariaDBParameter(t, f, paramGroup, mariaDynamicParameter, mariaDynamicValue, "immediate") + assert.Equal(t, mariaDynamicValue, mariaGlobal(t, client, conn, mariaDynamicParameter), + "an immediate edit to an attached group must reload without a reboot") + + before := mariaGlobal(t, client, conn, mariaStaticParameter) + require.NotEqual(t, mariaStaticValue, before, + "the chosen static value must differ from the catalog default") + + setMariaDBParameter(t, f, paramGroup, mariaStaticParameter, mariaStaticValue, "pending-reboot") + pending, err := harness.DescribeDBInstance(f.AWS, id) + require.NoError(t, err, "describe the pending static parameter") + require.NotEmpty(t, pending.DBParameterGroups) + assert.Equal(t, "pending-reboot", aws.StringValue(pending.DBParameterGroups[0].ParameterApplyStatus)) + assert.Equal(t, before, mariaGlobal(t, client, conn, mariaStaticParameter), + "a static edit must not take effect before a reboot") + + _, err = f.AWS.RDS.RebootDBInstance(&rds.RebootDBInstanceInput{DBInstanceIdentifier: aws.String(id)}) + require.NoError(t, err, "reboot-db-instance for the static parameter") + instance = waitForAvailable(t, f, id) + + require.NotEmpty(t, instance.DBParameterGroups) + assert.Equal(t, "in-sync", aws.StringValue(instance.DBParameterGroups[0].ParameterApplyStatus), + "the reboot applied the pending parameter, so nothing is outstanding") + assertMariaDBOverridesInForce(t, client, conn) + }) + + // A stop keeps the data volume, the customer ENI and its address, so a start + // comes back on the same datadir at the same endpoint. The VM is stopped + // rather than terminated — only a class change replaces it. + t.Run("StopAndStartKeepTheDataTheAddressAndTheOverrides", func(t *testing.T) { + _, err := f.AWS.RDS.StopDBInstance(&rds.StopDBInstanceInput{DBInstanceIdentifier: aws.String(id)}) + require.NoError(t, err, "stop-db-instance") + stopped := harness.WaitForDBInstanceStatus(t, f.AWS, id, harness.DBInstanceStopped) + + harness.WaitForInstanceState(t, system, vmID, ec2.InstanceStateNameStopped) + assert.Equal(t, dataVolumeID, aws.StringValue(harness.DBInstanceDataVolume(t, system, id).VolumeId), + "the data volume must be retained across a stop") + require.NotNil(t, stopped.Endpoint, "a stopped instance still reports where it will be when it comes back") + assert.Equal(t, endpoint, aws.StringValue(stopped.Endpoint.Address)) + + _, err = f.AWS.RDS.StartDBInstance(&rds.StartDBInstanceInput{DBInstanceIdentifier: aws.String(id)}) + require.NoError(t, err, "start-db-instance") + started := waitForAvailable(t, f, id) + + require.NotNil(t, started.Endpoint) + assert.Equal(t, endpoint, aws.StringValue(started.Endpoint.Address), "the endpoint must survive a stop/start") + assert.Equal(t, privateIP, aws.StringValue(harness.DBEndpointENI(t, f.AWS, id).PrivateIpAddress), + "the private address is what the endpoint name resolves to, so it must persist") + + assert.Equal(t, mariaNote, mariaValue(t, client, conn, + fmt.Sprintf("SELECT note FROM %s WHERE id = 1;", mariaTable)), + "the row written before the stop must still be there") + assertMariaDBOverridesInForce(t, client, conn) + }) + + // The class moves, which replaces the VM. The endpoint is kept through it — + // the data volume and the customer ENI are re-attached to the replacement — + // and so are the parameters the customer set, which is what putting the + // generated configuration on the data volume exists to guarantee: nothing + // issues an apply on a plain replace, so a configuration living on the boot + // volume would be discarded with it and the engine would silently revert to + // catalog defaults. + t.Run("AClassChangeReplacesTheVMAndKeepsTheOverrides", func(t *testing.T) { + poolAtFloor := mariaGlobalInt(t, client, conn, "innodb_buffer_pool_size") + + harness.Phase(t, "Changing the class of %q from %s to %s", id, dbClass, grownClass) + out, err := f.AWS.RDS.ModifyDBInstance(&rds.ModifyDBInstanceInput{ + DBInstanceIdentifier: aws.String(id), + DBInstanceClass: aws.String(grownClass), + ApplyImmediately: aws.Bool(true), + }) + require.NoError(t, err, "modify-db-instance class") + require.NotNil(t, out.DBInstance) + assert.Equal(t, harness.DBInstanceModifying, aws.StringValue(out.DBInstance.DBInstanceStatus)) + + changed := waitForAvailable(t, f, id) + assert.Equal(t, grownClass, aws.StringValue(changed.DBInstanceClass)) + require.NotNil(t, changed.Endpoint) + assert.Equal(t, endpoint, aws.StringValue(changed.Endpoint.Address), + "the endpoint is the customer's handle on the instance and must survive the VM being replaced") + assert.Equal(t, privateIP, aws.StringValue(harness.DBEndpointENI(t, f.AWS, id).PrivateIpAddress), + "the ENI is re-attached to the replacement, so the address the name resolves to is unchanged") + assert.Equal(t, dataVolumeID, aws.StringValue(harness.DBInstanceDataVolume(t, system, id).VolumeId), + "the data volume is re-attached rather than rebuilt") + + replacementID := aws.StringValue(harness.DBInstanceVM(t, system, id).InstanceId) + assert.NotEqual(t, vmID, replacementID, "a class change is delivered by replacing the VM") + harness.AssertVMGone(t, system, vmID) + + assert.Equal(t, mariaNote, mariaValue(t, client, conn, + fmt.Sprintf("SELECT note FROM %s WHERE id = 1;", mariaTable)), + "the row written before the replacement must survive it") + assertMariaDBOverridesInForce(t, client, conn) + + // The size-derived defaults are re-resolved against the class the instance + // is now on, so a replacement that carried the old configuration over + // wholesale would leave a 2 GiB instance running a 1 GiB buffer pool. The + // assertion is the direction rather than a literal: the server rounds the + // pool to its own granularity, which is not ours to predict. + assert.Greater(t, mariaGlobalInt(t, client, conn, "innodb_buffer_pool_size"), poolAtFloor, + "the buffer pool default must be re-resolved against %s", grownClass) + + require.NotEmpty(t, changed.DBParameterGroups) + assert.Equal(t, "in-sync", aws.StringValue(changed.DBParameterGroups[0].ParameterApplyStatus), + "the replacement booted on the re-resolved set, so there is nothing left for a reboot to apply") + }) +} + +// TestMariaDBSnapshotRestore proves the snapshot path is engine-neutral where it +// claims to be and engine-aware where it must be: the snapshot carries MariaDB's +// own engine, version and port, and the instance restored from it comes up on +// that datadir holding the rows as of the moment it was taken and the +// credentials the source already had. +func TestMariaDBSnapshotRestore(t *testing.T) { + f := requireRDSFixture(t) + t.Parallel() + // The source and the instance restored from its snapshot, which have to be + // alive together for the restore to be provably a point in time. + reserveDBVMs(t, dbClass, dbClass) + + suffix := time.Now().Unix() + sourceID := fmt.Sprintf("%s-snapsrc-%d", mariaInstancePfx, suffix) + restoredID := fmt.Sprintf("%s-restored-%d", mariaInstancePfx, suffix) + snapshotID := fmt.Sprintf("%s-snapshot-%d", mariaInstancePfx, suffix) + + harness.Phase(t, "Creating source MariaDB instance %q", sourceID) + createDBInstanceFrom(t, f, validMariaDBCreateInput(sourceID)) + client := rdsClient(t, f) + + source := waitForAvailable(t, f, sourceID) + sourceConn := harness.MariaDBConnFor(t, source, mariaMasterUser, mariaMasterPassword, mariaDBName) + + harness.MariaDB(t, client, sourceConn, fmt.Sprintf( + "CREATE TABLE %s (id int primary key, note text); INSERT INTO %s VALUES (1, '%s');", + snapshotTable, snapshotTable, rowBeforeSnapshot)) + + harness.Phase(t, "Snapshotting %q as %q", sourceID, snapshotID) + snapshot := createDBSnapshot(t, f, sourceID, snapshotID) + + // The snapshot has to carry enough of the instance to rebuild it, because the + // instance may be gone by the time it is used — and a snapshot that recorded + // the wrong engine would be restorable onto an image that cannot read it. + assert.Equal(t, harness.DBSnapshotAvailable, aws.StringValue(snapshot.Status)) + assert.Equal(t, mariaEngine, aws.StringValue(snapshot.Engine)) + assert.Equal(t, mariaVersion, aws.StringValue(snapshot.EngineVersion)) + assert.Equal(t, int64(harness.MariaDBEnginePort), aws.Int64Value(snapshot.Port)) + assert.Equal(t, mariaMasterUser, aws.StringValue(snapshot.MasterUsername)) + + // Written after the snapshot and therefore the row that must be missing from + // the restore. Its absence is what separates a point-in-time copy from a + // second reference to the live volume. + harness.MariaDB(t, client, sourceConn, fmt.Sprintf( + "INSERT INTO %s VALUES (2, '%s');", snapshotTable, rowAfterSnapshot)) + + // The engine was available, so the quiesce had every opportunity to run and + // the crash-consistent fallback — whose MariaDB wording warns that + // non-transactional tables may not recover — must not have been reached. + t.Run("TheSnapshotWasTakenOverAQuiescedEngine", func(t *testing.T) { + events := dbInstanceEventMessages(t, f, sourceID) + assert.NotContains(t, strings.Join(events, "\n"), crashConsistentNotice, + "BACKUP STAGE must have held the engine; the crash-consistent fallback is for when it cannot") + }) + + t.Run("ARestoreHoldsTheDataAsOfTheSnapshot", func(t *testing.T) { + harness.Phase(t, "Restoring %q from %q", restoredID, snapshotID) + restored := restoreFromSnapshot(t, f, restoredID, snapshotID) + assert.Equal(t, harness.DBInstanceCreating, aws.StringValue(restored.DBInstanceStatus)) + + instance := waitForAvailable(t, f, restoredID) + assert.Equal(t, mariaEngine, aws.StringValue(instance.Engine)) + assert.Equal(t, mariaMasterUser, aws.StringValue(instance.MasterUsername)) + require.NotNil(t, instance.Endpoint) + assert.NotEqual(t, sourceConn.Host, aws.StringValue(instance.Endpoint.Address), + "the restore is its own instance and must publish its own endpoint") + + // No password was supplied to the restore and none could be: the account + // and its hash came out of the datadir, so the source's credentials are + // the restore's. + conn := harness.MariaDBConnFor(t, instance, mariaMasterUser, mariaMasterPassword, mariaDBName) + assert.Equal(t, strconv.Itoa(snapshotRowsBefore), + mariaValue(t, client, conn, fmt.Sprintf("SELECT count(*) FROM %s;", snapshotTable)), + "the restore must hold the rows the snapshot captured and nothing written after it") + assert.Equal(t, rowBeforeSnapshot, mariaValue(t, client, conn, + fmt.Sprintf("SELECT note FROM %s WHERE id = 1;", snapshotTable))) + + assert.Equal(t, "2", mariaValue(t, client, sourceConn, + fmt.Sprintf("SELECT count(*) FROM %s;", snapshotTable)), + "the source is untouched by the restore reading from its snapshot") + }) +} + +// TestMariaDBValidation is the second engine's negative matrix: the versions and +// the cross-engine parameter groups that must be refused, and the implicit +// default group registering MariaDB brings with it. It boots nothing, so it +// fails in seconds rather than after a VM. +func TestMariaDBValidation(t *testing.T) { + f := requireRDSFixture(t) + t.Parallel() + // Every create below is asserted to be refused, so in a passing run nothing + // boots. The reservation covers the run where one is wrongly accepted: + // expectCreateRefused deletes it, but the VM is charged to the suite until it + // does. + reserveDBVMs(t, dbClass) + + suffix := time.Now().Unix() + rejectedID := fmt.Sprintf("%s-rejected-%d", mariaInstancePfx, suffix) + mariaGroup := fmt.Sprintf("%s-family-%d", mariaInstancePfx, suffix) + + createMariaDBParameterGroup(t, f, mariaGroup) + + // Implicit rather than stored: registering the engine is what makes it + // describable, and it must be so without ever having been created. + t.Run("TheDefaultMariaDBGroupIsImplicitAndCarriesTheCatalog", func(t *testing.T) { + out, err := f.AWS.RDS.DescribeDBParameterGroups(&rds.DescribeDBParameterGroupsInput{ + DBParameterGroupName: aws.String(mariaDefaultGroup), + }) + require.NoError(t, err, "describe-db-parameter-groups") + require.Len(t, out.DBParameterGroups, 1) + assert.Equal(t, mariaFamily, aws.StringValue(out.DBParameterGroups[0].DBParameterGroupFamily)) + + params := describeAllParameters(t, f, mariaDefaultGroup) + pool, ok := params["innodb_buffer_pool_size"] + require.True(t, ok, "the MariaDB catalog must be reachable through its own default group") + assert.Equal(t, "engine-default", aws.StringValue(pool.Source)) + assert.Equal(t, "static", aws.StringValue(pool.ApplyType)) + assert.NotContains(t, aws.StringValue(pool.ParameterValue), "{", + "a size-derived default must reach the customer as a literal") + + assert.NotContains(t, params, "shared_buffers", + "the MariaDB catalog must not carry the other engine's parameters") + }) + + t.Run("AnUnpinnedEngineVersionIsRefused", func(t *testing.T) { + for _, version := range mariaUnpinnedVersions { + t.Run(version, func(t *testing.T) { + in := validMariaDBCreateInput(rejectedID) + in.EngineVersion = aws.String(version) + expectCreateRefused(t, f, f.AWS, "InvalidParameterValue", in) + }) + } + }) + + // Aliasing mysql onto the MariaDB image would report an engine and a version + // the instance is not running, and the lie would propagate into the describe, + // the parameter-group family and every snapshot taken of it. + t.Run("MySQLIsNotAnAliasForMariaDB", func(t *testing.T) { + in := validMariaDBCreateInput(rejectedID) + in.Engine = aws.String(unsupportedDBEngine) + expectCreateRefused(t, f, f.AWS, "InvalidParameterValue", in) + }) + + // Both directions, because the check is one funnel and a group carrying the + // wrong engine's parameters would only fail once the engine refused to start. + t.Run("AParameterGroupOfTheOtherEnginesFamilyIsRefusedAtCreate", func(t *testing.T) { + mariaOnPostgres := validCreateInput(rejectedID) + mariaOnPostgres.DBParameterGroupName = aws.String(mariaGroup) + expectCreateRefused(t, f, f.AWS, "InvalidParameterCombination", mariaOnPostgres) + + postgresOnMaria := validMariaDBCreateInput(rejectedID) + postgresOnMaria.DBParameterGroupName = aws.String("default." + dbParameterGroupFamily) + expectCreateRefused(t, f, f.AWS, "InvalidParameterCombination", postgresOnMaria) + }) + + // The other engine's parameter names must not be storable into a MariaDB + // group either: caught here, the customer learns of it on the group they got + // wrong rather than on the instance that later refuses to start. + t.Run("TheOtherEnginesParametersCannotBeStored", func(t *testing.T) { + harness.ExpectError(t, "InvalidParameterValue", func() error { + _, err := f.AWS.RDS.ModifyDBParameterGroup(&rds.ModifyDBParameterGroupInput{ + DBParameterGroupName: aws.String(mariaGroup), + Parameters: []*rds.Parameter{{ + ParameterName: aws.String("shared_buffers"), + ParameterValue: aws.String("32768"), + }}, + }) + return err + }) + }) +} + +// An empty MariaDB parameter group and its teardown. Empty because what each +// test then stores into it is the assertion. +func createMariaDBParameterGroup(t *testing.T, f *Fixture, name string) { + t.Helper() + _, err := f.AWS.RDS.CreateDBParameterGroup(&rds.CreateDBParameterGroupInput{ + DBParameterGroupName: aws.String(name), + DBParameterGroupFamily: aws.String(mariaFamily), + Description: aws.String("rds e2e mariadb parameter group"), + }) + require.NoError(t, err, "create-db-parameter-group %s", name) + t.Cleanup(func() { + if _, err := f.AWS.RDS.DeleteDBParameterGroup(&rds.DeleteDBParameterGroupInput{ + DBParameterGroupName: aws.String(name), + }); err != nil && !harness.ErrorCodeIs(err, "DBParameterGroupNotFound") { + t.Logf("cleanup: delete-db-parameter-group %s: %v", name, err) + } + }) +} + +func setMariaDBParameter(t *testing.T, f *Fixture, group, name, value, applyMethod string) { + t.Helper() + _, err := f.AWS.RDS.ModifyDBParameterGroup(&rds.ModifyDBParameterGroupInput{ + DBParameterGroupName: aws.String(group), + Parameters: []*rds.Parameter{{ + ParameterName: aws.String(name), + ParameterValue: aws.String(value), + ApplyMethod: aws.String(applyMethod), + }}, + }) + require.NoError(t, err, "modify-db-parameter-group %s: %s=%s", group, name, value) +} + +// Both overrides as the running engine reports them. Asserted after every +// transition, because the failure this catches is silent: an instance that came +// back on catalog defaults is healthy, available and running a configuration +// its owner did not choose. +func assertMariaDBOverridesInForce(t *testing.T, tgt harness.SSHTarget, conn harness.MariaDBConn) { + t.Helper() + assert.Equal(t, mariaDynamicValue, mariaGlobal(t, tgt, conn, mariaDynamicParameter), + "the dynamic override must still be in force") + assert.Equal(t, mariaStaticValue, mariaGlobal(t, tgt, conn, mariaStaticParameter), + "the static override must still be in force") +} + +// One global system variable as the running server reports it. GLOBAL rather +// than the session copy, since that is the scope a parameter group sets. +func mariaGlobal(t *testing.T, tgt harness.SSHTarget, conn harness.MariaDBConn, name string) string { + t.Helper() + return mariaValue(t, tgt, conn, "SELECT @@GLOBAL."+name+";") +} + +func mariaGlobalInt(t *testing.T, tgt harness.SSHTarget, conn harness.MariaDBConn, name string) int64 { + t.Helper() + raw := mariaGlobal(t, tgt, conn, name) + value, err := strconv.ParseInt(raw, 10, 64) + if err != nil { + t.Fatalf("@@GLOBAL.%s reported %q, which is not an integer", name, raw) + } + return value +} + +// The cipher the connection negotiated, empty when it is in the clear. SHOW +// rather than performance_schema, which the catalog leaves off by default. +func mariaSessionCipher(t *testing.T, tgt harness.SSHTarget, conn harness.MariaDBConn) string { + t.Helper() + _, cipher, _ := strings.Cut(mariaValue(t, tgt, conn, "SHOW SESSION STATUS LIKE 'Ssl_cipher';"), "\t") + return strings.TrimSpace(cipher) +} + +// One scalar as the client printed it. The guest exec folds stderr into the same +// stream, so a client-side notice would otherwise be compared against the value: +// the answer is the last line the command produced. +func mariaValue(t *testing.T, tgt harness.SSHTarget, conn harness.MariaDBConn, sql string) string { + t.Helper() + out := harness.MariaDB(t, tgt, conn, sql) + lines := strings.Split(strings.TrimSpace(out), "\n") + return strings.TrimSpace(lines[len(lines)-1]) +} diff --git a/tests/e2e/rds/snapshot_restore_test.go b/tests/e2e/rds/snapshot_restore_test.go index a2e2e2703..20446b84f 100644 --- a/tests/e2e/rds/snapshot_restore_test.go +++ b/tests/e2e/rds/snapshot_restore_test.go @@ -88,7 +88,7 @@ func TestSnapshotRestore(t *testing.T) { assert.Equal(t, dbEngine, aws.StringValue(snapshot.Engine)) assert.Equal(t, int64(dbStorageGiB), aws.Int64Value(snapshot.AllocatedStorage)) assert.Equal(t, dbMasterUser, aws.StringValue(snapshot.MasterUsername)) - assert.Equal(t, int64(harness.DBEnginePort), aws.Int64Value(snapshot.Port)) + assert.Equal(t, int64(harness.PostgresEnginePort), aws.Int64Value(snapshot.Port)) assert.True(t, aws.BoolValue(snapshot.Encrypted), "the snapshot inherits the data volume's encryption") assert.Equal(t, int64(100), aws.Int64Value(snapshot.PercentProgress), "a snapshot reported as available must not also report itself part-done")