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.

Sign up for GitHub

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

Jump to bottom

resolver: Add new fields to resolver.BuildOption struct to support dialing a remote name resolver #3098

Merged
merged 3 commits into from Nov 4, 2019
Merged

resolver: Add new fields to resolver.BuildOption struct to support dialing a remote name resolver #3098

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5d46e1b
Add new fields to resolver.BuildOption struct.
easwars Oct 14, 2019
5af9c01
Add code to populate the newly added fields.
easwars Oct 15, 2019
e947757
Review comments.
easwars Nov 1, 2019
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
20 changes: 19 additions & 1 deletion resolver/resolver.go
View file
Open in desktop
Expand Up @@ -21,6 +21,10 @@
package resolver

import (
"context"
"net"

"google.golang.org/grpc/credentials"
"google.golang.org/grpc/serviceconfig"
)

Expand Down Expand Up @@ -105,8 +109,22 @@ type Address struct {
// BuildOption includes additional information for the builder to create
// the resolver.
type BuildOption struct {
// DisableServiceConfig indicates whether resolver should fetch service config data.
// DisableServiceConfig indicates whether a resolver implementation should
// fetch service config data.
DisableServiceConfig bool
// DialCreds is the transport credentials that a resolver implementation
// can use to dial a remote name resolution server. Resolver
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this should say simply that it's the DialCreds for the ClientConn? It may not be appropriate to use these credentials for your name resolver in many (practically all?) cases. Same for the other two fields, except that a Bundle is a little more likely to be OK to use. Possibly include a warning about using this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does this sound OK? If so, I can make similar modifications to the other two fields as well.

DialCreds is the transport credentials used by the ClientConn (set as a DialOption
using a call to WithTransportCredentials) while communicating with the gRPC
server. In some cases, it may be appropriate for a resolver implementation to use 
these credentials to talk to a remote name resolution server. In most cases though, 
it might not be appropriate and should be used with caution.

// implementations which do not need to talk to another party securely can
// safely ignore this field.
DialCreds credentials.TransportCredentials
// CredsBundle is the credentials bundle that a resolver implementation can
// use while dialing a remote name resolution server. If both DialCreds and
// CredsBundle are set, the former takes precedence.
CredsBundle credentials.Bundle
// Dialer is the custom dialer that a resolver implementation can use to
// dial a remote name resolution server. Resolver implementations which do
// not need to talk to another party securely can safely ignore this field.
Dialer func(context.Context, string) (net.Conn, error)
}

// State contains the current Resolver state relevant to the ClientConn.
Expand Down
14 changes: 13 additions & 1 deletion resolver_conn_wrapper.go
View file
Open in desktop
Expand Up @@ -25,6 +25,7 @@ import (
"time"

"google.golang.org/grpc/balancer"
"google.golang.org/grpc/credentials"
"google.golang.org/grpc/grpclog"
"google.golang.org/grpc/internal/channelz"
"google.golang.org/grpc/internal/grpcsync"
Expand Down Expand Up @@ -90,13 +91,24 @@ func newCCResolverWrapper(cc *ClientConn) (*ccResolverWrapper, error) {
done: grpcsync.NewEvent(),
}

var credsClone credentials.TransportCredentials
if creds := cc.dopts.copts.TransportCredentials; creds != nil {
credsClone = creds.Clone()
}
rbo := resolver.BuildOption{
DisableServiceConfig: cc.dopts.disableServiceConfig,
DialCreds: credsClone,
CredsBundle: cc.dopts.copts.CredsBundle,
Dialer: cc.dopts.copts.Dialer,
}

var err error
// We need to hold the lock here while we assign to the ccr.resolver field
// to guard against a data race caused by the following code path,
// rb.Build-->ccr.ReportError-->ccr.poll-->ccr.resolveNow, would end up
// accessing ccr.resolver which is being assigned here.
ccr.resolverMu.Lock()
ccr.resolver, err = rb.Build(cc.parsedTarget, ccr, resolver.BuildOption{DisableServiceConfig: cc.dopts.disableServiceConfig})
ccr.resolver, err = rb.Build(cc.parsedTarget, ccr, rbo)
if err != nil {
return nil, err
}
Expand Down