Fix arm platform matching

dmcgowan · dmcgowan · commit f2709edd1a20 · 2018-06-22T13:18:36.000-07:00
The normalization was being inconsistently applied causing a failure to match some platforms in manifest lists. Fix the matcher and normalization to be more consistent and add changes to parser to prevent the defaulted variants from being set in the platform structure. Signed-off-by: Derek McGowan <derek@mcgstyle.net> (cherry picked from commit 37ab93e)
diff --git a/platforms/database.go b/platforms/database.go
@@ -89,18 +89,21 @@ func normalizeArch(arch, variant string) (string, string) {
 	case "x86_64", "x86-64":
 		arch = "amd64"
 		variant = ""
-	case "aarch64":
+	case "aarch64", "arm64":
 		arch = "arm64"
-		variant = "" // v8 is implied
+		switch variant {
+		case "", "8":
+			variant = "v8"
+		}
 	case "armhf":
 		arch = "arm"
-		variant = ""
+		variant = "v7"
 	case "armel":
 		arch = "arm"
 		variant = "v6"
 	case "arm":
 		switch variant {
-		case "v7", "7":
+		case "", "7":
 			variant = "v7"
 		case "5", "6", "8":
 			variant = "v" + variant
@@ -109,3 +112,15 @@ func normalizeArch(arch, variant string) (string, string) {
 
 	return arch, variant
 }
+
+// defaultVariant detects default variants on normalized arch/variant
+func defaultVariant(arch, variant string) bool {
+	switch arch {
+	case "arm64":
+		return variant == "v8"
+	case "arm":
+		return variant == "v7"
+	default:
+		return true
+	}
+}
diff --git a/platforms/platforms.go b/platforms/platforms.go
@@ -135,7 +135,7 @@ type Matcher interface {
 // Applications should opt to use `Match` over directly parsing specifiers.
 func NewMatcher(platform specs.Platform) Matcher {
 	return &matcher{
-		Platform: platform,
+		Platform: Normalize(platform),
 	}
 }
 
@@ -197,6 +197,9 @@ func Parse(specifier string) (specs.Platform, error) {
 		}
 
 		p.Architecture, p.Variant = normalizeArch(parts[0], "")
+		if defaultVariant(p.Architecture, p.Variant) {
+			p.Variant = ""
+		}
 		if isKnownArch(p.Architecture) {
 			p.OS = runtime.GOOS
 			return p, nil
@@ -208,6 +211,9 @@ func Parse(specifier string) (specs.Platform, error) {
 		// about whether or not we know of the platform.
 		p.OS = normalizeOS(parts[0])
 		p.Architecture, p.Variant = normalizeArch(parts[1], "")
+		if defaultVariant(p.Architecture, p.Variant) {
+			p.Variant = ""
+		}
 
 		return p, nil
 	case 3:
diff --git a/platforms/platforms_test.go b/platforms/platforms_test.go
@@ -17,7 +17,6 @@
 package platforms
 
 import (
-	"fmt"
 	"reflect"
 	"runtime"
 	"testing"
@@ -35,6 +34,7 @@ func TestParseSelector(t *testing.T) {
 		skip      bool
 		input     string
 		expected  specs.Platform
+		matches   []specs.Platform
 		formatted string
 	}{
 		// While wildcards are a valid use case for platform selection,
@@ -66,8 +66,72 @@ func TestParseSelector(t *testing.T) {
 				OS:           "*",
 				Architecture: "arm64",
 			},
+			matches: []specs.Platform{
+				{
+					OS:           "*",
+					Architecture: "aarch64",
+				},
+				{
+					OS:           "*",
+					Architecture: "aarch64",
+					Variant:      "v8",
+				},
+				{
+					OS:           "*",
+					Architecture: "arm64",
+					Variant:      "v8",
+				},
+			},
 			formatted: "*/arm64",
 		},
+		{
+			input: "linux/arm64",
+			expected: specs.Platform{
+				OS:           "linux",
+				Architecture: "arm64",
+			},
+			matches: []specs.Platform{
+				{
+					OS:           "linux",
+					Architecture: "aarch64",
+				},
+				{
+					OS:           "linux",
+					Architecture: "aarch64",
+					Variant:      "v8",
+				},
+				{
+					OS:           "linux",
+					Architecture: "arm64",
+					Variant:      "v8",
+				},
+			},
+			formatted: "linux/arm64",
+		},
+		{
+			input: "linux/arm64/v8",
+			expected: specs.Platform{
+				OS:           "linux",
+				Architecture: "arm64",
+				Variant:      "v8",
+			},
+			matches: []specs.Platform{
+				{
+					OS:           "linux",
+					Architecture: "aarch64",
+				},
+				{
+					OS:           "linux",
+					Architecture: "aarch64",
+					Variant:      "v8",
+				},
+				{
+					OS:           "linux",
+					Architecture: "arm64",
+				},
+			},
+			formatted: "linux/arm64/v8",
+		},
 		{
 			// NOTE(stevvooe): In this case, the consumer can assume this is v7
 			// but we leave the variant blank. This will represent the vast
@@ -77,6 +141,22 @@ func TestParseSelector(t *testing.T) {
 				OS:           "linux",
 				Architecture: "arm",
 			},
+			matches: []specs.Platform{
+				{
+					OS:           "linux",
+					Architecture: "arm",
+					Variant:      "v7",
+				},
+				{
+					OS:           "linux",
+					Architecture: "armhf",
+				},
+				{
+					OS:           "linux",
+					Architecture: "arm",
+					Variant:      "7",
+				},
+			},
 			formatted: "linux/arm",
 		},
 		{
@@ -86,6 +166,12 @@ func TestParseSelector(t *testing.T) {
 				Architecture: "arm",
 				Variant:      "v6",
 			},
+			matches: []specs.Platform{
+				{
+					OS:           "linux",
+					Architecture: "armel",
+				},
+			},
 			formatted: "linux/arm/v6",
 		},
 		{
@@ -95,6 +181,16 @@ func TestParseSelector(t *testing.T) {
 				Architecture: "arm",
 				Variant:      "v7",
 			},
+			matches: []specs.Platform{
+				{
+					OS:           "linux",
+					Architecture: "arm",
+				},
+				{
+					OS:           "linux",
+					Architecture: "armhf",
+				},
+			},
 			formatted: "linux/arm/v7",
 		},
 		{
@@ -204,11 +300,12 @@ func TestParseSelector(t *testing.T) {
 
 			// ensure that match works on the input to the output.
 			if ok := m.Match(testcase.expected); !ok {
-				t.Fatalf("expected specifier %q matches %v", testcase.input, testcase.expected)
+				t.Fatalf("expected specifier %q matches %#v", testcase.input, testcase.expected)
 			}
-
-			if fmt.Sprint(m) != testcase.formatted {
-				t.Fatalf("unexpected matcher string:  %q != %q", fmt.Sprint(m), testcase.formatted)
+			for _, mc := range testcase.matches {
+				if ok := m.Match(mc); !ok {
+					t.Fatalf("expected specifier %q matches %#v", testcase.input, mc)
+				}
 			}
 
 			formatted := Format(p)
