fix: align kube-rs with client-go config parsing (#1077)

* fix: align kube-rs with client-go config parsing Signed-off-by: goenning <me@goenning.net> * remove serde_with, make field optional Signed-off-by: goenning <me@goenning.net> * remove commented test Signed-off-by: goenning <me@goenning.net> * remove unnecessary default Signed-off-by: goenning <me@goenning.net> * adjust based on PR review Signed-off-by: goenning <me@goenning.net> * command and server are optional Signed-off-by: goenning <me@goenning.net> * name on named structs is optional Signed-off-by: goenning <me@goenning.net> * remove CurrentContextNotSet error Signed-off-by: goenning <me@goenning.net> * run fmt Signed-off-by: goenning <me@goenning.net> * revert named strings to be mandatory Signed-off-by: goenning <me@goenning.net> * other PR comments Signed-off-by: goenning <me@goenning.net> * revert Signed-off-by: goenning <me@goenning.net> * revert Signed-off-by: goenning <me@goenning.net> Signed-off-by: goenning <me@goenning.net> Co-authored-by: Eirik A <sszynrae@gmail.com>
kube-rs · Dec 6, 2022 · be05e7c · be05e7c
1 parent a4bcf97
commit be05e7c
Show file tree

Hide file tree

Showing 4 changed files with 85 additions and 46 deletions.
diff --git 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")))]
@@ -476,7 +480,11 @@ pub struct ExecCredentialStatus {
 }
 
 fn auth_exec(auth: &ExecConfig) -> Result<ExecCredential, Error> {
-    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);
     }
@@ -547,7 +555,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
@@ -72,21 +72,23 @@ pub struct NamedExtension {
 }
 
 /// NamedCluster associates name with cluster.
-#[derive(Clone, Debug, Serialize, Deserialize)]
+#[derive(Clone, Debug, Serialize, Deserialize, Default)]
 #[cfg_attr(test, derive(PartialEq, Eq))]
 pub struct NamedCluster {
     /// Name of cluster
     pub name: String,
     /// Information about how to communicate with a kubernetes cluster
-    pub cluster: Cluster,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub cluster: Option<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).
-    pub server: String,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub server: Option<String>,
     /// 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")]
@@ -109,14 +111,15 @@ pub struct Cluster {
 }
 
 /// NamedAuthInfo associates name with authentication.
-#[derive(Clone, Debug, Serialize, Deserialize)]
+#[derive(Clone, Debug, Serialize, Deserialize, Default)]
 #[cfg_attr(test, derive(PartialEq))]
 pub struct NamedAuthInfo {
     /// Name of the user
     pub name: String,
     /// Information that describes identity of the user
     #[serde(rename = "user")]
-    pub auth_info: AuthInfo,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub auth_info: Option<AuthInfo>,
 }
 
 fn serialize_secretstring<S>(pw: &Option<SecretString>, serializer: S) -> Result<S::Ok, S::Error>
@@ -133,8 +136,9 @@ fn deserialize_secretstring<'de, D>(deserializer: D) -> Result<Option<SecretStri
 where
     D: Deserializer<'de>,
 {
-    match String::deserialize(deserializer) {
-        Ok(secret) => Ok(Some(SecretString::new(secret))),
+    match Option::<String>::deserialize(deserializer) {
+        Ok(Some(secret)) => Ok(Some(SecretString::new(secret))),
+        Ok(None) => Ok(None),
         Err(e) => Err(e),
     }
 }
@@ -220,6 +224,7 @@ pub struct AuthProviderConfig {
     /// Name of the auth provider
     pub name: String,
     /// Auth provider configuration
+    #[serde(default)]
     pub config: HashMap<String, String>,
 }
 
@@ -234,7 +239,8 @@ pub struct ExecConfig {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub api_version: Option<String>,
     /// Command to execute.
-    pub command: String,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub command: Option<String>,
     /// Arguments to pass to the command when executing it.
     #[serde(skip_serializing_if = "Option::is_none")]
     pub args: Option<Vec<String>>,
@@ -252,17 +258,18 @@ 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
     pub name: String,
     /// Associations for the context
-    pub context: Context,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub context: Option<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
@@ -291,26 +298,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);
+                            }
                         }
                     }
                 }
@@ -525,10 +536,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()
         };
@@ -537,18 +548,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()
@@ -558,19 +569,21 @@ 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, "red-user");
         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());
+        assert_eq!(merged.auth_infos[1].name, "green-user");
     }
 
     #[test]
@@ -634,10 +647,10 @@ users:
 
         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!(
-            config.clusters[1].cluster.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()))
         );
     }
@@ -701,6 +714,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
@@ -66,33 +66,34 @@ 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)
+            .and_then(|named_context| named_context.context.clone())
             .ok_or_else(|| KubeconfigError::LoadContext(context_name.clone()))?;
 
         let cluster_name = cluster.unwrap_or(&current_context.cluster);
         let cluster = config
             .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(&current_context.user);
         let user = config
             .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(),
-            user: user.clone(),
+            current_context: current_context,
+            cluster: cluster,
+            user: user,
         })
     }
 

diff --git 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),
@@ -299,6 +303,8 @@ impl Config {
         let cluster_url = loader
             .cluster
             .server
+            .clone()
+            .ok_or(KubeconfigError::MissingClusterUrl)?
             .parse::<http::Uri>()
             .map_err(KubeconfigError::ParseClusterUrl)?;