A65: mTLS Credentials in xDS Bootstrap File #372
Conversation
A65-xds-mtls-creds-in-bootstrap.md
Outdated
|
|
||
| ### Temporary environment variable protection | ||
|
|
||
| This feature is not enabled via I/O, so no env var protection is needed. |
There was a problem hiding this comment.
It is enabled via I/O, because we read the bootstrap file, and newer bootstrap generators may want to support older clients.
I'd have to dig in to see if that distinction matters in this case though. I know we don't support a list of credentials here, so old clients would appear to fail anyway. There could be complications if we supported multiple servers in the list, but we don't do that either. So I could agree it isn't needed here (especially since this isn't a difficult feature and we're probably not going to have interop tests for it), but the reasoning would need to change.
There was a problem hiding this comment.
Good point -- what I really meant here was that it's not enabled via remote I/O. I think the local file isn't necessarily a problem for everyone, because even though we use a centrally maintained bootstrap generator for TD, other environments might just distribute the bootstrap config with the clients. But I do take your point that reading a file is still I/O and can be an issue in some environments.
FWIW, note that we do actually support a list of credentials for each server, and the client will stop at the first one that it supports, so there is a mechanism here for handling older clients. But that doesn't help if there's a bug in parsing the config or in actually configuring the credentials, which is the sort of thing that we'd normally use an env var guard for.
You're also right that we don't yet support multiple servers, but I don't think that would actually cause any significant complications here, because there's a separate list of creds for each server.
Anyway, I've updated the language here.
There was a problem hiding this comment.
Oh, we do support a list; good. I couldn't understand how we had such an oversight. Good to see we didn't.
For a feature like this, it is useful for an implementation to fully-implement the feature and be tested before registering it as an possible option in the bootstrap. That way all the keys here are supported, or not. (Modulo Java which has tls but none of these.) If it is merged as a single PR, there's no problem, but if the work is split up...
Addresses grpc/grpc#32977.