bugfix: terminate server if invalid grpc cert is specified

synse_server/errors.py

@@ -115,3 +115,13 @@ class ServerError(SynseError):
 
     http_code = 500
     description = 'error processing the request'
+
+
+class ClientCreateError(ServerError):
+    """Synse Server was unable to create a client (e.g. gRPC).
+
+    This error does not indicate errors with communicating with a connected
+    client. It should only be used in cases where client creation itself fails
+    for any reason, whether it is invalid TLS options, or other invalid
+    configuration.
+    """

synse_server/plugin.py

@@ -7,7 +7,7 @@
 from structlog import get_logger
 from synse_grpc import client, utils
 
-from synse_server import backoff, config, loop
+from synse_server import backoff, config, errors, loop
 from synse_server.discovery import kubernetes
 from synse_server.metrics import MetricsInterceptor, Monitor
 
@@ -96,13 +96,28 @@ def register(self, address: str, protocol: str) -> str:
         # and ensure that we can connect to the plugin. These calls may raise
         # an exception - we want to let them propagate up to signal that registration
         # for the particular address failed.
-        c = client.PluginClientV3(
-            address=address,
-            protocol=protocol,
-            timeout=config.options.get('grpc.timeout'),
-            tls=config.options.get('grpc.tls.cert'),
-            interceptors=interceptors,
-        )
+        try:
+            c = client.PluginClientV3(
+                address=address,
+                protocol=protocol,
+                timeout=config.options.get('grpc.timeout'),
+                tls=config.options.get('grpc.tls.cert'),
+                interceptors=interceptors,
+            )
+        except Exception as e:
+            logger.error(
+                'failed to create plugin client',
+                address=address,
+                protocol=protocol,
+                timeout=config.options.get('grpc.timeout'),
+                tls=config.options.get('grpc.tls.cert'),
+                interceptors=interceptors,
+            )
+            raise errors.ClientCreateError('error creating plugin client') from e
+
+        # Let any exceptions here raise up. The caller should handle appropriately.
+        # Generally any exceptions raised here should not propagate past the caller,
+        # as a failure to communicate may be intermittent and should be retried later.
         meta = c.metadata()
         ver = c.version()
 
@@ -248,6 +263,12 @@ def refresh(self) -> None:
             for plugin in new:
                 try:
                     self.register(address=plugin[0], protocol=plugin[1])
+                except errors.ClientCreateError as e:
+                    logger.error(
+                        'failed client refresh - unable to configure client',
+                        address=plugin[0], protocol=plugin[1], error=e,
+                    )
+                    raise
                 except Exception as e:
                     # Do not raise. This could happen if we can't communicate with
                     # the configured plugin. Future refreshes will attempt to re-register

synse_server/server.py

@@ -11,7 +11,8 @@
 from structlog import get_logger
 
 import synse_server
-from synse_server import app, cache, config, loop, metrics, plugin, tasks
+from synse_server import (app, cache, config, errors, loop, metrics, plugin,
+                          tasks)
 from synse_server.log import setup_logger
 
 logger = get_logger()
@@ -146,6 +147,18 @@ def run(self) -> None:
         else:
             logger.info('running server without SSL (no key/cert configured)')
 
+        # Check if the gRPC cert is configured, and if so, verify that the cert
+        # exists. (https://github.com/vapor-ware/synse-server/issues/388)
+        # This is done here as opposed to waiting to fail on client creation
+        # so the server can hard fail with certs that don't exist.
+        if config.options.get('grpc.tls.cert'):
+            cert = config.options.get('grpc.tls.cert')
+            logger.debug('checking for gRPC cert', cert=cert)
+            if not os.path.exists(cert):
+                raise FileNotFoundError(
+                    f'gRPC cert not found: {cert}'
+                )
+
         # Load the plugins defined in the configuration.
         plugin.manager.refresh()
 

tests/unit/test_plugin.py

@@ -10,6 +10,7 @@
 from synse_grpc import client, errors
 from synse_grpc.api import V3Metadata, V3Version
 
+from synse_server import errors as synse_errors
 from synse_server import plugin
 
 
@@ -150,6 +151,18 @@ def test_all_ready_false_has_plugins_disabled(self):
 
         assert m.all_ready() is False
 
+    @mock.patch('synse_server.plugin.client.PluginClientV3.__init__', side_effect=ValueError)
+    def test_register_fail_client_create(self, mock_init):
+        m = plugin.PluginManager()
+
+        with pytest.raises(synse_errors.ClientCreateError):
+            m.register('localhost:5432', 'tcp')
+
+        # Ensure nothing was added to the manager.
+        assert len(m.plugins) == 0
+
+        mock_init.assert_called_once()
+
     @mock.patch('synse_server.plugin.client.PluginClientV3.metadata', side_effect=RpcError)
     def test_register_fail_metadata_call(self, mock_metadata):
         m = plugin.PluginManager()
@@ -408,6 +421,19 @@ def test_refresh_no_addresses(self):
         m.refresh()
         assert len(m.plugins) == 0
 
+    @mock.patch('synse_server.plugin.PluginManager.load', return_value=[('localhost:5001', 'tcp')])
+    @mock.patch('synse_server.plugin.PluginManager.register', side_effect=synse_errors.ClientCreateError)  # noqa
+    @mock.patch('synse_server.plugin.Plugin.refresh_state')
+    def test_refresh_client_create_error(self, mock_refresh, mock_register, mock_load):
+        m = plugin.PluginManager()
+
+        with pytest.raises(synse_errors.ClientCreateError):
+            m.refresh()
+
+        mock_load.assert_called_once()
+        mock_register.assert_called_once_with(address='localhost:5001', protocol='tcp')
+        mock_refresh.assert_not_called()
+
     @mock.patch('synse_server.plugin.PluginManager.load', return_value=[('localhost:5001', 'tcp')])
     @mock.patch('synse_server.plugin.PluginManager.register')
     @mock.patch('synse_server.plugin.Plugin.refresh_state')

tests/unit/test_server.py

@@ -64,6 +64,23 @@ def test_run_ok_with_ssl(self, mock_write, mock_run, mock_init):
         mock_init.assert_called_once()
         mock_run.assert_called_once()
 
+    @mock.patch.dict(
+        'synse_server.config.options._full_config',
+        {'grpc': {'tls': {'cert': 'test-cert'}}},
+    )
+    @mock.patch('synse_server.server.Synse._initialize')
+    @mock.patch('synse_server.loop.synse_loop.run_forever')
+    @mock.patch('sys.stdout.write')
+    def test_run_grpc_cert_error(self, mock_write, mock_run, mock_init):
+        synse = server.Synse(log_header=False)
+
+        with pytest.raises(FileNotFoundError):
+            synse.run()
+
+        mock_write.assert_not_called()
+        mock_init.assert_called_once()
+        mock_run.assert_not_called()
+
     @mock.patch('synse_server.server.Synse._initialize')
     @mock.patch('synse_server.loop.synse_loop.run_forever', side_effect=ValueError)
     def test_run_error(self, mock_run, mock_init):