Ncid upper case hotfix (#1943)

* NCID uppercase hotfix

Author: paulyufan2

cns/restserver/api_test.go

@@ -1292,6 +1292,37 @@ func TestCreateHostNCApipaEndpoint(t *testing.T) {
 	fmt.Printf("createHostNCApipaEndpoint Responded with %+v\n", createHostNCApipaEndpointResponse)
 }
 
+// TestNCIDCaseInSensitive() tests NCID case insensitive that either uppercase or lowercase NCID should be accepted
+// get the ncVersionList with nc GUID as lower case and when looking up if the ncid is present in ncVersionList,
+// convert it to lowercase and then check if Vfp programming completes
+func TestNCIDCaseInSensitive(t *testing.T) {
+	setEnv(t)
+	err := setOrchestratorType(t, cns.Kubernetes)
+	if err != nil {
+		t.Fatalf("TestNCIDCaseSensitive failed with error:%+v", err)
+	}
+
+	// add a list of NCIDs with upper-case NCID, lower-case NCID, upper-case cns-managed mode NCID and lower-case cns-managed mode NCID
+	ncids := []string{
+		"Swift_89063DBF-AA31-4BFC-9663-3842A361F189", "Swift_f5750a6e-f05a-11ed-a05b-0242ac120003",
+		"17289C8E-F05B-11ED-A05B-0242AC120003", "0f6d764a-f05b-11ed-a05b-0242ac120003",
+	}
+
+	ncVersionList := map[string]string{}
+	// add lower-case NCIDs to ncVersionList
+	for _, ncid := range ncids {
+		ncVersionList[lowerCaseNCGuid(ncid)] = "1"
+	}
+
+	for _, ncid := range ncids {
+		_, returnCode, errMsg := svc.isNCWaitingForUpdate("0", ncid, ncVersionList)
+		// verify if Vfp programming completes with all types of incoming NCIDs
+		if returnCode != types.NetworkContainerVfpProgramComplete {
+			t.Fatalf("failed to verify TestNCIDCaseInSensitive for ncid %s due to %s", ncid, errMsg)
+		}
+	}
+}
+
 func TestGetAllNetworkContainers(t *testing.T) {
 	setEnv(t)
 	err := setOrchestratorType(t, cns.Kubernetes)

cns/restserver/internalapi.go

@@ -12,6 +12,7 @@ import (
 	"net/http/httptest"
 	"reflect"
 	"strconv"
+	"strings"
 	"time"
 
 	"github.com/Azure/azure-container-networking/cns"
@@ -104,7 +105,7 @@ func (service *HTTPRestService) SyncNodeStatus(dncEP, infraVnet, nodeID string,
 	if !skipNCVersionCheck {
 		nmaNCs := map[string]string{}
 		for _, nc := range ncVersionListResp.Containers {
-			nmaNCs[cns.SwiftPrefix+nc.NetworkContainerID] = nc.Version
+			nmaNCs[cns.SwiftPrefix+strings.ToLower(nc.NetworkContainerID)] = nc.Version
 		}
 
 		// check if the version is valid and save it to service state
@@ -220,7 +221,7 @@ func (service *HTTPRestService) syncHostNCVersion(ctx context.Context, channelMo
 
 	nmaNCs := map[string]string{}
 	for _, nc := range ncVersionListResp.Containers {
-		nmaNCs[nc.NetworkContainerID] = nc.Version
+		nmaNCs[strings.ToLower(nc.NetworkContainerID)] = nc.Version
 	}
 	for ncID := range outdatedNCs {
 		nmaNCVersionStr, ok := nmaNCs[ncID]

cns/restserver/internalapi_test.go

@@ -32,6 +32,7 @@ const (
 	requestPercent      = 100
 	batchSize           = 10
 	initPoolSize        = 10
+	ncID                = "6a07155a-32d7-49af-872f-1e70ee366dc0"
 )
 
 var dnsservers = []string{"8.8.8.8", "8.8.4.4"}
@@ -61,7 +62,6 @@ func TestCreateAndUpdateNCWithSecondaryIPNCVersion(t *testing.T) {
 	// NC version set as 0 which is the default initial value.
 	ncVersion := 0
 	secondaryIPConfigs := make(map[string]cns.SecondaryIPConfig)
-	ncID := "testNc1"
 
 	// Build secondaryIPConfig, it will have one item as {IPAddress:"10.0.0.16", NCVersion: 0}
 	ipAddress := "10.0.0.16"
@@ -219,7 +219,6 @@ func createNCReqeustForSyncHostNCVersion(t *testing.T) cns.CreateNetworkContaine
 	// NC version set as 0 which is the default initial value.
 	ncVersion := 0
 	secondaryIPConfigs := make(map[string]cns.SecondaryIPConfig)
-	ncID := "testNc1"
 
 	// Build secondaryIPConfig, it will have one item as {IPAddress:"10.0.0.16", NCVersion: 0}
 	ipAddress := "10.0.0.16"
@@ -393,7 +392,6 @@ func setOrchestratorTypeInternal(orchestratorType string) {
 
 func validateCreateNCInternal(t *testing.T, secondaryIpCount int, ncVersion string) {
 	secondaryIPConfigs := make(map[string]cns.SecondaryIPConfig)
-	ncId := "testNc1"
 	ncVersionInInt, _ := strconv.Atoi(ncVersion)
 	startingIndex := 6
 	for i := 0; i < secondaryIpCount; i++ {
@@ -404,12 +402,11 @@ func validateCreateNCInternal(t *testing.T, secondaryIpCount int, ncVersion stri
 		startingIndex++
 	}
 
-	createAndValidateNCRequest(t, secondaryIPConfigs, ncId, ncVersion)
+	createAndValidateNCRequest(t, secondaryIPConfigs, ncID, ncVersion)
 }
 
 func validateCreateOrUpdateNCInternal(t *testing.T, secondaryIpCount int, ncVersion string) {
 	secondaryIPConfigs := make(map[string]cns.SecondaryIPConfig)
-	ncId := "testNc1"
 	ncVersionInInt, _ := strconv.Atoi(ncVersion)
 	startingIndex := 6
 	for i := 0; i < secondaryIpCount; i++ {
@@ -420,7 +417,7 @@ func validateCreateOrUpdateNCInternal(t *testing.T, secondaryIpCount int, ncVers
 		startingIndex++
 	}
 
-	createAndValidateNCRequest(t, secondaryIPConfigs, ncId, ncVersion)
+	createAndValidateNCRequest(t, secondaryIPConfigs, ncID, ncVersion)
 
 	// now Validate Update, add more secondaryIPConfig and it should handle the update
 	fmt.Println("Validate Scaleup")
@@ -432,7 +429,7 @@ func validateCreateOrUpdateNCInternal(t *testing.T, secondaryIpCount int, ncVers
 		startingIndex++
 	}
 
-	createAndValidateNCRequest(t, secondaryIPConfigs, ncId, ncVersion)
+	createAndValidateNCRequest(t, secondaryIPConfigs, ncID, ncVersion)
 
 	// now Scale down, delete 3 ipaddresses from secondaryIPConfig req
 	fmt.Println("Validate Scale down")
@@ -446,15 +443,15 @@ func validateCreateOrUpdateNCInternal(t *testing.T, secondaryIpCount int, ncVers
 		}
 	}
 
-	createAndValidateNCRequest(t, secondaryIPConfigs, ncId, ncVersion)
+	createAndValidateNCRequest(t, secondaryIPConfigs, ncID, ncVersion)
 
 	// Cleanup all SecondaryIps
 	fmt.Println("Validate no SecondaryIpconfigs")
 	for ipid := range secondaryIPConfigs {
 		delete(secondaryIPConfigs, ipid)
 	}
 
-	createAndValidateNCRequest(t, secondaryIPConfigs, ncId, ncVersion)
+	createAndValidateNCRequest(t, secondaryIPConfigs, ncID, ncVersion)
 }
 
 func createAndValidateNCRequest(t *testing.T, secondaryIPConfigs map[string]cns.SecondaryIPConfig, ncId, ncVersion string) {

cns/restserver/util.go

@@ -413,7 +413,9 @@ func (service *HTTPRestService) getAllNetworkContainerResponses(
 		}
 		nmaNCs := map[string]string{}
 		for _, nc := range ncVersionListResp.Containers {
-			nmaNCs[cns.SwiftPrefix+nc.NetworkContainerID] = nc.Version
+			// store nmaNCID as lower case to allow case insensitive comparison with nc stored in CNS
+			nmaNCID := cns.SwiftPrefix + strings.ToLower(nc.NetworkContainerID)
+			nmaNCs[nmaNCID] = nc.Version
 		}
 
 		if !skipNCVersionCheck {
@@ -606,7 +608,8 @@ func (service *HTTPRestService) attachOrDetachHelper(req cns.ConfigureContainerN
 				}
 				nmaNCs := map[string]string{}
 				for _, nc := range ncVersionListResp.Containers {
-					nmaNCs[nc.NetworkContainerID] = nc.Version
+					// store nmaNCID as lower case to allow case insensitive comparison with nc stored in CNS
+					nmaNCs[strings.ToLower(nc.NetworkContainerID)] = nc.Version
 				}
 				_, returnCode, message := service.isNCWaitingForUpdate(existing.CreateNetworkContainerRequest.Version, req.NetworkContainerid, nmaNCs)
 				if returnCode == types.NetworkContainerVfpProgramPending {
@@ -829,6 +832,16 @@ func (service *HTTPRestService) populateIPConfigInfoUntransacted(ipConfigStatus
 	return nil
 }
 
+// lowerCaseNCGuid() splits incoming NCID by "Swift_" and lowercase NC GUID; i.e,"Swift_ABCD-CD" -> "Swift_abcd-cd"
+func lowerCaseNCGuid(ncid string) string {
+	ncidHasSwiftPrefix := strings.HasPrefix(ncid, cns.SwiftPrefix)
+	if ncidHasSwiftPrefix {
+		return cns.SwiftPrefix + strings.ToLower(strings.Split(ncid, cns.SwiftPrefix)[1])
+	}
+
+	return strings.ToLower(ncid)
+}
+
 // isNCWaitingForUpdate :- Determine whether NC version on NMA matches programmed version
 // Return error and waitingForUpdate as true only CNS gets response from NMAgent indicating
 // the VFP programming is pending
@@ -853,13 +866,17 @@ func (service *HTTPRestService) isNCWaitingForUpdate(
 			"Skipping GetNCVersionStatus check from NMAgent", ncVersion, ncid)
 		return true, types.NetworkContainerVfpProgramPending, ""
 	}
-	nmaProgrammedNCVersionStr, ok := ncVersionList[ncid]
+
+	// get the ncVersionList with nc GUID as lower case
+	// when looking up if the ncid is present in ncVersionList, convert it to lowercase and then look up
+	nmaProgrammedNCVersionStr, ok := ncVersionList[lowerCaseNCGuid(ncid)]
 	if !ok {
 		// NMA doesn't have this NC that we need programmed yet, bail out
 		logger.Printf("[Azure CNS] Failed to get NC %s doesn't exist in NMAgent NC version list "+
 			"Skipping GetNCVersionStatus check from NMAgent", ncid)
 		return true, types.NetworkContainerVfpProgramPending, ""
 	}
+
 	nmaProgrammedNCVersion, err := strconv.Atoi(nmaProgrammedNCVersionStr)
 	if err != nil {
 		// it's unclear whether or not this can actually happen. In the NMAgent