Merge pull request #149 from stackhpc/netapp-self-signed-caracal

[NetApp]:Cinder support for self-signed transport (2024.1)

Author: MoteHue

cinder/tests/unit/volume/drivers/netapp/dataontap/client/test_api.py

@@ -44,6 +44,37 @@ def setUp(self):
         self.root = netapp_api.NaServer('127.0.0.1')
         super(NetAppApiServerTests, self).setUp()
 
+    @ddt.data(
+        {'host': '127.0.0.1', 'ssl_cert_path': None,
+         'port': 8080, 'api_trace_pattern': None
+         },
+        {'host': '127.0.0.1', 'ssl_cert_path': '/test/fake_cert.pem',
+         'port': 8080, 'api_trace_pattern': 'pattern'
+         },
+    )
+    @ddt.unpack
+    def test__init__ssl_verify(self, host, ssl_cert_path, port,
+                               api_trace_pattern):
+
+        with mock.patch(
+            'cinder.volume.drivers.netapp.utils.setup_api_trace_pattern'
+        ) as mock_trace:
+            server = netapp_api.NaServer(
+                host=host,
+                ssl_cert_path=ssl_cert_path,
+                port=port,
+                api_trace_pattern=api_trace_pattern
+            )
+
+            self.assertEqual(server._host, host)
+            self.assertEqual(server._port, str(port) if port else None)
+            self.assertEqual(server._refresh_conn, True)
+
+            if api_trace_pattern:
+                mock_trace.assert_called_once_with(api_trace_pattern)
+            else:
+                mock_trace.assert_not_called()
+
     @ddt.data(None, 'ftp')
     def test_set_transport_type_value_error(self, transport_type):
         """Tests setting an invalid transport type"""
@@ -189,6 +220,31 @@ def test__build_opener_valid(self):
 
         self.assertTrue(mock_invoke.called)
 
+    @mock.patch('ssl._create_unverified_context')
+    @mock.patch('six.moves.urllib.request.build_opener')
+    def test_build_opener_with_ssl_verification_disabled(
+            self, mock_build_opener, mock_unverified_context):
+        self.root._ssl_verify = False
+        mock_unverified_context.return_value = 'mock_unverified_context'
+
+        self.root._build_opener()
+
+        mock_unverified_context.assert_called_once()
+        mock_build_opener.assert_called_once()
+
+    @mock.patch('urllib.request.HTTPPasswordMgrWithDefaultRealm')
+    @mock.patch('six.moves.urllib.request.build_opener')
+    def test_build_opener_with_basic_auth(self, mock_build_opener,
+                                          mock_password_mgr):
+        self.root._username = 'user'
+        self.root._password = 'pass'
+        mock_password_mgr.return_value = mock.Mock()
+
+        self.root._build_opener()
+
+        mock_password_mgr.assert_called_once()
+        mock_build_opener.assert_called_once()
+
     @ddt.data(None, zapi_fakes.FAKE_XML_STR)
     def test_send_http_request_value_error(self, na_element):
         """Tests whether invalid NaElement parameter causes error"""

cinder/tests/unit/volume/drivers/netapp/dataontap/utils/test_utils.py

@@ -98,7 +98,8 @@ def test_get_client_for_backend(self, use_legacy):
             self.mock_cmode_client.assert_called_once_with(
                 hostname='fake_hostname', password='fake_password',
                 username='fake_user', transport_type='https', port=8866,
-                trace=mock.ANY, vserver=None, api_trace_pattern="fake_regex")
+                trace=mock.ANY, vserver=None, api_trace_pattern="fake_regex",
+                ssl_cert_path='fake_ca')
             self.mock_cmode_rest_client.assert_not_called()
         else:
             self.mock_cmode_rest_client.assert_called_once_with(
@@ -124,7 +125,7 @@ def test_get_client_for_backend_with_vserver(self, use_legacy):
                 hostname='fake_hostname', password='fake_password',
                 username='fake_user', transport_type='https', port=8866,
                 trace=mock.ANY, vserver='fake_vserver',
-                api_trace_pattern="fake_regex")
+                api_trace_pattern="fake_regex", ssl_cert_path='fake_ca')
             self.mock_cmode_rest_client.assert_not_called()
         else:
             self.mock_cmode_rest_client.assert_called_once_with(

cinder/volume/drivers/netapp/common.py

@@ -61,6 +61,7 @@ def __new__(cls, *args, **kwargs):
                 reason=_('Required configuration not found'))
 
         config.append_config_values(options.netapp_proxy_opts)
+        config.append_config_values(options.netapp_transport_opts)
         na_utils.check_flags(NetAppDriver.REQUIRED_FLAGS, config)
 
         app_version = na_utils.OpenStackInfo().info()

cinder/volume/drivers/netapp/dataontap/client/api.py

