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

Add http proxy support for exec/port-forward #11694

Merged
merged 6 commits into from
Oct 15, 2015

Conversation

ncdc
Copy link
Member

@ncdc ncdc commented Jul 22, 2015

Add http proxy support for exec/port-forward in SpdyRoundTripper

@ncdc
Copy link
Member Author

ncdc commented Jul 22, 2015

@smarterclayton


// proxying logic adapted from http://blog.h6t.eu/post/74098062923/golang-websocket-with-http-proxy-support
proxyReq := http.Request{
Method: "CONNECT",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if it's appropriate to do a CONNECT tunnel for regular http traffic. If not, what's the right way to do it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doing a quick read I think so

@k8s-bot
Copy link

k8s-bot commented Jul 22, 2015

GCE e2e build/test failed for commit 60fe71a049c49129e03fc23a9c2ce4e40af724d3.

@smarterclayton
Copy link
Contributor

Needs a unit test somehow... no idea how to do that easily though.

@smarterclayton
Copy link
Contributor

http://www.infoq.com/articles/Web-Sockets-Proxy-Servers seems to cover this for WS

@ncdc
Copy link
Member Author

ncdc commented Jul 24, 2015

@smarterclayton I'm not sure how you'd unit test it, since everything would be running on the same host. But I agree some sort of way to test is highly desirable.

@smarterclayton
Copy link
Contributor

You'd need a connect level proxy - which you could run in a container
during e2e

On Fri, Jul 24, 2015 at 1:18 PM, Andy Goldstein notifications@github.com
wrote:

@smarterclayton https://github.com/smarterclayton I'm not sure how
you'd unit test it, since everything would be running on the same host. But
I agree some sort of way to test is highly desirable.


Reply to this email directly or view it on GitHub
#11694 (comment)
.

Clayton Coleman | Lead Engineer, OpenShift

@ncdc
Copy link
Member Author

ncdc commented Aug 25, 2015

I'm thinking that the way to e2e test this is to do the following:

  1. Create a pod with a proxy server such as squid in it
  2. Create a pod with kubectl in it
  3. Configure the kubectl container such that it's only allowed to talk to the squid pod (or a service fronting it)
  4. Have the kubectl container run http_proxy=$squid kubectl -s $apiServer exec $somePod $someCommand

Questions:

  1. Do you think this will work? Is there an easier way?
  2. Can we use iptables to restrict the kubectl container's outgoing packets?
    1. Is it safe to assume iptables will work inside a container wherever e2e is running?
    2. This means the kubectl container will need to be privileged - is that ok?
    3. Is there some other way to do this?
  3. Ideally I'd like to test http_proxy and https_proxy - can we test both http and https with e2e?

@smarterclayton @thockin @bgrant0607 @brendandburns

@ncdc
Copy link
Member Author

ncdc commented Aug 25, 2015

@kubernetes/rh-cluster-infra @kubernetes/rh-networking

@bgrant0607
Copy link
Member

cc @lavalamp

@bgrant0607
Copy link
Member

Why do you want to restrict packets?

@ncdc
Copy link
Member Author

ncdc commented Aug 26, 2015

I'd like to prove without a doubt that proxying is working. If we can't restrict packets, I could write a test that runs http_proxy=... kubectl exec ... and succeeds with the current code, which doesn't support proxying.

@thockin
Copy link
Member

thockin commented Aug 26, 2015

regarding iptables - it should work inside containers, but it does require
privileges.

On Wed, Aug 26, 2015 at 6:48 AM, Andy Goldstein notifications@github.com
wrote:

I'd like to prove without a doubt that proxying is working. If we can't
restrict packets, I could write a test that runs http_proxy=... kubectl
exec ... and succeeds with the current code, which doesn't support
proxying.


Reply to this email directly or view it on GitHub
#11694 (comment)
.

@ncdc
Copy link
Member Author

ncdc commented Aug 26, 2015

regarding iptables - it should work inside containers, but it does require
privileges.

Yes, it seems somewhat backwards to require a privileged container to be able to test that proxying is working, but that's all I've been able to come up with so far...

@mvdan
Copy link
Contributor

mvdan commented Aug 26, 2015

@ncdc could you elaborate? The intended behaviour of the code in portforward is the same, with the only difference that the old one was broken.

@ncdc
Copy link
Member Author

ncdc commented Aug 26, 2015

@mvdan sorry that comment was meant for the go1.5 race pr

@lavalamp
Copy link
Member

Why not just examine the kubectl log to verify that all traffic went through its proxy?

@ncdc
Copy link
Member Author

ncdc commented Aug 26, 2015

@lavalamp good idea

@ncdc
Copy link
Member Author

ncdc commented Aug 26, 2015

Do you all have a preference for what image to use for the proxy? Something with squid or polipo in it?

@ncdc
Copy link
Member Author

ncdc commented Aug 26, 2015

Actually I'll try nginx since I know e2e already uses that

@ncdc
Copy link
Member Author

ncdc commented Aug 26, 2015

Looks like nginx doesn't support CONNECT when acting as a forward proxy. That leaves me with squid, polipo, ...

@ncdc ncdc force-pushed the add-spdy-proxy-support branch from 60fe71a to 43f4590 Compare August 27, 2015 14:18
@k8s-bot
Copy link

k8s-bot commented Aug 27, 2015

GCE e2e build/test failed for commit 43f4590318ec10fbd9cd47f7364db6751f7d4ba3.

@k8s-bot
Copy link

k8s-bot commented Aug 27, 2015

GCE e2e build/test passed for commit b9290c46be53cde9eb9611bdde0e7ca3d1ad67aa.

@ncdc
Copy link
Member Author

ncdc commented Aug 27, 2015

If we run the forward proxy as a pod, and we run kubectl exec from outside the cluster, there's no easy way for kubectl to use the proxy pod as its http_proxy, since the pod's IP isn't routable from outside the cluster.

We could try to find a way to run kubectl inside the cluster, but that requires that kubectl be built locally and then somehow run in the cluster, perhaps in a custom image that's built on the fly, pushed to a publicly accessible registry, and then run as a pod. I don't like that idea because of the need to build and push a kubectl image somewhere.

Another option would be to run the proxy pod with a hostPort, perhaps, and have kubectl connect to that.

Does anyone have any thoughts about how to put together an e2e test where kubectl uses an http proxy to talk to the apiserver?

@smarterclayton
Copy link
Contributor

Run kubectl in a pod

On Aug 27, 2015, at 5:26 PM, Andy Goldstein notifications@github.com
wrote:

If we run the forward proxy as a pod, and we run kubectl exec from outside
the cluster, there's no easy way for kubectl to use the proxy pod as its
http_proxy, since the pod's IP isn't routable from outside the cluster.

We could try to find a way to run kubectl inside the cluster, but that
requires that kubectl be built locally and then somehow run in the cluster,
perhaps in a custom image that's built on the fly, pushed to a publicly
accessible registry, and then run as a pod. I don't like that idea because
of the need to build and push a kubectl image somewhere.

Another option would be to run the proxy pod with a hostPort, perhaps, and
have kubectl connect to that.

Does anyone have any thoughts about how to put together an e2e test where
kubectl uses an http proxy to talk to the apiserver?


Reply to this email directly or view it on GitHub
#11694 (comment)
.

@k8s-github-robot k8s-github-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 27, 2015
@ncdc
Copy link
Member Author

ncdc commented Oct 13, 2015

@krousey not fixed yet, @ashcrow is working on something else at the moment, but we'll come back to it soon.

@krousey
Copy link
Contributor

krousey commented Oct 13, 2015

@ncdc Thanks. I mainly wanted to get that status in the main comment stream so that I don't get confused again and again 😄

@k8s-bot
Copy link

k8s-bot commented Oct 13, 2015

GCE e2e build/test failed for commit cd497a6f5768a1b3253740261d7bd3128b080abc.

@ashcrow
Copy link
Contributor

ashcrow commented Oct 13, 2015

@krousey jumping back over now 😄

* release tar now includes test/images/*
* kubectl is now built as a static binary in the test
@ashcrow ashcrow force-pushed the add-spdy-proxy-support branch from cd497a6 to 57fc4bf Compare October 14, 2015 13:43
@k8s-bot
Copy link

k8s-bot commented Oct 14, 2015

GCE e2e test build/test passed for commit 57fc4bf.

@ashcrow
Copy link
Contributor

ashcrow commented Oct 14, 2015

@k8s-teamcity-mesosphere failure seems like a flake

[13:54:14][Step 4/4] Error response from daemon: Cannot start container 49b2e6b7cb245420fc0b4b270842a38f781329fabbdb123d1afcac79b12de43c: Cannot link to a non running container: /docker_apiserver_1 AS /prickly_bose/apiserver
[13:54:14][Step 4/4] Dumping logs to '/tmp/kubernetes/log'
[13:54:15][Step 4/4] + timeout 5m ./cluster/kube-down.sh
[13:54:15][Step 4/4]  [1BBringing down cluster using provider: mesos/docker
[13:54:15][Step 4/4] Verifying required commands
[13:54:15][Step 4/4] Stopping mesos/docker cluster

@ashcrow
Copy link
Contributor

ashcrow commented Oct 14, 2015

Shippable also looks like a flake. One of them passed in 40 minutes and the other failed in 2 hours.

@ashcrow
Copy link
Contributor

ashcrow commented Oct 14, 2015

@krousey mind taking a look now that this no longer forces a static kubectl for everything?

@krousey krousey added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 14, 2015
@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented Oct 15, 2015

GCE e2e build/test failed for commit 57fc4bf.

@ashcrow
Copy link
Contributor

ashcrow commented Oct 15, 2015

Unrelated Jenkins e2e error:

• Failure [94.077 seconds]
EmptyDir volumes
/go/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/test/e2e/empty_dir.go:95
  should support (root,0644,tmpfs) [Conformance] [It]
  /go/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/test/e2e/empty_dir.go:46

  Oct 14 21:49:39.061: Failed to create pod: Pod "pod-243d789a-72f8-11e5-bdf9-42010af0226b" is forbidden: no API token found for service account e2e-tests-emptydir-vjc6c/default, retry after the token is automatically created and added to the service account

  /go/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/test/e2e/util.go:1112

@wojtek-t
Copy link
Member

@k8s-bot test this please

@k8s-bot
Copy link

k8s-bot commented Oct 15, 2015

GCE e2e test build/test passed for commit 57fc4bf.

@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented Oct 15, 2015

GCE e2e test build/test passed for commit 57fc4bf.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

k8s-github-robot pushed a commit that referenced this pull request Oct 15, 2015
@k8s-github-robot k8s-github-robot merged commit a3f2ba2 into kubernetes:master Oct 15, 2015
j3ffml added a commit that referenced this pull request Nov 18, 2015
@ncdc ncdc deleted the add-spdy-proxy-support branch February 13, 2017 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.