From 3f563eb09cbb7b04c249c5b1f2078197ecadb939 Mon Sep 17 00:00:00 2001 From: Dimitar Pavlov Date: Thu, 21 Aug 2025 18:25:11 +0100 Subject: [PATCH] remove internal/ and xds/ changes --- credentials/jwt/jwt_token_file_call_creds.go | 10 +- internal/envconfig/xds.go | 5 - internal/xds/bootstrap/bootstrap.go | 87 +-- internal/xds/bootstrap/bootstrap_test.go | 606 +------------------ xds/bootstrap/bootstrap.go | 4 - xds/bootstrap/bootstrap_test.go | 42 +- xds/bootstrap/credentials.go | 14 - xds/internal/xdsclient/clientimpl.go | 4 +- xds/internal/xdsclient/clientimpl_test.go | 91 --- 9 files changed, 19 insertions(+), 844 deletions(-) diff --git a/credentials/jwt/jwt_token_file_call_creds.go b/credentials/jwt/jwt_token_file_call_creds.go index 81e1165bd..3e691b016 100644 --- a/credentials/jwt/jwt_token_file_call_creds.go +++ b/credentials/jwt/jwt_token_file_call_creds.go @@ -86,7 +86,7 @@ func (c *jwtTokenFileCallCreds) GetRequestMetadata(ctx context.Context, _ ...str if c.isTokenValidLocked() { if c.needsPreemptiveRefreshLocked() { // Start refresh if not pending (handling the prior RPC may have - // just spwaned a goroutine). + // just spawned a goroutine). if !c.pendingRefresh { c.pendingRefresh = true go c.refreshToken() @@ -105,7 +105,7 @@ func (c *jwtTokenFileCallCreds) GetRequestMetadata(ctx context.Context, _ ...str // At this point, the token is either invalid or expired and we are no // longer backing off. So refresh it. token, expiry, err := c.fileReader.ReadToken() - c.setCacheLocked(token, expiry, err) + c.updateCacheLocked(token, expiry, err) if c.cachedError != nil { return nil, c.cachedError @@ -148,17 +148,17 @@ func (c *jwtTokenFileCallCreds) refreshToken() { c.mu.Lock() defer c.mu.Unlock() - c.setCacheLocked(token, expiry, err) + c.updateCacheLocked(token, expiry, err) // Reset pending refresh and broadcast to waiting goroutines c.pendingRefresh = false } -// setCacheLocked updates the cached token, expiry, and error state. +// updateCacheLocked updates the cached token, expiry, and error state. // If an error is provided, it determines whether to set it as an UNAVAILABLE // or UNAUTHENTICATED error based on the error type. // Caller must hold c.mu lock. -func (c *jwtTokenFileCallCreds) setCacheLocked(token string, expiry time.Time, err error) { +func (c *jwtTokenFileCallCreds) updateCacheLocked(token string, expiry time.Time, err error) { if err != nil { // Convert to gRPC status codes if strings.Contains(err.Error(), "failed to read token file") || strings.Contains(err.Error(), "token file") && strings.Contains(err.Error(), "is empty") { diff --git a/internal/envconfig/xds.go b/internal/envconfig/xds.go index 6420558c0..e87551552 100644 --- a/internal/envconfig/xds.go +++ b/internal/envconfig/xds.go @@ -68,9 +68,4 @@ var ( // trust. For more details, see: // https://github.com/grpc/proposal/blob/master/A87-mtls-spiffe-support.md XDSSPIFFEEnabled = boolFromEnv("GRPC_EXPERIMENTAL_XDS_MTLS_SPIFFE", false) - - // XDSBootstrapCallCredsEnabled controls if JWT call credentials can be used - // in xDS bootstrap configuration. For more details, see: - // https://github.com/grpc/proposal/blob/master/A97-xds-jwt-call-creds.md - XDSBootstrapCallCredsEnabled = boolFromEnv("GRPC_EXPERIMENTAL_XDS_BOOTSTRAP_CALL_CREDS", false) ) diff --git a/internal/xds/bootstrap/bootstrap.go b/internal/xds/bootstrap/bootstrap.go index 46dbf6bc9..f409e4bd7 100644 --- a/internal/xds/bootstrap/bootstrap.go +++ b/internal/xds/bootstrap/bootstrap.go @@ -31,7 +31,6 @@ import ( "strings" "google.golang.org/grpc" - "google.golang.org/grpc/credentials" "google.golang.org/grpc/credentials/tls/certprovider" "google.golang.org/grpc/internal" "google.golang.org/grpc/internal/envconfig" @@ -65,26 +64,11 @@ type ChannelCreds struct { Config json.RawMessage `json:"config,omitempty"` } -// CallCreds contains the call credentials configuration for individual RPCs. -// This type implements RFC A97 call credentials structure. -type CallCreds struct { - // Type contains a unique name identifying the call credentials type. - // Currently only "jwt_token_file" is supported. - Type string `json:"type,omitempty"` - // Config contains the JSON configuration associated with the call credentials. - Config json.RawMessage `json:"config,omitempty"` -} - // Equal reports whether cc and other are considered equal. func (cc ChannelCreds) Equal(other ChannelCreds) bool { return cc.Type == other.Type && bytes.Equal(cc.Config, other.Config) } -// Equal reports whether cc and other are considered equal. -func (cc CallCreds) Equal(other CallCreds) bool { - return cc.Type == other.Type && bytes.Equal(cc.Config, other.Config) -} - // String returns a string representation of the credentials. It contains the // type and the config (if non-nil) separated by a "-". func (cc ChannelCreds) String() string { @@ -188,15 +172,13 @@ type ServerConfig struct { serverURI string channelCreds []ChannelCreds serverFeatures []string - callCreds []CallCreds // As part of unmarshalling the JSON config into this struct, we ensure that // the credentials config is valid by building an instance of the specified // credentials and store it here for easy access. - selectedCreds ChannelCreds - credsDialOption grpc.DialOption - extraDialOptions []grpc.DialOption - selectedCallCreds []credentials.PerRPCCredentials // Built call credentials + selectedCreds ChannelCreds + credsDialOption grpc.DialOption + extraDialOptions []grpc.DialOption cleanups []func() } @@ -218,17 +200,6 @@ func (sc *ServerConfig) ServerFeatures() []string { return sc.serverFeatures } -// CallCreds returns the call credentials configuration for this server. -func (sc *ServerConfig) CallCreds() []CallCreds { - return sc.callCreds -} - -// SelectedCallCreds returns the built call credentials that are ready to use. -// These are the credentials that were successfully built from the call_creds configuration. -func (sc *ServerConfig) SelectedCallCreds() []credentials.PerRPCCredentials { - return sc.selectedCallCreds -} - // ServerFeaturesIgnoreResourceDeletion returns true if this server supports a // feature where the xDS client can ignore resource deletions from this server, // as described in gRFC A53. @@ -262,28 +233,6 @@ func (sc *ServerConfig) DialOptions() []grpc.DialOption { return dopts } -// DialOptionsWithCallCredsForTransport returns dial options including call credentials -// only if they are compatible with the specified transport credentials type. -// Call credentials that require transport security will be skipped for insecure transports. -func (sc *ServerConfig) DialOptionsWithCallCredsForTransport(transportCredsType string, transportCreds credentials.TransportCredentials) []grpc.DialOption { - dopts := sc.DialOptions() - - // Check if transport is insecure - isInsecureTransport := transportCredsType == "insecure" || - (transportCreds != nil && transportCreds.Info().SecurityProtocol == "insecure") - - // Add call credentials only if compatible with transport security - for _, callCred := range sc.selectedCallCreds { - // Skip call credentials that require transport security on insecure transports - if isInsecureTransport && callCred.RequireTransportSecurity() { - continue - } - dopts = append(dopts, grpc.WithPerRPCCredentials(callCred)) - } - - return dopts -} - // Cleanups returns a collection of functions to be called when the xDS client // for this server is closed. Allows cleaning up resources created specifically // for this server. @@ -302,8 +251,6 @@ func (sc *ServerConfig) Equal(other *ServerConfig) bool { return false case !slices.EqualFunc(sc.channelCreds, other.channelCreds, func(a, b ChannelCreds) bool { return a.Equal(b) }): return false - case !slices.EqualFunc(sc.callCreds, other.callCreds, func(a, b CallCreds) bool { return a.Equal(b) }): - return false case !slices.Equal(sc.serverFeatures, other.serverFeatures): return false case !sc.selectedCreds.Equal(other.selectedCreds): @@ -326,7 +273,6 @@ type serverConfigJSON struct { ServerURI string `json:"server_uri,omitempty"` ChannelCreds []ChannelCreds `json:"channel_creds,omitempty"` ServerFeatures []string `json:"server_features,omitempty"` - CallCreds []CallCreds `json:"call_creds,omitempty"` } // MarshalJSON returns marshaled JSON bytes corresponding to this server config. @@ -335,7 +281,6 @@ func (sc *ServerConfig) MarshalJSON() ([]byte, error) { ServerURI: sc.serverURI, ChannelCreds: sc.channelCreds, ServerFeatures: sc.serverFeatures, - CallCreds: sc.callCreds, } return json.Marshal(server) } @@ -356,7 +301,6 @@ func (sc *ServerConfig) UnmarshalJSON(data []byte) error { sc.serverURI = server.ServerURI sc.channelCreds = server.ChannelCreds sc.serverFeatures = server.ServerFeatures - sc.callCreds = server.CallCreds for _, cc := range server.ChannelCreds { // We stop at the first credential type that we support. @@ -376,27 +320,6 @@ func (sc *ServerConfig) UnmarshalJSON(data []byte) error { sc.cleanups = append(sc.cleanups, cancel) break } - - // Process call credentials - unlike channel creds, we use ALL supported types - // Call credentials are optional per RFC A97 - for _, callCred := range server.CallCreds { - c := bootstrap.GetCredentials(callCred.Type) - if c == nil { - // Skip unsupported call credential types (don't fail bootstrap) - continue - } - bundle, cancel, err := c.Build(callCred.Config) - if err != nil { - // Call credential validation failed - this should fail bootstrap - return fmt.Errorf("failed to build call credentials from bootstrap for %q: %v", callCred.Type, err) - } - // Extract the PerRPCCredentials from the bundle. Sanity check for nil just in case - if callCredentials := bundle.PerRPCCredentials(); callCredentials != nil { - sc.selectedCallCreds = append(sc.selectedCallCreds, callCredentials) - } - sc.cleanups = append(sc.cleanups, cancel) - } - if sc.serverURI == "" { return fmt.Errorf("xds: `server_uri` field in server config cannot be empty: %s", string(data)) } @@ -418,9 +341,6 @@ type ServerConfigTestingOptions struct { ChannelCreds []ChannelCreds // ServerFeatures represents the list of features supported by this server. ServerFeatures []string - // CallCreds contains a list of call credentials to use for individual RPCs - // to this server. Optional. - CallCreds []CallCreds } // ServerConfigForTesting creates a new ServerConfig from the passed in options, @@ -436,7 +356,6 @@ func ServerConfigForTesting(opts ServerConfigTestingOptions) (*ServerConfig, err ServerURI: opts.URI, ChannelCreds: cc, ServerFeatures: opts.ServerFeatures, - CallCreds: opts.CallCreds, } scJSON, err := json.Marshal(scInternal) if err != nil { diff --git a/internal/xds/bootstrap/bootstrap_test.go b/internal/xds/bootstrap/bootstrap_test.go index 93e90144f..d05719780 100644 --- a/internal/xds/bootstrap/bootstrap_test.go +++ b/internal/xds/bootstrap/bootstrap_test.go @@ -19,21 +19,15 @@ package bootstrap import ( - "context" "encoding/json" "errors" "fmt" - "net" "os" - "strings" "testing" v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" "github.com/google/go-cmp/cmp" "google.golang.org/grpc" - "google.golang.org/grpc/credentials" - "google.golang.org/grpc/credentials/insecure" - "google.golang.org/grpc/credentials/jwt" "google.golang.org/grpc/credentials/tls/certprovider" "google.golang.org/grpc/internal" "google.golang.org/grpc/internal/envconfig" @@ -202,74 +196,6 @@ var ( "server_features" : ["ignore_resource_deletion", "xds_v3"] }] }`, - // example data seeded from - // https://github.com/istio/istio/blob/master/pkg/istio-agent/testdata/grpc-bootstrap.json - "istioStyleWithJWTCallCreds": ` - { - "node": { - "id": "sidecar~127.0.0.1~pod1.fake-namespace~fake-namespace.svc.cluster.local", - "metadata": { - "GENERATOR": "grpc", - "INSTANCE_IPS": "127.0.0.1", - "ISTIO_VERSION": "1.26.2", - "WORKLOAD_IDENTITY_SOCKET_FILE": "socket" - }, - "locality": {} - }, - "xds_servers" : [{ - "server_uri": "unix:///etc/istio/XDS", - "channel_creds": [ - { "type": "insecure" } - ], - "call_creds": [ - { "type": "jwt_token_file", "config": {"jwt_token_file": "/var/run/secrets/tokens/istio-token"} } - ], - "server_features" : ["xds_v3"] - }] - }`, - "istioStyleWithoutCallCreds": ` - { - "node": { - "id": "sidecar~127.0.0.1~pod1.fake-namespace~fake-namespace.svc.cluster.local", - "metadata": { - "GENERATOR": "grpc", - "INSTANCE_IPS": "127.0.0.1", - "ISTIO_VERSION": "1.26.2", - "WORKLOAD_IDENTITY_SOCKET_FILE": "socket" - }, - "locality": {} - }, - "xds_servers" : [{ - "server_uri": "unix:///etc/istio/XDS", - "channel_creds": [ - { "type": "insecure" } - ], - "server_features" : ["xds_v3"] - }] - }`, - "istioStyleWithTLSAndJWT": ` - { - "node": { - "id": "sidecar~127.0.0.1~pod1.fake-namespace~fake-namespace.svc.cluster.local", - "metadata": { - "GENERATOR": "grpc", - "INSTANCE_IPS": "127.0.0.1", - "ISTIO_VERSION": "1.26.2", - "WORKLOAD_IDENTITY_SOCKET_FILE": "socket" - }, - "locality": {} - }, - "xds_servers" : [{ - "server_uri": "unix:///etc/istio/XDS", - "channel_creds": [ - { "type": "tls", "config": {} } - ], - "call_creds": [ - { "type": "jwt_token_file", "config": {"jwt_token_file": "/var/run/secrets/tokens/istio-token"} } - ], - "server_features" : ["xds_v3"] - }] - }`, } metadata = &structpb.Struct{ Fields: map[string]*structpb.Value{ @@ -350,82 +276,6 @@ var ( node: v3Node, clientDefaultListenerResourceNameTemplate: "%s", } - - istioNodeMetadata = &structpb.Struct{ - Fields: map[string]*structpb.Value{ - "GENERATOR": { - Kind: &structpb.Value_StringValue{StringValue: "grpc"}, - }, - "INSTANCE_IPS": { - Kind: &structpb.Value_StringValue{StringValue: "127.0.0.1"}, - }, - "ISTIO_VERSION": { - Kind: &structpb.Value_StringValue{StringValue: "1.26.2"}, - }, - "WORKLOAD_IDENTITY_SOCKET_FILE": { - Kind: &structpb.Value_StringValue{StringValue: "socket"}, - }, - }, - } - jwtCallCreds, _ = jwt.NewTokenFileCallCredentials("/var/run/secrets/tokens/istio-token") - selectedJWTCallCreds = []credentials.PerRPCCredentials{jwtCallCreds} - configWithIstioJWTCallCreds = &Config{ - xDSServers: []*ServerConfig{{ - serverURI: "unix:///etc/istio/XDS", - channelCreds: []ChannelCreds{{Type: "insecure"}}, - callCreds: []CallCreds{{Type: "jwt_token_file", Config: json.RawMessage("{\n\"jwt_token_file\": \"/var/run/secrets/tokens/istio-token\"\n}")}}, - serverFeatures: []string{"xds_v3"}, - selectedCreds: ChannelCreds{Type: "insecure"}, - selectedCallCreds: selectedJWTCallCreds, - }}, - node: node{ - ID: "sidecar~127.0.0.1~pod1.fake-namespace~fake-namespace.svc.cluster.local", - Metadata: istioNodeMetadata, - userAgentName: gRPCUserAgentName, - userAgentVersionType: userAgentVersion{UserAgentVersion: grpc.Version}, - clientFeatures: []string{clientFeatureNoOverprovisioning, clientFeatureResourceWrapper}, - }, - certProviderConfigs: map[string]*certprovider.BuildableConfig{}, - clientDefaultListenerResourceNameTemplate: "%s", - } - - configWithIstioStyleNoCallCreds = &Config{ - xDSServers: []*ServerConfig{{ - serverURI: "unix:///etc/istio/XDS", - channelCreds: []ChannelCreds{{Type: "insecure"}}, - serverFeatures: []string{"xds_v3"}, - selectedCreds: ChannelCreds{Type: "insecure"}, - }}, - node: node{ - ID: "sidecar~127.0.0.1~pod1.fake-namespace~fake-namespace.svc.cluster.local", - Metadata: istioNodeMetadata, - userAgentName: gRPCUserAgentName, - userAgentVersionType: userAgentVersion{UserAgentVersion: grpc.Version}, - clientFeatures: []string{clientFeatureNoOverprovisioning, clientFeatureResourceWrapper}, - }, - certProviderConfigs: map[string]*certprovider.BuildableConfig{}, - clientDefaultListenerResourceNameTemplate: "%s", - } - - configWithIstioStyleWithTLSAndJWT = &Config{ - xDSServers: []*ServerConfig{{ - serverURI: "unix:///etc/istio/XDS", - channelCreds: []ChannelCreds{{Type: "tls", Config: json.RawMessage("{}")}}, - callCreds: []CallCreds{{Type: "jwt_token_file", Config: json.RawMessage("{\n\"jwt_token_file\": \"/var/run/secrets/tokens/istio-token\"\n}")}}, - serverFeatures: []string{"xds_v3"}, - selectedCreds: ChannelCreds{Type: "tls", Config: json.RawMessage("{}")}, - selectedCallCreds: selectedJWTCallCreds, - }}, - node: node{ - ID: "sidecar~127.0.0.1~pod1.fake-namespace~fake-namespace.svc.cluster.local", - Metadata: istioNodeMetadata, - userAgentName: gRPCUserAgentName, - userAgentVersionType: userAgentVersion{UserAgentVersion: grpc.Version}, - clientFeatures: []string{clientFeatureNoOverprovisioning, clientFeatureResourceWrapper}, - }, - certProviderConfigs: map[string]*certprovider.BuildableConfig{}, - clientDefaultListenerResourceNameTemplate: "%s", - } ) func fileReadFromFileMap(bootstrapFileMap map[string]string, name string) ([]byte, error) { @@ -575,35 +425,6 @@ func (s) TestGetConfiguration_Success(t *testing.T) { {"goodBootstrap", configWithGoogleDefaultCredsAndV3}, {"multipleXDSServers", configWithMultipleServers}, {"serverSupportsIgnoreResourceDeletion", configWithGoogleDefaultCredsAndIgnoreResourceDeletion}, - {"istioStyleWithoutCallCreds", configWithIstioStyleNoCallCreds}, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - testGetConfigurationWithFileNameEnv(t, test.name, false, test.wantConfig) - testGetConfigurationWithFileContentEnv(t, test.name, false, test.wantConfig) - }) - } -} - -// Tests Istio-style bootstrap configurations with JWT call credentials -func (s) TestGetConfiguration_IstioStyleWithCallCreds(t *testing.T) { - // Enable JWT call credentials feature - original := envconfig.XDSBootstrapCallCredsEnabled - envconfig.XDSBootstrapCallCredsEnabled = true - defer func() { - envconfig.XDSBootstrapCallCredsEnabled = original - }() - - cancel := setupBootstrapOverride(v3BootstrapFileMap) - defer cancel() - - tests := []struct { - name string - wantConfig *Config - }{ - {"istioStyleWithJWTCallCreds", configWithIstioJWTCallCreds}, - {"istioStyleWithTLSAndJWT", configWithIstioStyleWithTLSAndJWT}, } for _, test := range tests { @@ -1197,203 +1018,12 @@ func (s) TestDefaultBundles(t *testing.T) { } } -func (s) TestCallCreds_Equal(t *testing.T) { - tests := []struct { - name string - cc1 CallCreds - cc2 CallCreds - expect bool - }{ - { - name: "identical configs", - cc1: CallCreds{Type: "jwt_token_file", Config: json.RawMessage(`{"jwt_token_file": "/path/to/token"}`)}, - cc2: CallCreds{Type: "jwt_token_file", Config: json.RawMessage(`{"jwt_token_file": "/path/to/token"}`)}, - expect: true, - }, - { - name: "different types", - cc1: CallCreds{Type: "jwt_token_file", Config: json.RawMessage(`{"jwt_token_file": "/path/to/token"}`)}, - cc2: CallCreds{Type: "other_type", Config: json.RawMessage(`{"jwt_token_file": "/path/to/token"}`)}, - expect: false, - }, - { - name: "different configs", - cc1: CallCreds{Type: "jwt_token_file", Config: json.RawMessage(`{"jwt_token_file": "/path/to/token"}`)}, - cc2: CallCreds{Type: "jwt_token_file", Config: json.RawMessage(`{"jwt_token_file": "/different/path"}`)}, - expect: false, - }, - { - name: "nil vs non-nil configs", - cc1: CallCreds{Type: "jwt_token_file", Config: nil}, - cc2: CallCreds{Type: "jwt_token_file", Config: json.RawMessage(`{"jwt_token_file": "/path/to/token"}`)}, - expect: false, - }, - { - name: "both nil configs", - cc1: CallCreds{Type: "jwt_token_file", Config: nil}, - cc2: CallCreds{Type: "jwt_token_file", Config: nil}, - expect: true, - }, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - result := test.cc1.Equal(test.cc2) - if result != test.expect { - t.Errorf("CallCreds.Equal() = %v, want %v", result, test.expect) - } - }) - } +type s struct { + grpctest.Tester } -func (s) TestServerConfig_UnmarshalJSON_WithCallCreds(t *testing.T) { - original := envconfig.XDSBootstrapCallCredsEnabled - defer func() { envconfig.XDSBootstrapCallCredsEnabled = original }() - envconfig.XDSBootstrapCallCredsEnabled = true // Enable call creds in bootstrap - tests := []struct { - name string - json string - wantCallCreds []CallCreds - wantErr bool - errContains string - }{ - { - name: "valid call_creds with jwt_token_file", - json: `{ - "server_uri": "xds-server:443", - "channel_creds": [{"type": "insecure"}], - "call_creds": [ - { - "type": "jwt_token_file", - "config": {"jwt_token_file": "/path/to/token.jwt"} - } - ] - }`, - wantCallCreds: []CallCreds{{ - Type: "jwt_token_file", - Config: json.RawMessage(`{"jwt_token_file": "/path/to/token.jwt"}`), - }}, - }, - { - name: "multiple call_creds types", - json: `{ - "server_uri": "xds-server:443", - "channel_creds": [{"type": "insecure"}], - "call_creds": [ - {"type": "jwt_token_file", "config": {"jwt_token_file": "/token1.jwt"}}, - {"type": "unsupported_type", "config": {}} - ] - }`, - wantCallCreds: []CallCreds{ - {Type: "jwt_token_file", Config: json.RawMessage(`{"jwt_token_file": "/token1.jwt"}`)}, - {Type: "unsupported_type", Config: json.RawMessage(`{}`)}, - }, - }, - { - name: "empty call_creds array", - json: `{ - "server_uri": "xds-server:443", - "channel_creds": [{"type": "insecure"}], - "call_creds": [] - }`, - wantCallCreds: []CallCreds{}, - }, - { - name: "missing call_creds field", - json: `{ - "server_uri": "xds-server:443", - "channel_creds": [{"type": "insecure"}] - }`, - wantCallCreds: nil, - }, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - var sc ServerConfig - err := sc.UnmarshalJSON([]byte(test.json)) - - if test.wantErr { - if err == nil { - t.Fatal("Expected error, got nil") - } - if test.errContains != "" && !strings.Contains(err.Error(), test.errContains) { - t.Errorf("Error %v should contain %q", err, test.errContains) - } - return - } - - if err != nil { - t.Fatalf("Unexpected error: %v", err) - } - - if diff := cmp.Diff(test.wantCallCreds, sc.CallCreds()); diff != "" { - t.Errorf("CallCreds mismatch (-want +got):\n%s", diff) - } - }) - } -} - -func (s) TestServerConfig_Equal_WithCallCreds(t *testing.T) { - callCreds := []CallCreds{{ - Type: "jwt_token_file", - Config: json.RawMessage(`{"jwt_token_file": "/test/token.jwt"}`), - }} - sc1 := &ServerConfig{ - serverURI: "server1", - channelCreds: []ChannelCreds{{Type: "insecure"}}, - callCreds: callCreds, - serverFeatures: []string{"feature1"}, - } - sc2 := &ServerConfig{ - serverURI: "server1", - channelCreds: []ChannelCreds{{Type: "insecure"}}, - callCreds: callCreds, - serverFeatures: []string{"feature1"}, - } - sc3 := &ServerConfig{ - serverURI: "server1", - channelCreds: []ChannelCreds{{Type: "insecure"}}, - callCreds: []CallCreds{{Type: "different"}}, - serverFeatures: []string{"feature1"}, - } - - if !sc1.Equal(sc2) { - t.Error("Equal ServerConfigs with same call creds should be equal") - } - if sc1.Equal(sc3) { - t.Error("ServerConfigs with different call creds should not be equal") - } -} - -func (s) TestServerConfig_MarshalJSON_WithCallCreds(t *testing.T) { - original := envconfig.XDSBootstrapCallCredsEnabled - defer func() { envconfig.XDSBootstrapCallCredsEnabled = original }() - envconfig.XDSBootstrapCallCredsEnabled = true // Enable call creds in bootstrap - sc := &ServerConfig{ - serverURI: "test-server:443", - channelCreds: []ChannelCreds{{Type: "insecure"}}, - callCreds: []CallCreds{{ - Type: "jwt_token_file", - Config: json.RawMessage(`{"jwt_token_file":"/test/token.jwt"}`), - }}, - serverFeatures: []string{"test_feature"}, - } - - data, err := sc.MarshalJSON() - if err != nil { - t.Fatalf("MarshalJSON failed: %v", err) - } - - // confirm Marshal/Unmarshal symmetry - var unmarshaled ServerConfig - if err := json.Unmarshal(data, &unmarshaled); err != nil { - t.Fatalf("Unmarshal failed: %v", err) - } - - if diff := cmp.Diff(sc.CallCreds(), unmarshaled.CallCreds()); diff != "" { - t.Errorf("Marshal/Unmarshal call credentials produces differences:\n%s", diff) - } +func Test(t *testing.T) { + grpctest.RunSubTests(t, s{}) } func newStructProtoFromMap(t *testing.T, input map[string]any) *structpb.Struct { @@ -1639,231 +1269,3 @@ func (s) TestGetConfiguration_FallbackDisabled(t *testing.T) { testGetConfigurationWithFileContentEnv(t, "multipleXDSServers", false, wantConfig) }) } - -func (s) TestBootstrap_SelectedCredsAndCallCreds(t *testing.T) { - // Enable JWT call credentials - original := envconfig.XDSBootstrapCallCredsEnabled - envconfig.XDSBootstrapCallCredsEnabled = true - defer func() { - envconfig.XDSBootstrapCallCredsEnabled = original - }() - - tokenFile := "/token.jwt" - tests := []struct { - name string - bootstrapConfig string - expectCallCreds int - expectTransportType string - }{ - { - name: "JWT call creds with TLS channel creds", - bootstrapConfig: `{ - "server_uri": "xds-server:443", - "channel_creds": [{"type": "tls", "config": {}}], - "call_creds": [ - { - "type": "jwt_token_file", - "config": {"jwt_token_file": "` + tokenFile + `"} - } - ] - }`, - expectCallCreds: 1, - expectTransportType: "tls", - }, - { - name: "JWT call creds with multiple channel creds", - bootstrapConfig: `{ - "server_uri": "xds-server:443", - "channel_creds": [{"type": "tls", "config": {}}, {"type": "insecure"}], - "call_creds": [ - { - "type": "jwt_token_file", - "config": {"jwt_token_file": "` + tokenFile + `"} - }, - { - "type": "jwt_token_file", - "config": {"jwt_token_file": "` + tokenFile + `"} - } - ] - }`, - expectCallCreds: 2, - expectTransportType: "tls", // the first channel creds is selected - }, - { - name: "JWT call creds with insecure channel creds", - bootstrapConfig: `{ - "server_uri": "xds-server:443", - "channel_creds": [{"type": "insecure"}], - "call_creds": [ - { - "type": "jwt_token_file", - "config": {"jwt_token_file": "` + tokenFile + `"} - } - ] - }`, - expectCallCreds: 1, - expectTransportType: "insecure", - }, - { - name: "No call creds", - bootstrapConfig: `{ - "server_uri": "xds-server:443", - "channel_creds": [{"type": "insecure"}] - }`, - expectCallCreds: 0, - expectTransportType: "insecure", - }, - { - name: "No call creds multiple channel creds", - bootstrapConfig: `{ - "server_uri": "xds-server:443", - "channel_creds": [{"type": "insecure"}, {"type": "tls", "config": {}}] - }`, - expectCallCreds: 0, - expectTransportType: "insecure", - }, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - var sc ServerConfig - err := sc.UnmarshalJSON([]byte(test.bootstrapConfig)) - if err != nil { - t.Fatalf("Failed to unmarshal bootstrap config: %v", err) - } - - // Verify call credentials processing - callCreds := sc.CallCreds() - selectedCallCreds := sc.SelectedCallCreds() - - if len(callCreds) != test.expectCallCreds { - t.Errorf("Call creds count = %d, want %d", len(callCreds), test.expectCallCreds) - } - if len(selectedCallCreds) != test.expectCallCreds { - t.Errorf("Selected call creds count = %d, want %d", len(selectedCallCreds), test.expectCallCreds) - } - - // Verify transport credentials are properly selected - if sc.SelectedCreds().Type != test.expectTransportType { - t.Errorf("Selected transport creds type = %q, want %q", - sc.SelectedCreds().Type, test.expectTransportType) - } - }) - } -} - -func (s) TestDialOptionsWithCallCredsForTransport(t *testing.T) { - // Create test JWT credentials that require transport security - testJWTCreds := &testPerRPCCreds{requireSecurity: true} - testInsecureCreds := &testPerRPCCreds{requireSecurity: false} - - sc := &ServerConfig{ - selectedCallCreds: []credentials.PerRPCCredentials{ - testJWTCreds, - testInsecureCreds, - }, - extraDialOptions: []grpc.DialOption{ - grpc.WithUserAgent("test-agent"), // Test extra option - }, - } - - tests := []struct { - name string - transportType string - transportCreds credentials.TransportCredentials - expectJWTCreds bool - expectOtherCreds bool - }{ - { - name: "insecure transport by type", - transportType: "insecure", - transportCreds: nil, - expectJWTCreds: false, // JWT requires security - expectOtherCreds: true, // Non-security creds allowed - }, - { - name: "insecure transport by protocol", - transportType: "custom", - transportCreds: insecure.NewCredentials(), - expectJWTCreds: false, // JWT requires security - expectOtherCreds: true, // Non-security creds allowed - }, - { - name: "secure transport", - transportType: "tls", - transportCreds: &testTransportCreds{securityProtocol: "tls"}, - expectJWTCreds: true, // JWT allowed on secure transport - expectOtherCreds: true, // All creds allowed - }, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - opts := sc.DialOptionsWithCallCredsForTransport(test.transportType, test.transportCreds) - - // Count dial options (should include extra options + applicable call creds) - expectedCount := 2 // extraDialOptions + always include non-security creds - if test.expectJWTCreds { - expectedCount++ - } - - if len(opts) != expectedCount { - t.Errorf("DialOptions count = %d, want %d", len(opts), expectedCount) - } - }) - } -} - -type testPerRPCCreds struct { - requireSecurity bool -} - -func (c *testPerRPCCreds) GetRequestMetadata(_ context.Context, _ ...string) (map[string]string, error) { - return map[string]string{"test": "metadata"}, nil -} - -func (c *testPerRPCCreds) RequireTransportSecurity() bool { - return c.requireSecurity -} - -type testTransportCreds struct { - securityProtocol string -} - -func (c *testTransportCreds) ClientHandshake(_ context.Context, _ string, rawConn net.Conn) (net.Conn, credentials.AuthInfo, error) { - return rawConn, &testAuthInfo{}, nil -} - -func (c *testTransportCreds) ServerHandshake(rawConn net.Conn) (net.Conn, credentials.AuthInfo, error) { - return rawConn, &testAuthInfo{}, nil -} - -func (c *testTransportCreds) Info() credentials.ProtocolInfo { - return credentials.ProtocolInfo{SecurityProtocol: c.securityProtocol} -} - -func (c *testTransportCreds) Clone() credentials.TransportCredentials { - return &testTransportCreds{securityProtocol: c.securityProtocol} -} - -func (c *testTransportCreds) OverrideServerName(string) error { - return nil -} - -type testAuthInfo struct{} - -func (a *testAuthInfo) AuthType() string { - return "test" -} - -func (a *testAuthInfo) GetCommonAuthInfo() credentials.CommonAuthInfo { - return credentials.CommonAuthInfo{} -} - -type s struct { - grpctest.Tester -} - -func Test(t *testing.T) { - grpctest.RunSubTests(t, s{}) -} diff --git a/xds/bootstrap/bootstrap.go b/xds/bootstrap/bootstrap.go index b1a5e831b..ef55ff0c0 100644 --- a/xds/bootstrap/bootstrap.go +++ b/xds/bootstrap/bootstrap.go @@ -29,7 +29,6 @@ import ( "encoding/json" "google.golang.org/grpc/credentials" - "google.golang.org/grpc/internal/envconfig" ) // registry is a map from credential type name to Credential builder. @@ -59,9 +58,6 @@ func RegisterCredentials(c Credentials) { // GetCredentials returns the credentials associated with a given name. // If no credentials are registered with the name, nil will be returned. func GetCredentials(name string) Credentials { - if name == "jwt_token_file" && !envconfig.XDSBootstrapCallCredsEnabled { - return nil - } if c, ok := registry[name]; ok { return c } diff --git a/xds/bootstrap/bootstrap_test.go b/xds/bootstrap/bootstrap_test.go index 935976975..d1f7a1b64 100644 --- a/xds/bootstrap/bootstrap_test.go +++ b/xds/bootstrap/bootstrap_test.go @@ -22,7 +22,6 @@ import ( "testing" "google.golang.org/grpc/credentials" - "google.golang.org/grpc/internal/envconfig" ) const testCredsBuilderName = "test_creds" @@ -65,14 +64,12 @@ func TestRegisterNew(t *testing.T) { func TestCredsBuilders(t *testing.T) { tests := []struct { - typename string - builder Credentials - minimumRequiredConfig json.RawMessage + typename string + builder Credentials }{ - {"google_default", &googleDefaultCredsBuilder{}, nil}, - {"insecure", &insecureCredsBuilder{}, nil}, - {"tls", &tlsCredsBuilder{}, nil}, - {"jwt_token_file", &jwtCallCredsBuilder{}, json.RawMessage(`{"jwt_token_file":"/path/to/token.jwt"}`)}, + {"google_default", &googleDefaultCredsBuilder{}}, + {"insecure", &insecureCredsBuilder{}}, + {"tls", &tlsCredsBuilder{}}, } for _, test := range tests { @@ -81,13 +78,10 @@ func TestCredsBuilders(t *testing.T) { t.Errorf("%T.Name = %v, want %v", test.builder, got, want) } - bundle, stop, err := test.builder.Build(test.minimumRequiredConfig) + _, stop, err := test.builder.Build(nil) if err != nil { t.Fatalf("%T.Build failed: %v", test.builder, err) } - if bundle == nil { - t.Errorf("%T.Build returned nil bundle, expected non-nil", test.builder) - } stop() }) } @@ -106,27 +100,3 @@ func TestTlsCredsBuilder(t *testing.T) { stop() } } - -func TestJwtCallCredentials_BuildDisabledIfFeatureNotEnabled(t *testing.T) { - builder := GetCredentials("jwt_call_creds") - if builder != nil { - t.Fatal("Expected nil Credentials for jwt_call_creds when the feature is disabled.") - } - - // Enable JWT call credentials - original := envconfig.XDSBootstrapCallCredsEnabled - envconfig.XDSBootstrapCallCredsEnabled = true - defer func() { - envconfig.XDSBootstrapCallCredsEnabled = original - }() - - // Test that GetCredentials returns the JWT builder - builder = GetCredentials("jwt_token_file") - if builder == nil { - t.Fatal("GetCredentials(\"jwt_token_file\") returned nil") - } - - if got, want := builder.Name(), "jwt_token_file"; got != want { - t.Errorf("Retrieved builder name = %q, want %q", got, want) - } -} diff --git a/xds/bootstrap/credentials.go b/xds/bootstrap/credentials.go index 38018972f..578e12789 100644 --- a/xds/bootstrap/credentials.go +++ b/xds/bootstrap/credentials.go @@ -24,7 +24,6 @@ import ( "google.golang.org/grpc/credentials" "google.golang.org/grpc/credentials/google" "google.golang.org/grpc/credentials/insecure" - "google.golang.org/grpc/internal/xds/bootstrap/jwtcreds" "google.golang.org/grpc/internal/xds/bootstrap/tlscreds" ) @@ -32,7 +31,6 @@ func init() { RegisterCredentials(&insecureCredsBuilder{}) RegisterCredentials(&googleDefaultCredsBuilder{}) RegisterCredentials(&tlsCredsBuilder{}) - RegisterCredentials(&jwtCallCredsBuilder{}) } // insecureCredsBuilder implements the `Credentials` interface defined in @@ -70,15 +68,3 @@ func (d *googleDefaultCredsBuilder) Build(json.RawMessage) (credentials.Bundle, func (d *googleDefaultCredsBuilder) Name() string { return "google_default" } - -// jwtCallCredsBuilder implements the `Credentials` interface defined in -// package `xds/bootstrap` and encapsulates JWT call credentials. -type jwtCallCredsBuilder struct{} - -func (j *jwtCallCredsBuilder) Build(configJSON json.RawMessage) (credentials.Bundle, func(), error) { - return jwtcreds.NewBundle(configJSON) -} - -func (j *jwtCallCredsBuilder) Name() string { - return "jwt_token_file" -} diff --git a/xds/internal/xdsclient/clientimpl.go b/xds/internal/xdsclient/clientimpl.go index 80bf8d0e8..967182740 100644 --- a/xds/internal/xdsclient/clientimpl.go +++ b/xds/internal/xdsclient/clientimpl.go @@ -229,9 +229,7 @@ func populateGRPCTransportConfigsFromServerConfig(sc *bootstrap.ServerConfig, gr grpcTransportConfigs[cc.Type] = grpctransport.Config{ Credentials: bundle, GRPCNewClient: func(target string, opts ...grpc.DialOption) (*grpc.ClientConn, error) { - // Only add call credentials that are compatible with this transport type - // Call credentials requiring transport security are skipped for insecure transports - opts = append(opts, sc.DialOptionsWithCallCredsForTransport(cc.Type, bundle.TransportCredentials())...) + opts = append(opts, sc.DialOptions()...) return grpc.NewClient(target, opts...) }, } diff --git a/xds/internal/xdsclient/clientimpl_test.go b/xds/internal/xdsclient/clientimpl_test.go index c7884e8eb..fbfc24a07 100644 --- a/xds/internal/xdsclient/clientimpl_test.go +++ b/xds/internal/xdsclient/clientimpl_test.go @@ -19,10 +19,8 @@ package xdsclient import ( - "context" "encoding/json" "fmt" - "net" "reflect" "sync" "testing" @@ -30,9 +28,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "google.golang.org/grpc" - "google.golang.org/grpc/credentials" "google.golang.org/grpc/credentials/insecure" - "google.golang.org/grpc/internal/envconfig" "google.golang.org/grpc/internal/testutils/stats" "google.golang.org/grpc/internal/xds/bootstrap" "google.golang.org/grpc/xds/internal/clients" @@ -263,90 +259,3 @@ func (s) TestBuildXDSClientConfig_Success(t *testing.T) { }) } } - -func TestServerConfigCallCredsIntegration(t *testing.T) { - // Enable JWT call credentials - originalJWTEnabled := envconfig.XDSBootstrapCallCredsEnabled - envconfig.XDSBootstrapCallCredsEnabled = true - defer func() { - envconfig.XDSBootstrapCallCredsEnabled = originalJWTEnabled - }() - - tokenFile := "/token.jwt" - // Test server config with both channel and call credentials - serverConfigJSON := `{ - "server_uri": "xds-server:443", - "channel_creds": [{"type": "tls", "config": {}}], - "call_creds": [ - { - "type": "jwt_token_file", - "config": {"jwt_token_file": "` + tokenFile + `"} - } - ] - }` - - var sc bootstrap.ServerConfig - if err := sc.UnmarshalJSON([]byte(serverConfigJSON)); err != nil { - t.Fatalf("Failed to unmarshal server config: %v", err) - } - - // Verify call credentials are processed - callCreds := sc.CallCreds() - if len(callCreds) != 1 { - t.Errorf("Expected 1 call credential, got %d", len(callCreds)) - } - - selectedCallCreds := sc.SelectedCallCreds() - if len(selectedCallCreds) != 1 { - t.Errorf("Expected 1 selected call credential, got %d", len(selectedCallCreds)) - } - - // Test dial options for secure transport (should include JWT) - secureOpts := sc.DialOptionsWithCallCredsForTransport("tls", &mockTransportCreds{protocol: "tls"}) - if len(secureOpts) != 1 { - t.Errorf("Expected dial options for secure transport. Got: %#v", secureOpts) - } - - // Test dial options for insecure transport (should exclude JWT) - insecureOpts := sc.DialOptionsWithCallCredsForTransport("insecure", &mockTransportCreds{protocol: "insecure"}) - - // JWT should be filtered out for insecure transport - if len(insecureOpts) >= len(secureOpts) { - t.Error("Expected fewer dial options for insecure transport (JWT should be filtered)") - } -} - -// Mock transport credentials for testing -type mockTransportCreds struct { - protocol string -} - -func (m *mockTransportCreds) ClientHandshake(_ context.Context, _ string, rawConn net.Conn) (net.Conn, credentials.AuthInfo, error) { - return rawConn, &mockAuthInfo{}, nil -} - -func (m *mockTransportCreds) ServerHandshake(rawConn net.Conn) (net.Conn, credentials.AuthInfo, error) { - return rawConn, &mockAuthInfo{}, nil -} - -func (m *mockTransportCreds) Info() credentials.ProtocolInfo { - return credentials.ProtocolInfo{SecurityProtocol: m.protocol} -} - -func (m *mockTransportCreds) Clone() credentials.TransportCredentials { - return &mockTransportCreds{protocol: m.protocol} -} - -func (m *mockTransportCreds) OverrideServerName(string) error { - return nil -} - -type mockAuthInfo struct{} - -func (m *mockAuthInfo) AuthType() string { - return "mock" -} - -func (m *mockAuthInfo) GetCommonAuthInfo() credentials.CommonAuthInfo { - return credentials.CommonAuthInfo{} -}