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.

Sign up for GitHub

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

kube-config - Allow arbitrary 'extension' objects #425

Merged
merged 3 commits into from
Feb 22, 2021
Merged

kube-config - Allow arbitrary 'extension' objects #425

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c631ec8
kube-config - Allow arbitrary 'extension' objects
kppullin-nt Feb 21, 2021
d713f48
kube-config - set 'extensions' type to 'serde_json'
kppullin-nt Feb 21, 2021
d914e63
kube-config - Remove unsupported Cluster fields
kppullin-nt Feb 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 78 additions & 1 deletion kube/src/config/file_config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ pub struct Preferences {
#[derive(Clone, Debug, Serialize, Deserialize)]
pub struct NamedExtension {
pub name: String,
pub extension: String,
pub extension: serde_json::Value,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there specs for this anywhere? It's probably fine to have that as dynamic, though it is technically a breaking change for previous String extensions.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see it under https://github.com/kubernetes/client-go/blob/30548acd0a9e231d42d406433aef55b111b36240/tools/clientcmd/api/v1/types.go#L178-L184 .

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interestingly it is a runtime.RawExtension from https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/#rawextension

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't found much in the way of RawExtension specs beyond the links you've listed above and some of the references in this thread: kubernetes/apimachinery#102 and the recent PR for minikube that began using the extensions: kubernetes/minikube#10126 (which seems to have required a similar change to this one in the C# client: kubernetes-client/csharp#556)

}

/// NamedCluster associates name with cluster.
Expand All @@ -52,12 +52,17 @@ pub struct NamedCluster {
#[derive(Clone, Debug, Serialize, Deserialize)]
pub struct Cluster {
pub server: String,
#[serde(rename = "tls-server-name")]
pub tls_server_name: Option<String>,
#[serde(rename = "insecure-skip-tls-verify")]
pub insecure_skip_tls_verify: Option<bool>,
#[serde(rename = "certificate-authority")]
pub certificate_authority: Option<String>,
#[serde(rename = "certificate-authority-data")]
pub certificate_authority_data: Option<String>,
#[serde(rename = "proxy-url")]
pub proxy_url: Option<String>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, is this new? i didn't see any good info from a quick google. are we meant to actually proxy if this is set?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically we don't support proxying at the moment. We could include this in the config, but we should have a comment caveating that it's not used in our client yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better to omit these for now? My goal here is to just avoid the panics with the minikube generated config : )

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, probably best to avoid it to minimise confusion until we have support for it :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

pub extensions: Option<Vec<NamedExtension>>,
}

/// NamedAuthInfo associates name with authentication.
Expand Down Expand Up @@ -289,6 +294,7 @@ impl AuthInfo {
#[cfg(test)]
mod tests {
use super::*;
use serde_json::Value;

#[test]
fn kubeconfig_merge() {
Expand Down Expand Up @@ -336,4 +342,75 @@ mod tests {
// New named auth info is appended
assert_eq!(merged.auth_infos[1].name, "green-user".to_owned());
}

#[test]
fn kubeconfig_deserialize() {
let config_yaml = "apiVersion: v1
clusters:
- cluster:
certificate-authority-data: LS0t<SNIP>LS0tLQo=
server: https://ABCDEF0123456789.gr7.us-west-2.eks.amazonaws.com
name: eks
- cluster:
certificate-authority: /home/kevin/.minikube/ca.crt
extensions:
- extension:
last-update: Thu, 18 Feb 2021 16:59:26 PST
provider: minikube.sigs.k8s.io
version: v1.17.1
name: cluster_info
server: https://192.168.49.2:8443
name: minikube
contexts:
- context:
cluster: minikube
extensions:
- extension:
last-update: Thu, 18 Feb 2021 16:59:26 PST
provider: minikube.sigs.k8s.io
version: v1.17.1
name: context_info
namespace: default
user: minikube
name: minikube
- context:
cluster: arn:aws:eks:us-west-2:012345678912:cluster/eks
user: arn:aws:eks:us-west-2:012345678912:cluster/eks
name: eks
current-context: minikube
kind: Config
preferences: {}
users:
- name: arn:aws:eks:us-west-2:012345678912:cluster/eks
user:
exec:
apiVersion: client.authentication.k8s.io/v1alpha1
args:
- --region
- us-west-2
- eks
- get-token
- --cluster-name
- eks
command: aws
env: null
provideClusterInfo: false
- name: minikube
user:
client-certificate: /home/kevin/.minikube/profiles/minikube/client.crt
client-key: /home/kevin/.minikube/profiles/minikube/client.key";

let config: Kubeconfig = serde_yaml::from_str(config_yaml)
.map_err(ConfigError::ParseYaml)
.unwrap();

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]
.extension
.get("provider"),
Some(&Value::String("minikube.sigs.k8s.io".to_owned()))
);
}
}