{"action":"created","issue":{"url":"https://api.github.com/repos/envoyproxy/envoy/issues/4789","repository_url":"https://api.github.com/repos/envoyproxy/envoy","labels_url":"https://api.github.com/repos/envoyproxy/envoy/issues/4789/labels{/name}","comments_url":"https://api.github.com/repos/envoyproxy/envoy/issues/4789/comments","events_url":"https://api.github.com/repos/envoyproxy/envoy/issues/4789/events","html_url":"https://github.com/envoyproxy/envoy/pull/4789","id":371917836,"node_id":"MDExOlB1bGxSZXF1ZXN0MjI0MjU1NjY1","number":4789,"title":"[RFC] Add client CONNECT support. Partial implementation of #1451","user":{"login":"nickolaev","id":2077718,"node_id":"MDQ6VXNlcjIwNzc3MTg=","avatar_url":"https://avatars0.githubusercontent.com/u/2077718?v=4","gravatar_id":"","url":"https://api.github.com/users/nickolaev","html_url":"https://github.com/nickolaev","followers_url":"https://api.github.com/users/nickolaev/followers","following_url":"https://api.github.com/users/nickolaev/following{/other_user}","gists_url":"https://api.github.com/users/nickolaev/gists{/gist_id}","starred_url":"https://api.github.com/users/nickolaev/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nickolaev/subscriptions","organizations_url":"https://api.github.com/users/nickolaev/orgs","repos_url":"https://api.github.com/users/nickolaev/repos","events_url":"https://api.github.com/users/nickolaev/events{/privacy}","received_events_url":"https://api.github.com/users/nickolaev/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":{"login":"alyssawilk","id":18220477,"node_id":"MDQ6VXNlcjE4MjIwNDc3","avatar_url":"https://avatars2.githubusercontent.com/u/18220477?v=4","gravatar_id":"","url":"https://api.github.com/users/alyssawilk","html_url":"https://github.com/alyssawilk","followers_url":"https://api.github.com/users/alyssawilk/followers","following_url":"https://api.github.com/users/alyssawilk/following{/other_user}","gists_url":"https://api.github.com/users/alyssawilk/gists{/gist_id}","starred_url":"https://api.github.com/users/alyssawilk/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/alyssawilk/subscriptions","organizations_url":"https://api.github.com/users/alyssawilk/orgs","repos_url":"https://api.github.com/users/alyssawilk/repos","events_url":"https://api.github.com/users/alyssawilk/events{/privacy}","received_events_url":"https://api.github.com/users/alyssawilk/received_events","type":"User","site_admin":false},"assignees":[{"login":"alyssawilk","id":18220477,"node_id":"MDQ6VXNlcjE4MjIwNDc3","avatar_url":"https://avatars2.githubusercontent.com/u/18220477?v=4","gravatar_id":"","url":"https://api.github.com/users/alyssawilk","html_url":"https://github.com/alyssawilk","followers_url":"https://api.github.com/users/alyssawilk/followers","following_url":"https://api.github.com/users/alyssawilk/following{/other_user}","gists_url":"https://api.github.com/users/alyssawilk/gists{/gist_id}","starred_url":"https://api.github.com/users/alyssawilk/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/alyssawilk/subscriptions","organizations_url":"https://api.github.com/users/alyssawilk/orgs","repos_url":"https://api.github.com/users/alyssawilk/repos","events_url":"https://api.github.com/users/alyssawilk/events{/privacy}","received_events_url":"https://api.github.com/users/alyssawilk/received_events","type":"User","site_admin":false}],"milestone":null,"comments":7,"created_at":"2018-10-19T11:30:13Z","updated_at":"2018-10-23T14:07:55Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/envoyproxy/envoy/pulls/4789","html_url":"https://github.com/envoyproxy/envoy/pull/4789","diff_url":"https://github.com/envoyproxy/envoy/pull/4789.diff","patch_url":"https://github.com/envoyproxy/envoy/pull/4789.patch"},"body":"The implementation is based on the old Web Socket support, which is proposed to\r\nbe removed in PR#4745. Refactor WebSocketProxyCallbacks and the tunnel counterparts\r\nin a common interface and replace it everywhere.\r\n\r\nTunnelHandlerImpl is hooked directly in to the connection manager and CONNECT tunnels are\r\ncreated accordingly.\r\n\r\nThe tunnel implementation assumes the upstream is the origin and always sends 200.\r\n\r\nTesting done: build the envoy proxy, add backend echo server, create a CONNECT tunnel,\r\nverify we get 200 on CONNECT and we get the exact same paylond in the response as in the\r\nrequest.\r\n\r\n*Description*:\r\nThe purpose of this PR is to verify with the wilder community,. our approach to implementing the accept side of CONNECT as described in #1451. \r\nFeedback and discussion is expected.\r\n\r\n*Risk Level*: Medium - we touched core functionality.\r\n*Testing*:\r\nWe run this implementation in our internal environment to verify the new CONNECT functionality. Also run the integrated testing to confirm we did not break anything.\r\n*Docs Changes*:\r\n*Release Notes*:\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/envoyproxy/envoy/issues/comments/432261805","html_url":"https://github.com/envoyproxy/envoy/pull/4789#issuecomment-432261805","issue_url":"https://api.github.com/repos/envoyproxy/envoy/issues/4789","id":432261805,"node_id":"MDEyOklzc3VlQ29tbWVudDQzMjI2MTgwNQ==","user":{"login":"georgi-d","id":11193886,"node_id":"MDQ6VXNlcjExMTkzODg2","avatar_url":"https://avatars3.githubusercontent.com/u/11193886?v=4","gravatar_id":"","url":"https://api.github.com/users/georgi-d","html_url":"https://github.com/georgi-d","followers_url":"https://api.github.com/users/georgi-d/followers","following_url":"https://api.github.com/users/georgi-d/following{/other_user}","gists_url":"https://api.github.com/users/georgi-d/gists{/gist_id}","starred_url":"https://api.github.com/users/georgi-d/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/georgi-d/subscriptions","organizations_url":"https://api.github.com/users/georgi-d/orgs","repos_url":"https://api.github.com/users/georgi-d/repos","events_url":"https://api.github.com/users/georgi-d/events{/privacy}","received_events_url":"https://api.github.com/users/georgi-d/received_events","type":"User","site_admin":false},"created_at":"2018-10-23T14:07:55Z","updated_at":"2018-10-23T14:07:55Z","author_association":"CONTRIBUTOR","body":"> > (Functionality) CONNECT requests can have different behavior. They can either be directly terminated at the upstream or they can be forwarded to the upstream. This needs to be configurable on per route bases and not generally for the whole connection manager.\r\n> \r\n> What is the difference here? It's not completely clear to me.\r\n\r\n@mattklein123 Difference in what? The difference in behavior is that when Envoy receives a CONNECT request there are three ways to handle it:\r\n1) reject the request (this should be the default for security reasons)\r\n2) tunnel to the upstream\r\n- tcp connect to the upstream\r\n- return 200 OK to the downstream\r\n- tcp_tunnel between downstream and upstream\r\n3) forward to upstream\r\n- forward the CONNECT request to the upstream \r\n- wait for upstream to return 200 OK\r\n- forward the 200 OK to downstream\r\n- tcp_tunnel between downstream and upstream\r\n\r\n> \r\n> > If we need to keep something like this for pure tunnels so be it. I don't really see a cleaner way to do it unless @mattklein123 has any clever ideas.\r\n> \r\n> I would need to sit down and look at all the code again to have a fully formed opinion, but it seems like it should still be possible to use most of the \"new style upgrade\" code even if we have different configurations and override certain portions of it? E.g., skip upstream rejection, etc. and just go straight to the tunnel based on config? It would be nice to not keep the old code path just for this case, since at a high level it seems like the new code path basically does what we want?\r\n\r\nImplementation wise the new code path most likely does what we need. (I have to check in more detail). I am mostly saying that we need fine grained control of the behavior.\r\n"}}
{"actor":{"display_login":"georgi-d"}}