feat: SetNetworkConfig — forwarding / masquerade / isolation (decision 16)#7
Open
bodaay wants to merge 1 commit into
Open
feat: SetNetworkConfig — forwarding / masquerade / isolation (decision 16)#7bodaay wants to merge 1 commit into
bodaay wants to merge 1 commit into
Conversation
…n 16)
Close the correctness gap helm called out: the admin UI has been letting
operators toggle forwarding / masquerade / isolation since helm M5, but
no caller pushed the result down to a node, so the rules never took
effect. helm PR #30 wires the controller side (helm nodes push-policy);
this lands the buoy side.
- internal/netpolicy: new package owning the on-node application of
NetworkConfig (DESIGN §3, decision 16). Policy.Validate repeats helm's
rejection of masquerade/isolation without forwarding. NftApplier
renders an nftables ruleset (its own 'pharos_buoy' table, atomically
reset via 'add table; delete table; table {…}') and writes the
/proc/sys ip_forward switches. Manager wraps the applier with an
in-memory last-applied snapshot for idempotent replays — a second
identical call returns applied=false without re-running nft.
- internal/control/service.go: SetNetworkConfig RPC — validates the
three booleans, calls netpolicy.Manager.Apply, returns
SetNetworkConfigResponse{Applied}. InvalidArgument on a missing or
helm-prevalidated-rejectable Policy; Internal on applier failure.
- internal/cli/run.go: wires netpolicy.NewManager(NewNftApplier()) into
control.Options.NetPolicy. The applier is system-resolved at call
time (nft on PATH, /proc/sys defaults).
- Tests: Policy.Validate truth table; nft renderer per combination
(forwarding-only emits no table body; masquerade and isolation rules
appear/disappear individually); NftApplier exercised with a
shell-stub nft + tempdir /proc paths (no root needed); Manager
idempotency (applies on change, no-ops on replay, doesn't poison
cache on failure); SetNetworkConfig over mTLS — applies → replays
→ applies on change → InvalidArgument for masquerade-without-forwarding.
Existing 'unimplemented canary' in server_test moved from
SetNetworkConfig to RestartService (still Unimplemented).
- BUILD.md: record the network-policy section and the nftables choice.
Signed-off-by: Khalefa <Khalefa@alahmad.org>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the correctness gap helm called out: the admin UI has let operators toggle forwarding / masquerade / isolation since helm M5, but no caller pushed the result to a node, so the rules never took effect. helm PR #30 wires the controller side (
helm nodes push-policy); this PR is the buoy half.What it does
internal/netpolicy— new package owning the on-node application ofNetworkConfig(DESIGN §3, decision 16).Policy.Validatemirrors helm's rejection ofmasquerade/isolationwithoutforwarding(defense in depth).NftApplierrenders an nftables ruleset into its ownpharos_buoytable, applied atomically viaadd table; delete table; table { … }(the idempotent-reset idiom — works whether the table existed before or not). The/proc/sysforwarding switches are written separately for v4 and v6.Managerwraps the applier with an in-memory last-applied snapshot — an identical second call returnsapplied=falsewithout re-runningnft. A failed Apply does not poison the cache; the next call still runs.SetNetworkConfigRPC — validates the three booleans, callsManager.Apply, returnsSetNetworkConfigResponse{applied}.InvalidArgumenton missing config or invalid combinations;Internalon applier failure.cli/run.gowiresnetpolicy.NewManager(NewNftApplier())intocontrol.Options.NetPolicy. The applier resolvesnftand/proc/sys/...at call time, so production picks up system defaults and tests inject paths.Design notes
awg0; the interface name isDefaultWGInterfacewith an injection point if helm ever needs to drive a non-default interface.Tests
`gofmt` / `vet` / `golangci-lint` clean; `go test -race ./...` green.
Policy.Validatetruth table — every combination of the three booleans, plus the two invalid ones (masquerade/isolationwithoutforwarding).NftApplierexercised end-to-end with a shell-stubnftand tempdir `/proc/sys` paths — captures the nft stdin and asserts the forwarding switches flip. No root needed.Manageridempotency — applies on change, no-ops on identical replay, doesn't poison the cache on a failed Apply.SetNetworkConfigover mTLS — applies → replays as no-op → re-applies on change → returnsInvalidArgumentfor masquerade-without-forwarding and for missing config.server_test.gomoved fromSetNetworkConfigtoRestartService(still Unimplemented) so the invariant keeps being verified.Next
Per your sequence: B3 XRay when you open the docs PR defining
XRayConfig, then B6 packaging.