doc: clarify handling of duplicate xDS resource names #12756
Conversation
Signed-off-by: Mark D. Roth <[email protected]>
api/xds_protocol.rst
Outdated
| ^^^^^^^^^^^^^^^^^^^^^^^^ | ||
|
|
||
| It is an error for a server to send a single response that contains the same resource name | ||
| twice. Clients must NACK responses that contain multiple instances of the same resource name. |
There was a problem hiding this comment.
I wonder if we should underspecify this, i.e. say that clients may NACK or chose their own behavior here, e.g. accept first or last resource of the same name. This seems to impose less burden on client implementations for a path that servers should not be following in the first place.
There was a problem hiding this comment.
Changed "must" to "should". Does that work?
There was a problem hiding this comment.
Seems OK, but is there a reason for "should" vs. "may" from your perspective?
There was a problem hiding this comment.
In general, I lean toward a stricter definition of protocol behavior, because I think being more liberal leads to confusing and hard-to-debug interoperability problems. If a server is currently sending a duplicate resource and its current clients happen to be fine with that, and then they start using a new client that NACKs in that case, it may be hard to figure out what's wrong. But if the clients are all strict, then the server will never get into that situation to begin with.
Also, in the context of this specific change, there are a lot fewer client implementations than server implementations, so I'm not really worried about imposing an undue burden on clients.
There was a problem hiding this comment.
Sounds good. Please run fix_format, since there is some double space issue to resolve.
Signed-off-by: Mark D. Roth <[email protected]>
Signed-off-by: Mark D. Roth <[email protected]>
* envoy/master: (90 commits) cleanup: use structured binding (envoyproxy#12791) docs: fix header name for retries in gRPC services (envoyproxy#12790) docs: clarify meaning of HeaderValueOption.append (envoyproxy#12792) doc: clarify handling of duplicate xDS resource names (envoyproxy#12756) Dependencies: build updates. (envoyproxy#12786) Ratelimit: Add optional descriptor key to generic_key action (envoyproxy#12734) test: refactor header inclusion to speed up building (for test/mocks/upstream:upstream_mocks) (envoyproxy#12407) docs: Fix omitted word (envoyproxy#12782) ci: avoid uploading dwp as separate artifact (envoyproxy#12777) doc: Fix small typos (envoyproxy#12769) fix cache factory category (envoyproxy#12765) docs: fix typo v1.15.0.rst (envoyproxy#12680) Add clang-cl RBE toolchain for Windows (envoyproxy#12776) fuzz: add router fuzz proto (envoyproxy#12727) header: New HeaderMatcher and StringMatcher type - Contains (envoyproxy#12623) tcp_proxy: use dynamicMetadata() from StreamInfo for load balancing (envoyproxy#12595) network: add io handle recv function for http inspector (envoyproxy#12736) jwt_authn: supports jwt payload without "iss" field (envoyproxy#12744) Add support for nested JSON format in json logging mode (envoyproxy#12602) http: fixing a fuzz flake by setting details on connection teardown (envoyproxy#12737) ... Signed-off-by: Scott LaVigne <[email protected]>
Signed-off-by: Mark D. Roth [email protected]
Commit Message: doc: clarify handling of duplicate xDS resource names
Additional Description: Updates the xDS protocol spec.
Risk Level: Low
Testing: N/A
Docs Changes: Included in PR
Release Notes: N/A
CC @htuch