Feature-flagged support for socks5 proxy in Client

[Razz4780]

Closes #1270

Motivation

kubectl supports using SOCKS5 proxy via proxy-url property in the kubeconfig. In kube-client, this property is only parsed into the Config struct and never used.

Solution

Added hyper-socks2 dependency to use a different HttpConnector when creating the ClientBuilder. Had to move most of the TryFrom<Config> for ClientBuilder implementation into a separate generic function and add generic parameters to some tls-related traits.

New logic is hidden behind the socks5 feature. If the feature is disabled, proxy-url is ignored (as was before this change).

Tested manually on a minikube cluster using this plugin.

[codecov]

Codecov Report

Merging #1311 (954a4bf) into main (4083979) will decrease coverage by 0.2%.
The diff coverage is 61.3%.

Additional details and impacted files

@@           Coverage Diff           @@
##            main   #1311     +/-   ##
=======================================
- Coverage   72.4%   72.1%   -0.2%     
=======================================
  Files         75      75             
  Lines       6352    6377     +25     
=======================================
+ Hits        4595    4597      +2     
- Misses      1757    1780     +23     

Files	Coverage Δ
kube-client/src/client/config_ext.rs	41.7% <100.0%> (-6.4%)	⬇️
kube-client/src/config/mod.rs	45.4% <ø> (ø)
kube-client/src/client/builder.rs	59.0% <60.7%> (-12.5%)	⬇️

... and 1 file with indirect coverage changes

[nightkr]

Haven't looked into the implementation yet, but in theory this is a +1 from me.

New logic is hidden behind the socks5 feature. If the feature is disabled, proxy-url is ignored (as was before this change).

A connection URL doesn't mean the same thing if a proxy is involved, so IMO it's invalid to just ignore the proxy address if the feature is enabled. If we don't support the protocol required to access the service then that should be a hardfail, just like you wouldn't silently downgrade a HTTPS request to HTTP.

[Razz4780]

Haven't looked into the implementation yet, but in theory this is a +1 from me.

New logic is hidden behind the socks5 feature. If the feature is disabled, proxy-url is ignored (as was before this change).

A connection URL doesn't mean the same thing if a proxy is involved, so IMO it's invalid to just ignore the proxy address if the feature is enabled. If we don't support the protocol required to access the service then that should be a hardfail, just like you wouldn't silently downgrade a HTTPS request to HTTP.

That's true, thanks. I wanted to preserve the current behavior in case the new feature is disabled, but probably this is somehting we should check

[clux]

Had a quick look at this. I think in general this looks sensible. While the hyper-socks2 library does not receive a lot of attention, it also is not big.

I am getting some errors actually using it with a server that has proxy though:

2023-10-16T10:51:03.908265Z ERROR kube_client::client::builder: failed with error error trying to connect: tcp connect error: Connection refused (os error 61)
Error: failed to perform initial object list: HyperError: error trying to connect: tcp connect error: Connection refused (os error 61)

Caused by:
    0: HyperError: error trying to connect: tcp connect error: Connection refused (os error 61)
    1: error trying to connect: tcp connect error: Connection refused (os error 61)
    2: tcp connect error: Connection refused (os error 61)
    3: Connection refused (os error 61)

on a config kind of like:

- cluster:
    certificate-authority-data: ...
    proxy-url: socks5://0.0.0.0:3126
    server: https://127.0.0.1:6443
  name: myproxy

which is socks5 proxied over ssh (in ~/.ssh/config):

Host myproxy
  HostName myproxyhost
  User clux
  IdentityFile ~/.ssh/main_id
  DynamicForward 3126

which works with kubectl get pods

haven't dug in too deep yet.

[nightkr]

@clux

By default OpenSSH will only bind DynamicForward to localhost (see DynamicForward, GatewayPorts), so you'd need to set proxy-url: socks5://127.0.0.1:3126.

I'm more surprised that it worked with kubectl...

[clux]

i think that's just 0.0.0.0 as the default route is just 127.0.0.1 for me. either way, the outcome is the same for both kubectl and kube.

[clux]

Ok, apologies, I did some digging and found the reason it is not working; the socks5 feature is not enabled in examples/Cargo.toml that i was using to test... 🤦

Do you mind applying this diff:

diff --git a/examples/Cargo.toml b/examples/Cargo.toml
index 2224fe7..5e95e5c 100644
--- a/examples/Cargo.toml
+++ b/examples/Cargo.toml
@@ -14,11 +14,12 @@ license = "Apache-2.0"
 release = false
 
 [features]
-default = ["rustls-tls", "kubederive", "ws", "latest", "runtime", "refresh"]
+default = ["rustls-tls", "kubederive", "ws", "latest", "socks5", "runtime", "refresh"]
 kubederive = ["kube/derive"]
 openssl-tls = ["kube/client", "kube/openssl-tls"]
 rustls-tls = ["kube/client", "kube/rustls-tls"]
 runtime = ["kube/runtime", "kube/unstable-runtime"]
+socks5 = ["kube/socks5"]
 refresh = ["kube/oauth", "kube/oidc"]
 ws = ["kube/ws"]
 latest = ["k8s-openapi/latest"]

then it's a 👍 from me as well.

[deepu105]

Does this impl also support the http_proxy env vars?

[clux]

currently this only supports socks5.

the http_proxy evars will still be read and used to override the config.proxy_url though, so i imagine it could be difficult to use this feature if you want to support both https_proxy and socks proxy (because we don't check the protocol atm and just send it down to the socks lib where i imagine it will fail).

it probably makes sense long term to add another feature that handles http proxying so we could switch on the protocol scheme. i see there's https://crates.io/crates/hyper-proxy that could perhaps be used, but it's not well maintained (related issue: tafia/hyper-proxy#37).

depending on failure modes encountered for https_proxy with the socks5 feature enabled, it might make sense to add some extra error handling here between that time, or maybe just some scheme matching so we don't enable socks5 proxy in cases where people aren't asking for a socks5:// url?