diff --git a/cns/restserver/api.go b/cns/restserver/api.go index 07cfc283c8..848ce8d6ba 100644 --- a/cns/restserver/api.go +++ b/cns/restserver/api.go @@ -4,6 +4,7 @@ package restserver import ( + "bytes" "context" "encoding/json" "fmt" @@ -20,6 +21,7 @@ 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" ) @@ -1029,7 +1031,28 @@ func (service *HTTPRestService) unpublishNetworkContainer(w http.ResponseWriter, ctx := r.Context() - if !service.isNetworkJoined(req.NetworkID) { + var unpublishBody nmagent.DeleteContainerRequest + var azrNC bool + err = json.Unmarshal(req.DeleteNetworkContainerRequestBody, &unpublishBody) + if err != nil { + // If the body contains only `""\n`, it is non-AZR NC + // In this case, we should not return an error + // However, if the body is not `""\n`, it is invalid and therefore, we must return an error + // []byte{34, 34, 10} here represents []byte(`""`+"\n") + if !bytes.Equal(req.DeleteNetworkContainerRequestBody, []byte{34, 34, 10}) { + http.Error(w, fmt.Sprintf("could not unmarshal delete network container body: %v", err), http.StatusBadRequest) + return + } + } else { + // If unmarshalling was successful, it is an AZR NC + azrNC = true + } + + /* For AZR scenarios, if NMAgent is restarted, it loses state and does not know what VNETs to subscribe to. + As it no longer has VNET state, delete nc calls would fail. We need to add join VNET call for all AZR + 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 if err != nil { resp := cns.UnpublishNetworkContainerResponse{ @@ -1062,7 +1085,7 @@ func (service *HTTPRestService) unpublishNetworkContainer(w http.ResponseWriter, } service.setNetworkStateJoined(req.NetworkID) - logger.Printf("[Azure-CNS] joined vnet %s during nc %s unpublish. wireserver response: %v", req.NetworkID, req.NetworkContainerID, string(joinBytes)) + 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) diff --git a/cns/restserver/api_test.go b/cns/restserver/api_test.go index 002eddfd84..98d5f3dbb4 100644 --- a/cns/restserver/api_test.go +++ b/cns/restserver/api_test.go @@ -1057,7 +1057,7 @@ func TestUnpublishNCViaCNS(t *testing.T) { "/machine/plugins/?comp=nmagent&type=NetworkManagement/interfaces/dummyIntf/networkContainers/dummyNCURL/authenticationToke/" + "8636c99d-7861-401f-b0d3-7e5b7dc8183c" + "/api-version/1/method/DELETE" - err = unpublishNCViaCNS("vnet1", "ethWebApp", deleteNetworkContainerURL) + err = unpublishNCViaCNS("vnet1", "ethWebApp", deleteNetworkContainerURL, []byte(`""`+"\n")) if err == nil { t.Fatal("Expected a bad request error due to delete network url being incorrect") } @@ -1068,7 +1068,7 @@ func TestUnpublishNCViaCNS(t *testing.T) { "/machine/plugins/?comp=nmagent&NetworkManagement/interfaces/dummyIntf/networkContainers/dummyNCURL/authenticationToken/" + "8636c99d-7861-401f-b0d3-7e5b7dc8183c8636c99d-7861-401f-b0d3-7e5b7dc8183c" + "/api-version/1/method/DELETE" - err = unpublishNCViaCNS("vnet1", "ethWebApp", deleteNetworkContainerURL) + err = unpublishNCViaCNS("vnet1", "ethWebApp", deleteNetworkContainerURL, []byte(`""`+"\n")) if err == nil { t.Fatal("Expected a bad request error due to create network url having more characters than permitted in auth token") } @@ -1076,12 +1076,64 @@ func TestUnpublishNCViaCNS(t *testing.T) { // now actually perform the deletion: deleteNetworkContainerURL = "http://" + nmagentEndpoint + "/machine/plugins/?comp=nmagent&type=NetworkManagement/interfaces/dummyIntf/networkContainers/dummyNCURL/authenticationToken/dummyT/api-version/1/method/DELETE" - err = unpublishNCViaCNS("vnet1", "ethWebApp", deleteNetworkContainerURL) + err = unpublishNCViaCNS("vnet1", "ethWebApp", deleteNetworkContainerURL, []byte(`""`+"\n")) if err != nil { t.Fatal(err) } } +func TestUnpublishViaCNSRequestBody(t *testing.T) { + createNetworkContainerURL := "http://" + nmagentEndpoint + + "/machine/plugins/?comp=nmagent&type=NetworkManagement/interfaces/dummyIntf/networkContainers/dummyNCURL/authenticationToken/dummyT/api-version/1" + deleteNetworkContainerURL := "http://" + nmagentEndpoint + + "/machine/plugins/?comp=nmagent&type=NetworkManagement/interfaces/dummyIntf/networkContainers/dummyNCURL/authenticationToken/dummyT/api-version/1/method/DELETE" + vnet := "vnet1" + wsProxy := fakes.WireserverProxyFake{} + cleanup := setWireserverProxy(svc, &wsProxy) + defer cleanup() + + tests := []struct { + name string + ncID string + body []byte + requireError bool + }{ + { + name: "Delete NC with invalid body", + ncID: "ncID1", + body: []byte(`invalid` + "\n"), + requireError: true, + }, + { + name: "Delete NC with valid non-AZR body", + ncID: "ncID2", + body: []byte(`""` + "\n"), + requireError: false, + }, + { + name: "Delete NC with valid AZR body", + ncID: "ncID3", + body: []byte(`{"azID":1,"azrEnabled":true}`), + requireError: false, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + errPublish := publishNCViaCNS(vnet, tt.ncID, createNetworkContainerURL) + require.NoError(t, errPublish) + errUnpublish := unpublishNCViaCNS(vnet, tt.ncID, deleteNetworkContainerURL, tt.body) + if tt.requireError { + require.Error(t, errUnpublish) + require.Contains(t, errUnpublish.Error(), "error decoding json") + } else { + require.NoError(t, errUnpublish) + } + }) + } +} + func TestUnpublishNCViaCNS401(t *testing.T) { wsproxy := fakes.WireserverProxyFake{ UnpublishNCFunc: func(_ context.Context, _ cns.NetworkContainerParameters, i []byte) (*http.Response, error) { @@ -1161,7 +1213,7 @@ func TestUnpublishNCViaCNS401(t *testing.T) { } } -func unpublishNCViaCNS(networkID, networkContainerID, deleteNetworkContainerURL string) error { +func unpublishNCViaCNS(networkID, networkContainerID, deleteNetworkContainerURL string, bodyBytes []byte) error { joinNetworkURL := "http://" + nmagentEndpoint + "/dummyVnetURL" unpublishNCRequest := &cns.UnpublishNetworkContainerRequest{ @@ -1169,7 +1221,7 @@ func unpublishNCViaCNS(networkID, networkContainerID, deleteNetworkContainerURL NetworkContainerID: networkContainerID, JoinNetworkURL: joinNetworkURL, DeleteNetworkContainerURL: deleteNetworkContainerURL, - DeleteNetworkContainerRequestBody: []byte("{}"), + DeleteNetworkContainerRequestBody: bodyBytes, } var body bytes.Buffer