feat(grpc-xds): gRFC A74 model validated xDS resources and XdsConfig snapshot - #2775
Conversation
dfawley
left a comment
dfawley
left a comment
There was a problem hiding this comment.
Looks great overall, thanks! I've found a handful of things that should be fixed, but it should all be pretty small.
|
@dfawley all prev comments addressed, ready for another round of review. Thanks! |
dfawley
left a comment
dfawley
left a comment
There was a problem hiding this comment.
I'm not sure about this health enum, but aside from that everything LGTM. Thanks!
| // `HealthStatus` is an open enum, so a newer control plane can send | ||
| // a status this build does not know. Per gRFC A27 only HEALTHY and | ||
| // UNKNOWN are usable, so an unrecognized status fails closed. | ||
| other => Self::Other(i32::from(other)), |
There was a problem hiding this comment.
I assume our other languages are treating anything outside this range as UNKNOWN, which would then cause us to use it? That seems wrong to me, but then this is a behavior difference. But also I think if envoy ever added values to this enum, it would cause some trouble.
There was a problem hiding this comment.
No, for clarification the other languages also handles a out of range status as a special error case. UNKNOWN is genuinely only used for when status equals UKNOWN. The previous commit of this PR made the mistake of converting out of range to unknown, the current commit matches other langs.
There was a problem hiding this comment.
Yes, it sounds like you're right. Go explicitly checks for UNKNOWN or HEALTHY, and per the proto spec, this is an open enum that is expected to represent out of range values directly in the field itself. I didn't realize that had changed from proto2 behavior. Thanks for clarifying.
2 participants
Motivation
Ref: #2754
Per gRFC A74, to avoid config tear across xDS layers, all watches (LDS, RDS, CDS, EDS) are moved into a single resolver behind an
XdsDependencyManagerthat emits a single completeXdsConfig.Solution
This PR implements the
XdsConfigmodeling, along with its xDS resource types(the validated types corresponding to the Envoy proto types) and validation logic. This PR is a pre-req to theXdsDependencyManager.The shape of
XdsConfigclosely mirrors gRFC A74's C++ sketch, as well as other gRPC implementations such as grpc-go. A few design choices were made for Rust ergonomics, such as using an index to store selected virtual host rather than a reference to avoid self-referential borrow.