Edge pod attachments: unchecked caller inputs and weak isolation tests #989

Activity

1. ecv commented on Oct 22, 2026

   

   ecv

   on Oct 22, 2026

   ContributorAuthor

   More actions

   Detail by source PR:

   #947, admission

   • Admit takes grants and req.VPC from the caller and binds neither to a tenant identity. An empty req.VPC or provider is not rejected, so a grant with an empty VPC matches. Reject both in validate, and document the caller contract.
   • A request with no prefixes skips the prefix check. Say so, or require at least one prefix.
   • Core may set any non-Ready condition type. Use a closed list.

   #950, datapath

   • Attachment.MTU must be at least 1280 and has no upper bound against the encapsulated underlay. Readback now reports MTU drift, so attachments made by a feat(edge): Attach socket and packet consumer pods to a VPC from the host #949 build will read as drifted.
   • datapath_test.go shows identical addresses answering with different identities but never sends a packet that must not arrive, such as an address registered only in VPC B to VPC A's SID. Add one negative per direction.
   • TUN attachments register as attachreg.InterfaceTypeVeth. Confirm that matches what the CNI registers for a packet consumer.

   #951, policy

   • Policy is bound only on VPC A's attachments. Add a VPC B dial that must fail while A's policy allows.
   • The ICMP denial case has no deny-counter assertion.

   #952, real-node test

   • attachedPrograms treats a FilterList error as no programs, and the control checkCNIProgramsVisible runs only when GALACTIC_EDGE_TARGET_BPF_CNI is set, so the CNI negative can pass vacuously.
   • Cleanup errors are discarded and nothing asserts the VRF, table 0x7EDC0001 and edgefx-* links are gone from the live node.
   • TestTargetConsumer sleeps 10 minutes by default, equal to go test's default timeout.
   • No run on a real node is recorded on the PR.
2. ecv

   added

   tech-debtInternal quality/maintenance: unwind shortcuts, config drift, overdue upgrades, fork/dep hygieneInternal quality/maintenance: unwind shortcuts, config drift, overdue upgrades, fork/dep hygiene

   on Oct 22, 2026
3. ecv commented on Oct 22, 2026

   

   ecv

   on Oct 22, 2026

   ContributorAuthor

   More actions

   From #945 at 762314d and #946 at eb52c8f (both approved once their blockers were fixed):

   • Writer uniqueness, state-directory lifetime and the missing deadline on mutate and Fence are documented but not enforced. The wiring PR has to honor them.
   • claim.Instance holds one mutex across store calls, so a slow Acquire or Release on one key delays Renew on another and can let that lease lapse. Document one instance per key, or drop the lock around store I/O.
   • The claim package has no production Duration, MaxDrift or Margin yet, and its safety depends on the store never reusing versions. Check both when the store adapter lands.
4. ecv commented on Oct 22, 2026

   

   ecv

   on Oct 22, 2026

   ContributorAuthor

   More actions

   From #949 at 217bc0c (approved once its blockers were fixed):

   • verifyPair compares hostLink.NetNsID with the pod's namespace ID, but netlink reports 0 when IFLA_LINK_NETNSID is absent, which is the case for a same-namespace peer. A pod namespace that was assigned ID 0 therefore matches a host link whose peer is in the host namespace. The reciprocal index check has to line up as well, so this is contrived. Treat the ID as meaningful only when the attribute is present.
   • TestPeerNamespaceOwnership passes even without the namespace check, because its impostor already fails the index check. Add a case where only the namespace differs.
   • A failed re-provision downs a live attachment, so a failed drift repair takes it offline.
   • The renamed recovery test no longer checks traffic after recovery.
   • Extra routes through the pod link are neither pruned nor reported.

   Still open after the #950 to #952 rewrite: isolation negatives per direction (counters are compared only after both pings), TUN registration as veth, the VPC B policy case, the ICMP counter, the opt-in CNI control, cleanup assertions, the 10-minute sleep, and a recorded real-node run.