Merge "Error log for missing certs with NB and SB DBs"

Author: Zuul

ovn_octavia_provider/helper.py

@@ -29,7 +29,7 @@
 from oslo_log import log as logging
 from oslo_serialization import jsonutils
 from oslo_utils import strutils
-from ovs.stream import Stream
+from ovn_octavia_provider.ovsdb import ovsdb_monitor
 from ovsdbapp.backend.ovs_idl import connection
 from ovsdbapp.backend.ovs_idl import idlutils
 from ovsdbapp.schema.ovn_northbound import commands as cmd
@@ -57,7 +57,7 @@ def __init__(self, notifier=True):
         self.helper_thread = threading.Thread(target=self.request_handler)
         self.helper_thread.daemon = True
         self._octavia_driver_lib = o_driver_lib.DriverLibrary()
-        self._check_and_set_ssl_files()
+        ovsdb_monitor.check_and_set_ssl_files('OVN_Northbound')
         self._init_lb_actions()
 
         i = impl_idl_ovn.OvnNbIdlForLb(notifier=notifier)
@@ -106,21 +106,6 @@ def _delete_disabled_from_status(status):
                 for i in v]
             for k, v in status.items()}
 
-    def _check_and_set_ssl_files(self):
-        # TODO(reedip): Make ovsdb_monitor's _check_and_set_ssl_files() public
-        # This is a copy of ovsdb_monitor._check_and_set_ssl_files
-        priv_key_file = ovn_conf.get_ovn_nb_private_key()
-        cert_file = ovn_conf.get_ovn_nb_certificate()
-        ca_cert_file = ovn_conf.get_ovn_nb_ca_cert()
-        if priv_key_file:
-            Stream.ssl_set_private_key_file(priv_key_file)
-
-        if cert_file:
-            Stream.ssl_set_certificate_file(cert_file)
-
-        if ca_cert_file:
-            Stream.ssl_set_ca_cert_file(ca_cert_file)
-
     def shutdown(self):
         self.requests.put({'type': ovn_const.REQ_TYPE_EXIT},
                           timeout=ovn_const.MAX_TIMEOUT_REQUEST)

ovn_octavia_provider/ovsdb/impl_idl_ovn.py

@@ -213,7 +213,7 @@ class OvnNbIdlForLb(ovsdb_monitor.OvnIdl):
 
     def __init__(self, event_lock_name=None, notifier=True):
         self.conn_string = config.get_ovn_nb_connection()
-        ovsdb_monitor._check_and_set_ssl_files(self.SCHEMA)
+        ovsdb_monitor.check_and_set_ssl_files(self.SCHEMA)
         helper = self._get_ovsdb_helper(self.conn_string)
         for table in OvnNbIdlForLb.TABLES:
             helper.register_table(table)
@@ -235,7 +235,7 @@ class OvnSbIdlForLb(ovsdb_monitor.OvnIdl):
 
     def __init__(self, event_lock_name=None):
         self.conn_string = config.get_ovn_sb_connection()
-        ovsdb_monitor._check_and_set_ssl_files(self.SCHEMA)
+        ovsdb_monitor.check_and_set_ssl_files(self.SCHEMA)
         helper = self._get_ovsdb_helper(self.conn_string)
         for table in OvnSbIdlForLb.TABLES:
             helper.register_table(table)

ovn_octavia_provider/ovsdb/ovsdb_monitor.py

@@ -13,6 +13,7 @@
 #    under the License.
 
 import abc
+import os
 
 from oslo_config import cfg
 from oslo_log import log
@@ -30,7 +31,7 @@
 class BaseOvnIdl(connection.OvsdbIdl):
     @classmethod
     def from_server(cls, connection_string, schema_name):
-        _check_and_set_ssl_files(schema_name)
+        check_and_set_ssl_files(schema_name)
         helper = idlutils.get_schema_helper(connection_string, schema_name)
         helper.register_all()
         return cls(connection_string, helper)
@@ -85,21 +86,33 @@ def __init__(self, driver):
         self.driver = driver
 
 
