From e69654322e3500af5ea24c77f94624cd99f9523e Mon Sep 17 00:00:00 2001 From: kazk Date: Fri, 29 Oct 2021 13:21:15 -0700 Subject: [PATCH 1/9] Remove `impl From for kube::Error` Require explicitly mapping errors. This commit shows how much we need to mentally process while reading the code (maintenance cost), and how badly grouped they are (usability issue). Removing `impl From` forces us to attach context. Signed-off-by: kazk --- kube-client/src/api/core_methods.rs | 6 +- kube-client/src/client/auth/mod.rs | 70 ++++++++++++------- kube-client/src/client/auth/oauth.rs | 44 +++++++----- .../src/client/middleware/refresh_token.rs | 2 +- kube-client/src/client/mod.rs | 49 ++++++++++--- kube-client/src/client/tls.rs | 11 +-- kube-client/src/config/file_config.rs | 27 ++++--- kube-client/src/config/file_loader.rs | 24 ++++--- kube-client/src/config/mod.rs | 29 ++++---- kube-client/src/config/utils.rs | 12 ++-- kube-client/src/discovery/apigroup.rs | 11 +-- kube-client/src/discovery/oneshot.rs | 6 +- kube-client/src/discovery/parse.rs | 4 +- kube-client/src/error.rs | 35 ++++------ kube-core/src/admission.rs | 9 +-- kube-core/src/error.rs | 4 +- kube-core/src/params.rs | 2 +- kube-core/src/request.rs | 4 +- kube-core/src/subresource.rs | 3 +- 19 files changed, 216 insertions(+), 136 deletions(-) diff --git a/kube-client/src/api/core_methods.rs b/kube-client/src/api/core_methods.rs index b6755da93..e7531c2c9 100644 --- a/kube-client/src/api/core_methods.rs +++ b/kube-client/src/api/core_methods.rs @@ -3,7 +3,7 @@ use futures::Stream; use serde::{de::DeserializeOwned, Serialize}; use std::fmt::Debug; -use crate::{api::Api, Result}; +use crate::{api::Api, Error, Result}; use kube_core::{object::ObjectList, params::*, response::Status, WatchEvent}; /// PUSH/PUT/POST/GET abstractions @@ -74,7 +74,7 @@ where where K: Serialize, { - let bytes = serde_json::to_vec(&data)?; + let bytes = serde_json::to_vec(&data).map_err(Error::SerdeError)?; let mut req = self.request.create(pp, bytes)?; req.extensions_mut().insert("create"); self.client.request::(req).await @@ -233,7 +233,7 @@ where where K: Serialize, { - let bytes = serde_json::to_vec(&data)?; + let bytes = serde_json::to_vec(&data).map_err(Error::SerdeError)?; let mut req = self.request.replace(name, pp, bytes)?; req.extensions_mut().insert("replace"); self.client.request::(req).await diff --git a/kube-client/src/client/auth/mod.rs b/kube-client/src/client/auth/mod.rs index 7c41f9c7c..f9ef2f0f6 100644 --- a/kube-client/src/client/auth/mod.rs +++ b/kube-client/src/client/auth/mod.rs @@ -46,7 +46,7 @@ impl RefreshableToken { if Utc::now() + Duration::seconds(60) >= locked_data.1 { match Auth::try_from(&locked_data.2)? { Auth::None | Auth::Basic(_, _) | Auth::Bearer(_) => { - return Err(ConfigError::UnrefreshableTokenResponse).map_err(Error::from); + return Err(Error::Kubeconfig(ConfigError::UnrefreshableTokenResponse)); } Auth::RefreshableToken(RefreshableToken::Exec(d)) => { @@ -65,7 +65,7 @@ impl RefreshableToken { } let mut value = HeaderValue::try_from(format!("Bearer {}", &locked_data.0)) - .map_err(ConfigError::InvalidBearerToken)?; + .map_err(|err| Error::Kubeconfig(ConfigError::InvalidBearerToken(err)))?; value.set_sensitive(true); Ok(value) } @@ -75,7 +75,7 @@ impl RefreshableToken { let gcp_oauth = data.lock().await; let token = (*gcp_oauth).token().await?; let mut value = HeaderValue::try_from(format!("Bearer {}", &token.access_token)) - .map_err(ConfigError::InvalidBearerToken)?; + .map_err(|err| Error::Kubeconfig(ConfigError::InvalidBearerToken(err)))?; value.set_sensitive(true); Ok(value) } @@ -128,13 +128,16 @@ impl TryFrom<&AuthInfo> for Auth { Some(token) => (Some(token.clone()), None), None => { if let Some(exec) = &auth_info.exec { - let creds = auth_exec(exec)?; - let status = creds.status.ok_or(ConfigError::ExecPluginFailed)?; + let creds = auth_exec(exec).map_err(Error::Kubeconfig)?; + let status = creds + .status + .ok_or(ConfigError::ExecPluginFailed) + .map_err(Error::Kubeconfig)?; let expiration = status .expiration_timestamp .map(|ts| ts.parse()) .transpose() - .map_err(ConfigError::MalformedTokenExpirationDate)?; + .map_err(|err| Error::Kubeconfig(ConfigError::MalformedTokenExpirationDate(err)))?; (status.token, expiration) } else if let Some(file) = &auth_info.token_file { (Some(read_file_to_string(file)?), None) @@ -169,18 +172,19 @@ fn token_from_provider(provider: &AuthProviderConfig) -> Result { match provider.name.as_ref() { "oidc" => token_from_oidc_provider(provider), "gcp" => token_from_gcp_provider(provider), - _ => Err(ConfigError::AuthExec(format!( + _ => Err(Error::Kubeconfig(ConfigError::AuthExec(format!( "Authentication with provider {:} not supported", provider.name - )) - .into()), + )))), } } fn token_from_oidc_provider(provider: &AuthProviderConfig) -> Result { match provider.config.get("id-token") { Some(id_token) => Ok(ProviderToken::Oidc(id_token.clone())), - None => Err(ConfigError::AuthExec("No id-token for oidc Authentication provider".into()).into()), + None => Err(Error::Kubeconfig(ConfigError::AuthExec( + "No id-token for oidc Authentication provider".into(), + ))), } } @@ -194,7 +198,7 @@ fn token_from_gcp_provider(provider: &AuthProviderConfig) -> Result>() - .map_err(ConfigError::MalformedTokenExpirationDate)?; + .map_err(|err| Error::Kubeconfig(ConfigError::MalformedTokenExpirationDate(err)))?; if Utc::now() + Duration::seconds(60) < expiry_date { return Ok(ProviderToken::GcpCommand(access_token.clone(), Some(expiry_date))); } @@ -209,32 +213,39 @@ fn token_from_gcp_provider(provider: &AuthProviderConfig) -> Result>() - .map_err(ConfigError::MalformedTokenExpirationDate)?; + .map_err(|err| Error::Kubeconfig(ConfigError::MalformedTokenExpirationDate(err)))?; return Ok(ProviderToken::GcpCommand(token, Some(expiry))); } else { return Ok(ProviderToken::GcpCommand(token, None)); } } else { let token = std::str::from_utf8(&output.stdout) - .map_err(|e| ConfigError::AuthExec(format!("Result is not a string {:?} ", e)))? + .map_err(|e| { + Error::Kubeconfig(ConfigError::AuthExec(format!("Result is not a string {:?} ", e))) + })? .to_owned(); return Ok(ProviderToken::GcpCommand(token, None)); } @@ -249,10 +260,9 @@ fn token_from_gcp_provider(provider: &AuthProviderConfig) -> Result Result { if let serde_json::Value::String(res) = v[0] { Ok(res.clone()) } else { - Err(ConfigError::AuthExec(format!("Target value at {:} is not a string", pure_path)).into()) + Err(Error::Kubeconfig(ConfigError::AuthExec(format!( + "Target value at {:} is not a string", + pure_path + )))) } } - Err(e) => Err(ConfigError::AuthExec(format!("Could not extract JSON value: {:}", e)).into()), + Err(e) => Err(Error::Kubeconfig(ConfigError::AuthExec(format!( + "Could not extract JSON value: {:}", + e + )))), - _ => Err(ConfigError::AuthExec(format!("Target value {:} not found", pure_path)).into()), + _ => Err(Error::Kubeconfig(ConfigError::AuthExec(format!( + "Target value {:} not found", + pure_path + )))), } } @@ -366,7 +385,8 @@ mod test { expiry = expiry ); - let config: Kubeconfig = serde_yaml::from_str(&test_file).map_err(ConfigError::ParseYaml)?; + let config: Kubeconfig = + serde_yaml::from_str(&test_file).map_err(|err| Error::Kubeconfig(ConfigError::ParseYaml(err)))?; let auth_info = &config.auth_infos[0].auth_info; match Auth::try_from(auth_info).unwrap() { Auth::RefreshableToken(RefreshableToken::Exec(refreshable)) => { diff --git a/kube-client/src/client/auth/oauth.rs b/kube-client/src/client/auth/oauth.rs index fdca550c6..94fda45ad 100644 --- a/kube-client/src/client/auth/oauth.rs +++ b/kube-client/src/client/auth/oauth.rs @@ -34,8 +34,9 @@ impl Gcp { const DEFAULT_SCOPES: &str = "https://www.googleapis.com/auth/cloud-platform,https://www.googleapis.com/auth/userinfo.email"; // Initialize ServiceAccountAccess so we can request later when needed. - let info = gcloud_account_info()?; - let access = ServiceAccountAccess::new(info).map_err(OAuthError::InvalidKeyFormat)?; + let info = gcloud_account_info().map_err(Error::Kubeconfig)?; + let access = ServiceAccountAccess::new(info) + .map_err(|err| Error::Kubeconfig(ConfigError::OAuth(OAuthError::InvalidKeyFormat(err))))?; let scopes = scopes .map(String::to_owned) .unwrap_or_else(|| DEFAULT_SCOPES.to_owned()) @@ -65,21 +66,21 @@ impl Gcp { let res = client .request(request.map(hyper::Body::from)) .await - .map_err(OAuthError::RequestToken)?; + .map_err(|err| Error::Kubeconfig(ConfigError::OAuth(OAuthError::RequestToken(err))))?; // Convert response body to `Vec` for parsing. let (parts, body) = res.into_parts(); - let bytes = hyper::body::to_bytes(body).await?; + let bytes = hyper::body::to_bytes(body).await.map_err(Error::HyperError)?; let response = http::Response::from_parts(parts, bytes.to_vec()); match self.access.parse_token_response(scope_hash, response) { Ok(token) => Ok(token), - Err(err) => match err { + Err(err) => Err(Error::Kubeconfig(ConfigError::OAuth(match err { tame_oauth::Error::AuthError(_) | tame_oauth::Error::HttpStatus(_) => { - Err(OAuthError::RetrieveCredentials(err).into()) + OAuthError::RetrieveCredentials(err) } - tame_oauth::Error::Json(e) => Err(OAuthError::ParseToken(e).into()), - err => Err(OAuthError::Unknown(err.to_string()).into()), - }, + tame_oauth::Error::Json(e) => OAuthError::ParseToken(e), + err => OAuthError::Unknown(err.to_string()), + }))), } } @@ -88,9 +89,15 @@ impl Gcp { Err(err) => match err { // Request builder failed. tame_oauth::Error::Http(e) => Err(Error::HttpError(e)), - tame_oauth::Error::InvalidRsaKey => Err(OAuthError::InvalidRsaKey(err).into()), - tame_oauth::Error::InvalidKeyFormat => Err(OAuthError::InvalidKeyFormat(err).into()), - e => Err(OAuthError::Unknown(e.to_string()).into()), + tame_oauth::Error::InvalidRsaKey => Err(Error::Kubeconfig(ConfigError::OAuth( + OAuthError::InvalidRsaKey(err), + ))), + tame_oauth::Error::InvalidKeyFormat => Err(Error::Kubeconfig(ConfigError::OAuth( + OAuthError::InvalidKeyFormat(err), + ))), + e => Err(Error::Kubeconfig(ConfigError::OAuth(OAuthError::Unknown( + e.to_string(), + )))), }, } } @@ -101,10 +108,13 @@ const GOOGLE_APPLICATION_CREDENTIALS: &str = "GOOGLE_APPLICATION_CREDENTIALS"; pub(crate) fn gcloud_account_info() -> Result { let path = env::var_os(GOOGLE_APPLICATION_CREDENTIALS) .map(PathBuf::from) - .ok_or(OAuthError::MissingGoogleCredentials)?; - let data = std::fs::read_to_string(path).map_err(OAuthError::LoadCredentials)?; - ServiceAccountInfo::deserialize(data).map_err(|err| match err { - tame_oauth::Error::Json(e) => OAuthError::ParseCredentials(e).into(), - _ => OAuthError::Unknown(err.to_string()).into(), + .ok_or(ConfigError::OAuth(OAuthError::MissingGoogleCredentials))?; + let data = + std::fs::read_to_string(path).map_err(|err| ConfigError::OAuth(OAuthError::LoadCredentials(err)))?; + ServiceAccountInfo::deserialize(data).map_err(|err| { + ConfigError::OAuth(match err { + tame_oauth::Error::Json(e) => OAuthError::ParseCredentials(e), + _ => OAuthError::Unknown(err.to_string()), + }) }) } diff --git a/kube-client/src/client/middleware/refresh_token.rs b/kube-client/src/client/middleware/refresh_token.rs index 83488c0ee..97669860a 100644 --- a/kube-client/src/client/middleware/refresh_token.rs +++ b/kube-client/src/client/middleware/refresh_token.rs @@ -8,7 +8,7 @@ use http::{header::AUTHORIZATION, Request, Response}; use pin_project::pin_project; use tower::{layer::Layer, BoxError, Service}; -use crate::{client::auth::RefreshableToken, Result}; +use crate::client::auth::RefreshableToken; /// `Layer` to decorate the request with `Authorization` header with refreshable token. /// Token is refreshed automatically when necessary. diff --git a/kube-client/src/client/mod.rs b/kube-client/src/client/mod.rs index 07cb64134..de908b7de 100644 --- a/kube-client/src/client/mod.rs +++ b/kube-client/src/client/mod.rs @@ -211,8 +211,10 @@ impl Client { let res = self.send(request.map(Body::from)).await?; let status = res.status(); // trace!("Status = {:?} for {}", status, res.url()); - let body_bytes = hyper::body::to_bytes(res.into_body()).await?; - let text = String::from_utf8(body_bytes.to_vec())?; + let body_bytes = hyper::body::to_bytes(res.into_body()) + .await + .map_err(Error::HyperError)?; + let text = String::from_utf8(body_bytes.to_vec()).map_err(Error::FromUtf8)?; handle_api_errors(&text, status)?; Ok(text) @@ -237,7 +239,7 @@ impl Client { { let text = self.request_text(request).await?; // It needs to be JSON: - let v: Value = serde_json::from_str(&text)?; + let v: Value = serde_json::from_str(&text).map_err(Error::SerdeError)?; if v["kind"] == "Status" { tracing::trace!("Status from {}", text); Ok(Right(serde_json::from_str::(&text).map_err(|e| { @@ -332,13 +334,24 @@ impl Client { impl Client { /// Returns apiserver version. pub async fn apiserver_version(&self) -> Result { - self.request(Request::builder().uri("/version").body(vec![])?) - .await + self.request( + Request::builder() + .uri("/version") + .body(vec![]) + .map_err(Error::HttpError)?, + ) + .await } /// Lists api groups that apiserver serves. pub async fn list_api_groups(&self) -> Result { - self.request(Request::builder().uri("/apis").body(vec![])?).await + self.request( + Request::builder() + .uri("/apis") + .body(vec![]) + .map_err(Error::HttpError)?, + ) + .await } /// Lists resources served in given API group. @@ -361,18 +374,36 @@ impl Client { /// ``` pub async fn list_api_group_resources(&self, apiversion: &str) -> Result { let url = format!("/apis/{}", apiversion); - self.request(Request::builder().uri(url).body(vec![])?).await + self.request( + Request::builder() + .uri(url) + .body(vec![]) + .map_err(Error::HttpError)?, + ) + .await } /// Lists versions of `core` a.k.a. `""` legacy API group. pub async fn list_core_api_versions(&self) -> Result { - self.request(Request::builder().uri("/api").body(vec![])?).await + self.request( + Request::builder() + .uri("/api") + .body(vec![]) + .map_err(Error::HttpError)?, + ) + .await } /// Lists resources served in particular `core` group version. pub async fn list_core_api_resources(&self, version: &str) -> Result { let url = format!("/api/{}", version); - self.request(Request::builder().uri(url).body(vec![])?).await + self.request( + Request::builder() + .uri(url) + .body(vec![]) + .map_err(Error::HttpError)?, + ) + .await } } diff --git a/kube-client/src/client/tls.rs b/kube-client/src/client/tls.rs index f3c10b7c6..06cd5d482 100644 --- a/kube-client/src/client/tls.rs +++ b/kube-client/src/client/tls.rs @@ -40,10 +40,13 @@ pub mod native_tls { // TODO Replace this with pure Rust implementation to avoid depending on openssl on macOS and Win fn pkcs12_from_pem(pem: &[u8], password: &str) -> Result> { use openssl::{pkcs12::Pkcs12, pkey::PKey, x509::X509}; - let x509 = X509::from_pem(pem)?; - let pkey = PKey::private_key_from_pem(pem)?; - let p12 = Pkcs12::builder().build(password, "kubeconfig", &pkey, &x509)?; - let der = p12.to_der()?; + // TODO These are all treated as the same error. Add specific errors. + let x509 = X509::from_pem(pem).map_err(Error::OpensslError)?; + let pkey = PKey::private_key_from_pem(pem).map_err(Error::OpensslError)?; + let p12 = Pkcs12::builder() + .build(password, "kubeconfig", &pkey, &x509) + .map_err(Error::OpensslError)?; + let der = p12.to_der().map_err(Error::OpensslError)?; Ok(der) } } diff --git a/kube-client/src/config/file_config.rs b/kube-client/src/config/file_config.rs index b671d4235..b51df68f6 100644 --- a/kube-client/src/config/file_config.rs +++ b/kube-client/src/config/file_config.rs @@ -1,5 +1,5 @@ #![allow(missing_docs)] -use crate::{config::utils, error::ConfigError, Result}; +use crate::{config::utils, error::ConfigError, Error, Result}; use serde::{Deserialize, Serialize}; use std::{collections::HashMap, fs, path::Path}; @@ -207,15 +207,19 @@ const KUBECONFIG: &str = "KUBECONFIG"; impl Kubeconfig { /// Read a Config from an arbitrary location pub fn read_from>(path: P) -> Result { - let data = fs::read_to_string(&path).map_err(|source| ConfigError::ReadFile { - path: path.as_ref().into(), - source, + let data = fs::read_to_string(&path).map_err(|source| { + Error::Kubeconfig(ConfigError::ReadFile { + path: path.as_ref().into(), + source, + }) })?; // support multiple documents let mut documents: Vec = vec![]; for doc in serde_yaml::Deserializer::from_str(&data) { - let value = serde_yaml::Value::deserialize(doc).map_err(ConfigError::ParseYaml)?; - let kconf = serde_yaml::from_value(value).map_err(ConfigError::ParseYaml)?; + let value = serde_yaml::Value::deserialize(doc) + .map_err(|err| Error::Kubeconfig(ConfigError::ParseYaml(err)))?; + let kconf = serde_yaml::from_value(value) + .map_err(|err| Error::Kubeconfig(ConfigError::ParseYaml(err)))?; documents.push(kconf) } @@ -254,7 +258,8 @@ impl Kubeconfig { merged_docs = Some(config); } } - let config = merged_docs.ok_or_else(|| ConfigError::EmptyKubeconfig(path.as_ref().to_path_buf()))?; + let config = merged_docs + .ok_or_else(|| Error::Kubeconfig(ConfigError::EmptyKubeconfig(path.as_ref().to_path_buf())))?; Ok(config) } @@ -263,7 +268,9 @@ impl Kubeconfig { match Self::from_env()? { Some(config) => Ok(config), None => { - let path = utils::default_kube_path().ok_or(ConfigError::NoKubeconfigPath)?; + let path = utils::default_kube_path() + .ok_or(ConfigError::NoKubeconfigPath) + .map_err(Error::Kubeconfig)?; Self::read_from(path) } } @@ -309,10 +316,10 @@ impl Kubeconfig { /// > Even if the second file has non-conflicting entries under `red-user`, discard them. fn merge(mut self, next: Kubeconfig) -> Result { if self.kind.is_some() && next.kind.is_some() && self.kind != next.kind { - return Err(ConfigError::KindMismatch.into()); + return Err(Error::Kubeconfig(ConfigError::KindMismatch)); } if self.api_version.is_some() && next.api_version.is_some() && self.api_version != next.api_version { - return Err(ConfigError::ApiVersionMismatch.into()); + return Err(Error::Kubeconfig(ConfigError::ApiVersionMismatch)); } self.kind = self.kind.or(next.kind); diff --git a/kube-client/src/config/file_loader.rs b/kube-client/src/config/file_loader.rs index 8e0b59001..e71c7e5e1 100644 --- a/kube-client/src/config/file_loader.rs +++ b/kube-client/src/config/file_loader.rs @@ -2,7 +2,7 @@ use super::{ file_config::{AuthInfo, Cluster, Context, Kubeconfig}, utils, }; -use crate::{error::ConfigError, Result}; +use crate::{error::ConfigError, Error, Result}; /// KubeConfigOptions stores options used when loading kubeconfig file. #[derive(Default, Clone)] @@ -62,15 +62,17 @@ impl ConfigLoader { } else if let Some(name) = &config.current_context { name } else { - return Err(ConfigError::CurrentContextNotSet.into()); + return Err(Error::Kubeconfig(ConfigError::CurrentContextNotSet)); }; let current_context = config .contexts .iter() .find(|named_context| &named_context.name == context_name) .map(|named_context| &named_context.context) - .ok_or_else(|| ConfigError::LoadContext { - context_name: context_name.clone(), + .ok_or_else(|| { + Error::Kubeconfig(ConfigError::LoadContext { + context_name: context_name.clone(), + }) })?; let cluster_name = cluster.unwrap_or(¤t_context.cluster); @@ -79,8 +81,10 @@ impl ConfigLoader { .iter() .find(|named_cluster| &named_cluster.name == cluster_name) .map(|named_cluster| &named_cluster.cluster) - .ok_or_else(|| ConfigError::LoadClusterOfContext { - cluster_name: cluster_name.clone(), + .ok_or_else(|| { + Error::Kubeconfig(ConfigError::LoadClusterOfContext { + cluster_name: cluster_name.clone(), + }) })?; let user_name = user.unwrap_or(¤t_context.user); @@ -89,8 +93,10 @@ impl ConfigLoader { .iter() .find(|named_user| &named_user.name == user_name) .map(|named_user| &named_user.auth_info) - .ok_or_else(|| ConfigError::FindUser { - user_name: user_name.clone(), + .ok_or_else(|| { + Error::Kubeconfig(ConfigError::FindUser { + user_name: user_name.clone(), + }) })?; Ok(ConfigLoader { @@ -123,7 +129,7 @@ impl ConfigLoader { .or_else(|| nonempty(std::env::var("HTTP_PROXY").ok())) .or_else(|| nonempty(std::env::var("HTTPS_PROXY").ok())) { - Ok(Some(proxy.parse::()?)) + Ok(Some(proxy.parse::().map_err(Error::InvalidUri)?)) } else { Ok(None) } diff --git a/kube-client/src/config/mod.rs b/kube-client/src/config/mod.rs index 4b89be1f6..5a9f11e1d 100644 --- a/kube-client/src/config/mod.rs +++ b/kube-client/src/config/mod.rs @@ -9,7 +9,7 @@ mod file_loader; mod incluster_config; mod utils; -use crate::{error::ConfigError, Result}; +use crate::{error::ConfigError, Error, Result}; use file_loader::ConfigLoader; pub use file_loader::KubeConfigOptions; #[cfg(feature = "client")] pub(crate) use utils::read_file_to_string; @@ -82,9 +82,11 @@ impl Config { tracing::trace!("Falling back to local kubeconfig"); let config = Self::from_kubeconfig(&KubeConfigOptions::default()) .await - .map_err(|kubeconfig_err| ConfigError::ConfigInferenceExhausted { - cluster_env: Box::new(cluster_env_err), - kubeconfig: Box::new(kubeconfig_err), + .map_err(|kubeconfig_err| { + Error::Kubeconfig(ConfigError::ConfigInferenceExhausted { + cluster_env: Box::new(cluster_env_err), + kubeconfig: Box::new(kubeconfig_err), + }) })?; Ok(config) @@ -105,22 +107,21 @@ impl Config { incluster_config::kube_dns() } else { incluster_config::kube_server() - .ok_or(ConfigError::MissingInClusterVariables { + .ok_or(Error::Kubeconfig(ConfigError::MissingInClusterVariables { hostenv: incluster_config::SERVICE_HOSTENV, portenv: incluster_config::SERVICE_PORTENV, - })? - .parse::()? + }))? + .parse::() + .map_err(Error::InvalidUri)? }; let default_namespace = incluster_config::load_default_ns() - .map_err(Box::new) - .map_err(ConfigError::InvalidInClusterNamespace)?; + .map_err(|err| Error::Kubeconfig(ConfigError::InvalidInClusterNamespace(Box::new(err))))?; let root_cert = incluster_config::load_cert()?; let token = incluster_config::load_token() - .map_err(Box::new) - .map_err(ConfigError::InvalidInClusterToken)?; + .map_err(|err| Error::Kubeconfig(ConfigError::InvalidInClusterToken(Box::new(err))))?; Ok(Self { cluster_url, @@ -156,7 +157,11 @@ impl Config { } async fn new_from_loader(loader: ConfigLoader) -> Result { - let cluster_url = loader.cluster.server.parse::()?; + let cluster_url = loader + .cluster + .server + .parse::() + .map_err(Error::InvalidUri)?; let default_namespace = loader .current_context diff --git a/kube-client/src/config/utils.rs b/kube-client/src/config/utils.rs index 55565047b..31d019828 100644 --- a/kube-client/src/config/utils.rs +++ b/kube-client/src/config/utils.rs @@ -17,7 +17,7 @@ pub fn data_or_file_with_base64>(data: &Option, file: &Op .map_err(ConfigError::Base64Decode) .map_err(Error::Kubeconfig), (_, Some(f)) => read_file(f), - _ => Err(ConfigError::NoBase64FileOrData.into()), + _ => Err(Error::Kubeconfig(ConfigError::NoBase64FileOrData)), }?; //Ensure there is a trailing newline in the blob //Don't bother if the blob is empty @@ -29,21 +29,19 @@ pub fn data_or_file_with_base64>(data: &Option, file: &Op pub fn read_file>(file: P) -> Result> { fs::read(&file).map_err(|source| { - ConfigError::ReadFile { + Error::Kubeconfig(ConfigError::ReadFile { path: file.as_ref().into(), source, - } - .into() + }) }) } pub fn read_file_to_string>(file: P) -> Result { fs::read_to_string(&file).map_err(|source| { - ConfigError::ReadFile { + Error::Kubeconfig(ConfigError::ReadFile { path: file.as_ref().into(), source, - } - .into() + }) }) } diff --git a/kube-client/src/discovery/apigroup.rs b/kube-client/src/discovery/apigroup.rs index bb9b34abf..e43e2bdea 100644 --- a/kube-client/src/discovery/apigroup.rs +++ b/kube-client/src/discovery/apigroup.rs @@ -2,7 +2,7 @@ use super::{ parse::{self, GroupVersionData}, version::Version, }; -use crate::{error::DiscoveryError, Client, Result}; +use crate::{error::DiscoveryError, Client, Error, Result}; use k8s_openapi::apimachinery::pkg::apis::meta::v1::{APIGroup, APIVersions}; pub use kube_core::discovery::{verbs, ApiCapabilities, ApiResource, Scope}; use kube_core::gvk::{GroupVersion, GroupVersionKind}; @@ -80,7 +80,7 @@ impl ApiGroup { tracing::debug!(name = g.name.as_str(), "Listing group versions"); let key = g.name; if g.versions.is_empty() { - return Err(DiscoveryError::EmptyApiGroup(key).into()); + return Err(Error::Discovery(DiscoveryError::EmptyApiGroup(key))); } let mut data = vec![]; for vers in &g.versions { @@ -100,7 +100,7 @@ impl ApiGroup { let mut data = vec![]; let key = ApiGroup::CORE_GROUP.to_string(); if coreapis.versions.is_empty() { - return Err(DiscoveryError::EmptyApiGroup(key).into()); + return Err(Error::Discovery(DiscoveryError::EmptyApiGroup(key))); } for v in coreapis.versions { let resources = client.list_core_api_resources(&v).await?; @@ -138,7 +138,10 @@ impl ApiGroup { return Ok((ar, caps)); } } - Err(DiscoveryError::MissingKind(format!("{:?}", gvk)).into()) + Err(Error::Discovery(DiscoveryError::MissingKind(format!( + "{:?}", + gvk + )))) } // shortcut method to give cheapest return for a pinned group diff --git a/kube-client/src/discovery/oneshot.rs b/kube-client/src/discovery/oneshot.rs index 20dd37bfa..6e4c990dd 100644 --- a/kube-client/src/discovery/oneshot.rs +++ b/kube-client/src/discovery/oneshot.rs @@ -12,7 +12,7 @@ //! [`oneshot::pinned_kind`]: crate::discovery::pinned_kind use super::ApiGroup; -use crate::{error::DiscoveryError, Client, Result}; +use crate::{error::DiscoveryError, Client, Error, Result}; use kube_core::{ discovery::{ApiCapabilities, ApiResource}, gvk::{GroupVersion, GroupVersionKind}, @@ -50,7 +50,9 @@ pub async fn group(client: &Client, apigroup: &str) -> Result { return ApiGroup::query_apis(client, g).await; } } - Err(DiscoveryError::MissingApiGroup(apigroup.to_string()).into()) + Err(Error::Discovery(DiscoveryError::MissingApiGroup( + apigroup.to_string(), + ))) } /// Discovers all APIs available under a certain group at a pinned version diff --git a/kube-client/src/discovery/parse.rs b/kube-client/src/discovery/parse.rs index afd481377..52c596bb2 100644 --- a/kube-client/src/discovery/parse.rs +++ b/kube-client/src/discovery/parse.rs @@ -1,5 +1,5 @@ //! Abstractions on top of k8s_openapi::apimachinery::pkg::apis::meta::v1 -use crate::{error::DiscoveryError, Result}; +use crate::{error::DiscoveryError, Error, Result}; use k8s_openapi::apimachinery::pkg::apis::meta::v1::{APIResource, APIResourceList}; use kube_core::{ discovery::{ApiCapabilities, ApiResource, Scope}, @@ -29,7 +29,7 @@ pub(crate) fn parse_apicapabilities(list: &APIResourceList, name: &str) -> Resul .resources .iter() .find(|r| r.name == name) - .ok_or_else(|| DiscoveryError::MissingResource(name.into()))?; + .ok_or_else(|| Error::Discovery(DiscoveryError::MissingResource(name.into())))?; let scope = if ar.namespaced { Scope::Namespaced } else { diff --git a/kube-client/src/error.rs b/kube-client/src/error.rs index b8346a87d..89bd912be 100644 --- a/kube-client/src/error.rs +++ b/kube-client/src/error.rs @@ -19,20 +19,20 @@ pub enum Error { /// ConnectionError for when TcpStream fails to connect. #[error("ConnectionError: {0}")] - Connection(std::io::Error), + Connection(#[source] std::io::Error), /// Hyper error #[cfg(feature = "client")] #[error("HyperError: {0}")] - HyperError(#[from] hyper::Error), + HyperError(#[source] hyper::Error), /// Service error #[cfg(feature = "client")] #[error("ServiceError: {0}")] - Service(tower::BoxError), + Service(#[source] tower::BoxError), /// UTF-8 Error #[error("UTF-8 Error: {0}")] - FromUtf8(#[from] std::string::FromUtf8Error), + FromUtf8(#[source] std::string::FromUtf8Error), /// Returned when failed to find a newline character within max length. /// Only returned by `Client::request_events` and this should never happen as @@ -42,19 +42,19 @@ pub enum Error { /// Returned on `std::io::Error` when reading event stream. #[error("Error reading events stream: {0}")] - ReadEvents(std::io::Error), + ReadEvents(#[source] std::io::Error), /// Http based error #[error("HttpError: {0}")] - HttpError(#[from] http::Error), + HttpError(#[source] http::Error), /// Failed to construct a URI. - #[error(transparent)] - InvalidUri(#[from] http::uri::InvalidUri), + #[error("InvalidUri: {0}")] + InvalidUri(#[source] http::uri::InvalidUri), /// Common error case when requesting parsing into own structs #[error("Error deserializing response")] - SerdeError(#[from] serde_json::Error), + SerdeError(#[source] serde_json::Error), /// Error building a request #[error("Error building request")] @@ -74,11 +74,11 @@ pub enum Error { /// Configuration error #[error("Error loading kubeconfig: {0}")] - Kubeconfig(#[from] ConfigError), + Kubeconfig(#[source] ConfigError), /// Discovery errors #[error("Error from discovery: {0}")] - Discovery(#[from] DiscoveryError), + Discovery(#[source] DiscoveryError), /// An error with configuring SSL occured #[error("SslError: {0}")] @@ -88,7 +88,7 @@ pub enum Error { #[cfg(feature = "native-tls")] #[cfg_attr(docsrs, doc(cfg(feature = "native-tls")))] #[error("OpensslError: {0}")] - OpensslError(#[from] openssl::error::ErrorStack), + OpensslError(#[source] openssl::error::ErrorStack), /// The server did not respond with [`SWITCHING_PROTOCOLS`] status when upgrading the /// connection. @@ -179,7 +179,7 @@ pub enum ConfigError { #[cfg(feature = "oauth")] #[cfg_attr(docsrs, doc(cfg(feature = "oauth")))] #[error("OAuth Error: {0}")] - OAuth(#[from] OAuthError), + OAuth(#[source] OAuthError), #[error("Unable to load config file: {0}")] LoadConfigFile(#[source] Box), @@ -256,14 +256,6 @@ pub enum OAuthError { Unknown(String), } -#[cfg(feature = "oauth")] -#[cfg_attr(docsrs, doc(cfg(feature = "oauth")))] -impl From for Error { - fn from(e: OAuthError) -> Self { - ConfigError::OAuth(e).into() - } -} - #[derive(Error, Debug)] // Redundant with the error messages and machine names #[allow(missing_docs)] @@ -281,6 +273,7 @@ pub enum DiscoveryError { EmptyApiGroup(String), } +// TODO Remove this impl From for Error { fn from(error: kube_core::Error) -> Self { match error { diff --git a/kube-core/src/admission.rs b/kube-core/src/admission.rs index 82921b8ed..e2cbd25f3 100644 --- a/kube-core/src/admission.rs +++ b/kube-core/src/admission.rs @@ -304,7 +304,7 @@ impl AdmissionResponse { /// Add JSON patches to the response, modifying the object from the request. pub fn with_patch(mut self, patch: json_patch::Patch) -> Result { - self.patch = Some(serde_json::to_vec(&patch)?); + self.patch = Some(serde_json::to_vec(&patch).map_err(Error::SerdeError)?); self.patch_type = Some(PatchType::JsonPatch); Ok(self) @@ -335,18 +335,19 @@ mod test { use crate::{ admission::{AdmissionResponse, AdmissionReview}, - DynamicObject, Result, + DynamicObject, Error, Result, }; #[test] fn v1_webhook_unmarshals() -> Result<()> { - serde_json::from_str::>(WEBHOOK_BODY)?; + serde_json::from_str::>(WEBHOOK_BODY).map_err(Error::SerdeError)?; Ok(()) } #[test] fn version_passes_through() -> Result<()> { - let rev = serde_json::from_str::>(WEBHOOK_BODY)?; + let rev = serde_json::from_str::>(WEBHOOK_BODY) + .map_err(Error::SerdeError)?; let rev_typ = rev.types.clone(); let res = AdmissionResponse::from(&rev.try_into()?).into_review(); diff --git a/kube-core/src/error.rs b/kube-core/src/error.rs index 8e0b5bb31..36c2ac3b8 100644 --- a/kube-core/src/error.rs +++ b/kube-core/src/error.rs @@ -10,11 +10,11 @@ pub enum Error { /// Common error case when requesting parsing into own structs #[error("Error deserializing response")] - SerdeError(#[from] serde_json::Error), + SerdeError(#[source] serde_json::Error), /// Http based error #[error("HttpError: {0}")] - HttpError(#[from] http::Error), + HttpError(#[source] http::Error), /// Invalid GroupVersion #[error("Invalid GroupVersion: {0}")] diff --git a/kube-core/src/params.rs b/kube-core/src/params.rs index 27379d869..c2f53d5de 100644 --- a/kube-core/src/params.rs +++ b/kube-core/src/params.rs @@ -253,7 +253,7 @@ impl Patch { Self::Strategic(p) => serde_json::to_vec(p), Self::Merge(p) => serde_json::to_vec(p), } - .map_err(Into::into) + .map_err(Error::SerdeError) } } diff --git a/kube-core/src/request.rs b/kube-core/src/request.rs index e62d032d1..77f272c30 100644 --- a/kube-core/src/request.rs +++ b/kube-core/src/request.rs @@ -113,7 +113,7 @@ impl Request { let target = format!("{}/{}?", self.url_path, name); let mut qp = form_urlencoded::Serializer::new(target); let urlstr = qp.finish(); - let body = serde_json::to_vec(&dp)?; + let body = serde_json::to_vec(&dp).map_err(Error::SerdeError)?; let req = http::Request::delete(urlstr).header(http::header::CONTENT_TYPE, JSON_MIME); req.body(body).map_err(Error::HttpError) } @@ -129,7 +129,7 @@ impl Request { qp.append_pair("labelSelector", labels); } let urlstr = qp.finish(); - let body = serde_json::to_vec(&dp)?; + let body = serde_json::to_vec(&dp).map_err(Error::SerdeError)?; let req = http::Request::delete(urlstr).header(http::header::CONTENT_TYPE, JSON_MIME); req.body(body).map_err(Error::HttpError) } diff --git a/kube-core/src/subresource.rs b/kube-core/src/subresource.rs index 0d6c0f373..b275fb668 100644 --- a/kube-core/src/subresource.rs +++ b/kube-core/src/subresource.rs @@ -111,7 +111,8 @@ impl Request { let data = serde_json::to_vec(&serde_json::json!({ "delete_options": ep.delete_options, "metadata": { "name": name } - }))?; + })) + .map_err(Error::SerdeError)?; let req = http::Request::post(urlstr).header(http::header::CONTENT_TYPE, JSON_MIME); req.body(data).map_err(Error::HttpError) } From 0c4e039dc8189fd0521d988a1dc01b1d92c04976 Mon Sep 17 00:00:00 2001 From: kazk Date: Fri, 29 Oct 2021 17:45:20 -0700 Subject: [PATCH 2/9] Refine errors for `kube_core::admissions` Signed-off-by: kazk --- kube-core/src/admission.rs | 37 ++++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/kube-core/src/admission.rs b/kube-core/src/admission.rs index e2cbd25f3..0a39c5ebc 100644 --- a/kube-core/src/admission.rs +++ b/kube-core/src/admission.rs @@ -10,7 +10,6 @@ use crate::{ gvk::{GroupVersionKind, GroupVersionResource}, metadata::TypeMeta, resource::Resource, - Error, Result, }; use std::collections::HashMap; @@ -20,6 +19,18 @@ use k8s_openapi::{ apimachinery::pkg::{apis::meta::v1::Status, runtime::RawExtension}, }; use serde::{Deserialize, Serialize}; +use thiserror::Error; + +#[derive(Debug, Error)] +#[error("failed to serialize patch")] +/// Failed to serialize patch. +pub struct SerializePatchError(#[source] serde_json::Error); + +#[derive(Debug, Error)] +#[error("failed to convert AdmissionReview into AdmissionRequest")] +/// Failed to convert `AdmissionReview` into `AdmissionRequest`. +pub struct ConvertAdmissionReviewError; + /// The `kind` field in [`TypeMeta`]. pub const META_KIND: &str = "AdmissionReview"; @@ -46,7 +57,7 @@ pub struct AdmissionReview { } impl TryInto> for AdmissionReview { - type Error = Error; + type Error = ConvertAdmissionReviewError; fn try_into(self) -> Result, Self::Error> { match self.request { @@ -54,9 +65,7 @@ impl TryInto> for AdmissionReview { req.types = self.types; Ok(req) } - None => Err(Error::RequestValidation( - "invalid AdmissionRequest. expected Some but got None".to_owned(), - )), + None => Err(ConvertAdmissionReviewError), } } } @@ -303,8 +312,8 @@ impl AdmissionResponse { } /// Add JSON patches to the response, modifying the object from the request. - pub fn with_patch(mut self, patch: json_patch::Patch) -> Result { - self.patch = Some(serde_json::to_vec(&patch).map_err(Error::SerdeError)?); + pub fn with_patch(mut self, patch: json_patch::Patch) -> Result { + self.patch = Some(serde_json::to_vec(&patch).map_err(SerializePatchError)?); self.patch_type = Some(PatchType::JsonPatch); Ok(self) @@ -334,20 +343,18 @@ mod test { const WEBHOOK_BODY: &str = r#"{"kind":"AdmissionReview","apiVersion":"admission.k8s.io/v1","request":{"uid":"0c9a8d74-9cb7-44dd-b98e-09fd62def2f4","kind":{"group":"","version":"v1","kind":"Pod"},"resource":{"group":"","version":"v1","resource":"pods"},"requestKind":{"group":"","version":"v1","kind":"Pod"},"requestResource":{"group":"","version":"v1","resource":"pods"},"name":"echo-pod","namespace":"colin-coder","operation":"CREATE","userInfo":{"username":"colin@coder.com","groups":["system:authenticated"],"extra":{"iam.gke.io/user-assertion":["REDACTED"],"user-assertion.cloud.google.com":["REDACTED"]}},"object":{"kind":"Pod","apiVersion":"v1","metadata":{"name":"echo-pod","namespace":"colin-coder","creationTimestamp":null,"labels":{"app":"echo-server"},"annotations":{"kubectl.kubernetes.io/last-applied-configuration":"{\"apiVersion\":\"v1\",\"kind\":\"Pod\",\"metadata\":{\"annotations\":{},\"labels\":{\"app\":\"echo-server\"},\"name\":\"echo-pod\",\"namespace\":\"colin-coder\"},\"spec\":{\"containers\":[{\"image\":\"jmalloc/echo-server\",\"name\":\"echo-server\",\"ports\":[{\"containerPort\":8080,\"name\":\"http-port\"}]}]}}\n"},"managedFields":[{"manager":"kubectl","operation":"Update","apiVersion":"v1","time":"2021-03-29T23:02:16Z","fieldsType":"FieldsV1","fieldsV1":{"f:metadata":{"f:annotations":{".":{},"f:kubectl.kubernetes.io/last-applied-configuration":{}},"f:labels":{".":{},"f:app":{}}},"f:spec":{"f:containers":{"k:{\"name\":\"echo-server\"}":{".":{},"f:image":{},"f:imagePullPolicy":{},"f:name":{},"f:ports":{".":{},"k:{\"containerPort\":8080,\"protocol\":\"TCP\"}":{".":{},"f:containerPort":{},"f:name":{},"f:protocol":{}}},"f:resources":{},"f:terminationMessagePath":{},"f:terminationMessagePolicy":{}}},"f:dnsPolicy":{},"f:enableServiceLinks":{},"f:restartPolicy":{},"f:schedulerName":{},"f:securityContext":{},"f:terminationGracePeriodSeconds":{}}}}]},"spec":{"volumes":[{"name":"default-token-rxbqq","secret":{"secretName":"default-token-rxbqq"}}],"containers":[{"name":"echo-server","image":"jmalloc/echo-server","ports":[{"name":"http-port","containerPort":8080,"protocol":"TCP"}],"resources":{},"volumeMounts":[{"name":"default-token-rxbqq","readOnly":true,"mountPath":"/var/run/secrets/kubernetes.io/serviceaccount"}],"terminationMessagePath":"/dev/termination-log","terminationMessagePolicy":"File","imagePullPolicy":"Always"}],"restartPolicy":"Always","terminationGracePeriodSeconds":30,"dnsPolicy":"ClusterFirst","serviceAccountName":"default","serviceAccount":"default","securityContext":{},"schedulerName":"default-scheduler","tolerations":[{"key":"node.kubernetes.io/not-ready","operator":"Exists","effect":"NoExecute","tolerationSeconds":300},{"key":"node.kubernetes.io/unreachable","operator":"Exists","effect":"NoExecute","tolerationSeconds":300}],"priority":0,"enableServiceLinks":true},"status":{}},"oldObject":null,"dryRun":false,"options":{"kind":"CreateOptions","apiVersion":"meta.k8s.io/v1"}}}"#; use crate::{ - admission::{AdmissionResponse, AdmissionReview}, - DynamicObject, Error, Result, + admission::{AdmissionResponse, AdmissionReview, ConvertAdmissionReviewError}, + DynamicObject, }; #[test] - fn v1_webhook_unmarshals() -> Result<()> { - serde_json::from_str::>(WEBHOOK_BODY).map_err(Error::SerdeError)?; - Ok(()) + fn v1_webhook_unmarshals() { + serde_json::from_str::>(WEBHOOK_BODY).unwrap(); } #[test] - fn version_passes_through() -> Result<()> { - let rev = serde_json::from_str::>(WEBHOOK_BODY) - .map_err(Error::SerdeError)?; + fn version_passes_through() -> Result<(), ConvertAdmissionReviewError> { + let rev = serde_json::from_str::>(WEBHOOK_BODY).unwrap(); let rev_typ = rev.types.clone(); let res = AdmissionResponse::from(&rev.try_into()?).into_review(); From 9d020e158b587a72f208562555f8f62b1a8ece18 Mon Sep 17 00:00:00 2001 From: kazk Date: Fri, 29 Oct 2021 17:57:36 -0700 Subject: [PATCH 3/9] Refine error for `kube_core::gvk` Signed-off-by: kazk --- kube-client/src/discovery/apigroup.rs | 6 ++++-- kube-client/src/discovery/parse.rs | 16 ++++++++++++---- kube-client/src/error.rs | 3 --- kube-core/src/error.rs | 4 ---- kube-core/src/gvk.rs | 14 ++++++++++---- 5 files changed, 26 insertions(+), 17 deletions(-) diff --git a/kube-client/src/discovery/apigroup.rs b/kube-client/src/discovery/apigroup.rs index e43e2bdea..1f546351b 100644 --- a/kube-client/src/discovery/apigroup.rs +++ b/kube-client/src/discovery/apigroup.rs @@ -5,7 +5,7 @@ use super::{ use crate::{error::DiscoveryError, Client, Error, Result}; use k8s_openapi::apimachinery::pkg::apis::meta::v1::{APIGroup, APIVersions}; pub use kube_core::discovery::{verbs, ApiCapabilities, ApiResource, Scope}; -use kube_core::gvk::{GroupVersion, GroupVersionKind}; +use kube_core::gvk::{GroupVersion, GroupVersionKind, ParseGroupVersionError}; /// Describes one API groups collected resources and capabilities. @@ -133,7 +133,9 @@ impl ApiGroup { }; for res in &list.resources { if res.kind == gvk.kind && !res.name.contains('/') { - let ar = parse::parse_apiresource(res, &list.group_version)?; + let ar = parse::parse_apiresource(res, &list.group_version).map_err( + |ParseGroupVersionError(s)| Error::Discovery(DiscoveryError::InvalidGroupVersion(s)), + )?; let caps = parse::parse_apicapabilities(&list, &res.name)?; return Ok((ar, caps)); } diff --git a/kube-client/src/discovery/parse.rs b/kube-client/src/discovery/parse.rs index 52c596bb2..683c51311 100644 --- a/kube-client/src/discovery/parse.rs +++ b/kube-client/src/discovery/parse.rs @@ -3,13 +3,16 @@ use crate::{error::DiscoveryError, Error, Result}; use k8s_openapi::apimachinery::pkg::apis::meta::v1::{APIResource, APIResourceList}; use kube_core::{ discovery::{ApiCapabilities, ApiResource, Scope}, - gvk::GroupVersion, + gvk::{GroupVersion, ParseGroupVersionError}, }; /// Creates an `ApiResource` from a `meta::v1::APIResource` instance + its groupversion. /// /// Returns a `DiscoveryError` if the passed group_version cannot be parsed -pub(crate) fn parse_apiresource(ar: &APIResource, group_version: &str) -> Result { +pub(crate) fn parse_apiresource( + ar: &APIResource, + group_version: &str, +) -> Result { let gv: GroupVersion = group_version.parse()?; // NB: not safe to use this with subresources (they don't have api_versions) Ok(ApiResource { @@ -40,7 +43,10 @@ pub(crate) fn parse_apicapabilities(list: &APIResourceList, name: &str) -> Resul let mut subresources = vec![]; for res in &list.resources { if let Some(subresource_name) = res.name.strip_prefix(&subresource_name_prefix) { - let mut api_resource = parse_apiresource(res, &list.group_version)?; + let mut api_resource = + parse_apiresource(res, &list.group_version).map_err(|ParseGroupVersionError(s)| { + Error::Discovery(DiscoveryError::InvalidGroupVersion(s)) + })?; api_resource.plural = subresource_name.to_string(); let caps = parse_apicapabilities(list, &res.name)?; // NB: recursion subresources.push((api_resource, caps)); @@ -71,7 +77,9 @@ impl GroupVersionData { continue; } // NB: these two should be infallible from discovery when k8s api is well-behaved, but.. - let ar = parse_apiresource(res, &list.group_version)?; + let ar = parse_apiresource(res, &list.group_version).map_err(|ParseGroupVersionError(s)| { + Error::Discovery(DiscoveryError::InvalidGroupVersion(s)) + })?; let caps = parse_apicapabilities(&list, &res.name)?; resources.push((ar, caps)); } diff --git a/kube-client/src/error.rs b/kube-client/src/error.rs index 89bd912be..d3d0754d9 100644 --- a/kube-client/src/error.rs +++ b/kube-client/src/error.rs @@ -280,9 +280,6 @@ impl From for Error { kube_core::Error::RequestValidation(s) => Error::RequestValidation(s), kube_core::Error::SerdeError(e) => Error::SerdeError(e), kube_core::Error::HttpError(e) => Error::HttpError(e), - kube_core::Error::InvalidGroupVersion(s) => { - Error::Discovery(DiscoveryError::InvalidGroupVersion(s)) - } } } } diff --git a/kube-core/src/error.rs b/kube-core/src/error.rs index 36c2ac3b8..65a4df701 100644 --- a/kube-core/src/error.rs +++ b/kube-core/src/error.rs @@ -15,10 +15,6 @@ pub enum Error { /// Http based error #[error("HttpError: {0}")] HttpError(#[source] http::Error), - - /// Invalid GroupVersion - #[error("Invalid GroupVersion: {0}")] - InvalidGroupVersion(String), } /// An error response from the API. diff --git a/kube-core/src/gvk.rs b/kube-core/src/gvk.rs index 4d2f2136f..4c8a3ef0e 100644 --- a/kube-core/src/gvk.rs +++ b/kube-core/src/gvk.rs @@ -1,8 +1,14 @@ //! Type information structs for dynamic resources. -use crate::Error; -use serde::{Deserialize, Serialize}; use std::str::FromStr; +use serde::{Deserialize, Serialize}; +use thiserror::Error; + +#[derive(Debug, Error)] +#[error("failed to parse group version: {0}")] +/// Failed to parse group version. +pub struct ParseGroupVersionError(pub String); + /// Core information about an API Resource. #[derive(Deserialize, Serialize, Debug, Clone, PartialEq, Eq, Hash)] pub struct GroupVersionKind { @@ -44,14 +50,14 @@ impl GroupVersion { } impl FromStr for GroupVersion { - type Err = Error; + type Err = ParseGroupVersionError; fn from_str(gv: &str) -> Result { let gvsplit = gv.splitn(2, '/').collect::>(); let (group, version) = match *gvsplit.as_slice() { [g, v] => (g.to_string(), v.to_string()), // standard case [v] => ("".to_string(), v.to_string()), // core v1 case - _ => return Err(Error::InvalidGroupVersion(gv.into())), + _ => return Err(ParseGroupVersionError(gv.into())), }; Ok(Self { group, version }) } From 15657e198a6a4e5166051670f04b7805ee22856b Mon Sep 17 00:00:00 2001 From: kazk Date: Fri, 29 Oct 2021 19:27:12 -0700 Subject: [PATCH 4/9] Refine errors for `kube_core::request` Signed-off-by: kazk --- kube-client/src/api/core_methods.rs | 22 +++++--- kube-client/src/api/subresource.rs | 45 +++++++++++---- kube-client/src/api/util.rs | 4 +- kube-client/src/error.rs | 23 +++----- kube-core/src/dynamic.rs | 6 +- kube-core/src/error.rs | 16 ------ kube-core/src/lib.rs | 5 +- kube-core/src/params.rs | 21 +++---- kube-core/src/request.rs | 86 +++++++++++++++++++---------- kube-core/src/subresource.rs | 32 ++++++----- kube-core/src/util.rs | 4 +- 11 files changed, 145 insertions(+), 119 deletions(-) diff --git a/kube-client/src/api/core_methods.rs b/kube-client/src/api/core_methods.rs index e7531c2c9..d4d2bb9d8 100644 --- a/kube-client/src/api/core_methods.rs +++ b/kube-client/src/api/core_methods.rs @@ -25,7 +25,7 @@ where /// } /// ``` pub async fn get(&self, name: &str) -> Result { - let mut req = self.request.get(name)?; + let mut req = self.request.get(name).map_err(Error::BuildRequest)?; req.extensions_mut().insert("get"); self.client.request::(req).await } @@ -49,7 +49,7 @@ where /// } /// ``` pub async fn list(&self, lp: &ListParams) -> Result> { - let mut req = self.request.list(lp)?; + let mut req = self.request.list(lp).map_err(Error::BuildRequest)?; req.extensions_mut().insert("list"); self.client.request::>(req).await } @@ -75,7 +75,7 @@ where K: Serialize, { let bytes = serde_json::to_vec(&data).map_err(Error::SerdeError)?; - let mut req = self.request.create(pp, bytes)?; + let mut req = self.request.create(pp, bytes).map_err(Error::BuildRequest)?; req.extensions_mut().insert("create"); self.client.request::(req).await } @@ -103,7 +103,7 @@ where /// } /// ``` pub async fn delete(&self, name: &str, dp: &DeleteParams) -> Result> { - let mut req = self.request.delete(name, dp)?; + let mut req = self.request.delete(name, dp).map_err(Error::BuildRequest)?; req.extensions_mut().insert("delete"); self.client.request_status::(req).await } @@ -140,7 +140,10 @@ where dp: &DeleteParams, lp: &ListParams, ) -> Result, Status>> { - let mut req = self.request.delete_collection(dp, lp)?; + let mut req = self + .request + .delete_collection(dp, lp) + .map_err(Error::BuildRequest)?; req.extensions_mut().insert("delete_collection"); self.client.request_status::>(req).await } @@ -180,7 +183,7 @@ where pp: &PatchParams, patch: &Patch

