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

Support generic outbound proxy #1606

Closed
jmillikin-stripe opened this issue Sep 7, 2017 · 38 comments · Fixed by #7565
Closed

Support generic outbound proxy #1606

jmillikin-stripe opened this issue Sep 7, 2017 · 38 comments · Fixed by #7565
Assignees
Labels
enhancement Feature requests. Not bugs or questions. no stalebot Disables stalebot from closing an issue
Milestone

Comments

@jmillikin-stripe
Copy link
Contributor

jmillikin-stripe commented Sep 7, 2017

When using Envoy as a general egress proxy, there doesn't seem to be a way to have http_connection_manager routes send to a cluster and have the :authority header be used to select the destination. I expected a cluster with type: ORIGINAL_DST lb_policy: ORIGINAL_DST_LB to do this, but it seems original_dst is actually something else (iptables level?).

http_connection_manager supports routing to different clusters based on a header (RouteAction::cluster_header), but we can't plumb this to the cluster level unless each hostname somehow dynamically generated a cluster.

I'm not sure what to do about DNS in this case -- the process behind Envoy can't do the resolution because it doesn't have a network, but having Envoy resolve would violate the documented behavior "Envoy never synchronously resolves DNS in the forwarding path". Presumably ORIGINAL_DST doesn't worry about this if its forwarding is IP-level.

@mattklein123 mattklein123 added the enhancement Feature requests. Not bugs or questions. label Sep 7, 2017
@mattklein123
Copy link
Member

mattklein123 commented Sep 7, 2017

@jmillikin-stripe I think you are asking for the feature discussed in this thread? https://groups.google.com/forum/#!topic/envoy-dev/xVvy3Q26VNM

@jmillikin-stripe
Copy link
Contributor Author

Yes, it looks like that thread is about pretty much the same feature. I see the DNS resolution issue was already being considered. As to your many-cluster-vs-single-cluster question there, we definitely want (2). Not just for simplicity of implementation, but also because it prevents unbounded growth of metric tags.

@mattklein123
Copy link
Member

OK. I meant to open an issue to track that thread so here we are. This feature is not easy to add but I think will be useful. I will mark this as "help wanted." If someone wants to work on it please let me know and we can go into the details.

@mattklein123 mattklein123 added the help wanted Needs help! label Sep 7, 2017
@mattklein123 mattklein123 changed the title Support something like ORIGINAL_DST but based on the :authority header Support generic outbound proxy Oct 31, 2017
@mattklein123
Copy link
Member

Retitled from: Support something like ORIGINAL_DST but based on the :authority header

@mattklein123
Copy link
Member

FYI unless this somehow gets picked up soon I may do this as my "holiday programming project."

@mattklein123 mattklein123 added this to the 1.6.0 milestone Nov 26, 2017
@mattklein123 mattklein123 removed the help wanted Needs help! label Nov 26, 2017
@mattklein123 mattklein123 self-assigned this Nov 26, 2017
@jippi
Copy link

jippi commented Dec 22, 2017

This would be amazing feature to have

@mattklein123 mattklein123 removed this from the 1.6.0 milestone Dec 31, 2017
@mattklein123 mattklein123 added this to the 1.6.0 milestone Feb 2, 2018
@mattklein123
Copy link
Member

Enough people have asked for this (including at Lyft) that I'm going to implement this. My rough plan is a trio of HTTP filter, cluster, and load balancer that all work together. Roughly:

  1. The filter will work similarly to what I described in Lazy loading (on demand) for Routes, Clusters and Endpoints #2500 (comment) with request holding, but instead will look in a DNS cache. If a request needs to be held while initial DNS resolution is taking place it will be done.
  2. The cluster will by very similar to logical_dns in operation. Perhaps it will be called "inline_dns" or something.
  3. The special load balancer will use :authority in the load balancer context to select the right host which should be available after being cached in (1).

So the idea is that the user will have to install all three things (filter, cluster, load balancer type) together to get this functionality. At face value, this sounds convoluted, but IMO it's simpler than the alternative which basically involves making load balancer host selection asynchronous. The repercussions from that are wide and IMO will make the rest of the code substantially more complicated and harder to reason about vs. what I'm proposing here. I may also look into this being turned on via a router config flag and then implicitly installing a new filter prior to the router filter on behalf of the user. I will look into this.

I can't promise when I will deliver this but I will work on it in my "spare time" over next couple of months.

cc @rshriram

@mattklein123
Copy link
Member

@rshriram from Istio perspective, how will cluster configuration work? E.g., things like HTTP/1 vs. HTTP/2, etc.? It seems like for HTTP stuff in general we would have to use HTTP/1 in most cases, until we eventually implement upgrade and/or potentially using ALPN for TLS connections (with potentially some automated way to turn on SAN checking based on authority).

@rshriram
Copy link
Member

rshriram commented Feb 4, 2018

Istio use case is primarily for the egress proxy, where all external traffic out of the mesh (e.g., consuming AWS apis) has to transit through a dedicated proxy node. H1 should suffice for the near term.

But we have a need for TCP connections as well. I think the TCP is going to require some funny setup (Will have to look into the CONNECT semantics?).

@mattklein123
Copy link
Member

Yeah for TCP the only way to make this sanely work would be via CONNECT, which will be a separate work item, tracked here: #1451

I will think through the CONNECT case fully to make sure that will be supported. My rough intuition is to potentially handle CONNECT in a new filter that would run before the router but after the "inline_DNS" filter.

@stale
Copy link

stale bot commented Jun 28, 2018

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 other activity occurs. Thank you for your contributions.

