From b35d7d552a737bb66b83040c5e9694238050a1ff Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Mon, 16 Mar 2020 12:55:43 -0700 Subject: [PATCH 1/2] Use value from ContextDetails to populate Namespace This is a fix for: #372 This change uses the value from ContextDetails.Namespace to populate KubernetesClientConfiguration.Namespace. The issue is there's a Namespace property on both Context and ContextDetails - The property on Context is used today - The property on ContextDetails is not - The property on ContextDetails maps to the actual yaml config --- .../KubeConfigModels/ContextDetails.cs | 2 +- ...ubernetesClientConfiguration.ConfigFile.cs | 11 +++++-- .../KubernetesClientConfigurationTests.cs | 23 ++++++++++++++ .../assets/kubeconfig.no-context-details.yml | 31 +++++++++++++++++++ 4 files changed, 64 insertions(+), 3 deletions(-) create mode 100644 tests/KubernetesClient.Tests/assets/kubeconfig.no-context-details.yml diff --git a/src/KubernetesClient/KubeConfigModels/ContextDetails.cs b/src/KubernetesClient/KubeConfigModels/ContextDetails.cs index 97a94c209..43dc917fd 100644 --- a/src/KubernetesClient/KubeConfigModels/ContextDetails.cs +++ b/src/KubernetesClient/KubeConfigModels/ContextDetails.cs @@ -16,7 +16,7 @@ public class ContextDetails public string Cluster { get; set; } /// - /// Gets or sets the anem of the user for this context. + /// Gets or sets the name of the user for this context. /// [YamlMember(Alias = "user")] public string User { get; set; } diff --git a/src/KubernetesClient/KubernetesClientConfiguration.ConfigFile.cs b/src/KubernetesClient/KubernetesClientConfiguration.ConfigFile.cs index 74a150931..4ef078923 100644 --- a/src/KubernetesClient/KubernetesClientConfiguration.ConfigFile.cs +++ b/src/KubernetesClient/KubernetesClientConfiguration.ConfigFile.cs @@ -193,6 +193,13 @@ private void InitializeContext(K8SConfiguration k8SConfig, string currentContext throw new KubeConfigException($"CurrentContext: {currentContext} not found in contexts in kubeconfig"); } + if (string.IsNullOrEmpty(activeContext.ContextDetails?.Cluster)) + { + // This serves as validation for any of the properties of ContextDetails being set. + // Other locations in code assume that ContextDetails is non-null. + throw new KubeConfigException($"Cluster not set for context `{currentContext}` in kubeconfig"); + } + CurrentContext = activeContext.Name; // cluster @@ -202,7 +209,7 @@ private void InitializeContext(K8SConfiguration k8SConfig, string currentContext SetUserDetails(k8SConfig, activeContext); // namespace - Namespace = activeContext.Namespace; + Namespace = activeContext.Namespace ?? activeContext.ContextDetails?.Namespace; } private void SetClusterDetails(K8SConfiguration k8SConfig, Context activeContext) @@ -254,7 +261,7 @@ private void SetUserDetails(K8SConfiguration k8SConfig, Context activeContext) if (userDetails == null) { - throw new KubeConfigException("User not found for context {activeContext.Name} in kubeconfig"); + throw new KubeConfigException($"User not found for context {activeContext.Name} in kubeconfig"); } if (userDetails.UserCredentials == null) diff --git a/tests/KubernetesClient.Tests/KubernetesClientConfigurationTests.cs b/tests/KubernetesClient.Tests/KubernetesClientConfigurationTests.cs index de55ca3be..557c5b5c8 100755 --- a/tests/KubernetesClient.Tests/KubernetesClientConfigurationTests.cs +++ b/tests/KubernetesClient.Tests/KubernetesClientConfigurationTests.cs @@ -21,6 +21,19 @@ public void ContextHost(string context, string host) Assert.Equal(host, cfg.Host); } + /// + /// Check if namespace is properly loaded, per context + /// + [Theory] + [InlineData("federal-context", "chisel-ns")] + [InlineData("queen-anne-context", "saw-ns")] + public void ContextNamespace(string context, string @namespace) + { + var fi = new FileInfo("assets/kubeconfig.yml"); + var cfg = KubernetesClientConfiguration.BuildConfigFromConfigFile(fi, context, useRelativePaths: false); + Assert.Equal(@namespace, cfg.Namespace); + } + /// /// Checks if user-based token is loaded properly from the config file, per context /// @@ -190,6 +203,16 @@ public void NoContextsExplicit() KubernetesClientConfiguration.BuildConfigFromConfigFile(fi, "context")); } + /// + /// Checks that a KubeConfigException is thrown when the current context exists but has no details specified + /// + [Fact] + public void ContextNoDetails() + { + var fi = new FileInfo("assets/kubeconfig.no-context-details.yml"); + Assert.Throws(() => KubernetesClientConfiguration.BuildConfigFromConfigFile(fi)); + } + /// /// Checks that a KubeConfigException is thrown when the server property is not set in cluster /// diff --git a/tests/KubernetesClient.Tests/assets/kubeconfig.no-context-details.yml b/tests/KubernetesClient.Tests/assets/kubeconfig.no-context-details.yml new file mode 100644 index 000000000..62bf3e59d --- /dev/null +++ b/tests/KubernetesClient.Tests/assets/kubeconfig.no-context-details.yml @@ -0,0 +1,31 @@ +# Sample file based on https://kubernetes.io/docs/tasks/access-application-cluster/authenticate-across-clusters-kubeconfig/ +# WARNING: File includes minor fixes +--- +current-context: federal-context +apiVersion: v1 +clusters: +- cluster: + server: http://cow.org:8080 + name: cow-cluster +- cluster: + certificate-authority: assets/ca.crt + server: https://horse.org:4443 + name: horse-cluster +- cluster: + insecure-skip-tls-verify: true + server: https://pig.org:443 + name: pig-cluster +contexts: + - name: federal-context +kind: Config +users: +- name: blue-user + user: + token: blue-token +- name: green-user + user: + client-certificate: assets/client.crt + client-key: assets/client.key +- name: black-user + user: + token: black-token \ No newline at end of file From e687129d5aa1ba60eae09ae563c979e779b8bdfb Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Mon, 16 Mar 2020 13:00:27 -0700 Subject: [PATCH 2/2] Obsolete Context.Namespace This property doesn't map to anything in the YAML and thus will never be set. Other clients I checked (java, golang) don't look for a property at this level. I think this was likely a mistake, and it should be obsoleted because it will never be populated. Example: ```yaml contexts: - context: cluster: ... namespace: ... # this is ContextDetails.Namespace user: ... name: foo ``` ```yaml contexts: - context: cluster: ... namespace: ... user: ... name: foo namespace: ... # this is Context.Namespace ``` --- src/KubernetesClient/KubeConfigModels/Context.cs | 2 ++ .../KubernetesClientConfiguration.ConfigFile.cs | 2 +- .../KubernetesClientConfigurationTests.cs | 1 - 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/KubernetesClient/KubeConfigModels/Context.cs b/src/KubernetesClient/KubeConfigModels/Context.cs index 75622d034..d23fc2ddc 100644 --- a/src/KubernetesClient/KubeConfigModels/Context.cs +++ b/src/KubernetesClient/KubeConfigModels/Context.cs @@ -1,5 +1,6 @@ namespace k8s.KubeConfigModels { + using System; using YamlDotNet.Serialization; /// @@ -19,6 +20,7 @@ public class Context [YamlMember(Alias = "name")] public string Name { get; set; } + [Obsolete("This property is not set by the YAML config. Use ContextDetails.Namespace instead.")] [YamlMember(Alias = "namespace")] public string Namespace { get; set; } } diff --git a/src/KubernetesClient/KubernetesClientConfiguration.ConfigFile.cs b/src/KubernetesClient/KubernetesClientConfiguration.ConfigFile.cs index 4ef078923..e6ead2092 100644 --- a/src/KubernetesClient/KubernetesClientConfiguration.ConfigFile.cs +++ b/src/KubernetesClient/KubernetesClientConfiguration.ConfigFile.cs @@ -209,7 +209,7 @@ private void InitializeContext(K8SConfiguration k8SConfig, string currentContext SetUserDetails(k8SConfig, activeContext); // namespace - Namespace = activeContext.Namespace ?? activeContext.ContextDetails?.Namespace; + Namespace = activeContext.ContextDetails?.Namespace; } private void SetClusterDetails(K8SConfiguration k8SConfig, Context activeContext) diff --git a/tests/KubernetesClient.Tests/KubernetesClientConfigurationTests.cs b/tests/KubernetesClient.Tests/KubernetesClientConfigurationTests.cs index 557c5b5c8..564a96c75 100755 --- a/tests/KubernetesClient.Tests/KubernetesClientConfigurationTests.cs +++ b/tests/KubernetesClient.Tests/KubernetesClientConfigurationTests.cs @@ -436,7 +436,6 @@ private void AssertConfigEqual(K8SConfiguration expected, K8SConfiguration actua private void AssertContextEqual(Context expected, Context actual) { Assert.Equal(expected.Name, actual.Name); - Assert.Equal(expected.Namespace, actual.Namespace); Assert.Equal(expected.ContextDetails.Cluster, actual.ContextDetails.Cluster); Assert.Equal(expected.ContextDetails.User, actual.ContextDetails.User); Assert.Equal(expected.ContextDetails.Namespace, actual.ContextDetails.Namespace);