diff --git a/api/machina/v1alpha3/site_types.go b/api/machina/v1alpha3/site_types.go index 2111234845..4129364b35 100644 --- a/api/machina/v1alpha3/site_types.go +++ b/api/machina/v1alpha3/site_types.go @@ -67,10 +67,10 @@ type SiteSpec struct { // CNI config is written and all same-site nodes are tunnel peers. // When false, only tunnel links to gateway pools for other sites are created, // allowing an external CNI plugin to manage intra-site networking. - // Pod CIDR assignment is also disabled when manageCniPlugin is false, - // regardless of the assignmentEnabled setting on individual podCidrAssignments - // rules. The podCidrAssignments are still required to define the CIDR ranges - // used for inter-site routing. + // Omitted assignmentEnabled values on podCidrAssignments default to this + // setting. Explicit assignmentEnabled values override that default. + // The podCidrAssignments are still required to define the CIDR ranges + // used for inter-site routing, even when assignment is disabled. // +optional ManageCniPlugin *bool `json:"manageCniPlugin,omitempty"` @@ -107,6 +107,16 @@ type SiteSpec struct { Components SiteComponents `json:"components,omitempty"` } +// PodCIDRAssignmentEnabled resolves an assignment's explicit setting or the +// site's CNI management default without persisting an inherited value. +func (s SiteSpec) PodCIDRAssignmentEnabled(assignment unboundednetv1alpha1.PodCidrAssignment) bool { + if assignment.AssignmentEnabled != nil { + return *assignment.AssignmentEnabled + } + + return s.ManageCniPlugin == nil || *s.ManageCniPlugin +} + // SiteComponents declares optional components managed by the unbounded operator. // Networking (unbounded-net) is not a component: it is a cluster singleton the // operator deploys whenever at least one Site exists. diff --git a/api/machina/v1alpha3/site_types_test.go b/api/machina/v1alpha3/site_types_test.go index 8bc9b60f85..2ee0e903ee 100644 --- a/api/machina/v1alpha3/site_types_test.go +++ b/api/machina/v1alpha3/site_types_test.go @@ -36,6 +36,47 @@ func TestSiteResourceAndAddToScheme(t *testing.T) { } } +func TestPodCIDRAssignmentEnabled(t *testing.T) { + settings := []struct { + name string + value *bool + }{ + {"omitted", nil}, + {"true", new(true)}, + {"false", new(false)}, + } + for _, manage := range settings { + for _, assignment := range settings { + t.Run(manage.name+"/"+assignment.name, func(t *testing.T) { + spec := SiteSpec{ManageCniPlugin: manage.value} + rule := unboundednetv1alpha1.PodCidrAssignment{AssignmentEnabled: assignment.value} + + want := assignment.name == "true" || (assignment.name == "omitted" && manage.name != "false") + if got := spec.PodCIDRAssignmentEnabled(rule); got != want { + t.Fatalf("effective assignment enabled = %v, want %v", got, want) + } + + if rule.AssignmentEnabled != assignment.value { + t.Fatal("resolution mutated the assignment setting") + } + }) + } + } + + spec := SiteSpec{} + rule := unboundednetv1alpha1.PodCidrAssignment{} + + spec.ManageCniPlugin = new(false) + if spec.PodCIDRAssignmentEnabled(rule) { + t.Fatal("omitted assignment did not inherit changed CNI setting") + } + + spec.ManageCniPlugin = new(true) + if !spec.PodCIDRAssignmentEnabled(rule) || rule.AssignmentEnabled != nil { + t.Fatal("inherited default was persisted instead of resolved") + } +} + func TestDeepCopySiteAndList(t *testing.T) { enabled := true priority := int32(10) diff --git a/api/net/v1alpha1/types.go b/api/net/v1alpha1/types.go index 8882224cb8..98cce7d8ee 100644 --- a/api/net/v1alpha1/types.go +++ b/api/net/v1alpha1/types.go @@ -36,7 +36,9 @@ const ( // PodCidrAssignment defines a pod CIDR allocation rule for nodes in a site. // +k8s:deepcopy-gen=true type PodCidrAssignment struct { - // AssignmentEnabled controls whether this assignment is active. Defaults to true. + // AssignmentEnabled controls whether this assignment is active. When omitted, + // it inherits the Site's manageCniPlugin setting, which defaults to true. + // Explicit true or false overrides the inherited value. // +optional AssignmentEnabled *bool `json:"assignmentEnabled,omitempty"` diff --git a/cmd/unbounded-net-controller/node_auth_informers_test.go b/cmd/unbounded-net-controller/node_auth_informers_test.go index 190f592c10..4370105934 100644 --- a/cmd/unbounded-net-controller/node_auth_informers_test.go +++ b/cmd/unbounded-net-controller/node_auth_informers_test.go @@ -18,6 +18,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/watch" k8sfake "k8s.io/client-go/kubernetes/fake" k8stesting "k8s.io/client-go/testing" "k8s.io/client-go/tools/cache" @@ -129,8 +130,28 @@ func TestNodeAuthInformersScopeAndLifecycle(t *testing.T) { otherSA := sa.DeepCopy() otherSA.Namespace = "other" client := k8sfake.NewClientset(pod, sa, unlabeled, otherPod, otherSA) + podWatchStarted := make(chan struct{}, 1) + watchReactor := client.WatchReactionChain[0] + client.PrependWatchReactor("pods", func(action k8stesting.Action) (bool, watch.Interface, error) { + handled, stream, err := watchReactor.React(action) + if handled && err == nil { + select { + case podWatchStarted <- struct{}{}: + default: + } + } + + return handled, stream, err + }) caches, stop := startNodeAuthTestCaches(t, client) + // The fake tracker cannot replay deletions between the initial list and watch registration. + select { + case <-podWatchStarted: + case <-time.After(5 * time.Second): + t.Fatal("Pod watch did not start") + } + if _, err := caches.pods.Lister().Pods(pod.Namespace).Get(pod.Name); err != nil { t.Fatal(err) } diff --git a/cmd/unbounded-net-node/bootstrap_helpers.go b/cmd/unbounded-net-node/bootstrap_helpers.go index 861e1b65c4..0e1ab5100d 100644 --- a/cmd/unbounded-net-node/bootstrap_helpers.go +++ b/cmd/unbounded-net-node/bootstrap_helpers.go @@ -24,6 +24,7 @@ import ( "k8s.io/client-go/kubernetes" "k8s.io/klog/v2" + "github.com/Azure/unbounded/internal/net/ipam" "github.com/Azure/unbounded/internal/version" ) @@ -78,6 +79,10 @@ func waitForPodCIDRsAndConfigure(ctx context.Context, clientset kubernetes.Inter // Check if podCIDRs are already assigned if len(node.Spec.PodCIDRs) > 0 { + if _, err := ipam.ConfigFromCIDRs(node.Spec.PodCIDRs); err != nil { + return nil, err + } + klog.V(4).Infof("Node %s has podCIDRs: %v", cfg.NodeName, node.Spec.PodCIDRs) if err := guardedWriteCNIConfig(ctx, cfg, node.Spec.PodCIDRs, healthState); err != nil { @@ -137,6 +142,11 @@ func waitForPodCIDRsAndConfigure(ctx context.Context, clientset kubernetes.Inter } if len(updatedNode.Spec.PodCIDRs) > 0 { + if _, err := ipam.ConfigFromCIDRs(updatedNode.Spec.PodCIDRs); err != nil { + watcher.Stop() + return nil, err + } + klog.Infof("Node %s podCIDRs assigned: %v", cfg.NodeName, updatedNode.Spec.PodCIDRs) watcher.Stop() @@ -204,10 +214,9 @@ func writeCNIConfigUnchecked(cfg *config, podCIDRs []string) error { } func doWriteCNIConfig(cfg *config, podCIDRs []string) error { - // Build IPAM ranges from podCIDRs - var ranges [][]IPRange - for _, cidr := range podCIDRs { - ranges = append(ranges, []IPRange{{Subnet: cidr}}) + socketPath := cfg.IPAMSocketPath + if socketPath == ipam.DefaultSocketPath { + socketPath = "" } // Bridge MTU matches the configured WireGuard tunnel MTU (cfg.MTU is @@ -229,8 +238,8 @@ func doWriteCNIConfig(cfg *config, podCIDRs []string) error { HairpinMode: true, MTU: bridgeMTU, IPAM: &IPAMConfig{ - Type: "host-local", - Ranges: ranges, + Type: "unbounded-ipam", + SocketPath: socketPath, }, }, { diff --git a/cmd/unbounded-net-node/bootstrap_helpers_test.go b/cmd/unbounded-net-node/bootstrap_helpers_test.go index 72486ec5a9..4b6e23f106 100644 --- a/cmd/unbounded-net-node/bootstrap_helpers_test.go +++ b/cmd/unbounded-net-node/bootstrap_helpers_test.go @@ -201,12 +201,12 @@ func TestWriteCNIConfigWritesExpectedConflist(t *testing.T) { t.Fatalf("unexpected bridge plugin config: %#v", got.Plugins[0]) } - if len(got.Plugins[0].IPAM.Ranges) != 2 { - t.Fatalf("expected two IPAM ranges, got %d", len(got.Plugins[0].IPAM.Ranges)) + if got.Plugins[0].IPAM.Type != "unbounded-ipam" || got.Plugins[0].IPAM.SocketPath != "" { + t.Fatalf("expected argument-free unbounded-ipam, got %#v", got.Plugins[0].IPAM) } - if got.Plugins[0].IPAM.Ranges[0][0].Subnet != podCIDRs[0] || got.Plugins[0].IPAM.Ranges[1][0].Subnet != podCIDRs[1] { - t.Fatalf("unexpected IPAM ranges: %#v", got.Plugins[0].IPAM.Ranges) + if strings.Contains(string(raw), "10.244.0.0") || strings.Contains(string(raw), "fd00:10::") { + t.Fatalf("conflist contains cached PodCIDRs: %s", raw) } if _, err := os.Stat(confPath + ".tmp"); !os.IsNotExist(err) { diff --git a/cmd/unbounded-net-node/cni_reconcile_test.go b/cmd/unbounded-net-node/cni_reconcile_test.go index 54fe326f15..cff9bdf260 100644 --- a/cmd/unbounded-net-node/cni_reconcile_test.go +++ b/cmd/unbounded-net-node/cni_reconcile_test.go @@ -7,6 +7,7 @@ import ( "context" "errors" "os" + "slices" "testing" corev1 "k8s.io/api/core/v1" @@ -18,6 +19,80 @@ import ( unboundednetnetlink "github.com/Azure/unbounded/internal/net/netlink" ) +func TestDelegatedAssignmentReconcilesAfterEmptyBootstrap(t *testing.T) { + cfg := newCNISafetyTestConfig(t) + health := &nodeHealthState{} + health.markCNIUnmanaged() + + state := &wireGuardState{ + clientset: fake.NewClientset(&corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: cfg.NodeName}}), + nodeName: cfg.NodeName, + healthState: health, + } + + originalConfigure := configureWireGuardFunc + + t.Cleanup(func() { configureWireGuardFunc = originalConfigure }) + + var configured [][]string + + fail := false + configureWireGuardFunc = func(_ context.Context, _ *config, _ string, _ []meshPeerInfo, _ []gatewayPeerInfo, _ string, _, _, _ map[string]bool, _, _, _, _, _ map[string]string, _, _, _, _, _ map[string]int, _ []unboundednetnetlink.DesiredRoute, _ map[string]bool, current *wireGuardState, _ map[string]healthcheck.HealthCheckSettings) error { + configured = append(configured, slices.Clone(current.nodePodCIDRs)) + + if fail { + return errors.New("injected routing failure") + } + + return nil + } + + sites := newInformerWithObjects(toUnstructured(t, &unboundedv1alpha3.Site{ + ObjectMeta: metav1.ObjectMeta{Name: "site-a"}, + })) + empty := newInformerWithObjects() + reconcile := func() error { + return updateWireGuardFromSlices(t.Context(), nil, sites, empty, empty, empty, empty, empty, empty, cfg, "site-a", "private", "public", false, state) + } + + if err := reconcile(); err != nil { + t.Fatal(err) + } + + cidrs := []string{"10.244.3.0/24", "fd00:3::/64"} + health.setVerifiedIPAMAssignment(cidrs) + + fail = true + + if err := reconcile(); err == nil { + t.Fatal("routing failure was not reported") + } + + if len(state.nodePodCIDRs) != 0 { + t.Fatalf("failed routing committed assignment: %v", state.nodePodCIDRs) + } + + fail = false + + if err := reconcile(); err != nil { + t.Fatal(err) + } + + if !slices.Equal(state.nodePodCIDRs, cidrs) || len(configured) == 0 || !slices.Equal(configured[len(configured)-1], cidrs) { + t.Fatalf("delayed assignment not configured: state=%v configured=%v", state.nodePodCIDRs, configured) + } + + count := len(configured) + + if err := reconcile(); err != nil { + t.Fatal(err) + } + + if len(configured) != count { + t.Fatal("unchanged assignment unnecessarily reconfigured routing") + } +} + func TestCNIReconciliationDisablesAndRecoversWithoutMTUChange(t *testing.T) { cfg := newCNISafetyTestConfig(t) cidrs := []string{"10.244.2.0/24"} diff --git a/cmd/unbounded-net-node/cni_safety_test.go b/cmd/unbounded-net-node/cni_safety_test.go index 52a0c408e4..8e90997077 100644 --- a/cmd/unbounded-net-node/cni_safety_test.go +++ b/cmd/unbounded-net-node/cni_safety_test.go @@ -382,7 +382,7 @@ func TestGuardedWriteCNIConfigPreservesCollisions(t *testing.T) { } default: got, readErr := os.ReadFile(cniConfigPath(cfg)) - if readErr != nil || !strings.Contains(string(got), "10.244.2.0/24") { + if readErr != nil || !strings.Contains(string(got), `"type": "unbounded-ipam"`) || strings.Contains(string(got), "10.244.2.0/24") { t.Fatalf("safe bridge did not publish current configuration: %q err=%v", got, readErr) } diff --git a/cmd/unbounded-net-node/delegated_routes_test.go b/cmd/unbounded-net-node/delegated_routes_test.go new file mode 100644 index 0000000000..100cbecec7 --- /dev/null +++ b/cmd/unbounded-net-node/delegated_routes_test.go @@ -0,0 +1,90 @@ +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "testing" + + unboundedv1alpha3 "github.com/Azure/unbounded/api/machina/v1alpha3" + unboundednetv1alpha1 "github.com/Azure/unbounded/api/net/v1alpha1" +) + +func TestUnmanagedLocalPoolsDoNotAttractThirdPartyTraffic(t *testing.T) { + state := &wireGuardState{sitePodCIDRPools: []string{"10.244.0.0/16"}} + for _, managed := range []bool{false, true} { + supernets := collectSupernets(state, nil, nil, nil, nil, managed) + if supernets["10.244.0.0/16"] != managed { + t.Fatalf("local pool routing with management=%v: %v", managed, supernets) + } + } +} + +func TestUnmanagedGatewayPoolsDoNotAttractThirdPartyTraffic(t *testing.T) { + sites := map[string]*unboundedv1alpha3.Site{ + "local": {Spec: unboundedv1alpha3.SiteSpec{ + ManageCniPlugin: new(false), + PodCidrAssignments: []unboundednetv1alpha1.PodCidrAssignment{{ + CidrBlocks: []string{"10.244.0.0/16", "fd00:0244::/48"}, + }}, + }}, + "remote": {Spec: unboundedv1alpha3.SiteSpec{ + NodeCidrs: []string{"192.0.2.0/24"}, + PodCidrAssignments: []unboundednetv1alpha1.PodCidrAssignment{{ + CidrBlocks: []string{"10.245.0.0/16"}, + }}, + }}, + } + + routing, err := collectSiteRoutingConfig(sites, "local", true) + if err != nil { + t.Fatal(err) + } + + routing.gatewaySupernets = append(routing.gatewaySupernets, "fd00:244::/48") + state := &wireGuardState{ + sitePodCIDRPools: []string{"10.244.0.0/16", "fd00:0244::/48"}, + sitePodCIDRs: routing.gatewaySupernets, + } + + gateways := []gatewayPeerInfo{{RoutedCidrs: routing.gatewaySupernets}} + for _, managed := range []bool{false, true} { + supernets := collectSupernets(state, nil, gateways, gateways, routing.gatewaySupernets, managed) + for _, cidr := range []string{"10.244.0.0/16", "fd00:0244::/48", "fd00:244::/48"} { + if supernets[cidr] != managed { + t.Fatalf("gateway local pool routing with management=%v: %v", managed, supernets) + } + } + + for _, cidr := range []string{"10.245.0.0/16", "192.0.2.0/24"} { + if !supernets[cidr] { + t.Fatalf("gateway lost remote route %s with management=%v: %v", cidr, managed, supernets) + } + } + } +} + +func TestUnmanagedSupernetsHonorSkippedPeerPodCIDRs(t *testing.T) { + state := &wireGuardState{sitePodCIDRPools: []string{"10.244.0.0/16"}} + peers := []meshPeerInfo{ + {PodCIDRs: []string{"10.244.1.0/24"}, SkipPodCIDRRoutes: true}, + {PodCIDRs: []string{"10.245.1.0/24"}}, + } + gateways := []gatewayPeerInfo{ + {PodCIDRs: []string{"10.244.2.0/24"}, SkipPodCIDRRoutes: true}, + {PodCIDRs: []string{"10.246.1.0/24"}}, + } + + supernets := collectSupernets(state, peers, gateways, gateways, nil, false) + for _, cidr := range []string{"10.244.0.0/16", "10.244.1.0/24", "10.244.2.0/24"} { + if supernets[cidr] { + t.Fatalf("unmanaged local route %s was added: %v", cidr, supernets) + } + } + + for _, cidr := range []string{"10.245.1.0/24", "10.246.1.0/24"} { + if !supernets[cidr] { + t.Fatalf("non-skipped peer route %s was lost: %v", cidr, supernets) + } + } +} diff --git a/cmd/unbounded-net-node/ipam.go b/cmd/unbounded-net-node/ipam.go new file mode 100644 index 0000000000..7cc0fd7dda --- /dev/null +++ b/cmd/unbounded-net-node/ipam.go @@ -0,0 +1,203 @@ +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "context" + "errors" + "fmt" + "io" + "net" + "os" + "path/filepath" + "slices" + + "github.com/spf13/pflag" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" + "k8s.io/klog/v2" + + "github.com/Azure/unbounded/internal/net/ipam" + ipamproto "github.com/Azure/unbounded/internal/net/ipam/proto" + unboundednetnetlink "github.com/Azure/unbounded/internal/net/netlink" +) + +func setPodCIDRPreferredSources(manager *unboundednetnetlink.UnifiedRouteManager, cidrs []string) { + var ipv4, ipv6 net.IP + + for _, cidr := range cidrs { + gateway := getGatewayIPFromCIDR(cidr) + if gateway == nil { + continue + } + + if gateway.To4() != nil { + ipv4 = gateway + } else { + ipv6 = gateway + } + } + + manager.SetPreferredSourceIPs(ipv4, ipv6) +} + +func registerIPAMFlags(flags *pflag.FlagSet, cfg *config) { + flags.StringVar(&cfg.IPAMSocketPath, "ipam-socket-path", ipam.DefaultSocketPath, "Unix socket shared with unbounded-ipam on the host") + flags.BoolVar(&cfg.InstallIPAMPlugin, "install-ipam-plugin", true, "Install or upgrade the bundled unbounded-ipam binary") + flags.StringVar(&cfg.IPAMBinDir, "ipam-bin-dir", "/host/opt/cni/bin", "Mounted host CNI binary directory for unbounded-ipam installation") +} + +func validateIPAMConfig(cfg *config) error { + if cfg.IPAMSocketPath == "" { + cfg.IPAMSocketPath = ipam.DefaultSocketPath + } + + if err := ipam.ValidateSocketPath(cfg.IPAMSocketPath); err != nil { + return err + } + + if cfg.InstallIPAMPlugin && (!filepath.IsAbs(cfg.IPAMBinDir) || filepath.Clean(cfg.IPAMBinDir) != cfg.IPAMBinDir) { + return fmt.Errorf("IPAM binary directory must be a clean absolute path: %q", cfg.IPAMBinDir) + } + + return nil +} + +func (h *nodeHealthState) setVerifiedIPAMAssignment(cidrs []string) { + h.mu.Lock() + defer h.mu.Unlock() + + h.verifiedIPAMCIDRs = slices.Clone(cidrs) + h.podCIDRs = slices.Clone(cidrs) +} + +func (h *nodeHealthState) verifiedIPAMAssignment() []string { + h.mu.RLock() + defer h.mu.RUnlock() + + return slices.Clone(h.verifiedIPAMCIDRs) +} + +func (h *nodeHealthState) getIPAMConfig() (*ipamproto.Config, error) { + h.mu.RLock() + defer h.mu.RUnlock() + + if h.cniManaged && !h.cniReady { + return nil, fmt.Errorf("managed CNI is not ready: %s", h.cniReason) + } + + return ipam.ConfigFromCIDRs(h.verifiedIPAMCIDRs) +} + +func acquireDelegatedIPAMAssignment(ctx context.Context, client kubernetes.Interface, cfg *config, health *nodeHealthState) { + for { + node, err := client.CoreV1().Nodes().Get(ctx, cfg.NodeName, metav1.GetOptions{}) + if err == nil { + _, err = ipam.ConfigFromCIDRs(node.Spec.PodCIDRs) + if err == nil { + health.setVerifiedIPAMAssignment(node.Spec.PodCIDRs) + klog.Infof("Verified delegated IPAM PodCIDRs for node %s: %v", cfg.NodeName, node.Spec.PodCIDRs) + + return + } + } + + if ctx.Err() != nil { + return + } + + klog.Warningf("Delegated IPAM assignment pending for node %s: %v", cfg.NodeName, err) + + if err := waitForCNIRetry(ctx, cfg.cniRetryInterval); err != nil { + return + } + } +} + +func installIPAMBinary(source, directory string) (returnErr error) { + if !filepath.IsAbs(directory) || filepath.Clean(directory) != directory { + return fmt.Errorf("invalid IPAM install directory %q", directory) + } + + info, err := os.Lstat(directory) + if err != nil { + return fmt.Errorf("inspect mounted IPAM binary directory: %w", err) + } + + if !info.IsDir() { + return fmt.Errorf("IPAM binary destination %s is not a directory", directory) + } + + destination := filepath.Join(directory, "unbounded-ipam") + if targetInfo, err := os.Lstat(destination); err == nil { + if !targetInfo.Mode().IsRegular() { + return fmt.Errorf("refusing to replace non-regular IPAM binary %s", destination) + } + } else if !errors.Is(err, os.ErrNotExist) { + return err + } + + input, err := os.Open(source) + if err != nil { + return err + } + defer func() { returnErr = errors.Join(returnErr, input.Close()) }() + + info, err = input.Stat() + if err != nil { + return err + } + + if !info.Mode().IsRegular() || info.Mode().Perm()&0o111 == 0 { + return fmt.Errorf("bundled IPAM binary %s is not executable", source) + } + + output, err := os.CreateTemp(directory, ".unbounded-ipam-*") + if err != nil { + return err + } + + closed := false + defer func() { + if !closed { + returnErr = errors.Join(returnErr, output.Close()) + } + + if err := os.Remove(output.Name()); err != nil && !errors.Is(err, os.ErrNotExist) { + returnErr = errors.Join(returnErr, err) + } + }() + + if _, err := io.Copy(output, input); err != nil { + return err + } + + if err := output.Chmod(0o755); err != nil { + return err + } + + if err := output.Sync(); err != nil { + return err + } + + err = output.Close() + closed = true + + if err != nil { + return err + } + + if err := os.Rename(output.Name(), destination); err != nil { + return err + } + + directoryFile, err := os.Open(directory) + if err != nil { + return err + } + + defer func() { returnErr = errors.Join(returnErr, directoryFile.Close()) }() + + return directoryFile.Sync() +} diff --git a/cmd/unbounded-net-node/ipam_test.go b/cmd/unbounded-net-node/ipam_test.go new file mode 100644 index 0000000000..5f0bf1d84b --- /dev/null +++ b/cmd/unbounded-net-node/ipam_test.go @@ -0,0 +1,306 @@ +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "context" + "errors" + "os" + "path/filepath" + "strings" + "testing" + "time" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/fake" + "sigs.k8s.io/yaml" + + "github.com/Azure/unbounded/hack/cmd/render-manifests/render" + configpkg "github.com/Azure/unbounded/internal/net/config" + "github.com/Azure/unbounded/internal/net/ipam" +) + +func TestIPAMVerificationAndGuard(t *testing.T) { + health := &nodeHealthState{} + + directory := t.TempDir() + if err := os.Chmod(directory, 0o700); err != nil { + t.Fatal(err) + } + + path := filepath.Join(directory, "ipam.sock") + + server, err := ipam.Listen(t.Context(), path, health.getIPAMConfig) + if err != nil { + t.Fatal(err) + } + defer server.Close() + + health.setCNIAssignment([]string{"10.244.1.0/24"}) + + if _, err := ipam.GetConfig(t.Context(), path); err == nil { + t.Fatal("status assignment alone enabled allocation") + } + + health.beginManagedCNI("cbr0") + health.setCNIReady("cbr0", []string{"10.244.2.0/24"}) + + conf, err := ipam.GetConfig(t.Context(), path) + if err != nil || conf.GetRanges()[0].GetRanges()[0].GetSubnet() != "10.244.2.0/24" { + t.Fatalf("verified configuration: %v, %v", conf, err) + } + + health.setCNIBlocked("stale bridge") + + if _, err := ipam.GetConfig(t.Context(), path); err == nil { + t.Fatal("blocked bridge enabled IPAM") + } + + health.setCNIReady("cbr0", []string{"10.244.2.0/24"}) + + if _, err := ipam.GetConfig(t.Context(), path); err != nil { + t.Fatal(err) + } + + unmanaged := &nodeHealthState{} + unmanaged.markCNIUnmanaged() + unmanaged.setVerifiedIPAMAssignment([]string{"fd00::/64"}) + + if _, err := unmanaged.getIPAMConfig(); err != nil { + t.Fatal(err) + } +} + +func TestDelegatedIPAMWaitsForStartupAssignment(t *testing.T) { + client := fake.NewClientset(&corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node-a"}}) + health := &nodeHealthState{} + health.markCNIUnmanaged() + + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + + done := make(chan struct{}) + go func() { + defer close(done) + + acquireDelegatedIPAMAssignment(ctx, client, &config{NodeName: "node-a", cniRetryInterval: time.Millisecond}, health) + }() + + if _, err := health.getIPAMConfig(); err == nil { + t.Fatal("empty Node assignment enabled IPAM") + } + + node, err := client.CoreV1().Nodes().Get(t.Context(), "node-a", metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + + node.Spec.PodCIDRs = []string{"10.244.3.0/24"} + if _, err := client.CoreV1().Nodes().Update(t.Context(), node, metav1.UpdateOptions{}); err != nil { + t.Fatal(err) + } + + select { + case <-done: + case <-time.After(3 * time.Second): + t.Fatal("assignment acquisition did not finish") + } + + actions := len(client.Actions()) + + if _, err := health.getIPAMConfig(); err != nil { + t.Fatal(err) + } + + if len(client.Actions()) != actions { + t.Fatal("IPAM request contacted Kubernetes after startup verification") + } +} + +func TestInstallIPAMBinary(t *testing.T) { + source := filepath.Join(t.TempDir(), "source") + directory := t.TempDir() + target := filepath.Join(directory, "unbounded-ipam") + + for _, contents := range []string{"first", "upgraded"} { + if err := os.WriteFile(source, []byte(contents), 0o755); err != nil { + t.Fatal(err) + } + + if err := installIPAMBinary(source, directory); err != nil { + t.Fatal(err) + } + + got, err := os.ReadFile(target) + if err != nil || string(got) != contents { + t.Fatalf("installed %q: %v", got, err) + } + + info, err := os.Stat(target) + if err != nil || info.Mode().Perm() != 0o755 { + t.Fatalf("binary permissions: %v %v", info, err) + } + } + + entries, err := os.ReadDir(directory) + if err != nil || len(entries) != 1 { + t.Fatalf("temporary binary leaked: %v %v", entries, err) + } + + if err := os.Remove(target); err != nil { + t.Fatal(err) + } + + if err := os.Symlink(source, target); err != nil { + t.Fatal(err) + } + + if err := installIPAMBinary(source, directory); err == nil { + t.Fatal("installer replaced symlink") + } + + if err := installIPAMBinary(source, filepath.Join(directory, "unmounted")); err == nil { + t.Fatal("installer created an unmounted directory") + } +} + +func TestIPAMRuntimeConfiguration(t *testing.T) { + for _, override := range []bool{false, true} { + t.Run(map[bool]string{false: "yaml", true: "flags"}[override], func(t *testing.T) { + path := filepath.Join(t.TempDir(), "config.yaml") + + data := "node:\n ipamSocketPath: /run/custom/ipam.sock\n installIPAMPlugin: false\n ipamBinDir: /mounted/cni\n" + if err := os.WriteFile(path, []byte(data), 0o600); err != nil { + t.Fatal(err) + } + + cfg := &config{ + ConfigFile: path, GeneveInterfaceName: "geneve0", VXLANInterfaceName: "vxlan0", + IPIPInterfaceName: "ipip0", WireGuardInterfacePrefix: "wg", + } + cmd := newNodeConfigTestCommand(cfg) + + if override { + for key, value := range map[string]string{ + "ipam-socket-path": "/run/override/ipam.sock", "install-ipam-plugin": "true", "ipam-bin-dir": "/override/cni", + } { + if err := cmd.Flags().Set(key, value); err != nil { + t.Fatal(err) + } + } + } + + if err := applyNodeRuntimeConfig(cmd, cfg); err != nil { + t.Fatal(err) + } + + if override { + if cfg.IPAMSocketPath != "/run/override/ipam.sock" || !cfg.InstallIPAMPlugin || cfg.IPAMBinDir != "/override/cni" { + t.Fatalf("flag precedence: %#v", cfg) + } + } else if cfg.IPAMSocketPath != "/run/custom/ipam.sock" || cfg.InstallIPAMPlugin || cfg.IPAMBinDir != "/mounted/cni" { + t.Fatalf("YAML settings: %#v", cfg) + } + }) + } + + for _, cfg := range []*config{ + {IPAMSocketPath: "relative"}, + {IPAMSocketPath: "/run/../tmp/ipam.sock"}, + {InstallIPAMPlugin: true, IPAMBinDir: "relative"}, + } { + if err := validateIPAMConfig(cfg); err == nil { + t.Fatalf("accepted invalid configuration %#v", cfg) + } + } +} + +func TestIPAMConflistSocketOverride(t *testing.T) { + cfg := newCNISafetyTestConfig(t) + + cfg.IPAMSocketPath = "/run/custom/ipam.sock" + if err := guardedWriteCNIConfig(t.Context(), cfg, []string{"10.244.0.0/24"}, &nodeHealthState{}); err != nil { + t.Fatal(err) + } + + raw, err := os.ReadFile(cniConfigPath(cfg)) + if err != nil || !strings.Contains(string(raw), cfg.IPAMSocketPath) || strings.Contains(string(raw), "10.244.0.0") { + t.Fatalf("custom socket conflist %s: %v", raw, err) + } +} + +func TestIPAMManifestSettings(t *testing.T) { + for _, value := range []string{"", "false"} { + t.Run("install="+value, func(t *testing.T) { + output := t.TempDir() + + data := map[string]string{} + if value != "" { + data["NodeInstallIPAMPlugin"] = value + data["NodeIPAMSocketPath"] = "/run/unbounded-net/custom.sock" + data["NodeIPAMBinDir"] = "/custom/bin" + } + + if err := render.Render("../../deploy/net", output, data); err != nil { + t.Fatal(err) + } + + var cm corev1.ConfigMap + + raw, err := os.ReadFile(filepath.Join(output, "01-configmap.yaml")) + if err != nil { + t.Fatal(err) + } + + if err := yaml.Unmarshal(raw, &cm); err != nil { + t.Fatal(err) + } + + configPath := filepath.Join(output, "config.yaml") + if err := os.WriteFile(configPath, []byte(cm.Data["config.yaml"]), 0o600); err != nil { + t.Fatal(err) + } + + cfg, err := configpkg.LoadRuntimeConfig(configPath) + if err != nil { + t.Fatal(err) + } + + if cfg.Node.InstallIPAMPlugin == nil || *cfg.Node.InstallIPAMPlugin != (value == "") { + t.Fatalf("installation setting %v", cfg.Node.InstallIPAMPlugin) + } + + if value != "" && (cfg.Node.IPAMSocketPath != data["NodeIPAMSocketPath"] || cfg.Node.IPAMBinDir != data["NodeIPAMBinDir"]) { + t.Fatalf("path overrides: %#v", cfg.Node) + } + + raw, err = os.ReadFile(filepath.Join(output, "node", "03-daemonset.yaml")) + if err != nil || !strings.Contains(string(raw), "path: /run/unbounded-net") || strings.Count(string(raw), "mountPath: /host/opt/cni/bin") != 2 { + t.Fatalf("missing socket or agent binary mount: %v", err) + } + }) + } +} + +func TestInvalidAssignmentFailsClosed(t *testing.T) { + cfg := newCNISafetyTestConfig(t) + client := fake.NewClientset(&corev1.Node{ + ObjectMeta: metav1.ObjectMeta{Name: cfg.NodeName}, + Spec: corev1.NodeSpec{PodCIDRs: []string{"malformed"}}, + }) + + health := &nodeHealthState{} + if _, err := waitForPodCIDRsAndConfigure(t.Context(), client, cfg, health); err == nil { + t.Fatal("invalid PodCIDR configured") + } + + if _, err := os.Stat(cniConfigPath(cfg)); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("invalid PodCIDR published conflist: %v", err) + } + + if _, err := health.getIPAMConfig(); err == nil { + t.Fatal("invalid PodCIDR served") + } +} diff --git a/cmd/unbounded-net-node/main.go b/cmd/unbounded-net-node/main.go index c2f8363431..63b70bb7e0 100644 --- a/cmd/unbounded-net-node/main.go +++ b/cmd/unbounded-net-node/main.go @@ -28,6 +28,7 @@ import ( unboundedv1alpha3 "github.com/Azure/unbounded/api/machina/v1alpha3" configpkg "github.com/Azure/unbounded/internal/net/config" + "github.com/Azure/unbounded/internal/net/ipam" "github.com/Azure/unbounded/internal/net/metrics" unboundednetnetlink "github.com/Azure/unbounded/internal/net/netlink" "github.com/Azure/unbounded/internal/version" @@ -56,13 +57,8 @@ type PluginConf struct { // IPAMConfig represents the IPAM configuration type IPAMConfig struct { - Type string `json:"type"` - Ranges [][]IPRange `json:"ranges,omitempty"` -} - -// IPRange represents an IP range for IPAM -type IPRange struct { - Subnet string `json:"subnet"` + Type string `json:"type"` + SocketPath string `json:"socketPath,omitempty"` } // Caps represents plugin capabilities @@ -81,10 +77,14 @@ type config struct { STUNRecheckInterval time.Duration CNIConfDir string CNIConfFile string + IPAMSocketPath string + InstallIPAMPlugin bool + IPAMBinDir string BridgeName string WireGuardDir string WireGuardPort int EnablePolicyRouting bool + ManageSameSitePodRoutes bool MTU int HealthPort int InformerResyncPeriod time.Duration @@ -264,7 +264,7 @@ networking by writing a CNI configuration file based on the node's podCIDRs. It watches the node object in Kubernetes and waits for podCIDRs to be assigned by the unbounded-net-controller. Once assigned, it writes a CNI configuration -file that sets up pod networking using the bridge plugin with host-local IPAM. +file that sets up pod networking using the bridge plugin with unbounded-ipam. It also generates WireGuard keys for the node and stores them in /etc/wireguard, then annotates the node with the public key.`, @@ -300,6 +300,7 @@ then annotates the node with the public key.`, // CNI configuration flags flags.StringVar(&cfg.CNIConfDir, "cni-conf-dir", "/etc/cni/net.d", "Directory to write CNI configuration") flags.StringVar(&cfg.CNIConfFile, "cni-conf-file", "10-unbounded.conflist", "Name of the CNI configuration file") + registerIPAMFlags(flags, cfg) flags.StringVar(&cfg.BridgeName, "bridge-name", "cbr0", "Name of the bridge interface") flags.IntVar(&cfg.MTU, "mtu", 0, "Maximum MTU for tunnel and bridge interfaces (0 automatically derives MTU from each underlay route)") @@ -307,6 +308,7 @@ then annotates the node with the public key.`, flags.StringVar(&cfg.WireGuardDir, "wireguard-dir", "/etc/wireguard", "Directory to store WireGuard keys") flags.IntVar(&cfg.WireGuardPort, "wireguard-port", 51820, "WireGuard listen port") flags.BoolVar(&cfg.EnablePolicyRouting, "enable-policy-routing", false, "Enable policy-based routing on gateway interfaces (deprecated, UNBOUNDED-FORWARD chain rules replace PBR)") + flags.BoolVar(&cfg.ManageSameSitePodRoutes, "manage-same-site-pod-routes", false, "Manage same-site PodCIDR routes even when the Site does not manage CNI") // Tunnel-interface configuration flags. All three shared tunnel device // names must be non-empty, distinct, and must not collide with @@ -417,6 +419,22 @@ func applyNodeRuntimeConfig(cmd *cobra.Command, cfg *config) error { cfg.CNIConfFile = nodeCfg.CNIConfFile } + if !flags.Changed("ipam-socket-path") && nodeCfg.IPAMSocketPath != "" { + cfg.IPAMSocketPath = nodeCfg.IPAMSocketPath + } + + if !flags.Changed("install-ipam-plugin") && nodeCfg.InstallIPAMPlugin != nil { + cfg.InstallIPAMPlugin = *nodeCfg.InstallIPAMPlugin + } + + if !flags.Changed("ipam-bin-dir") && nodeCfg.IPAMBinDir != "" { + cfg.IPAMBinDir = nodeCfg.IPAMBinDir + } + + if err := validateIPAMConfig(cfg); err != nil { + return err + } + if !flags.Changed("bridge-name") && nodeCfg.BridgeName != "" { cfg.BridgeName = nodeCfg.BridgeName } @@ -433,6 +451,10 @@ func applyNodeRuntimeConfig(cmd *cobra.Command, cfg *config) error { cfg.EnablePolicyRouting = *nodeCfg.EnablePolicyRouting //nolint:staticcheck // intentional use of deprecated field } + if !flags.Changed("manage-same-site-pod-routes") && nodeCfg.ManageSameSitePodRoutes != nil { + cfg.ManageSameSitePodRoutes = *nodeCfg.ManageSameSitePodRoutes + } + if !flags.Changed("mtu") && nodeCfg.MTU != nil { cfg.MTU = *nodeCfg.MTU } @@ -682,6 +704,34 @@ func run(cfg *config) error { ctx, cancel := context.WithCancel(context.Background()) defer cancel() + if err := validateIPAMConfig(cfg); err != nil { + return err + } + + if cfg.InstallIPAMPlugin { + if err := installIPAMBinary("/usr/local/bin/unbounded-ipam", cfg.IPAMBinDir); err != nil { + return fmt.Errorf("install unbounded-ipam: %w", err) + } + } + + healthState := &nodeHealthState{nodeName: cfg.NodeName} + + ipamServer, err := ipam.Listen(ctx, cfg.IPAMSocketPath, healthState.getIPAMConfig) + if err != nil { + return fmt.Errorf("start IPAM server: %w", err) + } + + defer func() { + if err := ipamServer.Close(); err != nil { + klog.Errorf("Stop IPAM server: %v", err) + } + }() + + go func() { + <-ipamServer.Done() + cancel() + }() + // Handle shutdown signals sigCh := make(chan os.Signal, 1) signal.Notify(sigCh, syscall.SIGINT, syscall.SIGTERM) @@ -708,7 +758,6 @@ func run(cfg *config) error { // Build Kubernetes client var ( restConfig *rest.Config - err error ) if cfg.KubeconfigPath != "" { @@ -814,17 +863,14 @@ func run(cfg *config) error { } // Create shared health state for health server - healthState := &nodeHealthState{ - nodeName: cfg.NodeName, - informersSynced: []cache.InformerSynced{ - sliceInformer.HasSynced, - siteInformer.HasSynced, - gatewayPoolInformer.HasSynced, - gatewayNodeInformer.HasSynced, - sitePeeringInformer.HasSynced, - assignmentInformer.HasSynced, - poolPeeringInformer.HasSynced, - }, + healthState.informersSynced = []cache.InformerSynced{ + sliceInformer.HasSynced, + siteInformer.HasSynced, + gatewayPoolInformer.HasSynced, + gatewayNodeInformer.HasSynced, + sitePeeringInformer.HasSynced, + assignmentInformer.HasSynced, + poolPeeringInformer.HasSynced, } // Start health server if enabled (readiness should not wait on site membership) @@ -899,6 +945,12 @@ func run(cfg *config) error { nodePodCIDRs = node.Spec.PodCIDRs healthState.setCNIAssignment(nodePodCIDRs) + + if _, err := ipam.ConfigFromCIDRs(nodePodCIDRs); err == nil { + healthState.setVerifiedIPAMAssignment(nodePodCIDRs) + } else { + go acquireDelegatedIPAMAssignment(ctx, clientset, cfg, healthState) + } } // After CNI is configured (or skipped), watch Site CRD for WireGuard peers diff --git a/cmd/unbounded-net-node/main_config_test.go b/cmd/unbounded-net-node/main_config_test.go index 1a87b877e3..7c7282d582 100644 --- a/cmd/unbounded-net-node/main_config_test.go +++ b/cmd/unbounded-net-node/main_config_test.go @@ -26,6 +26,7 @@ func newNodeConfigTestCommand(cfg *config) *cobra.Command { flags.DurationVar(&cfg.STUNRecheckInterval, "stun-recheck-interval", defaultSTUNRecheckInterval, "") flags.StringVar(&cfg.CNIConfDir, "cni-conf-dir", "/etc/cni/net.d", "") flags.StringVar(&cfg.CNIConfFile, "cni-conf-file", "10-unbounded.conflist", "") + registerIPAMFlags(flags, cfg) flags.StringVar(&cfg.BridgeName, "bridge-name", "cbr0", "") flags.StringVar(&cfg.WireGuardDir, "wireguard-dir", "/etc/wireguard", "") flags.IntVar(&cfg.WireGuardPort, "wireguard-port", 51820, "") diff --git a/cmd/unbounded-net-node/main_update_test.go b/cmd/unbounded-net-node/main_update_test.go index 5bdb9c792e..ac26969cd1 100644 --- a/cmd/unbounded-net-node/main_update_test.go +++ b/cmd/unbounded-net-node/main_update_test.go @@ -145,7 +145,7 @@ func TestUpdateWireGuardFromSlices_SitePodCIDRPoolChanges(t *testing.T) { configureCalls++ gotPools = append([]string(nil), state.sitePodCIDRPools...) - gotSupernets = collectSupernets(state, peers, gatewayPeers, gatewayPeers) + gotSupernets = collectSupernets(state, peers, gatewayPeers, gatewayPeers, nil, true) return configureErr } @@ -629,6 +629,15 @@ func TestUpdateWireGuardFromSlices_ManageCniPluginFalseSkipsPodCIDRRoutesForSame if !gotGatewayPeers[0].SkipPodCIDRRoutes { t.Fatalf("expected same-site gateway peer to skip podCIDR routes when manageCniPlugin is false, got %#v", gotGatewayPeers[0]) } + + cfg.ManageSameSitePodRoutes = true + if err := updateWireGuardFromSlices(t.Context(), nil, siteInformer, sliceInformer, gatewayPoolInformer, gatewayNodeInformer, sitePeeringInformer, assignmentInformer, poolPeeringInformer, cfg, mySiteName, "priv", myPubKey, false, state); err != nil { + t.Fatal(err) + } + + if len(gotGatewayPeers) != 1 || gotGatewayPeers[0].SkipPodCIDRRoutes { + t.Fatalf("explicit same-site routing override was ignored: %#v", gotGatewayPeers) + } }) t.Run("gateway node marks same-site mesh peer podCIDR routes to skip", func(t *testing.T) { diff --git a/cmd/unbounded-net-node/site_watch_reconcile.go b/cmd/unbounded-net-node/site_watch_reconcile.go index a5ed0b4be0..d0ac214fb0 100644 --- a/cmd/unbounded-net-node/site_watch_reconcile.go +++ b/cmd/unbounded-net-node/site_watch_reconcile.go @@ -11,6 +11,7 @@ import ( "os" "path/filepath" "reflect" + "slices" "sort" "strings" "sync" @@ -88,25 +89,7 @@ func watchSiteAndConfigureWireGuard(ctx context.Context, clientset kubernetes.In state.routeManager = routeManager state.routeTableID = cfg.RouteTableID - // Set preferred source IPs for routes based on node's podCIDRs. - // Collect both address families before calling SetPreferredSourceIPs - // to avoid one family overwriting the other with nil. - var preferredSrcIPv4, preferredSrcIPv6 net.IP - - for _, cidr := range nodePodCIDRs { - gwIP := getGatewayIPFromCIDR(cidr) - if gwIP == nil { - continue - } - - if gwIP.To4() != nil { - preferredSrcIPv4 = gwIP - } else { - preferredSrcIPv6 = gwIP - } - } - - routeManager.SetPreferredSourceIPs(preferredSrcIPv4, preferredSrcIPv6) + setPodCIDRPreferredSources(routeManager, nodePodCIDRs) // Initialize healthcheck manager for peer health monitoring. healthCheckMgr, err := healthcheck.NewManager(cfg.NodeName, cfg.HealthCheckPort, func(peerHostname string, newState, oldState healthcheck.SessionState) { @@ -931,7 +914,7 @@ var configureWireGuardFunc = configureWireGuard // It uses per-node podCIDRs from slices and gateway pools for routing // It also looks up gateway pools referenced by the site and adds gateway peers with all remote sites' CIDRs // For gateway nodes, it adds all nodes from all sites as peers (pubkey only, no endpoint) -// When manageCniPlugin is false on non-gateway nodes, same-site peers are skipped but +// Unless same-site routing is explicitly enabled, unmanaged non-gateway nodes skip same-site peers but // directly peered remote-site peers are still included for podCIDR routing. // Sites that are directly peered via SitePeering objects are treated as if they were the same site func updateWireGuardFromSlices(ctx context.Context, dynamicClient dynamic.Interface, siteInformer, sliceInformer, gatewayPoolInformer, gatewayNodeInformer, sitePeeringInformer, assignmentInformer, poolPeeringInformer cache.SharedIndexInformer, cfg *config, mySiteName, privKey, myPubKey string, manageCniPlugin bool, state *wireGuardState) error { @@ -1760,7 +1743,7 @@ func updateWireGuardFromSlices(ctx context.Context, dynamicClient dynamic.Interf RouteDistances: routeDistances, LearnedRoutes: learnedRoutes, PodCIDRs: gwNode.PodCIDRs, - SkipPodCIDRRoutes: !manageCniPlugin && !isGatewayNode && gwNode.SiteName == mySiteName, + SkipPodCIDRRoutes: !manageCniPlugin && !cfg.ManageSameSitePodRoutes && !isGatewayNode && gwNode.SiteName == mySiteName, }, siteRouting.localCIDRs)) } } @@ -1820,9 +1803,9 @@ func updateWireGuardFromSlices(ctx context.Context, dynamicClient dynamic.Interf continue } } - // When manageCniPlugin is false, skip same-site peers for non-gateway nodes. + // An unmanaged CNI owns same-site routes unless explicitly overridden. // Keep directly peered remote sites so unmanaged sites can still route to managed remote pods. - if !manageCniPlugin && !isGatewayNode && slice.SiteName == mySiteName { + if !manageCniPlugin && !cfg.ManageSameSitePodRoutes && !isGatewayNode && slice.SiteName == mySiteName { continue } @@ -1853,7 +1836,7 @@ func updateWireGuardFromSlices(ctx context.Context, dynamicClient dynamic.Interf WireGuardPublicKey: node.WireGuardPublicKey, InternalIPs: node.InternalIPs, PodCIDRs: node.PodCIDRs, - SkipPodCIDRRoutes: !manageCniPlugin && isGatewayNode && slice.SiteName == mySiteName, + SkipPodCIDRRoutes: !manageCniPlugin && !cfg.ManageSameSitePodRoutes && isGatewayNode && slice.SiteName == mySiteName, }) } } @@ -1892,7 +1875,7 @@ func updateWireGuardFromSlices(ctx context.Context, dynamicClient dynamic.Interf } } - if !manageCniPlugin && !isGatewayNode { + if !manageCniPlugin && !cfg.ManageSameSitePodRoutes && !isGatewayNode { klog.V(2).Info("manageCniPlugin is false - skipping same-site tunnel peers for non-gateway nodes") } @@ -1981,6 +1964,15 @@ func updateWireGuardFromSlices(ctx context.Context, dynamicClient dynamic.Interf // netlink operations so getNodeStatus() is not blocked. state.mu.Lock() + prevNodePodCIDRs := state.nodePodCIDRs + nodePodCIDRs := prevNodePodCIDRs + + if state.healthState != nil { + if verified := state.healthState.verifiedIPAMAssignment(); len(verified) > 0 { + nodePodCIDRs = verified + } + } + prevIsGatewayNode := state.isGatewayNode prevMyGatewayPort := state.myGatewayPort prevLocalGatewayPools := append([]string(nil), state.localGatewayPools...) @@ -1992,7 +1984,7 @@ func updateWireGuardFromSlices(ctx context.Context, dynamicClient dynamic.Interf !strSliceEqual(prevLocalGatewayPools, localGatewayPools) // Check if tunnel configuration needs to change - wgChanged := roleChanged || + wgChanged := !slices.Equal(prevNodePodCIDRs, nodePodCIDRs) || roleChanged || !reflect.DeepEqual(state.siteRouting, siteRouting) || !meshPeersEqual(state.peers, peers) || !strSliceEqual(state.sitePodCIDRs, sitePodCIDRs) || @@ -2043,9 +2035,14 @@ func updateWireGuardFromSlices(ctx context.Context, dynamicClient dynamic.Interf state.sitePodCIDRPools = append([]string(nil), sitePodCIDRPools...) state.sitePodCIDRs = append([]string(nil), sitePodCIDRs...) state.siteRouting = siteRouting + state.nodePodCIDRs = nodePodCIDRs state.mu.Unlock() + if state.routeManager != nil { + setPodCIDRPreferredSources(state.routeManager, nodePodCIDRs) + } + committed := false defer func() { @@ -2062,7 +2059,12 @@ func updateWireGuardFromSlices(ctx context.Context, dynamicClient dynamic.Interf state.sitePodCIDRPools = prevSitePodCIDRPools state.sitePodCIDRs = prevSitePodCIDRs state.siteRouting = prevSiteRouting + state.nodePodCIDRs = prevNodePodCIDRs state.mu.Unlock() + + if state.routeManager != nil { + setPodCIDRPreferredSources(state.routeManager, prevNodePodCIDRs) + } }() // Phase 2: Expensive network operations (lock-free). diff --git a/cmd/unbounded-net-node/status_server.go b/cmd/unbounded-net-node/status_server.go index 2cc9f6c26f..c4270de0dd 100644 --- a/cmd/unbounded-net-node/status_server.go +++ b/cmd/unbounded-net-node/status_server.go @@ -46,11 +46,12 @@ type nodeHealthState struct { statusServer *nodeStatusServer // Set once WireGuard state is available informersSynced []cache.InformerSynced // Informer HasSynced funcs for readiness - nodeName string - siteName string - pubKey string - podCIDRs []string - isGateway bool + nodeName string + siteName string + pubKey string + podCIDRs []string + verifiedIPAMCIDRs []string + isGateway bool cniManaged bool cniReady bool @@ -183,6 +184,7 @@ func (h *nodeHealthState) setCNIReady(bridgeName string, podCIDRs []string) { h.cniLastLogTime = time.Time{} h.podCIDRs = append([]string(nil), podCIDRs...) + h.verifiedIPAMCIDRs = append([]string(nil), podCIDRs...) h.mu.Unlock() if wasBlocked { diff --git a/cmd/unbounded-net-node/tunnel_config.go b/cmd/unbounded-net-node/tunnel_config.go index 8e0551479b..df96e19732 100644 --- a/cmd/unbounded-net-node/tunnel_config.go +++ b/cmd/unbounded-net-node/tunnel_config.go @@ -45,10 +45,7 @@ func buildSupernetRoutes(cfg *config, state *wireGuardState, meshPeers []meshPee return nil } - supernets := collectSupernets(state, meshPeers, gatewayPeers, gatewayPeers) - for _, cidr := range extraSupernets { - supernets[cidr] = true - } + supernets := collectSupernets(state, meshPeers, gatewayPeers, gatewayPeers, extraSupernets, state.manageCniPlugin || cfg.ManageSameSitePodRoutes) supernets = excludeLocalSupernets(supernets, meshPeers, state.siteRouting.localCIDRs) @@ -611,11 +608,13 @@ func resolveTunnelPeerTarget(protocol string, geneveIfIdx, ipipIfIdx, vxlanIfIdx // allGatewayPeers includes ALL gateway peers (GENEVE, VXLAN, and WG) so that // cross-site supernet routes are created regardless of the tunnel protocol // used for the gateway link. -func collectSupernets(state *wireGuardState, meshPeers []meshPeerInfo, gatewayPeers, allGatewayPeers []gatewayPeerInfo) map[string]bool { +func collectSupernets(state *wireGuardState, meshPeers []meshPeerInfo, gatewayPeers, allGatewayPeers []gatewayPeerInfo, extraSupernets []string, manageLocalPodRoutes bool) map[string]bool { supernets := make(map[string]bool) // Site pod CIDR assignment pools (supernets covering all node podCIDRs). - for _, cidr := range state.sitePodCIDRPools { - supernets[cidr] = true + if manageLocalPodRoutes { + for _, cidr := range state.sitePodCIDRPools { + supernets[cidr] = true + } } // Gateway pool RoutedCidrs (cross-site supernets). for _, cidr := range state.sitePodCIDRs { @@ -629,6 +628,19 @@ func collectSupernets(state *wireGuardState, meshPeers []meshPeerInfo, gatewayPe supernets[cidr] = true } } + + for _, cidr := range extraSupernets { + supernets[cidr] = true + } + + if !manageLocalPodRoutes { + localPools := buildNormalizedCIDRSet(state.sitePodCIDRPools) + for cidr := range supernets { + if _, local := localPools[normalizeCIDR(cidr)]; local { + delete(supernets, cidr) + } + } + } // Remove the local node's own podCIDRs (those are handled by the bridge directly). for _, cidr := range state.nodePodCIDRs { delete(supernets, cidr) @@ -660,12 +672,20 @@ func collectSupernets(state *wireGuardState, meshPeers []meshPeerInfo, gatewayPe } for _, peer := range meshPeers { + if peer.SkipPodCIDRRoutes { + continue + } + for _, cidr := range peer.PodCIDRs { addIfUncovered(cidr) } } for _, gwPeer := range gatewayPeers { + if gwPeer.SkipPodCIDRRoutes { + continue + } + for _, cidr := range gwPeer.PodCIDRs { addIfUncovered(cidr) } diff --git a/cmd/unbounded-net-node/wireguard_config.go b/cmd/unbounded-net-node/wireguard_config.go index 4c3e486b5d..5f5cce3e80 100644 --- a/cmd/unbounded-net-node/wireguard_config.go +++ b/cmd/unbounded-net-node/wireguard_config.go @@ -272,6 +272,9 @@ func configureWireGuard(ctx context.Context, cfg *config, privKey string, peers return fmt.Errorf("failed to bring up WireGuard interface: %w", err) } + // A delegated CNI can reset default.rp_filter after our init container. + disableRPFilter(iface) + // Accept forwarded traffic arriving on the mesh WG interface. if state.forwardManager != nil { state.forwardManager.EnsureInterface(iface) @@ -477,6 +480,8 @@ func configureWireGuard(ctx context.Context, cfg *config, privKey string, peers continue } + disableRPFilter(gwIfaceName) + // Accept forwarded traffic arriving on the gateway WG interface. if state.forwardManager != nil { state.forwardManager.EnsureInterface(gwIfaceName) diff --git a/deploy/machina/crd/unbounded-cloud.io_sites.yaml b/deploy/machina/crd/unbounded-cloud.io_sites.yaml index 7232df4607..af8d98bb89 100644 --- a/deploy/machina/crd/unbounded-cloud.io_sites.yaml +++ b/deploy/machina/crd/unbounded-cloud.io_sites.yaml @@ -188,10 +188,10 @@ spec: CNI config is written and all same-site nodes are tunnel peers. When false, only tunnel links to gateway pools for other sites are created, allowing an external CNI plugin to manage intra-site networking. - Pod CIDR assignment is also disabled when manageCniPlugin is false, - regardless of the assignmentEnabled setting on individual podCidrAssignments - rules. The podCidrAssignments are still required to define the CIDR ranges - used for inter-site routing. + Omitted assignmentEnabled values on podCidrAssignments default to this + setting. Explicit assignmentEnabled values override that default. + The podCidrAssignments are still required to define the CIDR ranges + used for inter-site routing, even when assignment is disabled. type: boolean nodeCidrs: description: NodeCidrs are the CIDRs that contain the internal IPs @@ -216,8 +216,10 @@ spec: for nodes in a site. properties: assignmentEnabled: - description: AssignmentEnabled controls whether this assignment - is active. Defaults to true. + description: |- + AssignmentEnabled controls whether this assignment is active. When omitted, + it inherits the Site's manageCniPlugin setting, which defaults to true. + Explicit true or false overrides the inherited value. type: boolean cidrBlocks: description: CidrBlocks are the CIDR pools to allocate from. diff --git a/deploy/net/01-configmap.yaml.tmpl b/deploy/net/01-configmap.yaml.tmpl index fcb5054078..fc80097785 100644 --- a/deploy/net/01-configmap.yaml.tmpl +++ b/deploy/net/01-configmap.yaml.tmpl @@ -47,6 +47,10 @@ data: stunRecheckInterval: "{{ default "1h" .NodeSTUNRecheckInterval }}" cniConfDir: "{{ default "/host/etc/cni/net.d" .NodeCNIConfDir }}" cniConfFile: "{{ default "10-unbounded.conflist" .NodeCNIConfFile }}" + ipamSocketPath: "{{ default "/run/unbounded-net/ipam.sock" .NodeIPAMSocketPath }}" + installIPAMPlugin: {{ default "true" .NodeInstallIPAMPlugin }} + ipamBinDir: "{{ default "/host/opt/cni/bin" .NodeIPAMBinDir }}" + manageSameSitePodRoutes: {{ default "false" .NodeManageSameSitePodRoutes }} bridgeName: "{{ default "cbr0" .NodeBridgeName }}" wireGuardDir: "{{ default "/host/etc/wireguard" .NodeWireGuardDir }}" wireGuardPort: {{ default "51820" .NodeWireGuardPort }} diff --git a/deploy/net/node/03-daemonset.yaml.tmpl b/deploy/net/node/03-daemonset.yaml.tmpl index 716a74acde..910e30d6f1 100644 --- a/deploy/net/node/03-daemonset.yaml.tmpl +++ b/deploy/net/node/03-daemonset.yaml.tmpl @@ -82,6 +82,10 @@ spec: volumeMounts: - name: cni-conf mountPath: /host/etc/cni/net.d + - name: cni-bin + mountPath: /host/opt/cni/bin + - name: ipam-socket + mountPath: /run/unbounded-net - name: wireguard mountPath: /host/etc/wireguard - name: iproute2 @@ -121,6 +125,10 @@ spec: periodSeconds: 5 timeoutSeconds: 5 volumes: + - name: ipam-socket + hostPath: + path: /run/unbounded-net + type: DirectoryOrCreate - name: cni-bin hostPath: path: /opt/cni/bin diff --git a/docs/content/reference/networking/architecture.md b/docs/content/reference/networking/architecture.md index e8adb75eb1..ad3e3f5144 100644 --- a/docs/content/reference/networking/architecture.md +++ b/docs/content/reference/networking/architecture.md @@ -108,8 +108,9 @@ with the `manageCniPlugin` field. ### Configuring the Primary Site For the Site that maps to your existing cluster, set `manageCniPlugin: false`. -This automatically disables pod CIDR assignment for that site regardless of the -`assignmentEnabled` value on individual `podCidrAssignments` rules. The +This disables pod CIDR assignment by default. An explicit +`assignmentEnabled: true` on a `podCidrAssignments` rule still enables assignment +without managing the existing CNI. The `podCidrAssignments` block is still required: it defines the CIDR ranges that the other sites and gateways use to route traffic back to the primary cluster's pods. @@ -125,7 +126,7 @@ spec: # config or mesh same-site nodes, and pod CIDR assignment is disabled. manageCniPlugin: false podCidrAssignments: - # Pod CIDR assignment is disabled by manageCniPlugin: false, but the CIDR + # Omitted assignmentEnabled inherits false, but the CIDR # range is still declared so other sites can route to these pods. - cidrBlocks: - "10.244.0.0/16" diff --git a/docs/content/reference/networking/custom-resources.md b/docs/content/reference/networking/custom-resources.md index 3c3ca5aef1..006ab31382 100644 --- a/docs/content/reference/networking/custom-resources.md +++ b/docs/content/reference/networking/custom-resources.md @@ -57,7 +57,7 @@ spec: | Field | Type | Required | Description | |-------|------|----------|-------------| -| `assignmentEnabled` | `*bool` | No | Enables this assignment (default: `true`). | +| `assignmentEnabled` | `*bool` | No | Enables this assignment. When omitted, inherits `manageCniPlugin` (default: `true`). Explicit `true` or `false` overrides that default. | | `cidrBlocks` | `[]string` | No | CIDR pools to allocate from (IPv4 and/or IPv6). | | `nodeBlockSizes.ipv4` | `int` | No | IPv4 subnet size for node allocations (default: `/24`). | | `nodeBlockSizes.ipv6` | `int` | No | IPv6 subnet size for node allocations (default: pool prefix + 16). | @@ -81,6 +81,12 @@ spec: Set to `false` when using an external CNI plugin (e.g., Cilium, Calico) for intra-site networking while unbounded-net handles only cross-site routing. +With `manageCniPlugin: false`, omitted `assignmentEnabled` values resolve to +`false`. Set `podCidrAssignments[].assignmentEnabled: true` to allocate Node +PodCIDRs without managing the external CNI. Explicit `false` always disables a +rule. Inherited values are not persisted and follow changes to `manageCniPlugin`; +existing explicit values remain unchanged. + ### Admission Validation - `nodeCidrs` must be valid CIDRs and cannot overlap across sites. diff --git a/docs/net/configuration.md b/docs/net/configuration.md index 05ca10429f..aa37bc680a 100644 --- a/docs/net/configuration.md +++ b/docs/net/configuration.md @@ -100,6 +100,9 @@ controller: node: cniConfDir: /host/etc/cni/net.d cniConfFile: 10-unbounded.conflist + ipamSocketPath: /run/unbounded-net/ipam.sock + installIPAMPlugin: true + ipamBinDir: /host/opt/cni/bin bridgeName: cbr0 wireGuardDir: /host/etc/wireguard wireGuardPort: 51820 @@ -293,6 +296,9 @@ graph TD |------|------|---------|-------------| | `--cni-conf-dir` | string | `/etc/cni/net.d` | Directory to write CNI configuration. | | `--cni-conf-file` | string | `10-unbounded.conflist` | Name of the CNI configuration file. | +| `--ipam-socket-path` | string | `/run/unbounded-net/ipam.sock` | Host-shared Unix socket used by the IPAM plugin. | +| `--install-ipam-plugin` | bool | `true` | Atomically install or upgrade the bundled IPAM binary on agent startup. | +| `--ipam-bin-dir` | string | `/host/opt/cni/bin` | Host-mounted binary installation directory, as seen by the agent. | | `--bridge-name` | string | `cbr0` | Name of the bridge interface. | | `--mtu` | int | `0` | Optional maximum MTU for tunnel and bridge interfaces. With `0`, the node agent derives each route's MTU from the selected underlay route and encapsulation overhead. Positive values cap the automatically detected and CRD-configured MTU. | @@ -628,11 +634,7 @@ The node agent generates a CNI configuration file with the following structure: "hairpinMode": true, "mtu": 1500, "ipam": { - "type": "host-local", - "ranges": [ - [{"subnet": "100.64.0.0/24"}], - [{"subnet": "fdde::100/80"}] - ] + "type": "unbounded-ipam" } }, { @@ -648,6 +650,102 @@ The node agent generates a CNI configuration file with the following structure: } ``` +### Verified IPAM allocation + +`unbounded-ipam` is a Go CNI IPAM plugin that embeds the upstream host-local +allocator and disk store. It takes standard CNI JSON on stdin and returns a +standard CNI JSON result on stdout. It does not create interfaces or program +routes. The default managed conflist carries no PodCIDRs or plugin arguments. + +On ADD, the plugin requests `GetIPAMConfig` over +`/run/unbounded-net/ipam.sock`. This is a length-delimited protobuf exchange, +not HTTP or gRPC, and is not exposed on the agent's TCP status endpoint. +The agent obtains the Node's nonempty PodCIDRs from Kubernetes during startup +and keeps the successful assignment only in memory. Managed sites also require +successful bridge inspection and CNI publication. Missing or unverified +assignments, a blocked managed bridge, an unavailable socket, and malformed or +timed-out replies cause ADD to fail without allocating an address. The runtime +can retry pod sandbox creation. No disk-cached CIDR or direct host-local fallback +is used. The exchange uses protocol version 1, a 64 KiB serialized-message +limit, and a five-second deadline. + +The plugin then uses the verified ranges with host-local's normal IPv4/IPv6 +allocation and locking. The calling CNI's network name remains the allocation +namespace; reservations remain compatible with host-local under +`/var/lib/cni/networks/`. Requested IPs must belong to verified +ranges. Caller-supplied IPAM allocation settings and `runtimeConfig.ipRanges` +are rejected so they cannot bypass verification. + +DEL releases the attachment's local reservations without contacting the agent +or needing the old CIDRs, including legacy host-local reservations. CHECK +checks local reservations, and VERSION reports supported CNI versions; both +also work without the agent. DEL is safe to repeat. + +The ConfigMap's `node.installIPAMPlugin` controls installation independently +of `Site.spec.manageCniPlugin`. Set it to false to use a preinstalled binary. +The agent still serves IPAM and generated conflists still use `unbounded-ipam`. +The default DaemonSet mounts the host's `/opt/cni/bin` at `/host/opt/cni/bin` +for installation and shares `/run/unbounded-net` with the host for the socket. +The socket is root-accessible only. + +Set `node.ipamSocketPath` and, for a third-party CNI, the matching optional +`ipam.socketPath` to use a nondefault socket. Managed conflists include that +option automatically when needed: + +```json +{ + "ipam": { + "type": "unbounded-ipam", + "socketPath": "/run/unbounded-net/custom.sock" + } +} +``` + +Custom socket directories and `node.ipamBinDir` require matching writable host +mounts through workload overrides. The installation directory must also be in +the runtime's CNI binary search path. The socket directory must be owned by the +agent user and must not be writable by other users. The agent requires an +existing installation directory rather than creating a container-local binary +directory. Ensure that directory is actually backed by the host mount. + +The agent retains a `.lock` file beside the socket. An exclusive +lock protects stale-socket recovery and shutdown cleanup across processes. +The file remains after shutdown; do not remove it while an agent is running. + +For a third-party CNI, set `manageCniPlugin: false` on the Site and delegate +IPAM to `unbounded-ipam`. Set `assignmentEnabled: true` on each desired +`spec.podCidrAssignments` rule to have Unbounded allocate Node PodCIDRs, or use +another controller that populates `Node.spec.podCIDRs`. With CNI management +disabled, omitted assignment settings default to disabled. The agent obtains +the startup assignment and serves IPAM without inspecting or changing that +CNI's bridge or conflist. Nodes with no assignment remain unavailable for +allocation without blocking their +otherwise-unmanaged routing bootstrap. To retain direct host-local instead, +provide your own conflist and set `manageCniPlugin: false`. + +Delegated IPAM does not provide inter-node routing. If the third-party CNI +does not install those routes, set `node.manageSameSitePodRoutes: true` to +have Unbounded manage same-site PodCIDR routes without managing the CNI +configuration. The default remains false for unmanaged CNI sites. A Site +with `tunnelProtocol: None` requires an underlay that routes unencapsulated +PodCIDR traffic between peers. Otherwise, select an encapsulated transport. +For delegated Cilium with Unbounded-managed routes, limit Cilium's managed +devices to its underlay interface (for example, Helm `devices=eth0`), so it +does not replace the TC programs on Unbounded's tunnel interfaces. + +**Migration:** The allocation gate only applies after the runtime loads a +conflist that invokes `unbounded-ipam`. Install the binary, replace the active +configuration, and verify your runtime has reloaded it; follow your runtime's +reload/restart procedure when necessary. A cached legacy conflist invoking +host-local directly still bypasses the gate. The agent does not restart the +runtime or evict existing pods. + +**Scope:** Verification is performed at startup, not before each ADD. An agent +restart requires verification again, but a temporary API outage after verified +startup does not stop allocation. Deletion/recreation while the same agent +process stays running is not detected by a long-term Node watch. The gate +does not fence allocations already in progress or repair existing stale pods. + ### Managed bridge PodCIDR safety For sites with `manageCniPlugin: true`, the node agent checks the live interfaces @@ -715,7 +813,10 @@ its CNI configuration, and another CNI configuration may still be active. Disabling this conflist does not terminate existing workloads, disable other CNIs, or guarantee that the runtime stops creating sandboxes. The guard is a pre-write snapshot check, not continuous monitoring or atomic coordination with -concurrent sandbox creation. +concurrent sandbox creation. The IPAM socket additionally blocks new +`unbounded-ipam` ADD requests while the managed guard is blocked, even if the +runtime has cached the new conflist. Cached legacy host-local configurations +and other IPAM plugins are not protected by that gate. ### CNI Plugin Chain @@ -723,7 +824,9 @@ concurrent sandbox creation. flowchart LR A[Container Created] --> B[bridge plugin] B -->|Create veth pair| C[Attach to cbr0] - C -->|Assign IP| D[host-local IPAM] + C -->|Assign IP| D[unbounded-ipam] + D -->|Verified ranges over Unix protobuf| I[Local node agent] + D -->|Embedded allocator| J[host-local allocator] D --> E[portmap plugin] E -->|Configure hostPort| F[iptables rules] F --> G[loopback plugin] diff --git a/docs/net/custom-resources.md b/docs/net/custom-resources.md index 9dbbe9c697..61bfe12313 100644 --- a/docs/net/custom-resources.md +++ b/docs/net/custom-resources.md @@ -65,7 +65,7 @@ status: | Field | Type | Required | Description | |-------|------|----------|-------------| -| `assignmentEnabled` | `*bool` | No | Enables this assignment (default: `true`). Disabled assignments are ignored. | +| `assignmentEnabled` | `*bool` | No | Enables this assignment. When omitted, inherits `manageCniPlugin` (default: `true`). Explicit `true` or `false` overrides that default. | | `cidrBlocks` | `[]string` | No | CIDR pools to allocate from (IPv4 and/or IPv6). | | `nodeBlockSizes.ipv4` | `int` | No | IPv4 subnet size for node allocations (default: `/24` when IPv4 pools exist). | | `nodeBlockSizes.ipv6` | `int` | No | IPv6 subnet size for node allocations (default: first IPv6 pool prefix + 16). | @@ -91,10 +91,10 @@ The node agent reads this field only at startup. Restart the node agents after c | Value | CNI Config | Same-Site WireGuard Peers | Pod CIDR Assignment | Gateway WireGuard Links | |-------|------------|---------------------------|---------------------|------------------------| -| `true` (default) | Written | Created | Enabled | Created | -| `false` | **Not written** | **Not created** | **Disabled** | Created | +| `true` (default) | Written | Created | Enabled by default | Created | +| `false` | **Not written** | **Not created** | Disabled by default | Created | -When `manageCniPlugin` is `false`, pod CIDR assignment by the controller is disabled even if `assignmentEnabled: true` is set on individual `podCidrAssignments` rules. The `podCidrAssignments` entries are still required because their CIDR blocks define the pod address ranges used for inter-site routing. +When `manageCniPlugin` is `false`, omitted `assignmentEnabled` values resolve to `false`. Set `assignmentEnabled: true` on an individual `podCidrAssignments` rule to have the controller allocate Node PodCIDRs while an external CNI manages networking. Explicit `false` disables a rule regardless of `manageCniPlugin`. Omitted values are resolved dynamically, not persisted, so they follow later changes to `manageCniPlugin`. Existing explicit values remain unchanged. The `podCidrAssignments` entries are still required because their CIDR blocks define the pod address ranges used for inter-site routing. **Use `manageCniPlugin: false` when:** - Using an external CNI plugin (e.g., Cilium, Calico) for intra-site networking diff --git a/images/net/Containerfile b/images/net/Containerfile index 4cd4d43bed..2a2c11ebe9 100644 --- a/images/net/Containerfile +++ b/images/net/Containerfile @@ -104,12 +104,13 @@ ARG BUILD_TIME= RUN --mount=type=cache,target=/go/pkg/mod \ --mount=type=cache,target=/root/.cache/go-build,id=go-build-${TARGETARCH} \ GOOS=${TARGETOS} GOARCH=${TARGETARCH} \ - make unbounded-net-controller-build unbounded-net-node-build unping-build unroute-build \ + make unbounded-net-controller-build unbounded-net-node-build unbounded-ipam-build unping-build unroute-build \ VERSION=${VERSION} \ ${GIT_COMMIT:+GIT_COMMIT=${GIT_COMMIT}} \ ${BUILD_TIME:+BUILD_TIME=${BUILD_TIME}} \ && cp bin/unbounded-net-controller /unbounded-net-controller \ && cp bin/unbounded-net-node /unbounded-net-node \ + && cp bin/unbounded-ipam /unbounded-ipam \ && cp bin/unping /unping \ && cp bin/unroute /unroute @@ -163,6 +164,7 @@ RUN chmod 0755 /usr/local/bin/node-agent-init.sh COPY --from=builder /cni-plugins/ /opt/cni/bin/ COPY --from=builder /usr/local/bin/ntttcp /usr/local/bin/ntttcp COPY --from=builder /unbounded-net-node /usr/local/bin/unbounded-net-node +COPY --from=builder /unbounded-ipam /usr/local/bin/unbounded-ipam COPY --from=builder /unping /usr/local/bin/unping COPY --from=builder /unroute /usr/local/bin/unroute COPY LICENSE NOTICE /unbounded/licenses/ diff --git a/internal/net/config/runtime_config.go b/internal/net/config/runtime_config.go index 992de96f74..9c7034299f 100644 --- a/internal/net/config/runtime_config.go +++ b/internal/net/config/runtime_config.go @@ -62,17 +62,21 @@ type ControllerLeaderElectionYAML struct { // NodeRuntimeConfig contains node-agent runtime settings. type NodeRuntimeConfig struct { - InformerResyncPeriod string `yaml:"informerResyncPeriod"` - NodeName string `yaml:"nodeName"` - STUNEnabled *bool `yaml:"stunEnabled"` - STUNHost string `yaml:"stunHost"` - STUNPort *int `yaml:"stunPort"` - STUNRecheckInterval string `yaml:"stunRecheckInterval"` - CNIConfDir string `yaml:"cniConfDir"` - CNIConfFile string `yaml:"cniConfFile"` - BridgeName string `yaml:"bridgeName"` - WireGuardDir string `yaml:"wireGuardDir"` - WireGuardPort *int `yaml:"wireGuardPort"` + InformerResyncPeriod string `yaml:"informerResyncPeriod"` + NodeName string `yaml:"nodeName"` + STUNEnabled *bool `yaml:"stunEnabled"` + STUNHost string `yaml:"stunHost"` + STUNPort *int `yaml:"stunPort"` + STUNRecheckInterval string `yaml:"stunRecheckInterval"` + CNIConfDir string `yaml:"cniConfDir"` + CNIConfFile string `yaml:"cniConfFile"` + IPAMSocketPath string `yaml:"ipamSocketPath"` + InstallIPAMPlugin *bool `yaml:"installIPAMPlugin"` + IPAMBinDir string `yaml:"ipamBinDir"` + ManageSameSitePodRoutes *bool `yaml:"manageSameSitePodRoutes"` + BridgeName string `yaml:"bridgeName"` + WireGuardDir string `yaml:"wireGuardDir"` + WireGuardPort *int `yaml:"wireGuardPort"` // Deprecated: EnablePolicyRouting enables connmark/fwmark/ip-rule policy // routing on gateway interfaces. Replaced by the UNBOUNDED-FORWARD chain. // Defaults to false; retained for backward compatibility. diff --git a/internal/net/controller/kubeproxy_controller.go b/internal/net/controller/kubeproxy_controller.go index b492aa0c7e..759e222df1 100644 --- a/internal/net/controller/kubeproxy_controller.go +++ b/internal/net/controller/kubeproxy_controller.go @@ -459,7 +459,7 @@ func siteKubeProxyClusterCIDR(site unboundedv1alpha3.Site) (string, bool) { var ipv4, ipv6 string for _, assignment := range site.Spec.PodCidrAssignments { - if !assignmentEnabled(assignment.AssignmentEnabled) { + if assignment.AssignmentEnabled != nil && !*assignment.AssignmentEnabled { continue } diff --git a/internal/net/controller/kubeproxy_controller_test.go b/internal/net/controller/kubeproxy_controller_test.go index 681e051818..6e53457db2 100644 --- a/internal/net/controller/kubeproxy_controller_test.go +++ b/internal/net/controller/kubeproxy_controller_test.go @@ -116,6 +116,46 @@ func TestSiteKubeProxyClusterCIDR(t *testing.T) { if !ok || got != "100.125.0.0/16,fd00:1::/64" { t.Fatalf("siteKubeProxyClusterCIDR() = %q,%v", got, ok) } + + site.Spec.ManageCniPlugin = new(false) + if got, ok := siteKubeProxyClusterCIDR(site); !ok || got != "100.125.0.0/16,fd00:1::/64" { + t.Fatalf("unmanaged omitted assignment lost cluster CIDRs: %q, %v", got, ok) + } + + site.Spec.PodCidrAssignments[1].AssignmentEnabled = new(true) + if got, ok := siteKubeProxyClusterCIDR(site); !ok || got != "100.125.0.0/16,fd00:1::/64" { + t.Fatalf("explicit unmanaged assignment lost cluster CIDRs: %q, %v", got, ok) + } +} + +func TestEnsureDaemonSetForUnmanagedSite(t *testing.T) { + site := unboundedv1alpha3.Site{ + ObjectMeta: metav1.ObjectMeta{Name: "external"}, + Spec: unboundedv1alpha3.SiteSpec{ + ManageCniPlugin: new(false), + PodCidrAssignments: []unboundednetv1alpha1.PodCidrAssignment{{ + CidrBlocks: []string{"10.244.0.0/16", "fd00:244::/48"}, + }}, + }, + } + clientset := k8sfake.NewClientset() + + c := &ManagedKubeProxyController{ + clientset: clientset, + options: ManagedKubeProxyOptions{Namespace: "unbounded-net", Image: "kube-proxy:v1"}, + } + if err := c.ensureDaemonSet(t.Context(), site); err != nil { + t.Fatal(err) + } + + ds, err := clientset.AppsV1().DaemonSets("unbounded-net").Get(t.Context(), managedKubeProxyDaemonSetName(site.Name), metav1.GetOptions{}) + if err != nil { + t.Fatalf("unmanaged Site has no kube-proxy DaemonSet: %v", err) + } + + if got := ds.Spec.Template.Spec.Containers[0].Command[3]; got != "--cluster-cidr=10.244.0.0/16,fd00:244::/48" { + t.Fatalf("unexpected cluster-cidr arg: %s", got) + } } func TestDaemonSetForSite(t *testing.T) { diff --git a/internal/net/controller/site_controller.go b/internal/net/controller/site_controller.go index 4106bac1e9..dc8e5e8d68 100644 --- a/internal/net/controller/site_controller.go +++ b/internal/net/controller/site_controller.go @@ -586,14 +586,6 @@ func assignmentKey(siteName string, assignmentIndex int) string { return fmt.Sprintf("%s/%d", siteName, assignmentIndex) } -func assignmentEnabled(enabled *bool) bool { - if enabled == nil { - return true - } - - return *enabled -} - func assignmentPriority(priority *int32) int32 { if priority == nil { return 100 @@ -615,7 +607,7 @@ func (sc *SiteController) collectEnabledAssignments(sites []unboundedv1alpha3.Si for _, site := range sites { for i, assignment := range site.Spec.PodCidrAssignments { - if !assignmentEnabled(assignment.AssignmentEnabled) { + if !site.Spec.PodCIDRAssignmentEnabled(assignment) { continue } @@ -1987,13 +1979,6 @@ func (sc *SiteController) assignPodCIDRsForNode(ctx context.Context, node *corev return nil } - // When manageCniPlugin is false, pod CIDR assignment is disabled. - // The CIDRs are still used for inter-site routing but nodes are not - // assigned individual pod CIDRs by the controller. - if site.Spec.ManageCniPlugin != nil && !*site.Spec.ManageCniPlugin { - return nil - } - state := sc.selectAssignmentForNode(*site, node.Name) if state == nil { return nil @@ -2569,6 +2554,15 @@ func (sc *SiteController) assignPodCIDRsForNodeWithLabel(ctx context.Context, no state := sc.selectAssignmentForNode(*site, node.Name) if state == nil { + patchData, err := siteLabelAddMergePatch(siteName) + if err != nil { + return fmt.Errorf("failed to marshal site label patch: %w", err) + } + + if _, err := sc.clientset.CoreV1().Nodes().Patch(ctx, node.Name, types.MergePatchType, patchData, metav1.PatchOptions{}); err != nil { + return fmt.Errorf("failed to label node without pod CIDR assignment: %w", err) + } + return nil } diff --git a/internal/net/controller/site_controller_helpers_test.go b/internal/net/controller/site_controller_helpers_test.go index 1bb15415e1..7fd82df659 100644 --- a/internal/net/controller/site_controller_helpers_test.go +++ b/internal/net/controller/site_controller_helpers_test.go @@ -40,7 +40,9 @@ func TestAssignmentPrimitiveHelpers(t *testing.T) { t.Fatalf("unexpected assignment key: %s", got) } - if !assignmentEnabled(nil) || assignmentEnabled(ptrBool(false)) { + spec := unboundedv1alpha3.SiteSpec{} + if !spec.PodCIDRAssignmentEnabled(unboundednetv1alpha1.PodCidrAssignment{}) || + spec.PodCIDRAssignmentEnabled(unboundednetv1alpha1.PodCidrAssignment{AssignmentEnabled: new(false)}) { t.Fatalf("assignmentEnabled nil/false behavior mismatch") } diff --git a/internal/net/controller/site_controller_podcidr_test.go b/internal/net/controller/site_controller_podcidr_test.go index 1c4400a81a..5a4b86702e 100644 --- a/internal/net/controller/site_controller_podcidr_test.go +++ b/internal/net/controller/site_controller_podcidr_test.go @@ -30,6 +30,92 @@ import ( const podCIDRTestNode = "node-a" +func TestSyncNodeAssignmentDefaultsAndOverrides(t *testing.T) { + settings := []struct { + name string + value *bool + }{ + {"omitted", nil}, + {"true", new(true)}, + {"false", new(false)}, + } + for _, manage := range settings { + for _, assignment := range settings { + for _, needsLabel := range []bool{false, true} { + t.Run(fmt.Sprintf("manage=%s/assignment=%s/needs-label=%v", manage.name, assignment.name, needsLabel), func(t *testing.T) { + h := newPodCIDRTestHarness(t, liveNodeWithRV("4")) + h.sites[0].Spec.NodeCidrs = []string{"10.0.0.0/16"} + h.sites[0].Spec.ManageCniPlugin = manage.value + h.sites[0].Spec.PodCidrAssignments[0].AssignmentEnabled = assignment.value + h.sc.sitesCache = h.sites + h.sc.updateAssignmentAllocators(h.sites) + + h.cached.Status.Addresses = []corev1.NodeAddress{{Type: corev1.NodeInternalIP, Address: "10.0.0.5"}} + if !needsLabel { + h.cached.Labels = map[string]string{} + for _, key := range siteLabelKeys() { + h.cached.Labels[key] = "site-a" + } + } + + if err := h.sc.syncNode(t.Context(), podCIDRTestNode); err != nil { + t.Fatal(err) + } + + node, err := h.client.CoreV1().Nodes().Get(t.Context(), podCIDRTestNode, metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + + want := assignment.name == "true" || (assignment.name == "omitted" && manage.name != "false") + if want { + if len(node.Spec.PodCIDRs) != 1 || node.Spec.PodCIDRs[0] != "10.244.0.0/24" || len(h.patches) != 1 { + t.Fatalf("enabled assignment did not patch PodCIDRs: node=%+v patches=%d", node.Spec, len(h.patches)) + } + } else { + wantPatches := 0 + if needsLabel { + wantPatches = 1 + } + + if len(node.Spec.PodCIDRs) != 0 || len(h.patches) != wantPatches || h.sc.getAssignmentAllocator("site-a", 0) != nil || h.pending(podCIDRTestNode) != nil { + t.Fatalf("disabled assignment allocated or skipped labeling: node=%+v patches=%d", node.Spec, len(h.patches)) + } + } + + if needsLabel && !nodeSiteLabelsCurrent(node, "site-a") { + t.Fatal("assignment setting prevented site labeling") + } + }) + } + } + } +} + +func TestDisabledAssignmentReportsSiteLabelFailure(t *testing.T) { + h := newPodCIDRTestHarness(t, liveNodeWithRV("4")) + h.sites[0].Spec.ManageCniPlugin = new(false) + h.sc.sitesCache = h.sites + h.sc.updateAssignmentAllocators(h.sites) + + expected := errors.New("site label patch failed") + h.failPatches(expected) + + err := h.sc.assignPodCIDRsForNodeWithLabel(t.Context(), h.cached, h.sites, "site-a") + if !errors.Is(err, expected) || len(h.patches) != 1 { + t.Fatalf("site label failure not propagated: err=%v patches=%d", err, len(h.patches)) + } + + node, err := h.client.CoreV1().Nodes().Get(t.Context(), podCIDRTestNode, metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + + if len(node.Spec.PodCIDRs) != 0 || len(node.Labels) != 0 || h.pending(podCIDRTestNode) != nil { + t.Fatal("failed label patch changed the Node or reserved PodCIDRs") + } +} + func TestAssignmentRejectsStaleSiteSnapshot(t *testing.T) { for _, withLabel := range []bool{false, true} { for _, change := range []string{"node-cidrs", "regex", "priority"} { diff --git a/internal/net/netlink/route_manager.go b/internal/net/netlink/route_manager.go index 6ea5a3eaac..9bf82baed4 100644 --- a/internal/net/netlink/route_manager.go +++ b/internal/net/netlink/route_manager.go @@ -66,7 +66,8 @@ type installedRouteState struct { mtu int table int linkScope bool - hasEncap bool // true when route has lightweight tunnel encap + hasEncap bool // true when route has lightweight tunnel encap + preferredSrc net.IP peerNexthops map[string]DesiredNexthop // peerID -> nexthop info } @@ -776,6 +777,7 @@ func (m *UnifiedRouteManager) buildInstalledState(dr DesiredRoute, active []Desi table: dr.Table, linkScope: isLinkScopeRoute(active), hasEncap: dr.Encap != nil, + preferredSrc: slices.Clone(m.preferredSrc(dr.Prefix)), peerNexthops: peerNH, } } @@ -783,6 +785,10 @@ func (m *UnifiedRouteManager) buildInstalledState(dr DesiredRoute, active []Desi // routeNeedsUpdate returns true when the installed state differs from the // desired state plus the set of currently-active nexthops. func (m *UnifiedRouteManager) routeNeedsUpdate(installed *installedRouteState, desired DesiredRoute, activeNH []DesiredNexthop) bool { + if !ipEqual(installed.preferredSrc, m.preferredSrc(desired.Prefix)) { + return true + } + if installed.metric != desired.Metric || installed.mtu != desired.MTU { return true } diff --git a/internal/net/netlink/route_manager_test.go b/internal/net/netlink/route_manager_test.go index f3a9bb9dd1..e25c3fbd8a 100644 --- a/internal/net/netlink/route_manager_test.go +++ b/internal/net/netlink/route_manager_test.go @@ -551,6 +551,61 @@ func TestRouteNeedsUpdate_GatewayChanged(t *testing.T) { } } +func TestRouteNeedsUpdate_PreferredSourceChanged(t *testing.T) { + for _, test := range []struct { + name string + prefix string + source string + next string + }{ + {"IPv4", "10.245.0.0/24", "10.244.3.1", "10.244.4.1"}, + {"IPv6", "fd00:245::/64", "fd00:3::1", "fd00:4::1"}, + } { + t.Run(test.name, func(t *testing.T) { + manager := NewUnifiedRouteManager("test", 0, "wg", "unbounded0") + + _, prefix, err := net.ParseCIDR(test.prefix) + if err != nil { + t.Fatal(err) + } + + desired := DesiredRoute{ + Prefix: *prefix, + Nexthops: []DesiredNexthop{{ + PeerID: "peer", LinkIndex: 1, + }}, + } + + installed := manager.buildInstalledState(desired, desired.Nexthops) + if manager.routeNeedsUpdate(installed, desired, desired.Nexthops) { + t.Fatal("initial source-free route unexpectedly needs replacement") + } + + for _, source := range []net.IP{net.ParseIP(test.source), net.ParseIP(test.next), nil} { + if test.name == "IPv4" { + manager.SetPreferredSourceIPs(source, nil) + } else { + manager.SetPreferredSourceIPs(nil, source) + } + + if !manager.routeNeedsUpdate(installed, desired, desired.Nexthops) { + t.Fatalf("source change to %v did not require route replacement", source) + } + + route := manager.buildKernelRoute(desired, desired.Nexthops) + if route == nil || !ipEqual(route.Src, source) || route.Dst.String() != test.prefix || route.LinkIndex != 1 { + t.Fatalf("replacement route lost source, destination, or nexthop: %v", route) + } + + installed = manager.buildInstalledState(desired, desired.Nexthops) + if manager.routeNeedsUpdate(installed, desired, desired.Nexthops) { + t.Fatal("unchanged preferred source unnecessarily replaced the route") + } + } + }) + } +} + func TestSetPreferredSourceIPs(t *testing.T) { m := NewUnifiedRouteManager("test", 0, "wg", "unbounded0") diff --git a/internal/net/webhook/validation.go b/internal/net/webhook/validation.go index 605634c732..bb65a480df 100644 --- a/internal/net/webhook/validation.go +++ b/internal/net/webhook/validation.go @@ -390,7 +390,7 @@ func validateSiteSpec(site unboundedv1alpha3.Site) error { ipv6Mask := 0 for i, assignment := range site.Spec.PodCidrAssignments { - if !assignmentEnabled(assignment.AssignmentEnabled) { + if !site.Spec.PodCIDRAssignmentEnabled(assignment) { continue } @@ -608,7 +608,7 @@ func collectPodCIDRs(sites []unboundedv1alpha3.Site) ([]cidrEntry, error) { for _, site := range sites { for i, assignment := range site.Spec.PodCidrAssignments { - if !assignmentEnabled(assignment.AssignmentEnabled) { + if assignment.AssignmentEnabled != nil && !*assignment.AssignmentEnabled { continue } @@ -663,14 +663,6 @@ func ipFamily(cidr *net.IPNet) string { return "ipv6" } -func assignmentEnabled(enabled *bool) bool { - if enabled == nil { - return true - } - - return *enabled -} - func decodeSiteFromRequest(req *admissionv1.AdmissionRequest) (unboundedv1alpha3.Site, error) { var ( site unboundedv1alpha3.Site diff --git a/internal/net/webhook/validation_test.go b/internal/net/webhook/validation_test.go index 814146fe45..0ef43d2152 100644 --- a/internal/net/webhook/validation_test.go +++ b/internal/net/webhook/validation_test.go @@ -365,15 +365,16 @@ func TestDecodeSiteFromRequest(t *testing.T) { // TestAssignmentEnabledAndIPFamily tests assignment enabled and ipfamily. func TestAssignmentEnabledAndIPFamily(t *testing.T) { - if !assignmentEnabled(nil) { + spec := unboundedv1alpha3.SiteSpec{} + if !spec.PodCIDRAssignmentEnabled(unboundednetv1alpha1.PodCidrAssignment{}) { t.Fatalf("expected nil assignmentEnabled to default true") } - if assignmentEnabled(boolPtr(false)) { + if spec.PodCIDRAssignmentEnabled(unboundednetv1alpha1.PodCidrAssignment{AssignmentEnabled: new(false)}) { t.Fatalf("expected explicit false assignmentEnabled") } - if !assignmentEnabled(boolPtr(true)) { + if !spec.PodCIDRAssignmentEnabled(unboundednetv1alpha1.PodCidrAssignment{AssignmentEnabled: new(true)}) { t.Fatalf("expected explicit true assignmentEnabled") } @@ -385,6 +386,80 @@ func TestAssignmentEnabledAndIPFamily(t *testing.T) { } } +func TestUnmanagedAssignmentValidation(t *testing.T) { + site := unboundedv1alpha3.Site{ + ObjectMeta: metav1.ObjectMeta{Name: "site-a"}, + Spec: unboundedv1alpha3.SiteSpec{ + ManageCniPlugin: new(false), + NodeCidrs: []string{"10.0.0.0/16"}, + PodCidrAssignments: []unboundednetv1alpha1.PodCidrAssignment{{ + CidrBlocks: []string{"10.244.0.0/16"}, + NodeRegex: []string{"("}, + }}, + }, + } + if err := validateSiteSpec(site); err != nil { + t.Fatalf("inherited disabled assignment validated allocation settings: %v", err) + } + + entries, err := collectPodCIDRs([]unboundedv1alpha3.Site{site}) + if err != nil || len(entries) != 1 { + t.Fatalf("unmanaged Site's declared pod networks missing from overlap checks: %v, %v", entries, err) + } + + site.Spec.PodCidrAssignments[0].AssignmentEnabled = new(true) + if err := validateSiteSpec(site); err == nil || !strings.Contains(err.Error(), "nodeRegex") { + t.Fatalf("explicit enabled assignment skipped regex validation: %v", err) + } + + entries, err = collectPodCIDRs([]unboundedv1alpha3.Site{site}) + if err != nil || len(entries) != 1 { + t.Fatalf("explicit enabled assignment missing from overlap checks: %v, %v", entries, err) + } + + site.Spec.PodCidrAssignments[0].AssignmentEnabled = new(false) + if err := validateSiteSpec(site); err != nil { + t.Fatalf("explicit disabled assignment validated allocation settings: %v", err) + } +} + +func TestManagedAndUnmanagedPodCIDRsCannotOverlap(t *testing.T) { + for _, cidrs := range [][]string{ + {"10.244.0.0/16", "10.244.1.0/24", "10.245.0.0/16"}, + {"fd00:244::/48", "fd00:244:0:1::/64", "fd00:245::/48"}, + } { + t.Run(cidrs[0], func(t *testing.T) { + sites := []unboundedv1alpha3.Site{ + { + ObjectMeta: metav1.ObjectMeta{Name: "external"}, + Spec: unboundedv1alpha3.SiteSpec{ + ManageCniPlugin: new(false), + PodCidrAssignments: []unboundednetv1alpha1.PodCidrAssignment{{ + CidrBlocks: []string{cidrs[0]}, + }}, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "managed"}, + Spec: unboundedv1alpha3.SiteSpec{ + PodCidrAssignments: []unboundednetv1alpha1.PodCidrAssignment{{ + CidrBlocks: []string{cidrs[1]}, + }}, + }, + }, + } + if err := validatePodCIDRsNoOverlap(sites); err == nil || !strings.Contains(err.Error(), "overlapping podCIDR") { + t.Fatalf("managed/unmanaged overlap was not rejected: %v", err) + } + + sites[1].Spec.PodCidrAssignments[0].CidrBlocks = []string{cidrs[2]} + if err := validatePodCIDRsNoOverlap(sites); err != nil { + t.Fatalf("disjoint managed/unmanaged networks rejected: %v", err) + } + }) + } +} + // TestDecodeGatewayPoolAndPeeringRequests tests decode gateway pool and peering requests. func TestDecodeGatewayPoolAndPeeringRequests(t *testing.T) { pool := unboundednetv1alpha1.GatewayPool{ObjectMeta: metav1.ObjectMeta{Name: "pool-a"}} diff --git a/internal/racerobject/azure_test.go b/internal/racerobject/azure_test.go index 28417baf98..604cc7b36e 100644 --- a/internal/racerobject/azure_test.go +++ b/internal/racerobject/azure_test.go @@ -659,6 +659,7 @@ func TestAzureOriginCancellation(t *testing.T) { for _, method := range []string{http.MethodHead, http.MethodGet} { t.Run(method, func(t *testing.T) { entered, canceled := make(chan struct{}), make(chan struct{}) + bodyReady := make(chan struct{}) var closes atomic.Int32 @@ -683,6 +684,8 @@ func TestAzureOriginCancellation(t *testing.T) { response, err := client.Do(r) if err == nil && r.Method == http.MethodGet { response.Body = &azureTestBody{ReadCloser: response.Body, closes: &closes} + + close(bodyReady) } return response, err @@ -719,6 +722,14 @@ func TestAzureOriginCancellation(t *testing.T) { t.Fatal("request did not start") } + if method == http.MethodGet { + select { + case <-bodyReady: + case <-time.After(5 * time.Second): + t.Fatal("response body was not wrapped") + } + } + cancel() select {