Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 14 commits into from Dec 6, 2022
2 changes: 1 addition & 1 deletion kube-client/src/client/auth/mod.rs
Expand Up @@ -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();
Expand Down
93 changes: 62 additions & 31 deletions kube-client/src/config/file_config.rs
Expand Up @@ -66,26 +66,30 @@ 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.
#[derive(Clone, Debug, Serialize, Deserialize)]
#[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
pub cluster: Cluster,
#[serde(skip_serializing_if = "Option::is_none")]
goenning marked this conversation as resolved.
Show resolved Hide resolved
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).
#[serde(default)]
goenning marked this conversation as resolved.
Show resolved Hide resolved
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")]
Expand All @@ -109,14 +113,16 @@ 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
#[serde(default)]
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>
Expand All @@ -133,8 +139,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),
}
}
Expand Down Expand Up @@ -218,8 +225,10 @@ impl PartialEq for AuthInfo {
#[cfg_attr(test, derive(PartialEq, Eq))]
pub struct AuthProviderConfig {
/// Name of the auth provider
#[serde(default)]
goenning marked this conversation as resolved.
Show resolved Hide resolved
pub name: String,
/// Auth provider configuration
#[serde(default)]
pub config: HashMap<String, String>,
}

Expand All @@ -234,6 +243,7 @@ pub struct ExecConfig {
#[serde(skip_serializing_if = "Option::is_none")]
pub api_version: Option<String>,
/// Command to execute.
#[serde(default)]
pub command: String,
/// Arguments to pass to the command when executing it.
#[serde(skip_serializing_if = "Option::is_none")]
Expand All @@ -252,22 +262,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")]
Expand All @@ -291,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);
}
}
}
}
Expand Down Expand Up @@ -525,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()
};
Expand All @@ -537,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()
Expand All @@ -562,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());
}
Expand Down Expand Up @@ -635,7 +655,7 @@ 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()))
Expand Down Expand Up @@ -701,6 +721,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#"
Expand Down
8 changes: 4 additions & 4 deletions kube-client/src/config/file_loader.rs
Expand Up @@ -78,21 +78,21 @@ 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(&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(),
cluster,
user,
})
}

Expand Down