envoy
envoy copied to clipboard
envoyproxy
dns_filter: add support for wildcard name in DNS filter inline table
Commit Message: Add support for wildcard name in DNS filter inline table
Additional Description:
Risk Level: Medium
Testing: I ran only unit tests relevant to dns_filter
Docs Changes: -
Release Notes: Added support wildcard names in :ref:inline_dns_table <envoy_v3_api_field_extensions.filters.udp.dns_filter.v3.DnsFilterConfig.ServerContextConfig.inline_dns_table>.
Platform Specific Features: N/A
Fixes #32941
Hi @StupidScience, welcome and thank you for your contribution.
We will try to review your Pull Request as quickly as possible.
In the meantime, please take a look at the contribution guidelines if you have not done so already.
:cat:
Caused by: https://github.com/envoyproxy/envoy/pull/34588 was opened by StupidScience.
![repokitteh-read-only[bot] avatar](https://avatars.githubusercontent.com/in/16729?v=4 "Published by a pub.dev verified publisher"){kind=small}
Assigning Yanjun for a first-pass review as a code-owner: /assign @yanjunxiang-google
Thanks @StupidScience for working on this! Could you please fix the CI error?
@yanjunxiang-google sorry, original error that is exposed (on screenshot) did not give me a good idea on what's wrong (it was speling) so I thought it was some CI infra failure.
Should be fixed now.
@StupidScience can you update the doc for the virtual_domain api/envoy/data/dns/v3/dns_table.proto please? It needs to clarify semantics for how how wildcards are matched.
/wait
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @wbpcode
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).
:cat:
Caused by: https://github.com/envoyproxy/envoy/pull/34588 was synchronize by StupidScience.
@yanavlasov done
Thanks. The important bit is how many subdomains can be matched by the wildcard. Will *.foo.com match only bar.foo.com or also baz.bar.foo.com ? Can you clarify it in the comment, please?
@yanavlasov thanks for feedback. Added this info as well as info about matching precedence. Can you think of any other info that should be covered?
LGTM from me. Will wait for LGTM from @yanjunxiang-google and then submit
@StupidScience please resolve the merge error.
/wait
@StupidScience Could you please also add a couple of integration test cases here: https://github.com/envoyproxy/envoy/blob/f3ee7c778bce18db1862774d70cd36e019344847/test/extensions/filters/udp/dns_filter/dns_filter_integration_test.cc#L263
It will help show how the wildcard domain name works end-to-end.
@yanjunxiang-google added integration test, please check if that's smth you had in mind?
@yanjunxiang-google added integration test, please check if that's smth you had in mind?
Looks good. Thanks for adding this feature!