fix: route issues on Swiftv2 Windows (#3375)

* updated CNS for adding default deny acl's

* added infra nic change

* added unit tests

* resolved pr comments

* updating to fix github checks

* added logging lines

* removing unnecessary logging lines

* removed cni circular dependency

* switch from having consts to making them inline

* cns changes based on update to network container contrac

* fixed spelling

* updated unit test

* updated test

* reverted a comment

* updated name of function

* changed policy type

* added a new line

* resolving pr comments

* resolving pr comments

* re-added back

* updated creating acl code to make it more modularized

* fixed golint errors

* fixed golint

* added tests

* fixed spelling

* moved an assertion line

* reformated creating acl's

* refactored code per pr comments

* fixed lint

* moved GetEndpointPolicy so that it is only run on init

* updated code

* updated error message

* updated getEndpointPolicy placement

* updated comment

* fixed golint issues

* refactored

* fixed comments

* updated return inline

* updated unit test returns

* corrected the go lint of file

* fix swiftv2 route issues

* ut fix

* fix linter issue

* fix comments

* fix comments

* fix comments

* enhance comment

* rename func names

---------

Signed-off-by: Paul Yu <[email protected]>
Co-authored-by: rejain456 <[email protected]>
Co-authored-by: rejain456 <[email protected]>

Author: 3 people

cns/middlewares/k8sSwiftV2.go

@@ -226,3 +226,51 @@ func (k *K8sSWIFTv2Middleware) UpdateIPConfigRequest(mtpnc v1alpha1.MultitenantP
 
 	return types.Success, ""
 }
+
+func (k *K8sSWIFTv2Middleware) AddRoutes(cidrs []string, gatewayIP string) []cns.Route {
+	routes := make([]cns.Route, len(cidrs))
+	for i, cidr := range cidrs {
+		routes[i] = cns.Route{
+			IPAddress:        cidr,
+			GatewayIPAddress: gatewayIP,
+		}
+	}
+	return routes
+}
+
+// Both Linux and Windows CNS gets infravnet and service CIDRs from configuration env
+// GetInfravnetAndServiceCidrs() returns v4CIDRs(infravnet and service cidrs) as first []string and v6CIDRs(infravnet and service) as second []string
+func (k *K8sSWIFTv2Middleware) GetInfravnetAndServiceCidrs() ([]string, []string, error) { //nolint
+	v4Cidrs := []string{}
+	v6Cidrs := []string{}
+
+	// Get and parse infraVNETCIDRs from env
+	infraVNETCIDRs, err := configuration.InfraVNETCIDRs()
+	if err != nil {
+		return nil, nil, errors.Wrapf(err, "failed to get infraVNETCIDRs from env")
+	}
+	infraVNETCIDRsv4, infraVNETCIDRsv6, err := utils.ParseCIDRs(infraVNETCIDRs)
+	if err != nil {
+		return nil, nil, errors.Wrapf(err, "failed to parse infraVNETCIDRs")
+	}
+
+	// Add infravnet CIDRs to v4 and v6 IPs
+	v4Cidrs = append(v4Cidrs, infraVNETCIDRsv4...)
+	v6Cidrs = append(v6Cidrs, infraVNETCIDRsv6...)
+
+	// Get and parse serviceCIDRs from env
+	serviceCIDRs, err := configuration.ServiceCIDRs()
+	if err != nil {
+		return nil, nil, errors.Wrapf(err, "failed to get serviceCIDRs from env")
+	}
+	serviceCIDRsV4, serviceCIDRsV6, err := utils.ParseCIDRs(serviceCIDRs)
+	if err != nil {
+		return nil, nil, errors.Wrapf(err, "failed to parse serviceCIDRs")
+	}
+
+	// Add service CIDRs to v4 and v6 IPs
+	v4Cidrs = append(v4Cidrs, serviceCIDRsV4...)
+	v6Cidrs = append(v6Cidrs, serviceCIDRsV6...)
+
+	return v4Cidrs, v6Cidrs, nil
+}

cns/middlewares/k8sSwiftV2_linux.go

@@ -32,50 +32,12 @@ func (k *K8sSWIFTv2Middleware) setRoutes(podIPInfo *cns.PodIpInfo) error {
 		routes = append(routes, virtualGWRoute, route)
 
 	case cns.InfraNIC:
-		// Get and parse infraVNETCIDRs from env
-		infraVNETCIDRs, err := configuration.InfraVNETCIDRs()
+		// Linux CNS middleware sets the infra routes(pod, infravnet and service cidrs) to infraNIC interface for the podIPInfo used in SWIFT V2 Linux scenario
+		infraRoutes, err := k.getInfraRoutes(podIPInfo)
 		if err != nil {
-			return errors.Wrapf(err, "failed to get infraVNETCIDRs from env")
-		}
-		infraVNETCIDRsv4, infraVNETCIDRsv6, err := utils.ParseCIDRs(infraVNETCIDRs)
-		if err != nil {
-			return errors.Wrapf(err, "failed to parse infraVNETCIDRs")
-		}
-
-		// Get and parse podCIDRs from env
-		podCIDRs, err := configuration.PodCIDRs()
-		if err != nil {
-			return errors.Wrapf(err, "failed to get podCIDRs from env")
-		}
-		podCIDRsV4, podCIDRv6, err := utils.ParseCIDRs(podCIDRs)
-		if err != nil {
-			return errors.Wrapf(err, "failed to parse podCIDRs")
-		}
-
-		// Get and parse serviceCIDRs from env
-		serviceCIDRs, err := configuration.ServiceCIDRs()
-		if err != nil {
-			return errors.Wrapf(err, "failed to get serviceCIDRs from env")
-		}
-		serviceCIDRsV4, serviceCIDRsV6, err := utils.ParseCIDRs(serviceCIDRs)
-		if err != nil {
-			return errors.Wrapf(err, "failed to parse serviceCIDRs")
-		}
-
-		ip, err := netip.ParseAddr(podIPInfo.PodIPConfig.IPAddress)
-		if err != nil {
-			return errors.Wrapf(err, "failed to parse podIPConfig IP address %s", podIPInfo.PodIPConfig.IPAddress)
-		}
-
-		if ip.Is4() {
-			routes = append(routes, addRoutes(podCIDRsV4, overlayGatewayv4)...)
-			routes = append(routes, addRoutes(serviceCIDRsV4, overlayGatewayv4)...)
-			routes = append(routes, addRoutes(infraVNETCIDRsv4, overlayGatewayv4)...)
-		} else {
-			routes = append(routes, addRoutes(podCIDRv6, overlayGatewayV6)...)
-			routes = append(routes, addRoutes(serviceCIDRsV6, overlayGatewayV6)...)
-			routes = append(routes, addRoutes(infraVNETCIDRsv6, overlayGatewayV6)...)
+			return errors.Wrap(err, "failed to get infra routes for infraNIC interface")
 		}
+		routes = infraRoutes
 		podIPInfo.SkipDefaultRoutes = true
 
 	case cns.NodeNetworkInterfaceBackendNIC: //nolint:exhaustive // ignore exhaustive types check
@@ -88,22 +50,68 @@ func (k *K8sSWIFTv2Middleware) setRoutes(podIPInfo *cns.PodIpInfo) error {
 	return nil
 }
 
-func addRoutes(cidrs []string, gatewayIP string) []cns.Route {
-	routes := make([]cns.Route, len(cidrs))
-	for i, cidr := range cidrs {
-		routes[i] = cns.Route{
-			IPAddress:        cidr,
-			GatewayIPAddress: gatewayIP,
-		}
+// Linux CNS gets pod CIDRs from configuration env
+// Containerd reassigns the IP to the adapter and kernel configures the pod cidr route by default on Windows VM
+// Hence the windows swiftv2 scenario does not require pod cidr
+// GetPodCidrs() will return v4PodCidrs as first []string and v6PodCidrs as second []string
+func (k *K8sSWIFTv2Middleware) GetPodCidrs() ([]string, []string, error) { //nolint
+	v4PodCidrs := []string{}
+	v6PodCidrs := []string{}
+
+	// Get and parse podCIDRs from env
+	podCIDRs, err := configuration.PodCIDRs()
+	if err != nil {
+		return nil, nil, errors.Wrapf(err, "failed to get podCIDRs from env")
+	}
+	podCIDRsV4, podCIDRv6, err := utils.ParseCIDRs(podCIDRs)
+	if err != nil {
+		return nil, nil, errors.Wrapf(err, "failed to parse podCIDRs")
+	}
+
+	v4PodCidrs = append(v4PodCidrs, podCIDRsV4...)
+	v6PodCidrs = append(v6PodCidrs, podCIDRv6...)
+
+	return v4PodCidrs, v6PodCidrs, nil
+}
+
+// getInfraRoutes() returns the infra routes including infravnet/pod/service cidrs for the podIPInfo used in SWIFT V2 Linux scenario
+// Linux uses 169.254.1.1 as the default ipv4 gateway and fe80::1234:5678:9abc as the default ipv6 gateway
+func (k *K8sSWIFTv2Middleware) getInfraRoutes(podIPInfo *cns.PodIpInfo) ([]cns.Route, error) {
+	var routes []cns.Route
+
+	ip, err := netip.ParseAddr(podIPInfo.PodIPConfig.IPAddress)
+	if err != nil {
+		return nil, errors.Wrapf(err, "failed to parse podIPConfig IP address %s", podIPInfo.PodIPConfig.IPAddress)
 	}
-	return routes
+
+	v4IPs, v6IPs, err := k.GetInfravnetAndServiceCidrs()
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to get infravnet and service CIDRs")
+	}
+
+	v4PodIPs, v6PodIPs, err := k.GetPodCidrs()
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to get pod CIDRs")
+	}
+
+	v4IPs = append(v4IPs, v4PodIPs...)
+	v6IPs = append(v6IPs, v6PodIPs...)
+
+	if ip.Is4() {
+		routes = append(routes, k.AddRoutes(v4IPs, overlayGatewayv4)...)
+	} else {
+		routes = append(routes, k.AddRoutes(v6IPs, overlayGatewayV6)...)
+	}
+
+	return routes, nil
 }
 
 // assignSubnetPrefixLengthFields is a no-op for linux swiftv2 as the default prefix-length is sufficient
 func (k *K8sSWIFTv2Middleware) assignSubnetPrefixLengthFields(_ *cns.PodIpInfo, _ v1alpha1.InterfaceInfo, _ string) error {
 	return nil
 }
 
+// add default route is done on setRoutes() for Linux swiftv2
 func (k *K8sSWIFTv2Middleware) addDefaultRoute(*cns.PodIpInfo, string) {}
 
 // IPConfigsRequestHandlerWrapper is the middleware function for handling SWIFT v2 IP configs requests for AKS-SWIFT. This function wrapped the default SWIFT request

cns/middlewares/k8sSwiftV2_linux_test.go

@@ -11,6 +11,7 @@ import (
 	"github.com/Azure/azure-container-networking/cns/middlewares/mock"
 	"github.com/Azure/azure-container-networking/cns/types"
 	"github.com/Azure/azure-container-networking/crd/multitenancy/api/v1alpha1"
+	"github.com/google/go-cmp/cmp"
 	"gotest.tools/v3/assert"
 )
 
@@ -342,10 +343,10 @@ func TestSetRoutesSuccess(t *testing.T) {
 		} else {
 			assert.Equal(t, ipInfo.SkipDefaultRoutes, false)
 		}
-
 	}
+
 	for i := range podIPInfo {
-		assert.DeepEqual(t, podIPInfo[i].Routes, desiredPodIPInfo[i].Routes)
+		cmp.Equal(podIPInfo[i].Routes, desiredPodIPInfo[i].Routes)
 	}
 }
 
@@ -378,9 +379,10 @@ func TestSetRoutesFailure(t *testing.T) {
 }
 
 func TestAddRoutes(t *testing.T) {
+	middleware := K8sSWIFTv2Middleware{Cli: mock.NewClient()}
 	cidrs := []string{"10.0.0.0/24", "20.0.0.0/24"}
 	gatewayIP := "192.168.1.1"
-	routes := addRoutes(cidrs, gatewayIP)
+	routes := middleware.AddRoutes(cidrs, gatewayIP)
 	expectedRoutes := []cns.Route{
 		{
 			IPAddress:        "10.0.0.0/24",

cns/middlewares/k8sSwiftV2_windows.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"net/netip"
 
 	"github.com/Azure/azure-container-networking/cns"
 	"github.com/Azure/azure-container-networking/cns/logger"
@@ -18,6 +19,10 @@ var defaultDenyEgressPolicy policy.Policy = mustGetEndpointPolicy(cns.DirectionT
 
 var defaultDenyIngressPolicy policy.Policy = mustGetEndpointPolicy(cns.DirectionTypeIn)
 
+const (
+	defaultGateway = "0.0.0.0"
+)
+
 // for AKS L1VH, do not set default route on infraNIC to avoid customer pod reaching all infra vnet services
 // default route is set for secondary interface NIC(i.e,delegatedNIC)
 func (k *K8sSWIFTv2Middleware) setRoutes(podIPInfo *cns.PodIpInfo) error {
@@ -27,10 +32,16 @@ func (k *K8sSWIFTv2Middleware) setRoutes(podIPInfo *cns.PodIpInfo) error {
 		// TODO: Remove this once HNS fix is ready
 		route := cns.Route{
 			IPAddress:        "0.0.0.0/0",
-			GatewayIPAddress: "0.0.0.0",
+			GatewayIPAddress: defaultGateway,
 		}
 		podIPInfo.Routes = append(podIPInfo.Routes, route)
 
+		// Windows CNS middleware sets the infra routes(infravnet and service cidrs) to infraNIC interface for the podIPInfo used in SWIFT V2 Windows scenario
+		infraRoutes, err := k.getInfraRoutes(podIPInfo)
+		if err != nil {
+			return errors.Wrap(err, "failed to set routes for infraNIC interface")
+		}
+		podIPInfo.Routes = append(podIPInfo.Routes, infraRoutes...)
 		podIPInfo.SkipDefaultRoutes = true
 	}
 	return nil
@@ -208,3 +219,27 @@ func GetDefaultDenyBool(mtpnc v1alpha1.MultitenantPodNetworkConfig) bool {
 	// returns the value of DefaultDenyACL from mtpnc
 	return mtpnc.Status.DefaultDenyACL
 }
+
+// getInfraRoutes() returns the infra routes including infravnet/ and service cidrs for the podIPInfo used in SWIFT V2 Windows scenario
+// Windows uses default route 0.0.0.0 as the gateway IP for containerd to configure;
+// For example, containerd would set route like: ip route 10.0.0.0/16 via 0.0.0.0 dev eth0
+func (k *K8sSWIFTv2Middleware) getInfraRoutes(podIPInfo *cns.PodIpInfo) ([]cns.Route, error) {
+	var routes []cns.Route
+
+	ip, err := netip.ParseAddr(podIPInfo.PodIPConfig.IPAddress)
+	if err != nil {
+		return nil, errors.Wrapf(err, "failed to parse podIPConfig IP address %s", podIPInfo.PodIPConfig.IPAddress)
+	}
+
+	// TODO: add ipv6 when supported
+	v4IPs, _, err := k.GetInfravnetAndServiceCidrs()
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to get infravnet and service CIDRs")
+	}
+
+	if ip.Is4() {
+		routes = append(routes, k.AddRoutes(v4IPs, defaultGateway)...)
+	}
+
+	return routes, nil
+}

cns/middlewares/k8sSwiftV2_windows_test.go

@@ -7,6 +7,7 @@ import (
 	"testing"
 
 	"github.com/Azure/azure-container-networking/cns"
+	"github.com/Azure/azure-container-networking/cns/configuration"
 	"github.com/Azure/azure-container-networking/cns/middlewares/mock"
 	"github.com/Azure/azure-container-networking/crd/multitenancy/api/v1alpha1"
 	"github.com/Azure/azure-container-networking/network/policy"
@@ -17,11 +18,14 @@ import (
 
 func TestSetRoutesSuccess(t *testing.T) {
 	middleware := K8sSWIFTv2Middleware{Cli: mock.NewClient()}
+	t.Setenv(configuration.EnvServiceCIDRs, "10.0.0.0/16")
+	t.Setenv(configuration.EnvInfraVNETCIDRs, "10.240.0.10/16")
+	t.Setenv(configuration.EnvPodCIDRs, "10.1.0.10/24") // make sure windows swiftv2 does not set pod cidr route
 
 	podIPInfo := []cns.PodIpInfo{
 		{
 			PodIPConfig: cns.IPSubnet{
-				IPAddress:    "10.0.1.10",
+				IPAddress:    "10.0.1.100",
 				PrefixLength: 32,
 			},
 			NICType: cns.InfraNIC,
@@ -35,6 +39,30 @@ func TestSetRoutesSuccess(t *testing.T) {
 			MacAddress: "12:34:56:78:9a:bc",
 		},
 	}
+
+	desiredPodIPInfo := []cns.PodIpInfo{
+		{
+			PodIPConfig: cns.IPSubnet{
+				IPAddress:    "10.0.1.100",
+				PrefixLength: 32,
+			},
+			NICType: cns.InfraNIC,
+			Routes: []cns.Route{
+				{
+					IPAddress:        "10.0.0.0/16",
+					GatewayIPAddress: "0.0.0.0",
+				},
+				{
+					IPAddress:        "10.240.0.10/16",
+					GatewayIPAddress: "0.0.0.0",
+				},
+				{
+					IPAddress:        "0.0.0.0/0",
+					GatewayIPAddress: "0.0.0.0",
+				},
+			},
+		},
+	}
 	for i := range podIPInfo {
 		ipInfo := &podIPInfo[i]
 		err := middleware.setRoutes(ipInfo)
@@ -45,6 +73,9 @@ func TestSetRoutesSuccess(t *testing.T) {
 			assert.Equal(t, ipInfo.SkipDefaultRoutes, false)
 		}
 	}
+
+	// check if the routes are set as expected
+	reflect.DeepEqual(podIPInfo[0].Routes, desiredPodIPInfo[0].Routes)
 }
 
 func TestAssignSubnetPrefixSuccess(t *testing.T) {