Http connect proxy #4358

kdorosh · 2021-02-26T23:14:11Z

Description

Adds support for HTTP Connect upstreams by setting tunneling config on the upstream. This can be used to route to other proxies (such as ones in a DMZ).

Context

Users desired route-level routing ability from envoy to HTTP-Connect enabled proxies for security reasons within their environments.

Checklist:

I included a concise, user-facing changelog (for details, see https://github.com/solo-io/go-utils/tree/master/changelogutils) which references the issue that is resolved.
If I updated APIs (our protos) or helm values, I ran make install-go-tools generated-code to ensure there will be no code diff
I followed guidelines laid out in the Gloo Edge contribution guide
I opened a draft PR or added the work in progress label if my PR is not ready for review
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have added tests that prove my fix is effective or that my feature works
BOT NOTES:
resolves [Migrated] support HTTP/1.1 CONNECT for proxied upstreams solo-io/gloo#3664

solo-changelog-bot · 2021-02-26T23:14:18Z

Issues linked to changelog:
solo-io#3664

…p_connect_proxy

npolshakova · 2021-03-02T19:44:02Z

docs/content/reference/api/github.com/solo-io/gloo/projects/gloo/api/v1/proxy.proto.sk.md

@@ -169,7 +169,7 @@ Note: the destination spec and subsets are not supported in this context and wil
 | `single` | [.gloo.solo.io.Destination](../proxy.proto.sk/#destination) | Use SingleDestination to route to a single upstream. Only one of `single`, `multi`, or `forwardSniClusterName` can be set. |
 | `multi` | [.gloo.solo.io.MultiDestination](../proxy.proto.sk/#multidestination) | Use MultiDestination to load balance requests between multiple upstreams (by weight). Only one of `multi`, `single`, or `forwardSniClusterName` can be set. |
 | `upstreamGroup` | [.core.solo.io.ResourceRef](../../../../../../solo-kit/api/v1/ref.proto.sk/#resourceref) | Use a reference to an upstream group for routing. Only one of `upstreamGroup`, `single`, or `forwardSniClusterName` can be set. |
-| `forwardSniClusterName` | [.google.protobuf.Empty](https://developers.google.com/protocol-buffers/docs/reference/csharp/class/google/protobuf/well-known-types/empty) | Forwards the SNI name into the destination cluster https://www.envoyproxy.io/docs/envoy/latest/api-v2/config/filter/network/sni_cluster/empty/sni_cluster Note: This filter will only work properly with TLS connections in which the upstream SNI domain is specified. Only one of `forwardSniClusterName`, `single`, or `upstreamGroup` can be set. |
+| `forwardSniClusterName` | [.google.protobuf.Empty](https://developers.google.com/protocol-buffers/docs/reference/csharp/class/google/protobuf/well-known-types/empty) | Forwards the request to a cluster name matching the TLS SNI name https://www.envoyproxy.io/docs/envoy/latest/api-v2/config/filter/network/sni_cluster/empty/sni_cluster Note: This filter will only work properly with TLS connections in which the upstream SNI domain is specified. Only one of `forwardSniClusterName`, `single`, or `upstreamGroup` can be set. |


Can forwardSniClusterName and multi be set?

no they cannot be set at the same time https://github.com/solo-io/gloo/blob/245ca47bc977c9c3121f7e7b77d7b5b64bb7702b/projects/gloo/api/v1/proxy.proto#L121-L136

Can we update the comment with that or is it autogenerated? It makes it seem only forwardSniClusterName, single, or upstreamGroup can't be set at the same time.

bizarre, this is auto-generated and appears to be a solo-kit bug: solo-io/solo-kit#407

npolshakova · 2021-03-02T20:03:20Z

projects/gloo/pkg/plugins/tunneling/tunneling.go

+						continue
+					}
+
+					selfCluster := "solo_io_generated_self_cluster_" + cluster


Is it fine hard coding the self cluster name here like this? It's going to be used anywhere else other than updating the route to the generated cluster in the tunneling plugin?

Should be fine to hard-code, so long as there are no collisions or clusters by the same name. It is only used when we update the existing route to route to the new generated cluster.

npolshakova · 2021-03-02T20:05:00Z

projects/gloo/pkg/plugins/tunneling/tunneling.go

+		},
+		ConnectTimeout:  &duration.Duration{Seconds: 5},
+		Name:            selfCluster,
+		TransportSocket: originalTransportSocket,


This looks like it's only copying over the TransportSocket? Should TransportSocketMatches also be copied over?

we only use transport socket matches to allow different SNI for each endpoint in a static upstream https://github.com/solo-io/gloo/blob/ef4e92117a1f20e403bb7ba74fdcb47f7de5138d/projects/gloo/pkg/plugins/static/plugin.go#L153

in this case, the new generated cluster has only one endpoint (the self pipe), so there's no need for transport socket matches to pick which SNI to use with the endpoint (as there's only one)

this does mean the current implementation cannot use different SNI for each endpoint behind our final routing destination, but since the request bytes are encrypted before the endpoint is selected for tunneling I'm not sure there's much we can do to get around that.

further, the TLS context is actually for the tunneling target rather than the endpoints behind the HTTP proxy

projects/gloo/pkg/translator/clusters.go

test/e2e/http_tunneling_test.go

projects/gloo/pkg/translator/utils_test.go

npolshakova

Looks good!

* Fix some comments * Expose some needed APIs * Don't expose low level envoy API * Put new API in right file * Simplify tunneling config * Identify where cluster duplication/modification logic should live * Identify where listener duplication/modification logic should live * Update plugin interface * Begin tunneling plugin * Get e2e working with SSL * Cleanup * Get both e2es working * Cleanup * Improve docs * Cleanup * Fix unit test * Cleanup * Merge branch 'master' into http_connect_proxy * Add changelog * Change to stringvalue * Merge refs/heads/master into http_connect_proxy * Merge refs/heads/master into http_connect_proxy * Fix e2e tests on local; still fail in ci * Merge branch 'http_connect_proxy' of github.com:solo-io/gloo into http_connect_proxy * Check ok * timeout * Merge refs/heads/master into http_connect_proxy * Adding changelog file to new location * Deleting changelog file from old location * connect proxy according to docs * only hijack when done with errors * Cleanup * Merge branch 'master' into http_connect_proxy * Improve comments * Merge refs/heads/master into http_connect_proxy * Add another test * Unfocus test * TCP proxy unit tests * Namespaced generated resources unit test * Cleanup * Fix comment * Add more comments * Merge refs/heads/master into http_connect_proxy * PR comments

* Http connect proxy (#4358) * Fix some comments * Expose some needed APIs * Don't expose low level envoy API * Put new API in right file * Simplify tunneling config * Identify where cluster duplication/modification logic should live * Identify where listener duplication/modification logic should live * Update plugin interface * Begin tunneling plugin * Get e2e working with SSL * Cleanup * Get both e2es working * Cleanup * Improve docs * Cleanup * Fix unit test * Cleanup * Merge branch 'master' into http_connect_proxy * Add changelog * Change to stringvalue * Merge refs/heads/master into http_connect_proxy * Merge refs/heads/master into http_connect_proxy * Fix e2e tests on local; still fail in ci * Merge branch 'http_connect_proxy' of github.com:solo-io/gloo into http_connect_proxy * Check ok * timeout * Merge refs/heads/master into http_connect_proxy * Adding changelog file to new location * Deleting changelog file from old location * connect proxy according to docs * only hijack when done with errors * Cleanup * Merge branch 'master' into http_connect_proxy * Improve comments * Merge refs/heads/master into http_connect_proxy * Add another test * Unfocus test * TCP proxy unit tests * Namespaced generated resources unit test * Cleanup * Fix comment * Add more comments * Merge refs/heads/master into http_connect_proxy * PR comments * Codegen * Move changelog

kdorosh added 19 commits February 24, 2021 15:08

Fix some comments

14c29f6

Expose some needed APIs

54cbfb0

Don't expose low level envoy API

fc10b69

Put new API in right file

70e7eac

Simplify tunneling config

38e2173

Identify where cluster duplication/modification logic should live

c8563bc

Identify where listener duplication/modification logic should live

a7aeff6

Update plugin interface

ceff0e8

Begin tunneling plugin

f5ea636

Get e2e working with SSL

51ccc38

Cleanup

c26b15f

Get both e2es working

fe22dd5

Cleanup

ac722f5

Improve docs

d77b7c3

Cleanup

bdde651

Fix unit test

df77e45

Cleanup

d327ad2

Merge branch 'master' into http_connect_proxy

78448b5

Add changelog

91486ec

github-actions bot added the keep pr updated signals bulldozer to keep pr up to date with base branch label Feb 26, 2021

kdorosh and others added 8 commits February 26, 2021 20:45

Change to stringvalue

dd6a1ed

Merge refs/heads/master into http_connect_proxy

0cf8a9b

Merge refs/heads/master into http_connect_proxy

f6a226e

Fix e2e tests on local; still fail in ci

8ff8f5e

Merge branch 'http_connect_proxy' of github.com:solo-io/gloo into htt…

75bf99b

…p_connect_proxy

Check ok

a946769

timeout

433e2e0

Merge refs/heads/master into http_connect_proxy

c18ff84

kdorosh removed the keep pr updated signals bulldozer to keep pr up to date with base branch label Mar 1, 2021

yuval-k and others added 3 commits March 1, 2021 16:37

only hijack when done with errors

83cf957

Cleanup

ab1c407

Merge branch 'master' into http_connect_proxy

a42eb59

kdorosh added the keep pr updated signals bulldozer to keep pr up to date with base branch label Mar 1, 2021

kdorosh and others added 9 commits March 1, 2021 17:46

Improve comments

0387041

Merge refs/heads/master into http_connect_proxy

3609b47

Add another test

2922689

Unfocus test

dff0c0d

TCP proxy unit tests

1c20a55

Namespaced generated resources unit test

59d065a

Cleanup

3baa2cf

Fix comment

245ca47

Add more comments

27ca2f5

kdorosh marked this pull request as ready for review March 2, 2021 19:35

npolshakova reviewed Mar 2, 2021

View reviewed changes

projects/gloo/pkg/translator/clusters.go Show resolved Hide resolved

npolshakova reviewed Mar 2, 2021

View reviewed changes

projects/gloo/pkg/translator/clusters.go Show resolved Hide resolved

npolshakova reviewed Mar 2, 2021

View reviewed changes

test/e2e/http_tunneling_test.go Outdated Show resolved Hide resolved

npolshakova reviewed Mar 2, 2021

View reviewed changes

projects/gloo/pkg/translator/utils_test.go Show resolved Hide resolved

soloio-bulldozer bot and others added 2 commits March 2, 2021 20:36

Merge refs/heads/master into http_connect_proxy

fdd37d9

PR comments

bb05202

npolshakova approved these changes Mar 2, 2021

View reviewed changes

soloio-bulldozer bot merged commit c5e0df1 into master Mar 2, 2021

soloio-bulldozer bot deleted the http_connect_proxy branch March 2, 2021 22:11

kdorosh mentioned this pull request Mar 2, 2021

Http connect proxy 1.6 #4377

Merged

8 tasks

github-actions bot mentioned this pull request Nov 14, 2024

[Migrated] Http connect proxy solo-io/gloo#4354

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Http connect proxy #4358

Http connect proxy #4358

kdorosh commented Feb 26, 2021 •

edited

Loading

solo-changelog-bot bot commented Feb 26, 2021

npolshakova Mar 2, 2021

kdorosh Mar 2, 2021

npolshakova Mar 2, 2021

kdorosh Mar 2, 2021

npolshakova Mar 2, 2021

kdorosh Mar 2, 2021

npolshakova Mar 2, 2021

kdorosh Mar 2, 2021

kdorosh Mar 2, 2021

npolshakova left a comment

Http connect proxy #4358

Http connect proxy #4358

Conversation

kdorosh commented Feb 26, 2021 • edited Loading

Description

Context

Checklist:

solo-changelog-bot bot commented Feb 26, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

npolshakova left a comment

Choose a reason for hiding this comment

kdorosh commented Feb 26, 2021 •

edited

Loading