feat(components): enforce source ownership on observation and deactivation RPCs by VLukiantsev · Pull Request #1238 · google/xmc

VLukiantsev · 2026-10-13T07:27:23Z

Summary

Adds RequestSource to UpdateObservedGeneration, PrepareDeactivation and CompleteDeactivation, enforces the cluster fence and source ownership on them in both stores, and wires the bridge's observation and deactivation calls through callWithOwnership.

Motivation & Context

After #1237 and #1274 only intents were ownership-checked. A stale or foreign writer could still move observed state or deactivate a component it does not own.

Key Changes

Proto: optional RequestSource on the three request messages.
Store: the same fence and ownership check (R3: the owner refreshes SourceLastSeenTime and keeps SourceGeneration; an R2/R5 adoption records SourceGeneration = 0 so the adopter's next intent is never stale) in MemoryStore and SpannerStore.
Store: an UpdateObservedGeneration that changes nothing (a heartbeat) keeps UpdateTime, the etag and the snapshot; Spanner still writes the owner columns, refreshing SourceLastSeenTime of a sourced call.
Bridge: observation and deactivation calls send the source and share the intent path's rejection handling.
Cascade exception: dependents changed by a cascade: true deactivation skip the caller's per-CR uid match and keep their recorded owner, because CMS changes them on behalf of the target's owner (stated in the PrepareDeactivationRequest.source proto comment and the README; covered by TestPrepareDeactivation_CascadeKeepsDependentOwner on both backends).
At most one RequestSource per call; extra sources are rejected with InvalidArgument.
Bridge: a rejected sourced UpdateObservedGeneration or CompleteDeactivation is projected as SourceConflict and stops the pass (TestReconcileComponent_LifecycleOwnershipRejections).

Stack Position

Layer 22 of 29 in stack #1277. Wave B (ownership and fencing; lands after wave A).
Depends on feat(components): adopt a same-cluster recreated Component via expected_owner_uid #1274 (same-cluster recreation takeover, above feat(components): send RequestSource from bridge and record status.componentGeneration #1237). Followed by feat(components): isolate per-CR Helm actuation state by UID #1239.

Assumptions

The cascade exception is safe — cascaded dependents are mutated by CMS on behalf of the owner of the deactivated component — impact if wrong: a cascade could touch a dependent owned by another CR.
The live GKE evidence still applies to this revision — it was collected before the last rebase onto main, and git range-diff shows no behavioral change apart from the hardening commit, which the rejection scenario below does not exercise — if wrong, the checkpoint must be re-run before merge.

Verification & Testing

Commands Executed: bazel test //pkg/components/store:store_test //pkg/components/cms:cms_test //pkg/components/bridge:bridge_test //api:protos_breaking_test
Coverage & Test Status: all pass against the memory store and the Spanner emulator.
KinD E2E: //test/e2e:components_lifecycle_test passed with the whole stack applied.
Diff size (tools/analyze_diff_size.py): 295 prod / 481 test / 776 total / 2 doc lines.

Live Deployment & Manual Testing

Deployment Target: Live GKE development environment (enable_components: true, Spanner-backed CMS), upgraded with the dev-environment upgrade script including Spanner migrations 014/015.
Cluster Health: Environment verification passed (5/5, 16/16 pods Ready, baseline Component CRs Ready and Synced).
Checkpoint: CP2/CP3, run with the whole stack deployed (through fix(components): reduce steady-state bridge log noise and use detail attribute #1244).
Revision note: the live evidence was collected before the latest rebase onto main. git range-diff against the current branches shows only rebase-induced import path updates (internal/logging) and the removal of unrelated MODULE.bazel.lock churn from feat(components): send RequestSource from bridge and record status.componentGeneration #1237; no behavioral change. The later hardening commit (SourceGeneration = 0 on non-intent adoption, no-op heartbeats keep UpdateTime) is covered by store unit tests on memory and the Spanner emulator, not by the live run.

Scenario	Result	Observation
Ownership on observation and deactivation RPCs	✅ PASS	`UpdateObservedGeneration`, `PrepareDeactivation` and `CompleteDeactivation` on `mt-cp3-recreate` with a non-owning uid all failed with `SOURCE_INCARNATION_MISMATCH`, leaving `ObservedGeneration=3` and `SourceLastSeenTime` unchanged.

Related Issues / Bugs

Bug: 548436300

TAG=agy
CONV=d833b0e6-4821-48f6-8576-776d53f55a5c

gemini-code-assist

Code Review

This pull request introduces cluster fence and source ownership enforcement for non-intent lifecycle mutations, specifically for the PrepareDeactivation, CompleteDeactivation, and UpdateObservedGeneration operations. It updates the protobuf definitions to include an optional RequestSource field, propagates this source from the bridge reconciler through the CMS server to the underlying memory and Spanner stores, and adds comprehensive tests to verify the ownership enforcement. There are no review comments to address, and the implementation looks solid.

VLukiantsev · 2026-10-13T10:42:03Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces cluster fence and source ownership enforcement for non-intent mutations, specifically during component deactivation preparation, completion, and observed generation updates. It updates the protobuf definitions to include a RequestSource field in the corresponding requests, adapts the CMS client, bridge lifecycle, CMS server, and both Memory and Spanner store backends to validate and record ownership details, and adds comprehensive parity tests. Feedback on the changes suggests adding a defensive nil check for the owner parameter in writeOwnerColumnsTx to prevent potential nil pointer dereferences.

…ation RPCs Add RequestSource to UpdateObservedGenerationRequest, PrepareDeactivationRequest, and CompleteDeactivationRequest; enforce cluster fence and source ownership across MemoryStore and SpannerStore; and wire bridge observation and deactivation calls through callWithOwnership. Bug: 548436300 TAG=agy CONV=d833b0e6-4821-48f6-8576-776d53f55a5c

Make writeOwnerColumnsTx a no-op for a nil owner so future call sites cannot dereference a nil SourceOwner. Comment link: #1238 (comment) Bug: 548436300 TAG=agy CONV=d833b0e6-4821-48f6-8576-776d53f55a5c

The source ownership layer replaced isOwnerObject with the nil-safe isOlderGeneration method. Use it for the stale-generation check on observation and deactivation RPCs, and compare the owner UID directly when preserving the committed intent generation. Bug: 548436300 TAG=agy CONV=d833b0e6-4821-48f6-8576-776d53f55a5c

…d deactivation calls Observation and deactivation RPCs now carry the request source, so the fenced standby envtest's epochOverrideCMS must raise their source epoch too, or UpdateObservedGeneration keeps the taken-over component fenced. Bug: 548436300 TAG=agy CONV=d833b0e6-4821-48f6-8576-776d53f55a5c

Restacking onto the #1233 hardening made staleGenerationError SetComponentIntent-only. Per the amended R3, UpdateObservedGeneration, PrepareDeactivation and CompleteDeactivation from the owner object are always allowed at any CR generation, so informer lag never blocks an observation or deactivation: checkSourceMutation now applies only the ownership rules. The parity test asserts that an older owner generation is admitted and keeps the recorded intent generation. Bug: 548436300 TAG=agy CONV=d833b0e6-4821-48f6-8576-776d53f55a5c

…ip no-op heartbeats An adoption (R2 or R5) through UpdateObservedGeneration, PrepareDeactivation or CompleteDeactivation now records SourceGeneration = 0, so the adopter's next intent is never rejected as STALE_GENERATION. An UpdateObservedGeneration that changes no observed generation, state or status leaves UpdateTime, the etag and the snapshot alone in both stores; Spanner still writes the owner columns, refreshing SourceLastSeenTime of a sourced call. Bug: 548436300 TAG=agy CONV=d833b0e6-4821-48f6-8576-776d53f55a5c

…test file Bug: 548436300 TAG=agy CONV=d833b0e6-4821-48f6-8576-776d53f55a5c

…e-driven Hoist the fenced-cluster, other-namespace and other-UID cases shared by the UpdateObservedGeneration, PrepareDeactivation and CompleteDeactivation rejection loops in TestSourcedObservationAndDeactivation_Parity into one named table, run through assertRejectedBy as op/case subtests in the original order and against the same store state. Run the unsourced and sourced unchanged heartbeats in TestUpdateObservedGeneration_UnchangedHeartbeat_Parity as named subtests. Inputs and assertions are unchanged. Bug: 548436300 TAG=agy CONV=d833b0e6-4821-48f6-8576-776d53f55a5c

…ct extra sources - The PrepareDeactivationRequest.source proto comment and the README state the cascade contract: with force, dependents moved to DISABLING are changed on behalf of the target's owner, skip the ownership check and keep their recorded owner. - optionalRequestSource rejects more than one RequestSource instead of silently dropping the extras. - Tests: the cascade keeps a dependent owned by another CR (both backends, SourceLastSeenTime on Spanner), and the bridge projects a rejected UpdateObservedGeneration or CompleteDeactivation as SourceConflict and stops the pass. Comment links: #1238 (comment) #1238 (comment) #1238 (comment) #1238 (comment) #1238 (comment) Bug: 548436300 TAG=agy CONV=d833b0e6-4821-48f6-8576-776d53f55a5c

…lifecycle RPCs Replace the local prepareDeactivationOperation, completeDeactivationOperation and updateObservedGenerationOperation constants with the shared operation* constants in error_info.go, adding operationCompleteDeactivation and operationUpdateObservedGeneration there. Bug: 548436300 TAG=agy CONV=d833b0e6-4821-48f6-8576-776d53f55a5c

VLukiantsev added this pull request to stack #1245 October 13, 2026 07:27

gemini-code-assist Bot reviewed Oct 13, 2026

View reviewed changes

This was referenced Oct 13, 2026

feat(components): send RequestSource from bridge and record status.componentGeneration #1237

Merged

feat(components): isolate per-CR Helm actuation state by UID #1239

Merged

gemini-code-assist Bot reviewed Oct 13, 2026

View reviewed changes

Comment thread pkg/components/store/spanner_store.go Resolved

VLukiantsev force-pushed the feat/components-observation-deactivation-ownership-b548436300 branch from 85a0218 to 91f3a1a Compare October 13, 2026 15:09

VLukiantsev force-pushed the feat/components-observation-deactivation-ownership-b548436300 branch 2 times, most recently from 327c51e to d60679f Compare October 14, 2026 09:37

VLukiantsev force-pushed the feat/components-observation-deactivation-ownership-b548436300 branch from d60679f to 92ac481 Compare October 14, 2026 09:49

VLukiantsev force-pushed the feat/components-observation-deactivation-ownership-b548436300 branch from 92ac481 to ef043b2 Compare October 14, 2026 10:54

VLukiantsev force-pushed the feat/components-observation-deactivation-ownership-b548436300 branch from ef043b2 to fe79a2b Compare October 14, 2026 11:58

VLukiantsev mentioned this pull request Oct 14, 2026

feat(components): adopt a same-cluster recreated Component via expected_owner_uid #1274

Merged

VLukiantsev removed this pull request from stack #1245 October 14, 2026 13:31

VLukiantsev changed the base branch from feat/components-bridge-request-source-b548436300 to feat/components-bridge-recreation-takeover-b548436300 October 14, 2026 13:31

VLukiantsev added this pull request to stack #1277 October 14, 2026 13:32

VLukiantsev force-pushed the feat/components-observation-deactivation-ownership-b548436300 branch 2 times, most recently from 5d2b99e to fbe322b Compare October 14, 2026 14:22

VLukiantsev force-pushed the feat/components-observation-deactivation-ownership-b548436300 branch 2 times, most recently from 5ad765f to b7a0210 Compare October 15, 2026 08:07

VLukiantsev force-pushed the feat/components-observation-deactivation-ownership-b548436300 branch from b7a0210 to 04cdf75 Compare October 15, 2026 08:40

VLukiantsev force-pushed the feat/components-observation-deactivation-ownership-b548436300 branch 2 times, most recently from 475ca0e to 6bfce07 Compare October 16, 2026 13:09

VLukiantsev force-pushed the feat/components-observation-deactivation-ownership-b548436300 branch from 6bfce07 to 1925d33 Compare October 16, 2026 13:26

VLukiantsev force-pushed the feat/components-observation-deactivation-ownership-b548436300 branch from 1925d33 to e70f232 Compare October 16, 2026 13:45

VLukiantsev force-pushed the feat/components-observation-deactivation-ownership-b548436300 branch 2 times, most recently from 1cb3d32 to a7b52de Compare October 16, 2026 15:16

VLukiantsev force-pushed the feat/components-observation-deactivation-ownership-b548436300 branch from a7b52de to 1a4a84f Compare October 16, 2026 15:27

VLukiantsev force-pushed the feat/components-observation-deactivation-ownership-b548436300 branch from 1a4a84f to 3c02b01 Compare October 19, 2026 07:21