Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dynamic_forward_proxy: Support SRV query lookup in DNS cache used by the custom cluster. #16374

Open
ntgsx92 opened this issue May 7, 2021 · 10 comments
Assignees

Comments

@ntgsx92
Copy link
Contributor

ntgsx92 commented May 7, 2021

Title: Support SRV query lookup in DNS cache used by the custom cluster in DFP filter.

Description:

This feature request is most likely a subset of existing issue for supporting SRV query in Envoy in general. However, it doesn't seem like there's much discussion around supporting it in Dynamic Forward Proxy filter based on the previous discussion. DNS cache used by Dynamic Forward Proxy filter currently performs A record lookup and works similarly to a cluster using Logical DNS for service discovery.

[optional Relevant Links:]

Any extra documentation required to understand the issue.

@ntgsx92 ntgsx92 added enhancement Feature requests. Not bugs or questions. triage Issue requires triage labels May 7, 2021
@Sooryaa-A
Copy link

Sooryaa-A commented May 7, 2021

we too have a use case with DNS srv query support for dynamic fwd proxy filter.

Have observed that even if url is having http , if port is not mentioned in URL, envoy is using default secure port "443" instead fo default http port 80

image

using envoy 1.16.1

@antoniovicente
Copy link
Contributor

cc @mattklein123 @alyssawilk

An option to use DNS SRV when doing dynamic forward proxy seems useful. We may also want an option to fallback to regular DNS records if the SRV lookup fails.

@antoniovicente antoniovicente removed the triage Issue requires triage label May 10, 2021
@alyssawilk
Copy link
Contributor

That'd be fantastic.
I don't think we have anyone who can work on it immediately, but I suspect it'll happen by end of year.
cc @DavidSchinazi who may have thoughts on timline, especially w.r.t QUIC

@DavidSchinazi
Copy link
Contributor

@alyssawilk this issue is about SRV records which only allow you to convey the port number for HTTP or HTTPS. It's a neat feature but not high priority in my mind. The new record that I've been eyeing is the HTTPS record which also conveys HTTP/3 support and TLS client hello encryption keys. But that's not what this issue is about.

@mattklein123 mattklein123 added area/forward proxy help wanted Needs help! and removed enhancement Feature requests. Not bugs or questions. labels May 21, 2021
@Shikugawa
Copy link
Member

@mattklein123 I can take this task if no one working on this.

@mattklein123
Copy link
Member

@mattklein123 I can take this task if no one working on this.

Sure go for it, though I would recommending finishing SRV support in general first. I would love to see that land.

@Shikugawa
Copy link
Member

Shikugawa commented Oct 21, 2021

Finally DNS resolution as extension was merged. We can start to work on SRV record support on c-ares extension.

@ntgsx92
Copy link
Contributor Author

ntgsx92 commented Aug 17, 2022

@Shikugawa I see the #19091 has became stale/closed. Is this something that you're still interested in doing? We have been using c-ares with SRV resolution in our forked envoy and would love upstream the change to the DNS extension if possible. I can pick up the pr if that's okay. Thanks!

@trvll
Copy link

trvll commented May 25, 2023

Any update on that? This feature would be handful on my current project. I would be more than happy to collaborate on that since I have been customizing dynamic forward proxy and DNS cache for different purposes.

@alyssawilk
Copy link
Contributor

alas no, but I'd be very happy to have support for it. I had a partial PR for adding a resolver using res_query, but while I got it to the point it passed manual testing I wasn't confident of parsing the corner cases correctly so never sent it out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants