From 0f11f3a6400e06a88e2511cad21b956574a26ba6 Mon Sep 17 00:00:00 2001 From: ZetaoZhuang Date: Thu, 14 Nov 2024 14:59:08 -0800 Subject: [PATCH 01/11] update for new home az contract --- cns/api.go | 1 + cns/restserver/homeazmonitor.go | 10 +++++++++- cns/restserver/homeazmonitor_test.go | 17 +++++++++++++++-- nmagent/client_test.go | 12 ++++++++++++ nmagent/responses.go | 3 ++- 5 files changed, 39 insertions(+), 4 deletions(-) diff --git a/cns/api.go b/cns/api.go index 9c0f418b41..f3252f605d 100644 --- a/cns/api.go +++ b/cns/api.go @@ -356,6 +356,7 @@ type NmAgentSupportedApisResponse struct { type HomeAzResponse struct { IsSupported bool `json:"isSupported"` HomeAz uint `json:"homeAz"` + APIVersion uint `json:"apiVersion"` } type GetHomeAzResponse struct { diff --git a/cns/restserver/homeazmonitor.go b/cns/restserver/homeazmonitor.go index 3fb21e590f..7923514144 100644 --- a/cns/restserver/homeazmonitor.go +++ b/cns/restserver/homeazmonitor.go @@ -154,7 +154,15 @@ func (h *HomeAzMonitor) Populate(ctx context.Context) { h.update(returnCode, returnMessage, cns.HomeAzResponse{IsSupported: true}) return } - h.update(types.Success, "Get Home Az succeeded", cns.HomeAzResponse{IsSupported: true, HomeAz: azResponse.HomeAz}) + // validate APIVersion, APIVersion is a uint, so its value >=0 + // 0 should be valid when NMA version is old and does not have the apiVersion value in home az response + if azResponse.APIVersion > 0 && azResponse.APIVersion != 2 { + returnMessage := fmt.Sprintf("[HomeAzMonitor] invalid APIVersion value from nmagent: %d", azResponse.APIVersion) + returnCode := types.UnexpectedError + h.update(returnCode, returnMessage, cns.HomeAzResponse{IsSupported: true}) + return + } + h.update(types.Success, "Get Home Az succeeded", cns.HomeAzResponse{IsSupported: true, HomeAz: azResponse.HomeAz, APIVersion: azResponse.APIVersion}) } // update constructs a GetHomeAzResponse entity and update its cache diff --git a/cns/restserver/homeazmonitor_test.go b/cns/restserver/homeazmonitor_test.go index b435f3cde1..77bcc19e1b 100644 --- a/cns/restserver/homeazmonitor_test.go +++ b/cns/restserver/homeazmonitor_test.go @@ -28,10 +28,10 @@ func TestHomeAzMonitor(t *testing.T) { return []string{"GetHomeAz"}, nil }, GetHomeAzF: func(ctx context.Context) (nmagent.AzResponse, error) { - return nmagent.AzResponse{HomeAz: uint(1)}, nil + return nmagent.AzResponse{HomeAz: uint(1), APIVersion: uint(2)}, nil }, }, - cns.HomeAzResponse{IsSupported: true, HomeAz: uint(1)}, + cns.HomeAzResponse{IsSupported: true, HomeAz: uint(1), APIVersion: uint(2)}, false, }, { @@ -60,6 +60,19 @@ func TestHomeAzMonitor(t *testing.T) { cns.HomeAzResponse{IsSupported: true}, true, }, + { + "api supported but apiVersion value is not valid", + &fakes.NMAgentClientFake{ + SupportedAPIsF: func(ctx context.Context) ([]string, error) { + return []string{GetHomeAzAPIName}, nil + }, + GetHomeAzF: func(ctx context.Context) (nmagent.AzResponse, error) { + return nmagent.AzResponse{HomeAz: uint(1), APIVersion: uint(3)}, nil + }, + }, + cns.HomeAzResponse{IsSupported: true}, + true, + }, { "api supported but got unexpected errors", &fakes.NMAgentClientFake{ diff --git a/nmagent/client_test.go b/nmagent/client_test.go index e4e0b36ece..33d270e5b8 100644 --- a/nmagent/client_test.go +++ b/nmagent/client_test.go @@ -756,6 +756,18 @@ func TestGetHomeAz(t *testing.T) { map[string]interface{}{ "httpStatusCode": "200", "HomeAz": 1, + "APIVersion": 0, + }, + false, + }, + { + "happy path with new version", + nmagent.AzResponse{HomeAz: uint(1), APIVersion: uint(2)}, + "/machine/plugins?comp=nmagent&type=GetHomeAz%2Fapi-version%2F1", + map[string]interface{}{ + "httpStatusCode": "200", + "HomeAz": 1, + "APIVersion": 2, }, false, }, diff --git a/nmagent/responses.go b/nmagent/responses.go index 4e917e8bc9..fcb9da11fc 100644 --- a/nmagent/responses.go +++ b/nmagent/responses.go @@ -38,7 +38,8 @@ type NCVersionList struct { } type AzResponse struct { - HomeAz uint `json:"homeAz"` + HomeAz uint `json:"homeAz"` + APIVersion uint `json:"apiVersion"` } type NodeIP struct { From 25e7ef19d01c00933c0617e027078560a40c65d5 Mon Sep 17 00:00:00 2001 From: Zetao Zhuang Date: Fri, 15 Nov 2024 00:21:04 -0800 Subject: [PATCH 02/11] fix lint --- cns/restserver/homeazmonitor_test.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/cns/restserver/homeazmonitor_test.go b/cns/restserver/homeazmonitor_test.go index 77bcc19e1b..f602a44e96 100644 --- a/cns/restserver/homeazmonitor_test.go +++ b/cns/restserver/homeazmonitor_test.go @@ -24,10 +24,10 @@ func TestHomeAzMonitor(t *testing.T) { { "happy path", &fakes.NMAgentClientFake{ - SupportedAPIsF: func(ctx context.Context) ([]string, error) { + SupportedAPIsF: func(_ context.Context) ([]string, error) { return []string{"GetHomeAz"}, nil }, - GetHomeAzF: func(ctx context.Context) (nmagent.AzResponse, error) { + GetHomeAzF: func(_ context.Context) (nmagent.AzResponse, error) { return nmagent.AzResponse{HomeAz: uint(1), APIVersion: uint(2)}, nil }, }, @@ -37,10 +37,10 @@ func TestHomeAzMonitor(t *testing.T) { { "getHomeAz is not supported in nmagent", &fakes.NMAgentClientFake{ - SupportedAPIsF: func(ctx context.Context) ([]string, error) { + SupportedAPIsF: func(_ context.Context) ([]string, error) { return []string{"dummy"}, nil }, - GetHomeAzF: func(ctx context.Context) (nmagent.AzResponse, error) { + GetHomeAzF: func(_ context.Context) (nmagent.AzResponse, error) { return nmagent.AzResponse{}, nil }, }, @@ -50,10 +50,10 @@ func TestHomeAzMonitor(t *testing.T) { { "api supported but home az value is not valid", &fakes.NMAgentClientFake{ - SupportedAPIsF: func(ctx context.Context) ([]string, error) { + SupportedAPIsF: func(_ context.Context) ([]string, error) { return []string{GetHomeAzAPIName}, nil }, - GetHomeAzF: func(ctx context.Context) (nmagent.AzResponse, error) { + GetHomeAzF: func(_ context.Context) (nmagent.AzResponse, error) { return nmagent.AzResponse{HomeAz: 0}, nil }, }, @@ -63,10 +63,10 @@ func TestHomeAzMonitor(t *testing.T) { { "api supported but apiVersion value is not valid", &fakes.NMAgentClientFake{ - SupportedAPIsF: func(ctx context.Context) ([]string, error) { + SupportedAPIsF: func(_ context.Context) ([]string, error) { return []string{GetHomeAzAPIName}, nil }, - GetHomeAzF: func(ctx context.Context) (nmagent.AzResponse, error) { + GetHomeAzF: func(_ context.Context) (nmagent.AzResponse, error) { return nmagent.AzResponse{HomeAz: uint(1), APIVersion: uint(3)}, nil }, }, @@ -76,10 +76,10 @@ func TestHomeAzMonitor(t *testing.T) { { "api supported but got unexpected errors", &fakes.NMAgentClientFake{ - SupportedAPIsF: func(ctx context.Context) ([]string, error) { + SupportedAPIsF: func(_ context.Context) ([]string, error) { return []string{GetHomeAzAPIName}, nil }, - GetHomeAzF: func(ctx context.Context) (nmagent.AzResponse, error) { + GetHomeAzF: func(_ context.Context) (nmagent.AzResponse, error) { return nmagent.AzResponse{}, errors.New("unexpected error") }, }, From 90301c25dfee2f72a5431a514a7becb2298bb82c Mon Sep 17 00:00:00 2001 From: Zetao Zhuang Date: Tue, 19 Nov 2024 11:39:01 -0800 Subject: [PATCH 03/11] address comment --- cns/api.go | 6 +++--- cns/restserver/homeazmonitor.go | 7 +++---- cns/restserver/homeazmonitor_test.go | 2 +- nmagent/responses.go | 9 +++++++++ 4 files changed, 16 insertions(+), 8 deletions(-) diff --git a/cns/api.go b/cns/api.go index f3252f605d..0427d4c94b 100644 --- a/cns/api.go +++ b/cns/api.go @@ -354,9 +354,9 @@ type NmAgentSupportedApisResponse struct { } type HomeAzResponse struct { - IsSupported bool `json:"isSupported"` - HomeAz uint `json:"homeAz"` - APIVersion uint `json:"apiVersion"` + IsSupported bool `json:"isSupported"` + HomeAz uint `json:"homeAz"` + NmaAppliedTheIPV6Fix bool `json:"NmaAppliedTheIPV6Fix"` } type GetHomeAzResponse struct { diff --git a/cns/restserver/homeazmonitor.go b/cns/restserver/homeazmonitor.go index 7923514144..a2810f8157 100644 --- a/cns/restserver/homeazmonitor.go +++ b/cns/restserver/homeazmonitor.go @@ -154,15 +154,14 @@ func (h *HomeAzMonitor) Populate(ctx context.Context) { h.update(returnCode, returnMessage, cns.HomeAzResponse{IsSupported: true}) return } - // validate APIVersion, APIVersion is a uint, so its value >=0 - // 0 should be valid when NMA version is old and does not have the apiVersion value in home az response - if azResponse.APIVersion > 0 && azResponse.APIVersion != 2 { + // validate APIVersion value + if !azResponse.Valid() { returnMessage := fmt.Sprintf("[HomeAzMonitor] invalid APIVersion value from nmagent: %d", azResponse.APIVersion) returnCode := types.UnexpectedError h.update(returnCode, returnMessage, cns.HomeAzResponse{IsSupported: true}) return } - h.update(types.Success, "Get Home Az succeeded", cns.HomeAzResponse{IsSupported: true, HomeAz: azResponse.HomeAz, APIVersion: azResponse.APIVersion}) + h.update(types.Success, "Get Home Az succeeded", cns.HomeAzResponse{IsSupported: true, HomeAz: azResponse.HomeAz, NmaAppliedTheIPV6Fix: azResponse.NmaAppliedTheIPV6Fix()}) } // update constructs a GetHomeAzResponse entity and update its cache diff --git a/cns/restserver/homeazmonitor_test.go b/cns/restserver/homeazmonitor_test.go index f602a44e96..6ef03ebb25 100644 --- a/cns/restserver/homeazmonitor_test.go +++ b/cns/restserver/homeazmonitor_test.go @@ -31,7 +31,7 @@ func TestHomeAzMonitor(t *testing.T) { return nmagent.AzResponse{HomeAz: uint(1), APIVersion: uint(2)}, nil }, }, - cns.HomeAzResponse{IsSupported: true, HomeAz: uint(1), APIVersion: uint(2)}, + cns.HomeAzResponse{IsSupported: true, HomeAz: uint(1), NmaAppliedTheIPV6Fix: true}, false, }, { diff --git a/nmagent/responses.go b/nmagent/responses.go index fcb9da11fc..bdcad65db6 100644 --- a/nmagent/responses.go +++ b/nmagent/responses.go @@ -42,6 +42,15 @@ type AzResponse struct { APIVersion uint `json:"apiVersion"` } +func (az AzResponse) Valid() bool { + // 0 should be valid when NMA version is old and does not have the apiVersion value in home az response + return az.APIVersion == 0 || az.APIVersion == 2 +} + +func (az AzResponse) NmaAppliedTheIPV6Fix() bool { + return az.APIVersion == 2 +} + type NodeIP struct { Address IPAddress `xml:"Address,attr"` IsPrimary bool `xml:"IsPrimary,attr"` From a1d5a079c2e18d900d762be7a13f8c2203b95c7d Mon Sep 17 00:00:00 2001 From: Zetao Zhuang Date: Tue, 19 Nov 2024 12:04:51 -0800 Subject: [PATCH 04/11] fix lint --- nmagent/responses.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/nmagent/responses.go b/nmagent/responses.go index bdcad65db6..8f830d85d8 100644 --- a/nmagent/responses.go +++ b/nmagent/responses.go @@ -44,10 +44,12 @@ type AzResponse struct { func (az AzResponse) Valid() bool { // 0 should be valid when NMA version is old and does not have the apiVersion value in home az response + //nolint:gomnd // these magic numbers are made by nma design return az.APIVersion == 0 || az.APIVersion == 2 } func (az AzResponse) NmaAppliedTheIPV6Fix() bool { + //nolint:gomnd // this magic number is made by nma design return az.APIVersion == 2 } From 4e356fcc56aa161fc0d209b1d1a096756e1325a7 Mon Sep 17 00:00:00 2001 From: Zetao Zhuang Date: Wed, 20 Nov 2024 11:52:56 -0800 Subject: [PATCH 05/11] address comment --- cns/restserver/homeazmonitor_test.go | 2 +- nmagent/responses.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cns/restserver/homeazmonitor_test.go b/cns/restserver/homeazmonitor_test.go index 6ef03ebb25..54ec2b2838 100644 --- a/cns/restserver/homeazmonitor_test.go +++ b/cns/restserver/homeazmonitor_test.go @@ -67,7 +67,7 @@ func TestHomeAzMonitor(t *testing.T) { return []string{GetHomeAzAPIName}, nil }, GetHomeAzF: func(_ context.Context) (nmagent.AzResponse, error) { - return nmagent.AzResponse{HomeAz: uint(1), APIVersion: uint(3)}, nil + return nmagent.AzResponse{HomeAz: uint(1), APIVersion: uint(1)}, nil }, }, cns.HomeAzResponse{IsSupported: true}, diff --git a/nmagent/responses.go b/nmagent/responses.go index 8f830d85d8..846681df4f 100644 --- a/nmagent/responses.go +++ b/nmagent/responses.go @@ -50,7 +50,7 @@ func (az AzResponse) Valid() bool { func (az AzResponse) NmaAppliedTheIPV6Fix() bool { //nolint:gomnd // this magic number is made by nma design - return az.APIVersion == 2 + return az.APIVersion >= 2 } type NodeIP struct { From eb9c641ccfd12d1ea471e980f9a5e4116cb3430f Mon Sep 17 00:00:00 2001 From: "Timothy J. Raymond" Date: Wed, 11 Dec 2024 12:57:22 -0500 Subject: [PATCH 06/11] Add "AppliedFixes" to HomeAzResponse The NMAgent team has added an API version field to the HomeAZ response as a means to indicate whether or not certain bugfixes have been applied. Since this API Version field conveys no meaningful information, this adds an "AppliedFixes" field to the response to make it meaningful to users. --- nmagent/client_test.go | 12 ---- nmagent/error.go | 8 +++ nmagent/responses.go | 67 ++++++++++++++++--- nmagent/responses_test.go | 137 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 203 insertions(+), 21 deletions(-) create mode 100644 nmagent/responses_test.go diff --git a/nmagent/client_test.go b/nmagent/client_test.go index 33d270e5b8..e4e0b36ece 100644 --- a/nmagent/client_test.go +++ b/nmagent/client_test.go @@ -756,18 +756,6 @@ func TestGetHomeAz(t *testing.T) { map[string]interface{}{ "httpStatusCode": "200", "HomeAz": 1, - "APIVersion": 0, - }, - false, - }, - { - "happy path with new version", - nmagent.AzResponse{HomeAz: uint(1), APIVersion: uint(2)}, - "/machine/plugins?comp=nmagent&type=GetHomeAz%2Fapi-version%2F1", - map[string]interface{}{ - "httpStatusCode": "200", - "HomeAz": 1, - "APIVersion": 2, }, false, }, diff --git a/nmagent/error.go b/nmagent/error.go index 582e4fca31..ef50a969a2 100644 --- a/nmagent/error.go +++ b/nmagent/error.go @@ -11,6 +11,14 @@ import ( pkgerrors "github.com/pkg/errors" ) +type HomeAzAPIVersionError struct { + ReceivedAPIVersion uint +} + +func (h HomeAzAPIVersionError) Error() string { + return fmt.Sprintf("invalid homeaz api version (must be 0 or 2): received %d", h.ReceivedAPIVersion) +} + var deleteNetworkPattern = regexp.MustCompile(`/NetworkManagement/joinedVirtualNetworks/[^/]+/api-version/\d+/method/DELETE`) // ContentError is encountered when an unexpected content type is obtained from diff --git a/nmagent/responses.go b/nmagent/responses.go index 846681df4f..4dc6bffd09 100644 --- a/nmagent/responses.go +++ b/nmagent/responses.go @@ -1,5 +1,11 @@ package nmagent +import ( + "encoding/json" + + "github.com/pkg/errors" +) + type VirtualNetwork struct { CNetSpace string `json:"cnetSpace"` DefaultGateway string `json:"defaultGateway"` @@ -37,20 +43,63 @@ type NCVersionList struct { Containers []NCVersion `json:"networkContainers"` } +// HomeAZFix is an indication that a particular bugfix has been applied to some +// HomeAZ. +type HomeAZFix int + +const ( + HomeAZFixInvalid HomeAZFix = iota + HomeAZFixIPv6 +) + type AzResponse struct { - HomeAz uint `json:"homeAz"` - APIVersion uint `json:"apiVersion"` + HomeAz uint + AppliedFixes []HomeAZFix } -func (az AzResponse) Valid() bool { - // 0 should be valid when NMA version is old and does not have the apiVersion value in home az response - //nolint:gomnd // these magic numbers are made by nma design - return az.APIVersion == 0 || az.APIVersion == 2 +func (az *AzResponse) UnmarshalJSON(in []byte) error { + type resp struct { + HomeAz uint `json:"homeAz"` + APIVersion uint `json:"apiVersion"` + } + + var rsp resp + err := json.Unmarshal(in, &rsp) + if err != nil { + return errors.Wrap(err, "unmarshaling raw home az response") + } + + if rsp.APIVersion != 0 && rsp.APIVersion != 2 { + return HomeAzAPIVersionError{ + ReceivedAPIVersion: rsp.APIVersion, + } + } + + az.HomeAz = rsp.HomeAz + + if rsp.APIVersion == 2 { + az.AppliedFixes = append(az.AppliedFixes, HomeAZFixIPv6) + } + + return nil } -func (az AzResponse) NmaAppliedTheIPV6Fix() bool { - //nolint:gomnd // this magic number is made by nma design - return az.APIVersion >= 2 +// ContainsFixes reports whether all fixes requested are present in the +// AzResponse returned. +func (az AzResponse) ContainsFixes(requestedFixes ...HomeAZFix) bool { + for _, requested := range requestedFixes { + found := false + for _, present := range az.AppliedFixes { + if requested == present { + found = true + } + } + + if !found { + return false + } + } + return true } type NodeIP struct { diff --git a/nmagent/responses_test.go b/nmagent/responses_test.go new file mode 100644 index 0000000000..86455eebc1 --- /dev/null +++ b/nmagent/responses_test.go @@ -0,0 +1,137 @@ +package nmagent_test + +import ( + "encoding/json" + "testing" + + "github.com/Azure/azure-container-networking/nmagent" + "github.com/google/go-cmp/cmp" +) + +func TestContainsFixes(t *testing.T) { + tests := []struct { + name string + resp nmagent.AzResponse + fixes []nmagent.HomeAZFix + exp bool + }{ + { + "empty", + nmagent.AzResponse{}, + []nmagent.HomeAZFix{}, + true, + }, + { + "one present", + nmagent.AzResponse{ + AppliedFixes: []nmagent.HomeAZFix{ + nmagent.HomeAZFixIPv6, + }, + }, + []nmagent.HomeAZFix{nmagent.HomeAZFixIPv6}, + true, + }, + { + "one absent", + nmagent.AzResponse{ + AppliedFixes: []nmagent.HomeAZFix{}, + }, + []nmagent.HomeAZFix{nmagent.HomeAZFixIPv6}, + false, + }, + { + "one with empty request", + nmagent.AzResponse{ + AppliedFixes: []nmagent.HomeAZFix{ + nmagent.HomeAZFixIPv6, + }, + }, + []nmagent.HomeAZFix{}, + true, + }, + } + + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + t.Parallel() + got := test.resp.ContainsFixes(test.fixes...) + + exp := test.exp + if got != exp { + t.Error("unexpected response from ContainsFixes: exp:", exp, "got:", got) + } + }) + } +} + +func TestUnmarshalAzResponse(t *testing.T) { + tests := []struct { + name string + in string + exp nmagent.AzResponse + shouldErr bool + }{ + { + "empty", + "{}", + nmagent.AzResponse{}, + false, + }, + { + "only homeaz", + `{"homeAz": 42}`, + nmagent.AzResponse{ + HomeAz: 42, + }, + false, + }, + { + "valid apiversion", + `{"homeAz": 42, "apiVersion": 0}`, + nmagent.AzResponse{ + HomeAz: 42, + }, + false, + }, + { + "valid apiversion ipv6", + `{"homeAz": 42, "apiVersion": 2}`, + nmagent.AzResponse{ + HomeAz: 42, + AppliedFixes: []nmagent.HomeAZFix{ + nmagent.HomeAZFixIPv6, + }, + }, + false, + }, + { + "invalid apiversion", + `{"homeAz": 42, "apiVersion": 42}`, + nmagent.AzResponse{}, + true, + }, + } + + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + var got nmagent.AzResponse + err := json.Unmarshal([]byte(test.in), &got) + if err != nil && !test.shouldErr { + t.Fatal("unexpected error unmarshaling JSON: err:", err) + } + + if err == nil && test.shouldErr { + t.Fatal("expected error but received none") + } + + exp := test.exp + if !cmp.Equal(got, exp) { + t.Error("received response differs from expected: diff:", cmp.Diff(got, exp)) + } + }) + } +} From 50c6509c60b31545a2dd8a334d1e7ad1d4c09deb Mon Sep 17 00:00:00 2001 From: "Timothy J. Raymond" Date: Wed, 11 Dec 2024 13:01:03 -0500 Subject: [PATCH 07/11] Implement fmt.Stringer for HomeAZFix This is to ensure that these are prett-printed properly for clients that log them. --- nmagent/responses.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/nmagent/responses.go b/nmagent/responses.go index 4dc6bffd09..de9ee2f3d0 100644 --- a/nmagent/responses.go +++ b/nmagent/responses.go @@ -47,6 +47,17 @@ type NCVersionList struct { // HomeAZ. type HomeAZFix int +func (h HomeAZFix) String() string { + switch h { + case HomeAZFixInvalid: + return "HomeAZFixInvalid" + case HomeAZFixIPv6: + return "HomeAZFixIPv6" + default: + return "Unknown HomeAZ Fix" + } +} + const ( HomeAZFixInvalid HomeAZFix = iota HomeAZFixIPv6 From ae215763e584276af4f4091d088539123dc7db51 Mon Sep 17 00:00:00 2001 From: "Timothy J. Raymond" Date: Wed, 11 Dec 2024 13:07:17 -0500 Subject: [PATCH 08/11] Use nmagent.(HomeAZResponse).ContainsFixes in CNS This alters the response to use the ContainsFixes method added to the HomeAzResponse. Rather than using API Version directly, the more semantically-meaningful constants from the nmagent package are used instead to indicate whether or not some bugfixes have been applied. --- cns/restserver/homeazmonitor.go | 9 +-------- cns/restserver/homeazmonitor_test.go | 4 ++-- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/cns/restserver/homeazmonitor.go b/cns/restserver/homeazmonitor.go index a2810f8157..f73c6e35af 100644 --- a/cns/restserver/homeazmonitor.go +++ b/cns/restserver/homeazmonitor.go @@ -154,14 +154,7 @@ func (h *HomeAzMonitor) Populate(ctx context.Context) { h.update(returnCode, returnMessage, cns.HomeAzResponse{IsSupported: true}) return } - // validate APIVersion value - if !azResponse.Valid() { - returnMessage := fmt.Sprintf("[HomeAzMonitor] invalid APIVersion value from nmagent: %d", azResponse.APIVersion) - returnCode := types.UnexpectedError - h.update(returnCode, returnMessage, cns.HomeAzResponse{IsSupported: true}) - return - } - h.update(types.Success, "Get Home Az succeeded", cns.HomeAzResponse{IsSupported: true, HomeAz: azResponse.HomeAz, NmaAppliedTheIPV6Fix: azResponse.NmaAppliedTheIPV6Fix()}) + h.update(types.Success, "Get Home Az succeeded", cns.HomeAzResponse{IsSupported: true, HomeAz: azResponse.HomeAz, NmaAppliedTheIPV6Fix: azResponse.ContainsFixes(nmagent.HomeAZFixIPv6)}) } // update constructs a GetHomeAzResponse entity and update its cache diff --git a/cns/restserver/homeazmonitor_test.go b/cns/restserver/homeazmonitor_test.go index 54ec2b2838..5373fc47d1 100644 --- a/cns/restserver/homeazmonitor_test.go +++ b/cns/restserver/homeazmonitor_test.go @@ -28,7 +28,7 @@ func TestHomeAzMonitor(t *testing.T) { return []string{"GetHomeAz"}, nil }, GetHomeAzF: func(_ context.Context) (nmagent.AzResponse, error) { - return nmagent.AzResponse{HomeAz: uint(1), APIVersion: uint(2)}, nil + return nmagent.AzResponse{HomeAz: uint(1), AppliedFixes: []nmagent.HomeAZFix{nmagent.HomeAZFixIPv6}}, nil }, }, cns.HomeAzResponse{IsSupported: true, HomeAz: uint(1), NmaAppliedTheIPV6Fix: true}, @@ -67,7 +67,7 @@ func TestHomeAzMonitor(t *testing.T) { return []string{GetHomeAzAPIName}, nil }, GetHomeAzF: func(_ context.Context) (nmagent.AzResponse, error) { - return nmagent.AzResponse{HomeAz: uint(1), APIVersion: uint(1)}, nil + return nmagent.AzResponse{HomeAz: uint(1), AppliedFixes: []nmagent.HomeAZFix{nmagent.HomeAZFixIPv6}}, nil }, }, cns.HomeAzResponse{IsSupported: true}, From b3d79c8c7a2a236d1012918252f4f5edd4666284 Mon Sep 17 00:00:00 2001 From: Zetao Zhuang Date: Sun, 15 Dec 2024 23:32:07 -0800 Subject: [PATCH 09/11] add nma client test --- nmagent/client_test.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/nmagent/client_test.go b/nmagent/client_test.go index e4e0b36ece..8ecbab92b3 100644 --- a/nmagent/client_test.go +++ b/nmagent/client_test.go @@ -751,7 +751,7 @@ func TestGetHomeAz(t *testing.T) { }{ { "happy path", - nmagent.AzResponse{HomeAz: uint(1)}, + nmagent.AzResponse{HomeAz: uint(1), AppliedFixes: nil}, "/machine/plugins?comp=nmagent&type=GetHomeAz%2Fapi-version%2F1", map[string]interface{}{ "httpStatusCode": "200", @@ -759,6 +759,17 @@ func TestGetHomeAz(t *testing.T) { }, false, }, + { + "happy path with new version", + nmagent.AzResponse{HomeAz: uint(1), AppliedFixes: []nmagent.HomeAZFix{nmagent.HomeAZFixIPv6}}, + "/machine/plugins?comp=nmagent&type=GetHomeAz%2Fapi-version%2F1", + map[string]interface{}{ + "httpStatusCode": "200", + "HomeAz": 1, + "APIVersion": 2, + }, + false, + }, { "empty response", nmagent.AzResponse{}, From f8d82dd67eeb8fb4849c8165ba63b930c0963da1 Mon Sep 17 00:00:00 2001 From: Zetao Zhuang Date: Sun, 15 Dec 2024 23:55:35 -0800 Subject: [PATCH 10/11] fix lint --- nmagent/responses.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nmagent/responses.go b/nmagent/responses.go index de9ee2f3d0..8a806ad24b 100644 --- a/nmagent/responses.go +++ b/nmagent/responses.go @@ -88,7 +88,7 @@ func (az *AzResponse) UnmarshalJSON(in []byte) error { az.HomeAz = rsp.HomeAz - if rsp.APIVersion == 2 { + if rsp.APIVersion == 2 { // nolint:gomnd // ignore magic number 2 az.AppliedFixes = append(az.AppliedFixes, HomeAZFixIPv6) } From d238c53ef90243419a0676709506d551f5da8084 Mon Sep 17 00:00:00 2001 From: Zetao Zhuang Date: Thu, 19 Dec 2024 12:02:52 -0800 Subject: [PATCH 11/11] fix UTs --- cns/restserver/homeazmonitor_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cns/restserver/homeazmonitor_test.go b/cns/restserver/homeazmonitor_test.go index 5373fc47d1..241f2089db 100644 --- a/cns/restserver/homeazmonitor_test.go +++ b/cns/restserver/homeazmonitor_test.go @@ -67,11 +67,11 @@ func TestHomeAzMonitor(t *testing.T) { return []string{GetHomeAzAPIName}, nil }, GetHomeAzF: func(_ context.Context) (nmagent.AzResponse, error) { - return nmagent.AzResponse{HomeAz: uint(1), AppliedFixes: []nmagent.HomeAZFix{nmagent.HomeAZFixIPv6}}, nil + return nmagent.AzResponse{HomeAz: uint(1), AppliedFixes: []nmagent.HomeAZFix{nmagent.HomeAZFixInvalid}}, nil }, }, - cns.HomeAzResponse{IsSupported: true}, - true, + cns.HomeAzResponse{IsSupported: true, HomeAz: uint(1), NmaAppliedTheIPV6Fix: false}, + false, }, { "api supported but got unexpected errors",