Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resolver: update Resolver.Scheme() docstring to mention requirement of lowercase scheme names #6014

Merged
merged 6 commits into from Feb 15, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 4 additions & 1 deletion clientconn.go
Expand Up @@ -1527,9 +1527,12 @@ func (c *channelzChannel) ChannelzMetric() *channelz.ChannelInternalMetric {
// referenced by users.
var ErrClientConnTimeout = errors.New("grpc: timed out when dialing")

// getResolver finds the scheme in the cc's resolvers or the global registry.
// scheme should always be lowercase (typically by virtue of url.Parse()
// performing proper RFC3986 behavior).
func (cc *ClientConn) getResolver(scheme string) resolver.Builder {
for _, rb := range cc.dopts.resolvers {
if scheme == rb.Scheme() {
if scheme == strings.ToLower(rb.Scheme()) {
return rb
}
}
Expand Down
9 changes: 6 additions & 3 deletions resolver/resolver.go
Expand Up @@ -41,14 +41,15 @@ var (

// TODO(bar) install dns resolver in init(){}.

// Register registers the resolver builder to the resolver map. b.Scheme will be
// used as the scheme registered with this builder.
// Register registers the resolver builder to the resolver map. b.Scheme will
// be used as the scheme registered with this builder. The registry is case
// insensitive.
//
// NOTE: this function must only be called during initialization time (i.e. in
// an init() function), and is not thread-safe. If multiple Resolvers are
// registered with the same name, the one registered last will take effect.
func Register(b Builder) {
m[b.Scheme()] = b
m[strings.ToLower(b.Scheme())] = b
}

// Get returns the resolver builder registered with the given scheme.
Expand Down Expand Up @@ -291,6 +292,8 @@ type Builder interface {
Build(target Target, cc ClientConn, opts BuildOptions) (Resolver, error)
// Scheme returns the scheme supported by this resolver.
// Scheme is defined at https://github.com/grpc/grpc/blob/master/doc/naming.md.
// The returned string should not contain upper-case characters, but will be
// converted by gRPC at runtime if needed.
Scheme() string
}

Expand Down
105 changes: 105 additions & 0 deletions resolver_test.go
@@ -0,0 +1,105 @@
/*
*
* 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 grpc

import (
"context"
"fmt"
"net"
"testing"

"google.golang.org/grpc/credentials/insecure"
"google.golang.org/grpc/resolver"
)

type wrapResolverBuilder struct {
resolver.Builder
scheme string
}

func (w *wrapResolverBuilder) Scheme() string {
return w.scheme
}

func init() {
resolver.Register(&wrapResolverBuilder{Builder: resolver.Get("dns"), scheme: "caseTEST"})
resolver.Register(&wrapResolverBuilder{Builder: resolver.Get("dns"), scheme: "caseTest2"})
resolver.Register(&wrapResolverBuilder{Builder: resolver.Get("passthrough"), scheme: "caSetest2"})
}

func (s) TestResolverCaseSensitivity(t *testing.T) {
// This should find the "caseTEST" resolver, which is "dns"
target := "casetest:///localhost:1234"
cc, err := Dial(target, WithTransportCredentials(insecure.NewCredentials()))
if err != nil {
t.Fatalf("Unexpected Dial(%q) error: %v", target, err)
}
cc.Close()

// This should find the "caSetest2" resolver instead of the "caseTest2"
// resolver, because it was registered later. "caSetest2" is "passthrough"
// and the overwritten one is "dns". With "passthrough" the dialer should
// see the target's address directly, but "dns" would be converted into a
// loopback IP (v4 or v6) address.
target = "caseTest2:///localhost:1234"
addrCh := make(chan string)
customDialer := func(ctx context.Context, addr string) (net.Conn, error) {
select {
case addrCh <- addr:
default:
}
return nil, fmt.Errorf("not dialing with custom dialer")
}

cc, err = Dial(target, WithContextDialer(customDialer), WithTransportCredentials(insecure.NewCredentials()))
if err != nil {
t.Fatalf("Unexpected Dial(%q) error: %v", target, err)
}
cc.Connect()
if got, want := <-addrCh, "localhost:1234"; got != want {
cc.Close()
t.Fatalf("Dialer got address %q; wanted %q", got, want)
}
cc.Close()

// Clear addrCh for future use.
select {
case <-addrCh:
default:
}

res := &wrapResolverBuilder{Builder: resolver.Get("dns"), scheme: "CaSetest2"}
// This should find the injected resolver instead of the "caSetest2"
// globally-registered resolver. Since it is "dns" instead of passthrough,
// we can validate by checking for an address that has been resolved
// (i.e. is not "localhost:port").

// WithDisableServiceConfig disables TXT lookups, which can hang for
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

// "localhost".
cc, err = Dial(target, WithContextDialer(customDialer), WithResolvers(res),
WithTransportCredentials(insecure.NewCredentials()), WithDisableServiceConfig())
if err != nil {
t.Fatalf("Unexpected Dial(%q) error: %v", target, err)
}
cc.Connect()
defer cc.Close()
if got, wantNot := <-addrCh, "localhost:1234"; got == wantNot {
t.Fatalf("Dialer got address %q; wanted something other than %q", got, wantNot)
}
}