fix(uefi): keep an unrecordable write-back owed and prune off the stop path

A stop whose write-back obligation the store record could not take now
marks the instance record with the owing node (uefi_write_back_owed),
carried by the stop's own record write, and keeps the obligation in
write-back.json beside seeded.json. A start on another node refuses while
the marked node is a cluster member. The reaper records a kept obligation
once the store record answers, lands it by the existing rules, and clears
the marker in the same compare-and-set.

Pruning no longer runs inside a stop: a landed write-back queues the prefix
and the reaper prunes it, keeping the pointer as read at prune time.

contracts/ec2/v1/README.md

@@ -37,6 +37,13 @@ contract or an approved compatibility plan; it is not a package refactor.
 Only the subject is owned here today.
 The request (`account_id`) and reply (`vpc_id`, `error`) bodies are still declared as anonymous structs on each side; moving them into this package is a contract change that needs its own characterization first.
 
+## Volume held elsewhere
+
+A start refused because another node holds one of the instance's volumes is an `IncorrectState` error payload on the `ec2.start`, `ec2.start.<node>` and `ec2.cmd.<instance-id>` replies, and the AWS body cannot say why.
+The owning node adds the `X-Volume-Held-Elsewhere: true` header to that reply, the `ec2.start` forwarder relays it, and `StartInstances` wraps `ErrVolumeHeldElsewhere` around the error it rebuilds, so a caller matches it with `errors.Is` rather than reading the message.
+The owning node sets the header only from the EBS provider's `volume_held_elsewhere` code, which every provider returns for this refusal; no reader matches the message text.
+`VolumeHeldElsewhereText` is the wording viperblockd puts in that refusal's message, not a signal.
+
 ## Platform inventory
 
 `PlatformInventorySubject` (`ec2.v1.PlatformInventory`) is a request/reply route on which a daemon in the `spinifex-workers` queue group reports one account's key pairs, placement groups, launch templates and their `ec2-tags` entries from the EC2 owner stores.

contracts/ec2/v1/volume_held.go

@@ -1,7 +1,13 @@
 package ec2v1
 
-// VolumeHeldElsewhereText is the text of a start refused because another node
-// holds one of the instance's volumes. It crosses NATS as message text, so
-// readers match on it; the provider that refuses and every reader share this
-// string so they cannot drift.
+import "errors"
+
+// VolumeHeldElsewhereText is the message viperblockd gives a lease refusal. It
+// is wording for the operator; callers classify the refusal by its code.
 const VolumeHeldElsewhereText = "volume is leased by another owner"
+
+// ErrVolumeHeldElsewhere marks a start refused because another node still
+// holds one of the instance's volumes. The owning node sets it from the EBS
+// provider's held-elsewhere code, and a start reply carries it across NATS as
+// the X-Volume-Held-Elsewhere header rather than as message text.
+var ErrVolumeHeldElsewhere = errors.New("a volume is leased by another node")

docs/COMMANDS.md

@@ -95,7 +95,7 @@ Operational commands for inspecting cluster state. These fan out NATS requests t
 
 | Command | Flags | Description |
 |---------|-------|-------------|
-| `spx admin init` | `--nodes`, `--node`, `--bind`, `--advertise` (external IP off-host clients dial; auto-detected from the default route), `--port`, `--region`, `--az`, `--cluster-name`, `--cluster-bind`, `--cluster-routes`, `--predastore-nodes`, `--predastore-compaction-interval` (seconds; test clusters set a short one), `--services`, `--formation-timeout`, `--token-ttl`, `--force`, `--discard-jetstream` (with `--nodes` >= 2, remove this node's existing JetStream streams once every node has joined; without it a multi-node init over a store that holds streams is refused, and it never proceeds while NATS is running), `--skip-host-dns`, `--email` (operator address for update and security notices), `--no-telemetry`, `--gpu-passthrough`, `--ipsec` (default true; encrypt intra-AZ Geneve via OVN native IPsec cluster-wide — disable only for a trusted single-rack lab; on a host whose IPsec stack is libreswan it defaults to false and `--ipsec=true` is refused), `--host` (leader to join; falls back to multicast discovery), `--external-mode` (`pool` \| `nat` routed NAT for non-bridgeable uplinks, single-node; pair with `setup-ovn.sh --nat-uplink`; note: system instances (ECS/EKS/load-balancer agents) are unsupported in nat v1 — they require `pool` mode), `--external-iface` (WAN NIC for br-external, auto-detected), `--external-source` (`dhcp` \| `static`), `--external-pool` (`start-end`), `--external-prefix-len` (default 24), `--external-gateway` (auto-detected), `--external-dns` (comma-separated IPv4 addresses, at most 3; the DNS servers written to the external pool's `dns_servers` and handed to guests, verbatim; when omitted they are detected from the host resolver at init time, so set it to give every node of a cluster the same value), `--external-bind-bridge` (default `br-wan` when `--external-source=dhcp`), `--gateway-ip` (OVN gateway router's external IP for SNAT; defaults to the pool's range start, required for `nat` mode without DHCP) | Generates root IAM credentials (AKIA-prefixed access key + secret) → creates master.key (AES-256, 32 bytes, 0600) → writes bootstrap.json (consumed on first start) → generates CA + server TLS certificates → generates join token (written to `join-token` file, displayed in join command) → creates NATS config with auth token → writes spinifex.toml, awsgw.toml, predastore.toml → configures AWS CLI `spx` profile → creates directory structure under `~/spinifex/` → points the host resolver at this node's local northstar so the Spinifex zones (ELB/EKS names) resolve from the node itself (opt out with `--skip-host-dns`). `--force` on an already-initialized node is idempotent for crypto: it preserves the existing keys (master.key, predastore/viperblock encryption keys), system + admin credentials, and CA, and refreshes only the config files and the CA-signed server certificate (so a changed bind IP / SANs is picked up without breaking already-joined nodes or CA-baked AMIs). A genuine clean slate requires removing the data dirs first. |
+| `spx admin init` | `--nodes`, `--node`, `--bind`, `--advertise` (external IP off-host clients dial; auto-detected from the default route), `--port`, `--region`, `--az`, `--cluster-name`, `--cluster-bind`, `--cluster-routes`, `--predastore-nodes`, `--predastore-compaction-interval` (seconds; test clusters set a short one), `--services`, `--formation-timeout`, `--token-ttl`, `--force`, `--discard-jetstream` (with `--nodes` >= 2, remove this node's existing JetStream streams once every node has joined; without it a multi-node init over a store that holds streams is refused, and it never proceeds while NATS is running), `--skip-host-dns`, `--email` (operator address for update and security notices), `--no-telemetry`, `--gpu-passthrough`, `--ipsec` (default true; encrypt intra-AZ Geneve via OVN native IPsec cluster-wide — disable only for a trusted single-rack lab; on a host whose IPsec stack is libreswan it defaults to false and `--ipsec=true` is refused), `--external-mode` (`pool` \| `nat` routed NAT for non-bridgeable uplinks, single-node; pair with `setup-ovn.sh --nat-uplink`; note: system instances (ECS/EKS/load-balancer agents) are unsupported in nat v1 — they require `pool` mode), `--external-iface` (WAN NIC for br-external, auto-detected), `--external-source` (`dhcp` \| `static`), `--external-pool` (`start-end`), `--external-prefix-len` (default 24), `--external-gateway` (auto-detected), `--external-dns` (comma-separated IPv4 addresses, at most 3; the DNS servers written to the external pool's `dns_servers` and handed to guests, verbatim; when omitted they are detected from the host resolver at init time, so set it to give every node of a cluster the same value), `--external-bind-bridge` (default `br-wan` when `--external-source=dhcp`), `--gateway-ip` (OVN gateway router's external IP for SNAT; defaults to the pool's range start, required for `nat` mode without DHCP) | Generates root IAM credentials (AKIA-prefixed access key + secret) → creates master.key (AES-256, 32 bytes, 0600) → writes bootstrap.json (consumed on first start) → generates CA + server TLS certificates → generates join token (written to `join-token` file, displayed in join command) → creates NATS config with auth token → writes spinifex.toml, awsgw.toml, predastore.toml → configures AWS CLI `spx` profile → creates directory structure under `~/spinifex/` → points the host resolver at this node's local northstar so the Spinifex zones (ELB/EKS names) resolve from the node itself (opt out with `--skip-host-dns`). `--force` on an already-initialized node is idempotent for crypto: it preserves the existing keys (master.key, predastore/viperblock encryption keys), system + admin credentials, and CA, and refreshes only the config files and the CA-signed server certificate (so a changed bind IP / SANs is picked up without breaking already-joined nodes or CA-baked AMIs). A genuine clean slate requires removing the data dirs first. |
 | `spx admin join` | `--host` (required), `--node` (required), `--token` (required), `--bind`, `--advertise` (defaults to `--bind`, or the auto-detected WAN IP when `--bind` is 0.0.0.0), `--port`, `--region`, `--az`, `--cluster-bind`, `--cluster-routes`, `--data-dir`, `--services`, `--skip-host-dns`, `--email`, `--no-telemetry`, `--predastore-compaction-interval`, `--join-timeout` (how long to keep retrying while the formation server is unreachable, default 20m), `--force` (join even though this node is already initialized or holds JetStream streams, **discarding its own CA, master key and JetStream store**; the store is removed only after formation succeeds, and the join is refused while NATS is running even with `--force`) | Connects to leader node with join token (Authorization: Bearer header) → retrieves cluster configuration → configures local node to join cluster and participate in distributed operations → points the host resolver at this node's local northstar so the Spinifex zones resolve from the node itself (opt out with `--skip-host-dns`) |
 
 ### Version
@@ -135,7 +135,7 @@ Operational commands for inspecting cluster state. These fan out NATS requests t
 
 | Command | Flags | Description |
 |---------|-------|-------------|
-| `spx admin volumes orphans` | — | Lists volumes the storage provider holds that the control plane has no record of. Their blocks consume space but they have no API handle, so they can be neither described nor deleted through the EC2 API. **Reports only, never deletes**: the evidence that an orphan is still wanted is the record that went missing, so removal is an operator decision taken per volume. |
+| `spx admin volumes orphans` | — | Lists volumes the storage provider holds that the control plane has no record of. Their blocks consume space but they have no API handle, so they can be neither described nor deleted through the EC2 API. **Reports only, never deletes**: the evidence that an orphan is still wanted is the record that went missing, so removal is an operator decision taken per volume. It also lists UEFI variable stores nothing owns: a `uefi/i-*/` prefix with no instance record and no store record, a `uefi/ami-*/` prefix with no AMI document, and a store record that is not `deleting` and has no instance record. These are reported and never deleted, because an absent instance record is not proof of a terminate. |
 | `spx admin volumes unsealed` | — | Lists volumes a node has open, or had open, without a confirmed seal to the backend — the node named holds writes predastore may not have. Starting such a volume elsewhere is refused while that node is alive (its daemon heartbeat is under 60 seconds old, or cannot be read) and is otherwise allowed, in which case it opens from the last checkpoint that did reach the backend. This is the list of volumes where that would cost something. |
 | `spx admin storage status` | `--endpoint` (default: this node's AWS gateway), `--region` (default: this node's region), `--ca-bundle` (default: this node's CA) | Sends the signed `GetStorageStatus` action to the AWS gateway with credentials from the standard AWS chain (e.g. `AWS_PROFILE=spinifex`) and prints what the console's Service Metrics page shows: predastore encoding, meta and blob node counts, cluster health, a meta node table (health, raft state, leader, term, commit and applied index) and the blob nodes. Cluster health is `Degraded` when any meta node is unreachable or the nodes reporting raft state do not agree on one leader. A meta node read only through its host's `/readyz` shows healthy with blank raft fields. |
 

docs/DESIGN.md

@@ -111,7 +111,7 @@ Daemons (`spinifex/runtime/daemon/daemon.go`) subscribe to NATS topics and handl
 When a daemon handles `RunInstances`:
 
 1. **Resource Check**: Validates CPU/memory availability via `ResourceManager`
-2. **Volume Generation**: Creates boot and EFI volumes via Viperblock
+2. **Volume Generation**: Creates the boot volume through the EBS provider and, for a UEFI guest, seeds its variable store as an object under `uefi/<instance-id>/` with a store record in `spinifex-uefi-stores` and a pointer on the instance record
 3. **Volume Mount**: Publishes each volume through the EBS provider contract (`PublishVolume` on `ebs.provider.v1.{node}.mount`), receives its export
 4. **QEMU Launch**: Builds and executes QEMU command with NBD-backed disks
 5. **QMP Monitoring**: Establishes QEMU Machine Protocol connection for VM management

internal/awsmodel/model.go

@@ -92,6 +92,7 @@ type Shape struct {
 	Min             *float64
 	Max             *float64
 	Pattern         string
+	Documentation   string
 	LocationName    string
 	TimestampFormat string
 	XMLNamespace    XMLNamespace

internal/awsmodel/smithy.go

@@ -443,6 +443,9 @@ func (t *smithyTranslator) applyShapeTraits(shape *Shape, traits smithyTraits) e
 	if shape.Pattern, err = traits.string("smithy.api#pattern"); err != nil {
 		return err
 	}
+	if shape.Documentation, err = traits.string("smithy.api#documentation"); err != nil {
+		return err
+	}
 	if shape.LocationName, err = traits.string("smithy.api#xmlName"); err != nil {
 		return err
 	}

internal/gwsign/signer.go

@@ -15,8 +15,7 @@ import (
 	v4 "github.com/aws/aws-sdk-go-v2/aws/signer/v4"
 	awsconfig "github.com/aws/aws-sdk-go-v2/config"
 	"github.com/aws/aws-sdk-go-v2/credentials"
-	"github.com/aws/aws-sdk-go-v2/credentials/ec2rolecreds"
-	"github.com/aws/aws-sdk-go-v2/feature/ec2/imds"
+	"github.com/mulgadc/spinifex/internal/imdscreds"
 )
 
 // Signer holds a credential provider and signs requests in place.
@@ -46,12 +45,14 @@ func NewIMDS(ctx context.Context, region string) (*Signer, error) {
 
 // NewInstanceRole returns a Signer whose credentials come only from the
 // instance metadata service: no environment, profile or static-key fallback.
+// A failed refresh of expired credentials is an error, not the stale key.
 func NewInstanceRole() *Signer {
-	return newInstanceRole(imds.New(imds.Options{}))
+	return newInstanceRole("")
 }
 
-func newInstanceRole(client *imds.Client) *Signer {
-	return &Signer{provider: aws.NewCredentialsCache(ec2rolecreds.New(func(o *ec2rolecreds.Options) { o.Client = client }))}
+// newInstanceRole reads from the IMDS root base; "" means the default endpoint.
+func newInstanceRole(base string) *Signer {
+	return &Signer{provider: aws.NewCredentialsCache(imdscreds.NewProvider(nil, base))}
 }
 
 // EnsureCredentials retrieves once from the provider, surfacing the error so a

internal/gwsign/signer_test.go

@@ -12,7 +12,6 @@ import (
 
 	"github.com/aws/aws-sdk-go-v2/aws"
 	"github.com/aws/aws-sdk-go-v2/credentials"
-	"github.com/aws/aws-sdk-go-v2/feature/ec2/imds"
 	"github.com/mulgadc/bluebottle/pkg/sigv4"
 )
 
@@ -145,7 +144,7 @@ func TestNewInstanceRole_UsesOnlyIMDS(t *testing.T) {
 	}))
 	t.Cleanup(srv.Close)
 
-	s := newInstanceRole(imds.New(imds.Options{Endpoint: srv.URL}))
+	s := newInstanceRole(srv.URL)
 	req := httptest.NewRequest(http.MethodPost, "https://gw.example/", nil)
 	if err := s.Sign(req, emptySHA, testService, testRegion); err != nil {
 		t.Fatal(err)
@@ -155,10 +154,42 @@ func TestNewInstanceRole_UsesOnlyIMDS(t *testing.T) {
 	}
 
 	srv.Close()
-	cold := newInstanceRole(imds.New(imds.Options{Endpoint: srv.URL}))
+	cold := newInstanceRole(srv.URL)
 	if err := cold.EnsureCredentials(context.Background()); err == nil {
 		t.Fatal("environment keys were used when IMDS was unreachable")
 	}
 }
 
+// Once the cached instance-role key has expired, a refresh that cannot reach
+// IMDS must fail the sign rather than reuse the expired key with a new expiry.
+func TestNewInstanceRole_FailedRefreshErrorsInsteadOfStaleKey(t *testing.T) {
+	const staleKey = "ASIASTALEEXAMPLE0001"
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		switch r.URL.Path {
+		case "/latest/api/token":
+			_, _ = w.Write([]byte("token"))
+		case "/latest/meta-data/iam/security-credentials/":
+			_, _ = w.Write([]byte("sqsWorkerRole"))
+		case "/latest/meta-data/iam/security-credentials/sqsWorkerRole":
+			_, _ = w.Write([]byte(`{"Code":"Success","AccessKeyId":"` + staleKey + `","SecretAccessKey":"s","Token":"t","Expiration":"2000-01-01T00:00:00Z"}`))
+		default:
+			http.NotFound(w, r)
+		}
+	}))
+	s := newInstanceRole(srv.URL)
+	if err := s.EnsureCredentials(t.Context()); err != nil {
+		t.Fatalf("first retrieve: %v", err)
+	}
+
+	srv.Close()
+	req := httptest.NewRequest(http.MethodPost, "https://gw.example/", nil)
+	err := s.Sign(req, emptySHA, testService, testRegion)
+	if err == nil {
+		t.Fatalf("Sign after failed refresh = nil, signed with %q", req.Header.Get("Authorization"))
+	}
+	if auth := req.Header.Get("Authorization"); auth != "" {
+		t.Fatalf("request carries Authorization %q after a failed refresh", auth)
+	}
+}
+
 const emptySHA = "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"

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

@@ -80,5 +80,5 @@ func DescribeInstanceAttribute(ctx context.Context, input *ec2.DescribeInstanceA
 	// terraform retries cleanly.
 	slog.WarnContext(ctx, "DescribeInstanceAttribute: instance absent or no responses",
 		"instance_id", *input.InstanceId, "received", sum.Received, "expected", expectedNodes)
-	return nil, errors.New(awserrors.ErrorInvalidInstanceIDNotFound)
+	return nil, awserrors.IDNotFound(awserrors.ErrorInvalidInstanceIDNotFound, "instance", *input.InstanceId)
 }

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

@@ -184,7 +184,7 @@ func TestDescribeInstanceAttribute_AllNodesNotFound(t *testing.T) {
 
 	_, err = DescribeInstanceAttribute(context.Background(), input, nc, 2, "123456789012")
 	require.Error(t, err)
-	assert.Equal(t, awserrors.ErrorInvalidInstanceIDNotFound, err.Error())
+	assert.EqualError(t, err, "The instance ID 'i-missing' does not exist: "+awserrors.ErrorInvalidInstanceIDNotFound)
 }
 
 // TestDescribeInstanceAttribute_ClientErrorPropagates ensures that a
@@ -254,7 +254,7 @@ func TestDescribeInstanceAttribute_NoResponders(t *testing.T) {
 	duration := time.Since(start)
 
 	require.Error(t, err)
-	assert.Equal(t, awserrors.ErrorInvalidInstanceIDNotFound, err.Error())
+	assert.EqualError(t, err, "The instance ID 'i-nowhere' does not exist: "+awserrors.ErrorInvalidInstanceIDNotFound)
 	// Must not block the caller past the 3 s deadline.
 	assert.Less(t, duration, 4*time.Second)
 }

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

@@ -126,7 +126,7 @@ func DescribeInstancesChecked(ctx context.Context, input *ec2.DescribeInstancesI
 		missing := missingInstanceIDs(reservations, input.InstanceIds)
 		if len(missing) > 0 {
 			if complete {
-				return nil, errors.New(awserrors.ErrorInvalidInstanceIDNotFound)
+				return nil, awserrors.IDsNotFound(awserrors.ErrorInvalidInstanceIDNotFound, "instance", missing)
 			}
 			// An incomplete sweep cannot prove absence, so a missing id is
 			// unresolved, not gone. NotFound would be a false negative and an

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

@@ -448,11 +448,13 @@ func TestDescribeInstancesChecked_ExplicitIDNotFound_CompleteSweep(t *testing.T)
 	require.NoError(t, err)
 	nc.Flush()
 
-	input := &ec2.DescribeInstancesInput{InstanceIds: []*string{aws.String("i-doesnotexist0000000")}}
+	input := &ec2.DescribeInstancesInput{InstanceIds: aws.StringSlice([]string{"i-doesnotexist0000000", "i-real", "i-doesnotexist0000001"})}
 	_, err = DescribeInstancesChecked(context.Background(), input, nc, 1, nil, "123456789012")
 
-	require.Error(t, err)
-	assert.Equal(t, awserrors.ErrorInvalidInstanceIDNotFound, err.Error())
+	code, message, ok := awserrors.ResolveErrorDetail(err)
+	require.True(t, ok, "err = %v", err)
+	assert.Equal(t, awserrors.ErrorInvalidInstanceIDNotFound, code)
+	assert.Equal(t, "The instance IDs 'i-doesnotexist0000000, i-doesnotexist0000001' do not exist", message)
 }
 
 // TestDescribeInstances_ExplicitIDNotFound_StaysSilent guards the internal
@@ -624,8 +626,7 @@ func TestDescribeInstancesChecked_Identity_AllNodesAnswer_NotFoundAsserted(t *te
 	_, err := DescribeInstancesChecked(context.Background(), input, nc, 0, []string{"node-1", "node-2"}, "123456789012",
 		WithFanoutTimeout(2*time.Second))
 
-	require.Error(t, err)
-	assert.Equal(t, awserrors.ErrorInvalidInstanceIDNotFound, err.Error())
+	assert.True(t, awserrors.IsErrorCode(err, awserrors.ErrorInvalidInstanceIDNotFound), "err = %v", err)
 }
 
 // A node answering with a decodable error envelope never becomes a valid
@@ -684,7 +685,7 @@ func TestDescribeInstancesChecked_Identity_NilReservationsIsValidResponder(t *te
 		WithFanoutTimeout(2*time.Second))
 
 	require.Error(t, err, "a nil-Reservations node is a complete, valid responder, so the sweep is complete and NotFound must fire")
-	assert.Equal(t, awserrors.ErrorInvalidInstanceIDNotFound, err.Error())
+	assert.True(t, awserrors.IsErrorCode(err, awserrors.ErrorInvalidInstanceIDNotFound), "err = %v", err)
 }
 
 // An empty nodeIDs falls back to the legacy expectedNodes path, which the

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

