From 935c554184f3f6284c7589b3dca55c25b5f1b81c Mon Sep 17 00:00:00 2001 From: goenning Date: Sun, 13 Nov 2022 17:28:30 +0000 Subject: [PATCH 01/13] fix: align kube-rs with client-go config parsing Signed-off-by: goenning --- kube-client/Cargo.toml | 1 + kube-client/src/config/file_config.rs | 28 +++++++++++++++++++++------ 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/kube-client/Cargo.toml b/kube-client/Cargo.toml index 9d4a2b066..517077857 100644 --- a/kube-client/Cargo.toml +++ b/kube-client/Cargo.toml @@ -42,6 +42,7 @@ dirs = { package = "dirs-next", optional = true, version = "2.0.0" } serde = { version = "1.0.130", features = ["derive"] } serde_json = "1.0.68" serde_yaml = { version = "0.8.21", optional = true } +serde_with = { version = "2.0.1" } http = "0.2.5" http-body = { version = "0.4.2", optional = true } either = { version = "1.6.1", optional = true } diff --git a/kube-client/src/config/file_config.rs b/kube-client/src/config/file_config.rs index 71e96ddbc..ec2556372 100644 --- a/kube-client/src/config/file_config.rs +++ b/kube-client/src/config/file_config.rs @@ -6,6 +6,7 @@ use std::{ use secrecy::{ExposeSecret, SecretString}; use serde::{Deserialize, Deserializer, Serialize, Serializer}; +use serde_with::serde_as; use super::{KubeconfigError, LoadDataError}; @@ -72,20 +73,24 @@ pub struct NamedExtension { } /// NamedCluster associates name with cluster. -#[derive(Clone, Debug, Serialize, Deserialize)] +#[serde_as] +#[derive(Clone, Debug, Serialize, Deserialize, Default)] #[cfg_attr(test, derive(PartialEq, Eq))] pub struct NamedCluster { /// Name of cluster + #[serde(default)] pub name: String, /// Information about how to communicate with a kubernetes cluster + #[serde_as(as = "serde_with::DefaultOnNull")] pub cluster: Cluster, } /// Cluster stores information to connect Kubernetes cluster. -#[derive(Clone, Debug, Serialize, Deserialize)] +#[derive(Clone, Debug, Serialize, Deserialize, Default)] #[cfg_attr(test, derive(PartialEq, Eq))] pub struct Cluster { /// The address of the kubernetes cluster (https://hostname:port). + #[serde(default)] pub server: String, /// Skips the validity check for the server's certificate. This will make your HTTPS connections insecure. #[serde(rename = "insecure-skip-tls-verify")] @@ -109,13 +114,16 @@ pub struct Cluster { } /// NamedAuthInfo associates name with authentication. -#[derive(Clone, Debug, Serialize, Deserialize)] +#[serde_as] +#[derive(Clone, Debug, Serialize, Deserialize, Default)] #[cfg_attr(test, derive(PartialEq))] pub struct NamedAuthInfo { /// Name of the user + #[serde(default)] pub name: String, /// Information that describes identity of the user - #[serde(rename = "user")] + #[serde(rename = "user", default)] + #[serde_as(as = "serde_with::DefaultOnNull")] pub auth_info: AuthInfo, } @@ -214,12 +222,16 @@ impl PartialEq for AuthInfo { } /// AuthProviderConfig stores auth for specified cloud provider. +#[serde_as] #[derive(Clone, Debug, Serialize, Deserialize)] #[cfg_attr(test, derive(PartialEq, Eq))] pub struct AuthProviderConfig { /// Name of the auth provider + #[serde(default)] pub name: String, /// Auth provider configuration + #[serde(default)] + #[serde_as(as = "serde_with::DefaultOnNull")] pub config: HashMap, } @@ -252,22 +264,26 @@ pub struct ExecConfig { } /// NamedContext associates name with context. -#[derive(Clone, Debug, Serialize, Deserialize)] +#[derive(Clone, Debug, Serialize, Deserialize, Default)] #[cfg_attr(test, derive(PartialEq, Eq))] pub struct NamedContext { /// Name of the context + #[serde(default)] pub name: String, /// Associations for the context + #[serde(default)] pub context: Context, } /// Context stores tuple of cluster and user information. -#[derive(Clone, Debug, Serialize, Deserialize)] +#[derive(Clone, Debug, Serialize, Deserialize, Default)] #[cfg_attr(test, derive(PartialEq, Eq))] pub struct Context { /// Name of the cluster for this context + #[serde(default)] pub cluster: String, /// Name of the `AuthInfo` for this context + #[serde(default)] pub user: String, /// The default namespace to use on unspecified requests #[serde(skip_serializing_if = "Option::is_none")] From d5f1325b7047ab73e75e96e169af3676508b06d8 Mon Sep 17 00:00:00 2001 From: goenning Date: Tue, 15 Nov 2022 18:27:41 +0000 Subject: [PATCH 02/13] remove serde_with, make field optional Signed-off-by: goenning --- kube-client/Cargo.toml | 1 - kube-client/src/client/auth/mod.rs | 2 +- kube-client/src/config/file_config.rs | 108 ++++++++++++++++++-------- kube-client/src/config/file_loader.rs | 4 +- 4 files changed, 78 insertions(+), 37 deletions(-) diff --git a/kube-client/Cargo.toml b/kube-client/Cargo.toml index 517077857..9d4a2b066 100644 --- a/kube-client/Cargo.toml +++ b/kube-client/Cargo.toml @@ -42,7 +42,6 @@ dirs = { package = "dirs-next", optional = true, version = "2.0.0" } serde = { version = "1.0.130", features = ["derive"] } serde_json = "1.0.68" serde_yaml = { version = "0.8.21", optional = true } -serde_with = { version = "2.0.1" } http = "0.2.5" http-body = { version = "0.4.2", optional = true } either = { version = "1.6.1", optional = true } diff --git a/kube-client/src/client/auth/mod.rs b/kube-client/src/client/auth/mod.rs index 76cf97ef1..6f1d2e950 100644 --- a/kube-client/src/client/auth/mod.rs +++ b/kube-client/src/client/auth/mod.rs @@ -533,7 +533,7 @@ mod test { ); let config: Kubeconfig = serde_yaml::from_str(&test_file).unwrap(); - let auth_info = &config.auth_infos[0].auth_info; + let auth_info = config.auth_infos[0].auth_info.as_ref().unwrap(); match Auth::try_from(auth_info).unwrap() { Auth::RefreshableToken(RefreshableToken::Exec(refreshable)) => { let (token, _expire, info) = Arc::try_unwrap(refreshable).unwrap().into_inner(); diff --git a/kube-client/src/config/file_config.rs b/kube-client/src/config/file_config.rs index ec2556372..0ad00488d 100644 --- a/kube-client/src/config/file_config.rs +++ b/kube-client/src/config/file_config.rs @@ -6,7 +6,6 @@ use std::{ use secrecy::{ExposeSecret, SecretString}; use serde::{Deserialize, Deserializer, Serialize, Serializer}; -use serde_with::serde_as; use super::{KubeconfigError, LoadDataError}; @@ -67,13 +66,13 @@ pub struct Preferences { #[cfg_attr(test, derive(PartialEq, Eq))] pub struct NamedExtension { /// Name of extension + #[serde(default)] pub name: String, /// Additional information for extenders so that reads and writes don't clobber unknown fields pub extension: serde_json::Value, } /// NamedCluster associates name with cluster. -#[serde_as] #[derive(Clone, Debug, Serialize, Deserialize, Default)] #[cfg_attr(test, derive(PartialEq, Eq))] pub struct NamedCluster { @@ -81,8 +80,8 @@ pub struct NamedCluster { #[serde(default)] pub name: String, /// Information about how to communicate with a kubernetes cluster - #[serde_as(as = "serde_with::DefaultOnNull")] - pub cluster: Cluster, + #[serde(skip_serializing_if = "Option::is_none")] + pub cluster: Option, } /// Cluster stores information to connect Kubernetes cluster. @@ -114,7 +113,6 @@ pub struct Cluster { } /// NamedAuthInfo associates name with authentication. -#[serde_as] #[derive(Clone, Debug, Serialize, Deserialize, Default)] #[cfg_attr(test, derive(PartialEq))] pub struct NamedAuthInfo { @@ -123,8 +121,8 @@ pub struct NamedAuthInfo { pub name: String, /// Information that describes identity of the user #[serde(rename = "user", default)] - #[serde_as(as = "serde_with::DefaultOnNull")] - pub auth_info: AuthInfo, + #[serde(skip_serializing_if = "Option::is_none")] + pub auth_info: Option, } fn serialize_secretstring(pw: &Option, serializer: S) -> Result @@ -141,8 +139,9 @@ fn deserialize_secretstring<'de, D>(deserializer: D) -> Result, { - match String::deserialize(deserializer) { - Ok(secret) => Ok(Some(SecretString::new(secret))), + match Option::::deserialize(deserializer) { + Ok(Some(secret)) => Ok(Some(SecretString::new(secret))), + Ok(None) => Ok(None), Err(e) => Err(e), } } @@ -222,7 +221,6 @@ impl PartialEq for AuthInfo { } /// AuthProviderConfig stores auth for specified cloud provider. -#[serde_as] #[derive(Clone, Debug, Serialize, Deserialize)] #[cfg_attr(test, derive(PartialEq, Eq))] pub struct AuthProviderConfig { @@ -231,7 +229,6 @@ pub struct AuthProviderConfig { pub name: String, /// Auth provider configuration #[serde(default)] - #[serde_as(as = "serde_with::DefaultOnNull")] pub config: HashMap, } @@ -246,6 +243,7 @@ pub struct ExecConfig { #[serde(skip_serializing_if = "Option::is_none")] pub api_version: Option, /// Command to execute. + #[serde(default)] pub command: String, /// Arguments to pass to the command when executing it. #[serde(skip_serializing_if = "Option::is_none")] @@ -307,26 +305,30 @@ impl Kubeconfig { for mut config in kubeconfig_from_yaml(&data)? { if let Some(dir) = path.as_ref().parent() { for named in config.clusters.iter_mut() { - if let Some(path) = &named.cluster.certificate_authority { - if let Some(abs_path) = to_absolute(dir, path) { - named.cluster.certificate_authority = Some(abs_path); + if let Some(cluster) = &mut named.cluster { + if let Some(path) = &cluster.certificate_authority { + if let Some(abs_path) = to_absolute(dir, path) { + cluster.certificate_authority = Some(abs_path); + } } } } for named in config.auth_infos.iter_mut() { - if let Some(path) = &named.auth_info.client_certificate { - if let Some(abs_path) = to_absolute(dir, path) { - named.auth_info.client_certificate = Some(abs_path); + if let Some(auth_info) = &mut named.auth_info { + if let Some(path) = &auth_info.client_certificate { + if let Some(abs_path) = to_absolute(dir, path) { + auth_info.client_certificate = Some(abs_path); + } } - } - if let Some(path) = &named.auth_info.client_key { - if let Some(abs_path) = to_absolute(dir, path) { - named.auth_info.client_key = Some(abs_path); + if let Some(path) = &auth_info.client_key { + if let Some(abs_path) = to_absolute(dir, path) { + auth_info.client_key = Some(abs_path); + } } - } - if let Some(path) = &named.auth_info.token_file { - if let Some(abs_path) = to_absolute(dir, path) { - named.auth_info.token_file = Some(abs_path); + if let Some(path) = &auth_info.token_file { + if let Some(abs_path) = to_absolute(dir, path) { + auth_info.token_file = Some(abs_path); + } } } } @@ -541,10 +543,10 @@ mod tests { current_context: Some("default".into()), auth_infos: vec![NamedAuthInfo { name: "red-user".into(), - auth_info: AuthInfo { + auth_info: Some(AuthInfo { token: Some(SecretString::from_str("first-token").unwrap()), ..Default::default() - }, + }), }], ..Default::default() }; @@ -553,18 +555,18 @@ mod tests { auth_infos: vec![ NamedAuthInfo { name: "red-user".into(), - auth_info: AuthInfo { + auth_info: Some(AuthInfo { token: Some(SecretString::from_str("second-token").unwrap()), username: Some("red-user".into()), ..Default::default() - }, + }), }, NamedAuthInfo { name: "green-user".into(), - auth_info: AuthInfo { + auth_info: Some(AuthInfo { token: Some(SecretString::from_str("new-token").unwrap()), ..Default::default() - }, + }), }, ], ..Default::default() @@ -578,13 +580,15 @@ mod tests { assert_eq!( merged.auth_infos[0] .auth_info + .as_ref() + .unwrap() .token .as_ref() .map(|t| t.expose_secret().to_string()), Some("first-token".to_string()) ); // Even if it's not conflicting - assert_eq!(merged.auth_infos[0].auth_info.username, None); + assert_eq!(merged.auth_infos[0].auth_info.as_ref().unwrap().username, None); // New named auth info is appended assert_eq!(merged.auth_infos[1].name, "green-user".to_owned()); } @@ -651,13 +655,40 @@ users: assert_eq!(config.clusters[0].name, "eks"); assert_eq!(config.clusters[1].name, "minikube"); assert_eq!( - config.clusters[1].cluster.extensions.as_ref().unwrap()[0] + config.clusters[1].cluster.as_ref().unwrap().extensions.as_ref().unwrap()[0] .extension .get("provider"), Some(&Value::String("minikube.sigs.k8s.io".to_owned())) ); } +// #[test] +// fn kubeconfig_deserialize_emptystrings() { +// let config_yaml = "apiVersion: v1 +// clusters: +// - cluster: +// certificate-authority: /home/kevin/.minikube/ca.crt +// server: d +// name: minikube +// contexts: +// - context: +// cluster: minikube +// user: minikube +// name: minikube +// current-context: minikube +// kind: Config +// preferences: {} +// users: +// - name: minikube +// user: +// client-certificate: /home/kevin/.minikube/profiles/minikube/client.crt +// client-key: /home/kevin/.minikube/profiles/minikube/client.key"; + +// let config = Kubeconfig::from_yaml(config_yaml).unwrap(); + +// assert_eq!(config.clusters[0].cluster.as_ref().unwrap().server, ""); +// } + #[test] fn kubeconfig_multi_document_merge() -> Result<(), KubeconfigError> { let config_yaml = r#"--- @@ -717,6 +748,17 @@ users: assert_eq!(cfg, Kubeconfig::default()); } + #[test] + fn authinfo_deserialize_null_secret() { + let authinfo_yaml = r#" +username: user +password: +"#; + let authinfo: AuthInfo = serde_yaml::from_str(authinfo_yaml).unwrap(); + assert_eq!(authinfo.username, Some("user".to_string())); + assert!(authinfo.password.is_none()); + } + #[test] fn authinfo_debug_does_not_output_password() { let authinfo_yaml = r#" diff --git a/kube-client/src/config/file_loader.rs b/kube-client/src/config/file_loader.rs index 7cf8552a8..7eb2029cd 100644 --- a/kube-client/src/config/file_loader.rs +++ b/kube-client/src/config/file_loader.rs @@ -91,8 +91,8 @@ impl ConfigLoader { Ok(ConfigLoader { current_context: current_context.clone(), - cluster: cluster.clone(), - user: user.clone(), + cluster: cluster.clone().unwrap_or_default(), + user: user.clone().unwrap_or_default(), }) } From af53d5051784e1765979602472bcb8c4ef9e6ccc Mon Sep 17 00:00:00 2001 From: goenning Date: Tue, 15 Nov 2022 18:29:35 +0000 Subject: [PATCH 03/13] remove commented test Signed-off-by: goenning --- kube-client/src/config/file_config.rs | 27 --------------------------- 1 file changed, 27 deletions(-) diff --git a/kube-client/src/config/file_config.rs b/kube-client/src/config/file_config.rs index 0ad00488d..5bfe506bd 100644 --- a/kube-client/src/config/file_config.rs +++ b/kube-client/src/config/file_config.rs @@ -662,33 +662,6 @@ users: ); } -// #[test] -// fn kubeconfig_deserialize_emptystrings() { -// let config_yaml = "apiVersion: v1 -// clusters: -// - cluster: -// certificate-authority: /home/kevin/.minikube/ca.crt -// server: d -// name: minikube -// contexts: -// - context: -// cluster: minikube -// user: minikube -// name: minikube -// current-context: minikube -// kind: Config -// preferences: {} -// users: -// - name: minikube -// user: -// client-certificate: /home/kevin/.minikube/profiles/minikube/client.crt -// client-key: /home/kevin/.minikube/profiles/minikube/client.key"; - -// let config = Kubeconfig::from_yaml(config_yaml).unwrap(); - -// assert_eq!(config.clusters[0].cluster.as_ref().unwrap().server, ""); -// } - #[test] fn kubeconfig_multi_document_merge() -> Result<(), KubeconfigError> { let config_yaml = r#"--- From 57f79adc2167b2e9fe5461266799de913312f1be Mon Sep 17 00:00:00 2001 From: goenning Date: Tue, 15 Nov 2022 19:21:49 +0000 Subject: [PATCH 04/13] remove unnecessary default Signed-off-by: goenning --- kube-client/src/config/file_config.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kube-client/src/config/file_config.rs b/kube-client/src/config/file_config.rs index 5bfe506bd..219b78eaf 100644 --- a/kube-client/src/config/file_config.rs +++ b/kube-client/src/config/file_config.rs @@ -120,7 +120,7 @@ pub struct NamedAuthInfo { #[serde(default)] pub name: String, /// Information that describes identity of the user - #[serde(rename = "user", default)] + #[serde(rename = "user")] #[serde(skip_serializing_if = "Option::is_none")] pub auth_info: Option, } From 42c92fdf239d4756147006c4d74dbcc104a37868 Mon Sep 17 00:00:00 2001 From: goenning Date: Wed, 16 Nov 2022 18:20:13 +0000 Subject: [PATCH 05/13] adjust based on PR review Signed-off-by: goenning --- kube-client/src/config/file_loader.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/kube-client/src/config/file_loader.rs b/kube-client/src/config/file_loader.rs index 7eb2029cd..96676c63b 100644 --- a/kube-client/src/config/file_loader.rs +++ b/kube-client/src/config/file_loader.rs @@ -78,7 +78,7 @@ impl ConfigLoader { .clusters .iter() .find(|named_cluster| &named_cluster.name == cluster_name) - .map(|named_cluster| &named_cluster.cluster) + .and_then(|named_cluster| named_cluster.cluster.clone()) .ok_or_else(|| KubeconfigError::LoadClusterOfContext(cluster_name.clone()))?; let user_name = user.unwrap_or(¤t_context.user); @@ -86,13 +86,13 @@ impl ConfigLoader { .auth_infos .iter() .find(|named_user| &named_user.name == user_name) - .map(|named_user| &named_user.auth_info) + .and_then(|named_user| named_user.auth_info.clone()) .ok_or_else(|| KubeconfigError::FindUser(user_name.clone()))?; Ok(ConfigLoader { current_context: current_context.clone(), - cluster: cluster.clone().unwrap_or_default(), - user: user.clone().unwrap_or_default(), + cluster, + user, }) } From 5964333346190ff556414a4224a070f937906512 Mon Sep 17 00:00:00 2001 From: goenning Date: Fri, 18 Nov 2022 10:27:20 +0000 Subject: [PATCH 06/13] command and server are optional Signed-off-by: goenning --- kube-client/src/client/auth/mod.rs | 10 +++++++++- kube-client/src/config/file_config.rs | 12 ++++-------- kube-client/src/config/mod.rs | 2 ++ 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/kube-client/src/client/auth/mod.rs b/kube-client/src/client/auth/mod.rs index 6f1d2e950..7e2b0afd0 100644 --- a/kube-client/src/client/auth/mod.rs +++ b/kube-client/src/client/auth/mod.rs @@ -76,6 +76,10 @@ pub enum Error { #[error("failed to parse token-key")] ParseTokenKey(#[source] serde_json::Error), + /// command was missing from exec config + #[error("command must be specified to use exec authentication plugin")] + MissingCommand, + /// OAuth error #[cfg(feature = "oauth")] #[cfg_attr(docsrs, doc(cfg(feature = "oauth")))] @@ -461,7 +465,11 @@ pub struct ExecCredentialStatus { } fn auth_exec(auth: &ExecConfig) -> Result { - let mut cmd = Command::new(&auth.command); + let mut cmd = match &auth.command { + Some(cmd) => Command::new(cmd), + None => return Err(Error::MissingCommand), + }; + if let Some(args) = &auth.args { cmd.args(args); } diff --git a/kube-client/src/config/file_config.rs b/kube-client/src/config/file_config.rs index 219b78eaf..31a1d0a40 100644 --- a/kube-client/src/config/file_config.rs +++ b/kube-client/src/config/file_config.rs @@ -66,7 +66,6 @@ pub struct Preferences { #[cfg_attr(test, derive(PartialEq, Eq))] pub struct NamedExtension { /// Name of extension - #[serde(default)] pub name: String, /// Additional information for extenders so that reads and writes don't clobber unknown fields pub extension: serde_json::Value, @@ -77,7 +76,6 @@ pub struct NamedExtension { #[cfg_attr(test, derive(PartialEq, Eq))] pub struct NamedCluster { /// Name of cluster - #[serde(default)] pub name: String, /// Information about how to communicate with a kubernetes cluster #[serde(skip_serializing_if = "Option::is_none")] @@ -89,8 +87,8 @@ pub struct NamedCluster { #[cfg_attr(test, derive(PartialEq, Eq))] pub struct Cluster { /// The address of the kubernetes cluster (https://hostname:port). - #[serde(default)] - pub server: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub server: Option, /// Skips the validity check for the server's certificate. This will make your HTTPS connections insecure. #[serde(rename = "insecure-skip-tls-verify")] #[serde(skip_serializing_if = "Option::is_none")] @@ -117,7 +115,6 @@ pub struct Cluster { #[cfg_attr(test, derive(PartialEq))] pub struct NamedAuthInfo { /// Name of the user - #[serde(default)] pub name: String, /// Information that describes identity of the user #[serde(rename = "user")] @@ -243,8 +240,8 @@ pub struct ExecConfig { #[serde(skip_serializing_if = "Option::is_none")] pub api_version: Option, /// Command to execute. - #[serde(default)] - pub command: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub command: Option, /// Arguments to pass to the command when executing it. #[serde(skip_serializing_if = "Option::is_none")] pub args: Option>, @@ -266,7 +263,6 @@ pub struct ExecConfig { #[cfg_attr(test, derive(PartialEq, Eq))] pub struct NamedContext { /// Name of the context - #[serde(default)] pub name: String, /// Associations for the context #[serde(default)] diff --git a/kube-client/src/config/mod.rs b/kube-client/src/config/mod.rs index 77ccf4def..5d279041f 100644 --- a/kube-client/src/config/mod.rs +++ b/kube-client/src/config/mod.rs @@ -299,6 +299,8 @@ impl Config { let cluster_url = loader .cluster .server + .clone() + .unwrap_or_default() .parse::() .map_err(KubeconfigError::ParseClusterUrl)?; From 071bfd517f9e0ec909ed60ba1ebd1132d2fc411a Mon Sep 17 00:00:00 2001 From: goenning Date: Fri, 18 Nov 2022 10:53:42 +0000 Subject: [PATCH 07/13] name on named structs is optional Signed-off-by: goenning --- kube-client/src/config/file_config.rs | 44 +++++++++++++++------------ kube-client/src/config/file_loader.rs | 15 +++++---- 2 files changed, 33 insertions(+), 26 deletions(-) diff --git a/kube-client/src/config/file_config.rs b/kube-client/src/config/file_config.rs index 31a1d0a40..f053f11ca 100644 --- a/kube-client/src/config/file_config.rs +++ b/kube-client/src/config/file_config.rs @@ -66,7 +66,8 @@ pub struct Preferences { #[cfg_attr(test, derive(PartialEq, Eq))] pub struct NamedExtension { /// Name of extension - pub name: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub name: Option, /// Additional information for extenders so that reads and writes don't clobber unknown fields pub extension: serde_json::Value, } @@ -76,7 +77,8 @@ pub struct NamedExtension { #[cfg_attr(test, derive(PartialEq, Eq))] pub struct NamedCluster { /// Name of cluster - pub name: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub name: Option, /// Information about how to communicate with a kubernetes cluster #[serde(skip_serializing_if = "Option::is_none")] pub cluster: Option, @@ -115,7 +117,8 @@ pub struct Cluster { #[cfg_attr(test, derive(PartialEq))] pub struct NamedAuthInfo { /// Name of the user - pub name: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub name: Option, /// Information that describes identity of the user #[serde(rename = "user")] #[serde(skip_serializing_if = "Option::is_none")] @@ -263,10 +266,11 @@ pub struct ExecConfig { #[cfg_attr(test, derive(PartialEq, Eq))] pub struct NamedContext { /// Name of the context - pub name: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub name: Option, /// Associations for the context - #[serde(default)] - pub context: Context, + #[serde(skip_serializing_if = "Option::is_none")] + pub context: Option, } /// Context stores tuple of cluster and user information. @@ -274,11 +278,11 @@ pub struct NamedContext { #[cfg_attr(test, derive(PartialEq, Eq))] pub struct Context { /// Name of the cluster for this context - #[serde(default)] - pub cluster: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub cluster: Option, /// Name of the `AuthInfo` for this context - #[serde(default)] - pub user: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub user: Option, /// The default namespace to use on unspecified requests #[serde(skip_serializing_if = "Option::is_none")] pub namespace: Option, @@ -428,7 +432,7 @@ fn kubeconfig_from_yaml(text: &str) -> Result, KubeconfigError> #[allow(clippy::redundant_closure)] fn append_new_named(base: &mut Vec, next: Vec, f: F) where - F: Fn(&T) -> &String, + F: Fn(&T) -> &Option, { use std::collections::HashSet; base.extend({ @@ -538,7 +542,7 @@ mod tests { let kubeconfig1 = Kubeconfig { current_context: Some("default".into()), auth_infos: vec![NamedAuthInfo { - name: "red-user".into(), + name: Some("red-user".into()), auth_info: Some(AuthInfo { token: Some(SecretString::from_str("first-token").unwrap()), ..Default::default() @@ -550,7 +554,7 @@ mod tests { current_context: Some("dev".into()), auth_infos: vec![ NamedAuthInfo { - name: "red-user".into(), + name: Some("red-user".into()), auth_info: Some(AuthInfo { token: Some(SecretString::from_str("second-token").unwrap()), username: Some("red-user".into()), @@ -558,7 +562,7 @@ mod tests { }), }, NamedAuthInfo { - name: "green-user".into(), + name: Some("green-user".into()), auth_info: Some(AuthInfo { token: Some(SecretString::from_str("new-token").unwrap()), ..Default::default() @@ -572,7 +576,7 @@ mod tests { // Preserves first `current_context` assert_eq!(merged.current_context, Some("default".into())); // Auth info with the same name does not overwrite - assert_eq!(merged.auth_infos[0].name, "red-user".to_owned()); + assert_eq!(merged.auth_infos[0].name.as_ref().unwrap(), "red-user"); assert_eq!( merged.auth_infos[0] .auth_info @@ -586,7 +590,7 @@ mod tests { // Even if it's not conflicting assert_eq!(merged.auth_infos[0].auth_info.as_ref().unwrap().username, None); // New named auth info is appended - assert_eq!(merged.auth_infos[1].name, "green-user".to_owned()); + assert_eq!(merged.auth_infos[1].name.as_ref().unwrap(), "green-user"); } #[test] @@ -648,8 +652,8 @@ users: let config = Kubeconfig::from_yaml(config_yaml).unwrap(); - assert_eq!(config.clusters[0].name, "eks"); - assert_eq!(config.clusters[1].name, "minikube"); + assert_eq!(config.clusters[0].name.as_ref().unwrap(), "eks"); + assert_eq!(config.clusters[1].name.as_ref().unwrap(), "minikube"); assert_eq!( config.clusters[1].cluster.as_ref().unwrap().extensions.as_ref().unwrap()[0] .extension @@ -704,8 +708,8 @@ users: let cfg = Kubeconfig::from_yaml(config_yaml)?; // Ensure we have data from both documents: - assert_eq!(cfg.clusters[0].name, "k3d-promstack"); - assert_eq!(cfg.clusters[1].name, "k3d-k3s-default"); + assert_eq!(cfg.clusters[0].name.as_ref().unwrap(), "k3d-promstack"); + assert_eq!(cfg.clusters[1].name.as_ref().unwrap(), "k3d-k3s-default"); Ok(()) } diff --git a/kube-client/src/config/file_loader.rs b/kube-client/src/config/file_loader.rs index 96676c63b..046b87c0e 100644 --- a/kube-client/src/config/file_loader.rs +++ b/kube-client/src/config/file_loader.rs @@ -66,26 +66,29 @@ impl ConfigLoader { } else { return Err(KubeconfigError::CurrentContextNotSet); }; + let current_context = config .contexts .iter() - .find(|named_context| &named_context.name == context_name) - .map(|named_context| &named_context.context) + .find(|named_context| &named_context.name.clone().unwrap_or_default() == context_name) + .and_then(|named_context| named_context.context.clone()) .ok_or_else(|| KubeconfigError::LoadContext(context_name.clone()))?; - let cluster_name = cluster.unwrap_or(¤t_context.cluster); + let current_cluster_name = ¤t_context.cluster.clone().unwrap_or_default(); + let cluster_name = cluster.unwrap_or(current_cluster_name); let cluster = config .clusters .iter() - .find(|named_cluster| &named_cluster.name == cluster_name) + .find(|named_cluster| &named_cluster.name.clone().unwrap_or_default() == cluster_name) .and_then(|named_cluster| named_cluster.cluster.clone()) .ok_or_else(|| KubeconfigError::LoadClusterOfContext(cluster_name.clone()))?; - let user_name = user.unwrap_or(¤t_context.user); + let current_user_name = ¤t_context.user.clone().unwrap_or_default(); + let user_name = user.unwrap_or(¤t_user_name); let user = config .auth_infos .iter() - .find(|named_user| &named_user.name == user_name) + .find(|named_user| &named_user.name.clone().unwrap_or_default() == user_name) .and_then(|named_user| named_user.auth_info.clone()) .ok_or_else(|| KubeconfigError::FindUser(user_name.clone()))?; From 447e546cd68742761ff485f63c7efc7c15db267c Mon Sep 17 00:00:00 2001 From: goenning Date: Fri, 18 Nov 2022 11:03:23 +0000 Subject: [PATCH 08/13] remove CurrentContextNotSet error Signed-off-by: goenning --- kube-client/src/config/file_loader.rs | 5 ++--- kube-client/src/config/mod.rs | 4 ---- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/kube-client/src/config/file_loader.rs b/kube-client/src/config/file_loader.rs index 046b87c0e..8c8388ad2 100644 --- a/kube-client/src/config/file_loader.rs +++ b/kube-client/src/config/file_loader.rs @@ -59,12 +59,11 @@ impl ConfigLoader { cluster: Option<&String>, user: Option<&String>, ) -> Result { + let current_context_name = &config.current_context.clone().unwrap_or_default(); let context_name = if let Some(name) = context { name - } else if let Some(name) = &config.current_context { - name } else { - return Err(KubeconfigError::CurrentContextNotSet); + current_context_name }; let current_context = config diff --git a/kube-client/src/config/mod.rs b/kube-client/src/config/mod.rs index 5d279041f..2edc36df0 100644 --- a/kube-client/src/config/mod.rs +++ b/kube-client/src/config/mod.rs @@ -29,10 +29,6 @@ pub struct InferConfigError { /// Possible errors when loading kubeconfig #[derive(Error, Debug)] pub enum KubeconfigError { - /// Failed to determine current context - #[error("failed to determine current context")] - CurrentContextNotSet, - /// Kubeconfigs with mismatching kind cannot be merged #[error("kubeconfigs with mismatching kind cannot be merged")] KindMismatch, From 772e9e1806d9561f23689ddafb3da520be7fc871 Mon Sep 17 00:00:00 2001 From: goenning Date: Fri, 18 Nov 2022 19:35:26 +0000 Subject: [PATCH 09/13] run fmt Signed-off-by: goenning --- kube-client/src/config/file_config.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/kube-client/src/config/file_config.rs b/kube-client/src/config/file_config.rs index f053f11ca..8e0654b32 100644 --- a/kube-client/src/config/file_config.rs +++ b/kube-client/src/config/file_config.rs @@ -305,7 +305,7 @@ impl Kubeconfig { for mut config in kubeconfig_from_yaml(&data)? { if let Some(dir) = path.as_ref().parent() { for named in config.clusters.iter_mut() { - if let Some(cluster) = &mut named.cluster { + if let Some(cluster) = &mut named.cluster { if let Some(path) = &cluster.certificate_authority { if let Some(abs_path) = to_absolute(dir, path) { cluster.certificate_authority = Some(abs_path); @@ -314,7 +314,7 @@ impl Kubeconfig { } } for named in config.auth_infos.iter_mut() { - if let Some(auth_info) = &mut named.auth_info { + if let Some(auth_info) = &mut named.auth_info { if let Some(path) = &auth_info.client_certificate { if let Some(abs_path) = to_absolute(dir, path) { auth_info.client_certificate = Some(abs_path); @@ -654,10 +654,10 @@ users: assert_eq!(config.clusters[0].name.as_ref().unwrap(), "eks"); assert_eq!(config.clusters[1].name.as_ref().unwrap(), "minikube"); + + let cluster1 = config.clusters[1].cluster.as_ref().unwrap(); assert_eq!( - config.clusters[1].cluster.as_ref().unwrap().extensions.as_ref().unwrap()[0] - .extension - .get("provider"), + cluster1.extensions.as_ref().unwrap()[0].extension.get("provider"), Some(&Value::String("minikube.sigs.k8s.io".to_owned())) ); } From a4bbc975bd6ccb57478f55939de001c9590f3453 Mon Sep 17 00:00:00 2001 From: goenning Date: Thu, 24 Nov 2022 10:43:27 +0000 Subject: [PATCH 10/13] revert named strings to be mandatory Signed-off-by: goenning --- kube-client/src/config/file_config.rs | 35 ++++++++++++--------------- kube-client/src/config/file_loader.rs | 23 +++++++++--------- kube-client/src/config/mod.rs | 4 +++ 3 files changed, 30 insertions(+), 32 deletions(-) diff --git a/kube-client/src/config/file_config.rs b/kube-client/src/config/file_config.rs index 8e0654b32..b26e8d143 100644 --- a/kube-client/src/config/file_config.rs +++ b/kube-client/src/config/file_config.rs @@ -77,8 +77,7 @@ pub struct NamedExtension { #[cfg_attr(test, derive(PartialEq, Eq))] pub struct NamedCluster { /// Name of cluster - #[serde(skip_serializing_if = "Option::is_none")] - pub name: Option, + pub name: String, /// Information about how to communicate with a kubernetes cluster #[serde(skip_serializing_if = "Option::is_none")] pub cluster: Option, @@ -117,8 +116,7 @@ pub struct Cluster { #[cfg_attr(test, derive(PartialEq))] pub struct NamedAuthInfo { /// Name of the user - #[serde(skip_serializing_if = "Option::is_none")] - pub name: Option, + pub name: String, /// Information that describes identity of the user #[serde(rename = "user")] #[serde(skip_serializing_if = "Option::is_none")] @@ -266,8 +264,7 @@ pub struct ExecConfig { #[cfg_attr(test, derive(PartialEq, Eq))] pub struct NamedContext { /// Name of the context - #[serde(skip_serializing_if = "Option::is_none")] - pub name: Option, + pub name: String, /// Associations for the context #[serde(skip_serializing_if = "Option::is_none")] pub context: Option, @@ -278,11 +275,9 @@ pub struct NamedContext { #[cfg_attr(test, derive(PartialEq, Eq))] pub struct Context { /// Name of the cluster for this context - #[serde(skip_serializing_if = "Option::is_none")] - pub cluster: Option, + pub cluster: String, /// Name of the `AuthInfo` for this context - #[serde(skip_serializing_if = "Option::is_none")] - pub user: Option, + pub user: String, /// The default namespace to use on unspecified requests #[serde(skip_serializing_if = "Option::is_none")] pub namespace: Option, @@ -432,7 +427,7 @@ fn kubeconfig_from_yaml(text: &str) -> Result, KubeconfigError> #[allow(clippy::redundant_closure)] fn append_new_named(base: &mut Vec, next: Vec, f: F) where - F: Fn(&T) -> &Option, + F: Fn(&T) -> &String, { use std::collections::HashSet; base.extend({ @@ -542,7 +537,7 @@ mod tests { let kubeconfig1 = Kubeconfig { current_context: Some("default".into()), auth_infos: vec![NamedAuthInfo { - name: Some("red-user".into()), + name: "red-user".into(), auth_info: Some(AuthInfo { token: Some(SecretString::from_str("first-token").unwrap()), ..Default::default() @@ -554,7 +549,7 @@ mod tests { current_context: Some("dev".into()), auth_infos: vec![ NamedAuthInfo { - name: Some("red-user".into()), + name: "red-user".into(), auth_info: Some(AuthInfo { token: Some(SecretString::from_str("second-token").unwrap()), username: Some("red-user".into()), @@ -562,7 +557,7 @@ mod tests { }), }, NamedAuthInfo { - name: Some("green-user".into()), + name: "green-user".into(), auth_info: Some(AuthInfo { token: Some(SecretString::from_str("new-token").unwrap()), ..Default::default() @@ -576,7 +571,7 @@ mod tests { // Preserves first `current_context` assert_eq!(merged.current_context, Some("default".into())); // Auth info with the same name does not overwrite - assert_eq!(merged.auth_infos[0].name.as_ref().unwrap(), "red-user"); + assert_eq!(merged.auth_infos[0].name, "red-user"); assert_eq!( merged.auth_infos[0] .auth_info @@ -590,7 +585,7 @@ mod tests { // Even if it's not conflicting assert_eq!(merged.auth_infos[0].auth_info.as_ref().unwrap().username, None); // New named auth info is appended - assert_eq!(merged.auth_infos[1].name.as_ref().unwrap(), "green-user"); + assert_eq!(merged.auth_infos[1].name, "green-user"); } #[test] @@ -652,8 +647,8 @@ users: let config = Kubeconfig::from_yaml(config_yaml).unwrap(); - assert_eq!(config.clusters[0].name.as_ref().unwrap(), "eks"); - assert_eq!(config.clusters[1].name.as_ref().unwrap(), "minikube"); + assert_eq!(config.clusters[0].name, "eks"); + assert_eq!(config.clusters[1].name, "minikube"); let cluster1 = config.clusters[1].cluster.as_ref().unwrap(); assert_eq!( @@ -708,8 +703,8 @@ users: let cfg = Kubeconfig::from_yaml(config_yaml)?; // Ensure we have data from both documents: - assert_eq!(cfg.clusters[0].name.as_ref().unwrap(), "k3d-promstack"); - assert_eq!(cfg.clusters[1].name.as_ref().unwrap(), "k3d-k3s-default"); + assert_eq!(cfg.clusters[0].name, "k3d-promstack"); + assert_eq!(cfg.clusters[1].name, "k3d-k3s-default"); Ok(()) } diff --git a/kube-client/src/config/file_loader.rs b/kube-client/src/config/file_loader.rs index 8c8388ad2..fb962ed2e 100644 --- a/kube-client/src/config/file_loader.rs +++ b/kube-client/src/config/file_loader.rs @@ -59,42 +59,41 @@ impl ConfigLoader { cluster: Option<&String>, user: Option<&String>, ) -> Result { - let current_context_name = &config.current_context.clone().unwrap_or_default(); let context_name = if let Some(name) = context { name + } else if let Some(name) = &config.current_context { + name } else { - current_context_name + return Err(KubeconfigError::CurrentContextNotSet); }; let current_context = config .contexts .iter() - .find(|named_context| &named_context.name.clone().unwrap_or_default() == context_name) + .find(|named_context| &named_context.name == context_name) .and_then(|named_context| named_context.context.clone()) .ok_or_else(|| KubeconfigError::LoadContext(context_name.clone()))?; - let current_cluster_name = ¤t_context.cluster.clone().unwrap_or_default(); - let cluster_name = cluster.unwrap_or(current_cluster_name); + let cluster_name = cluster.unwrap_or(¤t_context.cluster); let cluster = config .clusters .iter() - .find(|named_cluster| &named_cluster.name.clone().unwrap_or_default() == cluster_name) + .find(|named_cluster| &named_cluster.name == cluster_name) .and_then(|named_cluster| named_cluster.cluster.clone()) .ok_or_else(|| KubeconfigError::LoadClusterOfContext(cluster_name.clone()))?; - let current_user_name = ¤t_context.user.clone().unwrap_or_default(); - let user_name = user.unwrap_or(¤t_user_name); + let user_name = user.unwrap_or(¤t_context.user); let user = config .auth_infos .iter() - .find(|named_user| &named_user.name.clone().unwrap_or_default() == user_name) + .find(|named_user| &named_user.name == user_name) .and_then(|named_user| named_user.auth_info.clone()) .ok_or_else(|| KubeconfigError::FindUser(user_name.clone()))?; Ok(ConfigLoader { - current_context: current_context.clone(), - cluster, - user, + current_context: current_context, + cluster: cluster, + user: user, }) } diff --git a/kube-client/src/config/mod.rs b/kube-client/src/config/mod.rs index 2edc36df0..5d279041f 100644 --- a/kube-client/src/config/mod.rs +++ b/kube-client/src/config/mod.rs @@ -29,6 +29,10 @@ pub struct InferConfigError { /// Possible errors when loading kubeconfig #[derive(Error, Debug)] pub enum KubeconfigError { + /// Failed to determine current context + #[error("failed to determine current context")] + CurrentContextNotSet, + /// Kubeconfigs with mismatching kind cannot be merged #[error("kubeconfigs with mismatching kind cannot be merged")] KindMismatch, From 850fb5b19d68200552868eca1f9967c7e19c8c68 Mon Sep 17 00:00:00 2001 From: goenning Date: Thu, 24 Nov 2022 10:49:11 +0000 Subject: [PATCH 11/13] other PR comments Signed-off-by: goenning --- kube-client/src/config/file_config.rs | 1 - kube-client/src/config/mod.rs | 6 +++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/kube-client/src/config/file_config.rs b/kube-client/src/config/file_config.rs index b26e8d143..46022a55b 100644 --- a/kube-client/src/config/file_config.rs +++ b/kube-client/src/config/file_config.rs @@ -223,7 +223,6 @@ impl PartialEq for AuthInfo { #[cfg_attr(test, derive(PartialEq, Eq))] pub struct AuthProviderConfig { /// Name of the auth provider - #[serde(default)] pub name: String, /// Auth provider configuration #[serde(default)] diff --git a/kube-client/src/config/mod.rs b/kube-client/src/config/mod.rs index 5d279041f..27dcd381a 100644 --- a/kube-client/src/config/mod.rs +++ b/kube-client/src/config/mod.rs @@ -69,6 +69,10 @@ pub enum KubeconfigError { #[error("the structure of the parsed kubeconfig is invalid: {0}")] InvalidStructure(#[source] serde_yaml::Error), + /// Cluster url is missing on selected cluster + #[error("cluster url is missing on selected cluster")] + MissingClusterUrl, + /// Failed to parse cluster url #[error("failed to parse cluster url: {0}")] ParseClusterUrl(#[source] http::uri::InvalidUri), @@ -300,7 +304,7 @@ impl Config { .cluster .server .clone() - .unwrap_or_default() + .ok_or(KubeconfigError::MissingClusterUrl)? .parse::() .map_err(KubeconfigError::ParseClusterUrl)?; From d86c2c622bc5d952598c7af1742644465ba99d6f Mon Sep 17 00:00:00 2001 From: goenning Date: Sat, 3 Dec 2022 05:49:34 +0000 Subject: [PATCH 12/13] revert Signed-off-by: goenning --- kube-client/src/config/file_config.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/kube-client/src/config/file_config.rs b/kube-client/src/config/file_config.rs index 46022a55b..556e12be4 100644 --- a/kube-client/src/config/file_config.rs +++ b/kube-client/src/config/file_config.rs @@ -66,7 +66,6 @@ pub struct Preferences { #[cfg_attr(test, derive(PartialEq, Eq))] pub struct NamedExtension { /// Name of extension - #[serde(skip_serializing_if = "Option::is_none")] pub name: Option, /// Additional information for extenders so that reads and writes don't clobber unknown fields pub extension: serde_json::Value, From 1f51e4ebb01e0a69b1ae912cbc5a99ffe52c126f Mon Sep 17 00:00:00 2001 From: goenning Date: Sat, 3 Dec 2022 05:52:24 +0000 Subject: [PATCH 13/13] revert Signed-off-by: goenning --- kube-client/src/config/file_config.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kube-client/src/config/file_config.rs b/kube-client/src/config/file_config.rs index 556e12be4..963bedb38 100644 --- a/kube-client/src/config/file_config.rs +++ b/kube-client/src/config/file_config.rs @@ -66,7 +66,7 @@ pub struct Preferences { #[cfg_attr(test, derive(PartialEq, Eq))] pub struct NamedExtension { /// Name of extension - pub name: Option, + pub name: String, /// Additional information for extenders so that reads and writes don't clobber unknown fields pub extension: serde_json::Value, }