Skip to content

Commit

Permalink
grpcclient: return proper nil reference from grpcclient
Browse files Browse the repository at this point in the history
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 <jonathan.sternberg@docker.com>
  • Loading branch information
jsternberg committed Oct 8, 2024
1 parent 6860c80 commit 40fb31b
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 20 deletions.
36 changes: 36 additions & 0 deletions frontend/dockerfile/dockerfile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ var allTests = integration.TestFuncs(
testCommandSourceMapping,
testSBOMScannerArgs,
testNilContextInSolveGateway,
testMultiNilRefsInSolveGateway,
testCopyUnicodePath,
testFrontendDeduplicateSources,
testDuplicateLayersProvenance,
Expand Down Expand Up @@ -8297,6 +8298,41 @@ func testNilContextInSolveGateway(t *testing.T, sb integration.Sandbox) {
require.ErrorContains(t, err, "invalid nil input definition to definition op")
}

func testMultiNilRefsInSolveGateway(t *testing.T, sb integration.Sandbox) {
workers.CheckFeatureCompat(t, sb, workers.FeatureMultiPlatform)
ctx := sb.Context()

c, err := client.New(ctx, sb.Address())
require.NoError(t, err)
defer c.Close()

f := getFrontend(t, sb)

_, err = c.Build(sb.Context(), client.SolveOpt{}, "", func(ctx context.Context, c gateway.Client) (*gateway.Result, error) {
localDockerfile, err := llb.Scratch().
File(llb.Mkfile("Dockerfile", 0644, []byte(`FROM scratch`))).
Marshal(ctx)
if err != nil {
return nil, err
}

res, err := f.SolveGateway(ctx, c, gateway.SolveRequest{
Frontend: "dockerfile.v0",
FrontendOpt: map[string]string{
"platform": "linux/amd64,linux/arm64",
},
FrontendInputs: map[string]*pb.Definition{
dockerui.DefaultLocalNameDockerfile: localDockerfile.ToPB(),
},
})
if err != nil {
return nil, err
}
return res, nil
}, nil)
require.NoError(t, err)
}

func testCopyUnicodePath(t *testing.T, sb integration.Sandbox) {
f := getFrontend(t, sb)
c, err := client.New(sb.Context(), sb.Address())
Expand Down
29 changes: 9 additions & 20 deletions frontend/gateway/grpcclient/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -429,28 +429,21 @@ func (c *grpcClient) Solve(ctx context.Context, creq client.SolveRequest) (res *
}
case *pb.Result_RefsDeprecated:
for k, v := range pbRes.RefsDeprecated.Refs {
ref := &reference{id: v, c: c}
if v == "" {
ref = nil
var ref client.Reference
if v != "" {
ref = &reference{id: v, c: c}
}
res.AddRef(k, ref)
}
case *pb.Result_Ref:
if pbRes.Ref.Id != "" {
ref, err := newReference(c, pbRes.Ref)
if err != nil {
return nil, err
}
res.SetRef(ref)
res.SetRef(newReference(c, pbRes.Ref))
}
case *pb.Result_Refs:
for k, v := range pbRes.Refs.Refs {
var ref *reference
var ref client.Reference
if v.Id != "" {
ref, err = newReference(c, v)
if err != nil {
return nil, err
}
ref = newReference(c, v)
}
res.AddRef(k, ref)
}
Expand All @@ -464,11 +457,7 @@ func (c *grpcClient) Solve(ctx context.Context, creq client.SolveRequest) (res *
return nil, err
}
if a.Ref.Id != "" {
ref, err := newReference(c, a.Ref)
if err != nil {
return nil, err
}
att.Ref = ref
att.Ref = newReference(c, a.Ref)
}
res.AddAttestation(p, *att)
}
Expand Down Expand Up @@ -1168,8 +1157,8 @@ type reference struct {
def *opspb.Definition
}

func newReference(c *grpcClient, ref *pb.Ref) (*reference, error) {
return &reference{c: c, id: ref.Id, def: ref.Def}, nil
func newReference(c *grpcClient, ref *pb.Ref) *reference {
return &reference{c: c, id: ref.Id, def: ref.Def}
}

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

0 comments on commit 40fb31b

Please sign in to comment.