-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
xds/bootstrap: add plugin system for credentials specified in bootstrap file #5136
Conversation
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.
Mostly nits.
@anicr7 : Also, please don't mark conversations as resolved. We generally let the original commenter do that when they are satisfied. Thanks. |
registry implementations.
xds/credentials/*.go. Also add the relevant builders in the google creds package.
1. New bootstrap package that users can use. 2. Move insecure creds builder to google/creds. 3. Move registration to google/ and google/insecure.
credential/insecure instead. Don't use google/insecure in bootstrap.go as it leads to cyclic dependencies.
…undle() to return new transport credentials again
…e fails due to JSON parsing
942161d
to
1fa6ba1
Compare
credentials/insecure/insecure.go
Outdated
} | ||
|
||
// NewWithMode returns a new insecure Bundle. The mode is ignored. | ||
func (insecureBundle) NewWithMode(_ string) (credentials.Bundle, error) { |
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.
Nit: since this method takes no other parameters, you could skip the _
.
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.
Done
// JSON config. | ||
type insecureCredsBuilder struct{} | ||
|
||
func (i *insecureCredsBuilder) Build(_ json.RawMessage) (credentials.Bundle, error) { |
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.
Same here and in other places. The _
can be skipped since it is the only parameter to the method.
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.
Done
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.
LGTM
Currently the credentials used for communication with the xDS management
server is restricted to "google_default" and "insecure". There is no way
for users to inject custom credentials, even though the bootstrap can be
used to pass in different values.
This PR adds support for a new credentials package which can be used to
register supported credentials along with a name that can be recognized
by the bootstrap.
The xDS credential package also registers the currently supported
"google_default" and "insecure" options to maintain the status quo. To keep the
interface of the xDS Credentials builder clean, the insecure credentials now
satisfies the gRPC credentials bundle interface.
RELEASE NOTES: