From d63887ed167da260d3f26c71ec06e520d89a4b0f Mon Sep 17 00:00:00 2001 From: Dumitru Ceara Date: Fri, 6 Sep 2024 11:32:52 +0200 Subject: [PATCH] node: udn: Ensure UDN traffic doesn't leak into default network services. In local gateway mode all UDN traffic that needs to exit the node was redirected to the host (via the mp-X port). There however it was injected into the br-ex/breth0 bridge where it got sent into the default network patch port (because the destination IP is that of a service CIDR). That traffic eventually reached the default network endpoint and the connection was successfully set up. The mode above is not the correct mode of operation for UDN. UDN pods should not be allowed to access any default network services (except select ones like kapi). To achieve that we change the br-ex/breth0 flows and add new per-UDN flows that detect traffic originated by a given network and only direct it back to that network's patch port. This commit updates the e2e test to cover this scenario. This change also indirectly fixes the fact that node port default services should not be accessible from UDN when the target IP is the local node IP (that traffic is now sent back to the UDN GR where no LB is configured for the default service). New flow specific unit tests are also added to make sure these flows are not accidentally removed in the future. Signed-off-by: Dumitru Ceara --- go-controller/pkg/node/gateway_shared_intf.go | 73 +++++++++-- go-controller/pkg/node/gateway_udn.go | 35 +++-- go-controller/pkg/node/gateway_udn_test.go | 121 ++++++++++++++++-- go-controller/pkg/node/openflow_manager.go | 7 +- test/e2e/network_segmentation_services.go | 10 +- 5 files changed, 195 insertions(+), 51 deletions(-) diff --git a/go-controller/pkg/node/gateway_shared_intf.go b/go-controller/pkg/node/gateway_shared_intf.go index 0676cd8e42a..1edb2641fae 100644 --- a/go-controller/pkg/node/gateway_shared_intf.go +++ b/go-controller/pkg/node/gateway_shared_intf.go @@ -279,9 +279,9 @@ func (npw *nodePortWatcher) updateServiceFlowCache(service *kapi.Service, netInf // Add flows for default network services that are accessible from UDN networks if util.IsNetworkSegmentationSupportEnabled() { - // The flow added below has a higher priority than the default network service flow: - // priority=500,ip,in_port=LOCAL,nw_dst=10.96.0.0/16 actions=ct(commit,table=2,zone=64001,nat(src=169.254.0.2)) - // This ordering ensures that there is no SNAT for UDN originated traffic. + // The flow added below has a higher priority than the per UDN service flow: + // priority=200, table=2, ip, ip_src=169.254.0., actions=set_field:->eth_dst,output: + // This ordering ensures that traffic to UDN allowed default services goes to the the default patch port. if util.IsUDNEnabledService(ktypes.NamespacedName{Namespace: service.Namespace, Name: service.Name}.String()) { key = strings.Join([]string{"UDNAllowedSVC", service.Namespace, service.Name}, "_") @@ -295,10 +295,13 @@ func (npw *nodePortWatcher) updateServiceFlowCache(service *kapi.Service, netInf ipPrefix = "ipv6" masqueradeSubnet = config.Gateway.V6MasqueradeSubnet } - // table 0, user-defined network host -> OVN towards default cluster network services - npw.ofm.updateFlowCacheEntry(key, []string{fmt.Sprintf("cookie=%s, priority=600, in_port=%s, %s, %s_src=%s, %s_dst=%s,"+ - "actions=ct(commit,zone=%d,table=2)", - defaultOpenFlowCookie, npw.ofm.defaultBridge.ofPortHost, ipPrefix, ipPrefix, masqueradeSubnet, ipPrefix, service.Spec.ClusterIP, config.Default.HostMasqConntrackZone)}) + // table 2, user-defined network host -> OVN towards default cluster network services + defaultNetConfig := npw.ofm.defaultBridge.netConfig[types.DefaultNetworkName] + + npw.ofm.updateFlowCacheEntry(key, []string{fmt.Sprintf("cookie=%s, priority=300, table=2, %s, %s_src=%s, %s_dst=%s, "+ + "actions=set_field:%s->eth_dst,output:%s", + defaultOpenFlowCookie, ipPrefix, ipPrefix, masqueradeSubnet, ipPrefix, service.Spec.ClusterIP, + npw.ofm.getDefaultBridgeMAC().String(), defaultNetConfig.ofPortPatch)}) } } return utilerrors.Join(errors...) @@ -1316,11 +1319,23 @@ func flowsForDefaultBridge(bridge *bridgeConfiguration, extraIPs []net.IP) ([]st masqSubnet = config.Gateway.V6MasqueradeSubnet } - // table 0, Host -> OVN towards SVC, SNAT to special IP + // table 0, Host (default network) -> OVN towards SVC, SNAT to special IP. dftFlows = append(dftFlows, - fmt.Sprintf("cookie=%s, priority=500, in_port=%s, %s, %s_dst=%s,"+ + fmt.Sprintf("cookie=%s, priority=500, in_port=%s, %s, %s_dst=%s, "+ "actions=ct(commit,zone=%d,nat(src=%s),table=2)", - defaultOpenFlowCookie, ofPortHost, protoPrefix, protoPrefix, svcCIDR, config.Default.HostMasqConntrackZone, masqIP)) + defaultOpenFlowCookie, ofPortHost, protoPrefix, protoPrefix, + svcCIDR, config.Default.HostMasqConntrackZone, masqIP)) + + if util.IsNetworkSegmentationSupportEnabled() { + // table 0, Host (UDNs) -> OVN towards SVC, SNAT to special IP. + // For packets originating from UDN, commit without NATing, those + // have already been SNATed to the masq IP of the UDN. + dftFlows = append(dftFlows, + fmt.Sprintf("cookie=%s, priority=550, in_port=%s, %s, %s_src=%s, %s_dst=%s, "+ + "actions=ct(commit,zone=%d,table=2)", + defaultOpenFlowCookie, ofPortHost, protoPrefix, protoPrefix, + masqSubnet, protoPrefix, svcCIDR, config.Default.HostMasqConntrackZone)) + } masqDst := masqIP if util.IsNetworkSegmentationSupportEnabled() { @@ -1420,8 +1435,38 @@ func flowsForDefaultBridge(bridge *bridgeConfiguration, extraIPs []net.IP) ([]st // table 2, dispatch from Host -> OVN dftFlows = append(dftFlows, - fmt.Sprintf("cookie=%s, table=2, "+ - "actions=set_field:%s->eth_dst,output:%s", defaultOpenFlowCookie, bridgeMacAddress, defaultNetConfig.ofPortPatch)) + fmt.Sprintf("cookie=%s, priority=100, table=2, "+ + "actions=set_field:%s->eth_dst,output:%s", defaultOpenFlowCookie, + bridgeMacAddress, defaultNetConfig.ofPortPatch)) + + // table 2, priority 200, dispatch from UDN -> Host -> OVN. These packets have + // already been SNATed to the UDN's masq IP. + if config.IPv4Mode { + for _, netConfig := range bridge.patchedNetConfigs() { + if netConfig.masqCTMark == ctMarkOVN { + continue + } + dftFlows = append(dftFlows, + fmt.Sprintf("cookie=%s, priority=200, table=2, ip, ip_src=%s, "+ + "actions=set_field:%s->eth_dst,output:%s", + defaultOpenFlowCookie, netConfig.v4MasqIPs.ManagementPort.IP, + bridgeMacAddress, netConfig.ofPortPatch)) + } + } + + if config.IPv6Mode { + for _, netConfig := range bridge.patchedNetConfigs() { + if netConfig.masqCTMark == ctMarkOVN { + continue + } + + dftFlows = append(dftFlows, + fmt.Sprintf("cookie=%s, priority=200, table=2, ip6, ipv6_src=%s, "+ + "actions=set_field:%s->eth_dst,output:%s", + defaultOpenFlowCookie, netConfig.v6MasqIPs.ManagementPort.IP, + bridgeMacAddress, netConfig.ofPortPatch)) + } + } // table 3, dispatch from OVN -> Host dftFlows = append(dftFlows, @@ -1522,7 +1567,7 @@ func commonFlows(subnets []*net.IPNet, bridge *bridgeConfiguration) ([]string, e dftFlows = append(dftFlows, fmt.Sprintf("cookie=%s, priority=100, in_port=%s, dl_src=%s, ip, ip_src=%s, "+ "actions=ct(commit, zone=%d, nat(src=%s), exec(set_field:%s->ct_mark)), output:%s", - defaultOpenFlowCookie, netConfig.ofPortPatch, bridgeMacAddress, netConfig.v4MasqIP.IP, config.Default.ConntrackZone, + defaultOpenFlowCookie, netConfig.ofPortPatch, bridgeMacAddress, netConfig.v4MasqIPs.GatewayRouter.IP, config.Default.ConntrackZone, physicalIP.IP, netConfig.masqCTMark, ofPortPhys)) } } @@ -1597,7 +1642,7 @@ func commonFlows(subnets []*net.IPNet, bridge *bridgeConfiguration) ([]string, e dftFlows = append(dftFlows, fmt.Sprintf("cookie=%s, priority=100, in_port=%s, dl_src=%s, ipv6, ipv6_src=%s, "+ "actions=ct(commit, zone=%d, nat(src=%s), exec(set_field:%s->ct_mark)), output:%s", - defaultOpenFlowCookie, netConfig.ofPortPatch, bridgeMacAddress, netConfig.v6MasqIP.IP, config.Default.ConntrackZone, + defaultOpenFlowCookie, netConfig.ofPortPatch, bridgeMacAddress, netConfig.v6MasqIPs.GatewayRouter.IP, config.Default.ConntrackZone, physicalIP.IP, netConfig.masqCTMark, ofPortPhys)) } } diff --git a/go-controller/pkg/node/gateway_udn.go b/go-controller/pkg/node/gateway_udn.go index dafd22fc390..04160427319 100644 --- a/go-controller/pkg/node/gateway_udn.go +++ b/go-controller/pkg/node/gateway_udn.go @@ -52,10 +52,10 @@ type UserDefinedNetworkGateway struct { // masqCTMark holds the mark value for this network // which is used for egress traffic in shared gateway mode masqCTMark uint - // v4MasqIP holds the IPv4 masquerade IP for this network - v4MasqIP *net.IPNet - // v6MasqIP holds the IPv6 masquerade IP for this network - v6MasqIP *net.IPNet + // v4MasqIPs holds the IPv4 masquerade IPs for this network + v4MasqIPs *udn.MasqueradeIPs + // v6MasqIPs holds the IPv6 masquerade IPs for this network + v6MasqIPs *udn.MasqueradeIPs // stores the pointer to default network's gateway so that // we can leverage it from here to program UDN flows on breth0 // Currently we use the openflowmanager and nodeIPManager from @@ -83,7 +83,7 @@ func (b *bridgeConfiguration) getBridgePortConfigurations() ([]bridgeUDNConfigur } // addNetworkBridgeConfig adds the patchport and ctMark value for the provided netInfo into the bridge configuration cache -func (b *bridgeConfiguration) addNetworkBridgeConfig(nInfo util.NetInfo, masqCTMark uint, v4MasqIP, v6MasqIP *net.IPNet) { +func (b *bridgeConfiguration) addNetworkBridgeConfig(nInfo util.NetInfo, masqCTMark uint, v4MasqIPs, v6MasqIPs *udn.MasqueradeIPs) { b.Lock() defer b.Unlock() @@ -95,8 +95,8 @@ func (b *bridgeConfiguration) addNetworkBridgeConfig(nInfo util.NetInfo, masqCTM netConfig := &bridgeUDNConfiguration{ patchPort: patchPort, masqCTMark: fmt.Sprintf("0x%x", masqCTMark), - v4MasqIP: v4MasqIP, - v6MasqIP: v6MasqIP, + v4MasqIPs: v4MasqIPs, + v6MasqIPs: v6MasqIPs, } b.netConfig[netName] = netConfig @@ -149,8 +149,8 @@ type bridgeUDNConfiguration struct { patchPort string ofPortPatch string masqCTMark string - v4MasqIP *net.IPNet - v6MasqIP *net.IPNet + v4MasqIPs *udn.MasqueradeIPs + v6MasqIPs *udn.MasqueradeIPs } func (netConfig *bridgeUDNConfiguration) setBridgeNetworkOfPortsInternal() error { @@ -179,23 +179,22 @@ func NewUserDefinedNetworkGateway(netInfo util.NetInfo, networkID int, node *v1. defaultNetworkGateway Gateway) (*UserDefinedNetworkGateway, error) { // Generate a per network conntrack mark and masquerade IPs to be used for egress traffic. var ( - v4MasqIP *net.IPNet - v6MasqIP *net.IPNet + v4MasqIPs *udn.MasqueradeIPs + v6MasqIPs *udn.MasqueradeIPs + err error ) masqCTMark := ctMarkUDNBase + uint(networkID) if config.IPv4Mode { - v4MasqIPs, err := udn.AllocateV4MasqueradeIPs(networkID) + v4MasqIPs, err = udn.AllocateV4MasqueradeIPs(networkID) if err != nil { return nil, fmt.Errorf("failed to get v4 masquerade IP, network %s (%d): %v", netInfo.GetNetworkName(), networkID, err) } - v4MasqIP = v4MasqIPs.GatewayRouter } if config.IPv6Mode { - v6MasqIPs, err := udn.AllocateV6MasqueradeIPs(networkID) + v6MasqIPs, err = udn.AllocateV6MasqueradeIPs(networkID) if err != nil { return nil, fmt.Errorf("failed to get v6 masquerade IP, network %s (%d): %v", netInfo.GetNetworkName(), networkID, err) } - v6MasqIP = v6MasqIPs.GatewayRouter } gw, ok := defaultNetworkGateway.(*gateway) @@ -211,8 +210,8 @@ func NewUserDefinedNetworkGateway(netInfo util.NetInfo, networkID int, node *v1. kubeInterface: kubeInterface, vrfManager: vrfManager, masqCTMark: masqCTMark, - v4MasqIP: v4MasqIP, - v6MasqIP: v6MasqIP, + v4MasqIPs: v4MasqIPs, + v6MasqIPs: v6MasqIPs, gateway: gw, ruleManager: ruleManager, }, nil @@ -258,7 +257,7 @@ func (udng *UserDefinedNetworkGateway) AddNetwork() error { return fmt.Errorf("could not set loose mode for reverse path filtering on management port %s: %v", mgmtPortName, err) } if udng.openflowManager != nil { - udng.openflowManager.addNetwork(udng.NetInfo, udng.masqCTMark, udng.v4MasqIP, udng.v6MasqIP) + udng.openflowManager.addNetwork(udng.NetInfo, udng.masqCTMark, udng.v4MasqIPs, udng.v6MasqIPs) waiter := newStartupWaiterWithTimeout(waitForPatchPortTimeout) readyFunc := func() (bool, error) { diff --git a/go-controller/pkg/node/gateway_udn_test.go b/go-controller/pkg/node/gateway_udn_test.go index cc14e2aa2d4..2fc80c0a41a 100644 --- a/go-controller/pkg/node/gateway_udn_test.go +++ b/go-controller/pkg/node/gateway_udn_test.go @@ -18,6 +18,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/fake" + utilnet "k8s.io/utils/net" "k8s.io/utils/ptr" nadfake "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/client/clientset/versioned/fake" @@ -180,6 +181,69 @@ func setUpUDNOpenflowManagerFakeOVSCommands(fexec *ovntest.FakeExec) { }) } +func checkDefaultSvcIsolationOVSFlows(flows []string, defaultConfig *bridgeUDNConfiguration, ofPortHost, bridgeMAC string, svcCIDR *net.IPNet) { + By(fmt.Sprintf("Checking default service isolation flows for %s", svcCIDR.String())) + + var masqIP string + var masqSubnet string + var protoPrefix string + if utilnet.IsIPv4CIDR(svcCIDR) { + protoPrefix = "ip" + masqIP = config.Gateway.MasqueradeIPs.V4HostMasqueradeIP.String() + masqSubnet = config.Gateway.V4MasqueradeSubnet + } else { + protoPrefix = "ip6" + masqIP = config.Gateway.MasqueradeIPs.V6HostMasqueradeIP.String() + masqSubnet = config.Gateway.V6MasqueradeSubnet + } + + var nTable0DefaultFlows int + var nTable0UDNMasqFlows int + var nTable2Flows int + for _, flow := range flows { + if strings.Contains(flow, fmt.Sprintf("priority=500, in_port=%s, %s, %s_dst=%s, actions=ct(commit,zone=%d,nat(src=%s),table=2)", + ofPortHost, protoPrefix, protoPrefix, svcCIDR, config.Default.HostMasqConntrackZone, + masqIP)) { + nTable0DefaultFlows++ + } else if strings.Contains(flow, fmt.Sprintf("priority=550, in_port=%s, %s, %s_src=%s, %s_dst=%s, actions=ct(commit,zone=%d,table=2)", + ofPortHost, protoPrefix, protoPrefix, masqSubnet, protoPrefix, svcCIDR, config.Default.HostMasqConntrackZone)) { + nTable0UDNMasqFlows++ + } else if strings.Contains(flow, fmt.Sprintf("priority=100, table=2, actions=set_field:%s->eth_dst,output:%s", + bridgeMAC, defaultConfig.ofPortPatch)) { + nTable2Flows++ + } + } + + Expect(nTable0DefaultFlows).To(Equal(1)) + Expect(nTable0UDNMasqFlows).To(Equal(1)) + Expect(nTable2Flows).To(Equal(1)) +} + +func checkUDNSvcIsolationOVSFlows(flows []string, netConfig *bridgeUDNConfiguration, netName, ofPortHost, bridgeMAC string, svcCIDR *net.IPNet, expectedNFlows int) { + By(fmt.Sprintf("Checking UDN %s service isolation flows for %s; expected %d flows", + netName, svcCIDR.String(), expectedNFlows)) + + var mgmtMasqIP string + var protoPrefix string + if utilnet.IsIPv4CIDR(svcCIDR) { + mgmtMasqIP = netConfig.v4MasqIPs.ManagementPort.IP.String() + protoPrefix = "ip" + } else { + mgmtMasqIP = netConfig.v6MasqIPs.ManagementPort.IP.String() + protoPrefix = "ip6" + } + + var nFlows int + for _, flow := range flows { + if strings.Contains(flow, fmt.Sprintf("priority=200, table=2, %s, %s_src=%s, actions=set_field:%s->eth_dst,output:%s", + protoPrefix, protoPrefix, mgmtMasqIP, bridgeMAC, netConfig.ofPortPatch)) { + nFlows++ + } + } + + Expect(nFlows).To(Equal(expectedNFlows)) +} + var _ = Describe("UserDefinedNetworkGateway", func() { var ( netName = "bluenet" @@ -533,7 +597,8 @@ var _ = Describe("UserDefinedNetworkGateway", func() { // FIXME: extract openflow manager func from the spawning of a go routine so it can be called directly below. localGw.openflowManager.syncFlows() flowMap := udnGateway.gateway.openflowManager.flowCache - Expect(len(flowMap["DEFAULT"])).To(Equal(45)) + + Expect(len(flowMap["DEFAULT"])).To(Equal(46)) Expect(udnGateway.masqCTMark).To(Equal(udnGateway.masqCTMark)) var udnFlows int for _, flows := range flowMap { @@ -550,25 +615,37 @@ var _ = Describe("UserDefinedNetworkGateway", func() { Expect(udnGateway.AddNetwork()).To(Succeed()) flowMap = udnGateway.gateway.openflowManager.flowCache - Expect(len(flowMap["DEFAULT"])).To(Equal(59)) // 14 UDN Flows are added by default + Expect(len(flowMap["DEFAULT"])).To(Equal(62)) // 16 UDN Flows are added by default Expect(len(udnGateway.openflowManager.defaultBridge.netConfig)).To(Equal(2)) // default network + UDN network + defaultUdnConfig := udnGateway.openflowManager.defaultBridge.netConfig["default"] + bridgeUdnConfig := udnGateway.openflowManager.defaultBridge.netConfig["bluenet"] + bridgeMAC := udnGateway.openflowManager.defaultBridge.macAddress.String() + ofPortHost := udnGateway.openflowManager.defaultBridge.ofPortHost for _, flows := range flowMap { for _, flow := range flows { if strings.Contains(flow, fmt.Sprintf("0x%x", udnGateway.masqCTMark)) { // UDN Flow udnFlows++ - } else if strings.Contains(flow, fmt.Sprintf("in_port=%s", udnGateway.openflowManager.defaultBridge.netConfig["bluenet"].ofPortPatch)) { + } else if strings.Contains(flow, fmt.Sprintf("in_port=%s", bridgeUdnConfig.ofPortPatch)) { udnFlows++ } } } Expect(udnFlows).To(Equal(14)) + for _, svcCIDR := range config.Kubernetes.ServiceCIDRs { + // Check flows for default network service CIDR. + checkDefaultSvcIsolationOVSFlows(flowMap["DEFAULT"], defaultUdnConfig, ofPortHost, bridgeMAC, svcCIDR) + + // Expect exactly one flow per UDN for table 2 for service isolation. + checkUDNSvcIsolationOVSFlows(flowMap["DEFAULT"], bridgeUdnConfig, "bluenet", ofPortHost, bridgeMAC, svcCIDR, 1) + } + cnode := node.DeepCopy() kubeMock.On("UpdateNodeStatus", cnode).Return(nil) // check if network key gets deleted from annotation Expect(udnGateway.DelNetwork()).To(Succeed()) flowMap = udnGateway.gateway.openflowManager.flowCache - Expect(len(flowMap["DEFAULT"])).To(Equal(45)) // only default network flows are present + Expect(len(flowMap["DEFAULT"])).To(Equal(46)) // only default network flows are present Expect(len(udnGateway.openflowManager.defaultBridge.netConfig)).To(Equal(1)) // default network only udnFlows = 0 for _, flows := range flowMap { @@ -580,6 +657,14 @@ var _ = Describe("UserDefinedNetworkGateway", func() { } } Expect(udnFlows).To(Equal(0)) + + for _, svcCIDR := range config.Kubernetes.ServiceCIDRs { + // Check flows for default network service CIDR. + checkDefaultSvcIsolationOVSFlows(flowMap["DEFAULT"], defaultUdnConfig, ofPortHost, bridgeMAC, svcCIDR) + + // Expect no more flows per UDN for table 2 for service isolation. + checkUDNSvcIsolationOVSFlows(flowMap["DEFAULT"], bridgeUdnConfig, "bluenet", ofPortHost, bridgeMAC, svcCIDR, 0) + } return nil }) Expect(err).NotTo(HaveOccurred()) @@ -709,7 +794,7 @@ var _ = Describe("UserDefinedNetworkGateway", func() { // FIXME: extract openflow manager func from the spawning of a go routine so it can be called directly below. localGw.openflowManager.syncFlows() flowMap := udnGateway.gateway.openflowManager.flowCache - Expect(len(flowMap["DEFAULT"])).To(Equal(45)) + Expect(len(flowMap["DEFAULT"])).To(Equal(46)) Expect(udnGateway.masqCTMark).To(Equal(udnGateway.masqCTMark)) var udnFlows int for _, flows := range flowMap { @@ -726,25 +811,37 @@ var _ = Describe("UserDefinedNetworkGateway", func() { Expect(udnGateway.AddNetwork()).To(Succeed()) flowMap = udnGateway.gateway.openflowManager.flowCache - Expect(len(flowMap["DEFAULT"])).To(Equal(59)) // 14 UDN Flows are added by default + Expect(len(flowMap["DEFAULT"])).To(Equal(62)) // 16 UDN Flows are added by default Expect(len(udnGateway.openflowManager.defaultBridge.netConfig)).To(Equal(2)) // default network + UDN network + defaultUdnConfig := udnGateway.openflowManager.defaultBridge.netConfig["default"] + bridgeUdnConfig := udnGateway.openflowManager.defaultBridge.netConfig["bluenet"] + bridgeMAC := udnGateway.openflowManager.defaultBridge.macAddress.String() + ofPortHost := udnGateway.openflowManager.defaultBridge.ofPortHost for _, flows := range flowMap { for _, flow := range flows { if strings.Contains(flow, fmt.Sprintf("0x%x", udnGateway.masqCTMark)) { // UDN Flow udnFlows++ - } else if strings.Contains(flow, fmt.Sprintf("in_port=%s", udnGateway.openflowManager.defaultBridge.netConfig["bluenet"].ofPortPatch)) { + } else if strings.Contains(flow, fmt.Sprintf("in_port=%s", bridgeUdnConfig.ofPortPatch)) { udnFlows++ } } } Expect(udnFlows).To(Equal(14)) + for _, svcCIDR := range config.Kubernetes.ServiceCIDRs { + // Check flows for default network service CIDR. + checkDefaultSvcIsolationOVSFlows(flowMap["DEFAULT"], defaultUdnConfig, ofPortHost, bridgeMAC, svcCIDR) + + // Expect exactly one flow per UDN for tables 0 and 2 for service isolation. + checkUDNSvcIsolationOVSFlows(flowMap["DEFAULT"], bridgeUdnConfig, "bluenet", ofPortHost, bridgeMAC, svcCIDR, 1) + } + cnode := node.DeepCopy() kubeMock.On("UpdateNodeStatus", cnode).Return(nil) // check if network key gets deleted from annotation Expect(udnGateway.DelNetwork()).To(Succeed()) flowMap = udnGateway.gateway.openflowManager.flowCache - Expect(len(flowMap["DEFAULT"])).To(Equal(45)) // only default network flows are present + Expect(len(flowMap["DEFAULT"])).To(Equal(46)) // only default network flows are present Expect(len(udnGateway.openflowManager.defaultBridge.netConfig)).To(Equal(1)) // default network only udnFlows = 0 for _, flows := range flowMap { @@ -756,6 +853,14 @@ var _ = Describe("UserDefinedNetworkGateway", func() { } } Expect(udnFlows).To(Equal(0)) + + for _, svcCIDR := range config.Kubernetes.ServiceCIDRs { + // Check flows for default network service CIDR. + checkDefaultSvcIsolationOVSFlows(flowMap["DEFAULT"], defaultUdnConfig, ofPortHost, bridgeMAC, svcCIDR) + + // Expect no more flows per UDN for tables 0 and 2 for service isolation. + checkUDNSvcIsolationOVSFlows(flowMap["DEFAULT"], bridgeUdnConfig, "bluenet", ofPortHost, bridgeMAC, svcCIDR, 0) + } return nil }) Expect(err).NotTo(HaveOccurred()) diff --git a/go-controller/pkg/node/openflow_manager.go b/go-controller/pkg/node/openflow_manager.go index 71bb4e2db16..58759977942 100644 --- a/go-controller/pkg/node/openflow_manager.go +++ b/go-controller/pkg/node/openflow_manager.go @@ -10,6 +10,7 @@ import ( "time" "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/config" + "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/generator/udn" "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/types" "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/util" @@ -39,10 +40,10 @@ func (c *openflowManager) getExGwBridgePortConfigurations() ([]bridgeUDNConfigur return c.externalGatewayBridge.getBridgePortConfigurations() } -func (c *openflowManager) addNetwork(nInfo util.NetInfo, masqCTMark uint, v4MasqIP, v6MasqIP *net.IPNet) { - c.defaultBridge.addNetworkBridgeConfig(nInfo, masqCTMark, v4MasqIP, v6MasqIP) +func (c *openflowManager) addNetwork(nInfo util.NetInfo, masqCTMark uint, v4MasqIPs, v6MasqIPs *udn.MasqueradeIPs) { + c.defaultBridge.addNetworkBridgeConfig(nInfo, masqCTMark, v4MasqIPs, v6MasqIPs) if c.externalGatewayBridge != nil { - c.externalGatewayBridge.addNetworkBridgeConfig(nInfo, masqCTMark, v4MasqIP, v6MasqIP) + c.externalGatewayBridge.addNetworkBridgeConfig(nInfo, masqCTMark, v4MasqIPs, v6MasqIPs) } } diff --git a/test/e2e/network_segmentation_services.go b/test/e2e/network_segmentation_services.go index f66fa853e53..83940d35785 100644 --- a/test/e2e/network_segmentation_services.go +++ b/test/e2e/network_segmentation_services.go @@ -249,15 +249,9 @@ var _ = Describe("Network Segmentation: services", func() { By("Verify the UDN client connection to the default network service") checkConnectionToNodePort(f, udnClientPod2, defaultService, &nodes.Items[0], "server node", defaultServerPod.Name) - // TODO(kyrtapz): This doesn't work in L2, the condtion will change to checkNoConnectionToNodePort in https://github.com/ovn-org/ovn-kubernetes/pull/4705 - if netConfigParams.topology != types.Layer2Topology { - checkConnectionToNodePort(f, udnClientPod2, defaultService, &nodes.Items[1], "local node", defaultServerPod.Name) - } + checkNoConnectionToNodePort(f, udnClientPod2, defaultService, &nodes.Items[1], "local node") checkConnectionToNodePort(f, udnClientPod2, defaultService, &nodes.Items[2], "other node", defaultServerPod.Name) - // FIXME(tssurya): https://github.com/ovn-org/ovn-kubernetes/issues/4687 - if !IsGatewayModeLocal() { - checkNoConnectionToClusterIPs(f, udnClientPod2, defaultService) - } + checkNoConnectionToClusterIPs(f, udnClientPod2, defaultService) }, Entry(