listener: detect filter chains with overlapping matching rules.#6906
listener: detect filter chains with overlapping matching rules.#6906htuch merged 1 commit intoenvoyproxy:masterfrom
Conversation
This is an alternative to envoyproxy#6022. Fixes envoyproxy#6905. Signed-off-by: Piotr Sikora <piotrsikora@google.com>
htuch
left a comment
There was a problem hiding this comment.
LGTM modulo one comment. Thinking about this, it would be nice to make "not-implemented-yet" a proto annotation and write this check once at proto ingestion time. We could just file an issue for now, WDYT?
| for (const auto& filter_chain : config.filter_chains()) { | ||
| const auto& filter_chain_match = filter_chain.filter_chain_match(); | ||
| if (!filter_chain_match.address_suffix().empty() || filter_chain_match.has_suffix_len() || | ||
| filter_chain_match.source_prefix_ranges_size() || filter_chain_match.source_ports_size()) { |
There was a problem hiding this comment.
Should there be tests for each of these cases? Not sure if it's worth the boilerplate, but it's a bit weird to have explicit rejections that we don't test.
There was a problem hiding this comment.
I think it's not worth the extra tests, since those are already unsupported edge cases, and we have the line coverage from one of them, but I'm happy to add them if you prefer, let me know.
Agreed, see discussion in #6022 :) |
This is an alternative to #6022.
Fixes #6905.
Signed-off-by: Piotr Sikora piotrsikora@google.com