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.

Author: timraymond

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

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,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 {

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