Skip to content
Open
Show file tree
Hide file tree
Changes from 52 commits
Commits
Show all changes
70 commits
Select commit Hold shift + click to select a range
6e63daa
xds: read JWT credentials from file as per A97
dimpavloff Jun 14, 2025
3268ea5
remove example
dimpavloff Jul 6, 2025
b18a1f5
refactor test creation
dimpavloff Jul 6, 2025
eb391af
refactor token string padding
dimpavloff Jul 26, 2025
d43893a
remove example; mark as experimental
dimpavloff Jul 29, 2025
167b86e
reorganise struct attributes
dimpavloff Jul 29, 2025
439d28c
rename methods with Locked suffix
dimpavloff Jul 29, 2025
b36d4b6
remove context param from refreshTokenSync
dimpavloff Jul 29, 2025
26e0451
reformat comments; remove redundant cachedErrorTime field
dimpavloff Jul 29, 2025
da2de8c
add defaultTestTimeout const
dimpavloff Jul 29, 2025
51ce34c
refactor test to use wantErr string only
dimpavloff Jul 29, 2025
f87f1f2
fix punctuation
dimpavloff Jul 29, 2025
15dd057
less prosaic subtest names
dimpavloff Jul 29, 2025
54cbbcb
remove unit test
dimpavloff Jul 30, 2025
9c5035d
rename preemptiveRefresh to forceRefresh
dimpavloff Jul 31, 2025
ec915dc
remove unused context param
dimpavloff Jul 31, 2025
1d95fa2
rename files
dimpavloff Aug 21, 2025
a797ed9
use cond variable
dimpavloff Aug 21, 2025
fd388d1
refactor to no longer need cond
dimpavloff Aug 21, 2025
790a2d9
fix docstring comment
dimpavloff Aug 21, 2025
6713190
cache authorization header instead of token
dimpavloff Aug 21, 2025
3f563eb
remove internal/ and xds/ changes
dimpavloff Aug 21, 2025
a38573b
remove xds/bootstrap
dimpavloff Aug 21, 2025
12fedd5
fix comment docstrings
dimpavloff Aug 21, 2025
2c80eab
xds: read JWT credentials from file as per A97
dimpavloff Jun 14, 2025
b344fdb
comment punctuation
dimpavloff Aug 7, 2025
517c6ad
remove leftover
dimpavloff Aug 7, 2025
34689c8
refactor test to use wantErr string only
dimpavloff Aug 7, 2025
0d5d0a5
tidy comments
dimpavloff Aug 7, 2025
e465e7a
Merge remote-tracking branch 'origin/master' into a97-stacked-3
dimpavloff Aug 22, 2025
63ff226
cleanup leftover files after rebase
dimpavloff Aug 22, 2025
5673c18
fix merge conflict
dimpavloff Aug 22, 2025
51bb1b5
Merge remote-tracking branch 'origin/master' into a97-stacked-3
dimpavloff Sep 16, 2025
41d248f
remove renamed files
dimpavloff Sep 16, 2025
5d1d1f6
move callcreds type and method a bit
dimpavloff Sep 16, 2025
9ca7db8
use gRFC instead of RFC
dimpavloff Sep 16, 2025
ce93551
subtest names and want vs expected
dimpavloff Sep 16, 2025
2de494e
refactor tests to not do string comparison
dimpavloff Sep 16, 2025
6e5d5ce
remove overlapping test assertions
dimpavloff Sep 17, 2025
167e6b9
reorder fields
dimpavloff Sep 17, 2025
6026155
fix test
dimpavloff Sep 17, 2025
3b22242
rename test
dimpavloff Sep 17, 2025
1ab6a7e
rename selectedCreds to selectedChannelCreds
dimpavloff Sep 26, 2025
aa00501
rename Credentials to ChannelCredentials in xds/bootstrap and introdu…
dimpavloff Sep 26, 2025
cf10bb9
simplify jwtcreds to not implement Bundle but instead just construct …
dimpavloff Sep 26, 2025
0533504
make jwtCallCredsBuilder adhere to CallCredentials interface
dimpavloff Sep 26, 2025
cc1aa48
remove envvar check from ChannelCredentials
dimpavloff Sep 26, 2025
851f56d
remove DialOptionsWithCallCredsForTransport
dimpavloff Sep 26, 2025
9cb3bf4
rename files
dimpavloff Sep 26, 2025
63b00bd
move the check for XDSBootstrapCallCredsEnabled when unmarshalling bo…
dimpavloff Sep 26, 2025
3dbb546
add comment to map var
dimpavloff Sep 26, 2025
651cba1
xds -> xDS consistency
dimpavloff Sep 26, 2025
c09c763
change xds env var comment
dimpavloff Oct 3, 2025
deaa239
rename CallCreds to CallCredsConfig and SelectedCallCreds to CallCreds
dimpavloff Oct 3, 2025
6a9dc14
split success and failure test cases for TestNewCallCredentials
dimpavloff Oct 3, 2025
5cbe1a1
method receiver; document idempotency
dimpavloff Oct 3, 2025
a1944c6
include CallCredsConfigs in ServerConfig.String method
dimpavloff Oct 3, 2025
92c8a49
use grpctest.Tester
dimpavloff Oct 3, 2025
0dc3856
add docstring
dimpavloff Oct 3, 2025
2a2dcb4
punctuation and docstring
dimpavloff Oct 13, 2025
9a3488e
add TODO for renaming ChannelCreds
dimpavloff Oct 20, 2025
57a114b
remove sc.CallCreds method
dimpavloff Oct 20, 2025
4d9b7a6
use SetEnvConfig
dimpavloff Oct 20, 2025
355afcc
inline token.jwt path in some unit tests
dimpavloff Oct 20, 2025
035d7b8
test formatting and test error -- got, then want
dimpavloff Oct 20, 2025
7190b24
include call param in NewCallCredentials test failure message; change…
dimpavloff Oct 20, 2025
9f1c96f
rename tempfile to jwt_token
dimpavloff Oct 20, 2025
3bc7632
credentials, not bundle in err message
dimpavloff Oct 20, 2025
c776def
return empty func for failures, too
dimpavloff Oct 20, 2025
3205bd4
defer stop; remove unnecessary field
dimpavloff Oct 20, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions internal/envconfig/xds.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,4 +74,9 @@ var (
// For more details, see:
// https://github.com/grpc/proposal/blob/master/A86-xds-http-connect.md
XDSHTTPConnectEnabled = boolFromEnv("GRPC_EXPERIMENTAL_XDS_HTTP_CONNECT", false)

// XDSBootstrapCallCredsEnabled controls if JWT call credentials can be used
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: This environment variable guards the newly added bootstrap field call_creds. I agree that the only call credentials that we currently support via this mechanism is the JWT call creds, but that could change any day (before we get rid of this env var). So, I would prefer we more explicitly call out what this env var guards.

// 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)
)
84 changes: 73 additions & 11 deletions internal/xds/bootstrap/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ 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"
Expand Down Expand Up @@ -83,6 +84,20 @@ func (cc ChannelCreds) String() string {
return cc.Type + "-" + string(b)
}

