xds: introduce generic xds clients xDS and LRS Client API signatures

xds/internal/clients/config.go

@@ -16,11 +16,10 @@
  *
  */
 
-// Package clients provides implementations of the xDS and LRS clients,
-// enabling applications to communicate with xDS management servers and report
-// load.
+// Package clients provides implementations of the clients to interact with
+// xDS and LRS servers.
 //
-// xDS Client
+// # xDS Client
 //
 // The xDS client allows applications to:
 //   - Create client instances with in-memory configurations.
@@ -41,100 +40,33 @@
 //
 // NOTICE: This package is EXPERIMENTAL and may be changed or removed
 // in a later release.
-//
-// See [README](https://github.com/grpc/grpc-go/tree/master/xds/clients/README.md).
 package clients
 
-import (
-	"fmt"
-	"slices"
-	"strings"
-
-	"google.golang.org/protobuf/proto"
-	"google.golang.org/protobuf/types/known/structpb"
-
-	v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
-)
-
-// ServerConfig holds settings for connecting to an xDS management server.
-type ServerConfig struct {
-	// ServerURI is the target URI of the xDS management server.
+// ServerIdentifier holds identifying information for connecting to an xDS
+// management or LRS server.
+type ServerIdentifier struct {
+	// ServerURI is the target URI of the server.
 	ServerURI string
 
-	// IgnoreResourceDeletion is a server feature which if set to true,
-	// indicates that resource deletion errors can be ignored and cached
-	// resource data can be used.
-	//
-	// This will be removed in the future once we implement gRFC A88
-	// and two new fields FailOnDataErrors and
-	// ResourceTimerIsTransientError will be introduced.
-	IgnoreResourceDeletion bool
-
 	// Extensions can be populated with arbitrary data to be passed to the
-	// [TransportBuilder] and/or xDS Client's ResourceType implementations.
+	// TransportBuilder and/or xDS Client's ResourceType implementations.
 	// This field can be used to provide additional configuration or context
 	// specific to the user's needs.
 	//
 	// The xDS and LRS clients do not interpret the contents of this field.
-	// It is the responsibility of the user's custom [TransportBuilder] and/or
+	// It is the responsibility of the user's custom TransportBuilder and/or
 	// ResourceType implementations to handle and interpret these extensions.
 	//
-	// For example, a custom [TransportBuilder] might use this field to
+	// For example, a custom TransportBuilder might use this field to
 	// configure a specific security credentials.
 	//
 	// Note: For custom types used in Extensions, ensure an Equal(any) bool
-	// method is implemented for equality checks on ServerConfig.
-	Extensions any
-}
-
-// equal returns true if sc and other are considered equal.
-func (sc *ServerConfig) equal(other *ServerConfig) bool {
-	switch {
-	case sc == nil && other == nil:
-		return true
-	case (sc != nil) != (other != nil):
-		return false
-	case sc.ServerURI != other.ServerURI:
-		return false
-	case sc.IgnoreResourceDeletion != other.IgnoreResourceDeletion:
-		return false
-	}
-	if sc.Extensions == nil && other.Extensions == nil {
-		return true
-	}
-	if ex, ok := sc.Extensions.(interface{ Equal(any) bool }); ok && ex.Equal(other.Extensions) {
-		return true
-	}
-	return false
-}
-
-// String returns a string representation of the [ServerConfig].
-//
-// WARNING: This method is primarily intended for logging and testing
-// purposes. The output returned by this method is not guaranteed to be stable
-// and may change at any time. Do not rely on it for production use.
-func (sc *ServerConfig) String() string {
-	return strings.Join([]string{sc.ServerURI, fmt.Sprintf("%v", sc.IgnoreResourceDeletion)}, "-")
-}
-
-// Authority contains configuration for an xDS control plane authority.
-type Authority struct {
-	// XDSServers contains the list of server configurations for this authority.
-	XDSServers []ServerConfig
-
-	// Extensions can be populated with arbitrary data to be passed to the xDS
-	// Client's user specific implementations. This field can be used to
-	// provide additional configuration or context specific to the user's
-	// needs.
-	//
-	// The xDS and LRS clients do not interpret the contents of this field. It
-	// is the responsibility of the user's implementations to handle and
-	// interpret these extensions.
+	// method is implemented for equality checks on ServerIdentifier.
 	Extensions any
 }
 
-// Node represents the identity of the xDS client, allowing
-// management servers to identify the source of xDS requests.
+// Node represents the identity of the xDS client, allowing xDS and LRS servers
+// to identify the source of xDS requests.
 type Node struct {
 	// ID is a string identifier of the application.
 	ID string
@@ -150,40 +82,6 @@ type Node struct {
 	UserAgentName string
 	// UserAgentVersion is the user agent version of application.
 	UserAgentVersion string
-	// ClientFeatures is a list of xDS features supported by this client.
-	// These features are set within the xDS client, but may be overridden only
-	// for testing purposes.
-	clientFeatures []string
-}
-
-// toProto converts an instance of [Node] to its protobuf representation.
-func (n Node) toProto() *v3corepb.Node {
-	return &v3corepb.Node{
-		Id:      n.ID,
-		Cluster: n.Cluster,
-		Locality: func() *v3corepb.Locality {
-			if n.Locality.isEmpty() {
-				return nil
-			}
-			return &v3corepb.Locality{
-				Region:  n.Locality.Region,
-				Zone:    n.Locality.Zone,
-				SubZone: n.Locality.SubZone,
-			}
-		}(),
-		Metadata: func() *structpb.Struct {
-			if n.Metadata == nil {
-				return nil
-			}
-			if md, ok := n.Metadata.(*structpb.Struct); ok {
-				return proto.Clone(md).(*structpb.Struct)
-			}
-			return nil
-		}(),
-		UserAgentName:        n.UserAgentName,
-		UserAgentVersionType: &v3corepb.Node_UserAgentVersion{UserAgentVersion: n.UserAgentVersion},
-		ClientFeatures:       slices.Clone(n.clientFeatures),
-	}
 }
 
 // Locality represents the location of the xDS client application.
@@ -195,13 +93,3 @@ type Locality struct {
 	// SubZone is the further subdivision within a zone.
 	SubZone string
 }
-
-// isEmpty reports whether l is considered empty.
-func (l Locality) isEmpty() bool {
-	return l.equal(Locality{})
-}
-
-// equal returns true if l and other are considered equal.
-func (l Locality) equal(other Locality) bool {
-	return l.Region == other.Region && l.Zone == other.Zone && l.SubZone == other.SubZone
-}

xds/internal/clients/grpctransport/grpc_transport.go

@@ -31,38 +31,38 @@
 	"google.golang.org/grpc/xds/internal/clients"
 )
 
-// ServerConfigExtension holds settings for connecting to a gRPC server,
+// ServerIdentifierExtension holds settings for connecting to a gRPC server,
 // such as an xDS management or an LRS server.
-type ServerConfigExtension struct {
+type ServerIdentifierExtension struct {
 	// Credentials will be used for all gRPC transports. If it is unset,
 	// transport creation will fail.
 	Credentials credentials.Bundle
 }
 
-// Builder creates gRPC-based Transports. It must be paired with ServerConfigs
-// that contain an Extension field of type ServerConfigExtension.
+// Builder creates gRPC-based Transports. It must be paired with ServerIdentifiers
+// that contain an Extension field of type ServerIdentifierExtension.
 type Builder struct{}
 
 // Build returns a gRPC-based clients.Transport.
 //
-// The Extension field of the ServerConfig must be a ServerConfigExtension.
-func (b *Builder) Build(sc clients.ServerConfig) (clients.Transport, error) {
-	if sc.ServerURI == "" {
-		return nil, fmt.Errorf("grpctransport: ServerURI is not set in ServerConfig")
+// The Extension field of the ServerIdentifier must be a ServerIdentifierExtension.
+func (b *Builder) Build(si clients.ServerIdentifier) (clients.Transport, error) {
+	if si.ServerURI == "" {
+		return nil, fmt.Errorf("grpctransport: ServerURI is not set in ServerIdentifier")
 	}
-	if sc.Extensions == nil {
-		return nil, fmt.Errorf("grpctransport: Extensions is not set in ServerConfig")
+	if si.Extensions == nil {
+		return nil, fmt.Errorf("grpctransport: Extensions is not set in ServerIdentifier")
 	}
-	sce, ok := sc.Extensions.(ServerConfigExtension)
+	sce, ok := si.Extensions.(ServerIdentifierExtension)
 	if !ok {
-		return nil, fmt.Errorf("grpctransport: Extensions field is %T, but must be %T in ServerConfig", sc.Extensions, ServerConfigExtension{})
+		return nil, fmt.Errorf("grpctransport: Extensions field is %T, but must be %T in ServerIdentifier", si.Extensions, ServerIdentifierExtension{})
 	}
 	if sce.Credentials == nil {
-		return nil, fmt.Errorf("grptransport: Credentials field is not set in ServerConfigExtension")
+		return nil, fmt.Errorf("grptransport: Credentials field is not set in ServerIdentifierExtension")
 	}
 
 	// TODO: Incorporate reference count map for existing transports and
-	// deduplicate transports based on the provided ServerConfig so that
+	// deduplicate transports based on the provided ServerIdentifier so that
 	// transport channel to same server can be shared between xDS and LRS
 	// client.
 
@@ -74,9 +74,9 @@
 		Time:    5 * time.Minute,
 		Timeout: 20 * time.Second,
 	})
-	cc, err := grpc.NewClient(sc.ServerURI, kpCfg, grpc.WithCredentialsBundle(sce.Credentials), grpc.WithDefaultCallOptions(grpc.ForceCodec(&byteCodec{})))
+	cc, err := grpc.NewClient(si.ServerURI, kpCfg, grpc.WithCredentialsBundle(sce.Credentials), grpc.WithDefaultCallOptions(grpc.ForceCodec(&byteCodec{})))
 	if err != nil {
-		return nil, fmt.Errorf("grpctransport: failed to create transport to server %q: %v", sc.ServerURI, err)
+		return nil, fmt.Errorf("grpctransport: failed to create transport to server %q: %v", si.ServerURI, err)
 	}
 
 	return &grpcTransport{cc: cc}, nil

xds/internal/clients/grpctransport/grpc_transport_test.go

@@ -105,6 +105,8 @@ func (s *testServer) StreamAggregatedResources(stream v3discoverygrpc.Aggregated
 			return err // Handle other errors
 		}
 
+		// Push received request for client to verify the correct request was
+		// received.
 		select {
 		case s.requestChan <- req:
 		case <-ctx.Done():
@@ -130,9 +132,9 @@ func (tc *testCredentials) TransportCredentials() credentials.TransportCredentia
 // TestBuild_Success verifies that the Builder successfully creates a new
 // Transport with a non-nil grpc.ClientConn.
 func (s) TestBuild_Success(t *testing.T) {
-	serverCfg := clients.ServerConfig{
+	serverCfg := clients.ServerIdentifier{
 		ServerURI:  "server-address",
-		Extensions: ServerConfigExtension{Credentials: &testCredentials{transportCredentials: local.NewCredentials()}},
+		Extensions: ServerIdentifierExtension{Credentials: &testCredentials{transportCredentials: local.NewCredentials()}},
 	}
 
 	b := &Builder{}
@@ -151,41 +153,41 @@ func (s) TestBuild_Success(t *testing.T) {
 }
 
 // TestBuild_Failure verifies that the Builder returns error when incorrect
-// ServerConfig is provided.
+// ServerIdentifier is provided.
 //
 // It covers the following scenarios:
 // - ServerURI is empty.
 // - Extensions is nil.
-// - Extensions is not ServerConfigExtension.
+// - Extensions is not ServerIdentifierExtension.
 // - Credentials are nil.
 func (s) TestBuild_Failure(t *testing.T) {
 	tests := []struct {
 		name      string
-		serverCfg clients.ServerConfig
+		serverCfg clients.ServerIdentifier
 	}{
 		{
 			name: "ServerURI is empty",
-			serverCfg: clients.ServerConfig{
+			serverCfg: clients.ServerIdentifier{
 				ServerURI:  "",
-				Extensions: ServerConfigExtension{Credentials: insecure.NewBundle()},
+				Extensions: ServerIdentifierExtension{Credentials: insecure.NewBundle()},
 			},
 		},
 		{
 			name:      "Extensions is nil",
-			serverCfg: clients.ServerConfig{ServerURI: "server-address"},
+			serverCfg: clients.ServerIdentifier{ServerURI: "server-address"},
 		},
 		{
-			name: "Extensions is not a ServerConfigExtension",
-			serverCfg: clients.ServerConfig{
+			name: "Extensions is not a ServerIdentifierExtension",
+			serverCfg: clients.ServerIdentifier{
 				ServerURI:  "server-address",
 				Extensions: 1,
 			},
 		},
 		{
-			name: "ServerConfigExtension Credentials is nil",
-			serverCfg: clients.ServerConfig{
+			name: "ServerIdentifierExtension Credentials is nil",
+			serverCfg: clients.ServerIdentifier{
 				ServerURI:  "server-address",
-				Extensions: ServerConfigExtension{},
+				Extensions: ServerIdentifierExtension{},
 			},
 		},
 	}
@@ -208,9 +210,9 @@ func (s) TestBuild_Failure(t *testing.T) {
 func (s) TestNewStream_Success(t *testing.T) {
 	ts := setupTestServer(t, &v3discoverypb.DiscoveryResponse{VersionInfo: "1"})
 
-	serverCfg := clients.ServerConfig{
+	serverCfg := clients.ServerIdentifier{
 		ServerURI:  ts.address,
-		Extensions: ServerConfigExtension{Credentials: insecure.NewBundle()},
+		Extensions: ServerIdentifierExtension{Credentials: insecure.NewBundle()},
 	}
 	builder := Builder{}
 	transport, err := builder.Build(serverCfg)
@@ -229,9 +231,9 @@ func (s) TestNewStream_Success(t *testing.T) {
 // TestNewStream_Error verifies that NewStream() returns an error
 // when attempting to create a stream with an invalid server URI.
 func (s) TestNewStream_Error(t *testing.T) {
-	serverCfg := clients.ServerConfig{
+	serverCfg := clients.ServerIdentifier{
 		ServerURI:  "invalid-server-uri",
-		Extensions: ServerConfigExtension{Credentials: insecure.NewBundle()},
+		Extensions: ServerIdentifierExtension{Credentials: insecure.NewBundle()},
 	}
 	builder := Builder{}
 	transport, err := builder.Build(serverCfg)
@@ -262,9 +264,9 @@ func (s) TestStream_SendAndRecv(t *testing.T) {
 	ts := setupTestServer(t, &v3discoverypb.DiscoveryResponse{VersionInfo: "1"})
 
 	// Build a grpc-based transport to the above server.
-	serverCfg := clients.ServerConfig{
+	serverCfg := clients.ServerIdentifier{
 		ServerURI:  ts.address,
-		Extensions: ServerConfigExtension{Credentials: insecure.NewBundle()},
+		Extensions: ServerIdentifierExtension{Credentials: insecure.NewBundle()},
 	}
 	builder := Builder{}
 	transport, err := builder.Build(serverCfg)