From 89768d2f5fcb68875a9004edc047ccc7377db701 Mon Sep 17 00:00:00 2001 From: Harshdeep Singh Date: Fri, 6 Nov 2026 14:11:22 +0530 Subject: [PATCH] Log successful connection to the coordination backend (#4783) st2 services logged an error when connecting to the [coordination] backend failed, but nothing when the connection succeeded. This is inconsistent with the MongoDB and RabbitMQ connection logs and makes it hard to tell which services actually use coordination. Log "Connecting to ..." and "Successfully connected to ..." INFO messages in coordinator_setup(), with any password in the URL (netloc or query string) masked. The no-op driver used when no backend is configured is not logged. --- CHANGELOG.rst | 3 + st2common/st2common/services/coordination.py | 57 +++++++- .../tests/unit/services/test_coordination.py | 123 ++++++++++++++++++ 3 files changed, 182 insertions(+), 1 deletion(-) create mode 100644 st2common/tests/unit/services/test_coordination.py diff --git a/CHANGELOG.rst b/CHANGELOG.rst index e063fbff18..8203c321ed 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -38,6 +38,9 @@ Changed Added ~~~~~ * added raw_string type to allow template strings to pass through variable processing (by @guzzijones12@gmail.com) #6351 +* Services now log an ``INFO`` message when connecting to and when successfully connected to + the ``[coordination]`` backend (credentials in the URL are masked), consistent with the + existing MongoDB and RabbitMQ connection log messages. #4783 3.9.0 - October 10, 2025 ------------------------ diff --git a/st2common/st2common/services/coordination.py b/st2common/st2common/services/coordination.py index 803b8f694d..149bb048d8 100644 --- a/st2common/st2common/services/coordination.py +++ b/st2common/st2common/services/coordination.py @@ -16,6 +16,10 @@ from __future__ import absolute_import import sys +from urllib.parse import parse_qsl +from urllib.parse import urlencode +from urllib.parse import urlsplit +from urllib.parse import urlunsplit import six @@ -201,6 +205,51 @@ def get_driver_name() -> str: return driver_name +def get_masked_url(url): + """ + Return coordination backend URL with any credentials (password in the netloc part or + in the query string) masked so it can be safely logged. + + :rtype: ``str`` + """ + if not url: + return url + + try: + parsed = urlsplit(url) + except ValueError: + return "" + + netloc = parsed.netloc + query = parsed.query + masked = False + + if parsed.password is not None: + userinfo, hostinfo = netloc.rsplit("@", 1) + username = userinfo.split(":", 1)[0] + netloc = "%s:**@%s" % (username, hostinfo) + masked = True + + query_params = parse_qsl(query, keep_blank_values=True) + + if any("password" in key.lower() for key, _ in query_params): + query = urlencode( + [ + (key, "**" if "password" in key.lower() else value) + for key, value in query_params + ], + safe="*", + ) + masked = True + + if not masked: + # Nothing to mask, return URL as-is (urlunsplit would e.g. turn "zake://" into + # "zake:") + return url + + return urlunsplit((parsed.scheme, netloc, parsed.path, query, parsed.fragment)) + + def coordinator_setup(start_heart=True): """ Sets up the client for the coordination service. @@ -216,17 +265,23 @@ def coordinator_setup(start_heart=True): member_id = get_member_id() if url: + masked_url = get_masked_url(url) + LOG.info('Connecting to coordination backend "%s".' % (masked_url)) + coordinator = coordination.get_coordinator( url, member_id, lock_timeout=lock_timeout ) + coordinator.start(start_heart=start_heart) + + LOG.info('Successfully connected to coordination backend "%s".' % (masked_url)) else: # Use a no-op backend # Note: We don't use tooz to obtain a reference since for this to work we would need to # register a plugin inside setup.py entry_point and use python setup.py develop for tests # to work coordinator = NoOpDriver(member_id) + coordinator.start(start_heart=start_heart) - coordinator.start(start_heart=start_heart) return coordinator diff --git a/st2common/tests/unit/services/test_coordination.py b/st2common/tests/unit/services/test_coordination.py new file mode 100644 index 0000000000..24c99710b6 --- /dev/null +++ b/st2common/tests/unit/services/test_coordination.py @@ -0,0 +1,123 @@ +# Copyright 2026 The StackStorm Authors. +# +# 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 __future__ import absolute_import + +import unittest + +import mock +from oslo_config import cfg + +from st2common.services import coordination +import st2tests.config as tests_config + + +class CoordinationServiceTestCase(unittest.TestCase): + @classmethod + def setUpClass(cls): + super(CoordinationServiceTestCase, cls).setUpClass() + tests_config.parse_args() + + def tearDown(self): + cfg.CONF.clear_override(name="url", group="coordination") + super(CoordinationServiceTestCase, self).tearDown() + + def test_get_masked_url(self): + values = [ + (None, None), + ("", ""), + ("zake://", "zake://"), + ("file:///tmp", "file:///tmp"), + ("redis://127.0.0.1:6379", "redis://127.0.0.1:6379"), + ("redis://user@127.0.0.1:6379", "redis://user@127.0.0.1:6379"), + ("redis://user:secret@127.0.0.1:6379", "redis://user:**@127.0.0.1:6379"), + ("redis://:secret@127.0.0.1:6379", "redis://:**@127.0.0.1:6379"), + ( + "redis://127.0.0.1:26379?sentinel=mymaster&password=secret", + "redis://127.0.0.1:26379?sentinel=mymaster&password=**", + ), + ( + "redis://127.0.0.1:26379?sentinel_password=secret&timeout=5", + "redis://127.0.0.1:26379?sentinel_password=**&timeout=5", + ), + ( + "mysql://st2:secret@127.0.0.1:3306/st2", + "mysql://st2:**@127.0.0.1:3306/st2", + ), + ] + + for url, expected in values: + self.assertEqual(coordination.get_masked_url(url), expected) + if url: + self.assertNotIn("secret", coordination.get_masked_url(url)) + + @mock.patch.object(coordination, "LOG") + @mock.patch.object(coordination.coordination, "get_coordinator") + def test_coordinator_setup_logs_successful_connection( + self, mock_get_coordinator, mock_log + ): + cfg.CONF.set_override( + name="url", + override="redis://user:secret@127.0.0.1:6379", + group="coordination", + ) + + coordinator = coordination.coordinator_setup(start_heart=False) + + self.assertEqual(coordinator, mock_get_coordinator.return_value) + coordinator.start.assert_called_once_with(start_heart=False) + + info_messages = [call[0][0] for call in mock_log.info.call_args_list] + self.assertEqual( + info_messages, + [ + 'Connecting to coordination backend "redis://user:**@127.0.0.1:6379".', + 'Successfully connected to coordination backend "redis://user:**@' + '127.0.0.1:6379".', + ], + ) + + @mock.patch.object(coordination, "LOG") + @mock.patch.object(coordination.coordination, "get_coordinator") + def test_coordinator_setup_connection_failure_not_logged_as_success( + self, mock_get_coordinator, mock_log + ): + cfg.CONF.set_override( + name="url", override="redis://127.0.0.1:6379", group="coordination" + ) + mock_get_coordinator.return_value.start.side_effect = Exception( + "Connection refused" + ) + + self.assertRaisesRegex( + Exception, + "Connection refused", + coordination.coordinator_setup, + start_heart=False, + ) + + info_messages = [call[0][0] for call in mock_log.info.call_args_list] + self.assertEqual( + info_messages, + ['Connecting to coordination backend "redis://127.0.0.1:6379".'], + ) + + @mock.patch.object(coordination, "LOG") + def test_coordinator_setup_noop_driver_no_connection_log(self, mock_log): + cfg.CONF.set_override(name="url", override=None, group="coordination") + + coordinator = coordination.coordinator_setup(start_heart=False) + + self.assertIsInstance(coordinator, coordination.NoOpDriver) + mock_log.info.assert_not_called()