feat(api)!: collapse typed CIDR fields into flat allowedNetworks#21
Conversation
BREAKING CHANGE: the ClusterEntry typed CIDR fields (podCIDRs, wireguardCIDR, serviceCIDR, additionalCIDRs) are replaced by a single flat allowedNetworks []string. Validation and Peer construction treat every entry uniformly: a node is eligible when its PodCIDR is a subset of some entry and its wireguard-ip host IP falls within some entry. The anchor now advertises only the residual allowedNetworks entries that no per-node Peer already carries (CollectAnchorCIDRs became node-aware): pod aggregates and the WG CIDR are dropped because the per-node Peers announce them, while service CIDRs and host-network ranges survive into the anchor. No conversion/migration is provided. Both CRD copies (config/crd/bases and the embedded internal/crd) are regenerated and synced. Tests: existing ClusterEntry constructions migrated to allowedNetworks; added unit tests for the node-aware CollectAnchorCIDRs and for the discovered-endpoint logic (kilonode.DiscoveredEndpointsByKey, parseDiscoveredEndpoint, enrichEndpointsFromDiscovered). Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (20)
📝 WalkthroughWalkthroughReplaces the multiple per-cluster CIDR fields ( ChangesAllowedNetworks API migration
Discovered-endpoint test coverage
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request simplifies the ClusterMesh configuration by replacing several specific network fields (podCIDRs, wireguardCIDR, serviceCIDR, and additionalCIDRs) in ClusterEntry with a single flat list called allowedNetworks. This change unifies validation and peer construction, with any networks not covered by individual nodes being dynamically folded into the anchor peer. Feedback on the changes suggests optimizing the CollectAnchorCIDRs function by pre-parsing node CIDRs and IPs to avoid redundant string parsing operations inside nested loops.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| func CollectAnchorCIDRs(entry *v1alpha1.ClusterEntry, nodes []*corev1.Node) []string { | ||
| var residual []string | ||
|
|
||
| for _, networkStr := range entry.AllowedNetworks { | ||
| network, err := netutil.ParseCIDR(networkStr) | ||
| if err != nil { | ||
| // Keep unparseable entries verbatim: validation never trusted | ||
| // these for announcement either, and dropping them would | ||
| // silently discard operator intent. | ||
| residual = append(residual, networkStr) | ||
|
|
||
| continue | ||
| } | ||
|
|
||
| if !networkCoveredByAnyNode(network, nodes) { | ||
| residual = append(residual, networkStr) | ||
| } | ||
| } | ||
|
|
||
| return residual | ||
| } | ||
|
|
||
| // networkCoveredByAnyNode reports whether some node already advertises network | ||
| // via its per-node Peer: either the node's first PodCIDR is a subset of | ||
| // network, or the host IP of its kilo.squat.ai/wireguard-ip annotation falls | ||
| // within network. Nodes with missing/invalid PodCIDR or wireguard-ip do not | ||
| // cover anything. | ||
| func networkCoveredByAnyNode(network *net.IPNet, nodes []*corev1.Node) bool { | ||
| for _, node := range nodes { | ||
| if nodeCoversNetwork(network, node) { | ||
| return true | ||
| } | ||
| } | ||
|
|
||
| return false | ||
| } | ||
|
|
||
| if entry.ServiceCIDR != "" { | ||
| cidrs = append(cidrs, entry.ServiceCIDR) | ||
| // nodeCoversNetwork reports whether a single node already advertises network, | ||
| // either through its first PodCIDR (subset of network) or through the host IP | ||
| // of its kilo.squat.ai/wireguard-ip annotation (contained in network). A | ||
| // missing or unparseable PodCIDR / wireguard-ip simply does not cover. | ||
| func nodeCoversNetwork(network *net.IPNet, node *corev1.Node) bool { | ||
| if len(node.Spec.PodCIDRs) > 0 { | ||
| nodeCIDR, err := netutil.ParseCIDR(node.Spec.PodCIDRs[0]) | ||
| if err == nil && netutil.CIDRContains(network, nodeCIDR) { | ||
| return true | ||
| } | ||
| } | ||
|
|
||
| cidrs = append(cidrs, entry.AdditionalCIDRs...) | ||
| wgIP := node.Annotations[kilonode.AnnotationWireguardIP] | ||
| if wgIP == "" { | ||
| return false | ||
| } | ||
|
|
||
| hostIP, _, err := netutil.ParseHostInCIDR(wgIP) | ||
| if err != nil { | ||
| return false | ||
| } | ||
|
|
||
| return cidrs | ||
| return network.Contains(hostIP) | ||
| } |
There was a problem hiding this comment.
The current implementation of CollectAnchorCIDRs loops over all AllowedNetworks and, for each network, iterates through all nodes to check if the network is covered. Inside nodeCoversNetwork, it repeatedly parses the node's PodCIDR and wireguard-ip from strings. This results in
We can optimize this to
type parsedNode struct {
podCIDR *net.IPNet
wgIP net.IP
}
func CollectAnchorCIDRs(entry *v1alpha1.ClusterEntry, nodes []*corev1.Node) []string {
var residual []string
parsedNodes := make([]parsedNode, 0, len(nodes))
for _, node := range nodes {
pn := parsedNode{}
if len(node.Spec.PodCIDRs) > 0 {
if cidr, err := netutil.ParseCIDR(node.Spec.PodCIDRs[0]); err == nil {
pn.podCIDR = cidr
}
}
if wgIP := node.Annotations[kilonode.AnnotationWireguardIP]; wgIP != "" {
if hostIP, _, err := netutil.ParseHostInCIDR(wgIP); err == nil {
pn.wgIP = hostIP
}
}
if pn.podCIDR != nil || pn.wgIP != nil {
parsedNodes = append(parsedNodes, pn)
}
}
for _, networkStr := range entry.AllowedNetworks {
network, err := netutil.ParseCIDR(networkStr)
if err != nil {
residual = append(residual, networkStr)
continue
}
covered := false
for _, pn := range parsedNodes {
if pn.podCIDR != nil && netutil.CIDRContains(network, pn.podCIDR) {
covered = true
break
}
if pn.wgIP != nil && network.Contains(pn.wgIP) {
covered = true
break
}
}
if !covered {
residual = append(residual, networkStr)
}
}
return residual
}1 participant
Summary
Collapses the four typed CIDR fields on
ClusterEntry(podCIDRs,wireguardCIDR,serviceCIDR,additionalCIDRs) into a single flatallowedNetworks []string.The typed split carried no real semantic value:
serviceCIDRandadditionalCIDRswere handled identically (folded into the anchor Peer), andpodCIDRs/wireguardCIDRserved only as per-node validation bounds — the values actually advertised come from each node's ownNode.Spec.PodCIDRs/wireguard-ip. A flat list does everything the typed fields did, keeps the "nothing outsideallowedNetworkscan appear in a Peer" bound, and removes the two-ways-to-express-the-same-CIDR footgun. Aligns with the upstream ClusterMesh design direction.Changes
ClusterEntry: typed CIDR fields ->allowedNetworks(MinItems=1);wireguardPort,persistentKeepalive,name,local,kubeconfigSecretRefunchanged;AllCIDRs()returnsallowedNetworks.PodCIDRis a subset of someallowedNetworksentry and itswireguard-iphost falls within some entry.CollectAnchorCIDRs(entry, nodes)returns only the entries no node already represents (service CIDR, host-network ranges); pod aggregates and the WG CIDR are carried by per-node Peers and no longer double-advertised.config/crd/basesand the embeddedinternal/crdcopy, now in sync) and deepcopy.Tests
allowedNetworks.TestCollectAnchorCIDRs(service/host-net -> anchor; pod/WG aggregates -> not).DiscoveredEndpointsByKey,parseDiscoveredEndpoint,enrichEndpointsFromDiscovered).go build,go vet, lint, and unit tests green; integration tests compile (need envtest to run).Breaking change - no conversion
Intentional breaking CRD change with no conversion webhook / v1alpha2. Existing
ClusterMeshresources must be re-written toallowedNetworks. The downstreamkubernetes-switchcloudchart (which emits the typed fields) must be updated in lockstep.Summary by CodeRabbit
Release Notes
Breaking Changes
podCIDRs,wireguardCIDR,serviceCIDR, andadditionalCIDRsfields replaced with singleallowedNetworksfield containing all network ranges.New Features
persistentKeepalivefield to control WireGuard keepalive behavior (0-65535 seconds).Documentation
allowedNetworkssemantics and configuration format.