@@ -20,6 +20,7 @@
 Contains classes required to issue API calls to Data ONTAP and OnCommand DFM.
 """
 import random
+import ssl
 
 from eventlet import greenthread
 from eventlet import semaphore
@@ -72,7 +73,7 @@ class NaServer(object):
 
     def __init__(self, host, server_type=SERVER_TYPE_FILER,
                  transport_type=TRANSPORT_TYPE_HTTP,
-                 style=STYLE_LOGIN_PASSWORD, username=None,
+                 style=STYLE_LOGIN_PASSWORD, ssl_cert_path=None, username=None,
                  password=None, port=None, api_trace_pattern=None):
         self._host = host
         self.set_server_type(server_type)
@@ -83,6 +84,7 @@ def __init__(self, host, server_type=SERVER_TYPE_FILER,
         self._username = username
         self._password = password
         self._refresh_conn = True
+        self._ssl_cert_path = ssl_cert_path
 
         if api_trace_pattern is not None:
             na_utils.setup_api_trace_pattern(api_trace_pattern)
@@ -305,7 +307,16 @@ def _build_opener(self):
             auth_handler = self._create_basic_auth_handler()
         else:
             auth_handler = self._create_certificate_auth_handler()
-        opener = urllib.request.build_opener(auth_handler)
+
+        # Create an SSL context based on _ssl_cert_path
+        if isinstance(self._ssl_cert_path, str):  # with cert path
+            ssl_context = (
+                ssl.create_default_context(cafile=self._ssl_cert_path))
+        else:  # Disable SSL verification
+            ssl_context = ssl._create_unverified_context()
+
+        https_handler = urllib.request.HTTPSHandler(context=ssl_context)
+        opener = urllib.request.build_opener(auth_handler, https_handler)
         self._opener = opener
 
     def _create_basic_auth_handler(self):

cinder/volume/drivers/netapp/dataontap/client/client_base.py

@@ -42,10 +42,12 @@ def __init__(self, **kwargs):
         username = kwargs['username']
         password = kwargs['password']
         api_trace_pattern = kwargs['api_trace_pattern']
+        ssl_cert_path = kwargs.get('ssl_cert_path')
         self.connection = netapp_api.NaServer(
             host=host,
             transport_type=kwargs['transport_type'],
             port=kwargs['port'],
+            ssl_cert_path=ssl_cert_path,
             username=username,
             password=password,
             api_trace_pattern=api_trace_pattern)

cinder/volume/drivers/netapp/dataontap/client/client_cmode_rest.py

@@ -73,10 +73,11 @@ def __init__(self, **kwargs):
         username = kwargs['username']
         password = kwargs['password']
         api_trace_pattern = kwargs['api_trace_pattern']
+        ssl_cert_path = kwargs['ssl_cert_path']
         self.connection = netapp_api.RestNaServer(
             host=host,
             transport_type=kwargs['transport_type'],
-            ssl_cert_path=kwargs.pop('ssl_cert_path'),
+            ssl_cert_path=ssl_cert_path,
             port=kwargs['port'],
             username=username,
             password=password,

cinder/volume/drivers/netapp/dataontap/utils/utils.py

@@ -69,6 +69,7 @@ def get_client_for_backend(backend_name, vserver_name=None, force_rest=False):
     if config.netapp_use_legacy_client and not force_rest:
         client = client_cmode.Client(
             transport_type=config.netapp_transport_type,
+            ssl_cert_path=config.netapp_ssl_cert_path,
             username=config.netapp_login,
             password=config.netapp_password,
             hostname=config.netapp_server_hostname,

releasenotes/notes/bp-netapp-self-signed-https-support-cb30081d4465acd1.yaml

@@ -0,0 +1,18 @@
+---
+features:
+  - |
+    NetApp ONTAP driver: Added support for self-signed certificate
+    support for HTTPS transport for management communication between
+    Cinder and NetApp ONTAP.
+
+    ONTAP systems utilize self-signed certificates for HTTPS management
+    access by default. These certificates are generated automatically
+    during the initial setup or deployment of ONTAP. When ssl_cert_path
+    is configured with the extracted certificate file (.PEM format),
+    Cinder establishes HTTPS communication with full certificate validation.
+    When ssl_cert_path is not provided, Cinder automatically uses HTTPS
+    with an unverified SSL context, which provides encrypted communication
+    but skips certificate validation. This allows secure transport while
+    maintaining ease of configuration with ONTAP's default self-signed
+    certificates. Administrators can extract the certificate using tools
+    such as openssl or curl for full certificate validation if desired.

tox.ini

@@ -27,6 +27,8 @@ setenv =
 deps =
   -r{toxinidir}/test-requirements.txt
   -r{toxinidir}/requirements.txt
+  # we need pkg_resources because of os-win
+  setuptools<82
 
 # By default stestr will set concurrency
 # to ncpu, to specify something else use
@@ -108,6 +110,9 @@ install_command = {[testenv:py3]install_command}
 deps = -r{toxinidir}/requirements.txt
        -r{toxinidir}/test-requirements.txt
        pylint==3.0.2
+       # we need pkg_resources because of os-win
+       setuptools<82
+
 commands =
   {toxinidir}/tools/coding-checks.sh --pylint {posargs:all}
 
@@ -156,6 +161,8 @@ allowlist_externals = rm
 deps =
   doc8
   -r{toxinidir}/doc/requirements.txt
+  # we need pkg_resources because of os-win
+  setuptools<82
 commands =
   doc8
   rm -rf doc/source/contributor/api doc/build/html doc/build/doctrees