Skip to content

Commit

Permalink
Always use http::Uri instead of url::Url
Browse files Browse the repository at this point in the history
  • Loading branch information
kazk committed May 27, 2021
1 parent 77437d5 commit 0a570c5
Show file tree
Hide file tree
Showing 8 changed files with 57 additions and 61 deletions.
2 changes: 1 addition & 1 deletion kube-core/Cargo.toml
Expand Up @@ -21,7 +21,7 @@ serde = { version = "1.0.118", features = ["derive"] }
serde_json = "1.0.61"
thiserror = "1.0.23"
once_cell = "1.7.2"
url = "2.2.0"
form_urlencoded = "1.0.1"
http = "0.2.2"
json-patch = { version = "0.2.6", optional = true }

Expand Down
2 changes: 1 addition & 1 deletion kube-core/src/params.rs
Expand Up @@ -287,7 +287,7 @@ impl PatchParams {
Ok(())
}

pub(crate) fn populate_qp(&self, qp: &mut url::form_urlencoded::Serializer<String>) {
pub(crate) fn populate_qp(&self, qp: &mut form_urlencoded::Serializer<String>) {
if self.dry_run {
qp.append_pair("dryRun", "All");
}
Expand Down
22 changes: 11 additions & 11 deletions kube-core/src/request.rs
Expand Up @@ -27,7 +27,7 @@ impl Request {
/// List a collection of a resource
pub fn list(&self, lp: &ListParams) -> Result<http::Request<Vec<u8>>> {
let target = format!("{}?", self.url_path);
let mut qp = url::form_urlencoded::Serializer::new(target);
let mut qp = form_urlencoded::Serializer::new(target);

if let Some(fields) = &lp.field_selector {
qp.append_pair("fieldSelector", &fields);
Expand All @@ -50,7 +50,7 @@ impl Request {
/// Watch a resource at a given version
pub fn watch(&self, lp: &ListParams, ver: &str) -> Result<http::Request<Vec<u8>>> {
let target = format!("{}?", self.url_path);
let mut qp = url::form_urlencoded::Serializer::new(target);
let mut qp = form_urlencoded::Serializer::new(target);
lp.validate()?;
if lp.limit.is_some() {
return Err(Error::RequestValidation(
Expand Down Expand Up @@ -86,7 +86,7 @@ impl Request {
/// Get a single instance
pub fn get(&self, name: &str) -> Result<http::Request<Vec<u8>>> {
let target = format!("{}/{}", self.url_path, name);
let mut qp = url::form_urlencoded::Serializer::new(target);
let mut qp = form_urlencoded::Serializer::new(target);
let urlstr = qp.finish();
let req = http::Request::get(urlstr);
req.body(vec![]).map_err(Error::HttpError)
Expand All @@ -96,7 +96,7 @@ impl Request {
pub fn create(&self, pp: &PostParams, data: Vec<u8>) -> Result<http::Request<Vec<u8>>> {
pp.validate()?;
let target = format!("{}?", self.url_path);
let mut qp = url::form_urlencoded::Serializer::new(target);
let mut qp = form_urlencoded::Serializer::new(target);
if pp.dry_run {
qp.append_pair("dryRun", "All");
}
Expand All @@ -108,7 +108,7 @@ impl Request {
/// Delete an instance of a resource
pub fn delete(&self, name: &str, dp: &DeleteParams) -> Result<http::Request<Vec<u8>>> {
let target = format!("{}/{}?", self.url_path, name);
let mut qp = url::form_urlencoded::Serializer::new(target);
let mut qp = form_urlencoded::Serializer::new(target);
let urlstr = qp.finish();
let body = serde_json::to_vec(&dp)?;
let req = http::Request::delete(urlstr);
Expand All @@ -118,7 +118,7 @@ impl Request {
/// Delete a collection of a resource
pub fn delete_collection(&self, dp: &DeleteParams, lp: &ListParams) -> Result<http::Request<Vec<u8>>> {
let target = format!("{}?", self.url_path);
let mut qp = url::form_urlencoded::Serializer::new(target);
let mut qp = form_urlencoded::Serializer::new(target);
if let Some(fields) = &lp.field_selector {
qp.append_pair("fieldSelector", &fields);
}
Expand All @@ -142,7 +142,7 @@ impl Request {
) -> Result<http::Request<Vec<u8>>> {
pp.validate(patch)?;
let target = format!("{}/{}?", self.url_path, name);
let mut qp = url::form_urlencoded::Serializer::new(target);
let mut qp = form_urlencoded::Serializer::new(target);
pp.populate_qp(&mut qp);
let urlstr = qp.finish();

Expand All @@ -158,7 +158,7 @@ impl Request {
/// Requires `metadata.resourceVersion` set in data
pub fn replace(&self, name: &str, pp: &PostParams, data: Vec<u8>) -> Result<http::Request<Vec<u8>>> {
let target = format!("{}/{}?", self.url_path, name);
let mut qp = url::form_urlencoded::Serializer::new(target);
let mut qp = form_urlencoded::Serializer::new(target);
if pp.dry_run {
qp.append_pair("dryRun", "All");
}
Expand All @@ -173,7 +173,7 @@ impl Request {
/// Get an instance of the subresource
pub fn get_subresource(&self, subresource_name: &str, name: &str) -> Result<http::Request<Vec<u8>>> {
let target = format!("{}/{}/{}", self.url_path, name, subresource_name);
let mut qp = url::form_urlencoded::Serializer::new(target);
let mut qp = form_urlencoded::Serializer::new(target);
let urlstr = qp.finish();
let req = http::Request::get(urlstr);
req.body(vec![]).map_err(Error::HttpError)
Expand All @@ -189,7 +189,7 @@ impl Request {
) -> Result<http::Request<Vec<u8>>> {
pp.validate(patch)?;
let target = format!("{}/{}/{}?", self.url_path, name, subresource_name);
let mut qp = url::form_urlencoded::Serializer::new(target);
let mut qp = form_urlencoded::Serializer::new(target);
pp.populate_qp(&mut qp);
let urlstr = qp.finish();

Expand All @@ -209,7 +209,7 @@ impl Request {
data: Vec<u8>,
) -> Result<http::Request<Vec<u8>>> {
let target = format!("{}/{}/{}?", self.url_path, name, subresource_name);
let mut qp = url::form_urlencoded::Serializer::new(target);
let mut qp = form_urlencoded::Serializer::new(target);
if pp.dry_run {
qp.append_pair("dryRun", "All");
}
Expand Down
10 changes: 5 additions & 5 deletions kube-core/src/subresource.rs
Expand Up @@ -42,7 +42,7 @@ impl Request {
/// Get a pod logs
pub fn logs(&self, name: &str, lp: &LogParams) -> Result<http::Request<Vec<u8>>> {
let target = format!("{}/{}/log?", self.url_path, name);
let mut qp = url::form_urlencoded::Serializer::new(target);
let mut qp = form_urlencoded::Serializer::new(target);

if let Some(container) = &lp.container {
qp.append_pair("container", &container);
Expand Down Expand Up @@ -102,7 +102,7 @@ impl Request {
// This is technically identical to Request::create, but different url
let pp = &ep.post_options;
pp.validate()?;
let mut qp = url::form_urlencoded::Serializer::new(target);
let mut qp = form_urlencoded::Serializer::new(target);
if pp.dry_run {
qp.append_pair("dryRun", "All");
}
Expand Down Expand Up @@ -263,7 +263,7 @@ impl AttachParams {
Ok(())
}

fn append_to_url_serializer(&self, qp: &mut url::form_urlencoded::Serializer<String>) {
fn append_to_url_serializer(&self, qp: &mut form_urlencoded::Serializer<String>) {
if self.stdin {
qp.append_pair("stdin", "true");
}
Expand All @@ -290,7 +290,7 @@ impl Request {
ap.validate()?;

let target = format!("{}/{}/attach?", self.url_path, name);
let mut qp = url::form_urlencoded::Serializer::new(target);
let mut qp = form_urlencoded::Serializer::new(target);
ap.append_to_url_serializer(&mut qp);

let req = http::Request::get(qp.finish());
Expand All @@ -313,7 +313,7 @@ impl Request {
ap.validate()?;

let target = format!("{}/{}/exec?", self.url_path, name);
let mut qp = url::form_urlencoded::Serializer::new(target);
let mut qp = form_urlencoded::Serializer::new(target);
ap.append_to_url_serializer(&mut qp);

for c in command.into_iter() {
Expand Down
1 change: 0 additions & 1 deletion kube/Cargo.toml
Expand Up @@ -44,7 +44,6 @@ serde = { version = "1.0.118", features = ["derive"] }
serde_json = "1.0.61"
serde_yaml = { version = "0.8.17", optional = true }
http = "0.2.2"
url = "2.2.0"
either = { version = "1.6.1", optional = true }
thiserror = "1.0.23"
futures = { version = "0.3.8", optional = true }
Expand Down
8 changes: 4 additions & 4 deletions kube/src/config/mod.rs
Expand Up @@ -23,7 +23,7 @@ use std::time::Duration;
#[derive(Debug, Clone)]
pub struct Config {
/// The configured cluster url
pub cluster_url: url::Url,
pub cluster_url: http::Uri,
/// The configured default namespace
pub default_ns: String,
/// The configured root certificate
Expand All @@ -49,7 +49,7 @@ impl Config {
///
/// Most likely you want to use [`Config::infer`] to infer the config from
/// the environment.
pub fn new(cluster_url: url::Url) -> Self {
pub fn new(cluster_url: http::Uri) -> Self {
Self {
cluster_url,
default_ns: String::from("default"),
Expand Down Expand Up @@ -96,7 +96,7 @@ impl Config {
hostenv: incluster_config::SERVICE_HOSTENV,
portenv: incluster_config::SERVICE_PORTENV,
})?;
let cluster_url = url::Url::parse(&cluster_url)?;
let cluster_url = cluster_url.parse::<http::Uri>()?;

let default_ns = incluster_config::load_default_ns()
.map_err(Box::new)
Expand Down Expand Up @@ -143,7 +143,7 @@ impl Config {
}

async fn new_from_loader(loader: ConfigLoader) -> Result<Self> {
let cluster_url = url::Url::parse(&loader.cluster.server)?;
let cluster_url = loader.cluster.server.parse::<http::Uri>()?;

let default_ns = loader
.current_context
Expand Down
9 changes: 3 additions & 6 deletions kube/src/error.rs
Expand Up @@ -47,9 +47,9 @@ pub enum Error {
#[error("HttpError: {0}")]
HttpError(#[from] http::Error),

/// Url conversion error
#[error("InternalUrlError: {0}")]
InternalUrlError(#[from] url::ParseError),
/// Failed to construct a URI.
#[error(transparent)]
InvalidUri(#[from] http::uri::InvalidUri),

/// Common error case when requesting parsing into own structs
#[error("Error deserializing response")]
Expand Down Expand Up @@ -162,9 +162,6 @@ pub enum ConfigError {
#[error("Unable to load in cluster token: {0}")]
InvalidInClusterToken(#[source] Box<Error>),

#[error("Malformed url: {0}")]
MalformedUrl(#[from] url::ParseError),

#[error("exec-plugin response did not contain a status")]
ExecPluginFailed,

Expand Down
64 changes: 32 additions & 32 deletions kube/src/service/url.rs
@@ -1,56 +1,56 @@
use http::Request;
use http::{uri, Request};
use hyper::Body;

/// Set cluster URL.
pub fn set_cluster_url(req: Request<Body>, url: &url::Url) -> Request<Body> {
pub fn set_cluster_url(req: Request<Body>, base_uri: &http::Uri) -> Request<Body> {
let (mut parts, body) = req.into_parts();
let pandq = parts.uri.path_and_query().expect("valid path+query from kube");
parts.uri = finalize_url(url, &pandq).parse().expect("valid URL");
let request_pandq = parts.uri.path_and_query().expect("nonempty path+query");
parts.uri = finalize_url(base_uri, request_pandq);
Request::from_parts(parts, body)
}

/// An internal url joiner to deal with the two different interfaces
///
/// - api module produces a http::Uri which we can turn into a PathAndQuery (has a leading slash by construction)
/// - config module produces a url::Url from user input (sometimes contains path segments)
///
/// This deals with that in a pretty easy way (tested below)
fn finalize_url(cluster_url: &url::Url, request_pandq: &http::uri::PathAndQuery) -> String {
let base = cluster_url.as_str().trim_end_matches('/'); // pandq always starts with a slash
format!("{}{}", base, request_pandq)
// Join base URI and Path+Query, preserving any path in the base.
fn finalize_url(base_uri: &http::Uri, request_pandq: &uri::PathAndQuery) -> http::Uri {
let mut builder = uri::Builder::new();
if let Some(scheme) = base_uri.scheme() {
builder = builder.scheme(scheme.as_str());
}
if let Some(authority) = base_uri.authority() {
builder = builder.authority(authority.as_str());
}
if let Some(pandq) = base_uri.path_and_query() {
// If `base_uri` has path, remove any trailing space and join.
// `PathAndQuery` always starts with a slash.
let base_path = pandq.path().trim_end_matches('/');
builder = builder.path_and_query(format!("{}{}", base_path, request_pandq));
} else {
builder = builder.path_and_query(request_pandq.as_str());
}
builder.build().expect("valid URI")
}

#[cfg(test)]
mod tests {
#[test]
fn normal_host() {
let minikube_host = "https://192.168.1.65:8443";
let cluster_url = url::Url::parse(minikube_host).unwrap();
let apipath: http::Uri = "/api/v1/nodes?hi=yes".parse().unwrap();
let base_uri = http::Uri::from_static("https://192.168.1.65:8443");
let apipath = http::Uri::from_static("/api/v1/nodes?hi=yes");
let pandq = apipath.path_and_query().expect("could pandq apipath");
let final_url = super::finalize_url(&cluster_url, &pandq);
assert_eq!(
final_url.as_str(),
super::finalize_url(&base_uri, &pandq),
"https://192.168.1.65:8443/api/v1/nodes?hi=yes"
);
}

#[test]
fn rancher_host() {
// in rancher, kubernetes server names are not hostnames, but a host with a path:
let rancher_host = "https://hostname/foo/bar";
let cluster_url = url::Url::parse(rancher_host).unwrap();
assert_eq!(cluster_url.host_str().unwrap(), "hostname");
assert_eq!(cluster_url.path(), "/foo/bar");
// we must be careful when using Url::join on our http::Uri result
// as a straight two Uri::join would trim away rancher's initial path
// case in point (discards original path):
assert_eq!(cluster_url.join("/api/v1/nodes").unwrap().path(), "/api/v1/nodes");

let apipath: http::Uri = "/api/v1/nodes?hi=yes".parse().unwrap();
let pandq = apipath.path_and_query().expect("could pandq apipath");

let final_url = super::finalize_url(&cluster_url, &pandq);
assert_eq!(final_url.as_str(), "https://hostname/foo/bar/api/v1/nodes?hi=yes");
let base_uri = http::Uri::from_static("https://example.com/foo/bar");
let api_path = http::Uri::from_static("/api/v1/nodes?hi=yes");
let pandq = api_path.path_and_query().unwrap();
assert_eq!(
super::finalize_url(&base_uri, &pandq),
"https://example.com/foo/bar/api/v1/nodes?hi=yes"
);
}
}

0 comments on commit 0a570c5

Please sign in to comment.