A27 update: "channel_creds" bootstrap field is required and supports "insecure" #204
Conversation
| // the same channel creds as the backends, but with any associated | ||
| // call creds stripped off. | ||
| // supports. This field is required and must contain at least one | ||
| // channel creds type that the client supports. |
There was a problem hiding this comment.
Why not make "insecure" default instead of making a breaking change?
There was a problem hiding this comment.
That would still be a breaking change. If someone is relying on getting, e.g., TlsCreds from the parent channel, that would still break if we change the default to "insecure".
I suspect no one will notice this change either way at this point, which is why I'm willing to do it (although we could certainly change our mind if someone objects in this PR). But I don't see that making "insecure" the default is any better.
There was a problem hiding this comment.
The "inherit from parent channel" is anyway going to break since that is going away. By making "insecure" default, at least the insecure case is not broken.
There was a problem hiding this comment.
The only way to currently get insecure creds is to inherit it from the parent channel, and that's only one of many possible types of creds that could be inherited from the parent channel. I don't see why insecure creds is special here.
In general, I don't think insecure creds is a very good default. This could lead to people accidentally having an insecure connection when they didn't expect it. I don't want to do that unless the user explicitly asks us to. This is the same reason why (e.g.) our CreateChannel() API doesn't default to insecure; it creates an insecure connection only if the application explicitly uses InsecureCreds.
There was a problem hiding this comment.
I agree with @markdroth. Especially if the default behavior is changing, changing it to something that gives an error is preferable to something that will only fail later with a more confusing failure mode -- or, worse, leads to a security problem due to "insecure".
|
cc @menghanl |
No description provided.