, ) -> Result { - let mut req = self.request.patch(name, pp, patch)?; + let mut req = self.request.patch(name, pp, patch).map_err(Error::BuildRequest)?; req.extensions_mut().insert("patch"); self.client.request::(req).await } @@ -234,7 +237,10 @@ where K: Serialize, { let bytes = serde_json::to_vec(&data).map_err(Error::SerdeError)?; - let mut req = self.request.replace(name, pp, bytes)?; + let mut req = self + .request + .replace(name, pp, bytes) + .map_err(Error::BuildRequest)?; req.extensions_mut().insert("replace"); self.client.request::(req).await } @@ -281,7 +287,7 @@ where lp: &ListParams, version: &str, ) -> Result>>> { - let mut req = self.request.watch(lp, version)?; + let mut req = self.request.watch(lp, version).map_err(Error::BuildRequest)?; req.extensions_mut().insert("watch"); self.client.request_events::(req).await } diff --git a/kube-client/src/api/subresource.rs b/kube-client/src/api/subresource.rs index 1067eec35..e0be1c1cf 100644 --- a/kube-client/src/api/subresource.rs +++ b/kube-client/src/api/subresource.rs @@ -5,7 +5,7 @@ use std::fmt::Debug; use crate::{ api::{Api, Patch, PatchParams, PostParams}, - Result, + Error, Result, }; use kube_core::response::Status; @@ -26,7 +26,10 @@ where { /// Fetch the scale subresource pub async fn get_scale(&self, name: &str) -> Result { - let mut req = self.request.get_subresource("scale", name)?; + let mut req = self + .request + .get_subresource("scale", name) + .map_err(Error::BuildRequest)?; req.extensions_mut().insert("get_scale"); self.client.request::(req).await } @@ -38,14 +41,20 @@ where pp: &PatchParams, patch: &Patch

, ) -> Result { - let mut req = self.request.patch_subresource("scale", name, pp, patch)?; + let mut req = self + .request + .patch_subresource("scale", name, pp, patch) + .map_err(Error::BuildRequest)?; req.extensions_mut().insert("patch_scale"); self.client.request::(req).await } /// Replace the scale subresource pub async fn replace_scale(&self, name: &str, pp: &PostParams, data: Vec) -> Result { - let mut req = self.request.replace_subresource("scale", name, pp, data)?; + let mut req = self + .request + .replace_subresource("scale", name, pp, data) + .map_err(Error::BuildRequest)?; req.extensions_mut().insert("replace_scale"); self.client.request::(req).await } @@ -64,7 +73,10 @@ where /// /// This actually returns the whole K, with metadata, and spec. pub async fn get_status(&self, name: &str) -> Result { - let mut req = self.request.get_subresource("status", name)?; + let mut req = self + .request + .get_subresource("status", name) + .map_err(Error::BuildRequest)?; req.extensions_mut().insert("get_status"); self.client.request::(req).await } @@ -98,7 +110,10 @@ where pp: &PatchParams, patch: &Patch

