Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Support OS version in platform string #5614

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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()
}
2 changes: 1 addition & 1 deletion client/llb/imagemetaresolver/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion exporter/containerimage/annotations.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion exporter/containerimage/exptypes/annotations.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion exporter/containerimage/exptypes/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}},
}
Expand Down
32 changes: 24 additions & 8 deletions exporter/verifier/platforms.go
Original file line number Diff line number Diff line change
Expand Up @@ -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})
}

Expand All @@ -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
}
Expand All @@ -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
}
Expand All @@ -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, ",")
Expand Down
2 changes: 1 addition & 1 deletion frontend/dockerfile/builder/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
Expand Down
10 changes: 5 additions & 5 deletions frontend/dockerfile/dockerfile2llb/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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 + " "
Expand Down Expand Up @@ -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 ""
}
Expand Down Expand Up @@ -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)
}
Expand Down
2 changes: 2 additions & 0 deletions frontend/dockerfile/dockerfile2llb/platform.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down
139 changes: 139 additions & 0 deletions frontend/dockerfile/dockerfile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ var allTests = integration.TestFuncs(
testStepNames,
testPowershellInDefaultPathOnWindows,
testOCILayoutMultiname,
testPlatformWithOSVersion,
)

// Tests that depend on the `security.*` entitlements
Expand Down Expand Up @@ -9425,6 +9426,144 @@ COPY Dockerfile /foo
}
}

func testPlatformWithOSVersion(t *testing.T, sb integration.Sandbox) {
Copy link
Collaborator

@profnandaa profnandaa Jan 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thinking of a workaround to test this on Windows, given we don't have support for scratch and the unionfs-like mounting... shouldn't be blocking if it's the only one pending, we can skip on Windows for now and file tracking issue on me.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// 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
Expand Down
2 changes: 1 addition & 1 deletion frontend/dockerui/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion frontend/dockerui/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions solver/llbsolver/bridge.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion source/containerimage/source.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading
Loading