From 1c67001037d8339165cf453b3e01f8ebed00705e Mon Sep 17 00:00:00 2001 From: Yun-Tang Hsu Date: Fri, 15 Nov 2024 19:16:11 +0000 Subject: [PATCH] Revert "Add tls_version option when creating NSXHTTPAdapter" This reverts commit 49d9ef1031d9c9373d53c9be10e00d4cc5d7bdb9. Reason for revert: Not required change Change-Id: Ie856ab86ac7659f21007881163f28e1e70b451c5 --- .../tests/unit/v3/nsxlib_testcase.py | 14 ++-- vmware_nsxlib/tests/unit/v3/test_client.py | 4 +- vmware_nsxlib/tests/unit/v3/test_cluster.py | 79 +++---------------- vmware_nsxlib/v3/cluster.py | 40 ++-------- vmware_nsxlib/v3/config.py | 4 +- 5 files changed, 25 insertions(+), 116 deletions(-) diff --git a/vmware_nsxlib/tests/unit/v3/nsxlib_testcase.py b/vmware_nsxlib/tests/unit/v3/nsxlib_testcase.py index 16dfe2cb..14f19393 100644 --- a/vmware_nsxlib/tests/unit/v3/nsxlib_testcase.py +++ b/vmware_nsxlib/tests/unit/v3/nsxlib_testcase.py @@ -14,7 +14,6 @@ # limitations under the License. # import copy -import ssl import unittest from unittest import mock @@ -117,8 +116,7 @@ def get_default_nsxlib_config(allow_passthrough=True): dns_domain=DNS_DOMAIN, allow_passthrough=allow_passthrough, realization_max_attempts=3, - realization_wait_sec=0.2, - tls_version=ssl.TLSVersion.TLSv1_3 + realization_wait_sec=0.2 ) @@ -214,7 +212,7 @@ class NsxClientTestCase(NsxLibTestCase): retries=retries or NSX_HTTP_RETRIES, insecure=insecure if insecure is not None else NSX_INSECURE, token_provider=None, - ca_file=ca_file, + ca_file=ca_file or NSX_CERT, concurrent_connections=(concurrent_connections or NSX_CONCURENT_CONN), http_timeout=http_timeout or NSX_HTTP_TIMEOUT, @@ -287,12 +285,10 @@ class NsxClientTestCase(NsxLibTestCase): else: self._session_responses = None - def new_connection(self, cluster_api, provider, tls_version=None): + def new_connection(self, cluster_api, provider): # wrapper the session so we can intercept and record calls session = super(NsxClientTestCase.MockHTTPProvider, - self).new_connection(cluster_api, - provider, - tls_version=None) + self).new_connection(cluster_api, provider) mock_adapter = mock.Mock() session_send = session.send @@ -428,5 +424,5 @@ class NsxClientTestCase(NsxLibTestCase): headers = self.default_headers() cluster.assert_called_once( method, - **{'url': url, 'verify': True, 'body': data, + **{'url': url, 'verify': NSX_CERT, 'body': data, 'headers': headers, 'cert': None, 'timeout': timeout}) diff --git a/vmware_nsxlib/tests/unit/v3/test_client.py b/vmware_nsxlib/tests/unit/v3/test_client.py index b39ca7e8..22677459 100644 --- a/vmware_nsxlib/tests/unit/v3/test_client.py +++ b/vmware_nsxlib/tests/unit/v3/test_client.py @@ -54,7 +54,7 @@ def _headers(**kwargs): def assert_call(verb, client_or_resource, - url, verify=True, + url, verify=nsxlib_testcase.NSX_CERT, data=None, headers=DFT_ACCEPT_HEADERS, timeout=(nsxlib_testcase.NSX_HTTP_TIMEOUT, nsxlib_testcase.NSX_HTTP_READ_TIMEOUT), @@ -84,7 +84,7 @@ def mock_calls_count(verb, client_or_resource): def assert_json_call(verb, client_or_resource, url, - verify=True, + verify=nsxlib_testcase.NSX_CERT, data=None, headers=JSON_DFT_ACCEPT_HEADERS, single_call=True): diff --git a/vmware_nsxlib/tests/unit/v3/test_cluster.py b/vmware_nsxlib/tests/unit/v3/test_cluster.py index e8a433d8..f5035d49 100644 --- a/vmware_nsxlib/tests/unit/v3/test_cluster.py +++ b/vmware_nsxlib/tests/unit/v3/test_cluster.py @@ -13,7 +13,6 @@ # License for the specific language governing permissions and limitations # under the License. # -import ssl import unittest from unittest import mock from urllib import parse as urlparse @@ -67,7 +66,6 @@ class RequestsHTTPProviderTestCase(unittest.TestCase): mock_api.nsxlib_config.http_timeout = 99 mock_api.nsxlib_config.conn_idle_timeout = 39 mock_api.nsxlib_config.client_cert_provider = None - mock_api.nsxlib_config.tls_version = ssl.TLSVersion.TLSv1_3 provider = cluster.NSXRequestsHTTPProvider() with mock.patch.object( cluster.TimeoutSession, 'request', @@ -164,17 +162,13 @@ class RequestsHTTPProviderTestCase(unittest.TestCase): return_value=get_sess_create_resp()): session = provider.new_connection( mock_api, cluster.Provider('9.8.7.6', 'https://9.8.7.6', - None, None, "ca_file"), - tls_version=ssl.TLSVersion.TLSv1_3 - ) + None, None, "ca_file")) - self.assertEqual(True, session.verify) + self.assertEqual("ca_file", session.verify) mock_adaptor_init.assert_called_once_with( pool_connections=1, pool_maxsize=1, max_retries=100, pool_block=False, - thumbprint=None, assert_hostname=None, nsx_cert_der=None, - tls_version=ssl.TLSVersion.TLSv1_3, - ca_file='ca_file') + thumbprint=None, assert_hostname=None, nsx_cert_der=None) @mock.patch("vmware_nsxlib.v3.debug_retry.RetryDebug.from_int") @mock.patch("vmware_nsxlib.v3.cluster.NSXHTTPAdapter.__init__") @@ -192,16 +186,13 @@ class RequestsHTTPProviderTestCase(unittest.TestCase): return_value=get_sess_create_resp()): session = provider.new_connection( mock_api, cluster.Provider('9.8.7.6', 'https://9.8.7.6', - None, None, "ca_file"), - tls_version=ssl.TLSVersion.TLSv1_3) + None, None, "ca_file")) - self.assertEqual(True, session.verify) + self.assertEqual("ca_file", session.verify) mock_adaptor_init.assert_called_once_with( pool_connections=1, pool_maxsize=1, max_retries=100, pool_block=False, - thumbprint=None, assert_hostname=False, nsx_cert_der=None, - tls_version=ssl.TLSVersion.TLSv1_3, - ca_file='ca_file') + thumbprint=None, assert_hostname=False, nsx_cert_der=None) @mock.patch("vmware_nsxlib.v3.debug_retry.RetryDebug.from_int") @mock.patch("vmware_nsxlib.v3.cluster.NSXHTTPAdapter.__init__") @@ -219,16 +210,14 @@ class RequestsHTTPProviderTestCase(unittest.TestCase): return_value=get_sess_create_resp()): session = provider.new_connection( mock_api, cluster.Provider('9.8.7.6', 'https://9.8.7.6', - None, None, None, "thumbprint"), - tls_version=ssl.TLSVersion.TLSv1_3) + None, None, None, "thumbprint")) self.assertIsNone(session.verify) mock_adaptor_init.assert_called_once_with( pool_connections=1, pool_maxsize=1, max_retries=100, pool_block=False, thumbprint="thumbprint", assert_hostname=None, - nsx_cert_der=None, tls_version=ssl.TLSVersion.TLSv1_3, - ca_file=None) + nsx_cert_der=None) @mock.patch("vmware_nsxlib.v3.debug_retry.RetryDebug.from_int") @mock.patch("vmware_nsxlib.v3.cluster.NSXHTTPAdapter.__init__") @@ -247,17 +236,14 @@ class RequestsHTTPProviderTestCase(unittest.TestCase): session = provider.new_connection( mock_api, cluster.Provider( '9.8.7.6', 'https://9.8.7.6', None, - None, None, None, "nsx_cert_der"), - tls_version=ssl.TLSVersion.TLSv1_3) + None, None, None, "nsx_cert_der")) self.assertIsNone(session.verify) mock_adaptor_init.assert_called_once_with( pool_connections=1, pool_maxsize=1, max_retries=100, pool_block=False, thumbprint=None, assert_hostname=None, - nsx_cert_der="nsx_cert_der", - tls_version=ssl.TLSVersion.TLSv1_3, - ca_file=None) + nsx_cert_der="nsx_cert_der") def test_validate_connection_keep_alive(self): mock_conn = mocks.MockRequestSessionApi() @@ -314,50 +300,8 @@ class NSXHTTPAdapterTestCase(nsxlib_testcase.NsxClientTestCase): cluster.NSXHTTPAdapter(thumbprint="thumbprint") mock_init_poolmanager.assert_called_once_with( mock.ANY, mock.ANY, block=mock.ANY, - ssl_context=mock.ANY, assert_fingerprint="thumbprint") - @mock.patch("ssl.create_default_context") - @mock.patch("ssl.SSLContext") - @mock.patch("requests.adapters.HTTPAdapter.init_poolmanager") - def test_init_poolmanager_with_tls_version( - self, mock_init_poolmanager, mock_ssl, mock_ssl_create_default): - mock_ctx = mock.Mock() - mock_ctx_default = mock.Mock() - mock_ssl.return_value = mock_ctx - mock_ssl_create_default.return_value = mock_ctx_default - cluster.NSXHTTPAdapter(thumbprint="thumbprint", - tls_version=ssl.TLSVersion.TLSv1_3, - ca_file=None) - mock_init_poolmanager.assert_called_once_with( - mock.ANY, mock.ANY, block=mock.ANY, - ssl_context=mock_ctx, - assert_fingerprint="thumbprint") - mock_ssl.assert_called_once_with(ssl.PROTOCOL_TLS_CLIENT) - mock_ssl_create_default.assert_called_once_with(cafile=None) - self.assertEqual(mock_ctx.minimum_version, - ssl.TLSVersion.TLSv1_3) - self.assertEqual(mock_ctx.check_hostname, False) - - @mock.patch("ssl.create_default_context") - @mock.patch("ssl.SSLContext") - @mock.patch("requests.adapters.HTTPAdapter.init_poolmanager") - def test_init_poolmanager_with_tls_version_with_ca( - self, mock_init_poolmanager, mock_ssl, mock_ssl_create_default): - mock_ctx = mock.Mock() - mock_ctx_default = mock.Mock() - mock_ssl_create_default.return_value = mock_ctx_default - cluster.NSXHTTPAdapter(thumbprint=None, - tls_version=ssl.TLSVersion.TLSv1_3, - ca_file='test') - mock_init_poolmanager.assert_called_once_with( - mock.ANY, mock.ANY, block=mock.ANY, - ssl_context=mock_ctx_default) - mock_ssl_create_default.assert_called_once_with(cafile='test') - mock_ctx.assert_not_called() - self.assertEqual(mock_ctx_default.minimum_version, - ssl.TLSVersion.TLSv1_3) - class NsxV3ClusteredAPITestCase(nsxlib_testcase.NsxClientTestCase): @@ -387,8 +331,7 @@ class NsxV3ClusteredAPITestCase(nsxlib_testcase.NsxClientTestCase): self._assert_providers( api, [(urlparse.urlparse(p).netloc, p) for p in conf_managers]) - @mock.patch("ssl.create_default_context") - def test_http_retries(self, mock_ssl): + def test_http_retries(self): api = self.mock_nsx_clustered_api(retries=9) with api.endpoints['1.2.3.4'].pool.item() as session: self.assertEqual( diff --git a/vmware_nsxlib/v3/cluster.py b/vmware_nsxlib/v3/cluster.py index 2b9ffd25..0c809ee5 100644 --- a/vmware_nsxlib/v3/cluster.py +++ b/vmware_nsxlib/v3/cluster.py @@ -24,7 +24,6 @@ import inspect import itertools import logging import re -import ssl import time from urllib import parse as urlparse @@ -206,7 +205,7 @@ class NSXRequestsHTTPProvider(AbstractHTTPProvider): raise exceptions.ResourceNotFound( manager=endpoint.provider.url, operation=msg) - def new_connection(self, cluster_api, provider, tls_version=None): + def new_connection(self, cluster_api, provider): config = cluster_api.nsxlib_config session = TimeoutSession(config.http_timeout, config.http_read_timeout) @@ -221,15 +220,13 @@ class NSXRequestsHTTPProvider(AbstractHTTPProvider): session.max_redirects = 0 thumbprint = None nsx_cert_der = None - ca_file = None if config.insecure: # no verification on server certificate session.verify = False elif provider.ca_file: # verify using the said ca bundle path - session.verify = True - ca_file = provider.ca_file + session.verify = provider.ca_file elif provider.nsx_cert_der: session.verify = None nsx_cert_der = provider.nsx_cert_der @@ -247,10 +244,7 @@ class NSXRequestsHTTPProvider(AbstractHTTPProvider): max_retries=RetryDebug.from_int(config.retries), pool_block=False, thumbprint=thumbprint, assert_hostname=config.ssl_assert_hostname, - nsx_cert_der=nsx_cert_der, - tls_version=tls_version, - ca_file=ca_file - ) + nsx_cert_der=nsx_cert_der) session.mount('http://', adapter) session.mount('https://', adapter) @@ -344,27 +338,9 @@ class NSXHTTPAdapter(adapters.HTTPAdapter): self.thumbprint = kwargs.pop("thumbprint", None) self.assert_hostname = kwargs.pop("assert_hostname", None) self.nsx_cert_der = kwargs.pop("nsx_cert_der", None) - self.tls_version = kwargs.pop("tls_version", None) - self.ca_file = kwargs.pop("ca_file", None) - # PROTOCOL_TLS_CLIENT supports TLSv1.2 and TLSv1.3. - # check_hostname and CERT_REQUIRED is enabled by default. - try: - self.ssl_context = ssl.create_default_context( - cafile=self.ca_file) - except ssl.SSLError: - raise exceptions.CertificateError - if self.thumbprint or self.nsx_cert_der: - self.ssl_context = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) - # Hostname checking is bypassed with thumbprint verification - self.ssl_context.check_hostname = False - self.ssl_context.verify_mode = ssl.CERT_NONE - # Set the minimum TLS version to the target version - if self.tls_version: - self.ssl_context.minimum_version = self.tls_version super(NSXHTTPAdapter, self).__init__(*args, **kwargs) def init_poolmanager(self, *args, **kwargs): - kwargs['ssl_context'] = self.ssl_context if self.thumbprint: kwargs["assert_fingerprint"] = self.thumbprint if self.assert_hostname is not None: @@ -564,8 +540,7 @@ class ClusteredAPI(object): api_rate_limit=None, api_rate_mode=None, api_log_mode=None, - enable_health_check=True, - tls_version=None): + enable_health_check=True): self._http_provider = http_provider self._keepalive_interval = keepalive_interval @@ -573,7 +548,6 @@ class ClusteredAPI(object): self._silent = False self._api_call_collectors = [] self._enable_health_check = enable_health_check - self._tls_version = tls_version def _init_cluster(*args, **kwargs): self._init_endpoints(providers, min_conns_per_pool, @@ -598,8 +572,7 @@ class ClusteredAPI(object): def _create_conn(p): def _conn(): - return self._http_provider.new_connection( - self, p, tls_version=self._tls_version) + return self._http_provider.new_connection(self, p) return _conn @@ -965,8 +938,7 @@ class NSXClusteredAPI(ClusteredAPI): api_rate_limit=self.nsxlib_config.api_rate_limit_per_endpoint, api_rate_mode=self.nsxlib_config.api_rate_mode, api_log_mode=self.nsxlib_config.api_log_mode, - enable_health_check=self.nsxlib_config.enable_health_check, - tls_version=self.nsxlib_config.tls_version) + enable_health_check=self.nsxlib_config.enable_health_check) LOG.debug("Created NSX clustered API with '%s' " "provider", self._http_provider.provider_id) diff --git a/vmware_nsxlib/v3/config.py b/vmware_nsxlib/v3/config.py index 239f48e2..581213ab 100644 --- a/vmware_nsxlib/v3/config.py +++ b/vmware_nsxlib/v3/config.py @@ -216,8 +216,7 @@ class NsxLibConfig(object): api_log_mode=None, enable_health_check=True, ssl_assert_hostname=None, - nsx_cert_der=None, - tls_version=None): + nsx_cert_der=None): self.nsx_api_managers = nsx_api_managers self._username = username @@ -252,7 +251,6 @@ class NsxLibConfig(object): self.enable_health_check = enable_health_check self.ssl_assert_hostname = ssl_assert_hostname self._nsx_cert_der = nsx_cert_der - self.tls_version = tls_version if len(nsx_api_managers) == 1 and not self.cluster_unavailable_retry: LOG.warning("When only one endpoint is provided, keepalive probes"