Merge pull request #7796 from yuxiang-zhang/fix-kubeconfig-validations

Fix kubeconfig validations

Author: stealthycoin

.changes/next-release/bugfix-eks-66737.json

@@ -0,0 +1,5 @@
+{
+  "type": "bugfix",
+  "category": "eks",
+  "description": "Fix eks kubeconfig validations closes `#6564 <https://github.com/aws/aws-cli/issues/6564>`__, fixes `#4843 <https://github.com/aws/aws-cli/issues/4843>`__, fixes `#5532 <https://github.com/aws/aws-cli/issues/5532>`__"
+}

awscli/customizations/eks/kubeconfig.py

@@ -62,10 +62,10 @@ def has_cluster(self, name):
         Return true if this kubeconfig contains an entry
         For the passed cluster name.
         """
-        if 'clusters' not in self.content:
+        if self.content.get('clusters') is None:
             return False
         return name in [cluster['name']
-                        for cluster in self.content['clusters']]
+                        for cluster in self.content['clusters'] if 'name' in cluster]
 
 
 class KubeconfigValidator(object):
@@ -83,7 +83,7 @@ def validate_config(self, config):
         """
         if not isinstance(config, Kubeconfig):
             raise KubeconfigCorruptedError("Internal error: "
-                                           "Not a Kubeconfig object.")
+                                           f"Not a {Kubeconfig}.")
         self._validate_config_types(config)
         self._validate_list_entry_types(config)
 
@@ -96,18 +96,14 @@ def _validate_config_types(self, config):
         :type config: Kubeconfig
         """
         if not isinstance(config.content, dict):
-            raise KubeconfigCorruptedError("Content not a dictionary.")
+            raise KubeconfigCorruptedError(f"Content not a {dict}.")
         for key, value in self._validation_content.items():
             if (key in config.content and
                     config.content[key] is not None and
                     not isinstance(config.content[key], type(value))):
                 raise KubeconfigCorruptedError(
-                    "{0} is wrong type:{1} "
-                    "(Should be {2})".format(
-                        key,
-                        type(config.content[key]),
-                        type(value)
-                    )
+                    f"{key} is wrong type: {type(config.content[key])} "
+                    f"(Should be {type(value)})"
                 )
 
     def _validate_list_entry_types(self, config):
@@ -124,14 +120,14 @@ def _validate_list_entry_types(self, config):
                 for element in config.content[key]:
                     if not isinstance(element, OrderedDict):
                         raise KubeconfigCorruptedError(
-                            "Entry in {0} not a dictionary.".format(key))
+                            f"Entry in {key} not a {dict}. ")
 
 
 class KubeconfigLoader(object):
-    def __init__(self, validator=None):
+    def __init__(self, validator = None):
         if validator is None:
-            validator = KubeconfigValidator()
-        self._validator = validator
+            validator=KubeconfigValidator()
+        self._validator=validator
 
     def load_kubeconfig(self, path):
         """
@@ -152,18 +148,18 @@ def load_kubeconfig(self, path):
         """
         try:
             with open(path, "r") as stream:
-                loaded_content = ordered_yaml_load(stream)
+                loaded_content=ordered_yaml_load(stream)
         except IOError as e:
             if e.errno == errno.ENOENT:
-                loaded_content = None
+                loaded_content=None
             else:
                 raise KubeconfigInaccessableError(
-                    "Can't open kubeconfig for reading: {0}".format(e))
+                    f"Can't open kubeconfig for reading: {e}")
         except yaml.YAMLError as e:
             raise KubeconfigCorruptedError(
-                "YamlError while loading kubeconfig: {0}".format(e))
+                f"YamlError while loading kubeconfig: {e}")
 
-        loaded_config = Kubeconfig(path, loaded_content)
+        loaded_config=Kubeconfig(path, loaded_content)
         self._validator.validate_config(loaded_config)
 
         return loaded_config
@@ -181,14 +177,14 @@ def write_kubeconfig(self, config):
         :raises KubeconfigInaccessableError: if the kubeconfig
         can't be opened for writing
         """
