From 3fe1123b7931ef3bf59fdb673bd33b989a30ed47 Mon Sep 17 00:00:00 2001 From: Henrique Vicente Date: Tue, 7 Nov 2023 22:09:43 +0100 Subject: [PATCH] resolver: manual resolver crashes if grpc.Dial isn't called before some methods (#6754) --- resolver/manual/manual.go | 15 +++++-- resolver/manual/manual_test.go | 73 ++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 4 deletions(-) create mode 100644 resolver/manual/manual_test.go diff --git a/resolver/manual/manual.go b/resolver/manual/manual.go index 0a4262342f35..f2efa2a2cb5a 100644 --- a/resolver/manual/manual.go +++ b/resolver/manual/manual.go @@ -78,12 +78,12 @@ func (r *Resolver) InitialState(s resolver.State) { func (r *Resolver) Build(target resolver.Target, cc resolver.ClientConn, opts resolver.BuildOptions) (resolver.Resolver, error) { r.BuildCallback(target, cc, opts) r.mu.Lock() + defer r.mu.Unlock() r.CC = cc if r.lastSeenState != nil { err := r.CC.UpdateState(*r.lastSeenState) go r.UpdateStateCallback(err) } - r.mu.Unlock() return r, nil } @@ -105,15 +105,22 @@ func (r *Resolver) Close() { // UpdateState calls CC.UpdateState. func (r *Resolver) UpdateState(s resolver.State) { r.mu.Lock() - err := r.CC.UpdateState(s) + defer r.mu.Unlock() + var err error + if r.CC == nil { + panic("cannot update state as grpc.Dial with resolver has not been called") + } + err = r.CC.UpdateState(s) r.lastSeenState = &s - r.mu.Unlock() r.UpdateStateCallback(err) } // ReportError calls CC.ReportError. func (r *Resolver) ReportError(err error) { r.mu.Lock() + defer r.mu.Unlock() + if r.CC == nil { + panic("cannot report error as grpc.Dial with resolver has not been called") + } r.CC.ReportError(err) - r.mu.Unlock() } diff --git a/resolver/manual/manual_test.go b/resolver/manual/manual_test.go new file mode 100644 index 000000000000..3c118134b870 --- /dev/null +++ b/resolver/manual/manual_test.go @@ -0,0 +1,73 @@ +/* + * + * Copyright 2023 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package manual_test + +import ( + "errors" + "testing" + + "google.golang.org/grpc" + "google.golang.org/grpc/credentials/insecure" + "google.golang.org/grpc/resolver" + "google.golang.org/grpc/resolver/manual" +) + +func TestResolver(t *testing.T) { + r := manual.NewBuilderWithScheme("whatever") + r.InitialState(resolver.State{ + Addresses: []resolver.Address{ + {Addr: "address"}, + }, + }) + + t.Run("update_state_panics", func(t *testing.T) { + defer func() { + want := "cannot update state as grpc.Dial with resolver has not been called" + if r := recover(); r != want { + t.Errorf("expected panic %q, got %q", want, r) + } + }() + r.UpdateState(resolver.State{Addresses: []resolver.Address{ + {Addr: "address"}, + {Addr: "anotheraddress"}, + }}) + }) + t.Run("report_error_panics", func(t *testing.T) { + defer func() { + want := "cannot report error as grpc.Dial with resolver has not been called" + if r := recover(); r != want { + t.Errorf("expected panic %q, got %q", want, r) + } + }() + r.ReportError(errors.New("example")) + }) + + t.Run("happy_path", func(t *testing.T) { + _, err := grpc.Dial("whatever://localhost", + grpc.WithTransportCredentials(insecure.NewCredentials()), + grpc.WithResolvers(r)) + if err != nil { + t.Errorf("dial setup error: %v", err) + } + r.UpdateState(resolver.State{Addresses: []resolver.Address{ + {Addr: "ok"}, + }}) + r.ReportError(errors.New("example")) + }) +}