Skip to content

Commit 37ab93e

Browse files
dmcgowandmcgowan
committed
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 <[email protected]>
1 parent 7ff2748 commit 37ab93e

3 files changed

Lines changed: 128 additions & 10 deletions

File tree

‎platforms/database.go‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -89,18 +89,21 @@ func normalizeArch(arch, variant string) (string, string) {
8989
case "x86_64", "x86-64":
9090
arch = "amd64"
9191
variant = ""
92-
case "aarch64":
92+
case "aarch64", "arm64":
9393
arch = "arm64"
94-
variant = "" // v8 is implied
94+
switch variant {
95+
case "", "8":
96+
variant = "v8"
97+
}
9598
case "armhf":
9699
arch = "arm"
97-
variant = ""
100+
variant = "v7"
98101
case "armel":
99102
arch = "arm"
100103
variant = "v6"
101104
case "arm":
102105
switch variant {
103-
case "v7", "7":
106+
case "", "7":
104107
variant = "v7"
105108
case "5", "6", "8":
106109
variant = "v" + variant
@@ -109,3 +112,15 @@ func normalizeArch(arch, variant string) (string, string) {
109112

110113
return arch, variant
111114
}
115+
116+
// defaultVariant detects default variants on normalized arch/variant
117+
func defaultVariant(arch, variant string) bool {
118+
switch arch {
119+
case "arm64":
120+
return variant == "v8"
121+
case "arm":
122+
return variant == "v7"
123+
default:
124+
return true
125+
}
126+
}

‎platforms/platforms.go‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ type Matcher interface {
135135
// Applications should opt to use `Match` over directly parsing specifiers.
136136
func NewMatcher(platform specs.Platform) Matcher {
137137
return &matcher{
138-
Platform: platform,
138+
Platform: Normalize(platform),
139139
}
140140
}
141141

@@ -197,6 +197,9 @@ func Parse(specifier string) (specs.Platform, error) {
197197
}
198198

199199
p.Architecture, p.Variant = normalizeArch(parts[0], "")
200+
if defaultVariant(p.Architecture, p.Variant) {
201+
p.Variant = ""
202+
}
200203
if isKnownArch(p.Architecture) {
201204
p.OS = runtime.GOOS
202205
return p, nil
@@ -208,6 +211,9 @@ func Parse(specifier string) (specs.Platform, error) {
208211
// about whether or not we know of the platform.
209212
p.OS = normalizeOS(parts[0])
210213
p.Architecture, p.Variant = normalizeArch(parts[1], "")
214+
if defaultVariant(p.Architecture, p.Variant) {
215+
p.Variant = ""
216+
}
211217

212218
return p, nil
213219
case 3:

‎platforms/platforms_test.go‎

Lines changed: 102 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
package platforms
1818

1919
import (
20-
"fmt"
2120
"reflect"
2221
"runtime"
2322
"testing"
@@ -35,6 +34,7 @@ func TestParseSelector(t *testing.T) {
3534
skip bool
3635
input string
3736
expected specs.Platform
37+
matches []specs.Platform
3838
formatted string
3939
}{
4040
// While wildcards are a valid use case for platform selection,
@@ -66,8 +66,72 @@ func TestParseSelector(t *testing.T) {
6666
OS: "*",
6767
Architecture: "arm64",
6868
},
69+
matches: []specs.Platform{
70+
{
71+
OS: "*",
72+
Architecture: "aarch64",
73+
},
74+
{
75+
OS: "*",
76+
Architecture: "aarch64",
77+
Variant: "v8",
78+
},
79+
{
80+
OS: "*",
81+
Architecture: "arm64",
82+
Variant: "v8",
83+
},
84+
},
6985
formatted: "*/arm64",
7086
},
87+
{
88+
input: "linux/arm64",
89+
expected: specs.Platform{
90+
OS: "linux",
91+
Architecture: "arm64",
92+
},
93+
matches: []specs.Platform{
94+
{
95+
OS: "linux",
96+
Architecture: "aarch64",
97+
},
98+
{
99+
OS: "linux",
100+
Architecture: "aarch64",
101+
Variant: "v8",
102+
},
103+
{
104+
OS: "linux",
105+
Architecture: "arm64",
106+
Variant: "v8",
107+
},
108+
},
109+
formatted: "linux/arm64",
110+
},
111+
{
112+
input: "linux/arm64/v8",
113+
expected: specs.Platform{
114+
OS: "linux",
115+
Architecture: "arm64",
116+
Variant: "v8",
117+
},
118+
matches: []specs.Platform{
119+
{
120+
OS: "linux",
121+
Architecture: "aarch64",
122+
},
123+
{
124+
OS: "linux",
125+
Architecture: "aarch64",
126+
Variant: "v8",
127+
},
128+
{
129+
OS: "linux",
130+
Architecture: "arm64",
131+
},
132+
},
133+
formatted: "linux/arm64/v8",
134+
},
71135
{
72136
// NOTE(stevvooe): In this case, the consumer can assume this is v7
73137
// but we leave the variant blank. This will represent the vast
@@ -77,6 +141,22 @@ func TestParseSelector(t *testing.T) {
77141
OS: "linux",
78142
Architecture: "arm",
79143
},
144+
matches: []specs.Platform{
145+
{
146+
OS: "linux",
147+
Architecture: "arm",
148+
Variant: "v7",
149+
},
150+
{
151+
OS: "linux",
152+
Architecture: "armhf",
153+
},
154+
{
155+
OS: "linux",
156+
Architecture: "arm",
157+
Variant: "7",
158+
},
159+
},
80160
formatted: "linux/arm",
81161
},
82162
{
@@ -86,6 +166,12 @@ func TestParseSelector(t *testing.T) {
86166
Architecture: "arm",
87167
Variant: "v6",
88168
},
169+
matches: []specs.Platform{
170+
{
171+
OS: "linux",
172+
Architecture: "armel",
173+
},
174+
},
89175
formatted: "linux/arm/v6",
90176
},
91177
{
@@ -95,6 +181,16 @@ func TestParseSelector(t *testing.T) {
95181
Architecture: "arm",
96182
Variant: "v7",
97183
},
184+
matches: []specs.Platform{
185+
{
186+
OS: "linux",
187+
Architecture: "arm",
188+
},
189+
{
190+
OS: "linux",
191+
Architecture: "armhf",
192+
},
193+
},
98194
formatted: "linux/arm/v7",
99195
},
100196
{
@@ -204,11 +300,12 @@ func TestParseSelector(t *testing.T) {
204300

205301
// ensure that match works on the input to the output.
206302
if ok := m.Match(testcase.expected); !ok {
207-
t.Fatalf("expected specifier %q matches %v", testcase.input, testcase.expected)
303+
t.Fatalf("expected specifier %q matches %#v", testcase.input, testcase.expected)
208304
}
209-
210-
if fmt.Sprint(m) != testcase.formatted {
211-
t.Fatalf("unexpected matcher string: %q != %q", fmt.Sprint(m), testcase.formatted)
305+
for _, mc := range testcase.matches {
306+
if ok := m.Match(mc); !ok {
307+
t.Fatalf("expected specifier %q matches %#v", testcase.input, mc)
308+
}
212309
}
213310

214311
formatted := Format(p)

0 commit comments

Comments
 (0)