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

Support for NO_PROXY #1203

Open
goenning opened this issue Apr 18, 2023 · 7 comments
Open

Support for NO_PROXY #1203

goenning opened this issue Apr 18, 2023 · 7 comments
Labels
client http issues with the client help wanted Not immediately prioritised, please help!

Comments

@goenning
Copy link
Contributor

goenning commented Apr 18, 2023

Would you like to work on this feature?

yes

What problem are you trying to solve?

kube-rs currently supports the HTTP_PROXY (and all its variants) environment variable, but not the NO_PROXY var.

pub fn proxy_url(&self) -> Result<Option<http::Uri>, KubeconfigError> {
let nonempty = |o: Option<String>| o.filter(|s| !s.is_empty());
if let Some(proxy) = nonempty(self.cluster.proxy_url.clone())
.or_else(|| nonempty(std::env::var("HTTP_PROXY").ok()))
.or_else(|| nonempty(std::env::var("http_proxy").ok()))
.or_else(|| nonempty(std::env::var("HTTPS_PROXY").ok()))
.or_else(|| nonempty(std::env::var("https_proxy").ok()))
{
Ok(Some(
proxy
.parse::<http::Uri>()
.map_err(KubeconfigError::ParseProxyUrl)?,
))
} else {
Ok(None)
}
}

The NO_PROXY var is not standardised, but it's supported by Go and I think we could follow with similar support in here.

Describe the solution you'd like

change the proxy_url fn so that if NO_PROXY is set and the content matches the cluster.server url, return None, otherwise continue as usual.

The matching impl could be similar to seanmonstar/reqwest#877

Describe alternatives you've considered

None

Documentation, Adoption, Migration Strategy

N/A

Target crate for feature

kube-client

@goenning
Copy link
Contributor Author

Go's NO_PROXY impl also supports cidr blocks which would require something like the ipnet crate.

@clux clux added help wanted Not immediately prioritised, please help! client http issues with the client labels Apr 18, 2023
@clux
Copy link
Member

clux commented Apr 18, 2023

Seems reasonable to me. PRs welcome.

@goenning
Copy link
Contributor Author

Ok so I spent some time on this and I had a look at reqwest, attohttpc, the no_proxy crate, Go's implementation and that article.

It's a big mess 🙈

The rust implementations above seem to be closer to curl although they all have some minor differences. They all seem to support CIDR (which curl doesn't), but they are missing support for ports. I'm not sure why they decided to ignore ports, maybe because it simplifies the impl?

As for Kube, given the context we're in, IMO we should match Go's implementation as much as possible. Given the no_proxy's crate goal of following the proposed standard from this article, it's unlikely that it'll ever match Go's. And given that Go has had this feature for years, they probably won't be introducing a breaking change either.

I ended up authoring this crate proxyvars with the explicit goal of matching Go's impl, which I'll be using until no_proxy support lands on kube-rs itself.

But before we move on, what are your thoughts on this? Adding either no_proxy or proxyvars to kube would result in something like:

    pub fn proxy_url(&self) -> Result<Option<http::Uri>, KubeconfigError> {
        if let Some(server_url) = &self.cluster.server {
            if let Some(no_proxy) = proxyvars::no_proxy() {
                if no_proxy.matches(server_url) {
                    return Ok(None);
                }
            }
        }

@WeetA34
Copy link

WeetA34 commented May 26, 2023

Hello,
Proxy through environment variables doesn't work for me.
I see that proxy_url is properly detected and set in config but connections to GKE cluster are direct.
Any ideas?
Thank you

edit: I'm using rust 1.69.0 on macOS arm64 with kube 0.82.2, k8s-openapi 0.18.0

@goenning
Copy link
Contributor Author

This package does not actually proxy the connection, you'll need to use something like hyper-proxy for that

@WeetA34
Copy link

WeetA34 commented May 26, 2023

Ok. So, it supports http(s)_proxy environment variables but not proxies calls itself. It’s a bit curious for me 😀 thx

@nobody4t
Copy link

nobody4t commented Jul 4, 2023

Ok. So, it supports http(s)_proxy environment variables but not proxies calls itself. It’s a bit curious for me 😀 thx

So the proxy functionality is not implemented here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client http issues with the client help wanted Not immediately prioritised, please help!
Projects
None yet
Development

No branches or pull requests

4 participants