From 5da214fd1676a144ad130be9f66088968ea46dd9 Mon Sep 17 00:00:00 2001 From: smittal22 Date: Tue, 28 Feb 2023 12:13:46 -0800 Subject: [PATCH 1/9] initial delete NC changes to include body --- cns/NetworkContainerContract.go | 9 +++++---- cns/restserver/api_test.go | 9 +++++---- cns/wireserver/proxy.go | 4 ++-- nmagent/requests.go | 4 +++- 4 files changed, 15 insertions(+), 11 deletions(-) diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index afb7978b9b..3d7530dac2 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -529,10 +529,11 @@ func (p PublishNetworkContainerResponse) String() string { // UnpublishNetworkContainerRequest specifies request to unpublish network container via NMAgent. type UnpublishNetworkContainerRequest struct { - NetworkID string - NetworkContainerID string - JoinNetworkURL string - DeleteNetworkContainerURL string + NetworkID string + NetworkContainerID string + JoinNetworkURL string + DeleteNetworkContainerURL string + DeleteNetworkContainerRequestBody []byte } // UnpublishNetworkContainerResponse specifies the response to unpublish network container request. diff --git a/cns/restserver/api_test.go b/cns/restserver/api_test.go index fb6b39c216..b6c9d465c0 100644 --- a/cns/restserver/api_test.go +++ b/cns/restserver/api_test.go @@ -1035,10 +1035,11 @@ func unpublishNCViaCNS(networkID, networkContainerID, deleteNetworkContainerURL joinNetworkURL := "http://" + nmagentEndpoint + "/dummyVnetURL" unpublishNCRequest := &cns.UnpublishNetworkContainerRequest{ - NetworkID: networkID, - NetworkContainerID: networkContainerID, - JoinNetworkURL: joinNetworkURL, - DeleteNetworkContainerURL: deleteNetworkContainerURL, + NetworkID: networkID, + NetworkContainerID: networkContainerID, + JoinNetworkURL: joinNetworkURL, + DeleteNetworkContainerURL: deleteNetworkContainerURL, + DeleteNetworkContainerRequestBody: []byte("{}"), } var body bytes.Buffer diff --git a/cns/wireserver/proxy.go b/cns/wireserver/proxy.go index ad2540c453..d3d74e9cf6 100644 --- a/cns/wireserver/proxy.go +++ b/cns/wireserver/proxy.go @@ -57,10 +57,10 @@ func (p *Proxy) PublishNC(ctx context.Context, ncParams cns.NetworkContainerPara return resp, nil } -func (p *Proxy) UnpublishNC(ctx context.Context, ncParams cns.NetworkContainerParameters) (*http.Response, error) { +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) - req, err := http.NewRequestWithContext(ctx, http.MethodPost, reqURL, bytes.NewBufferString(`""`)) + req, err := http.NewRequestWithContext(ctx, http.MethodPost, reqURL, bytes.NewBuffer(payload)) if err != nil { return nil, errors.Wrap(err, "wireserver proxy: unpublish nc: could not build http request") } diff --git a/nmagent/requests.go b/nmagent/requests.go index c520c3b773..16083ce673 100644 --- a/nmagent/requests.go +++ b/nmagent/requests.go @@ -278,7 +278,9 @@ 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 + NCID string `json:"-"` // the Network Container ID + AzID uint `json:"azID"` // home AZ of the Network Container + EnableAZR bool `json:"enableAZR"` // whether AZR is enabled or not // PrimaryAddress is the primary customer address of the interface in the // management VNET From 6e3818ea6f208e885698f1d2ab9fe793421871f2 Mon Sep 17 00:00:00 2001 From: smittal22 Date: Tue, 28 Feb 2023 12:18:47 -0800 Subject: [PATCH 2/9] initial delete NC changes to include body --- cns/fakes/wireserverproxyfake.go | 6 +++--- cns/restserver/api.go | 2 +- cns/restserver/restserver.go | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/cns/fakes/wireserverproxyfake.go b/cns/fakes/wireserverproxyfake.go index 2e8bd3195a..e6e52e7c54 100644 --- a/cns/fakes/wireserverproxyfake.go +++ b/cns/fakes/wireserverproxyfake.go @@ -12,7 +12,7 @@ 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) (*http.Response, error) + UnpublishNCFunc func(context.Context, cns.NetworkContainerParameters, []byte) (*http.Response, error) } const defaultResponseBody = `{"httpStatusCode":"200"}` @@ -41,9 +41,9 @@ func (w *WireserverProxyFake) PublishNC(ctx context.Context, ncParams cns.Networ return defaultResponse(), nil } -func (w *WireserverProxyFake) UnpublishNC(ctx context.Context, ncParams cns.NetworkContainerParameters) (*http.Response, error) { +func (w *WireserverProxyFake) UnpublishNC(ctx context.Context, ncParams cns.NetworkContainerParameters, payload []byte) (*http.Response, error) { if w.UnpublishNCFunc != nil { - return w.UnpublishNCFunc(ctx, ncParams) + return w.UnpublishNCFunc(ctx, ncParams, payload) } return defaultResponse(), nil diff --git a/cns/restserver/api.go b/cns/restserver/api.go index eb8de07981..171657c666 100644 --- a/cns/restserver/api.go +++ b/cns/restserver/api.go @@ -1298,7 +1298,7 @@ func (service *HTTPRestService) unpublishNetworkContainer(w http.ResponseWriter, logger.Printf("[Azure-CNS] joined vnet %s during nc %s unpublish. wireserver response: %v", req.NetworkID, req.NetworkContainerID, string(joinBytes)) } - publishResp, err := service.wsproxy.UnpublishNC(ctx, ncParams) + publishResp, err := service.wsproxy.UnpublishNC(ctx, ncParams, req.DeleteNetworkContainerRequestBody) if err != nil { resp := cns.UnpublishNetworkContainerResponse{ Response: cns.Response{ diff --git a/cns/restserver/restserver.go b/cns/restserver/restserver.go index 743a5e6408..3ec5359a6a 100644 --- a/cns/restserver/restserver.go +++ b/cns/restserver/restserver.go @@ -47,7 +47,7 @@ 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) (*http.Response, error) + UnpublishNC(ctx context.Context, ncParams cns.NetworkContainerParameters, payload []byte) (*http.Response, error) } // HTTPRestService represents http listener for CNS - Container Networking Service. From 5fc67843a0359cae26dcb92070d66bf657a0190f Mon Sep 17 00:00:00 2001 From: smittal22 Date: Tue, 28 Feb 2023 12:39:33 -0800 Subject: [PATCH 3/9] lint error --- cns/restserver/api_test.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/cns/restserver/api_test.go b/cns/restserver/api_test.go index b6c9d465c0..a60289a5c3 100644 --- a/cns/restserver/api_test.go +++ b/cns/restserver/api_test.go @@ -955,7 +955,7 @@ func TestUnpublishNCViaCNS(t *testing.T) { func TestUnpublishNCViaCNS401(t *testing.T) { wsproxy := fakes.WireserverProxyFake{ - UnpublishNCFunc: func(_ context.Context, _ cns.NetworkContainerParameters) (*http.Response, error) { + UnpublishNCFunc: func(_ context.Context, _ cns.NetworkContainerParameters, i []byte) (*http.Response, error) { return &http.Response{ StatusCode: http.StatusOK, Body: io.NopCloser(bytes.NewBufferString(`{"httpStatusCode":"401"}`)), @@ -974,10 +974,11 @@ func TestUnpublishNCViaCNS401(t *testing.T) { joinNetworkURL := "http://" + nmagentEndpoint + "/dummyVnetURL" unpublishNCRequest := &cns.UnpublishNetworkContainerRequest{ - NetworkID: networkID, - NetworkContainerID: networkContainerID, - JoinNetworkURL: joinNetworkURL, - DeleteNetworkContainerURL: deleteNetworkContainerURL, + NetworkID: networkID, + NetworkContainerID: networkContainerID, + JoinNetworkURL: joinNetworkURL, + DeleteNetworkContainerURL: deleteNetworkContainerURL, + DeleteNetworkContainerRequestBody: []byte("{}"), } var body bytes.Buffer From 010c833e90af2121f0c644ac2764cb174583fb0f Mon Sep 17 00:00:00 2001 From: smittal22 Date: Tue, 28 Feb 2023 12:13:46 -0800 Subject: [PATCH 4/9] initial delete NC changes to include body --- cns/NetworkContainerContract.go | 9 +++++---- cns/restserver/api_test.go | 9 +++++---- cns/wireserver/proxy.go | 4 ++-- nmagent/requests.go | 4 +++- 4 files changed, 15 insertions(+), 11 deletions(-) diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index afb7978b9b..3d7530dac2 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -529,10 +529,11 @@ func (p PublishNetworkContainerResponse) String() string { // UnpublishNetworkContainerRequest specifies request to unpublish network container via NMAgent. type UnpublishNetworkContainerRequest struct { - NetworkID string - NetworkContainerID string - JoinNetworkURL string - DeleteNetworkContainerURL string + NetworkID string + NetworkContainerID string + JoinNetworkURL string + DeleteNetworkContainerURL string + DeleteNetworkContainerRequestBody []byte } // UnpublishNetworkContainerResponse specifies the response to unpublish network container request. diff --git a/cns/restserver/api_test.go b/cns/restserver/api_test.go index fb6b39c216..b6c9d465c0 100644 --- a/cns/restserver/api_test.go +++ b/cns/restserver/api_test.go @@ -1035,10 +1035,11 @@ func unpublishNCViaCNS(networkID, networkContainerID, deleteNetworkContainerURL joinNetworkURL := "http://" + nmagentEndpoint + "/dummyVnetURL" unpublishNCRequest := &cns.UnpublishNetworkContainerRequest{ - NetworkID: networkID, - NetworkContainerID: networkContainerID, - JoinNetworkURL: joinNetworkURL, - DeleteNetworkContainerURL: deleteNetworkContainerURL, + NetworkID: networkID, + NetworkContainerID: networkContainerID, + JoinNetworkURL: joinNetworkURL, + DeleteNetworkContainerURL: deleteNetworkContainerURL, + DeleteNetworkContainerRequestBody: []byte("{}"), } var body bytes.Buffer diff --git a/cns/wireserver/proxy.go b/cns/wireserver/proxy.go index ad2540c453..d3d74e9cf6 100644 --- a/cns/wireserver/proxy.go +++ b/cns/wireserver/proxy.go @@ -57,10 +57,10 @@ func (p *Proxy) PublishNC(ctx context.Context, ncParams cns.NetworkContainerPara return resp, nil } -func (p *Proxy) UnpublishNC(ctx context.Context, ncParams cns.NetworkContainerParameters) (*http.Response, error) { +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) - req, err := http.NewRequestWithContext(ctx, http.MethodPost, reqURL, bytes.NewBufferString(`""`)) + req, err := http.NewRequestWithContext(ctx, http.MethodPost, reqURL, bytes.NewBuffer(payload)) if err != nil { return nil, errors.Wrap(err, "wireserver proxy: unpublish nc: could not build http request") } diff --git a/nmagent/requests.go b/nmagent/requests.go index c520c3b773..16083ce673 100644 --- a/nmagent/requests.go +++ b/nmagent/requests.go @@ -278,7 +278,9 @@ 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 + NCID string `json:"-"` // the Network Container ID + AzID uint `json:"azID"` // home AZ of the Network Container + EnableAZR bool `json:"enableAZR"` // whether AZR is enabled or not // PrimaryAddress is the primary customer address of the interface in the // management VNET From 2506ba7280b2aa7b7d49d0e36f133baa8493314f Mon Sep 17 00:00:00 2001 From: smittal22 Date: Tue, 28 Feb 2023 12:18:47 -0800 Subject: [PATCH 5/9] initial delete NC changes to include body --- cns/fakes/wireserverproxyfake.go | 6 +++--- cns/restserver/api.go | 2 +- cns/restserver/restserver.go | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/cns/fakes/wireserverproxyfake.go b/cns/fakes/wireserverproxyfake.go index 2e8bd3195a..e6e52e7c54 100644 --- a/cns/fakes/wireserverproxyfake.go +++ b/cns/fakes/wireserverproxyfake.go @@ -12,7 +12,7 @@ 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) (*http.Response, error) + UnpublishNCFunc func(context.Context, cns.NetworkContainerParameters, []byte) (*http.Response, error) } const defaultResponseBody = `{"httpStatusCode":"200"}` @@ -41,9 +41,9 @@ func (w *WireserverProxyFake) PublishNC(ctx context.Context, ncParams cns.Networ return defaultResponse(), nil } -func (w *WireserverProxyFake) UnpublishNC(ctx context.Context, ncParams cns.NetworkContainerParameters) (*http.Response, error) { +func (w *WireserverProxyFake) UnpublishNC(ctx context.Context, ncParams cns.NetworkContainerParameters, payload []byte) (*http.Response, error) { if w.UnpublishNCFunc != nil { - return w.UnpublishNCFunc(ctx, ncParams) + return w.UnpublishNCFunc(ctx, ncParams, payload) } return defaultResponse(), nil diff --git a/cns/restserver/api.go b/cns/restserver/api.go index eb8de07981..171657c666 100644 --- a/cns/restserver/api.go +++ b/cns/restserver/api.go @@ -1298,7 +1298,7 @@ func (service *HTTPRestService) unpublishNetworkContainer(w http.ResponseWriter, logger.Printf("[Azure-CNS] joined vnet %s during nc %s unpublish. wireserver response: %v", req.NetworkID, req.NetworkContainerID, string(joinBytes)) } - publishResp, err := service.wsproxy.UnpublishNC(ctx, ncParams) + publishResp, err := service.wsproxy.UnpublishNC(ctx, ncParams, req.DeleteNetworkContainerRequestBody) if err != nil { resp := cns.UnpublishNetworkContainerResponse{ Response: cns.Response{ diff --git a/cns/restserver/restserver.go b/cns/restserver/restserver.go index 743a5e6408..3ec5359a6a 100644 --- a/cns/restserver/restserver.go +++ b/cns/restserver/restserver.go @@ -47,7 +47,7 @@ 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) (*http.Response, error) + UnpublishNC(ctx context.Context, ncParams cns.NetworkContainerParameters, payload []byte) (*http.Response, error) } // HTTPRestService represents http listener for CNS - Container Networking Service. From acf694c7a3f890441b6d0e797aecf394a1edade8 Mon Sep 17 00:00:00 2001 From: smittal22 Date: Tue, 28 Feb 2023 12:39:33 -0800 Subject: [PATCH 6/9] lint error --- cns/restserver/api_test.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/cns/restserver/api_test.go b/cns/restserver/api_test.go index b6c9d465c0..a60289a5c3 100644 --- a/cns/restserver/api_test.go +++ b/cns/restserver/api_test.go @@ -955,7 +955,7 @@ func TestUnpublishNCViaCNS(t *testing.T) { func TestUnpublishNCViaCNS401(t *testing.T) { wsproxy := fakes.WireserverProxyFake{ - UnpublishNCFunc: func(_ context.Context, _ cns.NetworkContainerParameters) (*http.Response, error) { + UnpublishNCFunc: func(_ context.Context, _ cns.NetworkContainerParameters, i []byte) (*http.Response, error) { return &http.Response{ StatusCode: http.StatusOK, Body: io.NopCloser(bytes.NewBufferString(`{"httpStatusCode":"401"}`)), @@ -974,10 +974,11 @@ func TestUnpublishNCViaCNS401(t *testing.T) { joinNetworkURL := "http://" + nmagentEndpoint + "/dummyVnetURL" unpublishNCRequest := &cns.UnpublishNetworkContainerRequest{ - NetworkID: networkID, - NetworkContainerID: networkContainerID, - JoinNetworkURL: joinNetworkURL, - DeleteNetworkContainerURL: deleteNetworkContainerURL, + NetworkID: networkID, + NetworkContainerID: networkContainerID, + JoinNetworkURL: joinNetworkURL, + DeleteNetworkContainerURL: deleteNetworkContainerURL, + DeleteNetworkContainerRequestBody: []byte("{}"), } var body bytes.Buffer From 651b8cf131623a580dd978a4a12e12cca46f438c Mon Sep 17 00:00:00 2001 From: smittal22 Date: Wed, 1 Mar 2023 09:17:58 -0800 Subject: [PATCH 7/9] test change --- cns/wireserver/proxy.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/cns/wireserver/proxy.go b/cns/wireserver/proxy.go index d3d74e9cf6..817e3010b6 100644 --- a/cns/wireserver/proxy.go +++ b/cns/wireserver/proxy.go @@ -60,7 +60,13 @@ func (p *Proxy) PublishNC(ctx context.Context, ncParams cns.NetworkContainerPara 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) - req, err := http.NewRequestWithContext(ctx, http.MethodPost, reqURL, bytes.NewBuffer(payload)) + var body []byte + if len(payload) != 0 { + body = payload + } else { + body = []byte("") + } + req, err := http.NewRequestWithContext(ctx, http.MethodPost, reqURL, bytes.NewBuffer(body)) if err != nil { return nil, errors.Wrap(err, "wireserver proxy: unpublish nc: could not build http request") } From 5523e8405916796f8e850544e4a465bbf43440b6 Mon Sep 17 00:00:00 2001 From: smittal22 Date: Wed, 1 Mar 2023 09:36:08 -0800 Subject: [PATCH 8/9] test change --- cns/wireserver/proxy.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/cns/wireserver/proxy.go b/cns/wireserver/proxy.go index e236cdb76f..f51d077393 100644 --- a/cns/wireserver/proxy.go +++ b/cns/wireserver/proxy.go @@ -60,13 +60,11 @@ func (p *Proxy) PublishNC(ctx context.Context, ncParams cns.NetworkContainerPara 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) - var body []byte - if len(payload) != 0 { + body := []byte(`""`) + if len(payload) > 0 { body = payload - } else { - body = []byte("") } - + req, err := http.NewRequestWithContext(ctx, http.MethodPost, reqURL, bytes.NewBuffer(body)) if err != nil { return nil, errors.Wrap(err, "wireserver proxy: unpublish nc: could not build http request") From 28697addf6de81fa0d9dfa431d8514f029db0a33 Mon Sep 17 00:00:00 2001 From: smittal22 Date: Wed, 1 Mar 2023 09:38:47 -0800 Subject: [PATCH 9/9] add comment --- cns/wireserver/proxy.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cns/wireserver/proxy.go b/cns/wireserver/proxy.go index f51d077393..d180ff9ccc 100644 --- a/cns/wireserver/proxy.go +++ b/cns/wireserver/proxy.go @@ -60,6 +60,8 @@ func (p *Proxy) PublishNC(ctx context.Context, ncParams cns.NetworkContainerPara 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) + // a POST to wireserver must contain a body. For legacy purposes, + // an empty json string (two quote characters) should be sent by default. body := []byte(`""`) if len(payload) > 0 { body = payload