Merge pull request #5545 from tonistiigi/v0.17.2-picks

[v0.17] cherry-picks for v0.17.2

Author: crazy-max

go.mod

@@ -101,7 +101,7 @@ require (
 	golang.org/x/sys v0.26.0
 	golang.org/x/time v0.6.0
 	google.golang.org/genproto/googleapis/rpc v0.0.0-20240604185151-ef581f913117
-	google.golang.org/grpc v1.66.2
+	google.golang.org/grpc v1.66.3
 	google.golang.org/grpc/cmd/protoc-gen-go-grpc v1.5.1
 	google.golang.org/protobuf v1.35.1
 	kernel.org/pub/linux/libs/security/libcap/cap v1.2.70

go.sum

@@ -538,8 +538,8 @@ google.golang.org/grpc v1.23.0/go.mod h1:Y5yQAOtifL1yxbo5wqy6BxZv8vAUGQwXBOALyac
 google.golang.org/grpc v1.25.1/go.mod h1:c3i+UQWmh7LiEpx4sFZnkU36qjEYZ0imhYfXVyQciAY=
 google.golang.org/grpc v1.27.0/go.mod h1:qbnxyOmOxrQa7FizSgH+ReBfzJrCY1pSN7KXBS8abTk=
 google.golang.org/grpc v1.33.2/go.mod h1:JMHMWHQWaTccqQQlmk3MJZS+GWXOdAesneDmEnv2fbc=
-google.golang.org/grpc v1.66.2 h1:3QdXkuq3Bkh7w+ywLdLvM56cmGvQHUMZpiCzt6Rqaoo=
-google.golang.org/grpc v1.66.2/go.mod h1:s3/l6xSSCURdVfAnL+TqCNMyTDAGN6+lZeVxnZR128Y=
+google.golang.org/grpc v1.66.3 h1:TWlsh8Mv0QI/1sIbs1W36lqRclxrmF+eFJ4DbI0fuhA=
+google.golang.org/grpc v1.66.3/go.mod h1:s3/l6xSSCURdVfAnL+TqCNMyTDAGN6+lZeVxnZR128Y=
 google.golang.org/grpc/cmd/protoc-gen-go-grpc v1.5.1 h1:F29+wU6Ee6qgu9TddPgooOdaqsxTMunOoj8KA5yuS5A=
 google.golang.org/grpc/cmd/protoc-gen-go-grpc v1.5.1/go.mod h1:5KF+wpkbTSbGcR9zteSqZV6fqFOWBl4Yde8En8MryZA=
 google.golang.org/protobuf v0.0.0-20200109180630-ec00e32a8dfd/go.mod h1:DFci5gLYBciE7Vtevhsrf46CRTquxDuWsQurQQe4oz8=

solver/llbsolver/testdata/gogoproto.data

@@ -14,7 +14,6 @@ import (
 	digest "github.com/opencontainers/go-digest"
 	ocispecs "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
-	"google.golang.org/protobuf/proto"
 )
 
 type vertex struct {
@@ -208,7 +207,6 @@ func recomputeDigests(ctx context.Context, all map[digest.Digest]*pb.Op, visited
 		return "", errors.Errorf("invalid missing input digest %s", dgst)
 	}
 
-	var mutated bool
 	for _, input := range op.Inputs {
 		select {
 		case <-ctx.Done():
@@ -220,25 +218,20 @@ func recomputeDigests(ctx context.Context, all map[digest.Digest]*pb.Op, visited
 		if err != nil {
 			return "", err
 		}
-		if digest.Digest(input.Digest) != iDgst {
-			mutated = true
-			input.Digest = string(iDgst)
-		}
-	}
-
-	if !mutated {
-		visited[dgst] = dgst
-		return dgst, nil
+		input.Digest = string(iDgst)
 	}
 
-	dt, err := deterministicMarshal(op)
+	dt, err := op.Marshal()
 	if err != nil {
 		return "", err
 	}
+
 	newDgst := digest.FromBytes(dt)
+	if newDgst != dgst {
+		all[newDgst] = op
+		delete(all, dgst)
+	}
 	visited[dgst] = newDgst
-	all[newDgst] = op
-	delete(all, dgst)
 	return newDgst, nil
 }
 
@@ -250,7 +243,6 @@ func loadLLB(ctx context.Context, def *pb.Definition, polEngine SourcePolicyEval
 	}
 
 	allOps := make(map[digest.Digest]*pb.Op)
-	mutatedDigests := make(map[digest.Digest]digest.Digest) // key: old, val: new
 
 	var lastDgst digest.Digest
 
@@ -261,27 +253,18 @@ func loadLLB(ctx context.Context, def *pb.Definition, polEngine SourcePolicyEval
 		}
 		dgst := digest.FromBytes(dt)
 		if polEngine != nil {
-			mutated, err := polEngine.Evaluate(ctx, op.GetSource())
-			if err != nil {
+			if _, err := polEngine.Evaluate(ctx, op.GetSource()); err != nil {
 				return solver.Edge{}, errors.Wrap(err, "error evaluating the source policy")
 			}
-			if mutated {
-				dtMutated, err := deterministicMarshal(&op)
-				if err != nil {
-					return solver.Edge{}, err
-				}
-				dgstMutated := digest.FromBytes(dtMutated)
-				mutatedDigests[dgst] = dgstMutated
-				dgst = dgstMutated
-			}
 		}
+
 		allOps[dgst] = &op
 		lastDgst = dgst
 	}
 
+	mutatedDigests := make(map[digest.Digest]digest.Digest) // key: old, val: new
 	for dgst := range allOps {
-		_, err := recomputeDigests(ctx, allOps, mutatedDigests, dgst)
-		if err != nil {
+		if _, err := recomputeDigests(ctx, allOps, mutatedDigests, dgst); err != nil {
 			return solver.Edge{}, err
 		}
 	}
@@ -400,7 +383,3 @@ func fileOpName(actions []*pb.FileAction) string {
 
 	return strings.Join(names, ", ")
 }
-
-func deterministicMarshal[Message proto.Message](m Message) ([]byte, error) {
-	return proto.MarshalOptions{Deterministic: true}.Marshal(m)
-}

solver/llbsolver/vertex.go

@@ -2,6 +2,8 @@ package llbsolver
 
 import (
 	"context"
+	_ "embed"
+	"fmt"
 	"testing"
 
 	"github.com/moby/buildkit/solver/pb"
@@ -53,3 +55,62 @@ func TestRecomputeDigests(t *testing.T) {
 	require.Equal(t, newDigest, digest.Digest(op2.Inputs[0].Digest))
 	assert.NotEqual(t, op2Digest, updated)
 }
+
+//go:embed testdata/gogoproto.data
+var gogoprotoData []byte
+
+func TestIngestDigest(t *testing.T) {
+	op1 := &pb.Op{
+		Op: &pb.Op_Source{
+			Source: &pb.SourceOp{
+				Identifier: "docker-image://docker.io/library/busybox:latest",
+			},
+		},
+	}
+	op1Data, err := op1.Marshal()
+	require.NoError(t, err)
+	op1Digest := digest.FromBytes(op1Data)
+
+	op2 := &pb.Op{
+		Inputs: []*pb.Input{
+			{Digest: string(op1Digest)}, // Input is the old digest, this should be updated after recomputeDigests
+		},
+	}
+	op2Data, err := op2.Marshal()
+	require.NoError(t, err)
+	op2Digest := digest.FromBytes(op2Data)
+
+	var def pb.Definition
+	err = def.Unmarshal(gogoprotoData)
+	require.NoError(t, err)
+	require.Len(t, def.Def, 2)
+
+	// Read the definition from the test data and ensure it uses the
+	// canonical digests after recompute.
+	var lastDgst digest.Digest
+	all := map[digest.Digest]*pb.Op{}
+	for _, in := range def.Def {
+		op := new(pb.Op)
+		err := op.Unmarshal(in)
+		require.NoError(t, err)
+
+		lastDgst = digest.FromBytes(in)
+		all[lastDgst] = op
+	}
+	fmt.Println(all, lastDgst)
+
+	visited := map[digest.Digest]digest.Digest{}
+	newDgst, err := recomputeDigests(context.Background(), all, visited, lastDgst)
+	require.NoError(t, err)
+	require.Len(t, visited, 2)
+	require.Equal(t, op2Digest, newDgst)
+	require.Equal(t, op2Digest, visited[newDgst])
+	delete(visited, newDgst)
+
+	// Last element should correspond to op1.
+	// The old digest doesn't really matter.
+	require.Len(t, visited, 1)
+	for _, newDgst := range visited {
+		require.Equal(t, op1Digest, newDgst)
+	}
+}

solver/llbsolver/vertex_test.go

@@ -1,5 +1,7 @@
 package pb
 
+import proto "google.golang.org/protobuf/proto"
+
 func (m *Definition) IsNil() bool {
 	return m == nil || m.Metadata == nil
 }
@@ -13,7 +15,7 @@ func (m *Definition) Unmarshal(dAtA []byte) error {
 }
 
 func (m *Op) Marshal() ([]byte, error) {
-	return m.MarshalVT()
+	return proto.MarshalOptions{Deterministic: true}.Marshal(m)
 }
 
 func (m *Op) Unmarshal(dAtA []byte) error {

solver/pb/ops.go

@@ -971,7 +971,7 @@ google.golang.org/genproto/googleapis/api/httpbody
 google.golang.org/genproto/googleapis/rpc/code
 google.golang.org/genproto/googleapis/rpc/errdetails
 google.golang.org/genproto/googleapis/rpc/status
-# google.golang.org/grpc v1.66.2
+# google.golang.org/grpc v1.66.3
 ## explicit; go 1.21
 google.golang.org/grpc
 google.golang.org/grpc/attributes