@stale stale bot added the stale stalebot believes this issue/PR has not been touched recently label Jun 28, 2018
@mattklein123 mattklein123 added the help wanted Needs help! label Jun 28, 2018
@stale stale bot removed the stale stalebot believes this issue/PR has not been touched recently label Jun 28, 2018
@mattklein123
Copy link
Member

Marking help wanted to avoid auto close. I still plan on working on this, not sure when though in case someone wants to do this before me.

mattklein123 added a commit that referenced this issue Jun 17, 2019
This allows using Envoy as a generic HTTP proxy without any
prior configuration of DNS targets. See the included documentation
for more information.

Part of #1606

Signed-off-by: Matt Klein <[email protected]>
@mattklein123
Copy link
Member

mattklein123 commented Jun 18, 2019

Initial PR out. Tracking additional work items here to call this issue done for v1:

  • Configurable DNS cache circuit breakers and maximums
  • Stats
  • <IP>:<port> host names
  • Show host name in admin output
  • Dynamic per-host TLS, at least for SNI and verifying subject alt name

@lizan @alyssawilk and others lmk if I'm missing anything.

mattklein123 added a commit that referenced this issue Jun 20, 2019
This allows using Envoy as a generic HTTP proxy without any
prior configuration of DNS targets. See the included documentation
for more information.

Part of #1606

Signed-off-by: Matt Klein <[email protected]>
mattklein123 pushed a commit to envoyproxy/data-plane-api that referenced this issue Jun 20, 2019
This allows using Envoy as a generic HTTP proxy without any
prior configuration of DNS targets. See the included documentation
for more information.

Part of envoyproxy/envoy#1606

Signed-off-by: Matt Klein <[email protected]>

Mirrored from https://github.com/envoyproxy/envoy @ 79e53f21c680b4a02695b1761e7d91868c3481d4
mattklein123 added a commit that referenced this issue Jun 21, 2019
mattklein123 added a commit that referenced this issue Jun 21, 2019
lizan pushed a commit that referenced this issue Jun 24, 2019
Part of #1606

Risk Level: Low
Testing: Modified UT
Docs Changes: N/A
Release Notes: Added

Signed-off-by: Matt Klein <[email protected]>
mattklein123 pushed a commit to envoyproxy/data-plane-api that referenced this issue Jun 24, 2019
Part of envoyproxy/envoy#1606

Risk Level: Low
Testing: Modified UT
Docs Changes: N/A
Release Notes: Added

Signed-off-by: Matt Klein <[email protected]>

Mirrored from https://github.com/envoyproxy/envoy @ 7816d5e4baa1dee7558572393c83fa1d96a5052a
mattklein123 added a commit that referenced this issue Jun 26, 2019
mattklein123 pushed a commit to envoyproxy/data-plane-api that referenced this issue Jun 26, 2019
Part of envoyproxy/envoy#1606

Signed-off-by: Matt Klein <[email protected]>

Mirrored from https://github.com/envoyproxy/envoy @ a659c5ecfd93a9d3a6580cbc968aa3fcca8eb480
yxue pushed a commit to yxue/envoy that referenced this issue Jun 26, 2019
Part of envoyproxy#1606

Risk Level: Low
Testing: Modified UT
Docs Changes: N/A
Release Notes: Added

Signed-off-by: Matt Klein <[email protected]>
yxue pushed a commit to yxue/envoy that referenced this issue Jun 26, 2019
Part of envoyproxy#1606

Risk Level: Low
Testing: Modified UT
Docs Changes: N/A
Release Notes: Added

Signed-off-by: Matt Klein <[email protected]>
mattklein123 added a commit that referenced this issue Jul 3, 2019
1) Implement auto SAN/SNI setting on a per-host basis.
2) Cleanup some v1 config translation.

Part of #1606

Signed-off-by: Matt Klein <[email protected]>
@alyssawilk alyssawilk modified the milestones: 1.11.0, 1.12.0 Jul 8, 2019
@ravikumarkgit
Copy link

ravikumarkgit commented Jul 9, 2019

Any updates on this @mattklein123 @davidben @lizan @PiotrSikora
i am looking for the same kind of implementation, pls.. direct me here.

anything on this space (envoy as generic forward proxy) is very helpful.

@mattklein123
Copy link
Member

@davidben @lizan @PiotrSikora I have a question about IP certs which I'm looking to handle in my final PR for this issue:

[alt_names]
DNS.1 = *.lyft.com
IP.1 = 0.0.0.0
IP.2 = ::

I assume an IP cert would be issued using an IP alt name, right?

In looking at our verification code I don't think we are checking for SAN types of GEN_IPADD, so I would need to add that. Is that right? Thank you.

@PiotrSikora
Copy link
Contributor

@mattklein123 correct.

mattklein123 added a commit that referenced this issue Jul 12, 2019
1) Implement auto SAN/SNI setting on a per-host basis.
2) Cleanup some v1 config translation.

Part of #1606

Signed-off-by: Matt Klein <[email protected]>
mattklein123 added a commit that referenced this issue Jul 12, 2019
Support IP addresses in host headers.

This completes the MVP of dynamic forward proxy.

Fixes #1606

Signed-off-by: Matt Klein <[email protected]>
mattklein123 added a commit that referenced this issue Jul 22, 2019
Support IP addresses in host headers.

This completes the MVP of dynamic forward proxy.

Fixes #1606

Signed-off-by: Matt Klein <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature requests. Not bugs or questions. no stalebot Disables stalebot from closing an issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.