diff --git a/cns/api.go b/cns/api.go index 9c0f418b41..0427d4c94b 100644 --- a/cns/api.go +++ b/cns/api.go @@ -354,8 +354,9 @@ type NmAgentSupportedApisResponse struct { } type HomeAzResponse struct { - IsSupported bool `json:"isSupported"` - HomeAz uint `json:"homeAz"` + 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 3fb21e590f..f73c6e35af 100644 --- a/cns/restserver/homeazmonitor.go +++ b/cns/restserver/homeazmonitor.go @@ -154,7 +154,7 @@ 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}) + 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 b435f3cde1..241f2089db 100644 --- a/cns/restserver/homeazmonitor_test.go +++ b/cns/restserver/homeazmonitor_test.go @@ -24,23 +24,23 @@ 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) { - return nmagent.AzResponse{HomeAz: uint(1)}, nil + GetHomeAzF: func(_ context.Context) (nmagent.AzResponse, error) { + return nmagent.AzResponse{HomeAz: uint(1), AppliedFixes: []nmagent.HomeAZFix{nmagent.HomeAZFixIPv6}}, nil }, }, - cns.HomeAzResponse{IsSupported: true, HomeAz: uint(1)}, + cns.HomeAzResponse{IsSupported: true, HomeAz: uint(1), NmaAppliedTheIPV6Fix: true}, false, }, { "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,23 +50,36 @@ 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 }, }, cns.HomeAzResponse{IsSupported: true}, true, }, + { + "api supported but apiVersion value is not valid", + &fakes.NMAgentClientFake{ + SupportedAPIsF: func(_ context.Context) ([]string, error) { + return []string{GetHomeAzAPIName}, nil + }, + GetHomeAzF: func(_ context.Context) (nmagent.AzResponse, error) { + return nmagent.AzResponse{HomeAz: uint(1), AppliedFixes: []nmagent.HomeAZFix{nmagent.HomeAZFixInvalid}}, nil + }, + }, + cns.HomeAzResponse{IsSupported: true, HomeAz: uint(1), NmaAppliedTheIPV6Fix: false}, + false, + }, { "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") }, }, 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{}, 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 4e917e8bc9..8a806ad24b 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,8 +43,74 @@ type NCVersionList struct { Containers []NCVersion `json:"networkContainers"` } +// HomeAZFix is an indication that a particular bugfix has been applied to some +// 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 +) + type AzResponse struct { - HomeAz uint `json:"homeAz"` + HomeAz uint + AppliedFixes []HomeAZFix +} + +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 { // nolint:gomnd // ignore magic number 2 + az.AppliedFixes = append(az.AppliedFixes, HomeAZFixIPv6) + } + + return nil +} + +// 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)) + } + }) + } +}