From e2cdc67bdd933172625e9bf69093ef863b4962d3 Mon Sep 17 00:00:00 2001 From: Riya Date: Wed, 15 Oct 2025 16:10:44 -0700 Subject: [PATCH 01/11] added logic to fix cns bug for overlay subnet expansion --- cns/NetworkContainerContract.go | 1 + .../nodenetworkconfig/conversion.go | 1 + .../nodenetworkconfig/conversion_linux.go | 1 + cns/restserver/internalapi.go | 16 +++++++++------- cns/restserver/util.go | 1 - 5 files changed, 12 insertions(+), 8 deletions(-) diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index 8f5939c28e..cf1ff35a10 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -129,6 +129,7 @@ type CreateNetworkContainerRequest struct { EndpointPolicies []NetworkContainerRequestPolicies NCStatus v1alpha.NCStatus NetworkInterfaceInfo NetworkInterfaceInfo //nolint // introducing new field for backendnic, to be used later by cni code + Scenario v1alpha.NCType //nolint // introducing new field for nnc reconciler } func (req *CreateNetworkContainerRequest) Validate() error { diff --git a/cns/kubecontroller/nodenetworkconfig/conversion.go b/cns/kubecontroller/nodenetworkconfig/conversion.go index f2f3d9e9cf..cc34d682c8 100644 --- a/cns/kubecontroller/nodenetworkconfig/conversion.go +++ b/cns/kubecontroller/nodenetworkconfig/conversion.go @@ -67,6 +67,7 @@ func CreateNCRequestFromDynamicNC(nc v1alpha.NetworkContainer) (*cns.CreateNetwo GatewayIPAddress: nc.DefaultGateway, }, NCStatus: nc.Status, + Scenario: nc.Type, }, nil } diff --git a/cns/kubecontroller/nodenetworkconfig/conversion_linux.go b/cns/kubecontroller/nodenetworkconfig/conversion_linux.go index 9d425aa48f..dd9141dfc3 100644 --- a/cns/kubecontroller/nodenetworkconfig/conversion_linux.go +++ b/cns/kubecontroller/nodenetworkconfig/conversion_linux.go @@ -63,5 +63,6 @@ func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPre NetworkInterfaceInfo: cns.NetworkInterfaceInfo{ MACAddress: nc.MacAddress, }, + Scenario: nc.Type, }, nil } diff --git a/cns/restserver/internalapi.go b/cns/restserver/internalapi.go index efefb3f2d3..0e2635a09e 100644 --- a/cns/restserver/internalapi.go +++ b/cns/restserver/internalapi.go @@ -630,13 +630,15 @@ func (service *HTTPRestService) CreateOrUpdateNetworkContainerInternal(req *cns. if ok { existingReq := existingNCInfo.CreateNetworkContainerRequest if !reflect.DeepEqual(existingReq.IPConfiguration.IPSubnet, req.IPConfiguration.IPSubnet) { - logger.Errorf("[Azure CNS] Error. PrimaryCA is not same, NCId %s, old CA %s/%d, new CA %s/%d", - req.NetworkContainerid, - existingReq.IPConfiguration.IPSubnet.IPAddress, - existingReq.IPConfiguration.IPSubnet.PrefixLength, - req.IPConfiguration.IPSubnet.IPAddress, - req.IPConfiguration.IPSubnet.PrefixLength) - return types.PrimaryCANotSame + if req.Scenario != v1alpha.Overlay { // if overlay -> potentially an overlay subnet expansion is occuring, skip this check + logger.Errorf("[Azure CNS] Error. PrimaryCA is not same, NCId %s, old CA %s/%d, new CA %s/%d", + req.NetworkContainerid, + existingReq.IPConfiguration.IPSubnet.IPAddress, + existingReq.IPConfiguration.IPSubnet.PrefixLength, + req.IPConfiguration.IPSubnet.IPAddress, + req.IPConfiguration.IPSubnet.PrefixLength) + return types.PrimaryCANotSame + } } } diff --git a/cns/restserver/util.go b/cns/restserver/util.go index a84eb8cef0..bc47b34d4e 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -174,7 +174,6 @@ func (service *HTTPRestService) saveNetworkContainerGoalState(req cns.CreateNetw // Remove the auth token before saving the containerStatus to cns json file createNetworkContainerRequest := req createNetworkContainerRequest.AuthorizationToken = "" - service.state.ContainerStatus[req.NetworkContainerid] = containerstatus{ ID: req.NetworkContainerid, VMVersion: req.Version, From b317c4512e0e7900725d733e97d21bb91a5e84ee Mon Sep 17 00:00:00 2001 From: Riya Date: Wed, 15 Oct 2025 16:13:33 -0700 Subject: [PATCH 02/11] reverted a line change --- cns/restserver/internalapi.go | 1 + cns/restserver/util.go | 1 + 2 files changed, 2 insertions(+) diff --git a/cns/restserver/internalapi.go b/cns/restserver/internalapi.go index 0e2635a09e..ef8b1709c5 100644 --- a/cns/restserver/internalapi.go +++ b/cns/restserver/internalapi.go @@ -639,6 +639,7 @@ func (service *HTTPRestService) CreateOrUpdateNetworkContainerInternal(req *cns. req.IPConfiguration.IPSubnet.PrefixLength) return types.PrimaryCANotSame } + logger.Printf("[Azure CNS] Its an overlay cluster! Skipping Primary CA check for NCId %s, old CA %s/%d, new CA %s/%d") } } diff --git a/cns/restserver/util.go b/cns/restserver/util.go index bc47b34d4e..a84eb8cef0 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -174,6 +174,7 @@ func (service *HTTPRestService) saveNetworkContainerGoalState(req cns.CreateNetw // Remove the auth token before saving the containerStatus to cns json file createNetworkContainerRequest := req createNetworkContainerRequest.AuthorizationToken = "" + service.state.ContainerStatus[req.NetworkContainerid] = containerstatus{ ID: req.NetworkContainerid, VMVersion: req.Version, From a0b5a56870ac190d2cb7a203e2f27cc0845c22ea Mon Sep 17 00:00:00 2001 From: Riya Date: Wed, 15 Oct 2025 16:14:30 -0700 Subject: [PATCH 03/11] fixed spelling --- cns/restserver/internalapi.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cns/restserver/internalapi.go b/cns/restserver/internalapi.go index ef8b1709c5..c3275a09bf 100644 --- a/cns/restserver/internalapi.go +++ b/cns/restserver/internalapi.go @@ -630,7 +630,7 @@ func (service *HTTPRestService) CreateOrUpdateNetworkContainerInternal(req *cns. if ok { existingReq := existingNCInfo.CreateNetworkContainerRequest if !reflect.DeepEqual(existingReq.IPConfiguration.IPSubnet, req.IPConfiguration.IPSubnet) { - if req.Scenario != v1alpha.Overlay { // if overlay -> potentially an overlay subnet expansion is occuring, skip this check + if req.Scenario != v1alpha.Overlay { // if overlay -> potentially an overlay subnet expansion is occurring, skip this check logger.Errorf("[Azure CNS] Error. PrimaryCA is not same, NCId %s, old CA %s/%d, new CA %s/%d", req.NetworkContainerid, existingReq.IPConfiguration.IPSubnet.IPAddress, From 01ecd911c1c76e2916c377cf0739d3df5599a9f2 Mon Sep 17 00:00:00 2001 From: Riya Date: Wed, 15 Oct 2025 16:32:03 -0700 Subject: [PATCH 04/11] added unit test --- cns/restserver/internalapi_test.go | 45 ++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/cns/restserver/internalapi_test.go b/cns/restserver/internalapi_test.go index 4df797a498..de62874bd4 100644 --- a/cns/restserver/internalapi_test.go +++ b/cns/restserver/internalapi_test.go @@ -101,6 +101,51 @@ func TestReconcileNCStatePrimaryIPChangeShouldFail(t *testing.T) { assert.Equal(t, types.PrimaryCANotSame, resp) } +// TestReconcileNCStatePrimaryIPChangeShouldNotFail tests that reconciling NC state with +// a NC whose IP has changed should not fail +func TestReconcileNCStatePrimaryIPChangeShouldNotFail(t *testing.T) { + restartService() + setEnv(t) + setOrchestratorTypeInternal(cns.KubernetesCRD) + svc.state.ContainerStatus = make(map[string]containerstatus) + + // start with a NC in state + ncID := "555ac5c9-89f2-4b5d-b8d0-616894d6d151" + svc.state.ContainerStatus[ncID] = containerstatus{ + ID: ncID, + VMVersion: "0", + HostVersion: "0", + CreateNetworkContainerRequest: cns.CreateNetworkContainerRequest{ + NetworkContainerid: ncID, + IPConfiguration: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{ + IPAddress: "10.0.1.0", + PrefixLength: 24, + }, + }, + }, + } + + ncReqs := []*cns.CreateNetworkContainerRequest{ + { + NetworkContainerid: ncID, + IPConfiguration: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{ + IPAddress: "10.0.2.0", // note this IP has changed + PrefixLength: 24, + }, + }, + Scenario: v1alpha.Overlay, // overlay cluster - skip primary CA check + NetworkContainerType: cns.Kubernetes, + }, + } + + // now try to reconcile the state where the NC primary IP has changed + resp := svc.ReconcileIPAMStateForSwift(ncReqs, map[string]cns.PodInfo{}, &v1alpha.NodeNetworkConfig{}) + + assert.Equal(t, types.Success, resp) +} + // TestReconcileNCStateGatewayChange tests that NC state gets updated when reconciled // if the NC's gateway IP has changed func TestReconcileNCStateGatewayChange(t *testing.T) { From c853b6936d23a6b171bc3059ab806e5ec9bbeafb Mon Sep 17 00:00:00 2001 From: Riya Date: Thu, 16 Oct 2025 23:58:06 +0000 Subject: [PATCH 05/11] fixing go lint --- cns/restserver/internalapi.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/cns/restserver/internalapi.go b/cns/restserver/internalapi.go index c3275a09bf..b76ed5d01c 100644 --- a/cns/restserver/internalapi.go +++ b/cns/restserver/internalapi.go @@ -23,6 +23,7 @@ import ( "github.com/Azure/azure-container-networking/common" "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" "github.com/pkg/errors" + "go.uber.org/zap" ) const ( @@ -35,6 +36,9 @@ const ( // internal APIs (definde in internalapi.go). // This will be used internally (say by RequestController in case of AKS) +// Initialize a zap logger instance +var zapLogger, _ = zap.NewProduction() + // GetPartitionKey - Get dnc/service partition key func (service *HTTPRestService) GetPartitionKey() (dncPartitionKey string) { service.RLock() @@ -631,15 +635,12 @@ func (service *HTTPRestService) CreateOrUpdateNetworkContainerInternal(req *cns. existingReq := existingNCInfo.CreateNetworkContainerRequest if !reflect.DeepEqual(existingReq.IPConfiguration.IPSubnet, req.IPConfiguration.IPSubnet) { if req.Scenario != v1alpha.Overlay { // if overlay -> potentially an overlay subnet expansion is occurring, skip this check - logger.Errorf("[Azure CNS] Error. PrimaryCA is not same, NCId %s, old CA %s/%d, new CA %s/%d", - req.NetworkContainerid, - existingReq.IPConfiguration.IPSubnet.IPAddress, - existingReq.IPConfiguration.IPSubnet.PrefixLength, - req.IPConfiguration.IPSubnet.IPAddress, - req.IPConfiguration.IPSubnet.PrefixLength) + zapLogger.Error("[Azure CNS] Error. PrimaryCA is not same", + zap.String("NCId", req.NetworkContainerid), + zap.String("oldCA", fmt.Sprintf("%s/%d", existingReq.IPConfiguration.IPSubnet.IPAddress, existingReq.IPConfiguration.IPSubnet.PrefixLength)), + zap.String("newCA", fmt.Sprintf("%s/%d", req.IPConfiguration.IPSubnet.IPAddress, req.IPConfiguration.IPSubnet.PrefixLength))) return types.PrimaryCANotSame } - logger.Printf("[Azure CNS] Its an overlay cluster! Skipping Primary CA check for NCId %s, old CA %s/%d, new CA %s/%d") } } From d452d9ff820392c9b8ce30cef861e04e2b43c254 Mon Sep 17 00:00:00 2001 From: Riya Date: Fri, 17 Oct 2025 07:57:14 +0000 Subject: [PATCH 06/11] expanded on a comment --- cns/restserver/internalapi_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cns/restserver/internalapi_test.go b/cns/restserver/internalapi_test.go index de62874bd4..0b061a8a36 100644 --- a/cns/restserver/internalapi_test.go +++ b/cns/restserver/internalapi_test.go @@ -102,7 +102,7 @@ func TestReconcileNCStatePrimaryIPChangeShouldFail(t *testing.T) { } // TestReconcileNCStatePrimaryIPChangeShouldNotFail tests that reconciling NC state with -// a NC whose IP has changed should not fail +// a NC whose IP has changed should not fail for overlay clusters func TestReconcileNCStatePrimaryIPChangeShouldNotFail(t *testing.T) { restartService() setEnv(t) From b414cf5f93259b4608dcecd7bdb9cd212c387374 Mon Sep 17 00:00:00 2001 From: Riya Date: Fri, 17 Oct 2025 22:28:46 +0000 Subject: [PATCH 07/11] updated logic --- cns/NetworkContainerContract.go | 1 - .../nodenetworkconfig/conversion.go | 1 - .../nodenetworkconfig/conversion_linux.go | 1 - cns/restserver/internalapi.go | 54 +++++-- cns/restserver/internalapi_test.go | 138 +++++++++++------- 5 files changed, 128 insertions(+), 67 deletions(-) diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index cf1ff35a10..8f5939c28e 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -129,7 +129,6 @@ type CreateNetworkContainerRequest struct { EndpointPolicies []NetworkContainerRequestPolicies NCStatus v1alpha.NCStatus NetworkInterfaceInfo NetworkInterfaceInfo //nolint // introducing new field for backendnic, to be used later by cni code - Scenario v1alpha.NCType //nolint // introducing new field for nnc reconciler } func (req *CreateNetworkContainerRequest) Validate() error { diff --git a/cns/kubecontroller/nodenetworkconfig/conversion.go b/cns/kubecontroller/nodenetworkconfig/conversion.go index cc34d682c8..f2f3d9e9cf 100644 --- a/cns/kubecontroller/nodenetworkconfig/conversion.go +++ b/cns/kubecontroller/nodenetworkconfig/conversion.go @@ -67,7 +67,6 @@ func CreateNCRequestFromDynamicNC(nc v1alpha.NetworkContainer) (*cns.CreateNetwo GatewayIPAddress: nc.DefaultGateway, }, NCStatus: nc.Status, - Scenario: nc.Type, }, nil } diff --git a/cns/kubecontroller/nodenetworkconfig/conversion_linux.go b/cns/kubecontroller/nodenetworkconfig/conversion_linux.go index dd9141dfc3..9d425aa48f 100644 --- a/cns/kubecontroller/nodenetworkconfig/conversion_linux.go +++ b/cns/kubecontroller/nodenetworkconfig/conversion_linux.go @@ -63,6 +63,5 @@ func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPre NetworkInterfaceInfo: cns.NetworkInterfaceInfo{ MACAddress: nc.MacAddress, }, - Scenario: nc.Type, }, nil } diff --git a/cns/restserver/internalapi.go b/cns/restserver/internalapi.go index b76ed5d01c..1a0baecb5c 100644 --- a/cns/restserver/internalapi.go +++ b/cns/restserver/internalapi.go @@ -23,7 +23,6 @@ import ( "github.com/Azure/azure-container-networking/common" "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" "github.com/pkg/errors" - "go.uber.org/zap" ) const ( @@ -36,9 +35,6 @@ const ( // internal APIs (definde in internalapi.go). // This will be used internally (say by RequestController in case of AKS) -// Initialize a zap logger instance -var zapLogger, _ = zap.NewProduction() - // GetPartitionKey - Get dnc/service partition key func (service *HTTPRestService) GetPartitionKey() (dncPartitionKey string) { service.RLock() @@ -634,11 +630,15 @@ func (service *HTTPRestService) CreateOrUpdateNetworkContainerInternal(req *cns. if ok { existingReq := existingNCInfo.CreateNetworkContainerRequest if !reflect.DeepEqual(existingReq.IPConfiguration.IPSubnet, req.IPConfiguration.IPSubnet) { - if req.Scenario != v1alpha.Overlay { // if overlay -> potentially an overlay subnet expansion is occurring, skip this check - zapLogger.Error("[Azure CNS] Error. PrimaryCA is not same", - zap.String("NCId", req.NetworkContainerid), - zap.String("oldCA", fmt.Sprintf("%s/%d", existingReq.IPConfiguration.IPSubnet.IPAddress, existingReq.IPConfiguration.IPSubnet.PrefixLength)), - zap.String("newCA", fmt.Sprintf("%s/%d", req.IPConfiguration.IPSubnet.IPAddress, req.IPConfiguration.IPSubnet.PrefixLength))) + // check for potential overlay subnet expansion - checking if new subnet is a superset of old subnet + err := validateCIDRSuperset(req.IPConfiguration.IPSubnet.IPAddress, existingReq.IPConfiguration.IPSubnet.IPAddress) + if err != nil { + logger.Errorf("[Azure CNS] Error. PrimaryCA is not same, NCId %s, old CA %s/%d, new CA %s/%d", //nolint:staticcheck // Suppress SA1019: logger.Errorf is deprecated + req.NetworkContainerid, + existingReq.IPConfiguration.IPSubnet.IPAddress, + existingReq.IPConfiguration.IPSubnet.PrefixLength, + req.IPConfiguration.IPSubnet.IPAddress, + req.IPConfiguration.IPSubnet.PrefixLength) return types.PrimaryCANotSame } } @@ -726,3 +726,39 @@ func (service *HTTPRestService) GetIMDSNCs(ctx context.Context) (map[string]stri return ncs, nil } + +// IsCIDRSuperset returns true if newCIDR is a superset of oldCIDR (i.e., all IPs in oldCIDR are contained in newCIDR). +func validateCIDRSuperset(newCIDR, oldCIDR string) error { + _, newNet, err := net.ParseCIDR(newCIDR) + if err != nil { + return errors.Wrapf(err, "parsing newCIDR %q", newCIDR) + } + _, oldNet, err := net.ParseCIDR(oldCIDR) + if err != nil { + return errors.Wrapf(err, "parsing oldCIDR %q", oldCIDR) + } + + // Check that the network family matches (both IPv4 or both IPv6) + if len(newNet.IP) != len(oldNet.IP) { + return errors.New("CIDRs belong to different IP families") + } + + // Check that the old network's base IP is contained in the new network + if !newNet.Contains(oldNet.IP) { + return errors.New("old network's base IP is not contained in the new network") + } + + // Calculate the last IP in oldNet + oldLastIP := make(net.IP, len(oldNet.IP)) + for i := range oldNet.IP { + oldLastIP[i] = oldNet.IP[i] | ^oldNet.Mask[i] + } + + // Check that the last IP in oldNet is also contained in newNet + if !newNet.Contains(oldLastIP) { + return errors.New("last IP of old network is not contained in new network") + } + + // If both the first and last IPs of oldNet are in newNet, oldNet is fully contained in newNet + return nil +} diff --git a/cns/restserver/internalapi_test.go b/cns/restserver/internalapi_test.go index 0b061a8a36..50662a142b 100644 --- a/cns/restserver/internalapi_test.go +++ b/cns/restserver/internalapi_test.go @@ -66,84 +66,112 @@ func TestReconcileNCStatePrimaryIPChangeShouldFail(t *testing.T) { setOrchestratorTypeInternal(cns.KubernetesCRD) svc.state.ContainerStatus = make(map[string]containerstatus) - // start with a NC in state - ncID := "555ac5c9-89f2-4b5d-b8d0-616894d6d151" - svc.state.ContainerStatus[ncID] = containerstatus{ - ID: ncID, - VMVersion: "0", - HostVersion: "0", - CreateNetworkContainerRequest: cns.CreateNetworkContainerRequest{ - NetworkContainerid: ncID, - IPConfiguration: cns.IPConfiguration{ - IPSubnet: cns.IPSubnet{ - IPAddress: "10.0.1.0", - PrefixLength: 24, + testCases := []struct { + existingIPAddress string + requestIPAddress string + }{ + {"", "10.240.0.0/16"}, + {"10.240.0.0", "2001:db8::/64"}, + {"2001:db8::/64", "10.240.0.0/16"}, + {"10.0.1.0/22", "10.0.2.0/24"}, + {"10.0.1.0/21", "10.0.1.0/23"}, + {"10.0.1.0", "10.0.0.0/15"}, + {"10.0.1.0/15", "10.0.0.0"}, + } + + // Run test cases + for _, tc := range testCases { + // start with a NC in state + ncID := "555ac5c9-89f2-4b5d-b8d0-616894d6d151" + svc.state.ContainerStatus[ncID] = containerstatus{ + ID: ncID, + VMVersion: "0", + HostVersion: "0", + CreateNetworkContainerRequest: cns.CreateNetworkContainerRequest{ + NetworkContainerid: ncID, + IPConfiguration: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{ + IPAddress: tc.existingIPAddress, + PrefixLength: 24, + }, }, }, - }, - } + } - ncReqs := []*cns.CreateNetworkContainerRequest{ - { - NetworkContainerid: ncID, - IPConfiguration: cns.IPConfiguration{ - IPSubnet: cns.IPSubnet{ - IPAddress: "10.0.2.0", // note this IP has changed - PrefixLength: 24, + ncReqs := []*cns.CreateNetworkContainerRequest{ + { + NetworkContainerid: ncID, + IPConfiguration: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{ + IPAddress: tc.requestIPAddress, + PrefixLength: 24, + }, }, }, - }, - } + } - // now try to reconcile the state where the NC primary IP has changed - resp := svc.ReconcileIPAMStateForSwift(ncReqs, map[string]cns.PodInfo{}, &v1alpha.NodeNetworkConfig{}) + // now try to reconcile the state where the NC primary IP has changed + resp := svc.ReconcileIPAMStateForSwift(ncReqs, map[string]cns.PodInfo{}, &v1alpha.NodeNetworkConfig{}) + + assert.Equal(t, types.PrimaryCANotSame, resp) + } - assert.Equal(t, types.PrimaryCANotSame, resp) } // TestReconcileNCStatePrimaryIPChangeShouldNotFail tests that reconciling NC state with -// a NC whose IP has changed should not fail for overlay clusters +// a NC whose IP has changed should not fail if new IP is superset of old IP func TestReconcileNCStatePrimaryIPChangeShouldNotFail(t *testing.T) { restartService() setEnv(t) setOrchestratorTypeInternal(cns.KubernetesCRD) svc.state.ContainerStatus = make(map[string]containerstatus) - // start with a NC in state - ncID := "555ac5c9-89f2-4b5d-b8d0-616894d6d151" - svc.state.ContainerStatus[ncID] = containerstatus{ - ID: ncID, - VMVersion: "0", - HostVersion: "0", - CreateNetworkContainerRequest: cns.CreateNetworkContainerRequest{ - NetworkContainerid: ncID, - IPConfiguration: cns.IPConfiguration{ - IPSubnet: cns.IPSubnet{ - IPAddress: "10.0.1.0", - PrefixLength: 24, + testCases := []struct { + existingIPAddress string + requestIPAddress string + }{ + {"10.0.1.0/24", "10.0.2.0/22"}, + {"10.0.1.0/20", "10.0.1.0/18"}, + {"10.0.1.0/19", "10.0.0.0/15"}, + } + + // Run test cases + for _, tc := range testCases { + // start with a NC in state + ncID := "555ac5c9-89f2-4b5d-b8d0-616894d6d151" + svc.state.ContainerStatus[ncID] = containerstatus{ + ID: ncID, + VMVersion: "0", + HostVersion: "0", + CreateNetworkContainerRequest: cns.CreateNetworkContainerRequest{ + NetworkContainerid: ncID, + IPConfiguration: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{ + IPAddress: tc.existingIPAddress, + PrefixLength: 24, + }, }, }, - }, - } + } - ncReqs := []*cns.CreateNetworkContainerRequest{ - { - NetworkContainerid: ncID, - IPConfiguration: cns.IPConfiguration{ - IPSubnet: cns.IPSubnet{ - IPAddress: "10.0.2.0", // note this IP has changed - PrefixLength: 24, + ncReqs := []*cns.CreateNetworkContainerRequest{ + { + NetworkContainerid: ncID, + IPConfiguration: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{ + IPAddress: tc.requestIPAddress, + PrefixLength: 24, + }, }, + NetworkContainerType: cns.Kubernetes, }, - Scenario: v1alpha.Overlay, // overlay cluster - skip primary CA check - NetworkContainerType: cns.Kubernetes, - }, - } + } - // now try to reconcile the state where the NC primary IP has changed - resp := svc.ReconcileIPAMStateForSwift(ncReqs, map[string]cns.PodInfo{}, &v1alpha.NodeNetworkConfig{}) + // now try to reconcile the state where the NC primary IP has changed + resp := svc.ReconcileIPAMStateForSwift(ncReqs, map[string]cns.PodInfo{}, &v1alpha.NodeNetworkConfig{}) - assert.Equal(t, types.Success, resp) + assert.Equal(t, types.Success, resp) + } } // TestReconcileNCStateGatewayChange tests that NC state gets updated when reconciled From e3dd026a3dd65ce5ca609cf63faf67b780606070 Mon Sep 17 00:00:00 2001 From: Riya Date: Fri, 17 Oct 2025 22:41:45 +0000 Subject: [PATCH 08/11] updated test --- cns/restserver/internalapi_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cns/restserver/internalapi_test.go b/cns/restserver/internalapi_test.go index 50662a142b..6700c71348 100644 --- a/cns/restserver/internalapi_test.go +++ b/cns/restserver/internalapi_test.go @@ -82,7 +82,7 @@ func TestReconcileNCStatePrimaryIPChangeShouldFail(t *testing.T) { // Run test cases for _, tc := range testCases { // start with a NC in state - ncID := "555ac5c9-89f2-4b5d-b8d0-616894d6d151" + ncID := "555ac5c9-89f2-4b5d-b8d0-616894d6d150" svc.state.ContainerStatus[ncID] = containerstatus{ ID: ncID, VMVersion: "0", @@ -138,7 +138,7 @@ func TestReconcileNCStatePrimaryIPChangeShouldNotFail(t *testing.T) { // Run test cases for _, tc := range testCases { // start with a NC in state - ncID := "555ac5c9-89f2-4b5d-b8d0-616894d6d151" + ncID := "555ac5c9-89f2-4b5d-b8d0-616894d6d150" svc.state.ContainerStatus[ncID] = containerstatus{ ID: ncID, VMVersion: "0", From 59441be37b05324707eab509683372d1fb98c96a Mon Sep 17 00:00:00 2001 From: Riya Date: Fri, 17 Oct 2025 23:18:56 +0000 Subject: [PATCH 09/11] updated validate superset logic --- cns/restserver/internalapi.go | 31 +++++++++++-------------------- 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/cns/restserver/internalapi.go b/cns/restserver/internalapi.go index 1a0baecb5c..658f6ccefe 100644 --- a/cns/restserver/internalapi.go +++ b/cns/restserver/internalapi.go @@ -11,6 +11,7 @@ import ( "net" "net/http" "net/http/httptest" + "net/netip" "reflect" "strconv" "strings" @@ -729,36 +730,26 @@ func (service *HTTPRestService) GetIMDSNCs(ctx context.Context) (map[string]stri // IsCIDRSuperset returns true if newCIDR is a superset of oldCIDR (i.e., all IPs in oldCIDR are contained in newCIDR). func validateCIDRSuperset(newCIDR, oldCIDR string) error { - _, newNet, err := net.ParseCIDR(newCIDR) + // Parse newCIDR and oldCIDR into netip.Prefix + newPrefix, err := netip.ParsePrefix(newCIDR) if err != nil { return errors.Wrapf(err, "parsing newCIDR %q", newCIDR) } - _, oldNet, err := net.ParseCIDR(oldCIDR) + + oldPrefix, err := netip.ParsePrefix(oldCIDR) if err != nil { return errors.Wrapf(err, "parsing oldCIDR %q", oldCIDR) } - // Check that the network family matches (both IPv4 or both IPv6) - if len(newNet.IP) != len(oldNet.IP) { - return errors.New("CIDRs belong to different IP families") - } - - // Check that the old network's base IP is contained in the new network - if !newNet.Contains(oldNet.IP) { - return errors.New("old network's base IP is not contained in the new network") - } - - // Calculate the last IP in oldNet - oldLastIP := make(net.IP, len(oldNet.IP)) - for i := range oldNet.IP { - oldLastIP[i] = oldNet.IP[i] | ^oldNet.Mask[i] + // Condition 1: Check if the new prefix length is smaller (larger range) than the old prefix length + if newPrefix.Bits() >= oldPrefix.Bits() { + return errors.New("newCIDR does not have a larger range than oldCIDR") } - // Check that the last IP in oldNet is also contained in newNet - if !newNet.Contains(oldLastIP) { - return errors.New("last IP of old network is not contained in new network") + // Condition 2: Check if the base IP of oldCIDR is contained in newCIDR + if !newPrefix.Contains(oldPrefix.Addr()) { + return errors.New("old subnet's base IP is not contained in new subnet") } - // If both the first and last IPs of oldNet are in newNet, oldNet is fully contained in newNet return nil } From b05635818a370d832f903685231ce0c01403b929 Mon Sep 17 00:00:00 2001 From: Riya Date: Fri, 17 Oct 2025 23:33:31 +0000 Subject: [PATCH 10/11] updated to return bool instead of error for checking cidr superset --- cns/restserver/internalapi.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/cns/restserver/internalapi.go b/cns/restserver/internalapi.go index 658f6ccefe..d291082a35 100644 --- a/cns/restserver/internalapi.go +++ b/cns/restserver/internalapi.go @@ -632,8 +632,8 @@ func (service *HTTPRestService) CreateOrUpdateNetworkContainerInternal(req *cns. existingReq := existingNCInfo.CreateNetworkContainerRequest if !reflect.DeepEqual(existingReq.IPConfiguration.IPSubnet, req.IPConfiguration.IPSubnet) { // check for potential overlay subnet expansion - checking if new subnet is a superset of old subnet - err := validateCIDRSuperset(req.IPConfiguration.IPSubnet.IPAddress, existingReq.IPConfiguration.IPSubnet.IPAddress) - if err != nil { + isCIDRSuperset := validateCIDRSuperset(req.IPConfiguration.IPSubnet.IPAddress, existingReq.IPConfiguration.IPSubnet.IPAddress) + if !isCIDRSuperset { logger.Errorf("[Azure CNS] Error. PrimaryCA is not same, NCId %s, old CA %s/%d, new CA %s/%d", //nolint:staticcheck // Suppress SA1019: logger.Errorf is deprecated req.NetworkContainerid, existingReq.IPConfiguration.IPSubnet.IPAddress, @@ -729,27 +729,27 @@ func (service *HTTPRestService) GetIMDSNCs(ctx context.Context) (map[string]stri } // IsCIDRSuperset returns true if newCIDR is a superset of oldCIDR (i.e., all IPs in oldCIDR are contained in newCIDR). -func validateCIDRSuperset(newCIDR, oldCIDR string) error { +func validateCIDRSuperset(newCIDR, oldCIDR string) bool { // Parse newCIDR and oldCIDR into netip.Prefix newPrefix, err := netip.ParsePrefix(newCIDR) if err != nil { - return errors.Wrapf(err, "parsing newCIDR %q", newCIDR) + return false } oldPrefix, err := netip.ParsePrefix(oldCIDR) if err != nil { - return errors.Wrapf(err, "parsing oldCIDR %q", oldCIDR) + return false } // Condition 1: Check if the new prefix length is smaller (larger range) than the old prefix length if newPrefix.Bits() >= oldPrefix.Bits() { - return errors.New("newCIDR does not have a larger range than oldCIDR") + return false } // Condition 2: Check if the base IP of oldCIDR is contained in newCIDR if !newPrefix.Contains(oldPrefix.Addr()) { - return errors.New("old subnet's base IP is not contained in new subnet") + return false } - return nil + return true } From e704062ff58f927f112ddc2f069dff6bdae10692 Mon Sep 17 00:00:00 2001 From: Riya Date: Mon, 20 Oct 2025 08:01:15 +0000 Subject: [PATCH 11/11] updated logic to check for containment --- cns/restserver/internalapi.go | 4 ++-- cns/restserver/internalapi_test.go | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/cns/restserver/internalapi.go b/cns/restserver/internalapi.go index d291082a35..de52c3e050 100644 --- a/cns/restserver/internalapi.go +++ b/cns/restserver/internalapi.go @@ -746,8 +746,8 @@ func validateCIDRSuperset(newCIDR, oldCIDR string) bool { return false } - // Condition 2: Check if the base IP of oldCIDR is contained in newCIDR - if !newPrefix.Contains(oldPrefix.Addr()) { + // Condition 2: Check for Overlap - this will also ensure containment + if !newPrefix.Overlaps(oldPrefix) { return false } diff --git a/cns/restserver/internalapi_test.go b/cns/restserver/internalapi_test.go index 6700c71348..81ccc85154 100644 --- a/cns/restserver/internalapi_test.go +++ b/cns/restserver/internalapi_test.go @@ -133,6 +133,7 @@ func TestReconcileNCStatePrimaryIPChangeShouldNotFail(t *testing.T) { {"10.0.1.0/24", "10.0.2.0/22"}, {"10.0.1.0/20", "10.0.1.0/18"}, {"10.0.1.0/19", "10.0.0.0/15"}, + {"10.0.1.0/18", "10.0.1.0/18"}, } // Run test cases