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()