Skip to content

Commit 3175345

Browse files
committed
grpcclient: return proper nil reference from grpcclient
The grpcclient would take an empty reference and convert it into a `(*reference)(nil)` and then store that in a `client.Reference`. This caused the `nil` check in `convertRef` to return false because it wasn't `nil` but then it panicked because it was `nil`. `newReference` has been minorly refactored to not return an error. The method is not exported, the error value is not used, and it obscured the functionality which made it harder to use correctly. Signed-off-by: Jonathan A. Sternberg <[email protected]>
1 parent de2f8b6 commit 3175345

File tree

2 files changed

+45
-20
lines changed

2 files changed

+45
-20
lines changed

frontend/dockerfile/dockerfile_test.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,7 @@ var allTests = integration.TestFuncs(
196196
testCommandSourceMapping,
197197
testSBOMScannerArgs,
198198
testNilContextInSolveGateway,
199+
testMultiNilRefsInSolveGateway,
199200
testCopyUnicodePath,
200201
testFrontendDeduplicateSources,
201202
testDuplicateLayersProvenance,
@@ -8297,6 +8298,41 @@ func testNilContextInSolveGateway(t *testing.T, sb integration.Sandbox) {
82978298
require.ErrorContains(t, err, "invalid nil input definition to definition op")
82988299
}
82998300

8301+
func testMultiNilRefsInSolveGateway(t *testing.T, sb integration.Sandbox) {
8302+
workers.CheckFeatureCompat(t, sb, workers.FeatureMultiPlatform)
8303+
ctx := sb.Context()
8304+
8305+
c, err := client.New(ctx, sb.Address())
8306+
require.NoError(t, err)
8307+
defer c.Close()
8308+
8309+
f := getFrontend(t, sb)
8310+
8311+
_, err = c.Build(sb.Context(), client.SolveOpt{}, "", func(ctx context.Context, c gateway.Client) (*gateway.Result, error) {
8312+
localDockerfile, err := llb.Scratch().
8313+
File(llb.Mkfile("Dockerfile", 0644, []byte(`FROM scratch`))).
8314+
Marshal(ctx)
8315+
if err != nil {
8316+
return nil, err
8317+
}
8318+
8319+
res, err := f.SolveGateway(ctx, c, gateway.SolveRequest{
8320+
Frontend: "dockerfile.v0",
8321+
FrontendOpt: map[string]string{
8322+
"platform": "linux/amd64,linux/arm64",
8323+
},
8324+
FrontendInputs: map[string]*pb.Definition{
8325+
dockerui.DefaultLocalNameDockerfile: localDockerfile.ToPB(),
8326+
},
8327+
})
8328+
if err != nil {
8329+
return nil, err
8330+
}
8331+
return res, nil
8332+
}, nil)
8333+
require.NoError(t, err)
8334+
}
8335+
83008336
func testCopyUnicodePath(t *testing.T, sb integration.Sandbox) {
83018337
f := getFrontend(t, sb)
83028338
c, err := client.New(sb.Context(), sb.Address())

frontend/gateway/grpcclient/client.go

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -429,28 +429,21 @@ func (c *grpcClient) Solve(ctx context.Context, creq client.SolveRequest) (res *
429429
}
430430
case *pb.Result_RefsDeprecated:
431431
for k, v := range pbRes.RefsDeprecated.Refs {
432-
ref := &reference{id: v, c: c}
433-
if v == "" {
434-
ref = nil
432+
var ref client.Reference
433+
if v != "" {
434+
ref = &reference{id: v, c: c}
435435
}
436436
res.AddRef(k, ref)
437437
}
438438
case *pb.Result_Ref:
439439
if pbRes.Ref.Id != "" {
440-
ref, err := newReference(c, pbRes.Ref)
441-
if err != nil {
442-
return nil, err
443-
}
444-
res.SetRef(ref)
440+
res.SetRef(newReference(c, pbRes.Ref))
445441
}
446442
case *pb.Result_Refs:
447443
for k, v := range pbRes.Refs.Refs {
448-
var ref *reference
444+
var ref client.Reference
449445
if v.Id != "" {
450-
ref, err = newReference(c, v)
451-
if err != nil {
452-
return nil, err
453-
}
446+
ref = newReference(c, v)
454447
}
455448
res.AddRef(k, ref)
456449
}
@@ -464,11 +457,7 @@ func (c *grpcClient) Solve(ctx context.Context, creq client.SolveRequest) (res *
464457
return nil, err
465458
}
466459
if a.Ref.Id != "" {
467-
ref, err := newReference(c, a.Ref)
468-
if err != nil {
469-
return nil, err
470-
}
471-
att.Ref = ref
460+
att.Ref = newReference(c, a.Ref)
472461
}
473462
res.AddAttestation(p, *att)
474463
}
@@ -1168,8 +1157,8 @@ type reference struct {
11681157
def *opspb.Definition
11691158
}
11701159

1171-
func newReference(c *grpcClient, ref *pb.Ref) (*reference, error) {
1172-
return &reference{c: c, id: ref.Id, def: ref.Def}, nil
1160+
func newReference(c *grpcClient, ref *pb.Ref) *reference {
1161+
return &reference{c: c, id: ref.Id, def: ref.Def}
11731162
}
11741163

11751164
func (r *reference) ToState() (st llb.State, err error) {

0 commit comments

Comments
 (0)