From 37ab93e2c89fd8bc1e08c914246926fabf902893 Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Thu, 21 Jun 2018 16:04:03 -0700 Subject: [PATCH] Fix arm platform matching 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 --- platforms/database.go | 23 ++++++-- platforms/platforms.go | 8 ++- platforms/platforms_test.go | 107 ++++++++++++++++++++++++++++++++++-- 3 files changed, 128 insertions(+), 10 deletions(-) diff --git a/platforms/database.go b/platforms/database.go index df9cc2392..6eda1f18d 100644 --- 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 index 1f91ed288..6743dae03 100644 --- 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 index 1a1be3650..e72f71b54 100644 --- 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)