Merge pull request #2414 from dmcgowan/platform-arm-normalization

Fix arm platform matching

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

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:

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)
+			}
+			for _, mc := range testcase.matches {
+				if ok := m.Match(mc); !ok {
+					t.Fatalf("expected specifier %q matches %#v", testcase.input, mc)
 				}
-
-			if fmt.Sprint(m) != testcase.formatted {
-				t.Fatalf("unexpected matcher string:  %q != %q", fmt.Sprint(m), testcase.formatted)
 			}
 
 			formatted := Format(p)