Whenever copying OCI Platform data, include OSVersion and OSFeatures

TBBle · TBBle · commit ca6d8c57ba7d · 2023-11-02T23:55:28.000+09:00
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 &lt;Paul.Hampson@Pobox.com&gt;
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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...),
 		}
 	}
 

Original file line number	Diff line number	Diff line change
`@@ -262,6 +262,8 @@ func (s State) WithImageConfig(c []byte) (State, error) {`
`262`	`262`	`OS: img.OS,`
`263`	`263`	`Architecture: img.Architecture,`
`264`	`264`	`Variant: img.Variant,`
	`265`	`+ OSVersion: img.OSVersion,`
	`266`	`+ OSFeatures: append([]string{}, img.OSFeatures...),`
`265`	`267`	`})`
`266`	`268`	`}`
`267`	`269`	`return s, nil`
Original file line number	Diff line number	Diff line change
`@@ -33,7 +33,7 @@ func ParsePlatforms(meta map[string][]byte) (Platforms, error) {`
`33`	`33`	`Architecture: img.Architecture,`
`34`	`34`	`OS: img.OS,`
`35`	`35`	`OSVersion: img.OSVersion,`
`36`		`- OSFeatures: img.OSFeatures,`
	`36`	`+ OSFeatures: append([]string{}, img.OSFeatures...),`
`37`	`37`	`Variant: img.Variant,`
`38`	`38`	`}`
`39`	`39`	`}`
Original file line number	Diff line number	Diff line change
`@@ -274,6 +274,8 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS`
`274`	`274`	`OS: img.OS,`
`275`	`275`	`Architecture: img.Architecture,`
`276`	`276`	`Variant: img.Variant,`
	`277`	`+ OSVersion: img.OSVersion,`
	`278`	`+ OSFeatures: append([]string{}, img.OSFeatures...),`
`277`	`279`	`}`
`278`	`280`	`}`
`279`	`281`	`}`
`@@ -595,6 +597,8 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS`
`595`	`597`	`target.image.OS = platformOpt.targetPlatform.OS`
`596`	`598`	`target.image.Architecture = platformOpt.targetPlatform.Architecture`
`597`	`599`	`target.image.Variant = platformOpt.targetPlatform.Variant`
	`600`	`+ target.image.OSVersion = platformOpt.targetPlatform.OSVersion`
	`601`	`+ target.image.OSFeatures = append([]string{}, platformOpt.targetPlatform.OSFeatures...)`
`598`	`602`	`}`
`599`	`603`
`600`	`604`	`return target, nil`
Original file line number	Diff line number	Diff line change
`@@ -57,7 +57,7 @@ func (bc Client) Build(ctx context.Context, fn BuildFunc) (ResultBuilder, erro`
`57`	`57`	`p.OSVersion = img.OSVersion`
`58`	`58`	`}`
`59`	`59`	`if p.OSFeatures == nil && len(img.OSFeatures) > 0 {`
`60`		`- p.OSFeatures = img.OSFeatures`
	`60`	`+ p.OSFeatures = append([]string{}, img.OSFeatures...)`
`61`	`61`	`}`
`62`	`62`	`}`
`63`	`63`
Original file line number	Diff line number	Diff line change
`@@ -566,7 +566,7 @@ func (lbf llbBridgeForwarder) ResolveImageConfig(ctx context.Context, req pb.R`
`566`	`566`	`Architecture: p.Architecture,`
`567`	`567`	`Variant: p.Variant,`
`568`	`568`	`OSVersion: p.OSVersion,`
`569`		`- OSFeatures: p.OSFeatures,`
	`569`	`+ OSFeatures: append([]string{}, p.OSFeatures...),`
`570`	`570`	`}`
`571`	`571`	`}`
`572`	`572`	`ref, dgst, dt, err := lbf.llbBridge.ResolveImageConfig(ctx, req.Ref, llb.ResolveImageConfigOpt{`
Original file line number	Diff line number	Diff line change
`@@ -486,7 +486,7 @@ func (c *grpcClient) ResolveImageConfig(ctx context.Context, ref string, opt llb`
`486`	`486`	`Architecture: platform.Architecture,`
`487`	`487`	`Variant: platform.Variant,`
`488`	`488`	`OSVersion: platform.OSVersion,`
`489`		`- OSFeatures: platform.OSFeatures,`
	`489`	`+ OSFeatures: append([]string{}, platform.OSFeatures...),`
`490`	`490`	`}`
`491`	`491`	`}`
`492`	`492`