fix: Windows Overlay Gateway Bug (#1852)

* fix overlay gatway bug that prevented cloud-node-manager-windows from initializing node because it couldn't reach IMDS

* use parseip instead of mask

* scope CNI overlay gateway bug fix to windows only

* update test

* adding UT to verify overlay NAT info is empty

* add podsubnet netInfo test

* fix tests and convert to old structs names

---------

Co-authored-by: Jaeryn <[email protected]>

Author: jaer-tsun

cni/network/invoker_cns.go

@@ -20,9 +20,8 @@ import (
 )
 
 var (
-	errEmptyCNIArgs  = errors.New("empty CNI cmd args not allowed")
-	errInvalidArgs   = errors.New("invalid arg(s)")
-	overlayGatewayIP = "169.254.1.1"
+	errEmptyCNIArgs = errors.New("empty CNI cmd args not allowed")
+	errInvalidArgs  = errors.New("invalid arg(s)")
 )
 
 type CNSIPAMInvoker struct {
@@ -99,19 +98,22 @@ func (invoker *CNSIPAMInvoker) Add(addConfig IPAMAddConfig) (IPAMAddResult, erro
 
 	log.Printf("[cni-invoker-cns] Received info %+v for pod %v", info, podInfo)
 
+	// set result ipconfigArgument from CNS Response Body
+	ip, ncipnet, err := net.ParseCIDR(info.podIPAddress + "/" + fmt.Sprint(info.ncSubnetPrefix))
+	if ip == nil {
+		return IPAMAddResult{}, errors.Wrap(err, "Unable to parse IP from response: "+info.podIPAddress+" with err %w")
+	}
+
 	ncgw := net.ParseIP(info.ncGatewayIPAddress)
 	if ncgw == nil {
 		if invoker.ipamMode != util.V4Overlay {
 			return IPAMAddResult{}, errors.Wrap(errInvalidArgs, "%w: Gateway address "+info.ncGatewayIPAddress+" from response is invalid")
 		}
 
-		ncgw = net.ParseIP(overlayGatewayIP)
-	}
-
-	// set result ipconfigArgument from CNS Response Body
-	ip, ncipnet, err := net.ParseCIDR(info.podIPAddress + "/" + fmt.Sprint(info.ncSubnetPrefix))
-	if ip == nil {
-		return IPAMAddResult{}, errors.Wrap(err, "Unable to parse IP from response: "+info.podIPAddress+" with err %w")
+		ncgw, err = getOverlayGateway(ncipnet)
+		if err != nil {
+			return IPAMAddResult{}, err
+		}
 	}
 
 	// construct ipnet for result

cni/network/invoker_cns_test.go

@@ -2,10 +2,13 @@ package network
 
 import (
 	"errors"
+	"fmt"
 	"net"
+	"runtime"
 	"testing"
 
 	"github.com/Azure/azure-container-networking/cni"
+	"github.com/Azure/azure-container-networking/cni/util"
 	"github.com/Azure/azure-container-networking/cns"
 	"github.com/Azure/azure-container-networking/iptables"
 	"github.com/Azure/azure-container-networking/network"
@@ -25,12 +28,21 @@ func getTestIPConfigRequest() cns.IPConfigRequest {
 	}
 }
 
+func getTestOverlayGateway() net.IP {
+	if runtime.GOOS == "windows" {
+		return net.ParseIP("10.240.0.1")
+	}
+
+	return net.ParseIP("169.254.1.1")
+}
+
 func TestCNSIPAMInvoker_Add(t *testing.T) {
 	require := require.New(t) //nolint further usage of require without passing t
 	type fields struct {
 		podName      string
 		podNamespace string
 		cnsClient    cnsclient
+		ipamMode     util.IpamMode
 	}
 	type args struct {
 		nwCfg            *cni.NetworkConfig
@@ -127,6 +139,76 @@ func TestCNSIPAMInvoker_Add(t *testing.T) {
 			},
 			wantErr: true,
 		},
+		{
+			name: "Test happy CNI Overlay add",
+			fields: fields{
+				podName:      testPodInfo.PodName,
+				podNamespace: testPodInfo.PodNamespace,
+				ipamMode:     util.V4Overlay,
+				cnsClient: &MockCNSClient{
+					require: require,
+					request: requestIPAddressHandler{
+						ipconfigArgument: cns.IPConfigRequest{
+							PodInterfaceID:      "testcont-testifname3",
+							InfraContainerID:    "testcontainerid3",
+							OrchestratorContext: marshallPodInfo(testPodInfo),
+						},
+						result: &cns.IPConfigResponse{
+							PodIpInfo: cns.PodIpInfo{
+								PodIPConfig: cns.IPSubnet{
+									IPAddress:    "10.240.1.242",
+									PrefixLength: 16,
+								},
+								NetworkContainerPrimaryIPConfig: cns.IPConfiguration{
+									IPSubnet: cns.IPSubnet{
+										IPAddress:    "10.240.1.0",
+										PrefixLength: 16,
+									},
+									DNSServers:       nil,
+									GatewayIPAddress: "",
+								},
+								HostPrimaryIPInfo: cns.HostIPInfo{
+									Gateway:   "10.224.0.1",
+									PrimaryIP: "10.224.0.5",
+									Subnet:    "10.224.0.0/16",
+								},
+							},
+							Response: cns.Response{
+								ReturnCode: 0,
+								Message:    "",
+							},
+						},
+						err: nil,
+					},
+				},
+			},
+			args: args{
+				nwCfg: &cni.NetworkConfig{},
+				args: &cniSkel.CmdArgs{
+					ContainerID: "testcontainerid3",
+					Netns:       "testnetns3",
+					IfName:      "testifname3",
+				},
+				hostSubnetPrefix: getCIDRNotationForAddress("10.224.0.0/16"),
+				options:          map[string]interface{}{},
+			},
+			want: &cniTypesCurr.Result{
+				IPs: []*cniTypesCurr.IPConfig{
+					{
+						Address: *getCIDRNotationForAddress("10.240.1.242/16"),
+						Gateway: getTestOverlayGateway(),
+					},
+				},
+				Routes: []*cniTypes.Route{
+					{
+						Dst: network.Ipv4DefaultRouteDstPrefix,
+						GW:  getTestOverlayGateway(),
+					},
+				},
+			},
+			want1:   nil,
+			wantErr: false,
+		},
 	}
 	for _, tt := range tests {
 		tt := tt
@@ -136,13 +218,17 @@ func TestCNSIPAMInvoker_Add(t *testing.T) {
 				podNamespace: tt.fields.podNamespace,
 				cnsClient:    tt.fields.cnsClient,
 			}
+			if tt.fields.ipamMode != "" {
+				invoker.ipamMode = tt.fields.ipamMode
+			}
 			ipamAddResult, err := invoker.Add(IPAMAddConfig{nwCfg: tt.args.nwCfg, args: tt.args.args, options: tt.args.options})
 			if tt.wantErr {
 				require.Error(err)
 			} else {
 				require.NoError(err)
 			}
 
+			fmt.Printf("want:%+v\nrest:%+v\n", tt.want, ipamAddResult.ipv4Result)
 			require.Equalf(tt.want, ipamAddResult.ipv4Result, "incorrect ipv4 response")
 			require.Equalf(tt.want1, ipamAddResult.ipv6Result, "incorrect ipv6 response")
 		})

cni/network/network.go

@@ -536,7 +536,7 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error {
 		logAndSendEvent(plugin, fmt.Sprintf("[cni-net] Created network %v with subnet %v.", networkID, ipamAddResult.hostSubnetPrefix.String()))
 	}
 
-	natInfo := getNATInfo(nwCfg.ExecutionMode, options[network.SNATIPKey], nwCfg.MultiTenancy, enableSnatForDNS)
+	natInfo := getNATInfo(nwCfg, options[network.SNATIPKey], enableSnatForDNS)
 
 	createEndpointInternalOpt := createEndpointInternalOpt{
 		nwCfg:            nwCfg,

cni/network/network_linux.go

@@ -136,8 +136,12 @@ func (plugin *NetPlugin) getNetworkName(_ string, _ *IPAMAddResult, nwCfg *cni.N
 	return nwCfg.Name, nil
 }
 
-func getNATInfo(_ string, _ interface{}, _, _ bool) (natInfo []policy.NATInfo) {
+func getNATInfo(_ *cni.NetworkConfig, _ interface{}, _ bool) (natInfo []policy.NATInfo) {
 	return natInfo
 }
 
 func platformInit(cniConfig *cni.NetworkConfig) {}
+
+func getOverlayGateway(_ *net.IPNet) (net.IP, error) {
+	return net.ParseIP("169.254.1.1"), nil
+}

cni/network/network_test.go

@@ -4,13 +4,16 @@ import (
 	"fmt"
 	"net"
 	"os"
+	"runtime"
 	"testing"
 
 	"github.com/Azure/azure-container-networking/cni"
 	"github.com/Azure/azure-container-networking/cni/api"
 	"github.com/Azure/azure-container-networking/cni/util"
 	"github.com/Azure/azure-container-networking/common"
 	acnnetwork "github.com/Azure/azure-container-networking/network"
+	"github.com/Azure/azure-container-networking/network/networkutils"
+	"github.com/Azure/azure-container-networking/network/policy"
 	"github.com/Azure/azure-container-networking/nns"
 	"github.com/Azure/azure-container-networking/telemetry"
 	cniSkel "github.com/containernetworking/cni/pkg/skel"
@@ -706,7 +709,7 @@ func TestPluginMultitenancyDelete(t *testing.T) {
 }
 
 /*
-	Baremetal scenarios
+Baremetal scenarios
 */
 func TestPluginBaremetalAdd(t *testing.T) {
 	plugin, _ := cni.NewPlugin("test", "0.3.0")
@@ -835,7 +838,7 @@ func TestPluginBaremetalDelete(t *testing.T) {
 }
 
 /*
-	AKS-Swift scenario
+AKS-Swift scenario
 */
 func TestPluginAKSSwiftAdd(t *testing.T) {
 	plugin := GetTestResources()
@@ -1067,3 +1070,24 @@ func TestGetNetworkName(t *testing.T) {
 		})
 	}
 }
+
+func TestGetOverlayNatInfo(t *testing.T) {
+	nwCfg := &cni.NetworkConfig{ExecutionMode: string(util.V4Swift)}
+	nwCfg.Ipam.Mode = string(util.V4Overlay)
+	natInfo := getNATInfo(nwCfg, nil, false)
+	require.Empty(t, natInfo, "overlay natInfo should be empty")
+}
+
+func TestGetPodSubnetNatInfo(t *testing.T) {
+	ncPrimaryIP := "10.241.0.4"
+	nwCfg := &cni.NetworkConfig{ExecutionMode: string(util.V4Swift)}
+	natInfo := getNATInfo(nwCfg, ncPrimaryIP, false)
+	if runtime.GOOS == "windows" {
+		require.Equalf(t, natInfo, []policy.NATInfo{
+			{VirtualIP: ncPrimaryIP, Destinations: []string{networkutils.AzureDNS}},
+			{Destinations: []string{networkutils.AzureIMDS}},
+		}, "invalid windows podsubnet natInfo")
+	} else {
+		require.Empty(t, natInfo, "linux podsubnet natInfo should be empty")
+	}
+}

cni/network/network_windows.go

@@ -378,15 +378,15 @@ func determineWinVer() {
 	}
 }
 
-func getNATInfo(executionMode string, ncPrimaryIPIface interface{}, multitenancy, enableSnatForDNS bool) (natInfo []policy.NATInfo) {
-	if executionMode == string(util.V4Swift) {
+func getNATInfo(nwCfg *cni.NetworkConfig, ncPrimaryIPIface interface{}, enableSnatForDNS bool) (natInfo []policy.NATInfo) {
+	if nwCfg.ExecutionMode == string(util.V4Swift) && nwCfg.Ipam.Mode != string(util.V4Overlay) {
 		ncPrimaryIP := ""
 		if ncPrimaryIPIface != nil {
 			ncPrimaryIP = ncPrimaryIPIface.(string)
 		}
 
 		natInfo = append(natInfo, []policy.NATInfo{{VirtualIP: ncPrimaryIP, Destinations: []string{networkutils.AzureDNS}}, {Destinations: []string{networkutils.AzureIMDS}}}...)
-	} else if multitenancy && enableSnatForDNS {
+	} else if nwCfg.MultiTenancy && enableSnatForDNS {
 		natInfo = append(natInfo, policy.NATInfo{Destinations: []string{networkutils.AzureDNS}})
 	}
 
@@ -400,3 +400,14 @@ func platformInit(cniConfig *cni.NetworkConfig) {
 		network.EnableHnsV2Timeout(cniConfig.WindowsSettings.HnsTimeoutDurationInSeconds)
 	}
 }
+
+func getOverlayGateway(podsubnet *net.IPNet) (net.IP, error) {
+	ncgw := podsubnet.IP
+	ncgw[3]++
+	ncgw = net.ParseIP(ncgw.String())
+	if ncgw == nil || !podsubnet.Contains(ncgw) {
+		return nil, errors.Wrap(errInvalidArgs, "%w: Failed to retrieve overlay gateway from podsubnet"+podsubnet.IP.String())
+	}
+
+	return ncgw, nil
+}