-        directory = os.path.dirname(config.path)
+        directory=os.path.dirname(config.path)
 
         try:
             os.makedirs(directory)
         except OSError as e:
             if e.errno != errno.EEXIST:
                 raise KubeconfigInaccessableError(
-                        "Can't create directory for writing: {0}".format(e))
+                    f"Can't create directory for writing: {e}")
         try:
             with os.fdopen(
                     os.open(
@@ -199,42 +195,44 @@ def write_kubeconfig(self, config):
                 ordered_yaml_dump(config.content, stream)
         except (IOError, OSError) as e:
             raise KubeconfigInaccessableError(
-                "Can't open kubeconfig for writing: {0}".format(e))
+                f"Can't open kubeconfig for writing: {e}")
 
 
 class KubeconfigAppender(object):
-    def insert_entry(self, config, key, entry):
+    def insert_entry(self, config, key, new_entry):
         """
-        Insert entry into the array at content[key]
+        Insert entry into the entries list at content[key]
         Overwrite an existing entry if they share the same name
 
         :param config: The kubeconfig to insert an entry into
         :type config: Kubeconfig
         """
-        if key not in config.content:
-            config.content[key] = []
-        array = config.content[key]
-        if not isinstance(array, list):
-            raise KubeconfigError("Tried to insert into {0},"
-                                  "which is a {1} "
-                                  "not a {2}".format(key,
-                                                     type(array),
-                                                     list))
-        found = False
-        for counter, existing_entry in enumerate(array):
-            if "name" in existing_entry and\
-               "name" in entry and\
-               existing_entry["name"] == entry["name"]:
-                array[counter] = entry
-                found = True
-
-        if not found:
-            array.append(entry)
-
-        config.content[key] = array
+        entries=self._setdefault_existing_entries(config, key)
+        same_name_index=self._index_same_name(entries, new_entry)
+        if same_name_index is None:
+            entries.append(new_entry)
+        else:
+            entries[same_name_index]=new_entry
         return config
 
-    def _make_context(self, cluster, user, alias=None):
+    def _setdefault_existing_entries(self, config, key):
+        config.content[key]=config.content.get(key) or []
+        entries=config.content[key]
+        if not isinstance(entries, list):
+            raise KubeconfigError(f"Tried to insert into {key}, "
+                                  f"which is a {type(entries)} "
+                                  f"not a {list}")
+        return entries
+
+    def _index_same_name(self, entries, new_entry):
+        if "name" in new_entry:
+            name_to_search=new_entry["name"]
+            for i, entry in enumerate(entries):
+                if "name" in entry and entry["name"] == name_to_search:
+                    return i
+        return None
+
+    def _make_context(self, cluster, user, alias = None):
         """ Generate a context to associate cluster and user with a given alias."""
         return OrderedDict([
             ("context", OrderedDict([
@@ -244,7 +242,7 @@ def _make_context(self, cluster, user, alias=None):
             ("name", alias or user["name"])
         ])
 
-    def insert_cluster_user_pair(self, config, cluster, user, alias=None):
+    def insert_cluster_user_pair(self, config, cluster, user, alias = None):
         """
         Insert the passed cluster entry and user entry,
         then make a context to associate them
@@ -266,11 +264,11 @@ def insert_cluster_user_pair(self, config, cluster, user, alias=None):
         :return: The generated context
         :rtype: OrderedDict
         """
-        context = self._make_context(cluster, user, alias=alias)
+        context=self._make_context(cluster, user, alias = alias)
         self.insert_entry(config, "clusters", cluster)
         self.insert_entry(config, "users", user)
         self.insert_entry(config, "contexts", context)
 
-        config.content["current-context"] = context["name"]
+        config.content["current-context"]=context["name"]
 
         return context

tests/functional/eks/testdata/valid_null_cluster

@@ -0,0 +1,24 @@
+apiVersion: v1
+clusters: null
+contexts:
+- context:
+    cluster: arn:aws:eks:us-west-2:111222333444:cluster/Existing
+    user: arn:aws:eks:us-west-2:111222333444:cluster/Existing
+  name: arn:aws:eks:us-west-2:111222333444:cluster/Existing
+current-context: arn:aws:eks:us-west-2:111222333444:cluster/Existing
+kind: Config
+preferences: {}
+users:
+- name: arn:aws:eks:us-west-2:111222333444:cluster/Existing
+  user:
+    exec:
+      apiVersion: client.authentication.k8s.io/v1alpha1
+      args:
+      - --region
+      - us-west-2
+      - eks
+      - get-token
+      - --cluster-name
+      - Existing
+      command: aws
+

tests/functional/eks/testdata/valid_null_context

@@ -0,0 +1,24 @@
+apiVersion: v1
+contexts: null
+clusters:
+- cluster:
+    certificate-authority-data: DATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATA=
+    server: https://existingEndpoint.eks.amazonaws.com
+  name: arn:aws:eks:us-west-2:111222333444:cluster/Existing
+current-context: arn:aws:eks:us-west-2:111222333444:cluster/Existing
+kind: Config
+preferences: {}
+users:
+- name: arn:aws:eks:us-west-2:111222333444:cluster/Existing
+  user:
+    exec:
+      apiVersion: client.authentication.k8s.io/v1alpha1
+      args:
+      - --region
+      - us-west-2
+      - eks
+      - get-token
+      - --cluster-name
+      - Existing
+      command: aws
+

tests/functional/eks/testdata/valid_null_user

@@ -0,0 +1,16 @@
+apiVersion: v1
+clusters:
+- cluster:
+    certificate-authority-data: DATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATADATA=
+    server: https://existingEndpoint.eks.amazonaws.com
+  name: arn:aws:eks:us-west-2:111222333444:cluster/Existing
+contexts:
+- context:
+    cluster: arn:aws:eks:us-west-2:111222333444:cluster/Existing
+    user: arn:aws:eks:us-west-2:111222333444:cluster/Existing
+  name: arn:aws:eks:us-west-2:111222333444:cluster/Existing
+current-context: arn:aws:eks:us-west-2:111222333444:cluster/Existing
+users: null
+kind: Config
+preferences: {}
+

tests/unit/customizations/eks/test_kubeconfig.py

@@ -65,6 +65,31 @@ def test_has_cluster_with_no_clusters(self):
         config = Kubeconfig(self._path, self._content)
         self.assertFalse(config.has_cluster("clustername"))
 
+    def test_has_cluster_with_none_clusters(self):
+        self._content["clusters"] = None
+
+        config = Kubeconfig(self._path, self._content)
+        self.assertFalse(config.has_cluster("anyclustername"))
+
+    def test_has_cluster_with_cluster_no_name(self):
+        self._content["clusters"] = [
+            OrderedDict([
+                ("cluster", None),
+                ("name", "clustername")
+            ]),
+            OrderedDict([
+                ("cluster", None),
+                ("name", None),
+            ]),
+            OrderedDict([
+                ("cluster", None),
+            ]),
+        ]
+
+        config = Kubeconfig(self._path, self._content)
+        self.assertTrue(config.has_cluster("clustername"))
+        self.assertFalse(config.has_cluster("othercluster"))
+
 
 class TestKubeconfigWriter(unittest.TestCase):
 
@@ -257,6 +282,45 @@ def test_key_not_exist(self):
                                               cluster)
         self.assertDictEqual(updated.content, correct)
 
+    def test_key_none(self):
+        initial = OrderedDict([
+            ("apiVersion", "v1"),
+            ("clusters", None),
+            ("contexts", []),
+            ("current-context", None),
+            ("kind", "Config"),
+            ("preferences", OrderedDict()),
+            ("users", [])
+        ])
+        cluster = OrderedDict([
+            ("cluster", OrderedDict([
+                ("certificate-authority-data", "data"),
+                ("server", "endpoint")
+            ])),
+            ("name", "clustername")
+        ])
+        correct = OrderedDict([
+            ("apiVersion", "v1"),
+            ("clusters", [
+                OrderedDict([
+                    ("cluster", OrderedDict([
+                        ("certificate-authority-data", "data"),
+                        ("server", "endpoint")
+                    ])),
+                    ("name", "clustername")
+                ])
+            ]),
+            ("contexts", []),
+            ("current-context", None),
+            ("kind", "Config"),
+            ("preferences", OrderedDict()),
+            ("users", []),
+        ])
+        updated = self._appender.insert_entry(Kubeconfig(None, initial),
+                                              "clusters",
+                                              cluster)
+        self.assertDictEqual(updated.content, correct)
+
     def test_key_not_array(self):
         initial = OrderedDict([
             ("apiVersion", "v1"),