Repository navigation
refactor(gateway): Drop NetworkRule's vpcAttachmentRef - #820
Conversation
A NetworkRule's backends come from backendSelector, which matches any number of VPCAttachments in the rule's VPC, so a single attachment reference no longer identifies anything. Galactic never read it: the gateway copied it into DesiredRule, compared it in rulesEqual (so changing it alone reprogrammed an unchanged rule), and the admission webhook printed it in its denial message. Remove it from DesiredRule, rulesEqual, the webhook message and comments, the test fixtures, the six containerlab NetworkRules and the gateway docs. Correct DesiredRule's comment: VPCRef feeds the per-tenant rule quota, not only telemetry. Also fix ARCHITECTURE-GATEWAY.md's NetworkRule row, which still listed the static backends field the selector replaced. Bumps go.datum.net/network to the commit that removes the field (datum-cloud/network#31). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Point at network main now that datum-cloud/network#31, which drops NetworkRule's vpcAttachmentRef, has merged, rather than at that PR's branch commit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Second pass on head 53068f9, both reviewers agreed. pr-rereviewer verdict: "VERDICT: merge. Both body findings are settled in the current PR body, the head is unchanged, and the decided merge order raises no open items." What each reviewer ran. pr-rereviewer ran gh pr view for the body and head, git rev-parse HEAD, and git log --show-signature on the range 1f6410b..53068f9. pr-conventions-reviewer ran gh pr view, git diff --stat, gh pr list, git diff on go.mod, internal and docs, git grep for vpcAttachmentRef, git log, go build ./..., go vet, go list -m, a diff of the network module versions, and a diff of egresspolicy_types.go. Applied in this fix phase, with no new commit: the test plan now names the lab's gateway ingress check in words instead of the task identifier, and the Deployment section says this PR merges before #738, #813, #815, #817 and #818, which rebase after it. The e2e box is ticked because the E2E Tests job passed. The lab box stays open. Skipped: the nit to fold the two go.mod bump commits into one. Rewriting history would need another review for a cosmetic gain, so the branch keeps both. CI on the head: Build, Lint, Unit Tests, Unit Tests (root), E2E Tests, the publish image builds and the kustomize bundle publish all passed. The PR had checks, so none were skipped. Merge method: merge, from the main ruleset. Auto-merge is enabled. The ruleset requires one approving review, code owner review and approval of the last push, and it dismisses stale reviews on push, so the merge waits for a human approval. No human reviewer is configured for this repository, so code owners are the only requested reviewers. |
2 participants
Summary
A NetworkRule's backends come from its selector, which can match many VPCAttachments, so the single-attachment reference no longer identifies anything. datum-cloud/network#31 removed it from the API.
This change drops the field from the gateway, the admission webhook, the lab manifests and the docs. The gateway no longer reprograms a rule when only that field changes.
Deployment
The containerlab lab and staging install network CRDs from network main, which already rejects the field. The lab's NetworkRules fail to apply until this merges. The staging lab's NetworkRule needs the same one-line removal.
This PR should merge before the open PRs that touch the same files (#738, #813, #815, #817, #818), which rebase after it.
Test plan
🤖 Generated with Claude Code