update for new home az contract from nma (#3151)

* update for new home az contract

* fix lint

* address comment

* fix lint

* address comment

* 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.

* Implement fmt.Stringer for HomeAZFix

This is to ensure that these are prett-printed properly for clients that
log them.

* 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.

* add nma client test

* fix lint

* fix UTs

---------

Co-authored-by: Timothy J. Raymond <traymond@microsoft.com>

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 {

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

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")
 				},
 			},

nmagent/client_test.go

@@ -751,14 +751,25 @@ 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",
 				"HomeAz":         1,
 			},
 			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{},

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

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 {

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))
+			}
+		})
+	}
+}