-def _check_and_set_ssl_files(schema_name):
-    if schema_name == 'OVN_Northbound':
-        priv_key_file = ovn_config.get_ovn_nb_private_key()
-        cert_file = ovn_config.get_ovn_nb_certificate()
-        ca_cert_file = ovn_config.get_ovn_nb_ca_cert()
-
-        Stream.ssl_set_private_key_file(priv_key_file)
-        Stream.ssl_set_certificate_file(cert_file)
-        Stream.ssl_set_ca_cert_file(ca_cert_file)
-
-    if schema_name == 'OVN_Southbound':
-        priv_key_file = ovn_config.get_ovn_sb_private_key()
-        cert_file = ovn_config.get_ovn_sb_certificate()
-        ca_cert_file = ovn_config.get_ovn_sb_ca_cert()
-
-        Stream.ssl_set_private_key_file(priv_key_file)
-        Stream.ssl_set_certificate_file(cert_file)
-        Stream.ssl_set_ca_cert_file(ca_cert_file)
+def check_and_set_ssl_files(schema_name):
+    if schema_name in ['OVN_Northbound', 'OVN_Southbound']:
+        if schema_name == 'OVN_Northbound':
+            priv_key_file = ovn_config.get_ovn_nb_private_key()
+            cert_file = ovn_config.get_ovn_nb_certificate()
+            ca_cert_file = ovn_config.get_ovn_nb_ca_cert()
+        else:
+            priv_key_file = ovn_config.get_ovn_sb_private_key()
+            cert_file = ovn_config.get_ovn_sb_certificate()
+            ca_cert_file = ovn_config.get_ovn_sb_ca_cert()
+
+        if priv_key_file:
+            if not os.path.exists(priv_key_file):
+                LOG.error(f"Cannot find private key file for {schema_name}")
+            else:
+                Stream.ssl_set_private_key_file(priv_key_file)
+
+        if cert_file:
+            if not os.path.exists(cert_file):
+                LOG.error(f"Cannot find private cert file for {schema_name}")
+            else:
+                Stream.ssl_set_certificate_file(cert_file)
+
+        if ca_cert_file:
+            if not os.path.exists(ca_cert_file):
+                LOG.error(f"Cannot find ca cert file for {schema_name}")
+            else:
+                Stream.ssl_set_ca_cert_file(ca_cert_file)
+    else:
+        LOG.error(f"{schema_name} not supported")

ovn_octavia_provider/tests/unit/ovsdb/test_ovsdb_monitor.py

@@ -0,0 +1,88 @@
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+#
+
+from unittest import mock
+
+from oslo_config import cfg
+import testscenarios
+
+from ovn_octavia_provider.ovsdb import ovsdb_monitor
+from ovn_octavia_provider.tests.unit import base as base_test
+
+
+OPTS = ('ovn_nb_private_key', 'ovn_nb_certificate',
+        'ovn_nb_ca_cert', 'ovn_sb_private_key',
+        'ovn_sb_certificate', 'ovn_sb_ca_cert')
+
+
+class TestOvsdbMonitor(testscenarios.WithScenarios,
+                       base_test.TestOvnOctaviaBase):
+
+    scenarios = [
+        ('OVN_Northbound', {'schema': 'OVN_Northbound',
+                            'private_key': 'ovn_nb_private_key',
+                            'certificate': 'ovn_nb_certificate',
+                            'ca_cert': 'ovn_nb_ca_cert'}),
+        ('OVN_Southbound', {'schema': 'OVN_Southbound',
+                            'private_key': 'ovn_sb_private_key',
+                            'certificate': 'ovn_sb_certificate',
+                            'ca_cert': 'ovn_sb_ca_cert'})
+    ]
+
+    def setUp(self):
+        super().setUp()
+        self._register_opts()
+        self.mock_os_path = mock.patch('os.path.exists').start()
+        self.mock_stream = mock.patch.object(ovsdb_monitor, 'Stream').start()
+
+    @staticmethod
+    def _register_opts():
+        for opt in OPTS:
+            try:
+                cfg.CONF.register_opt(cfg.StrOpt(opt), group='ovn')
+            except cfg.DuplicateOptError:
+                pass
+
+    def test_set_ssl(self):
+        cfg.CONF.set_override(self.private_key, 'key', group='ovn')
+        cfg.CONF.set_override(self.certificate, 'cert', group='ovn')
+        cfg.CONF.set_override(self.ca_cert, 'ca-cert', group='ovn')
+        self.mock_os_path.return_value = True
+        ovsdb_monitor.check_and_set_ssl_files(self.schema)
+        self.mock_stream.ssl_set_private_key_file.assert_called_with('key')
+        self.mock_stream.ssl_set_certificate_file.assert_called_with('cert')
+        self.mock_stream.ssl_set_ca_cert_file.assert_called_with('ca-cert')
+
+    def test_no_key_and_certs(self):
+        cfg.CONF.set_override(self.private_key, '', group='ovn')
+        cfg.CONF.set_override(self.certificate, '', group='ovn')
+        cfg.CONF.set_override(self.ca_cert, '', group='ovn')
+        self.mock_os_path.return_value = False
+        ovsdb_monitor.check_and_set_ssl_files(self.schema)
+        self.mock_stream.ssl_set_private_key_file.assert_not_called()
+        self.mock_stream.ssl_set_certificate_file.assert_not_called()
+        self.mock_stream.ssl_set_ca_cert_file.assert_not_called()
+
+    def test_no_nonexisting_files(self):
+        cfg.CONF.set_override(self.private_key, 'key', group='ovn')
+        cfg.CONF.set_override(self.certificate, 'cert', group='ovn')
+        cfg.CONF.set_override(self.ca_cert, 'ca-cert', group='ovn')
+        self.mock_os_path.return_value = False
+
+        with self.assertLogs():
+            ovsdb_monitor.check_and_set_ssl_files(self.schema)
+
+        self.mock_stream.ssl_set_private_key_file.assert_not_called()
+        self.mock_stream.ssl_set_certificate_file.assert_not_called()
+        self.mock_stream.ssl_set_ca_cert_file.assert_not_called()