// CallCreds contains the call credentials configuration for individual RPCs.
// It implements gRFC A97 call credentials structure.
type CallCreds struct {
// Type contains a unique name identifying the call credentials type.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm .. The gRFC does not state any uniqueness requirements for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, you're right, it doesn't. I was influenced by the ChannelCreds.Type but there it makes more sense given that only one gets picked up. I've updated the comment

Type string `json:"type,omitempty"`
// Config contains the JSON configuration for this call credentials.
Config json.RawMessage `json:"config,omitempty"`
}

// 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)
}

// ServerConfigs represents a collection of server configurations.
type ServerConfigs []*ServerConfig

Expand Down Expand Up @@ -165,14 +180,16 @@ func (a *Authority) Equal(other *Authority) bool {
type ServerConfig struct {
serverURI string
channelCreds []ChannelCreds
callCreds []CallCreds
serverFeatures []string

// 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
selectedChannelCreds ChannelCreds
selectedCallCreds []credentials.PerRPCCredentials
credsDialOption grpc.DialOption
extraDialOptions []grpc.DialOption

cleanups []func()
}
Expand All @@ -194,6 +211,18 @@ func (sc *ServerConfig) ServerFeatures() []string {
return sc.serverFeatures
}

// CallCreds returns the call credentials configuration for this server.
func (sc *ServerConfig) CallCreds() []CallCreds {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we call this CallCredsConfig and the next method CallCreds?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was trying to keep symmetry with ChannelCreds and SelectedChannelCreds. However, SelectedCallCreds now returns []credentials.PerRPCCredentials rather than the config, still, which breaks this symmetry.

Realistically, I'm not sure even if the public methods are needed -- the PerRPCCredentials are added to the ServerConfig.extraDialOptions and used via ServerConfig.DialOptions()

WDYT, shall I remove the public methods? I've renamed them in the meantime. I've used CallCredsConfigs, plural, because I also renamed the Go type to CallCredsConfig and the method returns a collection of those. I've also renamed the relevant fields in ServerConfig (and ServerConfigTestingOptions) in similar way.

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.
Expand All @@ -211,10 +240,10 @@ func (sc *ServerConfig) ServerFeaturesIgnoreResourceDeletion() bool {
return false
}

// SelectedCreds returns the selected credentials configuration for
// SelectedChannelCreds returns the selected credentials configuration for
// communicating with this server.
func (sc *ServerConfig) SelectedCreds() ChannelCreds {
return sc.selectedCreds
func (sc *ServerConfig) SelectedChannelCreds() ChannelCreds {
return sc.selectedChannelCreds
}

// DialOptions returns a slice of all the configured dial options for this
Expand Down Expand Up @@ -245,9 +274,11 @@ 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):
case !sc.selectedChannelCreds.Equal(other.selectedChannelCreds):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is existing code, but I'm wondering if we can remove this field from the equality check for a couple of reasons:

  • we are already checking the channel credentials configuration (from which we selected this credentials) for equality
  • this type is supposed to semantically represent configuration corresponding to an xDS server and not any runtime state. We are already abusing it a little (or optimizing) by storing the selected credentials in it, so that we don't have to parse the configuration again when we actually want to use the credentials.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree - because we're already comparing sc.channelCreds and other.channelCreds and because the config doesn't get dynamically updated, it makes sense.
Shall I just remove it in this PR or send a follow-up PR in case we're missing something?

return false
}
return true
Expand All @@ -256,16 +287,17 @@ func (sc *ServerConfig) Equal(other *ServerConfig) bool {
// String returns the string representation of the ServerConfig.
func (sc *ServerConfig) String() string {
if len(sc.serverFeatures) == 0 {
return fmt.Sprintf("%s-%s", sc.serverURI, sc.selectedCreds.String())
return fmt.Sprintf("%s-%s", sc.serverURI, sc.selectedChannelCreds.String())
}
features := strings.Join(sc.serverFeatures, "-")
return strings.Join([]string{sc.serverURI, sc.selectedCreds.String(), features}, "-")
return strings.Join([]string{sc.serverURI, sc.selectedChannelCreds.String(), features}, "-")
}

// The following fields correspond 1:1 with the JSON schema for ServerConfig.
type serverConfigJSON struct {
ServerURI string `json:"server_uri,omitempty"`
ChannelCreds []ChannelCreds `json:"channel_creds,omitempty"`
CallCreds []CallCreds `json:"call_creds,omitempty"`
ServerFeatures []string `json:"server_features,omitempty"`
}

Expand All @@ -274,6 +306,7 @@ func (sc *ServerConfig) MarshalJSON() ([]byte, error) {
server := &serverConfigJSON{
ServerURI: sc.serverURI,
ChannelCreds: sc.channelCreds,
CallCreds: sc.callCreds,
ServerFeatures: sc.serverFeatures,
}
return json.Marshal(server)
Expand All @@ -294,26 +327,51 @@ func (sc *ServerConfig) UnmarshalJSON(data []byte) error {

sc.serverURI = server.ServerURI
sc.channelCreds = server.ChannelCreds
sc.callCreds = server.CallCreds
sc.serverFeatures = server.ServerFeatures

for _, cc := range server.ChannelCreds {
// We stop at the first credential type that we support.
c := bootstrap.GetCredentials(cc.Type)
c := bootstrap.GetChannelCredentials(cc.Type)
if c == nil {
continue
}
bundle, cancel, err := c.Build(cc.Config)
if err != nil {
return fmt.Errorf("failed to build credentials bundle from bootstrap for %q: %v", cc.Type, err)
}
sc.selectedCreds = cc
sc.selectedChannelCreds = cc
sc.credsDialOption = grpc.WithCredentialsBundle(bundle)
if d, ok := bundle.(extraDialOptions); ok {
sc.extraDialOptions = d.DialOptions()
}
sc.cleanups = append(sc.cleanups, cancel)
break
}

if envconfig.XDSBootstrapCallCredsEnabled {
// Process call credentials - unlike channel creds, we use ALL supported
// types. Also, call credentials are optional as per gRFC A97.
for _, callCredConfig := range server.CallCreds {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Can we use a shorter name for the loop variable. Maybe cfg?

c := bootstrap.GetCallCredentials(callCredConfig.Type)
if c == nil {
// Skip unsupported call credential types (don't fail bootstrap).
continue
}
callCred, cancel, err := c.Build(callCredConfig.Config)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We refer to them as credentials and not credential even when there is a single one. This was a major source of confusion to me when I joined the team as well :)

So, s/callCred/callCreds/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haha, yes, this did confuse me a bit when starting. Thanks for spotting this

if err != nil {
// Call credential validation failed - this should fail bootstrap.
return fmt.Errorf("failed to build call credentials from bootstrap for %q: %v", callCredConfig.Type, err)
}
if callCred == nil {
continue
}
sc.selectedCallCreds = append(sc.selectedCallCreds, callCred)
sc.extraDialOptions = append(sc.extraDialOptions, grpc.WithPerRPCCredentials(callCred))
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))
}
Expand All @@ -333,6 +391,9 @@ type ServerConfigTestingOptions struct {
// ChannelCreds contains a list of channel credentials to use when talking
// to this server. If unspecified, `insecure` credentials will be used.
ChannelCreds []ChannelCreds
// CallCreds contains a list of call credentials to use for individual RPCs
// to this server. Optional.
CallCreds []CallCreds
// ServerFeatures represents the list of features supported by this server.
ServerFeatures []string
}
Expand All @@ -349,6 +410,7 @@ func ServerConfigForTesting(opts ServerConfigTestingOptions) (*ServerConfig, err
scInternal := &serverConfigJSON{
ServerURI: opts.URI,
ChannelCreds: cc,
CallCreds: opts.CallCreds,
ServerFeatures: opts.ServerFeatures,
}
scJSON, err := json.Marshal(scInternal)
Expand Down
Loading
Loading