@@ -47,7 +47,7 @@ func GetConsoleOutput(ctx context.Context, input *ec2.GetConsoleOutputInput, nat
 	if err != nil {
 		// No daemon subscription or timeout: stopped/terminated/non-existent instances all surface as NotFound.
 		if errors.Is(err, nats.ErrNoResponders) || errors.Is(err, nats.ErrTimeout) {
-			return nil, errors.New(awserrors.ErrorInvalidInstanceIDNotFound)
+			return nil, awserrors.IDNotFound(awserrors.ErrorInvalidInstanceIDNotFound, "instance", *input.InstanceId)
 		}
 		return nil, fmt.Errorf("failed to get console output: %w", err)
 	}

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

@@ -52,7 +52,7 @@ func GetPasswordData(ctx context.Context, input *ec2.GetPasswordDataInput, natsC
 	if err != nil {
 		// No daemon subscription or timeout: stopped/terminated/non-existent instances all surface as NotFound.
 		if errors.Is(err, nats.ErrNoResponders) || errors.Is(err, nats.ErrTimeout) {
-			return nil, errors.New(awserrors.ErrorInvalidInstanceIDNotFound)
+			return nil, awserrors.IDNotFound(awserrors.ErrorInvalidInstanceIDNotFound, "instance", *input.InstanceId)
 		}
 		return nil, fmt.Errorf("failed to get password data: %w", err)
 	}

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

@@ -120,7 +120,7 @@ func TestGetPasswordData_NoResponders(t *testing.T) {
 
 	require.Error(t, err)
 	assert.Nil(t, output)
-	assert.Equal(t, awserrors.ErrorInvalidInstanceIDNotFound, err.Error())
+	assert.EqualError(t, err, "The instance ID 'i-nosubscriber' does not exist: "+awserrors.ErrorInvalidInstanceIDNotFound)
 }
 
 func TestGetPasswordData_Timeout(t *testing.T) {
@@ -140,7 +140,7 @@ func TestGetPasswordData_Timeout(t *testing.T) {
 
 	require.Error(t, err)
 	assert.Nil(t, output)
-	assert.Equal(t, awserrors.ErrorInvalidInstanceIDNotFound, err.Error())
+	assert.EqualError(t, err, "The instance ID 'i-slow' does not exist: "+awserrors.ErrorInvalidInstanceIDNotFound)
 }
 
 func TestGetPasswordData_DaemonError(t *testing.T) {

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

@@ -82,7 +82,7 @@ func AssociateIamInstanceProfile(ctx context.Context, input *ec2.AssociateIamIns
 	assoc, err := natsmsg.NATSRequest[ec2.IamInstanceProfileAssociation](ctx, natsConn, subject, command, fanOutTimeout, accountID)
 	if err != nil {
 		if errors.Is(err, nats.ErrNoResponders) {
-			return nil, errors.New(awserrors.ErrorInvalidInstanceIDNotFound)
+			return nil, awserrors.IDNotFound(awserrors.ErrorInvalidInstanceIDNotFound, "instance", *input.InstanceId)
 		}
 		return nil, err
 	}

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

@@ -293,7 +293,7 @@ func TestAssociateIamInstanceProfile_NoResponders(t *testing.T) {
 	// a raw NATS timeout.
 	_, err := AssociateIamInstanceProfile(context.Background(), in, nc, svc, testGwAccountID, nil)
 	require.Error(t, err)
-	assert.Equal(t, awserrors.ErrorInvalidInstanceIDNotFound, err.Error())
+	assert.EqualError(t, err, "The instance ID 'i-no-daemon' does not exist: "+awserrors.ErrorInvalidInstanceIDNotFound)
 }
 
 func TestAssociateIamInstanceProfile_Success(t *testing.T) {

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

@@ -87,7 +87,7 @@ func RebootInstances(ctx context.Context, input *ec2.RebootInstancesInput, natsC
 			case state != bucketStateAbsent:
 				return nil, errors.New(awserrors.ErrorIncorrectInstanceState)
 			case queryOK:
-				return nil, errors.New(awserrors.ErrorInvalidInstanceIDNotFound)
+				return nil, awserrors.IDNotFound(awserrors.ErrorInvalidInstanceIDNotFound, "instance", instanceID)
 			default:
 				return nil, errors.New(awserrors.ErrorServerInternal)
 			}

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

@@ -88,7 +88,7 @@ func TestRebootInstances_NeverExistedIsNotFound(t *testing.T) {
 
 	_, err := RebootInstances(context.Background(), input, nc, "123456789012")
 	require.Error(t, err)
-	assert.Equal(t, awserrors.ErrorInvalidInstanceIDNotFound, err.Error())
+	assert.EqualError(t, err, "The instance ID 'i-d2d5fe795b6b57882' does not exist: "+awserrors.ErrorInvalidInstanceIDNotFound)
 }
 
 func TestRebootInstances_BucketsUnreachableIsServerError(t *testing.T) {

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

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
+	"fmt"
 	"github.com/mulgadc/spinifex/spinifex/foundation/messaging/nats"
 	"log/slog"
 	"time"
@@ -102,7 +103,7 @@ func startLiveInstance(ctx context.Context, natsConn *nats.Conn, instanceID, acc
 
 	if responseError, parseErr := awserrors.ValidateErrorPayload(msg.Data); parseErr != nil {
 		slog.ErrorContext(ctx, "StartInstances: owner returned error", "instance_id", instanceID, "code", *responseError.Code)
-		return nil, false, startRefusalError(responseError)
+		return nil, false, startRefusalError(responseError, msg.Header)
 	}
 
 	slog.InfoContext(ctx, "StartInstances: restarted via owner node", "instance_id", instanceID, "response", string(msg.Data))
@@ -135,18 +136,25 @@ func startStoppedInstance(ctx context.Context, natsConn *nats.Conn, instanceID,
 
 	if responseError, parseErr := awserrors.ValidateErrorPayload(msg.Data); parseErr != nil {
 		slog.ErrorContext(ctx, "StartInstances: Daemon returned error", "instance_id", instanceID, "code", *responseError.Code)
-		return nil, startRefusalError(responseError)
+		return nil, startRefusalError(responseError, msg.Header)
 	}
 
 	slog.InfoContext(ctx, "StartInstances: Command sent successfully", "instance_id", instanceID, "response", string(msg.Data))
 	return newStateChange(instanceID, 0, "pending", 80, "stopped"), nil
 }
 
-// startRefusalError rebuilds the daemon's error, keeping the message it sent.
-// Reconstructing from the code alone renders the generic ErrorLookup text
-// instead, which replaces the name of the node holding the volume with
-// boilerplate about attaching volumes -- the one fact the operator needed.
-func startRefusalError(responseError ec2.ResponseError) error {
+// startRefusalError rebuilds the daemon's error, keeping the message it sent,
+// which names the node holding the volume, and wrapping ErrVolumeHeldElsewhere
+// when the reply's header says that is why the start was refused.
+func startRefusalError(responseError ec2.ResponseError, header nats.Header) error {
+	err := startRefusalCodeError(responseError)
+	if header.Get(natsmsg.VolumeHeldElsewhereHeader) == "true" {
+		return fmt.Errorf("%w: %w", ec2v1.ErrVolumeHeldElsewhere, err)
+	}
+	return err
+}
+
+func startRefusalCodeError(responseError ec2.ResponseError) error {
 	var code, message string
 	if responseError.Code != nil {
 		code = *responseError.Code

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

@@ -3,11 +3,14 @@ package instance
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"testing"
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/ec2"
+	"github.com/mulgadc/spinifex/contracts/ec2/v1"
 	"github.com/mulgadc/spinifex/spinifex/foundation/aws/errors"
+	"github.com/mulgadc/spinifex/spinifex/foundation/messaging/nats"
 	"github.com/nats-io/nats.go"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -161,3 +164,33 @@ func TestStartInstances_MixedSuccessAndFailure(t *testing.T) {
 	require.Error(t, err)
 	assert.Equal(t, "InvalidInstanceID.NotFound", err.Error())
 }
+
+// The AWS body says only IncorrectState, so the header is the one thing that
+// tells a held volume apart from any other refusal in that code.
+func TestStartInstances_VolumeHeldElsewhereHeader(t *testing.T) {
+	t.Parallel()
+	for name, held := range map[string]bool{"with header": true, "without header": false} {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+			_, nc := startTestNATSServer(t)
+			const id = "i-0abc0000000000002"
+			_, err := nc.Subscribe(ec2v1.InstanceCommandSubject(id), func(msg *nats.Msg) {
+				reply := nats.NewMsg(msg.Reply)
+				reply.Data = awserrors.GenerateErrorPayloadWithMessage(awserrors.ErrorIncorrectState, "volume vol-1 is published to node-a")
+				if held {
+					reply.Header.Set(natsmsg.VolumeHeldElsewhereHeader, "true")
+				}
+				_ = msg.RespondMsg(reply)
+			})
+			require.NoError(t, err)
+
+			_, err = StartInstances(t.Context(), &ec2.StartInstancesInput{InstanceIds: []*string{aws.String(id)}}, nc, "123456789012")
+			require.Error(t, err)
+			assert.Equal(t, held, errors.Is(err, ec2v1.ErrVolumeHeldElsewhere))
+			code, message, ok := awserrors.ResolveErrorDetail(err)
+			require.True(t, ok)
+			assert.Equal(t, awserrors.ErrorIncorrectState, code)
+			assert.Equal(t, "volume vol-1 is published to node-a", message)
+		})
+	}
+}

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

@@ -73,7 +73,7 @@ func StopInstances(ctx context.Context, input *ec2.StopInstancesInput, natsConn
 			case state == bucketStateTerminated:
 				return nil, errors.New(awserrors.ErrorIncorrectInstanceState)
 			case queryOK:
-				return nil, errors.New(awserrors.ErrorInvalidInstanceIDNotFound)
+				return nil, awserrors.IDNotFound(awserrors.ErrorInvalidInstanceIDNotFound, "instance", instanceID)
 			default:
 				return nil, errors.New(awserrors.ErrorServerInternal)
 			}

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

@@ -124,7 +124,7 @@ func TestStopInstances_NeverExistedIsNotFound(t *testing.T) {
 	}, nc, "123456789012")
 
 	require.Error(t, err)
-	assert.Equal(t, awserrors.ErrorInvalidInstanceIDNotFound, err.Error())
+	assert.EqualError(t, err, "The instance ID 'i-d2d5fe795b6b57882' does not exist: "+awserrors.ErrorInvalidInstanceIDNotFound)
 	assert.Nil(t, output)
 }
 
@@ -220,6 +220,6 @@ func TestStopInstances_MixedBatchWithUnknownIDFails(t *testing.T) {
 	}, nc, "123456789012")
 
 	require.Error(t, err)
-	assert.Equal(t, awserrors.ErrorInvalidInstanceIDNotFound, err.Error())
+	assert.EqualError(t, err, "The instance ID 'i-587b21c4147f9d9d8' does not exist: "+awserrors.ErrorInvalidInstanceIDNotFound)
 	assert.Nil(t, output)
 }

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

@@ -115,7 +115,7 @@ func TerminateInstances(ctx context.Context, input *ec2.TerminateInstancesInput,
 				}
 				if queryOK && state == bucketStateAbsent && daemonReportedAbsent {
 					slog.InfoContext(ctx, "TerminateInstances: Instance does not exist", "instance_id", instanceID)
-					return nil, errors.New(awserrors.ErrorInvalidInstanceIDNotFound)
+					return nil, awserrors.IDNotFound(awserrors.ErrorInvalidInstanceIDNotFound, "instance", instanceID)
 				}
 				slog.ErrorContext(ctx, "TerminateInstances: cannot confirm instance state",
 					"instance_id", instanceID, "bucket_state", state, "query_ok", queryOK, "terminate_err", terminateErr)

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

@@ -291,7 +291,7 @@ func TestTerminateInstances_NeverExistedIsNotFound(t *testing.T) {
 	}, nc, "123456789012")
 
 	require.Error(t, err)
-	assert.Equal(t, awserrors.ErrorInvalidInstanceIDNotFound, err.Error())
+	assert.EqualError(t, err, "The instance ID 'i-d419ad3c922169e6e' does not exist: "+awserrors.ErrorInvalidInstanceIDNotFound)
 	assert.Nil(t, output)
 }
 
@@ -380,6 +380,6 @@ func TestTerminateInstances_MixedBatchWithUnknownIDFails(t *testing.T) {
 	}, nc, "123456789012")
 
 	require.Error(t, err)
-	assert.Equal(t, awserrors.ErrorInvalidInstanceIDNotFound, err.Error())
+	assert.EqualError(t, err, "The instance ID 'i-0ee1ff203142536a7' does not exist: "+awserrors.ErrorInvalidInstanceIDNotFound)
 	assert.Nil(t, output)
 }

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

@@ -448,7 +448,7 @@ func extractClientError(results []nodeLaunchResult) error {
 		if r.Err == nil {
 			continue
 		}
-		code, ok := awserrors.ResolveErrorCode(r.Err)
+		code, msg, ok := awserrors.ResolveErrorDetail(r.Err)
 		if !ok {
 			continue
 		}
@@ -462,6 +462,9 @@ func extractClientError(results []nodeLaunchResult) error {
 			awserrors.ErrorInvalidGroupNotFound,
 			awserrors.ErrorInvalidParameterValue,
 			awserrors.ErrorSecurityGroupsPerInterfaceLimitExceeded:
+			if msg != "" {
+				return awserrors.Errorf(code, "%s", msg)
+			}
 			return errors.New(code)
 		}
 	}

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

@@ -424,6 +424,19 @@ func TestExtractClientError_NestedWrappedCode(t *testing.T) {
 	require.EqualError(t, err, awserrors.ErrorInvalidParameterValue)
 }
 
+func TestExtractClientError_KeepsMessage(t *testing.T) {
+	t.Parallel()
+	inner := awserrors.Errorf(awserrors.ErrorInvalidParameterValue, "architecture mismatch")
+	results := []nodeLaunchResult{
+		{NodeID: "node-1", Err: fmt.Errorf("launch on node-1: %w", inner)},
+	}
+
+	code, msg, ok := awserrors.ResolveErrorDetail(extractClientError(results))
+	require.True(t, ok)
+	assert.Equal(t, awserrors.ErrorInvalidParameterValue, code)
+	assert.Equal(t, "architecture mismatch", msg)
+}
+
 func TestExtractClientError_JoinedCode(t *testing.T) {
 	t.Parallel()
 	joined := errors.Join(assert.AnError, errors.New(awserrors.ErrorInvalidGroupNotFound))

spinifex/domains/ec2/awsapi/request.go

@@ -82,7 +82,7 @@ func (h *Handler) Dispatch(w http.ResponseWriter, inv dispatch.Invocation) error
 		return err
 	}
 	// AWS refuses a bad tag resource type even on a DryRun request.
-	if err := validateTagSpecificationTypes(action, input); err != nil {
+	if err := validateTagSpecifications(action, input); err != nil {
 		return err
 	}
 	if dryRunRequested(input) {

spinifex/domains/ec2/awsapi/validation.go

@@ -5,6 +5,7 @@ import (
 	"math"
 	"reflect"
 	"slices"
+	"strings"
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/ec2"
@@ -52,15 +53,26 @@ var ec2TaggableTypes = map[string]taggableTypes{
 	"RunInstances":                    taggable("instance", "volume", "network-interface", "spot-instances-request"),
 }
 
-// validateTagSpecificationTypes refuses a TagSpecifications entry whose
-// ResourceType the action cannot tag, rather than silently dropping its tags.
-func validateTagSpecificationTypes(action string, input any) error {
+// validateTagSpecifications refuses a TagSpecifications entry whose ResourceType
+// the action cannot tag, or a tag key with the reserved aws: prefix. Only
+// customer requests come through here, so system callers may still set aws: tags.
+func validateTagSpecifications(action string, input any) error {
 	allowed, ok := ec2TaggableTypes[action]
 	if !ok {
 		return nil
 	}
 	for _, spec := range tagSpecifications(input) {
-		if spec == nil || spec.ResourceType == nil {
+		if spec == nil {
+			continue
+		}
+		for _, tag := range spec.Tags {
+			// AWS reserves the prefix in any case.
+			if key := aws.StringValue(tag.Key); strings.HasPrefix(strings.ToLower(key), "aws:") {
+				return awserrors.Errorf(awserrors.ErrorInvalidParameterValue,
+					"Value ( %s ) for parameter key is invalid. Tag keys starting with 'aws:' are reserved for internal use", key)
+			}
+		}
+		if spec.ResourceType == nil {
 			continue
 		}
 		if rt := aws.StringValue(spec.ResourceType); !slices.Contains(allowed.types, rt) {

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

@@ -84,7 +84,7 @@ func AttachVolume(ctx context.Context, input *ec2.AttachVolumeInput, natsConn *n
 			if isStoppedInstance(ctx, instanceID, natsConn, accountID) {
 				return output, errors.New(awserrors.ErrorIncorrectInstanceState)
 			}
-			return output, errors.New(awserrors.ErrorInvalidInstanceIDNotFound)
+			return output, awserrors.IDNotFound(awserrors.ErrorInvalidInstanceIDNotFound, "instance", instanceID)
 		}
 		return output, errors.New(awserrors.ErrorServerInternal)
 	}

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

@@ -110,7 +110,7 @@ func DetachVolume(ctx context.Context, input *ec2.DetachVolumeInput, natsConn *n
 			if force {
 				return forceDetach(ctx, natsConn, accountID, volumeID, instanceID)
 			}
-			return output, errors.New(awserrors.ErrorInvalidInstanceIDNotFound)
+			return output, awserrors.IDNotFound(awserrors.ErrorInvalidInstanceIDNotFound, "instance", instanceID)
 		}
 		return output, errors.New(awserrors.ErrorServerInternal)
 	}

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

@@ -152,7 +152,7 @@ func TestDetachVolume_WithoutForceStillReportsTheMissingInstance(t *testing.T) {
 		InstanceId: aws.String("i-gone"),
 	}, nc, "000000000001")
 	require.Error(t, err)
-	assert.Equal(t, awserrors.ErrorInvalidInstanceIDNotFound, err.Error())
+	assert.EqualError(t, err, "The instance ID 'i-gone' does not exist: "+awserrors.ErrorInvalidInstanceIDNotFound)
 }
 
 // The gateway's deadline must outlast the daemon's, or a detach that is still

spinifex/domains/ec2/awsapi/vpc/AttachNetworkInterface.go

@@ -69,7 +69,7 @@ func AttachNetworkInterface(ctx context.Context, input *ec2.AttachNetworkInterfa
 		slog.ErrorContext(ctx, "AttachNetworkInterface: NATS request failed",
 			"instanceId", instanceID, "eniId", eniID, "err", err)
 		if errors.Is(err, nats.ErrNoResponders) {
-			return output, errors.New(awserrors.ErrorInvalidInstanceIDNotFound)
+			return output, awserrors.IDNotFound(awserrors.ErrorInvalidInstanceIDNotFound, "instance", instanceID)
 		}
 		return output, errors.New(awserrors.ErrorServerInternal)
 	}

spinifex/domains/ec2/awsapi/vpc/DetachNetworkInterface.go

@@ -69,7 +69,7 @@ func DetachNetworkInterface(ctx context.Context, input *ec2.DetachNetworkInterfa
 		slog.ErrorContext(ctx, "DetachNetworkInterface: NATS request failed",
 			"instanceId", instanceID, "attachmentId", attachmentID, "err", err)
 		if errors.Is(err, nats.ErrNoResponders) {
-			return output, errors.New(awserrors.ErrorInvalidInstanceIDNotFound)
+			return output, awserrors.IDNotFound(awserrors.ErrorInvalidInstanceIDNotFound, "instance", instanceID)
 		}
 		return output, errors.New(awserrors.ErrorServerInternal)
 	}

spinifex/domains/ec2/awsapi/vpc/network_interface_hotplug_test.go

@@ -92,7 +92,7 @@ func TestAttachNetworkInterface_NoResponders(t *testing.T) {
 		InstanceId:         aws.String("i-no-responder"),
 		DeviceIndex:        aws.Int64(0),
 	}, nc, testAccountID)
-	assert.EqualError(t, err, awserrors.ErrorInvalidInstanceIDNotFound)
+	assert.EqualError(t, err, "The instance ID 'i-no-responder' does not exist: "+awserrors.ErrorInvalidInstanceIDNotFound)
 }
 
 func TestAttachNetworkInterface_Success(t *testing.T) {
@@ -245,7 +245,7 @@ func TestDetachNetworkInterface_NoResponders(t *testing.T) {
 	_, err := DetachNetworkInterface(context.Background(), &ec2.DetachNetworkInterfaceInput{
 		AttachmentId: aws.String("eni-attach-1"),
 	}, nc, testAccountID)
-	assert.EqualError(t, err, awserrors.ErrorInvalidInstanceIDNotFound)
+	assert.EqualError(t, err, "The instance ID 'i-no-daemon' does not exist: "+awserrors.ErrorInvalidInstanceIDNotFound)
 }
 
 func TestDetachNetworkInterface_Success(t *testing.T) {

spinifex/domains/ec2/igw/service_impl.go

@@ -14,6 +14,7 @@ import (
 	"github.com/aws/aws-sdk-go/service/ec2"
 	networkv1 "github.com/mulgadc/spinifex/contracts/network/v1"
 	"github.com/mulgadc/spinifex/spinifex/bootstrap/config"
+	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/projection"
 	"github.com/mulgadc/spinifex/spinifex/foundation/aws/errors"
@@ -74,6 +75,7 @@ type IGWServiceImpl struct {
 	config        *config.Config
 	igwKV         jetstream.KeyValue
 	vpcKV         jetstream.KeyValue
+	eipKV         jetstream.KeyValue
 	natsConn      *nats.Conn
 	gatePublisher GatePublisher
 }
@@ -105,12 +107,19 @@ func NewIGWServiceImplWithNATS(ctx context.Context, cfg *config.Config, natsConn
 		return nil, fmt.Errorf("failed to get VPC KV bucket: %w", err)
 	}
 
+	// Read EIP associations so detach can refuse while addresses are mapped.
+	eipKV, err := kvutil.GetOrCreateBucket(ctx, js, ec2eip.KVBucketEIPs, 10)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get EIP KV bucket: %w", err)
+	}
+
 	slog.Info("IGW service initialized with JetStream KV", "bucket", KVBucketIGW)
 
 	return &IGWServiceImpl{
 		config:   cfg,
 		igwKV:    igwKV,
 		vpcKV:    vpcKV,
+		eipKV:    eipKV,
 		natsConn: natsConn,
 	}, nil
 }
@@ -400,6 +409,16 @@ func (s *IGWServiceImpl) DetachInternetGateway(ctx context.Context, input *ec2.D
 		return nil, errors.New(awserrors.ErrorGatewayNotAttached)
 	}
 
+	mapped, err := s.vpcHasMappedAddress(ctx, accountID, vpcID)
+	if err != nil {
+		slog.ErrorContext(ctx, "DetachInternetGateway: failed to read EIP associations", "vpcId", vpcID, "err", err)
+		return nil, errors.New(awserrors.ErrorServerInternal)
+	}
+	if mapped {
+		return nil, awserrors.Errorf(awserrors.ErrorDependencyViolation,
+			"Network %s has some mapped public address(es). Please unmap those public address(es) before detaching the gateway.", vpcID)
+	}
+
 	record.VpcId = ""
 	record.State = "available"
 	record.AttachState = ""
@@ -431,6 +450,42 @@ func (s *IGWServiceImpl) DetachInternetGateway(ctx context.Context, input *ec2.D
 	return &ec2.DetachInternetGatewayOutput{}, nil
 }
 
+// vpcHasMappedAddress reports whether any of the account's elastic IPs is
+// associated into vpcID, whether to an interface or a NAT gateway. The listing
+// is filtered server-side to the account's keys.
+func (s *IGWServiceImpl) vpcHasMappedAddress(ctx context.Context, accountID, vpcID string) (bool, error) {
+	lister, err := s.eipKV.ListKeysFiltered(ctx, kvutil.AccountKey(accountID, "*"))
+	if err != nil {
+		return false, err
+	}
+	var keys []string
+	for k := range lister.Keys() {
+		keys = append(keys, k)
+	}
+	// Cancellation closes the lister early, and a short list must not read as unmapped.
+	if err := ctx.Err(); err != nil {
+		return false, err
+	}
+	for _, k := range keys {
+		entry, err := s.eipKV.Get(ctx, k)
+		if err != nil {
+			if errors.Is(err, jetstream.ErrKeyNotFound) {
+				continue
+			}
+			return false, fmt.Errorf("eipKV.Get(%s): %w", k, err)
+		}
+		var record ec2eip.EIPRecord
+		if err := json.Unmarshal(entry.Value(), &record); err != nil {
+			slog.WarnContext(ctx, "vpcHasMappedAddress: malformed EIP record", "key", k, "err", err)
+			continue
+		}
+		if record.AssociationId != "" && record.VpcId == vpcID {
+			return true, nil
+		}
+	}
+	return false, nil
+}
+
 // publishAttach asks vpcd to create the OVN external switch, gateway and SNAT.
 func (s *IGWServiceImpl) publishAttach(ctx context.Context, igwID, vpcID string) {
 	projection.New(s.natsConn).AttachInternetGateway(networkv1.InternetGatewayEvent{InternetGatewayId: igwID, VpcId: vpcID})

spinifex/domains/ec2/igw/service_impl_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/ec2"
 	"github.com/mulgadc/spinifex/internal/testkit"
+	ec2eip "github.com/mulgadc/spinifex/spinifex/domains/ec2/eip"
 	ec2vpc "github.com/mulgadc/spinifex/spinifex/domains/ec2/vpc"
 	"github.com/mulgadc/spinifex/spinifex/foundation/aws/errors"
 	"github.com/mulgadc/spinifex/spinifex/foundation/state/kvutil"
@@ -597,6 +598,59 @@ func TestDetachInternetGateway(t *testing.T) {
 	assert.Empty(t, desc.InternetGateways[0].Attachments)
 }
 
+// AWS refuses the detach while an elastic IP is associated into the VPC and
+// leaves the attachment in place; it succeeds once the address is unmapped.
+func TestDetachInternetGateway_MappedPublicAddress(t *testing.T) {
+	svc, _ := setupTestIGWService(t)
+	igwID := createTestIGW(t, svc)
+	_, err := svc.AttachInternetGateway(context.Background(), &ec2.AttachInternetGatewayInput{
+		InternetGatewayId: aws.String(igwID),
+		VpcId:             aws.String("vpc-test123"),
+	}, testAccountID)
+	require.NoError(t, err)
+	confirmAttach(t, svc, igwID, "vpc-test123")
+
+	putEIP := func(rec ec2eip.EIPRecord) {
+		data, err := json.Marshal(rec)
+		require.NoError(t, err)
+		_, err = svc.eipKV.Put(context.Background(), kvutil.AccountKey(testAccountID, rec.AllocationId), data)
+		require.NoError(t, err)
+	}
+	putEIP(ec2eip.EIPRecord{AllocationId: "eipalloc-other", AssociationId: "eipassoc-other", VpcId: "vpc-other", State: "associated"})
+	putEIP(ec2eip.EIPRecord{AllocationId: "eipalloc-mapped", AssociationId: "eipassoc-mapped", ENIId: "eni-1", VpcId: "vpc-test123", State: "associated"})
+
+	// Another account's address naming the same VPC ID is not this account's dependency.
+	foreign, err := json.Marshal(ec2eip.EIPRecord{AllocationId: "eipalloc-foreign", AssociationId: "eipassoc-foreign", VpcId: "vpc-test123", State: "associated"})
+	require.NoError(t, err)
+	_, err = svc.eipKV.Put(context.Background(), kvutil.AccountKey("999999999999", "eipalloc-foreign"), foreign)
+	require.NoError(t, err)
+
+	detach := func() error {
+		_, err := svc.DetachInternetGateway(context.Background(), &ec2.DetachInternetGatewayInput{
+			InternetGatewayId: aws.String(igwID),
+			VpcId:             aws.String("vpc-test123"),
+		}, testAccountID)
+		return err
+	}
+
+	err = detach()
+	require.Error(t, err)
+	code, msg, ok := awserrors.ResolveErrorDetail(err)
+	require.True(t, ok)
+	assert.Equal(t, awserrors.ErrorDependencyViolation, code)
+	assert.Contains(t, msg, "mapped public address(es)")
+
+	desc, err := svc.DescribeInternetGateways(context.Background(), &ec2.DescribeInternetGatewaysInput{
+		InternetGatewayIds: []*string{aws.String(igwID)},
+	}, testAccountID)
+	require.NoError(t, err)
+	require.Len(t, desc.InternetGateways[0].Attachments, 1)
+	assert.Equal(t, "vpc-test123", aws.StringValue(desc.InternetGateways[0].Attachments[0].VpcId))
+
+	putEIP(ec2eip.EIPRecord{AllocationId: "eipalloc-mapped", State: "allocated"})
+	require.NoError(t, detach())
+}
+
 func TestDetachInternetGateway_NotAttached(t *testing.T) {
 	svc, _ := setupTestIGWService(t)
 	igwID := createTestIGW(t, svc)

spinifex/domains/ec2/instance/instance_filter_test.go

@@ -213,3 +213,67 @@ func TestEC2TagsToMap(t *testing.T) {
 func TestEC2TagsToMap_Nil(t *testing.T) {
 	assert.Nil(t, awsfilters.EC2TagsToMap(nil))
 }
+
+func TestInstanceMatchesFilters_PrivateIPAddress(t *testing.T) {
+	inst := &vm.VM{ID: "i-abc"}
+	ic := &ec2.Instance{
+		InstanceId:       aws.String("i-abc"),
+		PrivateIpAddress: aws.String("10.0.1.5"),
+		NetworkInterfaces: []*ec2.InstanceNetworkInterface{{
+			PrivateIpAddress: aws.String("10.0.1.5"),
+			PrivateIpAddresses: []*ec2.InstancePrivateIpAddress{
+				{PrivateIpAddress: aws.String("10.0.1.5"), Primary: aws.Bool(true)},
+				{PrivateIpAddress: aws.String("10.0.1.9"), Primary: aws.Bool(false)},
+			},
+		}},
+	}
+	tests := []struct {
+		name  string
+		value string
+		want  bool
+	}{
+		{"private-ip-address", "10.0.1.5", true},
+		{"private-ip-address", "10.0.1.9", false},
+		{"private-ip-address", "10.0.1.7", false},
+		{"network-interface.private-ip-address", "10.0.1.5", true},
+		{"network-interface.private-ip-address", "10.0.1.9", false},
+		{"network-interface.private-ip-address", "10.0.1.7", false},
+		{"network-interface.addresses.private-ip-address", "10.0.1.5", true},
+		{"network-interface.addresses.private-ip-address", "10.0.1.9", true},
+		{"network-interface.addresses.private-ip-address", "10.0.1.7", false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name+"="+tt.value, func(t *testing.T) {
+			parsed, err := awsfilters.ParseFilters([]*ec2.Filter{
+				{Name: aws.String(tt.name), Values: []*string{aws.String(tt.value)}},
+			}, DescribeInstancesValidFilters)
+			require.NoError(t, err)
+			assert.Equal(t, tt.want, instanceMatchesFilters(inst, ic, parsed))
+		})
+	}
+}
+
+// A stored interface can carry its primary address with no PrivateIpAddresses
+// list, and the addresses filter must still match it.
+func TestInstanceMatchesFilters_AddressesPrivateIPWithoutAddressList(t *testing.T) {
+	inst := &vm.VM{ID: "i-abc"}
+	ic := &ec2.Instance{
+		InstanceId:        aws.String("i-abc"),
+		NetworkInterfaces: []*ec2.InstanceNetworkInterface{{PrivateIpAddress: aws.String("10.0.1.5")}},
+	}
+	assert.True(t, instanceMatchesFilters(inst, ic, map[string][]string{"network-interface.addresses.private-ip-address": {"10.0.1.5"}}))
+}
+
+// A terminated instance keeps its stored address, which it no longer owns.
+func TestInstanceMatchesFilters_PrivateIPTerminated(t *testing.T) {
+	inst := &vm.VM{ID: "i-abc"}
+	ic := &ec2.Instance{
+		InstanceId:        aws.String("i-abc"),
+		State:             &ec2.InstanceState{Name: aws.String(ec2.InstanceStateNameTerminated)},
+		PrivateIpAddress:  aws.String("10.0.1.5"),
+		NetworkInterfaces: []*ec2.InstanceNetworkInterface{{PrivateIpAddress: aws.String("10.0.1.5")}},
+	}
+	for _, name := range []string{"private-ip-address", "network-interface.private-ip-address", "network-interface.addresses.private-ip-address"} {
+		assert.False(t, instanceMatchesFilters(inst, ic, map[string][]string{name: {"10.0.1.5"}}), name)
+	}
+}

spinifex/domains/ec2/instance/provider_efi_volume_test.go

@@ -1,126 +0,0 @@
-package instance
-
-import (
-	"bytes"
-	"context"
-	"testing"
-
-	"github.com/mulgadc/spinifex/spinifex/foundation/aws/errors"
-	"github.com/mulgadc/spinifex/spinifex/providers/ebs"
-	"github.com/mulgadc/spinifex/spinifex/providers/objectstore"
-	"github.com/mulgadc/spinifex/spinifex/runtime/compute/vm"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-)
-
-// varsTemplateBytes stands in for a firmware VARS template. The real one is
-// 540,672 bytes, but nothing here depends on the size beyond it being exact.
-func varsTemplateBytes() []byte { return bytes.Repeat([]byte{0xAB}, 4096) }
-
-func providerEFIService(t *testing.T, provider ebsprovider.EBSProvider) *InstanceServiceImpl {
-	t.Helper()
-	return providerRootVolumeService(t, provider, rootVolumeAMILoader(), objectstore.NewMemoryObjectStore())
-}
-
-// TestCreateEFIVolume_Provider_SeedsFromTemplate is the core of the EFI
-// decouple: the variable store is allocated through the provider with the
-// template as seed bytes, so no control-plane engine is built for it.
-func TestCreateEFIVolume_Provider_SeedsFromTemplate(t *testing.T) {
-	provider := ebsprovider.NewMemoryProvider(ebsprovider.Capabilities{VolumeSeeding: true})
-	svc := providerEFIService(t, provider)
-	template := varsTemplateBytes()
-
-	require.NoError(t, svc.createEFIVolumeViaProvider(context.Background(), "vol-efiseed001-efi", template))
-
-	volume, err := provider.GetVolume(context.Background(), ebsprovider.GetVolumeRequest{
-		Versioned: ebsprovider.NewVersioned(), VolumeID: "vol-efiseed001-efi",
-	})
-	require.NoError(t, err)
-	assert.Equal(t, int64(len(template)), volume.CapacityBytes,
-		"the store must be sized byte-exactly to the VARS template")
-
-	seed, ok := provider.SeedData("vol-efiseed001-efi")
-	require.True(t, ok)
-	assert.Equal(t, template, seed, "the template must reach the provider as seed bytes")
-}
-
-// A provider that rounds capacity to a GiB boundary produces a VARS region
-// pflash refuses, which would surface as an unbootable guest rather than a
-// failed launch. The mismatch has to fail the launch instead.
-func TestCreateEFIVolume_Provider_CapacityMismatchFails(t *testing.T) {
-	provider := &roundingProvider{
-		EBSProvider: ebsprovider.NewMemoryProvider(ebsprovider.Capabilities{VolumeSeeding: true}),
-	}
-	svc := providerEFIService(t, provider)
-
-	err := svc.createEFIVolumeViaProvider(context.Background(), "vol-efiround01-efi", varsTemplateBytes())
-	require.Error(t, err)
-	assert.Equal(t, awserrors.ErrorServerInternal, err.Error())
-}
-
-// A relaunch reissues the same create. Reseeding would overwrite the BootOrder
-// the guest wrote on its first boot, so the second call must be a no-op.
-func TestCreateEFIVolume_Provider_RepeatDoesNotReseed(t *testing.T) {
-	provider := ebsprovider.NewMemoryProvider(ebsprovider.Capabilities{VolumeSeeding: true})
-	svc := providerEFIService(t, provider)
-	ctx := context.Background()
-	template := varsTemplateBytes()
-
-	require.NoError(t, svc.createEFIVolumeViaProvider(ctx, "vol-efirepeat1-efi", template))
-	require.NoError(t, svc.createEFIVolumeViaProvider(ctx, "vol-efirepeat1-efi", template))
-
-	seed, ok := provider.SeedData("vol-efirepeat1-efi")
-	require.True(t, ok)
-	assert.Equal(t, template, seed)
-}
-
-// A seed above the wire bound must fail the launch with an error naming the
-// limit, not reach NATS and fail as an oversized publish.
-func TestCreateEFIVolume_Provider_OversizedTemplateFails(t *testing.T) {
-	provider := ebsprovider.NewMemoryProvider(ebsprovider.Capabilities{VolumeSeeding: true})
-	svc := providerEFIService(t, provider)
-
-	err := svc.createEFIVolumeViaProvider(context.Background(), "vol-efitoobig1-efi", make([]byte, ebsprovider.MaxSeedBytes+1))
-	require.Error(t, err)
-	assert.Equal(t, awserrors.ErrorServerInternal, err.Error())
-}
-
-// TestPrepareEFIVolume_Provider_RegistersEBSRequest covers the whole function
-// against this host's real firmware, including the name the VM launches with.
-func TestPrepareEFIVolume_Provider_RegistersEBSRequest(t *testing.T) {
-	if _, _, _, err := vm.FirmwarePaths("x86_64"); err != nil {
-		t.Skipf("no UEFI firmware on this host: %v", err)
-	}
-
-	provider := ebsprovider.NewMemoryProvider(ebsprovider.Capabilities{VolumeSeeding: true})
-	svc := providerEFIService(t, provider)
-	instance := &vm.VM{}
-
-	require.NoError(t, svc.prepareEFIVolume(context.Background(), "vol-efiprepare001", instance, "x86_64"))
-
-	instance.EBSRequests.Mu.Lock()
-	defer instance.EBSRequests.Mu.Unlock()
-	require.Len(t, instance.EBSRequests.Requests, 1)
-	assert.Equal(t, "vol-efiprepare001-efi", instance.EBSRequests.Requests[0].Name)
-	assert.True(t, instance.EBSRequests.Requests[0].EFI)
-	assert.False(t, instance.EBSRequests.Requests[0].Boot)
-
-	seed, ok := provider.SeedData("vol-efiprepare001-efi")
-	require.True(t, ok)
-	assert.NotEmpty(t, seed, "the store must be seeded from this host's VARS template")
-}
-
-// roundingProvider reports a GiB-rounded capacity the way a provider with no
-// sub-GiB volume granularity would.
-type roundingProvider struct {
-	ebsprovider.EBSProvider
-}
-
-func (p *roundingProvider) CreateVolume(ctx context.Context, req ebsprovider.CreateVolumeRequest) (*ebsprovider.Volume, error) {
-	volume, err := p.EBSProvider.CreateVolume(ctx, req)
-	if err != nil {
-		return nil, err
-	}
-	volume.CapacityBytes = 1 << 30
-	return volume, nil
-}

spinifex/domains/ec2/instance/service.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/aws/aws-sdk-go/service/ec2"
 	"github.com/mulgadc/spinifex/spinifex/domains/ec2/ebs/metadata"
+	"github.com/mulgadc/spinifex/spinifex/domains/ec2/instance/uefistore"
 	"github.com/mulgadc/spinifex/spinifex/runtime/compute/gpu"
 	"github.com/mulgadc/spinifex/spinifex/runtime/compute/vm"
 )
@@ -121,6 +122,15 @@ type StoppedInstanceStore interface {
 	ClaimStoppedInstance(instanceID string) (*vm.VM, error)
 }
 
+// UEFIStoreMarker records a terminated instance's UEFI variable store for
+// deletion. Implemented by uefistore.Store.
+type UEFIStoreMarker interface {
+	Exists(ctx context.Context, instanceID string) (bool, error)
+	MarkDeleting(ctx context.Context, instanceID string) (bool, error)
+}
+
+var _ UEFIStoreMarker = (*uefistore.Store)(nil)
+
 // InstanceTagWriter projects an instance record's full tag set into the
 // central tag store, and removes it on terminate. Implemented by
 // domains/ec2/tags.TagsServiceImpl.

spinifex/domains/ec2/instance/service_impl.go

@@ -9,7 +9,6 @@ import (
 	"github.com/mulgadc/spinifex/spinifex/domains/ec2/tagmirror"
 	"log/slog"
 	"maps"
-	"os"
 	"slices"
 	"sort"
 	"strconv"
@@ -234,6 +233,7 @@ type InstanceServiceImpl struct {
 	eniDeleter        ENIDeleter
 	ipReleaser        PublicIPReleaser
 	tagWriter         InstanceTagWriter
+	uefiStores        UEFIStoreMarker
 	gpuClaimer        GPUClaimer
 	amiLoader         AMIMetaLoader
 	ebsProvider       ebsprovider.EBSProvider
@@ -245,6 +245,8 @@ type InstanceServiceImpl struct {
 	ipAllocator       PublicIPAllocator
 	dnsBaseDomain     string
 	dnsInternalDomain string
+	uefiRecords       UEFIStoreRecords
+	uefiNode          string
 }
 
 // SetEBSProvider injects the provider boundary used for instance-created
@@ -314,6 +316,12 @@ func (s *InstanceServiceImpl) SetTerminationDeps(vd VolumeDeleter, ed ENIDeleter
 	s.tagWriter = tw
 }
 
+// SetUEFIStores wires the store records a stopped instance's terminate marks
+// for deletion. nil leaves terminate recording no UEFI store obligation.
+func (s *InstanceServiceImpl) SetUEFIStores(m UEFIStoreMarker) {
+	s.uefiStores = m
+}
+
 // SetGPUClaimer wires the GPU passthrough claim/release dependency used by
 // StartStoppedInstance. nil disables GPU passthrough for the service.
 func (s *InstanceServiceImpl) SetGPUClaimer(g GPUClaimer) {
@@ -680,6 +688,10 @@ func (s *InstanceServiceImpl) PrepareRunInstances(ctx context.Context, input *ec
 		slog.WarnContext(ctx, "PrepareRunInstances: AMI not owned by caller", "imageId", *input.ImageId, "amiOwner", amiOwner, "accountID", accountID)
 		return nil, nil, nil, errors.New(awserrors.ErrorInvalidAMIIDNotFound)
 	}
+	if err := checkImageArchitecture(instanceType, amiMeta.Architecture); err != nil {
+		slog.WarnContext(ctx, "PrepareRunInstances: AMI architecture does not match instance type", "imageId", *input.ImageId, "InstanceType", *input.InstanceType, "amiArchitecture", amiMeta.Architecture)
+		return nil, nil, nil, err
+	}
 
 	if input.KeyName != nil && *input.KeyName != "" {
 		if s.keyValidator == nil {
@@ -1236,8 +1248,17 @@ func (s *InstanceServiceImpl) StartInstance(ctx context.Context, instance *vm.VM
 			slog.WarnContext(ctx, "StartInstance: instance not found in manager",
 				"instanceId", command.ID, "err", err)
 			return errors.New(awserrors.ErrorInvalidInstanceIDNotFound)
+		case errors.Is(err, vm.ErrUEFIWriteBackPending):
+			// Retryable: the owing node lands its write-back once predastore answers.
+			slog.WarnContext(ctx, "StartInstance: vmMgr.Start failed", "instanceId", command.ID, "err", err)
+			return errors.New(awserrors.ErrorServerInternal)
 		default:
-			slog.ErrorContext(ctx, "StartInstance: vmMgr.Start failed", "err", err)
+			// A volume held elsewhere is the caller's 400, not a fault on this node.
+			if refusal := volumeHeldElsewhereError(err); refusal != nil {
+				slog.WarnContext(ctx, "StartInstance: vmMgr.Start failed", "instanceId", command.ID, "err", err)
+				return refusal
+			}
+			slog.ErrorContext(ctx, "StartInstance: vmMgr.Start failed", "instanceId", command.ID, "err", err)
 			return errors.New(awserrors.ErrorServerInternal)
 		}
 	}
@@ -1597,18 +1618,15 @@ func (s *InstanceServiceImpl) GenerateVolumes(ctx context.Context, input *ec2.Ru
 		return nil, err
 	}
 
-	// Step 2: Create EFI variable store (only when the AMI is UEFI; BIOS
-	// guests must not allocate an orphan VARS volume).
-	if instance.BootMode == "uefi" || instance.BootMode == "uefi-preferred" {
+	// Step 2: Seed the UEFI variable store. A BIOS guest has none.
+	if vm.UsesUEFIVarStore(instance) {
 		arch := instanceArchitecture(s.instanceTypes[*input.InstanceType])
-		err = s.prepareEFIVolume(ctx, imageId, instance, arch)
-		if err != nil {
+		if err := s.seedUEFIVarStore(ctx, instance, aws.StringValue(input.ImageId), arch); err != nil {
 			return nil, err
 		}
 	}
 
-	// The EFI store is internal, so it is not reported; every volume the
-	// request named is.
+	// Every volume the request named is reported.
 	volumeInfos := []VolumeInfo{
 		{
 			VolumeId:            imageId,
@@ -1870,79 +1888,22 @@ func rootVolumeIOPS(requested int) int {
 	return requested
 }
 
-// prepareEFIVolume creates the per-VM EFI variable store, sized exactly to the
-// firmware VARS template (pflash requires byte-exact size) and seeded from it.
-// arch is "x86_64" | "arm64".
-func (s *InstanceServiceImpl) prepareEFIVolume(ctx context.Context, volumeID string, instance *vm.VM, arch string) error {
-	codePath, varsTemplate, varsSize, err := vm.FirmwarePaths(arch)
-	if err != nil {
-		slog.ErrorContext(ctx, "UEFI firmware not installed on this host", "arch", arch, "err", err)
-		return errors.New(awserrors.ErrorServerInternal)
+// checkImageArchitecture refuses an AMI whose architecture the instance type
+// does not support, as AWS does. An unrecorded architecture on either side is
+// not a mismatch.
+func checkImageArchitecture(it *ec2.InstanceTypeInfo, amiArch string) error {
+	if amiArch == "aarch64" { // admin image import accepts it as an alias
+		amiArch = "arm64"
 	}
-	template, err := os.ReadFile(varsTemplate)
-	if err != nil {
-		slog.ErrorContext(ctx, "Failed to read VARS template", "path", varsTemplate, "err", err)
-		return errors.New(awserrors.ErrorServerInternal)
+	if amiArch == "" || it == nil || it.ProcessorInfo == nil || len(it.ProcessorInfo.SupportedArchitectures) == 0 {
+		return nil
 	}
-	if int64(len(template)) != varsSize {
-		slog.ErrorContext(ctx, "VARS template size mismatch between stat and read", "path", varsTemplate, "statSize", varsSize, "readSize", len(template))
-		return errors.New(awserrors.ErrorServerInternal)
+	if slices.Contains(aws.StringValueSlice(it.ProcessorInfo.SupportedArchitectures), amiArch) {
+		return nil
 	}
-	slog.InfoContext(ctx, "Preparing EFI variable store", "arch", arch, "firmwarePath", codePath, "varsTemplate", varsTemplate, "size", varsSize)
-
-	efiVolumeName := fmt.Sprintf("%s-efi", volumeID)
-	if s.ebsProvider == nil {
-		slog.ErrorContext(ctx, "no EBS provider configured", "op", "prepareEFIVolume")
-		return errors.New(awserrors.ErrorServerInternal)
-	}
-	if err := s.createEFIVolumeViaProvider(ctx, efiVolumeName, template); err != nil {
-		return err
-	}
-
-	instance.EBSRequests.Mu.Lock()
-	instance.EBSRequests.Requests = append(instance.EBSRequests.Requests, viperblocklegacyv1.EBSRequest{
-		Name: efiVolumeName,
-		Boot: false,
-		EFI:  true,
-	})
-	instance.EBSRequests.Mu.Unlock()
-
-	return nil
-}
-
-// createEFIVolumeViaProvider allocates the EFI variable store through the
-// provider, seeding it with this node's firmware VARS template. The bytes
-// travel with the request so the region matches the firmware that will run.
-func (s *InstanceServiceImpl) createEFIVolumeViaProvider(ctx context.Context, efiVolumeName string, template []byte) error {
-	slog.InfoContext(ctx, "Creating EFI variable store via EBS provider", "name", efiVolumeName, "seedBytes", len(template))
-
-	// The store is internal to the launch, never attachable and filtered out of
-	// DescribeVolumes, so it gets no ebsmetadata document the way a root volume
-	// does. Its lifecycle is the root volume's DeleteVolume.
-	created, err := s.ebsProvider.CreateVolume(ctx, ebsprovider.CreateVolumeRequest{
-		Versioned:        ebsprovider.NewVersioned(),
-		VolumeID:         efiVolumeName,
-		CapacityRange:    ebsprovider.CapacityRange{RequiredBytes: int64(len(template))},
-		AvailabilityZone: s.config.AZ,
-		SeedData:         template,
-	})
-	if err != nil {
-		slog.ErrorContext(ctx, "Provider EFI volume creation failed", "name", efiVolumeName, "err", err)
-		return errors.New(awserrors.ErrorServerInternal)
-	}
-	if created == nil {
-		slog.ErrorContext(ctx, "Provider returned no volume for the EFI variable store", "name", efiVolumeName)
-		return errors.New(awserrors.ErrorServerInternal)
-	}
-
-	// pflash rejects a VARS region that is not byte-exact, so a provider that
-	// rounded the capacity up to a GiB boundary yields a guest that cannot boot.
-	if created.CapacityBytes != int64(len(template)) {
-		slog.ErrorContext(ctx, "Provider EFI volume capacity does not match the VARS template",
-			"name", efiVolumeName, "wantBytes", len(template), "gotBytes", created.CapacityBytes)
-		return errors.New(awserrors.ErrorServerInternal)
-	}
-	return nil
+	return awserrors.Errorf(awserrors.ErrorInvalidParameterValue,
+		"The architecture '%s' of the specified instance type does not match the architecture '%s' of the specified AMI. Specify an instance type and an AMI that have matching architectures, and try again. You can use 'describe-instance-types' or 'describe-images' to discover the architecture of the instance type or AMI.",
+		instanceArchitecture(it), amiArch)
 }
 
 // instanceArchitecture returns the AWS architecture string from the instance type.
@@ -1964,6 +1925,10 @@ var DescribeInstancesValidFilters = map[string]bool{
 	"subnet-id":           true,
 	"tag-key":             true,
 	"tag-value":           true,
+	"private-ip-address":  true,
+
+	"network-interface.private-ip-address":           true,
+	"network-interface.addresses.private-ip-address": true,
 }
 
 // DescribeInstanceStatusValidFilters lists the filter names accepted by
@@ -2002,6 +1967,11 @@ func instanceMatchesFilters(inst *vm.VM, ic *ec2.Instance, filters map[string][]
 			continue
 		}
 
+		// A terminated instance has released its addresses, so AWS matches none.
+		if strings.HasSuffix(name, "private-ip-address") && ic.State != nil && aws.StringValue(ic.State.Name) == ec2.InstanceStateNameTerminated {
+			return false
+		}
+
 		var field string
 		switch name {
 		case "instance-state-name":
@@ -2020,6 +1990,18 @@ func instanceMatchesFilters(inst *vm.VM, ic *ec2.Instance, filters map[string][]
 			if ic.SubnetId != nil {
 				field = *ic.SubnetId
 			}
+		case "private-ip-address":
+			field = aws.StringValue(ic.PrivateIpAddress)
+		case "network-interface.private-ip-address":
+			if !matchNICPrivateIP(ic.NetworkInterfaces, values) {
+				return false
+			}
+			continue
+		case "network-interface.addresses.private-ip-address":
+			if !matchNICAnyPrivateIP(ic.NetworkInterfaces, values) {
+				return false
+			}
+			continue
 		case "tag-key":
 			if !matchTagKey(ic.Tags, values) {
 				return false
@@ -2046,6 +2028,31 @@ func instanceMatchesFilters(inst *vm.VM, ic *ec2.Instance, filters map[string][]
 	return awsfilters.MatchesTags(filters, tags)
 }
 
+// matchNICPrivateIP reports whether any interface's private IPv4 address
+// matches the values. Interfaces carry only a primary address.
+func matchNICPrivateIP(nics []*ec2.InstanceNetworkInterface, values []string) bool {
+	return slices.ContainsFunc(nics, func(nic *ec2.InstanceNetworkInterface) bool {
+		return nic != nil && nic.PrivateIpAddress != nil && awsfilters.MatchesAny(values, *nic.PrivateIpAddress)
+	})
+}
+
+// matchNICAnyPrivateIP matches any address an interface carries: its primary
+// address, checked directly because the stored list can omit it, and every
+// PrivateIpAddresses entry.
+func matchNICAnyPrivateIP(nics []*ec2.InstanceNetworkInterface, values []string) bool {
+	return slices.ContainsFunc(nics, func(nic *ec2.InstanceNetworkInterface) bool {
+		if nic == nil {
+			return false
+		}
+		if nic.PrivateIpAddress != nil && awsfilters.MatchesAny(values, *nic.PrivateIpAddress) {
+			return true
+		}
+		return slices.ContainsFunc(nic.PrivateIpAddresses, func(a *ec2.InstancePrivateIpAddress) bool {
+			return a != nil && a.PrivateIpAddress != nil && awsfilters.MatchesAny(values, *a.PrivateIpAddress)
+		})
+	})
+}
+
 // matchTagKey returns true if any tag key on the resource matches any of the filter values.
 func matchTagKey(tags []*ec2.Tag, values []string) bool {
 	return slices.ContainsFunc(tags, func(t *ec2.Tag) bool {
@@ -2530,7 +2537,7 @@ func (s *InstanceServiceImpl) startLocalRecord(ctx context.Context, local *vm.VM
 	if !IsInstanceVisible(accountID, local.AccountID) {
 		slog.WarnContext(ctx, "StartStoppedInstance: instance not visible",
 			"instanceId", local.ID, "callerAccount", accountID, "ownerAccount", local.AccountID)
-		return nil, errors.New(awserrors.ErrorInvalidInstanceIDNotFound)
+		return nil, awserrors.IDNotFound(awserrors.ErrorInvalidInstanceIDNotFound, "instance", local.ID)
 	}
 
 	status := s.vmMgr.Status(local)
@@ -2544,7 +2551,7 @@ func (s *InstanceServiceImpl) startLocalRecord(ctx context.Context, local *vm.VM
 		return &StartStoppedInstanceOutput{Status: "running", InstanceID: local.ID}, nil
 	case vm.StateTerminated:
 		slog.WarnContext(ctx, "StartStoppedInstance: local record is terminated", "instanceId", local.ID)
-		return nil, errors.New(awserrors.ErrorInvalidInstanceIDNotFound)
+		return nil, awserrors.IDNotFound(awserrors.ErrorInvalidInstanceIDNotFound, "instance", local.ID)
 	default:
 		slog.WarnContext(ctx, "StartStoppedInstance: instance already being started locally",
 			"instanceId", local.ID, "status", string(status))
@@ -2584,7 +2591,7 @@ func (s *InstanceServiceImpl) StartStoppedInstance(ctx context.Context, input *S
 			return s.startLocalRecord(ctx, local, accountID)
 		}
 		slog.WarnContext(ctx, "StartStoppedInstance: instance not found in shared KV", "instanceId", input.InstanceID)
-		return nil, errors.New(awserrors.ErrorInvalidInstanceIDNotFound)
+		return nil, awserrors.IDNotFound(awserrors.ErrorInvalidInstanceIDNotFound, "instance", input.InstanceID)
 	}
 	if instance.Status != vm.StateStopped {
 		slog.ErrorContext(ctx, "StartStoppedInstance: instance not in stopped state", "instanceId", input.InstanceID, "status", instance.Status)
@@ -2593,7 +2600,7 @@ func (s *InstanceServiceImpl) StartStoppedInstance(ctx context.Context, input *S
 	if !IsInstanceVisible(accountID, instance.AccountID) {
 		slog.WarnContext(ctx, "StartStoppedInstance: instance not visible",
 			"instanceId", input.InstanceID, "callerAccount", accountID, "ownerAccount", instance.AccountID)
-		return nil, errors.New(awserrors.ErrorInvalidInstanceIDNotFound)
+		return nil, awserrors.IDNotFound(awserrors.ErrorInvalidInstanceIDNotFound, "instance", input.InstanceID)
 	}
 
 	// Atomic claim: at most one caller across the cluster can win this race
@@ -2689,6 +2696,15 @@ func (s *InstanceServiceImpl) capacityRefusal(ctx context.Context, instance *vm.
 // start returns it to the stopped store, a recovery returns it to the node it
 // was taken from.
 func (s *InstanceServiceImpl) launchClaimedInstance(ctx context.Context, instance *vm.VM, undo func()) error {
+	// Before anything is stamped or claimed, so the record handed back is the
+	// one that was claimed.
+	if err := vm.CheckUEFIVarStore(instance); err != nil {
+		slog.ErrorContext(ctx, "launchClaimedInstance: refusing to launch a UEFI instance without a variable store",
+			"instanceId", instance.ID, "err", err)
+		undo()
+		return errors.New(awserrors.ErrorServerInternal)
+	}
+
 	instance.ResetNodeLocalState()
 
 	// AWS restamps LaunchTime on every start. The pending watchdog times a
@@ -2844,40 +2860,15 @@ func (s *InstanceServiceImpl) RecoverInstance(ctx context.Context, instance *vm.
 	return s.launchClaimedInstance(ctx, instance, undo)
 }
 
-// volumeExclusionMarkers are the refusals that mean another node is writing this
-// volume right now, or is still live and holds its unsealed writes. A previous
-// owner that has stopped heartbeating is opened over with a warning instead. The
-// text arrives having crossed NATS from viperblockd.
-var volumeExclusionMarkers = []string{
-	ec2v1.VolumeHeldElsewhereText,
-}
-
-// ErrVolumeHeldElsewhere reports a launch refused because another node still
-// holds one of the instance's volume leases.
-//
-// Recovery treats it differently from every other failure — it clears on the
-// lease's own clock rather than needing anything fixed — so it has to be
-// recognisable with errors.Is rather than inferred from text that has already
-// crossed NATS once.
-var ErrVolumeHeldElsewhere = errors.New("a volume is leased by another node")
-
-// volumeHeldElsewhereError converts a mount refused because another node holds
-// the volume into a client-facing error carrying that reason. Returns nil for
-// anything else. Without it the caller gets ServerInternal and is told to
-// retry, which cannot succeed while the holder is down, and the text naming the
-// holder never leaves the node's log.
+// volumeHeldElsewhereError converts a mount the EBS provider refused with its
+// held-elsewhere code into IncorrectState naming the holder, wrapping
+// ec2v1.ErrVolumeHeldElsewhere, and returns nil for anything else.
 func volumeHeldElsewhereError(err error) error {
-	if err == nil {
+	if !errors.Is(err, ebsprovider.ErrVolumeHeldElsewhere) {
 		return nil
 	}
-	msg := err.Error()
-	for _, marker := range volumeExclusionMarkers {
-		if strings.Contains(msg, marker) {
-			return fmt.Errorf("%w: %w", ErrVolumeHeldElsewhere,
-				awserrors.Errorf(awserrors.ErrorIncorrectState, "%s", msg))
-		}
-	}
-	return nil
+	return fmt.Errorf("%w: %w", ec2v1.ErrVolumeHeldElsewhere,
+		awserrors.Errorf(awserrors.ErrorIncorrectState, "%s", err.Error()))
 }
 
 // restoreClaimedStoppedInstance writes instance back to the shared
@@ -2933,6 +2924,7 @@ func (s *InstanceServiceImpl) TerminateStoppedInstance(ctx context.Context, inpu
 	s.deleteInstanceVolumes(ctx, instance, input.InstanceID)
 	s.releaseInstancePublicIP(ctx, instance, input.InstanceID)
 	s.deleteInstanceENI(ctx, instance, input.InstanceID)
+	hasUEFIStore := s.stampUEFIStoreTeardown(ctx, instance)
 
 	// Write to terminated KV FIRST so the instance is visible in DescribeInstances.
 	// If this fails the instance stays in the stopped bucket — safe to retry.
@@ -2942,6 +2934,16 @@ func (s *InstanceServiceImpl) TerminateStoppedInstance(ctx context.Context, inpu
 		return nil, errors.New(awserrors.ErrorServerInternal)
 	}
 
+	// Marked only once the terminate is durable: a stopped instance whose
+	// terminate failed keeps its store. A failed mark leaves the teardown entry
+	// pending, from which the store reaper completes it.
+	if hasUEFIStore {
+		if _, err := s.uefiStores.MarkDeleting(ctx, input.InstanceID); err != nil {
+			slog.WarnContext(ctx, "TerminateStoppedInstance: failed to record UEFI variable store for deletion",
+				"instanceId", input.InstanceID, "err", err)
+		}
+	}
+
 	// Now safe to remove from stopped KV. Retry once on failure so the instance
 	// doesn't appear in both buckets.
 	if err := s.stoppedStore.DeleteStoppedInstance(input.InstanceID); err != nil {
@@ -2959,6 +2961,29 @@ func (s *InstanceServiceImpl) TerminateStoppedInstance(ctx context.Context, inpu
 	return &TerminateStoppedInstanceOutput{Status: "terminated", InstanceID: input.InstanceID}, nil
 }
 
+// stampUEFIStoreTeardown records the UEFI store teardown entry the terminated
+// record carries, and reports whether the instance has a store to mark. An
+// unreadable bucket is stamped failed, which the store reaper resolves.
+func (s *InstanceServiceImpl) stampUEFIStoreTeardown(ctx context.Context, instance *vm.VM) bool {
+	if s.uefiStores == nil {
+		return false
+	}
+	has, err := s.uefiStores.Exists(ctx, instance.ID)
+	state := vm.TeardownPending
+	switch {
+	case err != nil:
+		slog.WarnContext(ctx, "TerminateStoppedInstance: failed to read UEFI store record", "instanceId", instance.ID, "err", err)
+		state = vm.TeardownFailed
+	case !has:
+		return false
+	}
+	if instance.Teardown == nil {
+		instance.Teardown = make(map[string]string)
+	}
+	instance.Teardown[vm.TeardownUEFI] = string(state)
+	return has
+}
+
 func (s *InstanceServiceImpl) deleteInstanceVolumes(ctx context.Context, instance *vm.VM, instanceID string) {
 	instance.EBSRequests.Mu.Lock()
 	defer instance.EBSRequests.Mu.Unlock()

spinifex/domains/ec2/instance/service_impl_test.go

@@ -2424,7 +2424,7 @@ func TestStartStoppedInstance_NotFound(t *testing.T) {
 	}
 	_, err := svc.StartStoppedInstance(context.Background(), &StartStoppedInstanceInput{InstanceID: "i-missing"}, "acc")
 	require.Error(t, err)
-	assert.Equal(t, awserrors.ErrorInvalidInstanceIDNotFound, err.Error())
+	assert.EqualError(t, err, "The instance ID 'i-missing' does not exist: "+awserrors.ErrorInvalidInstanceIDNotFound)
 }
 
 // TestStartStoppedInstance_NotFoundButRunningLocally covers the local-race
@@ -2526,7 +2526,7 @@ func TestStartStoppedInstance_LocalTerminatedRecordIsNotFound(t *testing.T) {
 	_, err := svc.StartStoppedInstance(context.Background(), &StartStoppedInstanceInput{InstanceID: id}, "acc")
 
 	require.Error(t, err)
-	assert.Equal(t, awserrors.ErrorInvalidInstanceIDNotFound, err.Error())
+	assert.EqualError(t, err, "The instance ID 'i-gone' does not exist: "+awserrors.ErrorInvalidInstanceIDNotFound)
 }
 
 // TestStartStoppedInstance_LocalRecordOfAnotherAccount holds the visibility
@@ -2546,7 +2546,7 @@ func TestStartStoppedInstance_LocalRecordOfAnotherAccount(t *testing.T) {
 	_, err := svc.StartStoppedInstance(context.Background(), &StartStoppedInstanceInput{InstanceID: id}, "other-acc")
 
 	require.Error(t, err)
-	assert.Equal(t, awserrors.ErrorInvalidInstanceIDNotFound, err.Error())
+	assert.EqualError(t, err, "The instance ID 'i-someone-elses' does not exist: "+awserrors.ErrorInvalidInstanceIDNotFound)
 }
 
 func TestStartStoppedInstance_NotStopped(t *testing.T) {
@@ -2576,7 +2576,7 @@ func TestStartStoppedInstance_NotVisible(t *testing.T) {
 	}
 	_, err := svc.StartStoppedInstance(context.Background(), &StartStoppedInstanceInput{InstanceID: id}, "other-acc")
 	require.Error(t, err)
-	assert.Equal(t, awserrors.ErrorInvalidInstanceIDNotFound, err.Error())
+	assert.EqualError(t, err, "The instance ID 'i-foreign' does not exist: "+awserrors.ErrorInvalidInstanceIDNotFound)
 
 	// Cross-tenant rejection must not delete from KV.
 	assert.Empty(t, store.DeletedStopped, "cross-tenant rejection must not delete stopped instance")
@@ -2605,6 +2605,34 @@ func TestStartStoppedInstance_InstanceTypeUnknown(t *testing.T) {
 	assert.Equal(t, vm.StateStopped, store.WroteStopped[id].Status)
 }
 
+// A UEFI instance with no variable store pointer is refused before anything is
+// stamped or claimed, so the record handed back is the one that was claimed.
+func TestStartStoppedInstance_UEFIInstanceWithoutAPointerIsRefused(t *testing.T) {
+	id := "i-uefi-nopointer"
+	itype := "t3.micro"
+	launched := time.Unix(1700000000, 0).UTC()
+	stopped := &vm.VM{
+		ID: id, Status: vm.StateStopped, AccountID: "acc", InstanceType: itype, BootMode: "uefi",
+		Instance: &ec2.Instance{LaunchTime: aws.Time(launched)},
+	}
+	before := stopped.Record()
+	store := &vmmock.StateStore{Stopped: map[string]*vm.VM{id: stopped}}
+	prov := &fakeResourceCapacityProvider{instanceTypes: map[string]*ec2.InstanceTypeInfo{
+		itype: {InstanceType: aws.String(itype)},
+	}}
+	svc := &InstanceServiceImpl{stoppedStore: store, resourceMgr: prov, vmMgr: vm.NewManager()}
+
+	_, err := svc.StartStoppedInstance(context.Background(), &StartStoppedInstanceInput{InstanceID: id}, "acc")
+
+	require.Error(t, err)
+	assert.Equal(t, awserrors.ErrorServerInternal, err.Error())
+	assert.Empty(t, prov.allocated, "nothing is allocated for a start that cannot boot")
+	require.NotNil(t, store.WroteStopped[id], "the claimed instance is handed back")
+	assert.Equal(t, before, store.WroteStopped[id].Record(), "the record is unchanged")
+	_, inManager := svc.vmMgr.Get(id)
+	assert.False(t, inManager)
+}
+
 func TestStartStoppedInstance_AllocateFails(t *testing.T) {
 	id := "i-alloc-fail"
 	itype := "t3.micro"
@@ -2771,7 +2799,7 @@ func TestStartStoppedInstance_ConcurrentClaimRace(t *testing.T) {
 	var conflicted, winnerFailed int
 	for _, err := range errs {
 		require.Error(t, err, "vmMgr.Run is wired to fail fast — no caller should succeed end-to-end")
-		switch err.Error() {
+		switch code, _ := awserrors.ResolveErrorCode(err); code {
 		case awserrors.ErrorIncorrectInstanceState, awserrors.ErrorInvalidInstanceIDNotFound:
 			conflicted++
 		case awserrors.ErrorServerInternal:
@@ -5119,3 +5147,52 @@ func TestDescribeInstanceAttribute_UserDataUnsetCarriesNoValue(t *testing.T) {
 		})
 	}
 }
+
+// AWS refuses an AMI built for another architecture than the instance type
+// with InvalidParameterValue, before any capacity or ENI is touched.
+func TestPrepareRunInstances_ArchitectureMismatch(t *testing.T) {
+	types := map[string]*ec2.InstanceTypeInfo{
+		"t3.micro": {
+			InstanceType:  aws.String("t3.micro"),
+			ProcessorInfo: &ec2.ProcessorInfo{SupportedArchitectures: []*string{aws.String("x86_64")}},
+		},
+		"t4g.micro": {
+			InstanceType:  aws.String("t4g.micro"),
+			ProcessorInfo: &ec2.ProcessorInfo{SupportedArchitectures: []*string{aws.String("arm64")}},
+		},
+	}
+	for _, tc := range []struct{ instanceType, amiArch, typeArch string }{
+		{"t3.micro", "arm64", "x86_64"},
+		{"t4g.micro", "x86_64", "arm64"},
+	} {
+		t.Run(tc.instanceType, func(t *testing.T) {
+			prov := &fakeResourceCapacityProvider{
+				instanceTypes: types,
+				canAllocFn: func(*ec2.InstanceTypeInfo, int) int {
+					t.Fatal("capacity consulted for a mismatched architecture")
+					return 0
+				},
+			}
+			svc := &InstanceServiceImpl{
+				instanceTypes: types,
+				amiLoader: &fakeAMILoader{byID: map[string]ebsmetadata.AMI{
+					"ami-1": {ImageOwnerAlias: "acc", Architecture: tc.amiArch},
+				}},
+				resourceMgr: prov,
+			}
+			_, instances, _, err := svc.PrepareRunInstances(context.Background(), &ec2.RunInstancesInput{
+				InstanceType: aws.String(tc.instanceType),
+				ImageId:      aws.String("ami-1"),
+				MinCount:     aws.Int64(1),
+				MaxCount:     aws.Int64(1),
+			}, "acc", "")
+			require.Error(t, err)
+			assert.Nil(t, instances)
+			code, message, ok := awserrors.ResolveErrorDetail(err)
+			require.True(t, ok)
+			assert.Equal(t, awserrors.ErrorInvalidParameterValue, code)
+			assert.Contains(t, message, "The architecture '"+tc.typeArch+"' of the specified instance type does not match the architecture '"+tc.amiArch+"' of the specified AMI.")
+		})
+	}
+	assert.NoError(t, checkImageArchitecture(types["t4g.micro"], "aarch64"))
+}

spinifex/domains/ec2/instance/uefi_seed.go

@@ -0,0 +1,98 @@
+package instance
+
+import (
+	"context"
+	"errors"
+	"log/slog"
+	"os"
+	"time"
+
+	"github.com/mulgadc/spinifex/spinifex/domains/ec2/instance/uefistore"
+	awserrors "github.com/mulgadc/spinifex/spinifex/foundation/aws/errors"
+	otelsetup "github.com/mulgadc/spinifex/spinifex/foundation/telemetry"
+	"github.com/mulgadc/spinifex/spinifex/runtime/compute/vm"
+)
+
+// UEFIStoreRecords creates the store record that owns a new instance's
+// variable store objects.
+type UEFIStoreRecords interface {
+	Create(ctx context.Context, rec *uefistore.UEFIStoreRecord) (uint64, error)
+}
+
+var _ UEFIStoreRecords = (*uefistore.Store)(nil)
+
+// SetUEFIVarStore wires where RunInstances records and writes a new UEFI
+// instance's variable store, and the node named as its writer.
+func (s *InstanceServiceImpl) SetUEFIVarStore(records UEFIStoreRecords, node string) {
+	s.uefiRecords = records
+	s.uefiNode = node
+}
+
+// seedUEFIVarStore gives a new UEFI instance its variable store: the store
+// record, live, then the seed object at a fresh key, then the pointer on the
+// in-memory instance, which the launch's record writes carry.
+func (s *InstanceServiceImpl) seedUEFIVarStore(ctx context.Context, instance *vm.VM, imageID, arch string) error {
+	bucket := ""
+	if s.config != nil {
+		bucket = s.config.Predastore.Bucket
+	}
+	if s.uefiRecords == nil || s.objectStore == nil || bucket == "" || s.uefiNode == "" {
+		slog.ErrorContext(ctx, "UEFI variable store seeding is not wired on this node", "instanceId", instance.ID)
+		return errors.New(awserrors.ErrorServerInternal)
+	}
+	fw, err := vm.NodeFirmwareFor(arch)
+	if err != nil {
+		slog.ErrorContext(ctx, "UEFI firmware not installed on this host", "instanceId", instance.ID, "arch", arch, "err", err)
+		return errors.New(awserrors.ErrorServerInternal)
+	}
+	region, source, err := s.uefiSeedRegion(ctx, imageID, fw)
+	if err != nil {
+		slog.ErrorContext(ctx, "Failed to read the UEFI variable store seed", "instanceId", instance.ID, "err", err)
+		return errors.New(awserrors.ErrorServerInternal)
+	}
+	key, err := uefistore.NewObjectKey(instance.ID)
+	if err != nil {
+		slog.ErrorContext(ctx, "Failed to name the UEFI variable store object", "instanceId", instance.ID, "err", err)
+		return errors.New(awserrors.ErrorServerInternal)
+	}
+
+	// The record first, so terminate can reach every object under the prefix.
+	if _, err := s.uefiRecords.Create(ctx, &uefistore.UEFIStoreRecord{
+		InstanceID: instance.ID,
+		AccountID:  instance.AccountID,
+		State:      uefistore.StateLive,
+	}); err != nil {
+		slog.ErrorContext(ctx, "Failed to create the UEFI store record", "instanceId", instance.ID, "err", err)
+		return errors.New(awserrors.ErrorServerInternal)
+	}
+
+	start := time.Now()
+	if err := uefistore.PutObject(ctx, s.objectStore, bucket, key, region, uefistore.ObjectMeta{
+		OwnerID:    instance.ID,
+		WriterNode: s.uefiNode,
+		Firmware:   fw.Identity,
+		CodePath:   fw.Code,
+		Source:     source,
+	}); err != nil {
+		slog.ErrorContext(ctx, "Failed to write the UEFI variable store seed", "instanceId", instance.ID, "key", key,
+			"put_ms", otelsetup.Millis(time.Since(start)), "err", err)
+		return errors.New(awserrors.ErrorServerInternal)
+	}
+
+	ref := &vm.UEFIVarStoreRef{Key: key, Firmware: fw.Identity, WriterNode: s.uefiNode, WrittenAt: time.Now().UTC()}
+	s.vmMgr.Inspect(instance, func(v *vm.VM) { v.UEFIVarStore = ref })
+	slog.InfoContext(ctx, "Seeded the UEFI variable store", "instanceId", instance.ID, "key", key, "source", source,
+		"arch", fw.Identity.Arch, "varsSize", fw.Identity.VarsSize, "put_ms", otelsetup.Millis(time.Since(start)))
+	return nil
+}
+
+// uefiSeedRegion returns the region a new instance's store starts from and
+// the source its object records. AMI documents name no variable store yet,
+// so every launch seeds from this node's VARS template.
+func (s *InstanceServiceImpl) uefiSeedRegion(_ context.Context, _ string, fw vm.NodeFirmware) ([]byte, string, error) {
+	region, err := os.ReadFile(fw.VarsTemplate)
+	if err != nil {
+		return nil, "", err
+	}
+	return region, uefistore.SourceTemplate, nil
+}

spinifex/domains/ec2/instance/uefi_seed_test.go

@@ -0,0 +1,273 @@
+package instance
+
+import (
+	"bytes"
+	"compress/gzip"
+	"context"
+	"crypto/sha256"
+	"encoding/hex"
+	"errors"
+	"io"
+	"os"
+	"path/filepath"
+	"strconv"
+	"strings"
+	"sync"
+	"testing"
+
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/service/ec2"
+	"github.com/aws/aws-sdk-go/service/s3"
+	testutil "github.com/mulgadc/spinifex/internal/testkit"
+	"github.com/mulgadc/spinifex/spinifex/domains/ec2/instance/uefistore"
+	awserrors "github.com/mulgadc/spinifex/spinifex/foundation/aws/errors"
+	"github.com/mulgadc/spinifex/spinifex/foundation/state/kvstore"
+	ebsprovider "github.com/mulgadc/spinifex/spinifex/providers/ebs"
+	"github.com/mulgadc/spinifex/spinifex/providers/objectstore"
+	"github.com/mulgadc/spinifex/spinifex/runtime/compute/vm"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+const (
+	testSeedInstance = "i-0a1b2c3d4e5f60718"
+	testSeedNode     = "node-a"
+	testSeedType     = "t3.nano"
+)
+
+// recordingObjects keeps each put's metadata, which the memory store drops,
+// and can fail store puts or check the store record exists when one lands.
+type recordingObjects struct {
+	*objectstore.MemoryObjectStore
+
+	mu       sync.Mutex
+	metadata map[string]map[string]string
+	putErr   error
+	onPut    func(key string)
+}
+
+var _ objectstore.ObjectStore = (*recordingObjects)(nil)
+
+func (r *recordingObjects) PutObject(ctx context.Context, in *s3.PutObjectInput) (*s3.PutObjectOutput, error) {
+	if r.onPut != nil {
+		r.onPut(aws.StringValue(in.Key))
+	}
+	if r.putErr != nil && strings.HasPrefix(aws.StringValue(in.Key), "uefi/") {
+		return nil, r.putErr
+	}
+	r.mu.Lock()
+	r.metadata[aws.StringValue(in.Key)] = aws.StringValueMap(in.Metadata)
+	r.mu.Unlock()
+	return r.MemoryObjectStore.PutObject(ctx, in)
+}
+
+type seedFixture struct {
+	svc      *InstanceServiceImpl
+	provider *ebsprovider.MemoryProvider
+	objects  *recordingObjects
+	records  *uefistore.Store
+	template []byte
+	codePath string
+	identity uefistore.FirmwareIdentity
+}
+
+func newSeedFixture(t *testing.T) *seedFixture {
+	t.Helper()
+	fwDir := t.TempDir()
+	code := filepath.Join(fwDir, "OVMF_CODE.fd")
+	vars := filepath.Join(fwDir, "OVMF_VARS.fd")
+	codeBytes := []byte("ovmf code build 7")
+	template := bytes.Repeat([]byte("VARS"), 1024)
+	require.NoError(t, os.WriteFile(code, codeBytes, 0o600))
+	require.NoError(t, os.WriteFile(vars, template, 0o600))
+	orig := vm.FirmwarePathCandidates
+	vm.FirmwarePathCandidates = map[string][]vm.FirmwareCandidate{"x86_64": {{Code: code, VarsTemplate: vars}}} //nolint:reassign // exported for tests to swap, restored below
+
+	t.Cleanup(func() { vm.FirmwarePathCandidates = orig }) //nolint:reassign // restoring the candidates captured above
+
+	_, _, js := testutil.StartTestJetStream(t)
+	records := uefistore.NewStore(js)
+	require.NoError(t, records.Open(t.Context()))
+
+	provider := ebsprovider.NewMemoryProvider(ebsprovider.Capabilities{CrashConsistentSnapshot: true, VolumeSeeding: true, VolumeEnumeration: true})
+	seedProviderSnapshot(t, provider, "vol-origin", "snap-source")
+	objects := &recordingObjects{MemoryObjectStore: objectstore.NewMemoryObjectStore(), metadata: map[string]map[string]string{}}
+	svc := providerRootVolumeService(t, provider, rootVolumeAMILoader(), objects)
+	svc.vmMgr = vm.NewManager()
+	svc.instanceTypes = map[string]*ec2.InstanceTypeInfo{testSeedType: {
+		InstanceType:  aws.String(testSeedType),
+		ProcessorInfo: &ec2.ProcessorInfo{SupportedArchitectures: []*string{aws.String("x86_64")}},
+	}}
+	svc.SetUEFIVarStore(records, testSeedNode)
+
+	sum := sha256.Sum256(codeBytes)
+	return &seedFixture{
+		svc: svc, provider: provider, objects: objects, records: records, template: template, codePath: code,
+		identity: uefistore.FirmwareIdentity{Arch: "x86_64", VarsSize: int64(len(template)), CodeSHA256: hex.EncodeToString(sum[:])},
+	}
+}
+
+func (f *seedFixture) instance(bootMode string) *vm.VM {
+	v := &vm.VM{ID: testSeedInstance, AccountID: testRootAccount, InstanceType: testSeedType, BootMode: bootMode, Instance: &ec2.Instance{}}
+	f.svc.vmMgr.Insert(v)
+	return v
+}
+
+func seedRunInput() *ec2.RunInstancesInput {
+	return &ec2.RunInstancesInput{ImageId: aws.String("ami-1"), InstanceType: aws.String(testSeedType)}
+}
+
+func (f *seedFixture) objectKeys(t *testing.T) []string {
+	t.Helper()
+	out, err := f.objects.ListObjectsV2(t.Context(), &s3.ListObjectsV2Input{
+		Bucket: aws.String(testRootBucket), Prefix: aws.String(uefistore.KeyPrefix(testSeedInstance)),
+	})
+	require.NoError(t, err)
+	keys := make([]string, 0, len(out.Contents))
+	for _, o := range out.Contents {
+		keys = append(keys, aws.StringValue(o.Key))
+	}
+	return keys
+}
+
+func (f *seedFixture) providerVolumeIDs(t *testing.T) []string {
+	t.Helper()
+	out, err := f.provider.ListVolumes(t.Context(), ebsprovider.ListVolumesRequest{Versioned: ebsprovider.NewVersioned()})
+	require.NoError(t, err)
+	ids := make([]string, 0, len(out.Volumes))
+	for _, v := range out.Volumes {
+		ids = append(ids, v.ID)
+	}
+	return ids
+}
+
+// A UEFI launch seeds its store as an object and a pointer: no <root>-efi
+// volume, no EFI request, a live store record, and a gzip of this node's
+// template whose metadata and pointer name the firmware and the writer.
+func TestGenerateVolumes_UEFISeedsTheStoreFromTheTemplate(t *testing.T) {
+	f := newSeedFixture(t)
+	instance := f.instance("uefi")
+
+	infos, err := f.svc.GenerateVolumes(t.Context(), seedRunInput(), instance)
+	require.NoError(t, err)
+
+	require.Len(t, infos, 1, "only the root volume is reported")
+	for _, id := range f.providerVolumeIDs(t) {
+		assert.False(t, strings.HasSuffix(id, "-efi"), "no EFI volume is created, found %s", id)
+	}
+	for _, r := range instance.EBSRequests.Requests {
+		assert.False(t, r.EFI, "no EFI request is appended, found %s", r.Name)
+	}
+
+	ref := instance.UEFIVarStore
+	require.NotNil(t, ref, "the launch's record carries a pointer")
+	assert.Equal(t, []string{ref.Key}, f.objectKeys(t), "the pointer names the one object under the prefix")
+	assert.Equal(t, f.identity, ref.Firmware)
+	assert.Equal(t, testSeedNode, ref.WriterNode)
+	assert.False(t, ref.WrittenAt.IsZero())
+	assert.Equal(t, ref, instance.Record().Status.UEFIVarStore, "the pointer is in the record's status")
+
+	out, err := f.objects.GetObject(t.Context(), &s3.GetObjectInput{Bucket: aws.String(testRootBucket), Key: aws.String(ref.Key)})
+	require.NoError(t, err)
+	defer out.Body.Close()
+	gz, err := gzip.NewReader(out.Body)
+	require.NoError(t, err, "the body is gzip")
+	region, err := io.ReadAll(gz)
+	require.NoError(t, err)
+	assert.Equal(t, f.template, region, "the store starts as this node's VARS template")
+
+	assert.Equal(t, map[string]string{
+		"instance-id":          testSeedInstance,
+		"writer-node":          testSeedNode,
+		"firmware-arch":        "x86_64",
+		"firmware-vars-size":   strconv.Itoa(len(f.template)),
+		"firmware-code-sha256": f.identity.CodeSHA256,
+		"firmware-code-path":   f.codePath,
+		"source":               "template",
+	}, f.objects.metadata[ref.Key])
+
+	rec, _, err := f.records.Get(t.Context(), testSeedInstance)
+	require.NoError(t, err)
+	assert.Equal(t, uefistore.StateLive, rec.State)
+	assert.Equal(t, testRootAccount, rec.AccountID)
+	assert.Nil(t, rec.WriteBack)
+}
+
+// The store record exists before the object is put, so terminate can reach
+// every object under the prefix, including one whose put failed midway.
+func TestGenerateVolumes_UEFIRecordsTheStoreBeforeItsObject(t *testing.T) {
+	f := newSeedFixture(t)
+	instance := f.instance("uefi")
+	recordAtPut := errors.New("no put")
+	f.objects.onPut = func(key string) {
+		if strings.HasPrefix(key, uefistore.KeyPrefix(testSeedInstance)) {
+			_, _, recordAtPut = f.records.Get(context.Background(), testSeedInstance)
+		}
+	}
+
+	_, err := f.svc.GenerateVolumes(t.Context(), seedRunInput(), instance)
+
+	require.NoError(t, err)
+	assert.NoError(t, recordAtPut, "the record is live when the object is put")
+}
+
+// A failed put fails the launch with InternalError and sets no pointer, and
+// leaves the record so terminate can still settle the prefix.
+func TestGenerateVolumes_UEFIPutFailureFailsTheLaunch(t *testing.T) {
+	f := newSeedFixture(t)
+	instance := f.instance("uefi")
+	f.objects.putErr = errors.New("predastore unreachable")
+
+	_, err := f.svc.GenerateVolumes(t.Context(), seedRunInput(), instance)
+
+	require.Error(t, err)
+	assert.Equal(t, awserrors.ErrorServerInternal, err.Error())
+	assert.Nil(t, instance.UEFIVarStore)
+	rec, _, getErr := f.records.Get(t.Context(), testSeedInstance)
+	require.NoError(t, getErr)
+	assert.Equal(t, uefistore.StateLive, rec.State)
+}
+
+// A record already under the instance's ID is not this launch's, so the
+// launch writes nothing under its prefix.
+func TestGenerateVolumes_UEFIExistingRecordFailsTheLaunch(t *testing.T) {
+	f := newSeedFixture(t)
+	instance := f.instance("uefi")
+	_, err := f.records.Create(t.Context(), &uefistore.UEFIStoreRecord{InstanceID: testSeedInstance, AccountID: "000000000099", State: uefistore.StateLive})
+	require.NoError(t, err)
+
+	_, err = f.svc.GenerateVolumes(t.Context(), seedRunInput(), instance)
+
+	require.Error(t, err)
+	assert.Equal(t, awserrors.ErrorServerInternal, err.Error())
+	assert.Empty(t, f.objectKeys(t))
+	assert.Nil(t, instance.UEFIVarStore)
+}
+
+// A BIOS guest has no variable store: no record, no object, no pointer.
+func TestGenerateVolumes_BIOSSeedsNoStore(t *testing.T) {
+	f := newSeedFixture(t)
+	instance := f.instance("legacy-bios")
+
+	_, err := f.svc.GenerateVolumes(t.Context(), seedRunInput(), instance)
+
+	require.NoError(t, err)
+	assert.Nil(t, instance.UEFIVarStore)
+	assert.Empty(t, f.objectKeys(t))
+	_, _, err = f.records.Get(t.Context(), testSeedInstance)
+	assert.ErrorIs(t, err, kvstore.ErrNotFound)
+}
+
+// A node without the store wired cannot seed, so it fails the launch rather
+// than booting a UEFI guest with no store.
+func TestGenerateVolumes_UEFIWithoutStoreWiringFails(t *testing.T) {
+	f := newSeedFixture(t)
+	f.svc.SetUEFIVarStore(nil, "")
+	instance := f.instance("uefi")
+
+	_, err := f.svc.GenerateVolumes(t.Context(), seedRunInput(), instance)
+
+	require.Error(t, err)
+	assert.Equal(t, awserrors.ErrorServerInternal, err.Error())
+	assert.Nil(t, instance.UEFIVarStore)
+}

spinifex/domains/ec2/instance/uefi_terminate_test.go

@@ -0,0 +1,129 @@
+package instance
+
+import (
+	"bytes"
+	"context"
+	"errors"
+	"testing"
+
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/service/s3"
+	testutil "github.com/mulgadc/spinifex/internal/testkit"
+	"github.com/mulgadc/spinifex/spinifex/domains/ec2/instance/uefistore"
+	"github.com/mulgadc/spinifex/spinifex/providers/objectstore"
+	"github.com/mulgadc/spinifex/spinifex/runtime/compute/vm"
+	vmmock "github.com/mulgadc/spinifex/spinifex/runtime/compute/vm/mock"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+const uefiTerminateID = "i-0123456789abcdef0"
+
+// stoppedUEFIFixture is a stopped instance with a live store record and three
+// retained objects, the state a launch and a few stops leave.
+func stoppedUEFIFixture(t *testing.T) (*InstanceServiceImpl, *vmmock.StateStore, *uefistore.Store, *objectstore.MemoryObjectStore) {
+	t.Helper()
+	_, _, js := testutil.StartTestJetStream(t)
+	stores := uefistore.NewStore(js)
+	require.NoError(t, stores.Open(t.Context()))
+	_, err := stores.Create(t.Context(), &uefistore.UEFIStoreRecord{InstanceID: uefiTerminateID, AccountID: "acc", State: uefistore.StateLive})
+	require.NoError(t, err)
+
+	objects := objectstore.NewMemoryObjectStore()
+	for _, k := range []string{"a", "b", "c"} {
+		_, err := objects.PutObject(t.Context(), &s3.PutObjectInput{
+			Bucket: aws.String("spinifex"), Key: aws.String(uefistore.KeyPrefix(uefiTerminateID) + k), Body: bytes.NewReader([]byte("vars")),
+		})
+		require.NoError(t, err)
+	}
+
+	store := &vmmock.StateStore{Stopped: map[string]*vm.VM{
+		uefiTerminateID: {ID: uefiTerminateID, Status: vm.StateStopped, AccountID: "acc"},
+	}}
+	svc := &InstanceServiceImpl{stoppedStore: store, objectStore: objects}
+	svc.SetUEFIStores(stores)
+	return svc, store, stores, objects
+}
+
+func storeState(t *testing.T, stores *uefistore.Store) string {
+	t.Helper()
+	rec, _, err := stores.Get(t.Context(), uefiTerminateID)
+	require.NoError(t, err)
+	return rec.State
+}
+
+// TestS8_TerminateStoppedInstanceRecordsTheUEFIStoreObligationAndReturns
+// enforces ADR-0003:S8: the request deletes no store object, it records the
+// obligation on the store record and the terminated record, and returns.
+func TestS8_TerminateStoppedInstanceRecordsTheUEFIStoreObligationAndReturns(t *testing.T) {
+	svc, store, stores, objects := stoppedUEFIFixture(t)
+
+	_, err := svc.TerminateStoppedInstance(t.Context(), &TerminateStoppedInstanceInput{InstanceID: uefiTerminateID}, "acc")
+	require.NoError(t, err)
+
+	keys, err := uefistore.ListObjects(t.Context(), objects, "spinifex", uefiTerminateID)
+	require.NoError(t, err)
+	assert.Len(t, keys, 3, "ADR-0003:S8: the request must leave the object deletion to the reaper")
+	assert.Equal(t, uefistore.StateDeleting, storeState(t, stores), "the store record carries the obligation")
+	require.NotNil(t, store.WroteTerminated[uefiTerminateID])
+	assert.Equal(t, string(vm.TeardownPending), store.WroteTerminated[uefiTerminateID].Teardown[vm.TeardownUEFI])
+}
+
+// TestS7_FailedStoppedTerminateKeepsTheUEFIStoreLive enforces ADR-0003:S7: a
+// terminate that fails before it is durable leaves the instance stopped, so
+// its store must stay live rather than be handed to the reaper.
+func TestS7_FailedStoppedTerminateKeepsTheUEFIStoreLive(t *testing.T) {
+	svc, store, stores, _ := stoppedUEFIFixture(t)
+	store.WriteTerminatedErr = errors.New("kv write boom")
+
+	_, err := svc.TerminateStoppedInstance(t.Context(), &TerminateStoppedInstanceInput{InstanceID: uefiTerminateID}, "acc")
+	require.Error(t, err)
+
+	assert.Equal(t, uefistore.StateLive, storeState(t, stores),
+		"ADR-0003:S7: a stopped instance whose terminate failed must keep its store")
+}
+
+func TestTerminateStoppedInstance_NoUEFIStoreStampsNoEntry(t *testing.T) {
+	_, _, js := testutil.StartTestJetStream(t)
+	stores := uefistore.NewStore(js)
+	require.NoError(t, stores.Open(t.Context()))
+	store := &vmmock.StateStore{Stopped: map[string]*vm.VM{
+		uefiTerminateID: {ID: uefiTerminateID, Status: vm.StateStopped, AccountID: "acc"},
+	}}
+	svc := &InstanceServiceImpl{stoppedStore: store}
+	svc.SetUEFIStores(stores)
+
+	_, err := svc.TerminateStoppedInstance(t.Context(), &TerminateStoppedInstanceInput{InstanceID: uefiTerminateID}, "acc")
+	require.NoError(t, err)
+
+	_, stamped := store.WroteTerminated[uefiTerminateID].Teardown[vm.TeardownUEFI]
+	assert.False(t, stamped)
+}
+
+type unreadableUEFIStores struct{ marked bool }
+
+func (u *unreadableUEFIStores) Exists(context.Context, string) (bool, error) {
+	return false, errors.New("kv unavailable")
+}
+
+func (u *unreadableUEFIStores) MarkDeleting(context.Context, string) (bool, error) {
+	u.marked = true
+	return false, nil
+}
+
+// An unreadable bucket does not fail the terminate; the entry is stamped
+// failed so the store reaper settles it from the terminated record.
+func TestTerminateStoppedInstance_UnreadableUEFIStoreIsStampedFailed(t *testing.T) {
+	store := &vmmock.StateStore{Stopped: map[string]*vm.VM{
+		uefiTerminateID: {ID: uefiTerminateID, Status: vm.StateStopped, AccountID: "acc"},
+	}}
+	stores := &unreadableUEFIStores{}
+	svc := &InstanceServiceImpl{stoppedStore: store}
+	svc.SetUEFIStores(stores)
+
+	_, err := svc.TerminateStoppedInstance(t.Context(), &TerminateStoppedInstanceInput{InstanceID: uefiTerminateID}, "acc")
+	require.NoError(t, err)
+
+	assert.Equal(t, string(vm.TeardownFailed), store.WroteTerminated[uefiTerminateID].Teardown[vm.TeardownUEFI])
+	assert.False(t, stores.marked)
+}

spinifex/domains/ec2/instance/uefistore/cleanup.go

@@ -0,0 +1,141 @@
+package uefistore
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"slices"
+	"strings"
+
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/service/s3"
+	"github.com/mulgadc/spinifex/spinifex/foundation/state/kvstore"
+	"github.com/mulgadc/spinifex/spinifex/foundation/state/kvutil"
+	"github.com/mulgadc/spinifex/spinifex/providers/objectstore"
+	"github.com/nats-io/nats.go/jetstream"
+)
+
+// markAttempts bounds MarkDeleting's retries against concurrent writers. Each
+// lost race means another writer changed the record, and none of them can
+// return it to live, so a few rounds always settle.
+const markAttempts = 5
+
+// Exists reports whether instanceID has a store record, without decoding it,
+// so a record this node cannot read still counts as one.
+func (s *Store) Exists(ctx context.Context, instanceID string) (bool, error) {
+	return s.kv.Exists(ctx, instanceID)
+}
+
+// MarkDeleting records that instanceID's store is to be deleted, and drops any
+// write-back still owed. It reports false with no error when the instance has
+// no store record. Only a terminate or an operator's forget may call it.
+func (s *Store) MarkDeleting(ctx context.Context, instanceID string) (bool, error) {
+	for range markAttempts {
+		rec, rev, err := s.Get(ctx, instanceID)
+		if errors.Is(err, kvstore.ErrNotFound) {
+			return false, nil
+		}
+		if err != nil {
+			return false, err
+		}
+		if rec.State == StateDeleting {
+			return true, nil
+		}
+		rec.State = StateDeleting
+		rec.WriteBack = nil
+		_, err = s.CompareAndSet(ctx, rec, rev)
+		if err == nil {
+			return true, nil
+		}
+		if !errors.Is(err, kvstore.ErrConflict) {
+			return false, err
+		}
+	}
+	return false, fmt.Errorf("uefistore: mark %s deleting: %w after %d attempts", instanceID, kvstore.ErrConflict, markAttempts)
+}
+
+// InstanceIDs lists the key of every store record, sorted. It decodes
+// nothing, so a caller reads each record through Get and its validation.
+func (s *Store) InstanceIDs(ctx context.Context) ([]string, error) {
+	kv, err := s.kv.KV(ctx)
+	if err != nil {
+		return nil, err
+	}
+	lister, err := kv.ListKeys(ctx)
+	if err != nil {
+		if errors.Is(err, jetstream.ErrNoKeysFound) {
+			return nil, nil
+		}
+		return nil, fmt.Errorf("uefistore: list %s: %w", BucketName, err)
+	}
+	defer func() { _ = lister.Stop() }()
+
+	var ids []string
+	for key := range lister.Keys() {
+		if key != kvutil.VersionKey {
+			ids = append(ids, key)
+		}
+	}
+	slices.Sort(ids)
+	return ids, nil
+}
+
+// validOwner reports whether ownerID is an instance or image ID that names
+// exactly one prefix, so a malformed ID can never widen a delete.
+func validOwner(ownerID string) bool {
+	if strings.ContainsAny(ownerID, "/*") {
+		return false
+	}
+	for _, p := range []string{"i-", "ami-"} {
+		if rest, ok := strings.CutPrefix(ownerID, p); ok && rest != "" {
+			return true
+		}
+	}
+	return false
+}
+
+// ListObjects returns the key of every object under ownerID's prefix, across
+// every page of the listing.
+func ListObjects(ctx context.Context, objects objectstore.ObjectStore, bucket, ownerID string) ([]string, error) {
+	if !validOwner(ownerID) {
+		return nil, fmt.Errorf("uefistore: %q is not an instance or image ID", ownerID)
+	}
+	contents, _, err := objectstore.ListAll(ctx, objects, &s3.ListObjectsV2Input{
+		Bucket: aws.String(bucket),
+		Prefix: aws.String(KeyPrefix(ownerID)),
+	})
+	if err != nil {
+		return nil, fmt.Errorf("uefistore: list %s: %w", KeyPrefix(ownerID), err)
+	}
+	keys := make([]string, 0, len(contents))
+	for _, obj := range contents {
+		keys = append(keys, aws.StringValue(obj.Key))
+	}
+	return keys, nil
+}
+
+// DeleteObjects deletes every object under ownerID's prefix and reports how
+// many it removed. It succeeds only once a fresh listing finds the prefix
+// empty, so an object written during the delete is not left unaccounted for.
+func DeleteObjects(ctx context.Context, objects objectstore.ObjectStore, bucket, ownerID string) (int, error) {
+	keys, err := ListObjects(ctx, objects, bucket, ownerID)
+	if err != nil {
+		return 0, err
+	}
+	deleted := 0
+	for _, key := range keys {
+		_, err := objects.DeleteObject(ctx, &s3.DeleteObjectInput{Bucket: aws.String(bucket), Key: aws.String(key)})
+		if err != nil && !objectstore.IsNoSuchKeyError(err) {
+			return deleted, fmt.Errorf("uefistore: delete %s: %w", key, err)
+		}
+		deleted++
+	}
+	left, err := ListObjects(ctx, objects, bucket, ownerID)
+	if err != nil {
+		return deleted, err
+	}
+	if len(left) > 0 {
+		return deleted, fmt.Errorf("uefistore: %d object(s) remain under %s", len(left), KeyPrefix(ownerID))
+	}
+	return deleted, nil
+}

spinifex/domains/ec2/instance/uefistore/cleanup_test.go

@@ -0,0 +1,213 @@
+package uefistore_test
+
+import (
+	"bytes"
+	"context"
+	"errors"
+	"testing"
+
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/service/s3"
+	"github.com/mulgadc/spinifex/spinifex/domains/ec2/instance/uefistore"
+	"github.com/mulgadc/spinifex/spinifex/foundation/state/kvstore"
+	"github.com/mulgadc/spinifex/spinifex/providers/objectstore"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+const bucket = "spinifex"
+
+func putObject(t *testing.T, objects objectstore.ObjectStore, key string) {
+	t.Helper()
+	_, err := objects.PutObject(t.Context(), &s3.PutObjectInput{
+		Bucket: aws.String(bucket), Key: aws.String(key), Body: bytes.NewReader([]byte("vars")),
+	})
+	require.NoError(t, err)
+}
+
+func keysUnder(t *testing.T, objects objectstore.ObjectStore, prefix string) []string {
+	t.Helper()
+	contents, _, err := objectstore.ListAll(t.Context(), objects, &s3.ListObjectsV2Input{Bucket: aws.String(bucket), Prefix: aws.String(prefix)})
+	require.NoError(t, err)
+	var keys []string
+	for _, o := range contents {
+		keys = append(keys, aws.StringValue(o.Key))
+	}
+	return keys
+}
+
+// Deletion is final: a writer holding the revision of a deleting record, such
+// as a write-back retry, must not bring the store back to live.
+func TestCompareAndSet_RefusesDeletingToLive(t *testing.T) {
+	s, _ := openStore(t)
+	rev, err := s.Create(t.Context(), liveRecord())
+	require.NoError(t, err)
+	deleting := liveRecord()
+	deleting.State = uefistore.StateDeleting
+	rev, err = s.CompareAndSet(t.Context(), deleting, rev)
+	require.NoError(t, err)
+
+	revived := liveRecord()
+	revived.WriteBack = writeBack()
+	_, err = s.CompareAndSet(t.Context(), revived, rev)
+	require.ErrorIs(t, err, uefistore.ErrDeleting)
+
+	got, gotRev, err := s.Get(t.Context(), instanceID)
+	require.NoError(t, err)
+	assert.Equal(t, rev, gotRev)
+	assert.Equal(t, uefistore.StateDeleting, got.State)
+}
+
+// A write-back owed on a deleting store would put an object behind the delete.
+func TestCompareAndSet_RefusesDeletingWithAWriteBack(t *testing.T) {
+	s, _ := openStore(t)
+	rev, err := s.Create(t.Context(), liveRecord())
+	require.NoError(t, err)
+	rec := liveRecord()
+	rec.State = uefistore.StateDeleting
+	rec.WriteBack = writeBack()
+
+	_, err = s.CompareAndSet(t.Context(), rec, rev)
+	require.ErrorIs(t, err, uefistore.ErrInvalidRecord)
+}
+
+func TestMarkDeleting_MovesLiveToDeletingAndDropsTheWriteBack(t *testing.T) {
+	s, _ := openStore(t)
+	rec := liveRecord()
+	rec.WriteBack = writeBack()
+	_, err := s.Create(t.Context(), rec)
+	require.NoError(t, err)
+
+	has, err := s.MarkDeleting(t.Context(), instanceID)
+	require.NoError(t, err)
+	assert.True(t, has)
+
+	got, rev, err := s.Get(t.Context(), instanceID)
+	require.NoError(t, err)
+	assert.Equal(t, uefistore.StateDeleting, got.State)
+	assert.Nil(t, got.WriteBack)
+	assert.Equal(t, "111122223333", got.AccountID)
+
+	has, err = s.MarkDeleting(t.Context(), instanceID)
+	require.NoError(t, err)
+	assert.True(t, has, "a repeated terminate finds the obligation already recorded")
+	_, again, err := s.Get(t.Context(), instanceID)
+	require.NoError(t, err)
+	assert.Equal(t, rev, again, "a repeat writes nothing")
+}
+
+func TestMarkDeleting_NoRecordIsNoStore(t *testing.T) {
+	s, _ := openStore(t)
+	has, err := s.MarkDeleting(t.Context(), instanceID)
+	require.NoError(t, err)
+	assert.False(t, has)
+	_, _, err = s.Get(t.Context(), instanceID)
+	require.ErrorIs(t, err, kvstore.ErrNotFound, "marking must not create a record")
+}
+
+// A record this node cannot trust is reported, never rewritten as deleting.
+func TestMarkDeleting_RefusesAnUntrustedRecord(t *testing.T) {
+	s, js := openStore(t)
+	rawPut(t, js, instanceID, `{"schema_version":2,"instance_id":"i-0123456789abcdef0","state":"live"}`)
+
+	_, err := s.MarkDeleting(t.Context(), instanceID)
+	require.ErrorIs(t, err, uefistore.ErrInvalidRecord)
+}
+
+func TestExists_CountsAnUndecodableRecord(t *testing.T) {
+	s, js := openStore(t)
+	has, err := s.Exists(t.Context(), instanceID)
+	require.NoError(t, err)
+	assert.False(t, has)
+
+	rawPut(t, js, instanceID, `not json`)
+	has, err = s.Exists(t.Context(), instanceID)
+	require.NoError(t, err)
+	assert.True(t, has)
+}
+
+func TestInstanceIDs_ListsRecordsButNotTheVersionKey(t *testing.T) {
+	s, _ := openStore(t)
+	ids, err := s.InstanceIDs(t.Context())
+	require.NoError(t, err)
+	assert.Empty(t, ids)
+
+	for _, id := range []string{"i-0000000000000000b", "i-0000000000000000a"} {
+		rec := liveRecord()
+		rec.InstanceID = id
+		_, err := s.Create(t.Context(), rec)
+		require.NoError(t, err)
+	}
+	ids, err = s.InstanceIDs(t.Context())
+	require.NoError(t, err)
+	assert.Equal(t, []string{"i-0000000000000000a", "i-0000000000000000b"}, ids)
+}
+
+// The prefix carries its trailing slash, so an instance whose ID extends
+// another's keeps its objects.
+func TestDeleteObjects_RemovesOnlyTheOwnersPrefix(t *testing.T) {
+	objects := objectstore.NewMemoryObjectStore()
+	const other = instanceID + "1"
+	for _, key := range []string{"a", "b", "c"} {
+		putObject(t, objects, uefistore.KeyPrefix(instanceID)+key)
+	}
+	putObject(t, objects, uefistore.KeyPrefix(other)+"a")
+	putObject(t, objects, uefistore.KeyPrefix("ami-0123456789abcdef0")+"a")
+
+	deleted, err := uefistore.DeleteObjects(t.Context(), objects, bucket, instanceID)
+	require.NoError(t, err)
+	assert.Equal(t, 3, deleted)
+	assert.Empty(t, keysUnder(t, objects, uefistore.KeyPrefix(instanceID)))
+	assert.Len(t, keysUnder(t, objects, uefistore.KeyPrefix(other)), 1)
+	assert.Len(t, keysUnder(t, objects, uefistore.KeyPrefix("ami-0123456789abcdef0")), 1)
+
+	deleted, err = uefistore.DeleteObjects(t.Context(), objects, bucket, instanceID)
+	require.NoError(t, err, "an empty prefix is already deleted")
+	assert.Zero(t, deleted)
+}
+
+func TestDeleteObjects_RefusesAnIDThatWidensTheDelete(t *testing.T) {
+	objects := objectstore.NewMemoryObjectStore()
+	putObject(t, objects, uefistore.KeyPrefix(instanceID)+"a")
+	for _, id := range []string{"", "i-", "ami-", "vol-0123", "i-0/../", "i-*", "x"} {
+		_, err := uefistore.DeleteObjects(t.Context(), objects, bucket, id)
+		require.Errorf(t, err, "owner %q", id)
+	}
+	assert.Len(t, keysUnder(t, objects, uefistore.ObjectRoot), 1)
+}
+
+// failingDeletes refuses every delete, as predastore does while it is down.
+type failingDeletes struct {
+	objectstore.ObjectStore
+}
+
+func (failingDeletes) DeleteObject(context.Context, *s3.DeleteObjectInput) (*s3.DeleteObjectOutput, error) {
+	return nil, errors.New("connection refused")
+}
+
+func TestDeleteObjects_ReportsAFailedDelete(t *testing.T) {
+	mem := objectstore.NewMemoryObjectStore()
+	putObject(t, mem, uefistore.KeyPrefix(instanceID)+"a")
+
+	_, err := uefistore.DeleteObjects(t.Context(), failingDeletes{mem}, bucket, instanceID)
+	require.Error(t, err)
+	assert.Len(t, keysUnder(t, mem, uefistore.KeyPrefix(instanceID)), 1)
+}
+
+// noopDeletes reports success without deleting, so the closing listing is
+// what notices the prefix is not empty.
+type noopDeletes struct {
+	objectstore.ObjectStore
+}
+
+func (noopDeletes) DeleteObject(context.Context, *s3.DeleteObjectInput) (*s3.DeleteObjectOutput, error) {
+	return &s3.DeleteObjectOutput{}, nil
+}
+
+func TestDeleteObjects_FailsWhileAnObjectRemains(t *testing.T) {
+	mem := objectstore.NewMemoryObjectStore()
+	putObject(t, mem, uefistore.KeyPrefix(instanceID)+"a")
+
+	_, err := uefistore.DeleteObjects(t.Context(), noopDeletes{mem}, bucket, instanceID)
+	require.Error(t, err)
+}

spinifex/domains/ec2/instance/uefistore/object.go

@@ -0,0 +1,128 @@
+package uefistore
+
+import (
+	"bytes"
+	"compress/gzip"
+	"context"
+	"crypto/rand"
+	"errors"
+	"fmt"
+	"strconv"
+	"strings"
+	"time"
+
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/service/s3"
+	"github.com/mulgadc/spinifex/spinifex/providers/objectstore"
+)
+
+const (
+	// SourceTemplate is a store seeded from the launching node's VARS template.
+	SourceTemplate = "template"
+	// SourceWriteBack is a store written back from a node's live copy.
+	SourceWriteBack = "write-back"
+
+	sourceImagePrefix = "image:"
+
+	// PutTimeout bounds one put of a store object. The object is a fixed-size
+	// region, so a put that takes longer is a wedged object store.
+	PutTimeout = 10 * time.Second
+)
+
+// ErrInvalidObject is a store object whose key, region or metadata cannot be
+// written as the owner's store.
+var ErrInvalidObject = errors.New("uefistore: invalid store object")
+
+// SourceImage is a store seeded from imageID's own store.
+func SourceImage(imageID string) string {
+	return sourceImagePrefix + imageID
+}
+
+// ObjectMeta is what a store object records about itself, as x-amz-meta-
+// entries, so its owner, writer and firmware are known from the object alone.
+type ObjectMeta struct {
+	OwnerID    string
+	WriterNode string
+	Firmware   FirmwareIdentity
+	CodePath   string
+	Source     string
+}
+
+// Metadata returns m as object metadata, keyed by the owner's kind.
+func (m ObjectMeta) Metadata() (map[string]*string, error) {
+	var ownerKey string
+	switch {
+	case strings.HasPrefix(m.OwnerID, "i-"):
+		ownerKey = "instance-id"
+	case strings.HasPrefix(m.OwnerID, "ami-"):
+		ownerKey = "image-id"
+	default:
+		return nil, fmt.Errorf("%w: owner %q is neither an instance nor an image", ErrInvalidObject, m.OwnerID)
+	}
+	switch {
+	case m.WriterNode == "":
+		return nil, fmt.Errorf("%w: %s names no writer node", ErrInvalidObject, m.OwnerID)
+	case m.Source == "":
+		return nil, fmt.Errorf("%w: %s names no source", ErrInvalidObject, m.OwnerID)
+	case m.Firmware.Arch == "" || m.Firmware.VarsSize <= 0 || m.Firmware.CodeSHA256 == "":
+		return nil, fmt.Errorf("%w: %s has an incomplete firmware identity", ErrInvalidObject, m.OwnerID)
+	}
+	return map[string]*string{
+		ownerKey:               aws.String(m.OwnerID),
+		"writer-node":          aws.String(m.WriterNode),
+		"firmware-arch":        aws.String(m.Firmware.Arch),
+		"firmware-vars-size":   aws.String(strconv.FormatInt(m.Firmware.VarsSize, 10)),
+		"firmware-code-sha256": aws.String(m.Firmware.CodeSHA256),
+		"firmware-code-path":   aws.String(m.CodePath),
+		"source":               aws.String(m.Source),
+	}, nil
+}
+
+// NewObjectKey returns a fresh key under ownerID's prefix, named by a random
+// version 4 UUID, so no write ever replaces an object a record points at.
+func NewObjectKey(ownerID string) (string, error) {
+	var b [16]byte
+	if _, err := rand.Read(b[:]); err != nil {
+		return "", fmt.Errorf("generate UEFI store key for %s: %w", ownerID, err)
+	}
+	b[6] = b[6]&0x0f | 0x40
+	b[8] = b[8]&0x3f | 0x80
+	return fmt.Sprintf("%s%x-%x-%x-%x-%x", KeyPrefix(ownerID), b[0:4], b[4:6], b[6:8], b[8:10], b[10:16]), nil
+}
+
+// PutObject writes region gzip-compressed to key with meta, within
+// PutTimeout. The region must be exactly the firmware's VARS size, because
+// pflash refuses any other.
+func PutObject(ctx context.Context, objects objectstore.ObjectStore, bucket, key string, region []byte, meta ObjectMeta) error {
+	if !ownsKey(meta.OwnerID, key) {
+		return fmt.Errorf("%w: key %q is not under %s", ErrInvalidObject, key, KeyPrefix(meta.OwnerID))
+	}
+	if int64(len(region)) != meta.Firmware.VarsSize {
+		return fmt.Errorf("%w: %s is a %d-byte region under firmware with a %d-byte VARS region",
+			ErrInvalidObject, key, len(region), meta.Firmware.VarsSize)
+	}
+	md, err := meta.Metadata()
+	if err != nil {
+		return err
+	}
+	var body bytes.Buffer
+	zw := gzip.NewWriter(&body)
+	if _, err := zw.Write(region); err != nil {
+		return fmt.Errorf("compress UEFI store %s: %w", key, err)
+	}
+	if err := zw.Close(); err != nil {
+		return fmt.Errorf("compress UEFI store %s: %w", key, err)
+	}
+
+	putCtx, cancel := context.WithTimeout(ctx, PutTimeout)
+	defer cancel()
+	if _, err := objects.PutObject(putCtx, &s3.PutObjectInput{
+		Bucket:   aws.String(bucket),
+		Key:      aws.String(key),
+		Body:     bytes.NewReader(body.Bytes()),
+		Metadata: md,
+	}); err != nil {
+		return fmt.Errorf("put UEFI store %s: %w", key, err)
+	}
+	return nil
+}

spinifex/domains/ec2/instance/uefistore/object_test.go

@@ -0,0 +1,164 @@
+package uefistore_test
+
+import (
+	"bytes"
+	"compress/gzip"
+	"context"
+	"errors"
+	"io"
+	"regexp"
+	"testing"
+
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/service/s3"
+	"github.com/mulgadc/spinifex/spinifex/domains/ec2/instance/uefistore"
+	"github.com/mulgadc/spinifex/spinifex/providers/objectstore"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+const testBucket = "instances"
+
+func testMeta(owner string) uefistore.ObjectMeta {
+	return uefistore.ObjectMeta{
+		OwnerID:    owner,
+		WriterNode: "node-1",
+		Firmware:   uefistore.FirmwareIdentity{Arch: "arm64", VarsSize: 1 << 16, CodeSHA256: "abc123"},
+		CodePath:   "/usr/share/AAVMF/AAVMF_CODE.fd",
+		Source:     uefistore.SourceTemplate,
+	}
+}
+
+var uuidV4 = regexp.MustCompile(`^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$`)
+
+// Every key is a fresh version 4 UUID directly under the owner's prefix, so
+// a write never lands on an object a record already points at.
+func TestNewObjectKey_IsAFreshUUIDUnderTheOwnersPrefix(t *testing.T) {
+	seen := map[string]bool{}
+	for range 64 {
+		key, err := uefistore.NewObjectKey(instanceID)
+		require.NoError(t, err)
+		name, ok := bytes.CutPrefix([]byte(key), []byte(uefistore.KeyPrefix(instanceID)))
+		require.True(t, ok, key)
+		assert.Regexp(t, uuidV4, string(name))
+		assert.False(t, seen[key], "key %s repeated", key)
+		seen[key] = true
+	}
+}
+
+func TestObjectMeta_Metadata_NamesTheOwnerByKind(t *testing.T) {
+	md, err := testMeta(instanceID).Metadata()
+	require.NoError(t, err)
+	assert.Equal(t, map[string]string{
+		"instance-id":          instanceID,
+		"writer-node":          "node-1",
+		"firmware-arch":        "arm64",
+		"firmware-vars-size":   "65536",
+		"firmware-code-sha256": "abc123",
+		"firmware-code-path":   "/usr/share/AAVMF/AAVMF_CODE.fd",
+		"source":               "template",
+	}, aws.StringValueMap(md))
+
+	img := testMeta("ami-0123456789abcdef0")
+	img.Source = uefistore.SourceImage("ami-0fedcba9876543210")
+	md, err = img.Metadata()
+	require.NoError(t, err)
+	assert.Equal(t, "ami-0123456789abcdef0", aws.StringValue(md["image-id"]))
+	assert.NotContains(t, md, "instance-id")
+	assert.Equal(t, "image:ami-0fedcba9876543210", aws.StringValue(md["source"]))
+}
+
+func TestObjectMeta_Metadata_RefusesAnIncompleteObject(t *testing.T) {
+	tests := map[string]func(*uefistore.ObjectMeta){
+		"unknown owner":  func(m *uefistore.ObjectMeta) { m.OwnerID = "vol-1" },
+		"no writer":      func(m *uefistore.ObjectMeta) { m.WriterNode = "" },
+		"no source":      func(m *uefistore.ObjectMeta) { m.Source = "" },
+		"no arch":        func(m *uefistore.ObjectMeta) { m.Firmware.Arch = "" },
+		"no vars size":   func(m *uefistore.ObjectMeta) { m.Firmware.VarsSize = 0 },
+		"no code sha256": func(m *uefistore.ObjectMeta) { m.Firmware.CodeSHA256 = "" },
+	}
+	for name, mutate := range tests {
+		t.Run(name, func(t *testing.T) {
+			m := testMeta(instanceID)
+			mutate(&m)
+			_, err := m.Metadata()
+			require.ErrorIs(t, err, uefistore.ErrInvalidObject)
+		})
+	}
+}
+
+// The 64 MiB arm64 region is mostly erased flash, so it is stored compressed
+// and decompresses to the exact region.
+func TestPutObject_StoresTheRegionCompressed(t *testing.T) {
+	objects := objectstore.NewMemoryObjectStore()
+	meta := testMeta(instanceID)
+	region := bytes.Repeat([]byte{0xff}, int(meta.Firmware.VarsSize))
+	copy(region, "BootOrder")
+	key, err := uefistore.NewObjectKey(instanceID)
+	require.NoError(t, err)
+
+	require.NoError(t, uefistore.PutObject(t.Context(), objects, testBucket, key, region, meta))
+
+	out, err := objects.GetObject(t.Context(), &s3.GetObjectInput{Bucket: aws.String(testBucket), Key: aws.String(key)})
+	require.NoError(t, err)
+	body, err := io.ReadAll(out.Body)
+	require.NoError(t, err)
+	assert.Less(t, len(body), len(region)/10, "the erased region compresses")
+	gz, err := gzip.NewReader(bytes.NewReader(body))
+	require.NoError(t, err)
+	got, err := io.ReadAll(gz)
+	require.NoError(t, err)
+	assert.Equal(t, region, got)
+}
+
+// pflash needs a byte-exact region, and a key outside the owner's prefix is
+// one no store record owns; either is refused before anything is put.
+func TestPutObject_RefusesWhatTheOwnerCannotBoot(t *testing.T) {
+	meta := testMeta(instanceID)
+	good, err := uefistore.NewObjectKey(instanceID)
+	require.NoError(t, err)
+	other, err := uefistore.NewObjectKey("i-0fedcba9876543210")
+	require.NoError(t, err)
+	tests := []struct {
+		name   string
+		key    string
+		region []byte
+	}{
+		{name: "short region", key: good, region: make([]byte, meta.Firmware.VarsSize-1)},
+		{name: "long region", key: good, region: make([]byte, meta.Firmware.VarsSize+1)},
+		{name: "another owner's key", key: other, region: make([]byte, meta.Firmware.VarsSize)},
+		{name: "nested key", key: uefistore.KeyPrefix(instanceID) + "a/b", region: make([]byte, meta.Firmware.VarsSize)},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			objects := objectstore.NewMemoryObjectStore()
+			err := uefistore.PutObject(t.Context(), objects, testBucket, tt.key, tt.region, meta)
+			require.ErrorIs(t, err, uefistore.ErrInvalidObject)
+			out, listErr := objects.ListObjectsV2(t.Context(), &s3.ListObjectsV2Input{Bucket: aws.String(testBucket)})
+			require.NoError(t, listErr)
+			assert.Empty(t, out.Contents)
+		})
+	}
+}
+
+type failingPut struct {
+	*objectstore.MemoryObjectStore
+}
+
+func (failingPut) PutObject(ctx context.Context, _ *s3.PutObjectInput) (*s3.PutObjectOutput, error) {
+	if _, ok := ctx.Deadline(); !ok {
+		return nil, errors.New("put had no deadline")
+	}
+	return nil, errors.New("predastore unreachable")
+}
+
+// A put runs under a deadline, so a wedged object store fails the write
+// rather than holding the caller, and its error reaches the caller.
+func TestPutObject_IsBoundedAndReportsAFailedPut(t *testing.T) {
+	key, err := uefistore.NewObjectKey(instanceID)
+	require.NoError(t, err)
+	meta := testMeta(instanceID)
+	err = uefistore.PutObject(t.Context(), failingPut{objectstore.NewMemoryObjectStore()}, testBucket, key, make([]byte, meta.Firmware.VarsSize), meta)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "predastore unreachable")
+}

spinifex/domains/ec2/instance/uefistore/prune.go

@@ -0,0 +1,69 @@
+package uefistore
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"slices"
+	"strings"
+	"time"
+
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/service/s3"
+	"github.com/mulgadc/spinifex/spinifex/providers/objectstore"
+)
+
+const (
+	// RetainedVersions is how many of an owner's newest objects a prune keeps,
+	// besides every object it is told to keep.
+	RetainedVersions = 3
+
+	// PruneTimeout bounds one prune: a list of a prefix that holds at most a
+	// handful of objects, and the deletes of the few past retention.
+	PruneTimeout = 10 * time.Second
+)
+
+// Prune deletes ownerID's objects other than the RetainedVersions newest and
+// the keys in keep, returning the keys it deleted. A failed list deletes
+// nothing; a failed delete is reported and the rest still run.
+func Prune(ctx context.Context, objects objectstore.ObjectStore, bucket, ownerID string, keep ...string) ([]string, error) {
+	ctx, cancel := context.WithTimeout(ctx, PruneTimeout)
+	defer cancel()
+
+	listed, _, err := objectstore.ListAll(ctx, objects, &s3.ListObjectsV2Input{
+		Bucket: aws.String(bucket),
+		Prefix: aws.String(KeyPrefix(ownerID)),
+	})
+	if err != nil {
+		return nil, fmt.Errorf("list UEFI stores of %s: %w", ownerID, err)
+	}
+	var owned []*s3.Object
+	for _, obj := range listed {
+		if obj != nil && ownsKey(ownerID, aws.StringValue(obj.Key)) {
+			owned = append(owned, obj)
+		}
+	}
+	// Newest first, with the key breaking a tie so every prune of one listing
+	// agrees on which objects are the newest.
+	slices.SortFunc(owned, func(a, b *s3.Object) int {
+		if c := aws.TimeValue(b.LastModified).Compare(aws.TimeValue(a.LastModified)); c != 0 {
+			return c
+		}
+		return strings.Compare(aws.StringValue(b.Key), aws.StringValue(a.Key))
+	})
+
+	var deleted []string
+	var errs []error
+	for i, obj := range owned {
+		key := aws.StringValue(obj.Key)
+		if i < RetainedVersions || slices.Contains(keep, key) {
+			continue
+		}
+		if _, err := objects.DeleteObject(ctx, &s3.DeleteObjectInput{Bucket: aws.String(bucket), Key: aws.String(key)}); err != nil && !objectstore.IsNoSuchKeyError(err) {
+			errs = append(errs, fmt.Errorf("delete UEFI store %s: %w", key, err))
+			continue
+		}
+		deleted = append(deleted, key)
+	}
+	return deleted, errors.Join(errs...)
+}

spinifex/domains/ec2/instance/uefistore/prune_test.go

@@ -0,0 +1,177 @@
+package uefistore_test
+
+import (
+	"bytes"
+	"context"
+	"errors"
+	"fmt"
+	"math/rand/v2"
+	"slices"
+	"testing"
+	"time"
+
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/service/s3"
+	"github.com/mulgadc/spinifex/spinifex/domains/ec2/instance/uefistore"
+	"github.com/mulgadc/spinifex/spinifex/providers/objectstore"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// clockedStore lists each object at the time the test wrote it, so which
+// objects are the newest does not depend on the wall clock's resolution.
+type clockedStore struct {
+	*objectstore.MemoryObjectStore
+
+	written   map[string]time.Time
+	listErr   error
+	deleteErr map[string]error
+	deleted   []string
+}
+
+func newClockedStore() *clockedStore {
+	return &clockedStore{MemoryObjectStore: objectstore.NewMemoryObjectStore(), written: map[string]time.Time{}, deleteErr: map[string]error{}}
+}
+
+func (c *clockedStore) putAt(t *testing.T, key string, at time.Time) {
+	t.Helper()
+	_, err := c.PutObject(t.Context(), &s3.PutObjectInput{Bucket: aws.String(testBucket), Key: aws.String(key), Body: bytes.NewReader([]byte("x"))})
+	require.NoError(t, err)
+	c.written[key] = at
+}
+
+func (c *clockedStore) ListObjectsV2(ctx context.Context, in *s3.ListObjectsV2Input) (*s3.ListObjectsV2Output, error) {
+	if c.listErr != nil {
+		return nil, c.listErr
+	}
+	out, err := c.MemoryObjectStore.ListObjectsV2(ctx, in)
+	if err != nil {
+		return nil, err
+	}
+	for _, o := range out.Contents {
+		o.LastModified = aws.Time(c.written[aws.StringValue(o.Key)])
+	}
+	return out, nil
+}
+
+func (c *clockedStore) DeleteObject(ctx context.Context, in *s3.DeleteObjectInput) (*s3.DeleteObjectOutput, error) {
+	if err := c.deleteErr[aws.StringValue(in.Key)]; err != nil {
+		return nil, err
+	}
+	c.deleted = append(c.deleted, aws.StringValue(in.Key))
+	return c.MemoryObjectStore.DeleteObject(ctx, in)
+}
+
+func (c *clockedStore) keys(t *testing.T, owner string) []string {
+	t.Helper()
+	listed, _, err := objectstore.ListAll(t.Context(), c.MemoryObjectStore, &s3.ListObjectsV2Input{Bucket: aws.String(testBucket), Prefix: aws.String(uefistore.KeyPrefix(owner))})
+	require.NoError(t, err)
+	var keys []string
+	for _, o := range listed {
+		keys = append(keys, aws.StringValue(o.Key))
+	}
+	return keys
+}
+
+var t0 = time.Date(2026, 11, 3, 0, 0, 0, 0, time.UTC)
+
+func version(n int) string { return uefistore.KeyPrefix(instanceID) + fmt.Sprintf("v%02d", n) }
+
+// The three newest versions and the pointed one are kept; everything else
+// under the owner's prefix goes.
+func TestPrune_KeepsTheThreeNewestAndThePointedVersion(t *testing.T) {
+	store := newClockedStore()
+	for n := 1; n <= 6; n++ {
+		store.putAt(t, version(n), t0.Add(time.Duration(n)*time.Minute))
+	}
+
+	deleted, err := uefistore.Prune(t.Context(), store, testBucket, instanceID, version(1))
+
+	require.NoError(t, err)
+	assert.ElementsMatch(t, []string{version(2), version(3)}, deleted)
+	assert.ElementsMatch(t, []string{version(1), version(4), version(5), version(6)}, store.keys(t, instanceID))
+}
+
+// pruneS7Clause is quoted by every invariant failure below: prune never
+// deletes the pointed object or a retained version.
+const pruneS7Clause = "ADR-0003:S7: a reaper's judgement is never the decision that deletes customer data; " +
+	"the pointed and the retained versions survive every prune"
+
+// Pruning deletes customer data on its own, so whatever the listing order,
+// times and keep set, it never removes a kept key, one of the three newest,
+// or anything outside the owner's own prefix.
+func TestS7_UEFIPruneNeverDeletesThePointedObjectOrARetainedVersion(t *testing.T) {
+	const other = "i-0fedcba9876543210"
+	rng := rand.New(rand.NewPCG(1, 2))
+	for trial := range 200 {
+		store := newClockedStore()
+		n := 1 + rng.IntN(8)
+		times := map[string]time.Time{}
+		var keys []string
+		for i := range n {
+			key := version(i)
+			at := t0.Add(time.Duration(rng.IntN(5)) * time.Second)
+			store.putAt(t, key, at)
+			times[key] = at
+			keys = append(keys, key)
+		}
+		store.putAt(t, uefistore.KeyPrefix(other)+"newest", t0.Add(time.Hour))
+		store.putAt(t, uefistore.KeyPrefix(instanceID)+"nested/obj", t0)
+		keep := []string{keys[rng.IntN(n)]}
+		if rng.IntN(2) == 0 {
+			keep = append(keep, keys[rng.IntN(n)])
+		}
+
+		deleted, err := uefistore.Prune(t.Context(), store, testBucket, instanceID, keep...)
+		require.NoError(t, err)
+
+		for _, key := range deleted {
+			require.NotContains(t, keep, key, "%s: trial %d: a kept key was deleted", pruneS7Clause, trial)
+			require.Contains(t, keys, key, "%s: trial %d: deleted %s outside the owner's own objects", pruneS7Clause, trial, key)
+			survivors := 0
+			for _, k := range keys {
+				if !slices.Contains(deleted, k) && !times[k].Before(times[key]) {
+					survivors++
+				}
+			}
+			require.GreaterOrEqual(t, survivors, uefistore.RetainedVersions,
+				"%s: trial %d: deleted %s while only %d versions at least as new survive", pruneS7Clause, trial, key, survivors)
+		}
+		remaining := store.keys(t, instanceID)
+		require.Contains(t, remaining, uefistore.KeyPrefix(instanceID)+"nested/obj", pruneS7Clause)
+		require.LessOrEqual(t, len(remaining)-1, uefistore.RetainedVersions+len(keep), "trial %d", trial)
+		require.Len(t, store.keys(t, other), 1, pruneS7Clause)
+	}
+}
+
+// A listing that fails decides nothing, so nothing is deleted on a guess.
+func TestPrune_FailedListDeletesNothing(t *testing.T) {
+	store := newClockedStore()
+	for n := 1; n <= 5; n++ {
+		store.putAt(t, version(n), t0.Add(time.Duration(n)*time.Minute))
+	}
+	store.listErr = errors.New("predastore unavailable")
+
+	deleted, err := uefistore.Prune(t.Context(), store, testBucket, instanceID)
+
+	require.Error(t, err)
+	assert.Empty(t, deleted)
+	assert.Empty(t, store.deleted)
+	assert.Len(t, store.keys(t, instanceID), 5)
+}
+
+// One failed delete is reported and does not stop the others; an object
+// already gone counts as deleted.
+func TestPrune_FailedDeleteIsReportedAndTheRestStillRun(t *testing.T) {
+	store := newClockedStore()
+	for n := 1; n <= 6; n++ {
+		store.putAt(t, version(n), t0.Add(time.Duration(n)*time.Minute))
+	}
+	store.deleteErr[version(1)] = errors.New("500")
+	store.deleteErr[version(2)] = &objectstore.NoSuchKeyError{Key: version(2)}
+
+	deleted, err := uefistore.Prune(t.Context(), store, testBucket, instanceID)
+
+	require.ErrorContains(t, err, version(1))
+	assert.ElementsMatch(t, []string{version(2), version(3)}, deleted)
+}

spinifex/domains/ec2/instance/uefistore/uefistore.go

@@ -28,13 +28,19 @@ const (
 	// StateDeleting is a store whose objects are to be deleted, and then the record.
 	StateDeleting = "deleting"
 
-	objectRoot = "uefi/"
+	// ObjectRoot is the object-store prefix every store object is under.
+	ObjectRoot = "uefi/"
 )
 
 // ErrInvalidRecord is returned for a store record that breaks its invariants,
 // whether it is about to be written or was just read.
 var ErrInvalidRecord = errors.New("uefistore: invalid store record")
 
+// ErrDeleting is returned for a change that would return a deleting store to
+// live. Deletion follows only a terminate or an operator's forget, and both are
+// final, so a writer that meets it drops whatever it was about to record.
+var ErrDeleting = errors.New("uefistore: store is being deleted")
+
 // FirmwareIdentity names the firmware a variable store was written under. A
 // store only boots where Arch and VarsSize match the node's firmware, because
 // pflash needs a byte-exact region; CodeSHA256 is recorded, not enforced.
@@ -68,7 +74,7 @@ type UEFIWriteBack struct {
 // KeyPrefix is the object-store prefix every store object of ownerID (an
 // instance or image ID) is written under.
 func KeyPrefix(ownerID string) string {
-	return objectRoot + ownerID + "/"
+	return ObjectRoot + ownerID + "/"
 }
 
 // ownsKey reports whether key names one object directly under ownerID's prefix.
@@ -92,6 +98,9 @@ func (rec *UEFIStoreRecord) validate(key string) error {
 		return fmt.Errorf("%w: instance_id %s stored at key %s", ErrInvalidRecord, rec.InstanceID, key)
 	case rec.State != StateLive && rec.State != StateDeleting:
 		return fmt.Errorf("%w: %s has state %q", ErrInvalidRecord, key, rec.State)
+	case rec.State == StateDeleting && rec.WriteBack != nil:
+		// A write-back retry puts an object, which would land behind the delete.
+		return fmt.Errorf("%w: %s is deleting with a write-back still owed", ErrInvalidRecord, key)
 	}
 	if wb := rec.WriteBack; wb != nil {
 		switch {
@@ -134,6 +143,15 @@ func NewStore(js jetstream.JetStream) *Store {
 	return &Store{kv: kvstore.New[UEFIStoreRecord](js, bucketConfig())}
 }
 
+// NewReader returns a Store over the existing bucket that neither creates it
+// nor migrates it, for an operator report that must not change the cluster.
+func NewReader(js jetstream.JetStream) *Store {
+	cfg := bucketConfig()
+	cfg.AttachOnly = true
+	cfg.OnOpen = nil
+	return &Store{kv: kvstore.New[UEFIStoreRecord](js, cfg)}
+}
+
 // Open creates the bucket if it is missing and stamps its schema version.
 func (s *Store) Open(ctx context.Context) error {
 	_, err := s.kv.KV(ctx)
@@ -169,12 +187,24 @@ func (s *Store) Create(ctx context.Context, rec *UEFIStoreRecord) (uint64, error
 }
 
 // CompareAndSet replaces the record only if it is still at rev, returning
-// kvstore.ErrConflict when it is not, and the revision written otherwise.
+// kvstore.ErrConflict when it is not, and the revision written otherwise. A
+// record at rev that is deleting cannot be set live again: that is ErrDeleting.
 func (s *Store) CompareAndSet(ctx context.Context, rec *UEFIStoreRecord, rev uint64) (uint64, error) {
 	rec.SchemaVersion = RecordSchemaVersion
 	if err := rec.validate(rec.InstanceID); err != nil {
 		return 0, err
 	}
+	if rec.State == StateLive {
+		// Read unvalidated: only the state at rev matters, and any other
+		// revision fails the guarded write below anyway.
+		cur, curRev, err := s.kv.Get(ctx, rec.InstanceID)
+		if err != nil && !errors.Is(err, kvstore.ErrNotFound) {
+			return 0, err
+		}
+		if err == nil && curRev == rev && cur.State == StateDeleting {
+			return 0, fmt.Errorf("%w: %s", ErrDeleting, rec.InstanceID)
+		}
+	}
 	return s.kv.CompareAndSet(ctx, rec.InstanceID, rec, rev)
 }
 

spinifex/domains/ec2/instance/uefistore/writeback.go

@@ -0,0 +1,109 @@
+package uefistore
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"log/slog"
+
+	"github.com/mulgadc/spinifex/spinifex/foundation/state/kvstore"
+)
+
+// ErrNotLive is a store record that is missing or deleting: its instance is
+// gone or going, so no write-back is owed and nothing may be written under it.
+var ErrNotLive = errors.New("uefistore: store record is not live")
+
+// writeBackAttempts bounds the read-and-CAS loop that records or clears an
+// obligation, so a record under constant contention fails rather than spins.
+const writeBackAttempts = 3
+
+// LiveWriteBack returns the write-back owed on instanceID's live record, or
+// nil when none is. A missing or deleting record is ErrNotLive.
+func (s *Store) LiveWriteBack(ctx context.Context, instanceID string) (*UEFIWriteBack, error) {
+	rec, _, err := s.liveRecord(ctx, instanceID)
+	if err != nil {
+		return nil, err
+	}
+	return rec.WriteBack, nil
+}
+
+// RecordWriteBack sets wb as the write-back owed on instanceID's live record,
+// replacing any other: an instance has at most one open.
+func (s *Store) RecordWriteBack(ctx context.Context, instanceID string, wb UEFIWriteBack) error {
+	return s.mutateLive(ctx, instanceID, func(rec *UEFIStoreRecord) bool {
+		rec.WriteBack = &wb
+		return true
+	})
+}
+
+// ClearWriteBack drops the write-back owed on instanceID's live record. With
+// a key it drops only an obligation recorded for that key, so a retry does
+// not clear one recorded after it read the record.
+func (s *Store) ClearWriteBack(ctx context.Context, instanceID, key string) error {
+	return s.mutateLive(ctx, instanceID, func(rec *UEFIStoreRecord) bool {
+		if rec.WriteBack == nil || (key != "" && rec.WriteBack.Key != key) {
+			return false
+		}
+		rec.WriteBack = nil
+		return true
+	})
+}
+
+// WriteBacksOwedBy returns the instances whose live record owes a write-back
+// from node. A record that breaks its invariants is logged and passed over.
+func (s *Store) WriteBacksOwedBy(ctx context.Context, node string) ([]string, error) {
+	// Records are keyed by instance ID; the prefix passes over the bucket's
+	// schema stamp, which shares the key space.
+	recs, err := s.kv.List(ctx, "i-")
+	if err != nil {
+		return nil, err
+	}
+	var ids []string
+	for i := range recs {
+		rec := &recs[i]
+		if err := rec.validate(rec.InstanceID); err != nil {
+			slog.WarnContext(ctx, "Passing over a UEFI store record that cannot be read", "instanceId", rec.InstanceID, "err", err)
+			continue
+		}
+		if rec.State == StateLive && rec.WriteBack != nil && rec.WriteBack.Node == node {
+			ids = append(ids, rec.InstanceID)
+		}
+	}
+	return ids, nil
+}
+
+// liveRecord reads instanceID's record, reporting a missing or deleting one
+// as ErrNotLive.
+func (s *Store) liveRecord(ctx context.Context, instanceID string) (*UEFIStoreRecord, uint64, error) {
+	rec, rev, err := s.Get(ctx, instanceID)
+	if errors.Is(err, kvstore.ErrNotFound) {
+		return nil, 0, fmt.Errorf("%w: %s has no store record", ErrNotLive, instanceID)
+	}
+	if err != nil {
+		return nil, 0, err
+	}
+	if rec.State != StateLive {
+		return nil, 0, fmt.Errorf("%w: %s is %s", ErrNotLive, instanceID, rec.State)
+	}
+	return rec, rev, nil
+}
+
+// mutateLive applies change to instanceID's live record under CAS, re-reading
+// on a lost race. change reports whether it changed anything to write.
+func (s *Store) mutateLive(ctx context.Context, instanceID string, change func(*UEFIStoreRecord) bool) error {
+	for range writeBackAttempts {
+		rec, rev, err := s.liveRecord(ctx, instanceID)
+		if err != nil {
+			return err
+		}
+		if !change(rec) {
+			return nil
+		}
+		_, err = s.CompareAndSet(ctx, rec, rev)
+		if errors.Is(err, kvstore.ErrConflict) {
+			continue
+		}
+		return err
+	}
+	return fmt.Errorf("%w: %s changed under %d attempts", kvstore.ErrConflict, instanceID, writeBackAttempts)
+}

spinifex/domains/ec2/instance/uefistore/writeback_test.go

@@ -0,0 +1,109 @@
+package uefistore_test
+
+import (
+	"testing"
+
+	"github.com/mulgadc/spinifex/spinifex/domains/ec2/instance/uefistore"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func markDeleting(t *testing.T, s *uefistore.Store, id string) {
+	t.Helper()
+	_, err := s.MarkDeleting(t.Context(), id)
+	require.NoError(t, err)
+}
+
+// An obligation is recorded on the live record, replacing any other, and
+// read back as recorded.
+func TestRecordWriteBack_SetsTheOneOpenObligation(t *testing.T) {
+	s, _ := openStore(t)
+	_, err := s.Create(t.Context(), liveRecord())
+	require.NoError(t, err)
+
+	got, err := s.LiveWriteBack(t.Context(), instanceID)
+	require.NoError(t, err)
+	assert.Nil(t, got)
+
+	first := *writeBack()
+	require.NoError(t, s.RecordWriteBack(t.Context(), instanceID, first))
+	second := first
+	second.Node, second.Key = "node-3", uefistore.KeyPrefix(instanceID)+"c"
+	require.NoError(t, s.RecordWriteBack(t.Context(), instanceID, second))
+
+	got, err = s.LiveWriteBack(t.Context(), instanceID)
+	require.NoError(t, err)
+	assert.Equal(t, &second, got)
+}
+
+// A clear for one key leaves an obligation recorded for another, so a retry
+// cannot drop an obligation recorded after it read the record.
+func TestClearWriteBack_ClearsOnlyTheNamedKey(t *testing.T) {
+	s, _ := openStore(t)
+	_, err := s.Create(t.Context(), liveRecord())
+	require.NoError(t, err)
+	wb := *writeBack()
+	require.NoError(t, s.RecordWriteBack(t.Context(), instanceID, wb))
+
+	require.NoError(t, s.ClearWriteBack(t.Context(), instanceID, uefistore.KeyPrefix(instanceID)+"other"))
+	got, err := s.LiveWriteBack(t.Context(), instanceID)
+	require.NoError(t, err)
+	assert.Equal(t, &wb, got)
+
+	require.NoError(t, s.ClearWriteBack(t.Context(), instanceID, wb.Key))
+	got, err = s.LiveWriteBack(t.Context(), instanceID)
+	require.NoError(t, err)
+	assert.Nil(t, got)
+
+	require.NoError(t, s.RecordWriteBack(t.Context(), instanceID, wb))
+	require.NoError(t, s.ClearWriteBack(t.Context(), instanceID, ""), "an empty key clears whatever is owed")
+	got, err = s.LiveWriteBack(t.Context(), instanceID)
+	require.NoError(t, err)
+	assert.Nil(t, got)
+}
+
+// A store whose record is missing or deleting owes no write-back, and none
+// can be recorded on it.
+func TestWriteBack_RecordThatIsNotLiveIsErrNotLive(t *testing.T) {
+	s, _ := openStore(t)
+
+	_, err := s.LiveWriteBack(t.Context(), instanceID)
+	require.ErrorIs(t, err, uefistore.ErrNotLive)
+	require.ErrorIs(t, s.RecordWriteBack(t.Context(), instanceID, *writeBack()), uefistore.ErrNotLive)
+
+	_, err = s.Create(t.Context(), liveRecord())
+	require.NoError(t, err)
+	markDeleting(t, s, instanceID)
+
+	_, err = s.LiveWriteBack(t.Context(), instanceID)
+	require.ErrorIs(t, err, uefistore.ErrNotLive)
+	require.ErrorIs(t, s.RecordWriteBack(t.Context(), instanceID, *writeBack()), uefistore.ErrNotLive)
+	require.ErrorIs(t, s.ClearWriteBack(t.Context(), instanceID, ""), uefistore.ErrNotLive)
+	rec, _, err := s.Get(t.Context(), instanceID)
+	require.NoError(t, err)
+	assert.Equal(t, uefistore.StateDeleting, rec.State)
+	assert.Nil(t, rec.WriteBack, "a deleting record is never given an obligation")
+}
+
+// The reaper is handed only the live records whose obligation names its node.
+func TestWriteBacksOwedBy_ListsOnlyThisNodesLiveObligations(t *testing.T) {
+	s, _ := openStore(t)
+	ids := []string{"i-0000000000000000a", "i-0000000000000000b", "i-0000000000000000c", "i-0000000000000000d"}
+	for _, id := range ids {
+		_, err := s.Create(t.Context(), &uefistore.UEFIStoreRecord{InstanceID: id, State: uefistore.StateLive})
+		require.NoError(t, err)
+	}
+	owe := func(id, node string) {
+		wb := *writeBack()
+		wb.Node, wb.Key, wb.SeededFrom = node, uefistore.KeyPrefix(id)+"b", uefistore.KeyPrefix(id)+"a"
+		require.NoError(t, s.RecordWriteBack(t.Context(), id, wb))
+	}
+	owe(ids[0], "node-1")
+	owe(ids[1], "node-2")
+	owe(ids[2], "node-1")
+	markDeleting(t, s, ids[2])
+
+	got, err := s.WriteBacksOwedBy(t.Context(), "node-1")
+	require.NoError(t, err)
+	assert.Equal(t, []string{ids[0]}, got)
+}

spinifex/domains/ec2/instance/volume_held_elsewhere_test.go

@@ -4,50 +4,19 @@ package instance
 //would widen the handler API for a test rather than for a caller.
 
 import (
+	"context"
 	"errors"
 	"fmt"
 	"testing"
 
+	"github.com/mulgadc/spinifex/contracts/ec2/v1"
 	"github.com/mulgadc/spinifex/spinifex/foundation/aws/errors"
+	"github.com/mulgadc/spinifex/spinifex/providers/ebs"
+	"github.com/mulgadc/spinifex/spinifex/runtime/compute/vm"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
-// TestVolumeHeldElsewhere_NamesTheHolder covers both refusals that mean the
-// data is on another node. An operator who gets ServerInternal is told to retry
-// something that cannot succeed while that node is down, so the node name and
-// the reason have to survive the trip out of the daemon.
-func TestVolumeHeldElsewhere_NamesTheHolder(t *testing.T) {
-	cases := []struct {
-		name string
-		err  error
-	}{
-		{
-			name: "live holder, refused by the lease",
-			err: errors.New("failed to mount volume: volume is leased by another owner: " +
-				"held by node-b since 2026-09-02T17:04:39Z"),
-		},
-	}
-
-	for _, tc := range cases {
-		t.Run(tc.name, func(t *testing.T) {
-			got := volumeHeldElsewhereError(tc.err)
-			require.Error(t, got, "a volume held elsewhere must not fall through to ServerInternal")
-
-			code := awserrors.ValidErrorCodeFromError(got)
-			assert.NotEqual(t, awserrors.ErrorServerInternal, code,
-				"ServerInternal reads as a transient fault and invites a retry that cannot succeed")
-			assert.Contains(t, got.Error(), "node-b", "the caller has to learn where the data is")
-
-			assert.ErrorIs(t, got, ErrVolumeHeldElsewhere,
-				"recovery retries this on a shorter clock than anything else, so it has to be matchable")
-			resolved, _, ok := awserrors.ResolveErrorDetail(got)
-			require.True(t, ok, "the sentinel must not hide the code from the layer that returns it")
-			assert.Equal(t, awserrors.ErrorIncorrectState, resolved)
-		})
-	}
-}
-
 // TestVolumeHeldElsewhere_IgnoresUnrelatedFailures guards the other direction:
 // a genuine internal fault must keep reporting as one rather than being
 // relabelled as somebody else's volume.
@@ -60,4 +29,76 @@ func TestVolumeHeldElsewhere_IgnoresUnrelatedFailures(t *testing.T) {
 	// than refused, so this text is not an exclusion and must not become one.
 	assert.NoError(t, volumeHeldElsewhereError(errors.New(
 		"took over from node-b, which held unsealed writes since 2026-09-02T17:04:39Z")))
+
+	// Only the provider's code classifies a refusal, so its wording alone does not.
+	assert.NoError(t, volumeHeldElsewhereError(errors.New(
+		"failed to mount volume: "+ec2v1.VolumeHeldElsewhereText+": held by node-b")))
+}
+
+// A provider is free to word its refusal however it likes; the held-elsewhere
+// code is what says another node has the volume.
+func TestVolumeHeldElsewhere_MatchesTheProviderCodeWhateverTheWording(t *testing.T) {
+	refusal := fmt.Errorf("publish volume vol-1: %w", &ebsprovider.ProviderError{
+		Code:    ebsprovider.ErrorCodeVolumeHeldElsewhere,
+		Message: "volume vol-1 is published to node-a",
+	})
+
+	got := volumeHeldElsewhereError(refusal)
+	require.Error(t, got, "a held-elsewhere code must be classified even without viperblockd's text")
+	assert.ErrorIs(t, got, ec2v1.ErrVolumeHeldElsewhere)
+	assert.Contains(t, got.Error(), "node-a", "the caller has to learn where the data is")
+	code, _, ok := awserrors.ResolveErrorDetail(got)
+	require.True(t, ok)
+	assert.Equal(t, awserrors.ErrorIncorrectState, code)
+}
+
+// volume_in_use is a read-only mismatch, not another node's claim, so it must
+// not be relabelled as a volume held elsewhere even though the sentinels nest.
+func TestVolumeHeldElsewhere_VolumeInUseAloneIsNotHeldElsewhere(t *testing.T) {
+	mismatch := fmt.Errorf("publish volume vol-1: %w", &ebsprovider.ProviderError{
+		Code:    ebsprovider.ErrorCodeVolumeInUse,
+		Message: "volume vol-1 is exported read-write, not read-only",
+	})
+	assert.NoError(t, volumeHeldElsewhereError(mismatch))
+}
+
+// heldElsewhereMounter refuses every mount the way the contract mounter
+// reports a provider's held-elsewhere code, in wording that is not viperblockd's.
+type heldElsewhereMounter struct{ raceVolumeMounter }
+
+func (heldElsewhereMounter) Mount(context.Context, *vm.VM) error {
+	return fmt.Errorf("publish volume vol-1: %w", &ebsprovider.ProviderError{
+		Code:    ebsprovider.ErrorCodeVolumeHeldElsewhere,
+		Message: "volume vol-1 is published to node-a",
+	})
+}
+
+// Both start paths a caller can reach must hand back the classified refusal:
+// the stopped-instance claim (ec2.start) and the live owner (ec2.cmd), which
+// the EKS reconciler tries first.
+func TestStartPaths_ReportAVolumeHeldElsewhereByCode(t *testing.T) {
+	t.Setenv("XDG_RUNTIME_DIR", t.TempDir())
+	types, _ := defaultPrepareInstanceTypes()
+
+	t.Run("claimed relaunch", func(t *testing.T) {
+		mgr := vm.NewManager()
+		mgr.SetDeps(vm.Deps{NodeID: "node-b", VolumeMounter: heldElsewhereMounter{}})
+		svc := &InstanceServiceImpl{resourceMgr: &fakeResourceCapacityProvider{instanceTypes: types}, vmMgr: mgr}
+
+		err := svc.launchClaimedInstance(t.Context(), &vm.VM{ID: "i-held-claim", AccountID: "acc", InstanceType: "t3.micro", Status: vm.StatePending}, func() {})
+		assert.ErrorIs(t, err, ec2v1.ErrVolumeHeldElsewhere)
+		assert.Equal(t, awserrors.ErrorIncorrectState, awserrors.ValidErrorCodeFromError(err))
+	})
+
+	t.Run("live owner", func(t *testing.T) {
+		const id = "i-held-live"
+		mgr := mgrWith(map[string]*vm.VM{id: {ID: id, Status: vm.StateStopped, InstanceType: "t3.micro"}})
+		mgr.SetDeps(vm.Deps{NodeID: "node-b", VolumeMounter: heldElsewhereMounter{}})
+		v, _ := mgr.Get(id)
+		svc := &InstanceServiceImpl{resourceMgr: &fakeResourceCapacityProvider{instanceTypes: types}, vmMgr: mgr}
+
+		err := svc.StartInstance(t.Context(), v, ec2v1.EC2InstanceCommand{ID: id})
+		assert.ErrorIs(t, err, ec2v1.ErrVolumeHeldElsewhere)
+		assert.Equal(t, awserrors.ErrorIncorrectState, awserrors.ValidErrorCodeFromError(err))
+	})
 }

spinifex/domains/ec2/snapshot/service_impl.go

@@ -254,6 +254,8 @@ var describeSnapshotsValidFilters = map[string]bool{
 	"volume-id":   true,
 	"volume-size": true,
 	"owner-id":    true,
+	"tag-key":     true,
+	"tag-value":   true,
 }
 
 // DescribeSnapshots lists snapshots matching the specified criteria, scoped to the caller's account.
@@ -455,7 +457,7 @@ func ownerIDsIncludeCaller(ownerIDs []*string, accountID string) bool {
 // snapshotMatchesFilters checks whether a snapshot document satisfies all parsed awsfilters.
 func snapshotMatchesFilters(cfg ebsmetadata.Snapshot, filters map[string][]string) bool {
 	for name, values := range filters {
-		if strings.HasPrefix(name, "tag:") {
+		if awsfilters.IsTagFilter(name) {
 			continue
 		}
 

spinifex/domains/ec2/snapshot/service_impl_test.go

@@ -1571,3 +1571,32 @@ func TestCreateSnapshot_ProviderFailureMapsToSpecificError(t *testing.T) {
 		})
 	}
 }
+
+func TestDescribeSnapshots_FilterByTagKeyAndTagValue(t *testing.T) {
+	svc, store := setupTestSnapshotService(t)
+	tagged := createTestSnapshot(t, svc, store, "vol-tagged", 10, map[string]string{"Keep": "x"})
+	createTestSnapshot(t, svc, store, "vol-other", 20, map[string]string{"Other": "y"})
+
+	tests := []struct {
+		name, filter, value string
+		want                []string
+	}{
+		{"tag-key match", "tag-key", "Keep", []string{tagged}},
+		{"tag-value match", "tag-value", "x", []string{tagged}},
+		{"tag-key miss", "tag-key", "Missing", nil},
+		{"tag-value miss", "tag-value", "x-missing", nil},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			out, err := svc.DescribeSnapshots(context.Background(), &ec2.DescribeSnapshotsInput{
+				Filters: []*ec2.Filter{{Name: aws.String(tt.filter), Values: []*string{aws.String(tt.value)}}},
+			}, testAccountID)
+			require.NoError(t, err)
+			var got []string
+			for _, s := range out.Snapshots {
+				got = append(got, *s.SnapshotId)
+			}
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}

spinifex/domains/ec2/volume/service_impl.go

@@ -359,6 +359,8 @@ var describeVolumesValidFilters = map[string]bool{
 	"attachment.device":      true,
 	"availability-zone":      true,
 	"snapshot-id":            true,
+	"tag-key":                true,
+	"tag-value":              true,
 }
 
 // DescribeVolumes lists EBS volumes by reading config.json files from S3.
@@ -415,7 +417,7 @@ func (s *VolumeServiceImpl) DescribeVolumes(ctx context.Context, input *ec2.Desc
 // volumeMatchesFilters checks whether an ec2.Volume satisfies all parsed awsfilters.
 func volumeMatchesFilters(vol *ec2.Volume, filters map[string][]string) bool {
 	for name, values := range filters {
-		if strings.HasPrefix(name, "tag:") {
+		if awsfilters.IsTagFilter(name) {
 			continue
 		}
 
@@ -484,7 +486,6 @@ func volumeMatchesFilters(vol *ec2.Volume, filters map[string][]string) bool {
 		}
 	}
 
-	// Check tag:Key filters
 	tags := awsfilters.EC2TagsToMap(vol.Tags)
 	return awsfilters.MatchesTags(filters, tags)
 }

spinifex/domains/ec2/volume/service_impl_test.go

@@ -2812,3 +2812,40 @@ func TestDescribeVolumesModifications_UnknownFilter(t *testing.T) {
 	assert.Equal(t, awserrors.ErrorInvalidParameterValue, code)
 	assert.Contains(t, message, "not-a-real-filter")
 }
+
+func TestDescribeVolumes_FilterByTagKeyAndTagValue(t *testing.T) {
+	store := objectstore.NewMemoryObjectStore()
+	svc := newTestVolumeServiceWithStore("ap-southeast-2a", store)
+
+	createVolumeInStoreWithMeta(t, svc, store, "vol-tagged", ebsmetadata.Volume{
+		VolumeID: "vol-tagged", CapacityGiB: 10, State: "available", TenantID: "000000000001",
+		Tags: map[string]string{"Keep": "x"},
+	})
+	createVolumeInStoreWithMeta(t, svc, store, "vol-other", ebsmetadata.Volume{
+		VolumeID: "vol-other", CapacityGiB: 10, State: "available", TenantID: "000000000001",
+		Tags: map[string]string{"Other": "y"},
+	})
+
+	tests := []struct {
+		name, filter, value string
+		want                []string
+	}{
+		{"tag-key match", "tag-key", "Keep", []string{"vol-tagged"}},
+		{"tag-value match", "tag-value", "x", []string{"vol-tagged"}},
+		{"tag-key miss", "tag-key", "Missing", nil},
+		{"tag-value miss", "tag-value", "x-missing", nil},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			out, err := svc.DescribeVolumes(context.Background(), &ec2.DescribeVolumesInput{
+				Filters: []*ec2.Filter{{Name: aws.String(tt.filter), Values: []*string{aws.String(tt.value)}}},
+			}, "000000000001")
+			require.NoError(t, err)
+			var got []string
+			for _, v := range out.Volumes {
+				got = append(got, *v.VolumeId)
+			}
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}

spinifex/domains/ec2/vpc/eni.go

@@ -1,6 +1,7 @@
 package vpc
 
 import (
+	"cmp"
 	"context"
 	"encoding/json"
 	"errors"
@@ -1147,9 +1148,16 @@ func (s *VPCServiceImpl) eniRecordToEC2(record *ENIRecord, accountID string, gro
 	}
 
 	if record.AttachmentId != "" {
+		// A system VM's owner is the internal system account, which AWS never
+		// exposes: its service-managed ENIs report the ELB service alias instead.
+		instanceOwner := cmp.Or(record.InstanceOwnerId, accountID)
+		if instanceOwner == awsidentifiers.GlobalAccountID {
+			instanceOwner = "amazon-elb"
+		}
 		eni.Attachment = &ec2.NetworkInterfaceAttachment{
 			AttachmentId:        aws.String(record.AttachmentId),
 			InstanceId:          aws.String(record.InstanceId),
+			InstanceOwnerId:     aws.String(instanceOwner),
 			DeviceIndex:         aws.Int64(record.DeviceIndex),
 			Status:              aws.String("attached"),
 			DeleteOnTermination: aws.Bool(record.DeleteOnTermination == nil || *record.DeleteOnTermination),

spinifex/domains/ec2/vpc/eni_attachment_owner_test.go

@@ -0,0 +1,56 @@
+package vpc
+
+import (
+	"context"
+	"testing"
+
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/service/ec2"
+	"github.com/mulgadc/spinifex/spinifex/foundation/aws/tags"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func describeOneENI(t *testing.T, svc *VPCServiceImpl, eniID string) *ec2.NetworkInterface {
+	t.Helper()
+	out, err := svc.DescribeNetworkInterfaces(context.Background(), &ec2.DescribeNetworkInterfacesInput{
+		NetworkInterfaceIds: []*string{aws.String(eniID)},
+	}, testAccountID)
+	require.NoError(t, err)
+	require.Len(t, out.NetworkInterfaces, 1)
+	require.NotNil(t, out.NetworkInterfaces[0].Attachment)
+	return out.NetworkInterfaces[0]
+}
+
+func TestDescribeNetworkInterfaces_CustomerInstanceAttachmentOwnerIsTheAccount(t *testing.T) {
+	t.Parallel()
+	svc := setupTestVPCService(t)
+	vpcID := createTestVPC(t, svc, "10.0.0.0/16")
+	subnetID := createTestSubnet(t, svc, vpcID, "10.0.1.0/24")
+	eniID := createTestENI(t, svc, subnetID)
+
+	_, err := svc.AttachENI(testAccountID, eniID, "i-customer", 0)
+	require.NoError(t, err)
+
+	eni := describeOneENI(t, svc, eniID)
+	assert.Equal(t, testAccountID, aws.StringValue(eni.Attachment.InstanceOwnerId))
+}
+
+func TestDescribeNetworkInterfaces_LBAttachmentOwnerIsAmazonELB(t *testing.T) {
+	t.Parallel()
+	svc := setupTestVPCService(t)
+	vpcID := createTestVPC(t, svc, "10.0.0.0/16")
+	subnetID := createTestSubnet(t, svc, vpcID, "10.0.1.0/24")
+	eniID := createTestENI(t, svc, subnetID)
+	require.NoError(t, svc.ApplyRecordTags(&ec2.CreateTagsInput{
+		Resources: []*string{aws.String(eniID)},
+		Tags:      []*ec2.Tag{{Key: aws.String(tags.ManagedByKey), Value: aws.String(tags.ManagedByELBv2)}},
+	}, testAccountID))
+
+	_, err := svc.AttachENIWithOwner(testAccountID, eniID, "i-lb", 0, "000000000000")
+	require.NoError(t, err)
+
+	eni := describeOneENI(t, svc, eniID)
+	assert.Equal(t, "amazon-elb", aws.StringValue(eni.Attachment.InstanceOwnerId))
+	assert.True(t, aws.BoolValue(eni.RequesterManaged))
+}

spinifex/domains/ec2/vpc/external_ipam.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"net"
 	"net/netip"
 
 	"github.com/mulgadc/spinifex/spinifex/domains/network/external"
@@ -231,58 +230,3 @@ func filterStatic(pools []external.ExternalPoolConfig) []external.ExternalPoolCo
 	}
 	return out
 }
-
-// ValidatePoolConfig checks that a pool config is valid.
-func ValidatePoolConfig(pool external.ExternalPoolConfig) error {
-	if pool.Name == "" {
-		return fmt.Errorf("pool name is required")
-	}
-	if pool.Gateway == "" {
-		return fmt.Errorf("gateway is required for pool %q", pool.Name)
-	}
-	if net.ParseIP(pool.Gateway) == nil {
-		return fmt.Errorf("invalid gateway IP: %q", pool.Gateway)
-	}
-	if pool.GatewayIP != "" && net.ParseIP(pool.GatewayIP) == nil {
-		return fmt.Errorf("invalid gateway_ip: %q", pool.GatewayIP)
-	}
-	startIP := net.ParseIP(pool.RangeStart)
-	if startIP == nil {
-		return fmt.Errorf("invalid range_start: %q", pool.RangeStart)
-	}
-	endIP := net.ParseIP(pool.RangeEnd)
-	if endIP == nil {
-		return fmt.Errorf("invalid range_end: %q", pool.RangeEnd)
-	}
-	if compareIPs(startIP, endIP) > 0 {
-		return fmt.Errorf("range_start %s is greater than range_end %s", pool.RangeStart, pool.RangeEnd)
-	}
-	if pool.GwLrpRangeStart != "" || pool.GwLrpRangeEnd != "" {
-		gwS := net.ParseIP(pool.GwLrpRangeStart)
-		gwE := net.ParseIP(pool.GwLrpRangeEnd)
-		if gwS == nil {
-			return fmt.Errorf("invalid gw_lrp_range_start: %q", pool.GwLrpRangeStart)
-		}
-		if gwE == nil {
-			return fmt.Errorf("invalid gw_lrp_range_end: %q", pool.GwLrpRangeEnd)
-		}
-		if compareIPs(gwS, gwE) > 0 {
-			return fmt.Errorf("gw_lrp_range_start %s is greater than gw_lrp_range_end %s",
-				pool.GwLrpRangeStart, pool.GwLrpRangeEnd)
-		}
-		// Overlap test: !(gwE < rangeS || gwS > rangeE)
-		if compareIPs(gwE, startIP) >= 0 && compareIPs(gwS, endIP) <= 0 {
-			return fmt.Errorf("gw_lrp_range %s-%s overlaps range %s-%s",
-				pool.GwLrpRangeStart, pool.GwLrpRangeEnd, pool.RangeStart, pool.RangeEnd)
-		}
-	}
-	return nil
-}
-
-// compareIPs orders two addresses. Callers parse both first, so an invalid Addr
-// only arises from a nil net.IP and sorts before every valid address.
-func compareIPs(a, b net.IP) int {
-	x, _ := netip.AddrFromSlice(a)
-	y, _ := netip.AddrFromSlice(b)
-	return x.Unmap().Compare(y.Unmap())
-}

spinifex/domains/ec2/vpc/external_ipam_test.go

@@ -286,67 +286,6 @@ func TestExternalIPAM_SpecificPool(t *testing.T) {
 	assert.Equal(t, "10.0.1.2", ip) // .1 reserved for gw
 }
 
-func TestExternalIPAM_RangeValidation(t *testing.T) {
-	t.Parallel()
-	tests := []struct {
-		name    string
-		pool    external.ExternalPoolConfig
-		wantErr string
-	}{
-		{
-			name:    "empty name",
-			pool:    external.ExternalPoolConfig{RangeStart: "10.0.0.1", RangeEnd: "10.0.0.10", Gateway: "10.0.0.254"},
-			wantErr: "pool name is required",
-		},
-		{
-			name:    "bad range_start",
-			pool:    external.ExternalPoolConfig{Name: "bad", RangeStart: "not-an-ip", RangeEnd: "10.0.0.10", Gateway: "10.0.0.254"},
-			wantErr: "invalid range_start",
-		},
-		{
-			name:    "bad range_end",
-			pool:    external.ExternalPoolConfig{Name: "bad", RangeStart: "10.0.0.1", RangeEnd: "not-an-ip", Gateway: "10.0.0.254"},
-			wantErr: "invalid range_end",
-		},
-		{
-			name:    "start > end",
-			pool:    external.ExternalPoolConfig{Name: "bad", RangeStart: "10.0.0.10", RangeEnd: "10.0.0.1", Gateway: "10.0.0.254"},
-			wantErr: "greater than range_end",
-		},
-		{
-			name:    "missing gateway",
-			pool:    external.ExternalPoolConfig{Name: "bad", RangeStart: "10.0.0.1", RangeEnd: "10.0.0.10"},
-			wantErr: "gateway is required",
-		},
-		{
-			name:    "bad gateway",
-			pool:    external.ExternalPoolConfig{Name: "bad", RangeStart: "10.0.0.1", RangeEnd: "10.0.0.10", Gateway: "nope"},
-			wantErr: "invalid gateway IP",
-		},
-		{
-			name:    "bad gateway_ip",
-			pool:    external.ExternalPoolConfig{Name: "ok", RangeStart: "10.0.0.1", RangeEnd: "10.0.0.10", Gateway: "10.0.0.254", GatewayIP: "nope"},
-			wantErr: "invalid gateway_ip",
-		},
-		{
-			name: "valid",
-			pool: external.ExternalPoolConfig{Name: "ok", RangeStart: "10.0.0.1", RangeEnd: "10.0.0.10", Gateway: "10.0.0.254"},
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-			err := ValidatePoolConfig(tt.pool)
-			if tt.wantErr != "" {
-				assert.ErrorContains(t, err, tt.wantErr)
-			} else {
-				assert.NoError(t, err)
-			}
-		})
-	}
-}
-
 func TestExternalIPAM_InitFromConfig(t *testing.T) {
 	t.Parallel()
 	pool := testPool()
@@ -415,67 +354,3 @@ func TestExternalIPAM_FindPoolByName_NotFound(t *testing.T) {
 	_, err := ipam2.AllocateFromPool(context.Background(), "nonexistent", PurposeENIPublic, "", "eni-1", "i-1")
 	assert.Error(t, err)
 }
-
-func TestValidatePoolConfig(t *testing.T) {
-	t.Parallel()
-	base := func() external.ExternalPoolConfig {
-		return external.ExternalPoolConfig{
-			Name:       "wan",
-			Gateway:    "192.168.1.1",
-			RangeStart: "192.168.1.100",
-			RangeEnd:   "192.168.1.110",
-			PrefixLen:  24,
-		}
-	}
-	cases := []struct {
-		name    string
-		mutate  func(*external.ExternalPoolConfig)
-		wantErr string
-	}{
-		{name: "ok", mutate: func(p *external.ExternalPoolConfig) {}},
-		{name: "no name", mutate: func(p *external.ExternalPoolConfig) { p.Name = "" }, wantErr: "pool name is required"},
-		{name: "no gateway", mutate: func(p *external.ExternalPoolConfig) { p.Gateway = "" }, wantErr: "gateway is required"},
-		{name: "bad gateway", mutate: func(p *external.ExternalPoolConfig) { p.Gateway = "x" }, wantErr: "invalid gateway IP"},
-		{name: "bad gateway_ip", mutate: func(p *external.ExternalPoolConfig) { p.GatewayIP = "x" }, wantErr: "invalid gateway_ip"},
-		{name: "bad range_start", mutate: func(p *external.ExternalPoolConfig) { p.RangeStart = "x" }, wantErr: "invalid range_start"},
-		{name: "bad range_end", mutate: func(p *external.ExternalPoolConfig) { p.RangeEnd = "x" }, wantErr: "invalid range_end"},
-		{name: "range reversed", mutate: func(p *external.ExternalPoolConfig) {
-			p.RangeStart = "192.168.1.200"
-			p.RangeEnd = "192.168.1.100"
-		}, wantErr: "greater than range_end"},
-		{name: "bad gw_lrp_start", mutate: func(p *external.ExternalPoolConfig) {
-			p.GwLrpRangeStart = "x"
-			p.GwLrpRangeEnd = "192.168.1.20"
-		}, wantErr: "invalid gw_lrp_range_start"},
-		{name: "bad gw_lrp_end", mutate: func(p *external.ExternalPoolConfig) {
-			p.GwLrpRangeStart = "192.168.1.20"
-			p.GwLrpRangeEnd = "x"
-		}, wantErr: "invalid gw_lrp_range_end"},
-		{name: "gw_lrp reversed", mutate: func(p *external.ExternalPoolConfig) {
-			p.GwLrpRangeStart = "192.168.1.30"
-			p.GwLrpRangeEnd = "192.168.1.20"
-		}, wantErr: "greater than gw_lrp_range_end"},
-		{name: "gw_lrp overlaps range", mutate: func(p *external.ExternalPoolConfig) {
-			p.GwLrpRangeStart = "192.168.1.105"
-			p.GwLrpRangeEnd = "192.168.1.108"
-		}, wantErr: "overlaps range"},
-		{name: "gw_lrp valid below range", mutate: func(p *external.ExternalPoolConfig) {
-			p.GwLrpRangeStart = "192.168.1.20"
-			p.GwLrpRangeEnd = "192.168.1.29"
-		}},
-	}
-	for _, tc := range cases {
-		t.Run(tc.name, func(t *testing.T) {
-			t.Parallel()
-			p := base()
-			tc.mutate(&p)
-			err := ValidatePoolConfig(p)
-			if tc.wantErr == "" {
-				assert.NoError(t, err)
-				return
-			}
-			require.Error(t, err)
-			assert.Contains(t, err.Error(), tc.wantErr)
-		})
-	}
-}

spinifex/domains/ec2/vpc/ipam_test.go

@@ -2,7 +2,6 @@ package vpc
 
 import (
 	"errors"
-	"net"
 	"strconv"
 	"sync"
 	"testing"
@@ -337,13 +336,6 @@ func TestIPAM_ClaimIP_ConcurrentRace(t *testing.T) {
 	assert.Equal(t, "10.0.38.50", ips[0].IP)
 }
 
-func TestCompareIPs_NilSortsFirst(t *testing.T) {
-	t.Parallel()
-	assert.Negative(t, compareIPs(nil, net.ParseIP("10.0.0.1")))
-	assert.Zero(t, compareIPs(net.ParseIP("10.0.0.1"), net.ParseIP("10.0.0.1")))
-	assert.Positive(t, compareIPs(net.ParseIP("10.0.0.2"), net.ParseIP("10.0.0.1")))
-}
-
 func itoa(i int) string {
 	return strconv.Itoa(i)
 }

spinifex/domains/ec2/vpc/security_group_test.go

@@ -2653,3 +2653,29 @@ func TestApplyRecordTags_ReachesASecurityGroupRule(t *testing.T) {
 	require.NoError(t, err)
 	assert.Empty(t, desc.SecurityGroupRules[0].Tags)
 }
+
+// The gateway refuses aws: keys from customers; system callers such as EKS
+// reach the service directly and still set and filter on them.
+func TestCreateSecurityGroup_SystemCallerSetsReservedTag(t *testing.T) {
+	t.Parallel()
+	svc := setupTestVPCService(t)
+	vpcID := createTestVPC(t, svc, "10.0.0.0/16")
+
+	out, err := svc.CreateSecurityGroup(context.Background(), &ec2.CreateSecurityGroupInput{
+		GroupName:   aws.String("eks-cluster-sg"),
+		Description: aws.String("eks"),
+		VpcId:       aws.String(vpcID),
+		TagSpecifications: []*ec2.TagSpecification{{
+			ResourceType: aws.String("security-group"),
+			Tags:         []*ec2.Tag{{Key: aws.String("aws:eks:cluster-name"), Value: aws.String("alpha")}},
+		}},
+	}, testAccountID)
+	require.NoError(t, err)
+
+	desc, err := svc.DescribeSecurityGroups(context.Background(), &ec2.DescribeSecurityGroupsInput{
+		Filters: []*ec2.Filter{{Name: aws.String("tag:aws:eks:cluster-name"), Values: aws.StringSlice([]string{"alpha"})}},
+	}, testAccountID)
+	require.NoError(t, err)
+	require.Len(t, desc.SecurityGroups, 1)
+	assert.Equal(t, *out.GroupId, *desc.SecurityGroups[0].GroupId)
+}

spinifex/domains/ecr/registry/bridge.go

@@ -185,13 +185,14 @@ func (b *Bridge) serveRegistry(w http.ResponseWriter, r *http.Request) {
 			WriteError(w, http.StatusServiceUnavailable, "UNKNOWN", "authorization unavailable")
 			return
 		}
-		if err := b.authorizer.Authorize(r, principal, policy.IAMAction("ecr", req.Action), []string{resource}); err != nil {
+		action := policy.IAMAction("ecr", req.Action)
+		if err := b.authorizer.Authorize(r, principal, action, []string{resource}); err != nil {
 			if errors.Is(err, ErrUnavailable) {
 				// IAM/STS/NATS dependency failure: fail closed, never dispatch.
 				WriteError(w, http.StatusServiceUnavailable, "UNKNOWN", "authorization unavailable")
 				return
 			}
-			WriteError(w, http.StatusForbidden, "DENIED", "insufficient permissions")
+			WriteError(w, http.StatusForbidden, "DENIED", fmt.Sprintf("User: %s is not authorized to perform: %s on resource: %s", principal.CallerARN, action, resource))
 			return
 		}
 	}

spinifex/domains/ecr/registry/bridge_test.go

@@ -571,6 +571,28 @@ func TestBridge_OperationAuthorization(t *testing.T) {
 	})
 }
 
+func TestBridge_DenyMessageNamesCallerActionAndRepository(t *testing.T) {
+	cases := []struct {
+		name, method, path, action string
+	}{
+		{"pull", http.MethodGet, "/v2/team/app/manifests/latest", "ecr:BatchGetImage"},
+		{"push", http.MethodPut, "/v2/team/app/manifests/latest", "ecr:PutImage"},
+		{"list", http.MethodGet, "/v2/team/app/tags/list", "ecr:ListImages"},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			e := newBridgeEnv(t)
+			e.rehyd.principal = e.roleLive.principal
+			w := e.do(c.method, c.path, "Bearer "+e.roleToken(t, bridgeRoleID))
+			assert.Equal(t, http.StatusForbidden, w.Code)
+			want := "User: " + bridgeRoleSessionARN + " is not authorized to perform: " + c.action + " on resource: " + repoARN("team/app")
+			body, err := json.Marshal(map[string]any{"errors": []map[string]string{{"code": "DENIED", "message": want}}})
+			require.NoError(t, err)
+			assert.JSONEq(t, string(body), w.Body.String())
+		})
+	}
+}
+
 func TestBridge_TokenEndpoint(t *testing.T) {
 	t.Run("exchange mints a fresh token for the same principal", func(t *testing.T) {
 		e := newBridgeEnv(t)

spinifex/domains/ecs/capacity_providers.go

@@ -37,7 +37,13 @@ func (s *Service) PutClusterCapacityProviders(ctx context.Context, input *ecs.Pu
 	if err := taskdefinition.PutJSON(ctx, kv, ClusterMetaKey(cluster), &rec); err != nil {
 		return nil, err
 	}
-	return &ecs.PutClusterCapacityProvidersOutput{Cluster: rec.toAWS()}, nil
+	c := rec.toAWS()
+	c.Tags = []*ecs.Tag{}
+	c.Attachments = []*ecs.Attachment{}
+	if _, err := s.applyClusterCounts(ctx, kv, cluster, c); err != nil {
+		return nil, err
+	}
+	return &ecs.PutClusterCapacityProvidersOutput{Cluster: c}, nil
 }
 
 // CreateCapacityProvider persists a new capacity provider. Idempotent:

spinifex/domains/ecs/cluster_teardown.go

@@ -56,7 +56,9 @@ func (s *Service) DeleteCluster(ctx context.Context, input *ecs.DeleteClusterInp
 	}
 
 	rec.Status = ClusterStatusInactive
-	return &ecs.DeleteClusterOutput{Cluster: rec.toAWS()}, nil
+	c := rec.toAWS()
+	c.Tags = []*ecs.Tag{}
+	return &ecs.DeleteClusterOutput{Cluster: c}, nil
 }
 
 // DeregisterContainerInstance removes a container instance from a cluster. With

spinifex/domains/ecs/service.go

@@ -9,6 +9,7 @@ import (
 	"log/slog"
 	"maps"
 	"slices"
+	"strconv"
 	"strings"
 	"time"
 
@@ -225,11 +226,18 @@ func (s *Service) CreateCluster(ctx context.Context, input *ecs.CreateClusterInp
 			return nil, err
 		}
 	}
-	return &ecs.CreateClusterOutput{Cluster: rec.toAWS()}, nil
+	c := rec.toAWS()
+	if found {
+		if _, err := s.applyClusterCounts(ctx, kv, name, c); err != nil {
+			return nil, err
+		}
+	}
+	return &ecs.CreateClusterOutput{Cluster: c}, nil
 }
 
 // DescribeClusters returns meta for the named clusters; unknown names surface
-// as MISSING failures. Settings are only returned under Include=SETTINGS.
+// as MISSING failures. Settings, tags, statistics and attachments are only
+// filled under the matching Include value.
 func (s *Service) DescribeClusters(ctx context.Context, input *ecs.DescribeClustersInput, accountID string) (*ecs.DescribeClustersOutput, error) {
 	kv, err := s.bucket(ctx, accountID)
 	if err != nil {
@@ -239,7 +247,7 @@ func (s *Service) DescribeClusters(ctx context.Context, input *ecs.DescribeClust
 	if len(names) == 0 {
 		names = []string{defaultCluster}
 	}
-	includeSettings := slices.Contains(awsStringSlice(input.Include), ecs.ClusterFieldSettings)
+	include := awsStringSlice(input.Include)
 	out := &ecs.DescribeClustersOutput{Clusters: []*ecs.Cluster{}, Failures: []*ecs.Failure{}}
 	for _, name := range names {
 		name = ClusterShortName(name)
@@ -253,55 +261,99 @@ func (s *Service) DescribeClusters(ctx context.Context, input *ecs.DescribeClust
 			continue
 		}
 		c := rec.toAWS()
-		if !includeSettings {
+		if !slices.Contains(include, ecs.ClusterFieldSettings) {
 			c.Settings = nil
 		}
-		if err := s.applyClusterCounts(ctx, kv, name, c); err != nil {
+		if !slices.Contains(include, ecs.ClusterFieldTags) {
+			c.Tags = []*ecs.Tag{}
+		}
+		if slices.Contains(include, ecs.ClusterFieldAttachments) {
+			c.Attachments = []*ecs.Attachment{}
+		}
+		counts, err := s.applyClusterCounts(ctx, kv, name, c)
+		if err != nil {
 			return nil, err
 		}
+		if slices.Contains(include, ecs.ClusterFieldStatistics) {
+			c.Statistics = counts.statistics()
+		}
 		out.Clusters = append(out.Clusters, c)
 	}
 	return out, nil
 }
 
+// clusterCounts are a cluster's tallies, all EC2 launch type.
+type clusterCounts struct {
+	running, pending, activeServices int64
+}
+
+// statistics renders the Include=STATISTICS pairs in the order AWS returns
+// them; Spinifex runs no Fargate, External or ManagedInstances work.
+func (c clusterCounts) statistics() []*ecs.KeyValuePair {
+	pairs := []struct {
+		name  string
+		value int64
+	}{
+		{"runningEC2TasksCount", c.running},
+		{"runningFargateTasksCount", 0},
+		{"pendingEC2TasksCount", c.pending},
+		{"pendingFargateTasksCount", 0},
+		{"runningExternalTasksCount", 0},
+		{"pendingExternalTasksCount", 0},
+		{"runningManagedInstancesTasksCount", 0},
+		{"pendingManagedInstancesTasksCount", 0},
+		{"activeEC2ServiceCount", c.activeServices},
+		{"activeFargateServiceCount", 0},
+		{"drainingEC2ServiceCount", 0},
+		{"drainingFargateServiceCount", 0},
+		{"activeExternalServiceCount", 0},
+		{"drainingExternalServiceCount", 0},
+		{"activeManagedInstancesServiceCount", 0},
+		{"drainingManagedInstancesServiceCount", 0},
+	}
+	out := make([]*ecs.KeyValuePair, 0, len(pairs))
+	for _, p := range pairs {
+		out = append(out, &ecs.KeyValuePair{Name: aws.String(p.name), Value: aws.String(strconv.FormatInt(p.value, 10))})
+	}
+	return out
+}
+
 // applyClusterCounts fills the four count fields AWS always returns on a
-// cluster, tallied at read time from the existing listers. The separate
-// Statistics key/value list, which Include=STATISTICS adds, stays unset.
-func (s *Service) applyClusterCounts(ctx context.Context, kv jetstream.KeyValue, cluster string, c *ecs.Cluster) error {
+// cluster, tallied at read time from the existing listers.
+func (s *Service) applyClusterCounts(ctx context.Context, kv jetstream.KeyValue, cluster string, c *ecs.Cluster) (clusterCounts, error) {
+	var counts clusterCounts
 	instances, err := s.listInstanceRecords(ctx, kv, cluster)
 	if err != nil {
-		return err
+		return counts, err
 	}
 	c.RegisteredContainerInstancesCount = aws.Int64(int64(len(instances)))
 
 	tasks, err := s.listTaskRecords(ctx, kv, cluster)
 	if err != nil {
-		return err
+		return counts, err
 	}
-	var running, pending int64
 	for i := range tasks {
 		switch tasks[i].LastStatus {
 		case TaskStatusRunning:
-			running++
+			counts.running++
 		case TaskStatusPending:
-			pending++
+			counts.pending++
 		}
 	}
-	c.RunningTasksCount = aws.Int64(running)
-	c.PendingTasksCount = aws.Int64(pending)
+	c.RunningTasksCount = aws.Int64(counts.running)
+	c.PendingTasksCount = aws.Int64(counts.pending)
 
 	services, err := s.listServiceRecords(ctx, kv, cluster)
 	if err != nil {
-		return err
+		return counts, err
 	}
-	var active int64
 	for i := range services {
 		if services[i].Status == ServiceStatusActive {
-			active++
+			counts.activeServices++
 		}
 	}
-	c.ActiveServicesCount = aws.Int64(active)
-	return nil
+	c.ActiveServicesCount = aws.Int64(counts.activeServices)
+	return counts, nil
 }
 
 // ListClusters returns the ARNs of all clusters in the account.
@@ -331,15 +383,24 @@ func (s *Service) ListClusters(ctx context.Context, _ *ecs.ListClustersInput, ac
 	return &ecs.ListClustersOutput{ClusterArns: aws.StringSlice(arns)}, nil
 }
 
+// toAWS renders the stored cluster with the counts zeroed and every list AWS
+// always returns present, so an empty one marshals as [] rather than vanishing.
 func (r *ClusterRecord) toAWS() *ecs.Cluster {
 	c := &ecs.Cluster{
-		ClusterName: aws.String(r.Name),
-		ClusterArn:  aws.String(r.ARN),
-		Status:      aws.String(r.Status),
-		Tags:        tagsToAWS(r.Tags),
+		ClusterName:                       aws.String(r.Name),
+		ClusterArn:                        aws.String(r.ARN),
+		Status:                            aws.String(r.Status),
+		Tags:                              tagsToAWS(r.Tags),
+		RegisteredContainerInstancesCount: aws.Int64(0),
+		RunningTasksCount:                 aws.Int64(0),
+		PendingTasksCount:                 aws.Int64(0),
+		ActiveServicesCount:               aws.Int64(0),
+		Statistics:                        []*ecs.KeyValuePair{},
+		CapacityProviders:                 aws.StringSlice(r.CapacityProviders),
+		DefaultCapacityProviderStrategy:   make([]*ecs.CapacityProviderStrategyItem, 0, len(r.DefaultCapacityProviderStrategy)),
 	}
-	if len(r.CapacityProviders) > 0 {
-		c.CapacityProviders = aws.StringSlice(r.CapacityProviders)
+	if c.Tags == nil {
+		c.Tags = []*ecs.Tag{}
 	}
 	for _, item := range r.DefaultCapacityProviderStrategy {
 		c.DefaultCapacityProviderStrategy = append(c.DefaultCapacityProviderStrategy, item.toAWS())

spinifex/domains/ecs/service_test.go

@@ -108,7 +108,7 @@ func TestService_CreateCluster_RejectsUnsupportedSetting(t *testing.T) {
 	assert.Empty(t, list.ClusterArns)
 }
 
-func TestService_DescribeClusters_ReturnsTags(t *testing.T) {
+func TestService_DescribeClusters_ReturnsTagsUnderInclude(t *testing.T) {
 	svc, _ := newTestService(t)
 	_, err := svc.CreateCluster(context.Background(), &ecs.CreateClusterInput{
 		ClusterName: aws.String("web"),
@@ -119,10 +119,18 @@ func TestService_DescribeClusters_ReturnsTags(t *testing.T) {
 	}, testAccountID)
 	require.NoError(t, err)
 
-	out, err := svc.DescribeClusters(context.Background(), &ecs.DescribeClustersInput{
+	plain, err := svc.DescribeClusters(context.Background(), &ecs.DescribeClustersInput{
 		Clusters: []*string{aws.String("web")},
 	}, testAccountID)
 	require.NoError(t, err)
+	require.Len(t, plain.Clusters, 1)
+	assert.NotNil(t, plain.Clusters[0].Tags)
+	assert.Empty(t, plain.Clusters[0].Tags, "tags are only returned under Include=TAGS")
+
+	out, err := svc.DescribeClusters(context.Background(), &ecs.DescribeClustersInput{
+		Clusters: []*string{aws.String("web")}, Include: aws.StringSlice([]string{ecs.ClusterFieldTags}),
+	}, testAccountID)
+	require.NoError(t, err)
 	require.Len(t, out.Clusters, 1)
 
 	tags := out.Clusters[0].Tags
@@ -865,6 +873,109 @@ func TestService_DescribeClusters_CountsAlwaysPresent(t *testing.T) {
 	assertCounts(t, stats.Clusters[0])
 }
 
+// TestService_DescribeClusters_IncludeStatisticsAndAttachments checks the
+// sixteen string pairs AWS returns under Include=STATISTICS, in AWS's order,
+// and that ATTACHMENTS adds an empty attachments list.
+func TestService_DescribeClusters_IncludeStatisticsAndAttachments(t *testing.T) {
+	svc, _, _ := serviceTestRig(t)
+	_, err := svc.CreateService(context.Background(), &ecs.CreateServiceInput{
+		Cluster: aws.String("web"), ServiceName: aws.String("web"), TaskDefinition: aws.String("app"),
+		DesiredCount: aws.Int64(1),
+	}, testAccountID)
+	require.NoError(t, err)
+
+	plain, err := svc.DescribeClusters(context.Background(), &ecs.DescribeClustersInput{
+		Clusters: []*string{aws.String("web")},
+	}, testAccountID)
+	require.NoError(t, err)
+	require.Len(t, plain.Clusters, 1)
+	assert.NotNil(t, plain.Clusters[0].Statistics)
+	assert.Empty(t, plain.Clusters[0].Statistics)
+	assert.Nil(t, plain.Clusters[0].Attachments)
+
+	out, err := svc.DescribeClusters(context.Background(), &ecs.DescribeClustersInput{
+		Clusters: []*string{aws.String("web")},
+		Include:  aws.StringSlice([]string{ecs.ClusterFieldStatistics, ecs.ClusterFieldAttachments}),
+	}, testAccountID)
+	require.NoError(t, err)
+	require.Len(t, out.Clusters, 1)
+	c := out.Clusters[0]
+	assert.NotNil(t, c.Attachments)
+	assert.Empty(t, c.Attachments)
+
+	want := [][2]string{
+		{"runningEC2TasksCount", "0"},
+		{"runningFargateTasksCount", "0"},
+		{"pendingEC2TasksCount", "1"},
+		{"pendingFargateTasksCount", "0"},
+		{"runningExternalTasksCount", "0"},
+		{"pendingExternalTasksCount", "0"},
+		{"runningManagedInstancesTasksCount", "0"},
+		{"pendingManagedInstancesTasksCount", "0"},
+		{"activeEC2ServiceCount", "1"},
+		{"activeFargateServiceCount", "0"},
+		{"drainingEC2ServiceCount", "0"},
+		{"drainingFargateServiceCount", "0"},
+		{"activeExternalServiceCount", "0"},
+		{"drainingExternalServiceCount", "0"},
+		{"activeManagedInstancesServiceCount", "0"},
+		{"drainingManagedInstancesServiceCount", "0"},
+	}
+	got := make([][2]string, 0, len(c.Statistics))
+	for _, kv := range c.Statistics {
+		got = append(got, [2]string{aws.StringValue(kv.Name), aws.StringValue(kv.Value)})
+	}
+	assert.Equal(t, want, got)
+}
+
+// TestService_ClusterResponses_AlwaysCarryCountsAndLists checks every cluster
+// response carries the four counts and the statistics and capacity-provider
+// lists as present empty lists, as AWS does, so they marshal as [].
+func TestService_ClusterResponses_AlwaysCarryCountsAndLists(t *testing.T) {
+	svc, _ := newTestService(t)
+	ctx := context.Background()
+	assertShape := func(t *testing.T, op string, c *ecs.Cluster) {
+		t.Helper()
+		for name, v := range map[string]*int64{
+			"registeredContainerInstancesCount": c.RegisteredContainerInstancesCount,
+			"runningTasksCount":                 c.RunningTasksCount,
+			"pendingTasksCount":                 c.PendingTasksCount,
+			"activeServicesCount":               c.ActiveServicesCount,
+		} {
+			assert.NotNil(t, v, "%s: %s", op, name)
+		}
+		assert.NotNil(t, c.Statistics, "%s: statistics", op)
+		assert.NotNil(t, c.CapacityProviders, "%s: capacityProviders", op)
+		assert.NotNil(t, c.DefaultCapacityProviderStrategy, "%s: defaultCapacityProviderStrategy", op)
+		assert.NotNil(t, c.Tags, "%s: tags", op)
+	}
+
+	created, err := svc.CreateCluster(ctx, &ecs.CreateClusterInput{
+		ClusterName: aws.String("web"), Tags: []*ecs.Tag{{Key: aws.String("team"), Value: aws.String("infra")}},
+	}, testAccountID)
+	require.NoError(t, err)
+	assertShape(t, "CreateCluster", created.Cluster)
+	assert.Len(t, created.Cluster.Tags, 1, "CreateCluster echoes the tags")
+
+	described, err := svc.DescribeClusters(ctx, &ecs.DescribeClustersInput{Clusters: aws.StringSlice([]string{"web"})}, testAccountID)
+	require.NoError(t, err)
+	require.Len(t, described.Clusters, 1)
+	assertShape(t, "DescribeClusters", described.Clusters[0])
+
+	put, err := svc.PutClusterCapacityProviders(ctx, &ecs.PutClusterCapacityProvidersInput{
+		Cluster: aws.String("web"), CapacityProviders: []*string{}, DefaultCapacityProviderStrategy: []*ecs.CapacityProviderStrategyItem{},
+	}, testAccountID)
+	require.NoError(t, err)
+	assertShape(t, "PutClusterCapacityProviders", put.Cluster)
+	assert.Empty(t, put.Cluster.Tags)
+	assert.NotNil(t, put.Cluster.Attachments)
+
+	deleted, err := svc.DeleteCluster(ctx, &ecs.DeleteClusterInput{Cluster: aws.String("web")}, testAccountID)
+	require.NoError(t, err)
+	assertShape(t, "DeleteCluster", deleted.Cluster)
+	assert.Empty(t, deleted.Cluster.Tags)
+}
+
 // The following cover the ECS list operations that must return the ARNs key
 // as a present, empty list rather than omit it: the SDK's JSON marshaller
 // drops a nil slice unconditionally, regardless of the "omitempty" AWS itself

spinifex/domains/eks/awsapi/registration.go

@@ -38,7 +38,7 @@ func NewRegistration(deps Deps) dispatch.Registration {
 
 // OperationInventory classifies the actions the registration serves.
 func OperationInventory() dispatch.Inventory {
-	return dispatch.Inventory{Registered: actionNames()}
+	return dispatch.Inventory{Registered: router.Actions()}
 }
 
 func resolveAction(r *http.Request) string {

spinifex/domains/eks/awsapi/routes.go

@@ -2,7 +2,6 @@ package awsapi
 
 import (
 	"context"
-	"slices"
 
 	"github.com/mulgadc/spinifex/spinifex/ingress/aws/rest"
 	"github.com/nats-io/nats.go"
@@ -179,16 +178,5 @@ var routes = []route{
 		}},
 }
 
-// actionNames returns the distinct actions in routes in stable order.
-// Deduplicated because an action may be reachable by more than one route.
-func actionNames() []string {
-	names := make([]string, 0, len(routes))
-	for _, route := range routes {
-		names = append(names, route.Action)
-	}
-	slices.Sort(names)
-	return slices.Compact(names)
-}
-
 // router matches an escaped request path against routes.
 var router = rest.NewRouter("eks", routes)

spinifex/domains/eks/awsapi/routes_test.go

@@ -1,7 +1,6 @@
 package awsapi
 
 import (
-	"slices"
 	"strings"
 	"testing"
 
@@ -224,10 +223,3 @@ func TestRESTRouter_NoDuplicateEKSRoutes(t *testing.T) {
 		seen[key] = route.Action
 	}
 }
-
-func TestActionNamesAreSortedAndDistinct(t *testing.T) {
-	names := actionNames()
-
-	require.True(t, slices.IsSorted(names), "actions are not sorted: %v", names)
-	require.Len(t, slices.Compact(slices.Clone(names)), len(names), "actions contain duplicates: %v", names)
-}

spinifex/domains/eks/cluster/cluster_reconciler.go

@@ -795,8 +795,7 @@ func retryableStartRefusal(err error) bool {
 	if code, ok := awserrors.ResolveErrorCode(err); ok && code == awserrors.ErrorInsufficientInstanceCapacity {
 		return true
 	}
-	_, message, _ := awserrors.ResolveErrorDetail(err)
-	return strings.Contains(message, ec2v1.VolumeHeldElsewhereText) || strings.Contains(err.Error(), ec2v1.VolumeHeldElsewhereText)
+	return errors.Is(err, ec2v1.ErrVolumeHeldElsewhere)
 }
 
 // maybeReformEtcdQuorum escalates a wedged HA control plane that the in-place

spinifex/domains/eks/cluster/cluster_reconciler_start_refusal_test.go

@@ -3,9 +3,11 @@ package cluster
 import (
 	"context"
 	"errors"
+	"fmt"
 	"testing"
 	"time"
 
+	"github.com/mulgadc/spinifex/contracts/ec2/v1"
 	"github.com/mulgadc/spinifex/spinifex/foundation/aws/errors"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -30,12 +32,12 @@ func (c *refusingCPControl) StartInstance(context.Context, string) error {
 
 func (c *refusingCPControl) StopInstance(context.Context, string) error { return nil }
 
-// What the start path returns after crossing NATS: the code and the message the
-// owning node sent, with no sentinel left to match on.
+// What the start path returns after crossing NATS: StartInstances wraps the
+// sentinel back on when the owner's reply carries the held-elsewhere header.
 var (
 	errStartCapacity = errors.New(awserrors.ErrorInsufficientInstanceCapacity)
-	errStartHeld     = awserrors.Errorf(awserrors.ErrorIncorrectState, "%s",
-		"volume is leased by another owner: held by node-a since 2026-10-30T01:00:00Z")
+	errStartHeld     = fmt.Errorf("%w: %w", ec2v1.ErrVolumeHeldElsewhere,
+		awserrors.Errorf(awserrors.ErrorIncorrectState, "%s", "volume vol-1 is published to node-a"))
 )
 
 // A refusal that clears by itself must not use up the restart budget, or the
@@ -72,18 +74,30 @@ func TestMaybeRecoverCP_RetryableStartRefusalIsRetriedNotReplaced(t *testing.T)
 	}
 }
 
-// The cap still bounds the faults nothing says will clear.
+// The cap still bounds the faults nothing says will clear. IncorrectState
+// without the held-elsewhere sentinel is one, whatever its message says.
 func TestMaybeRecoverCP_UnknownStartFailureStillCountsAgainstTheBudget(t *testing.T) {
 	t.Parallel()
-	cp := &refusingCPControl{err: errors.New(awserrors.ErrorServerInternal)}
-	r := newRecoveryReconciler(t, cp)
-	r.degradedSince = time.Now().Add(-10 * time.Minute)
+	for name, refusal := range map[string]error{
+		"server internal": errors.New(awserrors.ErrorServerInternal),
+		"unrelated incorrect state": awserrors.Errorf(awserrors.ErrorIncorrectState, "%s",
+			"instance i-cp is not in a state from which it can be started"),
+		"lease text without the sentinel": awserrors.Errorf(awserrors.ErrorIncorrectState, "%s",
+			ec2v1.VolumeHeldElsewhereText+": held by node-a since 2026-10-30T01:00:00Z"),
+	} {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+			cp := &refusingCPControl{err: refusal}
+			r := newRecoveryReconciler(t, cp)
+			r.degradedSince = time.Now().Add(-10 * time.Minute)
 
-	for range r.maxRestartAttempts + 2 {
-		r.lastRestartAt = time.Time{}
-		r.maybeRecoverControlPlane(t.Context(), metaWithCP("i-cp"), "stale")
+			for range r.maxRestartAttempts + 2 {
+				r.lastRestartAt = time.Time{}
+				r.maybeRecoverControlPlane(t.Context(), metaWithCP("i-cp"), "stale")
+			}
+
+			require.Equal(t, r.maxRestartAttempts, cp.startCalls, "stops at the attempt cap")
+			assert.Equal(t, r.maxRestartAttempts, r.restartAttempts)
+		})
 	}
-
-	require.Equal(t, r.maxRestartAttempts, cp.startCalls, "stops at the attempt cap")
-	assert.Equal(t, r.maxRestartAttempts, r.restartAttempts)
 }

spinifex/domains/eks/cluster/nodegroup.go

@@ -95,7 +95,7 @@ const ngCASMaxRetries = 16
 var ErrNodegroupNotFound = errors.New("eks: nodegroup not found")
 
 // nodegroupNotFound is the AWS answer for a missing node group on an existing
-// cluster; a missing cluster keeps the code's default cluster message.
+// cluster; a missing cluster keeps the code's default resource message.
 func nodegroupNotFound(ng string) error {
 	return awserrors.Errorf(awserrors.ErrorEKSResourceNotFound, "No node group found for name: %s.", ng)
 }

spinifex/domains/eks/cluster/nodegroup_delete_test.go

@@ -176,7 +176,9 @@ func TestDeleteNodegroup_ReaperEndsWorkersThenRemovesGroup(t *testing.T) {
 	require.Len(t, f.worker.terminateCalls, 1)
 	assert.ElementsMatch(t, rec.InstanceIDs, f.worker.terminateCalls[0])
 	_, err = describeNodegroupCall(f, "c1", "ng1")
-	requireErrorCode(t, err, awserrors.ErrorEKSResourceNotFound)
+	code, msg, _ := awserrors.ResolveErrorDetail(err)
+	assert.Equal(t, awserrors.ErrorEKSResourceNotFound, code)
+	assert.Equal(t, "No node group found for name: ng1.", msg, "the live cluster is not reported missing")
 	_, ok := fixtureIAM(t, f).Profiles[rec.InstanceProfileName]
 	assert.False(t, ok, "the group is not gone until its profile is")
 }

spinifex/domains/eks/cluster/nodegroup_test.go

@@ -1433,7 +1433,7 @@ func TestNodegroupCalls_MissingNodegroupVsMissingCluster(t *testing.T) {
 			code, msg, ok := awserrors.ResolveErrorDetail(call("nope"))
 			require.True(t, ok)
 			assert.Equal(t, awserrors.ErrorEKSResourceNotFound, code)
-			assert.Empty(t, msg, "a missing cluster keeps the default cluster message")
+			assert.Empty(t, msg, "a missing cluster keeps the default resource message")
 		})
 	}
 }

spinifex/domains/elbv2/service_impl.go

@@ -1230,10 +1230,11 @@ func certFilesToSDK(certFiles map[string]string) []*CertFile {
 }
 
 // resolveTargetIP looks up the private IP for an instance by finding its primary ENI.
-// Returns empty string if VPC service is unavailable or no ENI found.
-func (s *ELBv2ServiceImpl) resolveTargetIP(ctx context.Context, instanceID, accountID string) string {
+// found is false when no ENI in the account is attached to the instance, which
+// is how a non-existent instance shows; without a VPC service nothing is checked.
+func (s *ELBv2ServiceImpl) resolveTargetIP(ctx context.Context, instanceID, accountID string) (ip string, found bool, err error) {
 	if s.VPCService == nil {
-		return ""
+		return "", true, nil
 	}
 
 	result, err := s.VPCService.DescribeNetworkInterfaces(ctx, &ec2.DescribeNetworkInterfacesInput{
@@ -1241,35 +1242,35 @@ func (s *ELBv2ServiceImpl) resolveTargetIP(ctx context.Context, instanceID, acco
 			{Name: aws.String("attachment.instance-id"), Values: []*string{aws.String(instanceID)}},
 		},
 	}, accountID)
-	if err != nil || len(result.NetworkInterfaces) == 0 {
-		slog.WarnContext(ctx, "Could not resolve target IP — target will not receive traffic until ENI is attached", "instanceId", instanceID, "err", err)
-		return ""
+	if err != nil {
+		slog.ErrorContext(ctx, "RegisterTargets: ENI lookup failed", "instanceId", instanceID, "err", err)
+		return "", false, errors.New(awserrors.ErrorServerInternal)
 	}
-
-	// Use the first (primary) ENI's private IP
-	if result.NetworkInterfaces[0].PrivateIpAddress != nil {
-		return *result.NetworkInterfaces[0].PrivateIpAddress
+	// The filter matches wildcards, so require the attachment to name this exact instance.
+	if len(result.NetworkInterfaces) == 0 || result.NetworkInterfaces[0].Attachment == nil ||
+		aws.StringValue(result.NetworkInterfaces[0].Attachment.InstanceId) != instanceID {
+		return "", false, nil
 	}
-	return ""
+	return aws.StringValue(result.NetworkInterfaces[0].PrivateIpAddress), true, nil
 }
 
 // resolveRegisteredTargetIP returns the private IP for a target being registered.
 // For ip-type targets the ID is the IP; for instance targets it is resolved via
-// the primary ENI. Returns an error if the ID doesn't match the target type.
-func (s *ELBv2ServiceImpl) resolveRegisteredTargetIP(ctx context.Context, targetType, id, accountID string) (string, error) {
+// the primary ENI, and found is false when the instance does not exist.
+func (s *ELBv2ServiceImpl) resolveRegisteredTargetIP(ctx context.Context, targetType, id, accountID string) (ip string, found bool, err error) {
 	switch targetType {
 	case TargetTypeIP:
 		if net.ParseIP(id) == nil {
 			slog.WarnContext(ctx, "RegisterTargets: ip target id is not a valid IP", "id", id)
-			return "", errors.New(awserrors.ErrorInvalidParameterValue)
+			return "", false, errors.New(awserrors.ErrorInvalidParameterValue)
 		}
-		return id, nil
+		return id, true, nil
 	default: // instance
 		if net.ParseIP(id) != nil {
 			slog.WarnContext(ctx, "RegisterTargets: instance target group given an IP address", "id", id)
-			return "", errors.New(awserrors.ErrorInvalidParameterValue)
+			return "", false, errors.New(awserrors.ErrorInvalidParameterValue)
 		}
-		return s.resolveTargetIP(ctx, id, accountID), nil
+		return s.resolveTargetIP(ctx, id, accountID)
 	}
 }
 
@@ -1342,6 +1343,11 @@ func (s *ELBv2ServiceImpl) createLoadBalancer(ctx context.Context, input *elbv2.
 
 	name := *input.Name
 
+	tags, err := tagsFromSDK(input.Tags)
+	if err != nil {
+		return nil, err
+	}
+
 	// Check for duplicate name
 	existing, err := s.store.GetLoadBalancerByName(ctx, name, accountID)
 	if err != nil {
@@ -1414,8 +1420,6 @@ func (s *ELBv2ServiceImpl) createLoadBalancer(ctx context.Context, input *elbv2.
 		}
 	}
 
-	tags := tagsFromSDK(input.Tags)
-
 	// Create ENIs in each subnet (when VPC service is available)
 	var eniIDs []string
 	var availZones []AvailZoneInfo
@@ -1972,7 +1976,16 @@ func (s *ELBv2ServiceImpl) DescribeLoadBalancers(ctx context.Context, input *elb
 		result = append(result, s.lbRecordToSDK(ctx, lb))
 	}
 
-	// AWS fails the whole call when any requested name is unknown.
+	// AWS fails the whole call when any requested ARN or name is unknown.
+	if len(arnFilter) > 0 {
+		for _, lb := range result {
+			delete(arnFilter, aws.StringValue(lb.LoadBalancerArn))
+		}
+		if len(arnFilter) > 0 {
+			return nil, awserrors.Errorf(awserrors.ErrorELBv2LoadBalancerNotFound,
+				"One or more load balancers not found")
+		}
+	}
 	if len(nameFilter) > 0 {
 		for _, lb := range result {
 			delete(nameFilter, aws.StringValue(lb.LoadBalancerName))
@@ -2128,7 +2141,10 @@ func (s *ELBv2ServiceImpl) CreateTargetGroup(ctx context.Context, input *elbv2.C
 	tgID := awsidentifiers.GenerateResourceID("tg")
 	tgArn := resourcearn.FormatELBv2TargetGroup(s.region, accountID, name, tgID)
 
-	tags := tagsFromSDK(input.Tags)
+	tags, err := tagsFromSDK(input.Tags)
+	if err != nil {
+		return nil, err
+	}
 
 	record := &TargetGroupRecord{
 		TargetGroupArn:  tgArn,
@@ -2454,6 +2470,7 @@ func (s *ELBv2ServiceImpl) RegisterTargets(ctx context.Context, input *elbv2.Reg
 		registered[targetKey(t.Id, t.Port)] = true
 	}
 	pending := make([]Target, 0, len(input.Targets))
+	var invalid []string
 	for _, td := range input.Targets {
 		if td.Id == nil {
 			continue
@@ -2465,10 +2482,14 @@ func (s *ELBv2ServiceImpl) RegisterTargets(ctx context.Context, input *elbv2.Reg
 		if registered[targetKey(*td.Id, port)] {
 			continue // Already registered
 		}
-		privateIP, err := s.resolveRegisteredTargetIP(ctx, tg.TargetType, *td.Id, accountID)
+		privateIP, found, err := s.resolveRegisteredTargetIP(ctx, tg.TargetType, *td.Id, accountID)
 		if err != nil {
 			return nil, err
 		}
+		if !found {
+			invalid = append(invalid, "'"+*td.Id+"'")
+			continue
+		}
 		pending = append(pending, Target{
 			Id:          *td.Id,
 			Port:        port,
@@ -2478,6 +2499,10 @@ func (s *ELBv2ServiceImpl) RegisterTargets(ctx context.Context, input *elbv2.Reg
 		})
 	}
 
+	if len(invalid) > 0 {
+		return nil, awserrors.Errorf(awserrors.ErrorValidationError, "The following targets are not valid instances: %s", strings.Join(invalid, ", "))
+	}
+
 	// The append runs against the record as committed, not the copy read above,
 	// so two concurrent registrations on one group both land.
 	if _, err := s.updateTargetGroupCAS(ctx, tgArn, accountID, "RegisterTargets", func(rec *TargetGroupRecord) (bool, error) {
@@ -2912,7 +2937,10 @@ func (s *ELBv2ServiceImpl) CreateListener(ctx context.Context, input *elbv2.Crea
 		actions = append(actions, action)
 	}
 
-	tags := tagsFromSDK(input.Tags)
+	tags, err := tagsFromSDK(input.Tags)
+	if err != nil {
+		return nil, err
+	}
 
 	record := &ListenerRecord{
 		ListenerArn:     listenerArn,
@@ -3329,15 +3357,57 @@ func (s *ELBv2ServiceImpl) DescribeTags(ctx context.Context, input *elbv2.Descri
 	}, nil
 }
 
-// tagsFromSDK converts an SDK Tag slice into a tag map, skipping nil key/value entries.
-func tagsFromSDK(tags []*elbv2.Tag) map[string]string {
-	m := make(map[string]string, len(tags))
-	for _, tag := range tags {
-		if tag.Key != nil && tag.Value != nil {
-			m[*tag.Key] = *tag.Value
-		}
+// maxResourceTags is AWS's per-resource tag limit for ELBv2 resources.
+const maxResourceTags = 50
+
+// tagCharsPattern is the ELBv2 model's TagKey and TagValue pattern.
+var tagCharsPattern = regexp.MustCompile(`^([\p{L}\p{Z}\p{N}_.:/=+\-@]*)$`)
+
+// tagsFromSDK validates tags against the ELBv2 model and converts them into a
+// tag map, storing a missing Value as "".
+func tagsFromSDK(tags []*elbv2.Tag) (map[string]string, error) {
+	if tags != nil && len(tags) == 0 {
+		return nil, awserrors.Errorf(awserrors.ErrorValidationError, "1 validation error detected: Value at 'tags' failed to satisfy constraint: Member must have length greater than or equal to 1")
 	}
-	return m
+	m := make(map[string]string, len(tags))
+	for i, tag := range tags {
+		if tag == nil || tag.Key == nil {
+			return nil, awserrors.Errorf(awserrors.ErrorValidationError, "1 validation error detected: Value null at 'tags.%d.member.key' failed to satisfy constraint: Member must not be null", i+1)
+		}
+		if err := validateTagString(*tag.Key, fmt.Sprintf("tags.%d.member.key", i+1), 1, 128); err != nil {
+			return nil, err
+		}
+		v := aws.StringValue(tag.Value)
+		if err := validateTagString(v, fmt.Sprintf("tags.%d.member.value", i+1), 0, 256); err != nil {
+			return nil, err
+		}
+		if _, dup := m[*tag.Key]; dup {
+			return nil, errors.New(awserrors.ErrorELBv2DuplicateTagKeys)
+		}
+		m[*tag.Key] = v
+	}
+	if len(m) > maxResourceTags {
+		return nil, errTooManyTags()
+	}
+	return m, nil
+}
+
+func validateTagString(v, member string, minLen, maxLen int) error {
+	n := utf8.RuneCountInString(v)
+	if n < minLen {
+		return awserrors.Errorf(awserrors.ErrorValidationError, "1 validation error detected: Value at '%s' failed to satisfy constraint: Member must have length greater than or equal to %d", member, minLen)
+	}
+	if n > maxLen {
+		return awserrors.Errorf(awserrors.ErrorValidationError, "1 validation error detected: Value at '%s' failed to satisfy constraint: Member must have length less than or equal to %d", member, maxLen)
+	}
+	if !tagCharsPattern.MatchString(v) {
+		return awserrors.Errorf(awserrors.ErrorValidationError, "1 validation error detected: Value at '%s' failed to satisfy constraint: Member must satisfy regular expression pattern: %s", member, tagCharsPattern.String())
+	}
+	return nil
+}
+
+func errTooManyTags() error {
+	return awserrors.Errorf(awserrors.ErrorELBv2TooManyTags, "You've reached the limit on the number of tags for this resource.")
 }
 
 // tagsMapToSDK converts a tag map into a sorted SDK Tag slice; returns nil for empty input.

spinifex/domains/elbv2/service_impl_health_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/mulgadc/spinifex/internal/testkit"
 	ec2vpc "github.com/mulgadc/spinifex/spinifex/domains/ec2/vpc"
 	iamdomain "github.com/mulgadc/spinifex/spinifex/domains/iam"
+	awserrors "github.com/mulgadc/spinifex/spinifex/foundation/aws/errors"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -91,26 +92,66 @@ func TestRegisterTargets_ResolvesPrivateIP(t *testing.T) {
 	assert.Equal(t, "10.0.1.50", tg.Targets[0].PrivateIP)
 }
 
-func TestRegisterTargets_UnresolvableIP(t *testing.T) {
+func TestRegisterTargets_UnknownInstanceFailsWholeCall(t *testing.T) {
 	t.Parallel()
 	svc, _, _ := setupTestServiceWithInstance(t, "i-web001", "10.0.1.50")
 
-	tgOut, _ := svc.CreateTargetGroup(context.Background(), &elbv2.CreateTargetGroupInput{
-		Name: aws.String("unresolvable-tg"),
+	tgOut, err := svc.CreateTargetGroup(context.Background(), &elbv2.CreateTargetGroupInput{
+		Name: aws.String("unknown-instance-tg"),
 		Port: aws.Int64(80),
 	}, testAccountID)
+	require.NoError(t, err)
 	tgArn := tgOut.TargetGroups[0].TargetGroupArn
 
-	// Register an instance that doesn't exist — should still succeed with empty IP
-	_, err := svc.RegisterTargets(context.Background(), &elbv2.RegisterTargetsInput{
-		TargetGroupArn: tgArn,
-		Targets:        []*elbv2.TargetDescription{{Id: aws.String("i-nonexistent")}},
+	for _, ids := range [][]string{{"i-nonexistent"}, {"i-web001", "i-nonexistent"}, {"i-web00?"}} {
+		targets := make([]*elbv2.TargetDescription, 0, len(ids))
+		for _, id := range ids {
+			targets = append(targets, &elbv2.TargetDescription{Id: aws.String(id)})
+		}
+		_, err = svc.RegisterTargets(context.Background(), &elbv2.RegisterTargetsInput{
+			TargetGroupArn: tgArn,
+			Targets:        targets,
+		}, testAccountID)
+		code, msg, ok := awserrors.ResolveErrorDetail(err)
+		require.True(t, ok, "ids %v: %v", ids, err)
+		assert.Equal(t, awserrors.ErrorValidationError, code)
+		assert.Equal(t, "The following targets are not valid instances: '"+ids[len(ids)-1]+"'", msg)
+
+		tg, err := svc.store.GetTargetGroupByArn(t.Context(), *tgArn)
+		require.NoError(t, err)
+		assert.Empty(t, tg.Targets, "ids %v", ids)
+	}
+}
+
+func TestRegisterTargets_ReregisterSkipsInstanceLookup(t *testing.T) {
+	t.Parallel()
+	svc, vpcSvc, _ := setupTestServiceWithInstance(t, "i-web001", "10.0.1.50")
+
+	tgOut, err := svc.CreateTargetGroup(context.Background(), &elbv2.CreateTargetGroupInput{
+		Name: aws.String("reregister-tg"),
+		Port: aws.Int64(80),
 	}, testAccountID)
 	require.NoError(t, err)
+	tgArn := tgOut.TargetGroups[0].TargetGroupArn
+	in := &elbv2.RegisterTargetsInput{
+		TargetGroupArn: tgArn,
+		Targets:        []*elbv2.TargetDescription{{Id: aws.String("i-web001")}},
+	}
+	_, err = svc.RegisterTargets(context.Background(), in, testAccountID)
+	require.NoError(t, err)
 
-	tg, _ := svc.store.GetTargetGroupByArn(t.Context(), *tgArn)
+	enis, err := vpcSvc.DescribeNetworkInterfaces(context.Background(), &ec2.DescribeNetworkInterfacesInput{}, testAccountID)
+	require.NoError(t, err)
+	require.Len(t, enis.NetworkInterfaces, 1)
+	require.NoError(t, vpcSvc.DetachENI(context.Background(), testAccountID, *enis.NetworkInterfaces[0].NetworkInterfaceId))
+
+	_, err = svc.RegisterTargets(context.Background(), in, testAccountID)
+	require.NoError(t, err)
+
+	tg, err := svc.store.GetTargetGroupByArn(t.Context(), *tgArn)
+	require.NoError(t, err)
 	require.Len(t, tg.Targets, 1)
-	assert.Empty(t, tg.Targets[0].PrivateIP)
+	assert.Equal(t, "10.0.1.50", tg.Targets[0].PrivateIP)
 }
 
 func TestRegisterTargets_WithoutVPCService(t *testing.T) {

spinifex/domains/elbv2/service_impl_rules.go

@@ -85,7 +85,10 @@ func (s *ELBv2ServiceImpl) CreateRule(ctx context.Context, input *elbv2.CreateRu
 		return nil, errors.New(awserrors.ErrorServerInternal)
 	}
 
-	tags := tagsFromSDK(input.Tags)
+	tags, err := tagsFromSDK(input.Tags)
+	if err != nil {
+		return nil, err
+	}
 
 	record := &RuleRecord{
 		RuleArn:     ruleArn,

spinifex/domains/elbv2/service_impl_tags.go

@@ -112,10 +112,10 @@ func (s *ELBv2ServiceImpl) resolveOwnedTaggable(ctx context.Context, arn, accoun
 	return h, nil
 }
 
-// validateAllTaggable checks every ARN before the caller writes anything, so
-// an unknown or cross-account ARN anywhere in the request fails it with no
-// resource modified. It returns the ARNs with repeats removed.
-func (s *ELBv2ServiceImpl) validateAllTaggable(ctx context.Context, arns []*string, accountID string) ([]string, error) {
+// validateAllTaggable checks every ARN, and that adding newTags keeps each
+// resource within the tag limit, before the caller writes anything, so a bad
+// request modifies no resource. It returns the ARNs with repeats removed.
+func (s *ELBv2ServiceImpl) validateAllTaggable(ctx context.Context, arns []*string, accountID string, newTags map[string]string) ([]string, error) {
 	unique := make([]string, 0, len(arns))
 	seen := make(map[string]struct{}, len(arns))
 	for _, arnPtr := range arns {
@@ -128,9 +128,19 @@ func (s *ELBv2ServiceImpl) validateAllTaggable(ctx context.Context, arns []*stri
 		}
 		seen[arn] = struct{}{}
 
-		if _, err := s.resolveOwnedTaggable(ctx, arn, accountID); err != nil {
+		h, err := s.resolveOwnedTaggable(ctx, arn, accountID)
+		if err != nil {
 			return nil, err
 		}
+		total := len(h.tags)
+		for k := range newTags {
+			if _, ok := h.tags[k]; !ok {
+				total++
+			}
+		}
+		if total > maxResourceTags {
+			return nil, errTooManyTags()
+		}
 		unique = append(unique, arn)
 	}
 	return unique, nil
@@ -144,19 +154,12 @@ func (s *ELBv2ServiceImpl) AddTags(ctx context.Context, input *elbv2.AddTagsInpu
 		return nil, errors.New(awserrors.ErrorMissingParameter)
 	}
 
-	newTags := make(map[string]string, len(input.Tags))
-	for _, t := range input.Tags {
-		if t == nil || t.Key == nil || *t.Key == "" {
-			return nil, errors.New(awserrors.ErrorInvalidParameterValue)
-		}
-		v := ""
-		if t.Value != nil {
-			v = *t.Value
-		}
-		newTags[*t.Key] = v
+	newTags, err := tagsFromSDK(input.Tags)
+	if err != nil {
+		return nil, err
 	}
 
-	arns, err := s.validateAllTaggable(ctx, input.ResourceArns, accountID)
+	arns, err := s.validateAllTaggable(ctx, input.ResourceArns, accountID, newTags)
 	if err != nil {
 		return nil, err
 	}
@@ -198,7 +201,7 @@ func (s *ELBv2ServiceImpl) RemoveTags(ctx context.Context, input *elbv2.RemoveTa
 		keys = append(keys, *k)
 	}
 
-	arns, err := s.validateAllTaggable(ctx, input.ResourceArns, accountID)
+	arns, err := s.validateAllTaggable(ctx, input.ResourceArns, accountID, nil)
 	if err != nil {
 		return nil, err
 	}

spinifex/domains/elbv2/service_impl_tags_test.go

@@ -3,6 +3,8 @@ package elbv2
 import (
 	"context"
 	"encoding/json"
+	"fmt"
+	"strings"
 	"sync"
 	"testing"
 
@@ -139,18 +141,133 @@ func TestAddTags_MissingParams(t *testing.T) {
 	assert.Contains(t, err.Error(), awserrors.ErrorMissingParameter)
 }
 
-func TestAddTags_EmptyKeyRejected(t *testing.T) {
+func numberedTags(prefix string, n int) []*elbv2.Tag {
+	out := make([]*elbv2.Tag, 0, n)
+	for i := range n {
+		out = append(out, &elbv2.Tag{Key: aws.String(fmt.Sprintf("%s-%02d", prefix, i)), Value: aws.String("v")})
+	}
+	return out
+}
+
+// badTagCases are tag lists the ELBv2 model or service limits refuse, with
+// the error code AWS returns for each.
+var badTagCases = []struct {
+	name string
+	tags []*elbv2.Tag
+	code string
+}{
+	{"EmptyList", []*elbv2.Tag{}, awserrors.ErrorValidationError},
+	{"NilKey", []*elbv2.Tag{{Value: aws.String("v")}}, awserrors.ErrorValidationError},
+	{"EmptyKey", []*elbv2.Tag{{Key: aws.String(""), Value: aws.String("v")}}, awserrors.ErrorValidationError},
+	{"KeyTooLong", []*elbv2.Tag{{Key: aws.String(strings.Repeat("k", 129)), Value: aws.String("v")}}, awserrors.ErrorValidationError},
+	{"ValueTooLong", []*elbv2.Tag{{Key: aws.String("k"), Value: aws.String(strings.Repeat("v", 257))}}, awserrors.ErrorValidationError},
+	{"KeyPattern", []*elbv2.Tag{{Key: aws.String("bad*key"), Value: aws.String("v")}}, awserrors.ErrorValidationError},
+	{"ValuePattern", []*elbv2.Tag{{Key: aws.String("k"), Value: aws.String("bad;value")}}, awserrors.ErrorValidationError},
+	{"DuplicateKey", []*elbv2.Tag{{Key: aws.String("k"), Value: aws.String("a")}, {Key: aws.String("k"), Value: aws.String("b")}}, awserrors.ErrorELBv2DuplicateTagKeys},
+	{"TooMany", numberedTags("k", 51), awserrors.ErrorELBv2TooManyTags},
+}
+
+func TestAddTags_RejectsBadTags(t *testing.T) {
 	t.Parallel()
 	svc := setupTestService(t)
-	lbOut, _ := svc.CreateLoadBalancer(context.Background(), &elbv2.CreateLoadBalancerInput{Name: aws.String("addtags-ek")}, testAccountID)
+	lbOut, err := svc.CreateLoadBalancer(context.Background(), &elbv2.CreateLoadBalancerInput{Name: aws.String("addtags-bad")}, testAccountID)
+	require.NoError(t, err)
 	arn := lbOut.LoadBalancers[0].LoadBalancerArn
 
-	_, err := svc.AddTags(context.Background(), &elbv2.AddTagsInput{
-		ResourceArns: []*string{arn},
-		Tags:         []*elbv2.Tag{{Key: aws.String(""), Value: aws.String("v")}},
+	for _, tc := range badTagCases {
+		if tc.name == "EmptyList" {
+			continue // AddTags requires Tags, so an empty list is MissingParameter
+		}
+		_, err := svc.AddTags(context.Background(), &elbv2.AddTagsInput{ResourceArns: []*string{arn}, Tags: tc.tags}, testAccountID)
+		assert.Equal(t, tc.code, awserrors.ValidErrorCodeFromError(err), tc.name)
+	}
+	assert.Empty(t, tagsFor(t, svc, *arn))
+}
+
+// The 50-tag limit counts the resource's existing tags; overwriting an
+// existing key does not add to the total.
+func TestAddTags_LimitCountsExistingTags(t *testing.T) {
+	t.Parallel()
+	svc := setupTestService(t)
+	lbOut, err := svc.CreateLoadBalancer(context.Background(), &elbv2.CreateLoadBalancerInput{
+		Name: aws.String("addtags-limit"),
+		Tags: numberedTags("old", 49),
 	}, testAccountID)
-	require.Error(t, err)
-	assert.Contains(t, err.Error(), awserrors.ErrorInvalidParameterValue)
+	require.NoError(t, err)
+	arn := lbOut.LoadBalancers[0].LoadBalancerArn
+
+	_, err = svc.AddTags(context.Background(), &elbv2.AddTagsInput{ResourceArns: []*string{arn}, Tags: numberedTags("new", 2)}, testAccountID)
+	assert.Equal(t, awserrors.ErrorELBv2TooManyTags, awserrors.ValidErrorCodeFromError(err))
+	assert.Len(t, tagsFor(t, svc, *arn), 49)
+
+	tags := append(numberedTags("old", 1), numberedTags("new", 1)...)
+	_, err = svc.AddTags(context.Background(), &elbv2.AddTagsInput{ResourceArns: []*string{arn}, Tags: tags}, testAccountID)
+	require.NoError(t, err)
+	assert.Len(t, tagsFor(t, svc, *arn), 50)
+}
+
+func TestAddTags_MissingValueStoredEmpty(t *testing.T) {
+	t.Parallel()
+	svc := setupTestService(t)
+	lbOut, err := svc.CreateLoadBalancer(context.Background(), &elbv2.CreateLoadBalancerInput{Name: aws.String("addtags-noval")}, testAccountID)
+	require.NoError(t, err)
+	arn := lbOut.LoadBalancers[0].LoadBalancerArn
+
+	_, err = svc.AddTags(context.Background(), &elbv2.AddTagsInput{ResourceArns: []*string{arn}, Tags: []*elbv2.Tag{{Key: aws.String("k")}}}, testAccountID)
+	require.NoError(t, err)
+	assert.Equal(t, map[string]string{"k": ""}, tagsFor(t, svc, *arn))
+}
+
+func TestCreateLoadBalancer_RejectsBadTags(t *testing.T) {
+	t.Parallel()
+	svc := setupTestService(t)
+	for _, tc := range badTagCases {
+		_, err := svc.CreateLoadBalancer(context.Background(), &elbv2.CreateLoadBalancerInput{Name: aws.String("badtags-lb"), Tags: tc.tags}, testAccountID)
+		assert.Equal(t, tc.code, awserrors.ValidErrorCodeFromError(err), tc.name)
+	}
+
+	// No rejected create may have claimed the name.
+	out, err := svc.CreateLoadBalancer(context.Background(), &elbv2.CreateLoadBalancerInput{
+		Name: aws.String("badtags-lb"),
+		Tags: []*elbv2.Tag{{Key: aws.String("k")}},
+	}, testAccountID)
+	require.NoError(t, err)
+	assert.Equal(t, map[string]string{"k": ""}, tagsFor(t, svc, *out.LoadBalancers[0].LoadBalancerArn))
+}
+
+// Each other create call that accepts tags runs them through the same validator.
+func TestCreateTaggedResources_RejectBadTags(t *testing.T) {
+	t.Parallel()
+	env := newRuleTestEnv(t, "badtags")
+	cases := []struct {
+		tags []*elbv2.Tag
+		code string
+	}{
+		{numberedTags("k", 51), awserrors.ErrorELBv2TooManyTags},
+		{[]*elbv2.Tag{{Key: aws.String("k"), Value: aws.String("a")}, {Key: aws.String("k"), Value: aws.String("b")}}, awserrors.ErrorELBv2DuplicateTagKeys},
+		{[]*elbv2.Tag{{Key: aws.String("bad*key")}}, awserrors.ErrorValidationError},
+	}
+
+	for _, tc := range cases {
+		_, err := env.svc.CreateTargetGroup(context.Background(), &elbv2.CreateTargetGroupInput{Name: aws.String("badtags-tg3"), Tags: tc.tags}, testAccountID)
+		assert.Equal(t, tc.code, awserrors.ValidErrorCodeFromError(err), "CreateTargetGroup")
+		_, err = env.svc.CreateListener(context.Background(), &elbv2.CreateListenerInput{
+			LoadBalancerArn: aws.String(env.lbArn),
+			Protocol:        aws.String("HTTP"),
+			Port:            aws.Int64(8080),
+			DefaultActions:  []*elbv2.Action{{Type: aws.String("forward"), TargetGroupArn: aws.String(env.tgArn)}},
+			Tags:            tc.tags,
+		}, testAccountID)
+		assert.Equal(t, tc.code, awserrors.ValidErrorCodeFromError(err), "CreateListener")
+		_, err = env.svc.CreateRule(context.Background(), &elbv2.CreateRuleInput{
+			ListenerArn: aws.String(env.listenerArn),
+			Priority:    aws.Int64(10),
+			Conditions:  []*elbv2.RuleCondition{{Field: aws.String("path-pattern"), Values: aws.StringSlice([]string{"/x"})}},
+			Actions:     []*elbv2.Action{{Type: aws.String("forward"), TargetGroupArn: aws.String(env.tgArn)}},
+			Tags:        tc.tags,
+		}, testAccountID)
+		assert.Equal(t, tc.code, awserrors.ValidErrorCodeFromError(err), "CreateRule")
+	}
 }
 
 func TestRemoveTags_LoadBalancer(t *testing.T) {

spinifex/domains/elbv2/service_impl_test.go

@@ -357,6 +357,42 @@ func TestDescribeLoadBalancers_UnknownNameReturnsNotFound(t *testing.T) {
 	assert.Equal(t, awserrors.ErrorELBv2LoadBalancerNotFound, code)
 }
 
+func TestDescribeLoadBalancers_UnknownArnReturnsNotFound(t *testing.T) {
+	t.Parallel()
+	svc := setupTestService(t)
+
+	lbOut, err := svc.CreateLoadBalancer(context.Background(), &elbv2.CreateLoadBalancerInput{Name: aws.String("nf-arn-alb")}, testAccountID)
+	require.NoError(t, err)
+	lbArn := aws.StringValue(lbOut.LoadBalancers[0].LoadBalancerArn)
+	missing := "arn:aws:elasticloadbalancing:ap-southeast-2:" + testAccountID + ":loadbalancer/app/nosuchlb/0123456789abcdef"
+
+	for _, arns := range [][]string{{missing}, {lbArn, missing}} {
+		_, err = svc.DescribeLoadBalancers(context.Background(), &elbv2.DescribeLoadBalancersInput{
+			LoadBalancerArns: aws.StringSlice(arns),
+		}, testAccountID)
+		code, msg, _ := awserrors.ResolveErrorDetail(err)
+		assert.Equal(t, awserrors.ErrorELBv2LoadBalancerNotFound, code, "arns %v", arns)
+		assert.Equal(t, "One or more load balancers not found", msg)
+	}
+
+	// Another account's load balancer is not visible by ARN.
+	_, err = svc.DescribeLoadBalancers(context.Background(), &elbv2.DescribeLoadBalancersInput{
+		LoadBalancerArns: aws.StringSlice([]string{lbArn}),
+	}, "999999999999")
+	code, _, _ := awserrors.ResolveErrorDetail(err)
+	assert.Equal(t, awserrors.ErrorELBv2LoadBalancerNotFound, code)
+}
+
+func TestDescribeLoadBalancers_NoFilterEmptyAccountReturnsEmptyList(t *testing.T) {
+	t.Parallel()
+	svc := setupTestService(t)
+
+	desc, err := svc.DescribeLoadBalancers(context.Background(), &elbv2.DescribeLoadBalancersInput{}, testAccountID)
+	require.NoError(t, err)
+	assert.NotNil(t, desc.LoadBalancers)
+	assert.Empty(t, desc.LoadBalancers)
+}
+
 func TestDescribeListeners_UnknownLoadBalancerReturnsNotFound(t *testing.T) {
 	t.Parallel()
 	svc := setupTestService(t)

spinifex/domains/elbv2/tagging_current_behaviour_test.go

@@ -2,8 +2,6 @@ package elbv2
 
 import (
 	"context"
-	"fmt"
-	"strings"
 	"testing"
 
 	"github.com/aws/aws-sdk-go/aws"
@@ -37,25 +35,16 @@ func TestCurrentBehaviour_Tagging_ELBv2AddTagsAppliesNothingWhenALaterARNIsUnkno
 	assert.Empty(t, tagsFor(t, svc, arn), "the first resource must not be tagged")
 }
 
-// AddTags refuses only an empty key: 51 tags, a 129-character key, a
-// 257-character value and an aws: key are all stored.
-func TestCurrentBehaviour_Tagging_ELBv2AddTagsAppliesNoTagLimits(t *testing.T) {
+// AddTags stores a key with the aws: prefix.
+func TestCurrentBehaviour_Tagging_ELBv2AddTagsAcceptsAwsPrefixedKey(t *testing.T) {
 	svc := setupTestService(t)
 	lbOut, err := svc.CreateLoadBalancer(context.Background(), &elbv2.CreateLoadBalancerInput{Name: aws.String("cb-limits-lb")}, testAccountID)
 	require.NoError(t, err)
 	arn := *lbOut.LoadBalancers[0].LoadBalancerArn
 
-	tags := []*elbv2.Tag{
-		{Key: aws.String("aws:reserved"), Value: aws.String("v")},
-		{Key: aws.String(strings.Repeat("k", 129)), Value: aws.String(strings.Repeat("v", 257))},
-	}
-	for i := range 49 {
-		tags = append(tags, &elbv2.Tag{Key: aws.String(fmt.Sprintf("key-%02d", i)), Value: aws.String("v")})
-	}
+	tags := []*elbv2.Tag{{Key: aws.String("aws:reserved"), Value: aws.String("v")}}
 	_, err = svc.AddTags(context.Background(), &elbv2.AddTagsInput{ResourceArns: []*string{aws.String(arn)}, Tags: tags}, testAccountID)
 	require.NoError(t, err)
 
-	got := tagsFor(t, svc, arn)
-	assert.Len(t, got, 51)
-	assert.Contains(t, got, "aws:reserved")
+	assert.Contains(t, tagsFor(t, svc, arn), "aws:reserved")
 }

spinifex/domains/lambda/awsapi/create.go

@@ -158,11 +158,7 @@ func configuration(rec function.Record, revision uint64, env map[string]string)
 		cfg.CodeSize = aws.Int64(rec.Code.Size)
 	}
 	if len(env) > 0 {
-		vars := make(map[string]*string, len(env))
-		for k, v := range env {
-			vars[k] = aws.String(v)
-		}
-		cfg.Environment = &lambda.EnvironmentResponse{Variables: vars}
+		cfg.Environment = &lambda.EnvironmentResponse{Variables: aws.StringMap(env)}
 	}
 	return cfg, nil
 }

spinifex/domains/lambda/awsapi/registration.go

@@ -59,7 +59,7 @@ func NewRegistration(deps Deps) dispatch.Registration {
 		ResolveAction: resolveAction,
 		Dispatch:      deps.dispatch,
 		Errors:        dispatch.ErrorEnvelopeRESTJSON,
-		Inventory:     dispatch.Inventory{Registered: actionNames()},
+		Inventory:     dispatch.Inventory{Registered: router.Actions()},
 	}
 }
 

spinifex/domains/lambda/awsapi/routes.go

@@ -3,7 +3,6 @@ package awsapi
 import (
 	"context"
 	"net/url"
-	"slices"
 
 	"github.com/aws/aws-sdk-go/service/lambda"
 	"github.com/mulgadc/spinifex/spinifex/ingress/aws/rest"
@@ -59,14 +58,4 @@ var routes = []route{
 		}},
 }
 
-// actionNames returns the distinct actions in routes in stable order.
-func actionNames() []string {
-	names := make([]string, 0, len(routes))
-	for _, r := range routes {
-		names = append(names, r.Action)
-	}
-	slices.Sort(names)
-	return slices.Compact(names)
-}
-
 var router = rest.NewRouter("lambda", routes)

spinifex/domains/network/external/dhcp/kv.go

@@ -206,7 +206,7 @@ func toWireEntry(e Entry) wireEntry {
 		Routers:     ipsToStrings(l.Routers),
 		DNS:         ipsToStrings(l.DNS),
 		ServerID:    ipToString(l.ServerID),
-		HWAddr:      hwToString(l.HWAddr),
+		HWAddr:      l.HWAddr.String(),
 		UseIfaceMAC: l.UseIfaceMAC,
 		Bridge:      l.Bridge,
 		Hostname:    l.Hostname,
@@ -272,13 +272,6 @@ func ipsToStrings(ips []net.IP) []string {
 	return out
 }
 
-func hwToString(hw net.HardwareAddr) string {
-	if len(hw) == 0 {
-		return ""
-	}
-	return hw.String()
-}
-
 func maskToString(m net.IPMask) string {
 	if len(m) == 0 {
 		return ""

spinifex/domains/network/external/dhcp/nats_client.go

@@ -62,7 +62,7 @@ func (c *NATSClient) RequestAcquire(ctx context.Context, p AcquireParams) (*Leas
 		ClientID:    p.ClientID,
 		Hostname:    p.Hostname,
 		VendorClass: p.VendorClass,
-		HWAddr:      hwToString(p.HWAddr),
+		HWAddr:      p.HWAddr.String(),
 		UseIfaceMAC: p.UseIfaceMAC,
 		Purpose:     p.Purpose,
 		PoolName:    p.PoolName,

spinifex/domains/network/external/dhcp/wire.go

@@ -166,7 +166,7 @@ func toWireLease(l *Lease) *wireLease {
 		ClientID:      l.ClientID,
 		Hostname:      l.Hostname,
 		VendorClass:   l.VendorClass,
-		HWAddr:        hwToString(l.HWAddr),
+		HWAddr:        l.HWAddr.String(),
 		UseIfaceMAC:   l.UseIfaceMAC,
 		IP:            ipToString(l.IP),
 		SubnetMask:    maskToString(l.SubnetMask),

spinifex/domains/network/ovn/live.go

@@ -905,10 +905,7 @@ func (c *LiveClient) deleteRouterNATs(ctx context.Context, routerName string, pr
 	if err != nil {
 		return fmt.Errorf("get logical router for NAT delete: %w", err)
 	}
-	routerNATs := make(map[string]struct{}, len(lr.NAT))
-	for _, uuid := range lr.NAT {
-		routerNATs[uuid] = struct{}{}
-	}
+	routerNATs := uuidSet(lr.NAT)
 
 	var nats []nbdb.NAT
 	err = c.client.WhereCache(func(n *nbdb.NAT) bool {
@@ -1032,10 +1029,7 @@ func (c *LiveClient) FindNATByLogicalIP(ctx context.Context, routerName, natType
 	if err != nil {
 		return nil, fmt.Errorf("get logical router for NAT lookup: %w", err)
 	}
-	owned := make(map[string]struct{}, len(lr.NAT))
-	for _, u := range lr.NAT {
-		owned[u] = struct{}{}
-	}
+	owned := uuidSet(lr.NAT)
 	var nats []nbdb.NAT
 	if err := c.client.WhereCache(func(n *nbdb.NAT) bool {
 		if _, ok := owned[n.UUID]; !ok {
@@ -1111,10 +1105,7 @@ func (c *LiveClient) FindStaticRoute(ctx context.Context, routerName, ipPrefix s
 	if err != nil {
 		return nil, fmt.Errorf("get logical router for static route lookup: %w", err)
 	}
-	owned := make(map[string]struct{}, len(lr.StaticRoutes))
-	for _, u := range lr.StaticRoutes {
-		owned[u] = struct{}{}
-	}
+	owned := uuidSet(lr.StaticRoutes)
 	var routes []nbdb.LogicalRouterStaticRoute
 	if err := c.client.WhereCache(func(r *nbdb.LogicalRouterStaticRoute) bool {
 		if r.IPPrefix != ipPrefix {
@@ -1210,10 +1201,7 @@ func (c *LiveClient) FindLogicalRouterPolicy(ctx context.Context, routerName str
 	if err != nil {
 		return nil, fmt.Errorf("get logical router for policy lookup: %w", err)
 	}
-	owned := make(map[string]struct{}, len(lr.Policies))
-	for _, u := range lr.Policies {
-		owned[u] = struct{}{}
-	}
+	owned := uuidSet(lr.Policies)
 	var policies []nbdb.LogicalRouterPolicy
 	if err := c.client.WhereCache(func(p *nbdb.LogicalRouterPolicy) bool {
 		if p.Priority != priority || p.Match != match {
@@ -1238,10 +1226,7 @@ func (c *LiveClient) ListLogicalRouterPolicies(ctx context.Context, routerName s
 	if len(lr.Policies) == 0 {
 		return nil, nil
 	}
-	owned := make(map[string]struct{}, len(lr.Policies))
-	for _, u := range lr.Policies {
-		owned[u] = struct{}{}
-	}
+	owned := uuidSet(lr.Policies)
 	var policies []nbdb.LogicalRouterPolicy
 	if err := c.client.WhereCache(func(p *nbdb.LogicalRouterPolicy) bool {
 		_, ok := owned[p.UUID]
@@ -1737,3 +1722,12 @@ func (c *LiveClient) ReplaceACLs(ctx context.Context, portGroupName string, spec
 	}
 	return nil
 }
+
+// uuidSet indexes a router's row-reference column for membership checks.
+func uuidSet(uuids []string) map[string]struct{} {
+	set := make(map[string]struct{}, len(uuids))
+	for _, u := range uuids {
+		set[u] = struct{}{}
+	}
+	return set
+}

spinifex/domains/ochre/catalog.go

@@ -252,18 +252,9 @@ func (e catalogEntry) toSummary() *bedrock.FoundationModelSummary {
 	}
 }
 
+// FoundationModelDetails and FoundationModelSummary have identical fields.
 func (e catalogEntry) toDetails() *bedrock.FoundationModelDetails {
-	return &bedrock.FoundationModelDetails{
-		ModelArn:                   aws.String(modelARN(e.ModelID)),
-		ModelId:                    aws.String(e.ModelID),
-		ModelName:                  aws.String(e.ModelName),
-		ProviderName:               aws.String(e.ProviderName),
-		InputModalities:            aws.StringSlice(e.InputModalities),
-		OutputModalities:           aws.StringSlice(e.OutputModalities),
-		ResponseStreamingSupported: aws.Bool(e.ResponseStreamingSupported),
-		InferenceTypesSupported:    aws.StringSlice(e.InferenceTypesSupported),
-		CustomizationsSupported:    aws.StringSlice(e.CustomizationsSupported),
-	}
+	return (*bedrock.FoundationModelDetails)(e.toSummary())
 }
 
 func modelARN(modelID string) string {

spinifex/domains/ochre/converse_stream.go

@@ -192,35 +192,10 @@ func ConverseStream(ctx context.Context, w http.ResponseWriter, accountID, model
 // client disconnect (ctx.Done), or upstream fault — the deferred closure
 // records exactly one InvocationRecord via rc.recorder.
 func pumpConverseStream(ctx context.Context, fw *frameWriter, src converseStreamSource, modelID string, rc streamRecordCtx) {
-	if rc.recorder == nil {
-		rc.recorder = NoopRecorder
-	}
 	partial := true
 	errCode := ""
 	var outputText strings.Builder
-
-	defer func() {
-		inputTokens, outputTokens, estimated := int64(0), int64(0), true
-		if ur, ok := src.(usageReporter); ok {
-			inputTokens, outputTokens, estimated = ur.usage()
-		}
-		rc.recorder.Record(ctx, InvocationRecord{
-			RequestID:      rc.requestID,
-			AccountID:      rc.accountID,
-			ModelID:        modelID,
-			Operation:      rc.operation,
-			Backend:        rc.backend,
-			LatencyMs:      time.Since(rc.start).Milliseconds(),
-			HTTPStatus:     http.StatusOK, // the 200 header is already committed by the time the pump runs
-			ErrorCode:      errCode,
-			InputTokens:    inputTokens,
-			OutputTokens:   outputTokens,
-			UsageEstimated: estimated,
-			Partial:        partial,
-			InputText:      rc.inputText,
-			OutputText:     outputText.String(),
-		})
-	}()
+	defer func() { rc.record(ctx, src, modelID, errCode, partial, outputText.String()) }()
 
 	for {
 		select {
@@ -232,16 +207,7 @@ func pumpConverseStream(ctx context.Context, fw *frameWriter, src converseStream
 
 		event, ok, err := src.Next(ctx)
 		if err != nil {
-			excType := excInternalServerException
-			if _, ok := errors.AsType[*streamFaultError](err); ok {
-				excType = excModelStreamErrorException
-			}
-			errCode = awserrors.ValidErrorCodeFromError(err)
-			slog.Error("converse-stream: upstream fault", "model", modelID, "err", err)
-			if werr := fw.writeException(excType, exceptionPayload(err)); werr != nil {
-				slog.Error("converse-stream: failed to write exception frame", "model", modelID, "err", werr)
-				errCode = errCodeClientDisconnected
-			}
+			errCode = fw.writeUpstreamFault("converse-stream", modelID, err)
 			return
 		}
 		if !ok {

spinifex/domains/ochre/eventstream.go

@@ -3,6 +3,7 @@ package ochre
 import (
 	"encoding/json"
 	"errors"
+	"log/slog"
 	"net/http"
 
 	esv2 "github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream"
@@ -133,6 +134,22 @@ func (fw *frameWriter) writeException(excType string, payload []byte) error {
 	return fw.writeMessage(headers, payload)
 }
 
+// writeUpstreamFault frames a mid-stream source error as an in-band exception
+// and returns the error code to record: the fault's own, or ClientDisconnected
+// when the exception frame cannot be written either.
+func (fw *frameWriter) writeUpstreamFault(op, modelID string, err error) string {
+	excType := excInternalServerException
+	if _, ok := errors.AsType[*streamFaultError](err); ok {
+		excType = excModelStreamErrorException
+	}
+	slog.Error(op+": upstream fault", "model", modelID, "err", err)
+	if werr := fw.writeException(excType, exceptionPayload(err)); werr != nil {
+		slog.Error(op+": failed to write exception frame", "model", modelID, "err", werr)
+		return errCodeClientDisconnected
+	}
+	return awserrors.ValidErrorCodeFromError(err)
+}
+
 // writeChunk frames a provider-native chunk as the
 // InvokeModelWithResponseStream "chunk" event, base64-encoding nativeChunk
 // into Bedrock's PayloadPart{Bytes} shape via the SDK's own restjson

spinifex/domains/ochre/invoke_stream.go

@@ -2,13 +2,10 @@ package ochre
 
 import (
 	"context"
-	"errors"
 	"log/slog"
 	"net/http"
 	"time"
 	"uuid"
-
-	"github.com/mulgadc/spinifex/spinifex/foundation/aws/errors"
 )
 
 // invokeStreamSource is the backend-agnostic InvokeModelWithResponseStream
@@ -83,36 +80,11 @@ func InvokeModelWithResponseStream(ctx context.Context, w http.ResponseWriter, a
 // disconnect (ctx.Done), or upstream fault — the deferred closure records
 // exactly one InvocationRecord via rc.recorder.
 func pumpInvokeStream(ctx context.Context, fw *frameWriter, src invokeStreamSource, modelID string, rc streamRecordCtx) {
-	if rc.recorder == nil {
-		rc.recorder = NoopRecorder
-	}
 	partial := true
 	errCode := ""
-
-	defer func() {
-		inputTokens, outputTokens, estimated := int64(0), int64(0), true
-		if ur, ok := src.(usageReporter); ok {
-			inputTokens, outputTokens, estimated = ur.usage()
-		}
-		rc.recorder.Record(ctx, InvocationRecord{
-			RequestID:      rc.requestID,
-			AccountID:      rc.accountID,
-			ModelID:        modelID,
-			Operation:      rc.operation,
-			Backend:        rc.backend,
-			LatencyMs:      time.Since(rc.start).Milliseconds(),
-			HTTPStatus:     http.StatusOK, // the 200 header is already committed by the time the pump runs
-			ErrorCode:      errCode,
-			InputTokens:    inputTokens,
-			OutputTokens:   outputTokens,
-			UsageEstimated: estimated,
-			Partial:        partial,
-			InputText:      rc.inputText,
-			// OutputText is intentionally left empty: chunks are opaque
-			// provider-native bytes (Llama JSON or raw Anthropic SSE data),
-			// with no generic way to recover assistant text across both.
-		})
-	}()
+	// OutputText stays empty: chunks are opaque provider-native bytes with no
+	// generic way to recover assistant text across Llama and Anthropic.
+	defer func() { rc.record(ctx, src, modelID, errCode, partial, "") }()
 
 	for {
 		select {
@@ -124,16 +96,7 @@ func pumpInvokeStream(ctx context.Context, fw *frameWriter, src invokeStreamSour
 
 		chunk, ok, err := src.Next(ctx)
 		if err != nil {
-			excType := excInternalServerException
-			if _, ok := errors.AsType[*streamFaultError](err); ok {
-				excType = excModelStreamErrorException
-			}
-			errCode = awserrors.ValidErrorCodeFromError(err)
-			slog.Error("invoke-with-response-stream: upstream fault", "model", modelID, "err", err)
-			if werr := fw.writeException(excType, exceptionPayload(err)); werr != nil {
-				slog.Error("invoke-with-response-stream: failed to write exception frame", "model", modelID, "err", werr)
-				errCode = errCodeClientDisconnected
-			}
+			errCode = fw.writeUpstreamFault("invoke-with-response-stream", modelID, err)
 			return
 		}
 		if !ok {

spinifex/domains/ochre/logging.go

@@ -162,6 +162,35 @@ type streamRecordCtx struct {
 	inputText string
 }
 
+// record emits the stream's one InvocationRecord. The status is always 200:
+// the header is already committed by the time a pump runs.
+func (rc streamRecordCtx) record(ctx context.Context, src any, modelID, errCode string, partial bool, outputText string) {
+	recorder := rc.recorder
+	if recorder == nil {
+		recorder = NoopRecorder
+	}
+	inputTokens, outputTokens, estimated := int64(0), int64(0), true
+	if ur, ok := src.(usageReporter); ok {
+		inputTokens, outputTokens, estimated = ur.usage()
+	}
+	recorder.Record(ctx, InvocationRecord{
+		RequestID:      rc.requestID,
+		AccountID:      rc.accountID,
+		ModelID:        modelID,
+		Operation:      rc.operation,
+		Backend:        rc.backend,
+		LatencyMs:      time.Since(rc.start).Milliseconds(),
+		HTTPStatus:     http.StatusOK,
+		ErrorCode:      errCode,
+		InputTokens:    inputTokens,
+		OutputTokens:   outputTokens,
+		UsageEstimated: estimated,
+		Partial:        partial,
+		InputText:      rc.inputText,
+		OutputText:     outputText,
+	})
+}
+
 type noopRecorder struct{}
 
 func (noopRecorder) Record(context.Context, InvocationRecord) {}

spinifex/domains/route53/service.go

@@ -8,6 +8,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"maps"
 	"slices"
 	"strconv"
 	"strings"
@@ -618,11 +619,7 @@ func changeInfo(c *ChangeRecord) *route53.ChangeInfo {
 }
 
 func tagSet(z *ZoneRecord) *route53.ResourceTagSet {
-	keys := make([]string, 0, len(z.Tags))
-	for k := range z.Tags {
-		keys = append(keys, k)
-	}
-	slices.Sort(keys)
+	keys := slices.Sorted(maps.Keys(z.Tags))
 	tags := make([]*route53.Tag, 0, len(keys))
 	for _, k := range keys {
 		tags = append(tags, &route53.Tag{Key: aws.String(k), Value: aws.String(z.Tags[k])})

spinifex/foundation/aws/errors/awserrors.go

@@ -581,6 +581,10 @@ var (
 	ErrorELBv2IncompatibleProtocols        = "IncompatibleProtocols"
 	ErrorELBv2CertificateNotFound          = "CertificateNotFound"
 	ErrorELBv2SSLPolicyNotFound            = "SSLPolicyNotFound"
+	ErrorELBv2DuplicateTagKeys             = "DuplicateTagKeys"
+	// ErrorELBv2TooManyTags aliases ErrorRoute53TooManyTagsForZone: both wire
+	// codes are "TooManyTags", so ELBv2 returns it via Errorf with its own message.
+	ErrorELBv2TooManyTags = ErrorRoute53TooManyTagsForZone
 
 	// Bedrock/bedrock-runtime error codes.
 	ErrorValidationException = "ValidationException"
@@ -700,6 +704,15 @@ func IDNotFound(code, kind, id string) error {
 	return Errorf(code, "The %s ID '%s' does not exist", kind, id)
 }
 
+// IDsNotFound is IDNotFound for one or more IDs; several render as EC2's
+// plural "The <kind> IDs '<a>, <b>' do not exist".
+func IDsNotFound(code, kind string, ids []string) error {
+	if len(ids) == 1 {
+		return IDNotFound(code, kind, ids[0])
+	}
+	return Errorf(code, "The %s IDs '%s' do not exist", kind, strings.Join(ids, ", "))
+}
+
 // HasDependencies returns DependencyViolation with EC2's message for a
 // resource that cannot be deleted while others depend on it.
 func HasDependencies(kind, id string) error {
@@ -983,7 +996,7 @@ var ErrorLookup = map[string]ErrorMessage{
 	ErrorInterfaceInUseByTrafficMirrorSession:                  {HTTPCode: 409, Message: "The Traffic Mirror source that you are trying to create uses an interface that is already associated with a session. An interface can only be associated with a session, or with a target, but not both."},
 	ErrorInterfaceInUseByTrafficMirrorTarget:                   {HTTPCode: 409, Message: "The Traffic Mirror source that you are trying to create uses an interface that is already associated with a target. An interface can only be associated with a session, or with a target, but not both. If the interface is associated with a target, it cannot be associated with another target."},
 	ErrorInternalError:                                         {HTTPCode: 500, Message: "An internal error has occurred. Retry your request, and if the problem persists, contact Mulga support with the details."},
-	ErrorInternalFailure:                                       {HTTPCode: 404, Message: "The request processing has failed because of an unknown error, exception, or failure."},
+	ErrorInternalFailure:                                       {HTTPCode: 500, Message: "The request processing has failed because of an unknown error, exception, or failure."},
 	ErrorInternetGatewayLimitExceeded:                          {HTTPCode: 400, Message: "You've reached the limit on the number of internet gateways that you can create. For more information, see Amazon VPC quotas."},
 	ErrorInvalidAMIAttributeItemValue:                          {HTTPCode: 400, Message: "The value of an item added to, or removed from, an image attribute is not valid. If you are specifying a userId, check that it is in the form of an AWS account ID, without hyphens."},
 	ErrorInvalidAMIIDMalformed:                                 {HTTPCode: 400, Message: "The specified AMI ID is malformed. Ensure that you provide the full AMI ID, in the form ami-xxxxxxxxxxxxxxxxx."},
@@ -1278,7 +1291,7 @@ var ErrorLookup = map[string]ErrorMessage{
 	ErrorResourceLimitExceeded:                                 {HTTPCode: 400, Message: "You have exceeded an Amazon EC2 resource limit. For example, you might have too many snapshot copies in progress."},
 	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."},
+	ErrorEKSResourceNotFound:                                   {HTTPCode: 404, Message: "The specified resource could not be found. You can view your available clusters with ListClusters. You can view your available managed node groups with ListNodegroups. Amazon EKS clusters and node groups are Amazon Web Services 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."},
@@ -1423,6 +1436,7 @@ var ErrorLookup = map[string]ErrorMessage{
 	ErrorELBv2IncompatibleProtocols:        {HTTPCode: 400, Message: "The listener protocol is incompatible with the target group protocol."},
 	ErrorELBv2CertificateNotFound:          {HTTPCode: 400, Message: "One or more specified certificates do not exist."},
 	ErrorELBv2SSLPolicyNotFound:            {HTTPCode: 400, Message: "The specified SSL policy does not exist."},
+	ErrorELBv2DuplicateTagKeys:             {HTTPCode: 400, Message: "A tag key was specified more than once."},
 
 	// Bedrock/bedrock-runtime error codes. ResourceNotFoundException reuses the
 	// EKS entry (ErrorEKSResourceNotFound) — same wire code, same HTTP status.