From ca6d8c57ba7dfa9dbf1f78faa9445747b8713a07 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 used the append-to-nil idiom to avoid sharing the slice storage after copying. Signed-off-by: Paul "TBBle" Hampson --- client/llb/marshal.go | 2 +- client/llb/state.go | 2 ++ exporter/containerimage/exptypes/parse.go | 2 +- exporter/containerimage/writer.go | 4 ++- frontend/dockerfile/dockerfile2llb/convert.go | 4 +++ frontend/dockerfile/dockerfile2llb/image.go | 2 ++ frontend/dockerui/build.go | 2 +- frontend/gateway/gateway.go | 2 +- frontend/gateway/grpcclient/client.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 | 12 ++++++++- solver/pb/ops.proto | 2 +- solver/pb/platform.go | 4 +-- source/containerimage/pull.go | 24 ++++++++++------- source/containerimage/source.go | 4 +-- 17 files changed, 67 insertions(+), 33 deletions(-) diff --git a/client/llb/marshal.go b/client/llb/marshal.go index 3b02299e431d9..f73c8588b7530 100644 --- a/client/llb/marshal.go +++ b/client/llb/marshal.go @@ -101,7 +101,7 @@ func MarshalConstraints(base, override *Constraints) (*pb.Op, *pb.OpMetadata) { Architecture: c.Platform.Architecture, Variant: c.Platform.Variant, OSVersion: c.Platform.OSVersion, - OSFeatures: c.Platform.OSFeatures, + OSFeatures: append([]string{}, c.Platform.OSFeatures...), }, Constraints: &pb.WorkerConstraints{ Filter: c.WorkerConstraints, diff --git a/client/llb/state.go b/client/llb/state.go index b40b450fd7e77..51f38cc9555dd 100644 --- a/client/llb/state.go +++ b/client/llb/state.go @@ -262,6 +262,8 @@ func (s State) WithImageConfig(c []byte) (State, error) { OS: img.OS, Architecture: img.Architecture, Variant: img.Variant, + OSVersion: img.OSVersion, + OSFeatures: append([]string{}, img.OSFeatures...), }) } return s, nil diff --git a/exporter/containerimage/exptypes/parse.go b/exporter/containerimage/exptypes/parse.go index f77cd3f525652..9b7771a93b8a8 100644 --- a/exporter/containerimage/exptypes/parse.go +++ b/exporter/containerimage/exptypes/parse.go @@ -33,7 +33,7 @@ func ParsePlatforms(meta map[string][]byte) (Platforms, error) { Architecture: img.Architecture, OS: img.OS, OSVersion: img.OSVersion, - OSFeatures: img.OSFeatures, + OSFeatures: append([]string{}, img.OSFeatures...), Variant: img.Variant, } } diff --git a/exporter/containerimage/writer.go b/exporter/containerimage/writer.go index b43761aeda852..3381108ebe7b2 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 = append([]string{}, pl.OSFeatures...) img.Variant = pl.Variant img.RootFS.Type = "layers" img.Config.WorkingDir = "/" @@ -611,7 +613,7 @@ func attestationsConfig(layers []ocispecs.Descriptor) ([]byte, error) { img.Architecture = intotoPlatform.Architecture img.OS = intotoPlatform.OS img.OSVersion = intotoPlatform.OSVersion - img.OSFeatures = intotoPlatform.OSFeatures + img.OSFeatures = append([]string{}, intotoPlatform.OSFeatures...) img.Variant = intotoPlatform.Variant img.RootFS.Type = "layers" for _, layer := range layers { diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index 7cd57a343f630..f0537dbd081bf 100644 --- a/frontend/dockerfile/dockerfile2llb/convert.go +++ b/frontend/dockerfile/dockerfile2llb/convert.go @@ -274,6 +274,8 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS OS: img.OS, Architecture: img.Architecture, Variant: img.Variant, + OSVersion: img.OSVersion, + OSFeatures: append([]string{}, img.OSFeatures...), } } } @@ -595,6 +597,8 @@ 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 + 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 70d81262bcf2b..c1521470d7fb4 100644 --- a/frontend/dockerfile/dockerfile2llb/image.go +++ b/frontend/dockerfile/dockerfile2llb/image.go @@ -19,6 +19,8 @@ func emptyImage(platform ocispecs.Platform) image.Image { img := image.Image{} img.Architecture = platform.Architecture img.OS = platform.OS + img.OSVersion = platform.OSVersion + 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 8fc9bbbff11ea..0bba78f48b792 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/frontend/gateway/gateway.go b/frontend/gateway/gateway.go index 3b23386fc8e4d..4f36f38e1847f 100644 --- a/frontend/gateway/gateway.go +++ b/frontend/gateway/gateway.go @@ -566,7 +566,7 @@ func (lbf *llbBridgeForwarder) ResolveImageConfig(ctx context.Context, req *pb.R Architecture: p.Architecture, Variant: p.Variant, OSVersion: p.OSVersion, - OSFeatures: p.OSFeatures, + OSFeatures: append([]string{}, p.OSFeatures...), } } ref, dgst, dt, err := lbf.llbBridge.ResolveImageConfig(ctx, req.Ref, llb.ResolveImageConfigOpt{ diff --git a/frontend/gateway/grpcclient/client.go b/frontend/gateway/grpcclient/client.go index 524b3ba2a966b..7fbe0ee0135ca 100644 --- a/frontend/gateway/grpcclient/client.go +++ b/frontend/gateway/grpcclient/client.go @@ -486,7 +486,7 @@ func (c *grpcClient) ResolveImageConfig(ctx context.Context, ref string, opt llb Architecture: platform.Architecture, Variant: platform.Variant, OSVersion: platform.OSVersion, - OSFeatures: platform.OSFeatures, + OSFeatures: append([]string{}, platform.OSFeatures...), } } diff --git a/solver/llbsolver/ops/exec.go b/solver/llbsolver/ops/exec.go index 4966269262b76..c48bbd8f75e31 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: append([]string{}, 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: append([]string{}, 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 c2c5504cc36bc..4d2097eb99900 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: append([]string{}, p.OSFeatures...), Variant: p.Variant, }) diff --git a/solver/llbsolver/provenance/capture.go b/solver/llbsolver/provenance/capture.go index eb77ef6bc3a09..941401d02893c 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 + 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 154cc75dce52a..8996eb77e3e8d 100644 --- a/solver/llbsolver/vertex.go +++ b/solver/llbsolver/vertex.go @@ -79,17 +79,27 @@ func NormalizeRuntimePlatforms() LoadOpt { OS: p.OS, Architecture: p.Architecture, Variant: p.Variant, + OSVersion: p.OSVersion, + OSFeatures: append([]string{}, 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: append([]string{}, op.Platform.OSFeatures...), + } normalizedPlatform := platforms.Normalize(platform) op.Platform = &pb.Platform{ OS: normalizedPlatform.OS, Architecture: normalizedPlatform.Architecture, Variant: normalizedPlatform.Variant, + OSVersion: normalizedPlatform.OSVersion, + OSFeatures: append([]string{}, normalizedPlatform.OSFeatures...), } return nil diff --git a/solver/pb/ops.proto b/solver/pb/ops.proto index ffada2719e0da..2f78628f42f9c 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 0fd48a07d4369..a3fd7daf55ece 100644 --- a/solver/pb/platform.go +++ b/solver/pb/platform.go @@ -10,7 +10,7 @@ func (p *Platform) Spec() ocispecs.Platform { Architecture: p.Architecture, Variant: p.Variant, OSVersion: p.OSVersion, - OSFeatures: p.OSFeatures, + OSFeatures: append([]string{}, p.OSFeatures...), } } @@ -20,7 +20,7 @@ func PlatformFromSpec(p ocispecs.Platform) Platform { Architecture: p.Architecture, Variant: p.Variant, OSVersion: p.OSVersion, - OSFeatures: p.OSFeatures, + OSFeatures: append([]string{}, p.OSFeatures...), } } diff --git a/source/containerimage/pull.go b/source/containerimage/pull.go index cdc6dae84cfa9..57f2f4ef0a0b1 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: append([]string{}, platform.OSFeatures...), + Limit: layerLimit, }) if err != nil { return "", err diff --git a/source/containerimage/source.go b/source/containerimage/source.go index eec0be4f20cf5..c77a648483083 100644 --- a/source/containerimage/source.go +++ b/source/containerimage/source.go @@ -202,7 +202,7 @@ func (is *Source) registryIdentifier(ref string, attrs map[string]string, platfo Architecture: platform.Architecture, Variant: platform.Variant, OSVersion: platform.OSVersion, - OSFeatures: platform.OSFeatures, + OSFeatures: append([]string{}, platform.OSFeatures...), } } @@ -247,7 +247,7 @@ func (is *Source) ociIdentifier(ref string, attrs map[string]string, platform *p Architecture: platform.Architecture, Variant: platform.Variant, OSVersion: platform.OSVersion, - OSFeatures: platform.OSFeatures, + OSFeatures: append([]string{}, platform.OSFeatures...), } }