test to validate xdsclient ServerIdentifier.Extensions without Equal methodd

Author: purnesh42H

xds/internal/clients/xdsclient/authority.go

@@ -132,7 +132,7 @@ type authorityBuildOptions struct {
 // Note that no channels to management servers are created at this time. Instead
 // a channel to the first server configuration is created when the first watch
 // is registered, and more channels are created as needed by the fallback logic.
-func newAuthority(args authorityBuildOptions) (*authority, error) {
+func newAuthority(args authorityBuildOptions) *authority {
 	ctx, cancel := context.WithCancel(context.Background())
 	l := grpclog.Component("xds")
 	logPrefix := args.logPrefix + fmt.Sprintf("[authority %q] ", args.name)
@@ -152,12 +152,9 @@ func newAuthority(args authorityBuildOptions) (*authority, error) {
 	// first watch is registered, and channels to other server configurations
 	// are created as needed to support fallback.
 	for _, sc := range args.serverConfigs {
-		if _, ok := sc.ServerIdentifier.Extensions.(interface{ Equal(any) bool }); sc.ServerIdentifier.Extensions != nil && !ok {
-			return nil, fmt.Errorf("xdsclient: authority %s has a server config %s with non-nil ServerIdentifier.Extensions without implementing the `func (any) Equal(any) bool` method", args.name, serverConfigString(&sc))
-		}
 		ret.xdsChannelConfigs = append(ret.xdsChannelConfigs, &xdsChannelWithConfig{serverConfig: &sc})
 	}
-	return ret, nil
+	return ret
 }
 
 // adsStreamFailure is called to notify the authority about an ADS stream

xds/internal/clients/xdsclient/clientimpl.go

@@ -99,37 +99,30 @@ func newClientImpl(config *Config, watchExpiryTimeout time.Duration, streamBacko
 		xdsActiveChannels:  newServerConfigMap(),
 	}
 
-	var err error
 	for name, cfg := range config.Authorities {
 		// If server configs are specified in the authorities map, use that.
 		// Else, use the top-level server configs.
 		serverCfg := config.Servers
 		if len(cfg.XDSServers) >= 1 {
 			serverCfg = cfg.XDSServers
 		}
-		c.authorities[name], err = newAuthority(authorityBuildOptions{
+		c.authorities[name] = newAuthority(authorityBuildOptions{
 			serverConfigs:    serverCfg,
 			name:             name,
 			serializer:       c.serializer,
 			getChannelForADS: c.getChannelForADS,
 			logPrefix:        clientPrefix(c),
 			target:           target,
 		})
-		if err != nil {
-			return nil, err
-		}
 	}
-	c.topLevelAuthority, err = newAuthority(authorityBuildOptions{
+	c.topLevelAuthority = newAuthority(authorityBuildOptions{
 		serverConfigs:    config.Servers,
 		name:             "",
 		serializer:       c.serializer,
 		getChannelForADS: c.getChannelForADS,
 		logPrefix:        clientPrefix(c),
 		target:           target,
 	})
-	if err != nil {
-		return nil, err
-	}
 	c.logger = prefixLogger(c)
 	return c, nil
 }

xds/internal/clients/xdsclient/xdsclient.go

@@ -32,6 +32,7 @@ package xdsclient
 
 import (
 	"errors"
+	"fmt"
 	"time"
 
 	v3statuspb "github.com/envoyproxy/go-control-plane/envoy/service/status/v3"
@@ -58,6 +59,24 @@ func New(config Config) (*XDSClient, error) {
 		return nil, errors.New("xdsclient: no servers or authorities specified")
 	}
 
+	for name, cfg := range config.Authorities {
+		serverCfg := config.Servers
+		if len(cfg.XDSServers) >= 1 {
+			serverCfg = cfg.XDSServers
+		}
+		for _, sc := range serverCfg {
+			if _, ok := sc.ServerIdentifier.Extensions.(interface{ Equal(any) bool }); sc.ServerIdentifier.Extensions != nil && !ok {
+				return nil, fmt.Errorf("xdsclient: authority %s has a server config %s with non-nil ServerIdentifier.Extensions without implementing the `func (any) Equal(any) bool` method", name, serverConfigString(&sc))
+			}
+		}
+	}
+
+	for _, sc := range config.Servers {
+		if _, ok := sc.ServerIdentifier.Extensions.(interface{ Equal(any) bool }); sc.ServerIdentifier.Extensions != nil && !ok {
+			return nil, fmt.Errorf("xdsclient: one of the default servers has a server config %s with non-nil ServerIdentifier.Extensions without implementing the `func (any) Equal(any) bool` method", serverConfigString(&sc))
+		}
+	}
+
 	clientImpl, err := newClientImpl(&config, defaultWatchExpiryTimeout, defaultExponentialBackoff, "xds-client")
 	if err != nil {
 		return nil, err

xds/internal/clients/xdsclient/xdsclient_test.go

@@ -19,6 +19,7 @@
 package xdsclient
 
 import (
+	"strings"
 	"testing"
 
 	"google.golang.org/grpc/xds/internal/clients"
@@ -35,22 +36,22 @@ func (s) TestXDSClient_New(t *testing.T) {
 		{
 			name:    "empty node ID",
 			config:  Config{},
-			wantErr: "xdsclient: node ID is empty",
+			wantErr: "node ID is empty",
 		},
 		{
 			name: "nil resource types",
 			config: Config{
 				Node: clients.Node{ID: "node-id"},
 			},
-			wantErr: "xdsclient: resource types map is nil",
+			wantErr: "resource types map is nil",
 		},
 		{
 			name: "nil transport builder",
 			config: Config{
 				Node:          clients.Node{ID: "node-id"},
 				ResourceTypes: map[string]ResourceType{xdsresource.V3ListenerURL: listenerType},
 			},
-			wantErr: "xdsclient: transport builder is nil",
+			wantErr: "transport builder is nil",
 		},
 		{
 			name: "no servers or authorities",
@@ -59,7 +60,7 @@ func (s) TestXDSClient_New(t *testing.T) {
 				ResourceTypes:    map[string]ResourceType{xdsresource.V3ListenerURL: listenerType},
 				TransportBuilder: grpctransport.NewBuilder(),
 			},
-			wantErr: "xdsclient: no servers or authorities specified",
+			wantErr: "no servers or authorities specified",
 		},
 		{
 			name: "success with servers",
@@ -77,10 +78,40 @@ func (s) TestXDSClient_New(t *testing.T) {
 				Node:             clients.Node{ID: "node-id"},
 				ResourceTypes:    map[string]ResourceType{xdsresource.V3ListenerURL: listenerType},
 				TransportBuilder: grpctransport.NewBuilder(),
-				Authorities:      map[string]Authority{"authority-name": {}},
+				Authorities:      map[string]Authority{"authority-name": {XDSServers: []ServerConfig{{ServerIdentifier: clients.ServerIdentifier{ServerURI: "dummy-server"}}}}},
 			},
 			wantErr: "",
 		},
+		{
+			name: "success with server identifier extensions with equal",
+			config: Config{
+				Node:             clients.Node{ID: "node-id"},
+				ResourceTypes:    map[string]ResourceType{xdsresource.V3ListenerURL: listenerType},
+				TransportBuilder: grpctransport.NewBuilder(),
+				Servers:          []ServerConfig{{ServerIdentifier: clients.ServerIdentifier{ServerURI: "dummy-server", Extensions: &testServerIdentifierExtension{x: 1}}}},
+			},
+			wantErr: "",
+		},
+		{
+			name: "default servers with server identifier extensions without equal",
+			config: Config{
+				Node:             clients.Node{ID: "node-id"},
+				ResourceTypes:    map[string]ResourceType{xdsresource.V3ListenerURL: listenerType},
+				TransportBuilder: grpctransport.NewBuilder(),
+				Servers:          []ServerConfig{{ServerIdentifier: clients.ServerIdentifier{ServerURI: "dummy-server", Extensions: &testServerIdentifierExtensionWithoutEqual{x: 1}}}},
+			},
+			wantErr: "non-nil ServerIdentifier.Extensions without implementing the `func (any) Equal(any) bool` method",
+		},
+		{
+			name: "authorities with server identifier extensions without equal",
+			config: Config{
+				Node:             clients.Node{ID: "node-id"},
+				ResourceTypes:    map[string]ResourceType{xdsresource.V3ListenerURL: listenerType},
+				TransportBuilder: grpctransport.NewBuilder(),
+				Authorities:      map[string]Authority{"authority-name": {XDSServers: []ServerConfig{{ServerIdentifier: clients.ServerIdentifier{ServerURI: "dummy-server", Extensions: &testServerIdentifierExtensionWithoutEqual{x: 1}}}}}},
+			},
+			wantErr: "non-nil ServerIdentifier.Extensions without implementing the `func (any) Equal(any) bool` method",
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -90,7 +121,7 @@ func (s) TestXDSClient_New(t *testing.T) {
 					t.Fatalf("New(%+v) failed: %v", tt.config, err)
 				}
 			} else {
-				if err == nil || err.Error() != tt.wantErr {
+				if err == nil || !strings.Contains(err.Error(), tt.wantErr) {
 					t.Fatalf("New(%+v) returned error %v, want error %q", tt.config, err, tt.wantErr)
 				}
 			}