From 98e0d8dcff0f36c15bf23b79b2a84a1af8cc5c97 Mon Sep 17 00:00:00 2001 From: "Paul \"TBBle\" Hampson" Date: Fri, 27 Oct 2023 23:02:14 +0900 Subject: [PATCH] Whenever copying OCI Platform data, include OSVersion and OSFeatures Trivially created by looking for every reference to .Variant and adding OSVersion and OSFeatures, except the ones related to the string representation of a Platform instance. I then went through and ensured every assignment of OSFeatures that might leak out, i.e., not local-only or for marhsalling purposes, uses the append-to-nil idiom to avoid sharing the slice storage and allowing accidental mutation after-the-fact. Signed-off-by: Paul "TBBle" Hampson --- client/llb/marshal.go | 18 ++++++++----- client/llb/state.go | 9 +++++-- exporter/containerimage/writer.go | 2 ++ frontend/dockerfile/dockerfile2llb/convert.go | 8 ++++++ frontend/dockerfile/dockerfile2llb/image.go | 4 +++ frontend/dockerui/build.go | 2 +- solver/llbsolver/ops/exec.go | 26 ++++++++++++------- solver/llbsolver/ops/exec_binfmt.go | 2 ++ solver/llbsolver/provenance/capture.go | 4 ++- solver/llbsolver/vertex.go | 14 +++++++++- solver/pb/ops.proto | 2 +- solver/pb/platform.go | 14 +++++++--- source/containerimage/pull.go | 24 ++++++++++------- source/containerimage/source.go | 8 ++++-- 14 files changed, 98 insertions(+), 39 deletions(-) diff --git a/client/llb/marshal.go b/client/llb/marshal.go index 3b02299e4..48adaed80 100644 --- a/client/llb/marshal.go +++ b/client/llb/marshal.go @@ -95,14 +95,18 @@ func MarshalConstraints(base, override *Constraints) (*pb.Op, *pb.OpMetadata) { c.Platform = &defaultPlatform } + opPlatform := pb.Platform{ + OS: c.Platform.OS, + Architecture: c.Platform.Architecture, + Variant: c.Platform.Variant, + OSVersion: c.Platform.OSVersion, + } + if c.Platform.OSFeatures != nil { + opPlatform.OSFeatures = append([]string{}, c.Platform.OSFeatures...) + } + return &pb.Op{ - Platform: &pb.Platform{ - OS: c.Platform.OS, - Architecture: c.Platform.Architecture, - Variant: c.Platform.Variant, - OSVersion: c.Platform.OSVersion, - OSFeatures: c.Platform.OSFeatures, - }, + Platform: &opPlatform, Constraints: &pb.WorkerConstraints{ Filter: c.WorkerConstraints, }, diff --git a/client/llb/state.go b/client/llb/state.go index b40b450fd..74f496128 100644 --- a/client/llb/state.go +++ b/client/llb/state.go @@ -258,11 +258,16 @@ func (s State) WithImageConfig(c []byte) (State, error) { } s = s.Dir(img.Config.WorkingDir) if img.Architecture != "" && img.OS != "" { - s = s.Platform(ocispecs.Platform{ + plat := ocispecs.Platform{ OS: img.OS, Architecture: img.Architecture, Variant: img.Variant, - }) + OSVersion: img.OSVersion, + } + if img.OSFeatures != nil { + plat.OSFeatures = append([]string{}, img.OSFeatures...) + } + s = s.Platform(plat) } return s, nil } diff --git a/exporter/containerimage/writer.go b/exporter/containerimage/writer.go index b43761aed..a215b1aff 100644 --- a/exporter/containerimage/writer.go +++ b/exporter/containerimage/writer.go @@ -598,6 +598,8 @@ func defaultImageConfig() ([]byte, error) { img := ocispecs.Image{} img.Architecture = pl.Architecture img.OS = pl.OS + img.OSVersion = pl.OSVersion + img.OSFeatures = pl.OSFeatures img.Variant = pl.Variant img.RootFS.Type = "layers" img.Config.WorkingDir = "/" diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index 7cd57a343..f9c3cabc1 100644 --- a/frontend/dockerfile/dockerfile2llb/convert.go +++ b/frontend/dockerfile/dockerfile2llb/convert.go @@ -274,6 +274,10 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS OS: img.OS, Architecture: img.Architecture, Variant: img.Variant, + OSVersion: img.OSVersion, + } + if img.OSFeatures != nil { + ds.platform.OSFeatures = append([]string{}, img.OSFeatures...) } } } @@ -595,6 +599,10 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS target.image.OS = platformOpt.targetPlatform.OS target.image.Architecture = platformOpt.targetPlatform.Architecture target.image.Variant = platformOpt.targetPlatform.Variant + target.image.OSVersion = platformOpt.targetPlatform.OSVersion + if platformOpt.targetPlatform.OSFeatures != nil { + target.image.OSFeatures = append([]string{}, platformOpt.targetPlatform.OSFeatures...) + } } return target, nil diff --git a/frontend/dockerfile/dockerfile2llb/image.go b/frontend/dockerfile/dockerfile2llb/image.go index 70d81262b..1cb158f1f 100644 --- a/frontend/dockerfile/dockerfile2llb/image.go +++ b/frontend/dockerfile/dockerfile2llb/image.go @@ -19,6 +19,10 @@ func emptyImage(platform ocispecs.Platform) image.Image { img := image.Image{} img.Architecture = platform.Architecture img.OS = platform.OS + img.OSVersion = platform.OSVersion + if platform.OSFeatures != nil { + img.OSFeatures = append([]string{}, platform.OSFeatures...) + } img.Variant = platform.Variant img.RootFS.Type = "layers" img.Config.WorkingDir = "/" diff --git a/frontend/dockerui/build.go b/frontend/dockerui/build.go index 8fc9bbbff..0bba78f48 100644 --- a/frontend/dockerui/build.go +++ b/frontend/dockerui/build.go @@ -57,7 +57,7 @@ func (bc *Client) Build(ctx context.Context, fn BuildFunc) (*ResultBuilder, erro p.OSVersion = img.OSVersion } if p.OSFeatures == nil && len(img.OSFeatures) > 0 { - p.OSFeatures = img.OSFeatures + p.OSFeatures = append([]string{}, img.OSFeatures...) } } diff --git a/solver/llbsolver/ops/exec.go b/solver/llbsolver/ops/exec.go index 496626926..c5b8c35b9 100644 --- a/solver/llbsolver/ops/exec.go +++ b/solver/llbsolver/ops/exec.go @@ -113,6 +113,8 @@ func (e *ExecOp) CacheMap(ctx context.Context, g session.Group, index int) (*sol OS: e.platform.OS, Architecture: e.platform.Architecture, Variant: e.platform.Variant, + OSVersion: e.platform.OSVersion, + OSFeatures: e.platform.OSFeatures, } } @@ -133,17 +135,21 @@ func (e *ExecOp) CacheMap(ctx context.Context, g session.Group, index int) (*sol } dt, err := json.Marshal(struct { - Type string - Exec *pb.ExecOp - OS string - Arch string - Variant string `json:",omitempty"` + Type string + Exec *pb.ExecOp + OS string + Arch string + Variant string `json:",omitempty"` + OSVersion string `json:",omitempty"` + OSFeatures []string `json:",omitempty"` }{ - Type: execCacheType, - Exec: &op, - OS: p.OS, - Arch: p.Architecture, - Variant: p.Variant, + Type: execCacheType, + Exec: &op, + OS: p.OS, + Arch: p.Architecture, + Variant: p.Variant, + OSVersion: p.OSVersion, + OSFeatures: p.OSFeatures, }) if err != nil { return nil, false, err diff --git a/solver/llbsolver/ops/exec_binfmt.go b/solver/llbsolver/ops/exec_binfmt.go index c2c5504cc..41a8047dc 100644 --- a/solver/llbsolver/ops/exec_binfmt.go +++ b/solver/llbsolver/ops/exec_binfmt.go @@ -90,6 +90,8 @@ func getEmulator(ctx context.Context, p *pb.Platform, idmap *idtools.IdentityMap pp := platforms.Normalize(ocispecs.Platform{ Architecture: p.Architecture, OS: p.OS, + OSVersion: p.OSVersion, + OSFeatures: p.OSFeatures, Variant: p.Variant, }) diff --git a/solver/llbsolver/provenance/capture.go b/solver/llbsolver/provenance/capture.go index eb77ef6bc..54645eb61 100644 --- a/solver/llbsolver/provenance/capture.go +++ b/solver/llbsolver/provenance/capture.go @@ -152,7 +152,9 @@ func (c *Capture) AddImage(i ImageSource) { return } if v.Platform != nil && i.Platform != nil { - if v.Platform.Architecture == i.Platform.Architecture && v.Platform.OS == i.Platform.OS && v.Platform.Variant == i.Platform.Variant { + // NOTE: Deliberately excluding OSFeatures, as there's no extant (or rational) case where a source image is an index and contains images distinguished only by OSFeature + // See https://github.com/moby/buildkit/pull/4387#discussion_r1376234241 and https://github.com/opencontainers/image-spec/issues/1147 + if v.Platform.Architecture == i.Platform.Architecture && v.Platform.OS == i.Platform.OS && v.Platform.OSVersion == i.Platform.OSVersion && v.Platform.Variant == i.Platform.Variant { return } } diff --git a/solver/llbsolver/vertex.go b/solver/llbsolver/vertex.go index 154cc75dc..bd3cb30db 100644 --- a/solver/llbsolver/vertex.go +++ b/solver/llbsolver/vertex.go @@ -79,17 +79,29 @@ func NormalizeRuntimePlatforms() LoadOpt { OS: p.OS, Architecture: p.Architecture, Variant: p.Variant, + OSVersion: p.OSVersion, + OSFeatures: p.OSFeatures, } } op.Platform = defaultPlatform } - platform := ocispecs.Platform{OS: op.Platform.OS, Architecture: op.Platform.Architecture, Variant: op.Platform.Variant} + platform := ocispecs.Platform{ + OS: op.Platform.OS, + Architecture: op.Platform.Architecture, + Variant: op.Platform.Variant, + OSVersion: op.Platform.OSVersion, + OSFeatures: op.Platform.OSFeatures, + } normalizedPlatform := platforms.Normalize(platform) op.Platform = &pb.Platform{ OS: normalizedPlatform.OS, Architecture: normalizedPlatform.Architecture, Variant: normalizedPlatform.Variant, + OSVersion: normalizedPlatform.OSVersion, + } + if normalizedPlatform.OSFeatures != nil { + op.Platform.OSFeatures = append([]string{}, normalizedPlatform.OSFeatures...) } return nil diff --git a/solver/pb/ops.proto b/solver/pb/ops.proto index ffada2719..2f78628f4 100644 --- a/solver/pb/ops.proto +++ b/solver/pb/ops.proto @@ -29,7 +29,7 @@ message Platform { string Architecture = 1; string OS = 2; string Variant = 3; - string OSVersion = 4; // unused + string OSVersion = 4; repeated string OSFeatures = 5; // unused } diff --git a/solver/pb/platform.go b/solver/pb/platform.go index 0fd48a07d..00473a5b3 100644 --- a/solver/pb/platform.go +++ b/solver/pb/platform.go @@ -5,23 +5,29 @@ import ( ) func (p *Platform) Spec() ocispecs.Platform { - return ocispecs.Platform{ + result := ocispecs.Platform{ OS: p.OS, Architecture: p.Architecture, Variant: p.Variant, OSVersion: p.OSVersion, - OSFeatures: p.OSFeatures, } + if p.OSFeatures != nil { + result.OSFeatures = append([]string{}, p.OSFeatures...) + } + return result } func PlatformFromSpec(p ocispecs.Platform) Platform { - return Platform{ + result := Platform{ OS: p.OS, Architecture: p.Architecture, Variant: p.Variant, OSVersion: p.OSVersion, - OSFeatures: p.OSFeatures, } + if p.OSFeatures != nil { + result.OSFeatures = append([]string{}, p.OSFeatures...) + } + return result } func ToSpecPlatforms(p []Platform) []ocispecs.Platform { diff --git a/source/containerimage/pull.go b/source/containerimage/pull.go index cdc6dae84..8c0d7a252 100644 --- a/source/containerimage/pull.go +++ b/source/containerimage/pull.go @@ -60,17 +60,21 @@ type puller struct { func mainManifestKey(ctx context.Context, desc ocispecs.Descriptor, platform ocispecs.Platform, layerLimit *int) (digest.Digest, error) { dt, err := json.Marshal(struct { - Digest digest.Digest - OS string - Arch string - Variant string `json:",omitempty"` - Limit *int `json:",omitempty"` + Digest digest.Digest + OS string + Arch string + Variant string `json:",omitempty"` + OSVersion string `json:",omitempty"` + OSFeatures []string `json:",omitempty"` + Limit *int `json:",omitempty"` }{ - Digest: desc.Digest, - OS: platform.OS, - Arch: platform.Architecture, - Variant: platform.Variant, - Limit: layerLimit, + Digest: desc.Digest, + OS: platform.OS, + Arch: platform.Architecture, + Variant: platform.Variant, + OSVersion: platform.OSVersion, + OSFeatures: platform.OSFeatures, + Limit: layerLimit, }) if err != nil { return "", err diff --git a/source/containerimage/source.go b/source/containerimage/source.go index eec0be4f2..914cb0be8 100644 --- a/source/containerimage/source.go +++ b/source/containerimage/source.go @@ -202,7 +202,9 @@ func (is *Source) registryIdentifier(ref string, attrs map[string]string, platfo Architecture: platform.Architecture, Variant: platform.Variant, OSVersion: platform.OSVersion, - OSFeatures: platform.OSFeatures, + } + if platform.OSFeatures != nil { + id.Platform.OSFeatures = append([]string{}, platform.OSFeatures...) } } @@ -247,7 +249,9 @@ func (is *Source) ociIdentifier(ref string, attrs map[string]string, platform *p Architecture: platform.Architecture, Variant: platform.Variant, OSVersion: platform.OSVersion, - OSFeatures: platform.OSFeatures, + } + if platform.OSFeatures != nil { + id.Platform.OSFeatures = append([]string{}, platform.OSFeatures...) } }