WIP: Support OS version in platform string

This allows platforms following the new `platforms.FormatAll` function, which allows for setting the `OSVersion` field of the platform with `<os>(<ver>)/<arch>`. Signed-off-by: Brian Goff <[email protected]>
moby · Jan 7, 2025 · 212a947 · 212a947
1 parent 4c68785
commit 212a947
Show file tree

Hide file tree

Showing 19 changed files with 310 additions and 169 deletions.
diff --git a/client/client_test.go b/client/client_test.go
@@ -10414,7 +10414,7 @@ func testFrontendVerifyPlatforms(t *testing.T, sb integration.Sandbox) {
 	require.NoError(t, err)
 
 	warnings = wc.wait()
-	require.Len(t, warnings, 0)
+	require.Len(t, warnings, 0, warningsListOutput(warnings))
 
 	frontend = func(ctx context.Context, c gateway.Client) (*gateway.Result, error) {
 		res := gateway.NewResult()
@@ -11011,3 +11011,17 @@ func testRunValidExitCodes(t *testing.T, sb integration.Sandbox) {
 	require.Error(t, err)
 	require.ErrorContains(t, err, "exit code: 0")
 }
+
+type warningsListOutput []*VertexWarning
+
+func (w warningsListOutput) String() string {
+	if len(w) == 0 {
+		return ""
+	}
+	var b strings.Builder
+
+	for _, warn := range w {
+		_, _ = b.Write(warn.Short)
+	}
+	return b.String()
+}
diff --git a/client/llb/imagemetaresolver/resolver.go b/client/llb/imagemetaresolver/resolver.go
@@ -107,7 +107,7 @@ func (imr *imageMetaResolver) ResolveImageConfig(ctx context.Context, ref string
 
 func (imr *imageMetaResolver) key(ref string, platform *ocispecs.Platform) string {
 	if platform != nil {
-		ref += platforms.Format(*platform)
+		ref += platforms.FormatAll(*platform)
 	}
 	return ref
 }

diff --git a/exporter/containerimage/annotations.go b/exporter/containerimage/annotations.go
@@ -71,7 +71,7 @@ func (ag AnnotationsGroup) Platform(p *ocispecs.Platform) *Annotations {
 
 	ps := []string{""}
 	if p != nil {
-		ps = append(ps, platforms.Format(*p))
+		ps = append(ps, platforms.FormatAll(*p))
 	}
 
 	for _, a := range ag {

diff --git a/exporter/containerimage/exptypes/annotations.go b/exporter/containerimage/exptypes/annotations.go
@@ -49,7 +49,7 @@ func (k AnnotationKey) PlatformString() string {
 	if k.Platform == nil {
 		return ""
 	}
-	return platforms.Format(*k.Platform)
+	return platforms.FormatAll(*k.Platform)
 }
 
 func AnnotationIndexKey(key string) string {

diff --git a/exporter/containerimage/exptypes/parse.go b/exporter/containerimage/exptypes/parse.go
@@ -53,7 +53,7 @@ func ParsePlatforms(meta map[string][]byte) (Platforms, error) {
 		}
 	}
 	p = platforms.Normalize(p)
-	pk := platforms.Format(p)
+	pk := platforms.FormatAll(p)
 	ps := Platforms{
 		Platforms: []Platform{{ID: pk, Platform: p}},
 	}

diff --git a/exporter/verifier/platforms.go b/exporter/verifier/platforms.go
@@ -46,13 +46,14 @@ func CheckInvalidPlatforms[T comparable](ctx context.Context, res *result.Result
 			})
 		}
 		p = platforms.Normalize(p)
-		_, ok := reqMap[platforms.Format(p)]
+		formatted := platforms.FormatAll(p)
+		_, ok := reqMap[formatted]
 		if ok {
 			warnings = append(warnings, client.VertexWarning{
 				Short: []byte(fmt.Sprintf("Duplicate platform result requested %q", v)),
 			})
 		}
-		reqMap[platforms.Format(p)] = struct{}{}
+		reqMap[formatted] = struct{}{}
 		reqList = append(reqList, exptypes.Platform{Platform: p})
 	}
 
@@ -62,10 +63,25 @@ func CheckInvalidPlatforms[T comparable](ctx context.Context, res *result.Result
 
 	if len(reqMap) == 1 && len(ps.Platforms) == 1 {
 		pp := platforms.Normalize(ps.Platforms[0].Platform)
-		if _, ok := reqMap[platforms.Format(pp)]; !ok {
-			return []client.VertexWarning{{
-				Short: []byte(fmt.Sprintf("Requested platform %q does not match result platform %q", req.Platforms[0], platforms.Format(pp))),
-			}}, nil
+		if _, ok := reqMap[platforms.FormatAll(pp)]; !ok {
+			// The requested platform will often not have an OSVersion on it, but the
+			// resulting platform may have one.
+			// This should not be considered a mismatch, so check again after clearing
+			// the OSVersion from the returned platform.
+			reqP, err := platforms.Parse(req.Platforms[0])
+			if err != nil {
+				return nil, err
+			}
+			reqP = platforms.Normalize(reqP)
+			if reqP.OSVersion == "" && reqP.OSVersion != pp.OSVersion {
+				pp.OSVersion = ""
+			}
+
+			if _, ok := reqMap[platforms.FormatAll(pp)]; !ok {
+				return []client.VertexWarning{{
+					Short: []byte(fmt.Sprintf("Requested platform %q does not match result platform %q", req.Platforms[0], platforms.FormatAll(pp))),
+				}}, nil
+			}
 		}
 		return nil, nil
 	}
@@ -81,7 +97,7 @@ func CheckInvalidPlatforms[T comparable](ctx context.Context, res *result.Result
 	if !mismatch {
 		for _, p := range ps.Platforms {
 			pp := platforms.Normalize(p.Platform)
-			if _, ok := reqMap[platforms.Format(pp)]; !ok {
+			if _, ok := reqMap[platforms.FormatAll(pp)]; !ok {
 				mismatch = true
 				break
 			}
@@ -100,7 +116,7 @@ func CheckInvalidPlatforms[T comparable](ctx context.Context, res *result.Result
 func platformsString(ps []exptypes.Platform) string {
 	var ss []string
 	for _, p := range ps {
-		ss = append(ss, platforms.Format(platforms.Normalize(p.Platform)))
+		ss = append(ss, platforms.FormatAll(platforms.Normalize(p.Platform)))
 	}
 	sort.Strings(ss)
 	return strings.Join(ss, ",")

diff --git a/frontend/dockerfile/builder/build.go b/frontend/dockerfile/builder/build.go
@@ -160,7 +160,7 @@ func Build(ctx context.Context, c client.Client) (_ *client.Result, err error) {
 		if platform != nil {
 			p = *platform
 		}
-		scanTargets.Store(platforms.Format(platforms.Normalize(p)), scanTarget)
+		scanTargets.Store(platforms.FormatAll(platforms.Normalize(p)), scanTarget)
 
 		return ref, img, baseImg, nil
 	})

diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go
@@ -520,7 +520,7 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
 						if reachable {
 							prefix := "["
 							if opt.MultiPlatformRequested && platform != nil {
-								prefix += platforms.Format(*platform) + " "
+								prefix += platforms.FormatAll(*platform) + " "
 							}
 							prefix += "internal]"
 							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
 	}
 	out := "["
 	if prefixPlatform && platform != nil {
-		out += platforms.Format(*platform) + formatTargetPlatform(*platform, platformFromEnv(env)) + " "
+		out += platforms.FormatAll(*platform) + formatTargetPlatform(*platform, platformFromEnv(env)) + " "
 	}
 	if ds.stageName != "" {
 		out += ds.stageName + " "
@@ -2144,7 +2144,7 @@ func formatTargetPlatform(base ocispecs.Platform, target *ocispecs.Platform) str
 		return "->" + archVariant
 	}
 	if p.OS != base.OS {
-		return "->" + platforms.Format(p)
+		return "->" + platforms.FormatAll(p)
 	}
 	return ""
 }
@@ -2491,8 +2491,8 @@ func wrapSuggestAny(err error, keys map[string]struct{}, options []string) error
 
 func validateBaseImagePlatform(name string, expected, actual ocispecs.Platform, location []parser.Range, lint *linter.Linter) {
 	if expected.OS != actual.OS || expected.Architecture != actual.Architecture {
-		expectedStr := platforms.Format(platforms.Normalize(expected))
-		actualStr := platforms.Format(platforms.Normalize(actual))
+		expectedStr := platforms.FormatAll(platforms.Normalize(expected))
+		actualStr := platforms.FormatAll(platforms.Normalize(actual))
 		msg := linter.RuleInvalidBaseImagePlatform.Format(name, expectedStr, actualStr)
 		lint.Run(&linter.RuleInvalidBaseImagePlatform, location, msg)
 	}

diff --git a/frontend/dockerfile/dockerfile2llb/platform.go b/frontend/dockerfile/dockerfile2llb/platform.go
@@ -45,10 +45,12 @@ func defaultArgs(po *platformOpt, overrides map[string]string, target string) *l
 	s := [...][2]string{
 		{"BUILDPLATFORM", platforms.Format(bp)},
 		{"BUILDOS", bp.OS},
+		{"BUILDOSVERSION", bp.OSVersion},
 		{"BUILDARCH", bp.Architecture},
 		{"BUILDVARIANT", bp.Variant},
 		{"TARGETPLATFORM", platforms.Format(tp)},
 		{"TARGETOS", tp.OS},
+		{"TARGETOSVERSION", tp.OSVersion},
 		{"TARGETARCH", tp.Architecture},
 		{"TARGETVARIANT", tp.Variant},
 		{"TARGETSTAGE", target},

diff --git a/frontend/dockerfile/dockerfile_test.go b/frontend/dockerfile/dockerfile_test.go
@@ -215,6 +215,7 @@ var allTests = integration.TestFuncs(
 	testStepNames,
 	testPowershellInDefaultPathOnWindows,
 	testOCILayoutMultiname,
+	testPlatformWithOSVersion,
 )
 
 // Tests that depend on the `security.*` entitlements
@@ -9425,6 +9426,144 @@ COPY Dockerfile /foo
 	}
 }
 
+func testPlatformWithOSVersion(t *testing.T, sb integration.Sandbox) {
+	// This test cannot be run on Windows currently due to `FROM scratch` and
+	// layer formatting not being supported on Windows.
+	integration.SkipOnPlatform(t, "windows")
+
+	ctx := sb.Context()
+
+	c, err := client.New(ctx, sb.Address())
+	require.NoError(t, err)
+	defer c.Close()
+
+	f := getFrontend(t, sb)
+	p1 := ocispecs.Platform{
+		OS:           "foo",
+		OSVersion:    "1.2.3",
+		Architecture: "bar",
+	}
+	p2 := ocispecs.Platform{
+		OS:           "foo",
+		OSVersion:    "1.1.0",
+		Architecture: "bar",
+	}
+
+	p1Str := platforms.FormatAll(p1)
+	p2Str := platforms.FormatAll(p2)
+
+	registry, err := sb.NewRegistry()
+	if errors.Is(err, integration.ErrRequirements) {
+		t.Skip(err.Error())
+	}
+	require.NoError(t, err)
+	target := registry + "/buildkit/testplatformwithosversion:latest"
+
+	dockerfile := []byte(`
+ARG TARGETOS TARGETOSVERSION TARGETARCH
+FROM --platform=${TARGETOS}(${TARGETOSVERSION})/${TARGETARCH} ` + target + ` AS reg
+
+FROM scratch AS base
+ARG TARGETOSVERSION
+COPY <<EOF /osversion
+${TARGETOSVERSION}
+EOF
+`)
+
+	destDir := t.TempDir()
+	dir := integration.Tmpdir(
+		t,
+		fstest.CreateFile("Dockerfile", dockerfile, 0600),
+	)
+
+	// build the base target as a multi-platform image and push to the registry
+	_, err = f.Solve(sb.Context(), c, client.SolveOpt{
+		FrontendAttrs: map[string]string{
+			"platform": p1Str + "," + p2Str,
+			"target":   "base",
+		},
+		Exports: []client.ExportEntry{
+			{
+				Type:      client.ExporterLocal,
+				OutputDir: destDir,
+			},
+			{
+				Type: client.ExporterImage,
+				Attrs: map[string]string{
+					"name": target,
+					"push": "true",
+				},
+			},
+		},
+
+		LocalMounts: map[string]fsutil.FS{
+			dockerui.DefaultLocalNameDockerfile: dir,
+			dockerui.DefaultLocalNameContext:    dir,
+		},
+	}, nil)
+
+	require.NoError(t, err)
+
+	dt, err := os.ReadFile(filepath.Join(destDir, strings.Replace(p1Str, "/", "_", 1), "osversion"))
+	require.NoError(t, err)
+	require.Equal(t, p1.OSVersion+"\n", string(dt))
+
+	dt, err = os.ReadFile(filepath.Join(destDir, strings.Replace(p2Str, "/", "_", 1), "osversion"))
+	require.NoError(t, err)
+	require.Equal(t, p2.OSVersion+"\n", string(dt))
+
+	// Now build the "reg" target, which should pull the base image from the registry
+	// This should select the image with the requested os version.
+	destDir = t.TempDir()
+	_, err = f.Solve(sb.Context(), c, client.SolveOpt{
+		FrontendAttrs: map[string]string{
+			"platform": p1Str,
+			"target":   "reg",
+		},
+		Exports: []client.ExportEntry{
+			{
+				Type:      client.ExporterLocal,
+				OutputDir: destDir,
+			},
+		},
+
+		LocalMounts: map[string]fsutil.FS{
+			dockerui.DefaultLocalNameDockerfile: dir,
+			dockerui.DefaultLocalNameContext:    dir,
+		},
+	}, nil)
+	require.NoError(t, err)
+
+	dt, err = os.ReadFile(filepath.Join(destDir, "osversion"))
+	require.NoError(t, err)
+	require.Equal(t, p1.OSVersion+"\n", string(dt))
+
+	// And again with the other os version
+	destDir = t.TempDir()
+	_, err = f.Solve(sb.Context(), c, client.SolveOpt{
+		FrontendAttrs: map[string]string{
+			"platform": p2Str,
+			"target":   "reg",
+		},
+		Exports: []client.ExportEntry{
+			{
+				Type:      client.ExporterLocal,
+				OutputDir: destDir,
+			},
+		},
+
+		LocalMounts: map[string]fsutil.FS{
+			dockerui.DefaultLocalNameDockerfile: dir,
+			dockerui.DefaultLocalNameContext:    dir,
+		},
+	}, nil)
+	require.NoError(t, err)
+
+	dt, err = os.ReadFile(filepath.Join(destDir, "osversion"))
+	require.NoError(t, err)
+	require.Equal(t, p2.OSVersion+"\n", string(dt))
+}
+
 func runShell(dir string, cmds ...string) error {
 	for _, args := range cmds {
 		var cmd *exec.Cmd

diff --git a/frontend/dockerui/build.go b/frontend/dockerui/build.go
@@ -70,7 +70,7 @@ func (bc *Client) Build(ctx context.Context, fn BuildFunc) (*ResultBuilder, erro
 			}
 
 			p = platforms.Normalize(p)
-			k := platforms.Format(p)
+			k := platforms.FormatAll(p)
 
 			if bc.MultiPlatformRequested {
 				res.AddRef(k, ref)

diff --git a/frontend/dockerui/config.go b/frontend/dockerui/config.go
@@ -463,7 +463,7 @@ func (bc *Client) NamedContext(ctx context.Context, name string, opt ContextOpt)
 	if opt.Platform != nil {
 		pp = *opt.Platform
 	}
-	pname := name + "::" + platforms.Format(platforms.Normalize(pp))
+	pname := name + "::" + platforms.FormatAll(platforms.Normalize(pp))
 	st, img, err := bc.namedContext(ctx, name, pname, opt)
 	if err != nil || st != nil {
 		return st, img, err

diff --git a/solver/llbsolver/bridge.go b/solver/llbsolver/bridge.go
@@ -354,9 +354,9 @@ func (b *llbBridge) ResolveSourceMetadata(ctx context.Context, op *pb.SourceOp,
 	}
 	id := op.Identifier
 	if opt.Platform != nil {
-		id += platforms.Format(*opt.Platform)
+		id += platforms.FormatAll(*opt.Platform)
 	} else {
-		id += platforms.Format(platforms.DefaultSpec())
+		id += platforms.FormatAll(platforms.DefaultSpec())
 	}
 	pol, err := loadSourcePolicy(b.builder)
 	if err != nil {

diff --git a/source/containerimage/source.go b/source/containerimage/source.go
@@ -162,7 +162,7 @@ func (is *Source) ResolveImageConfig(ctx context.Context, ref string, opt source
 		err   error
 	)
 	if platform := opt.Platform; platform != nil {
-		key += platforms.Format(*platform)
+		key += platforms.FormatAll(*platform)
 	}
 
 	switch is.ResolverType {