, ) -> Result { - let mut req = self.request.patch_subresource("status", name, pp, patch)?; + let mut req = self + .request + .patch_subresource("status", name, pp, patch) + .map_err(Error::BuildRequest)?; req.extensions_mut().insert("patch_status"); self.client.request::(req).await } @@ -123,7 +138,10 @@ where /// } /// ``` pub async fn replace_status(&self, name: &str, pp: &PostParams, data: Vec) -> Result { - let mut req = self.request.replace_subresource("status", name, pp, data)?; + let mut req = self + .request + .replace_subresource("status", name, pp, data) + .map_err(Error::BuildRequest)?; req.extensions_mut().insert("replace_status"); self.client.request::(req).await } @@ -157,14 +175,14 @@ where { /// Fetch logs as a string pub async fn logs(&self, name: &str, lp: &LogParams) -> Result { - let mut req = self.request.logs(name, lp)?; + let mut req = self.request.logs(name, lp).map_err(Error::BuildRequest)?; req.extensions_mut().insert("logs"); self.client.request_text(req).await } /// Fetch logs as a stream of bytes pub async fn log_stream(&self, name: &str, lp: &LogParams) -> Result>> { - let mut req = self.request.logs(name, lp)?; + let mut req = self.request.logs(name, lp).map_err(Error::BuildRequest)?; req.extensions_mut().insert("log_stream"); self.client.request_text_stream(req).await } @@ -195,7 +213,7 @@ where { /// Create an eviction pub async fn evict(&self, name: &str, ep: &EvictParams) -> Result { - let mut req = self.request.evict(name, ep)?; + let mut req = self.request.evict(name, ep).map_err(Error::BuildRequest)?; req.extensions_mut().insert("evict"); self.client.request::(req).await } @@ -239,7 +257,7 @@ where { /// Attach to pod pub async fn attach(&self, name: &str, ap: &AttachParams) -> Result { - let mut req = self.request.attach(name, ap)?; + let mut req = self.request.attach(name, ap).map_err(Error::BuildRequest)?; req.extensions_mut().insert("attach"); let stream = self.client.connect(req).await?; Ok(AttachedProcess::new(stream, ap)) @@ -294,7 +312,10 @@ where I: IntoIterator, T: Into, { - let mut req = self.request.exec(name, command, ap)?; + let mut req = self + .request + .exec(name, command, ap) + .map_err(Error::BuildRequest)?; req.extensions_mut().insert("exec"); let stream = self.client.connect(req).await?; Ok(AttachedProcess::new(stream, ap)) diff --git a/kube-client/src/api/util.rs b/kube-client/src/api/util.rs index 6554a3f96..413f6f4bb 100644 --- a/kube-client/src/api/util.rs +++ b/kube-client/src/api/util.rs @@ -1,6 +1,6 @@ use crate::{ api::{Api, Resource}, - Result, + Error, Result, }; use kube_core::util::Restart; use serde::de::DeserializeOwned; @@ -11,7 +11,7 @@ where { /// Trigger a restart of a Resource. pub async fn restart(&self, name: &str) -> Result { - let mut req = self.request.restart(name)?; + let mut req = self.request.restart(name).map_err(Error::BuildRequest)?; req.extensions_mut().insert("restart"); self.client.request::(req).await } diff --git a/kube-client/src/error.rs b/kube-client/src/error.rs index d3d0754d9..56fe2d92f 100644 --- a/kube-client/src/error.rs +++ b/kube-client/src/error.rs @@ -1,9 +1,11 @@ //! Error handling in [`kube`][crate] -use http::header::InvalidHeaderValue; -pub use kube_core::ErrorResponse; use std::path::PathBuf; + +use http::header::InvalidHeaderValue; use thiserror::Error; +pub use kube_core::ErrorResponse; + /// Possible errors when working with [`kube`][crate] #[cfg_attr(docsrs, doc(cfg(any(feature = "config", feature = "client"))))] #[derive(Error, Debug)] @@ -68,9 +70,9 @@ pub enum Error { #[error("Error parsing response")] RequestParse, - /// A request validation failed - #[error("Request validation failed with {0}")] - RequestValidation(String), + /// Failed to build request + #[error("Failed to build request: {0}")] + BuildRequest(#[source] kube_core::request::Error), /// Configuration error #[error("Error loading kubeconfig: {0}")] @@ -272,14 +274,3 @@ pub enum DiscoveryError { #[error("Empty Api Group: {0}")] EmptyApiGroup(String), } - -// TODO Remove this -impl From for Error { - fn from(error: kube_core::Error) -> Self { - match error { - kube_core::Error::RequestValidation(s) => Error::RequestValidation(s), - kube_core::Error::SerdeError(e) => Error::SerdeError(e), - kube_core::Error::HttpError(e) => Error::HttpError(e), - } - } -} diff --git a/kube-core/src/dynamic.rs b/kube-core/src/dynamic.rs index 65b43fb7c..4e6fdb9da 100644 --- a/kube-core/src/dynamic.rs +++ b/kube-core/src/dynamic.rs @@ -92,7 +92,6 @@ mod test { params::{Patch, PatchParams, PostParams}, request::Request, resource::Resource, - Result, }; #[test] fn raw_custom_resource() { @@ -112,14 +111,13 @@ mod test { } #[test] - fn raw_resource_in_default_group() -> Result<()> { + fn raw_resource_in_default_group() { let gvk = GroupVersionKind::gvk("", "v1", "Service"); let api_resource = ApiResource::from_gvk(&gvk); let url = DynamicObject::url_path(&api_resource, None); let pp = PostParams::default(); - let req = Request::new(url).create(&pp, vec![])?; + let req = Request::new(url).create(&pp, vec![]).unwrap(); assert_eq!(req.uri(), "/api/v1/services?"); - Ok(()) } #[cfg(feature = "derive")] diff --git a/kube-core/src/error.rs b/kube-core/src/error.rs index 65a4df701..c65d27981 100644 --- a/kube-core/src/error.rs +++ b/kube-core/src/error.rs @@ -1,22 +1,6 @@ use serde::{Deserialize, Serialize}; use thiserror::Error; -/// Core error types. -#[derive(Error, Debug)] -pub enum Error { - /// A request validation failed - #[error("Request validation failed with {0}")] - RequestValidation(String), - - /// Common error case when requesting parsing into own structs - #[error("Error deserializing response")] - SerdeError(#[source] serde_json::Error), - - /// Http based error - #[error("HttpError: {0}")] - HttpError(#[source] http::Error), -} - /// An error response from the API. #[derive(Error, Deserialize, Serialize, Debug, Clone, Eq, PartialEq)] #[error("{message}: {reason}")] diff --git a/kube-core/src/lib.rs b/kube-core/src/lib.rs index 162dbd448..a16472c31 100644 --- a/kube-core/src/lib.rs +++ b/kube-core/src/lib.rs @@ -50,7 +50,4 @@ pub mod watch; pub use watch::WatchEvent; mod error; -pub use error::{Error, ErrorResponse}; - -/// Convient alias for `Result` -pub type Result = std::result::Result; +pub use error::ErrorResponse; diff --git a/kube-core/src/params.rs b/kube-core/src/params.rs index c2f53d5de..a46be7b2e 100644 --- a/kube-core/src/params.rs +++ b/kube-core/src/params.rs @@ -1,5 +1,5 @@ //! A port of request parameter *Optionals from apimachinery/types.go -use crate::{Error, Result}; +use crate::request::Error; use serde::Serialize; /// Common query parameters used in watch/list/delete calls on collections @@ -63,13 +63,11 @@ impl Default for ListParams { } impl ListParams { - pub(crate) fn validate(&self) -> Result<()> { + pub(crate) fn validate(&self) -> Result<(), Error> { if let Some(to) = &self.timeout { // https://github.com/kubernetes/kubernetes/issues/6513 if *to >= 295 { - return Err(Error::RequestValidation( - "ListParams::timeout must be < 295s".into(), - )); + return Err(Error::Validation("ListParams::timeout must be < 295s".into())); } } Ok(()) @@ -146,12 +144,12 @@ pub struct PostParams { } impl PostParams { - pub(crate) fn validate(&self) -> Result<()> { + pub(crate) fn validate(&self) -> Result<(), Error> { if let Some(field_manager) = &self.field_manager { // Implement the easy part of validation, in future this may be extended to provide validation as in go code // For now it's fine, because k8s API server will return an error if field_manager.len() > 128 { - return Err(Error::RequestValidation( + return Err(Error::Validation( "Failed to validate PostParams::field_manager!".into(), )); } @@ -244,7 +242,7 @@ impl Patch { } impl Patch { - pub(crate) fn serialize(&self) -> Result> { + pub(crate) fn serialize(&self) -> Result, serde_json::Error> { match self { Self::Apply(p) => serde_json::to_vec(p), #[cfg(feature = "jsonpatch")] @@ -253,7 +251,6 @@ impl Patch { Self::Strategic(p) => serde_json::to_vec(p), Self::Merge(p) => serde_json::to_vec(p), } - .map_err(Error::SerdeError) } } @@ -270,18 +267,18 @@ pub struct PatchParams { } impl PatchParams { - pub(crate) fn validate(&self, patch: &Patch

) -> Result<()> { + pub(crate) fn validate(&self, patch: &Patch

) -> Result<(), Error> { if let Some(field_manager) = &self.field_manager { // Implement the easy part of validation, in future this may be extended to provide validation as in go code // For now it's fine, because k8s API server will return an error if field_manager.len() > 128 { - return Err(Error::RequestValidation( + return Err(Error::Validation( "Failed to validate PatchParams::field_manager!".into(), )); } } if self.force && !patch.is_apply() { - return Err(Error::RequestValidation( + return Err(Error::Validation( "PatchParams::force only works with Patch::Apply".into(), )); } diff --git a/kube-core/src/request.rs b/kube-core/src/request.rs index 77f272c30..532f3b6af 100644 --- a/kube-core/src/request.rs +++ b/kube-core/src/request.rs @@ -1,9 +1,24 @@ //! Request builder type for arbitrary api types +use thiserror::Error; + use super::params::{DeleteParams, ListParams, Patch, PatchParams, PostParams}; -use crate::{Error, Result}; pub(crate) const JSON_MIME: &str = "application/json"; +/// Possible errors when building a request. +#[derive(Debug, Error)] +pub enum Error { + /// Failed to build a request. + #[error("failed to build request: {0}")] + BuildRequest(#[source] http::Error), + /// Failed to serialize body. + #[error("failed to serialize body: {0}")] + SerializeBody(#[source] serde_json::Error), + /// Failed to validate request. + #[error("failed to validate request")] + Validation(String), +} + /// A Kubernetes request builder /// /// Takes a base_path and supplies constructors for common operations @@ -28,7 +43,7 @@ impl Request { /// Convenience methods found from API conventions impl Request { /// List a collection of a resource - pub fn list(&self, lp: &ListParams) -> Result>> { + pub fn list(&self, lp: &ListParams) -> Result>, Error> { let target = format!("{}?", self.url_path); let mut qp = form_urlencoded::Serializer::new(target); @@ -47,21 +62,21 @@ impl Request { let urlstr = qp.finish(); let req = http::Request::get(urlstr); - req.body(vec![]).map_err(Error::HttpError) + req.body(vec![]).map_err(Error::BuildRequest) } /// Watch a resource at a given version - pub fn watch(&self, lp: &ListParams, ver: &str) -> Result>> { + pub fn watch(&self, lp: &ListParams, ver: &str) -> Result>, Error> { let target = format!("{}?", self.url_path); let mut qp = form_urlencoded::Serializer::new(target); lp.validate()?; if lp.limit.is_some() { - return Err(Error::RequestValidation( + return Err(Error::Validation( "ListParams::limit cannot be used with a watch.".into(), )); } if lp.continue_token.is_some() { - return Err(Error::RequestValidation( + return Err(Error::Validation( "ListParams::continue_token cannot be used with a watch.".into(), )); } @@ -83,20 +98,20 @@ impl Request { let urlstr = qp.finish(); let req = http::Request::get(urlstr); - req.body(vec![]).map_err(Error::HttpError) + req.body(vec![]).map_err(Error::BuildRequest) } /// Get a single instance - pub fn get(&self, name: &str) -> Result>> { + pub fn get(&self, name: &str) -> Result>, Error> { let target = format!("{}/{}", self.url_path, name); 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) + req.body(vec![]).map_err(Error::BuildRequest) } /// Create an instance of a resource - pub fn create(&self, pp: &PostParams, data: Vec) -> Result>> { + pub fn create(&self, pp: &PostParams, data: Vec) -> Result>, Error> { pp.validate()?; let target = format!("{}?", self.url_path); let mut qp = form_urlencoded::Serializer::new(target); @@ -105,21 +120,25 @@ impl Request { } let urlstr = qp.finish(); let req = http::Request::post(urlstr).header(http::header::CONTENT_TYPE, JSON_MIME); - req.body(data).map_err(Error::HttpError) + req.body(data).map_err(Error::BuildRequest) } /// Delete an instance of a resource - pub fn delete(&self, name: &str, dp: &DeleteParams) -> Result>> { + pub fn delete(&self, name: &str, dp: &DeleteParams) -> Result>, Error> { let target = format!("{}/{}?", self.url_path, name); let mut qp = form_urlencoded::Serializer::new(target); let urlstr = qp.finish(); - let body = serde_json::to_vec(&dp).map_err(Error::SerdeError)?; + let body = serde_json::to_vec(&dp).map_err(Error::SerializeBody)?; let req = http::Request::delete(urlstr).header(http::header::CONTENT_TYPE, JSON_MIME); - req.body(body).map_err(Error::HttpError) + req.body(body).map_err(Error::BuildRequest) } /// Delete a collection of a resource - pub fn delete_collection(&self, dp: &DeleteParams, lp: &ListParams) -> Result>> { + pub fn delete_collection( + &self, + dp: &DeleteParams, + lp: &ListParams, + ) -> Result>, Error> { let target = format!("{}?", self.url_path); let mut qp = form_urlencoded::Serializer::new(target); if let Some(fields) = &lp.field_selector { @@ -129,9 +148,9 @@ impl Request { qp.append_pair("labelSelector", labels); } let urlstr = qp.finish(); - let body = serde_json::to_vec(&dp).map_err(Error::SerdeError)?; + let body = serde_json::to_vec(&dp).map_err(Error::SerializeBody)?; let req = http::Request::delete(urlstr).header(http::header::CONTENT_TYPE, JSON_MIME); - req.body(body).map_err(Error::HttpError) + req.body(body).map_err(Error::BuildRequest) } /// Patch an instance of a resource @@ -142,7 +161,7 @@ impl Request { name: &str, pp: &PatchParams, patch: &Patch

, - ) -> Result>> { + ) -> Result>, Error> { pp.validate(patch)?; let target = format!("{}/{}?", self.url_path, name); let mut qp = form_urlencoded::Serializer::new(target); @@ -152,14 +171,19 @@ impl Request { http::Request::patch(urlstr) .header(http::header::ACCEPT, JSON_MIME) .header(http::header::CONTENT_TYPE, patch.content_type()) - .body(patch.serialize()?) - .map_err(Error::HttpError) + .body(patch.serialize().map_err(Error::SerializeBody)?) + .map_err(Error::BuildRequest) } /// Replace an instance of a resource /// /// Requires `metadata.resourceVersion` set in data - pub fn replace(&self, name: &str, pp: &PostParams, data: Vec) -> Result>> { + pub fn replace( + &self, + name: &str, + pp: &PostParams, + data: Vec, + ) -> Result>, Error> { let target = format!("{}/{}?", self.url_path, name); let mut qp = form_urlencoded::Serializer::new(target); if pp.dry_run { @@ -167,19 +191,23 @@ impl Request { } let urlstr = qp.finish(); let req = http::Request::put(urlstr).header(http::header::CONTENT_TYPE, JSON_MIME); - req.body(data).map_err(Error::HttpError) + req.body(data).map_err(Error::BuildRequest) } } /// Subresources impl Request { /// Get an instance of the subresource - pub fn get_subresource(&self, subresource_name: &str, name: &str) -> Result>> { + pub fn get_subresource( + &self, + subresource_name: &str, + name: &str, + ) -> Result>, Error> { let target = format!("{}/{}/{}", self.url_path, name, subresource_name); 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) + req.body(vec![]).map_err(Error::BuildRequest) } /// Patch an instance of the subresource @@ -189,7 +217,7 @@ impl Request { name: &str, pp: &PatchParams, patch: &Patch

, - ) -> Result>> { + ) -> Result>, Error> { pp.validate(patch)?; let target = format!("{}/{}/{}?", self.url_path, name, subresource_name); let mut qp = form_urlencoded::Serializer::new(target); @@ -199,8 +227,8 @@ impl Request { http::Request::patch(urlstr) .header(http::header::ACCEPT, JSON_MIME) .header(http::header::CONTENT_TYPE, patch.content_type()) - .body(patch.serialize()?) - .map_err(Error::HttpError) + .body(patch.serialize().map_err(Error::SerializeBody)?) + .map_err(Error::BuildRequest) } /// Replace an instance of the subresource @@ -210,7 +238,7 @@ impl Request { name: &str, pp: &PostParams, data: Vec, - ) -> Result>> { + ) -> Result>, Error> { let target = format!("{}/{}/{}?", self.url_path, name, subresource_name); let mut qp = form_urlencoded::Serializer::new(target); if pp.dry_run { @@ -218,7 +246,7 @@ impl Request { } let urlstr = qp.finish(); let req = http::Request::put(urlstr).header(http::header::CONTENT_TYPE, JSON_MIME); - req.body(data).map_err(Error::HttpError) + req.body(data).map_err(Error::BuildRequest) } } diff --git a/kube-core/src/subresource.rs b/kube-core/src/subresource.rs index b275fb668..86326a4f0 100644 --- a/kube-core/src/subresource.rs +++ b/kube-core/src/subresource.rs @@ -3,8 +3,7 @@ use std::fmt::Debug; use crate::{ params::{DeleteParams, PostParams}, - request::{Request, JSON_MIME}, - Error, Result, + request::{Error, Request, JSON_MIME}, }; pub use k8s_openapi::api::autoscaling::v1::{Scale, ScaleSpec, ScaleStatus}; @@ -40,7 +39,7 @@ pub struct LogParams { impl Request { /// Get a pod logs - pub fn logs(&self, name: &str, lp: &LogParams) -> Result>> { + pub fn logs(&self, name: &str, lp: &LogParams) -> Result>, Error> { let target = format!("{}/{}/log?", self.url_path, name); let mut qp = form_urlencoded::Serializer::new(target); @@ -78,7 +77,7 @@ impl Request { let urlstr = qp.finish(); let req = http::Request::get(urlstr); - req.body(vec![]).map_err(Error::HttpError) + req.body(vec![]).map_err(Error::BuildRequest) } } @@ -97,7 +96,7 @@ pub struct EvictParams { impl Request { /// Create an eviction - pub fn evict(&self, name: &str, ep: &EvictParams) -> Result>> { + pub fn evict(&self, name: &str, ep: &EvictParams) -> Result>, Error> { let target = format!("{}/{}/eviction?", self.url_path, name); // This is technically identical to Request::create, but different url let pp = &ep.post_options; @@ -112,9 +111,9 @@ impl Request { "delete_options": ep.delete_options, "metadata": { "name": name } })) - .map_err(Error::SerdeError)?; + .map_err(Error::SerializeBody)?; let req = http::Request::post(urlstr).header(http::header::CONTENT_TYPE, JSON_MIME); - req.body(data).map_err(Error::HttpError) + req.body(data).map_err(Error::BuildRequest) } } @@ -249,16 +248,16 @@ impl AttachParams { self } - fn validate(&self) -> Result<()> { + fn validate(&self) -> Result<(), Error> { if !self.stdin && !self.stdout && !self.stderr { - return Err(Error::RequestValidation( + return Err(Error::Validation( "AttachParams: one of stdin, stdout, or stderr must be true".into(), )); } if self.stderr && self.tty { // Multiplexing is not supported with TTY - return Err(Error::RequestValidation( + return Err(Error::Validation( "AttachParams: tty and stderr cannot both be true".into(), )); } @@ -289,7 +288,7 @@ impl AttachParams { #[cfg_attr(docsrs, doc(cfg(feature = "ws")))] impl Request { /// Attach to a pod - pub fn attach(&self, name: &str, ap: &AttachParams) -> Result>> { + pub fn attach(&self, name: &str, ap: &AttachParams) -> Result>, Error> { ap.validate()?; let target = format!("{}/{}/attach?", self.url_path, name); @@ -297,7 +296,7 @@ impl Request { ap.append_to_url_serializer(&mut qp); let req = http::Request::get(qp.finish()); - req.body(vec![]).map_err(Error::HttpError) + req.body(vec![]).map_err(Error::BuildRequest) } } @@ -308,7 +307,12 @@ impl Request { #[cfg_attr(docsrs, doc(cfg(feature = "ws")))] impl Request { /// Execute command in a pod - pub fn exec(&self, name: &str, command: I, ap: &AttachParams) -> Result>> + pub fn exec( + &self, + name: &str, + command: I, + ap: &AttachParams, + ) -> Result>, Error> where I: IntoIterator, T: Into, @@ -324,6 +328,6 @@ impl Request { } let req = http::Request::get(qp.finish()); - req.body(vec![]).map_err(Error::HttpError) + req.body(vec![]).map_err(Error::BuildRequest) } } diff --git a/kube-core/src/util.rs b/kube-core/src/util.rs index 171a94096..02d260c60 100644 --- a/kube-core/src/util.rs +++ b/kube-core/src/util.rs @@ -2,7 +2,7 @@ use crate::{ params::{Patch, PatchParams}, - Request, Result, + request, Request, }; use chrono::Utc; use k8s_openapi::api::apps::v1::{DaemonSet, Deployment, ReplicaSet, StatefulSet}; @@ -17,7 +17,7 @@ impl Restart for ReplicaSet {} impl Request { /// Restart a resource - pub fn restart(&self, name: &str) -> Result>> { + pub fn restart(&self, name: &str) -> Result>, request::Error> { let patch = serde_json::json!({ "spec": { "template": { From d0df55bde381dd4b271e18b1d0d29ac8157ae49a Mon Sep 17 00:00:00 2001 From: kazk Date: Fri, 29 Oct 2021 23:15:54 -0700 Subject: [PATCH 5/9] Fix doctests Signed-off-by: kazk --- kube-client/src/api/core_methods.rs | 14 +++++++------- kube-client/src/api/subresource.rs | 4 ++-- kube-client/src/discovery/apigroup.rs | 8 ++++---- kube-client/src/discovery/mod.rs | 2 +- kube-client/src/discovery/oneshot.rs | 6 +++--- kube-client/src/lib.rs | 2 +- kube-runtime/src/controller/mod.rs | 2 +- kube/src/lib.rs | 2 +- 8 files changed, 20 insertions(+), 20 deletions(-) diff --git a/kube-client/src/api/core_methods.rs b/kube-client/src/api/core_methods.rs index d4d2bb9d8..609059aaa 100644 --- a/kube-client/src/api/core_methods.rs +++ b/kube-client/src/api/core_methods.rs @@ -17,7 +17,7 @@ where /// use kube::{Api, Client}; /// use k8s_openapi::api::core::v1::Pod; /// #[tokio::main] - /// async fn main() -> Result<(), kube::Error> { + /// async fn main() -> Result<(), Box> { /// let client = Client::try_default().await?; /// let pods: Api = Api::namespaced(client, "apps"); /// let p: Pod = pods.get("blog").await?; @@ -38,7 +38,7 @@ where /// use kube::{api::{Api, ListParams, ResourceExt}, Client}; /// use k8s_openapi::api::core::v1::Pod; /// #[tokio::main] - /// async fn main() -> Result<(), kube::Error> { + /// async fn main() -> Result<(), Box> { /// let client = Client::try_default().await?; /// let pods: Api = Api::namespaced(client, "apps"); /// let lp = ListParams::default().labels("app=blog"); // for this app only @@ -93,7 +93,7 @@ where /// use k8s_openapi::apiextensions_apiserver::pkg::apis::apiextensions::v1 as apiexts; /// use apiexts::CustomResourceDefinition; /// #[tokio::main] - /// async fn main() -> Result<(), kube::Error> { + /// async fn main() -> Result<(), Box> { /// let client = Client::try_default().await?; /// let crds: Api = Api::all(client); /// crds.delete("foos.clux.dev", &DeleteParams::default()).await? @@ -120,7 +120,7 @@ where /// use kube::{api::{Api, DeleteParams, ListParams, ResourceExt}, Client}; /// use k8s_openapi::api::core::v1::Pod; /// #[tokio::main] - /// async fn main() -> Result<(), kube::Error> { + /// async fn main() -> Result<(), Box> { /// let client = Client::try_default().await?; /// let pods: Api = Api::namespaced(client, "apps"); /// match pods.delete_collection(&DeleteParams::default(), &ListParams::default()).await? { @@ -156,7 +156,7 @@ where /// use kube::{api::{Api, PatchParams, Patch, Resource}, Client}; /// use k8s_openapi::api::core::v1::Pod; /// #[tokio::main] - /// async fn main() -> Result<(), kube::Error> { + /// async fn main() -> Result<(), Box> { /// let client = Client::try_default().await?; /// let pods: Api = Api::namespaced(client, "apps"); /// let patch = serde_json::json!({ @@ -200,7 +200,7 @@ where /// use kube::{api::{Api, PostParams, ResourceExt}, Client}; /// use k8s_openapi::api::batch::v1::Job; /// #[tokio::main] - /// async fn main() -> Result<(), kube::Error> { + /// async fn main() -> Result<(), Box> { /// let client = Client::try_default().await?; /// let jobs: Api = Api::namespaced(client, "apps"); /// let j = jobs.get("baz").await?; @@ -261,7 +261,7 @@ where /// use k8s_openapi::api::batch::v1::Job; /// use futures::{StreamExt, TryStreamExt}; /// #[tokio::main] - /// async fn main() -> Result<(), kube::Error> { + /// async fn main() -> Result<(), Box> { /// let client = Client::try_default().await?; /// let jobs: Api = Api::namespaced(client, "apps"); /// let lp = ListParams::default() diff --git a/kube-client/src/api/subresource.rs b/kube-client/src/api/subresource.rs index e0be1c1cf..d05a037ca 100644 --- a/kube-client/src/api/subresource.rs +++ b/kube-client/src/api/subresource.rs @@ -89,7 +89,7 @@ where /// use kube::{api::{Api, PatchParams, Patch}, Client}; /// use k8s_openapi::api::batch::v1::Job; /// #[tokio::main] - /// async fn main() -> Result<(), kube::Error> { + /// async fn main() -> Result<(), Box> { /// let client = Client::try_default().await?; /// let jobs: Api = Api::namespaced(client, "apps"); /// let mut j = jobs.get("baz").await?; @@ -127,7 +127,7 @@ where /// use kube::{api::{Api, PostParams}, Client}; /// use k8s_openapi::api::batch::v1::{Job, JobStatus}; /// #[tokio::main] - /// async fn main() -> Result<(), kube::Error> { + /// async fn main() -> Result<(), Box> { /// let client = Client::try_default().await?; /// let jobs: Api = Api::namespaced(client, "apps"); /// let mut o = jobs.get_status("baz").await?; // retrieve partial object diff --git a/kube-client/src/discovery/apigroup.rs b/kube-client/src/discovery/apigroup.rs index 1f546351b..db1843804 100644 --- a/kube-client/src/discovery/apigroup.rs +++ b/kube-client/src/discovery/apigroup.rs @@ -18,7 +18,7 @@ use kube_core::gvk::{GroupVersion, GroupVersionKind, ParseGroupVersionError}; /// ```no_run /// use kube::{Client, api::{Api, DynamicObject}, discovery, ResourceExt}; /// #[tokio::main] -/// async fn main() -> Result<(), kube::Error> { +/// async fn main() -> Result<(), Box> { /// let client = Client::try_default().await?; /// let apigroup = discovery::group(&client, "apiregistration.k8s.io").await?; /// for (apiresource, caps) in apigroup.versioned_resources("v1") { @@ -42,7 +42,7 @@ use kube_core::gvk::{GroupVersion, GroupVersionKind, ParseGroupVersionError}; /// ```no_run /// use kube::{Client, api::{Api, DynamicObject}, discovery, ResourceExt}; /// #[tokio::main] -/// async fn main() -> Result<(), kube::Error> { +/// async fn main() -> Result<(), Box> { /// let client = Client::try_default().await?; /// let apigroup = discovery::group(&client, "apiregistration.k8s.io").await?; /// let (ar, caps) = apigroup.recommended_kind("APIService").unwrap(); @@ -222,7 +222,7 @@ impl ApiGroup { /// ```no_run /// use kube::{Client, api::{Api, DynamicObject}, discovery::{self, verbs}, ResourceExt}; /// #[tokio::main] - /// async fn main() -> Result<(), kube::Error> { + /// async fn main() -> Result<(), Box> { /// let client = Client::try_default().await?; /// let apigroup = discovery::group(&client, "apiregistration.k8s.io").await?; /// for (ar, caps) in apigroup.recommended_resources() { @@ -249,7 +249,7 @@ impl ApiGroup { /// ```no_run /// use kube::{Client, api::{Api, DynamicObject}, discovery, ResourceExt}; /// #[tokio::main] - /// async fn main() -> Result<(), kube::Error> { + /// async fn main() -> Result<(), Box> { /// let client = Client::try_default().await?; /// let apigroup = discovery::group(&client, "apiregistration.k8s.io").await?; /// let (ar, caps) = apigroup.recommended_kind("APIService").unwrap(); diff --git a/kube-client/src/discovery/mod.rs b/kube-client/src/discovery/mod.rs index deab4170a..608ed0d41 100644 --- a/kube-client/src/discovery/mod.rs +++ b/kube-client/src/discovery/mod.rs @@ -89,7 +89,7 @@ impl Discovery { /// ```no_run /// use kube::{Client, api::{Api, DynamicObject}, discovery::{Discovery, verbs, Scope}, ResourceExt}; /// #[tokio::main] - /// async fn main() -> Result<(), kube::Error> { + /// async fn main() -> Result<(), Box> { /// let client = Client::try_default().await?; /// let discovery = Discovery::new(client.clone()).run().await?; /// for group in discovery.groups() { diff --git a/kube-client/src/discovery/oneshot.rs b/kube-client/src/discovery/oneshot.rs index 6e4c990dd..5cd8998fc 100644 --- a/kube-client/src/discovery/oneshot.rs +++ b/kube-client/src/discovery/oneshot.rs @@ -26,7 +26,7 @@ use kube_core::{ /// ```no_run /// use kube::{Client, api::{Api, DynamicObject}, discovery, ResourceExt}; /// #[tokio::main] -/// async fn main() -> Result<(), kube::Error> { +/// async fn main() -> Result<(), Box> { /// let client = Client::try_default().await?; /// let apigroup = discovery::group(&client, "apiregistration.k8s.io").await?; /// let (ar, caps) = apigroup.recommended_kind("APIService").unwrap(); @@ -62,7 +62,7 @@ pub async fn group(client: &Client, apigroup: &str) -> Result { /// ```no_run /// use kube::{Client, api::{Api, DynamicObject}, discovery, ResourceExt}; /// #[tokio::main] -/// async fn main() -> Result<(), kube::Error> { +/// async fn main() -> Result<(), Box> { /// let client = Client::try_default().await?; /// let gv = "apiregistration.k8s.io/v1".parse()?; /// let apigroup = discovery::pinned_group(&client, &gv).await?; @@ -90,7 +90,7 @@ pub async fn pinned_group(client: &Client, gv: &GroupVersion) -> Result Result<(), kube::Error> { +/// async fn main() -> Result<(), Box> { /// let client = Client::try_default().await?; /// let gvk = GroupVersionKind::gvk("apiregistration.k8s.io", "v1", "APIService"); /// let (ar, caps) = discovery::pinned_kind(&client, &gvk).await?; diff --git a/kube-client/src/lib.rs b/kube-client/src/lib.rs index 7eaf81b6e..7416407d0 100644 --- a/kube-client/src/lib.rs +++ b/kube-client/src/lib.rs @@ -15,7 +15,7 @@ //! use k8s_openapi::api::core::v1::Pod; //! //! #[tokio::main] -//! async fn main() -> Result<(), kube_client::Error> { +//! async fn main() -> Result<(), Box> { //! // Read the environment to find config for kube client. //! // Note that this tries an in-cluster configuration first, //! // then falls back on a kubeconfig file. diff --git a/kube-runtime/src/controller/mod.rs b/kube-runtime/src/controller/mod.rs index 382e8b868..0ca4d6f0e 100644 --- a/kube-runtime/src/controller/mod.rs +++ b/kube-runtime/src/controller/mod.rs @@ -375,7 +375,7 @@ where /// /// /// something to drive the controller /// #[tokio::main] -/// async fn main() -> Result<(), kube::Error> { +/// async fn main() -> Result<(), Box> { /// let client = Client::try_default().await?; /// let context = Context::new(()); // bad empty context - put client in here /// let cmgs = Api::::all(client.clone()); diff --git a/kube/src/lib.rs b/kube/src/lib.rs index 298ecd9cc..c47123470 100644 --- a/kube/src/lib.rs +++ b/kube/src/lib.rs @@ -23,7 +23,7 @@ //! use k8s_openapi::api::core::v1::Pod; //! //! #[tokio::main] -//! async fn main() -> Result<(), kube::Error> { +//! async fn main() -> Result<(), Box> { //! // Infer the runtime environment and try to create a Kubernetes Client //! let client = Client::try_default().await?; //! From e61cc04c59d197a1c1aeaf3578f31f804eae89ff Mon Sep 17 00:00:00 2001 From: kazk Date: Sat, 30 Oct 2021 16:42:29 -0700 Subject: [PATCH 6/9] Replace `snafu` with `thiserror` in examples Signed-off-by: kazk --- examples/Cargo.toml | 2 +- examples/configmapgen_controller.rs | 35 ++++++++++++++--------------- examples/secret_syncer.rs | 23 ++++++++++++------- 3 files changed, 33 insertions(+), 27 deletions(-) diff --git a/examples/Cargo.toml b/examples/Cargo.toml index 4c09cf365..7f762faac 100644 --- a/examples/Cargo.toml +++ b/examples/Cargo.toml @@ -40,7 +40,6 @@ serde_json = "1.0.68" serde_yaml = "0.8.21" tokio = { version = "1.12.0", features = ["full"] } color-eyre = "0.5.10" -snafu = { version = "0.6.10", features = ["futures"] } # Some Api::delete methods use Either either = "1.6.1" schemars = "0.8.6" @@ -54,6 +53,7 @@ json-patch = "0.2.6" tower = { version = "0.4.6" } tower-http = { version = "0.1.0", features = ["trace", "decompression-gzip"] } hyper = { version = "0.14.13", features = ["client", "http1", "stream", "tcp"] } +thiserror = "1.0.29" [[example]] name = "configmapgen_controller" diff --git a/examples/configmapgen_controller.rs b/examples/configmapgen_controller.rs index 7264b364d..14982deeb 100644 --- a/examples/configmapgen_controller.rs +++ b/examples/configmapgen_controller.rs @@ -12,21 +12,16 @@ use kube::{ }; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; -use snafu::{Backtrace, OptionExt, ResultExt, Snafu}; use std::{collections::BTreeMap, io::BufRead}; +use thiserror::Error; use tokio::time::Duration; -#[derive(Debug, Snafu)] +#[derive(Debug, Error)] enum Error { - #[snafu(display("Failed to create ConfigMap: {}", source))] - ConfigMapCreationFailed { - source: kube::Error, - backtrace: Backtrace, - }, - MissingObjectKey { - name: &'static str, - backtrace: Backtrace, - }, + #[error("Failed to create ConfigMap: {0}")] + ConfigMapCreationFailed(#[source] kube::Error), + #[error("MissingObjectKey: {name}")] + MissingObjectKey { name: &'static str }, } #[derive(CustomResource, Debug, Clone, Deserialize, Serialize, JsonSchema)] @@ -42,10 +37,10 @@ fn object_to_owner_reference>( Ok(OwnerReference { api_version: K::api_version(&()).to_string(), kind: K::kind(&()).to_string(), - name: meta.name.context(MissingObjectKey { + name: meta.name.ok_or(Error::MissingObjectKey { name: ".metadata.name", })?, - uid: meta.uid.context(MissingObjectKey { + uid: meta.uid.ok_or(Error::MissingObjectKey { name: ".metadata.uid", })?, ..OwnerReference::default() @@ -76,20 +71,24 @@ async fn reconcile(generator: ConfigMapGenerator, ctx: Context) -> Result< }; let cm_api = Api::::namespaced( client.clone(), - generator.metadata.namespace.as_ref().context(MissingObjectKey { - name: ".metadata.namespace", - })?, + generator + .metadata + .namespace + .as_ref() + .ok_or(Error::MissingObjectKey { + name: ".metadata.namespace", + })?, ); cm_api .patch( - cm.metadata.name.as_ref().context(MissingObjectKey { + cm.metadata.name.as_ref().ok_or(Error::MissingObjectKey { name: ".metadata.name", })?, &PatchParams::apply("configmapgenerator.kube-rt.nullable.se"), &Patch::Apply(&cm), ) .await - .context(ConfigMapCreationFailed)?; + .map_err(Error::ConfigMapCreationFailed)?; Ok(ReconcilerAction { requeue_after: Some(Duration::from_secs(300)), }) diff --git a/examples/secret_syncer.rs b/examples/secret_syncer.rs index 51758bddf..7a3047187 100644 --- a/examples/secret_syncer.rs +++ b/examples/secret_syncer.rs @@ -14,20 +14,27 @@ use kube::{ finalizer::{finalizer, Event}, }, }; -use snafu::{OptionExt, ResultExt, Snafu}; use std::time::Duration; +use thiserror::Error; -#[derive(Debug, Snafu)] +#[derive(Debug, Error)] enum Error { + #[error("NoName")] NoName, + #[error("NoNamespace")] NoNamespace, - UpdateSecret { source: kube::Error }, - DeleteSecret { source: kube::Error }, + #[error("UpdateSecret: {0}")] + UpdateSecret(#[source] kube::Error), + #[error("DeleteSecret: {0}")] + DeleteSecret(#[source] kube::Error), } type Result = std::result::Result; fn secret_name_for_configmap(cm: &ConfigMap) -> Result { - Ok(format!("cm---{}", cm.metadata.name.as_deref().context(NoName)?)) + Ok(format!( + "cm---{}", + cm.metadata.name.as_deref().ok_or(Error::NoName)? + )) } async fn apply(cm: ConfigMap, secrets: &kube::Api) -> Result { @@ -48,7 +55,7 @@ async fn apply(cm: ConfigMap, secrets: &kube::Api) -> Result) -> Result Ok(()), err => Err(err), }) - .context(DeleteSecret)?; + .map_err(Error::DeleteSecret)?; Ok(ReconcilerAction { requeue_after: None }) } @@ -78,7 +85,7 @@ async fn main() -> color_eyre::Result<()> { ) .run( |cm, _| { - let ns = cm.meta().namespace.as_deref().context(NoNamespace).unwrap(); + let ns = cm.meta().namespace.as_deref().ok_or(Error::NoNamespace).unwrap(); let cms: Api = Api::namespaced(kube.clone(), ns); let secrets: Api = Api::namespaced(kube.clone(), ns); async move { From dc3e45624287d9ba79fe682506b7210a1a9678c8 Mon Sep 17 00:00:00 2001 From: kazk Date: Sat, 30 Oct 2021 16:19:49 -0700 Subject: [PATCH 7/9] Replace `snafu` with `thiserror` in `kube-runtime` Signed-off-by: kazk --- kube-runtime/Cargo.toml | 2 +- kube-runtime/src/controller/mod.rs | 44 ++++++++++---------------- kube-runtime/src/finalizer.rs | 34 ++++++++++---------- kube-runtime/src/scheduler.rs | 13 +++----- kube-runtime/src/wait.rs | 33 +++++++++---------- kube-runtime/src/watcher.rs | 51 ++++++++++++------------------ 6 files changed, 75 insertions(+), 102 deletions(-) diff --git a/kube-runtime/Cargo.toml b/kube-runtime/Cargo.toml index 1e7ce77ab..52c961793 100644 --- a/kube-runtime/Cargo.toml +++ b/kube-runtime/Cargo.toml @@ -27,12 +27,12 @@ serde = "1.0.130" smallvec = "1.7.0" pin-project = "1.0.2" tokio = { version = "1.12.0", features = ["time"] } -snafu = { version = "0.6.10", features = ["futures"] } dashmap = "4.0.1" tokio-util = { version = "0.6.8", features = ["time"] } tracing = "0.1.29" json-patch = "0.2.6" serde_json = "1.0.68" +thiserror = "1.0.29" [dependencies.k8s-openapi] version = "0.13.1" diff --git a/kube-runtime/src/controller/mod.rs b/kube-runtime/src/controller/mod.rs index 0ca4d6f0e..2246892ab 100644 --- a/kube-runtime/src/controller/mod.rs +++ b/kube-runtime/src/controller/mod.rs @@ -22,7 +22,6 @@ use futures::{ }; use kube_client::api::{Api, DynamicObject, ListParams, Resource}; use serde::de::DeserializeOwned; -use snafu::{futures::TryStreamExt as SnafuTryStreamExt, Backtrace, ResultExt, Snafu}; use std::{ fmt::{Debug, Display}, hash::Hash, @@ -30,30 +29,23 @@ use std::{ time::Duration, }; use stream::BoxStream; +use thiserror::Error; use tokio::{runtime::Handle, time::Instant}; use tracing::{info_span, Instrument}; mod future_hash_map; mod runner; -#[derive(Snafu, Debug)] +#[derive(Debug, Error)] pub enum Error { - ObjectNotFound { - obj_ref: ObjectRef, - backtrace: Backtrace, - }, - ReconcilerFailed { - source: ReconcilerErr, - backtrace: Backtrace, - }, - SchedulerDequeueFailed { - #[snafu(backtrace)] - source: scheduler::Error, - }, - QueueError { - source: QueueErr, - backtrace: Backtrace, - }, + #[error("ObjectNotFound")] + ObjectNotFound(ObjectRef), + #[error("ReconcilerFailed: {0}")] + ReconcilerFailed(#[source] ReconcilerErr), + #[error("SchedulerDequeueFailed: {0}")] + SchedulerDequeueFailed(#[source] scheduler::Error), + #[error("QueueError: {0}")] + QueueError(#[source] QueueErr), } /// Results of the reconciliation attempt @@ -250,7 +242,7 @@ where // input: stream combining scheduled tasks and user specified inputs event Box::pin(stream::select( // 1. inputs from users queue stream - queue.context(QueueError).map_ok(|request| ScheduleRequest { + queue.map_err(Error::QueueError).map_ok(|request| ScheduleRequest { message: request.into(), run_at: Instant::now() + Duration::from_millis(1), }) @@ -281,15 +273,12 @@ where .left_future() }, None => future::err( - ObjectNotFound { - obj_ref: request.obj_ref.erase(), - } - .build(), + Error::ObjectNotFound(request.obj_ref.erase()) ) .right_future(), } }) - .context(SchedulerDequeueFailed) + .map_err(Error::SchedulerDequeueFailed) .map(|res| res.and_then(|x| x)) .on_complete(async { tracing::debug!("applier runner terminated") }) }, @@ -319,7 +308,7 @@ where } reconciler_result .map(|action| (obj_ref, action)) - .context(ReconcilerFailed) + .map_err(Error::ReconcilerFailed) } }) .on_complete(async { tracing::debug!("applier terminated") }) @@ -347,10 +336,11 @@ where /// use futures::StreamExt; /// use k8s_openapi::api::core::v1::ConfigMap; /// use schemars::JsonSchema; +/// use thiserror::Error; /// -/// use snafu::{Backtrace, OptionExt, ResultExt, Snafu}; -/// #[derive(Debug, Snafu)] +/// #[derive(Debug, Error)] /// enum Error {} +/// /// /// A custom resource /// #[derive(CustomResource, Debug, Clone, Deserialize, Serialize, JsonSchema)] /// #[kube(group = "nullable.se", version = "v1", kind = "ConfigMapGenerator", namespaced)] diff --git a/kube-runtime/src/finalizer.rs b/kube-runtime/src/finalizer.rs index f89e87154..f246840c1 100644 --- a/kube-runtime/src/finalizer.rs +++ b/kube-runtime/src/finalizer.rs @@ -7,23 +7,23 @@ use kube_client::{ Api, Resource, ResourceExt, }; use serde::{de::DeserializeOwned, Serialize}; -use snafu::{OptionExt, ResultExt, Snafu}; use std::{error::Error as StdError, fmt::Debug}; +use thiserror::Error; -#[derive(Debug, Snafu)] +#[derive(Debug, Error)] pub enum Error where ReconcileErr: StdError + 'static, { - #[snafu(display("failed to apply object: {}", source))] - ApplyFailed { source: ReconcileErr }, - #[snafu(display("failed to clean up object: {}", source))] - CleanupFailed { source: ReconcileErr }, - #[snafu(display("failed to add finalizer: {}", source))] - AddFinalizer { source: kube_client::Error }, - #[snafu(display("failed to remove finalizer: {}", source))] - RemoveFinalizer { source: kube_client::Error }, - #[snafu(display("object has no name"))] + #[error("failed to apply object: {0}")] + ApplyFailed(#[source] ReconcileErr), + #[error("failed to clean up object: {0}")] + CleanupFailed(#[source] ReconcileErr), + #[error("failed to add finalizer: {0}")] + AddFinalizer(#[source] kube_client::Error), + #[error("failed to remove finalizer: {0}")] + RemoveFinalizer(#[source] kube_client::Error), + #[error("object has no name")] UnnamedObject, } @@ -116,18 +116,18 @@ where } => reconcile(Event::Apply(obj)) .into_future() .await - .context(ApplyFailed), + .map_err(Error::ApplyFailed), FinalizerState { finalizer_index: Some(finalizer_i), is_deleting: true, } => { // Cleanup reconciliation must succeed before it's safe to remove the finalizer - let name = obj.meta().name.clone().context(UnnamedObject)?; + let name = obj.meta().name.clone().ok_or(Error::UnnamedObject)?; let action = reconcile(Event::Cleanup(obj)) .into_future() .await // Short-circuit, so that we keep the finalizer if cleanup fails - .context(CleanupFailed)?; + .map_err(Error::CleanupFailed)?; // Cleanup was successful, remove the finalizer so that deletion can continue let finalizer_path = format!("/metadata/finalizers/{}", finalizer_i); api.patch::( @@ -145,7 +145,7 @@ where ])), ) .await - .context(RemoveFinalizer)?; + .map_err(Error::RemoveFinalizer)?; Ok(action) } FinalizerState { @@ -171,12 +171,12 @@ where })] }); api.patch::( - obj.meta().name.as_deref().context(UnnamedObject)?, + obj.meta().name.as_deref().ok_or(Error::UnnamedObject)?, &PatchParams::default(), &Patch::Json(patch), ) .await - .context(AddFinalizer)?; + .map_err(Error::AddFinalizer)?; // No point applying here, since the patch will cause a new reconciliation Ok(ReconcilerAction { requeue_after: None }) } diff --git a/kube-runtime/src/scheduler.rs b/kube-runtime/src/scheduler.rs index 14dd65732..f63193c54 100644 --- a/kube-runtime/src/scheduler.rs +++ b/kube-runtime/src/scheduler.rs @@ -2,23 +2,20 @@ use futures::{stream::Fuse, Stream, StreamExt}; use pin_project::pin_project; -use snafu::{Backtrace, ResultExt, Snafu}; use std::{ collections::{hash_map::Entry, HashMap, HashSet}, hash::Hash, pin::Pin, task::{Context, Poll}, }; +use thiserror::Error; use tokio::time::{self, Instant}; use tokio_util::time::delay_queue::{self, DelayQueue}; -#[derive(Debug, Snafu)] +#[derive(Debug, Error)] pub enum Error { - #[snafu(display("timer failure: {}", source))] - TimerError { - source: time::error::Error, - backtrace: Backtrace, - }, + #[error("timer failure: {0}")] + TimerError(#[source] time::error::Error), } pub type Result = std::result::Result; @@ -150,7 +147,7 @@ where } match scheduler.poll_pop_queue_message(cx, &can_take_message) { - Poll::Ready(expired) => Poll::Ready(Some(expired.context(TimerError))), + Poll::Ready(expired) => Poll::Ready(Some(expired.map_err(Error::TimerError))), Poll::Pending => Poll::Pending, } } diff --git a/kube-runtime/src/wait.rs b/kube-runtime/src/wait.rs index 24826b99e..5f40a7470 100644 --- a/kube-runtime/src/wait.rs +++ b/kube-runtime/src/wait.rs @@ -2,18 +2,15 @@ use futures::TryStreamExt; use kube_client::{Api, Resource}; use serde::de::DeserializeOwned; -use snafu::{futures::TryStreamExt as _, Snafu}; use std::fmt::Debug; +use thiserror::Error; use crate::watcher::{self, watch_object}; -#[derive(Debug, Snafu)] +#[derive(Debug, Error)] pub enum Error { - #[snafu(display("failed to probe for whether the condition is fulfilled yet: {}", source))] - ProbeFailed { - #[snafu(backtrace)] - source: watcher::Error, - }, + #[error("failed to probe for whether the condition is fulfilled yet: {0}")] + ProbeFailed(#[source] watcher::Error), } /// Watch an object, and Wait for some condition `cond` to return `true`. @@ -52,7 +49,7 @@ where K: Clone + Debug + Send + DeserializeOwned + Resource + 'static, { watch_object(api, name) - .context(ProbeFailed) + .map_err(Error::ProbeFailed) .try_take_while(|obj| { let result = !cond.matches_object(obj.as_ref()); async move { Ok(result) } @@ -233,17 +230,17 @@ pub mod delete { use super::{await_condition, conditions}; use kube_client::{api::DeleteParams, Api, Resource}; use serde::de::DeserializeOwned; - use snafu::{OptionExt, ResultExt, Snafu}; use std::fmt::Debug; + use thiserror::Error; - #[derive(Snafu, Debug)] + #[derive(Debug, Error)] pub enum Error { - #[snafu(display("deleted object has no UID to wait for"))] + #[error("deleted object has no UID to wait for")] NoUid, - #[snafu(display("failed to delete object: {}", source))] - Delete { source: kube_client::Error }, - #[snafu(display("failed to wait for object to be deleted: {}", source))] - Await { source: super::Error }, + #[error("failed to delete object: {0}")] + Delete(#[source] kube_client::Error), + #[error("failed to wait for object to be deleted: {0}")] + Await(#[source] super::Error), } /// Delete an object, and wait for it to be removed from the Kubernetes API (including waiting for all finalizers to unregister themselves). @@ -260,14 +257,14 @@ pub mod delete { let deleted_obj_uid = api .delete(name, delete_params) .await - .context(Delete)? + .map_err(Error::Delete)? .either( |mut obj| obj.meta_mut().uid.take(), |status| status.details.map(|details| details.uid), ) - .context(NoUid)?; + .ok_or(Error::NoUid)?; await_condition(api, name, conditions::is_deleted(&deleted_obj_uid)) .await - .context(Await) + .map_err(Error::Await) } } diff --git a/kube-runtime/src/watcher.rs b/kube-runtime/src/watcher.rs index c8de81b69..62b690a64 100644 --- a/kube-runtime/src/watcher.rs +++ b/kube-runtime/src/watcher.rs @@ -8,33 +8,21 @@ use kube_client::{ }; use serde::de::DeserializeOwned; use smallvec::SmallVec; -use snafu::{Backtrace, ResultExt, Snafu}; use std::{clone::Clone, fmt::Debug}; +use thiserror::Error; -#[derive(Snafu, Debug)] +#[derive(Debug, Error)] pub enum Error { - #[snafu(display("failed to perform initial object list: {}", source))] - InitialListFailed { - source: kube_client::Error, - backtrace: Backtrace, - }, - #[snafu(display("failed to start watching object: {}", source))] - WatchStartFailed { - source: kube_client::Error, - backtrace: Backtrace, - }, - #[snafu(display("error returned by apiserver during watch: {}", source))] - WatchError { - source: kube_client::error::ErrorResponse, - backtrace: Backtrace, - }, - #[snafu(display("watch stream failed: {}", source))] - WatchFailed { - source: kube_client::Error, - backtrace: Backtrace, - }, - #[snafu(display("too many objects matched search criteria"))] - TooManyObjects { backtrace: Backtrace }, + #[error("failed to perform initial object list: {0}")] + InitialListFailed(#[source] kube_client::Error), + #[error("failed to start watching object: {0}")] + WatchStartFailed(#[source] kube_client::Error), + #[error("error returned by apiserver during watch: {0}")] + WatchError(#[source] kube_client::error::ErrorResponse), + #[error("watch stream failed: {0}")] + WatchFailed(#[source] kube_client::Error), + #[error("too many objects matched search criteria")] + TooManyObjects, } pub type Result = std::result::Result; @@ -120,16 +108,17 @@ async fn step_trampolined (Some(Ok(Event::Restarted(list.items))), State::InitListed { resource_version: list.metadata.resource_version.unwrap(), }), - Err(err) => (Some(Err(err).context(InitialListFailed)), State::Empty), + Err(err) => (Some(Err(err).map_err(Error::InitialListFailed)), State::Empty), }, State::InitListed { resource_version } => match api.watch(list_params, &resource_version).await { Ok(stream) => (None, State::Watching { resource_version, stream: stream.boxed(), }), - Err(err) => (Some(Err(err).context(WatchStartFailed)), State::InitListed { - resource_version, - }), + Err(err) => ( + Some(Err(err).map_err(Error::WatchStartFailed)), + State::InitListed { resource_version }, + ), }, State::Watching { resource_version, @@ -163,9 +152,9 @@ async fn step_trampolined (Some(Err(err).context(WatchFailed)), State::Watching { + Some(Err(err)) => (Some(Err(err).map_err(Error::WatchFailed)), State::Watching { resource_version, stream, }), @@ -270,7 +259,7 @@ pub fn watch_object 1 => TooManyObjects.fail(), + Event::Restarted(objs) if objs.len() > 1 => Err(Error::TooManyObjects), Event::Restarted(mut objs) => Ok(objs.pop()), Event::Applied(obj) => Ok(Some(obj)), }) From edc8affbcbf729db4e41719beb2dc08341e07601 Mon Sep 17 00:00:00 2001 From: kazk Date: Sat, 30 Oct 2021 22:27:54 -0700 Subject: [PATCH 8/9] Add changelog entries Signed-off-by: kazk --- CHANGELOG.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d5bf8ef9d..718aba724 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,16 @@ UNRELEASED =================== * see https://github.com/kube-rs/kube-rs/compare/0.63.2...master + * BREAKING: The following breaking changes were made as a part of an effort to refine errors (#688). + - Removed `kube::core::Error` and `kube::core::Result`. `kube::core::Error` was replaced by more specific errors as described below. + - Replaced `kube::core::Error::InvalidGroupVersion` with `kube::core::gvk::ParseGroupVersionError`. + - Changed the error returned from `kube::core::admission::AdmissionRequest::with_patch` to `kube::core::admission::SerializePatchError` (was `kube::core::Error::SerdeError`). + - Changed the error associated with `TryInto` to `kube::core::admission::ConvertAdmissionReviewError` (was `kube::core::Error::RequestValidation`). + - Changed the error returned from methods of `kube::core::Request` to `kube::core::request::Error` (was `kube::core::Error`). `kube::core::request::Error` represents possible errors when building an HTTP request. The removed `kube::core::Error` had `RequestValidation(String)`, `SerdeError(serde_json::Error)`, and `HttpError(http::Error)` variants. They are now `Validation(String)`, `SerializeBody(serde_json::Error)`, and `BuildRequest(http::Error)` respectively in `kube::core::request::Error`. + - Replaced `kube::Error::RequestValidation(String)` variant with `kube::Error::BuildRequest(kube::core::request::Error)`. This variant includes possible errors when building an HTTP request as described above, and contains errors that was previously grouped under `kube::Error::SerdeError` and `kube::Error::HttpError`. + - Removed `impl From for kube::Error` for the following types: `std::io::Error`, `hyper::Error`, `tower::BoxError`, `std::string::FromUtf8Error`, `http::Error`, `http::uri::InvalidUri`, `serde_json::Error`, `openssl::error::ErrorStack`, `kube::core::Error`, `kube::error::ConfigError`, `kube::error::DisoveryError`, `kube::error::OAuthError`. + - Changed variants of error enums in `kube::runtime`. Replaced `snafu` with `thiserror`. + 0.63.2 / 2021-10-28 =================== * `kube::runtime::events`: fix build and hide module on kubernetes < 1.19 (events/v1 missing there) - [#685](https://github.com/kube-rs/kube-rs/issues/685) From 567e472a5e7f8788086d184d8fc9865f9994f053 Mon Sep 17 00:00:00 2001 From: kazk Date: Sun, 31 Oct 2021 16:39:50 -0700 Subject: [PATCH 9/9] Use tuple variant for consistency Signed-off-by: kazk --- examples/configmapgen_controller.rs | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/examples/configmapgen_controller.rs b/examples/configmapgen_controller.rs index 14982deeb..785ec8660 100644 --- a/examples/configmapgen_controller.rs +++ b/examples/configmapgen_controller.rs @@ -20,8 +20,8 @@ use tokio::time::Duration; enum Error { #[error("Failed to create ConfigMap: {0}")] ConfigMapCreationFailed(#[source] kube::Error), - #[error("MissingObjectKey: {name}")] - MissingObjectKey { name: &'static str }, + #[error("MissingObjectKey: {0}")] + MissingObjectKey(&'static str), } #[derive(CustomResource, Debug, Clone, Deserialize, Serialize, JsonSchema)] @@ -37,12 +37,8 @@ fn object_to_owner_reference>( Ok(OwnerReference { api_version: K::api_version(&()).to_string(), kind: K::kind(&()).to_string(), - name: meta.name.ok_or(Error::MissingObjectKey { - name: ".metadata.name", - })?, - uid: meta.uid.ok_or(Error::MissingObjectKey { - name: ".metadata.uid", - })?, + name: meta.name.ok_or(Error::MissingObjectKey(".metadata.name"))?, + uid: meta.uid.ok_or(Error::MissingObjectKey(".metadata.uid"))?, ..OwnerReference::default() }) } @@ -75,15 +71,14 @@ async fn reconcile(generator: ConfigMapGenerator, ctx: Context) -> Result< .metadata .namespace .as_ref() - .ok_or(Error::MissingObjectKey { - name: ".metadata.namespace", - })?, + .ok_or(Error::MissingObjectKey(".metadata.namespace"))?, ); cm_api .patch( - cm.metadata.name.as_ref().ok_or(Error::MissingObjectKey { - name: ".metadata.name", - })?, + cm.metadata + .name + .as_ref() + .ok_or(Error::MissingObjectKey(".metadata.name"))?, &PatchParams::apply("configmapgenerator.kube-rt.nullable.se"), &Patch::Apply(&cm), )