diff --git a/README.md b/README.md index 095b0f4..6137d9a 100644 --- a/README.md +++ b/README.md @@ -26,7 +26,7 @@ The operator runs on a single cluster and reaches remote clusters via kubeconfig - **Multi-cluster WireGuard mesh** — declarative `ClusterMesh` CRD bridges any number of clusters - **Fork-aware Kilo support** — accepts WireGuard IP annotations in both upstream (`/32`) and Cozystack-patched (`/`) form; normalises to host routes automatically - **Endpoint resolution chain** — per-node endpoint determined by priority: `clustermesh-endpoint` annotation → `force-endpoint` annotation → Node `ExternalIP` combined with `wireguardPort`; nodes with no resolvable endpoint are skipped cleanly -- **Anchor peers** — a single per-cluster anchor `Peer` advertises `serviceCIDR` and `additionalCIDRs` so service and host-network ranges are reachable across clusters +- **Anchor peers** — a single per-cluster anchor `Peer` advertises the `allowedNetworks` entries that no individual node already carries (e.g. service and host-network ranges) so they are reachable across clusters - **Embedded CRD bootstrap** — the operator self-applies its CRD at startup; no separate CRD pre-install step required - **Safe cluster reconfiguration** — a change-watcher triggers a controlled pod restart when cluster topology or kubeconfig Secrets change, rebuilding the client registry from scratch - **Finalizer-based cleanup** — removing a `ClusterMesh` CR triggers deletion of all managed `Peer` objects on every cluster before the resource is released @@ -190,18 +190,20 @@ spec: clusters: - name: cluster-a local: true - podCIDRs: ["10.1.0.0/16"] - wireguardCIDR: "10.200.0.0/24" + allowedNetworks: # pod, WireGuard and service CIDRs, all in one flat list + - "10.1.0.0/16" + - "10.200.0.0/24" + - "10.96.0.0/12" wireguardPort: 51820 # default; set explicitly if your cluster uses a different port - serviceCIDR: "10.96.0.0/12" - name: cluster-b kubeconfigSecretRef: name: cluster-b-kubeconfig key: kubeconfig - podCIDRs: ["10.2.0.0/16"] - wireguardCIDR: "10.200.1.0/24" + allowedNetworks: + - "10.2.0.0/16" + - "10.200.1.0/24" + - "10.112.0.0/12" wireguardPort: 51820 - serviceCIDR: "10.112.0.0/12" ``` > **Warning:** Pod CIDRs, WireGuard CIDRs, and service CIDRs must not overlap between any two clusters in the same namespace. Overlapping CIDRs block reconciliation for all affected meshes. @@ -210,7 +212,7 @@ spec: ## How It Works -On each reconcile cycle, the operator connects to every cluster in the `ClusterMesh` spec, lists all `Node` objects, validates each node's pod CIDR and WireGuard IP against the declared spec, and creates or updates Kilo `Peer` objects accordingly. Nodes that fail validation or have no resolvable endpoint are skipped. For each cluster that declares a `serviceCIDR` or `additionalCIDRs`, an anchor `Peer` carrying those CIDRs is also created on every other cluster. The operator uses a finalizer to clean up all managed peers when a `ClusterMesh` resource is deleted. +On each reconcile cycle, the operator connects to every cluster in the `ClusterMesh` spec, lists all `Node` objects, validates that each node's pod CIDR and WireGuard IP fall within the cluster's declared `allowedNetworks`, and creates or updates Kilo `Peer` objects accordingly. Nodes that fail validation or have no resolvable endpoint are skipped. Any `allowedNetworks` entry that no individual node already advertises (e.g. the service CIDR or host-network ranges) is folded into a single anchor `Peer` on every other cluster. The operator uses a finalizer to clean up all managed peers when a `ClusterMesh` resource is deleted. See [./docs/architecture.md](./docs/architecture.md) for the full reconciliation flow and component details. diff --git a/api/v1alpha1/clustermesh_types.go b/api/v1alpha1/clustermesh_types.go index 4c1476a..27e017d 100644 --- a/api/v1alpha1/clustermesh_types.go +++ b/api/v1alpha1/clustermesh_types.go @@ -46,17 +46,17 @@ type ClusterEntry struct { // +optional KubeconfigSecretRef *SecretKeyRef `json:"kubeconfigSecretRef,omitempty"` - // PodCIDRs is the list of pod network CIDRs for this cluster. - // Node.Spec.PodCIDRs must be a subset of these CIDRs. + // AllowedNetworks is the flat list of every CIDR this cluster contributes + // to the mesh: pod CIDRs, the WireGuard (kilo0) CIDR, the service CIDR, + // host-network ranges, external subnets, and so on. There is no typed + // distinction between them — both validation and Peer construction treat + // every entry uniformly. A node is eligible when its PodCIDR is a subset + // of some entry and its kilo.squat.ai/wireguard-ip host IP falls within + // some entry; entries that have no per-node representative (e.g. the + // service CIDR or host-network ranges) are advertised via the anchor Peer. // Multiple entries support dual-stack (IPv4 + IPv6). // +kubebuilder:validation:MinItems=1 - PodCIDRs []string `json:"podCIDRs"` //nolint:tagliatelle // "podCIDRs" is the canonical field name; "CIDR" is a well-known acronym - - // WireguardCIDR is the CIDR for Kilo's WireGuard interface (kilo0) addresses. - // Each node's kilo.squat.ai/wireguard-ip must have its host IP within this CIDR. - // The annotation may carry any prefix length (e.g. "10.4.0.1/32" upstream Kilo - // or "10.4.0.1/16" cozystack-patched Kilo); only the host portion is validated. - WireguardCIDR string `json:"wireguardCIDR"` + AllowedNetworks []string `json:"allowedNetworks"` // WireguardPort is the UDP port of Kilo's WireGuard endpoint on each node in // this cluster. Used as a fallback when the operator synthesises the @@ -69,17 +69,6 @@ type ClusterEntry struct { // +optional WireguardPort uint16 `json:"wireguardPort,omitempty"` - // ServiceCIDR is the Kubernetes service network CIDR for this cluster. - // If set, it will be advertised via an anchor Peer so that services - // in this cluster are reachable from other mesh members. - // +optional - ServiceCIDR string `json:"serviceCIDR,omitempty"` - - // AdditionalCIDRs are extra CIDRs to advertise into the mesh - // (e.g., host-network ranges, external subnets). - // +optional - AdditionalCIDRs []string `json:"additionalCIDRs,omitempty"` //nolint:tagliatelle // "additionalCIDRs" is the canonical field name; "CIDR" is a well-known acronym - // PersistentKeepalive is the interval in seconds at which WireGuard // sends keepalive packets to peers in this cluster. Set to a non-zero // value (e.g. 25) for clusters behind NAT so that the stateful NAT @@ -92,21 +81,11 @@ type ClusterEntry struct { PersistentKeepalive int `json:"persistentKeepalive,omitempty"` } -// AllCIDRs returns the union of all CIDRs declared by this cluster entry. -// The order is: podCIDRs, wireguardCIDR, serviceCIDR (if set), additionalCIDRs. +// AllCIDRs returns every CIDR declared by this cluster entry. With the flat +// model this is simply the AllowedNetworks list; it is consumed by the +// cross-mesh overlap validation, whose behaviour is unchanged. func (c *ClusterEntry) AllCIDRs() []string { - cidrs := make([]string, 0, len(c.PodCIDRs)+1+1+len(c.AdditionalCIDRs)) - cidrs = append(cidrs, c.PodCIDRs...) - - cidrs = append(cidrs, c.WireguardCIDR) - - if c.ServiceCIDR != "" { - cidrs = append(cidrs, c.ServiceCIDR) - } - - cidrs = append(cidrs, c.AdditionalCIDRs...) - - return cidrs + return c.AllowedNetworks } // SecretKeyRef identifies a key within a Kubernetes Secret. diff --git a/api/v1alpha1/clustermesh_types_test.go b/api/v1alpha1/clustermesh_types_test.go index b141917..01afede 100644 --- a/api/v1alpha1/clustermesh_types_test.go +++ b/api/v1alpha1/clustermesh_types_test.go @@ -57,17 +57,13 @@ func TestClusterMeshJSONRoundTrip(t *testing.T) { { Name: "local-cluster", Local: true, - PodCIDRs: []string{"10.0.0.0/16", "fd00::/48"}, - WireguardCIDR: "172.30.0.0/24", + AllowedNetworks: []string{"10.0.0.0/16", "fd00::/48", "172.30.0.0/24", "10.96.0.0/12", "192.168.100.0/24"}, WireguardPort: 51820, - ServiceCIDR: "10.96.0.0/12", - AdditionalCIDRs: []string{"192.168.100.0/24"}, }, { Name: "remote-cluster", KubeconfigSecretRef: secretRef, - PodCIDRs: []string{"10.1.0.0/16"}, - WireguardCIDR: "172.30.1.0/24", + AllowedNetworks: []string{"10.1.0.0/16", "172.30.1.0/24"}, WireguardPort: 52000, }, }, @@ -119,9 +115,7 @@ func TestClusterMeshDeepCopy(t *testing.T) { { Name: "c1", Local: true, - PodCIDRs: []string{"10.0.0.0/16"}, - WireguardCIDR: "172.30.0.0/24", - AdditionalCIDRs: []string{"192.168.0.0/24"}, + AllowedNetworks: []string{"10.0.0.0/16", "172.30.0.0/24", "192.168.0.0/24"}, }, }, }, @@ -132,17 +126,17 @@ func TestClusterMeshDeepCopy(t *testing.T) { // Mutate the original after copying. original.Spec.Clusters[0].Name = "mutated" - original.Spec.Clusters[0].PodCIDRs[0] = "99.99.99.0/24" - original.Spec.Clusters[0].AdditionalCIDRs[0] = "1.2.3.0/24" + original.Spec.Clusters[0].AllowedNetworks[0] = "99.99.99.0/24" + original.Spec.Clusters[0].AllowedNetworks[2] = "1.2.3.0/24" // The copy must be unchanged. assert.Equal(t, "c1", copied.Spec.Clusters[0].Name) - assert.Equal(t, "10.0.0.0/16", copied.Spec.Clusters[0].PodCIDRs[0]) - assert.Equal(t, "192.168.0.0/24", copied.Spec.Clusters[0].AdditionalCIDRs[0]) + assert.Equal(t, "10.0.0.0/16", copied.Spec.Clusters[0].AllowedNetworks[0]) + assert.Equal(t, "192.168.0.0/24", copied.Spec.Clusters[0].AllowedNetworks[2]) } -// TestClusterEntryAllCIDRs verifies that AllCIDRs returns the correct union -// for various combinations of optional fields. +// TestClusterEntryAllCIDRs verifies that AllCIDRs returns the flat +// AllowedNetworks list verbatim, preserving order. func TestClusterEntryAllCIDRs(t *testing.T) { t.Parallel() @@ -152,50 +146,26 @@ func TestClusterEntryAllCIDRs(t *testing.T) { want []string }{ { - name: "required fields only", + name: "single entry", entry: v1alpha1.ClusterEntry{ - PodCIDRs: []string{"10.0.0.0/16"}, - WireguardCIDR: "172.30.0.0/24", + AllowedNetworks: []string{"10.0.0.0/16"}, }, - want: []string{"10.0.0.0/16", "172.30.0.0/24"}, - }, - { - name: "with service CIDR", - entry: v1alpha1.ClusterEntry{ - PodCIDRs: []string{"10.0.0.0/16"}, - WireguardCIDR: "172.30.0.0/24", - ServiceCIDR: "10.96.0.0/12", - }, - want: []string{"10.0.0.0/16", "172.30.0.0/24", "10.96.0.0/12"}, + want: []string{"10.0.0.0/16"}, }, { - name: "with additional CIDRs", + name: "pod and wireguard CIDRs", entry: v1alpha1.ClusterEntry{ - PodCIDRs: []string{"10.0.0.0/16"}, - WireguardCIDR: "172.30.0.0/24", - AdditionalCIDRs: []string{"192.168.1.0/24", "192.168.2.0/24"}, + AllowedNetworks: []string{"10.0.0.0/16", "172.30.0.0/24"}, }, - want: []string{"10.0.0.0/16", "172.30.0.0/24", "192.168.1.0/24", "192.168.2.0/24"}, + want: []string{"10.0.0.0/16", "172.30.0.0/24"}, }, { - name: "all fields set", + name: "with service and additional CIDRs", entry: v1alpha1.ClusterEntry{ - PodCIDRs: []string{"10.0.0.0/16", "fd00::/48"}, - WireguardCIDR: "172.30.0.0/24", - ServiceCIDR: "10.96.0.0/12", - AdditionalCIDRs: []string{"192.168.0.0/24"}, + AllowedNetworks: []string{"10.0.0.0/16", "fd00::/48", "172.30.0.0/24", "10.96.0.0/12", "192.168.0.0/24"}, }, want: []string{"10.0.0.0/16", "fd00::/48", "172.30.0.0/24", "10.96.0.0/12", "192.168.0.0/24"}, }, - { - name: "empty service CIDR not included", - entry: v1alpha1.ClusterEntry{ - PodCIDRs: []string{"10.0.0.0/16"}, - WireguardCIDR: "172.30.0.0/24", - ServiceCIDR: "", - }, - want: []string{"10.0.0.0/16", "172.30.0.0/24"}, - }, } for _, tc := range tests { diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index dd285d8..f24171a 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -33,13 +33,8 @@ func (in *ClusterEntry) DeepCopyInto(out *ClusterEntry) { *out = new(SecretKeyRef) **out = **in } - if in.PodCIDRs != nil { - in, out := &in.PodCIDRs, &out.PodCIDRs - *out = make([]string, len(*in)) - copy(*out, *in) - } - if in.AdditionalCIDRs != nil { - in, out := &in.AdditionalCIDRs, &out.AdditionalCIDRs + if in.AllowedNetworks != nil { + in, out := &in.AllowedNetworks, &out.AllowedNetworks *out = make([]string, len(*in)) copy(*out, *in) } diff --git a/cmd/main_test.go b/cmd/main_test.go index bf57936..b342ad6 100644 --- a/cmd/main_test.go +++ b/cmd/main_test.go @@ -39,9 +39,8 @@ func sourceFromEntry(meshNamespace string, e kilov1alpha1.ClusterEntry) multiclu func entry(name string, podCIDR string) kilov1alpha1.ClusterEntry { return kilov1alpha1.ClusterEntry{ - Name: name, - PodCIDRs: []string{podCIDR}, - WireguardCIDR: "10.4.0.0/16", + Name: name, + AllowedNetworks: []string{podCIDR, "10.4.0.0/16"}, } } diff --git a/config/crd/bases/kilo.squat.ai_clustermeshes.yaml b/config/crd/bases/kilo.squat.ai_clustermeshes.yaml index 9274636..fa96fef 100644 --- a/config/crd/bases/kilo.squat.ai_clustermeshes.yaml +++ b/config/crd/bases/kilo.squat.ai_clustermeshes.yaml @@ -54,12 +54,20 @@ spec: description: ClusterEntry describes a single cluster participating in the mesh. properties: - additionalCIDRs: + allowedNetworks: description: |- - AdditionalCIDRs are extra CIDRs to advertise into the mesh - (e.g., host-network ranges, external subnets). + AllowedNetworks is the flat list of every CIDR this cluster contributes + to the mesh: pod CIDRs, the WireGuard (kilo0) CIDR, the service CIDR, + host-network ranges, external subnets, and so on. There is no typed + distinction between them — both validation and Peer construction treat + every entry uniformly. A node is eligible when its PodCIDR is a subset + of some entry and its kilo.squat.ai/wireguard-ip host IP falls within + some entry; entries that have no per-node representative (e.g. the + service CIDR or host-network ranges) are advertised via the anchor Peer. + Multiple entries support dual-stack (IPv4 + IPv6). items: type: string + minItems: 1 type: array kubeconfigSecretRef: description: |- @@ -100,28 +108,6 @@ spec: maximum: 65535 minimum: 0 type: integer - podCIDRs: - description: |- - PodCIDRs is the list of pod network CIDRs for this cluster. - Node.Spec.PodCIDRs must be a subset of these CIDRs. - Multiple entries support dual-stack (IPv4 + IPv6). - items: - type: string - minItems: 1 - type: array - serviceCIDR: - description: |- - ServiceCIDR is the Kubernetes service network CIDR for this cluster. - If set, it will be advertised via an anchor Peer so that services - in this cluster are reachable from other mesh members. - type: string - wireguardCIDR: - description: |- - WireguardCIDR is the CIDR for Kilo's WireGuard interface (kilo0) addresses. - Each node's kilo.squat.ai/wireguard-ip must have its host IP within this CIDR. - The annotation may carry any prefix length (e.g. "10.4.0.1/32" upstream Kilo - or "10.4.0.1/16" cozystack-patched Kilo); only the host portion is validated. - type: string wireguardPort: default: 51820 description: |- @@ -134,9 +120,8 @@ spec: minimum: 1 type: integer required: + - allowedNetworks - name - - podCIDRs - - wireguardCIDR type: object minItems: 2 type: array diff --git a/internal/controller/clustermesh_controller.go b/internal/controller/clustermesh_controller.go index 31cdeb1..d7aeaf8 100644 --- a/internal/controller/clustermesh_controller.go +++ b/internal/controller/clustermesh_controller.go @@ -795,18 +795,19 @@ func (r *ClusterMeshReconciler) updateStatus(ctx context.Context, mesh *v1alpha1 } // buildDesiredPeers constructs the desired Peer slice for all valid nodes. -// The first valid node carries the cluster-wide CIDRs (serviceCIDR and any -// AdditionalCIDRs) folded into its Peer.AllowedIPs. The older design emitted -// a separate anchor Peer that reused the anchor node's WireGuard public key; -// WireGuard's per-pubkey dedup made the second `wg setconf` call to apply -// either the node or the anchor entry clobber the AllowedIPs of the other, -// silently losing pod-CIDR or service-CIDR routing in a racy way. Folding -// the anchor CIDRs into the first node Peer keeps a single WG peer entry -// per pubkey with the full union of AllowedIPs. +// The first valid node carries the cluster-wide CIDRs from AllowedNetworks +// that are not covered by any per-node value (e.g. the service CIDR or +// host-network ranges) folded into its Peer.AllowedIPs. The older design +// emitted a separate anchor Peer that reused the anchor node's WireGuard +// public key; WireGuard's per-pubkey dedup made the second `wg setconf` call +// to apply either the node or the anchor entry clobber the AllowedIPs of the +// other, silently losing pod-CIDR or service-CIDR routing in a racy way. +// Folding the anchor CIDRs into the first node Peer keeps a single WG peer +// entry per pubkey with the full union of AllowedIPs. func buildDesiredPeers(meshName string, entry *v1alpha1.ClusterEntry, nodes []*corev1.Node) ([]*kilov1alpha1.Peer, error) { peers := make([]*kilov1alpha1.Peer, 0, len(nodes)) - anchorExtras := peer.CollectAnchorCIDRs(entry) + anchorExtras := peer.CollectAnchorCIDRs(entry, nodes) for i, node := range nodes { var extras []string diff --git a/internal/controller/discovered_endpoint_test.go b/internal/controller/discovered_endpoint_test.go new file mode 100644 index 0000000..5384eee --- /dev/null +++ b/internal/controller/discovered_endpoint_test.go @@ -0,0 +1,216 @@ +/* +Copyright 2026 The Kilo Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "context" + "log/slog" + "os" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + "github.com/squat/kilo-clustermesh-operator/internal/kilonode" + kilov1alpha1 "github.com/squat/kilo-clustermesh-operator/pkg/kilo/v1alpha1" +) + +const discoveredTestPubKey = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=" + +func discardLogger() *slog.Logger { + return slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError})) +} + +func TestParseDiscoveredEndpoint(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + hostPort string + wantErr bool + wantIP string + wantPort uint32 + }{ + { + name: "valid IPv4 host:port", + hostPort: "203.0.113.7:51820", + wantIP: "203.0.113.7", + wantPort: 51820, + }, + { + name: "missing port", + hostPort: "203.0.113.7", + wantErr: true, + }, + { + name: "non-numeric port", + hostPort: "203.0.113.7:abc", + wantErr: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + got, err := parseDiscoveredEndpoint(tc.hostPort) + + if tc.wantErr { + require.Error(t, err) + assert.Nil(t, got) + + return + } + + require.NoError(t, err) + require.NotNil(t, got) + assert.Equal(t, tc.wantIP, got.IP) + assert.Equal(t, tc.wantPort, got.Port) + }) + } +} + +func enrichScheme(t *testing.T) *runtime.Scheme { + t.Helper() + + scheme := runtime.NewScheme() + require.NoError(t, corev1.AddToScheme(scheme)) + + return scheme +} + +func nodeWithDiscovered(name, rawJSON string) *corev1.Node { + return &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Annotations: map[string]string{kilonode.AnnotationDiscoveredEndpoints: rawJSON}, + }, + } +} + +func desiredPeer(name, pubKey, endpointIP string, port uint32) *kilov1alpha1.Peer { + return &kilov1alpha1.Peer{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Spec: kilov1alpha1.PeerSpec{ + PublicKey: pubKey, + AllowedIPs: []string{"10.1.0.0/24"}, + Endpoint: &kilov1alpha1.PeerEndpoint{ + DNSOrIP: kilov1alpha1.DNSOrIP{IP: endpointIP}, + Port: port, + }, + }, + } +} + +// TestEnrichEndpointsFromDiscovered_OverridesWhenDifferent verifies that when a +// target node has observed a different (NAT-egress) endpoint for a peer's +// public key, the enriched Peer carries the discovered endpoint instead of the +// configured one. +func TestEnrichEndpointsFromDiscovered_OverridesWhenDifferent(t *testing.T) { + t.Parallel() + + scheme := enrichScheme(t) + node := nodeWithDiscovered("target-1", + `{"`+discoveredTestPubKey+`":{"IP":"198.51.100.42","Port":51820}}`, + ) + fc := fake.NewClientBuilder().WithScheme(scheme).WithObjects(node).Build() + + r := &ClusterMeshReconciler{} + // Configured endpoint is the source's internal IP; discovered is the real + // NAT egress and must win. + desired := []*kilov1alpha1.Peer{ + desiredPeer("peer-1", discoveredTestPubKey, "10.0.0.5", 51820), + } + + got, err := r.enrichEndpointsFromDiscovered(context.Background(), discardLogger(), fc, desired) + + require.NoError(t, err) + require.Len(t, got, 1) + require.NotNil(t, got[0].Spec.Endpoint) + assert.Equal(t, "198.51.100.42", got[0].Spec.Endpoint.IP) + assert.Equal(t, uint32(51820), got[0].Spec.Endpoint.Port) +} + +// TestEnrichEndpointsFromDiscovered_KeepsWhenSame verifies that an already- +// correct configured endpoint is not rewritten when the discovered value +// matches it. +func TestEnrichEndpointsFromDiscovered_KeepsWhenSame(t *testing.T) { + t.Parallel() + + scheme := enrichScheme(t) + node := nodeWithDiscovered("target-1", + `{"`+discoveredTestPubKey+`":{"IP":"203.0.113.7","Port":51820}}`, + ) + fc := fake.NewClientBuilder().WithScheme(scheme).WithObjects(node).Build() + + r := &ClusterMeshReconciler{} + original := desiredPeer("peer-1", discoveredTestPubKey, "203.0.113.7", 51820) + desired := []*kilov1alpha1.Peer{original} + + got, err := r.enrichEndpointsFromDiscovered(context.Background(), discardLogger(), fc, desired) + + require.NoError(t, err) + require.Len(t, got, 1) + // Same value → the original Peer pointer is returned unchanged. + assert.Same(t, original, got[0]) +} + +// TestEnrichEndpointsFromDiscovered_NoDiscoveredData is the best-effort +// fallback: with no discovered-endpoint annotations on any target node, the +// original peers are returned unchanged. +func TestEnrichEndpointsFromDiscovered_NoDiscoveredData(t *testing.T) { + t.Parallel() + + scheme := enrichScheme(t) + fc := fake.NewClientBuilder().WithScheme(scheme).Build() + + r := &ClusterMeshReconciler{} + original := desiredPeer("peer-1", discoveredTestPubKey, "10.0.0.5", 51820) + desired := []*kilov1alpha1.Peer{original} + + got, err := r.enrichEndpointsFromDiscovered(context.Background(), discardLogger(), fc, desired) + + require.NoError(t, err) + require.Len(t, got, 1) + assert.Same(t, original, got[0]) +} + +// TestEnrichEndpointsFromDiscovered_NoMatchingKey verifies that a peer whose +// public key is not present in any discovered-endpoint map is left unchanged. +func TestEnrichEndpointsFromDiscovered_NoMatchingKey(t *testing.T) { + t.Parallel() + + scheme := enrichScheme(t) + node := nodeWithDiscovered("target-1", + `{"BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB=":{"IP":"198.51.100.42","Port":51820}}`, + ) + fc := fake.NewClientBuilder().WithScheme(scheme).WithObjects(node).Build() + + r := &ClusterMeshReconciler{} + original := desiredPeer("peer-1", discoveredTestPubKey, "10.0.0.5", 51820) + desired := []*kilov1alpha1.Peer{original} + + got, err := r.enrichEndpointsFromDiscovered(context.Background(), discardLogger(), fc, desired) + + require.NoError(t, err) + require.Len(t, got, 1) + assert.Same(t, original, got[0]) +} diff --git a/internal/crd/clustermeshes.yaml b/internal/crd/clustermeshes.yaml index 64bb653..fa96fef 100644 --- a/internal/crd/clustermeshes.yaml +++ b/internal/crd/clustermeshes.yaml @@ -54,12 +54,20 @@ spec: description: ClusterEntry describes a single cluster participating in the mesh. properties: - additionalCIDRs: + allowedNetworks: description: |- - AdditionalCIDRs are extra CIDRs to advertise into the mesh - (e.g., host-network ranges, external subnets). + AllowedNetworks is the flat list of every CIDR this cluster contributes + to the mesh: pod CIDRs, the WireGuard (kilo0) CIDR, the service CIDR, + host-network ranges, external subnets, and so on. There is no typed + distinction between them — both validation and Peer construction treat + every entry uniformly. A node is eligible when its PodCIDR is a subset + of some entry and its kilo.squat.ai/wireguard-ip host IP falls within + some entry; entries that have no per-node representative (e.g. the + service CIDR or host-network ranges) are advertised via the anchor Peer. + Multiple entries support dual-stack (IPv4 + IPv6). items: type: string + minItems: 1 type: array kubeconfigSecretRef: description: |- @@ -89,28 +97,17 @@ spec: maxLength: 63 pattern: ^[a-z0-9]([a-z0-9\-]{0,61}[a-z0-9])?$ type: string - podCIDRs: - description: |- - PodCIDRs is the list of pod network CIDRs for this cluster. - Node.Spec.PodCIDRs must be a subset of these CIDRs. - Multiple entries support dual-stack (IPv4 + IPv6). - items: - type: string - minItems: 1 - type: array - serviceCIDR: - description: |- - ServiceCIDR is the Kubernetes service network CIDR for this cluster. - If set, it will be advertised via an anchor Peer so that services - in this cluster are reachable from other mesh members. - type: string - wireguardCIDR: + persistentKeepalive: description: |- - WireguardCIDR is the CIDR for Kilo's WireGuard interface (kilo0) addresses. - Each node's kilo.squat.ai/wireguard-ip must have its host IP within this CIDR. - The annotation may carry any prefix length (e.g. "10.4.0.1/32" upstream Kilo - or "10.4.0.1/16" cozystack-patched Kilo); only the host portion is validated. - type: string + PersistentKeepalive is the interval in seconds at which WireGuard + sends keepalive packets to peers in this cluster. Set to a non-zero + value (e.g. 25) for clusters behind NAT so that the stateful NAT + mapping is refreshed before it expires, enabling bidirectional traffic + even when the cluster has no directly-routable public IP. + 0 disables persistent keepalive (default). + maximum: 65535 + minimum: 0 + type: integer wireguardPort: default: 51820 description: |- @@ -123,9 +120,8 @@ spec: minimum: 1 type: integer required: + - allowedNetworks - name - - podCIDRs - - wireguardCIDR type: object minItems: 2 type: array diff --git a/internal/kilonode/discovered_test.go b/internal/kilonode/discovered_test.go new file mode 100644 index 0000000..5b19751 --- /dev/null +++ b/internal/kilonode/discovered_test.go @@ -0,0 +1,127 @@ +/* +Copyright 2026 The Kilo Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package kilonode_test + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + "github.com/squat/kilo-clustermesh-operator/internal/kilonode" +) + +// nodeWithDiscovered returns a Node carrying the kilo.squat.ai/discovered-endpoints +// annotation with the given raw JSON value (empty string means no annotation). +func nodeWithDiscovered(name, rawJSON string) *corev1.Node { + annotations := map[string]string{} + if rawJSON != "" { + annotations[kilonode.AnnotationDiscoveredEndpoints] = rawJSON + } + + return &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{Name: name, Annotations: annotations}, + } +} + +func TestDiscoveredEndpointsByKey(t *testing.T) { + t.Parallel() + + const pubKeyA = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=" + + tests := []struct { + name string + nodes []*corev1.Node + want map[string]string + }{ + { + name: "single valid global endpoint is parsed", + nodes: []*corev1.Node{ + nodeWithDiscovered("n1", `{"`+pubKeyA+`":{"IP":"203.0.113.7","Port":51820}}`), + }, + want: map[string]string{pubKeyA: "203.0.113.7:51820"}, + }, + { + name: "malformed annotation is skipped silently", + nodes: []*corev1.Node{ + nodeWithDiscovered("n1", `{not valid json`), + }, + want: map[string]string{}, + }, + { + name: "loopback IP is filtered out", + nodes: []*corev1.Node{ + nodeWithDiscovered("n1", `{"`+pubKeyA+`":{"IP":"127.0.0.1","Port":51820}}`), + }, + want: map[string]string{}, + }, + { + name: "link-local IP is filtered out", + nodes: []*corev1.Node{ + nodeWithDiscovered("n1", `{"`+pubKeyA+`":{"IP":"169.254.1.1","Port":51820}}`), + }, + want: map[string]string{}, + }, + { + name: "entry with empty IP or zero port is skipped", + nodes: []*corev1.Node{ + nodeWithDiscovered("n1", `{"`+pubKeyA+`":{"IP":"","Port":51820}}`), + nodeWithDiscovered("n2", `{"`+pubKeyA+`":{"IP":"203.0.113.9","Port":0}}`), + }, + want: map[string]string{}, + }, + { + name: "no annotation yields empty map", + nodes: []*corev1.Node{nodeWithDiscovered("n1", "")}, + want: map[string]string{}, + }, + { + // Multiple nodes observing the same key report the same NAT egress + // (the documented invariant), so the deduplicated result is stable + // regardless of which node the fake client lists first. + name: "same key observed by multiple nodes deduplicates to one entry", + nodes: []*corev1.Node{ + nodeWithDiscovered("n1", `{"`+pubKeyA+`":{"IP":"203.0.113.7","Port":51820}}`), + nodeWithDiscovered("n2", `{"`+pubKeyA+`":{"IP":"203.0.113.7","Port":51820}}`), + }, + want: map[string]string{pubKeyA: "203.0.113.7:51820"}, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + scheme := ensureScheme(t) + + builder := fake.NewClientBuilder().WithScheme(scheme) + for _, n := range tc.nodes { + builder = builder.WithObjects(n) + } + + fc := builder.Build() + + got, err := kilonode.DiscoveredEndpointsByKey(context.Background(), fc) + require.NoError(t, err) + assert.Equal(t, tc.want, got) + }) + } +} diff --git a/internal/peer/builder.go b/internal/peer/builder.go index bd5cc09..44985c7 100644 --- a/internal/peer/builder.go +++ b/internal/peer/builder.go @@ -36,9 +36,10 @@ import ( // The Peer's AllowedIPs always include the node's PodCIDR plus a /32 (or // /128) host route derived from its kilo.squat.ai/wireguard-ip annotation. // -// extraAllowedIPs lets the caller fold cluster-wide CIDRs (serviceCIDR and -// any AdditionalCIDRs declared on the ClusterEntry) into the first valid -// node's Peer. This replaces the old "anchor Peer" pattern, which emitted +// extraAllowedIPs lets the caller fold cluster-wide CIDRs (the entries of +// AllowedNetworks that no individual node represents, e.g. the service CIDR +// or host-network ranges) into the first valid node's Peer. This replaces +// the old "anchor Peer" pattern, which emitted // a SEPARATE Peer object reusing the anchor node's public key. WireGuard // identifies peers exclusively by their public key and keeps only one // peer entry per pubkey: the second `wg setconf` call either dropped the @@ -102,22 +103,82 @@ func BuildPeer(meshName string, entry *v1alpha1.ClusterEntry, node *corev1.Node, return peer, nil } -// CollectAnchorCIDRs returns the cluster-wide CIDRs (serviceCIDR plus any -// AdditionalCIDRs) declared on a ClusterEntry. The caller is expected to -// pass these as extraAllowedIPs to BuildPeer for a single (anchor) node — -// see the commentary on BuildPeer for the WireGuard pubkey-dedup -// rationale that motivates folding cluster-wide CIDRs into a node Peer -// rather than emitting a separate anchor Peer. -func CollectAnchorCIDRs(entry *v1alpha1.ClusterEntry) []string { - var cidrs []string +// CollectAnchorCIDRs returns the residual entries of entry.AllowedNetworks +// that no individual node already advertises via its own Peer — i.e. the +// cluster-wide CIDRs (service CIDR, host-network ranges, external subnets) +// that have no per-node representative. These are folded as extraAllowedIPs +// into a single (anchor) node's Peer; see the commentary on BuildPeer for the +// WireGuard pubkey-dedup rationale that motivates folding cluster-wide CIDRs +// into a node Peer rather than emitting a separate anchor Peer. +// +// An AllowedNetworks entry N is considered "covered" — and therefore omitted +// from the anchor — when some node already carries it: either node N's first +// PodCIDR is a subset of N (the pod aggregate is announced by the per-node +// Peer's PodCIDR), or the host IP of node N's kilo.squat.ai/wireguard-ip +// annotation falls within N (the WG CIDR is announced by the per-node /32 or +// /128 host route). Nodes with a missing or unparseable PodCIDR / wireguard-ip +// simply do not cover anything; they are ignored here (validateNode already +// gates them out of the peered set). +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) } // resolvePeerEndpoint resolves a node's WireGuard endpoint via the kilonode diff --git a/internal/peer/builder_test.go b/internal/peer/builder_test.go index 0a519bf..516d397 100644 --- a/internal/peer/builder_test.go +++ b/internal/peer/builder_test.go @@ -227,15 +227,16 @@ func TestBuildPeer_AnchorExtras_WithServiceCIDR(t *testing.T) { // Replaces the legacy TestBuildAnchorPeer_WithServiceCIDR. The anchor // CIDRs now fold into the first node's Peer via extraAllowedIPs, so a // single WireGuard peer entry on the receiving side carries both the - // node-local routes and the cluster-wide routes. + // node-local routes and the cluster-wide routes. The service CIDR has no + // per-node representative, so CollectAnchorCIDRs keeps it as a residual. entry := &v1alpha1.ClusterEntry{ - Name: "cluster-a", - ServiceCIDR: "10.96.0.0/12", + Name: "cluster-a", + AllowedNetworks: []string{"10.96.0.0/12"}, } node := testNode("worker-1", testPodCIDR, baseAnnotations()) - got, err := peer.BuildPeer("my-mesh", entry, node, peer.CollectAnchorCIDRs(entry)) + got, err := peer.BuildPeer("my-mesh", entry, node, peer.CollectAnchorCIDRs(entry, []*corev1.Node{node})) require.NoError(t, err) require.NotNil(t, got) @@ -249,17 +250,17 @@ func TestBuildPeer_AnchorExtras_WithServiceCIDR(t *testing.T) { func TestBuildPeer_AnchorExtras_WithAdditionalCIDRs(t *testing.T) { t.Parallel() - // AdditionalCIDRs append after serviceCIDR in CollectAnchorCIDRs and - // must appear after the node's own routes in the merged AllowedIPs. + // Residual anchor CIDRs preserve their order in AllowedNetworks and must + // appear after the node's own routes in the merged AllowedIPs. None of + // these have a per-node representative, so all survive into the anchor. entry := &v1alpha1.ClusterEntry{ Name: "cluster-a", - ServiceCIDR: "10.96.0.0/12", - AdditionalCIDRs: []string{"192.168.100.0/24", "172.16.0.0/16"}, + AllowedNetworks: []string{"10.96.0.0/12", "192.168.100.0/24", "172.16.0.0/16"}, } node := testNode("worker-1", testPodCIDR, baseAnnotations()) - got, err := peer.BuildPeer("my-mesh", entry, node, peer.CollectAnchorCIDRs(entry)) + got, err := peer.BuildPeer("my-mesh", entry, node, peer.CollectAnchorCIDRs(entry, []*corev1.Node{node})) require.NoError(t, err) require.NotNil(t, got) @@ -274,18 +275,18 @@ func TestBuildPeer_AnchorExtras_WithAdditionalCIDRs(t *testing.T) { func TestBuildPeer_NoExtras_ProducesNodeOnlyAllowedIPs(t *testing.T) { t.Parallel() - // Replaces TestBuildAnchorPeer_NoAnchorCIDRs: when the caller passes - // no extras (non-anchor node, or anchor cluster has no serviceCIDR / - // additionalCIDRs), the AllowedIPs list is just the node's own routes. + // Replaces TestBuildAnchorPeer_NoAnchorCIDRs: when every AllowedNetworks + // entry already has a per-node representative (the pod aggregate covers + // the node's PodCIDR, the WG CIDR covers its wireguard-ip), CollectAnchorCIDRs + // returns no residual, so the AllowedIPs list is just the node's own routes. entry := &v1alpha1.ClusterEntry{ Name: "cluster-a", - ServiceCIDR: "", - AdditionalCIDRs: nil, + AllowedNetworks: []string{"10.244.0.0/16", "10.4.0.0/24"}, } node := testNode("worker-1", testPodCIDR, baseAnnotations()) - got, err := peer.BuildPeer("my-mesh", entry, node, peer.CollectAnchorCIDRs(entry)) + got, err := peer.BuildPeer("my-mesh", entry, node, peer.CollectAnchorCIDRs(entry, []*corev1.Node{node})) require.NoError(t, err) require.NotNil(t, got) @@ -384,8 +385,8 @@ func TestBuildPeer_AnchorExtras_NoEndpointSource_ReturnsError(t *testing.T) { // strictly stricter: the operator surfaces misconfiguration instead // of silently dropping cluster-wide CIDRs. entry := &v1alpha1.ClusterEntry{ - Name: "cluster-a", - ServiceCIDR: "10.96.0.0/12", + Name: "cluster-a", + AllowedNetworks: []string{"10.96.0.0/12"}, } annotations := baseAnnotations() @@ -394,7 +395,7 @@ func TestBuildPeer_AnchorExtras_NoEndpointSource_ReturnsError(t *testing.T) { node := testNode("worker-1", testPodCIDR, annotations) node.Status.Addresses = nil - got, err := peer.BuildPeer("my-mesh", entry, node, peer.CollectAnchorCIDRs(entry)) + got, err := peer.BuildPeer("my-mesh", entry, node, peer.CollectAnchorCIDRs(entry, []*corev1.Node{node})) require.Error(t, err) assert.Nil(t, got, "node Peer without resolvable endpoint must error, regardless of anchor CIDRs") @@ -408,9 +409,9 @@ func TestBuildPeer_AnchorExtras_ExternalIPFallback(t *testing.T) { // is the same for anchor vs non-anchor nodes — there is no separate // path anymore. entry := &v1alpha1.ClusterEntry{ - Name: "cluster-a", - ServiceCIDR: "10.96.0.0/12", - WireguardPort: 51820, + Name: "cluster-a", + AllowedNetworks: []string{"10.96.0.0/12"}, + WireguardPort: 51820, } annotations := baseAnnotations() @@ -421,7 +422,7 @@ func TestBuildPeer_AnchorExtras_ExternalIPFallback(t *testing.T) { {Type: corev1.NodeExternalIP, Address: "203.0.113.99"}, } - got, err := peer.BuildPeer("my-mesh", entry, node, peer.CollectAnchorCIDRs(entry)) + got, err := peer.BuildPeer("my-mesh", entry, node, peer.CollectAnchorCIDRs(entry, []*corev1.Node{node})) require.NoError(t, err) require.NotNil(t, got) @@ -452,3 +453,94 @@ func TestBuildPeer_BracketedDNSEndpoint(t *testing.T) { "brackets must be stripped from the DNS field; got %q", got.Spec.Endpoint.DNS) assert.Empty(t, got.Spec.Endpoint.IP) } + +// TestCollectAnchorCIDRs exercises the node-aware residual logic: an +// AllowedNetworks entry survives into the anchor only when no node already +// advertises it (no per-node PodCIDR subset, no contained wireguard-ip). +func TestCollectAnchorCIDRs(t *testing.T) { + t.Parallel() + + // A node carrying pod CIDR 10.244.1.0/24 and WireGuard IP 10.4.0.1/32. + // It covers the pod aggregate 10.244.0.0/16 and the WG CIDR 10.4.0.0/24. + coveringNode := testNode("worker-1", "10.244.1.0/24", map[string]string{ + kilonode.AnnotationWireguardIP: "10.4.0.1/32", + kilonode.AnnotationPublicKey: testPubKey, + }) + + tests := []struct { + name string + entry *v1alpha1.ClusterEntry + nodes []*corev1.Node + want []string + }{ + { + // Service CIDR has no per-node representative → stays in anchor. + name: "service CIDR with no per-node representative goes to anchor", + entry: &v1alpha1.ClusterEntry{ + Name: "cluster-a", + AllowedNetworks: []string{"10.96.0.0/12"}, + }, + nodes: []*corev1.Node{coveringNode}, + want: []string{"10.96.0.0/12"}, + }, + { + // Pod aggregate is covered by the node's PodCIDR → omitted. + name: "pod aggregate covered by node PodCIDR is not in anchor", + entry: &v1alpha1.ClusterEntry{ + Name: "cluster-a", + AllowedNetworks: []string{"10.244.0.0/16"}, + }, + nodes: []*corev1.Node{coveringNode}, + want: nil, + }, + { + // Host-network range has no per-node representative → stays in anchor. + name: "host-network range with no per-node representative goes to anchor", + entry: &v1alpha1.ClusterEntry{ + Name: "cluster-a", + AllowedNetworks: []string{"192.168.103.0/24"}, + }, + nodes: []*corev1.Node{coveringNode}, + want: []string{"192.168.103.0/24"}, + }, + { + // WG CIDR is covered by the node's wireguard-ip host IP → omitted. + name: "wireguard CIDR covered by node wireguard-ip is not in anchor", + entry: &v1alpha1.ClusterEntry{ + Name: "cluster-a", + AllowedNetworks: []string{"10.4.0.0/24"}, + }, + nodes: []*corev1.Node{coveringNode}, + want: nil, + }, + { + // Mixed list: only the residual (uncovered) entries survive, in order. + name: "mixed list keeps only uncovered entries in declared order", + entry: &v1alpha1.ClusterEntry{ + Name: "cluster-a", + AllowedNetworks: []string{"10.244.0.0/16", "10.96.0.0/12", "10.4.0.0/24", "192.168.103.0/24"}, + }, + nodes: []*corev1.Node{coveringNode}, + want: []string{"10.96.0.0/12", "192.168.103.0/24"}, + }, + { + // With no nodes at all, nothing is covered → every entry is residual. + name: "no nodes means every entry is residual", + entry: &v1alpha1.ClusterEntry{ + Name: "cluster-a", + AllowedNetworks: []string{"10.244.0.0/16", "10.4.0.0/24"}, + }, + nodes: nil, + want: []string{"10.244.0.0/16", "10.4.0.0/24"}, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + got := peer.CollectAnchorCIDRs(tc.entry, tc.nodes) + assert.Equal(t, tc.want, got) + }) + } +} diff --git a/internal/restart/watcher_test.go b/internal/restart/watcher_test.go index d1944ef..6488336 100644 --- a/internal/restart/watcher_test.go +++ b/internal/restart/watcher_test.go @@ -57,7 +57,7 @@ func TestReconcile_SameFingerprint_NoCancelCalled(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "mesh1", Namespace: "default"}, Spec: v1alpha1.ClusterMeshSpec{ Clusters: []v1alpha1.ClusterEntry{ - {Name: "local", Local: true, PodCIDRs: []string{"10.0.0.0/16"}, WireguardCIDR: "10.4.0.0/16"}, + {Name: "local", Local: true, AllowedNetworks: []string{"10.0.0.0/16", "10.4.0.0/16"}}, }, }, } @@ -90,7 +90,7 @@ func TestReconcile_NewMeshAdded_CancelCalled(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "mesh1", Namespace: "default"}, Spec: v1alpha1.ClusterMeshSpec{ Clusters: []v1alpha1.ClusterEntry{ - {Name: "local", Local: true, PodCIDRs: []string{"10.0.0.0/16"}, WireguardCIDR: "10.4.0.0/16"}, + {Name: "local", Local: true, AllowedNetworks: []string{"10.0.0.0/16", "10.4.0.0/16"}}, }, }, } @@ -113,7 +113,7 @@ func TestReconcile_NewMeshAdded_CancelCalled(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "mesh2", Namespace: "default"}, Spec: v1alpha1.ClusterMeshSpec{ Clusters: []v1alpha1.ClusterEntry{ - {Name: "remote", PodCIDRs: []string{"10.20.0.0/16"}, WireguardCIDR: "10.5.0.0/16"}, + {Name: "remote", AllowedNetworks: []string{"10.20.0.0/16", "10.5.0.0/16"}}, }, }, } @@ -144,8 +144,7 @@ func TestReconcile_SecretRVChanged_CancelCalled(t *testing.T) { { Name: "remote", KubeconfigSecretRef: &v1alpha1.SecretKeyRef{Name: "remote-kubeconfig", Key: "kubeconfig"}, - PodCIDRs: []string{"10.20.0.0/16"}, - WireguardCIDR: "10.5.0.0/16", + AllowedNetworks: []string{"10.20.0.0/16", "10.5.0.0/16"}, }, }, }, @@ -185,8 +184,8 @@ func TestFingerprint_StableUnderReordering(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "mesh-a", Namespace: "default"}, Spec: v1alpha1.ClusterMeshSpec{ Clusters: []v1alpha1.ClusterEntry{ - {Name: "alpha", Local: true, PodCIDRs: []string{"10.0.0.0/16"}, WireguardCIDR: "10.4.0.0/16"}, - {Name: "beta", PodCIDRs: []string{"10.1.0.0/16"}, WireguardCIDR: "10.5.0.0/16"}, + {Name: "alpha", Local: true, AllowedNetworks: []string{"10.0.0.0/16", "10.4.0.0/16"}}, + {Name: "beta", AllowedNetworks: []string{"10.1.0.0/16", "10.5.0.0/16"}}, }, }, } @@ -195,8 +194,8 @@ func TestFingerprint_StableUnderReordering(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "mesh-a", Namespace: "default"}, Spec: v1alpha1.ClusterMeshSpec{ Clusters: []v1alpha1.ClusterEntry{ - {Name: "beta", PodCIDRs: []string{"10.1.0.0/16"}, WireguardCIDR: "10.5.0.0/16"}, - {Name: "alpha", Local: true, PodCIDRs: []string{"10.0.0.0/16"}, WireguardCIDR: "10.4.0.0/16"}, + {Name: "beta", AllowedNetworks: []string{"10.1.0.0/16", "10.5.0.0/16"}}, + {Name: "alpha", Local: true, AllowedNetworks: []string{"10.0.0.0/16", "10.4.0.0/16"}}, }, }, } @@ -245,7 +244,7 @@ func TestReconcile_NilCancel_NoPanic(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "mesh1", Namespace: "default"}, Spec: v1alpha1.ClusterMeshSpec{ Clusters: []v1alpha1.ClusterEntry{ - {Name: "local", Local: true, PodCIDRs: []string{"10.0.0.0/16"}, WireguardCIDR: "10.4.0.0/16"}, + {Name: "local", Local: true, AllowedNetworks: []string{"10.0.0.0/16", "10.4.0.0/16"}}, }, }, } diff --git a/internal/validation/mesh_test.go b/internal/validation/mesh_test.go index 40f16b1..ee810cc 100644 --- a/internal/validation/mesh_test.go +++ b/internal/validation/mesh_test.go @@ -32,14 +32,23 @@ import ( "github.com/squat/kilo-clustermesh-operator/internal/validation" ) -// makeCluster is a helper that constructs a ClusterEntry with the given fields. +// makeCluster is a helper that constructs a ClusterEntry, folding the pod, +// wireguard, service and any additional CIDRs into the flat AllowedNetworks +// list. Empty CIDR strings are dropped so that the legacy `serviceCIDR == ""` +// call sites continue to express "no service CIDR" rather than injecting an +// unparseable empty entry. func makeCluster(name, podCIDR, wireguardCIDR, serviceCIDR string, additionalCIDRs ...string) v1alpha1.ClusterEntry { + networks := make([]string, 0, 3+len(additionalCIDRs)) + + for _, cidr := range append([]string{podCIDR, wireguardCIDR, serviceCIDR}, additionalCIDRs...) { + if cidr != "" { + networks = append(networks, cidr) + } + } + return v1alpha1.ClusterEntry{ Name: name, - PodCIDRs: []string{podCIDR}, - WireguardCIDR: wireguardCIDR, - ServiceCIDR: serviceCIDR, - AdditionalCIDRs: additionalCIDRs, + AllowedNetworks: networks, } } @@ -101,9 +110,8 @@ func TestValidateClusterNetworks(t *testing.T) { name: "invalid CIDR string", clusters: []v1alpha1.ClusterEntry{ { - Name: "cluster-a", - PodCIDRs: []string{"not-a-cidr"}, - WireguardCIDR: "10.4.0.0/24", + Name: "cluster-a", + AllowedNetworks: []string{"not-a-cidr", "10.4.0.0/24"}, }, }, wantErr: true, @@ -123,10 +131,7 @@ func TestValidateClusterNetworks(t *testing.T) { clusters: []v1alpha1.ClusterEntry{ { Name: "cluster-a", - PodCIDRs: []string{"10.0.0.0/16"}, - WireguardCIDR: "10.4.0.0/24", - ServiceCIDR: "10.96.0.0/12", - AdditionalCIDRs: []string{"172.16.0.0/12"}, + AllowedNetworks: []string{"10.0.0.0/16", "10.4.0.0/24", "10.96.0.0/12", "172.16.0.0/12"}, }, }, wantErr: false, diff --git a/internal/validation/node.go b/internal/validation/node.go index d42638e..14c6b46 100644 --- a/internal/validation/node.go +++ b/internal/validation/node.go @@ -81,10 +81,9 @@ func IsTransient(reason NodeSkipReason) bool { } // ValidateNode checks whether a node is eligible to be peered. -// It validates the node's PodCIDR against the cluster's PodCIDRs, -// the node's WireGuard IP against the cluster's WireguardCIDR, and -// that the node exposes a resolvable WireGuard endpoint via the -// kilonode fallback chain. +// It validates the node's PodCIDR and WireGuard IP against the cluster's +// AllowedNetworks (each must fall within some declared entry), and that the +// node exposes a resolvable WireGuard endpoint via the kilonode fallback chain. // Returns (true, reason, message) if the node should be skipped, (false, "", "") if valid. func ValidateNode(node *corev1.Node, entry *v1alpha1.ClusterEntry) (bool, NodeSkipReason, string) { if skip, reason, msg := validatePodCIDR(node, entry); skip { @@ -116,7 +115,7 @@ func validatePodCIDR(node *corev1.Node, entry *v1alpha1.ClusterEntry) (bool, Nod return true, ReasonNoPodCIDR, fmt.Sprintf("node %q has invalid PodCIDR %q: %v", node.Name, node.Spec.PodCIDRs[0], err) } - for _, clusterCIDRStr := range entry.PodCIDRs { + for _, clusterCIDRStr := range entry.AllowedNetworks { clusterCIDR, err := netutil.ParseCIDR(clusterCIDRStr) if err != nil { continue @@ -128,8 +127,8 @@ func validatePodCIDR(node *corev1.Node, entry *v1alpha1.ClusterEntry) (bool, Nod } return true, ReasonPodCIDROutOfRange, fmt.Sprintf( - "node %q PodCIDR %q is not a subset of any cluster PodCIDR %v", - node.Name, node.Spec.PodCIDRs[0], entry.PodCIDRs, + "node %q PodCIDR %q is not a subset of any cluster AllowedNetworks %v", + node.Name, node.Spec.PodCIDRs[0], entry.AllowedNetworks, ) } @@ -144,7 +143,7 @@ func validateWireguardIP(node *corev1.Node, entry *v1alpha1.ClusterEntry) (bool, // The annotation may carry any prefix length. Upstream Kilo writes a /32 // host route ("10.4.0.1/32"); the cozystack-patched Kilo writes the host // IP with the wireguard subnet mask ("100.66.0.3/16"). Both are accepted; - // only the host IP is checked against the cluster's wireguardCIDR. + // only the host IP is checked against the cluster's AllowedNetworks. hostIP, _, err := netutil.ParseHostInCIDR(wgIP) if err != nil { return true, ReasonWGIPInvalid, fmt.Sprintf( @@ -153,21 +152,21 @@ func validateWireguardIP(node *corev1.Node, entry *v1alpha1.ClusterEntry) (bool, ) } - wgCIDR, err := netutil.ParseCIDR(entry.WireguardCIDR) - if err != nil { - return true, ReasonWGIPOutOfRange, fmt.Sprintf( - "cluster WireguardCIDR %q is invalid: %v", entry.WireguardCIDR, err, - ) - } + for _, networkStr := range entry.AllowedNetworks { + network, parseErr := netutil.ParseCIDR(networkStr) + if parseErr != nil { + continue + } - if !wgCIDR.Contains(hostIP) { - return true, ReasonWGIPOutOfRange, fmt.Sprintf( - "node %q WireGuard IP %q is not within cluster WireguardCIDR %q", - node.Name, wgIP, entry.WireguardCIDR, - ) + if network.Contains(hostIP) { + return false, "", "" + } } - return false, "", "" + return true, ReasonWGIPOutOfRange, fmt.Sprintf( + "node %q WireGuard IP %q is not within any cluster AllowedNetworks %v", + node.Name, wgIP, entry.AllowedNetworks, + ) } func validatePublicKey(node *corev1.Node) (bool, NodeSkipReason, string) { diff --git a/internal/validation/node_test.go b/internal/validation/node_test.go index 17fc7fa..f91ae59 100644 --- a/internal/validation/node_test.go +++ b/internal/validation/node_test.go @@ -41,9 +41,8 @@ func makeNode(name string, podCIDRs []string, annotations map[string]string) *co } var baseEntry = &v1alpha1.ClusterEntry{ - Name: "test-cluster", - PodCIDRs: []string{"10.0.0.0/16"}, - WireguardCIDR: "10.4.0.0/24", + Name: "test-cluster", + AllowedNetworks: []string{"10.0.0.0/16", "10.4.0.0/24"}, } func baseAnnotations() map[string]string { @@ -108,7 +107,7 @@ func TestValidateNode(t *testing.T) { // prefix length other than /32 (or /128) via an IsHostRoute check. That // check was intentionally dropped to support cozystack-patched Kilo, which // writes the full subnet mask (e.g. "10.4.0.1/24") into the annotation. - // Only the host portion of the address is now validated against WireguardCIDR. + // Only the host portion of the address is now validated against AllowedNetworks. name: "wireguard IP with subnet mask (cozystack-Kilo style)", node: makeNode("node-1", []string{"10.0.1.0/24"}, map[string]string{ kilonode.AnnotationWireguardIP: "10.4.0.1/24", @@ -166,7 +165,7 @@ func TestValidateNode(t *testing.T) { wantSkipped: false, }, { - name: "wireguard IP outside wireguardCIDR", + name: "wireguard IP outside allowedNetworks", node: makeNode("node-1", []string{"10.0.1.0/24"}, map[string]string{ kilonode.AnnotationWireguardIP: "10.5.0.1/32", kilonode.AnnotationPublicKey: "dGVzdGtleQo=", diff --git a/test/integration/cleanup_test.go b/test/integration/cleanup_test.go index cc6455c..8f81a04 100644 --- a/test/integration/cleanup_test.go +++ b/test/integration/cleanup_test.go @@ -126,19 +126,17 @@ func TestCleanupStaleSourceClusters_SweepsClustersOutsideSpec(t *testing.T) { Spec: v1alpha1.ClusterMeshSpec{ Clusters: []v1alpha1.ClusterEntry{ { - Name: "local", - Local: true, - PodCIDRs: []string{"10.1.0.0/16"}, - WireguardCIDR: "10.100.0.0/24", + Name: "local", + Local: true, + AllowedNetworks: []string{"10.1.0.0/16", "10.100.0.0/24"}, }, { // Placeholder to satisfy the CRD's minItems=2 on // spec.clusters. Not in the registry — the // registry exposes "local" and "remote", so this // entry is effectively ignored by reconcile. - Name: "ghost-elsewhere", - PodCIDRs: []string{"10.2.0.0/16"}, - WireguardCIDR: "10.100.1.0/24", + Name: "ghost-elsewhere", + AllowedNetworks: []string{"10.2.0.0/16", "10.100.1.0/24"}, }, }, }, diff --git a/test/integration/helpers_test.go b/test/integration/helpers_test.go index b828514..4938ab3 100644 --- a/test/integration/helpers_test.go +++ b/test/integration/helpers_test.go @@ -209,15 +209,13 @@ func simpleMeshSpec(name, namespace string) *v1alpha1.ClusterMesh { Spec: v1alpha1.ClusterMeshSpec{ Clusters: []v1alpha1.ClusterEntry{ { - Name: "local", - Local: true, - PodCIDRs: []string{"10.1.0.0/16"}, - WireguardCIDR: "10.100.0.0/24", + Name: "local", + Local: true, + AllowedNetworks: []string{"10.1.0.0/16", "10.100.0.0/24"}, }, { - Name: "remote", - PodCIDRs: []string{"10.2.0.0/16"}, - WireguardCIDR: "10.100.1.0/24", + Name: "remote", + AllowedNetworks: []string{"10.2.0.0/16", "10.100.1.0/24"}, }, }, }, diff --git a/test/integration/labels_test.go b/test/integration/labels_test.go index ae6738a..46065e5 100644 --- a/test/integration/labels_test.go +++ b/test/integration/labels_test.go @@ -75,15 +75,13 @@ func TestLabelIsolation_TwoMeshes(t *testing.T) { Spec: v1alpha1.ClusterMeshSpec{ Clusters: []v1alpha1.ClusterEntry{ { - Name: "local", - Local: true, - PodCIDRs: []string{"10.0.0.0/16"}, - WireguardCIDR: "10.100.10.0/24", + Name: "local", + Local: true, + AllowedNetworks: []string{"10.0.0.0/16", "10.100.10.0/24"}, }, { - Name: "remote", - PodCIDRs: []string{"10.10.0.0/16"}, - WireguardCIDR: "10.100.11.0/24", + Name: "remote", + AllowedNetworks: []string{"10.10.0.0/16", "10.100.11.0/24"}, }, }, }, @@ -98,15 +96,13 @@ func TestLabelIsolation_TwoMeshes(t *testing.T) { Spec: v1alpha1.ClusterMeshSpec{ Clusters: []v1alpha1.ClusterEntry{ { - Name: "local", - Local: true, - PodCIDRs: []string{"10.20.0.0/16"}, - WireguardCIDR: "10.100.20.0/24", + Name: "local", + Local: true, + AllowedNetworks: []string{"10.20.0.0/16", "10.100.20.0/24"}, }, { - Name: "remote", - PodCIDRs: []string{"10.30.0.0/16"}, - WireguardCIDR: "10.100.21.0/24", + Name: "remote", + AllowedNetworks: []string{"10.30.0.0/16", "10.100.21.0/24"}, }, }, }, diff --git a/test/integration/validation_test.go b/test/integration/validation_test.go index 6092118..68e432d 100644 --- a/test/integration/validation_test.go +++ b/test/integration/validation_test.go @@ -50,15 +50,13 @@ func TestOverlappingNetworks_NoPeersCreated(t *testing.T) { Spec: v1alpha1.ClusterMeshSpec{ Clusters: []v1alpha1.ClusterEntry{ { - Name: "local-a", - Local: true, - PodCIDRs: []string{"10.0.0.0/16"}, - WireguardCIDR: "10.100.0.0/24", + Name: "local-a", + Local: true, + AllowedNetworks: []string{"10.0.0.0/16", "10.100.0.0/24"}, }, { - Name: "remote-a", - PodCIDRs: []string{"10.3.0.0/16"}, - WireguardCIDR: "10.100.1.0/24", + Name: "remote-a", + AllowedNetworks: []string{"10.3.0.0/16", "10.100.1.0/24"}, }, }, }, @@ -72,15 +70,13 @@ func TestOverlappingNetworks_NoPeersCreated(t *testing.T) { Spec: v1alpha1.ClusterMeshSpec{ Clusters: []v1alpha1.ClusterEntry{ { - Name: "local-b", - Local: true, - PodCIDRs: []string{"10.0.0.0/16"}, // same — overlaps with meshA's different cluster - WireguardCIDR: "10.100.2.0/24", + Name: "local-b", + Local: true, + AllowedNetworks: []string{"10.0.0.0/16", "10.100.2.0/24"}, }, { - Name: "remote-b", - PodCIDRs: []string{"10.4.0.0/16"}, - WireguardCIDR: "10.100.3.0/24", + Name: "remote-b", + AllowedNetworks: []string{"10.4.0.0/16", "10.100.3.0/24"}, }, }, }, @@ -121,9 +117,9 @@ func TestOverlappingNetworks_NoPeersCreated(t *testing.T) { } // TestInvalidNodeWGIP_NodeSkipped verifies that a node whose WireGuard IP is -// outside the cluster's WireguardCIDR is skipped: only 1 Peer is created in the -// remote cluster (for the valid node), and the local cluster status shows -// skippedNodes=1. +// outside every entry of the cluster's AllowedNetworks is skipped: only 1 Peer +// is created in the remote cluster (for the valid node), and the local cluster +// status shows skippedNodes=1. func TestInvalidNodeWGIP_NodeSkipped(t *testing.T) { ctx := context.Background()