Skip to content

Commit

Permalink
creds/google: fix CFE cluster name check (#4893)
Browse files Browse the repository at this point in the history
  • Loading branch information
menghanl committed Oct 26, 2021
1 parent 4f21cde commit 03753f5
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 4 deletions.
2 changes: 1 addition & 1 deletion credentials/google/google_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ func TestClientHandshakeBasedOnClusterName(t *testing.T) {
{
name: "with CFE cluster name",
ctx: icredentials.NewClientHandshakeInfoContext(context.Background(), credentials.ClientHandshakeInfo{
Attributes: internal.SetXDSHandshakeClusterName(resolver.Address{}, cfeClusterName).Attributes,
Attributes: internal.SetXDSHandshakeClusterName(resolver.Address{}, "google_cfe_bigtable.googleapis.com").Attributes,
}),
// CFE should use tls.
wantTyp: "tls",
Expand Down
7 changes: 4 additions & 3 deletions credentials/google/xds.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,19 @@ package google
import (
"context"
"net"
"strings"

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

const cfeClusterName = "google-cfe"
const cfeClusterNamePrefix = "google_cfe_"

// clusterTransportCreds is a combo of TLS + ALTS.
//
// On the client, ClientHandshake picks TLS or ALTS based on address attributes.
// - if attributes has cluster name
// - if cluster name is "google_cfe", use TLS
// - if cluster name has prefix "google_cfe_", use TLS
// - otherwise, use ALTS
// - else, do TLS
//
Expand All @@ -55,7 +56,7 @@ func (c *clusterTransportCreds) ClientHandshake(ctx context.Context, authority s
return c.tls.ClientHandshake(ctx, authority, rawConn)
}
cn, ok := internal.GetXDSHandshakeClusterName(chi.Attributes)
if !ok || cn == cfeClusterName {
if !ok || strings.HasPrefix(cn, cfeClusterNamePrefix) {
return c.tls.ClientHandshake(ctx, authority, rawConn)
}
// If attributes have cluster name, and cluster name is not cfe, it's a
Expand Down

0 comments on commit 03753f5

Please sign in to comment.