Skip to content

Commit eb8a9cb

Browse files
jsternbergtonistiigi
jsternberg
authored and
tonistiigi
committed
gateway: ensure llb digests are deterministic when sent by frontends
This ensures different valid protobuf serializations that are sent by frontends will be rewritten into digests that are normalized for the buildkit solver. The most recent example of this is that older frontends would generate protobuf with gogo and the newer buildkit is using the google protobuf library. These produce different serializations and cause the solver to think that identical operations are actually different. Signed-off-by: Jonathan A. Sternberg <[email protected]> (cherry picked from commit 9f65f8c)
1 parent 531826b commit eb8a9cb

4 files changed

Lines changed: 75 additions & 33 deletions

File tree

‎solver/llbsolver/testdata/gogoproto.data‎

284 Bytes
Binary file not shown.

‎solver/llbsolver/vertex.go‎

Lines changed: 11 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import (
1414
digest "github.com/opencontainers/go-digest"
1515
ocispecs "github.com/opencontainers/image-spec/specs-go/v1"
1616
"github.com/pkg/errors"
17-
"google.golang.org/protobuf/proto"
1817
)
1918

2019
type vertex struct {
@@ -208,7 +207,6 @@ func recomputeDigests(ctx context.Context, all map[digest.Digest]*pb.Op, visited
208207
return "", errors.Errorf("invalid missing input digest %s", dgst)
209208
}
210209

211-
var mutated bool
212210
for _, input := range op.Inputs {
213211
select {
214212
case <-ctx.Done():
@@ -220,25 +218,20 @@ func recomputeDigests(ctx context.Context, all map[digest.Digest]*pb.Op, visited
220218
if err != nil {
221219
return "", err
222220
}
223-
if digest.Digest(input.Digest) != iDgst {
224-
mutated = true
225-
input.Digest = string(iDgst)
226-
}
227-
}
228-
229-
if !mutated {
230-
visited[dgst] = dgst
231-
return dgst, nil
221+
input.Digest = string(iDgst)
232222
}
233223

234-
dt, err := deterministicMarshal(op)
224+
dt, err := op.Marshal()
235225
if err != nil {
236226
return "", err
237227
}
228+
238229
newDgst := digest.FromBytes(dt)
230+
if newDgst != dgst {
231+
all[newDgst] = op
232+
delete(all, dgst)
233+
}
239234
visited[dgst] = newDgst
240-
all[newDgst] = op
241-
delete(all, dgst)
242235
return newDgst, nil
243236
}
244237

@@ -250,7 +243,6 @@ func loadLLB(ctx context.Context, def *pb.Definition, polEngine SourcePolicyEval
250243
}
251244

252245
allOps := make(map[digest.Digest]*pb.Op)
253-
mutatedDigests := make(map[digest.Digest]digest.Digest) // key: old, val: new
254246

255247
var lastDgst digest.Digest
256248

@@ -261,27 +253,18 @@ func loadLLB(ctx context.Context, def *pb.Definition, polEngine SourcePolicyEval
261253
}
262254
dgst := digest.FromBytes(dt)
263255
if polEngine != nil {
264-
mutated, err := polEngine.Evaluate(ctx, op.GetSource())
265-
if err != nil {
256+
if _, err := polEngine.Evaluate(ctx, op.GetSource()); err != nil {
266257
return solver.Edge{}, errors.Wrap(err, "error evaluating the source policy")
267258
}
268-
if mutated {
269-
dtMutated, err := deterministicMarshal(&op)
270-
if err != nil {
271-
return solver.Edge{}, err
272-
}
273-
dgstMutated := digest.FromBytes(dtMutated)
274-
mutatedDigests[dgst] = dgstMutated
275-
dgst = dgstMutated
276-
}
277259
}
260+
278261
allOps[dgst] = &op
279262
lastDgst = dgst
280263
}
281264

265+
mutatedDigests := make(map[digest.Digest]digest.Digest) // key: old, val: new
282266
for dgst := range allOps {
283-
_, err := recomputeDigests(ctx, allOps, mutatedDigests, dgst)
284-
if err != nil {
267+
if _, err := recomputeDigests(ctx, allOps, mutatedDigests, dgst); err != nil {
285268
return solver.Edge{}, err
286269
}
287270
}
@@ -400,7 +383,3 @@ func fileOpName(actions []*pb.FileAction) string {
400383

401384
return strings.Join(names, ", ")
402385
}
403-
404-
func deterministicMarshal[Message proto.Message](m Message) ([]byte, error) {
405-
return proto.MarshalOptions{Deterministic: true}.Marshal(m)
406-
}

‎solver/llbsolver/vertex_test.go‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ package llbsolver
22

33
import (
44
"context"
5+
_ "embed"
6+
"fmt"
57
"testing"
68

79
"github.com/moby/buildkit/solver/pb"
@@ -53,3 +55,62 @@ func TestRecomputeDigests(t *testing.T) {
5355
require.Equal(t, newDigest, digest.Digest(op2.Inputs[0].Digest))
5456
assert.NotEqual(t, op2Digest, updated)
5557
}
58+
59+
//go:embed testdata/gogoproto.data
60+
var gogoprotoData []byte
61+
62+
func TestIngestDigest(t *testing.T) {
63+
op1 := &pb.Op{
64+
Op: &pb.Op_Source{
65+
Source: &pb.SourceOp{
66+
Identifier: "docker-image://docker.io/library/busybox:latest",
67+
},
68+
},
69+
}
70+
op1Data, err := op1.Marshal()
71+
require.NoError(t, err)
72+
op1Digest := digest.FromBytes(op1Data)
73+
74+
op2 := &pb.Op{
75+
Inputs: []*pb.Input{
76+
{Digest: string(op1Digest)}, // Input is the old digest, this should be updated after recomputeDigests
77+
},
78+
}
79+
op2Data, err := op2.Marshal()
80+
require.NoError(t, err)
81+
op2Digest := digest.FromBytes(op2Data)
82+
83+
var def pb.Definition
84+
err = def.Unmarshal(gogoprotoData)
85+
require.NoError(t, err)
86+
require.Len(t, def.Def, 2)
87+
88+
// Read the definition from the test data and ensure it uses the
89+
// canonical digests after recompute.
90+
var lastDgst digest.Digest
91+
all := map[digest.Digest]*pb.Op{}
92+
for _, in := range def.Def {
93+
op := new(pb.Op)
94+
err := op.Unmarshal(in)
95+
require.NoError(t, err)
96+
97+
lastDgst = digest.FromBytes(in)
98+
all[lastDgst] = op
99+
}
100+
fmt.Println(all, lastDgst)
101+
102+
visited := map[digest.Digest]digest.Digest{}
103+
newDgst, err := recomputeDigests(context.Background(), all, visited, lastDgst)
104+
require.NoError(t, err)
105+
require.Len(t, visited, 2)
106+
require.Equal(t, op2Digest, newDgst)
107+
require.Equal(t, op2Digest, visited[newDgst])
108+
delete(visited, newDgst)
109+
110+
// Last element should correspond to op1.
111+
// The old digest doesn't really matter.
112+
require.Len(t, visited, 1)
113+
for _, newDgst := range visited {
114+
require.Equal(t, op1Digest, newDgst)
115+
}
116+
}

‎solver/pb/ops.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package pb
22

3+
import proto "google.golang.org/protobuf/proto"
4+
35
func (m *Definition) IsNil() bool {
46
return m == nil || m.Metadata == nil
57
}
@@ -13,7 +15,7 @@ func (m *Definition) Unmarshal(dAtA []byte) error {
1315
}
1416

1517
func (m *Op) Marshal() ([]byte, error) {
16-
return m.MarshalVT()
18+
return proto.MarshalOptions{Deterministic: true}.Marshal(m)
1719
}
1820

1921
func (m *Op) Unmarshal(dAtA []byte) error {

0 commit comments

Comments
 (0)