The policy gate had the cluster and nodegroup name but not the trailing
UUID, so it evaluated nodegroup/<cluster>/<name>/*. That value matches the
AWS-documented wildcard policy spelling but not one naming the exact stored
ARN, which is what a tenant gets by pasting DescribeNodegroup output, so
that Deny stayed inert.
The discriminator is now derived from (account, cluster, name) as an RFC
9562 version-8 UUID, so the gateway and the handler compute the same value
independently and the gate names the stored ARN exactly with no lookup.
Nodegroups created before this keep their random discriminator and their
existing wildcard-only behaviour.
Drives every vpc.* topology topic end to end against the mock OVN client and
asserts the resulting rows: router, subnet switch and router port, logical
switch port addresses and port-group membership, gateway router port, NAT
rules and egress policies. Invalid CIDR, IP and MAC events are pinned to a
failed reply envelope with no partial write.
Moves the elbv2 dispatch table to an external test package.
subscribers 53.5% -> 73.4%.
Adds a 35-row dispatch table covering every NATS-backed elbv2 gateway
wrapper. Each row subscribes a stub only to the expected subject, so a
mis-wired subject fails as no-responders, and asserts the returned value,
the forwarded request body and the account header. A second table covers
the 28 validation guards that had no test, each pinned to its AWS error
code.
Package coverage 49.5% -> 93.8%.
Tables the twelve subjects Serve subscribes through the malformed-body,
version-skew and provider-failure arms, and round-trips every verb through
NATSProvider against a served MemoryProvider so client encoding and server
decoding are asserted against each other.
Covers MemoryProvider.ListVolumes, ListSnapshots and CopySnapshot, all of
which were at zero, plus NATSProvider's nil-payload and subject-token guards.
natsserve 28.8% -> 91.3%, ebsprovider 67.5% -> 86.3%, nullprovider 0% -> 100%.
Drives the model through Update with real key messages: a full walkthrough
asserting the config it produces, every validation gate keeping the operator on
the screen with the cursor on the field at fault, and the rendered output for
facts that appear nowhere else (role layout, pool geometry, folded planes).
The cross-host ICMP probe fired once, with no convergence budget, the moment
sshd first answered on the source guest. The default SG self-ingress rule
matches on the northd-derived $<pg>_ip4 address set, which only reaches the
remote chassis after northd regenerates it from port-group membership and
ovn-controller installs the flows there, so a first-packet burst can be
swallowed by the priority-900 default drop with no log line anywhere.
Give the probe the same 45s/2s budget the single-node suite already uses, and
snapshot NB/SB address sets, port groups, ACLs, port bindings and br-int flows
from every node plus the source guest on the first failed burst rather than at
teardown, so the state is captured while the drop is still observable. Emit
attempt count and convergence time so a pass that needed most of the budget
still surfaces as a propagation delay instead of a silent green.
One behavioural suite runs against LiveClient over the in-memory NB server and
against mock.Client, so the semantics reconcile, policy, external, topology,
subscribers and vpcd rely on are pinned in both. Covers NAT router scoping and
duplicate deletion, the not-found sentinels, gateway chassis idempotence,
ReplaceACLs no-churn, port group membership and DHCP lookups.
The readiness loop terminates on wall clock, so a stubbed sleep alone still
costs two minutes; clusterReadyMaxWait becomes a var alongside clusterReadySleep
so the timeout branch is reachable. Adds the in-package marker to the EIP
association tests, which drive setupTestEIP and read svc.natsConn.
setupTestEIP built the EIP service with a nil VPCService, so every ENI
lookup AssociateAddress performs was structurally unreachable: lookupENI,
lookupENIByInstance and AssociatedPublicIPForInstance had no test at all
and any test reaching them would have panicked. Wire a real VPC service
over the same embedded NATS and cover association by ENI id and by
instance id, the private-IP override, both not-found codes, and the
add-nat/delete-nat events the NAT rule is built from.
sgRulesToPolicyRules was never carried a tenant security-group rule: the
golden scenarios build IntentState by hand and bypass the adapter, so a
transposed FromPort/ToPort or a dropped SourceSG would have shipped
silently. Load real SG records through KV and pin the field mapping.
ECS and ECR take the existing both-directions shape against their Actions
tables. RDS carries its scopes on the dispatch table itself, so the gate is the
account-level half instead: every served action is either scoped or carries a
written reason, and one in both sets or neither fails. HasScope and
ScopedActions are added to drive it.
The condition-key gate pins the door's key set and asserts the IAM write path
accepts exactly the operator/key pairs the evaluator enforces.
The seven sourcePolicyARN actions resolved to the PolicyArn the caller sent,
so the gate decided against an ARN whose account the caller chose. The scope
now takes the ARN's path and name and rebuilds it on the caller's account, as
sourceOIDCARN beside it already does, and widens to account-wide when the ARN
carries no policy component. Lands with the cross-tenant integration suite that
found it.
snapshotVolumeEngine's unmounted branch built its engine by hand instead of
going through openVolumeVB, so it was the last control-plane path that opened
an engine over a volume without holding the cluster-wide lease. A snapshot
request landing on a node that does not have the volume mounted would open a
second engine over storage another node was writing.
Two engines on one encrypted volume read the same SeqNumHighWater and issue the
same AES-GCM nonces, which discloses the XOR of the plaintexts and permits
recovery of the authentication subkey, so this is not only a consistency
question.
Route the branch through openVolumeVB and give the lease the engine's lifetime.
The tail both branches shared moves into createSnapshotWithVB, mirroring
copySnapshotOnVolume, which was converted earlier and is what this one was
measured against.
A refusal is reported as unavailable rather than internal: the holder gives the
volume up on detach, so the caller should retry instead of treating a live
mount elsewhere as permanent.
The gateway does not serve S3, so an S3-scoped credential is rejected on
scope by the SigV4 middleware. That rejection was written through
GenerateEC2ErrorResponse, giving the client the EC2 query envelope
<Response><Errors><Error>. SDKs look for a flat top-level <Error><Code>
on an S3 response, find nothing, and report an empty code and an empty
message, which reads as a credential fault rather than a wrong endpoint.
Add the flat S3 envelope and resolve the envelope from the service the
client actually signed with: GetService reports an unserved scope as
empty, which is exactly the case that has to render as S3.
The service-to-envelope decision was copied into four emitters and the
copies had already drifted - writeClusterUnavailable put ELBv2 on the
EC2 envelope while ErrorHandler and writeThrottleError put it on the IAM
one. Collapse them onto a single xmlErrorBody so a new service cannot be
wired into some emitters and missed by others.
s3 is deliberately not added to supportedServices: only the shape of the
rejection changes, not the rejection.
* fix: ECS RunTask and CreateLaunchTemplateVersion honour ClientToken
A retried RunTask placed the tasks again: it mints a fresh task ID per task,
reserves CPU/memory/GPU, hot-plugs an ENI in awsvpc mode and starts the
containers, so a resend with Count 5 left ten running tasks, double the reserved
capacity and five leaked ENIs. CreateLaunchTemplateVersion added a second
identical version and moved $Latest.
RunTask gets a per-action wrapper over idempotency.Do rather than a dispatch
decorator: it is the only ECS action that needs one. A partial placement
finalizes rather than aborts, since re-running would add tasks on top of the
ones already started.
CreateLaunchTemplateVersion reuses the EC2 dispatch decorator, so it is an
opt-in rather than new machinery.
CreateLaunchTemplate, ModifyLaunchTemplate and ECS CreateService accept a token
they do not store; each is name-keyed or a CAS and cannot duplicate, which is
now recorded at the handler.
* test: cover the RunTask client-token binding layer
Split the token layer out of RunTask's NATS binding as runTaskIdempotent, the
same seam runTask already has, so the no-token, store-failure and replay paths
are reachable without a live connection.
CreateVolume, CreateNetworkInterface, CreateNatGateway, CreateRouteTable,
CreateEgressOnlyInternetGateway, CopyImage and CreateCapacityReservation parsed
a ClientToken and never read it, so a retried request created a second resource.
Adds ec2IdempotentHandler at the EC2 dispatch table rather than seven
hand-written wrappers, keyed <action>.<account>.<token> in the bucket
RunInstances already uses. RunInstances keeps its unprefixed keys: re-keying
live records would make a retry re-launch instead of replay.
Promotes the shared orchestration to idempotency.Do, now that eight callers
share it, and wraps store failures in ErrUnavailable so callers map every
non-mismatch token failure onto one API error.
ServeNATSRequestCtx ends its consumer span in a defer that runs after the
reply is published, so a test returning from NATSRequest could read the
span recorder before the consumer span had ended and see no account
attribute. The subscriber callbacks now signal completion and both tests
wait on that signal instead of on the reply.
The cache was a union that only grew: each run restored everything, added
what it compiled and saved the result, so objects superseded by later
commits stayed for Go's full five-day window. Deleting every entry and
letting it rebuild took Build and Test from 3434 MB to 478 MB, restore
from 75s to 13s and save from 64s to 7s.
Dropping entries nothing has touched in four hours holds it near that.
Go bumps an entry's mtime when a run uses it, so the working set
survives; the step runs after the tests, so a pruned entry costs a later
run a partial recompile at worst and never the current one.
Restore is what makes CI fast; save is what got expensive. A same-era A/B
across the switch to this cache shows what restore buys: wall time went
6:50 to 3:52, Build Spinifex 63s to 10s, test-race 385s to 188s. Since
then the cache has grown to its steady state and the work has not moved,
so Build and Test now spends 75s restoring and 64s saving where it once
spent 13s and 8s.
A PR's save is read by nothing else. Splitting restore from save and
gating the save on dev and main keeps every branch reading a cache a dev
merge refreshes minutes later, and takes the upload off the 56% of runs
that are pull requests.
pass and keyPass logged every failure at WARN as "retrying next cycle" and
returned the callee's deadline, which for a failed callee is zero. Zero means
no deadline, so the next attempt was the resync — five minutes by default.
Two things were wrong with that. A fault that clears in seconds left the world
unconverged for minutes, when the loop already had the mechanism to ask for
sooner. And a terminal failure was announced as recoverable: "retrying next
cycle" is false for a 400, which will be a 400 every cycle, forever, at WARN.
Both now go through failed(), which classifies with awserrors.IsTerminal — the
predicate added in #923 for exactly this decision and until now called by
nothing. A terminal error is logged at ERROR and asks for no early retry; any
other error asks to be revisited after DefaultErrorRetry. A deadline the callee
named survives when it is sooner, and clampRevisit still caps every request at
the resync, so a loop can only become more eager than it is today.
It answered its question: of 18 GB on disk, 1.45 GB is untouched for a
day and nothing at all survives three. Go's own trim and the rate CI
runs already keep the cache live, so pruning before the save would shed
under 4% of the bytes at a two-day cut and risk recompiles for it.
The lifecycle handlers published record changes onto the dns.recordset.change
queue group, which load-balances rather than serialises, so any daemon could be
read-modify-writing a zone object at any moment. zonelock.go existed to make
that safe. With the reconcile pass now applying its own batch on the node that
won the election, the desired set is the only path into a zone and the elected
node is its only writer, so both the publish path and the lock go.
Deleted: the four lifecycle publish call sites and their helpers, publish.go,
zonelock.go, SubjectRecordsetChange and QueueGroup, and the lock from
applyZone. Every record they published is already in the desired set the
reconcile rebuilds each pass, and every record they withdrew is already pruned
by it, so the classes DNS carries are unchanged.
The build cache key rolls per run, so each job restores the previous
run's cache, adds to it and uploads the result. Nothing removes entries,
and Go only trims at 5 days, so every restore and save carries whatever
dead weight has accumulated. Build and Test moves 3.2 GB per run: 59s
down, 56s up.
Whether pruning that is worth doing depends on how much of the 3.2 GB is
still live, which is not visible from outside the runner. This reports
the age distribution and deletes nothing. It comes out once the number
is known.
The reconnect tests paid the client's one-second default sleep between
attempts, which ConnectNATS already lets a caller override; the retry test
scales its floor with the delay it sets, so it still proves a retry happened.
7.9s to 5.0s.
The reconcile elected a leader, computed a converging batch, then published it
to the queue group, which load-balances rather than serialises. The batch was
therefore applied by an arbitrary peer, not by the node that won the election.
That split the read that decides which records are stale from the write that
removes them. A lifecycle upsert landing between the two was invisible to the
second, so a record re-added after the reconcile read it was deleted by a
decision that predated it. Applying locally closes the window, and makes the
elected node the only writer the reconcile puts on a zone.
Deleted with their tests: the batch splitter, the payload limits and the
partial-acknowledgement check, none of which have anything left to bound now
that no batch crosses a NATS connection. The lifecycle publishers and the zone
lock are untouched: they are still a second writer, and retiring them is
separate.
Every test already owns its embedded JetStream server on an ephemeral port
under its own temp dir, so nothing here has to run serially. The recovery
failure test also left its doomed Set on a deadline-free context, which made
it wait out the jetstream client's own five-second default.
9.5s to 0.8s, and 11.8s to 2.0s under -race.
A unit test cannot change what e2e exercises, so a push touching only _test.go
files no longer provisions the tofu VMs. tests/e2e/** is listed after the
exclusion so the e2e tests themselves still trigger their own suites.
The guard only reads types for the syntax it walks, which resolves from
dependency export data; NeedDeps type-checked the whole graph from source for
no extra coverage. Verified identical detection either way (160 packages, 993
files, 3684 slog call sites, and both violation classes still caught).
19.0s to 7.3s under -race.
Each test starts its own embedded JetStream server and bucket, so they were
already independent; running them serially just stacked up the fixed settle
sleeps. 13.3s to 2.8s under -race.
* fix: build the EC2 DNS desired set from the instance records so stale records prune
* fix: keep DNS records for stopped instances in the reconcile desired set
The desired set filtered on Running, but the lifecycle publish deliberately
does not withdraw a stopped instance's records: stop and start retain the
address, so they retain the name. With EC2 pruning enabled, absence from the
desired set became a deletion, so a stopped instance would have lost both its
names until the next publish.
Both sides now read one predicate, InstanceRetainsRecords, which replaces
needsDNSWithdrawal and lives next to the names it governs. Records carrying a
DeletionTimestamp are skipped as well, so a re-upsert cannot race the
withdrawal the terminate path has already published.
* refactor: wake the vpcd drift loop on intent changes, not a fixed 5-minute ticker
DriftLoop was the last ticker-driven reconcile loop in the event-driven
reconciliation work. It runs on reconciler.Run now, watching the eight KV
buckets LoadIntentFromKV reads, with DriftInterval kept as the resync
backstop and the existing backoff ladder returned as the pass deadline.
Repair latency for a dropped vpc.* event drops from a uniform 0-300s to
the debounce plus a 5s floor. The floor is new and deliberate: a pass
loads eight buckets and scans the OVN NB DB, so without it a launch storm
would turn a five-minute scan into a continuous one.
The buckets are enumerated rather than fixed so watching never creates
one: the EC2 handlers own them and set their history depth.
The bootstrap ReconcileApplyOnly pass stays the startup scan. The loop's
own startup pass does no work and returns the seed deadline instead, so a
clean boot still waits a full interval and a failed one still requeues at
5s.
* fix: keep the intent watchers up when a bucket lookup blips
A per-bucket resolve failure was skipped, which drops the bucket out of
the enumerated set and has the reconciler tear its watcher down until the
next resync. Only a genuine ErrBucketNotFound is a skip now; anything else
fails the enumeration, which is what tells the caller to leave the
watchers it already has in place.
* fix: measure the drift pass floor from the end of a pass, not its start
Stamping before the scan means a pass slower than the floor leaves no gap
at all behind it. The floor is meant to be the minimum time between
passes, so it is stamped once the scan returns.
* test: wait for the intent watcher before the write it must catch
* fix: follow ListObjectsV2 continuation tokens on every production caller
Six ListObjectsV2 call sites (DeleteSnapshot, DescribeSnapshots, EKS
etcd-snapshot selection, DescribeTags, and both key-pair listings) issued a
single request and never read IsTruncated or NextContinuationToken. This was
latent while predastore ignored max-keys and always answered IsTruncated:
false; predastore#117 pages listings at 1000 keys, turning a truncated
listing beyond that into a silently wrong answer rather than a short one.
DeleteSnapshot in particular could delete 1000 objects, report success, and
orphan the rest, and the EKS restore path could resolve to a stale snapshot
if the newest keys fell past the first page.
Add objectstore.ListAll, a shared helper that drains a listing to
exhaustion, accumulating Contents and CommonPrefixes across pages and
erroring instead of returning a partial result if the server claims
truncation with no token to resume from or never finishes within a bounded
page budget. All six call sites, plus the ebsfake test double with the same
shape, now use it. Bump the predastore pin to dev head ff6a1dc4 so a real
cluster picks up the server-side pagination fix this depends on.
* fix: use external test packages and exclude test fakes from diff coverage
test-package-check requires new test files to bind to the exported API so
internals can change without rewriting tests. Move list_test.go and
delete_snapshot_pagination_test.go to their _test packages, adding a small
export_test.go for the one unexported constant a test needs to assert
against. Simplify list_test.go's pagingStore to embed MemoryObjectStore
instead of hand-stubbing unused methods.
Exclude spinifex/testutil from diff-coverage.sh: those files are fakes and
harnesses driven only from other packages' _test.go files, so go test
records them as zero-covered under the package they live in however hard
another package's tests exercise them, which is not a real coverage gap.
Julian Sommer