envoyproxy / envoy

Cloud-native high-performance edge/middle/service proxy
https://www.envoyproxy.io
Apache License 2.0
24.28k stars 4.69k forks source link

[xds] ADS reject duplicate resources in grpc_mux #14847

Open stevenzzzz opened 3 years ago

stevenzzzz commented 3 years ago

Title: in grpc_mux_impl, let's reject a resonse when we see duplicate resources.

Description: see https://github.com/envoyproxy/envoy/blob/main/source/common/config/grpc_mux_impl.cc#L224 We want to not "dedup" but reject the response here when the emplace fails

new_grpc_mux_impl seems doesnt have same issue as it push dup resources to each xds impl, where the dup resources gets rejected.

Repro steps:

send a discoveryResponse with multiple resources with the same name. Envoy accepts it silently.

stevenzzzz commented 3 years ago

/cc @fredlas

stevenzzzz commented 3 years ago

/cc @htuch

stevenzzzz commented 3 years ago

/cc htuch

htuch commented 3 years ago

@dmitri-d in the unified gRPC mux do we still have htis problem? If not, we can try and get that merged (I know, I owe a review).

dmitri-d commented 3 years ago

@htuch: I'll check.

dmitri-d commented 3 years ago

@htuch: in the unified mux branch delta xDS errors out on a duplicate resource name in an update, but sotw xDS does not. Looks like a straightforward fix though.

github-actions[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

github-actions[bot] commented 3 years ago

This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted" or "no stalebot". Thank you for your contributions.