From c53203c581923c256e76e82eba1ec64f9744e684 Mon Sep 17 00:00:00 2001 From: Menghan Li Date: Mon, 8 Nov 2021 14:17:48 -0800 Subject: [PATCH] xds/federation: support populating resource template in xds-resolver (#4900) --- xds/internal/resolver/xds_resolver.go | 45 +++- xds/internal/resolver/xds_resolver_test.go | 226 +++++++++++++----- xds/internal/testutils/fakeclient/client.go | 5 +- xds/internal/xdsclient/bootstrap/template.go | 47 ++++ .../xdsclient/bootstrap/template_test.go | 97 ++++++++ xds/server.go | 7 +- 6 files changed, 349 insertions(+), 78 deletions(-) create mode 100644 xds/internal/xdsclient/bootstrap/template.go create mode 100644 xds/internal/xdsclient/bootstrap/template_test.go diff --git a/xds/internal/resolver/xds_resolver.go b/xds/internal/resolver/xds_resolver.go index 19ee01773e8..e04ce00b2d2 100644 --- a/xds/internal/resolver/xds_resolver.go +++ b/xds/internal/resolver/xds_resolver.go @@ -22,6 +22,7 @@ package resolver import ( "errors" "fmt" + "strings" "google.golang.org/grpc/credentials" "google.golang.org/grpc/internal/grpclog" @@ -30,6 +31,7 @@ import ( iresolver "google.golang.org/grpc/internal/resolver" "google.golang.org/grpc/resolver" "google.golang.org/grpc/xds/internal/xdsclient" + "google.golang.org/grpc/xds/internal/xdsclient/bootstrap" ) const xdsScheme = "xds" @@ -60,7 +62,7 @@ type xdsResolverBuilder struct { // // The xds bootstrap process is performed (and a new xds client is built) every // time an xds resolver is built. -func (b *xdsResolverBuilder) Build(t resolver.Target, cc resolver.ClientConn, opts resolver.BuildOptions) (resolver.Resolver, error) { +func (b *xdsResolverBuilder) Build(t resolver.Target, cc resolver.ClientConn, opts resolver.BuildOptions) (_ resolver.Resolver, retErr error) { r := &xdsResolver{ target: t, cc: cc, @@ -68,7 +70,14 @@ func (b *xdsResolverBuilder) Build(t resolver.Target, cc resolver.ClientConn, op updateCh: make(chan suWithError, 1), activeClusters: make(map[string]*clusterInfo), } - r.logger = prefixLogger((r)) + defer func() { + if retErr != nil { + if r.client != nil { + r.client.Close() + } + } + }() + r.logger = prefixLogger(r) r.logger.Infof("Creating resolver for target: %+v", t) newXDSClient := newXDSClient @@ -81,6 +90,10 @@ func (b *xdsResolverBuilder) Build(t resolver.Target, cc resolver.ClientConn, op return nil, fmt.Errorf("xds: failed to create xds-client: %v", err) } r.client = client + bootstrapConfig := client.BootstrapConfig() + if bootstrapConfig == nil { + return nil, errors.New("bootstrap configuration is empty") + } // If xds credentials were specified by the user, but bootstrap configs do // not contain any certificate provider configuration, it is better to fail @@ -94,14 +107,36 @@ func (b *xdsResolverBuilder) Build(t resolver.Target, cc resolver.ClientConn, op creds = opts.CredsBundle.TransportCredentials() } if xc, ok := creds.(interface{ UsesXDS() bool }); ok && xc.UsesXDS() { - bc := client.BootstrapConfig() - if len(bc.CertProviderConfigs) == 0 { + if len(bootstrapConfig.CertProviderConfigs) == 0 { return nil, errors.New("xds: xdsCreds specified but certificate_providers config missing in bootstrap file") } } + // Find the client listener template to use from the bootstrap config: + // - If authority is not set in the target, use the top level template + // - If authority is set, use the template from the authority map. + template := bootstrapConfig.ClientDefaultListenerResourceNameTemplate + if authority := r.target.URL.Host; authority != "" { + a := bootstrapConfig.Authorities[authority] + if a == nil { + return nil, fmt.Errorf("xds: authority %q is not found in the bootstrap file", authority) + } + if a.ClientListenerResourceNameTemplate != "" { + // This check will never be false, because + // ClientListenerResourceNameTemplate is required to start with + // xdstp://, and has a default value (not an empty string) if unset. + template = a.ClientListenerResourceNameTemplate + } + } + endpoint := r.target.URL.Path + if endpoint == "" { + endpoint = r.target.URL.Opaque + } + endpoint = strings.TrimPrefix(endpoint, "/") + resourceName := bootstrap.PopulateResourceTemplate(template, endpoint) + // Register a watch on the xdsClient for the user's dial target. - cancelWatch := watchService(r.client, r.target.Endpoint, r.handleServiceUpdate, r.logger) + cancelWatch := watchService(r.client, resourceName, r.handleServiceUpdate, r.logger) r.logger.Infof("Watch started on resource name %v with xds-client %p", r.target.Endpoint, r.client) r.cancelWatch = func() { cancelWatch() diff --git a/xds/internal/resolver/xds_resolver_test.go b/xds/internal/resolver/xds_resolver_test.go index c05a7422904..e366b9fb626 100644 --- a/xds/internal/resolver/xds_resolver_test.go +++ b/xds/internal/resolver/xds_resolver_test.go @@ -21,6 +21,7 @@ package resolver import ( "context" "errors" + "net/url" "reflect" "strings" "testing" @@ -62,7 +63,7 @@ const ( defaultTestShortTimeout = 100 * time.Microsecond ) -var target = resolver.Target{Endpoint: targetStr} +var target = resolver.Target{Endpoint: targetStr, URL: url.URL{Scheme: "xds", Path: "/" + targetStr}} var routerFilter = xdsresource.HTTPFilter{Name: "rtr", Filter: httpfilter.Get(router.TypeURL)} var routerFilterList = []xdsresource.HTTPFilter{routerFilter} @@ -117,6 +118,7 @@ func (s) TestResolverBuilder(t *testing.T) { tests := []struct { name string xdsClientFunc func() (xdsclient.XDSClient, error) + target resolver.Target wantErr bool }{ { @@ -124,6 +126,7 @@ func (s) TestResolverBuilder(t *testing.T) { xdsClientFunc: func() (xdsclient.XDSClient, error) { return fakeclient.NewClient(), nil }, + target: target, wantErr: false, }, { @@ -131,6 +134,29 @@ func (s) TestResolverBuilder(t *testing.T) { xdsClientFunc: func() (xdsclient.XDSClient, error) { return nil, errors.New("newXDSClient-throws-error") }, + target: target, + wantErr: true, + }, + { + name: "authority not defined in bootstrap", + xdsClientFunc: func() (xdsclient.XDSClient, error) { + c := fakeclient.NewClient() + c.SetBootstrapConfig(&bootstrap.Config{ + ClientDefaultListenerResourceNameTemplate: "%s", + Authorities: map[string]*bootstrap.Authority{ + "test-authority": { + ClientListenerResourceNameTemplate: "xdstp://test-authority/%s", + }, + }, + }) + return c, nil + }, + target: resolver.Target{ + URL: url.URL{ + Host: "non-existing-authority", + Path: "/" + targetStr, + }, + }, wantErr: true, }, } @@ -148,7 +174,7 @@ func (s) TestResolverBuilder(t *testing.T) { t.Fatalf("resolver.Get(%v) returned nil", xdsScheme) } - r, err := builder.Build(target, newTestClientConn(), resolver.BuildOptions{}) + r, err := builder.Build(test.target, newTestClientConn(), resolver.BuildOptions{}) if (err != nil) != test.wantErr { t.Fatalf("builder.Build(%v) returned err: %v, wantErr: %v", target, err, test.wantErr) } @@ -168,13 +194,20 @@ func (s) TestResolverBuilder(t *testing.T) { func (s) TestResolverBuilder_xdsCredsBootstrapMismatch(t *testing.T) { // Fake out the xdsClient creation process by providing a fake, which does // not have any certificate provider configuration. + fc := fakeclient.NewClient() + fc.SetBootstrapConfig(&bootstrap.Config{}) oldClientMaker := newXDSClient newXDSClient = func() (xdsclient.XDSClient, error) { - fc := fakeclient.NewClient() - fc.SetBootstrapConfig(&bootstrap.Config{}) return fc, nil } defer func() { newXDSClient = oldClientMaker }() + defer func() { + select { + case <-time.After(defaultTestTimeout): + t.Fatalf("timeout waiting for close") + case <-fc.Closed.Done(): + } + }() builder := resolver.Get(xdsScheme) if builder == nil { @@ -196,29 +229,45 @@ func (s) TestResolverBuilder_xdsCredsBootstrapMismatch(t *testing.T) { } type setupOpts struct { - xdsClientFunc func() (xdsclient.XDSClient, error) + bootstrapC *bootstrap.Config + target resolver.Target } -func testSetup(t *testing.T, opts setupOpts) (*xdsResolver, *testClientConn, func()) { +func testSetup(t *testing.T, opts setupOpts) (*xdsResolver, *fakeclient.Client, *testClientConn, func()) { t.Helper() + fc := fakeclient.NewClient() + if opts.bootstrapC != nil { + fc.SetBootstrapConfig(opts.bootstrapC) + } oldClientMaker := newXDSClient - newXDSClient = opts.xdsClientFunc + newXDSClient = func() (xdsclient.XDSClient, error) { + return fc, nil + } cancel := func() { + // Make sure the xDS client is closed, in all (successful or failed) + // cases. + select { + case <-time.After(defaultTestTimeout): + t.Fatalf("timeout waiting for close") + case <-fc.Closed.Done(): + } newXDSClient = oldClientMaker } - builder := resolver.Get(xdsScheme) if builder == nil { t.Fatalf("resolver.Get(%v) returned nil", xdsScheme) } tcc := newTestClientConn() - r, err := builder.Build(target, tcc, resolver.BuildOptions{}) + r, err := builder.Build(opts.target, tcc, resolver.BuildOptions{}) if err != nil { t.Fatalf("builder.Build(%v) returned err: %v", target, err) } - return r.(*xdsResolver), tcc, cancel + return r.(*xdsResolver), fc, tcc, func() { + r.Close() + cancel() + } } // waitForWatchListener waits for the WatchListener method to be called on the @@ -251,13 +300,97 @@ func waitForWatchRouteConfig(ctx context.Context, t *testing.T, xdsC *fakeclient } } +// TestXDSResolverResourceNameToWatch tests that the correct resource name is +// used to watch for the service. This covers cases with different bootstrap +// config, and different authority. +func (s) TestXDSResolverResourceNameToWatch(t *testing.T) { + tests := []struct { + name string + bc *bootstrap.Config + target resolver.Target + want string + }{ + { + name: "default %s old style", + bc: &bootstrap.Config{ + ClientDefaultListenerResourceNameTemplate: "%s", + }, + target: resolver.Target{ + URL: url.URL{Path: "/" + targetStr}, + }, + want: targetStr, + }, + { + name: "old style no percent encoding", + bc: &bootstrap.Config{ + ClientDefaultListenerResourceNameTemplate: "/path/to/%s", + }, + target: resolver.Target{ + URL: url.URL{Path: "/" + targetStr}, + }, + want: "/path/to/" + targetStr, + }, + { + name: "new style with %s", + bc: &bootstrap.Config{ + ClientDefaultListenerResourceNameTemplate: "xdstp://authority.com/%s", + Authorities: nil, + }, + target: resolver.Target{ + URL: url.URL{Path: "/0.0.0.0:8080"}, + }, + want: "xdstp://authority.com/0.0.0.0:8080", + }, + { + name: "new style percent encoding", + bc: &bootstrap.Config{ + ClientDefaultListenerResourceNameTemplate: "xdstp://authority.com/%s", + Authorities: nil, + }, + target: resolver.Target{ + URL: url.URL{Path: "/[::1]:8080"}, + }, + want: "xdstp://authority.com/%5B::1%5D:8080", + }, + { + name: "new style different authority", + bc: &bootstrap.Config{ + ClientDefaultListenerResourceNameTemplate: "xdstp://authority.com/%s", + Authorities: map[string]*bootstrap.Authority{ + "test-authority": { + ClientListenerResourceNameTemplate: "xdstp://test-authority/%s", + }, + }, + }, + target: resolver.Target{ + URL: url.URL{ + Host: "test-authority", + Path: "/" + targetStr, + }, + }, + want: "xdstp://test-authority/" + targetStr, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + xdsR, xdsC, _, cancel := testSetup(t, setupOpts{ + bootstrapC: tt.bc, + target: tt.target, + }) + defer cancel() + defer xdsR.Close() + + ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) + defer cancel() + waitForWatchListener(ctx, t, xdsC, tt.want) + }) + } +} + // TestXDSResolverWatchCallbackAfterClose tests the case where a service update // from the underlying xdsClient is received after the resolver is closed. func (s) TestXDSResolverWatchCallbackAfterClose(t *testing.T) { - xdsC := fakeclient.NewClient() - xdsR, tcc, cancel := testSetup(t, setupOpts{ - xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil }, - }) + xdsR, xdsC, tcc, cancel := testSetup(t, setupOpts{target: target}) defer cancel() ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) @@ -286,10 +419,7 @@ func (s) TestXDSResolverWatchCallbackAfterClose(t *testing.T) { // TestXDSResolverCloseClosesXDSClient tests that the XDS resolver's Close // method closes the XDS client. func (s) TestXDSResolverCloseClosesXDSClient(t *testing.T) { - xdsC := fakeclient.NewClient() - xdsR, _, cancel := testSetup(t, setupOpts{ - xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil }, - }) + xdsR, xdsC, _, cancel := testSetup(t, setupOpts{target: target}) defer cancel() xdsR.Close() if !xdsC.Closed.HasFired() { @@ -300,10 +430,7 @@ func (s) TestXDSResolverCloseClosesXDSClient(t *testing.T) { // TestXDSResolverBadServiceUpdate tests the case the xdsClient returns a bad // service update. func (s) TestXDSResolverBadServiceUpdate(t *testing.T) { - xdsC := fakeclient.NewClient() - xdsR, tcc, cancel := testSetup(t, setupOpts{ - xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil }, - }) + xdsR, xdsC, tcc, cancel := testSetup(t, setupOpts{target: target}) defer xdsR.Close() defer cancel() @@ -326,10 +453,7 @@ func (s) TestXDSResolverBadServiceUpdate(t *testing.T) { // TestXDSResolverGoodServiceUpdate tests the happy case where the resolver // gets a good service update from the xdsClient. func (s) TestXDSResolverGoodServiceUpdate(t *testing.T) { - xdsC := fakeclient.NewClient() - xdsR, tcc, cancel := testSetup(t, setupOpts{ - xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil }, - }) + xdsR, xdsC, tcc, cancel := testSetup(t, setupOpts{target: target}) defer xdsR.Close() defer cancel() @@ -465,10 +589,7 @@ func (s) TestXDSResolverRequestHash(t *testing.T) { env.RingHashSupport = true defer func() { env.RingHashSupport = oldRH }() - xdsC := fakeclient.NewClient() - xdsR, tcc, cancel := testSetup(t, setupOpts{ - xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil }, - }) + xdsR, xdsC, tcc, cancel := testSetup(t, setupOpts{target: target}) defer xdsR.Close() defer cancel() @@ -525,10 +646,7 @@ func (s) TestXDSResolverRequestHash(t *testing.T) { // TestXDSResolverRemovedWithRPCs tests the case where a config selector sends // an empty update to the resolver after the resource is removed. func (s) TestXDSResolverRemovedWithRPCs(t *testing.T) { - xdsC := fakeclient.NewClient() - xdsR, tcc, cancel := testSetup(t, setupOpts{ - xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil }, - }) + xdsR, xdsC, tcc, cancel := testSetup(t, setupOpts{target: target}) defer cancel() defer xdsR.Close() @@ -585,10 +703,7 @@ func (s) TestXDSResolverRemovedWithRPCs(t *testing.T) { // TestXDSResolverRemovedResource tests for proper behavior after a resource is // removed. func (s) TestXDSResolverRemovedResource(t *testing.T) { - xdsC := fakeclient.NewClient() - xdsR, tcc, cancel := testSetup(t, setupOpts{ - xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil }, - }) + xdsR, xdsC, tcc, cancel := testSetup(t, setupOpts{target: target}) defer cancel() defer xdsR.Close() @@ -693,10 +808,7 @@ func (s) TestXDSResolverRemovedResource(t *testing.T) { } func (s) TestXDSResolverWRR(t *testing.T) { - xdsC := fakeclient.NewClient() - xdsR, tcc, cancel := testSetup(t, setupOpts{ - xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil }, - }) + xdsR, xdsC, tcc, cancel := testSetup(t, setupOpts{target: target}) defer xdsR.Close() defer cancel() @@ -753,10 +865,7 @@ func (s) TestXDSResolverWRR(t *testing.T) { } func (s) TestXDSResolverMaxStreamDuration(t *testing.T) { - xdsC := fakeclient.NewClient() - xdsR, tcc, cancel := testSetup(t, setupOpts{ - xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil }, - }) + xdsR, xdsC, tcc, cancel := testSetup(t, setupOpts{target: target}) defer xdsR.Close() defer cancel() @@ -846,10 +955,7 @@ func (s) TestXDSResolverMaxStreamDuration(t *testing.T) { // TestXDSResolverDelayedOnCommitted tests that clusters remain in service // config if RPCs are in flight. func (s) TestXDSResolverDelayedOnCommitted(t *testing.T) { - xdsC := fakeclient.NewClient() - xdsR, tcc, cancel := testSetup(t, setupOpts{ - xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil }, - }) + xdsR, xdsC, tcc, cancel := testSetup(t, setupOpts{target: target}) defer xdsR.Close() defer cancel() @@ -995,10 +1101,7 @@ func (s) TestXDSResolverDelayedOnCommitted(t *testing.T) { // TestXDSResolverUpdates tests the cases where the resolver gets a good update // after an error, and an error after the good update. func (s) TestXDSResolverGoodUpdateAfterError(t *testing.T) { - xdsC := fakeclient.NewClient() - xdsR, tcc, cancel := testSetup(t, setupOpts{ - xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil }, - }) + xdsR, xdsC, tcc, cancel := testSetup(t, setupOpts{target: target}) defer xdsR.Close() defer cancel() @@ -1049,10 +1152,7 @@ func (s) TestXDSResolverGoodUpdateAfterError(t *testing.T) { // a ResourceNotFoundError. It should generate a service config picking // weighted_target, but no child balancers. func (s) TestXDSResolverResourceNotFoundError(t *testing.T) { - xdsC := fakeclient.NewClient() - xdsR, tcc, cancel := testSetup(t, setupOpts{ - xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil }, - }) + xdsR, xdsC, tcc, cancel := testSetup(t, setupOpts{target: target}) defer xdsR.Close() defer cancel() @@ -1095,10 +1195,7 @@ func (s) TestXDSResolverResourceNotFoundError(t *testing.T) { // // This test case also makes sure the resolver doesn't panic. func (s) TestXDSResolverMultipleLDSUpdates(t *testing.T) { - xdsC := fakeclient.NewClient() - xdsR, tcc, cancel := testSetup(t, setupOpts{ - xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil }, - }) + xdsR, xdsC, tcc, cancel := testSetup(t, setupOpts{target: target}) defer xdsR.Close() defer cancel() @@ -1270,10 +1367,7 @@ func (s) TestXDSResolverHTTPFilters(t *testing.T) { for i, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - xdsC := fakeclient.NewClient() - xdsR, tcc, cancel := testSetup(t, setupOpts{ - xdsClientFunc: func() (xdsclient.XDSClient, error) { return xdsC, nil }, - }) + xdsR, xdsC, tcc, cancel := testSetup(t, setupOpts{target: target}) defer xdsR.Close() defer cancel() diff --git a/xds/internal/testutils/fakeclient/client.go b/xds/internal/testutils/fakeclient/client.go index 132fa413a7e..871aa7288c6 100644 --- a/xds/internal/testutils/fakeclient/client.go +++ b/xds/internal/testutils/fakeclient/client.go @@ -306,17 +306,18 @@ func NewClient() *Client { func NewClientWithName(name string) *Client { return &Client{ name: name, - ldsWatchCh: testutils.NewChannel(), + ldsWatchCh: testutils.NewChannelWithSize(10), rdsWatchCh: testutils.NewChannelWithSize(10), cdsWatchCh: testutils.NewChannelWithSize(10), edsWatchCh: testutils.NewChannelWithSize(10), - ldsCancelCh: testutils.NewChannel(), + ldsCancelCh: testutils.NewChannelWithSize(10), rdsCancelCh: testutils.NewChannelWithSize(10), cdsCancelCh: testutils.NewChannelWithSize(10), edsCancelCh: testutils.NewChannelWithSize(10), loadReportCh: testutils.NewChannel(), lrsCancelCh: testutils.NewChannel(), loadStore: load.NewStore(), + bootstrapCfg: &bootstrap.Config{ClientDefaultListenerResourceNameTemplate: "%s"}, rdsCbs: make(map[string]func(xdsresource.RouteConfigUpdate, error)), cdsCbs: make(map[string]func(xdsresource.ClusterUpdate, error)), edsCbs: make(map[string]func(xdsresource.EndpointsUpdate, error)), diff --git a/xds/internal/xdsclient/bootstrap/template.go b/xds/internal/xdsclient/bootstrap/template.go new file mode 100644 index 00000000000..9b51fcc8397 --- /dev/null +++ b/xds/internal/xdsclient/bootstrap/template.go @@ -0,0 +1,47 @@ +/* + * + * Copyright 2021 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 bootstrap + +import ( + "net/url" + "strings" +) + +// PopulateResourceTemplate populates the given template using the target +// string. "%s", if exists in the template, will be replaced with target. +// +// If the template starts with "xdstp:", the replaced string will be %-encoded. +// But note that "/" is not percent encoded. +func PopulateResourceTemplate(template, target string) string { + if !strings.Contains(template, "%s") { + return template + } + if strings.HasPrefix(template, "xdstp:") { + target = percentEncode(target) + } + return strings.Replace(template, "%s", target, -1) +} + +// percentEncode percent encode t, except for "/". See the tests for examples. +func percentEncode(t string) string { + segs := strings.Split(t, "/") + for i := range segs { + segs[i] = url.PathEscape(segs[i]) + } + return strings.Join(segs, "/") +} diff --git a/xds/internal/xdsclient/bootstrap/template_test.go b/xds/internal/xdsclient/bootstrap/template_test.go new file mode 100644 index 00000000000..bc12eb42991 --- /dev/null +++ b/xds/internal/xdsclient/bootstrap/template_test.go @@ -0,0 +1,97 @@ +/* + * + * Copyright 2021 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 bootstrap + +import "testing" + +func Test_percentEncode(t *testing.T) { + tests := []struct { + name string + target string + want string + }{ + { + name: "normal name", + target: "server.example.com", + want: "server.example.com", + }, + { + name: "ipv4", + target: "0.0.0.0:8080", + want: "0.0.0.0:8080", + }, + { + name: "ipv6", + target: "[::1]:8080", + want: "%5B::1%5D:8080", // [ and ] are percent encoded. + }, + { + name: "/ should not be percent encoded", + target: "my/service/region", + want: "my/service/region", // "/"s are kept. + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := percentEncode(tt.target); got != tt.want { + t.Errorf("percentEncode() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestPopulateResourceTemplate(t *testing.T) { + tests := []struct { + name string + template string + target string + want string + }{ + { + name: "no %s", + template: "/name/template", + target: "[::1]:8080", + want: "/name/template", + }, + { + name: "with %s, no xdstp: prefix, ipv6", + template: "/name/template/%s", + target: "[::1]:8080", + want: "/name/template/[::1]:8080", + }, + { + name: "with %s, with xdstp: prefix", + template: "xdstp://authority.com/%s", + target: "0.0.0.0:8080", + want: "xdstp://authority.com/0.0.0.0:8080", + }, + { + name: "with %s, with xdstp: prefix, and ipv6", + template: "xdstp://authority.com/%s", + target: "[::1]:8080", + want: "xdstp://authority.com/%5B::1%5D:8080", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := PopulateResourceTemplate(tt.template, tt.target); got != tt.want { + t.Errorf("PopulateResourceTemplate() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/xds/server.go b/xds/server.go index 28abaf84f5f..0b47fd27ef4 100644 --- a/xds/server.go +++ b/xds/server.go @@ -23,7 +23,6 @@ import ( "errors" "fmt" "net" - "strings" "sync" "google.golang.org/grpc" @@ -42,6 +41,7 @@ import ( "google.golang.org/grpc/status" "google.golang.org/grpc/xds/internal/server" "google.golang.org/grpc/xds/internal/xdsclient" + "google.golang.org/grpc/xds/internal/xdsclient/bootstrap" "google.golang.org/grpc/xds/internal/xdsclient/xdsresource" ) @@ -217,10 +217,7 @@ func (s *GRPCServer) Serve(lis net.Listener) error { if cfg.ServerListenerResourceNameTemplate == "" { return errors.New("missing server_listener_resource_name_template in the bootstrap configuration") } - name := cfg.ServerListenerResourceNameTemplate - if strings.Contains(cfg.ServerListenerResourceNameTemplate, "%s") { - name = strings.Replace(cfg.ServerListenerResourceNameTemplate, "%s", lis.Addr().String(), -1) - } + name := bootstrap.PopulateResourceTemplate(cfg.ServerListenerResourceNameTemplate, lis.Addr().String()) modeUpdateCh := buffer.NewUnbounded() go func() {