xds: client-side security: xDS credentials implementation #3888
Conversation
|
@ZhenLian @jiangtaoli2016 |
|
Gentle ping @dfawley |
credentials/xds/xds.go
Outdated
| // On the client side, rootProvider is mandatory. IdentityProvider is | ||
| // optional based on whether the client is doing TLS or mTLS. | ||
| if isClient && chi.rootProvider == nil { | ||
| return errors.New("root certificate provider is missing") |
There was a problem hiding this comment.
These are going to surface to users IIUC; we should maybe have a better description of what went wrong in that case.
There was a problem hiding this comment.
What would this mean when it happens? It's a programming error on our part? Or a usage error on the user's part? The error should tell the end user what to do: either how to fix the problem or let them know it's a grpc-go bug.
There was a problem hiding this comment.
This is an extreme edge case I would think where the control plane is not sending us the correct config. So, I have added another line to the error message asking the user to check the control plane config.
credentials/xds/xds.go
Outdated
| // SetHandshakeInfo returns a copy of attr which is updated with hInfo. | ||
| func SetHandshakeInfo(attr *attributes.Attributes, hInfo *HandshakeInfo) *attributes.Attributes { | ||
| return attr.WithValues(handshakeAttrKey{}, hInfo) | ||
| } | ||
|
|
||
| // GetHandshakeInfo returns a pointer to the HandshakeInfo stored in attr. | ||
| func GetHandshakeInfo(attr *attributes.Attributes) *HandshakeInfo { |
There was a problem hiding this comment.
Please make these work on resolver.Addresses instead of attributes, e.g.:
grpc-go/internal/hierarchy/hierarchy.go
Line 35 in 759569b
There was a problem hiding this comment.
I have changed SetHandshakeInfo to work on resolver.Address since this will be invoked from the CDS balancer and it has a resolver.Address at that point.
GetHandshakeInfo is called from the credentials code, and there is no resolver.Address at that point.
credentials/xds/xds.go
Outdated
| // On the client side, rootProvider is mandatory. IdentityProvider is | ||
| // optional based on whether the client is doing TLS or mTLS. | ||
| if isClient && chi.rootProvider == nil { | ||
| return errors.New("root certificate provider is missing") |
There was a problem hiding this comment.
What would this mean when it happens? It's a programming error on our part? Or a usage error on the user's part? The error should tell the end user what to do: either how to fix the problem or let them know it's a grpc-go bug.
credentials/xds/xds.go
Outdated
| // updated with hInfo. | ||
| func SetHandshakeInfo(addr resolver.Address, hInfo *HandshakeInfo) resolver.Address { | ||
| addr.Attributes = addr.Attributes.WithValues(handshakeAttrKey{}, hInfo) | ||
| return addr | ||
| } | ||
|
|
||
| // GetHandshakeInfo returns a pointer to the HandshakeInfo stored in attr. |
There was a problem hiding this comment.
Should this be unexported? Does anyone else need access to it? If so that would eliminate the asymmetry of the API (setter takes address, getter takes attributes).
easwars
changed the title
easwars
changed the title
easwars
changed the title
No description provided.