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 kubectl over socks5 proxy support #1

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

drastawi
Copy link

@drastawi drastawi commented Jan 15, 2019

What type of PR is this?
/kind bug

What this PR does / why we need it:

The PR allows fo call kubectl commands like exec over socks5 proxy.

Which issue(s) this PR fixes:
Fixes #kubernetes#58065

Special notes for your reviewer:

The pr will only impact only proxies with scheme socks5. The proxy connections with http, https are unchanged.

Tests are not implemented, will need help with mocking the socks 5 setup.

Does this PR introduce a user-facing change?:

Allow to run kubectl over socks5 proxy

@drastawi drastawi requested a review from eladidan January 16, 2019 00:21
Copy link

@eladidan eladidan left a comment

Choose a reason for hiding this comment

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

@drastawi did you test somehow that this still works with http/https proxy (manually)?

We can run minikube + tinyproxy (in a docker container), and verify this works for both http and https proxies. Happy to help with setup

@@ -129,6 +130,19 @@ func (s *SpdyRoundTripper) dial(req *http.Request) (net.Conn, error) {
return s.dialWithoutProxy(req.Context(), req.URL)
}

if proxyURL.Scheme == "socks5" {

Choose a reason for hiding this comment

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

use switch with a fallthrough for http, and https, and error for default

@drastawi
Copy link
Author

}

// dialWithoutProxy dials the host specified by url through an http or an https proxy.
func (s *SpdyRoundTripper) dialWithHttpProxy(requestUrl *url.URL, proxyURL *url.URL) (net.Conn, error) {

Choose a reason for hiding this comment

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

The name of the function and the name in the comment do not match.

}

// dialWithoutProxy dials the host specified by url through a socks5 proxy.
func (s *SpdyRoundTripper) dialWithSocks5Proxy(requestUrl *url.URL, proxyURL *url.URL) (net.Conn, error) {

Choose a reason for hiding this comment

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

Function name and comment do not match.

@kplimack
Copy link

@drastawi are you planning on getting this PR finished? I'm also trying to make use of a proxy for kubectl and look forward to seeing this get merged

@drastawi
Copy link
Author

@kplimack the pr already works. You can build it from the first commit 2a9bbac. I'll try to merge it upstream eventually but the conversation there is a bit slow.

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

Successfully merging this pull request may close these issues.

4 participants