-
Notifications
You must be signed in to change notification settings - Fork 4.6k
xds bootstrap: enable using JWT Call Credentials (part 2 for A97) #8536
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
6e63daa
3268ea5
b18a1f5
eb391af
d43893a
167b86e
439d28c
b36d4b6
26e0451
da2de8c
51ce34c
f87f1f2
15dd057
54cbbcb
9c5035d
ec915dc
1d95fa2
a797ed9
fd388d1
790a2d9
6713190
3f563eb
a38573b
12fedd5
2c80eab
b344fdb
517c6ad
34689c8
0d5d0a5
e465e7a
63ff226
5673c18
51bb1b5
41d248f
5d1d1f6
9ca7db8
ce93551
2de494e
6e5d5ce
167e6b9
6026155
3b22242
1ab6a7e
aa00501
cf10bb9
0533504
cc1aa48
851f56d
9cb3bf4
63b00bd
3dbb546
651cba1
c09c763
deaa239
6a9dc14
5cbe1a1
a1944c6
92c8a49
0dc3856
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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" | ||
|
@@ -83,6 +84,40 @@ func (cc ChannelCreds) String() string { | |
return cc.Type + "-" + string(b) | ||
} | ||
|
||
// CallCredsConfig contains the call credentials configuration for individual RPCs. | ||
// It implements gRFC A97 call credentials structure. | ||
type CallCredsConfig struct { | ||
// Type contains a name identifying the call credentials type. | ||
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 CallCredsConfig) Equal(other CallCredsConfig) bool { | ||
return cc.Type == other.Type && bytes.Equal(cc.Config, other.Config) | ||
} | ||
|
||
func (cc CallCredsConfig) String() string { | ||
if cc.Config == nil { | ||
return cc.Type | ||
} | ||
// We do not expect the Marshal call to fail since we wrote to cc.Config | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: Here and in a few other places, please terminate comment lines with periods. https://google.github.io/styleguide/go/decisions#comment-sentences |
||
b, _ := json.Marshal(cc.Config) | ||
return cc.Type + "-" + string(b) | ||
} | ||
|
||
// CallCredsConfigs represents a collection of call credentials configurations. | ||
type CallCredsConfigs []CallCredsConfig | ||
|
||
func (ccs CallCredsConfigs) String() string { | ||
var creds []string | ||
for _, cc := range ccs { | ||
creds = append(creds, cc.String()) | ||
} | ||
return strings.Join(creds, ",") | ||
} | ||
|
||
// ServerConfigs represents a collection of server configurations. | ||
type ServerConfigs []*ServerConfig | ||
|
||
|
@@ -163,16 +198,18 @@ func (a *Authority) Equal(other *Authority) bool { | |
|
||
// ServerConfig contains the configuration to connect to a server. | ||
type ServerConfig struct { | ||
serverURI string | ||
channelCreds []ChannelCreds | ||
serverFeatures []string | ||
serverURI string | ||
channelCreds []ChannelCreds | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While you are here, could you please add a TODO to rename |
||
callCredsConfigs []CallCredsConfig | ||
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() | ||
} | ||
|
@@ -194,6 +231,18 @@ func (sc *ServerConfig) ServerFeatures() []string { | |
return sc.serverFeatures | ||
} | ||
|
||
// CallCredsConfigs returns the call credentials configuration for this server. | ||
func (sc *ServerConfig) CallCredsConfigs() CallCredsConfigs { | ||
return sc.callCredsConfigs | ||
} | ||
|
||
// CallCreds 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) CallCreds() []credentials.PerRPCCredentials { | ||
return sc.selectedCallCreds | ||
} | ||
Comment on lines
+239
to
+244
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need this method at all (and the |
||
|
||
// 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. | ||
|
@@ -211,10 +260,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 | ||
|
@@ -245,9 +294,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.callCredsConfigs, other.callCredsConfigs, func(a, b CallCredsConfig) 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): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
|
||
return false | ||
} | ||
return true | ||
|
@@ -256,25 +307,27 @@ 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 strings.Join([]string{sc.serverURI, sc.selectedChannelCreds.String(), sc.CallCredsConfigs().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, sc.CallCredsConfigs().String()}, "-") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I remember you asking me if we want to include the channel creds here and I said yes. But thinking about this now, I'm wondering if the returned string is going to be too verbose, especially since the |
||
} | ||
|
||
// 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"` | ||
ServerFeatures []string `json:"server_features,omitempty"` | ||
ServerURI string `json:"server_uri,omitempty"` | ||
ChannelCreds []ChannelCreds `json:"channel_creds,omitempty"` | ||
CallCredsConfigs []CallCredsConfig `json:"call_creds,omitempty"` | ||
ServerFeatures []string `json:"server_features,omitempty"` | ||
} | ||
|
||
// MarshalJSON returns marshaled JSON bytes corresponding to this server config. | ||
func (sc *ServerConfig) MarshalJSON() ([]byte, error) { | ||
server := &serverConfigJSON{ | ||
ServerURI: sc.serverURI, | ||
ChannelCreds: sc.channelCreds, | ||
ServerFeatures: sc.serverFeatures, | ||
ServerURI: sc.serverURI, | ||
ChannelCreds: sc.channelCreds, | ||
CallCredsConfigs: sc.callCredsConfigs, | ||
ServerFeatures: sc.serverFeatures, | ||
} | ||
return json.Marshal(server) | ||
} | ||
|
@@ -294,26 +347,51 @@ func (sc *ServerConfig) UnmarshalJSON(data []byte) error { | |
|
||
sc.serverURI = server.ServerURI | ||
sc.channelCreds = server.ChannelCreds | ||
sc.callCredsConfigs = server.CallCredsConfigs | ||
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 _, cfg := range server.CallCredsConfigs { | ||
c := bootstrap.GetCallCredentials(cfg.Type) | ||
if c == nil { | ||
// Skip unsupported call credential types (don't fail bootstrap). | ||
continue | ||
} | ||
callCreds, cancel, err := c.Build(cfg.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", cfg.Type, err) | ||
} | ||
if callCreds == nil { | ||
continue | ||
} | ||
sc.selectedCallCreds = append(sc.selectedCallCreds, callCreds) | ||
sc.extraDialOptions = append(sc.extraDialOptions, grpc.WithPerRPCCredentials(callCreds)) | ||
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)) | ||
} | ||
|
@@ -333,6 +411,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 | ||
// CallCredsConfigs contains a list of call credentials to use for individual RPCs | ||
// to this server. Optional. | ||
CallCredsConfigs []CallCredsConfig | ||
// ServerFeatures represents the list of features supported by this server. | ||
ServerFeatures []string | ||
} | ||
|
@@ -347,9 +428,10 @@ func ServerConfigForTesting(opts ServerConfigTestingOptions) (*ServerConfig, err | |
cc = []ChannelCreds{{Type: "insecure"}} | ||
} | ||
scInternal := &serverConfigJSON{ | ||
ServerURI: opts.URI, | ||
ChannelCreds: cc, | ||
ServerFeatures: opts.ServerFeatures, | ||
ServerURI: opts.URI, | ||
ChannelCreds: cc, | ||
CallCredsConfigs: opts.CallCredsConfigs, | ||
ServerFeatures: opts.ServerFeatures, | ||
} | ||
scJSON, err := json.Marshal(scInternal) | ||
if err != nil { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it might be better to clarify in this docstring that these call creds are used when talking to the xDS management server. The docstring on the
ChannelCreds
type does that. So, what do you think about something like:I think the part about A97 is not that important and could be skipped. Thanks.