Repository: tomplus/kubernetes_asyncio
Pull request #270 (2023-08-19 17:35:46)

Support loading a custom TLS server name from `kubeconfig`
This honors the `tls-server-name` parameter which can be present in a `kubeconfig` configuration file.

The implementation is similar to the one done in https://github.com/kubernetes-client/python/pull/1933

~This needs a released version of https://github.com/aio-libs/aiohttp/pull/7541~ This is implemented in [aiohttp v3.9.0](https://github.com/aio-libs/aiohttp/pull/7543).

Fix: #267 

multani: @tomplus could you take a look at this pull request preemptively?

I'm still waiting for an official release of aiohttp containing https://github.com/aio-libs/aiohttp/pull/7541 and until then, I'm not going to turn this pull request into an official one (the tests are not passing because of this).

But it would be nice to get a first overview on the general approach, in particular the patch post-client generation.
tomplus: @multani Thanks. It looks good to me. If you have time you can also create a PR to openapi-generator, it helps us to remove the patch in the feature (https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/resources/python/asyncio/rest.mustache).
multani: @tomplus Thanks, that was exactly the kind of feedback I was looking for :)

I'll make the pull request there as well then!
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/tomplus/kubernetes_asyncio/pull/270?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Tomasz+Prus) Report
Attention: `2 lines` in your changes are missing coverage. Please review.
> Comparison is base [(`8040241`)](https://app.codecov.io/gh/tomplus/kubernetes_asyncio/commit/8040241a73580b0c33211f781b753ea9038c2ac7?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Tomasz+Prus) 23.39% compared to head [(`5de47e8`)](https://app.codecov.io/gh/tomplus/kubernetes_asyncio/pull/270?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Tomasz+Prus) 23.40%.

| [Files](https://app.codecov.io/gh/tomplus/kubernetes_asyncio/pull/270?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Tomasz+Prus) | Patch % | Lines |
|---|---|---|
| [kubernetes\_asyncio/client/rest.py](https://app.codecov.io/gh/tomplus/kubernetes_asyncio/pull/270?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Tomasz+Prus#diff-a3ViZXJuZXRlc19hc3luY2lvL2NsaWVudC9yZXN0LnB5) | 33.33% | [2 Missing :warning: ](https://app.codecov.io/gh/tomplus/kubernetes_asyncio/pull/270?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Tomasz+Prus) |

<details><summary>Additional details and impacted files</summary>


```diff
@@           Coverage Diff           @@
##           master     #270   +/-   ##
=======================================
  Coverage   23.39%   23.40%           
=======================================
  Files         749      749           
  Lines      104991   105004   +13     
=======================================
+ Hits        24564    24575   +11     
- Misses      80427    80429    +2     
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/tomplus/kubernetes_asyncio/pull/270?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Tomasz+Prus).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Tomasz+Prus).

multani: The related patch for aiohttp has been released in [v3.9.0](https://github.com/aio-libs/aiohttp/releases/tag/v3.9.0): this patch also bumps the minimum version for this library.

Patch:
diff --git a/kubernetes_asyncio/client/configuration.py b/kubernetes_asyncio/client/configuration.py
index 1b403b0b..81872a5c 100644
--- a/kubernetes_asyncio/client/configuration.py
+++ b/kubernetes_asyncio/client/configuration.py
@@ -189,6 +189,10 @@ conf = client.Configuration(
         self.assert_hostname = None
         """Set this to True/False to enable/disable SSL hostname verification.
         """
+        self.tls_server_name = None
+        """SSL/TLS Server Name Indication (SNI)
+           Set this to the SNI value expected by Kubernetes API.
+        """
 
         self.connection_pool_maxsize = 100
         """This value is passed to the aiohttp to limit simultaneous connections.
diff --git a/kubernetes_asyncio/client/rest.py b/kubernetes_asyncio/client/rest.py
index 19ac9b84..0ec0505e 100644
--- a/kubernetes_asyncio/client/rest.py
+++ b/kubernetes_asyncio/client/rest.py
@@ -56,6 +56,8 @@ class RESTClientObject(object):
                 configuration.cert_file, keyfile=configuration.key_file
             )
 
+        self.server_hostname = configuration.tls_server_name
+
         if not configuration.verify_ssl:
             ssl_context.check_hostname = False
             ssl_context.verify_mode = ssl.CERT_NONE
@@ -135,6 +137,9 @@ class RESTClientObject(object):
         if query_params:
             args["url"] += '?' + urlencode(query_params)
 
+        if self.server_hostname:
+            args["server_hostname"] = self.server_hostname
+
         # For `POST`, `PUT`, `PATCH`, `OPTIONS`, `DELETE`
         if method in ['POST', 'PUT', 'PATCH', 'OPTIONS', 'DELETE']:
             if (
diff --git a/kubernetes_asyncio/config/kube_config.py b/kubernetes_asyncio/config/kube_config.py
index 2cb32142..3ea1452c 100644
--- a/kubernetes_asyncio/config/kube_config.py
+++ b/kubernetes_asyncio/config/kube_config.py
@@ -393,6 +393,8 @@ class KubeConfigLoader(object):
                         temp_file_path=self._temp_file_path).as_file()
         if 'insecure-skip-tls-verify' in self._cluster:
             self.verify_ssl = not self._cluster['insecure-skip-tls-verify']
+        if 'tls-server-name' in self._cluster:
+            self.tls_server_name = self._cluster['tls-server-name']
 
     def _set_config(self, client_configuration):
 
@@ -400,7 +402,7 @@ class KubeConfigLoader(object):
             client_configuration.api_key['BearerToken'] = self.token
 
         # copy these keys directly from self to configuration object
-        keys = ['host', 'ssl_ca_cert', 'cert_file', 'key_file', 'verify_ssl']
+        keys = ['host', 'ssl_ca_cert', 'cert_file', 'key_file', 'verify_ssl', 'tls_server_name']
         for key in keys:
             if key in self.__dict__:
                 setattr(client_configuration, key, getattr(self, key))
diff --git a/requirements.txt b/requirements.txt
index aa824b7a..c5fa241d 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -4,4 +4,4 @@ python-dateutil>=2.5.3  # BSD
 setuptools>=21.0.0  # PSF/ZPL
 urllib3>=1.24.2  # MIT
 pyyaml>=3.12  # MIT
-aiohttp>=3.7.0,<4.0.0 # # Apache-2.0
+aiohttp>=3.9.0,<4.0.0 # # Apache-2.0
diff --git a/scripts/rest_client_server_hostname_patch.diff b/scripts/rest_client_server_hostname_patch.diff
new file mode 100644
index 00000000..b034c20a
--- /dev/null
+++ b/scripts/rest_client_server_hostname_patch.diff
@@ -0,0 +1,23 @@
+diff --git a/kubernetes_asyncio/client/rest.py b/kubernetes_asyncio/client/rest.py
+index c5be7990..9e0daab4 100644
+--- a/kubernetes_asyncio/client/rest.py
++++ b/kubernetes_asyncio/client/rest.py
+@@ -56,6 +56,8 @@ class RESTClientObject(object):
+                 configuration.cert_file, keyfile=configuration.key_file
+             )
+ 
++        self.server_hostname = configuration.tls_server_name
++
+         if not configuration.verify_ssl:
+             ssl_context.check_hostname = False
+             ssl_context.verify_mode = ssl.CERT_NONE
+@@ -135,6 +137,9 @@ class RESTClientObject(object):
+         if query_params:
+             args["url"] += '?' + urlencode(query_params)
+ 
++        if self.server_hostname:
++            args["server_hostname"] = self.server_hostname
++
+         # For `POST`, `PUT`, `PATCH`, `OPTIONS`, `DELETE`
+         if method in ['POST', 'PUT', 'PATCH', 'OPTIONS', 'DELETE']:
+             if re.search('json', headers['Content-Type'], re.IGNORECASE):
diff --git a/scripts/update-client.sh b/scripts/update-client.sh
index 5c48062d..28a92add 100755
--- a/scripts/update-client.sh
+++ b/scripts/update-client.sh
@@ -68,6 +68,8 @@ echo ">>> fix generated rest client for patching with strategic merge..."
 patch "${CLIENT_ROOT}/client/rest.py" "${SCRIPT_ROOT}/rest_client_patch.diff"
 echo ">>> fix generated rest client by increasing aiohttp read buffer to 2MiB..."
 patch "${CLIENT_ROOT}/client/rest.py" "${SCRIPT_ROOT}/rest_client_patch_read_bufsize.diff"
+echo ">>> fix generated rest client to support customer server hostname TLS verification..."
+patch "${CLIENT_ROOT}/client/rest.py" "${SCRIPT_ROOT}/rest_client_server_hostname_patch.diff"
 
 
 echo ">>> Remove invalid tests (workaround https://github.com/OpenAPITools/openapi-generator/issues/5377)"


Test patch:
diff --git a/kubernetes_asyncio/config/kube_config_test.py b/kubernetes_asyncio/config/kube_config_test.py
index 9c489fc2..bee00131 100644
--- a/kubernetes_asyncio/config/kube_config_test.py
+++ b/kubernetes_asyncio/config/kube_config_test.py
@@ -72,6 +72,7 @@ TEST_CLIENT_KEY = "client-key"
 TEST_CLIENT_KEY_BASE64 = _base64(TEST_CLIENT_KEY)
 TEST_CLIENT_CERT = "client-cert"
 TEST_CLIENT_CERT_BASE64 = _base64(TEST_CLIENT_CERT)
+TEST_TLS_SERVER_NAME = "kubernetes.io"
 
 TEST_OIDC_TOKEN = "test-oidc-token"
 TEST_OIDC_INFO = "{\"name\": \"test\"}"
@@ -443,6 +444,13 @@ class TestKubeConfigLoader(BaseTestCase):
                     "user": "exec_cred_user_certificate"
                 }
             },
+            {
+                "name": "tls-server-name",
+                "context": {
+                    "cluster": "tls-server-name",
+                    "user": "ssl"
+                }
+            },
         ],
         "clusters": [
             {
@@ -488,6 +496,16 @@ class TestKubeConfigLoader(BaseTestCase):
                     "insecure-skip-tls-verify": False,
                 }
             },
+            {
+                "name": "tls-server-name",
+                "cluster": {
+                    "server": TEST_SSL_HOST,
+                    "certificate-authority-data":
+                        TEST_CERTIFICATE_AUTH_BASE64,
+                    "insecure-skip-tls-verify": False,
+                    "tls-server-name": TEST_TLS_SERVER_NAME,
+                }
+            },
         ],
         "users": [
             {
@@ -863,6 +881,22 @@ class TestKubeConfigLoader(BaseTestCase):
             active_context="ssl_verification").load_and_set(actual)
         self.assertEqual(expected, actual)
 
+    async def test_tls_server_name(self):
+        expected = FakeConfig(
+            host=TEST_SSL_HOST,
+            token=BEARER_TOKEN_FORMAT % TEST_DATA_BASE64,
+            cert_file=self._create_temp_file(TEST_CLIENT_CERT),
+            key_file=self._create_temp_file(TEST_CLIENT_KEY),
+            ssl_ca_cert=self._create_temp_file(TEST_CERTIFICATE_AUTH),
+            verify_ssl=True,
+            tls_server_name=TEST_TLS_SERVER_NAME
+        )
+        actual = FakeConfig()
+        await KubeConfigLoader(
+            config_dict=self.TEST_KUBE_CONFIG,
+            active_context="tls-server-name").load_and_set(actual)
+        self.assertEqual(expected, actual)
+
     def test_list_contexts(self):
         loader = KubeConfigLoader(
             config_dict=self.TEST_KUBE_CONFIG,