Skip to content

Commit e8c26b8

Browse files
tonistiigitonistiigi
authored
Merge pull request moby#5614 from cpuguy83/platforms_format_all
Support OS version in platform string
2 parents 7a53272 + 94ddeb7 commit e8c26b8

File tree

14 files changed

+210
-25
lines changed

14 files changed

+210
-25
lines changed

‎client/client_test.go

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10419,7 +10419,7 @@ func testFrontendVerifyPlatforms(t *testing.T, sb integration.Sandbox) {
1041910419
require.NoError(t, err)
1042010420

1042110421
warnings = wc.wait()
10422-
require.Len(t, warnings, 0)
10422+
require.Len(t, warnings, 0, warningsListOutput(warnings))
1042310423

1042410424
frontend = func(ctx context.Context, c gateway.Client) (*gateway.Result, error) {
1042510425
res := gateway.NewResult()
@@ -11016,3 +11016,17 @@ func testRunValidExitCodes(t *testing.T, sb integration.Sandbox) {
1101611016
require.Error(t, err)
1101711017
require.ErrorContains(t, err, "exit code: 0")
1101811018
}
11019+
11020+
type warningsListOutput []*VertexWarning
11021+
11022+
func (w warningsListOutput) String() string {
11023+
if len(w) == 0 {
11024+
return ""
11025+
}
11026+
var b strings.Builder
11027+
11028+
for _, warn := range w {
11029+
_, _ = b.Write(warn.Short)
11030+
}
11031+
return b.String()
11032+
}

‎client/llb/imagemetaresolver/resolver.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ func (imr *imageMetaResolver) ResolveImageConfig(ctx context.Context, ref string
107107

108108
func (imr *imageMetaResolver) key(ref string, platform *ocispecs.Platform) string {
109109
if platform != nil {
110-
ref += platforms.Format(*platform)
110+
ref += platforms.FormatAll(*platform)
111111
}
112112
return ref
113113
}

‎exporter/containerimage/annotations.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ func (ag AnnotationsGroup) Platform(p *ocispecs.Platform) *Annotations {
7171

7272
ps := []string{""}
7373
if p != nil {
74-
ps = append(ps, platforms.Format(*p))
74+
ps = append(ps, platforms.FormatAll(*p))
7575
}
7676

7777
for _, a := range ag {

‎exporter/containerimage/exptypes/annotations.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ func (k AnnotationKey) PlatformString() string {
4949
if k.Platform == nil {
5050
return ""
5151
}
52-
return platforms.Format(*k.Platform)
52+
return platforms.FormatAll(*k.Platform)
5353
}
5454

5555
func AnnotationIndexKey(key string) string {

‎exporter/containerimage/exptypes/parse.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ func ParsePlatforms(meta map[string][]byte) (Platforms, error) {
5353
}
5454
}
5555
p = platforms.Normalize(p)
56-
pk := platforms.Format(p)
56+
pk := platforms.FormatAll(p)
5757
ps := Platforms{
5858
Platforms: []Platform{{ID: pk, Platform: p}},
5959
}

‎exporter/verifier/platforms.go

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,13 +46,14 @@ func CheckInvalidPlatforms[T comparable](ctx context.Context, res *result.Result
4646
})
4747
}
4848
p = platforms.Normalize(p)
49-
_, ok := reqMap[platforms.Format(p)]
49+
formatted := platforms.FormatAll(p)
50+
_, ok := reqMap[formatted]
5051
if ok {
5152
warnings = append(warnings, client.VertexWarning{
5253
Short: []byte(fmt.Sprintf("Duplicate platform result requested %q", v)),
5354
})
5455
}
55-
reqMap[platforms.Format(p)] = struct{}{}
56+
reqMap[formatted] = struct{}{}
5657
reqList = append(reqList, exptypes.Platform{Platform: p})
5758
}
5859

@@ -62,10 +63,25 @@ func CheckInvalidPlatforms[T comparable](ctx context.Context, res *result.Result
6263

6364
if len(reqMap) == 1 && len(ps.Platforms) == 1 {
6465
pp := platforms.Normalize(ps.Platforms[0].Platform)
65-
if _, ok := reqMap[platforms.Format(pp)]; !ok {
66-
return []client.VertexWarning{{
67-
Short: []byte(fmt.Sprintf("Requested platform %q does not match result platform %q", req.Platforms[0], platforms.Format(pp))),
68-
}}, nil
66+
if _, ok := reqMap[platforms.FormatAll(pp)]; !ok {
67+
// The requested platform will often not have an OSVersion on it, but the
68+
// resulting platform may have one.
69+
// This should not be considered a mismatch, so check again after clearing
70+
// the OSVersion from the returned platform.
71+
reqP, err := platforms.Parse(req.Platforms[0])
72+
if err != nil {
73+
return nil, err
74+
}
75+
reqP = platforms.Normalize(reqP)
76+
if reqP.OSVersion == "" && reqP.OSVersion != pp.OSVersion {
77+
pp.OSVersion = ""
78+
}
79+
80+
if _, ok := reqMap[platforms.FormatAll(pp)]; !ok {
81+
return []client.VertexWarning{{
82+
Short: []byte(fmt.Sprintf("Requested platform %q does not match result platform %q", req.Platforms[0], platforms.FormatAll(pp))),
83+
}}, nil
84+
}
6985
}
7086
return nil, nil
7187
}
@@ -81,7 +97,7 @@ func CheckInvalidPlatforms[T comparable](ctx context.Context, res *result.Result
8197
if !mismatch {
8298
for _, p := range ps.Platforms {
8399
pp := platforms.Normalize(p.Platform)
84-
if _, ok := reqMap[platforms.Format(pp)]; !ok {
100+
if _, ok := reqMap[platforms.FormatAll(pp)]; !ok {
85101
mismatch = true
86102
break
87103
}
@@ -100,7 +116,7 @@ func CheckInvalidPlatforms[T comparable](ctx context.Context, res *result.Result
100116
func platformsString(ps []exptypes.Platform) string {
101117
var ss []string
102118
for _, p := range ps {
103-
ss = append(ss, platforms.Format(platforms.Normalize(p.Platform)))
119+
ss = append(ss, platforms.FormatAll(platforms.Normalize(p.Platform)))
104120
}
105121
sort.Strings(ss)
106122
return strings.Join(ss, ",")

‎frontend/dockerfile/builder/build.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ func Build(ctx context.Context, c client.Client) (_ *client.Result, err error) {
160160
if platform != nil {
161161
p = *platform
162162
}
163-
scanTargets.Store(platforms.Format(platforms.Normalize(p)), scanTarget)
163+
scanTargets.Store(platforms.FormatAll(platforms.Normalize(p)), scanTarget)
164164

165165
return ref, img, baseImg, nil
166166
})

‎frontend/dockerfile/dockerfile2llb/convert.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -520,7 +520,7 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
520520
if reachable {
521521
prefix := "["
522522
if opt.MultiPlatformRequested && platform != nil {
523-
prefix += platforms.Format(*platform) + " "
523+
prefix += platforms.FormatAll(*platform) + " "
524524
}
525525
prefix += "internal]"
526526
mutRef, dgst, dt, err := metaResolver.ResolveImageConfig(ctx, d.stage.BaseName, sourceresolver.Opt{
@@ -2110,7 +2110,7 @@ func prefixCommand(ds *dispatchState, str string, prefixPlatform bool, platform
21102110
}
21112111
out := "["
21122112
if prefixPlatform && platform != nil {
2113-
out += platforms.Format(*platform) + formatTargetPlatform(*platform, platformFromEnv(env)) + " "
2113+
out += platforms.FormatAll(*platform) + formatTargetPlatform(*platform, platformFromEnv(env)) + " "
21142114
}
21152115
if ds.stageName != "" {
21162116
out += ds.stageName + " "
@@ -2144,7 +2144,7 @@ func formatTargetPlatform(base ocispecs.Platform, target *ocispecs.Platform) str
21442144
return "->" + archVariant
21452145
}
21462146
if p.OS != base.OS {
2147-
return "->" + platforms.Format(p)
2147+
return "->" + platforms.FormatAll(p)
21482148
}
21492149
return ""
21502150
}
@@ -2491,8 +2491,8 @@ func wrapSuggestAny(err error, keys map[string]struct{}, options []string) error
24912491

24922492
func validateBaseImagePlatform(name string, expected, actual ocispecs.Platform, location []parser.Range, lint *linter.Linter) {
24932493
if expected.OS != actual.OS || expected.Architecture != actual.Architecture {
2494-
expectedStr := platforms.Format(platforms.Normalize(expected))
2495-
actualStr := platforms.Format(platforms.Normalize(actual))
2494+
expectedStr := platforms.FormatAll(platforms.Normalize(expected))
2495+
actualStr := platforms.FormatAll(platforms.Normalize(actual))
24962496
msg := linter.RuleInvalidBaseImagePlatform.Format(name, expectedStr, actualStr)
24972497
lint.Run(&linter.RuleInvalidBaseImagePlatform, location, msg)
24982498
}

‎frontend/dockerfile/dockerfile2llb/platform.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,12 @@ func defaultArgs(po *platformOpt, overrides map[string]string, target string) *l
4545
s := [...][2]string{
4646
{"BUILDPLATFORM", platforms.Format(bp)},
4747
{"BUILDOS", bp.OS},
48+
{"BUILDOSVERSION", bp.OSVersion},
4849
{"BUILDARCH", bp.Architecture},
4950
{"BUILDVARIANT", bp.Variant},
50-
{"TARGETPLATFORM", platforms.Format(tp)},
51+
{"TARGETPLATFORM", platforms.FormatAll(tp)},
5152
{"TARGETOS", tp.OS},
53+
{"TARGETOSVERSION", tp.OSVersion},
5254
{"TARGETARCH", tp.Architecture},
5355
{"TARGETVARIANT", tp.Variant},
5456
{"TARGETSTAGE", target},

‎frontend/dockerfile/dockerfile_test.go

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,7 @@ var allTests = integration.TestFuncs(
215215
testStepNames,
216216
testPowershellInDefaultPathOnWindows,
217217
testOCILayoutMultiname,
218+
testPlatformWithOSVersion,
218219
)
219220

220221
// Tests that depend on the `security.*` entitlements
@@ -9425,6 +9426,158 @@ COPY Dockerfile /foo
94259426
}
94269427
}
94279428

9429+
func testPlatformWithOSVersion(t *testing.T, sb integration.Sandbox) {
9430+
// This test cannot be run on Windows currently due to `FROM scratch` and
9431+
// layer formatting not being supported on Windows.
9432+
integration.SkipOnPlatform(t, "windows")
9433+
9434+
ctx := sb.Context()
9435+
9436+
c, err := client.New(ctx, sb.Address())
9437+
require.NoError(t, err)
9438+
defer c.Close()
9439+
9440+
f := getFrontend(t, sb)
9441+
9442+
// NOTE: currently "OS" *must* be set to "windows" for this to work.
9443+
// The platform matchers only do OSVersion comparisons when the OS is set to "windows".
9444+
p1 := ocispecs.Platform{
9445+
OS: "windows",
9446+
OSVersion: "1.2.3",
9447+
Architecture: "bar",
9448+
}
9449+
p2 := ocispecs.Platform{
9450+
OS: "windows",
9451+
OSVersion: "1.1.0",
9452+
Architecture: "bar",
9453+
}
9454+
9455+
p1Str := platforms.FormatAll(p1)
9456+
p2Str := platforms.FormatAll(p2)
9457+
9458+
registry, err := sb.NewRegistry()
9459+
if errors.Is(err, integration.ErrRequirements) {
9460+
t.Skip(err.Error())
9461+
}
9462+
require.NoError(t, err)
9463+
target := registry + "/buildkit/testplatformwithosversion:latest"
9464+
9465+
dockerfile := []byte(`
9466+
FROM ` + target + ` AS reg
9467+
9468+
FROM scratch AS base
9469+
ARG TARGETOSVERSION
9470+
COPY <<EOF /osversion
9471+
${TARGETOSVERSION}
9472+
EOF
9473+
ARG TARGETPLATFORM
9474+
COPY <<EOF /targetplatform
9475+
${TARGETPLATFORM}
9476+
EOF
9477+
`)
9478+
9479+
destDir := t.TempDir()
9480+
dir := integration.Tmpdir(
9481+
t,
9482+
fstest.CreateFile("Dockerfile", dockerfile, 0600),
9483+
)
9484+
9485+
// build the base target as a multi-platform image and push to the registry
9486+
_, err = f.Solve(sb.Context(), c, client.SolveOpt{
9487+
FrontendAttrs: map[string]string{
9488+
"platform": p1Str + "," + p2Str,
9489+
"target": "base",
9490+
},
9491+
Exports: []client.ExportEntry{
9492+
{
9493+
Type: client.ExporterLocal,
9494+
OutputDir: destDir,
9495+
},
9496+
{
9497+
Type: client.ExporterImage,
9498+
Attrs: map[string]string{
9499+
"name": target,
9500+
"push": "true",
9501+
},
9502+
},
9503+
},
9504+
9505+
LocalMounts: map[string]fsutil.FS{
9506+
dockerui.DefaultLocalNameDockerfile: dir,
9507+
dockerui.DefaultLocalNameContext: dir,
9508+
},
9509+
}, nil)
9510+
9511+
require.NoError(t, err)
9512+
9513+
dt, err := os.ReadFile(filepath.Join(destDir, strings.Replace(p1Str, "/", "_", 1), "osversion"))
9514+
require.NoError(t, err)
9515+
require.Equal(t, p1.OSVersion+"\n", string(dt))
9516+
9517+
dt, err = os.ReadFile(filepath.Join(destDir, strings.Replace(p1Str, "/", "_", 1), "targetplatform"))
9518+
require.NoError(t, err)
9519+
require.Equal(t, p1Str+"\n", string(dt))
9520+
9521+
dt, err = os.ReadFile(filepath.Join(destDir, strings.Replace(p2Str, "/", "_", 1), "osversion"))
9522+
require.NoError(t, err)
9523+
require.Equal(t, p2.OSVersion+"\n", string(dt))
9524+
9525+
dt, err = os.ReadFile(filepath.Join(destDir, strings.Replace(p2Str, "/", "_", 1), "targetplatform"))
9526+
require.NoError(t, err)
9527+
require.Equal(t, p2Str+"\n", string(dt))
9528+
9529+
// Now build the "reg" target, which should pull the base image from the registry
9530+
// This should select the image with the requested os version.
9531+
destDir = t.TempDir()
9532+
_, err = f.Solve(sb.Context(), c, client.SolveOpt{
9533+
FrontendAttrs: map[string]string{
9534+
"platform": p1Str,
9535+
"target": "reg",
9536+
},
9537+
Exports: []client.ExportEntry{
9538+
{
9539+
Type: client.ExporterLocal,
9540+
OutputDir: destDir,
9541+
},
9542+
},
9543+
9544+
LocalMounts: map[string]fsutil.FS{
9545+
dockerui.DefaultLocalNameDockerfile: dir,
9546+
dockerui.DefaultLocalNameContext: dir,
9547+
},
9548+
}, nil)
9549+
require.NoError(t, err)
9550+
9551+
dt, err = os.ReadFile(filepath.Join(destDir, "osversion"))
9552+
require.NoError(t, err)
9553+
require.Equal(t, p1.OSVersion+"\n", string(dt))
9554+
9555+
// And again with the other os version
9556+
destDir = t.TempDir()
9557+
_, err = f.Solve(sb.Context(), c, client.SolveOpt{
9558+
FrontendAttrs: map[string]string{
9559+
"platform": p2Str,
9560+
"target": "reg",
9561+
},
9562+
Exports: []client.ExportEntry{
9563+
{
9564+
Type: client.ExporterLocal,
9565+
OutputDir: destDir,
9566+
},
9567+
},
9568+
9569+
LocalMounts: map[string]fsutil.FS{
9570+
dockerui.DefaultLocalNameDockerfile: dir,
9571+
dockerui.DefaultLocalNameContext: dir,
9572+
},
9573+
}, nil)
9574+
require.NoError(t, err)
9575+
9576+
dt, err = os.ReadFile(filepath.Join(destDir, "osversion"))
9577+
require.NoError(t, err)
9578+
require.Equal(t, p2.OSVersion+"\n", string(dt))
9579+
}
9580+
94289581
func runShell(dir string, cmds ...string) error {
94299582
for _, args := range cmds {
94309583
var cmd *exec.Cmd

0 commit comments

Comments
 (0)