Merge pull request #3673 from tonistiigi/v0.11.4-picks

v0.11.4 cherry picks
moby · Feb 28, 2023 · 2590f95 · 2590f95
2 parents c327eb8 + 97b37f9
commit 2590f95
Show file tree

Hide file tree

Showing 5 changed files with 49 additions and 13 deletions.
diff --git a/cache/remote.go b/cache/remote.go
@@ -228,13 +228,13 @@ func (sr *immutableRef) getRemote(ctx context.Context, createIfNeeded bool, refC
 				newDesc.Size = blobDesc.Size
 				newDesc.URLs = blobDesc.URLs
 				newDesc.Annotations = nil
+				if len(addAnnotations) > 0 || len(blobDesc.Annotations) > 0 {
+					newDesc.Annotations = make(map[string]string)
+				}
 				for _, k := range addAnnotations {
 					newDesc.Annotations[k] = desc.Annotations[k]
 				}
 				for k, v := range blobDesc.Annotations {
-					if newDesc.Annotations == nil {
-						newDesc.Annotations = make(map[string]string)
-					}
 					newDesc.Annotations[k] = v
 				}
 				desc = newDesc

diff --git a/client/llb/definition.go b/client/llb/definition.go
@@ -29,6 +29,10 @@ type DefinitionOp struct {
 
 // NewDefinitionOp returns a new operation from a marshalled definition.
 func NewDefinitionOp(def *pb.Definition) (*DefinitionOp, error) {
+	if def == nil {
+		return nil, errors.New("invalid nil input definition to definition op")
+	}
+
 	ops := make(map[digest.Digest]*pb.Op)
 	defs := make(map[digest.Digest][]byte)
 	platforms := make(map[digest.Digest]*ocispecs.Platform)

diff --git a/client/llb/definition_test.go b/client/llb/definition_test.go
@@ -118,3 +118,9 @@ func TestDefinitionInputCache(t *testing.T) {
 	// 1 exec + 2x2 mounts from stA and stB + 1 src = 6 vertexes
 	require.Equal(t, 6, len(vertexCache))
 }
+
+func TestDefinitionNil(t *testing.T) {
+	// should be an error, not a panic
+	_, err := NewDefinitionOp(nil)
+	require.Error(t, err)
+}
diff --git a/frontend/dockerfile/dockerfile_test.go b/frontend/dockerfile/dockerfile_test.go
@@ -160,6 +160,7 @@ var allTests = integration.TestFuncs(
 	testNilProvenance,
 	testSBOMScannerArgs,
 	testMultiPlatformWarnings,
+	testNilContextInSolveGateway,
 )
 
 // Tests that depend on the `security.*` entitlements
@@ -6600,6 +6601,29 @@ COPY --from=0 / /
 	require.Equal(t, expectedDigest, outDigest)
 }
 
+func testNilContextInSolveGateway(t *testing.T, sb integration.Sandbox) {
+	f := getFrontend(t, sb)
+	c, err := client.New(sb.Context(), sb.Address())
+	require.NoError(t, err)
+	defer c.Close()
+
+	_, err = c.Build(sb.Context(), client.SolveOpt{}, "", func(ctx context.Context, c gateway.Client) (*gateway.Result, error) {
+		res, err := f.SolveGateway(ctx, c, gateway.SolveRequest{
+			Frontend: "dockerfile.v0",
+			FrontendInputs: map[string]*pb.Definition{
+				builder.DefaultLocalNameDockerfile: nil,
+				builder.DefaultLocalNameContext:    nil,
+			},
+		})
+		if err != nil {
+			return nil, err
+		}
+		return res, nil
+	}, nil)
+	// should not cause buildkitd to panic
+	require.ErrorContains(t, err, "invalid nil input definition to definition op")
+}
+
 func runShell(dir string, cmds ...string) error {
 	for _, args := range cmds {
 		var cmd *exec.Cmd

diff --git a/snapshot/diffapply_unix.go b/snapshot/diffapply_unix.go
@@ -379,6 +379,18 @@ func (a *applier) applyCopy(ctx context.Context, ca *changeApply) error {
 		return errors.Errorf("unhandled file type %d during merge at path %q", ca.srcStat.Mode&unix.S_IFMT, ca.srcPath)
 	}
 
+	// NOTE: it's important that chown happens before setting xattrs due to the fact that chown will
+	// reset the security.capabilities xattr which results in file capabilities being lost.
+	if err := os.Lchown(ca.dstPath, int(ca.srcStat.Uid), int(ca.srcStat.Gid)); err != nil {
+		return errors.Wrap(err, "failed to chown during apply")
+	}
+
+	if ca.srcStat.Mode&unix.S_IFMT != unix.S_IFLNK {
+		if err := unix.Chmod(ca.dstPath, ca.srcStat.Mode); err != nil {
+			return errors.Wrapf(err, "failed to chmod path %q during apply", ca.dstPath)
+		}
+	}
+
 	if ca.srcPath != "" {
 		xattrs, err := sysx.LListxattr(ca.srcPath)
 		if err != nil {
@@ -410,16 +422,6 @@ func (a *applier) applyCopy(ctx context.Context, ca *changeApply) error {
 		}
 	}
 
-	if err := os.Lchown(ca.dstPath, int(ca.srcStat.Uid), int(ca.srcStat.Gid)); err != nil {
-		return errors.Wrap(err, "failed to chown during apply")
-	}
-
-	if ca.srcStat.Mode&unix.S_IFMT != unix.S_IFLNK {
-		if err := unix.Chmod(ca.dstPath, ca.srcStat.Mode); err != nil {
-			return errors.Wrapf(err, "failed to chmod path %q during apply", ca.dstPath)
-		}
-	}
-
 	atimeSpec := unix.Timespec{Sec: ca.srcStat.Atim.Sec, Nsec: ca.srcStat.Atim.Nsec}
 	mtimeSpec := unix.Timespec{Sec: ca.srcStat.Mtim.Sec, Nsec: ca.srcStat.Mtim.Nsec}
 	if ca.srcStat.Mode&unix.S_IFMT != unix.S_IFDIR {