fix(acm): refuse to import a certificate whose NotAfter has passed (#1433)

* test: pin AWS-format EKS add-on versions

* feat: return EKS add-on versions in AWS format and accept them as input

* test: DescribeAddonVersions query parameters, compatibilities and paging

* feat: honour DescribeAddonVersions filters and report compatibilities

* test: EKS parameter refusals must carry a code the operation declares

* fix: EKS parameter refusals use a code the operation declares

* docs: describe AWS-format EKS add-on versions

Document the v<x.y.z>-eksbuild.<n> form, the bare-form leniency, the
DescribeAddonVersions filters and compatibilities, and how to pin a
version in Terraform. Replace the leave-addon_version-unset guidance.

contracts/ecs/v1/messages.go

@@ -126,8 +126,8 @@ type Assign struct {
 	ENIPrivateIP  string `json:"eniPrivateIp,omitempty"`
 	ENISubnetID   string `json:"eniSubnetId,omitempty"`
 	// TaskRoleARN is the task's IAM role (from the taskdef); when set, the agent
-	// serves its credentials at 169.254.170.2 under CredID, defaulting CredID to
-	// the taskID when the scheduler leaves it empty.
+	// serves its credentials at 169.254.170.2 under CredID, a scheduler-minted
+	// secret. An assign without a CredID gets no credentials.
 	CredID      string `json:"credId,omitempty"`
 	TaskRoleARN string `json:"taskRoleArn,omitempty"`
 	// ExecutionRoleARN is the task's execution role (from the taskdef); when set,

docs/containers/eks/README.md

@@ -143,6 +143,25 @@ No user-facing operation replaces a control plane, and automatic replacement doe
 Recreate the cluster to adopt it.
 User-workload injection through a cluster created with an older image is not measured.
 
+### Add-on versions
+
+Add-on versions use the AWS form `v<x.y.z>-eksbuild.<n>`, for example `v1.40.1-eksbuild.1`.
+`DescribeAddonVersions` returns versions in this form, newest first, and every other add-on response reports the version the same way.
+Spinifex ships one EKS build of each upstream version, so `<n>` is `1` today.
+
+`DescribeAddonVersions` honours `addonName`, `kubernetesVersion`, `types`, `publishers`, `owners`, `maxResults` and `nextToken`.
+Each version carries `compatibilities` with the `clusterVersion`, the `platformVersions`, and a `defaultVersion` flag that is true for exactly one version per add-on and cluster version.
+A `kubernetesVersion` that Spinifex does not run returns no add-ons.
+
+`CreateAddon` and `UpdateAddon` accept the AWS form of a version in the catalogue.
+They also accept the bare upstream form, such as `1.40.1`, and Spinifex logs each use at Info.
+This is a Spinifex leniency that AWS does not offer, and it exists so that older scripts keep working.
+Any other value is refused with a message that names `addonVersion` and lists the supported versions, for example `v1.40.1-eksbuild.2`, `1.40` or `v1.40.1`.
+
+In Terraform, pass the AWS form to `aws_eks_addon.addon_version`, either as a literal or from `data.aws_eks_addon_version`.
+The AWS provider validates the format before it calls the API, so it rejects the bare form.
+Leaving `addon_version` unset still installs the catalogue default.
+
 ## Prerequisites
 
 > [!IMPORTANT]
@@ -359,7 +378,7 @@ Control plane readiness and kubectl authentication above lists which clusters re
 
 **`create-cluster` rejected with an authentication-mode error.** `CONFIG_MAP` is the one mode Spinifex rejects, because it has no `aws-auth` ConfigMap path. Set `authentication_mode` to `API` or `API_AND_CONFIG_MAP`, or leave it unset, and grant access with access entries.
 
-**Addon install fails.** Only addons bundled into the registered `eks-node` image install successfully. If you pin an addon version in Terraform, leave `addon_version` unset so Spinifex selects the catalog default.
+**Addon install fails.** Only addons bundled into the registered `eks-node` image install successfully. If you pin an addon version in Terraform, use the AWS form from `DescribeAddonVersions` (see Add-on versions above), or leave `addon_version` unset so Spinifex selects the catalog default.
 
 ## Reference Architectures
 

docs/package-boundary/eks-addon-lifecycle.md

@@ -20,7 +20,7 @@ There is no generation, so the record mixes desired fields with the latest obser
 | Durable record | `clusters/{cluster}/addons/{addon}` holds `Record` (`domains/eks/addon/record.go`): `addonName`, `addonVersion`, `status`, `serviceAccountRoleArn`, `configurationValues`, `health`, `arn`, `tags`, `createdAt`, `modifiedAt` (the optional ones `omitempty`). `clusters/{cluster}/addons/{addon}/manifest` holds `Manifest` (`domains/eks/addon/record.go`): `addonName`, `addonVersion`, `serviceAccountRoleArn`, `configurationValues`. Listing skips the manifest sub-key by segment count. | `TestAddonPersistedLayout_RecordAndManifestKeysAndFieldNames`, `TestAddonRecord_DecodesHandWrittenJSON`, `domains/eks/addon/store_test.go` `TestRecord_RoundTrip`, `TestList_SkipsManifestSubKeys` |
 | Writers | Record: the owner in `domains/eks/addon/owner.go`, reached from the `CreateAddon`, `UpdateAddon` and `DeleteAddon` handlers, the cluster reconciler's report relay and the GPU node group launch: `Owner.Create` (unconditional put), `Owner.Update` and its failure marking (revision CAS), `Owner.Delete` (get then delete), `Owner.ApplyReport` (revision CAS), `Owner.EnsureGPUDevicePlugin` through `Owner.Create`; and the cluster prefix purge (`DeleteClusterChildren`). Manifest: `StagingInstaller.Install` (unconditional put on create and update), `StagingInstaller.Uninstall` (`domains/eks/addon/delivery.go`), `Owner.Delete` and the prefix purge. | `TestAddonPersistedLayout_RecordAndManifestKeysAndFieldNames`, `TestUpdateAddon_ReportsSuccessfulWhileRecordIsUpdating`, `TestDeleteAddon_ReturnsDeletingAfterErasingRecordAndManifest`, `TestDeleteClusterChildren_SweepsAddonRecordsAndManifestsScopedToCluster`, `domains/eks/addon/store_test.go` `TestCasUpdate`, `TestDeleteRecord_RemovesRecordAndManifest` |
 | Readers | `ListAddons`, `DescribeAddon`, `CreateAddon` (existence check) and the reconciler read the record. `ListStagedAddonManifests` reads every `/manifest` key for the cluster (`addon.ListManifests`); it is served on `eks.ListStagedAddonManifests` (`daemon/daemon.go`) and reached only through the gateway's internal-addons route. | `handlers/eks/addons_test.go` `TestAddon_DescribeListDelete`, `TestListStagedAddonManifests` |
-| Catalogue | Static in-binary catalogue (`domains/eks/addon/catalog.go`): `aws-load-balancer-controller`, `argocd`, `aws-ebs-csi-driver`, and hidden-but-creatable `nvidia-device-plugin` and `spinifex-noop`; one version each, the first being the default. Create and update reject unknown names and versions with `InvalidParameterValue`. | `domains/eks/addon/catalog_test.go`, `handlers/eks/addons_test.go` `TestDescribeAddonVersions_ReturnsCatalog`, `TestCreateAddon_UnknownAddonRejected`, `TestCreateAddon_UnknownVersionRejected`, `TestUpdateAddon_UnknownVersionRejected` |
+| Catalogue | Static in-binary catalogue (`domains/eks/addon/catalog.go`): `aws-load-balancer-controller`, `argocd`, `aws-ebs-csi-driver`, and hidden-but-creatable `nvidia-device-plugin` and `spinifex-noop`; one version each, the first being the default. Versions are stored and used internally in the bare upstream form, and API responses show the AWS form `v<x.y.z>-eksbuild.<n>` (`Spec.AWSVersion`). Create and update accept the AWS form or the exact bare form and reject unknown names and other version strings with `InvalidParameterValue`, which the gateway returns as `InvalidParameterException`. | `domains/eks/addon/catalog_test.go`, `handlers/eks/addons_test.go` `TestDescribeAddonVersions_ReturnsCatalog`, `TestCreateAddon_UnknownAddonRejected`, `TestCreateAddon_UnknownVersionRejected`, `TestUpdateAddon_UnknownVersionRejected` |
 | Desired state | Version, `serviceAccountRoleArn`, `configurationValues` and create-time tags. `resolveConflicts`, `podIdentityAssociations` and `clientRequestToken` are accepted and dropped; `preserve` on delete is ignored. `configurationValues` reaches the guest but the agent discards it (`mulga-eks-addon-sync.sh` `sync_once`), so it has no effect. | `TestAddonPersistedLayout_RecordAndManifestKeysAndFieldNames`, `TestDeleteAddon_ReturnsDeletingAfterErasingRecordAndManifest` |
 | Observed state | `status` and `health` are overwritten by guest reports on the same record as the desired fields. `health` is the latest failure message, projected as one `AddonIssue` with only `message`. Statuses held: `CREATING`, `ACTIVE`, `UPDATING`, `DEGRADED`, `CREATE_FAILED`; `DELETING` is defined but never stored. | `TestAddonRecord_DecodesHandWrittenJSON`, `handlers/eks/addon_status_reconciler_test.go` `TestApplyAddonStatusReport` |
 | Generation | None. Reports carry `version` and `ts`, and neither is read: any report for the add-on name applies to whatever is currently desired. | `TestAddonStatusReport_MismatchedVersionIsAccepted` |

docs/terraform-workbooks/eks-gitops-argocd/README.md

@@ -79,7 +79,8 @@ Terraform manages the cluster, the addons, the ACM cert, and the HTTPS Ingress;
 - The `argocd` and `aws-ebs-csi-driver` bundles must be baked into the `eks-node` AMI — `describe-addon` returns `no baked bundle` otherwise.
 - The EBS-CSI default StorageClass uses `provisioner: ebs.csi.aws.com`; a PVC against it provisions a **Viperblock-backed EBS volume**.
 - If the PVC stays `Pending`, the app still serves (the counter falls back to in-memory) — see [Troubleshooting](#troubleshooting).
-- Leave addon versions unset (the AWS provider and the catalog disagree on the version-string format).
+- Addon versions are unset, so Spinifex installs each catalog default.
+  To pin one, use the AWS form that `DescribeAddonVersions` returns, such as `v3.0.23-eksbuild.1`.
 - The Argo CD `Application` is a `kubernetes_manifest`, so the `argoproj.io` CRDs must exist before `apply` — apply the parent and let the addon reach `ACTIVE` first.
 
 **Prerequisites:**

docs/terraform-workbooks/eks-gitops-argocd/main.tf

@@ -520,8 +520,8 @@ resource "aws_eks_node_group" "workers" {
 #
 # Spinifex wires the controller's AWS credentials and ELB-eligible subnets at the
 # node level, so no IRSA role or subnet tagging is needed here. addon_version is
-# omitted: the AWS provider demands a v-prefixed version that the catalog rejects,
-# so let Spinifex default to its catalog version.
+# omitted so Spinifex installs its catalog default. To pin one, use the AWS form
+# from DescribeAddonVersions, for example v2.11.0-eksbuild.1.
 # ---------------------------------------------------------------------------
 
 resource "aws_eks_addon" "lbc" {
@@ -542,7 +542,7 @@ resource "aws_eks_addon" "lbc" {
 # Installed through the EKS API; Spinifex stages the bundle host-side and the
 # worker renders it into the K3s auto-deploy dir. The workloads module registers
 # the git repo and creates the Argo CD Application that syncs the demo app.
-# addon_version omitted (see the LBC note).
+# addon_version omitted: the catalog default is installed (see the LBC note).
 # ---------------------------------------------------------------------------
 
 resource "aws_eks_addon" "argocd" {
@@ -562,7 +562,7 @@ resource "aws_eks_addon" "argocd" {
 #
 # Ships a default gp3 StorageClass (provisioner ebs.csi.aws.com). A PVC in the
 # demo app's git manifests dynamically provisions a Viperblock-backed EBS volume
-# so the app's state survives pod restarts. addon_version omitted (see LBC note).
+# so the app's state survives pod restarts. addon_version omitted: catalog default.
 # ---------------------------------------------------------------------------
 
 resource "aws_eks_addon" "ebs_csi" {

docs/terraform-workbooks/eks-https-ingress/README.md

@@ -74,7 +74,8 @@ The LBC is installed as a managed **addon** (`aws_eks_addon`). Spinifex wires th
 - **`spinifex.io/managed-ingress = "false"`** disables the K3s built-in traefik/servicelb so the LBC owns ingress, matching AWS parity. Omit the tag (or set it `true`) and the built-in ingress stays on.
 - The ALB targets the workers' **NodePort** (`target-type: instance`). The one SG rule admits the VPC CIDR on that port so the in-VPC ALB can reach the workers; the worker SG is otherwise auto-managed.
 - Spinifex ACM supports **ImportCertificate only** — the cert is generated by the `tls` provider and imported, not requested.
-- **Leave `addon_version` unset** (see the [quickstart/addons notes](/docs/eks-quickstart)) — the AWS provider and the Spinifex catalog disagree on the version string format, so let the server choose.
+- **`addon_version` is unset**, so Spinifex installs its catalog default.
+  To pin a version, use the AWS form that `DescribeAddonVersions` returns, such as `v2.11.0-eksbuild.1`, or read it from `data.aws_eks_addon_version`.
 
 **Prerequisites:**
 

docs/terraform-workbooks/eks-https-ingress/main.tf

@@ -446,8 +446,8 @@ resource "aws_eks_node_group" "workers" {
 #
 # Spinifex wires the controller's AWS credentials and ELB-eligible subnets at the
 # node level, so no IRSA role or subnet tagging is needed here. addon_version is
-# omitted: the AWS provider demands a v-prefixed version that the catalog rejects,
-# so let Spinifex default to its catalog version.
+# omitted so Spinifex installs its catalog default. To pin one, use the AWS form
+# from DescribeAddonVersions, for example v2.11.0-eksbuild.1.
 # ---------------------------------------------------------------------------
 
 resource "aws_eks_addon" "lbc" {

spinifex/agents/ecs/agent/credendpoint.go

@@ -47,6 +47,13 @@ type credResponse struct {
 
 // credEntry caches one task's assumed-role credentials alongside the role it was
 // assumed from.
+// credRole is the task role a credID serves and the task it belongs to; the
+// task ID names the STS session, since the credID is a secret.
+type credRole struct {
+	roleARN string
+	taskID  string
+}
+
 type credEntry struct {
 	roleARN string
 	creds   stsauth.Credentials
@@ -71,7 +78,7 @@ type credEndpoint struct {
 	assume func(ctx context.Context, roleARN, sessionName string) (stsauth.Credentials, error)
 
 	mu         sync.Mutex
-	roles      map[string]string    // credID -> taskRoleARN
+	roles      map[string]credRole  // credID -> task role
 	cache      map[string]credEntry // credID -> cached creds
 	httpClient *http.Client
 
@@ -98,7 +105,7 @@ func newCredEndpoint(provider credentials.CredentialsProvider, region, gatewayUR
 		port:       port,
 		proxyPort:  defaultCredProxyPort,
 		run:        run,
-		roles:      map[string]string{},
+		roles:      map[string]credRole{},
 		cache:      map[string]credEntry{},
 	}
 	c.assume = c.assumeOverGateway
@@ -107,12 +114,12 @@ func newCredEndpoint(provider credentials.CredentialsProvider, region, gatewayUR
 
 // Register associates a credID with the task role it serves; Deregister removes
 // it and drops any cached credentials. Both are no-ops for an empty credID.
-func (c *credEndpoint) Register(credID, roleARN string) {
+func (c *credEndpoint) Register(credID, taskID, roleARN string) {
 	if credID == "" || roleARN == "" {
 		return
 	}
 	c.mu.Lock()
-	c.roles[credID] = roleARN
+	c.roles[credID] = credRole{roleARN: roleARN, taskID: taskID}
 	c.mu.Unlock()
 }
 
@@ -142,25 +149,25 @@ func (c *credEndpoint) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, "not found", http.StatusNotFound)
 		return
 	}
-	creds, roleARN, err := c.fetch(r.Context(), credID)
+	creds, role, err := c.fetch(r.Context(), credID)
 	if err != nil {
 		if errors.Is(err, errUnknownCredID) {
 			http.Error(w, "not found", http.StatusNotFound)
 			return
 		}
-		slog.Error("ecs-agent: assume task role failed", "credId", credID, "err", err)
+		slog.Error("ecs-agent: assume task role failed", "task", role.taskID, "role", role.roleARN, "err", err)
 		http.Error(w, "credentials unavailable", http.StatusBadGateway)
 		return
 	}
 	w.Header().Set("Content-Type", "application/json")
 	if err := json.NewEncoder(w).Encode(credResponse{
-		RoleArn:         roleARN,
+		RoleArn:         role.roleARN,
 		AccessKeyID:     creds.AccessKeyID,
 		SecretAccessKey: creds.SecretAccessKey,
 		Token:           creds.SessionToken,
 		Expiration:      creds.Expiration.UTC().Format(time.RFC3339),
 	}); err != nil {
-		slog.Warn("ecs-agent: encode credential response", "credId", credID, "err", err)
+		slog.Warn("ecs-agent: encode credential response", "task", role.taskID, "err", err)
 	}
 }
 
@@ -169,28 +176,28 @@ var errUnknownCredID = fmt.Errorf("unknown credId")
 
 // fetch returns valid credentials for credID, reusing the cache until within the
 // refresh margin and otherwise assuming the task role over the gateway.
-func (c *credEndpoint) fetch(ctx context.Context, credID string) (stsauth.Credentials, string, error) {
+func (c *credEndpoint) fetch(ctx context.Context, credID string) (stsauth.Credentials, credRole, error) {
 	c.mu.Lock()
-	roleARN, known := c.roles[credID]
+	role, known := c.roles[credID]
 	if !known {
 		c.mu.Unlock()
-		return stsauth.Credentials{}, "", errUnknownCredID
+		return stsauth.Credentials{}, credRole{}, errUnknownCredID
 	}
-	if e, ok := c.cache[credID]; ok && e.roleARN == roleARN && credValid(e.creds, credRefreshMargin) {
+	if e, ok := c.cache[credID]; ok && e.roleARN == role.roleARN && credValid(e.creds, credRefreshMargin) {
 		c.mu.Unlock()
-		return e.creds, roleARN, nil
+		return e.creds, role, nil
 	}
 	c.mu.Unlock()
 
-	creds, err := c.assume(ctx, roleARN, sessionName(credID))
+	creds, err := c.assume(ctx, role.roleARN, sessionName(role.taskID))
 	if err != nil {
-		return stsauth.Credentials{}, "", err
+		return stsauth.Credentials{}, role, err
 	}
 
 	c.mu.Lock()
-	c.cache[credID] = credEntry{roleARN: roleARN, creds: creds}
+	c.cache[credID] = credEntry{roleARN: role.roleARN, creds: creds}
 	c.mu.Unlock()
-	return creds, roleARN, nil
+	return creds, role, nil
 }
 
 // AssumeProvider returns a CredentialsProvider that mints credentials by assuming
@@ -390,10 +397,10 @@ func credValid(c stsauth.Credentials, margin time.Duration) bool {
 	return time.Now().Add(margin).Before(c.Expiration)
 }
 
-// sessionName derives an AssumeRole RoleSessionName from credID, satisfying the
+// sessionName derives an AssumeRole RoleSessionName from the task ID, satisfying the
 // STS [\w+=,.@-]{2,64} constraint.
-func sessionName(credID string) string {
-	name := "ecs-" + credID
+func sessionName(taskID string) string {
+	name := "ecs-" + taskID
 	if len(name) > 64 {
 		name = name[:64]
 	}

spinifex/agents/ecs/agent/credendpoint_test.go

@@ -29,7 +29,7 @@ func TestCredEndpoint_ServesRegisteredCredID(t *testing.T) {
 	c, _ := newTestCredEndpoint(stsauth.Credentials{
 		AccessKeyID: "AKIA", SecretAccessKey: "secret", SessionToken: "tok", Expiration: exp,
 	})
-	c.Register("cred-1", "arn:aws:iam::111122223333:role/task")
+	c.Register("cred-1", "task-1", "arn:aws:iam::111122223333:role/task")
 
 	rr := httptest.NewRecorder()
 	c.ServeHTTP(rr, httptest.NewRequest(http.MethodGet, "/v2/credentials/cred-1", nil))
@@ -51,6 +51,26 @@ func TestCredEndpoint_ServesRegisteredCredID(t *testing.T) {
 	}
 }
 
+// The STS session name lands in the assumed-role ARN and in STS logs, so it is
+// built from the public task ID and never from the secret credID.
+func TestCredEndpoint_SessionNameIsTaskIDNotCredID(t *testing.T) {
+	c, _ := newTestCredEndpoint(stsauth.Credentials{})
+	var gotSession string
+	c.assume = func(ctx context.Context, roleARN, session string) (stsauth.Credentials, error) {
+		gotSession = session
+		return stsauth.Credentials{AccessKeyID: "AKIA", SecretAccessKey: "secret"}, nil
+	}
+	c.Register("secret-cred", "task-1", "arn:role")
+	rr := httptest.NewRecorder()
+	c.ServeHTTP(rr, httptest.NewRequest(http.MethodGet, "/v2/credentials/secret-cred", nil))
+	if rr.Code != http.StatusOK {
+		t.Fatalf("status = %d, want 200", rr.Code)
+	}
+	if gotSession != "ecs-task-1" {
+		t.Errorf("session name = %q, want ecs-task-1", gotSession)
+	}
+}
+
 func TestCredEndpoint_UnknownCredIDIs404(t *testing.T) {
 	c, calls := newTestCredEndpoint(stsauth.Credentials{AccessKeyID: "x", SecretAccessKey: "y"})
 	rr := httptest.NewRecorder()
@@ -65,7 +85,7 @@ func TestCredEndpoint_UnknownCredIDIs404(t *testing.T) {
 
 func TestCredEndpoint_WrongMethodIs405(t *testing.T) {
 	c, _ := newTestCredEndpoint(stsauth.Credentials{AccessKeyID: "x", SecretAccessKey: "y"})
-	c.Register("cred-1", "arn:role")
+	c.Register("cred-1", "task-1", "arn:role")
 	rr := httptest.NewRecorder()
 	c.ServeHTTP(rr, httptest.NewRequest(http.MethodPost, "/v2/credentials/cred-1", nil))
 	if rr.Code != http.StatusMethodNotAllowed {
@@ -77,7 +97,7 @@ func TestCredEndpoint_CachesUntilExpiry(t *testing.T) {
 	c, calls := newTestCredEndpoint(stsauth.Credentials{
 		AccessKeyID: "AKIA", SecretAccessKey: "secret", Expiration: time.Now().Add(time.Hour),
 	})
-	c.Register("cred-1", "arn:role")
+	c.Register("cred-1", "task-1", "arn:role")
 	for hit := range 3 {
 		rr := httptest.NewRecorder()
 		c.ServeHTTP(rr, httptest.NewRequest(http.MethodGet, "/v2/credentials/cred-1", nil))
@@ -95,7 +115,7 @@ func TestCredEndpoint_RefreshesNearExpiry(t *testing.T) {
 	c, calls := newTestCredEndpoint(stsauth.Credentials{
 		AccessKeyID: "AKIA", SecretAccessKey: "secret", Expiration: time.Now().Add(time.Minute),
 	})
-	c.Register("cred-1", "arn:role")
+	c.Register("cred-1", "task-1", "arn:role")
 	for range 2 {
 		rr := httptest.NewRecorder()
 		c.ServeHTTP(rr, httptest.NewRequest(http.MethodGet, "/v2/credentials/cred-1", nil))
@@ -107,7 +127,7 @@ func TestCredEndpoint_RefreshesNearExpiry(t *testing.T) {
 
 func TestCredEndpoint_DeregisterDropsCredID(t *testing.T) {
 	c, _ := newTestCredEndpoint(stsauth.Credentials{AccessKeyID: "x", SecretAccessKey: "y"})
-	c.Register("cred-1", "arn:role")
+	c.Register("cred-1", "task-1", "arn:role")
 	c.Deregister("cred-1")
 	rr := httptest.NewRecorder()
 	c.ServeHTTP(rr, httptest.NewRequest(http.MethodGet, "/v2/credentials/cred-1", nil))
@@ -121,7 +141,7 @@ func TestCredEndpoint_AssumeFailureIs502(t *testing.T) {
 	c.assume = func(ctx context.Context, roleARN, sessionName string) (stsauth.Credentials, error) {
 		return stsauth.Credentials{}, context.DeadlineExceeded
 	}
-	c.Register("cred-1", "arn:role")
+	c.Register("cred-1", "task-1", "arn:role")
 	rr := httptest.NewRecorder()
 	c.ServeHTTP(rr, httptest.NewRequest(http.MethodGet, "/v2/credentials/cred-1", nil))
 	if rr.Code != http.StatusBadGateway {
@@ -133,7 +153,7 @@ func TestCredEndpoint_StartServesOverLoopback(t *testing.T) {
 	c, _ := newTestCredEndpoint(stsauth.Credentials{
 		AccessKeyID: "AKIA", SecretAccessKey: "secret", Expiration: time.Now().Add(time.Hour),
 	})
-	c.Register("cred-1", "arn:role")
+	c.Register("cred-1", "task-1", "arn:role")
 	c.port = 0 // bind an ephemeral port instead of privileged 80
 	if err := c.Start(); err != nil {
 		t.Fatalf("start: %v", err)

spinifex/agents/ecs/agent/credinject_test.go

@@ -1,8 +1,12 @@
 package agent
 
 import (
+	"net/http"
+	"net/http/httptest"
 	"testing"
 
+	"github.com/mulgadc/spinifex/internal/stsauth"
+
 	ecsv1 "github.com/mulgadc/spinifex/contracts/ecs/v1"
 )
 
@@ -13,7 +17,7 @@ func TestTaskCredID(t *testing.T) {
 		want string
 	}{
 		{"no role", ecsv1.Assign{TaskID: "t1"}, ""},
-		{"role, no credID -> taskID", ecsv1.Assign{TaskID: "t1", TaskRoleARN: "arn:role"}, "t1"},
+		{"role, no credID -> none", ecsv1.Assign{TaskID: "t1", TaskRoleARN: "arn:role"}, ""},
 		{"role + credID", ecsv1.Assign{TaskID: "t1", CredID: "c9", TaskRoleARN: "arn:role"}, "c9"},
 	}
 	for _, tc := range cases {
@@ -23,6 +27,37 @@ func TestTaskCredID(t *testing.T) {
 	}
 }
 
+// The task ID is public (it is the task ARN suffix), so naming another task's ID
+// at the credential endpoint must not yield that task's role.
+func TestRegisterTaskCreds_TaskIDDoesNotServeCredentials(t *testing.T) {
+	c, _ := newTestCredEndpoint(stsauth.Credentials{AccessKeyID: "AKIA", SecretAccessKey: "secret"})
+	a := Agent{cred: c}
+	get := func(id string) int {
+		rr := httptest.NewRecorder()
+		c.ServeHTTP(rr, httptest.NewRequest(http.MethodGet, credRelativeURI(id), nil))
+		return rr.Code
+	}
+
+	legacy := ecsv1.Assign{TaskID: "victim-task", TaskRoleARN: "arn:aws:iam::000000000010:role/high"}
+	if got := a.registerTaskCreds(&legacy); got != "" {
+		t.Errorf("assign without CredID registered credID %q", got)
+	}
+	if code := get("victim-task"); code != http.StatusNotFound {
+		t.Errorf("GET by task ID of CredID-less assign = %d, want 404", code)
+	}
+
+	minted := ecsv1.Assign{TaskID: "victim-2", CredID: "secret-cred", TaskRoleARN: "arn:aws:iam::000000000010:role/high"}
+	if got := a.registerTaskCreds(&minted); got != "secret-cred" {
+		t.Fatalf("registerTaskCreds = %q, want secret-cred", got)
+	}
+	if code := get("victim-2"); code != http.StatusNotFound {
+		t.Errorf("GET by task ID = %d, want 404", code)
+	}
+	if code := get("secret-cred"); code != http.StatusOK {
+		t.Errorf("GET by CredID = %d, want 200", code)
+	}
+}
+
 func TestWithCredEnv(t *testing.T) {
 	base := map[string]string{"FOO": "bar"}
 

spinifex/agents/ecs/agent/reconcile_test.go

@@ -63,7 +63,7 @@ func TestReconcile_AdoptsRunningLabeledContainer(t *testing.T) {
 	if !adopted["t-001"] {
 		t.Fatalf("t-001 not adopted; got %v", adopted)
 	}
-	if got := a.cred.roles["t-001"]; got != testTaskRole {
+	if got := a.cred.roles["t-001"].roleARN; got != testTaskRole {
 		t.Errorf("cred not re-registered: roles[t-001] = %q, want %q", got, testTaskRole)
 	}
 	if n := countStatus(cp.taskStates(), "t-001", ecsv1.TaskStatusRunning); n != 1 {

spinifex/agents/ecs/agent/taskrun.go

@@ -306,24 +306,24 @@ func (a *Agent) pullResolver(as *ecsv1.Assign) ctrruntime.Resolver {
 // credential endpoint and returns the credID (empty when the task has no role).
 func (a *Agent) registerTaskCreds(as *ecsv1.Assign) string {
 	credID := taskCredID(as)
+	if credID == "" && as.TaskRoleARN != "" {
+		slog.Warn("ecs-agent: task has a role but no credential ID, starting it without credentials", "task", as.TaskID, "role", as.TaskRoleARN)
+	}
 	if credID == "" || a.cred == nil {
 		return credID
 	}
-	a.cred.Register(credID, as.TaskRoleARN)
+	a.cred.Register(credID, as.TaskID, as.TaskRoleARN)
 	return credID
 }
 
 // taskCredID is the credential ID a task's containers fetch credentials under:
-// the scheduler-assigned CredID, falling back to the taskID. Empty when the task
-// carries no IAM role.
+// the scheduler-minted secret CredID. Empty when the task carries no IAM role or
+// no CredID; the public task ID is never used, since any API caller can learn it.
 func taskCredID(as *ecsv1.Assign) string {
 	if as.TaskRoleARN == "" {
 		return ""
 	}
-	if as.CredID != "" {
-		return as.CredID
-	}
-	return as.TaskID
+	return as.CredID
 }
 
 // withCredEnv returns env with AWS_CONTAINER_CREDENTIALS_RELATIVE_URI set for

spinifex/domains/acm/keyalgorithm_test.go

@@ -152,3 +152,30 @@ func TestImportCertificate_SerialZeroRejected(t *testing.T) {
 	assert.Equal(t, awserrors.ErrorValidationException, code)
 	assert.Equal(t, "The serial number in the certificate is not supported by ACM.", msg)
 }
+
+func TestImportCertificate_ExpiredRejected(t *testing.T) {
+	t.Parallel()
+	svc := setupACMService(t)
+	key, err := rsa.GenerateKey(rand.Reader, 1024)
+	require.NoError(t, err)
+	tmpl := &x509.Certificate{
+		SerialNumber: big.NewInt(time.Now().UnixNano()),
+		Subject:      pkix.Name{CommonName: "expired.example.com"},
+		NotBefore:    time.Now().Add(-48 * time.Hour),
+		NotAfter:     time.Now().Add(-24 * time.Hour),
+	}
+	der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key)
+	require.NoError(t, err)
+	_, err = svc.ImportCertificate(context.Background(), &acm.ImportCertificateInput{
+		Certificate: pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}),
+		PrivateKey:  pem.EncodeToMemory(&pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(key)}),
+	}, testAccountID)
+	code, msg, ok := awserrors.ResolveErrorDetail(err)
+	require.True(t, ok)
+	assert.Equal(t, awserrors.ErrorValidationException, code)
+	assert.Equal(t, "The certificate is expired. You can Import a certificate only during its validity period.", msg)
+
+	list, err := svc.ListCertificates(context.Background(), &acm.ListCertificatesInput{}, testAccountID)
+	require.NoError(t, err)
+	assert.Empty(t, list.CertificateSummaryList)
+}

spinifex/domains/acm/service_impl.go

@@ -223,10 +223,13 @@ func (s *ACMServiceImpl) ImportCertificate(ctx context.Context, input *acm.Impor
 	if leaf.SerialNumber.Sign() == 0 {
 		return nil, awserrors.Errorf(awserrors.ErrorValidationException, "The serial number in the certificate is not supported by ACM.")
 	}
-
 	certArn := aws.StringValue(input.CertificateArn)
-	var inUseBy []string
 	now := time.Now().UTC()
+	if certArn == "" && now.After(leaf.NotAfter) {
+		return nil, awserrors.Errorf(awserrors.ErrorValidationException, "The certificate is expired. You can Import a certificate only during its validity period.")
+	}
+
+	var inUseBy []string
 	createdAt := now
 	tags := tagsToMap(input.Tags)
 	reimport := certArn != ""

spinifex/domains/ec2/awsapi/actions.go

@@ -134,6 +134,9 @@ func marshalEC2Response(action string, output any) ([]byte, error) {
 
 var ec2Actions = map[string]ec2Action{
 	"DescribeInstances": ec2Handler(func(ctx context.Context, input *ec2.DescribeInstancesInput, c *call, accountID string) (any, error) {
+		if err := ec2instanceapi.ValidateDescribeInstancesInput(input); err != nil {
+			return nil, err
+		}
 		out, err := ec2instanceapi.DescribeInstancesChecked(ctx, input, c.NATS, 0, c.NodeIDs, accountID)
 		if err != nil {
 			return out, err
@@ -257,6 +260,9 @@ var ec2Actions = map[string]ec2Action{
 		}
 	}),
 	"DescribeImages": ec2Handler(func(ctx context.Context, input *ec2.DescribeImagesInput, c *call, accountID string) (any, error) {
+		if err := ec2imageapi.ValidateDescribeImagesInput(input); err != nil {
+			return nil, err
+		}
 		return ec2imageapi.DescribeImages(ctx, input, c.NATS, accountID)
 	}),
 	"CreateImage": ec2Handler(func(ctx context.Context, input *ec2.CreateImageInput, c *call, accountID string) (any, error) {
@@ -287,6 +293,9 @@ var ec2Actions = map[string]ec2Action{
 		return ec2zoneapi.DescribeAvailabilityZones(input, c.Region, c.AZ)
 	}),
 	"DescribeVolumes": ec2Handler(func(ctx context.Context, input *ec2.DescribeVolumesInput, c *call, accountID string) (any, error) {
+		if err := ec2volumeapi.ValidateDescribeVolumesInput(input); err != nil {
+			return nil, err
+		}
 		return ec2volumeapi.DescribeVolumes(ctx, input, c.NATS, accountID)
 	}),
 	"ModifyVolume": ec2Handler(func(ctx context.Context, input *ec2.ModifyVolumeInput, c *call, accountID string) (any, error) {

spinifex/domains/ec2/awsapi/actions_test.go

@@ -6,6 +6,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/mulgadc/spinifex/spinifex/foundation/aws/errors"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -30,3 +31,27 @@ func TestImportKeyPair_Base64PaddingWorkaround(t *testing.T) {
 	assert.NotContains(t, q["PublicKeyMaterial"], "%3D",
 		"Expected no URL-encoded padding remaining")
 }
+
+// A well-prefixed ID of the wrong shape is refused before any service is
+// asked, so it can never come back as NotFound.
+func TestDescribe_MalformedIDRejectedBeforeLookup(t *testing.T) {
+	tests := []struct {
+		action, param, id, code, msg string
+	}{
+		{"DescribeInstances", "InstanceId.1", "i-zzz", awserrors.ErrorInvalidInstanceIDMalformed, `Invalid id: "i-zzz"`},
+		{"DescribeImages", "ImageId.1", "ami-zzz", awserrors.ErrorInvalidAMIIDMalformed, `Invalid id: "ami-zzz"`},
+		{"DescribeVolumes", "VolumeId.1", "vol-zzz", awserrors.ErrorInvalidParameterValue, "Value (vol-zzz) for parameter volumes is invalid. Expected: 'vol-...'."},
+	}
+	for _, tt := range tests {
+		t.Run(tt.action, func(t *testing.T) {
+			h := ec2Actions[tt.action]
+			input, err := h.parse(map[string]string{tt.param: tt.id})
+			require.NoError(t, err)
+			_, err = h.dispatch(t.Context(), tt.action, input, &call{Handler: NewHandler(Deps{})}, "123456789012")
+			code, msg, ok := awserrors.ResolveErrorDetail(err)
+			require.True(t, ok)
+			assert.Equal(t, tt.code, code)
+			assert.Equal(t, tt.msg, msg)
+		})
+	}
+}

spinifex/domains/ec2/awsapi/image/DescribeImages.go

@@ -5,6 +5,7 @@ package image
 import (
 	"context"
 	"errors"
+	"regexp"
 	"strings"
 
 	"github.com/aws/aws-sdk-go/service/ec2"
@@ -13,8 +14,12 @@ import (
 	"github.com/nats-io/nats.go"
 )
 
-// ValidateDescribeImagesInput accepts a nil input and rejects any ImageIds entry without the ami-
-// prefix with InvalidAMIID.Malformed.
+// imageIDSuffixPattern is the AWS AMI ID shape after ami-: 8 (legacy) or 17
+// lowercase hex characters.
+var imageIDSuffixPattern = regexp.MustCompile(`^([0-9a-f]{8}|[0-9a-f]{17})$`)
+
+// ValidateDescribeImagesInput accepts a nil input and rejects any ImageIds entry that is not
+// ami- followed by 8 or 17 lowercase hex characters with InvalidAMIID.Malformed.
 func ValidateDescribeImagesInput(input *ec2.DescribeImagesInput) (err error) {
 	if input == nil {
 		return nil
@@ -22,9 +27,16 @@ func ValidateDescribeImagesInput(input *ec2.DescribeImagesInput) (err error) {
 
 	if input.ImageIds != nil {
 		for _, imageId := range input.ImageIds {
-			if imageId != nil && !strings.HasPrefix(*imageId, "ami-") {
+			if imageId == nil {
+				continue
+			}
+			suffix, ok := strings.CutPrefix(*imageId, "ami-")
+			if !ok {
 				return errors.New(awserrors.ErrorInvalidAMIIDMalformed)
 			}
+			if !imageIDSuffixPattern.MatchString(suffix) {
+				return awserrors.Errorf(awserrors.ErrorInvalidAMIIDMalformed, "Invalid id: %q", *imageId)
+			}
 		}
 	}
 
@@ -34,11 +46,6 @@ func ValidateDescribeImagesInput(input *ec2.DescribeImagesInput) (err error) {
 // DescribeImages implements the EC2 DescribeImages action, returning the AMIs visible to
 // accountID from the NATS image service.
 func DescribeImages(ctx context.Context, input *ec2.DescribeImagesInput, natsConn *nats.Conn, accountID string) (output ec2.DescribeImagesOutput, err error) {
-	err = ValidateDescribeImagesInput(input)
-	if err != nil {
-		return output, err
-	}
-
 	imageService := ec2image.NewNATSImageService(natsConn, 0)
 	result, err := imageService.DescribeImages(ctx, input, accountID)
 	if err != nil {

spinifex/domains/ec2/awsapi/image/DescribeImages_test.go

@@ -53,8 +53,8 @@ func TestValidateDescribeImagesInput(t *testing.T) {
 			name: "MultipleValidImageIds",
 			input: &ec2.DescribeImagesInput{
 				ImageIds: []*string{
-					aws.String("ami-111"),
-					aws.String("ami-222"),
+					aws.String("ami-1a2b3c4d"),
+					aws.String("ami-0123456789abcdef1"),
 				},
 			},
 			wantErr: false,
@@ -63,7 +63,7 @@ func TestValidateDescribeImagesInput(t *testing.T) {
 			name: "MixedValidAndInvalidImageIds",
 			input: &ec2.DescribeImagesInput{
 				ImageIds: []*string{
-					aws.String("ami-valid"),
+					aws.String("ami-0123456789abcdef0"),
 					aws.String("invalid-id"),
 				},
 			},
@@ -85,11 +85,24 @@ func TestValidateDescribeImagesInput(t *testing.T) {
 			err := ValidateDescribeImagesInput(tt.input)
 
 			if tt.wantErr {
-				assert.Error(t, err)
-				assert.Equal(t, tt.errMsg, err.Error())
+				code, ok := awserrors.ResolveErrorCode(err)
+				assert.True(t, ok)
+				assert.Equal(t, tt.errMsg, code)
 			} else {
 				assert.NoError(t, err)
 			}
 		})
 	}
 }
+
+// AWS answers a well-prefixed ID whose suffix is not 8 or 17 lowercase hex
+// characters with Malformed, not NotFound.
+func TestValidateDescribeImagesInput_MalformedSuffix(t *testing.T) {
+	for _, id := range []string{"ami-zzz", "ami-0123456789ABCDEF0", "ami-0123456789abcdef", "ami-"} {
+		err := ValidateDescribeImagesInput(&ec2.DescribeImagesInput{ImageIds: []*string{aws.String(id)}})
+		code, msg, ok := awserrors.ResolveErrorDetail(err)
+		assert.True(t, ok, id)
+		assert.Equal(t, awserrors.ErrorInvalidAMIIDMalformed, code, id)
+		assert.Equal(t, `Invalid id: "`+id+`"`, msg, id)
+	}
+}

spinifex/domains/ec2/awsapi/image/handler_test.go

@@ -39,13 +39,6 @@ func TestCreateImage_NilNATS(t *testing.T) {
 	assert.Error(t, err)
 }
 
-func TestDescribeImages_ValidationErrors(t *testing.T) {
-	_, err := DescribeImages(context.Background(), &ec2.DescribeImagesInput{
-		ImageIds: []*string{aws.String("bad-id")},
-	}, nil, "")
-	assert.EqualError(t, err, awserrors.ErrorInvalidAMIIDMalformed)
-}
-
 func TestDescribeImages_NilNATS(t *testing.T) {
 	_, err := DescribeImages(context.Background(), nil, nil, "acct-123")
 	assert.Error(t, err)

spinifex/domains/ec2/awsapi/instance/DescribeInstances.go

@@ -89,6 +89,12 @@ func DescribeInstances(ctx context.Context, input *ec2.DescribeInstancesInput, n
 	return &ec2.DescribeInstancesOutput{Reservations: reservations}, nil
 }
 
+// ValidateDescribeInstancesInput rejects any InstanceIds entry that is not a
+// well-formed instance ID with InvalidInstanceID.Malformed.
+func ValidateDescribeInstancesInput(input *ec2.DescribeInstancesInput) error {
+	return validateInstanceIDFormat(input.InstanceIds)
+}
+
 // DescribeInstancesChecked is the customer-facing variant served by the
 // gateway's DescribeInstances action. On top of DescribeInstances' aggregation,
 // it asserts InvalidInstanceID.NotFound for any explicitly named instance ID

spinifex/domains/ec2/awsapi/instance/instanceid.go

@@ -25,7 +25,7 @@ var instanceIDPattern = regexp.MustCompile(`^i-([0-9a-f]{8}|[0-9a-f]{17})$`)
 func validateInstanceIDFormat(ids []*string) error {
 	for _, id := range ids {
 		if id != nil && !instanceIDPattern.MatchString(*id) {
-			return errors.New(awserrors.ErrorInvalidInstanceIDMalformed)
+			return awserrors.Errorf(awserrors.ErrorInvalidInstanceIDMalformed, "Invalid id: %q", *id)
 		}
 	}
 	return nil

spinifex/domains/ec2/awsapi/instance/instanceid_test.go

@@ -57,8 +57,10 @@ func TestValidateInstanceIDFormat(t *testing.T) {
 	}
 	for _, id := range malformed {
 		err := validateInstanceIDFormat([]*string{aws.String(id)})
-		require.Error(t, err, id)
-		assert.Equal(t, awserrors.ErrorInvalidInstanceIDMalformed, err.Error(), id)
+		code, msg, ok := awserrors.ResolveErrorDetail(err)
+		require.True(t, ok, id)
+		assert.Equal(t, awserrors.ErrorInvalidInstanceIDMalformed, code, id)
+		assert.Equal(t, `Invalid id: "`+id+`"`, msg, id)
 	}
 	err := validateInstanceIDFormat([]*string{aws.String("i-0123456789abcdef0"), aws.String("i-abc")})
 	require.Error(t, err, "one malformed id fails the whole batch")
@@ -100,8 +102,9 @@ func TestLifecycleVerbs_MalformedIDRejectedBeforeNATS(t *testing.T) {
 	}
 	for name, call := range calls {
 		err := call()
-		require.Error(t, err, name)
-		assert.Equal(t, awserrors.ErrorInvalidInstanceIDMalformed, err.Error(), name)
+		code, ok := awserrors.ResolveErrorCode(err)
+		require.True(t, ok, name)
+		assert.Equal(t, awserrors.ErrorInvalidInstanceIDMalformed, code, name)
 	}
 	require.NoError(t, nc.Flush())
 	assert.Zero(t, requests.Load(), "no NATS request may be sent for a batch holding a malformed id")

spinifex/domains/ec2/awsapi/volume/DescribeVolumes.go

@@ -5,6 +5,7 @@ package volume
 import (
 	"context"
 	"errors"
+	"regexp"
 	"strings"
 
 	"github.com/aws/aws-sdk-go/service/ec2"
@@ -13,8 +14,13 @@ import (
 	"github.com/nats-io/nats.go"
 )
 
-// ValidateDescribeVolumesInput accepts a nil input and rejects any VolumeIds entry without the
-// vol- prefix with InvalidVolumeID.Malformed.
+// volumeIDSuffixPattern is the AWS volume ID shape after vol-: 8 (legacy) or
+// 17 lowercase hex characters.
+var volumeIDSuffixPattern = regexp.MustCompile(`^([0-9a-f]{8}|[0-9a-f]{17})$`)
+
+// ValidateDescribeVolumesInput accepts a nil input, rejects any VolumeIds entry without the
+// vol- prefix with InvalidVolumeID.Malformed, and any with a malformed suffix with
+// InvalidParameterValue, as AWS does.
 func ValidateDescribeVolumesInput(input *ec2.DescribeVolumesInput) error {
 	if input == nil {
 		return nil
@@ -22,9 +28,17 @@ func ValidateDescribeVolumesInput(input *ec2.DescribeVolumesInput) error {
 
 	if input.VolumeIds != nil {
 		for _, volumeId := range input.VolumeIds {
-			if volumeId != nil && !strings.HasPrefix(*volumeId, "vol-") {
+			if volumeId == nil {
+				continue
+			}
+			suffix, ok := strings.CutPrefix(*volumeId, "vol-")
+			if !ok {
 				return errors.New(awserrors.ErrorInvalidVolumeIDMalformed)
 			}
+			if !volumeIDSuffixPattern.MatchString(suffix) {
+				return awserrors.Errorf(awserrors.ErrorInvalidParameterValue,
+					"Value (%s) for parameter volumes is invalid. Expected: 'vol-...'.", *volumeId)
+			}
 		}
 	}
 
@@ -35,11 +49,6 @@ func ValidateDescribeVolumesInput(input *ec2.DescribeVolumesInput) error {
 func DescribeVolumes(ctx context.Context, input *ec2.DescribeVolumesInput, natsConn *nats.Conn, accountID string) (ec2.DescribeVolumesOutput, error) {
 	var output ec2.DescribeVolumesOutput
 
-	err := ValidateDescribeVolumesInput(input)
-	if err != nil {
-		return output, err
-	}
-
 	volumeService := ec2volume.NewNATSVolumeService(natsConn)
 	result, err := volumeService.DescribeVolumes(ctx, input, accountID)
 	if err != nil {

spinifex/domains/ec2/awsapi/volume/DescribeVolumes_test.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/ec2"
+	"github.com/mulgadc/spinifex/spinifex/foundation/aws/errors"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -36,9 +37,9 @@ func TestValidateDescribeVolumesInput(t *testing.T) {
 			name: "ValidMultipleVolumeIds",
 			input: &ec2.DescribeVolumesInput{
 				VolumeIds: []*string{
-					aws.String("vol-abc123"),
-					aws.String("vol-def456"),
-					aws.String("vol-ghi789"),
+					aws.String("vol-0123456789abcdef1"),
+					aws.String("vol-1a2b3c4d"),
+					aws.String("vol-0123456789abcdef2"),
 				},
 			},
 			wantErr: false,
@@ -71,7 +72,7 @@ func TestValidateDescribeVolumesInput(t *testing.T) {
 			name: "MixedValidAndInvalid",
 			input: &ec2.DescribeVolumesInput{
 				VolumeIds: []*string{
-					aws.String("vol-valid123"),
+					aws.String("vol-0123456789abcdef3"),
 					aws.String("invalid-id"),
 				},
 			},
@@ -81,7 +82,7 @@ func TestValidateDescribeVolumesInput(t *testing.T) {
 		{
 			name: "NilVolumeIdInList",
 			input: &ec2.DescribeVolumesInput{
-				VolumeIds: []*string{nil, aws.String("vol-abc123")},
+				VolumeIds: []*string{nil, aws.String("vol-0123456789abcdef1")},
 			},
 			wantErr: false,
 		},
@@ -100,3 +101,15 @@ func TestValidateDescribeVolumesInput(t *testing.T) {
 		})
 	}
 }
+
+// AWS answers a well-prefixed ID whose suffix is not 8 or 17 lowercase hex
+// characters with InvalidParameterValue, not NotFound.
+func TestValidateDescribeVolumesInput_MalformedSuffix(t *testing.T) {
+	for _, id := range []string{"vol-zzz", "vol-0123456789ABCDEF0", "vol-0123456789abcdef", "vol-"} {
+		err := ValidateDescribeVolumesInput(&ec2.DescribeVolumesInput{VolumeIds: []*string{aws.String(id)}})
+		code, msg, ok := awserrors.ResolveErrorDetail(err)
+		assert.True(t, ok, id)
+		assert.Equal(t, awserrors.ErrorInvalidParameterValue, code, id)
+		assert.Equal(t, "Value ("+id+") for parameter volumes is invalid. Expected: 'vol-...'.", msg, id)
+	}
+}

spinifex/domains/ec2/awsapi/volume/handler_test.go

@@ -96,13 +96,6 @@ func TestModifyVolume_NilNATS(t *testing.T) {
 	assert.Error(t, err)
 }
 
-func TestDescribeVolumes_ValidationErrors(t *testing.T) {
-	_, err := DescribeVolumes(context.Background(), &ec2.DescribeVolumesInput{
-		VolumeIds: []*string{aws.String("bad-id")},
-	}, nil, "")
-	assert.EqualError(t, err, awserrors.ErrorInvalidVolumeIDMalformed)
-}
-
 func TestDescribeVolumes_NilNATS(t *testing.T) {
 	_, err := DescribeVolumes(context.Background(), nil, nil, "acct-123")
 	assert.Error(t, err)

spinifex/domains/ec2/vpc/eni.go

@@ -224,7 +224,7 @@ func (s *VPCServiceImpl) CreateNetworkInterface(ctx context.Context, input *ec2.
 	groupNames := make(map[string]string)
 	s.fillSGNames(ctx, accountID, record.SecurityGroupIds, groupNames)
 	return &ec2.CreateNetworkInterfaceOutput{
-		NetworkInterface: s.eniRecordToEC2(&record, accountID, groupNames),
+		NetworkInterface: s.eniRecordToEC2(&record, accountID, groupNames, s.vpcDNSHostnames(ctx, accountID, record.VpcId)),
 	}, nil
 }
 
@@ -595,7 +595,7 @@ func (s *VPCServiceImpl) DescribeNetworkInterfaces(ctx context.Context, input *e
 		}
 
 		s.fillSGNames(ctx, accountID, record.SecurityGroupIds, groupNames)
-		enis = append(enis, s.eniRecordToEC2(&record, accountID, groupNames))
+		enis = append(enis, s.eniRecordToEC2(&record, accountID, groupNames, s.vpcDNSHostnames(ctx, accountID, record.VpcId)))
 	}
 
 	// If specific ENI IDs were requested but not found, return error
@@ -1076,9 +1076,26 @@ func (s *VPCServiceImpl) fillSGNames(ctx context.Context, accountID string, ids
 	}
 }
 
+// vpcDNSHostnames reports whether the VPC has enableDnsHostnames on. An
+// unreadable VPC reads as off, so its interfaces go unnamed.
+func (s *VPCServiceImpl) vpcDNSHostnames(ctx context.Context, accountID, vpcID string) bool {
+	entry, err := s.vpcKV.Get(ctx, kvutil.AccountKey(accountID, vpcID))
+	if err != nil {
+		slog.WarnContext(ctx, "vpcDNSHostnames: VPC read failed", "vpcId", vpcID, "err", err)
+		return false
+	}
+	var rec VPCRecord
+	if err := json.Unmarshal(entry.Value(), &rec); err != nil {
+		slog.WarnContext(ctx, "vpcDNSHostnames: VPC unmarshal failed", "vpcId", vpcID, "err", err)
+		return false
+	}
+	return rec.EnableDnsHostnames
+}
+
 // eniRecordToEC2 converts an ENI record to an EC2 NetworkInterface, naming
-// each security group from groupNames.
-func (s *VPCServiceImpl) eniRecordToEC2(record *ENIRecord, accountID string, groupNames map[string]string) *ec2.NetworkInterface {
+// each security group from groupNames. AWS names the interface only while its
+// VPC has enableDnsHostnames on.
+func (s *VPCServiceImpl) eniRecordToEC2(record *ENIRecord, accountID string, groupNames map[string]string, dnsHostnames bool) *ec2.NetworkInterface {
 	// ENIs with spinifex:managed-by tag are system-managed (e.g. by ELBv2)
 	requesterManaged := record.Tags["spinifex:managed-by"] != ""
 
@@ -1116,7 +1133,7 @@ func (s *VPCServiceImpl) eniRecordToEC2(record *ENIRecord, accountID string, gro
 		eni.Groups = groups
 	}
 
-	if s.privateDNSName != nil && record.PrivateIpAddress != "" {
+	if dnsHostnames && s.privateDNSName != nil && record.PrivateIpAddress != "" {
 		if name := s.privateDNSName(record.PrivateIpAddress); name != "" {
 			eni.PrivateDnsName = aws.String(name)
 			eni.PrivateIpAddresses[0].PrivateDnsName = aws.String(name)

spinifex/domains/ec2/vpc/eni_test.go

@@ -55,15 +55,11 @@ func TestNetworkInterface_NamesGroupsAndPrivateAddress(t *testing.T) {
 	vpcId := createTestVPC(t, svc, "10.0.0.0/16")
 	subnetId := createTestSubnet(t, svc, vpcId, "10.0.1.0/24")
 	sgId := createTestSG(t, svc, vpcId, "web")
+	setHostnames(t, svc, vpcId, true)
 
 	// Without a namer an interface carries no private name.
 	unnamed := createTestENI(t, svc, subnetId)
-	desc, err := svc.DescribeNetworkInterfaces(context.Background(), &ec2.DescribeNetworkInterfacesInput{
-		NetworkInterfaceIds: []*string{aws.String(unnamed)},
-	}, testAccountID)
-	require.NoError(t, err)
-	require.Len(t, desc.NetworkInterfaces, 1)
-	assert.Nil(t, desc.NetworkInterfaces[0].PrivateDnsName)
+	assert.Nil(t, describeTestENI(t, svc, unnamed).PrivateDnsName)
 
 	svc.SetPrivateDNSNamer(func(ip string) string { return "name-of-" + ip })
 	out, err := svc.CreateNetworkInterface(context.Background(), &ec2.CreateNetworkInterfaceInput{
@@ -72,13 +68,8 @@ func TestNetworkInterface_NamesGroupsAndPrivateAddress(t *testing.T) {
 	}, testAccountID)
 	require.NoError(t, err)
 
-	desc, err = svc.DescribeNetworkInterfaces(context.Background(), &ec2.DescribeNetworkInterfacesInput{
-		NetworkInterfaceIds: []*string{out.NetworkInterface.NetworkInterfaceId},
-	}, testAccountID)
-	require.NoError(t, err)
-	require.Len(t, desc.NetworkInterfaces, 1)
-
-	for name, eni := range map[string]*ec2.NetworkInterface{"create": out.NetworkInterface, "describe": desc.NetworkInterfaces[0]} {
+	desc := describeTestENI(t, svc, *out.NetworkInterface.NetworkInterfaceId)
+	for name, eni := range map[string]*ec2.NetworkInterface{"create": out.NetworkInterface, "describe": desc} {
 		want := "name-of-" + aws.StringValue(eni.PrivateIpAddress)
 		assert.Equal(t, want, aws.StringValue(eni.PrivateDnsName), name)
 		require.Len(t, eni.PrivateIpAddresses, 1, name)
@@ -89,6 +80,58 @@ func TestNetworkInterface_NamesGroupsAndPrivateAddress(t *testing.T) {
 	}
 }
 
+func TestNetworkInterface_PrivateDnsNameFollowsVpcDnsHostnames(t *testing.T) {
+	t.Parallel()
+	svc := setupTestVPCService(t)
+	svc.SetPrivateDNSNamer(func(ip string) string { return "name-of-" + ip })
+	vpcId := createTestVPC(t, svc, "10.0.0.0/16")
+	subnetId := createTestSubnet(t, svc, vpcId, "10.0.1.0/24")
+
+	// A new VPC has enableDnsHostnames off, so its interfaces go unnamed.
+	out, err := svc.CreateNetworkInterface(context.Background(), &ec2.CreateNetworkInterfaceInput{
+		SubnetId: aws.String(subnetId),
+	}, testAccountID)
+	require.NoError(t, err)
+	eniId := *out.NetworkInterface.NetworkInterfaceId
+	for name, eni := range map[string]*ec2.NetworkInterface{"create": out.NetworkInterface, "describe": describeTestENI(t, svc, eniId)} {
+		assert.Nil(t, eni.PrivateDnsName, name)
+		require.Len(t, eni.PrivateIpAddresses, 1, name)
+		assert.Nil(t, eni.PrivateIpAddresses[0].PrivateDnsName, name)
+	}
+
+	// Turning the flag on names the existing interface.
+	setHostnames(t, svc, vpcId, true)
+	eni := describeTestENI(t, svc, eniId)
+	want := "name-of-" + aws.StringValue(eni.PrivateIpAddress)
+	assert.Equal(t, want, aws.StringValue(eni.PrivateDnsName))
+	assert.Equal(t, want, aws.StringValue(eni.PrivateIpAddresses[0].PrivateDnsName))
+
+	// And turning it off again drops the name.
+	setHostnames(t, svc, vpcId, false)
+	eni = describeTestENI(t, svc, eniId)
+	assert.Nil(t, eni.PrivateDnsName)
+	assert.Nil(t, eni.PrivateIpAddresses[0].PrivateDnsName)
+}
+
+func setHostnames(t *testing.T, svc *VPCServiceImpl, vpcId string, on bool) {
+	t.Helper()
+	_, err := svc.ModifyVpcAttribute(context.Background(), &ec2.ModifyVpcAttributeInput{
+		VpcId:              aws.String(vpcId),
+		EnableDnsHostnames: &ec2.AttributeBooleanValue{Value: aws.Bool(on)},
+	}, testAccountID)
+	require.NoError(t, err)
+}
+
+func describeTestENI(t *testing.T, svc *VPCServiceImpl, eniId string) *ec2.NetworkInterface {
+	t.Helper()
+	desc, err := svc.DescribeNetworkInterfaces(context.Background(), &ec2.DescribeNetworkInterfacesInput{
+		NetworkInterfaceIds: []*string{aws.String(eniId)},
+	}, testAccountID)
+	require.NoError(t, err)
+	require.Len(t, desc.NetworkInterfaces, 1)
+	return desc.NetworkInterfaces[0]
+}
+
 func TestCreateNetworkInterface_SequentialIPs(t *testing.T) {
 	t.Parallel()
 	svc := setupTestVPCService(t)

spinifex/domains/ecs/records.go

@@ -272,11 +272,15 @@ type TaskRecord struct {
 	// Group / StartedBy mirror the AWS task fields. A service's tasks carry
 	// Group="service:{name}" so the reconciler counts them and the task-state
 	// hook resolves a RUNNING/STOPPED task back to its owning service.
-	Group                string `json:"group,omitempty"`
-	StartedBy            string `json:"startedBy,omitempty"`
-	TaskDefFamily        string `json:"taskDefFamily"`
-	TaskDefRevision      int    `json:"taskDefRevision"`
-	TaskDefARN           string `json:"taskDefArn"`
+	Group           string `json:"group,omitempty"`
+	StartedBy       string `json:"startedBy,omitempty"`
+	TaskDefFamily   string `json:"taskDefFamily"`
+	TaskDefRevision int    `json:"taskDefRevision"`
+	TaskDefARN      string `json:"taskDefArn"`
+	// CredID is the secret under which the task's containers fetch task-role
+	// credentials from the agent. Set only when the task has a role; it must
+	// never be projected into an API response or logged.
+	CredID               string `json:"credId,omitempty"`
 	ContainerInstanceID  string `json:"containerInstanceId,omitempty"`
 	ContainerInstanceARN string `json:"containerInstanceArn,omitempty"`
 	DesiredStatus        string `json:"desiredStatus"`

spinifex/domains/ecs/service.go

@@ -288,7 +288,7 @@ func (s *Service) DescribeClusters(ctx context.Context, input *ecs.DescribeClust
 		names = []string{defaultCluster}
 	}
 	includeSettings := slices.Contains(awsStringSlice(input.Include), ecs.ClusterFieldSettings)
-	out := &ecs.DescribeClustersOutput{}
+	out := &ecs.DescribeClustersOutput{Clusters: []*ecs.Cluster{}, Failures: []*ecs.Failure{}}
 	for _, name := range names {
 		name = ClusterShortName(name)
 		var rec ClusterRecord
@@ -458,7 +458,7 @@ func (s *Service) RegisterTaskDefinition(ctx context.Context, input *ecs.Registe
 	if family == "" {
 		return nil, errors.New(awserrors.ErrorECSInvalidParameter)
 	}
-	if err := validateContainerDefs(input.ContainerDefinitions); err != nil {
+	if err := validateContainerDefs(input.ContainerDefinitions, aws.StringValue(input.Memory)); err != nil {
 		return nil, err
 	}
 	warnUnsupportedLogDrivers(ctx, family, input.ContainerDefinitions)
@@ -497,12 +497,23 @@ func (s *Service) RegisterTaskDefinition(ctx context.Context, input *ecs.Registe
 // The test here is per value, not per field: an empty list or a false/unset
 // flag requests nothing and is accepted, because there is nothing to enforce
 // or refuse. Only a value that asks for the restriction is rejected.
-func validateContainerDefs(defs []*ecs.ContainerDefinition) error {
+//
+// Ahead of those, AWS's own rules: every container needs an image, and without
+// a task-level memory every container needs memory or memoryReservation.
+func validateContainerDefs(defs []*ecs.ContainerDefinition, taskMemory string) error {
 	for _, c := range defs {
 		if c == nil {
 			continue
 		}
 		name := aws.StringValue(c.Name)
+		if aws.StringValue(c.Image) == "" {
+			return awserrors.Errorf(awserrors.ErrorECSClientException,
+				"Container.image should not be null or empty.")
+		}
+		if taskMemory == "" && c.Memory == nil && c.MemoryReservation == nil {
+			return awserrors.Errorf(awserrors.ErrorECSClientException,
+				"Invalid setting for container '%s'. At least one of 'memory' or 'memoryReservation' must be specified.", name)
+		}
 		if len(c.Secrets) > 0 {
 			return errors.New(awserrors.ErrorECSInvalidParameter)
 		}

spinifex/domains/ecs/service_test.go

@@ -2,7 +2,9 @@ package ecs
 
 import (
 	"context"
+	"encoding/json"
 	"github.com/mulgadc/spinifex/spinifex/domains/ecs/taskdefinition"
+	"maps"
 	"strconv"
 	"testing"
 	"time"
@@ -302,6 +304,89 @@ func TestService_RunTask_AssignCarriesTaskRole(t *testing.T) {
 	assert.Equal(t, roleARN, poll.Assignments[0].TaskRoleARN)
 }
 
+// A role task's credential ID is a scheduler-minted secret: not the public task
+// ID, distinct per task, stable on re-delivery, persisted on the task record and
+// absent from every API response. A role-less task gets none.
+func TestService_TaskCredIDIsMintedSecret(t *testing.T) {
+	ctx := context.Background()
+	svc, _ := newTestService(t)
+	_, err := svc.CreateCluster(ctx, &ecs.CreateClusterInput{ClusterName: aws.String("web")}, testAccountID)
+	require.NoError(t, err)
+	_, err = svc.RegisterTaskDefinition(ctx, &ecs.RegisterTaskDefinitionInput{
+		Family:      aws.String("victim"),
+		TaskRoleArn: aws.String("arn:aws:iam::123456789012:role/task-high"),
+		ContainerDefinitions: []*ecs.ContainerDefinition{{
+			Name: aws.String("app"), Image: aws.String("registry/app:1"),
+			Cpu: aws.Int64(128), Memory: aws.Int64(256), Essential: aws.Bool(true),
+		}},
+	}, testAccountID)
+	require.NoError(t, err)
+	registerTaskDef(t, svc, "attacker", 128, 256)
+	registerInstance(t, svc, "web", "i-1", 1024, 2048)
+
+	run, err := svc.RunTask(ctx, &ecs.RunTaskInput{Cluster: aws.String("web"), TaskDefinition: aws.String("victim")}, testAccountID)
+	require.NoError(t, err)
+	start, err := svc.StartTask(ctx, &ecs.StartTaskInput{
+		Cluster: aws.String("web"), TaskDefinition: aws.String("victim"), ContainerInstances: []*string{aws.String("i-1")},
+	}, testAccountID)
+	require.NoError(t, err)
+	_, err = svc.StartTask(ctx, &ecs.StartTaskInput{
+		Cluster: aws.String("web"), TaskDefinition: aws.String("attacker"), ContainerInstances: []*string{aws.String("i-1")},
+	}, testAccountID)
+	require.NoError(t, err)
+
+	poll := func() map[string]ecsv1.Assign {
+		p, perr := svc.PollAssignments(ctx, &PollAssignmentsInput{Cluster: "web", ContainerInstance: "i-1"}, testAccountID)
+		require.NoError(t, perr)
+		byTask := map[string]ecsv1.Assign{}
+		for _, as := range p.Assignments {
+			byTask[as.TaskID] = as
+		}
+		return byTask
+	}
+	first, second := poll(), poll()
+	require.Len(t, first, 3)
+
+	kv, err := svc.bucket(ctx, testAccountID)
+	require.NoError(t, err)
+	var apiJSON []byte
+	for _, out := range []any{run, start} {
+		b, merr := json.Marshal(out)
+		require.NoError(t, merr)
+		apiJSON = append(apiJSON, b...)
+	}
+	var taskARNs []*string
+	for as := range maps.Values(first) {
+		taskARNs = append(taskARNs, aws.String(as.TaskARN))
+	}
+	desc, err := svc.DescribeTasks(ctx, &ecs.DescribeTasksInput{Cluster: aws.String("web"), Tasks: taskARNs}, testAccountID)
+	require.NoError(t, err)
+	b, err := json.Marshal(desc)
+	require.NoError(t, err)
+	apiJSON = append(apiJSON, b...)
+
+	seen := map[string]bool{}
+	for taskID, as := range first {
+		if as.TaskRoleARN == "" {
+			assert.Empty(t, as.CredID, "role-less task got a credential ID")
+			continue
+		}
+		require.NotEmpty(t, as.CredID)
+		assert.NotEqual(t, taskID, as.CredID)
+		assert.False(t, seen[as.CredID], "credential ID reused across tasks")
+		seen[as.CredID] = true
+		assert.Equal(t, as.CredID, second[taskID].CredID, "re-delivered assign changed credential ID")
+
+		var rec TaskRecord
+		found, gerr := getJSON(ctx, kv, TaskKey("web", taskID), &rec)
+		require.NoError(t, gerr)
+		require.True(t, found)
+		assert.Equal(t, as.CredID, rec.CredID)
+		assert.NotContains(t, string(apiJSON), as.CredID, "credential ID leaked in an API response")
+	}
+	assert.Len(t, seen, 2)
+}
+
 // PollAssignments is at-least-once: re-poll without ack re-delivers the assign;
 // acking it (then STOPPED) drains the inbox so it is never re-delivered.
 func TestService_PollAssignments_AckAndReclaim(t *testing.T) {
@@ -812,6 +897,12 @@ func TestService_ListTasks_EmptyIsPresentNotAbsent(t *testing.T) {
 	assert.Empty(t, out.TaskArns)
 }
 
+func TestService_ListTasks_ClusterNotFound(t *testing.T) {
+	svc, _ := newTestService(t)
+	_, err := svc.ListTasks(context.Background(), &ecs.ListTasksInput{Cluster: aws.String("ghost")}, testAccountID)
+	require.EqualError(t, err, awserrors.ErrorECSClusterNotFound)
+}
+
 func TestService_ListContainerInstances_EmptyIsPresentNotAbsent(t *testing.T) {
 	svc, _ := newTestService(t)
 	_, err := svc.CreateCluster(context.Background(), &ecs.CreateClusterInput{ClusterName: aws.String("web")}, testAccountID)
@@ -829,3 +920,21 @@ func TestAccountIDFromBucket(t *testing.T) {
 	_, ok = accountIDFromBucket("some-other-bucket")
 	assert.False(t, ok)
 }
+
+func TestService_DescribeClusters_ListsAlwaysPresent(t *testing.T) {
+	svc, _ := newTestService(t)
+	_, err := svc.CreateCluster(context.Background(), &ecs.CreateClusterInput{ClusterName: aws.String("web")}, testAccountID)
+	require.NoError(t, err)
+
+	found, err := svc.DescribeClusters(context.Background(), &ecs.DescribeClustersInput{Clusters: []*string{aws.String("web")}}, testAccountID)
+	require.NoError(t, err)
+	require.Len(t, found.Clusters, 1)
+	assert.NotNil(t, found.Failures)
+	assert.Empty(t, found.Failures)
+
+	miss, err := svc.DescribeClusters(context.Background(), &ecs.DescribeClustersInput{Clusters: []*string{aws.String("nope")}}, testAccountID)
+	require.NoError(t, err)
+	require.Len(t, miss.Failures, 1)
+	assert.NotNil(t, miss.Clusters)
+	assert.Empty(t, miss.Clusters)
+}

spinifex/domains/ecs/services_test.go

@@ -313,7 +313,7 @@ func TestService_CreateService_ProjectsNetworkConfigurationAndManagedTags(t *tes
 		Family:      aws.String("app"),
 		NetworkMode: aws.String(NetworkModeAwsvpc),
 		ContainerDefinitions: []*ecs.ContainerDefinition{{
-			Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true),
+			Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true), Memory: aws.Int64(256),
 		}},
 	}, testAccountID)
 	require.NoError(t, err)
@@ -352,7 +352,7 @@ func TestService_UpdateService_ChangesNetworkConfiguration(t *testing.T) {
 		Family:      aws.String("app"),
 		NetworkMode: aws.String(NetworkModeAwsvpc),
 		ContainerDefinitions: []*ecs.ContainerDefinition{{
-			Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true),
+			Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true), Memory: aws.Int64(256),
 		}},
 	}, testAccountID)
 	require.NoError(t, err)

spinifex/domains/ecs/task_accounting_test.go

@@ -16,6 +16,8 @@ import (
 // agent to reach anything other than PENDING.
 func seedTasks(t *testing.T, svc *ecsdomain.Service, cluster string, recs ...*ecsdomain.TaskRecord) {
 	t.Helper()
+	_, err := svc.CreateCluster(t.Context(), &ecs.CreateClusterInput{ClusterName: aws.String(cluster)}, ecsdomain.TestAccountID)
+	require.NoError(t, err)
 	kv, err := svc.Bucket(t.Context(), ecsdomain.TestAccountID)
 	require.NoError(t, err)
 	for _, r := range recs {

spinifex/domains/ecs/taskdef_validation_test.go

@@ -19,7 +19,7 @@ func TestRegisterTaskDefinition_RejectsSecrets(t *testing.T) {
 	_, err := svc.RegisterTaskDefinition(context.Background(), &ecs.RegisterTaskDefinitionInput{
 		Family: aws.String("app"),
 		ContainerDefinitions: []*ecs.ContainerDefinition{{
-			Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true),
+			Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true), Memory: aws.Int64(256),
 			Secrets: []*ecs.Secret{{
 				Name: aws.String("DB_PASSWORD"), ValueFrom: aws.String("arn:aws:ssm:::parameter/db"),
 			}},
@@ -37,7 +37,7 @@ func TestRegisterTaskDefinition_AcceptsUnsupportedLogDriver(t *testing.T) {
 	_, err := svc.RegisterTaskDefinition(context.Background(), &ecs.RegisterTaskDefinitionInput{
 		Family: aws.String("app"),
 		ContainerDefinitions: []*ecs.ContainerDefinition{{
-			Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true),
+			Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true), Memory: aws.Int64(256),
 			LogConfiguration: &ecs.LogConfiguration{
 				LogDriver: aws.String("awslogs"),
 				Options:   map[string]*string{"awslogs-group": aws.String("/ecs/app")},
@@ -67,7 +67,7 @@ func TestRegisterTaskDefinition_EchoesRequiresCompatibilities(t *testing.T) {
 		Family:                  aws.String("app"),
 		RequiresCompatibilities: aws.StringSlice([]string{"EC2"}),
 		ContainerDefinitions: []*ecs.ContainerDefinition{{
-			Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true),
+			Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true), Memory: aws.Int64(256),
 		}},
 	}, testAccountID)
 	require.NoError(t, err)
@@ -91,7 +91,7 @@ func TestRegisterTaskDefinition_EchoesRuntimePlatform(t *testing.T) {
 			OperatingSystemFamily: aws.String("LINUX"),
 		},
 		ContainerDefinitions: []*ecs.ContainerDefinition{{
-			Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true),
+			Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true), Memory: aws.Int64(256),
 		}},
 	}, testAccountID)
 	require.NoError(t, err)
@@ -113,7 +113,7 @@ func TestRegisterTaskDefinition_RuntimePlatformOmittedWhenUnset(t *testing.T) {
 	_, err := svc.RegisterTaskDefinition(context.Background(), &ecs.RegisterTaskDefinitionInput{
 		Family: aws.String("app"),
 		ContainerDefinitions: []*ecs.ContainerDefinition{{
-			Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true),
+			Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true), Memory: aws.Int64(256),
 		}},
 	}, testAccountID)
 	require.NoError(t, err)
@@ -253,7 +253,7 @@ func TestRegisterTaskDefinition_EchoesRuntimeFields(t *testing.T) {
 	_, err := svc.RegisterTaskDefinition(context.Background(), &ecs.RegisterTaskDefinitionInput{
 		Family: aws.String("app"),
 		ContainerDefinitions: []*ecs.ContainerDefinition{{
-			Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true),
+			Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true), Memory: aws.Int64(256),
 			User:                   aws.String("1000:1000"),
 			ReadonlyRootFilesystem: aws.Bool(true),
 			Privileged:             aws.Bool(false),
@@ -310,7 +310,7 @@ func TestRegisterTaskDefinition_RuntimeFieldsOmittedWhenUnset(t *testing.T) {
 	_, err := svc.RegisterTaskDefinition(context.Background(), &ecs.RegisterTaskDefinitionInput{
 		Family: aws.String("app"),
 		ContainerDefinitions: []*ecs.ContainerDefinition{{
-			Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true),
+			Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true), Memory: aws.Int64(256),
 		}},
 	}, testAccountID)
 	require.NoError(t, err)
@@ -347,7 +347,7 @@ func TestRegisterTaskDefinition_EchoesEmptyCollections(t *testing.T) {
 	_, err := svc.RegisterTaskDefinition(context.Background(), &ecs.RegisterTaskDefinitionInput{
 		Family: aws.String("app"),
 		ContainerDefinitions: []*ecs.ContainerDefinition{{
-			Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true),
+			Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true), Memory: aws.Int64(256),
 			Environment:     []*ecs.KeyValuePair{},
 			MountPoints:     []*ecs.MountPoint{},
 			VolumesFrom:     []*ecs.VolumeFrom{},
@@ -386,7 +386,7 @@ func TestRegisterTaskDefinition_FillsAWSDefaults(t *testing.T) {
 		Family:      aws.String("app"),
 		NetworkMode: aws.String("awsvpc"),
 		ContainerDefinitions: []*ecs.ContainerDefinition{{
-			Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true),
+			Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true), Memory: aws.Int64(256),
 			PortMappings: []*ecs.PortMapping{{ContainerPort: aws.Int64(80)}},
 		}},
 	}, testAccountID)
@@ -420,7 +420,7 @@ func TestRegisterTaskDefinition_BridgeLeavesHostPortUnset(t *testing.T) {
 		Family:      aws.String("app"),
 		NetworkMode: aws.String("bridge"),
 		ContainerDefinitions: []*ecs.ContainerDefinition{{
-			Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true),
+			Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true), Memory: aws.Int64(256),
 			PortMappings: []*ecs.PortMapping{{ContainerPort: aws.Int64(53), Protocol: aws.String("udp")}},
 		}},
 	}, testAccountID)
@@ -435,7 +435,7 @@ func TestRegisterTaskDefinition_AcceptsEmptyRuntimeFields(t *testing.T) {
 	_, err := svc.RegisterTaskDefinition(context.Background(), &ecs.RegisterTaskDefinitionInput{
 		Family: aws.String("app"),
 		ContainerDefinitions: []*ecs.ContainerDefinition{{
-			Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true),
+			Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true), Memory: aws.Int64(256),
 			MountPoints:    []*ecs.MountPoint{},
 			VolumesFrom:    []*ecs.VolumeFrom{},
 			SystemControls: []*ecs.SystemControl{},
@@ -455,7 +455,7 @@ func TestRegisterTaskDefinition_RejectsMountPoints(t *testing.T) {
 	_, err := svc.RegisterTaskDefinition(context.Background(), &ecs.RegisterTaskDefinitionInput{
 		Family: aws.String("app"),
 		ContainerDefinitions: []*ecs.ContainerDefinition{{
-			Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true),
+			Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true), Memory: aws.Int64(256),
 			MountPoints: []*ecs.MountPoint{{
 				SourceVolume: aws.String("data"), ContainerPath: aws.String("/data"),
 			}},
@@ -476,7 +476,7 @@ func TestRegisterTaskDefinition_RejectsVolumesFrom(t *testing.T) {
 	_, err := svc.RegisterTaskDefinition(context.Background(), &ecs.RegisterTaskDefinitionInput{
 		Family: aws.String("app"),
 		ContainerDefinitions: []*ecs.ContainerDefinition{{
-			Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true),
+			Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true), Memory: aws.Int64(256),
 			VolumesFrom: []*ecs.VolumeFrom{{SourceContainer: aws.String("other")}},
 		}},
 	}, testAccountID)
@@ -494,7 +494,7 @@ func TestRegisterTaskDefinition_RejectsLinuxParametersDevices(t *testing.T) {
 	_, err := svc.RegisterTaskDefinition(context.Background(), &ecs.RegisterTaskDefinitionInput{
 		Family: aws.String("app"),
 		ContainerDefinitions: []*ecs.ContainerDefinition{{
-			Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true),
+			Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true), Memory: aws.Int64(256),
 			LinuxParameters: &ecs.LinuxParameters{
 				Devices: []*ecs.Device{{HostPath: aws.String("/dev/fuse")}},
 			},
@@ -514,7 +514,7 @@ func TestRegisterTaskDefinition_RejectsInitProcessEnabled(t *testing.T) {
 	_, err := svc.RegisterTaskDefinition(context.Background(), &ecs.RegisterTaskDefinitionInput{
 		Family: aws.String("app"),
 		ContainerDefinitions: []*ecs.ContainerDefinition{{
-			Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true),
+			Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true), Memory: aws.Int64(256),
 			LinuxParameters: &ecs.LinuxParameters{InitProcessEnabled: aws.Bool(true)},
 		}},
 	}, testAccountID)
@@ -533,7 +533,7 @@ func TestRegisterTaskDefinition_RejectsSharedMemoryAndTmpfs(t *testing.T) {
 	_, err := svc.RegisterTaskDefinition(context.Background(), &ecs.RegisterTaskDefinitionInput{
 		Family: aws.String("shm"),
 		ContainerDefinitions: []*ecs.ContainerDefinition{{
-			Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true),
+			Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true), Memory: aws.Int64(256),
 			LinuxParameters: &ecs.LinuxParameters{SharedMemorySize: aws.Int64(64)},
 		}},
 	}, testAccountID)
@@ -545,7 +545,7 @@ func TestRegisterTaskDefinition_RejectsSharedMemoryAndTmpfs(t *testing.T) {
 	_, err = svc.RegisterTaskDefinition(context.Background(), &ecs.RegisterTaskDefinitionInput{
 		Family: aws.String("tmpfs"),
 		ContainerDefinitions: []*ecs.ContainerDefinition{{
-			Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true),
+			Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true), Memory: aws.Int64(256),
 			LinuxParameters: &ecs.LinuxParameters{
 				Tmpfs: []*ecs.Tmpfs{{ContainerPath: aws.String("/tmp"), Size: aws.Int64(64)}},
 			},
@@ -567,7 +567,7 @@ func TestRunTask_AssignCarriesRuntimeFields(t *testing.T) {
 	_, err = svc.RegisterTaskDefinition(context.Background(), &ecs.RegisterTaskDefinitionInput{
 		Family: aws.String("app"),
 		ContainerDefinitions: []*ecs.ContainerDefinition{{
-			Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true),
+			Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true), Memory: aws.Int64(256),
 			User:                   aws.String("1000"),
 			ReadonlyRootFilesystem: aws.Bool(true),
 			Privileged:             aws.Bool(true),
@@ -608,3 +608,42 @@ func TestRunTask_AssignCarriesRuntimeFields(t *testing.T) {
 	assert.Equal(t, "net.core.somaxconn", ac.SystemControls[0].Namespace)
 	assert.Equal(t, []string{"SYS_PTRACE"}, ac.CapAdd)
 }
+
+// AWS rejects a container with no image before checking its memory, and
+// without task-level memory requires memory or memoryReservation per container.
+func TestRegisterTaskDefinition_RequiresImageAndMemory(t *testing.T) {
+	tests := []struct {
+		name       string
+		taskMemory *string
+		container  *ecs.ContainerDefinition
+		wantErr    string
+	}{
+		{"no image or memory", nil, &ecs.ContainerDefinition{Name: aws.String("web")},
+			"Container.image should not be null or empty."},
+		{"empty image", aws.String("512"), &ecs.ContainerDefinition{Name: aws.String("web"), Image: aws.String("")},
+			"Container.image should not be null or empty."},
+		{"no memory", nil, &ecs.ContainerDefinition{Name: aws.String("web"), Image: aws.String("registry/web:1")},
+			"Invalid setting for container 'web'. At least one of 'memory' or 'memoryReservation' must be specified."},
+		{"memoryReservation only", nil, &ecs.ContainerDefinition{
+			Name: aws.String("web"), Image: aws.String("registry/web:1"), MemoryReservation: aws.Int64(128)}, ""},
+		{"task-level memory", aws.String("512"), &ecs.ContainerDefinition{
+			Name: aws.String("web"), Image: aws.String("registry/web:1")}, ""},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			svc, _ := newTestService(t)
+			_, err := svc.RegisterTaskDefinition(context.Background(), &ecs.RegisterTaskDefinitionInput{
+				Family: aws.String("app"), Memory: tc.taskMemory,
+				ContainerDefinitions: []*ecs.ContainerDefinition{tc.container},
+			}, testAccountID)
+			if tc.wantErr == "" {
+				require.NoError(t, err)
+				return
+			}
+			code, msg, ok := awserrors.ResolveErrorDetail(err)
+			require.True(t, ok)
+			assert.Equal(t, awserrors.ErrorECSClientException, code)
+			assert.Equal(t, tc.wantErr, msg)
+		})
+	}
+}

spinifex/domains/ecs/tasks.go

@@ -237,6 +237,9 @@ func (s *Service) newTaskRecord(accountID, cluster, taskID string, td *taskdefin
 		GPU:                  gpu,
 		CreatedAt:            now,
 	}
+	if td.TaskRoleArn != "" {
+		rec.CredID = uuid.NewV4().String()
+	}
 	for _, c := range td.Containers {
 		rec.Containers = append(rec.Containers, ContainerState{Name: c.Name, Status: TaskStatusPending})
 	}
@@ -257,6 +260,7 @@ func (s *Service) publishAssign(ctx context.Context, kv jetstream.KeyValue, acco
 		TaskDefFamily:    td.Family,
 		TaskDefRevision:  td.Revision,
 		TaskRoleARN:      td.TaskRoleArn,
+		CredID:           rec.CredID,
 		ExecutionRoleARN: td.ExecutionRoleArn,
 		ENIID:            rec.ENIID,
 		ENIMacAddress:    rec.ENIMacAddress,
@@ -303,6 +307,14 @@ func (s *Service) ListTasks(ctx context.Context, input *ecs.ListTasksInput, acco
 	if err != nil {
 		return nil, err
 	}
+	var clusterRec ClusterRecord
+	found, err := getJSON(ctx, kv, ClusterMetaKey(cluster), &clusterRec)
+	if err != nil {
+		return nil, err
+	}
+	if !found {
+		return nil, errors.New(awserrors.ErrorECSClusterNotFound)
+	}
 	keys, err := keysWithPrefix(ctx, kv, TasksPrefix(cluster))
 	if err != nil {
 		return nil, err

spinifex/domains/eks/addon/catalog.go

@@ -4,8 +4,14 @@ import (
 	"fmt"
 	"maps"
 	"slices"
+	"strconv"
+	"strings"
 )
 
+// firstBuild is the EKS build number of every bundled version; it moves per
+// version when Spinifex rebuilds a bundle.
+const firstBuild = 1
+
 // NvidiaDevicePlugin is staged by a GPU node group's launch (EnsureGPUDevicePlugin),
 // never user-requested directly.
 const NvidiaDevicePlugin = "nvidia-device-plugin"
@@ -18,6 +24,11 @@ type Spec struct {
 	// Versions lists supported versions newest-first; [0] is the default.
 	Versions       []string
 	DefaultVersion string
+	// Builds maps each upstream version to its EKS build number, the n in
+	// v<upstream>-eksbuild.<n>.
+	Builds map[string]int
+	// Type is the AWS add-on type reported by DescribeAddonVersions.
+	Type string
 	// RequiresIRSA indicates the add-on needs an IAM role for service-account binding.
 	RequiresIRSA bool
 	Description  string
@@ -28,26 +39,26 @@ type Spec struct {
 
 // catalog is the bundled add-on registry. Keep newest version first per slice.
 var catalog = buildCatalog(
-	newSpec("aws-load-balancer-controller", true,
+	typed("networking", newSpec("aws-load-balancer-controller", true,
 		"Provisions ELBv2 load balancers for Kubernetes Service/Ingress resources.",
-		"2.11.0"),
-	newSpec("argocd", false,
+		"2.11.0")),
+	typed("gitops", newSpec("argocd", false,
 		"Declarative GitOps continuous delivery for Kubernetes.",
-		"3.0.23"),
-	newSpec("aws-ebs-csi-driver", true,
+		"3.0.23")),
+	typed("storage", newSpec("aws-ebs-csi-driver", true,
 		"Container Storage Interface driver for Amazon EBS (Viperblock) volumes.",
-		"1.40.1"),
+		"1.40.1")),
 	// Staged automatically for GPU node groups, so hidden from the public
 	// catalogue like spinifex-noop but still creatable by name.
-	hidden(newSpec(NvidiaDevicePlugin, false,
+	hidden(typed("accelerator", newSpec(NvidiaDevicePlugin, false,
 		"NVIDIA device plugin exposing nvidia.com/gpu allocatable via CDI on GPU-tainted nodes.",
-		"0.17.4")),
+		"0.17.4"))),
 	// spinifex-noop is the delivery-transport fixture: a trivial bundle
 	// (Namespace + ConfigMap) the add-on e2e uses to prove stage, render,
 	// auto-deploy, ACTIVE and delete round-trip without a real workload.
-	hidden(newSpec("spinifex-noop", false,
+	hidden(typed("fixture", newSpec("spinifex-noop", false,
 		"No-op delivery-transport fixture (Namespace + ConfigMap).",
-		"0.1.0")),
+		"0.1.0"))),
 )
 
 // hidden marks a spec as internal: still creatable, but absent from the
@@ -57,6 +68,12 @@ func hidden(s Spec) Spec {
 	return s
 }
 
+// typed sets the AWS add-on type reported by DescribeAddonVersions.
+func typed(addonType string, s Spec) Spec {
+	s.Type = addonType
+	return s
+}
+
 // newSpec builds a spec with the first version as its default.
 func newSpec(name string, requiresIRSA bool, description string, versions ...string) Spec {
 	spec := Spec{
@@ -68,6 +85,10 @@ func newSpec(name string, requiresIRSA bool, description string, versions ...str
 	if len(versions) > 0 {
 		spec.DefaultVersion = versions[0]
 	}
+	spec.Builds = make(map[string]int, len(versions))
+	for _, v := range versions {
+		spec.Builds[v] = firstBuild
+	}
 	return spec
 }
 
@@ -111,6 +132,50 @@ func (s Spec) SupportsVersion(version string) bool {
 	return slices.Contains(s.Versions, version)
 }
 
+// awsForm renders a catalog version as v<upstream>-eksbuild.<build>.
+func (s Spec) awsForm(upstream string) string {
+	build, ok := s.Builds[upstream]
+	if !ok {
+		build = firstBuild
+	}
+	return "v" + upstream + "-eksbuild." + strconv.Itoa(build)
+}
+
+// AWSVersion returns the AWS form of a stored version. A version the catalogue
+// does not list, or one already in AWS form, is returned unchanged.
+func (s Spec) AWSVersion(stored string) string {
+	if !slices.Contains(s.Versions, stored) {
+		return stored
+	}
+	return s.awsForm(stored)
+}
+
+// AWSVersions returns every supported version in AWS form, newest first.
+func (s Spec) AWSVersions() []string {
+	out := make([]string, len(s.Versions))
+	for i, v := range s.Versions {
+		out[i] = s.awsForm(v)
+	}
+	return out
+}
+
+// ParseVersion maps a client-supplied version, in AWS form or bare upstream
+// form, to the upstream version stored on the record. Nothing else is accepted.
+func (s Spec) ParseVersion(input string) (string, bool) {
+	if slices.Contains(s.Versions, input) {
+		return input, true
+	}
+	if !strings.HasPrefix(input, "v") {
+		return "", false
+	}
+	for _, v := range s.Versions {
+		if input == s.awsForm(v) {
+			return v, true
+		}
+	}
+	return "", false
+}
+
 // Specs returns every catalogue entry, hidden ones included, sorted by name.
 func Specs() []Spec {
 	names := slices.Sorted(maps.Keys(catalog))

spinifex/domains/eks/addon/catalog_test.go

@@ -100,3 +100,78 @@ func TestValidateCatalog_ChecksTheBundledCatalog(t *testing.T) {
 	t.Cleanup(func() { catalog = previous })
 	require.EqualError(t, ValidateCatalog(), `add-on "broken" has no versions`)
 }
+
+// buildSpec is a two-version spec whose newest version is on EKS build 2, so
+// a test cannot pass by hardcoding build 1.
+func buildSpec() Spec {
+	return Spec{
+		Name:     "fixture",
+		Versions: []string{"2.0.0", "1.0.0"},
+		Builds:   map[string]int{"2.0.0": 2, "1.0.0": 1},
+	}
+}
+
+func TestSpec_AWSVersionUsesTheCatalogBuildNumber(t *testing.T) {
+	t.Parallel()
+	spec := buildSpec()
+	assert.Equal(t, "v2.0.0-eksbuild.2", spec.AWSVersion("2.0.0"))
+	assert.Equal(t, "v1.0.0-eksbuild.1", spec.AWSVersion("1.0.0"))
+}
+
+func TestSpec_AWSVersionIsIdempotent(t *testing.T) {
+	t.Parallel()
+	spec := buildSpec()
+	assert.Equal(t, "v2.0.0-eksbuild.2", spec.AWSVersion("v2.0.0-eksbuild.2"),
+		"a record already holding the AWS form must not be wrapped twice")
+}
+
+func TestSpec_AWSVersionsAreNewestFirst(t *testing.T) {
+	t.Parallel()
+	assert.Equal(t, []string{"v2.0.0-eksbuild.2", "v1.0.0-eksbuild.1"}, buildSpec().AWSVersions())
+}
+
+func TestSpec_ParseVersionAcceptsOnlyTheExactForms(t *testing.T) {
+	t.Parallel()
+	spec := buildSpec()
+	for input, want := range map[string]string{
+		"v2.0.0-eksbuild.2": "2.0.0",
+		"2.0.0":             "2.0.0",
+		"v1.0.0-eksbuild.1": "1.0.0",
+		"1.0.0":             "1.0.0",
+	} {
+		got, ok := spec.ParseVersion(input)
+		assert.True(t, ok, "%q must be accepted", input)
+		assert.Equal(t, want, got, "%q", input)
+	}
+}
+
+func TestSpec_ParseVersionRefusesEverythingElse(t *testing.T) {
+	t.Parallel()
+	spec := buildSpec()
+	for _, input := range []string{
+		"v2.0.0-eksbuild.1", // wrong build for 2.0.0
+		"v2.0.0-eksbuild.3",
+		"v1.0.0-eksbuild.2", // wrong build for 1.0.0
+		"v2.0.0",            // missing build
+		"2.0",               // partial
+		"v2.0.0-eksbuild.",
+		"2.0.0-eksbuild.2", // missing v
+		"V2.0.0-eksbuild.2",
+		" 2.0.0",
+		"9.9.9",
+		"",
+	} {
+		_, ok := spec.ParseVersion(input)
+		assert.False(t, ok, "%q must be refused", input)
+	}
+}
+
+func TestCatalog_EveryVersionHasAnExplicitBuild(t *testing.T) {
+	t.Parallel()
+	for _, spec := range Specs() {
+		for _, v := range spec.Versions {
+			assert.Equal(t, 1, spec.Builds[v], "%s %s needs an explicit build number", spec.Name, v)
+			assert.Equal(t, "v"+v+"-eksbuild.1", spec.AWSVersion(v), "%s %s", spec.Name, v)
+		}
+	}
+}

spinifex/domains/eks/awsapi/addon_versions_query_test.go

@@ -0,0 +1,105 @@
+package awsapi
+
+import (
+	"context"
+	"encoding/json"
+	"sync"
+	"testing"
+
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/service/eks"
+	"github.com/mulgadc/spinifex/internal/testkit"
+	"github.com/nats-io/nats.go"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// captureDescribeAddonVersions answers eks.DescribeAddonVersions and returns a
+// function reading the last input the gateway sent to the service.
+func captureDescribeAddonVersions(t *testing.T, nc *nats.Conn) func() eks.DescribeAddonVersionsInput {
+	t.Helper()
+	var (
+		mu   sync.Mutex
+		last eks.DescribeAddonVersionsInput
+	)
+	sub, err := nc.Subscribe("eks.DescribeAddonVersions", func(m *nats.Msg) {
+		mu.Lock()
+		last = eks.DescribeAddonVersionsInput{}
+		_ = json.Unmarshal(m.Data, &last)
+		mu.Unlock()
+		_ = m.Respond([]byte(`{}`))
+	})
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = sub.Unsubscribe() })
+	return func() eks.DescribeAddonVersionsInput {
+		mu.Lock()
+		defer mu.Unlock()
+		return last
+	}
+}
+
+// The gateway folds a GET query string into the body as a JSON object of
+// string lists; the model binds every field below from the query.
+func TestDescribeAddonVersions_PassesEveryModelQueryParameter(t *testing.T) {
+	_, nc := testutil.StartTestNATS(t)
+	got := captureDescribeAddonVersions(t, nc)
+
+	query := []byte(`{"addonName":["aws-ebs-csi-driver"],"kubernetesVersion":["1.32"],` +
+		`"maxResults":["2"],"nextToken":["tok"],"types":["storage","networking"],` +
+		`"publishers":["eks"],"owners":["aws"]}`)
+	_, err := DescribeAddonVersions(context.Background(), nc, acct, query)
+	require.NoError(t, err)
+
+	in := got()
+	assert.Equal(t, "aws-ebs-csi-driver", aws.StringValue(in.AddonName))
+	assert.Equal(t, "1.32", aws.StringValue(in.KubernetesVersion))
+	assert.Equal(t, int64(2), aws.Int64Value(in.MaxResults))
+	assert.Equal(t, "tok", aws.StringValue(in.NextToken))
+	assert.Equal(t, []string{"storage", "networking"}, aws.StringValueSlice(in.Types))
+	assert.Equal(t, []string{"eks"}, aws.StringValueSlice(in.Publishers))
+	assert.Equal(t, []string{"aws"}, aws.StringValueSlice(in.Owners))
+}
+
+func TestDescribeAddonVersions_NoQueryLeavesEveryFilterUnset(t *testing.T) {
+	_, nc := testutil.StartTestNATS(t)
+	got := captureDescribeAddonVersions(t, nc)
+
+	_, err := DescribeAddonVersions(context.Background(), nc, acct, nil)
+	require.NoError(t, err)
+
+	in := got()
+	assert.Nil(t, in.AddonName)
+	assert.Nil(t, in.KubernetesVersion)
+	assert.Nil(t, in.MaxResults)
+	assert.Nil(t, in.NextToken)
+	assert.Empty(t, in.Types)
+}
+
+func TestDescribeAddonVersions_RefusesMalformedQuery(t *testing.T) {
+	_, nc := testutil.StartTestNATS(t)
+	captureDescribeAddonVersions(t, nc)
+
+	_, err := DescribeAddonVersions(context.Background(), nc, acct, []byte(`{"maxResults":["many"]}`))
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "maxResults")
+
+	_, err = DescribeAddonVersions(context.Background(), nc, acct, []byte(`not json`))
+	require.Error(t, err)
+}
+
+// The route must hand the folded query to the wrapper, not drop it.
+func TestDescribeAddonVersionsRoute_PassesTheQuery(t *testing.T) {
+	_, nc := testutil.StartTestNATS(t)
+	got := captureDescribeAddonVersions(t, nc)
+
+	for _, r := range routes {
+		if r.Action != "DescribeAddonVersions" {
+			continue
+		}
+		_, err := r.Handler(context.Background(), nc, acct, "", nil, []byte(`{"addonName":["argocd"]}`))
+		require.NoError(t, err)
+		assert.Equal(t, "argocd", aws.StringValue(got().AddonName))
+		return
+	}
+	t.Fatal("no DescribeAddonVersions route")
+}

spinifex/domains/eks/awsapi/addons.go

@@ -16,8 +16,23 @@ func ListAddons(ctx context.Context, natsConn *nats.Conn, accountID, cluster str
 }
 
 // DescribeAddonVersions — GET /addons/supported-versions.
-func DescribeAddonVersions(ctx context.Context, natsConn *nats.Conn, accountID string) (*eks.DescribeAddonVersionsOutput, error) {
-	return ekscluster.NewNATSEKSService(natsConn).DescribeAddonVersions(ctx, &eks.DescribeAddonVersionsInput{}, accountID)
+func DescribeAddonVersions(ctx context.Context, natsConn *nats.Conn, accountID string, query []byte) (*eks.DescribeAddonVersionsOutput, error) {
+	q, err := parseListQuery(query)
+	if err != nil {
+		return nil, err
+	}
+	input := &eks.DescribeAddonVersionsInput{
+		AddonName:         q.str("addonName"),
+		KubernetesVersion: q.str("kubernetesVersion"),
+		NextToken:         q.str("nextToken"),
+		Types:             q.strs("types"),
+		Publishers:        q.strs("publishers"),
+		Owners:            q.strs("owners"),
+	}
+	if input.MaxResults, err = q.int64("maxResults"); err != nil {
+		return nil, err
+	}
+	return ekscluster.NewNATSEKSService(natsConn).DescribeAddonVersions(ctx, input, accountID)
 }
 
 // CreateAddon — POST /clusters/{name}/addons.

spinifex/domains/eks/awsapi/query.go

@@ -0,0 +1,61 @@
+package awsapi
+
+import (
+	"encoding/json"
+	"strconv"
+
+	"github.com/aws/aws-sdk-go/aws"
+	awserrors "github.com/mulgadc/spinifex/spinifex/foundation/aws/errors"
+)
+
+// listQuery holds the query-string inputs of a GET action, which the gateway
+// folds into the body as a JSON object of string lists.
+type listQuery map[string][]string
+
+// parseListQuery decodes the folded query of a GET action. An empty body means
+// the request carried no query string.
+func parseListQuery(body []byte) (listQuery, error) {
+	q := listQuery{}
+	if len(body) == 0 {
+		return q, nil
+	}
+	if err := json.Unmarshal(body, (*map[string][]string)(&q)); err != nil {
+		return nil, awserrors.Errorf(awserrors.ErrorEKSInvalidParameter, "malformed query string")
+	}
+	return q, nil
+}
+
+// str returns the first value of name, or nil when it is absent or empty.
+func (q listQuery) str(name string) *string {
+	if v := q[name]; len(v) > 0 && v[0] != "" {
+		return aws.String(v[0])
+	}
+	return nil
+}
+
+// strs returns every non-empty value of name, or nil when there are none.
+func (q listQuery) strs(name string) []*string {
+	var out []string
+	for _, v := range q[name] {
+		if v != "" {
+			out = append(out, v)
+		}
+	}
+	if len(out) == 0 {
+		return nil
+	}
+	return aws.StringSlice(out)
+}
+
+// int64 returns the first value of name as an integer, or nil when absent.
+func (q listQuery) int64(name string) (*int64, error) {
+	s := q.str(name)
+	if s == nil {
+		return nil, nil
+	}
+	n, err := strconv.ParseInt(*s, 10, 64)
+	if err != nil {
+		return nil, awserrors.Errorf(awserrors.ErrorEKSInvalidParameter, "%s must be an integer", name)
+	}
+	return aws.Int64(n), nil
+}

spinifex/domains/eks/awsapi/registration.go

@@ -82,17 +82,39 @@ func (d Deps) dispatch(w http.ResponseWriter, inv dispatch.Invocation) error {
 	if err == nil {
 		return nil
 	}
-	if action, _, _, ok := router.Lookup(inv.Request.Method, inv.Request.URL.EscapedPath()); ok && IsInternalAction(action) {
+	action, _, _, routed := router.Lookup(inv.Request.Method, inv.Request.URL.EscapedPath())
+	if routed && IsInternalAction(action) {
 		return err
 	}
 	code, msg, ok := awserrors.ResolveErrorDetail(err)
 	if !ok || code != awserrors.ErrorInvalidParameterValue {
 		return err
 	}
-	if msg == "" {
-		return errors.New(awserrors.ErrorEKSInvalidParameter)
+	wire := awserrors.ErrorEKSInvalidParameter
+	if override, ok := parameterRefusalCodes[action]; ok {
+		wire = override
+		if msg == "" {
+			msg = "a parameter of the request is not valid"
+		}
 	}
-	return awserrors.Errorf(awserrors.ErrorEKSInvalidParameter, "%s", msg)
+	if msg == "" {
+		return errors.New(wire)
+	}
+	return awserrors.Errorf(wire, "%s", msg)
+}
+
+// parameterRefusalCodes holds the operations whose model does not declare
+// InvalidParameterException, with the declared code a refused parameter uses.
+var parameterRefusalCodes = map[string]string{
+	"DescribeCluster":              awserrors.ErrorEKSClient,
+	"DeleteCluster":                awserrors.ErrorEKSClient,
+	"DescribeAccessEntry":          awserrors.ErrorEKSInvalidRequest,
+	"DeleteAccessEntry":            awserrors.ErrorEKSInvalidRequest,
+	"DisassociateAccessPolicy":     awserrors.ErrorEKSInvalidRequest,
+	"ListAssociatedAccessPolicies": awserrors.ErrorEKSInvalidRequest,
+	"TagResource":                  awserrors.ErrorEKSBadRequest,
+	"UntagResource":                awserrors.ErrorEKSBadRequest,
+	"ListTagsForResource":          awserrors.ErrorEKSBadRequest,
 }
 
 // serve resolves the route, authorizes the request, then runs the action.

spinifex/domains/eks/awsapi/registration_error_codes_test.go

@@ -0,0 +1,98 @@
+//test:in-package — routes is unexported, and the table must cover exactly the
+// actions the registration serves.
+
+package awsapi
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"strings"
+	"testing"
+
+	"github.com/mulgadc/bluebottle/pkg/iampolicy"
+	"github.com/mulgadc/spinifex/internal/awsmodel"
+	"github.com/mulgadc/spinifex/internal/testkit"
+	awserrors "github.com/mulgadc/spinifex/spinifex/foundation/aws/errors"
+	"github.com/mulgadc/spinifex/spinifex/ingress/aws/dispatch"
+	"github.com/nats-io/nats.go"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// routePath fills a route pattern with placeholder segments. The tag routes
+// take an ARN, which has to be escaped to stay one path segment.
+func routePath(pattern string) string {
+	arn := url.PathEscape("arn:aws:eks:us-east-1:111122223333:cluster/c")
+	path := strings.ReplaceAll(pattern, "*", arn)
+	for {
+		start := strings.Index(path, "{")
+		if start < 0 {
+			return path
+		}
+		end := strings.Index(path[start:], "}")
+		path = path[:start] + "x" + path[start+end+1:]
+	}
+}
+
+// A parameter refusal must reach the wire under a code the operation declares
+// in the EKS model, or SDK clients cannot match it to a typed error.
+func TestRegistration_ParameterRefusalCodeIsDeclaredByOperation(t *testing.T) {
+	_, nc := testutil.StartTestNATS(t)
+	sub, err := nc.Subscribe("eks.>", func(m *nats.Msg) {
+		if m.Reply != "" {
+			_ = m.Respond(awserrors.GenerateErrorPayload(awserrors.ErrorInvalidParameterValue))
+		}
+	})
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = sub.Unsubscribe() })
+
+	model, err := awsmodel.Load(awsmodel.EKS)
+	require.NoError(t, err)
+	registration := NewRegistration(Deps{NATS: nc})
+
+	for _, route := range routes {
+		if IsInternalAction(route.Action) {
+			continue
+		}
+		// The handler takes no input and the model declares no client error.
+		if route.Action == "ListAccessPolicies" {
+			continue
+		}
+		t.Run(route.Action, func(t *testing.T) {
+			operation, ok := model.Operation(route.Action)
+			require.True(t, ok, "%s is not an operation in the EKS model", route.Action)
+			declared := make([]string, 0, len(operation.Errors))
+			for _, ref := range operation.Errors {
+				declared = append(declared, ref.Shape)
+			}
+
+			req := httptest.NewRequest(route.Method, routePath(route.Pattern), strings.NewReader(`{}`))
+			inv := dispatch.Invocation{
+				Request:   req,
+				AccountID: "111122223333",
+				Region:    "us-east-1",
+				Authorize: func(string, string, []string, iampolicy.ConditionKeys) error { return nil },
+				Actor: func() (dispatch.Actor, error) {
+					return dispatch.Actor{AccountID: "111122223333", PrincipalType: "user", UserName: "alice"}, nil
+				},
+			}
+
+			err := registration.Dispatch(httptest.NewRecorder(), inv)
+
+			require.Error(t, err)
+			code, ok := awserrors.ResolveErrorCode(err)
+			require.True(t, ok, "unresolvable error code: %v", err)
+			assert.Contains(t, declared, code,
+				"%s %s refuses a parameter with %s, which the operation does not declare", route.Method, route.Pattern, code)
+		})
+	}
+}
+
+// The access operations declare InvalidRequestException, not a parameter code.
+func TestEKSInvalidRequestCodeIsRegistered(t *testing.T) {
+	assert.Equal(t, "InvalidRequestException", awserrors.ErrorEKSInvalidRequest)
+	assert.Equal(t, http.StatusBadRequest, awserrors.HTTPStatusForError(awserrors.Errorf(awserrors.ErrorEKSInvalidRequest, "x")))
+	assert.Equal(t, "BadRequestException", awserrors.ErrorEKSBadRequest)
+	assert.Equal(t, http.StatusBadRequest, awserrors.HTTPStatusForError(awserrors.Errorf(awserrors.ErrorEKSBadRequest, "x")))
+}

spinifex/domains/eks/awsapi/routes.go

@@ -131,7 +131,7 @@ var routes = []route{
 	// Addons
 	{Method: "GET", Pattern: "/addons/supported-versions", Action: "DescribeAddonVersions",
 		Handler: func(ctx context.Context, nc *nats.Conn, acct, callerARN string, p []string, b []byte) (any, error) {
-			return DescribeAddonVersions(ctx, nc, acct)
+			return DescribeAddonVersions(ctx, nc, acct, b)
 		}},
 	{Method: "GET", Pattern: "/clusters/{clusterName}/addons", Action: "ListAddons",
 		Handler: func(ctx context.Context, nc *nats.Conn, acct, callerARN string, p []string, b []byte) (any, error) {

spinifex/domains/eks/awsapi/wrappers_test.go

@@ -176,7 +176,7 @@ func TestGatewayWrappers_Addons(t *testing.T) {
 	require.NoError(t, err)
 	assert.NotNil(t, out1)
 
-	out2, err := DescribeAddonVersions(context.Background(), nc, acct)
+	out2, err := DescribeAddonVersions(context.Background(), nc, acct, nil)
 	require.NoError(t, err)
 	assert.NotNil(t, out2)
 

spinifex/domains/eks/cluster/addon_describe_versions_test.go

@@ -0,0 +1,171 @@
+package cluster
+
+import (
+	"context"
+	"testing"
+
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/service/eks"
+	"github.com/mulgadc/spinifex/spinifex/domains/eks/addon"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func describeVersions(t *testing.T, svc *EKSServiceImpl, in *eks.DescribeAddonVersionsInput) *eks.DescribeAddonVersionsOutput {
+	t.Helper()
+	out, err := svc.DescribeAddonVersions(context.Background(), in, testAccountID)
+	require.NoError(t, err)
+	return out
+}
+
+// The terraform aws_eks_addon_version data source asks for one add-on at the
+// cluster's Kubernetes version, then takes the version whose compatibility for
+// that cluster version is the default (or, with most_recent, the first one).
+func TestDescribeAddonVersions_TerraformProviderCall(t *testing.T) {
+	t.Parallel()
+	svc := setupTestService(t)
+
+	out := describeVersions(t, svc, &eks.DescribeAddonVersionsInput{
+		AddonName:         aws.String(csiAddon),
+		KubernetesVersion: aws.String(defaultK8sVersion),
+	})
+	require.Len(t, out.Addons, 1)
+	info := out.Addons[0]
+	assert.Equal(t, csiAddon, aws.StringValue(info.AddonName))
+	assert.Nil(t, out.NextToken)
+
+	spec, ok := addon.Lookup(csiAddon)
+	require.True(t, ok)
+	require.Equal(t, spec.AWSVersions(), versionStrings(info), "newest first, AWS form")
+
+	defaults := 0
+	for _, v := range info.AddonVersions {
+		require.Len(t, v.Compatibilities, 1, "one entry, for the requested cluster version")
+		c := v.Compatibilities[0]
+		assert.Equal(t, defaultK8sVersion, aws.StringValue(c.ClusterVersion))
+		assert.Equal(t, []string{"*"}, aws.StringValueSlice(c.PlatformVersions))
+		if aws.BoolValue(c.DefaultVersion) {
+			defaults++
+			assert.Equal(t, spec.AWSVersion(spec.DefaultVersion), aws.StringValue(v.AddonVersion))
+		}
+	}
+	assert.Equal(t, 1, defaults, "exactly one defaultVersion true")
+}
+
+// AWS observed (EKS-01): compatibilities holds only the requested version. The
+// response for a version no add-on supports is not captured; this pins the
+// shape #61 chose, an empty list with no error.
+func TestDescribeAddonVersions_UnsupportedKubernetesVersionIsEmpty(t *testing.T) {
+	t.Parallel()
+	svc := setupTestService(t)
+
+	out := describeVersions(t, svc, &eks.DescribeAddonVersionsInput{
+		AddonName:         aws.String(csiAddon),
+		KubernetesVersion: aws.String("1.29"),
+	})
+	assert.Empty(t, out.Addons)
+	assert.Nil(t, out.NextToken)
+}
+
+func TestDescribeAddonVersions_FiltersByNameAndHidesFixturesUnlessNamed(t *testing.T) {
+	t.Parallel()
+	svc := setupTestService(t)
+
+	for _, name := range []string{csiAddon, "argocd", albController} {
+		out := describeVersions(t, svc, &eks.DescribeAddonVersionsInput{AddonName: aws.String(name)})
+		require.Len(t, out.Addons, 1, name)
+		assert.Equal(t, name, aws.StringValue(out.Addons[0].AddonName))
+	}
+	assert.Empty(t, describeVersions(t, svc, &eks.DescribeAddonVersionsInput{AddonName: aws.String("vpc-cni")}).Addons)
+}
+
+// maxResults counts add-ons, not versions; a page boundary never splits an
+// add-on's version list, and the last page carries no token.
+func TestDescribeAddonVersions_PagesByAddon(t *testing.T) {
+	t.Parallel()
+	svc := setupTestService(t)
+
+	all := describeVersions(t, svc, &eks.DescribeAddonVersionsInput{})
+	require.Greater(t, len(all.Addons), 1)
+	assert.Nil(t, all.NextToken)
+
+	var names []string
+	in := &eks.DescribeAddonVersionsInput{MaxResults: aws.Int64(1)}
+	for range len(all.Addons) {
+		out := describeVersions(t, svc, in)
+		require.Len(t, out.Addons, 1)
+		names = append(names, aws.StringValue(out.Addons[0].AddonName))
+		assert.Equal(t, versionStrings(all.Addons[len(names)-1]), versionStrings(out.Addons[0]))
+		in.NextToken = out.NextToken
+	}
+	assert.Nil(t, in.NextToken, "last page carries no token")
+	var want []string
+	for _, a := range all.Addons {
+		want = append(want, aws.StringValue(a.AddonName))
+	}
+	assert.Equal(t, want, names)
+}
+
+// Every add-on carries the descriptive fields the data source and console read.
+func TestDescribeAddonVersions_CarriesAddonAndVersionMetadata(t *testing.T) {
+	t.Parallel()
+	svc := setupTestService(t)
+
+	for _, a := range describeVersions(t, svc, &eks.DescribeAddonVersionsInput{}).Addons {
+		name := aws.StringValue(a.AddonName)
+		assert.NotEmpty(t, aws.StringValue(a.Type), name)
+		assert.NotEmpty(t, aws.StringValue(a.Owner), name)
+		assert.NotEmpty(t, aws.StringValue(a.Publisher), name)
+		assert.Nil(t, a.MarketplaceInformation, name)
+		for _, v := range a.AddonVersions {
+			assert.NotEmpty(t, v.Architecture, name)
+			assert.NotNil(t, v.RequiresConfiguration, name)
+			assert.NotNil(t, v.RequiresIamPermissions, name)
+		}
+	}
+}
+
+// types, publishers and owners are model filters; each keeps add-ons whose
+// field matches one of the given values.
+func TestDescribeAddonVersions_FiltersByTypePublisherOwner(t *testing.T) {
+	t.Parallel()
+	svc := setupTestService(t)
+
+	first := describeVersions(t, svc, &eks.DescribeAddonVersionsInput{}).Addons[0]
+	cases := []struct {
+		name  string
+		match func(*eks.DescribeAddonVersionsInput, []*string)
+		value *string
+	}{
+		{"types", func(in *eks.DescribeAddonVersionsInput, v []*string) { in.Types = v }, first.Type},
+		{"publishers", func(in *eks.DescribeAddonVersionsInput, v []*string) { in.Publishers = v }, first.Publisher},
+		{"owners", func(in *eks.DescribeAddonVersionsInput, v []*string) { in.Owners = v }, first.Owner},
+	}
+	for _, c := range cases {
+		hit := &eks.DescribeAddonVersionsInput{}
+		c.match(hit, aws.StringSlice([]string{"nobody", aws.StringValue(c.value)}))
+		out := describeVersions(t, svc, hit)
+		require.NotEmpty(t, out.Addons, c.name)
+		assert.Contains(t, addonNames(out), aws.StringValue(first.AddonName), c.name)
+
+		miss := &eks.DescribeAddonVersionsInput{}
+		c.match(miss, aws.StringSlice([]string{"nobody"}))
+		assert.Empty(t, describeVersions(t, svc, miss).Addons, c.name)
+	}
+}
+
+func versionStrings(a *eks.AddonInfo) []string {
+	out := make([]string, 0, len(a.AddonVersions))
+	for _, v := range a.AddonVersions {
+		out = append(out, aws.StringValue(v.AddonVersion))
+	}
+	return out
+}
+
+func addonNames(o *eks.DescribeAddonVersionsOutput) []string {
+	out := make([]string, 0, len(o.Addons))
+	for _, a := range o.Addons {
+		out = append(out, aws.StringValue(a.AddonName))
+	}
+	return out
+}

spinifex/domains/eks/cluster/addon_version_format_test.go

@@ -0,0 +1,220 @@
+package cluster
+
+import (
+	"bytes"
+	"context"
+	"log/slog"
+	"testing"
+
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/service/eks"
+	"github.com/mulgadc/spinifex/spinifex/domains/eks/addon"
+	awserrors "github.com/mulgadc/spinifex/spinifex/foundation/aws/errors"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+const (
+	csiAddon       = "aws-ebs-csi-driver"
+	csiBare        = "1.40.1"
+	csiAWSVersion  = "v1.40.1-eksbuild.1"
+	csiWrongBuild  = "v1.40.1-eksbuild.2"
+	versionParam   = "addonVersion"
+	notBeforeAWSOK = "an AWS-format version must be accepted"
+)
+
+func TestDescribeAddonVersions_ReturnsAWSFormNewestFirst(t *testing.T) {
+	t.Parallel()
+	svc := setupTestService(t)
+
+	out, err := svc.DescribeAddonVersions(context.Background(), &eks.DescribeAddonVersionsInput{}, testAccountID)
+	require.NoError(t, err)
+	require.NotEmpty(t, out.Addons)
+	for _, a := range out.Addons {
+		spec, ok := addon.Lookup(aws.StringValue(a.AddonName))
+		require.True(t, ok)
+		require.Len(t, a.AddonVersions, len(spec.Versions))
+		for i, v := range a.AddonVersions {
+			assert.Equal(t, "v"+spec.Versions[i]+"-eksbuild.1", aws.StringValue(v.AddonVersion),
+				"%s version %d", spec.Name, i)
+		}
+	}
+}
+
+func TestCreateAddon_AcceptsBothFormsAndStoresTheBareForm(t *testing.T) {
+	t.Parallel()
+	for name, input := range map[string]string{"aws form": csiAWSVersion, "bare form": csiBare} {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+			svc, fake := setupAddonService(t)
+
+			out, err := svc.CreateAddon(context.Background(), &eks.CreateAddonInput{
+				ClusterName: aws.String("c1"), AddonName: aws.String(csiAddon),
+				AddonVersion: aws.String(input),
+			}, testAccountID)
+			require.NoError(t, err, notBeforeAWSOK)
+			assert.Equal(t, csiAWSVersion, aws.StringValue(out.Addon.AddonVersion), "response is in AWS form")
+
+			rec, err := addon.Get(t.Context(), acctKVForTest(t, svc), "c1", csiAddon)
+			require.NoError(t, err)
+			assert.Equal(t, csiBare, rec.AddonVersion, "the record keeps the upstream form")
+			require.Len(t, fake.installs, 1)
+			assert.Equal(t, csiBare, fake.installs[0].AddonVersion, "the bundle lookup sees the upstream form")
+		})
+	}
+}
+
+func TestCreateAddon_DefaultVersionResponseIsAWSForm(t *testing.T) {
+	t.Parallel()
+	svc, _ := setupAddonService(t)
+	out, err := svc.CreateAddon(context.Background(), &eks.CreateAddonInput{
+		ClusterName: aws.String("c1"), AddonName: aws.String(csiAddon),
+	}, testAccountID)
+	require.NoError(t, err)
+	assert.Equal(t, csiAWSVersion, aws.StringValue(out.Addon.AddonVersion))
+}
+
+func TestUpdateAddon_AcceptsBothFormsAndStoresTheBareForm(t *testing.T) {
+	t.Parallel()
+	for name, input := range map[string]string{"aws form": csiAWSVersion, "bare form": csiBare} {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+			svc, fake := setupAddonService(t)
+			_, err := svc.CreateAddon(context.Background(), &eks.CreateAddonInput{
+				ClusterName: aws.String("c1"), AddonName: aws.String(csiAddon),
+			}, testAccountID)
+			require.NoError(t, err)
+
+			_, err = svc.UpdateAddon(context.Background(), &eks.UpdateAddonInput{
+				ClusterName: aws.String("c1"), AddonName: aws.String(csiAddon),
+				AddonVersion: aws.String(input),
+			}, testAccountID)
+			require.NoError(t, err, notBeforeAWSOK)
+
+			rec, err := addon.Get(t.Context(), acctKVForTest(t, svc), "c1", csiAddon)
+			require.NoError(t, err)
+			assert.Equal(t, csiBare, rec.AddonVersion)
+			require.Len(t, fake.installs, 2)
+			assert.Equal(t, csiBare, fake.installs[1].AddonVersion)
+		})
+	}
+}
+
+func TestDescribeAddon_ReturnsAWSFormForAnyStoredForm(t *testing.T) {
+	t.Parallel()
+	for name, stored := range map[string]string{
+		"record stored bare before the change": csiBare,
+		"record already in AWS form":           csiAWSVersion,
+	} {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+			svc, _ := setupAddonService(t)
+			_, err := svc.addons().Create(context.Background(), acctKVForTest(t, svc), testAccountID, addon.Desired{
+				Cluster: "c1", Name: csiAddon, Version: stored,
+			})
+			require.NoError(t, err)
+
+			desc, err := svc.DescribeAddon(context.Background(), &eks.DescribeAddonInput{
+				ClusterName: aws.String("c1"), AddonName: aws.String(csiAddon),
+			}, testAccountID)
+			require.NoError(t, err)
+			assert.Equal(t, csiAWSVersion, aws.StringValue(desc.Addon.AddonVersion), "no double wrap")
+
+			list, err := svc.ListAddons(context.Background(), &eks.ListAddonsInput{ClusterName: aws.String("c1")}, testAccountID)
+			require.NoError(t, err)
+			assert.Equal(t, []string{csiAddon}, aws.StringValueSlice(list.Addons))
+		})
+	}
+}
+
+func TestUpdateAddon_ResponseAndRecordAfterBareRecord(t *testing.T) {
+	t.Parallel()
+	svc, _ := setupAddonService(t)
+	_, err := svc.addons().Create(context.Background(), acctKVForTest(t, svc), testAccountID, addon.Desired{
+		Cluster: "c1", Name: csiAddon, Version: csiBare,
+	})
+	require.NoError(t, err)
+
+	_, err = svc.UpdateAddon(context.Background(), &eks.UpdateAddonInput{
+		ClusterName: aws.String("c1"), AddonName: aws.String(csiAddon),
+		ConfigurationValues: aws.String(`{"a":1}`),
+	}, testAccountID)
+	require.NoError(t, err)
+	desc, err := svc.DescribeAddon(context.Background(), &eks.DescribeAddonInput{
+		ClusterName: aws.String("c1"), AddonName: aws.String(csiAddon),
+	}, testAccountID)
+	require.NoError(t, err)
+	assert.Equal(t, csiAWSVersion, aws.StringValue(desc.Addon.AddonVersion))
+}
+
+func TestAddonVersionRefusal_NamesTheParameterAndListsSupportedValues(t *testing.T) {
+	t.Parallel()
+	svc, _ := setupAddonService(t)
+	_, err := svc.CreateAddon(context.Background(), &eks.CreateAddonInput{
+		ClusterName: aws.String("c1"), AddonName: aws.String("argocd"),
+	}, testAccountID)
+	require.NoError(t, err)
+
+	refusals := map[string]func() error{
+		"create": func() error {
+			_, e := svc.CreateAddon(context.Background(), &eks.CreateAddonInput{
+				ClusterName: aws.String("c1"), AddonName: aws.String(csiAddon),
+				AddonVersion: aws.String(csiWrongBuild),
+			}, testAccountID)
+			return e
+		},
+		"update": func() error {
+			_, e := svc.UpdateAddon(context.Background(), &eks.UpdateAddonInput{
+				ClusterName: aws.String("c1"), AddonName: aws.String("argocd"),
+				AddonVersion: aws.String("v3.0.23-eksbuild.2"),
+			}, testAccountID)
+			return e
+		},
+	}
+	want := map[string]string{"create": csiAWSVersion, "update": "v3.0.23-eksbuild.1"}
+	for name, refuse := range refusals {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+			err := refuse()
+			require.Error(t, err)
+			code, message, ok := awserrors.ResolveErrorDetail(err)
+			require.True(t, ok)
+			assert.Equal(t, awserrors.ErrorInvalidParameterValue, code, "this change leaves the error code alone")
+			assert.Contains(t, message, versionParam)
+			assert.Contains(t, message, want[name], "the supported AWS-form value is listed")
+		})
+	}
+}
+
+func TestAddonBareVersionInput_LogsItsUseAtInfo(t *testing.T) {
+	var buf bytes.Buffer
+	restore := slog.Default()
+	slog.SetDefault(slog.New(slog.NewTextHandler(&buf, &slog.HandlerOptions{Level: slog.LevelInfo})))
+	t.Cleanup(func() { slog.SetDefault(restore) })
+
+	svc, _ := setupAddonService(t)
+	_, err := svc.CreateAddon(context.Background(), &eks.CreateAddonInput{
+		ClusterName: aws.String("c1"), AddonName: aws.String(csiAddon),
+	}, testAccountID)
+	require.NoError(t, err)
+
+	buf.Reset()
+	_, err = svc.UpdateAddon(context.Background(), &eks.UpdateAddonInput{
+		ClusterName: aws.String("c1"), AddonName: aws.String(csiAddon),
+		AddonVersion: aws.String(csiBare),
+	}, testAccountID)
+	require.NoError(t, err)
+	logged := buf.String()
+	assert.Contains(t, logged, "level=INFO")
+	assert.Contains(t, logged, "bare")
+	assert.Contains(t, logged, csiAddon)
+	assert.Contains(t, logged, "c1")
+
+	buf.Reset()
+	_, err = svc.UpdateAddon(context.Background(), &eks.UpdateAddonInput{
+		ClusterName: aws.String("c1"), AddonName: aws.String(csiAddon),
+		AddonVersion: aws.String(csiAWSVersion),
+	}, testAccountID)
+	require.NoError(t, err, notBeforeAWSOK)
+	assert.NotContains(t, buf.String(), "bare", "the AWS form is the intended form and logs nothing")
+}

spinifex/domains/eks/cluster/addons.go

@@ -2,8 +2,12 @@ package cluster
 
 import (
 	"context"
+	"encoding/base64"
 	"errors"
 	"fmt"
+	"log/slog"
+	"slices"
+	"strings"
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/eks"
@@ -35,15 +39,20 @@ func (s *EKSServiceImpl) ListAddons(ctx context.Context, input *eks.ListAddonsIn
 	return &eks.ListAddonsOutput{Addons: aws.StringSlice(names)}, nil
 }
 
-// DescribeAddonVersions returns the static add-on catalog, optionally filtered by name.
-func (s *EKSServiceImpl) DescribeAddonVersions(ctx context.Context, input *eks.DescribeAddonVersionsInput, _ string) (*eks.DescribeAddonVersionsOutput, error) {
-	filter := ""
-	if input != nil {
-		filter = aws.StringValue(input.AddonName)
+// DescribeAddonVersions returns the add-on catalogue filtered by name, owner,
+// publisher, type and the Kubernetes version its versions support, one page
+// of add-ons at a time.
+func (s *EKSServiceImpl) DescribeAddonVersions(_ context.Context, input *eks.DescribeAddonVersionsInput, _ string) (*eks.DescribeAddonVersionsOutput, error) {
+	if input == nil {
+		input = &eks.DescribeAddonVersionsInput{}
 	}
-	specs := addon.Specs()
-	out := make([]*eks.AddonInfo, 0, len(specs))
-	for _, spec := range specs {
+	// Every catalogue version supports only the K3s line the node image ships.
+	if v := aws.StringValue(input.KubernetesVersion); v != "" && v != defaultK8sVersion {
+		return &eks.DescribeAddonVersionsOutput{Addons: []*eks.AddonInfo{}}, nil
+	}
+	filter := aws.StringValue(input.AddonName)
+	var matched []addon.Spec
+	for _, spec := range addon.Specs() {
 		if filter != "" && spec.Name != filter {
 			continue
 		}
@@ -52,9 +61,51 @@ func (s *EKSServiceImpl) DescribeAddonVersions(ctx context.Context, input *eks.D
 		if spec.Hidden && filter == "" {
 			continue
 		}
+		if !matchesAny(input.Types, spec.Type) || !matchesAny(input.Owners, addonOwner) ||
+			!matchesAny(input.Publishers, addonPublisher) {
+			continue
+		}
+		matched = append(matched, spec)
+	}
+	page, next, err := pageAddons(matched, input.MaxResults, input.NextToken)
+	if err != nil {
+		return nil, err
+	}
+	out := make([]*eks.AddonInfo, 0, len(page))
+	for _, spec := range page {
 		out = append(out, addonSpecToAWS(spec))
 	}
-	return &eks.DescribeAddonVersionsOutput{Addons: out}, nil
+	return &eks.DescribeAddonVersionsOutput{Addons: out, NextToken: next}, nil
+}
+
+// matchesAny reports whether the filter is empty or lists value.
+func matchesAny(filter []*string, value string) bool {
+	return len(filter) == 0 || slices.Contains(aws.StringValueSlice(filter), value)
+}
+
+// maxAddonPage bounds maxResults, the model's range being 1 to 100.
+const maxAddonPage = 100
+
+// pageAddons returns the add-ons after the token, at most maxResults of them.
+// The token is the base64 name of the last add-on returned, opaque to clients.
+func pageAddons(specs []addon.Spec, maxResults *int64, token *string) ([]addon.Spec, *string, error) {
+	if t := aws.StringValue(token); t != "" {
+		raw, err := base64.RawURLEncoding.DecodeString(t)
+		if err != nil {
+			return nil, nil, awserrors.Errorf(awserrors.ErrorEKSInvalidParameter, "nextToken is not valid")
+		}
+		after := string(raw)
+		specs = slices.DeleteFunc(slices.Clone(specs), func(sp addon.Spec) bool { return sp.Name <= after })
+	}
+	limit := len(specs)
+	if maxResults != nil {
+		limit = min(max(int(*maxResults), 1), maxAddonPage)
+	}
+	if limit >= len(specs) {
+		return specs, nil, nil
+	}
+	next := base64.RawURLEncoding.EncodeToString([]byte(specs[limit-1].Name))
+	return specs[:limit], &next, nil
 }
 
 // validateAddonServiceAccountRoleArn rejects a non-empty value that is not
@@ -89,11 +140,12 @@ func (s *EKSServiceImpl) CreateAddon(ctx context.Context, input *eks.CreateAddon
 	if !ok {
 		return nil, errors.New(awserrors.ErrorInvalidParameterValue)
 	}
-	version := aws.StringValue(input.AddonVersion)
-	if version == "" {
-		version = spec.DefaultVersion
-	} else if !spec.SupportsVersion(version) {
-		return nil, errors.New(awserrors.ErrorInvalidParameterValue)
+	version := spec.DefaultVersion
+	if requested := aws.StringValue(input.AddonVersion); requested != "" {
+		var err error
+		if version, err = resolveAddonVersion(ctx, spec, requested, cluster); err != nil {
+			return nil, err
+		}
 	}
 	acctKV, err := s.acctKVForNewDependant(ctx, accountID, cluster)
 	if err != nil {
@@ -197,9 +249,13 @@ func (s *EKSServiceImpl) UpdateAddon(ctx context.Context, input *eks.UpdateAddon
 	version := aws.StringValue(input.AddonVersion)
 	if version != "" {
 		spec, ok := addon.Lookup(addonName)
-		if !ok || !spec.SupportsVersion(version) {
+		if !ok {
 			return nil, errors.New(awserrors.ErrorInvalidParameterValue)
 		}
+		var err error
+		if version, err = resolveAddonVersion(ctx, spec, version, cluster); err != nil {
+			return nil, err
+		}
 	}
 	rec, err := s.addons().Update(ctx, acctKV, accountID, cluster, addonName, addon.Change{
 		Version:               version,
@@ -246,7 +302,7 @@ func addonRecordToAWS(cluster string, rec *addon.Record) *eks.Addon {
 	out := &eks.Addon{
 		AddonArn:     aws.String(rec.Arn),
 		AddonName:    aws.String(rec.AddonName),
-		AddonVersion: aws.String(rec.AddonVersion),
+		AddonVersion: aws.String(awsAddonVersion(rec)),
 		ClusterName:  aws.String(cluster),
 		Status:       aws.String(string(rec.Status)),
 		CreatedAt:    aws.Time(rec.CreatedAt),
@@ -269,17 +325,36 @@ func addonRecordToAWS(cluster string, rec *addon.Record) *eks.Addon {
 	return out
 }
 
-// addonSpecToAWS converts a catalog spec to the SDK AddonInfo shape.
+// The owner and publisher Spinifex reports for every catalogue add-on, as AWS
+// does for the add-ons it publishes.
+const (
+	addonOwner     = "aws"
+	addonPublisher = "eks"
+)
+
+// addonSpecToAWS converts a catalog spec to the SDK AddonInfo shape. Every
+// version supports the one cluster version the node image ships, and exactly
+// one of them is that cluster version's default.
 func addonSpecToAWS(spec addon.Spec) *eks.AddonInfo {
 	versions := make([]*eks.AddonVersionInfo, 0, len(spec.Versions))
-	for _, v := range spec.Versions {
+	for _, v := range spec.AWSVersions() {
 		versions = append(versions, &eks.AddonVersionInfo{
 			AddonVersion:           aws.String(v),
+			Architecture:           aws.StringSlice([]string{"amd64"}),
+			RequiresConfiguration:  aws.Bool(false),
 			RequiresIamPermissions: aws.Bool(spec.RequiresIRSA),
+			Compatibilities: []*eks.Compatibility{{
+				ClusterVersion:   aws.String(defaultK8sVersion),
+				PlatformVersions: aws.StringSlice([]string{"*"}),
+				DefaultVersion:   aws.Bool(v == spec.AWSVersion(spec.DefaultVersion)),
+			}},
 		})
 	}
 	return &eks.AddonInfo{
 		AddonName:     aws.String(spec.Name),
+		Type:          aws.String(spec.Type),
+		Owner:         aws.String(addonOwner),
+		Publisher:     aws.String(addonPublisher),
 		AddonVersions: versions,
 	}
 }
@@ -308,3 +383,30 @@ func (s *EKSServiceImpl) addonBucket(ctx context.Context, accountID string) (jet
 	}
 	return GetOrCreateAccountBucket(ctx, js, accountID)
 }
+
+// resolveAddonVersion maps a client version, in AWS form or bare upstream form,
+// to the upstream form the record and the bundle lookup use. The bare form is
+// more lenient than AWS, so each use is logged.
+func resolveAddonVersion(ctx context.Context, spec addon.Spec, requested, cluster string) (string, error) {
+	upstream, ok := spec.ParseVersion(requested)
+	if !ok {
+		return "", awserrors.Errorf(awserrors.ErrorInvalidParameterValue,
+			"addonVersion %q is not supported for add-on %s; supported versions: %s",
+			requested, spec.Name, strings.Join(spec.AWSVersions(), ", "))
+	}
+	if requested == upstream {
+		slog.InfoContext(ctx, "EKS add-on version given in bare upstream form",
+			"addon", spec.Name, "cluster", cluster, "version", requested)
+	}
+	return upstream, nil
+}
+
+// awsAddonVersion returns a stored record's version in AWS form; an add-on the
+// catalogue no longer lists keeps its stored string.
+func awsAddonVersion(rec *addon.Record) string {
+	spec, ok := addon.Lookup(rec.AddonName)
+	if !ok {
+		return rec.AddonVersion
+	}
+	return spec.AWSVersion(rec.AddonVersion)
+}

spinifex/domains/eks/cluster/addons_test.go

@@ -98,7 +98,9 @@ func TestCreateAddon_UnknownVersionRejected(t *testing.T) {
 		ClusterName: aws.String("c1"), AddonName: aws.String(albController),
 		AddonVersion: aws.String("9.9.9-nope"),
 	}, testAccountID)
-	require.EqualError(t, err, awserrors.ErrorInvalidParameterValue)
+	code, ok := awserrors.ResolveErrorCode(err)
+	require.True(t, ok)
+	assert.Equal(t, awserrors.ErrorInvalidParameterValue, code)
 }
 
 func TestCreateAddon_CreatesStagingAndDefaultsVersion(t *testing.T) {
@@ -113,7 +115,7 @@ func TestCreateAddon_CreatesStagingAndDefaultsVersion(t *testing.T) {
 	require.NotNil(t, out.Addon)
 	// Version defaults to catalog default; status starts CREATING (honest until
 	// VM-side delivery confirms ACTIVE).
-	assert.Equal(t, spec.DefaultVersion, aws.StringValue(out.Addon.AddonVersion))
+	assert.Equal(t, spec.AWSVersion(spec.DefaultVersion), aws.StringValue(out.Addon.AddonVersion))
 	assert.Equal(t, string(addon.StatusCreating), aws.StringValue(out.Addon.Status))
 	assert.Contains(t, aws.StringValue(out.Addon.AddonArn), ":addon/c1/"+albController)
 
@@ -214,7 +216,7 @@ func TestUpdateAddon_ChangesVersionAndReinstalls(t *testing.T) {
 	}, testAccountID)
 	require.NoError(t, err)
 	assert.Equal(t, `{"replicaCount":2}`, aws.StringValue(desc.Addon.ConfigurationValues))
-	assert.Equal(t, spec.DefaultVersion, aws.StringValue(desc.Addon.AddonVersion))
+	assert.Equal(t, spec.AWSVersion(spec.DefaultVersion), aws.StringValue(desc.Addon.AddonVersion))
 	assert.Equal(t, string(addon.StatusUpdating), aws.StringValue(desc.Addon.Status))
 }
 
@@ -230,7 +232,9 @@ func TestUpdateAddon_UnknownVersionRejected(t *testing.T) {
 		ClusterName: aws.String("c1"), AddonName: aws.String("argocd"),
 		AddonVersion: aws.String("9.9.9-nope"),
 	}, testAccountID)
-	require.EqualError(t, err, awserrors.ErrorInvalidParameterValue)
+	code, ok := awserrors.ResolveErrorCode(err)
+	require.True(t, ok)
+	assert.Equal(t, awserrors.ErrorInvalidParameterValue, code)
 }
 
 func TestUpdateAddon_MissingIsNotFound(t *testing.T) {

spinifex/domains/elbv2/haproxy.go

@@ -46,7 +46,7 @@ backend {{.Name}}
 {{- if .Redirect}}
     http-request redirect {{if .Redirect.Scheme}}scheme {{.Redirect.Scheme}}{{else}}location {{.Redirect.Location}}{{end}} code {{.Redirect.Code}}
 {{- else if .FixedResponse}}
-    http-request return status {{.FixedResponse.StatusCode}}{{if .FixedResponse.ContentType}} content-type "{{.FixedResponse.ContentType}}"{{end}}{{if .FixedResponse.Body}} string "{{.FixedResponse.Body}}"{{end}}
+    http-request return status {{.FixedResponse.StatusCode}}{{if .FixedResponse.ContentType}} content-type {{squote .FixedResponse.ContentType}}{{end}}{{if .FixedResponse.Body}} string {{squote .FixedResponse.Body}}{{end}}
 {{- else}}
     balance roundrobin{{if .HealthCheckPath}}
     option httpchk
@@ -65,7 +65,14 @@ backend {{.Name}}
 {{end}}
 `
 
-var haproxyHTTPTmpl = template.Must(template.New("haproxy").Parse(haproxyConfigTemplate))
+var haproxyHTTPTmpl = template.Must(template.New("haproxy").Funcs(template.FuncMap{"squote": haproxySingleQuote}).Parse(haproxyConfigTemplate))
+
+// haproxySingleQuote renders s as one literal HAProxy word. HAProxy expands
+// environment variables inside double quotes but not single quotes, so tenant
+// text is single-quoted with each embedded quote closed, escaped and reopened.
+func haproxySingleQuote(s string) string {
+	return "'" + strings.ReplaceAll(s, "'", `'\''`) + "'"
+}
 
 // HAProxyConfig holds the full configuration for rendering the HAProxy template.
 type HAProxyConfig struct {
@@ -566,17 +573,10 @@ func validHTTPStatusCode(s string) bool {
 	return true
 }
 
-// validFixedResponseBody rejects bytes that would terminate or inject into the HAProxy directive.
+// validFixedResponseBody rejects bytes that would end the single-line HAProxy
+// directive the single-quoted body is rendered into.
 func validFixedResponseBody(s string) bool {
-	if s == "" {
-		return false
-	}
-	for _, r := range s {
-		if r == '\n' || r == '\r' || r == '"' || r == '\\' || r == '#' {
-			return false
-		}
-	}
-	return true
+	return s != "" && !strings.ContainsAny(s, "\n\r\x00")
 }
 
 // buildRedirect translates a RedirectAction into an HAProxy redirect directive.

spinifex/domains/elbv2/haproxy_test.go

@@ -279,7 +279,7 @@ func TestGenerateHAProxyConfig_FixedResponseDefault(t *testing.T) {
 	// Synthetic default backend named off the listener ARN, returning the fixed response.
 	assert.Contains(t, config, "default_backend bkdefault_lst-616760baf6a3031c3")
 	assert.Contains(t, config, "backend bkdefault_lst-616760baf6a3031c3")
-	assert.Contains(t, config, `http-request return status 404 content-type "text/plain" string "no route"`)
+	assert.Contains(t, config, `http-request return status 404 content-type 'text/plain' string 'no route'`)
 	// Host-header rule still routes to the app backend.
 	assert.Contains(t, config, "use_backend bk_tg-app1 if")
 	assert.Contains(t, config, "10.42.0.9:8080")
@@ -309,7 +309,59 @@ func TestGenerateHAProxyConfig_FixedResponseRejectsUnsafeBody(t *testing.T) {
 
 	assert.Contains(t, config, "http-request return status 200")
 	assert.NotContains(t, config, "always_true")
-	assert.NotContains(t, config, `string "evil`)
+	assert.NotContains(t, config, `string 'evil`)
+}
+
+// TestGenerateHAProxyConfig_FixedResponseIsLiteral renders the content type and
+// body in single quotes, where HAProxy performs no environment expansion, so a
+// body referencing the lb-agent's environment is served verbatim.
+func TestGenerateHAProxyConfig_FixedResponseIsLiteral(t *testing.T) {
+	t.Parallel()
+	lb := &LoadBalancerRecord{LoadBalancerID: "lb-env"}
+	listeners := []*ListenerRecord{
+		{
+			ListenerArn: "arn:lst-env",
+			Port:        80,
+			DefaultActions: []ListenerAction{
+				{Type: ActionTypeFixedResponse, FixedResponse: &FixedResponseAction{
+					StatusCode:  "200",
+					ContentType: "text/plain",
+					MessageBody: `sk=${LB_SECRET_KEY} ak=$LB_ACCESS_KEY it's "q" \ #x`,
+				}},
+			},
+		},
+	}
+
+	config, _, err := GenerateHAProxyConfigWithCerts(lb, listeners, nil, nil, "0.0.0.0", nil)
+	require.NoError(t, err)
+
+	assert.Contains(t, config, `http-request return status 200 content-type 'text/plain' string 'sk=${LB_SECRET_KEY} ak=$LB_ACCESS_KEY it'\''s "q" \ #x'`+"\n")
+	assert.NotContains(t, config, `"${LB_SECRET_KEY}`)
+}
+
+// TestGenerateHAProxyConfig_FixedResponseKeepsStoredTab keeps a stored body with
+// a tab, which is literal inside single quotes and rendered before API checks existed.
+func TestGenerateHAProxyConfig_FixedResponseKeepsStoredTab(t *testing.T) {
+	t.Parallel()
+	lb := &LoadBalancerRecord{LoadBalancerID: "lb-tab"}
+	listeners := []*ListenerRecord{
+		{
+			ListenerArn: "arn:lst-tab",
+			Port:        80,
+			DefaultActions: []ListenerAction{
+				{Type: ActionTypeFixedResponse, FixedResponse: &FixedResponseAction{
+					StatusCode:  "503",
+					ContentType: "text/html",
+					MessageBody: "<pre>\tdown</pre>",
+				}},
+			},
+		},
+	}
+
+	config, _, err := GenerateHAProxyConfigWithCerts(lb, listeners, nil, nil, "0.0.0.0", nil)
+	require.NoError(t, err)
+
+	assert.Contains(t, config, "http-request return status 503 content-type 'text/html' string '<pre>\tdown</pre>'\n")
 }
 
 // TestGenerateHAProxyConfig_RedirectSchemeDefault renders the canonical

spinifex/domains/elbv2/service_impl.go

@@ -17,6 +17,8 @@ import (
 	"strings"
 	"sync"
 	"time"
+	"unicode"
+	"unicode/utf8"
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/ec2"
@@ -2701,8 +2703,9 @@ func listenerActionFromSDK(a *elbv2.Action) ListenerAction {
 	return action
 }
 
-// validateListenerAction enforces the per-type action contract. Redirect fields
-// are validated here so invalid input fails at the API, not at render time.
+// validateListenerAction enforces the per-type action contract. Redirect and
+// fixed-response fields are validated here so invalid input fails at the API,
+// not at render time.
 func validateListenerAction(a ListenerAction) error {
 	if a.Type == ActionTypeRedirect {
 		if a.Redirect == nil {
@@ -2710,6 +2713,39 @@ func validateListenerAction(a ListenerAction) error {
 		}
 		return validateRedirectAction(a.Redirect)
 	}
+	if a.Type == ActionTypeFixedResponse {
+		return validateFixedResponseAction(a.FixedResponse)
+	}
+	return nil
+}
+
+// fixedResponseContentTypes are the only ContentType values ELBv2 accepts.
+var fixedResponseContentTypes = map[string]bool{
+	"text/plain":             true,
+	"text/css":               true,
+	"text/html":              true,
+	"application/javascript": true,
+	"application/json":       true,
+}
+
+// maxFixedResponseBodyLen is ELBv2's MessageBody length limit in characters.
+const maxFixedResponseBodyLen = 1024
+
+// validateFixedResponseAction rejects a ContentType ELBv2 does not allow, or a
+// MessageBody that is too long or holds control characters.
+func validateFixedResponseAction(fr *FixedResponseAction) error {
+	if fr == nil {
+		return errors.New(awserrors.ErrorMissingParameter)
+	}
+	if fr.ContentType != "" && !fixedResponseContentTypes[fr.ContentType] {
+		return errors.New(awserrors.ErrorInvalidParameterValue)
+	}
+	if utf8.RuneCountInString(fr.MessageBody) > maxFixedResponseBodyLen {
+		return errors.New(awserrors.ErrorInvalidParameterValue)
+	}
+	if strings.ContainsFunc(fr.MessageBody, unicode.IsControl) {
+		return errors.New(awserrors.ErrorInvalidParameterValue)
+	}
 	return nil
 }
 
@@ -3503,14 +3539,18 @@ func (s *ELBv2ServiceImpl) tgRecordToSDK(r *TargetGroupRecord, lbArns []string)
 		HealthCheckEnabled:         aws.Bool(r.HealthCheck.Enabled),
 		HealthCheckProtocol:        aws.String(r.HealthCheck.Protocol),
 		HealthCheckPort:            aws.String(r.HealthCheck.Port),
-		HealthCheckPath:            aws.String(r.HealthCheck.Path),
 		HealthCheckIntervalSeconds: aws.Int64(r.HealthCheck.IntervalSeconds),
 		HealthCheckTimeoutSeconds:  aws.Int64(r.HealthCheck.TimeoutSeconds),
 		HealthyThresholdCount:      aws.Int64(r.HealthCheck.HealthyThreshold),
 		UnhealthyThresholdCount:    aws.Int64(r.HealthCheck.UnhealthyThreshold),
-		Matcher: &elbv2.Matcher{
-			HttpCode: aws.String(r.HealthCheck.Matcher),
-		},
+	}
+
+	// AWS omits both keys on TCP target groups, which have no path or matcher.
+	if r.HealthCheck.Path != "" {
+		tg.HealthCheckPath = aws.String(r.HealthCheck.Path)
+	}
+	if r.HealthCheck.Matcher != "" {
+		tg.Matcher = &elbv2.Matcher{HttpCode: aws.String(r.HealthCheck.Matcher)}
 	}
 
 	// Omitted for NLB (TCP/UDP/TLS) target groups, matching AWS, which returns

spinifex/domains/elbv2/service_impl_rules.go

@@ -591,8 +591,8 @@ func (s *ELBv2ServiceImpl) validateAndConvertRuleActions(ctx context.Context, in
 				return nil, err
 			}
 		case ActionTypeFixedResponse:
-			if action.FixedResponse == nil {
-				return nil, errors.New(awserrors.ErrorMissingParameter)
+			if err := validateFixedResponseAction(action.FixedResponse); err != nil {
+				return nil, err
 			}
 		default:
 			return nil, errors.New(awserrors.ErrorELBv2InvalidConfigurationRequest)

spinifex/domains/elbv2/service_impl_rules_test.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/elbv2"
+	awserrors "github.com/mulgadc/spinifex/spinifex/foundation/aws/errors"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -660,3 +661,53 @@ func sortByPriority(rs []*RuleRecord) []*RuleRecord {
 	}
 	return out
 }
+
+func TestValidateListenerAction_FixedResponse(t *testing.T) {
+	t.Parallel()
+	fixed := func(ct, body string) ListenerAction {
+		return ListenerAction{Type: ActionTypeFixedResponse, FixedResponse: &FixedResponseAction{
+			StatusCode: "200", ContentType: ct, MessageBody: body,
+		}}
+	}
+
+	for _, ct := range []string{"", "text/plain", "text/css", "text/html", "application/javascript", "application/json"} {
+		assert.NoError(t, validateListenerAction(fixed(ct, "ok")), ct)
+	}
+	assert.NoError(t, validateListenerAction(fixed("text/plain", "")))
+	assert.NoError(t, validateListenerAction(fixed("text/plain", "sk=${LB_SECRET_KEY} it's \"q\" #x")))
+	assert.NoError(t, validateListenerAction(fixed("text/plain", strings.Repeat("é", 1024))))
+
+	bad := []ListenerAction{
+		fixed("text/$LB_SECRET_KEY", "ok"),
+		fixed("text/xml", "ok"),
+		fixed("TEXT/PLAIN", "ok"),
+		fixed("text/plain", strings.Repeat("a", 1025)),
+		fixed("text/plain", "line\nacl x always_true"),
+		fixed("text/plain", "cr\r"),
+		fixed("text/plain", "nul\x00"),
+	}
+	for _, a := range bad {
+		assert.EqualError(t, validateListenerAction(a), awserrors.ErrorInvalidParameterValue, "%+v", *a.FixedResponse)
+	}
+	assert.EqualError(t, validateListenerAction(ListenerAction{Type: ActionTypeFixedResponse}), awserrors.ErrorMissingParameter)
+}
+
+func TestCreateRule_RejectsFixedResponseContentType(t *testing.T) {
+	t.Parallel()
+	env := newRuleTestEnv(t, "cr-badfixed")
+
+	_, err := env.svc.CreateRule(context.Background(), &elbv2.CreateRuleInput{
+		ListenerArn: aws.String(env.listenerArn),
+		Priority:    aws.Int64(1),
+		Conditions:  []*elbv2.RuleCondition{{Field: aws.String("path-pattern"), Values: aws.StringSlice([]string{"/x"})}},
+		Actions: []*elbv2.Action{{
+			Type: aws.String("fixed-response"),
+			FixedResponseConfig: &elbv2.FixedResponseActionConfig{
+				StatusCode:  aws.String("200"),
+				ContentType: aws.String("text/$LB_SECRET_KEY"),
+				MessageBody: aws.String("ok"),
+			},
+		}},
+	}, testAccountID)
+	require.EqualError(t, err, awserrors.ErrorInvalidParameterValue)
+}

spinifex/domains/elbv2/service_impl_test.go

@@ -459,11 +459,12 @@ func TestCreateTargetGroup_TCPProtocol(t *testing.T) {
 	assert.Equal(t, int64(5432), *tg.Port)
 	// NLB health check defaults: TCP protocol, no path, no matcher.
 	assert.Equal(t, "TCP", *tg.HealthCheckProtocol)
-	assert.Empty(t, *tg.HealthCheckPath)
-	assert.Empty(t, *tg.Matcher.HttpCode)
+	assert.Nil(t, tg.HealthCheckPath)
+	assert.Nil(t, tg.Matcher)
+	assert.Equal(t, int64(30), *tg.HealthCheckIntervalSeconds)
 	assert.Equal(t, int64(10), *tg.HealthCheckTimeoutSeconds)
-	assert.Equal(t, int64(3), *tg.HealthyThresholdCount)
-	assert.Equal(t, int64(3), *tg.UnhealthyThresholdCount)
+	assert.Equal(t, int64(5), *tg.HealthyThresholdCount)
+	assert.Equal(t, int64(2), *tg.UnhealthyThresholdCount)
 }
 
 func TestCreateTargetGroup_NLBProtocols(t *testing.T) {

spinifex/domains/elbv2/types.go

@@ -99,8 +99,8 @@ const (
 	// Default health check values (NLB).
 	DefaultNLBHealthCheckInterval = 30
 	DefaultNLBHealthCheckTimeout  = 10
-	DefaultNLBHealthyThreshold    = 3
-	DefaultNLBUnhealthyThreshold  = 3
+	DefaultNLBHealthyThreshold    = 5
+	DefaultNLBUnhealthyThreshold  = 2
 	DefaultNLBHealthCheckProtocol = ProtocolTCP
 	DefaultNLBHealthCheckPort     = "traffic-port"
 

spinifex/domains/network/ovn/client.go

@@ -88,7 +88,6 @@ type Client interface {
 	CreateDHCPOptions(ctx context.Context, opts *nbdb.DHCPOptions) (string, error)
 	UpdateDHCPOptionsOptions(ctx context.Context, uuid string, options map[string]string) error
 	DeleteDHCPOptions(ctx context.Context, uuid string) error
-	FindDHCPOptionsByCIDR(ctx context.Context, cidr string) (*nbdb.DHCPOptions, error)
 	FindDHCPOptionsByExternalID(ctx context.Context, key, value string) (*nbdb.DHCPOptions, error)
 	ListDHCPOptions(ctx context.Context) ([]nbdb.DHCPOptions, error)
 

spinifex/domains/network/ovn/conformance_test.go

@@ -573,31 +573,26 @@ func conformDHCPOptions(t *testing.T, cli ovn.Client, ctx context.Context) {
 	require.NoError(t, err)
 	require.NotEmpty(t, uuid, "create must return the row UUID callers store")
 
-	byCIDR, err := cli.FindDHCPOptionsByCIDR(ctx, "10.0.1.0/24")
-	require.NoError(t, err)
-	assert.Equal(t, uuid, byCIDR.UUID)
-	assert.Equal(t, "10.0.1.1", byCIDR.Options["server_id"])
-
 	byExtID, err := cli.FindDHCPOptionsByExternalID(ctx, "spinifex:subnet_id", "subnet-1")
 	require.NoError(t, err)
 	assert.Equal(t, uuid, byExtID.UUID)
+	assert.Equal(t, "10.0.1.0/24", byExtID.CIDR)
+	assert.Equal(t, "10.0.1.1", byExtID.Options["server_id"])
 
 	require.NoError(t, cli.UpdateDHCPOptionsOptions(ctx, uuid, map[string]string{
 		"server_id": "10.0.1.1", "lease_time": "7200",
 	}))
-	byCIDR, err = cli.FindDHCPOptionsByCIDR(ctx, "10.0.1.0/24")
+	byExtID, err = cli.FindDHCPOptionsByExternalID(ctx, "spinifex:subnet_id", "subnet-1")
 	require.NoError(t, err)
-	assert.Equal(t, "7200", byCIDR.Options["lease_time"], "update must replace the options map")
+	assert.Equal(t, "7200", byExtID.Options["lease_time"], "update must replace the options map")
 
 	all, err := cli.ListDHCPOptions(ctx)
 	require.NoError(t, err)
 	assert.Len(t, all, 1)
 
-	_, err = cli.FindDHCPOptionsByCIDR(ctx, "10.9.9.0/24")
-	require.Error(t, err)
-	assert.Contains(t, err.Error(), "10.9.9.0/24")
 	_, err = cli.FindDHCPOptionsByExternalID(ctx, "spinifex:subnet_id", "subnet-nope")
-	assert.Error(t, err)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "subnet-nope")
 
 	require.NoError(t, cli.DeleteDHCPOptions(ctx, uuid))
 	all, err = cli.ListDHCPOptions(ctx)

spinifex/domains/network/ovn/live.go

@@ -826,20 +826,6 @@ func (c *LiveClient) DeleteDHCPOptions(ctx context.Context, uuid string) error {
 	return nil
 }
 
-func (c *LiveClient) FindDHCPOptionsByCIDR(ctx context.Context, cidr string) (*nbdb.DHCPOptions, error) {
-	var options []nbdb.DHCPOptions
-	err := c.client.WhereCache(func(o *nbdb.DHCPOptions) bool {
-		return o.CIDR == cidr
-	}).List(ctx, &options)
-	if err != nil {
-		return nil, fmt.Errorf("find DHCP options by CIDR: %w", err)
-	}
-	if len(options) == 0 {
-		return nil, fmt.Errorf("DHCP options for CIDR %q not found", cidr)
-	}
-	return &options[0], nil
-}
-
 func (c *LiveClient) FindDHCPOptionsByExternalID(ctx context.Context, key, value string) (*nbdb.DHCPOptions, error) {
 	var options []nbdb.DHCPOptions
 	err := c.client.WhereCache(func(o *nbdb.DHCPOptions) bool {

spinifex/domains/network/ovn/mock/mock.go

@@ -443,18 +443,6 @@ func (m *Client) DeleteDHCPOptions(_ context.Context, uuid string) error {
 	return nil
 }
 
-func (m *Client) FindDHCPOptionsByCIDR(_ context.Context, cidr string) (*nbdb.DHCPOptions, error) {
-	m.Mu.Lock()
-	defer m.Mu.Unlock()
-	for _, opts := range m.DHCPOpts {
-		if opts.CIDR == cidr {
-			result := *opts
-			return &result, nil
-		}
-	}
-	return nil, fmt.Errorf("DHCP options for CIDR %q not found", cidr)
-}
-
 func (m *Client) FindDHCPOptionsByExternalID(_ context.Context, key, value string) (*nbdb.DHCPOptions, error) {
 	m.Mu.Lock()
 	defer m.Mu.Unlock()

spinifex/domains/network/reconcile/apply.go

@@ -374,7 +374,8 @@ func (r *reconciler) applyPorts(ctx context.Context, intent IntentState, actual
 			continue
 		}
 
-		if _, err := r.ovn.GetLogicalSwitchPort(ctx, portName); err != nil {
+		existing, err := r.ovn.GetLogicalSwitchPort(ctx, portName)
+		if err != nil {
 			addrStr := spec.MAC.String() + " " + spec.PrivateIP.String()
 			lsp := &nbdb.LogicalSwitchPort{
 				Name:         portName,
@@ -398,6 +399,10 @@ func (r *reconciler) applyPorts(ctx context.Context, intent IntentState, actual
 			continue
 		}
 
+		if !spec.SuppressDHCP {
+			r.convergePortDHCP(ctx, existing, spec.SubnetID, res)
+		}
+
 		currentPGs, err := r.ovn.ListPortGroupsForPort(ctx, portName)
 		if err != nil {
 			slog.Warn("reconcile/apply: list port groups for port failed", "port", portName, "err", err)
@@ -420,6 +425,26 @@ func (r *reconciler) applyPorts(ctx context.Context, intent IntentState, actual
 	r.pruneOrphanPorts(ctx, intent, res)
 }
 
+// convergePortDHCP points an existing LSP's dhcpv4_options at its subnet's
+// current DHCP_Options row. The reference is weak, so deleting or recreating
+// the row leaves the port without OVN DHCP until it is re-linked here.
+func (r *reconciler) convergePortDHCP(ctx context.Context, lsp *nbdb.LogicalSwitchPort, subnetID string, res *passResult) {
+	dhcpOpts, err := r.ovn.FindDHCPOptionsByExternalID(ctx, "spinifex:subnet_id", subnetID)
+	if err != nil || dhcpOpts == nil {
+		return
+	}
+	if lsp.DHCPv4Options != nil && *lsp.DHCPv4Options == dhcpOpts.UUID {
+		return
+	}
+	lsp.DHCPv4Options = &dhcpOpts.UUID
+	if err := r.ovn.UpdateLogicalSwitchPort(ctx, lsp); err != nil {
+		slog.Warn("reconcile/apply: re-link port DHCP options failed", "port", lsp.Name, "subnet_id", subnetID, "err", err)
+		res.fail(classPort, lsp.Name, err)
+		return
+	}
+	slog.Info("reconcile/apply: re-linked port DHCP options", "port", lsp.Name, "subnet_id", subnetID)
+}
+
 // pruneOrphanPorts deletes guest LSPs whose spinifex:eni_id has no matching
 // intent ENI, closing the create-only gap that leaks ports across instance
 // terminate and host reinstall. DeletePort clears PG memberships then removes
@@ -872,7 +897,8 @@ func (r *reconciler) forgetIdlePorts(specs []policy.EIPSpec) {
 // never re-asserted, so a vpcd/OVN rebuild dropped it and its guest-port hop never
 // converged (inbound DNATs at the gateway but never reaches the guest; ICMP is
 // answered by the gateway LR, so ping works while TCP hangs). A user EIP on the same
-// private IP wins; the auto-assigned entry is skipped to avoid a duplicate rule.
+// private IP in the same VPC wins; the auto-assigned entry is skipped to avoid a
+// duplicate rule.
 func (r *reconciler) floatingIPSpecs(intent IntentState) []policy.EIPSpec {
 	specs := make([]policy.EIPSpec, 0, len(intent.EIPs)+len(intent.Ports))
 	for _, spec := range intent.EIPs {
@@ -883,7 +909,7 @@ func (r *reconciler) floatingIPSpecs(intent IntentState) []policy.EIPSpec {
 			continue
 		}
 		logicalIP := p.PrivateIP.String()
-		if _, hasEIP := intent.EIPs[logicalIP]; hasEIP {
+		if _, hasEIP := intent.EIPs[eipIntentKey(p.VPCID, logicalIP)]; hasEIP {
 			continue
 		}
 		specs = append(specs, policy.EIPSpec{
@@ -1014,7 +1040,7 @@ func (r *reconciler) applyPublicInstanceEgress(ctx context.Context, intent Inten
 		}
 	}
 	for _, e := range intent.EIPs {
-		r.ensureEIPEgressExemption(ctx, intent, e.VPCID, subnetIDForIP(intent.Subnets, e.LogicalIP), e.LogicalIP, res)
+		r.ensureEIPEgressExemption(ctx, intent, e.VPCID, subnetIDForIP(intent.Subnets, e.VPCID, e.LogicalIP), e.LogicalIP, res)
 	}
 }
 
@@ -1042,14 +1068,15 @@ func (r *reconciler) ensureEIPEgressExemption(ctx context.Context, intent Intent
 	}
 }
 
-// subnetIDForIP returns the SubnetID whose CIDR contains ip, or "" if none match.
-func subnetIDForIP(subnets map[string]topology.SubnetSpec, ip string) string {
+// subnetIDForIP returns the SubnetID in vpcID whose CIDR contains ip, or "" if
+// none match. VPC CIDRs overlap across tenants, so the CIDR alone is ambiguous.
+func subnetIDForIP(subnets map[string]topology.SubnetSpec, vpcID, ip string) string {
 	addr, err := netip.ParseAddr(ip)
 	if err != nil {
 		return ""
 	}
 	for _, s := range subnets {
-		if s.CIDR.IsValid() && s.CIDR.Contains(addr) {
+		if s.VPCID == vpcID && s.CIDR.IsValid() && s.CIDR.Contains(addr) {
 			return s.SubnetID
 		}
 	}

spinifex/domains/network/reconcile/apply_eip_concurrency_test.go

@@ -135,11 +135,11 @@ func TestApplyEIPs_HealthyPortDoesNotWaitForDeadOnes(t *testing.T) {
 	}
 
 	intent := IntentState{EIPs: map[string]policy.EIPSpec{
-		"10.0.0.1": {VPCID: "vpc-a", ExternalIP: "192.0.2.1", LogicalIP: "10.0.0.1", PortName: live},
+		eipIntentKey("vpc-a", "10.0.0.1"): {VPCID: "vpc-a", ExternalIP: "192.0.2.1", LogicalIP: "10.0.0.1", PortName: live},
 	}}
 	for i, name := range dead {
 		ip := "10.0.0." + string(rune('2'+i))
-		intent.EIPs[ip] = policy.EIPSpec{VPCID: "vpc-a", ExternalIP: "192.0.2.9", LogicalIP: ip, PortName: name}
+		intent.EIPs[eipIntentKey("vpc-a", ip)] = policy.EIPSpec{VPCID: "vpc-a", ExternalIP: "192.0.2.9", LogicalIP: ip, PortName: name}
 	}
 
 	res := &passResult{}

spinifex/domains/network/reconcile/apply_eip_overlap_test.go

@@ -0,0 +1,242 @@
+package reconcile
+
+import (
+	"context"
+	"net/netip"
+	"slices"
+	"sync"
+	"testing"
+
+	ec2eip "github.com/mulgadc/spinifex/spinifex/domains/ec2/eip"
+	ec2vpc "github.com/mulgadc/spinifex/spinifex/domains/ec2/vpc"
+	"github.com/mulgadc/spinifex/spinifex/domains/network/external"
+	"github.com/mulgadc/spinifex/spinifex/domains/network/policy"
+	"github.com/mulgadc/spinifex/spinifex/domains/network/topology"
+	"github.com/nats-io/nats.go/jetstream"
+)
+
+// hostBindings stands in for the routed-mode host route and proxy-ARP table.
+type hostBindings struct {
+	mu    sync.Mutex
+	bound map[string]struct{}
+}
+
+func (h *hostBindings) hooks() policy.HostEIPBinder {
+	return policy.HostEIPBinder{
+		Bind: func(eip policy.EIPSpec, _ string) error {
+			h.mu.Lock()
+			defer h.mu.Unlock()
+			h.bound[eip.ExternalIP] = struct{}{}
+			return nil
+		},
+		Unbind: func(externalIP string) error {
+			h.mu.Lock()
+			defer h.mu.Unlock()
+			delete(h.bound, externalIP)
+			return nil
+		},
+		List: func() ([]string, error) {
+			h.mu.Lock()
+			defer h.mu.Unlock()
+			out := make([]string, 0, len(h.bound))
+			for ip := range h.bound {
+				out = append(out, ip)
+			}
+			slices.Sort(out)
+			return out, nil
+		},
+	}
+}
+
+// seedOverlappingTenants writes two accounts' default-style VPCs on the same
+// CIDR, each with one ENI at 172.31.0.4. eni-a carries publicA on its stamp.
+func seedOverlappingTenants(t *testing.T, js jetstream.JetStream, publicA string) {
+	t.Helper()
+	for _, tenant := range []struct{ acct, suffix, mac, public string }{
+		{"111111111111", "a", "02:00:00:00:0a:04", publicA},
+		{"222222222222", "b", "02:00:00:00:0b:04", ""},
+	} {
+		vpcID, subnetID, sgID, eniID := "vpc-"+tenant.suffix, "subnet-"+tenant.suffix, "sg-"+tenant.suffix, "eni-"+tenant.suffix
+		seedKV(t, js, ec2vpc.KVBucketVPCs, tenant.acct+"."+vpcID, ec2vpc.VPCRecord{
+			VpcId: vpcID, CidrBlock: "172.31.0.0/16", AZ: "us-east-1a", VNI: 100,
+		})
+		seedKV(t, js, ec2vpc.KVBucketSubnets, tenant.acct+"."+subnetID, ec2vpc.SubnetRecord{
+			SubnetId: subnetID, VpcId: vpcID, CidrBlock: "172.31.0.0/20",
+		})
+		seedKV(t, js, ec2vpc.KVBucketSecurityGroups, tenant.acct+"."+sgID, ec2vpc.SecurityGroupRecord{
+			GroupId: sgID, VpcId: vpcID,
+		})
+		seedKV(t, js, ec2vpc.KVBucketENIs, tenant.acct+"."+eniID, ec2vpc.ENIRecord{
+			NetworkInterfaceId: eniID, SubnetId: subnetID, VpcId: vpcID,
+			PrivateIpAddress: "172.31.0.4", MacAddress: tenant.mac,
+			SecurityGroupIds: []string{sgID}, PublicIpAddress: tenant.public,
+		})
+	}
+}
+
+func seedAssociatedEIP(t *testing.T, js jetstream.JetStream, acct, suffix, publicIP, mac string) {
+	t.Helper()
+	seedKV(t, js, ec2eip.KVBucketEIPs, acct+".eipalloc-"+suffix, ec2eip.EIPRecord{
+		AllocationId: "eipalloc-" + suffix, PublicIp: publicIP, State: "associated",
+		AssociationId: "eipassoc-" + suffix, ENIId: "eni-" + suffix,
+		PrivateIp: "172.31.0.4", MacAddress: mac, VpcId: "vpc-" + suffix,
+	})
+}
+
+// Two tenants' guests share a private IP on overlapping VPC CIDRs. Tenant B
+// associating its own EIP must leave tenant A's public address, NAT row and
+// host binding alone, whether A's address is a user EIP or auto-assigned.
+func TestReconcile_OverlappingVPCsKeepEachTenantsPublicIP(t *testing.T) {
+	cases := []struct {
+		name    string
+		publicA string
+		eipA    bool
+	}{
+		{name: "eip beside eip", publicA: "203.0.113.10", eipA: true},
+		{name: "eip beside auto-assigned", publicA: "198.51.100.20"},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			ctx := context.Background()
+			js := startKV(t)
+			seedOverlappingTenants(t, js, tc.publicA)
+			if tc.eipA {
+				seedAssociatedEIP(t, js, "111111111111", "a", tc.publicA, "02:00:00:00:0a:04")
+			}
+
+			r, m := newTestReconciler(t)
+			hosts := &hostBindings{bound: map[string]struct{}{}}
+			nat, err := policy.NewNATManager(m, policy.NATModeRouted, policy.WithHostEIPBinder(hosts.hooks()))
+			if err != nil {
+				t.Fatalf("NewNATManager: %v", err)
+			}
+			r.nat = nat
+			r.reloadIntent = func(ctx context.Context) (IntentState, error) {
+				return LoadIntentFromKV(ctx, js, "us-east-1a")
+			}
+
+			pass := func() IntentState {
+				t.Helper()
+				intent, err := r.reloadIntent(ctx)
+				if err != nil {
+					t.Fatalf("LoadIntentFromKV: %v", err)
+				}
+				if err := r.Reconcile(ctx, intent); err != nil {
+					t.Fatalf("Reconcile: %v", err)
+				}
+				if err := r.ReconcileHostEIPs(ctx); err != nil {
+					t.Fatalf("ReconcileHostEIPs: %v", err)
+				}
+				return intent
+			}
+			assertPublic := func(stage, externalIP, port string) {
+				t.Helper()
+				row := findNATByExternal(m, "dnat_and_snat", externalIP)
+				if row == nil {
+					t.Fatalf("%s: dnat_and_snat for %s missing", stage, externalIP)
+				}
+				if got := row.ExternalIDs["spinifex:logical_port"]; got != topology.Port(port) {
+					t.Errorf("%s: %s row owned by %q, want %q", stage, externalIP, got, topology.Port(port))
+				}
+				if _, ok := hosts.bound[externalIP]; !ok {
+					t.Errorf("%s: host binding for %s missing", stage, externalIP)
+				}
+			}
+
+			pass()
+			assertPublic("before tenant B associates", tc.publicA, "eni-a")
+
+			seedAssociatedEIP(t, js, "222222222222", "b", "203.0.113.20", "02:00:00:00:0b:04")
+			for _, stage := range []string{"after tenant B associates", "on the next pass"} {
+				intent := pass()
+				// eni-a's stamp alone would keep A's row alive, so check that
+				// intent itself holds every tenant's association.
+				wantEIPs := map[string]string{eipIntentKey("vpc-b", "172.31.0.4"): "203.0.113.20"}
+				if tc.eipA {
+					wantEIPs[eipIntentKey("vpc-a", "172.31.0.4")] = tc.publicA
+				}
+				if len(intent.EIPs) != len(wantEIPs) {
+					t.Errorf("%s: intent holds %d EIPs, want %d: %+v", stage, len(intent.EIPs), len(wantEIPs), intent.EIPs)
+				}
+				for key, externalIP := range wantEIPs {
+					if got := intent.EIPs[key].ExternalIP; got != externalIP {
+						t.Errorf("%s: intent EIP %s = %q, want %q", stage, key, got, externalIP)
+					}
+				}
+				assertPublic(stage, tc.publicA, "eni-a")
+				assertPublic(stage, "203.0.113.20", "eni-b")
+			}
+		})
+	}
+}
+
+// The drop-gate exemption must name the EIP's own subnet, not a subnet in
+// another VPC whose CIDR happens to contain the same private IP.
+func TestSubnetIDForIP_ScopedToVPC(t *testing.T) {
+	subnets := map[string]topology.SubnetSpec{
+		"subnet-a": {SubnetID: "subnet-a", VPCID: "vpc-a", CIDR: netip.MustParsePrefix("172.31.0.0/20")},
+		"subnet-b": {SubnetID: "subnet-b", VPCID: "vpc-b", CIDR: netip.MustParsePrefix("172.31.0.0/20")},
+	}
+	for range 20 {
+		if got := subnetIDForIP(subnets, "vpc-a", "172.31.0.4"); got != "subnet-a" {
+			t.Fatalf("vpc-a lookup = %q, want subnet-a", got)
+		}
+		if got := subnetIDForIP(subnets, "vpc-b", "172.31.0.4"); got != "subnet-b" {
+			t.Fatalf("vpc-b lookup = %q, want subnet-b", got)
+		}
+	}
+	if got := subnetIDForIP(subnets, "vpc-c", "172.31.0.4"); got != "" {
+		t.Errorf("lookup in a VPC with no subnets = %q, want empty", got)
+	}
+}
+
+// egressCall is one EnsureEIPInstanceEgress invocation.
+type egressCall struct{ vpcID, subnetID, instanceIP string }
+
+// recordingIGW records EnsureEIPInstanceEgress calls instead of writing OVN.
+type recordingIGW struct {
+	external.IGWManager
+
+	calls []egressCall
+}
+
+func (g *recordingIGW) EnsureEIPInstanceEgress(_ context.Context, vpcID, subnetID, instanceIP string) error {
+	g.calls = append(g.calls, egressCall{vpcID, subnetID, instanceIP})
+	return nil
+}
+
+// An EIP's drop-gate exemption goes to its own VPC's subnet even when another
+// tenant's gated subnet has the same CIDR.
+func TestApplyPublicInstanceEgress_EIPExemptsOwnVPCSubnet(t *testing.T) {
+	cidr := netip.MustParsePrefix("172.31.0.0/20")
+	intent := IntentState{
+		Subnets: map[string]topology.SubnetSpec{
+			"subnet-a": {SubnetID: "subnet-a", VPCID: "vpc-a", CIDR: cidr},
+			"subnet-b": {SubnetID: "subnet-b", VPCID: "vpc-b", CIDR: cidr},
+		},
+		EIPs: map[string]policy.EIPSpec{
+			eipIntentKey("vpc-b", "172.31.0.4"): {VPCID: "vpc-b", ExternalIP: "203.0.113.20", LogicalIP: "172.31.0.4"},
+		},
+		DropGates: map[string]SubnetEgressIntent{},
+	}
+	for _, s := range intent.Subnets {
+		intent.DropGates[subnetEgressKey(s.SubnetID, netip.MustParsePrefix("0.0.0.0/0"))] = SubnetEgressIntent{
+			VPCID: s.VPCID, SubnetID: s.SubnetID, DestCIDR: netip.MustParsePrefix("0.0.0.0/0"),
+		}
+	}
+
+	for range 20 {
+		r, _ := newTestReconciler(t)
+		igw := new(recordingIGW)
+		r.igw = igw
+		res := new(passResult)
+		r.applyPublicInstanceEgress(context.Background(), intent, ActualState{}, res)
+		want := []egressCall{{"vpc-b", "subnet-b", "172.31.0.4"}}
+		if !slices.Equal(igw.calls, want) {
+			t.Fatalf("EnsureEIPInstanceEgress calls = %+v, want %+v", igw.calls, want)
+		}
+		if len(res.failures) != 0 {
+			t.Fatalf("pass failures = %+v, want none", res.failures)
+		}
+	}
+}

spinifex/domains/network/reconcile/apply_guest_port_idle_test.go

@@ -27,7 +27,7 @@ func captureReconcileLogs(t *testing.T) *bytes.Buffer {
 
 func idleEIPIntent(idle bool) IntentState {
 	intent := IntentState{EIPs: map[string]policy.EIPSpec{
-		"10.0.0.4": {VPCID: "vpc-a", ExternalIP: "192.0.2.46", LogicalIP: "10.0.0.4", PortName: "port-eni-1"},
+		eipIntentKey("vpc-a", "10.0.0.4"): {VPCID: "vpc-a", ExternalIP: "192.0.2.46", LogicalIP: "10.0.0.4", PortName: "port-eni-1"},
 	}}
 	if idle {
 		intent.IdlePorts = map[string]string{"port-eni-1": "i-stopped"}

spinifex/domains/network/reconcile/apply_test.go

@@ -115,6 +115,53 @@ func TestReconcile_PortSuppressDHCP(t *testing.T) {
 	}
 }
 
+// A deleted DHCP_Options row clears dhcpv4_options on every LSP that pointed at
+// it. The next pass recreates the row under a new UUID and must re-link the
+// existing ports to it, leaving a SuppressDHCP port unlinked.
+func TestReconcile_ExistingPortRelinkedToRecreatedDHCPOptions(t *testing.T) {
+	rec, m := newTestReconciler(t)
+	ctx := context.Background()
+
+	intent := freshIntent(t)
+	mac, _ := net.ParseMAC("02:00:00:00:00:09")
+	intent.Ports["eni-static"] = topology.PortSpec{
+		PortID: "eni-static", SubnetID: "subnet-a", VPCID: "vpc-a",
+		PrivateIP: netip.MustParseAddr("10.0.1.11"), MAC: mac,
+		SGIDs: []string{"sg-a"}, SuppressDHCP: true,
+	}
+	if err := rec.Reconcile(ctx, intent); err != nil {
+		t.Fatalf("Reconcile #1: %v", err)
+	}
+
+	old, err := m.FindDHCPOptionsByExternalID(ctx, "spinifex:subnet_id", "subnet-a")
+	if err != nil {
+		t.Fatalf("DHCP options after first pass: %v", err)
+	}
+	if err := m.DeleteDHCPOptions(ctx, old.UUID); err != nil {
+		t.Fatalf("DeleteDHCPOptions: %v", err)
+	}
+	portName := "port-" + intent.Ports["eni-a"].PortID
+	m.Ports[portName].DHCPv4Options = nil
+
+	if err := rec.Reconcile(ctx, intent); err != nil {
+		t.Fatalf("Reconcile #2: %v", err)
+	}
+
+	cur, err := m.FindDHCPOptionsByExternalID(ctx, "spinifex:subnet_id", "subnet-a")
+	if err != nil {
+		t.Fatalf("DHCP options after second pass: %v", err)
+	}
+	if cur.UUID == old.UUID {
+		t.Fatalf("DHCP options row was not recreated")
+	}
+	if got := m.Ports[portName].DHCPv4Options; got == nil || *got != cur.UUID {
+		t.Errorf("existing LSP dhcpv4_options = %v, want %s", got, cur.UUID)
+	}
+	if got := m.Ports["port-eni-static"].DHCPv4Options; got != nil {
+		t.Errorf("SuppressDHCP LSP must stay unlinked, got %q", *got)
+	}
+}
+
 func TestReconcile_PortJoinsPortGroupAtomically(t *testing.T) {
 	rec, m := newTestReconciler(t)
 	ctx := context.Background()
@@ -1160,10 +1207,10 @@ func TestFloatingIPSpecs(t *testing.T) {
 	intent := IntentState{
 		EIPs: map[string]policy.EIPSpec{
 			// User EIP on 172.31.0.9.
-			"172.31.0.9": {VPCID: "vpc-a", ExternalIP: "192.168.1.200", LogicalIP: "172.31.0.9", PortName: topology.Port("eni-eip")},
+			eipIntentKey("vpc-a", "172.31.0.9"): {VPCID: "vpc-a", ExternalIP: "192.168.1.200", LogicalIP: "172.31.0.9", PortName: topology.Port("eni-eip")},
 			// User EIP that also has an auto-assigned public IP recorded on its
 			// port: the EIP must win and the auto-assigned entry be skipped.
-			"172.31.0.4": {VPCID: "vpc-a", ExternalIP: "192.168.1.201", LogicalIP: "172.31.0.4", PortName: topology.Port("eni-dup")},
+			eipIntentKey("vpc-a", "172.31.0.4"): {VPCID: "vpc-a", ExternalIP: "192.168.1.201", LogicalIP: "172.31.0.4", PortName: topology.Port("eni-dup")},
 		},
 		Ports: map[string]topology.PortSpec{
 			// Auto-assigned public IP with no user EIP -> must produce a spec.
@@ -1172,6 +1219,9 @@ func TestFloatingIPSpecs(t *testing.T) {
 			// Same private IP as the user EIP above -> deduped out.
 			"eni-dup": {PortID: "eni-dup", VPCID: "vpc-a", PrivateIP: netip.MustParseAddr("172.31.0.4"),
 				PublicIP: netip.MustParseAddr("192.168.1.117"), MAC: mac},
+			// Same private IP as vpc-a's user EIP but in another VPC -> kept.
+			"eni-other-vpc": {PortID: "eni-other-vpc", VPCID: "vpc-b", PrivateIP: netip.MustParseAddr("172.31.0.4"),
+				PublicIP: netip.MustParseAddr("192.168.1.118"), MAC: mac},
 			// No public IP -> skipped.
 			"eni-nopub": {PortID: "eni-nopub", VPCID: "vpc-b", PrivateIP: netip.MustParseAddr("172.31.0.6")},
 		},
@@ -1203,9 +1253,12 @@ func TestFloatingIPSpecs(t *testing.T) {
 	if _, ok := byExternal["192.168.1.117"]; ok {
 		t.Errorf("auto-assigned 192.168.1.117 should be deduped by the user EIP on 172.31.0.4")
 	}
+	if _, ok := byExternal["192.168.1.118"]; !ok {
+		t.Errorf("auto-assigned 192.168.1.118 in vpc-b must not be deduped by vpc-a's EIP on the same private IP")
+	}
 	// No public IP -> no spec for that port.
-	if len(specs) != 3 {
-		t.Errorf("want 3 specs (2 EIP + 1 auto), got %d: %+v", len(specs), specs)
+	if len(specs) != 4 {
+		t.Errorf("want 4 specs (2 EIP + 2 auto), got %d: %+v", len(specs), specs)
 	}
 
 	// A NAT gateway's address holds an snat, never a dnat_and_snat, so it must
@@ -1230,7 +1283,7 @@ func TestHostBindSpecs_CarriesNATGatewayAddresses(t *testing.T) {
 
 	intent := IntentState{
 		EIPs: map[string]policy.EIPSpec{
-			"172.31.0.9": {VPCID: "vpc-a", ExternalIP: "192.168.1.200", LogicalIP: "172.31.0.9", PortName: topology.Port("eni-eip")},
+			eipIntentKey("vpc-a", "172.31.0.9"): {VPCID: "vpc-a", ExternalIP: "192.168.1.200", LogicalIP: "172.31.0.9", PortName: topology.Port("eni-eip")},
 		},
 		NATGWs: map[string]policy.NATGWSpec{
 			// One gateway, two associated subnets: the same address arrives
@@ -1283,7 +1336,7 @@ func TestHostBindSpecs_DedupesAgainstAnExistingEIP(t *testing.T) {
 
 	specs := r.hostBindSpecs(IntentState{
 		EIPs: map[string]policy.EIPSpec{
-			"172.31.0.9": {VPCID: "vpc-a", ExternalIP: "192.168.1.240", LogicalIP: "172.31.0.9",
+			eipIntentKey("vpc-a", "172.31.0.9"): {VPCID: "vpc-a", ExternalIP: "192.168.1.240", LogicalIP: "172.31.0.9",
 				PortName: topology.Port("eni-eip"), MAC: mac.String()},
 		},
 		NATGWs: map[string]policy.NATGWSpec{
@@ -1431,7 +1484,7 @@ func TestReconcile_MarksIGWAttachedOnlyOnSuccess(t *testing.T) {
 		// A distributed-NAT gateway LRP is link-local and gates the probe off,
 		// so an EIP is what gives the datapath check a target.
 		intent := igwIntent(t)
-		intent.EIPs["10.0.1.5"] = policy.EIPSpec{VPCID: "vpc-a", ExternalIP: "203.0.113.5", LogicalIP: "10.0.1.5"}
+		intent.EIPs[eipIntentKey("vpc-a", "10.0.1.5")] = policy.EIPSpec{VPCID: "vpc-a", ExternalIP: "203.0.113.5", LogicalIP: "10.0.1.5"}
 		if err := rec.Reconcile(ctx, intent); err != nil {
 			t.Fatalf("Reconcile = %v, want nil", err)
 		}
@@ -1607,7 +1660,7 @@ func TestReconcile_FailedAddEIPReportsPassIncomplete(t *testing.T) {
 	rec.nat = &stubNAT{NATManager: rec.nat, addErr: errors.New("ovsdb unreachable")}
 
 	intent := freshIntent(t)
-	intent.EIPs["10.0.1.10"] = policy.EIPSpec{
+	intent.EIPs[eipIntentKey("vpc-a", "10.0.1.10")] = policy.EIPSpec{
 		VPCID: "vpc-a", ExternalIP: "192.168.1.10", LogicalIP: "10.0.1.10",
 		PortName: topology.Port("eni-a"),
 	}

spinifex/domains/network/reconcile/intent.go

@@ -32,7 +32,7 @@ type IntentState struct {
 	Ports       map[string]topology.PortSpec
 	SGs         map[string]policy.SGSpec
 	IGWs        map[string]external.IGWSpec // attached only
-	EIPs        map[string]policy.EIPSpec   // keyed by logicalIP; associated only
+	EIPs        map[string]policy.EIPSpec   // keyed by eipIntentKey; associated only
 	NATGWs      map[string]policy.NATGWSpec
 	IGWRoutes   map[string]SubnetEgressIntent // per (subnet, dest) IGW egress reroute
 	NATGWRoutes map[string]SubnetEgressIntent // per (subnet, dest) NATGW egress reroute
@@ -57,6 +57,12 @@ type SubnetEgressIntent struct {
 	DestCIDR netip.Prefix
 }
 
+// eipIntentKey is the EIPs map key. Private IPs repeat across VPCs (every
+// default VPC is 172.31.0.0/16), so the VPC is part of the identity.
+func eipIntentKey(vpcID, privateIP string) string {
+	return vpcID + "|" + privateIP
+}
+
 // subnetEgressKey is the IGWRoutes/NATGWRoutes map key.
 func subnetEgressKey(subnetID string, prefix netip.Prefix) string {
 	return subnetID + "|" + prefix.String()
@@ -565,7 +571,7 @@ func loadEIPs(ctx context.Context, js jetstream.JetStream, localVPCs map[string]
 		}
 		// MAC drives distributed dnat_and_snat; empty falls back to centralised.
 		spec.MAC = rec.MacAddress
-		out[rec.PrivateIp] = spec
+		out[eipIntentKey(rec.VpcId, rec.PrivateIp)] = spec
 	}
 	return nil
 }

spinifex/domains/network/reconcile/intent_test.go

@@ -183,10 +183,10 @@ func TestLoadIntentFromKV_EIPStateFilter(t *testing.T) {
 		t.Fatalf("LoadIntentFromKV: %v", err)
 	}
 
-	if _, ok := intent.EIPs["10.0.1.5"]; !ok {
+	if _, ok := intent.EIPs[eipIntentKey("vpc-local", "10.0.1.5")]; !ok {
 		t.Errorf("associated EIP missing from intent")
 	}
-	if _, ok := intent.EIPs["10.0.1.6"]; ok {
+	if _, ok := intent.EIPs[eipIntentKey("vpc-local", "10.0.1.6")]; ok {
 		t.Errorf("non-associated EIP leaked into intent")
 	}
 }

spinifex/domains/network/reconcile/scenario_kitchensink_live_test.go

@@ -60,7 +60,7 @@ func kitchenSinkIntent(t *testing.T) IntentState {
 			"vpc-a": {VPCID: "vpc-a", InternetGatewayID: "igw-a"},
 		},
 		EIPs: map[string]policy.EIPSpec{
-			"10.0.1.10": {VPCID: "vpc-a", ExternalIP: "192.168.0.50", LogicalIP: "10.0.1.10",
+			eipIntentKey("vpc-a", "10.0.1.10"): {VPCID: "vpc-a", ExternalIP: "192.168.0.50", LogicalIP: "10.0.1.10",
 				PortName: topology.Port("eni-pub"), MAC: "02:00:00:00:00:01"},
 		},
 		NATGWs: map[string]policy.NATGWSpec{

spinifex/domains/network/topology/manager_live.go

@@ -278,10 +278,19 @@ func (m *liveManager) DeleteSubnet(ctx context.Context, spec SubnetSpec) error {
 	if err := m.ovn.DeleteLogicalRouterPort(ctx, routerName, routerPortName); err != nil {
 		slog.Warn("topology: DeleteSubnet router port", "port", routerPortName, "err", err)
 	}
-	if spec.CIDR.IsValid() {
-		if dhcpOpts, err := m.ovn.FindDHCPOptionsByCIDR(ctx, spec.CIDR.String()); err == nil {
-			if err := m.ovn.DeleteDHCPOptions(ctx, dhcpOpts.UUID); err != nil {
-				slog.Warn("topology: DeleteSubnet DHCP options", "cidr", spec.CIDR.String(), "err", err)
+	// Subnet CIDRs overlap across VPCs and accounts, so only the subnet_id
+	// external ID identifies this subnet's DHCP_Options rows.
+	if spec.SubnetID != "" {
+		dhcpOpts, err := m.ovn.ListDHCPOptions(ctx)
+		if err != nil {
+			slog.Warn("topology: DeleteSubnet list DHCP options", "subnet_id", spec.SubnetID, "err", err)
+		}
+		for _, opts := range dhcpOpts {
+			if opts.ExternalIDs["spinifex:subnet_id"] != spec.SubnetID {
+				continue
+			}
+			if err := m.ovn.DeleteDHCPOptions(ctx, opts.UUID); err != nil {
+				slog.Warn("topology: DeleteSubnet DHCP options", "uuid", opts.UUID, "subnet_id", spec.SubnetID, "err", err)
 			}
 		}
 	}

spinifex/domains/network/topology/manager_live_nb_test.go

@@ -0,0 +1,86 @@
+package topology_test
+
+import (
+	"context"
+	"net"
+	"net/netip"
+	"testing"
+	"time"
+
+	"github.com/mulgadc/spinifex/spinifex/domains/network/ovn"
+	"github.com/mulgadc/spinifex/spinifex/domains/network/ovn/ovntest"
+	"github.com/mulgadc/spinifex/spinifex/domains/network/topology"
+)
+
+// Two VPCs in different accounts commonly share a subnet CIDR (every default
+// subnet is 172.31.0.0/20). Deleting one subnet, repeatedly, must never touch
+// the other subnet's DHCP_Options row or the LSP reference to it.
+func TestLiveManager_DeleteSubnet_SameCIDRLeavesOtherSubnetDHCP(t *testing.T) {
+	nb := ovntest.StartNB(t)
+	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+	defer cancel()
+	cli := ovn.NewLiveClient(nb.Endpoint)
+	if err := cli.Connect(ctx); err != nil {
+		t.Fatalf("LiveClient.Connect: %v", err)
+	}
+	t.Cleanup(cli.Close)
+	mgr := topology.NewLiveManager(cli)
+
+	vpcCIDR := netip.MustParsePrefix("172.31.0.0/16")
+	subCIDR := netip.MustParsePrefix("172.31.0.0/20")
+
+	victim := topology.SubnetSpec{SubnetID: "subnet-bbbb", VPCID: "vpc-bbbb", CIDR: subCIDR}
+	if err := mgr.EnsureVPC(ctx, topology.VPCSpec{VPCID: victim.VPCID, CIDR: vpcCIDR}); err != nil {
+		t.Fatalf("EnsureVPC victim: %v", err)
+	}
+	if err := mgr.EnsureSubnet(ctx, victim); err != nil {
+		t.Fatalf("EnsureSubnet victim: %v", err)
+	}
+	if err := mgr.EnsureSGPortGroup(ctx, "sg-bbbb"); err != nil {
+		t.Fatalf("EnsureSGPortGroup: %v", err)
+	}
+	mac, _ := net.ParseMAC("02:00:00:00:bb:01")
+	port := topology.PortSpec{
+		PortID: "eni-bbbb", SubnetID: victim.SubnetID, VPCID: victim.VPCID,
+		PrivateIP: netip.MustParseAddr("172.31.0.10"), MAC: mac, SGIDs: []string{"sg-bbbb"},
+	}
+	if err := mgr.EnsurePort(ctx, port); err != nil {
+		t.Fatalf("EnsurePort: %v", err)
+	}
+	victimRow, err := cli.FindDHCPOptionsByExternalID(ctx, "spinifex:subnet_id", victim.SubnetID)
+	if err != nil {
+		t.Fatalf("victim DHCP options: %v", err)
+	}
+
+	if err := mgr.EnsureVPC(ctx, topology.VPCSpec{VPCID: "vpc-aaaa", CIDR: vpcCIDR}); err != nil {
+		t.Fatalf("EnsureVPC other: %v", err)
+	}
+	for i := range 10 {
+		other := topology.SubnetSpec{SubnetID: "subnet-aaaa" + string(rune('0'+i)), VPCID: "vpc-aaaa", CIDR: subCIDR}
+		if err := mgr.EnsureSubnet(ctx, other); err != nil {
+			t.Fatalf("cycle %d EnsureSubnet: %v", i, err)
+		}
+		if err := mgr.DeleteSubnet(ctx, other); err != nil {
+			t.Fatalf("cycle %d DeleteSubnet: %v", i, err)
+		}
+
+		rows, err := cli.ListDHCPOptions(ctx)
+		if err != nil {
+			t.Fatalf("ListDHCPOptions: %v", err)
+		}
+		if len(rows) != 1 || rows[0].UUID != victimRow.UUID {
+			owners := make([]string, 0, len(rows))
+			for _, r := range rows {
+				owners = append(owners, r.ExternalIDs["spinifex:subnet_id"])
+			}
+			t.Fatalf("cycle %d: DHCP_Options rows owned by %v, want only %s", i, owners, victim.SubnetID)
+		}
+		lsp, err := cli.GetLogicalSwitchPort(ctx, topology.Port(port.PortID))
+		if err != nil {
+			t.Fatalf("GetLogicalSwitchPort: %v", err)
+		}
+		if lsp.DHCPv4Options == nil || *lsp.DHCPv4Options != victimRow.UUID {
+			t.Fatalf("cycle %d: victim LSP dhcpv4_options = %v, want %s", i, lsp.DHCPv4Options, victimRow.UUID)
+		}
+	}
+}

spinifex/domains/network/topology/manager_live_test.go

@@ -473,7 +473,7 @@ func TestLiveManager_WithDNSServer(t *testing.T) {
 	ctx := context.Background()
 	_ = mgr.EnsureVPC(ctx, VPCSpec{VPCID: "vpc-dns", CIDR: netip.MustParsePrefix("10.6.0.0/16")})
 	_ = mgr.EnsureSubnet(ctx, SubnetSpec{SubnetID: "subnet-dns", VPCID: "vpc-dns", CIDR: netip.MustParsePrefix("10.6.1.0/24")})
-	opts, err := mockClient.FindDHCPOptionsByCIDR(ctx, "10.6.1.0/24")
+	opts, err := mockClient.FindDHCPOptionsByExternalID(ctx, "spinifex:subnet_id", "subnet-dns")
 	if err != nil {
 		t.Fatalf("DHCPOptions missing: %v", err)
 	}

spinifex/foundation/aws/errors/awserrors.go

@@ -393,6 +393,15 @@ var (
 	// real ECS both use the restJson1 wire code "InvalidParameterException",
 	// so this gives EKS call sites their own name for the same registered code.
 	ErrorEKSInvalidParameter = ErrorECSInvalidParameter
+	// ErrorEKSInvalidRequest is the code the EKS access-entry operations declare
+	// in place of a parameter code.
+	ErrorEKSInvalidRequest = "InvalidRequestException"
+	// ErrorEKSBadRequest is the code the EKS tagging operations declare for a
+	// malformed request.
+	ErrorEKSBadRequest = "BadRequestException"
+	// ErrorEKSClient aliases ErrorECSClientException: DescribeCluster and
+	// DeleteCluster declare the same wire code "ClientException".
+	ErrorEKSClient = ErrorECSClientException
 	// ErrorECRInvalidParameter aliases ErrorECSInvalidParameter: every ECR
 	// operation declares the AWS JSON 1.1 wire code "InvalidParameterException".
 	ErrorECRInvalidParameter = ErrorECSInvalidParameter
@@ -1267,8 +1276,10 @@ var ErrorLookup = map[string]ErrorMessage{
 	ErrorResourceNotFound:                                      {HTTPCode: 404, Message: "The specified resource was not found."},
 	ErrorEKSResourceInUse:                                      {HTTPCode: 409, Message: "A cluster with this name already exists in this account and Region. Delete the existing cluster, or choose a different name."},
 	ErrorEKSResourceNotFound:                                   {HTTPCode: 404, Message: "The specified cluster could not be found. You can view your available clusters with ListClusters. Amazon EKS clusters are Region specific."},
+	ErrorEKSInvalidRequest:                                     {HTTPCode: 400, Message: "The request is invalid given the state of the cluster or access entry."},
+	ErrorEKSBadRequest:                                         {HTTPCode: 400, Message: "A parameter of the request is not valid."},
 	ErrorECSClientException:                                    {HTTPCode: 400, Message: "Unable to describe task definition."},
-	ErrorECSClusterNotFound:                                    {HTTPCode: 400, Message: "The specified cluster was not found. You can view your available clusters with ListClusters. Amazon ECS clusters are Region specific."},
+	ErrorECSClusterNotFound:                                    {HTTPCode: 400, Message: "Cluster not found."},
 	ErrorECSInvalidParameter:                                   {HTTPCode: 400, Message: "The specified parameter is not valid. Review the available parameters for the API request."},
 	ErrorECSServiceNotFound:                                    {HTTPCode: 400, Message: "The specified service was not found. You can view your available services with ListServices. Amazon ECS services are cluster specific and Region specific."},
 	ErrorRetryableError:                                        {HTTPCode: 400, Message: "A request submitted by an AWS service on your behalf could not be completed. The requesting service might automatically retry the request."},

spinifex/providers/ebs/viperblock/nbdkit_recovery.go

@@ -363,11 +363,11 @@ func endpointReferencer(procRoot string, disc discoveredNbdkit) (pid int, found
 
 // cmdlineReferencesEndpoint checks a NUL-separated /proc/<pid>/cmdline for
 // disc's endpoint. A socket mount's path appears verbatim in whichever
-// argument named it (an EFI -drive's file= or a -blockdev's server.path=). A
-// TCP mount's 127.0.0.1:<port> pair may appear joined in one argument (an EFI
-// pflash drive's nbd://host:port) or split across server.host=/server.port=
-// within one -blockdev argument. The match is by substring, so the boot
-// volume's file.server.path=/file.server.host=/file.server.port= also match.
+// argument named it (QEMU's nbd+unix:///?socket=<path> file= or its
+// server.path=<path> -blockdev option). A TCP mount's 127.0.0.1:<port> pair
+// may appear joined in one argument (a boot/EFI drive's nbd://host:port) or
+// split across server.host=/server.port= within the same comma-joined
+// -blockdev argument, so both shapes are checked.
 func cmdlineReferencesEndpoint(cmdline []byte, disc discoveredNbdkit) bool {
 	args := bytes.Split(bytes.TrimRight(cmdline, "\x00"), []byte{0})
 

spinifex/providers/ebs/viperblock/nbdkit_recovery_test.go

@@ -415,7 +415,7 @@ func TestReapOrphanedNbdkit_LiveReferencer_LeftRunningAndLogged(t *testing.T) {
 // TestReapOrphanedNbdkit_TCPEndpointReferencedAcrossSplitBlockdevArgs proves
 // endpointReferencer recognizes a TCP endpoint even when QEMU's -blockdev
 // splits host and port into server.host=/server.port= within one argument,
-// not just the joined host:port form an EFI pflash drive uses.
+// not just the joined host:port form a boot/EFI drive uses.
 func TestReapOrphanedNbdkit_TCPEndpointReferencedAcrossSplitBlockdevArgs(t *testing.T) {
 	pid := spawnLongRunningProcess(t)
 	procRoot := t.TempDir()
@@ -428,46 +428,6 @@ func TestReapOrphanedNbdkit_TCPEndpointReferencedAcrossSplitBlockdevArgs(t *test
 	assert.Never(t, func() bool { return !hostprocess.ProcessAlive(pid) }, 50*time.Millisecond, 5*time.Millisecond, "a TCP endpoint split across server.host=/server.port= must still be recognized as referenced")
 }
 
-// TestReapOrphanedNbdkit_BootBlockdevReferencer_LeftRunning pins the boot
-// volume's -blockdev, whose server options carry a file. prefix: a live
-// guest's root nbdkit named only that way must never be signalled.
-func TestReapOrphanedNbdkit_BootBlockdevReferencer_LeftRunning(t *testing.T) {
-	socket := filepath.Join(t.TempDir(), "vol-boot-root.sock")
-	tests := []struct {
-		name string
-		arg  string
-		disc func(pid int) discoveredNbdkit
-	}{
-		{
-			name: "unix socket",
-			arg:  "driver=raw,node-name=os,cache.direct=on,file.driver=nbd,file.server.type=unix,file.server.path=" + socket + ",file.export=,file.reconnect-delay=30",
-			disc: func(pid int) discoveredNbdkit {
-				return discoveredNbdkit{PID: pid, Volume: "vol-boot-unix1", Socket: socket}
-			},
-		},
-		{
-			name: "tcp",
-			arg:  "driver=raw,node-name=os,cache.direct=on,file.driver=nbd,file.server.type=inet,file.server.host=127.0.0.1,file.server.port=10897,file.export=,file.reconnect-delay=30",
-			disc: func(pid int) discoveredNbdkit {
-				return discoveredNbdkit{PID: pid, Volume: "vol-boot-tcp1", Port: 10897}
-			},
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			pid := spawnLongRunningProcess(t)
-			procRoot := t.TempDir()
-			const referencerPID = 909092
-			writeReferencerFixture(t, procRoot, referencerPID, tt.arg)
-
-			logs := captureLogs(t)
-			assert.False(t, reapOrphanedNbdkit(procRoot, tt.disc(pid), reapReasonUnadoptable), "a boot volume's nbdkit must never be signalled")
-			assert.Never(t, func() bool { return !hostprocess.ProcessAlive(pid) }, 50*time.Millisecond, 5*time.Millisecond, "a boot volume's nbdkit must never be signalled")
-			assert.Contains(t, logs.String(), fmt.Sprintf("referencing_pid=%d", referencerPID))
-		})
-	}
-}
-
 // unreadableProcDir creates procRoot/<pid> with a cmdline file inside, then
 // strips all permissions from the directory itself, so os.ReadFile on the
 // cmdline path inside it fails with a permission error rather than

spinifex/runtime/compute/vm/device_map.go

@@ -84,10 +84,6 @@ func buildDeviceMap(devices []qmp.BlockDevice) map[string]string {
 
 		name := dev.Device
 		pciIndex := extractPCIIndex(dev.QDev)
-		// A -blockdev boot node has no BlockBackend name; its node-name is "os".
-		if name == "" && pciIndex >= 0 && dev.Inserted != nil {
-			name = dev.Inserted.NodeName
-		}
 
 		if pciIndex < 0 {
 			// Hot-plugged device: extract name and port from QDev path.

spinifex/runtime/compute/vm/device_map_test.go

@@ -127,23 +127,6 @@ func TestBuildDeviceMap(t *testing.T) {
 			},
 			wantLen: 4,
 		},
-		{
-			// A -blockdev boot node leaves the BlockBackend unnamed, so the
-			// entry is keyed by its node-name instead of "".
-			name: "blockdev boot device is named by its node-name",
-			devices: []qmp.BlockDevice{
-				{Device: "", Inserted: &qmp.BlockInserted{NodeName: "os"}, QDev: "/machine/peripheral-anon/device[0]/virtio-backend"},
-				{Device: "cloudinit", Inserted: &qmp.BlockInserted{NodeName: "#block123"}, QDev: "/machine/peripheral-anon/device[3]/virtio-backend"},
-				{Device: "", Inserted: &qmp.BlockInserted{NodeName: "nbd-vol-abc"}, QDev: "/machine/peripheral/vdisk-vol-abc/virtio-backend"},
-			},
-			want: map[string]string{
-				"os":            "/dev/vda",
-				"cloudinit":     "/dev/vdb",
-				"vdisk-vol-abc": "/dev/vdc",
-			},
-			absent:  []string{"", "nbd-vol-abc"},
-			wantLen: 3,
-		},
 		{
 			// Boot devices returned out of order: lowest PCI index wins /dev/vda;
 			// hot-plugged devices sort after boot devices.
@@ -443,15 +426,3 @@ func TestLogGuestDeviceMap_LeavesBlockDeviceMappingsAlone(t *testing.T) {
 	assert.Equal(t, "/dev/sdf", aws.StringValue(v.Instance.BlockDeviceMappings[1].DeviceName),
 		"a data mapping must still name the device the caller asked for, not /dev/vdb")
 }
-
-// TestBuildDeviceMap_UnnamedBootDeviceWithoutMedium covers an unnamed
-// boot-time BlockBackend that reports no inserted medium.
-func TestBuildDeviceMap_UnnamedBootDeviceWithoutMedium(t *testing.T) {
-	devices := []qmp.BlockDevice{
-		{Device: "", Inserted: nil, QDev: "/machine/peripheral-anon/device[0]/virtio-backend"},
-		{Device: "cloudinit", Inserted: &qmp.BlockInserted{}, QDev: "/machine/peripheral-anon/device[3]/virtio-backend"},
-	}
-	var got map[string]string
-	require.NotPanics(t, func() { got = buildDeviceMap(devices) })
-	assert.Equal(t, "/dev/vdb", got["cloudinit"], "the unnamed device still holds its PCI slot")
-}

spinifex/runtime/compute/vm/devices.go

@@ -6,8 +6,6 @@ import (
 	"slices"
 	"strconv"
 	"strings"
-
-	"github.com/mulgadc/spinifex/spinifex/runtime/compute/nbd"
 )
 
 // IsMicrovm reports whether machineType is the QEMU microvm machine, with or
@@ -164,14 +162,12 @@ type blkDeviceJSON struct {
 	NumQueues int                 `json:"num-queues,omitempty"`
 	BootIndex int                 `json:"bootindex,omitempty"`
 	Mapping   []iothreadVQMapping `json:"iothread-vq-mapping"`
-	Werror    string              `json:"werror"`
-	Rerror    string              `json:"rerror"`
 }
 
 // BlkDeviceMapped returns a virtio-blk -device that spreads its virtqueues
 // across a pool of poolSize IOThreads, round-robin. Falls back to BlkDevice's
-// scalar iothread= form when poolSize <= 1, when there is one queue, or on an
-// MMIO machine.
+// scalar iothread= form when poolSize <= 1, so the common small-guest case is
+// byte-for-byte what it was before this existed.
 //
 // Every queue is mapped exactly once and the scalar iothread= property is
 // absent: QEMU treats the two as alternative assignments and rejects both.
@@ -201,8 +197,6 @@ func BlkDeviceMapped(machineType, drive, iothreadBase string, queues, bootIdx, p
 		NumQueues: queues,
 		BootIndex: bootIdx,
 		Mapping:   mapping,
-		Werror:    "stop",
-		Rerror:    "stop",
 	})
 	if err != nil {
 		return Device{}, fmt.Errorf("marshal virtio-blk iothread-vq-mapping: %w", err)
@@ -210,9 +204,9 @@ func BlkDeviceMapped(machineType, drive, iothreadBase string, queues, bootIdx, p
 	return Device{Value: string(b)}, nil
 }
 
-// BlkDevice returns the virtio-blk device string for machineType, omitting an
-// empty iothread or zero queues; bootIdx is PCI-only. It sets werror/rerror=stop
-// (see VolumeBlkDeviceArgs), as a -blockdev node has no BlockBackend to hold it.
+// BlkDevice returns the appropriate QEMU virtio-blk device string for
+// machineType. iothread and queues are omitted when empty/zero. bootIdx is
+// only emitted for PCI machines (MMIO has no bootindex concept).
 func BlkDevice(machineType, drive, iothread string, queues int, bootIdx int) Device {
 	var b strings.Builder
 	if IsMMIO(machineType) {
@@ -230,7 +224,6 @@ func BlkDevice(machineType, drive, iothread string, queues int, bootIdx int) Dev
 	if !IsMMIO(machineType) && bootIdx > 0 {
 		fmt.Fprintf(&b, ",bootindex=%d", bootIdx)
 	}
-	b.WriteString(",werror=stop,rerror=stop")
 	return Device{Value: b.String()}
 }
 
@@ -289,9 +282,9 @@ func HotplugEBSBus(port int) string {
 // (rendered as a joined command-line string) so the two block-graph shapes
 // cannot diverge.
 //
-// werror=stop,rerror=stop pauses the VM and holds a failed request, so a
-// transient backend loss is resumed with cont instead of reaching the guest as
-// EIO. A -blockdev node has no BlockBackend to carry it, so the device sets it.
+// werror/rerror mirror the boot drive's on-error policy (see Drive.Werror):
+// a data volume has no -drive-backed BlockBackend to carry it, so the qdev
+// device itself sets werror=stop,rerror=stop.
 func VolumeBlkDeviceArgs(volumeID, nodeName, iothreadID, bus string) []string {
 	return []string{
 		fmt.Sprintf("id=%s", VolumeDeviceID(volumeID)),
@@ -390,54 +383,11 @@ func (o NBDServerOpts) CommandLineArgs() []string {
 // monitor-owned node that blockdev-del can later remove (unlike -drive,
 // which only creates a BlockBackend with an auto-generated node-name).
 func VolumeBlockdev(nodeName string, server NBDServerOpts) Blockdev {
-	opts := append([]string{"driver=nbd", fmt.Sprintf("node-name=%s", nodeName)}, nbdNodeOpts("", server)...)
+	opts := append([]string{"driver=nbd", fmt.Sprintf("node-name=%s", nodeName)}, server.CommandLineArgs()...)
+	opts = append(opts, "export=", fmt.Sprintf("reconnect-delay=%d", NBDReconnectDelaySeconds))
 	return Blockdev{Value: strings.Join(opts, ",")}
 }
 
-// nbdNodeOpts returns an nbd node's server, export and reconnect-delay options,
-// each prefixed with prefix: "" on the nbd node itself, "file." under raw.
-func nbdNodeOpts(prefix string, server NBDServerOpts) []string {
-	var opts []string
-	for _, arg := range server.CommandLineArgs() {
-		opts = append(opts, prefix+arg)
-	}
-	return append(opts, prefix+"export=", fmt.Sprintf("%sreconnect-delay=%d", prefix, NBDReconnectDelaySeconds))
-}
-
-// bootNodeName is the boot volume's block-node name, which its -device names
-// with drive=.
-const bootNodeName = "os"
-
-// nbdServerOpts parses an NBD URI into the server options a blockdev takes.
-func nbdServerOpts(uri string) (NBDServerOpts, error) {
-	serverType, socketPath, host, port, err := nbd.ParseNBDURI(uri)
-	if err != nil {
-		return NBDServerOpts{}, err
-	}
-	return NBDServerOpts{Type: serverType, Path: socketPath, Host: host, Port: port}, nil
-}
-
-// bootNBDBlockdev renders the boot volume's -blockdev for an NBD export: a raw
-// node over an nbd node, the graph -drive format=raw,cache=none built, with the
-// same O_DIRECT setting and reconnect delay.
-func bootNBDBlockdev(nodeName string, server NBDServerOpts) Blockdev {
-	opts := append([]string{"driver=raw", fmt.Sprintf("node-name=%s", nodeName), "cache.direct=on", "file.driver=nbd"}, nbdNodeOpts("file.", server)...)
-	return Blockdev{Value: strings.Join(opts, ",")}
-}
-
-// exportBlockdev renders v's -blockdev as nodeName: a host_device node for a
-// block export, otherwise the NBD node nbdBlockdev builds from its URI.
-func exportBlockdev(v EBSRequest, kind volumeExportKind, nodeName string, nbdBlockdev func(string, NBDServerOpts) Blockdev) (Blockdev, error) {
-	if kind == volumeExportBlock {
-		return VolumeHostDeviceBlockdev(nodeName, HostDeviceOpts{Path: v.BlockDevice, SizeBytes: v.BlockSizeBytes}), nil
-	}
-	server, err := nbdServerOpts(v.NBDURI)
-	if err != nil {
-		return Blockdev{}, fmt.Errorf("parse NBDURI for volume %s: %w", v.Name, err)
-	}
-	return nbdBlockdev(nodeName, server), nil
-}
-
 // HostDeviceOpts is a host block device export. QEMU opens Path through the
 // host_device protocol driver with O_DIRECT and native AIO, and a raw format
 // node over it exposes only the first SizeBytes to the guest.
@@ -463,7 +413,7 @@ func (o HostDeviceOpts) QMPArgs(nodeName string) map[string]any {
 }
 
 // VolumeHostDeviceBlockdev renders the -blockdev equivalent of QMPArgs for a
-// volume backed by a host block device.
+// data volume backed by a host block device.
 func VolumeHostDeviceBlockdev(nodeName string, dev HostDeviceOpts) Blockdev {
 	return Blockdev{Value: fmt.Sprintf(
 		"driver=raw,node-name=%s,size=%d,file.driver=host_device,file.filename=%s,file.aio=native,file.cache.direct=on",

spinifex/runtime/compute/vm/devices_test.go

@@ -130,28 +130,28 @@ func TestNICQueues_ClampedWhenEnabled(t *testing.T) {
 
 func TestBlkDevice_PCI(t *testing.T) {
 	d := BlkDevice("q35", "os", "ioth-os", 4, 1)
-	assert.Equal(t, "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=4,bootindex=1,werror=stop,rerror=stop", d.Value)
+	assert.Equal(t, "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=4,bootindex=1", d.Value)
 }
 
 func TestBlkDevice_PCI_NoBootIdx(t *testing.T) {
 	d := BlkDevice("q35", "data", "ioth-data", 2, 0)
-	assert.Equal(t, "virtio-blk-pci,drive=data,iothread=ioth-data,num-queues=2,werror=stop,rerror=stop", d.Value)
+	assert.Equal(t, "virtio-blk-pci,drive=data,iothread=ioth-data,num-queues=2", d.Value)
 }
 
 func TestBlkDevice_PCI_NoIOThread(t *testing.T) {
 	d := BlkDevice("q35", "data", "", 0, 0)
-	assert.Equal(t, "virtio-blk-pci,drive=data,werror=stop,rerror=stop", d.Value)
+	assert.Equal(t, "virtio-blk-pci,drive=data", d.Value)
 }
 
 func TestBlkDevice_MMIO(t *testing.T) {
 	d := BlkDevice("microvm", "os", "ioth-os", 4, 1)
 	// MMIO has no bootindex
-	assert.Equal(t, "virtio-blk-device,drive=os,iothread=ioth-os,num-queues=4,werror=stop,rerror=stop", d.Value)
+	assert.Equal(t, "virtio-blk-device,drive=os,iothread=ioth-os,num-queues=4", d.Value)
 }
 
 func TestBlkDevice_MMIO_NoIOThread(t *testing.T) {
 	d := BlkDevice("microvm,x-option-roms=off", "data", "", 0, 0)
-	assert.Equal(t, "virtio-blk-device,drive=data,werror=stop,rerror=stop", d.Value)
+	assert.Equal(t, "virtio-blk-device,drive=data", d.Value)
 }
 
 func TestRngDevice_PCI(t *testing.T) {

spinifex/runtime/compute/vm/lifecycle.go

@@ -315,7 +315,7 @@ func (m *Manager) startQEMU(instance *VM, appendConsole bool) error {
 		m.initENIRequests(instance)
 
 		instance.EBSRequests.Mu.Lock()
-		driveCfg, err := buildDiskBootDrives(instance.EBSRequests.Requests, spec.VCPUs, instance.Config.MachineType)
+		driveCfg, err := buildDrives(instance.EBSRequests.Requests, spec.VCPUs, instance.Config.MachineType)
 		instance.EBSRequests.Mu.Unlock()
 		if err != nil {
 			return err
@@ -1398,24 +1398,16 @@ func directBootLaunchConfig(base Config, requests []EBSRequest, cpuCount int) (C
 	return cfg, nil
 }
 
-// buildDiskBootDrives is buildDrives for a guest that boots from a volume: a
-// request list with no boot volume is refused, as QEMU would start with
-// nothing to boot from and the guest would never come up.
-func buildDiskBootDrives(requests []EBSRequest, cpuCount int, machineType string) (driveConfig, error) {
-	if !slices.ContainsFunc(requests, func(r EBSRequest) bool { return r.Boot }) {
-		return driveConfig{}, errors.New("no boot volume among the instance's EBS requests")
-	}
-	return buildDrives(requests, cpuCount, machineType)
-}
-
 // buildDrives converts EBS requests into QEMU drive/iothread/device/blockdev
 // configs. Returns an error if any volume has no valid export (see
 // classifyVolumeExport), or if the EBS hot-plug port pool is exhausted while
 // allocating one for a data volume.
 //
-// The boot volume is a -blockdev node named "os" under a bootindex=1 device
-// that carries werror/rerror; it is never detached, so it keeps fixed names.
-// EFI volumes emit pflash unit=1; Config.Execute adds the CODE blob (unit=0).
+// Boot volumes keep the existing anonymous if=none -drive shape exactly as
+// before — they are not detachable (see ErrVolumeNotDetachable), so the
+// anonymous-node problem this function otherwise fixes does not apply, and
+// changing the boot drive risks boot order. EFI volumes emit pflash unit=1;
+// the readonly CODE blob (unit=0) is added by Config.Execute.
 //
 // Every other (data) volume is given the same named node-name/device-id/
 // iothread-id shape AttachVolume's hot-plug path uses, via a -blockdev entry
@@ -1440,29 +1432,41 @@ func buildDrives(requests []EBSRequest, cpuCount int, machineType string) (drive
 		}
 
 		switch {
-		case kind == volumeExportBlock && v.EFI:
-			// The VARS store is a pflash -drive, which has no host_device form.
-			return driveConfig{}, fmt.Errorf("volume %s: block device export is not supported for an EFI volume", v.Name)
+		case kind == volumeExportBlock && (v.Boot || v.EFI):
+			// Boot and EFI keep the legacy -drive shape, which has no
+			// host_device form here; only data volumes take a block export.
+			return driveConfig{}, fmt.Errorf("volume %s: block device export is only supported for data volumes", v.Name)
 
 		case v.Boot:
-			blockdev, err := exportBlockdev(*v, kind, bootNodeName, bootNBDBlockdev)
-			if err != nil {
-				return driveConfig{}, err
+			drive := Drive{
+				File:   v.NBDURI,
+				Format: "raw",
+				If:     "none",
+				Media:  "disk",
+				ID:     "os",
+				Cache:  "none",
+
+				// Pause the VM and hold the failed request rather than
+				// delivering EIO to the guest, which aborts its journal. A
+				// transient backend loss is then recovered with cont.
+				Werror:         "stop",
+				Rerror:         "stop",
+				ReconnectDelay: NBDReconnectDelaySeconds,
 			}
 
 			// The boot disk's virtqueues are spread across a pool rather than
-			// all served by one host thread. A pool of 1 keeps the scalar
-			// iothread= form, which is every guest below 4 vCPUs.
+			// all served by one host thread. A pool of 1 emits exactly the
+			// command line this used to, which is every guest below 4 vCPUs.
 			pool := BlkIOThreadPoolSize(cpuCount)
 			for i := range pool {
 				cfg.IOThreads = append(cfg.IOThreads, IOThread{ID: BlkIOThreadID("ioth-os", i, pool)})
 			}
-			dev, err := BlkDeviceMapped(machineType, bootNodeName, "ioth-os", cpuCount, 1, pool)
+			dev, err := BlkDeviceMapped(machineType, drive.ID, "ioth-os", cpuCount, 1, pool)
 			if err != nil {
 				return driveConfig{}, fmt.Errorf("boot volume %s: %w", v.Name, err)
 			}
-			cfg.Blockdevs = append(cfg.Blockdevs, blockdev)
 			cfg.Devices = append(cfg.Devices, dev)
+			cfg.Drives = append(cfg.Drives, drive)
 
 		case v.EFI:
 			cfg.Drives = append(cfg.Drives, Drive{
@@ -1491,9 +1495,20 @@ func buildDrives(requests []EBSRequest, cpuCount int, machineType string) (drive
 			nodeName := VolumeNodeName(v.Name)
 			iothreadID := VolumeIOThreadID(v.Name)
 
-			blockdev, err := exportBlockdev(*v, kind, nodeName, VolumeBlockdev)
-			if err != nil {
-				return driveConfig{}, err
+			var blockdev Blockdev
+			if kind == volumeExportBlock {
+				blockdev = VolumeHostDeviceBlockdev(nodeName, HostDeviceOpts{Path: v.BlockDevice, SizeBytes: v.BlockSizeBytes})
+			} else {
+				serverType, socketPath, nbdHost, nbdPort, err := nbd.ParseNBDURI(v.NBDURI)
+				if err != nil {
+					return driveConfig{}, fmt.Errorf("parse NBDURI for volume %s: %w", v.Name, err)
+				}
+				blockdev = VolumeBlockdev(nodeName, NBDServerOpts{
+					Type: serverType,
+					Path: socketPath,
+					Host: nbdHost,
+					Port: nbdPort,
+				})
 			}
 
 			cfg.IOThreads = append(cfg.IOThreads, IOThread{ID: iothreadID})

spinifex/runtime/compute/vm/lifecycle_test.go

@@ -132,12 +132,12 @@ func TestBuildDrives(t *testing.T) {
 				{Name: "vol-boot", NBDURI: "nbd:unix:/tmp/boot.sock", Boot: true},
 			},
 			cpuCount: 2,
-			wantBlockdevs: []Blockdev{
-				{Value: "driver=raw,node-name=os,cache.direct=on,file.driver=nbd,file.server.type=unix,file.server.path=/tmp/boot.sock,file.export=,file.reconnect-delay=30"},
+			wantDrives: []Drive{
+				{File: "nbd:unix:/tmp/boot.sock", Format: "raw", If: "none", Media: "disk", ID: "os", Cache: "none", Werror: "stop", Rerror: "stop", ReconnectDelay: NBDReconnectDelaySeconds},
 			},
 			wantIOThreads: []IOThread{{ID: "ioth-os"}},
 			wantDevices: []Device{
-				{Value: "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=2,bootindex=1,werror=stop,rerror=stop"},
+				{Value: "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=2,bootindex=1"},
 			},
 		},
 		{
@@ -148,12 +148,12 @@ func TestBuildDrives(t *testing.T) {
 				{Name: "vol-boot", NBDURI: "nbd:unix:/tmp/boot.sock", Boot: true},
 			},
 			cpuCount: 4,
-			wantBlockdevs: []Blockdev{
-				{Value: "driver=raw,node-name=os,cache.direct=on,file.driver=nbd,file.server.type=unix,file.server.path=/tmp/boot.sock,file.export=,file.reconnect-delay=30"},
+			wantDrives: []Drive{
+				{File: "nbd:unix:/tmp/boot.sock", Format: "raw", If: "none", Media: "disk", ID: "os", Cache: "none", Werror: "stop", Rerror: "stop", ReconnectDelay: NBDReconnectDelaySeconds},
 			},
 			wantIOThreads: []IOThread{{ID: "ioth-os-0"}, {ID: "ioth-os-1"}},
 			wantDevices: []Device{
-				{Value: `{"driver":"virtio-blk-pci","drive":"os","num-queues":4,"bootindex":1,"iothread-vq-mapping":[{"iothread":"ioth-os-0","vqs":[0,2]},{"iothread":"ioth-os-1","vqs":[1,3]}],"werror":"stop","rerror":"stop"}`},
+				{Value: `{"driver":"virtio-blk-pci","drive":"os","num-queues":4,"bootindex":1,"iothread-vq-mapping":[{"iothread":"ioth-os-0","vqs":[0,2]},{"iothread":"ioth-os-1","vqs":[1,3]}]}`},
 			},
 		},
 		{
@@ -249,7 +249,7 @@ func TestBuildDrives(t *testing.T) {
 			wantErr:  "no free EBS hot-plug port for volume vol-overflow",
 		},
 		{
-			name: "mixed boot + EFI + data: boot is a -blockdev node and EFI keeps the pflash -drive",
+			name: "mixed boot + EFI + data: boot and EFI keep the legacy -drive shape",
 			requests: []EBSRequest{
 				{Name: "vol-boot", NBDURI: "nbd:unix:/tmp/boot.sock", Boot: true},
 				{Name: "vol-efi", NBDURI: "nbd:unix:/tmp/efi.sock", EFI: true},
@@ -260,6 +260,7 @@ func TestBuildDrives(t *testing.T) {
 			// unaffected by the pool either way — they keep one IOThread each.
 			cpuCount: 2,
 			wantDrives: []Drive{
+				{File: "nbd:unix:/tmp/boot.sock", Format: "raw", If: "none", Media: "disk", ID: "os", Cache: "none", Werror: "stop", Rerror: "stop", ReconnectDelay: NBDReconnectDelaySeconds},
 				{File: "nbd:unix:/tmp/efi.sock", Format: "raw", If: "pflash", Unit: 1},
 			},
 			wantIOThreads: []IOThread{
@@ -267,11 +268,10 @@ func TestBuildDrives(t *testing.T) {
 				{ID: "ioth-vol-data-a"},
 			},
 			wantBlockdevs: []Blockdev{
-				{Value: "driver=raw,node-name=os,cache.direct=on,file.driver=nbd,file.server.type=unix,file.server.path=/tmp/boot.sock,file.export=,file.reconnect-delay=30"},
 				{Value: "driver=nbd,node-name=nbd-vol-data-a,server.type=unix,server.path=/tmp/data-a.sock,export=,reconnect-delay=30"},
 			},
 			wantDevices: []Device{
-				{Value: "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=2,bootindex=1,werror=stop,rerror=stop"},
+				{Value: "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=2,bootindex=1"},
 				{Value: "virtio-blk-pci,id=vdisk-vol-data-a,drive=nbd-vol-data-a,iothread=ioth-vol-data-a,serial=voldataa,bus=hotplug-ebs3,werror=stop,rerror=stop"},
 			},
 			wantHotplugPorts: []int{0, 0, 3},
@@ -348,72 +348,31 @@ func TestBuildDrives(t *testing.T) {
 				{Name: "vol-boot", NBDURI: "nbd:unix:/tmp/boot.sock", Boot: true},
 				{Name: "vol-blk", BlockDevice: "/dev/sdx", BlockSizeBytes: 1 << 20},
 			},
-			cpuCount:      2,
+			cpuCount: 2,
+			wantDrives: []Drive{
+				{File: "nbd:unix:/tmp/boot.sock", Format: "raw", If: "none", Media: "disk", ID: "os", Cache: "none", Werror: "stop", Rerror: "stop", ReconnectDelay: NBDReconnectDelaySeconds},
+			},
 			wantIOThreads: []IOThread{{ID: "ioth-os"}, {ID: "ioth-vol-blk"}},
 			wantBlockdevs: []Blockdev{
-				{Value: "driver=raw,node-name=os,cache.direct=on,file.driver=nbd,file.server.type=unix,file.server.path=/tmp/boot.sock,file.export=,file.reconnect-delay=30"},
 				{Value: "driver=raw,node-name=nbd-vol-blk,size=1048576,file.driver=host_device,file.filename=/dev/sdx,file.aio=native,file.cache.direct=on"},
 			},
 			wantDevices: []Device{
-				{Value: "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=2,bootindex=1,werror=stop,rerror=stop"},
+				{Value: "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=2,bootindex=1"},
 				{Value: "virtio-blk-pci,id=vdisk-vol-blk,drive=nbd-vol-blk,iothread=ioth-vol-blk,serial=volblk,bus=hotplug-ebs1,werror=stop,rerror=stop"},
 			},
 			wantHotplugPorts: []int{0, 1},
 		},
 		{
-			name: "block boot volume is a raw node over host_device named os under the bootindex device",
-			requests: []EBSRequest{
-				{Name: "vol-boot", BlockDevice: "/dev/sdx", BlockSizeBytes: 8 << 30, Boot: true},
-			},
-			cpuCount:      2,
-			wantIOThreads: []IOThread{{ID: "ioth-os"}},
-			wantBlockdevs: []Blockdev{
-				{Value: "driver=raw,node-name=os,size=8589934592,file.driver=host_device,file.filename=/dev/sdx,file.aio=native,file.cache.direct=on"},
-			},
-			wantDevices: []Device{
-				{Value: "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=2,bootindex=1,werror=stop,rerror=stop"},
-			},
-			wantHotplugPorts: []int{0},
-		},
-		{
-			name: "block boot volume with four vCPUs keeps the mapped device and error policy",
-			requests: []EBSRequest{
-				{Name: "vol-boot", BlockDevice: "/dev/sdx", BlockSizeBytes: 8 << 30, Boot: true},
-			},
-			cpuCount:      4,
-			wantIOThreads: []IOThread{{ID: "ioth-os-0"}, {ID: "ioth-os-1"}},
-			wantBlockdevs: []Blockdev{
-				{Value: "driver=raw,node-name=os,size=8589934592,file.driver=host_device,file.filename=/dev/sdx,file.aio=native,file.cache.direct=on"},
-			},
-			wantDevices: []Device{
-				{Value: `{"driver":"virtio-blk-pci","drive":"os","num-queues":4,"bootindex":1,"iothread-vq-mapping":[{"iothread":"ioth-os-0","vqs":[0,2]},{"iothread":"ioth-os-1","vqs":[1,3]}],"werror":"stop","rerror":"stop"}`},
-			},
-		},
-		{
-			name: "NBD boot volume over TCP renders inet server options under file.",
-			requests: []EBSRequest{
-				{Name: "vol-boot", NBDURI: "nbd://127.0.0.1:44801", Boot: true},
-			},
-			cpuCount:      2,
-			wantIOThreads: []IOThread{{ID: "ioth-os"}},
-			wantBlockdevs: []Blockdev{
-				{Value: "driver=raw,node-name=os,cache.direct=on,file.driver=nbd,file.server.type=inet,file.server.host=127.0.0.1,file.server.port=44801,file.export=,file.reconnect-delay=30"},
-			},
-			wantDevices: []Device{
-				{Value: "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=2,bootindex=1,werror=stop,rerror=stop"},
-			},
-		},
-		{
-			name:     "unparseable NBD boot URI is rejected",
-			requests: []EBSRequest{{Name: "vol-boot", NBDURI: "http://example/boot", Boot: true}},
+			name:     "block boot volume is rejected",
+			requests: []EBSRequest{{Name: "vol-boot", BlockDevice: "/dev/sdx", BlockSizeBytes: 1 << 20, Boot: true}},
 			cpuCount: 2,
-			wantErr:  "parse NBDURI for volume vol-boot",
+			wantErr:  "volume vol-boot: block device export is only supported for data volumes",
 		},
 		{
 			name:     "block EFI volume is rejected",
 			requests: []EBSRequest{{Name: "vol-efi", BlockDevice: "/dev/sdx", BlockSizeBytes: 1 << 20, EFI: true}},
 			cpuCount: 2,
-			wantErr:  "volume vol-efi: block device export is not supported for an EFI volume",
+			wantErr:  "volume vol-efi: block device export is only supported for data volumes",
 		},
 		{
 			name:     "both NBDURI and BlockDevice set is rejected",
@@ -1150,7 +1109,6 @@ func TestStartQEMU_DiskBoot_MgmtTapCreated(t *testing.T) {
 		InstanceType: "t3.nano",
 		MgmtMAC:      "02:aa:bb:cc:dd:ee",
 	}
-	instance.EBSRequests.Requests = []EBSRequest{{Name: "vol-root", NBDURI: "nbd:unix:/tmp/root.sock", Boot: true}}
 
 	err := m.startQEMU(instance, false)
 
@@ -2210,88 +2168,6 @@ func TestStartQEMU_DiskBoot_DataVolumeStaysOnThePCIPath(t *testing.T) {
 	assert.Equal(t, 1, instance.EBSRequests.Requests[1].HotplugPort)
 }
 
-// TestStartQEMU_DiskBoot_BootVolumeIsABlockdevNode launches a disk-boot guest
-// from each export kind and checks the boot volume reaches QEMU as the "os"
-// -blockdev node behind the bootindex=1 device, with no -drive left for it.
-func TestStartQEMU_DiskBoot_BootVolumeIsABlockdevNode(t *testing.T) {
-	tests := []struct {
-		name         string
-		req          func(t *testing.T) EBSRequest
-		wantBlockdev func(req EBSRequest) string
-	}{
-		{
-			name: "nbd export",
-			req: func(t *testing.T) EBSRequest {
-				return EBSRequest{Name: "vol-root", NBDURI: nbdListener(t), Boot: true}
-			},
-			wantBlockdev: func(req EBSRequest) string {
-				hostPort := strings.TrimPrefix(req.NBDURI, "nbd://")
-				host, port, _ := strings.Cut(hostPort, ":")
-				return "driver=raw,node-name=os,cache.direct=on,file.driver=nbd,file.server.type=inet,file.server.host=" + host +
-					",file.server.port=" + port + ",file.export=,file.reconnect-delay=30"
-			},
-		},
-		{
-			name: "block export",
-			req: func(t *testing.T) EBSRequest {
-				dev := filepath.Join(t.TempDir(), "sdx")
-				require.NoError(t, os.WriteFile(dev, nil, 0o600))
-				return EBSRequest{Name: "vol-root", BlockDevice: dev, BlockSizeBytes: 8 << 30, Boot: true}
-			},
-			wantBlockdev: func(req EBSRequest) string {
-				return "driver=raw,node-name=os,size=8589934592,file.driver=host_device,file.filename=" + req.BlockDevice +
-					",file.aio=native,file.cache.direct=on"
-			},
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			m := directBootManager(t, &fakeNetworkPlumber{})
-			launches := withFakeQEMU(t)
-
-			req := tt.req(t)
-			instance := &VM{ID: "i-boot-blockdev", InstanceType: "t3.nano"}
-			instance.EBSRequests.Requests = []EBSRequest{req}
-			require.NoError(t, m.startQEMU(instance, false))
-
-			got := launches()
-			require.Len(t, got, 1)
-			assert.Equal(t, []string{tt.wantBlockdev(req)}, allArgValues(got[0], "-blockdev"))
-			assert.Contains(t, allArgValues(got[0], "-device"), "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=1,bootindex=1,werror=stop,rerror=stop")
-			for _, d := range allArgValues(got[0], "-drive") {
-				assert.NotContains(t, d, "id=os", "the boot volume must not also be a -drive")
-			}
-		})
-	}
-}
-
-// TestStartQEMU_DiskBoot_NoBootVolumeIsNotLaunched covers a disk-boot guest
-// whose only volumes are data volumes: their -blockdev nodes would satisfy
-// Execute, but QEMU would have nothing to boot from.
-func TestStartQEMU_DiskBoot_NoBootVolumeIsNotLaunched(t *testing.T) {
-	tests := []struct {
-		name     string
-		requests []EBSRequest
-	}{
-		{name: "data volume only", requests: []EBSRequest{{Name: "vol-data-a", NBDURI: "nbd:unix:/tmp/data-a.sock"}}},
-		{name: "no volumes", requests: nil},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			m := directBootManager(t, &fakeNetworkPlumber{})
-			launches := withFakeQEMU(t)
-
-			instance := &VM{ID: "i-no-boot", InstanceType: "t3.nano"}
-			instance.EBSRequests.Requests = tt.requests
-			err := m.startQEMU(instance, false)
-
-			require.Error(t, err)
-			assert.Contains(t, err.Error(), "no boot volume among the instance's EBS requests")
-			assert.Empty(t, launches(), "QEMU must not start")
-		})
-	}
-}
-
 func deviceValues(devs []Device) []string {
 	out := make([]string, len(devs))
 	for i, d := range devs {

spinifex/runtime/compute/vm/vm.go

@@ -371,8 +371,8 @@ type Device struct {
 	Value string `json:"value"`
 }
 
-// Drive is one QEMU -drive entry, used for the pflash firmware drives (the
-// CODE blob and the EFI volume). Each set field becomes a key=value option.
+// Drive is one QEMU -drive entry, used for boot, EFI and pflash firmware
+// drives. Each set field becomes a key=value option.
 type Drive struct {
 	File   string `json:"file"`
 	Format string `json:"format"`
@@ -380,6 +380,15 @@ type Drive struct {
 	Media  string `json:"media"`
 	ID     string `json:"id"`
 	Cache  string `json:"cache,omitempty"`
+	// Werror and Rerror set QEMU's block on-error policy. "stop" pauses the VM
+	// and holds the failed request, so a transient backend loss is resumed with
+	// cont instead of reaching the guest as EIO and corrupting its filesystem.
+	Werror string `json:"werror,omitempty"`
+	Rerror string `json:"rerror,omitempty"`
+	// ReconnectDelay emits file.reconnect-delay, the seconds an NBD disconnect
+	// pauses requests before failing them. The file. prefix is required: the
+	// bare name reaches the format driver, which has no such option.
+	ReconnectDelay int `json:"reconnect_delay,omitempty"`
 	// Unit selects the pflash slot when If=="pflash": 0 is the CODE blob,
 	// 1 is the per-VM VARS volume. Ignored for non-pflash drives.
 	Unit int `json:"unit,omitempty"`
@@ -396,9 +405,9 @@ type IOThread struct {
 // Blockdev is a QEMU -blockdev entry: the command-line equivalent of the
 // blockdev-add QMP command. Unlike a -drive with if=none, it produces a
 // monitor-owned node addressable by node-name, which is what makes
-// blockdev-del able to remove it later. Used for boot and data volumes, so a
-// cold-booted data volume has the same detachable block-graph shape as a
-// hot-attached one; EFI volumes keep the pflash -drive shape.
+// blockdev-del able to remove it later. Used for data (non-boot, non-EFI)
+// volumes so a cold-booted one has the same detachable block-graph shape as
+// a hot-attached one; boot/EFI volumes keep the legacy -drive shape.
 type Blockdev struct {
 	Value string `json:"value"`
 }
@@ -562,8 +571,8 @@ func (cfg *Config) Execute() (*exec.Cmd, error) {
 		args = append(args, "-object", fmt.Sprintf("iothread,id=%s", iot.ID))
 	}
 
-	if len(cfg.Drives) == 0 && len(cfg.Blockdevs) == 0 && cfg.KernelImage == "" {
-		return nil, fmt.Errorf("at least one drive, blockdev or a kernel image is required")
+	if len(cfg.Drives) == 0 && cfg.KernelImage == "" {
+		return nil, fmt.Errorf("at least one drive or a kernel image is required")
 	}
 
 	drives := cfg.Drives
@@ -613,6 +622,18 @@ func (cfg *Config) Execute() (*exec.Cmd, error) {
 			opts = append(opts, fmt.Sprintf("cache=%s", drive.Cache))
 		}
 
+		if drive.Werror != "" {
+			opts = append(opts, fmt.Sprintf("werror=%s", drive.Werror))
+		}
+
+		if drive.Rerror != "" {
+			opts = append(opts, fmt.Sprintf("rerror=%s", drive.Rerror))
+		}
+
+		if drive.ReconnectDelay > 0 {
+			opts = append(opts, fmt.Sprintf("file.reconnect-delay=%d", drive.ReconnectDelay))
+		}
+
 		args = append(args, "-drive", strings.Join(opts, ","))
 	}
 

spinifex/runtime/compute/vm/vm_test.go

@@ -39,7 +39,7 @@ func TestExecute(t *testing.T) {
 
 	// Expect error, at least one drive or kernel image required
 	assert.Error(t, err)
-	assert.ErrorContains(t, err, "at least one drive, blockdev or a kernel image is required")
+	assert.ErrorContains(t, err, "at least one drive or a kernel image is required")
 	assert.Nil(t, cmd)
 
 	cfg.Drives = []Drive{
@@ -347,14 +347,6 @@ func TestExecute_DriveAndKernelInvariant(t *testing.T) {
 		assert.NotNil(t, cmd)
 	})
 
-	t.Run("blockdevs-only is OK", func(t *testing.T) {
-		cfg := base
-		cfg.Blockdevs = []Blockdev{{Value: "driver=raw,node-name=os,file.driver=nbd,file.server.type=unix,file.server.path=/run/os.sock"}}
-		cmd, err := cfg.Execute()
-		assert.NoError(t, err)
-		assert.NotNil(t, cmd)
-	})
-
 	t.Run("kernel-only is OK", func(t *testing.T) {
 		cfg := base
 		cfg.KernelImage = "/boot/vmlinuz"
@@ -367,7 +359,7 @@ func TestExecute_DriveAndKernelInvariant(t *testing.T) {
 		cfg := base
 		cmd, err := cfg.Execute()
 		assert.Error(t, err)
-		assert.ErrorContains(t, err, "at least one drive, blockdev or a kernel image")
+		assert.ErrorContains(t, err, "at least one drive or a kernel image")
 		assert.Nil(t, cmd)
 	})
 }

spinifex/runtime/compute/vm/volumes.go

@@ -11,6 +11,7 @@ import (
 	"time"
 
 	"github.com/aws/aws-sdk-go/service/ec2"
+	"github.com/mulgadc/spinifex/spinifex/runtime/compute/nbd"
 	"github.com/mulgadc/spinifex/spinifex/runtime/compute/qmp"
 )
 
@@ -142,7 +143,7 @@ func (m *Manager) AttachVolume(ctx context.Context, id, volumeID, device string)
 	if kind == volumeExportBlock {
 		blockdevArgs = HostDeviceOpts{Path: ebsRequest.BlockDevice, SizeBytes: ebsRequest.BlockSizeBytes}.QMPArgs(nodeName)
 	} else {
-		server, err := nbdServerOpts(ebsRequest.NBDURI)
+		serverType, socketPath, nbdHost, nbdPort, err := nbd.ParseNBDURI(ebsRequest.NBDURI)
 		if err != nil {
 			slog.ErrorContext(ctx, "AttachVolume: failed to parse NBDURI", "uri", ebsRequest.NBDURI, "err", err)
 			_ = m.rollbackUnmount(ctx, instance.AccountID, ebsRequest)
@@ -151,7 +152,7 @@ func (m *Manager) AttachVolume(ctx context.Context, id, volumeID, device string)
 		blockdevArgs = map[string]any{
 			"node-name":       nodeName,
 			"driver":          "nbd",
-			"server":          server.QMPArg(),
+			"server":          NBDServerOpts{Type: serverType, Path: socketPath, Host: nbdHost, Port: nbdPort}.QMPArg(),
 			"export":          "",
 			"read-only":       false,
 			"reconnect-delay": NBDReconnectDelaySeconds,