From 44f9b4feef677f93e535d6dfc5aafdb9c89c2f85 Mon Sep 17 00:00:00 2001 From: "Saksham Mittal (from Dev Box)" Date: Tue, 28 Jul 2026 10:37:44 -0700 Subject: [PATCH 01/10] initial put/delete NC and subnet join related changes for RNC via NMA --- cns/NetworkContainerContract.go | 10 +-- cns/fakes/wireserverproxyfake.go | 27 +++++--- cns/restserver/api.go | 109 ++++++++++++++++++++++++++++--- cns/restserver/const.go | 1 + cns/restserver/restserver.go | 8 ++- cns/restserver/util.go | 17 +++++ cns/types/codes.go | 3 + cns/wireserver/proxy.go | 49 ++++++++++++-- nmagent/requests.go | 47 +++++++------ 9 files changed, 220 insertions(+), 51 deletions(-) diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index 647e4ba97dd..6f2f11fdfb0 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -675,6 +675,7 @@ type NetworkInterface struct { // PublishNetworkContainerRequest specifies request to publish network container via NMAgent. type PublishNetworkContainerRequest struct { NetworkID string + SubnetName string NetworkContainerID string JoinNetworkURL string CreateNetworkContainerURL string @@ -683,8 +684,8 @@ type PublishNetworkContainerRequest struct { func (p PublishNetworkContainerRequest) String() string { // %q as a verb on a byte slice prints safely escaped text instead of individual bytes - return fmt.Sprintf("{NetworkID:%s NetworkContainerID:%s JoinNetworkURL:%s CreateNetworkContainerURL:%s CreateNetworkContainerRequestBody:%q}", - p.NetworkID, p.NetworkContainerID, p.JoinNetworkURL, p.CreateNetworkContainerURL, p.CreateNetworkContainerRequestBody) + return fmt.Sprintf("{NetworkID:%s SubnetName:%s NetworkContainerID:%s JoinNetworkURL:%s CreateNetworkContainerURL:%s CreateNetworkContainerRequestBody:%q}", + p.NetworkID, p.SubnetName, p.NetworkContainerID, p.JoinNetworkURL, p.CreateNetworkContainerURL, p.CreateNetworkContainerRequestBody) } // NetworkContainerParameters parameters available in network container operations @@ -711,6 +712,7 @@ func (p PublishNetworkContainerResponse) String() string { // UnpublishNetworkContainerRequest specifies request to unpublish network container via NMAgent. type UnpublishNetworkContainerRequest struct { NetworkID string + SubnetName string NetworkContainerID string JoinNetworkURL string DeleteNetworkContainerURL string @@ -718,8 +720,8 @@ type UnpublishNetworkContainerRequest struct { } func (u UnpublishNetworkContainerRequest) String() string { - return fmt.Sprintf("{NetworkID:%s NetworkContainerID:%s JoinNetworkURL:%s DeleteNetworkContainerURL:%s DeleteNetworkContainerRequestBody:%q}", - u.NetworkID, u.NetworkContainerID, u.JoinNetworkURL, u.DeleteNetworkContainerURL, u.DeleteNetworkContainerRequestBody) + return fmt.Sprintf("{NetworkID:%s SubnetName:%s NetworkContainerID:%s JoinNetworkURL:%s DeleteNetworkContainerURL:%s DeleteNetworkContainerRequestBody:%q}", + u.NetworkID, u.SubnetName, u.NetworkContainerID, u.JoinNetworkURL, u.DeleteNetworkContainerURL, u.DeleteNetworkContainerRequestBody) } // UnpublishNetworkContainerResponse specifies the response to unpublish network container request. diff --git a/cns/fakes/wireserverproxyfake.go b/cns/fakes/wireserverproxyfake.go index e6e52e7c54d..01b20f33e45 100644 --- a/cns/fakes/wireserverproxyfake.go +++ b/cns/fakes/wireserverproxyfake.go @@ -10,9 +10,10 @@ import ( ) type WireserverProxyFake struct { - JoinNetworkFunc func(context.Context, string) (*http.Response, error) - PublishNCFunc func(context.Context, cns.NetworkContainerParameters, []byte) (*http.Response, error) - UnpublishNCFunc func(context.Context, cns.NetworkContainerParameters, []byte) (*http.Response, error) + JoinNetworkFunc func(context.Context, string, bool) (*http.Response, error) + JoinSubnetFunc func(context.Context, string, string, cns.NetworkContainerParameters) (*http.Response, error) + PublishNCFunc func(context.Context, cns.NetworkContainerParameters, []byte, bool) (*http.Response, error) + UnpublishNCFunc func(context.Context, cns.NetworkContainerParameters, []byte, bool) (*http.Response, error) } const defaultResponseBody = `{"httpStatusCode":"200"}` @@ -25,25 +26,33 @@ func defaultResponse() *http.Response { } } -func (w *WireserverProxyFake) JoinNetwork(ctx context.Context, vnetID string) (*http.Response, error) { +func (w *WireserverProxyFake) JoinNetwork(ctx context.Context, vnetID string, useRNCPublisher bool) (*http.Response, error) { if w.JoinNetworkFunc != nil { - return w.JoinNetworkFunc(ctx, vnetID) + return w.JoinNetworkFunc(ctx, vnetID, useRNCPublisher) } return defaultResponse(), nil } -func (w *WireserverProxyFake) PublishNC(ctx context.Context, ncParams cns.NetworkContainerParameters, payload []byte) (*http.Response, error) { +func (w *WireserverProxyFake) JoinSubnet(ctx context.Context, vnetID, subnetName string, ncParams cns.NetworkContainerParameters) (*http.Response, error) { + if w.JoinSubnetFunc != nil { + return w.JoinSubnetFunc(ctx, vnetID, subnetName, ncParams) + } + + return defaultResponse(), nil +} + +func (w *WireserverProxyFake) PublishNC(ctx context.Context, ncParams cns.NetworkContainerParameters, payload []byte, useRNCPublisher bool) (*http.Response, error) { if w.PublishNCFunc != nil { - return w.PublishNCFunc(ctx, ncParams, payload) + return w.PublishNCFunc(ctx, ncParams, payload, useRNCPublisher) } return defaultResponse(), nil } -func (w *WireserverProxyFake) UnpublishNC(ctx context.Context, ncParams cns.NetworkContainerParameters, payload []byte) (*http.Response, error) { +func (w *WireserverProxyFake) UnpublishNC(ctx context.Context, ncParams cns.NetworkContainerParameters, payload []byte, useRNCPublisher bool) (*http.Response, error) { if w.UnpublishNCFunc != nil { - return w.UnpublishNCFunc(ctx, ncParams, payload) + return w.UnpublishNCFunc(ctx, ncParams, payload, useRNCPublisher) } return defaultResponse(), nil diff --git a/cns/restserver/api.go b/cns/restserver/api.go index ac5ba5994ce..99488fab002 100644 --- a/cns/restserver/api.go +++ b/cns/restserver/api.go @@ -948,7 +948,20 @@ func (service *HTTPRestService) publishNetworkContainer(w http.ResponseWriter, r ctx := r.Context() - joinResp, err := service.wsproxy.JoinNetwork(ctx, req.NetworkID) //nolint:govet // ok to shadow + var publishBody nmagent.PutNetworkContainerRequest + var useRNCPublisher bool + + err = json.Unmarshal(req.CreateNetworkContainerRequestBody, &publishBody) + if err != nil { + http.Error(w, fmt.Sprintf("could not unmarshal create network container body: %v", err), http.StatusBadRequest) + return + } + + if publishBody.UseRNCPublisher { + useRNCPublisher = true + } + + joinResp, err := service.wsproxy.JoinNetwork(ctx, req.NetworkID, useRNCPublisher) //nolint:govet // ok to shadow if err != nil { resp := cns.PublishNetworkContainerResponse{ Response: cns.Response{ @@ -982,7 +995,43 @@ func (service *HTTPRestService) publishNetworkContainer(w http.ResponseWriter, r service.setNetworkStateJoined(req.NetworkID) logger.Printf("[Azure-CNS] joined vnet %s during nc %s publish. wireserver response: %v", req.NetworkID, req.NetworkContainerID, string(joinBytes)) - publishResp, err := service.wsproxy.PublishNC(ctx, ncParams, req.CreateNetworkContainerRequestBody) + if useRNCPublisher { + joinSubnetResp, errSubnetJoin := service.wsproxy.JoinSubnet(ctx, req.NetworkID, req.SubnetName, ncParams) //nolint:govet // ok to shadow + if errSubnetJoin != nil { + resp := cns.PublishNetworkContainerResponse{ + Response: cns.Response{ + ReturnCode: types.SubnetJoinFailed, + Message: fmt.Sprintf("failed to join subnet %s in network %s: %v", req.SubnetName, req.NetworkID, err), + }, + PublishErrorStr: errSubnetJoin.Error(), + } + respondJSON(w, http.StatusOK, resp) // legacy behavior + logger.Response(service.Name, resp, resp.Response.ReturnCode, errSubnetJoin) + return + } + + subnetJoinBytes, _ := io.ReadAll(joinSubnetResp.Body) + _ = joinSubnetResp.Body.Close() + + if joinSubnetResp.StatusCode != http.StatusOK { + resp := cns.PublishNetworkContainerResponse{ + Response: cns.Response{ + ReturnCode: types.SubnetJoinFailed, + Message: fmt.Sprintf("failed to join subnet %s in network %s. did not get 200 from wireserver", req.SubnetName, req.NetworkID), + }, + PublishStatusCode: joinSubnetResp.StatusCode, + PublishResponseBody: subnetJoinBytes, + } + respondJSON(w, http.StatusOK, resp) // legacy behavior + logger.Response(service.Name, resp, resp.Response.ReturnCode, nil) + return + } + + service.setSubnetStateJoined(req.NetworkID, req.SubnetName) + logger.Printf("[Azure-CNS] joined subnet %s in vnet %s during nc %s publish. wireserver response: %v", req.SubnetName, req.NetworkID, req.NetworkContainerID, string(subnetJoinBytes)) + } + + publishResp, err := service.wsproxy.PublishNC(ctx, ncParams, req.CreateNetworkContainerRequestBody, useRNCPublisher) if err != nil { resp := cns.PublishNetworkContainerResponse{ Response: cns.Response{ @@ -1046,6 +1095,7 @@ func (service *HTTPRestService) unpublishNetworkContainer(w http.ResponseWriter, var unpublishBody nmagent.DeleteContainerRequest var azrNC bool + var useRNCPublisher bool err = json.Unmarshal(req.DeleteNetworkContainerRequestBody, &unpublishBody) if err != nil { // If the body contains only `""\n`, it is non-AZR NC @@ -1059,6 +1109,9 @@ func (service *HTTPRestService) unpublishNetworkContainer(w http.ResponseWriter, } else { // If unmarshalling was successful, it is an AZR NC azrNC = true + if unpublishBody.UseRNCPublisher { + useRNCPublisher = true + } } /* For AZR scenarios, if NMAgent is restarted, it loses state and does not know what VNETs to subscribe to. @@ -1066,7 +1119,7 @@ func (service *HTTPRestService) unpublishNetworkContainer(w http.ResponseWriter, nc unpublish calls just like publish nc calls. */ if azrNC || !service.isNetworkJoined(req.NetworkID) { - joinResp, err := service.wsproxy.JoinNetwork(ctx, req.NetworkID) //nolint:govet // ok to shadow + joinResp, err := service.wsproxy.JoinNetwork(ctx, req.NetworkID, useRNCPublisher) //nolint:govet // ok to shadow if err != nil { resp := cns.UnpublishNetworkContainerResponse{ Response: cns.Response{ @@ -1101,7 +1154,45 @@ func (service *HTTPRestService) unpublishNetworkContainer(w http.ResponseWriter, logger.Printf("[Azure-CNS] joined vnet %s during nc %s unpublish. AZREnabled: %t, wireserver response: %v", req.NetworkID, req.NetworkContainerID, unpublishBody.AZREnabled, string(joinBytes)) } - publishResp, err := service.wsproxy.UnpublishNC(ctx, ncParams, req.DeleteNetworkContainerRequestBody) + if useRNCPublisher { + if !service.isSubnetJoined(req.NetworkID, req.SubnetName) { + joinSubnetResp, err := service.wsproxy.JoinSubnet(ctx, req.NetworkID, req.SubnetName, ncParams) //nolint:govet // ok to shadow + if err != nil { + resp := cns.UnpublishNetworkContainerResponse{ + Response: cns.Response{ + ReturnCode: types.SubnetJoinFailed, + Message: fmt.Sprintf("failed to join subnet %s in network %s: %v", req.SubnetName, req.NetworkID, err), + }, + UnpublishErrorStr: err.Error(), + } + respondJSON(w, http.StatusOK, resp) // legacy behavior + logger.Response(service.Name, resp, resp.Response.ReturnCode, err) + return + } + + subnetJoinBytes, _ := io.ReadAll(joinSubnetResp.Body) + _ = joinSubnetResp.Body.Close() + + if joinSubnetResp.StatusCode != http.StatusOK { + resp := cns.UnpublishNetworkContainerResponse{ + Response: cns.Response{ + ReturnCode: types.SubnetJoinFailed, + Message: fmt.Sprintf("failed to join subnet %s in network %s. did not get 200 from wireserver", req.SubnetName, req.NetworkID), + }, + UnpublishStatusCode: joinSubnetResp.StatusCode, + UnpublishResponseBody: subnetJoinBytes, + } + respondJSON(w, http.StatusOK, resp) // legacy behavior + logger.Response(service.Name, resp, resp.Response.ReturnCode, nil) + return + } + + service.setSubnetStateJoined(req.NetworkID, req.SubnetName) + logger.Printf("[Azure-CNS] joined subnet %s in vnet %s during nc %s unpublish. AZREnabled: %t, wireserver response: %v", req.SubnetName, req.NetworkID, req.NetworkContainerID, unpublishBody.AZREnabled, string(subnetJoinBytes)) + } + } + + unpublishResp, err := service.wsproxy.UnpublishNC(ctx, ncParams, req.DeleteNetworkContainerRequestBody, useRNCPublisher) if err != nil { resp := cns.UnpublishNetworkContainerResponse{ Response: cns.Response{ @@ -1115,15 +1206,15 @@ func (service *HTTPRestService) unpublishNetworkContainer(w http.ResponseWriter, return } - publishBytes, _ := io.ReadAll(publishResp.Body) - _ = publishResp.Body.Close() + unpublishBytes, _ := io.ReadAll(unpublishResp.Body) + _ = unpublishResp.Body.Close() resp := cns.UnpublishNetworkContainerResponse{ - UnpublishStatusCode: publishResp.StatusCode, - UnpublishResponseBody: publishBytes, + UnpublishStatusCode: unpublishResp.StatusCode, + UnpublishResponseBody: unpublishBytes, } - if publishResp.StatusCode != http.StatusOK { + if unpublishResp.StatusCode != http.StatusOK { resp.Response = cns.Response{ ReturnCode: types.NetworkContainerUnpublishFailed, Message: fmt.Sprintf("failed to unpublish nc %s. did not get 200 from wireserver", req.NetworkContainerID), diff --git a/cns/restserver/const.go b/cns/restserver/const.go index 0782755596d..3b4b0d77155 100644 --- a/cns/restserver/const.go +++ b/cns/restserver/const.go @@ -10,6 +10,7 @@ const ( detach = "Detach" // Rest service state identifier for named lock stateJoinedNetworks = "JoinedNetworks" + stateJoinedSubnets = "JoinedSubnets" dncApiVersion = "?api-version=2018-03-01" nmaAPICallTimeout = 2 * time.Second ) diff --git a/cns/restserver/restserver.go b/cns/restserver/restserver.go index ec0f3827482..4260dc2d574 100644 --- a/cns/restserver/restserver.go +++ b/cns/restserver/restserver.go @@ -47,9 +47,10 @@ type nmagentClient interface { } type wireserverProxy interface { - JoinNetwork(ctx context.Context, vnetID string) (*http.Response, error) - PublishNC(ctx context.Context, ncParams cns.NetworkContainerParameters, payload []byte) (*http.Response, error) - UnpublishNC(ctx context.Context, ncParams cns.NetworkContainerParameters, payload []byte) (*http.Response, error) + JoinNetwork(ctx context.Context, vnetID string, useRNCPublisher bool) (*http.Response, error) + JoinSubnet(ctx context.Context, vnetID, subnetName string, ncParams cns.NetworkContainerParameters) (*http.Response, error) + PublishNC(ctx context.Context, ncParams cns.NetworkContainerParameters, payload []byte, useRNCPublisher bool) (*http.Response, error) + UnpublishNC(ctx context.Context, ncParams cns.NetworkContainerParameters, payload []byte, useRNCPublisher bool) (*http.Response, error) } type imdsClient interface { @@ -200,6 +201,7 @@ type httpRestServiceState struct { Networks map[string]*networkInfo TimeStamp time.Time joinedNetworks map[string]struct{} + joinedSubnets map[string]struct{} primaryInterface *wireserver.InterfaceInfo PnpIDByMacAddress map[string]string } diff --git a/cns/restserver/util.go b/cns/restserver/util.go index f1e39ed2e07..5dcfe563ec6 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -742,6 +742,15 @@ func (service *HTTPRestService) isNetworkJoined(networkID string) bool { return exists } +// Check if the subnet is joined +func (service *HTTPRestService) isSubnetJoined(vnetID, subnetName string) bool { + namedLock.LockAcquire(stateJoinedSubnets) + defer namedLock.LockRelease(stateJoinedSubnets) + subnetKey := vnetID + "_" + subnetName + _, exists := service.state.joinedSubnets[subnetKey] + return exists +} + // Set the network as joined func (service *HTTPRestService) setNetworkStateJoined(networkID string) { namedLock.LockAcquire(stateJoinedNetworks) @@ -749,6 +758,14 @@ func (service *HTTPRestService) setNetworkStateJoined(networkID string) { service.state.joinedNetworks[networkID] = struct{}{} } +// Set the subnet as joined +func (service *HTTPRestService) setSubnetStateJoined(vnetID, subnetName string) { + namedLock.LockAcquire(stateJoinedSubnets) + defer namedLock.LockRelease(stateJoinedSubnets) + subnetKey := vnetID + "_" + subnetName + service.state.joinedSubnets[subnetKey] = struct{}{} +} + func logNCSnapshot(createNetworkContainerRequest cns.CreateNetworkContainerRequest) { aiEvent := aitelemetry.Event{ EventName: logger.CnsNCSnapshotEventStr, diff --git a/cns/types/codes.go b/cns/types/codes.go index 9492e92a596..98389b70a7e 100644 --- a/cns/types/codes.go +++ b/cns/types/codes.go @@ -47,6 +47,7 @@ const ( ConnectionError ResponseCode = 45 UnexpectedError ResponseCode = 99 NmAgentNCVersionListError ResponseCode = 100 + SubnetJoinFailed ResponseCode = 101 ) // nolint:gocyclo @@ -128,6 +129,8 @@ func (c ResponseCode) String() string { return "StatusUnauthorized" case FailedToAllocateBackendConfig: return "FailedToAllocateBackendConfig" + case SubnetJoinFailed: + return "SubnetJoinFailed" default: return "UnknownError" } diff --git a/cns/wireserver/proxy.go b/cns/wireserver/proxy.go index d180ff9cccc..e0e714c5b67 100644 --- a/cns/wireserver/proxy.go +++ b/cns/wireserver/proxy.go @@ -12,6 +12,7 @@ import ( const ( joinNetworkURLFmt = `http://%s/machine/plugins/?comp=nmagent&type=NetworkManagement/joinedVirtualNetworks/%s/api-version/1` + joinSubnetURLFmt = `http://%s/machine/plugins/?comp=nmagent&type=NetworkManagement/joinedVirtualNetworks/%s/joinedSubnets/%s/authenticationToken/%s/api-version/1?useLegacyChannel=false` publishNCURLFmt = `http://%s/machine/plugins/?comp=nmagent&type=NetworkManagement/interfaces/%s/networkContainers/%s/authenticationToken/%s/api-version/1` unpublishNCURLFmt = `http://%s/machine/plugins/?comp=nmagent&type=NetworkManagement/interfaces/%s/networkContainers/%s/authenticationToken/%s/api-version/1/method/DELETE` ) @@ -21,8 +22,14 @@ type Proxy struct { HTTPClient do } -func (p *Proxy) JoinNetwork(ctx context.Context, vnetID string) (*http.Response, error) { - reqURL := fmt.Sprintf(joinNetworkURLFmt, p.Host, vnetID) +func (p *Proxy) JoinNetwork(ctx context.Context, vnetID string, useRNCPublisher bool) (*http.Response, error) { + var joinNetworkURLFormat string + if useRNCPublisher { + joinNetworkURLFormat = joinNetworkURLFmt + "?useLegacyChannel=false" + } else { + joinNetworkURLFormat = joinNetworkURLFmt + } + reqURL := fmt.Sprintf(joinNetworkURLFormat, p.Host, vnetID) req, err := http.NewRequestWithContext(ctx, http.MethodPost, reqURL, bytes.NewBufferString(`""`)) if err != nil { @@ -39,8 +46,32 @@ func (p *Proxy) JoinNetwork(ctx context.Context, vnetID string) (*http.Response, return resp, nil } -func (p *Proxy) PublishNC(ctx context.Context, ncParams cns.NetworkContainerParameters, payload []byte) (*http.Response, error) { - reqURL := fmt.Sprintf(publishNCURLFmt, p.Host, ncParams.AssociatedInterfaceID, ncParams.NCID, ncParams.AuthToken) +func (p *Proxy) JoinSubnet(ctx context.Context, vnetID, subnetName string, ncParams cns.NetworkContainerParameters) (*http.Response, error) { + reqURL := fmt.Sprintf(joinSubnetURLFmt, p.Host, vnetID, subnetName, ncParams.AuthToken) + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, reqURL, bytes.NewBufferString(`""`)) + if err != nil { + return nil, errors.Wrap(err, "wireserver proxy: join subnet: could not build http request") + } + + req.Header.Set("Content-Type", "application/json") + + resp, err := p.HTTPClient.Do(req) + if err != nil { + return nil, errors.Wrap(err, "wireserver proxy: join subnet: could not perform http request") + } + + return resp, nil +} + +func (p *Proxy) PublishNC(ctx context.Context, ncParams cns.NetworkContainerParameters, payload []byte, useRNCPublisher bool) (*http.Response, error) { + var publishNCURLFormat string + if useRNCPublisher { + publishNCURLFormat = publishNCURLFmt + "?useLegacyChannel=false" + } else { + publishNCURLFormat = publishNCURLFmt + } + reqURL := fmt.Sprintf(publishNCURLFormat, p.Host, ncParams.AssociatedInterfaceID, ncParams.NCID, ncParams.AuthToken) req, err := http.NewRequestWithContext(ctx, http.MethodPost, reqURL, bytes.NewBuffer(payload)) if err != nil { @@ -57,8 +88,14 @@ func (p *Proxy) PublishNC(ctx context.Context, ncParams cns.NetworkContainerPara return resp, nil } -func (p *Proxy) UnpublishNC(ctx context.Context, ncParams cns.NetworkContainerParameters, payload []byte) (*http.Response, error) { - reqURL := fmt.Sprintf(unpublishNCURLFmt, p.Host, ncParams.AssociatedInterfaceID, ncParams.NCID, ncParams.AuthToken) +func (p *Proxy) UnpublishNC(ctx context.Context, ncParams cns.NetworkContainerParameters, payload []byte, useRNCPublisher bool) (*http.Response, error) { + var unpublishNCURLFormat string + if useRNCPublisher { + unpublishNCURLFormat = unpublishNCURLFmt + "?useLegacyChannel=false" + } else { + unpublishNCURLFormat = unpublishNCURLFmt + } + reqURL := fmt.Sprintf(unpublishNCURLFormat, p.Host, ncParams.AssociatedInterfaceID, ncParams.NCID, ncParams.AuthToken) // a POST to wireserver must contain a body. For legacy purposes, // an empty json string (two quote characters) should be sent by default. diff --git a/nmagent/requests.go b/nmagent/requests.go index 6a173080fa7..7aec484ae33 100644 --- a/nmagent/requests.go +++ b/nmagent/requests.go @@ -74,6 +74,9 @@ type PutNetworkContainerRequest struct { // AZREnabled denotes whether AZR is enabled for network container or not AZREnabled bool + + // UseRNCPublisher denotes whether NC should be published via RNC (used for auth) + UseRNCPublisher bool } type internalNC struct { @@ -83,27 +86,29 @@ type internalNC struct { Version string `json:"version"` // The rest of these are copied verbatim from the above struct and should be kept in sync. - VNetID string `json:"virtualNetworkId"` - SubnetName string `json:"subnetName"` - IPv4Addrs []string `json:"ipV4Addresses"` - Policies []Policy `json:"policies"` - VlanID int `json:"vlanId"` - GREKey uint16 `json:"greKey"` - AzID uint `json:"azID"` - AZREnabled bool `json:"azrEnabled"` + VNetID string `json:"virtualNetworkId"` + SubnetName string `json:"subnetName"` + IPv4Addrs []string `json:"ipV4Addresses"` + Policies []Policy `json:"policies"` + VlanID int `json:"vlanId"` + GREKey uint16 `json:"greKey"` + AzID uint `json:"azID"` + AZREnabled bool `json:"azrEnabled"` + UseRNCPublisher bool `json:"useRNCPublisher"` } func (p *PutNetworkContainerRequest) MarshalJSON() ([]byte, error) { pBody := internalNC{ - Version: strconv.Itoa(int(p.Version)), - VNetID: p.VNetID, - SubnetName: p.SubnetName, - IPv4Addrs: p.IPv4Addrs, - Policies: p.Policies, - VlanID: p.VlanID, - GREKey: p.GREKey, - AzID: p.AzID, - AZREnabled: p.AZREnabled, + Version: strconv.Itoa(int(p.Version)), + VNetID: p.VNetID, + SubnetName: p.SubnetName, + IPv4Addrs: p.IPv4Addrs, + Policies: p.Policies, + VlanID: p.VlanID, + GREKey: p.GREKey, + AzID: p.AzID, + AZREnabled: p.AZREnabled, + UseRNCPublisher: p.UseRNCPublisher, } body, err := json.Marshal(pBody) @@ -135,6 +140,7 @@ func (p *PutNetworkContainerRequest) UnmarshalJSON(in []byte) error { p.GREKey = req.GREKey p.AzID = req.AzID p.AZREnabled = req.AZREnabled + p.UseRNCPublisher = req.UseRNCPublisher return nil } @@ -317,9 +323,10 @@ var _ Request = DeleteContainerRequest{} // DeleteContainerRequest represents all information necessary to request that // NMAgent delete a particular network container type DeleteContainerRequest struct { - NCID string `json:"-"` // the Network Container ID - AzID uint `json:"azID"` // home AZ of the Network Container - AZREnabled bool `json:"azrEnabled"` // whether AZR is enabled or not + NCID string `json:"-"` // the Network Container ID + AzID uint `json:"azID"` // home AZ of the Network Container + AZREnabled bool `json:"azrEnabled"` // whether AZR is enabled or not + UseRNCPublisher bool `json:"useRNCPublisher"` // PrimaryAddress is the primary customer address of the interface in the // management VNET From 9082d27f36612c57c7fe536c7439bff1d7518fed Mon Sep 17 00:00:00 2001 From: "Saksham Mittal (from Dev Box)" Date: Tue, 11 Aug 2026 11:43:58 -0700 Subject: [PATCH 02/10] fix minor issues --- cns/restserver/api.go | 14 ++++++-- cns/restserver/api_test.go | 66 +++++++++++++++++++++++++++++++++--- cns/restserver/restserver.go | 1 + 3 files changed, 74 insertions(+), 7 deletions(-) diff --git a/cns/restserver/api.go b/cns/restserver/api.go index 99488fab002..74da7a479be 100644 --- a/cns/restserver/api.go +++ b/cns/restserver/api.go @@ -21,7 +21,6 @@ import ( "github.com/Azure/azure-container-networking/cns/types" "github.com/Azure/azure-container-networking/cns/wireserver" "github.com/Azure/azure-container-networking/common" - "github.com/Azure/azure-container-networking/nmagent" "github.com/pkg/errors" ) @@ -40,6 +39,15 @@ const ( ncURLExpectedMatches = 5 ) +type ncPublishBody struct { + UseRNCPublisher bool `json:"useRNCPublisher"` +} + +type ncUnpublishBody struct { + UseRNCPublisher bool `json:"useRNCPublisher"` + AZREnabled bool `json:"azrEnabled"` +} + // This file contains implementation of all HTTP APIs which are exposed to external clients. // TODO: break it even further per module (network, nc, etc) like it is done for ipam @@ -948,7 +956,7 @@ func (service *HTTPRestService) publishNetworkContainer(w http.ResponseWriter, r ctx := r.Context() - var publishBody nmagent.PutNetworkContainerRequest + var publishBody ncPublishBody var useRNCPublisher bool err = json.Unmarshal(req.CreateNetworkContainerRequestBody, &publishBody) @@ -1093,7 +1101,7 @@ func (service *HTTPRestService) unpublishNetworkContainer(w http.ResponseWriter, ctx := r.Context() - var unpublishBody nmagent.DeleteContainerRequest + var unpublishBody ncUnpublishBody var azrNC bool var useRNCPublisher bool err = json.Unmarshal(req.DeleteNetworkContainerRequestBody, &unpublishBody) diff --git a/cns/restserver/api_test.go b/cns/restserver/api_test.go index f68fb7facb3..8f1e50f7905 100644 --- a/cns/restserver/api_test.go +++ b/cns/restserver/api_test.go @@ -645,11 +645,11 @@ func TestGetNetworkContainerVersionStatus(t *testing.T) { return rsp, errors.New("boom") //nolint:goerr113 // it's just a test } - wsproxy.JoinNetworkFunc = func(ctx context.Context, s string) (*http.Response, error) { + wsproxy.JoinNetworkFunc = func(ctx context.Context, s string, _ bool) (*http.Response, error) { return nil, errors.New("boom") //nolint:goerr113 // it's just a test } - wsproxy.PublishNCFunc = func(ctx context.Context, parameters cns.NetworkContainerParameters, i []byte) (*http.Response, error) { + wsproxy.PublishNCFunc = func(ctx context.Context, parameters cns.NetworkContainerParameters, i []byte, _ bool) (*http.Response, error) { return nil, errors.New("boom") //nolint:goerr113 // it's just a test } @@ -764,7 +764,7 @@ func TestPublishNC_NMAgentApplicationErrors(t *testing.T) { wireserverBody := `{"httpStatusCode":"401"}` wsproxy := fakes.WireserverProxyFake{ - PublishNCFunc: func(_ context.Context, _ cns.NetworkContainerParameters, _ []byte) (*http.Response, error) { + PublishNCFunc: func(_ context.Context, _ cns.NetworkContainerParameters, _ []byte, _ bool) (*http.Response, error) { return &http.Response{ StatusCode: http.StatusOK, Body: io.NopCloser(bytes.NewBufferString(wireserverBody)), @@ -839,6 +839,64 @@ func TestPublishNC_NMAgentApplicationErrors(t *testing.T) { } } +func TestPublishNCAllowsEmptyRequestBody(t *testing.T) { + var ( + joinUsedRNCPublisher bool + publishUsedRNCPublisher bool + ) + + wsproxy := fakes.WireserverProxyFake{ + JoinNetworkFunc: func(_ context.Context, _ string, useRNCPublisher bool) (*http.Response, error) { + joinUsedRNCPublisher = useRNCPublisher + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(bytes.NewBufferString(`{"httpStatusCode":"200"}`)), + }, nil + }, + PublishNCFunc: func(_ context.Context, _ cns.NetworkContainerParameters, _ []byte, useRNCPublisher bool) (*http.Response, error) { + publishUsedRNCPublisher = useRNCPublisher + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(bytes.NewBufferString(`{"httpStatusCode":"200"}`)), + }, nil + }, + } + + cleanup := setWireserverProxy(svc, &wsproxy) + defer cleanup() + + joinNetworkURL := "http://" + nmagentEndpoint + "/dummyVnetURL" + createNetworkContainerURL := "http://" + nmagentEndpoint + + "/machine/plugins/?comp=nmagent&type=NetworkManagement/interfaces/dummyIntf/networkContainers/dummyNCURL/authenticationToken/dummyT/api-version/1" + publishNCRequest := &cns.PublishNetworkContainerRequest{ + NetworkID: "foo", + NetworkContainerID: "bar", + JoinNetworkURL: joinNetworkURL, + CreateNetworkContainerURL: createNetworkContainerURL, + CreateNetworkContainerRequestBody: []byte("{}"), + } + + var body bytes.Buffer + err := json.NewEncoder(&body).Encode(publishNCRequest) + require.NoError(t, err) + + //nolint:noctx // not needed in test + req, err := http.NewRequest(http.MethodPost, cns.PublishNetworkContainer, &body) + require.NoError(t, err) + + w := httptest.NewRecorder() + mux.ServeHTTP(w, req) + + require.Equal(t, http.StatusOK, w.Code) + + var resp cns.PublishNetworkContainerResponse + err = decodeResponse(w, &resp) + require.NoError(t, err) + require.Equal(t, types.Success, resp.Response.ReturnCode) + require.False(t, joinUsedRNCPublisher) + require.False(t, publishUsedRNCPublisher) +} + func publishNCViaCNS( networkID, networkContainerID, @@ -1001,7 +1059,7 @@ func TestUnpublishViaCNSRequestBody(t *testing.T) { func TestUnpublishNCViaCNS401(t *testing.T) { wsproxy := fakes.WireserverProxyFake{ - UnpublishNCFunc: func(_ context.Context, _ cns.NetworkContainerParameters, i []byte) (*http.Response, error) { + UnpublishNCFunc: func(_ context.Context, _ cns.NetworkContainerParameters, i []byte, _ bool) (*http.Response, error) { return &http.Response{ StatusCode: http.StatusOK, Body: io.NopCloser(bytes.NewBufferString(`{"httpStatusCode":"401"}`)), diff --git a/cns/restserver/restserver.go b/cns/restserver/restserver.go index 4260dc2d574..d561d5a42cb 100644 --- a/cns/restserver/restserver.go +++ b/cns/restserver/restserver.go @@ -244,6 +244,7 @@ func NewHTTPRestService(config *common.ServiceConfig, wscli interfaceGetter, wsp serviceState := &httpRestServiceState{ Networks: make(map[string]*networkInfo), joinedNetworks: make(map[string]struct{}), + joinedSubnets: make(map[string]struct{}), primaryInterface: primaryInterface, PnpIDByMacAddress: make(map[string]string), } From 6a0595f21c4e34aa46b1c4a88898e63920bab526 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 11 Aug 2026 19:43:15 +0000 Subject: [PATCH 03/10] Address RNC review and lint issues Co-authored-by: smittal22 <111590532+smittal22@users.noreply.github.com> --- cns/restserver/api.go | 37 ++++--- cns/restserver/api_test.go | 210 +++++++++++++++++++++++++++++++++++-- nmagent/requests.go | 2 +- 3 files changed, 230 insertions(+), 19 deletions(-) diff --git a/cns/restserver/api.go b/cns/restserver/api.go index 74da7a479be..b81468d3cc4 100644 --- a/cns/restserver/api.go +++ b/cns/restserver/api.go @@ -1003,18 +1003,18 @@ func (service *HTTPRestService) publishNetworkContainer(w http.ResponseWriter, r service.setNetworkStateJoined(req.NetworkID) logger.Printf("[Azure-CNS] joined vnet %s during nc %s publish. wireserver response: %v", req.NetworkID, req.NetworkContainerID, string(joinBytes)) - if useRNCPublisher { + if useRNCPublisher && !service.isSubnetJoined(req.NetworkID, req.SubnetName) { joinSubnetResp, errSubnetJoin := service.wsproxy.JoinSubnet(ctx, req.NetworkID, req.SubnetName, ncParams) //nolint:govet // ok to shadow if errSubnetJoin != nil { resp := cns.PublishNetworkContainerResponse{ Response: cns.Response{ ReturnCode: types.SubnetJoinFailed, - Message: fmt.Sprintf("failed to join subnet %s in network %s: %v", req.SubnetName, req.NetworkID, err), + Message: fmt.Sprintf("failed to join subnet %s in network %s: %v", req.SubnetName, req.NetworkID, errSubnetJoin), }, PublishErrorStr: errSubnetJoin.Error(), } - respondJSON(w, http.StatusOK, resp) // legacy behavior - logger.Response(service.Name, resp, resp.Response.ReturnCode, errSubnetJoin) + respondJSON(w, http.StatusOK, resp) // legacy behavior + logger.Response(service.Name, resp, resp.Response.ReturnCode, errSubnetJoin) //nolint:staticcheck // match existing logger usage in this handler return } @@ -1030,13 +1030,19 @@ func (service *HTTPRestService) publishNetworkContainer(w http.ResponseWriter, r PublishStatusCode: joinSubnetResp.StatusCode, PublishResponseBody: subnetJoinBytes, } - respondJSON(w, http.StatusOK, resp) // legacy behavior - logger.Response(service.Name, resp, resp.Response.ReturnCode, nil) + respondJSON(w, http.StatusOK, resp) // legacy behavior + logger.Response(service.Name, resp, resp.Response.ReturnCode, nil) //nolint:staticcheck // match existing logger usage in this handler return } service.setSubnetStateJoined(req.NetworkID, req.SubnetName) - logger.Printf("[Azure-CNS] joined subnet %s in vnet %s during nc %s publish. wireserver response: %v", req.SubnetName, req.NetworkID, req.NetworkContainerID, string(subnetJoinBytes)) + logger.Printf( //nolint:staticcheck // match existing logger usage in this handler + "[Azure-CNS] joined subnet %s in vnet %s during nc %s publish. wireserver response: %v", + req.SubnetName, + req.NetworkID, + req.NetworkContainerID, + string(subnetJoinBytes), + ) } publishResp, err := service.wsproxy.PublishNC(ctx, ncParams, req.CreateNetworkContainerRequestBody, useRNCPublisher) @@ -1173,8 +1179,8 @@ func (service *HTTPRestService) unpublishNetworkContainer(w http.ResponseWriter, }, UnpublishErrorStr: err.Error(), } - respondJSON(w, http.StatusOK, resp) // legacy behavior - logger.Response(service.Name, resp, resp.Response.ReturnCode, err) + respondJSON(w, http.StatusOK, resp) // legacy behavior + logger.Response(service.Name, resp, resp.Response.ReturnCode, err) //nolint:staticcheck // match existing logger usage in this handler return } @@ -1190,13 +1196,20 @@ func (service *HTTPRestService) unpublishNetworkContainer(w http.ResponseWriter, UnpublishStatusCode: joinSubnetResp.StatusCode, UnpublishResponseBody: subnetJoinBytes, } - respondJSON(w, http.StatusOK, resp) // legacy behavior - logger.Response(service.Name, resp, resp.Response.ReturnCode, nil) + respondJSON(w, http.StatusOK, resp) // legacy behavior + logger.Response(service.Name, resp, resp.Response.ReturnCode, nil) //nolint:staticcheck // match existing logger usage in this handler return } service.setSubnetStateJoined(req.NetworkID, req.SubnetName) - logger.Printf("[Azure-CNS] joined subnet %s in vnet %s during nc %s unpublish. AZREnabled: %t, wireserver response: %v", req.SubnetName, req.NetworkID, req.NetworkContainerID, unpublishBody.AZREnabled, string(subnetJoinBytes)) + logger.Printf( //nolint:staticcheck // match existing logger usage in this handler + "[Azure-CNS] joined subnet %s in vnet %s during nc %s unpublish. AZREnabled: %t, wireserver response: %v", + req.SubnetName, + req.NetworkID, + req.NetworkContainerID, + unpublishBody.AZREnabled, + string(subnetJoinBytes), + ) } } diff --git a/cns/restserver/api_test.go b/cns/restserver/api_test.go index 8f1e50f7905..2702f64782e 100644 --- a/cns/restserver/api_test.go +++ b/cns/restserver/api_test.go @@ -645,11 +645,11 @@ func TestGetNetworkContainerVersionStatus(t *testing.T) { return rsp, errors.New("boom") //nolint:goerr113 // it's just a test } - wsproxy.JoinNetworkFunc = func(ctx context.Context, s string, _ bool) (*http.Response, error) { + wsproxy.JoinNetworkFunc = func(_ context.Context, _ string, _ bool) (*http.Response, error) { return nil, errors.New("boom") //nolint:goerr113 // it's just a test } - wsproxy.PublishNCFunc = func(ctx context.Context, parameters cns.NetworkContainerParameters, i []byte, _ bool) (*http.Response, error) { + wsproxy.PublishNCFunc = func(_ context.Context, _ cns.NetworkContainerParameters, _ []byte, _ bool) (*http.Response, error) { return nil, errors.New("boom") //nolint:goerr113 // it's just a test } @@ -876,9 +876,7 @@ func TestPublishNCAllowsEmptyRequestBody(t *testing.T) { CreateNetworkContainerRequestBody: []byte("{}"), } - var body bytes.Buffer - err := json.NewEncoder(&body).Encode(publishNCRequest) - require.NoError(t, err) + body := encodeRequestBody(t, publishNCRequest) //nolint:noctx // not needed in test req, err := http.NewRequest(http.MethodPost, cns.PublishNetworkContainer, &body) @@ -897,6 +895,133 @@ func TestPublishNCAllowsEmptyRequestBody(t *testing.T) { require.False(t, publishUsedRNCPublisher) } +func TestPublishNCWithRNCPublisherJoinsSubnetOnce(t *testing.T) { + const ( + networkID = "vnet-rnc-publish" + subnetName = "subnet-rnc-publish" + networkContainerID = "nc-rnc-publish" + ) + + var ( + joinSubnetCalls int + publishCalls int + ) + + wsproxy := fakes.WireserverProxyFake{ + JoinNetworkFunc: func(_ context.Context, vnetID string, useRNCPublisher bool) (*http.Response, error) { + require.Equal(t, networkID, vnetID) + require.True(t, useRNCPublisher) + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(bytes.NewBufferString(`{"httpStatusCode":"200"}`)), + }, nil + }, + JoinSubnetFunc: func(_ context.Context, vnetID, gotSubnetName string, _ cns.NetworkContainerParameters) (*http.Response, error) { + joinSubnetCalls++ + require.Equal(t, networkID, vnetID) + require.Equal(t, subnetName, gotSubnetName) + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(bytes.NewBufferString(`{"httpStatusCode":"200"}`)), + }, nil + }, + PublishNCFunc: func(_ context.Context, _ cns.NetworkContainerParameters, _ []byte, useRNCPublisher bool) (*http.Response, error) { + publishCalls++ + require.True(t, useRNCPublisher) + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(bytes.NewBufferString(`{"httpStatusCode":"200"}`)), + }, nil + }, + } + + cleanup := setWireserverProxy(svc, &wsproxy) + t.Cleanup(cleanup) + + createNetworkContainerURL := "http://" + nmagentEndpoint + + "/machine/plugins/?comp=nmagent&type=NetworkManagement/interfaces/dummyIntf/networkContainers/dummyNCURL/authenticationToken/dummyT/api-version/1" + publishNCRequest := &cns.PublishNetworkContainerRequest{ + NetworkID: networkID, + SubnetName: subnetName, + NetworkContainerID: networkContainerID, + JoinNetworkURL: "http://" + nmagentEndpoint + "/dummyVnetURL", + CreateNetworkContainerURL: createNetworkContainerURL, + CreateNetworkContainerRequestBody: []byte(`{"useRNCPublisher":true}`), + } + + for i := 0; i < 2; i++ { + body := encodeRequestBody(t, publishNCRequest) + + //nolint:noctx // not needed in test + req, err := http.NewRequest(http.MethodPost, cns.PublishNetworkContainer, &body) + require.NoError(t, err) + + w := httptest.NewRecorder() + mux.ServeHTTP(w, req) + + var resp cns.PublishNetworkContainerResponse + err = decodeResponse(w, &resp) + require.NoError(t, err) + require.Equal(t, types.Success, resp.Response.ReturnCode) + } + + require.Equal(t, 1, joinSubnetCalls) + require.Equal(t, 2, publishCalls) +} + +func TestPublishNCWithRNCPublisherSubnetJoinFailure(t *testing.T) { + const ( + networkID = "vnet-rnc-subnet-failure" + subnetName = "subnet-rnc-subnet-failure" + networkContainerID = "nc-rnc-subnet-failure" + ) + + var publishCalls int + + wsproxy := fakes.WireserverProxyFake{ + JoinSubnetFunc: func(context.Context, string, string, cns.NetworkContainerParameters) (*http.Response, error) { + return nil, errors.New("subnet join failed") + }, + PublishNCFunc: func(context.Context, cns.NetworkContainerParameters, []byte, bool) (*http.Response, error) { + publishCalls++ + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(bytes.NewBufferString(`{"httpStatusCode":"200"}`)), + }, nil + }, + } + + cleanup := setWireserverProxy(svc, &wsproxy) + t.Cleanup(cleanup) + + createNetworkContainerURL := "http://" + nmagentEndpoint + + "/machine/plugins/?comp=nmagent&type=NetworkManagement/interfaces/dummyIntf/networkContainers/dummyNCURL/authenticationToken/dummyT/api-version/1" + publishNCRequest := &cns.PublishNetworkContainerRequest{ + NetworkID: networkID, + SubnetName: subnetName, + NetworkContainerID: networkContainerID, + JoinNetworkURL: "http://" + nmagentEndpoint + "/dummyVnetURL", + CreateNetworkContainerURL: createNetworkContainerURL, + CreateNetworkContainerRequestBody: []byte(`{"useRNCPublisher":true}`), + } + + body := encodeRequestBody(t, publishNCRequest) + + //nolint:noctx // not needed in test + req, err := http.NewRequest(http.MethodPost, cns.PublishNetworkContainer, &body) + require.NoError(t, err) + + w := httptest.NewRecorder() + mux.ServeHTTP(w, req) + + var resp cns.PublishNetworkContainerResponse + err = decodeResponse(w, &resp) + require.NoError(t, err) + require.Equal(t, types.SubnetJoinFailed, resp.Response.ReturnCode) + require.Contains(t, resp.PublishErrorStr, "subnet join failed") + require.Zero(t, publishCalls) +} + func publishNCViaCNS( networkID, networkContainerID, @@ -1059,7 +1184,7 @@ func TestUnpublishViaCNSRequestBody(t *testing.T) { func TestUnpublishNCViaCNS401(t *testing.T) { wsproxy := fakes.WireserverProxyFake{ - UnpublishNCFunc: func(_ context.Context, _ cns.NetworkContainerParameters, i []byte, _ bool) (*http.Response, error) { + UnpublishNCFunc: func(_ context.Context, _ cns.NetworkContainerParameters, _ []byte, _ bool) (*http.Response, error) { return &http.Response{ StatusCode: http.StatusOK, Body: io.NopCloser(bytes.NewBufferString(`{"httpStatusCode":"401"}`)), @@ -1136,6 +1261,69 @@ func TestUnpublishNCViaCNS401(t *testing.T) { } } +func TestUnpublishNCWithRNCPublisherSkipsJoinedSubnet(t *testing.T) { + const ( + networkID = "vnet-rnc-unpublish" + subnetName = "subnet-rnc-unpublish" + networkContainerID = "nc-rnc-unpublish" + ) + + svc.setSubnetStateJoined(networkID, subnetName) + + var ( + joinSubnetCalls int + unpublishCalls int + ) + + wsproxy := fakes.WireserverProxyFake{ + JoinSubnetFunc: func(context.Context, string, string, cns.NetworkContainerParameters) (*http.Response, error) { + joinSubnetCalls++ + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(bytes.NewBufferString(`{"httpStatusCode":"200"}`)), + }, nil + }, + UnpublishNCFunc: func(_ context.Context, _ cns.NetworkContainerParameters, _ []byte, useRNCPublisher bool) (*http.Response, error) { + unpublishCalls++ + require.True(t, useRNCPublisher) + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(bytes.NewBufferString(`{"httpStatusCode":"200"}`)), + }, nil + }, + } + + cleanup := setWireserverProxy(svc, &wsproxy) + t.Cleanup(cleanup) + + deleteNetworkContainerURL := "http://" + nmagentEndpoint + + "/machine/plugins/?comp=nmagent&type=NetworkManagement/interfaces/dummyIntf/networkContainers/dummyNCURL/authenticationToken/dummyT/api-version/1/method/DELETE" + unpublishNCRequest := &cns.UnpublishNetworkContainerRequest{ + NetworkID: networkID, + SubnetName: subnetName, + NetworkContainerID: networkContainerID, + JoinNetworkURL: "http://" + nmagentEndpoint + "/dummyVnetURL", + DeleteNetworkContainerURL: deleteNetworkContainerURL, + DeleteNetworkContainerRequestBody: []byte(`{"azrEnabled":true,"useRNCPublisher":true}`), + } + + body := encodeRequestBody(t, unpublishNCRequest) + + //nolint:noctx // not needed in test + req, err := http.NewRequest(http.MethodPost, cns.UnpublishNetworkContainer, &body) + require.NoError(t, err) + + w := httptest.NewRecorder() + mux.ServeHTTP(w, req) + + var resp cns.UnpublishNetworkContainerResponse + err = decodeResponse(w, &resp) + require.NoError(t, err) + require.Equal(t, types.Success, resp.Response.ReturnCode) + require.Zero(t, joinSubnetCalls) + require.Equal(t, 1, unpublishCalls) +} + func unpublishNCViaCNS(networkID, networkContainerID, deleteNetworkContainerURL string, bodyBytes []byte) error { joinNetworkURL := "http://" + nmagentEndpoint + "/dummyVnetURL" @@ -1696,6 +1884,16 @@ func decodeResponse(w *httptest.ResponseRecorder, response interface{}) error { return json.NewDecoder(w.Body).Decode(&response) } +func encodeRequestBody(t *testing.T, request any) bytes.Buffer { + t.Helper() + + var body bytes.Buffer + err := json.NewEncoder(&body).Encode(request) + require.NoError(t, err) + + return body +} + func setEnv(t *testing.T) *httptest.ResponseRecorder { envRequest := cns.SetEnvironmentRequest{Location: "Azure", NetworkType: "Underlay"} envRequestJSON := new(bytes.Buffer) diff --git a/nmagent/requests.go b/nmagent/requests.go index 7aec484ae33..7482cf065fd 100644 --- a/nmagent/requests.go +++ b/nmagent/requests.go @@ -99,7 +99,7 @@ type internalNC struct { func (p *PutNetworkContainerRequest) MarshalJSON() ([]byte, error) { pBody := internalNC{ - Version: strconv.Itoa(int(p.Version)), + Version: strconv.FormatUint(p.Version, 10), VNetID: p.VNetID, SubnetName: p.SubnetName, IPv4Addrs: p.IPv4Addrs, From fec5aa49f390c8a82c29fa9fae91d6af62c598af Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 11 Aug 2026 19:44:14 +0000 Subject: [PATCH 04/10] Clean up RNC subnet test state Co-authored-by: smittal22 <111590532+smittal22@users.noreply.github.com> --- cns/restserver/api_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/cns/restserver/api_test.go b/cns/restserver/api_test.go index 2702f64782e..16d81d8c8d9 100644 --- a/cns/restserver/api_test.go +++ b/cns/restserver/api_test.go @@ -937,6 +937,7 @@ func TestPublishNCWithRNCPublisherJoinsSubnetOnce(t *testing.T) { cleanup := setWireserverProxy(svc, &wsproxy) t.Cleanup(cleanup) + cleanupJoinedSubnetState(t, networkID, subnetName) createNetworkContainerURL := "http://" + nmagentEndpoint + "/machine/plugins/?comp=nmagent&type=NetworkManagement/interfaces/dummyIntf/networkContainers/dummyNCURL/authenticationToken/dummyT/api-version/1" @@ -1269,6 +1270,7 @@ func TestUnpublishNCWithRNCPublisherSkipsJoinedSubnet(t *testing.T) { ) svc.setSubnetStateJoined(networkID, subnetName) + cleanupJoinedSubnetState(t, networkID, subnetName) var ( joinSubnetCalls int @@ -1894,6 +1896,16 @@ func encodeRequestBody(t *testing.T, request any) bytes.Buffer { return body } +func cleanupJoinedSubnetState(t *testing.T, vnetID, subnetName string) { + t.Helper() + + t.Cleanup(func() { + namedLock.LockAcquire(stateJoinedSubnets) + defer namedLock.LockRelease(stateJoinedSubnets) + delete(svc.state.joinedSubnets, vnetID+"_"+subnetName) + }) +} + func setEnv(t *testing.T) *httptest.ResponseRecorder { envRequest := cns.SetEnvironmentRequest{Location: "Azure", NetworkType: "Underlay"} envRequestJSON := new(bytes.Buffer) From e5b1b94be3ee07ceeddf21ea5dbd3205bf0da4a8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 11 Aug 2026 19:45:31 +0000 Subject: [PATCH 05/10] Use shared subnet state cleanup helper Co-authored-by: smittal22 <111590532+smittal22@users.noreply.github.com> --- cns/restserver/api_test.go | 4 +--- cns/restserver/util.go | 16 ++++++++++++---- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/cns/restserver/api_test.go b/cns/restserver/api_test.go index 16d81d8c8d9..d6054ce0933 100644 --- a/cns/restserver/api_test.go +++ b/cns/restserver/api_test.go @@ -1900,9 +1900,7 @@ func cleanupJoinedSubnetState(t *testing.T, vnetID, subnetName string) { t.Helper() t.Cleanup(func() { - namedLock.LockAcquire(stateJoinedSubnets) - defer namedLock.LockRelease(stateJoinedSubnets) - delete(svc.state.joinedSubnets, vnetID+"_"+subnetName) + svc.clearSubnetStateJoined(vnetID, subnetName) }) } diff --git a/cns/restserver/util.go b/cns/restserver/util.go index 5dcfe563ec6..9f655189d93 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -746,11 +746,14 @@ func (service *HTTPRestService) isNetworkJoined(networkID string) bool { func (service *HTTPRestService) isSubnetJoined(vnetID, subnetName string) bool { namedLock.LockAcquire(stateJoinedSubnets) defer namedLock.LockRelease(stateJoinedSubnets) - subnetKey := vnetID + "_" + subnetName - _, exists := service.state.joinedSubnets[subnetKey] + _, exists := service.state.joinedSubnets[subnetStateKey(vnetID, subnetName)] return exists } +func subnetStateKey(vnetID, subnetName string) string { + return vnetID + "_" + subnetName +} + // Set the network as joined func (service *HTTPRestService) setNetworkStateJoined(networkID string) { namedLock.LockAcquire(stateJoinedNetworks) @@ -762,8 +765,13 @@ func (service *HTTPRestService) setNetworkStateJoined(networkID string) { func (service *HTTPRestService) setSubnetStateJoined(vnetID, subnetName string) { namedLock.LockAcquire(stateJoinedSubnets) defer namedLock.LockRelease(stateJoinedSubnets) - subnetKey := vnetID + "_" + subnetName - service.state.joinedSubnets[subnetKey] = struct{}{} + service.state.joinedSubnets[subnetStateKey(vnetID, subnetName)] = struct{}{} +} + +func (service *HTTPRestService) clearSubnetStateJoined(vnetID, subnetName string) { + namedLock.LockAcquire(stateJoinedSubnets) + defer namedLock.LockRelease(stateJoinedSubnets) + delete(service.state.joinedSubnets, subnetStateKey(vnetID, subnetName)) } func logNCSnapshot(createNetworkContainerRequest cns.CreateNetworkContainerRequest) { From 6a05d65aee2c5cca8ba5ab6fcbbb5923e8dfb4a3 Mon Sep 17 00:00:00 2001 From: "Saksham Mittal (from Dev Box)" Date: Wed, 12 Aug 2026 14:04:52 -0700 Subject: [PATCH 06/10] moderate changes and test coverage --- cns/restserver/api.go | 77 +++---- cns/restserver/api_test.go | 428 +++++++++++++++++++++++++++++++++-- cns/restserver/const.go | 1 - cns/restserver/restserver.go | 2 - cns/restserver/util.go | 25 -- cns/restserver/util_test.go | 26 +++ cns/wireserver/proxy.go | 8 +- cns/wireserver/proxy_test.go | 153 +++++++++++++ 8 files changed, 632 insertions(+), 88 deletions(-) create mode 100644 cns/wireserver/proxy_test.go diff --git a/cns/restserver/api.go b/cns/restserver/api.go index b81468d3cc4..f3b0124d9f6 100644 --- a/cns/restserver/api.go +++ b/cns/restserver/api.go @@ -45,6 +45,7 @@ type ncPublishBody struct { type ncUnpublishBody struct { UseRNCPublisher bool `json:"useRNCPublisher"` + AzID uint `json:"azID"` AZREnabled bool `json:"azrEnabled"` } @@ -1003,7 +1004,7 @@ func (service *HTTPRestService) publishNetworkContainer(w http.ResponseWriter, r service.setNetworkStateJoined(req.NetworkID) logger.Printf("[Azure-CNS] joined vnet %s during nc %s publish. wireserver response: %v", req.NetworkID, req.NetworkContainerID, string(joinBytes)) - if useRNCPublisher && !service.isSubnetJoined(req.NetworkID, req.SubnetName) { + if useRNCPublisher { joinSubnetResp, errSubnetJoin := service.wsproxy.JoinSubnet(ctx, req.NetworkID, req.SubnetName, ncParams) //nolint:govet // ok to shadow if errSubnetJoin != nil { resp := cns.PublishNetworkContainerResponse{ @@ -1035,7 +1036,6 @@ func (service *HTTPRestService) publishNetworkContainer(w http.ResponseWriter, r return } - service.setSubnetStateJoined(req.NetworkID, req.SubnetName) logger.Printf( //nolint:staticcheck // match existing logger usage in this handler "[Azure-CNS] joined subnet %s in vnet %s during nc %s publish. wireserver response: %v", req.SubnetName, @@ -1169,48 +1169,45 @@ func (service *HTTPRestService) unpublishNetworkContainer(w http.ResponseWriter, } if useRNCPublisher { - if !service.isSubnetJoined(req.NetworkID, req.SubnetName) { - joinSubnetResp, err := service.wsproxy.JoinSubnet(ctx, req.NetworkID, req.SubnetName, ncParams) //nolint:govet // ok to shadow - if err != nil { - resp := cns.UnpublishNetworkContainerResponse{ - Response: cns.Response{ - ReturnCode: types.SubnetJoinFailed, - Message: fmt.Sprintf("failed to join subnet %s in network %s: %v", req.SubnetName, req.NetworkID, err), - }, - UnpublishErrorStr: err.Error(), - } - respondJSON(w, http.StatusOK, resp) // legacy behavior - logger.Response(service.Name, resp, resp.Response.ReturnCode, err) //nolint:staticcheck // match existing logger usage in this handler - return + joinSubnetResp, err := service.wsproxy.JoinSubnet(ctx, req.NetworkID, req.SubnetName, ncParams) //nolint:govet // ok to shadow + if err != nil { + resp := cns.UnpublishNetworkContainerResponse{ + Response: cns.Response{ + ReturnCode: types.SubnetJoinFailed, + Message: fmt.Sprintf("failed to join subnet %s in network %s: %v", req.SubnetName, req.NetworkID, err), + }, + UnpublishErrorStr: err.Error(), } + respondJSON(w, http.StatusOK, resp) // legacy behavior + logger.Response(service.Name, resp, resp.Response.ReturnCode, err) //nolint:staticcheck // match existing logger usage in this handler + return + } - subnetJoinBytes, _ := io.ReadAll(joinSubnetResp.Body) - _ = joinSubnetResp.Body.Close() - - if joinSubnetResp.StatusCode != http.StatusOK { - resp := cns.UnpublishNetworkContainerResponse{ - Response: cns.Response{ - ReturnCode: types.SubnetJoinFailed, - Message: fmt.Sprintf("failed to join subnet %s in network %s. did not get 200 from wireserver", req.SubnetName, req.NetworkID), - }, - UnpublishStatusCode: joinSubnetResp.StatusCode, - UnpublishResponseBody: subnetJoinBytes, - } - respondJSON(w, http.StatusOK, resp) // legacy behavior - logger.Response(service.Name, resp, resp.Response.ReturnCode, nil) //nolint:staticcheck // match existing logger usage in this handler - return - } + subnetJoinBytes, _ := io.ReadAll(joinSubnetResp.Body) + _ = joinSubnetResp.Body.Close() - service.setSubnetStateJoined(req.NetworkID, req.SubnetName) - logger.Printf( //nolint:staticcheck // match existing logger usage in this handler - "[Azure-CNS] joined subnet %s in vnet %s during nc %s unpublish. AZREnabled: %t, wireserver response: %v", - req.SubnetName, - req.NetworkID, - req.NetworkContainerID, - unpublishBody.AZREnabled, - string(subnetJoinBytes), - ) + if joinSubnetResp.StatusCode != http.StatusOK { + resp := cns.UnpublishNetworkContainerResponse{ + Response: cns.Response{ + ReturnCode: types.SubnetJoinFailed, + Message: fmt.Sprintf("failed to join subnet %s in network %s. did not get 200 from wireserver", req.SubnetName, req.NetworkID), + }, + UnpublishStatusCode: joinSubnetResp.StatusCode, + UnpublishResponseBody: subnetJoinBytes, + } + respondJSON(w, http.StatusOK, resp) // legacy behavior + logger.Response(service.Name, resp, resp.Response.ReturnCode, nil) //nolint:staticcheck // match existing logger usage in this handler + return } + + logger.Printf( //nolint:staticcheck // match existing logger usage in this handler + "[Azure-CNS] joined subnet %s in vnet %s during nc %s unpublish. AZREnabled: %t, wireserver response: %v", + req.SubnetName, + req.NetworkID, + req.NetworkContainerID, + unpublishBody.AZREnabled, + string(subnetJoinBytes), + ) } unpublishResp, err := service.wsproxy.UnpublishNC(ctx, ncParams, req.DeleteNetworkContainerRequestBody, useRNCPublisher) diff --git a/cns/restserver/api_test.go b/cns/restserver/api_test.go index d6054ce0933..b021e4fbe8b 100644 --- a/cns/restserver/api_test.go +++ b/cns/restserver/api_test.go @@ -895,7 +895,135 @@ func TestPublishNCAllowsEmptyRequestBody(t *testing.T) { require.False(t, publishUsedRNCPublisher) } -func TestPublishNCWithRNCPublisherJoinsSubnetOnce(t *testing.T) { +func TestPublishNCRequestBodyParsingMatrix(t *testing.T) { + const ( + networkID = "vnet-publish-body-matrix" + subnetName = "subnet-publish-body-matrix" + networkContainerID = "nc-publish-body-matrix" + ) + + tests := []struct { + name string + body []byte + wantHTTPStatus int + wantReturnCode types.ResponseCode + wantUseRNCPublisher bool + wantJoinSubnetCalls int + wantPublishCalls int + }{ + { + name: "empty object succeeds without rnc", + body: []byte(`{}`), + wantHTTPStatus: http.StatusOK, + wantReturnCode: types.Success, + wantUseRNCPublisher: false, + wantJoinSubnetCalls: 0, + wantPublishCalls: 1, + }, + { + name: "unknown fields succeed without rnc", + body: []byte(`{"someField":"someValue"}`), + wantHTTPStatus: http.StatusOK, + wantReturnCode: types.Success, + wantUseRNCPublisher: false, + wantJoinSubnetCalls: 0, + wantPublishCalls: 1, + }, + { + name: "invalid version type no longer blocks request", + body: []byte(`{"version":"bad"}`), + wantHTTPStatus: http.StatusOK, + wantReturnCode: types.Success, + wantUseRNCPublisher: false, + wantJoinSubnetCalls: 0, + wantPublishCalls: 1, + }, + { + name: "rnc body triggers subnet join", + body: []byte(`{"useRNCPublisher":true}`), + wantHTTPStatus: http.StatusOK, + wantReturnCode: types.Success, + wantUseRNCPublisher: true, + wantJoinSubnetCalls: 1, + wantPublishCalls: 1, + }, + { + name: "invalid json returns bad request", + body: []byte("invalid\n"), + wantHTTPStatus: http.StatusBadRequest, + wantJoinSubnetCalls: 0, + wantPublishCalls: 0, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + var ( + joinSubnetCalls int + publishCalls int + capturedPublishRNC bool + ) + + wsproxy := fakes.WireserverProxyFake{ + JoinSubnetFunc: func(_ context.Context, vnetID, gotSubnetName string, _ cns.NetworkContainerParameters) (*http.Response, error) { + joinSubnetCalls++ + require.Equal(t, networkID, vnetID) + require.Equal(t, subnetName, gotSubnetName) + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(bytes.NewBufferString(`{"httpStatusCode":"200"}`)), + }, nil + }, + PublishNCFunc: func(_ context.Context, _ cns.NetworkContainerParameters, _ []byte, useRNCPublisher bool) (*http.Response, error) { + publishCalls++ + capturedPublishRNC = useRNCPublisher + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(bytes.NewBufferString(`{"httpStatusCode":"200"}`)), + }, nil + }, + } + + cleanup := setWireserverProxy(svc, &wsproxy) + t.Cleanup(cleanup) + + createNetworkContainerURL := "http://" + nmagentEndpoint + + "/machine/plugins/?comp=nmagent&type=NetworkManagement/interfaces/dummyIntf/networkContainers/dummyNCURL/authenticationToken/dummyT/api-version/1" + publishNCRequest := &cns.PublishNetworkContainerRequest{ + NetworkID: networkID, + SubnetName: subnetName, + NetworkContainerID: networkContainerID, + JoinNetworkURL: "http://" + nmagentEndpoint + "/dummyVnetURL", + CreateNetworkContainerURL: createNetworkContainerURL, + CreateNetworkContainerRequestBody: tt.body, + } + + body := encodeRequestBody(t, publishNCRequest) + + //nolint:noctx // not needed in test + req, err := http.NewRequest(http.MethodPost, cns.PublishNetworkContainer, &body) + require.NoError(t, err) + + w := httptest.NewRecorder() + mux.ServeHTTP(w, req) + require.Equal(t, tt.wantHTTPStatus, w.Code) + + if tt.wantHTTPStatus == http.StatusOK { + var resp cns.PublishNetworkContainerResponse + err = decodeResponse(w, &resp) + require.NoError(t, err) + require.Equal(t, tt.wantReturnCode, resp.Response.ReturnCode) + require.Equal(t, tt.wantUseRNCPublisher, capturedPublishRNC) + } + + require.Equal(t, tt.wantJoinSubnetCalls, joinSubnetCalls) + require.Equal(t, tt.wantPublishCalls, publishCalls) + }) + } +} + +func TestPublishNCWithRNCPublisherJoinsSubnetEveryTime(t *testing.T) { const ( networkID = "vnet-rnc-publish" subnetName = "subnet-rnc-publish" @@ -937,7 +1065,6 @@ func TestPublishNCWithRNCPublisherJoinsSubnetOnce(t *testing.T) { cleanup := setWireserverProxy(svc, &wsproxy) t.Cleanup(cleanup) - cleanupJoinedSubnetState(t, networkID, subnetName) createNetworkContainerURL := "http://" + nmagentEndpoint + "/machine/plugins/?comp=nmagent&type=NetworkManagement/interfaces/dummyIntf/networkContainers/dummyNCURL/authenticationToken/dummyT/api-version/1" @@ -966,7 +1093,7 @@ func TestPublishNCWithRNCPublisherJoinsSubnetOnce(t *testing.T) { require.Equal(t, types.Success, resp.Response.ReturnCode) } - require.Equal(t, 1, joinSubnetCalls) + require.Equal(t, 2, joinSubnetCalls) require.Equal(t, 2, publishCalls) } @@ -1023,6 +1150,119 @@ func TestPublishNCWithRNCPublisherSubnetJoinFailure(t *testing.T) { require.Zero(t, publishCalls) } +func TestPublishNCWithRNCPublisherSubnetJoinNon200(t *testing.T) { + const ( + networkID = "vnet-rnc-subnet-status-failure" + subnetName = "subnet-rnc-subnet-status-failure" + networkContainerID = "nc-rnc-subnet-status-failure" + ) + + var publishCalls int + const subnetJoinStatusCode = http.StatusInternalServerError + subnetJoinBody := []byte(`{"httpStatusCode":"500"}`) + + wsproxy := fakes.WireserverProxyFake{ + JoinSubnetFunc: func(context.Context, string, string, cns.NetworkContainerParameters) (*http.Response, error) { + return &http.Response{ + StatusCode: subnetJoinStatusCode, + Body: io.NopCloser(bytes.NewBuffer(subnetJoinBody)), + }, nil + }, + PublishNCFunc: func(context.Context, cns.NetworkContainerParameters, []byte, bool) (*http.Response, error) { + publishCalls++ + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(bytes.NewBufferString(`{"httpStatusCode":"200"}`)), + }, nil + }, + } + + cleanup := setWireserverProxy(svc, &wsproxy) + t.Cleanup(cleanup) + + createNetworkContainerURL := "http://" + nmagentEndpoint + + "/machine/plugins/?comp=nmagent&type=NetworkManagement/interfaces/dummyIntf/networkContainers/dummyNCURL/authenticationToken/dummyT/api-version/1" + publishNCRequest := &cns.PublishNetworkContainerRequest{ + NetworkID: networkID, + SubnetName: subnetName, + NetworkContainerID: networkContainerID, + JoinNetworkURL: "http://" + nmagentEndpoint + "/dummyVnetURL", + CreateNetworkContainerURL: createNetworkContainerURL, + CreateNetworkContainerRequestBody: []byte(`{"useRNCPublisher":true}`), + } + + body := encodeRequestBody(t, publishNCRequest) + + //nolint:noctx // not needed in test + req, err := http.NewRequest(http.MethodPost, cns.PublishNetworkContainer, &body) + require.NoError(t, err) + + w := httptest.NewRecorder() + mux.ServeHTTP(w, req) + + var resp cns.PublishNetworkContainerResponse + err = decodeResponse(w, &resp) + require.NoError(t, err) + require.Equal(t, types.SubnetJoinFailed, resp.Response.ReturnCode) + require.Equal(t, subnetJoinStatusCode, resp.PublishStatusCode) + require.Equal(t, subnetJoinBody, resp.PublishResponseBody) + require.Zero(t, publishCalls) +} + +func TestPublishNCWithRNCPublisherDisabledSkipsSubnetJoin(t *testing.T) { + var ( + joinSubnetCalls int + publishCalls int + ) + + wsproxy := fakes.WireserverProxyFake{ + JoinSubnetFunc: func(context.Context, string, string, cns.NetworkContainerParameters) (*http.Response, error) { + joinSubnetCalls++ + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(bytes.NewBufferString(`{"httpStatusCode":"200"}`)), + }, nil + }, + PublishNCFunc: func(_ context.Context, _ cns.NetworkContainerParameters, _ []byte, useRNCPublisher bool) (*http.Response, error) { + publishCalls++ + require.False(t, useRNCPublisher) + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(bytes.NewBufferString(`{"httpStatusCode":"200"}`)), + }, nil + }, + } + + cleanup := setWireserverProxy(svc, &wsproxy) + t.Cleanup(cleanup) + + createNetworkContainerURL := "http://" + nmagentEndpoint + + "/machine/plugins/?comp=nmagent&type=NetworkManagement/interfaces/dummyIntf/networkContainers/dummyNCURL/authenticationToken/dummyT/api-version/1" + publishNCRequest := &cns.PublishNetworkContainerRequest{ + NetworkID: "vnet-rnc-disabled-publish", + SubnetName: "subnet-rnc-disabled-publish", + NetworkContainerID: "nc-rnc-disabled-publish", + JoinNetworkURL: "http://" + nmagentEndpoint + "/dummyVnetURL", + CreateNetworkContainerURL: createNetworkContainerURL, + CreateNetworkContainerRequestBody: []byte(`{"useRNCPublisher":false}`), + } + + body := encodeRequestBody(t, publishNCRequest) + //nolint:noctx // not needed in test + req, err := http.NewRequest(http.MethodPost, cns.PublishNetworkContainer, &body) + require.NoError(t, err) + + w := httptest.NewRecorder() + mux.ServeHTTP(w, req) + + var resp cns.PublishNetworkContainerResponse + err = decodeResponse(w, &resp) + require.NoError(t, err) + require.Equal(t, types.Success, resp.Response.ReturnCode) + require.Zero(t, joinSubnetCalls) + require.Equal(t, 1, publishCalls) +} + func publishNCViaCNS( networkID, networkContainerID, @@ -1165,6 +1405,18 @@ func TestUnpublishViaCNSRequestBody(t *testing.T) { body: []byte(`{"azID":1,"azrEnabled":true}`), requireError: false, }, + { + name: "Delete NC with invalid AZR azID type", + ncID: "ncID4", + body: []byte(`{"azID":"bad","azrEnabled":true}`), + requireError: true, + }, + { + name: "Delete NC with empty object body", + ncID: "ncID5", + body: []byte(`{}`), + requireError: false, + }, } for _, tt := range tests { @@ -1262,16 +1514,13 @@ func TestUnpublishNCViaCNS401(t *testing.T) { } } -func TestUnpublishNCWithRNCPublisherSkipsJoinedSubnet(t *testing.T) { +func TestUnpublishNCWithRNCPublisherJoinsSubnet(t *testing.T) { const ( networkID = "vnet-rnc-unpublish" subnetName = "subnet-rnc-unpublish" networkContainerID = "nc-rnc-unpublish" ) - svc.setSubnetStateJoined(networkID, subnetName) - cleanupJoinedSubnetState(t, networkID, subnetName) - var ( joinSubnetCalls int unpublishCalls int @@ -1309,8 +1558,65 @@ func TestUnpublishNCWithRNCPublisherSkipsJoinedSubnet(t *testing.T) { DeleteNetworkContainerRequestBody: []byte(`{"azrEnabled":true,"useRNCPublisher":true}`), } - body := encodeRequestBody(t, unpublishNCRequest) + for i := 0; i < 2; i++ { + body := encodeRequestBody(t, unpublishNCRequest) + + //nolint:noctx // not needed in test + req, err := http.NewRequest(http.MethodPost, cns.UnpublishNetworkContainer, &body) + require.NoError(t, err) + + w := httptest.NewRecorder() + mux.ServeHTTP(w, req) + + var resp cns.UnpublishNetworkContainerResponse + err = decodeResponse(w, &resp) + require.NoError(t, err) + require.Equal(t, types.Success, resp.Response.ReturnCode) + } + + require.Equal(t, 2, joinSubnetCalls) + require.Equal(t, 2, unpublishCalls) +} + +func TestUnpublishNCWithRNCPublisherDisabledSkipsSubnetJoin(t *testing.T) { + var ( + joinSubnetCalls int + unpublishCalls int + ) + + wsproxy := fakes.WireserverProxyFake{ + JoinSubnetFunc: func(context.Context, string, string, cns.NetworkContainerParameters) (*http.Response, error) { + joinSubnetCalls++ + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(bytes.NewBufferString(`{"httpStatusCode":"200"}`)), + }, nil + }, + UnpublishNCFunc: func(_ context.Context, _ cns.NetworkContainerParameters, _ []byte, useRNCPublisher bool) (*http.Response, error) { + unpublishCalls++ + require.False(t, useRNCPublisher) + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(bytes.NewBufferString(`{"httpStatusCode":"200"}`)), + }, nil + }, + } + cleanup := setWireserverProxy(svc, &wsproxy) + t.Cleanup(cleanup) + + deleteNetworkContainerURL := "http://" + nmagentEndpoint + + "/machine/plugins/?comp=nmagent&type=NetworkManagement/interfaces/dummyIntf/networkContainers/dummyNCURL/authenticationToken/dummyT/api-version/1/method/DELETE" + unpublishNCRequest := &cns.UnpublishNetworkContainerRequest{ + NetworkID: "vnet-rnc-disabled-unpublish", + SubnetName: "subnet-rnc-disabled-unpublish", + NetworkContainerID: "nc-rnc-disabled-unpublish", + JoinNetworkURL: "http://" + nmagentEndpoint + "/dummyVnetURL", + DeleteNetworkContainerURL: deleteNetworkContainerURL, + DeleteNetworkContainerRequestBody: []byte(`{"azID":1,"azrEnabled":true,"useRNCPublisher":false}`), + } + + body := encodeRequestBody(t, unpublishNCRequest) //nolint:noctx // not needed in test req, err := http.NewRequest(http.MethodPost, cns.UnpublishNetworkContainer, &body) require.NoError(t, err) @@ -1326,6 +1632,104 @@ func TestUnpublishNCWithRNCPublisherSkipsJoinedSubnet(t *testing.T) { require.Equal(t, 1, unpublishCalls) } +func TestUnpublishNCWithRNCPublisherSubnetJoinFailure(t *testing.T) { + var unpublishCalls int + + wsproxy := fakes.WireserverProxyFake{ + JoinSubnetFunc: func(context.Context, string, string, cns.NetworkContainerParameters) (*http.Response, error) { + return nil, errors.New("subnet join failed") + }, + UnpublishNCFunc: func(context.Context, cns.NetworkContainerParameters, []byte, bool) (*http.Response, error) { + unpublishCalls++ + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(bytes.NewBufferString(`{"httpStatusCode":"200"}`)), + }, nil + }, + } + + cleanup := setWireserverProxy(svc, &wsproxy) + t.Cleanup(cleanup) + + deleteNetworkContainerURL := "http://" + nmagentEndpoint + + "/machine/plugins/?comp=nmagent&type=NetworkManagement/interfaces/dummyIntf/networkContainers/dummyNCURL/authenticationToken/dummyT/api-version/1/method/DELETE" + unpublishNCRequest := &cns.UnpublishNetworkContainerRequest{ + NetworkID: "vnet-rnc-unpublish-subnet-failure", + SubnetName: "subnet-rnc-unpublish-subnet-failure", + NetworkContainerID: "nc-rnc-unpublish-subnet-failure", + JoinNetworkURL: "http://" + nmagentEndpoint + "/dummyVnetURL", + DeleteNetworkContainerURL: deleteNetworkContainerURL, + DeleteNetworkContainerRequestBody: []byte(`{"azID":1,"azrEnabled":true,"useRNCPublisher":true}`), + } + + body := encodeRequestBody(t, unpublishNCRequest) + //nolint:noctx // not needed in test + req, err := http.NewRequest(http.MethodPost, cns.UnpublishNetworkContainer, &body) + require.NoError(t, err) + + w := httptest.NewRecorder() + mux.ServeHTTP(w, req) + + var resp cns.UnpublishNetworkContainerResponse + err = decodeResponse(w, &resp) + require.NoError(t, err) + require.Equal(t, types.SubnetJoinFailed, resp.Response.ReturnCode) + require.Contains(t, resp.UnpublishErrorStr, "subnet join failed") + require.Zero(t, unpublishCalls) +} + +func TestUnpublishNCWithRNCPublisherSubnetJoinNon200(t *testing.T) { + var unpublishCalls int + const subnetJoinStatusCode = http.StatusInternalServerError + subnetJoinBody := []byte(`{"httpStatusCode":"500"}`) + + wsproxy := fakes.WireserverProxyFake{ + JoinSubnetFunc: func(context.Context, string, string, cns.NetworkContainerParameters) (*http.Response, error) { + return &http.Response{ + StatusCode: subnetJoinStatusCode, + Body: io.NopCloser(bytes.NewBuffer(subnetJoinBody)), + }, nil + }, + UnpublishNCFunc: func(context.Context, cns.NetworkContainerParameters, []byte, bool) (*http.Response, error) { + unpublishCalls++ + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(bytes.NewBufferString(`{"httpStatusCode":"200"}`)), + }, nil + }, + } + + cleanup := setWireserverProxy(svc, &wsproxy) + t.Cleanup(cleanup) + + deleteNetworkContainerURL := "http://" + nmagentEndpoint + + "/machine/plugins/?comp=nmagent&type=NetworkManagement/interfaces/dummyIntf/networkContainers/dummyNCURL/authenticationToken/dummyT/api-version/1/method/DELETE" + unpublishNCRequest := &cns.UnpublishNetworkContainerRequest{ + NetworkID: "vnet-rnc-unpublish-subnet-status-failure", + SubnetName: "subnet-rnc-unpublish-subnet-status-failure", + NetworkContainerID: "nc-rnc-unpublish-subnet-status-failure", + JoinNetworkURL: "http://" + nmagentEndpoint + "/dummyVnetURL", + DeleteNetworkContainerURL: deleteNetworkContainerURL, + DeleteNetworkContainerRequestBody: []byte(`{"azID":1,"azrEnabled":true,"useRNCPublisher":true}`), + } + + body := encodeRequestBody(t, unpublishNCRequest) + //nolint:noctx // not needed in test + req, err := http.NewRequest(http.MethodPost, cns.UnpublishNetworkContainer, &body) + require.NoError(t, err) + + w := httptest.NewRecorder() + mux.ServeHTTP(w, req) + + var resp cns.UnpublishNetworkContainerResponse + err = decodeResponse(w, &resp) + require.NoError(t, err) + require.Equal(t, types.SubnetJoinFailed, resp.Response.ReturnCode) + require.Equal(t, subnetJoinStatusCode, resp.UnpublishStatusCode) + require.Equal(t, subnetJoinBody, resp.UnpublishResponseBody) + require.Zero(t, unpublishCalls) +} + func unpublishNCViaCNS(networkID, networkContainerID, deleteNetworkContainerURL string, bodyBytes []byte) error { joinNetworkURL := "http://" + nmagentEndpoint + "/dummyVnetURL" @@ -1896,14 +2300,6 @@ func encodeRequestBody(t *testing.T, request any) bytes.Buffer { return body } -func cleanupJoinedSubnetState(t *testing.T, vnetID, subnetName string) { - t.Helper() - - t.Cleanup(func() { - svc.clearSubnetStateJoined(vnetID, subnetName) - }) -} - func setEnv(t *testing.T) *httptest.ResponseRecorder { envRequest := cns.SetEnvironmentRequest{Location: "Azure", NetworkType: "Underlay"} envRequestJSON := new(bytes.Buffer) diff --git a/cns/restserver/const.go b/cns/restserver/const.go index 3b4b0d77155..0782755596d 100644 --- a/cns/restserver/const.go +++ b/cns/restserver/const.go @@ -10,7 +10,6 @@ const ( detach = "Detach" // Rest service state identifier for named lock stateJoinedNetworks = "JoinedNetworks" - stateJoinedSubnets = "JoinedSubnets" dncApiVersion = "?api-version=2018-03-01" nmaAPICallTimeout = 2 * time.Second ) diff --git a/cns/restserver/restserver.go b/cns/restserver/restserver.go index d561d5a42cb..93b15582db4 100644 --- a/cns/restserver/restserver.go +++ b/cns/restserver/restserver.go @@ -201,7 +201,6 @@ type httpRestServiceState struct { Networks map[string]*networkInfo TimeStamp time.Time joinedNetworks map[string]struct{} - joinedSubnets map[string]struct{} primaryInterface *wireserver.InterfaceInfo PnpIDByMacAddress map[string]string } @@ -244,7 +243,6 @@ func NewHTTPRestService(config *common.ServiceConfig, wscli interfaceGetter, wsp serviceState := &httpRestServiceState{ Networks: make(map[string]*networkInfo), joinedNetworks: make(map[string]struct{}), - joinedSubnets: make(map[string]struct{}), primaryInterface: primaryInterface, PnpIDByMacAddress: make(map[string]string), } diff --git a/cns/restserver/util.go b/cns/restserver/util.go index 9f655189d93..f1e39ed2e07 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -742,18 +742,6 @@ func (service *HTTPRestService) isNetworkJoined(networkID string) bool { return exists } -// Check if the subnet is joined -func (service *HTTPRestService) isSubnetJoined(vnetID, subnetName string) bool { - namedLock.LockAcquire(stateJoinedSubnets) - defer namedLock.LockRelease(stateJoinedSubnets) - _, exists := service.state.joinedSubnets[subnetStateKey(vnetID, subnetName)] - return exists -} - -func subnetStateKey(vnetID, subnetName string) string { - return vnetID + "_" + subnetName -} - // Set the network as joined func (service *HTTPRestService) setNetworkStateJoined(networkID string) { namedLock.LockAcquire(stateJoinedNetworks) @@ -761,19 +749,6 @@ func (service *HTTPRestService) setNetworkStateJoined(networkID string) { service.state.joinedNetworks[networkID] = struct{}{} } -// Set the subnet as joined -func (service *HTTPRestService) setSubnetStateJoined(vnetID, subnetName string) { - namedLock.LockAcquire(stateJoinedSubnets) - defer namedLock.LockRelease(stateJoinedSubnets) - service.state.joinedSubnets[subnetStateKey(vnetID, subnetName)] = struct{}{} -} - -func (service *HTTPRestService) clearSubnetStateJoined(vnetID, subnetName string) { - namedLock.LockAcquire(stateJoinedSubnets) - defer namedLock.LockRelease(stateJoinedSubnets) - delete(service.state.joinedSubnets, subnetStateKey(vnetID, subnetName)) -} - func logNCSnapshot(createNetworkContainerRequest cns.CreateNetworkContainerRequest) { aiEvent := aitelemetry.Event{ EventName: logger.CnsNCSnapshotEventStr, diff --git a/cns/restserver/util_test.go b/cns/restserver/util_test.go index c3c7ad76eb9..b1cc5ab26e7 100644 --- a/cns/restserver/util_test.go +++ b/cns/restserver/util_test.go @@ -185,6 +185,32 @@ func TestRestoreState(t *testing.T) { } } +func TestRestoreStateIgnoresLegacyJoinedSubnetsField(t *testing.T) { + mainStore := store.NewMockStore("") + require.NoError(t, mainStore.Write(storeKey, map[string]any{ + "NetworkType": "Underlay", + "joinedNetworks": map[string]struct{}{ + "vnet1": {}, + }, + "joinedSubnets": map[string]struct{}{ + "vnet1_subnet1": {}, + }, + })) + + svc := HTTPRestService{ + Service: &cns.Service{ + Service: &common.Service{Options: map[string]any{}}, + }, + store: mainStore, + state: &httpRestServiceState{}, + } + + svc.restoreState() + + require.Equal(t, "Underlay", svc.state.NetworkType) + require.Nil(t, svc.state.joinedNetworks) +} + // test to check if nc can be deleted from ncList for Delete() method func TestDeleteNCs(t *testing.T) { var ncs ncList diff --git a/cns/wireserver/proxy.go b/cns/wireserver/proxy.go index e0e714c5b67..60fccf37945 100644 --- a/cns/wireserver/proxy.go +++ b/cns/wireserver/proxy.go @@ -12,7 +12,7 @@ import ( const ( joinNetworkURLFmt = `http://%s/machine/plugins/?comp=nmagent&type=NetworkManagement/joinedVirtualNetworks/%s/api-version/1` - joinSubnetURLFmt = `http://%s/machine/plugins/?comp=nmagent&type=NetworkManagement/joinedVirtualNetworks/%s/joinedSubnets/%s/authenticationToken/%s/api-version/1?useLegacyChannel=false` + joinSubnetURLFmt = `http://%s/machine/plugins/?comp=nmagent&type=NetworkManagement/joinedVirtualNetworks/%s/joinedSubnets/%s/authenticationToken/%s/api-version/1&useLegacyChannel=false` publishNCURLFmt = `http://%s/machine/plugins/?comp=nmagent&type=NetworkManagement/interfaces/%s/networkContainers/%s/authenticationToken/%s/api-version/1` unpublishNCURLFmt = `http://%s/machine/plugins/?comp=nmagent&type=NetworkManagement/interfaces/%s/networkContainers/%s/authenticationToken/%s/api-version/1/method/DELETE` ) @@ -25,7 +25,7 @@ type Proxy struct { func (p *Proxy) JoinNetwork(ctx context.Context, vnetID string, useRNCPublisher bool) (*http.Response, error) { var joinNetworkURLFormat string if useRNCPublisher { - joinNetworkURLFormat = joinNetworkURLFmt + "?useLegacyChannel=false" + joinNetworkURLFormat = joinNetworkURLFmt + "&useLegacyChannel=false" } else { joinNetworkURLFormat = joinNetworkURLFmt } @@ -67,7 +67,7 @@ func (p *Proxy) JoinSubnet(ctx context.Context, vnetID, subnetName string, ncPar func (p *Proxy) PublishNC(ctx context.Context, ncParams cns.NetworkContainerParameters, payload []byte, useRNCPublisher bool) (*http.Response, error) { var publishNCURLFormat string if useRNCPublisher { - publishNCURLFormat = publishNCURLFmt + "?useLegacyChannel=false" + publishNCURLFormat = publishNCURLFmt + "&useLegacyChannel=false" } else { publishNCURLFormat = publishNCURLFmt } @@ -91,7 +91,7 @@ func (p *Proxy) PublishNC(ctx context.Context, ncParams cns.NetworkContainerPara func (p *Proxy) UnpublishNC(ctx context.Context, ncParams cns.NetworkContainerParameters, payload []byte, useRNCPublisher bool) (*http.Response, error) { var unpublishNCURLFormat string if useRNCPublisher { - unpublishNCURLFormat = unpublishNCURLFmt + "?useLegacyChannel=false" + unpublishNCURLFormat = unpublishNCURLFmt + "&useLegacyChannel=false" } else { unpublishNCURLFormat = unpublishNCURLFmt } diff --git a/cns/wireserver/proxy_test.go b/cns/wireserver/proxy_test.go new file mode 100644 index 00000000000..ea1432191fe --- /dev/null +++ b/cns/wireserver/proxy_test.go @@ -0,0 +1,153 @@ +package wireserver + +import ( + "bytes" + "context" + "io" + "net/http" + "net/url" + "testing" + + "github.com/Azure/azure-container-networking/cns" + "github.com/stretchr/testify/require" +) + +type testDo struct { + do func(*http.Request) (*http.Response, error) +} + +func (t *testDo) Do(req *http.Request) (*http.Response, error) { + return t.do(req) +} + +func TestProxyRNCPublisherQueryParam(t *testing.T) { + tests := []struct { + name string + call func(*Proxy) error + expectedFlag string + expectLegacySwitch bool + expectedTypePath string + }{ + { + name: "JoinNetwork adds useLegacyChannel=false for RNC", + call: func(p *Proxy) error { + _, err := p.JoinNetwork(context.Background(), "vnet-1", true) + return err + }, + expectedFlag: "false", + expectLegacySwitch: true, + expectedTypePath: "NetworkManagement/joinedVirtualNetworks/vnet-1/api-version/1", + }, + { + name: "PublishNC adds useLegacyChannel=false for RNC", + call: func(p *Proxy) error { + _, err := p.PublishNC(context.Background(), cns.NetworkContainerParameters{ + AssociatedInterfaceID: "iface-1", + NCID: "nc-1", + AuthToken: "token-1", + }, []byte(`{}`), true) + return err + }, + expectedFlag: "false", + expectLegacySwitch: true, + expectedTypePath: "NetworkManagement/interfaces/iface-1/networkContainers/nc-1/authenticationToken/token-1/api-version/1", + }, + { + name: "UnpublishNC adds useLegacyChannel=false for RNC", + call: func(p *Proxy) error { + _, err := p.UnpublishNC(context.Background(), cns.NetworkContainerParameters{ + AssociatedInterfaceID: "iface-1", + NCID: "nc-1", + AuthToken: "token-1", + }, []byte(`{}`), true) + return err + }, + expectedFlag: "false", + expectLegacySwitch: true, + expectedTypePath: "NetworkManagement/interfaces/iface-1/networkContainers/nc-1/authenticationToken/token-1/api-version/1/method/DELETE", + }, + { + name: "JoinSubnet includes useLegacyChannel=false", + call: func(p *Proxy) error { + _, err := p.JoinSubnet(context.Background(), "vnet-1", "subnet-1", cns.NetworkContainerParameters{ + AuthToken: "token-1", + }) + return err + }, + expectedFlag: "false", + expectLegacySwitch: true, + expectedTypePath: "NetworkManagement/joinedVirtualNetworks/vnet-1/joinedSubnets/subnet-1/authenticationToken/token-1/api-version/1", + }, + { + name: "JoinNetwork does not include useLegacyChannel when RNC disabled", + call: func(p *Proxy) error { + _, err := p.JoinNetwork(context.Background(), "vnet-1", false) + return err + }, + expectLegacySwitch: false, + expectedTypePath: "NetworkManagement/joinedVirtualNetworks/vnet-1/api-version/1", + }, + { + name: "PublishNC does not include useLegacyChannel when RNC disabled", + call: func(p *Proxy) error { + _, err := p.PublishNC(context.Background(), cns.NetworkContainerParameters{ + AssociatedInterfaceID: "iface-1", + NCID: "nc-1", + AuthToken: "token-1", + }, []byte(`{}`), false) + return err + }, + expectLegacySwitch: false, + expectedTypePath: "NetworkManagement/interfaces/iface-1/networkContainers/nc-1/authenticationToken/token-1/api-version/1", + }, + { + name: "UnpublishNC does not include useLegacyChannel when RNC disabled", + call: func(p *Proxy) error { + _, err := p.UnpublishNC(context.Background(), cns.NetworkContainerParameters{ + AssociatedInterfaceID: "iface-1", + NCID: "nc-1", + AuthToken: "token-1", + }, []byte(`{}`), false) + return err + }, + expectLegacySwitch: false, + expectedTypePath: "NetworkManagement/interfaces/iface-1/networkContainers/nc-1/authenticationToken/token-1/api-version/1/method/DELETE", + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + var reqURL *url.URL + + p := &Proxy{ + Host: "127.0.0.1:9001", + HTTPClient: &testDo{ + do: func(req *http.Request) (*http.Response, error) { + reqURL = req.URL + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(bytes.NewBufferString(`{}`)), + }, nil + }, + }, + } + + err := tt.call(p) + require.NoError(t, err) + require.NotNil(t, reqURL) + + q := reqURL.Query() + typeVal := q.Get("type") + require.NotContains(t, typeVal, "?useLegacyChannel=false") + require.Equal(t, tt.expectedTypePath, typeVal) + + if tt.expectLegacySwitch { + require.Equal(t, tt.expectedFlag, q.Get("useLegacyChannel")) + } else { + _, exists := q["useLegacyChannel"] + require.False(t, exists) + } + }) + } +} From 99a9bbcdc12dfefc372922db7fd70c4fd159f3b1 Mon Sep 17 00:00:00 2001 From: "Saksham Mittal (from Dev Box)" Date: Thu, 13 Aug 2026 10:37:17 -0700 Subject: [PATCH 07/10] test fix --- cni/network/network_windows_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cni/network/network_windows_test.go b/cni/network/network_windows_test.go index 4ee5785e8c1..1b32dcf6d32 100644 --- a/cni/network/network_windows_test.go +++ b/cni/network/network_windows_test.go @@ -1284,7 +1284,7 @@ func TestPluginWindowsAdd(t *testing.T) { NetNsPath: "bc526fae-4ba0-4e80-bc90-ad721e5850bf", NetNs: "bc526fae-4ba0-4e80-bc90-ad721e5850bf", HostSubnetPrefix: "", - Options: map[string]interface{}{}, + Options: nil, // matches with cns ip configuration IPAddresses: []net.IPNet{ { From 9f0f8eeb82e875c6b707fa74684f76f760b97afb Mon Sep 17 00:00:00 2001 From: "Saksham Mittal (from Dev Box)" Date: Thu, 13 Aug 2026 10:56:26 -0700 Subject: [PATCH 08/10] revert test change --- cni/network/network_windows_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cni/network/network_windows_test.go b/cni/network/network_windows_test.go index 1b32dcf6d32..4ee5785e8c1 100644 --- a/cni/network/network_windows_test.go +++ b/cni/network/network_windows_test.go @@ -1284,7 +1284,7 @@ func TestPluginWindowsAdd(t *testing.T) { NetNsPath: "bc526fae-4ba0-4e80-bc90-ad721e5850bf", NetNs: "bc526fae-4ba0-4e80-bc90-ad721e5850bf", HostSubnetPrefix: "", - Options: nil, + Options: map[string]interface{}{}, // matches with cns ip configuration IPAddresses: []net.IPNet{ { From 09d670732da8634dd876fe936e2d52b5f38d3b8e Mon Sep 17 00:00:00 2001 From: "Saksham Mittal (from Dev Box)" Date: Thu, 13 Aug 2026 11:13:23 -0700 Subject: [PATCH 09/10] lint fixes --- cns/restserver/api_test.go | 1 - cns/restserver/util_test.go | 11 +++-- cns/wireserver/proxy_test.go | 86 +++++++++++++++++++----------------- 3 files changed, 53 insertions(+), 45 deletions(-) diff --git a/cns/restserver/api_test.go b/cns/restserver/api_test.go index b021e4fbe8b..3305cda5b7a 100644 --- a/cns/restserver/api_test.go +++ b/cns/restserver/api_test.go @@ -315,7 +315,6 @@ func TestSetOrchestratorType_NCsPresent(t *testing.T) { }, } for _, tt := range tests { - tt := tt t.Run(tt.name, func(t *testing.T) { var resp cns.Response // Since this is global, we have to replace the state diff --git a/cns/restserver/util_test.go b/cns/restserver/util_test.go index b1cc5ab26e7..5faaff3a76e 100644 --- a/cns/restserver/util_test.go +++ b/cns/restserver/util_test.go @@ -186,11 +186,16 @@ func TestRestoreState(t *testing.T) { } func TestRestoreStateIgnoresLegacyJoinedSubnetsField(t *testing.T) { + const ( + underlayNetworkType = "Underlay" + vnetID = "vnet1" + ) + mainStore := store.NewMockStore("") require.NoError(t, mainStore.Write(storeKey, map[string]any{ - "NetworkType": "Underlay", + "NetworkType": underlayNetworkType, "joinedNetworks": map[string]struct{}{ - "vnet1": {}, + vnetID: {}, }, "joinedSubnets": map[string]struct{}{ "vnet1_subnet1": {}, @@ -207,7 +212,7 @@ func TestRestoreStateIgnoresLegacyJoinedSubnetsField(t *testing.T) { svc.restoreState() - require.Equal(t, "Underlay", svc.state.NetworkType) + require.Equal(t, underlayNetworkType, svc.state.NetworkType) require.Nil(t, svc.state.joinedNetworks) } diff --git a/cns/wireserver/proxy_test.go b/cns/wireserver/proxy_test.go index ea1432191fe..b94dac051d5 100644 --- a/cns/wireserver/proxy_test.go +++ b/cns/wireserver/proxy_test.go @@ -20,95 +20,95 @@ func (t *testDo) Do(req *http.Request) (*http.Response, error) { return t.do(req) } +const ( + useLegacyChannelFalse = "false" + interfaceID = "iface-1" + networkContainerID = "nc-1" + authToken = "token-1" +) + func TestProxyRNCPublisherQueryParam(t *testing.T) { tests := []struct { name string - call func(*Proxy) error + call func(*Proxy) (*http.Response, error) expectedFlag string expectLegacySwitch bool expectedTypePath string }{ { name: "JoinNetwork adds useLegacyChannel=false for RNC", - call: func(p *Proxy) error { - _, err := p.JoinNetwork(context.Background(), "vnet-1", true) - return err + call: func(p *Proxy) (*http.Response, error) { + return p.JoinNetwork(context.Background(), "vnet-1", true) }, - expectedFlag: "false", + expectedFlag: useLegacyChannelFalse, expectLegacySwitch: true, expectedTypePath: "NetworkManagement/joinedVirtualNetworks/vnet-1/api-version/1", }, { name: "PublishNC adds useLegacyChannel=false for RNC", - call: func(p *Proxy) error { - _, err := p.PublishNC(context.Background(), cns.NetworkContainerParameters{ - AssociatedInterfaceID: "iface-1", - NCID: "nc-1", - AuthToken: "token-1", + call: func(p *Proxy) (*http.Response, error) { + return p.PublishNC(context.Background(), cns.NetworkContainerParameters{ + AssociatedInterfaceID: interfaceID, + NCID: networkContainerID, + AuthToken: authToken, }, []byte(`{}`), true) - return err }, - expectedFlag: "false", + expectedFlag: useLegacyChannelFalse, expectLegacySwitch: true, expectedTypePath: "NetworkManagement/interfaces/iface-1/networkContainers/nc-1/authenticationToken/token-1/api-version/1", }, { name: "UnpublishNC adds useLegacyChannel=false for RNC", - call: func(p *Proxy) error { - _, err := p.UnpublishNC(context.Background(), cns.NetworkContainerParameters{ - AssociatedInterfaceID: "iface-1", - NCID: "nc-1", - AuthToken: "token-1", + call: func(p *Proxy) (*http.Response, error) { + return p.UnpublishNC(context.Background(), cns.NetworkContainerParameters{ + AssociatedInterfaceID: interfaceID, + NCID: networkContainerID, + AuthToken: authToken, }, []byte(`{}`), true) - return err }, - expectedFlag: "false", + expectedFlag: useLegacyChannelFalse, expectLegacySwitch: true, expectedTypePath: "NetworkManagement/interfaces/iface-1/networkContainers/nc-1/authenticationToken/token-1/api-version/1/method/DELETE", }, { name: "JoinSubnet includes useLegacyChannel=false", - call: func(p *Proxy) error { - _, err := p.JoinSubnet(context.Background(), "vnet-1", "subnet-1", cns.NetworkContainerParameters{ - AuthToken: "token-1", + call: func(p *Proxy) (*http.Response, error) { + return p.JoinSubnet(context.Background(), "vnet-1", "subnet-1", cns.NetworkContainerParameters{ + AuthToken: authToken, }) - return err }, - expectedFlag: "false", + expectedFlag: useLegacyChannelFalse, expectLegacySwitch: true, expectedTypePath: "NetworkManagement/joinedVirtualNetworks/vnet-1/joinedSubnets/subnet-1/authenticationToken/token-1/api-version/1", }, { name: "JoinNetwork does not include useLegacyChannel when RNC disabled", - call: func(p *Proxy) error { - _, err := p.JoinNetwork(context.Background(), "vnet-1", false) - return err + call: func(p *Proxy) (*http.Response, error) { + return p.JoinNetwork(context.Background(), "vnet-1", false) }, expectLegacySwitch: false, expectedTypePath: "NetworkManagement/joinedVirtualNetworks/vnet-1/api-version/1", }, { name: "PublishNC does not include useLegacyChannel when RNC disabled", - call: func(p *Proxy) error { - _, err := p.PublishNC(context.Background(), cns.NetworkContainerParameters{ - AssociatedInterfaceID: "iface-1", - NCID: "nc-1", - AuthToken: "token-1", + call: func(p *Proxy) (*http.Response, error) { + return p.PublishNC(context.Background(), cns.NetworkContainerParameters{ + AssociatedInterfaceID: interfaceID, + NCID: networkContainerID, + AuthToken: authToken, }, []byte(`{}`), false) - return err }, expectLegacySwitch: false, expectedTypePath: "NetworkManagement/interfaces/iface-1/networkContainers/nc-1/authenticationToken/token-1/api-version/1", }, { name: "UnpublishNC does not include useLegacyChannel when RNC disabled", - call: func(p *Proxy) error { - _, err := p.UnpublishNC(context.Background(), cns.NetworkContainerParameters{ - AssociatedInterfaceID: "iface-1", - NCID: "nc-1", - AuthToken: "token-1", + call: func(p *Proxy) (*http.Response, error) { + return p.UnpublishNC(context.Background(), cns.NetworkContainerParameters{ + AssociatedInterfaceID: interfaceID, + NCID: networkContainerID, + AuthToken: authToken, }, []byte(`{}`), false) - return err }, expectLegacySwitch: false, expectedTypePath: "NetworkManagement/interfaces/iface-1/networkContainers/nc-1/authenticationToken/token-1/api-version/1/method/DELETE", @@ -116,7 +116,6 @@ func TestProxyRNCPublisherQueryParam(t *testing.T) { } for _, tt := range tests { - tt := tt t.Run(tt.name, func(t *testing.T) { var reqURL *url.URL @@ -133,8 +132,13 @@ func TestProxyRNCPublisherQueryParam(t *testing.T) { }, } - err := tt.call(p) + resp, err := tt.call(p) require.NoError(t, err) + t.Cleanup(func() { + if resp != nil && resp.Body != nil { + _ = resp.Body.Close() + } + }) require.NotNil(t, reqURL) q := reqURL.Query() From 1ced29eb62881ae0eefae2577c1e02b7133b42fc Mon Sep 17 00:00:00 2001 From: "Saksham Mittal (from Dev Box)" Date: Thu, 13 Aug 2026 11:20:03 -0700 Subject: [PATCH 10/10] lint fixes --- cns/restserver/api_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/cns/restserver/api_test.go b/cns/restserver/api_test.go index 3305cda5b7a..8c217d74532 100644 --- a/cns/restserver/api_test.go +++ b/cns/restserver/api_test.go @@ -956,7 +956,6 @@ func TestPublishNCRequestBodyParsingMatrix(t *testing.T) { } for _, tt := range tests { - tt := tt t.Run(tt.name, func(t *testing.T) { var ( joinSubnetCalls int @@ -1419,7 +1418,6 @@ func TestUnpublishViaCNSRequestBody(t *testing.T) { } for _, tt := range tests { - tt := tt t.Run(tt.name, func(t *testing.T) { errPublish := publishNCViaCNS(vnet, tt.ncID, createNetworkContainerURL) require.NoError(t, errPublish)