Skip to content

local: fix platform-split=true option #6007

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.

Sign up for GitHub

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

Merged
tonistiigi merged 4 commits into from
Jun 11, 2025
Merged

local: fix platform-split=true option #6007

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
41ee238
local: fix platform-split=true option
tonistiigi Jun 3, 2025
40ddcfc
fix: wcow: privileges for local exporter
profnandaa Jun 5, 2025
3eb4e72
client: add test for forcing platform-split=true on single-arch build
tonistiigi Jun 3, 2025
db03322
test: use platforms spec instead of runtime const
crazy-max Jun 11, 2025
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
57 changes: 57 additions & 0 deletions client/client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ var allTests = []func(t *testing.T, sb integration.Sandbox){
testSnapshotWithMultipleBlobs,
testExportLocalNoPlatformSplit,
testExportLocalNoPlatformSplitOverwrite,
testExportLocalForcePlatformSplit,
testSolverOptLocalDirsStillWorks,
testOCIIndexMediatype,
testLayerLimitOnMounts,
Expand Down Expand Up @@ -6951,6 +6952,62 @@ func testExportLocalNoPlatformSplitOverwrite(t *testing.T, sb integration.Sandbo
},
}, "", frontend, nil)
require.Error(t, err)
require.ErrorContains(t, err, "cannot overwrite hello-linux from")
require.ErrorContains(t, err, "when split option is disabled")
}

func testExportLocalForcePlatformSplit(t *testing.T, sb integration.Sandbox) {
workers.CheckFeatureCompat(t, sb, workers.FeatureOCIExporter, workers.FeatureMultiPlatform)
c, err := New(sb.Context(), sb.Address())
require.NoError(t, err)
defer c.Close()

frontend := func(ctx context.Context, c gateway.Client) (*gateway.Result, error) {
st := llb.Scratch().File(
llb.Mkfile("foo", 0600, []byte("hello")),
)

def, err := st.Marshal(ctx)
if err != nil {
return nil, err
}

return c.Solve(ctx, gateway.SolveRequest{
Definition: def.ToPB(),
})
}

destDir := t.TempDir()
_, err = c.Build(sb.Context(), SolveOpt{
Exports: []ExportEntry{
{
Type: ExporterLocal,
OutputDir: destDir,
Attrs: map[string]string{
"platform-split": "true",
},
},
},
}, "", frontend, nil)
require.NoError(t, err)

fis, err := os.ReadDir(destDir)
require.NoError(t, err)

require.Len(t, fis, 1, "expected one files in the output directory")

expPlatform := strings.ReplaceAll(platforms.FormatAll(platforms.DefaultSpec()), "/", "_")
_, err = os.Stat(filepath.Join(destDir, expPlatform+"/"))
require.NoError(t, err)
Copy link
Collaborator

@profnandaa profnandaa Jun 5, 2025

Choose a reason for hiding this comment

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

how do we deal with the windows case where the build number is added to the os name? Unless you explicitly specify --opt platform=..., eg.

#10 exporting to client directory
#10 copying files windows(10.0.22631)/amd64
#10 copying files windows(10.0.22631)/amd64 6.01MB 11.4s
#10 copying files windows(10.0.22631)/amd64 150.83MB 16.5s
#10 copying files windows(10.0.22631)/amd64 239.47MB 23.7s
#10 copying files windows(10.0.22631)/amd64 239.47MB 23.9s done
#10 DONE 24.2s

should we normalize that from the code?

Copy link
Member Author

@tonistiigi tonistiigi Jun 5, 2025

Choose a reason for hiding this comment

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

are these () invalid names for windows?

Copy link
Collaborator

@profnandaa profnandaa Jun 6, 2025

Choose a reason for hiding this comment

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

they are not invalid, I think this change was brought in some time back here - containerd/platforms#6

Copy link
Collaborator

@profnandaa profnandaa Jun 6, 2025

Choose a reason for hiding this comment

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

what do you think about this? profnandaa@3fc6393


fis, err = os.ReadDir(filepath.Join(destDir, expPlatform))
require.NoError(t, err)

require.Len(t, fis, 1, "expected one files in the output directory for platform "+expPlatform)

dt, err := os.ReadFile(filepath.Join(destDir, expPlatform, "foo"))
require.NoError(t, err)
require.Equal(t, "hello", string(dt))
}

func readFileInImage(ctx context.Context, t *testing.T, c *Client, ref, path string) ([]byte, error) {
Expand Down
34 changes: 15 additions & 19 deletions exporter/local/export.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,17 +117,18 @@ func (e *localExporterInstance) Export(ctx context.Context, inp *exporter.Source

export := func(ctx context.Context, k string, ref cache.ImmutableRef, attestations []exporter.Attestation) func() error {
return func() error {
outputFS, cleanup, err := CreateFS(ctx, sessionID, k, ref, attestations, now, e.opts)
outputFS, cleanup, err := CreateFS(ctx, sessionID, k, ref, attestations, now, isMap, e.opts)
if err != nil {
return err
}
if cleanup != nil {
defer cleanup()
}

if !e.opts.PlatformSplit {
lbl := "copying files"
if !e.opts.UsePlatformSplit(isMap) {
// check for duplicate paths
err = outputFS.Walk(ctx, "", func(p string, entry os.DirEntry, err error) error {
err = fsWalk(ctx, outputFS, "", func(p string, entry os.DirEntry, err error) error {
if entry.IsDir() {
return nil
}
Expand All @@ -145,23 +146,18 @@ func (e *localExporterInstance) Export(ctx context.Context, inp *exporter.Source
if err != nil {
return err
}
}

lbl := "copying files"
if isMap {
} else {
lbl += " " + k
if e.opts.PlatformSplit {
st := &fstypes.Stat{
Mode: uint32(os.ModeDir | 0755),
Path: strings.ReplaceAll(k, "/", "_"),
}
if e.opts.Epoch != nil {
st.ModTime = e.opts.Epoch.UnixNano()
}
outputFS, err = fsutil.SubDirFS([]fsutil.Dir{{FS: outputFS, Stat: st}})
if err != nil {
return err
}
st := &fstypes.Stat{
Mode: uint32(os.ModeDir | 0755),
Path: strings.ReplaceAll(k, "/", "_"),
}
if e.opts.Epoch != nil {
st.ModTime = e.opts.Epoch.UnixNano()
}
outputFS, err = fsutil.SubDirFS([]fsutil.Dir{{FS: outputFS, Stat: st}})
if err != nil {
return err
}
}

Expand Down
14 changes: 14 additions & 0 deletions exporter/local/export_unix.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
//go:build !windows

package local

import (
"context"
gofs "io/fs"

"github.com/tonistiigi/fsutil"
)

func fsWalk(ctx context.Context, fs fsutil.FS, s string, walkFn gofs.WalkDirFunc) error {
return fs.Walk(ctx, s, walkFn)
}
17 changes: 17 additions & 0 deletions exporter/local/export_windows.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package local

import (
"context"
gofs "io/fs"

"github.com/Microsoft/go-winio"
"github.com/tonistiigi/fsutil"
)

func fsWalk(ctx context.Context, fs fsutil.FS, s string, walkFn gofs.WalkDirFunc) error {
// Windows has some special files that require
// SeBackupPrivilege to be accessed. Ref #4994
return winio.RunWithPrivilege(winio.SeBackupPrivilege, func() error {
return fs.Walk(ctx, s, walkFn)
})
}
17 changes: 12 additions & 5 deletions exporter/local/fs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,18 @@ const (
type CreateFSOpts struct {
Epoch *time.Time
AttestationPrefix string
PlatformSplit bool
PlatformSplit *bool
}

func (c *CreateFSOpts) UsePlatformSplit(isMap bool) bool {
if c.PlatformSplit == nil {
return isMap
}
return *c.PlatformSplit
}

func (c *CreateFSOpts) Load(opt map[string]string) (map[string]string, error) {
rest := make(map[string]string)
c.PlatformSplit = true

var err error
c.Epoch, opt, err = epoch.ParseExporterAttrs(opt)
Expand All @@ -60,7 +66,7 @@ func (c *CreateFSOpts) Load(opt map[string]string) (map[string]string, error) {
if err != nil {
return nil, errors.Wrapf(err, "non-bool value for %s: %s", keyPlatformSplit, v)
}
c.PlatformSplit = b
c.PlatformSplit = &b
default:
rest[k] = v
}
Expand All @@ -69,7 +75,7 @@ func (c *CreateFSOpts) Load(opt map[string]string) (map[string]string, error) {
return rest, nil
}

func CreateFS(ctx context.Context, sessionID string, k string, ref cache.ImmutableRef, attestations []exporter.Attestation, defaultTime time.Time, opt CreateFSOpts) (fsutil.FS, func() error, error) {
func CreateFS(ctx context.Context, sessionID string, k string, ref cache.ImmutableRef, attestations []exporter.Attestation, defaultTime time.Time, isMap bool, opt CreateFSOpts) (fsutil.FS, func() error, error) {
var cleanup func() error
var src string
var err error
Expand Down Expand Up @@ -174,6 +180,7 @@ func CreateFS(ctx context.Context, sessionID string, k string, ref cache.Immutab
return nil, nil, err
}
stmtFS := staticfs.NewFS()
split := opt.UsePlatformSplit(isMap)

names := map[string]struct{}{}
for i, stmt := range stmts {
Expand All @@ -183,7 +190,7 @@ func CreateFS(ctx context.Context, sessionID string, k string, ref cache.Immutab
}

name := opt.AttestationPrefix + path.Base(attestations[i].Path)
if !opt.PlatformSplit {
if !split {
nameExt := path.Ext(name)
namBase := strings.TrimSuffix(name, nameExt)
name = fmt.Sprintf("%s.%s%s", namBase, strings.ReplaceAll(k, "/", "_"), nameExt)
Expand Down
4 changes: 2 additions & 2 deletions exporter/tar/export.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,10 @@ func (e *localExporterInstance) Export(ctx context.Context, inp *exporter.Source
}

now := time.Now().Truncate(time.Second)
isMap := len(inp.Refs) > 0

getDir := func(ctx context.Context, k string, ref cache.ImmutableRef, attestations []exporter.Attestation) (*fsutil.Dir, error) {
outputFS, cleanup, err := local.CreateFS(ctx, sessionID, k, ref, attestations, now, e.opts)
outputFS, cleanup, err := local.CreateFS(ctx, sessionID, k, ref, attestations, now, isMap, e.opts)
if err != nil {
return nil, err
}
Expand All @@ -119,7 +120,6 @@ func (e *localExporterInstance) Export(ctx context.Context, inp *exporter.Source
}, nil
}

isMap := len(inp.Refs) > 0
if _, ok := inp.Metadata[exptypes.ExporterPlatformsKey]; isMap && !ok {
return nil, nil, errors.Errorf("unable to export multiple refs, missing platforms mapping")
}
Expand Down