From 95911cbfae763d1f5f2a2c4e384b6ff1e6c48fcc Mon Sep 17 00:00:00 2001 From: Julian Sommer <115516918+juliansommer@users.noreply.github.com> Date: Mon, 17 Aug 2026 15:04:12 +1000 Subject: [PATCH] feat: DescribeDBEngineVersions and DescribeOrderableDBInstanceOptions (#807) --- docs/COMMANDS.md | 18 +- docs/aws-model-operation-coverage.md | 34 +- spinifex/gateway/rds.go | 3 +- spinifex/gateway/rds/agent_test.go | 2 +- spinifex/gateway/rds/enginecatalog.go | 220 +++++++++++++ spinifex/gateway/rds/enginecatalog_test.go | 355 +++++++++++++++++++++ spinifex/gateway/rds/handler.go | 34 +- spinifex/gateway/rds/handler_test.go | 58 +++- spinifex/handlers/rds/catalog.go | 214 +++++++++++++ spinifex/handlers/rds/catalog_test.go | 311 ++++++++++++++++++ spinifex/handlers/rds/engine.go | 17 + spinifex/handlers/rds/engine_mariadb.go | 2 + spinifex/handlers/rds/engine_postgres.go | 2 + spinifex/handlers/rds/enginefamily_test.go | 2 + tests/e2e/rds/api_matrix_test.go | 84 ++++- 15 files changed, 1314 insertions(+), 42 deletions(-) create mode 100644 spinifex/gateway/rds/enginecatalog.go create mode 100644 spinifex/gateway/rds/enginecatalog_test.go create mode 100644 spinifex/handlers/rds/catalog.go create mode 100644 spinifex/handlers/rds/catalog_test.go diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index d777bcb39..886dbcff2 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -906,7 +906,7 @@ Everything below applies to both engines unless it names one. `Engine` is fixed **`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. +- **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. **`db.X` is the EC2 instance type `X`**, so a client reads a class's vCPU count and memory from `ec2:describe-instance-types` for the identically named type; an orderable option carries neither, because AWS's own shape has no field for them. `describe-orderable-db-instance-options` reports only the classes the cluster's nodes can actually run, which on a Graviton-only cluster is none of the six — a class that would fail at launch is not offered. - **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. - **`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 v1.3:** **required by default on both engines** — `rds.force_ssl` on `postgres18` and `require_secure_transport` on `mariadb11.8`, both boolean, modifiable and dynamic, both defaulting to `1`. Setting either to `0` in a parameter group restores plaintext without a reboot. Defaulting on is a **deliberate divergence** for MariaDB, which AWS leaves off. @@ -988,6 +988,20 @@ A group takes effect on an instance when `modify-db-instance --db-parameter-grou |---------|-------------------|---------------|--------| | `describe-events` | `--source-type` (db-instance, db-snapshot), `--source-identifier`, `--event-categories`, `--duration`, `--start-time`, `--end-time`, `--max-records` | `--marker` | **DONE** — 100-event ring per resource, 14-day retention, one-hour default window | +### RDS — Engine Versions & Orderable Options + +| Command | Implemented Flags | Missing Flags | Status | +|---------|-------------------|---------------|--------| +| `describe-db-engine-versions` | `--engine`, `--engine-version`, `--db-parameter-group-family`, `--filters` (`engine`, `engine-version`, `db-parameter-group-family`, `status`) | `--default-only`, `--include-all`, `--list-supported-character-sets`, `--list-supported-timezones` (accepted, provably no-ops), `--max-records` (accepted, no effect), `--marker` (rejected) | **DONE** — one row per engine | +| `describe-orderable-db-instance-options` | `--engine` (required), `--engine-version`, `--db-instance-class`, `--license-model`, `--vpc`, `--filters` (`engine`, `engine-version`, `db-instance-class`, `license-model`, `vpc`) | `--max-records` (accepted, no effect), `--marker`, `--availability-zone-group` (both rejected) | **DONE** — the engines crossed with the classes the cluster's nodes can run | + +Both are read from the tables the create path validates against, so an engine pin bump or a class-map change moves these answers with it. Neither paginates — at most 2 engine versions and 12 orderable options — so no `Marker` is ever returned, and one supplied is rejected rather than answered as page one, where the other RDS describes parse and ignore it. + +- **`AvailabilityZones` and `AvailableProcessorFeatures` are always empty**, and are gaps only in appearance: `--availability-zone` is rejected on create and `--processor-features` is ignored, so naming either here would advertise a knob that does nothing. +- **Only the classes the cluster's nodes can run are offered.** A cluster that answers nothing is `ServerInternal`, never the full class list; one that answers and runs none of them is an empty list. Free capacity is deliberately not consulted — exhausted capacity is a create-time failure, and filtering on it would make a Terraform data source return a different answer on every plan. + +Two further deliberate divergences: the four `describe-db-engine-versions` booleans are accepted as **provable identities** rather than the "parsed, not applied" the other describes document, because one version per engine and two empty lists leave them nothing to change; and `--filters` on `describe-orderable-db-instance-options` is **applied** where AWS documents it unsupported. Both actions reject an unrecognised filter name, and a non-boolean `vpc` value, with `InvalidParameterValue` rather than ignoring it — stricter than AWS rather than looser. + ### RDS — Rejected Parameters Policy: a parameter whose omission would create a false safety, security or availability guarantee is rejected with `InvalidParameterValue` rather than silently dropped. Parameters that are merely inert — `AutoMinorVersionUpgrade`, `CopyTagsToSnapshot`, `DeleteAutomatedBackups`, Performance Insights and Enhanced Monitoring fields — are accepted as no-ops. @@ -1001,6 +1015,7 @@ Policy: a parameter whose omission would create a false safety, security or avai | `Iops`, `StorageThroughput`, `StorageType` ≠ `gp3` | Provisioned performance classes are not implemented | | `KmsKeyId`, `TdeCredentialArn` | Storage is encrypted with the cluster key, not a customer-managed one | | `AvailabilityZone` | The platform exposes a single zone | +| `AvailabilityZoneGroup` (orderable options) | It selects a zone or local-zone group, and naming a zone is already refused | | `DBSecurityGroups` | EC2-Classic security groups — use `VpcSecurityGroupIds` | | `DBClusterIdentifier`, `DBClusterSnapshotIdentifier` | Clustered engines are not offered | | `EnableCloudwatchLogsExports` | Log export is not implemented | @@ -1030,7 +1045,6 @@ Recognised actions below return `OperationNotSupported`, so a client sees "not o | 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, 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/aws-model-operation-coverage.md b/docs/aws-model-operation-coverage.md index 2b9adaba9..11751a6ff 100644 --- a/docs/aws-model-operation-coverage.md +++ b/docs/aws-model-operation-coverage.md @@ -12,7 +12,7 @@ Implemented means a modelled operation is registered to a real handler. Stub and | ecs | 2014-11-13 | 56 | 39 | 31 | 5 | 0 | 20 | 3 | | elasticloadbalancingv2 | 2015-12-01 | 46 | 37 | 33 | 0 | 0 | 13 | 4 | | iam | 2010-05-08 | 159 | 75 | 75 | 0 | 0 | 84 | 0 | -| rds | 2014-10-31 | 162 | 41 | 24 | 0 | 12 | 126 | 5 | +| rds | 2014-10-31 | 162 | 43 | 26 | 0 | 12 | 124 | 5 | | s3 | 2006-03-01 | 99 | — | — | — | — | — | — | | sts | 2011-06-15 | 8 | 4 | 4 | 0 | 0 | 4 | 0 | @@ -462,20 +462,22 @@ None. ## rds -Implements **24 of 162** modelled operations (14.8%). +Implements **26 of 162** modelled operations (16.0%). -
Implemented (24) +
Implemented (26) `AddTagsToResource`, `CreateDBInstance`, `CreateDBParameterGroup`, `CreateDBSnapshot`, `CreateDBSubnetGroup`, `DeleteDBInstance`, `DeleteDBParameterGroup`, `DeleteDBSnapshot` -`DeleteDBSubnetGroup`, `DescribeDBInstanceAutomatedBackups`, `DescribeDBInstances`, `DescribeDBParameterGroups`, `DescribeDBParameters`, `DescribeDBSnapshots`, `DescribeDBSubnetGroups`, `DescribeEvents` +`DeleteDBSubnetGroup`, `DescribeDBEngineVersions`, `DescribeDBInstanceAutomatedBackups`, `DescribeDBInstances`, `DescribeDBParameterGroups`, `DescribeDBParameters`, `DescribeDBSnapshots`, `DescribeDBSubnetGroups` -`ListTagsForResource`, `ModifyDBInstance`, `ModifyDBParameterGroup`, `RebootDBInstance`, `RemoveTagsFromResource`, `RestoreDBInstanceFromDBSnapshot`, `StartDBInstance`, `StopDBInstance` +`DescribeEvents`, `DescribeOrderableDBInstanceOptions`, `ListTagsForResource`, `ModifyDBInstance`, `ModifyDBParameterGroup`, `RebootDBInstance`, `RemoveTagsFromResource`, `RestoreDBInstanceFromDBSnapshot` + +`StartDBInstance`, `StopDBInstance`
-
Missing from dispatch (126) +
Missing from dispatch (124) `AddRoleToDBCluster`, `AddRoleToDBInstance`, `AddSourceIdentifierToSubscription`, `ApplyPendingMaintenanceAction`, `AuthorizeDBSecurityGroupIngress`, `BacktrackDBCluster`, `CancelExportTask`, `CopyDBClusterParameterGroup` @@ -489,25 +491,25 @@ Implements **24 of 162** modelled operations (14.8%). `DeregisterDBProxyTargets`, `DescribeAccountAttributes`, `DescribeBlueGreenDeployments`, `DescribeCertificates`, `DescribeDBClusterAutomatedBackups`, `DescribeDBClusterBacktracks`, `DescribeDBClusterEndpoints`, `DescribeDBClusterParameterGroups` -`DescribeDBClusterParameters`, `DescribeDBClusterSnapshotAttributes`, `DescribeDBClusterSnapshots`, `DescribeDBEngineVersions`, `DescribeDBLogFiles`, `DescribeDBProxies`, `DescribeDBProxyEndpoints`, `DescribeDBProxyTargetGroups` +`DescribeDBClusterParameters`, `DescribeDBClusterSnapshotAttributes`, `DescribeDBClusterSnapshots`, `DescribeDBLogFiles`, `DescribeDBProxies`, `DescribeDBProxyEndpoints`, `DescribeDBProxyTargetGroups`, `DescribeDBProxyTargets` -`DescribeDBProxyTargets`, `DescribeDBRecommendations`, `DescribeDBSecurityGroups`, `DescribeDBShardGroups`, `DescribeDBSnapshotAttributes`, `DescribeDBSnapshotTenantDatabases`, `DescribeEngineDefaultClusterParameters`, `DescribeEngineDefaultParameters` +`DescribeDBRecommendations`, `DescribeDBSecurityGroups`, `DescribeDBShardGroups`, `DescribeDBSnapshotAttributes`, `DescribeDBSnapshotTenantDatabases`, `DescribeEngineDefaultClusterParameters`, `DescribeEngineDefaultParameters`, `DescribeEventCategories` -`DescribeEventCategories`, `DescribeEventSubscriptions`, `DescribeExportTasks`, `DescribeGlobalClusters`, `DescribeIntegrations`, `DescribeOptionGroupOptions`, `DescribeOrderableDBInstanceOptions`, `DescribePendingMaintenanceActions` +`DescribeEventSubscriptions`, `DescribeExportTasks`, `DescribeGlobalClusters`, `DescribeIntegrations`, `DescribeOptionGroupOptions`, `DescribePendingMaintenanceActions`, `DescribeReservedDBInstances`, `DescribeReservedDBInstancesOfferings` -`DescribeReservedDBInstances`, `DescribeReservedDBInstancesOfferings`, `DescribeSourceRegions`, `DescribeTenantDatabases`, `DescribeValidDBInstanceModifications`, `DisableHttpEndpoint`, `DownloadDBLogFilePortion`, `EnableHttpEndpoint` +`DescribeSourceRegions`, `DescribeTenantDatabases`, `DescribeValidDBInstanceModifications`, `DisableHttpEndpoint`, `DownloadDBLogFilePortion`, `EnableHttpEndpoint`, `FailoverGlobalCluster`, `ModifyActivityStream` -`FailoverGlobalCluster`, `ModifyActivityStream`, `ModifyCertificates`, `ModifyCurrentDBClusterCapacity`, `ModifyCustomDBEngineVersion`, `ModifyDBClusterEndpoint`, `ModifyDBClusterParameterGroup`, `ModifyDBClusterSnapshotAttribute` +`ModifyCertificates`, `ModifyCurrentDBClusterCapacity`, `ModifyCustomDBEngineVersion`, `ModifyDBClusterEndpoint`, `ModifyDBClusterParameterGroup`, `ModifyDBClusterSnapshotAttribute`, `ModifyDBProxy`, `ModifyDBProxyEndpoint` -`ModifyDBProxy`, `ModifyDBProxyEndpoint`, `ModifyDBProxyTargetGroup`, `ModifyDBRecommendation`, `ModifyDBShardGroup`, `ModifyDBSnapshot`, `ModifyDBSnapshotAttribute`, `ModifyDBSubnetGroup` +`ModifyDBProxyTargetGroup`, `ModifyDBRecommendation`, `ModifyDBShardGroup`, `ModifyDBSnapshot`, `ModifyDBSnapshotAttribute`, `ModifyDBSubnetGroup`, `ModifyEventSubscription`, `ModifyGlobalCluster` -`ModifyEventSubscription`, `ModifyGlobalCluster`, `ModifyIntegration`, `ModifyTenantDatabase`, `PromoteReadReplicaDBCluster`, `PurchaseReservedDBInstancesOffering`, `RebootDBCluster`, `RebootDBShardGroup` +`ModifyIntegration`, `ModifyTenantDatabase`, `PromoteReadReplicaDBCluster`, `PurchaseReservedDBInstancesOffering`, `RebootDBCluster`, `RebootDBShardGroup`, `RegisterDBProxyTargets`, `RemoveFromGlobalCluster` -`RegisterDBProxyTargets`, `RemoveFromGlobalCluster`, `RemoveRoleFromDBCluster`, `RemoveRoleFromDBInstance`, `RemoveSourceIdentifierFromSubscription`, `ResetDBClusterParameterGroup`, `ResetDBParameterGroup`, `RestoreDBClusterFromS3` +`RemoveRoleFromDBCluster`, `RemoveRoleFromDBInstance`, `RemoveSourceIdentifierFromSubscription`, `ResetDBClusterParameterGroup`, `ResetDBParameterGroup`, `RestoreDBClusterFromS3`, `RestoreDBClusterFromSnapshot`, `RestoreDBClusterToPointInTime` -`RestoreDBClusterFromSnapshot`, `RestoreDBClusterToPointInTime`, `RestoreDBInstanceFromS3`, `RevokeDBSecurityGroupIngress`, `StartActivityStream`, `StartDBCluster`, `StartDBInstanceAutomatedBackupsReplication`, `StartExportTask` +`RestoreDBInstanceFromS3`, `RevokeDBSecurityGroupIngress`, `StartActivityStream`, `StartDBCluster`, `StartDBInstanceAutomatedBackupsReplication`, `StartExportTask`, `StopActivityStream`, `StopDBCluster` -`StopActivityStream`, `StopDBCluster`, `StopDBInstanceAutomatedBackupsReplication`, `SwitchoverBlueGreenDeployment`, `SwitchoverGlobalCluster`, `SwitchoverReadReplica` +`StopDBInstanceAutomatedBackupsReplication`, `SwitchoverBlueGreenDeployment`, `SwitchoverGlobalCluster`, `SwitchoverReadReplica`
diff --git a/spinifex/gateway/rds.go b/spinifex/gateway/rds.go index f8fecd6f0..f1d69d5d5 100644 --- a/spinifex/gateway/rds.go +++ b/spinifex/gateway/rds.go @@ -54,7 +54,8 @@ func (gw *GatewayConfig) RDS_Request(w http.ResponseWriter, r *http.Request) err return errors.New(awserrors.ErrorServerInternal) } - xmlOutput, err := gateway_rds.Dispatch(r.Context(), action, queryArgs, gw.NATSConn, caller) + xmlOutput, err := gateway_rds.Dispatch(r.Context(), action, queryArgs, gw.NATSConn, caller, + gateway_rds.Env{ExpectedNodes: gw.ExpectedNodes}) if err != nil { return err } diff --git a/spinifex/gateway/rds/agent_test.go b/spinifex/gateway/rds/agent_test.go index 820c2fdce..49d2af2d7 100644 --- a/spinifex/gateway/rds/agent_test.go +++ b/spinifex/gateway/rds/agent_test.go @@ -121,7 +121,7 @@ func TestAuthorizeAgent_RejectsWhenNoInstancesExist(t *testing.T) { func TestInternalActions_GateBeforeDispatch(t *testing.T) { for _, action := range []string{"RegisterDBInstance", "SubmitDBStateChange", "PollDBCommands", "GetDBBootstrapConfig"} { t.Run(action, func(t *testing.T) { - _, err := Dispatch(t.Context(), action, map[string]string{"Action": action}, nil, testCaller) + _, err := Dispatch(t.Context(), action, map[string]string{"Action": action}, nil, testCaller, testEnv) require.Error(t, err) assert.Equal(t, awserrors.ErrorAccessDenied, err.Error()) }) diff --git a/spinifex/gateway/rds/enginecatalog.go b/spinifex/gateway/rds/enginecatalog.go new file mode 100644 index 000000000..07d4c7c0f --- /dev/null +++ b/spinifex/gateway/rds/enginecatalog.go @@ -0,0 +1,220 @@ +package gateway_rds + +import ( + "context" + "errors" + "log/slog" + "strconv" + "strings" + + "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/spinifex/awserrors" + gateway_ec2_instance "github.com/mulgadc/spinifex/spinifex/gateway/ec2/instance" + handlers_rds "github.com/mulgadc/spinifex/spinifex/handlers/rds" + "github.com/mulgadc/spinifex/spinifex/utils" + "github.com/nats-io/nats.go" +) + +// The filter names each action recognises. Rejecting an unknown name is only +// implementable against a closed vocabulary, and the two actions take disjoint +// typed parameters, so each keeps its own list rather than sharing one. +const ( + filterNameEngine = "engine" + filterNameEngineVersion = "engine-version" + filterNameParameterGroupFamily = "db-parameter-group-family" + filterNameStatus = "status" + filterNameDBInstanceClass = "db-instance-class" + filterNameLicenseModel = "license-model" + filterNameVpc = "vpc" +) + +var ( + engineVersionFilterNames = []string{ + filterNameEngine, filterNameEngineVersion, filterNameParameterGroupFamily, filterNameStatus, + } + orderableFilterNames = []string{ + filterNameEngine, filterNameEngineVersion, filterNameDBInstanceClass, filterNameLicenseModel, filterNameVpc, + } +) + +// The engine catalog is static, so this reads it with no I/O at all: Engine is a +// filter here rather than a required parameter, and an unknown one is an empty +// list rather than the rejection create-db-instance gives it. +func DescribeDBEngineVersions(ctx context.Context, input *rds.DescribeDBEngineVersionsInput, _ *nats.Conn, _ Caller) (any, error) { + if err := rejectMarker(input.Marker, "DescribeDBEngineVersions"); err != nil { + return nil, err + } + + filter := handlers_rds.EngineVersionFilter{} + filter.Engine.AddParam(aws.StringValue(input.Engine)) + filter.EngineVersion.AddParam(aws.StringValue(input.EngineVersion)) + filter.ParameterGroupFamily.AddParam(aws.StringValue(input.DBParameterGroupFamily)) + + for _, entry := range input.Filters { + name, values, err := filterEntry(entry) + if err != nil { + return nil, err + } + switch name { + case filterNameEngine: + filter.Engine.AddFilter(values) + case filterNameEngineVersion: + filter.EngineVersion.AddFilter(values) + case filterNameParameterGroupFamily: + filter.ParameterGroupFamily.AddFilter(values) + case filterNameStatus: + filter.Status.AddFilter(values) + default: + return nil, unknownFilterName(name, engineVersionFilterNames) + } + } + + // DefaultOnly, IncludeAll, ListSupportedCharacterSets and ListSupportedTimezones + // are accepted and not read: each is an identity on a catalog of one available + // version per engine with no character-set or timezone list to populate. + return &rds.DescribeDBEngineVersionsOutput{DBEngineVersions: handlers_rds.EngineVersions(filter)}, nil +} + +// Engine is required, so an absent one is MissingParameter and an unknown one is +// the InvalidParameterValue LookupEngine already words. Everything else narrows. +func DescribeOrderableDBInstanceOptions(ctx context.Context, input *rds.DescribeOrderableDBInstanceOptionsInput, nc *nats.Conn, _ Caller, env Env) (any, error) { + if err := rejectMarker(input.Marker, "DescribeOrderableDBInstanceOptions"); err != nil { + return nil, err + } + if aws.StringValue(input.AvailabilityZoneGroup) != "" { + return nil, awserrors.Errorf(awserrors.ErrorInvalidParameterValue, + "AvailabilityZoneGroup is not supported: this platform exposes a single zone and names none") + } + if strings.TrimSpace(aws.StringValue(input.Engine)) == "" { + return nil, awserrors.Errorf(awserrors.ErrorMissingParameter, "Engine is required") + } + engine, err := handlers_rds.LookupEngine(aws.StringValue(input.Engine)) + if err != nil { + return nil, err + } + + filter := handlers_rds.OrderableFilter{} + filter.Engine.AddParam(engine.Name) + filter.EngineVersion.AddParam(aws.StringValue(input.EngineVersion)) + filter.DBInstanceClass.AddParam(aws.StringValue(input.DBInstanceClass)) + filter.LicenseModel.AddParam(aws.StringValue(input.LicenseModel)) + if input.Vpc != nil { + filter.Vpc.AddParam(strconv.FormatBool(aws.BoolValue(input.Vpc))) + } + + for _, entry := range input.Filters { + name, values, err := filterEntry(entry) + if err != nil { + return nil, err + } + switch name { + case filterNameEngine: + filter.Engine.AddFilter(values) + case filterNameEngineVersion: + filter.EngineVersion.AddFilter(values) + case filterNameDBInstanceClass: + filter.DBInstanceClass.AddFilter(values) + case filterNameLicenseModel: + filter.LicenseModel.AddFilter(values) + case filterNameVpc: + parsed, perr := boolFilterValues(name, values) + if perr != nil { + return nil, perr + } + filter.Vpc.AddFilter(parsed) + default: + return nil, unknownFilterName(name, orderableFilterNames) + } + } + + runnable, err := clusterRunnableTypes(ctx, nc, env) + if err != nil { + return nil, err + } + return &rds.DescribeOrderableDBInstanceOptionsOutput{ + OrderableDBInstanceOptions: handlers_rds.OrderableOptions(filter, runnable), + }, nil +} + +// Which EC2 instance types the cluster's nodes report they can run, as a +// membership test. Capacity is deliberately not consulted: an exhausted cluster +// still offers the class, and a create that cannot be placed is a create-time +// failure rather than a class the region does not have. +// +// The probe names no instance type, because the union is what tells a cluster +// that answered nothing apart from one whose nodes run none of the db.* classes: +// DescribeInstanceTypes filters its reply by the requested types and reports a +// timed-out gather as an empty list with no error, so asking for the six would +// collapse both onto the same answer. +func clusterRunnableTypes(ctx context.Context, nc *nats.Conn, env Env) (func(string) bool, error) { + out, err := gateway_ec2_instance.DescribeInstanceTypes(ctx, &ec2.DescribeInstanceTypesInput{}, + nc, env.ExpectedNodes, utils.GlobalAccountID) + if err != nil { + slog.ErrorContext(ctx, "RDS: instance-type capability probe failed", "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) + } + + supported := make(map[string]bool, len(out.InstanceTypes)) + for _, info := range out.InstanceTypes { + if info != nil && info.InstanceType != nil { + supported[*info.InstanceType] = true + } + } + // No node answered. Falling back to the full class list here would offer + // classes that cannot launch, which is the failure this probe exists to stop. + if len(supported) == 0 { + slog.ErrorContext(ctx, "RDS: no node reported any instance type") + return nil, errors.New(awserrors.ErrorServerInternal) + } + return func(instanceType string) bool { return supported[instanceType] }, nil +} + +// Neither action ever issues a Marker, so one in a request can only have been +// fabricated. Answering it as page one would report the whole catalog as if it +// were a later page. +func rejectMarker(marker *string, action string) error { + if aws.StringValue(marker) == "" { + return nil + } + return awserrors.Errorf(awserrors.ErrorInvalidParameterValue, + "Marker is not supported: %s returns every row in a single page", action) +} + +// Both members are required by the shape, and treating either as absent would +// silently widen the result rather than narrow it. +func filterEntry(entry *rds.Filter) (string, []string, error) { + if entry == nil || strings.TrimSpace(aws.StringValue(entry.Name)) == "" { + return "", nil, awserrors.Errorf(awserrors.ErrorInvalidParameterValue, "each filter must carry a name") + } + name := strings.ToLower(strings.TrimSpace(aws.StringValue(entry.Name))) + if len(entry.Values) == 0 { + return "", nil, awserrors.Errorf(awserrors.ErrorInvalidParameterValue, + "filter %q must carry at least one value", name) + } + return name, aws.StringValueSlice(entry.Values), nil +} + +// A bool-shaped filter is parsed rather than compared as text, so it accepts +// every spelling the typed parameter does. Matching only "true" and "false" +// would answer "1" with an empty catalog the caller cannot tell from a real one. +func boolFilterValues(name string, values []string) ([]string, error) { + parsed := make([]string, 0, len(values)) + for _, value := range values { + b, err := strconv.ParseBool(strings.TrimSpace(value)) + if err != nil { + return nil, awserrors.Errorf(awserrors.ErrorInvalidParameterValue, + "filter %q takes a boolean, not %q", name, value) + } + parsed = append(parsed, strconv.FormatBool(b)) + } + return parsed, nil +} + +// Rejected rather than ignored: these two actions exist only to be filtered, so +// a dropped filter returns rows the caller asked not to see and cannot detect. +func unknownFilterName(name string, recognised []string) error { + return awserrors.Errorf(awserrors.ErrorInvalidParameterValue, + "filter %q is not recognised; supported filters are %s", name, strings.Join(recognised, ", ")) +} diff --git a/spinifex/gateway/rds/enginecatalog_test.go b/spinifex/gateway/rds/enginecatalog_test.go new file mode 100644 index 000000000..3096c6889 --- /dev/null +++ b/spinifex/gateway/rds/enginecatalog_test.go @@ -0,0 +1,355 @@ +package gateway_rds + +//test:in-package — shares newStubbedNATS, testCaller and testEnv with +// handler_test.go, which is in-package for the same reason: the stubbed NATS +// scaffolding every dispatch test needs is not reachable from outside. + +import ( + "bytes" + "encoding/xml" + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/private/protocol/xml/xmlutil" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/aws/aws-sdk-go/service/rds" + "github.com/mulgadc/spinifex/spinifex/awserrors" + handlers_rds "github.com/mulgadc/spinifex/spinifex/handlers/rds" + "github.com/mulgadc/spinifex/spinifex/testutil" + "github.com/nats-io/nats.go" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func describeEngineVersions(t *testing.T, query map[string]string) []*rds.DBEngineVersion { + t.Helper() + query["Action"] = "DescribeDBEngineVersions" + body, err := Dispatch(t.Context(), "DescribeDBEngineVersions", query, nil, testCaller, testEnv) + require.NoError(t, err) + + var out rds.DescribeDBEngineVersionsOutput + require.NoError(t, xmlutil.UnmarshalXML(&out, xml.NewDecoder(bytes.NewReader(body)), + "DescribeDBEngineVersionsResult")) + return out.DBEngineVersions +} + +func describeOrderable(t *testing.T, nc *nats.Conn, query map[string]string) []*rds.OrderableDBInstanceOption { + t.Helper() + query["Action"] = "DescribeOrderableDBInstanceOptions" + body, err := Dispatch(t.Context(), "DescribeOrderableDBInstanceOptions", query, nc, testCaller, testEnv) + require.NoError(t, err) + + var out rds.DescribeOrderableDBInstanceOptionsOutput + require.NoError(t, xmlutil.UnmarshalXML(&out, xml.NewDecoder(bytes.NewReader(body)), + "DescribeOrderableDBInstanceOptionsResult")) + return out.OrderableDBInstanceOptions +} + +func engineNames(rows []*rds.DBEngineVersion) []string { + names := make([]string, 0, len(rows)) + for _, row := range rows { + names = append(names, aws.StringValue(row.Engine)) + } + return names +} + +func classNames(options []*rds.OrderableDBInstanceOption) []string { + names := make([]string, 0, len(options)) + for _, option := range options { + names = append(names, aws.StringValue(option.DBInstanceClass)) + } + return names +} + +// AWS nests each row inside its list, and the SDK's own unmarshaler will not +// find one written any other way. +func TestDispatch_DescribeDBEngineVersionsRendersTheNestedList(t *testing.T) { + rows := describeEngineVersions(t, map[string]string{}) + require.Len(t, rows, len(handlers_rds.SupportedEngines())) + assert.Equal(t, handlers_rds.SupportedEngines(), engineNames(rows)) + + for _, row := range rows { + assert.NotEmpty(t, aws.StringValue(row.EngineVersion)) + assert.NotEmpty(t, aws.StringValue(row.DBParameterGroupFamily)) + assert.Equal(t, "available", aws.StringValue(row.Status)) + } +} + +func TestDispatch_DescribeOrderableDBInstanceOptionsRendersTheNestedList(t *testing.T) { + options := describeOrderable(t, newStubbedNATS(t), map[string]string{"Engine": "postgres"}) + require.Len(t, options, len(handlers_rds.SupportedInstanceClasses())) + assert.Equal(t, handlers_rds.SupportedInstanceClasses(), classNames(options)) + + for _, option := range options { + assert.Equal(t, "postgres", aws.StringValue(option.Engine)) + assert.Equal(t, "gp3", aws.StringValue(option.StorageType)) + assert.True(t, aws.BoolValue(option.Vpc)) + assert.Equal(t, []string{"IPV4"}, aws.StringValueSlice(option.SupportedNetworkTypes)) + } +} + +func TestDescribeDBEngineVersions_AppliesTypedParametersAndFilters(t *testing.T) { + cases := []struct { + name string + query map[string]string + want []string + }{ + {"engine parameter", map[string]string{"Engine": "postgres"}, []string{"postgres"}}, + {"engine filter", map[string]string{ + "Filters.Filter.1.Name": "engine", "Filters.Filter.1.Values.Value.1": "mariadb", + }, []string{"mariadb"}}, + {"engine filter with two values", map[string]string{ + "Filters.Filter.1.Name": "engine", + "Filters.Filter.1.Values.Value.1": "postgres", + "Filters.Filter.1.Values.Value.2": "mariadb", + }, handlers_rds.SupportedEngines()}, + {"engine version parameter", map[string]string{"EngineVersion": "18"}, []string{"postgres"}}, + {"parameter group family parameter", + map[string]string{"DBParameterGroupFamily": "mariadb11.8"}, []string{"mariadb"}}, + {"parameter group family filter", map[string]string{ + "Filters.Filter.1.Name": "db-parameter-group-family", "Filters.Filter.1.Values.Value.1": "postgres18", + }, []string{"postgres"}}, + {"status filter", map[string]string{ + "Filters.Filter.1.Name": "status", "Filters.Filter.1.Values.Value.1": "available", + }, handlers_rds.SupportedEngines()}, + // A query for a row that does not exist is empty; only a request to build + // something that cannot exist is refused. + {"unpinned version", map[string]string{"EngineVersion": "17"}, []string{}}, + {"unknown engine", map[string]string{"Engine": "oracle-ee"}, []string{}}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, engineNames(describeEngineVersions(t, tc.query))) + }) + } +} + +func TestDescribeOrderableDBInstanceOptions_AppliesTypedParametersAndFilters(t *testing.T) { + nc := newStubbedNATS(t) + all := handlers_rds.SupportedInstanceClasses() + + cases := []struct { + name string + query map[string]string + want []string + }{ + {"class parameter", map[string]string{"DBInstanceClass": "db.t3.micro"}, []string{"db.t3.micro"}}, + {"class filter", map[string]string{ + "Filters.Filter.1.Name": "db-instance-class", "Filters.Filter.1.Values.Value.1": "db.m5.large", + }, []string{"db.m5.large"}}, + {"engine version parameter", map[string]string{"EngineVersion": "18"}, all}, + {"license model parameter", map[string]string{"LicenseModel": "postgresql-license"}, all}, + {"license model filter", map[string]string{ + "Filters.Filter.1.Name": "license-model", "Filters.Filter.1.Values.Value.1": "general-public-license", + }, []string{}}, + {"vpc true", map[string]string{"Vpc": "true"}, all}, + // Every endpoint is a private VPC address, so there is no non-VPC option. + {"vpc false", map[string]string{"Vpc": "false"}, []string{}}, + {"vpc filter false", map[string]string{ + "Filters.Filter.1.Name": "vpc", "Filters.Filter.1.Values.Value.1": "false", + }, []string{}}, + // The typed parameter takes every spelling ParseBool does, so the filter + // has to agree with it: "1" narrowing to nothing would be indistinguishable + // from a cluster that runs none of the classes. + {"vpc numeric parameter", map[string]string{"Vpc": "1"}, all}, + {"vpc numeric filter", map[string]string{ + "Filters.Filter.1.Name": "vpc", "Filters.Filter.1.Values.Value.1": "1", + }, all}, + {"vpc numeric filter false", map[string]string{ + "Filters.Filter.1.Name": "vpc", "Filters.Filter.1.Values.Value.1": "0", + }, []string{}}, + {"vpc shouted filter", map[string]string{ + "Filters.Filter.1.Name": "vpc", "Filters.Filter.1.Values.Value.1": "TRUE", + }, all}, + {"unpinned version", map[string]string{"EngineVersion": "17"}, []string{}}, + {"unknown class", map[string]string{"DBInstanceClass": "db.r5.24xlarge"}, []string{}}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + tc.query["Engine"] = "postgres" + assert.Equal(t, tc.want, classNames(describeOrderable(t, nc, tc.query))) + }) + } +} + +// The two actions take disjoint typed parameters, so each has its own closed +// filter vocabulary and a name valid on the other one is still rejected. +func TestDescribeCatalogs_RejectAnUnrecognisedFilterName(t *testing.T) { + cases := []struct { + action string + filter string + }{ + {"DescribeDBEngineVersions", "not-a-filter"}, + {"DescribeDBEngineVersions", "db-instance-class"}, + {"DescribeDBEngineVersions", "license-model"}, + {"DescribeDBEngineVersions", "vpc"}, + {"DescribeOrderableDBInstanceOptions", "not-a-filter"}, + {"DescribeOrderableDBInstanceOptions", "db-parameter-group-family"}, + {"DescribeOrderableDBInstanceOptions", "status"}, + } + + nc := newStubbedNATS(t) + for _, tc := range cases { + t.Run(tc.action+"/"+tc.filter, func(t *testing.T) { + _, err := Dispatch(t.Context(), tc.action, map[string]string{ + "Action": tc.action, "Engine": "postgres", + "Filters.Filter.1.Name": tc.filter, "Filters.Filter.1.Values.Value.1": "anything", + }, nc, testCaller, testEnv) + require.Error(t, err) + assert.Contains(t, err.Error(), awserrors.ErrorInvalidParameterValue) + assert.Contains(t, err.Error(), tc.filter) + }) + } +} + +// A value the typed parameter would have refused outright is refused here too, +// rather than narrowing the catalog to nothing and reporting that as the answer. +func TestDescribeOrderableDBInstanceOptions_RejectANonBooleanVpcFilter(t *testing.T) { + for _, value := range []string{"yes", "", "2"} { + t.Run(value, func(t *testing.T) { + _, err := Dispatch(t.Context(), "DescribeOrderableDBInstanceOptions", map[string]string{ + "Action": "DescribeOrderableDBInstanceOptions", "Engine": "postgres", + "Filters.Filter.1.Name": "vpc", "Filters.Filter.1.Values.Value.1": value, + }, newStubbedNATS(t), testCaller, testEnv) + require.Error(t, err) + assert.Contains(t, err.Error(), awserrors.ErrorInvalidParameterValue) + }) + } +} + +func TestDescribeCatalogs_RejectMalformedFilters(t *testing.T) { + cases := []struct { + name string + query map[string]string + }{ + {"no values", map[string]string{"Filters.Filter.1.Name": "engine"}}, + {"no name", map[string]string{"Filters.Filter.1.Values.Value.1": "postgres"}}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + tc.query["Action"] = "DescribeDBEngineVersions" + _, err := Dispatch(t.Context(), "DescribeDBEngineVersions", tc.query, nil, testCaller, testEnv) + require.Error(t, err) + assert.Contains(t, err.Error(), awserrors.ErrorInvalidParameterValue) + }) + } +} + +// Neither action ever issues a Marker, so one in a request was fabricated by the +// caller. This diverges from the other RDS describes, which parse and ignore it, +// because those may one day paginate and these two never will. +func TestDescribeCatalogs_RejectAMarker(t *testing.T) { + nc := newStubbedNATS(t) + for _, action := range []string{"DescribeDBEngineVersions", "DescribeOrderableDBInstanceOptions"} { + t.Run(action, func(t *testing.T) { + _, err := Dispatch(t.Context(), action, map[string]string{ + "Action": action, "Engine": "postgres", "Marker": "page-two", + }, nc, testCaller, testEnv) + require.Error(t, err) + assert.Contains(t, err.Error(), awserrors.ErrorInvalidParameterValue) + assert.Contains(t, err.Error(), "Marker") + }) + } +} + +func TestDescribeOrderableDBInstanceOptions_RequiresAKnownEngine(t *testing.T) { + cases := []struct { + name string + query map[string]string + want string + }{ + {"absent", map[string]string{}, awserrors.ErrorMissingParameter}, + {"blank", map[string]string{"Engine": " "}, awserrors.ErrorMissingParameter}, + {"unknown", map[string]string{"Engine": "oracle-ee"}, awserrors.ErrorInvalidParameterValue}, + {"mysql is not an alias for mariadb", map[string]string{"Engine": "mysql"}, + awserrors.ErrorInvalidParameterValue}, + {"availability zone group", map[string]string{"Engine": "postgres", "AvailabilityZoneGroup": "az-group-1"}, + awserrors.ErrorInvalidParameterValue}, + } + + nc := newStubbedNATS(t) + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + tc.query["Action"] = "DescribeOrderableDBInstanceOptions" + _, err := Dispatch(t.Context(), "DescribeOrderableDBInstanceOptions", tc.query, nc, testCaller, testEnv) + require.Error(t, err) + assert.Contains(t, err.Error(), tc.want) + }) + } +} + +// The four booleans are accepted as provable identities on this catalog rather +// than parsed and dropped, and this is what keeps the claim provable: adding a +// second version per engine, or populating either list, fails here and forces +// the parameter to be implemented rather than left inert. +func TestDescribeDBEngineVersions_TheFourBooleansAreProvableNoOps(t *testing.T) { + // Compared as decoded responses rather than byte-wise: BuildXML emits sibling + // elements in map order, so a row's own members reorder between two calls that + // returned exactly the same rows. + baseline := describeEngineVersions(t, map[string]string{}) + + booleans := []string{"DefaultOnly", "IncludeAll", "ListSupportedCharacterSets", "ListSupportedTimezones"} + for _, name := range booleans { + for _, value := range []string{"true", "false"} { + t.Run(name+"="+value, func(t *testing.T) { + assert.Equal(t, baseline, describeEngineVersions(t, map[string]string{name: value})) + }) + } + } + + // The conditions that make each identity hold, so the assertion above cannot + // stay true by accident once the catalog grows. + rows := baseline + byEngine := map[string]int{} + for _, row := range rows { + byEngine[aws.StringValue(row.Engine)]++ + assert.Equal(t, "available", aws.StringValue(row.Status), + "IncludeAll cannot widen a set only while every version is available") + assert.Empty(t, row.SupportedCharacterSets, + "ListSupportedCharacterSets is an identity only while the list is empty") + assert.Empty(t, row.SupportedTimezones, + "ListSupportedTimezones is an identity only while the list is empty") + } + for engine, count := range byEngine { + assert.Equal(t, 1, count, + "DefaultOnly cannot narrow a set only while %s has exactly one version", engine) + } +} + +// A stubbed cluster that answers nothing must not fall back to the static class +// list: reporting classes that cannot launch is the failure this probe prevents. +func TestDescribeOrderableDBInstanceOptions_ASilentClusterIsServerInternal(t *testing.T) { + _, nc, _ := testutil.StartTestJetStream(t) + + _, err := Dispatch(t.Context(), "DescribeOrderableDBInstanceOptions", map[string]string{ + "Action": "DescribeOrderableDBInstanceOptions", "Engine": "postgres", + }, nc, testCaller, testEnv) + require.Error(t, err) + assert.Equal(t, awserrors.ErrorServerInternal, err.Error()) +} + +// A cluster whose nodes answered but run none of the db.* types is an empty +// option list, which is the truthful answer and not the same case as above. +func TestDescribeOrderableDBInstanceOptions_AnArmOnlyClusterOffersNothing(t *testing.T) { + _, nc, _ := testutil.StartTestJetStream(t) + respondWith(t, nc, "ec2.DescribeInstanceTypes", &ec2.DescribeInstanceTypesOutput{ + InstanceTypes: instanceTypeInfo("t4g.micro", "t4g.small", "m6g.large"), + }) + + assert.Empty(t, describeOrderable(t, nc, map[string]string{"Engine": "postgres"})) +} + +// Only the classes the nodes report are offered, so a class that would fail at +// launch is never in the answer. +func TestDescribeOrderableDBInstanceOptions_OffersOnlyWhatTheNodesRun(t *testing.T) { + _, nc, _ := testutil.StartTestJetStream(t) + respondWith(t, nc, "ec2.DescribeInstanceTypes", &ec2.DescribeInstanceTypesOutput{ + InstanceTypes: instanceTypeInfo("t3.micro", "t3.small", "t4g.micro", "m6g.large"), + }) + + assert.Equal(t, []string{"db.t3.micro", "db.t3.small"}, + classNames(describeOrderable(t, nc, map[string]string{"Engine": "postgres"}))) +} diff --git a/spinifex/gateway/rds/handler.go b/spinifex/gateway/rds/handler.go index de6a4cd1d..e50f9914e 100644 --- a/spinifex/gateway/rds/handler.go +++ b/spinifex/gateway/rds/handler.go @@ -29,12 +29,21 @@ type Caller struct { SessionName string } -type Handler func(ctx context.Context, action string, q map[string]string, nc *nats.Conn, caller Caller) ([]byte, error) +// Cluster facts a handler needs that the request cannot carry. It holds the +// ingredient rather than a ready-made probe, so the next action needing a +// fan-out gets it without another parameter. +type Env struct { + // How many nodes a fan-out waits for before it stops waiting. Without it a + // gather burns its full timeout on every call instead of early-exiting. + ExpectedNodes int +} + +type Handler func(ctx context.Context, action string, q map[string]string, nc *nats.Conn, caller Caller, env Env) ([]byte, error) // Allocates the input struct, parses the query params into it, calls handler and // marshals the output into the IAM-style envelope. -func typed[In any](handler func(context.Context, *In, *nats.Conn, Caller) (any, error)) Handler { - return func(ctx context.Context, action string, q map[string]string, nc *nats.Conn, caller Caller) ([]byte, error) { +func typedEnv[In any](handler func(context.Context, *In, *nats.Conn, Caller, Env) (any, error)) Handler { + return func(ctx context.Context, action string, q map[string]string, nc *nats.Conn, caller Caller, env Env) ([]byte, error) { input := new(In) if err := awsec2query.QueryParamsToStruct(q, input); err != nil { // An over-long indexed list is a client-side malformation, not an @@ -44,7 +53,7 @@ func typed[In any](handler func(context.Context, *In, *nats.Conn, Caller) (any, } return nil, errors.New(awserrors.ErrorInvalidParameterValue) } - output, err := handler(ctx, input, nc, caller) + output, err := handler(ctx, input, nc, caller, env) if err != nil { return nil, err } @@ -57,6 +66,14 @@ func typed[In any](handler func(context.Context, *In, *nats.Conn, Caller) (any, } } +// The same adapter for the majority of actions, which address one account's own +// state and need nothing from the cluster beyond the NATS connection. +func typed[In any](handler func(context.Context, *In, *nats.Conn, Caller) (any, error)) Handler { + return typedEnv(func(ctx context.Context, input *In, nc *nats.Conn, caller Caller, _ Env) (any, error) { + return handler(ctx, input, nc, caller) + }) +} + // One entry per action: what serves it, whether it is agent-only, and which // resource its policy check evaluates against. Both authorization facts live on // the table so a new action cannot be added without deciding either. @@ -118,6 +135,11 @@ var actions = map[string]actionDef{ // Events. The ring is per-account and a filter names no single resource. "DescribeEvents": {handler: typed(DescribeEvents)}, + // Catalogs. Neither reads per-account state, so both evaluate against "*"; + // the orderable one reads cluster capability, which is not tenant data either. + "DescribeDBEngineVersions": {handler: typed(DescribeDBEngineVersions)}, + "DescribeOrderableDBInstanceOptions": {handler: typedEnv(DescribeOrderableDBInstanceOptions)}, + // Internal agent actions, callable only by the in-guest agent's system role. // They share the namespace because the agent reaches the control plane over // SigV4-on-awsgw like every other in-guest agent. @@ -154,7 +176,7 @@ func HasAction(action string) bool { // Callers are expected to have authorized the action already; the unknown-action // check here is a backstop, not the enforcement point. The internal actions // re-run their own gate, so skipping AuthorizeCaller still cannot reach one. -func Dispatch(ctx context.Context, action string, q map[string]string, nc *nats.Conn, caller Caller) ([]byte, error) { +func Dispatch(ctx context.Context, action string, q map[string]string, nc *nats.Conn, caller Caller, env Env) ([]byte, error) { def, ok := actions[action] if !ok { return nil, errors.New(awserrors.ErrorInvalidAction) @@ -163,7 +185,7 @@ func Dispatch(ctx context.Context, action string, q map[string]string, nc *nats. slog.DebugContext(ctx, "RDS: action not available", "action", action) return nil, errors.New(awserrors.ErrorOperationNotSupported) } - return def.handler(ctx, action, q, nc, caller) + return def.handler(ctx, action, q, nc, caller, env) } // ActionNames returns every action the gateway recognises, in stable order. diff --git a/spinifex/gateway/rds/handler_test.go b/spinifex/gateway/rds/handler_test.go index 99c36caa3..afa2cb924 100644 --- a/spinifex/gateway/rds/handler_test.go +++ b/spinifex/gateway/rds/handler_test.go @@ -4,11 +4,13 @@ import ( "bytes" "encoding/json" "encoding/xml" + "maps" "strconv" "testing" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/private/protocol/xml/xmlutil" + "github.com/aws/aws-sdk-go/service/ec2" "github.com/aws/aws-sdk-go/service/rds" "github.com/mulgadc/spinifex/spinifex/awserrors" handlers_rds "github.com/mulgadc/spinifex/spinifex/handlers/rds" @@ -59,6 +61,9 @@ var v1Actions = []string{ "DescribeEvents", + "DescribeDBEngineVersions", + "DescribeOrderableDBInstanceOptions", + "RegisterDBInstance", "SubmitDBStateChange", "PollDBCommands", @@ -100,7 +105,7 @@ func TestHasAction_UnknownAction(t *testing.T) { } func TestDispatch_UnknownAction(t *testing.T) { - _, err := Dispatch(t.Context(), "NotAnRDSAction", nil, nil, testCaller) + _, err := Dispatch(t.Context(), "NotAnRDSAction", nil, nil, testCaller, testEnv) require.Error(t, err) assert.Equal(t, awserrors.ErrorInvalidAction, err.Error()) } @@ -118,7 +123,7 @@ func TestDispatch_MalformedScalarIsInvalidParameterValue(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - _, err := Dispatch(t.Context(), tc.action, tc.query, nil, testCaller) + _, err := Dispatch(t.Context(), tc.action, tc.query, nil, testCaller, testEnv) require.Error(t, err) assert.Equal(t, awserrors.ErrorInvalidParameterValue, err.Error()) }) @@ -131,7 +136,7 @@ func TestDispatch_OversizedListIsMalformedQueryString(t *testing.T) { query["Tags.Tag."+strconv.Itoa(i)+".Key"] = "key" } - _, err := Dispatch(t.Context(), "CreateDBInstance", query, nil, testCaller) + _, err := Dispatch(t.Context(), "CreateDBInstance", query, nil, testCaller, testEnv) require.Error(t, err) assert.Equal(t, awserrors.ErrorMalformedQueryString, err.Error()) } @@ -149,8 +154,25 @@ var liveActions = []string{ "CreateDBSnapshot", "DescribeDBSnapshots", "DeleteDBSnapshot", "RestoreDBInstanceFromDBSnapshot", "DescribeDBInstanceAutomatedBackups", + "DescribeDBEngineVersions", "DescribeOrderableDBInstanceOptions", } +// The parameters an action refuses to run without, so a table-driven dispatch +// reaches the handler rather than its own required-parameter check. +var requiredParams = map[string]map[string]string{ + "DescribeOrderableDBInstanceOptions": {"Engine": "postgres"}, +} + +func dispatchQuery(action string) map[string]string { + q := map[string]string{"Action": action} + maps.Copy(q, requiredParams[action]) + return q +} + +// One node, so the orderable action's capability probe early-exits on the first +// frame instead of paying the gather's full three-second timeout. +var testEnv = Env{ExpectedNodes: 1} + // What is under test in this file is the action table and the XML envelope, not // the orchestration behind the subject, so the responder returns a fixed output. func newStubbedNATS(t *testing.T) *nats.Conn { @@ -188,9 +210,23 @@ func newStubbedNATS(t *testing.T) *nats.Conn { &rds.RestoreDBInstanceFromDBSnapshotOutput{DBInstance: &rds.DBInstance{DBInstanceIdentifier: aws.String("orders-db-restored")}}) respondWith(t, nc, handlers_rds.SubjectDescribeDBInstanceAutomatedBackups, &rds.DescribeDBInstanceAutomatedBackupsOutput{}) + // The orderable catalog's capability probe. Unstubbed it would get no + // ErrNoResponders — Gather publishes and reads an inbox rather than + // requesting — so every dispatch over the table would pay its full timeout. + respondWith(t, nc, "ec2.DescribeInstanceTypes", &ec2.DescribeInstanceTypesOutput{ + InstanceTypes: instanceTypeInfo("t3.micro", "t3.small", "t3.medium", "t3.large", "m5.large", "m5.xlarge"), + }) return nc } +func instanceTypeInfo(names ...string) []*ec2.InstanceTypeInfo { + out := make([]*ec2.InstanceTypeInfo, 0, len(names)) + for _, name := range names { + out = append(out, &ec2.InstanceTypeInfo{InstanceType: aws.String(name)}) + } + return out +} + func respondWith(t *testing.T, nc *nats.Conn, subject string, output any) { t.Helper() payload, err := json.Marshal(output) @@ -211,7 +247,7 @@ func TestDispatch_EveryActionResolves(t *testing.T) { nc := newStubbedNATS(t) for action := range actions { t.Run(action, func(t *testing.T) { - body, err := Dispatch(t.Context(), action, map[string]string{"Action": action}, nc, testCaller) + body, err := Dispatch(t.Context(), action, dispatchQuery(action), nc, testCaller, testEnv) if err == nil { assert.NotEmpty(t, body, "a successful action must return an XML body") return @@ -230,7 +266,7 @@ func TestDispatch_EveryActionResolves(t *testing.T) { func TestDispatch_LiveActionsAreNotPending(t *testing.T) { nc := newStubbedNATS(t) for _, action := range liveActions { - body, err := Dispatch(t.Context(), action, map[string]string{"Action": action}, nc, testCaller) + body, err := Dispatch(t.Context(), action, dispatchQuery(action), nc, testCaller, testEnv) require.NoError(t, err, "action %q", action) assert.Contains(t, string(body), "<"+action+"Result", "action %q", action) } @@ -238,7 +274,7 @@ func TestDispatch_LiveActionsAreNotPending(t *testing.T) { func TestDispatch_OutOfScopeActionIsNotSupported(t *testing.T) { for _, action := range outOfScopeActions { - _, err := Dispatch(t.Context(), action, map[string]string{"Action": action}, nil, testCaller) + _, err := Dispatch(t.Context(), action, map[string]string{"Action": action}, nil, testCaller, testEnv) require.Error(t, err, "action %q", action) assert.Equal(t, awserrors.ErrorOperationNotSupported, err.Error(), "action %q", action) } @@ -246,7 +282,7 @@ func TestDispatch_OutOfScopeActionIsNotSupported(t *testing.T) { func TestDispatch_DescribeDBInstancesReturnsEmptyResultSet(t *testing.T) { body, err := Dispatch(t.Context(), "DescribeDBInstances", - map[string]string{"Action": "DescribeDBInstances", "Version": "2014-10-31"}, newStubbedNATS(t), testCaller) + map[string]string{"Action": "DescribeDBInstances", "Version": "2014-10-31"}, newStubbedNATS(t), testCaller, testEnv) require.NoError(t, err) // The IAM-style envelope the aws-sdk-go query unmarshaler expects, carrying @@ -263,7 +299,7 @@ func TestDispatch_ListTagsForResourceRendersTheNestedTagList(t *testing.T) { body, err := Dispatch(t.Context(), "ListTagsForResource", map[string]string{ "Action": "ListTagsForResource", "ResourceName": handlers_rds.DBInstanceARN("ap-southeast-2", testAccountID, "orders-db"), - }, newStubbedNATS(t), testCaller) + }, newStubbedNATS(t), testCaller, testEnv) require.NoError(t, err) // Decoded the way a real client decodes it, rather than compared byte-wise: @@ -301,7 +337,7 @@ func TestDispatch_AddTagsToResourceParsesTheTagList(t *testing.T) { "ResourceName": arn, "Tags.Tag.1.Key": "env", "Tags.Tag.1.Value": "prod", "Tags.Tag.2.Key": "team", "Tags.Tag.2.Value": "platform", - }, nc, testCaller) + }, nc, testCaller, testEnv) require.NoError(t, err) got := <-requests @@ -337,7 +373,7 @@ func TestDispatch_RemoveTagsFromResourceParsesMemberTagKeys(t *testing.T) { "ResourceName": handlers_rds.DBInstanceARN("ap-southeast-2", testAccountID, "orders-db"), "TagKeys.member.1": "env", "TagKeys.member.2": "team", - }, nc, testCaller) + }, nc, testCaller, testEnv) require.NoError(t, err) assert.Equal(t, []string{"env", "team"}, aws.StringValueSlice((<-requests).TagKeys)) @@ -350,7 +386,7 @@ func TestDispatch_DescribeDBInstancesWithFilters(t *testing.T) { "Action": "DescribeDBInstances", "DBInstanceIdentifier": "orders-db", "MaxRecords": "20", - }, newStubbedNATS(t), testCaller) + }, newStubbedNATS(t), testCaller, testEnv) require.NoError(t, err) assert.Contains(t, string(body), "