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

Make Client configurable with Layers #539

Closed
10 tasks done
kazk opened this issue May 24, 2021 · 3 comments · Fixed by #540
Closed
10 tasks done

Make Client configurable with Layers #539

kazk opened this issue May 24, 2021 · 3 comments · Fixed by #540
Assignees
Labels
client http issues with the client

Comments

@kazk
Copy link
Member

kazk commented May 24, 2021

kube::Client now takes Service<http::Request<hyper::Body>>, so users can create a client with custom Service. However, to make this useful beyond simple testing, we need to provide methods to create parts of the default Service (mainly TLS and auth providers).

The goal is to allow full customization at both connector and service levels:

let https = HttpsConnector::from((http, tls));
let connector = TimeoutConnector::new(https, timeout);
let https_client = hyper::Client::builder()
    .build(connector);

let service = tower::ServiceBuilder::new()
    .layer(set_cluster_url)
    .layer(add_authorization)
    .layer(decompression)
    .layer(trace_layer)
    .service(https_client);
let client = kube::Client::new(service);

We should also use layers from tower-http whenever possible.

Overview

TLS

Not having this makes it impossible for custom client to communicate with real clusters.
We currently do the steps described below, and we need to provide a way to create the necessary struct from
kube::Config (step 1).

Native TLS

  1. Create native_tls::TlsConnector from config.identity and config.root_cert (+ config.accept_invalid_certs)
  2. hyper_tls::HttpsConnector::from((http, tls))
    • http is hyper::client::HttpConnector and tls is tokio_native_tls::TlsConnector
    • tokio_native_tls::TlsConnector is created from native_tls::TlsConnector

Note that 1 depends on openssl to create PKCS12 archive for native_tls::Identity.
We should replace that part with pure Rust so openssl isn't necessary on macOS and Windows.
It's currently painful to do CI on Windows.

Rustls

  1. Create rustls::ClientConfig from config.identity and config.root_cert (+ config.accept_invalid_certs)
  2. hyper_rustls::HttpsConnector::from((http, config))
    • http is hyper::client::HttpConnector and config is Arc<rustls::ClientConfig>

Authorization

Authorization with Basic/Bearer are easy to implement, but refreshable token is pretty complex. We should make that public.

Basic

Users can use AddAuthorizationLayer::basic(user, pass) from tower_http.
Or use map_request to add Authorization header.

Bearer

Users can use AddAuthorizationLayer::bearer(token) from tower_http.
Or use map_request to add Authorization header.

Refreshable (exec and oauth)

kube::service::AuthLayer should be renamed and made public.
This layer refreshes the token when necessary before setting the header.

Cluster URL

It's not difficult to implement, but we should provide a convenient way to set cluster URL because it's almost always necessary. SetClusterUrlLayer::new(cluster_url).

SetBaseUrlLayer. Probably general enough to ask to add to tower_http.

Decompression Support

Use DecompressionLayer from tower_http.

Log/Trace

Use TraceLayer from tower_http.

Headers

We should remove headers from Config. Users can use a layer to add them.

TODO


@clux Just summarizing what I have in mind and what needs to be done. Let me know what you think. Hoping to make some time to work on this soon, but if not, I can at least review PRs.

@kazk kazk added the client http issues with the client label May 24, 2021
@clux
Copy link
Member

clux commented May 25, 2021

Yeah. Appreciate you writing this down. It seems like a very sensible plan overall.
Using the standard layers where appropriate seems more sustainable for us.

The SetClusterUrlLayer name is a bit of a mouthful, but then again something like ClusterConfigurationLayer or ConfigLayer is probably a bit too nebulous. Particularly if we move out the headers setting from Config. I thought initially that was something coming out of the kubeconfig (and we had to support it), but looking at it again I guess we just added it to let people inject arbitrary things.

For clarity; are you thinking of splitting the AuthLayer so that the Refreshable part is an optional oauth thing and basic/bearer auth being one or two different layers?

@kazk kazk self-assigned this May 25, 2021
@kazk
Copy link
Member Author

kazk commented May 25, 2021

Cool, I started doing TLS part last night. With this change, we can allow client with both TLS enabled or without. Awkward TLS features in kube-runtime are removed.

When both TLS features are enabled, native_tls is used by default because of #153. rustls can be still used with a custom client. Users will have an option to configure TLS at runtime (e.g., environment variables).
When no TLS features are enabled, TLS setup is simply skipped.

I'll open a PR later today.

For clarity; are you thinking of splitting the AuthLayer so that the Refreshable part is an optional oauth thing and basic/bearer auth being one or two different layers?

AuthLayer is already only responsible for refreshable tokens (oauth and exec). It doesn't handle basic/bearer. Those are handled by inserting headers:

https://github.com/clux/kube-rs/blob/e10dfe8adc5f5f1f64424f95ea132b91bcb92989/kube/src/client/mod.rs#L386-L403

The name is misleading, so it will be renamed.

@clux
Copy link
Member

clux commented Jun 5, 2021

Released in 0.56. Added most of the changes from the main body of the PR as the changelog as this one was huge 💯

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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants