From 7f36f21d2f29c5d909d16784e18645e0f1c7dc00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sinclert=20P=C3=A9rez?= Date: Mon, 10 Mar 2025 10:58:58 +0100 Subject: [PATCH 1/2] [LDAP] Define sync snap service --- src/charm.py | 84 +++++++++++++++++++++++++++++++++------- tests/unit/test_charm.py | 16 ++++++++ 2 files changed, 86 insertions(+), 14 deletions(-) diff --git a/src/charm.py b/src/charm.py index d2c2b1ba5a..ca2fa27fd2 100755 --- a/src/charm.py +++ b/src/charm.py @@ -16,6 +16,7 @@ from datetime import datetime from pathlib import Path from typing import Literal, get_args +from urllib.parse import urlparse import psycopg2 from charms.data_platform_libs.v0.data_interfaces import DataPeerData, DataPeerUnitData @@ -73,6 +74,7 @@ APP_SCOPE, BACKUP_USER, DATABASE_DEFAULT_NAME, + DATABASE_PORT, METRICS_PORT, MONITORING_PASSWORD_KEY, MONITORING_SNAP_SERVICE, @@ -1316,6 +1318,42 @@ def _restart_services_after_reboot(self): self._patroni.start_patroni() self.backup.start_stop_pgbackrest_service() + def _restart_metrics_service(self) -> None: + """Restart the monitoring service if the password was rotated.""" + cache = snap.SnapCache() + postgres_snap = cache[POSTGRESQL_SNAP_NAME] + + try: + snap_password = postgres_snap.get("exporter.password") + except snap.SnapError: + logger.warning("Early exit: Trying to reset metrics service with no configuration set") + return None + + if snap_password != self.get_secret(APP_SCOPE, MONITORING_PASSWORD_KEY): + self._setup_exporter() + + def _restart_ldap_sync_service(self) -> None: + """Restart the LDAP sync service in case any configuration changed.""" + if not self._patroni.member_started: + logger.debug("Restart LDAP sync early exit: Patroni has not started yet") + return + + cache = snap.SnapCache() + postgres_snap = cache[POSTGRESQL_SNAP_NAME] + sync_service = postgres_snap.services["ldap-sync"] + + if not self.is_primary and sync_service["active"]: + logger.debug("Stopping LDAP sync service. It must only run in the primary") + postgres_snap.stop(services=["ldap-sync"]) + + if self.is_primary and not self.is_ldap_enabled: + logger.debug("Stopping LDAP sync service") + postgres_snap.stop(services=["ldap-sync"]) + return + + if self.is_primary and self.is_ldap_enabled: + self._setup_ldap_sync() + def _setup_exporter(self) -> None: """Set up postgresql_exporter options.""" cache = snap.SnapCache() @@ -1339,6 +1377,36 @@ def _setup_exporter(self) -> None: postgres_snap.restart(services=[MONITORING_SNAP_SERVICE]) self.unit_peer_data.update({"exporter-started": "True"}) + def _setup_ldap_sync(self) -> None: + """Set up postgresql_ldap_sync options.""" + cache = snap.SnapCache() + postgres_snap = cache[POSTGRESQL_SNAP_NAME] + + ldap_params = self.get_ldap_parameters() + ldap_url = urlparse(ldap_params["ldapurl"]) + ldap_host = ldap_url.hostname + ldap_port = ldap_url.port + + ldap_base_dn = ldap_params["ldapbasedn"] + ldap_bind_username = ldap_params["ldapbinddn"] + ldap_bind_password = ldap_params["ldapbindpasswd"] + + postgres_snap.set({ + "ldap-sync.ldap_host": ldap_host, + "ldap-sync.ldap_port": ldap_port, + "ldap-sync.ldap_base_dn": ldap_base_dn, + "ldap-sync.ldap_bind_username": ldap_bind_username, + "ldap-sync.ldap_bind_password": ldap_bind_password, + "ldap-sync.postgres_host": "127.0.0.1", + "ldap-sync.postgres_port": DATABASE_PORT, + "ldap-sync.postgres_database": DATABASE_DEFAULT_NAME, + "ldap-sync.postgres_username": USER, + "ldap-sync.postgres_password": self._get_password(), + }) + + logger.debug("Starting LDAP sync service") + postgres_snap.restart(services=["ldap-sync"]) + def _start_primary(self, event: StartEvent) -> None: """Bootstrap the cluster.""" # Set some information needed by Patroni to bootstrap the cluster. @@ -1985,20 +2053,8 @@ def update_config(self, is_creating_backup: bool = False, no_peers: bool = False }) self._handle_postgresql_restart_need(enable_tls) - - # Restart the monitoring service if the password was rotated - cache = snap.SnapCache() - postgres_snap = cache[POSTGRESQL_SNAP_NAME] - - try: - snap_password = postgres_snap.get("exporter.password") - except snap.SnapError: - logger.warning( - "Early exit update_config: Trying to reset metrics service with no configuration set" - ) - return True - if snap_password != self.get_secret(APP_SCOPE, MONITORING_PASSWORD_KEY): - self._setup_exporter() + self._restart_metrics_service() + self._restart_ldap_sync_service() return True diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 025ab68e01..7909834d39 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -1274,6 +1274,12 @@ def test_update_config(harness): patch( "charm.PostgresqlOperatorCharm._handle_postgresql_restart_need" ) as _handle_postgresql_restart_need, + patch( + "charm.PostgresqlOperatorCharm._restart_metrics_service" + ) as _restart_metrics_service, + patch( + "charm.PostgresqlOperatorCharm._restart_ldap_sync_service" + ) as _restart_ldap_sync_service, patch("charm.Patroni.bulk_update_parameters_controller_by_patroni"), patch("charm.Patroni.member_started", new_callable=PropertyMock) as _member_started, patch( @@ -1313,10 +1319,14 @@ def test_update_config(harness): no_peers=False, ) _handle_postgresql_restart_need.assert_called_once_with(False) + _restart_ldap_sync_service.assert_called_once() + _restart_metrics_service.assert_called_once() assert "tls" not in harness.get_relation_data(rel_id, harness.charm.unit.name) # Test with TLS files available. _handle_postgresql_restart_need.reset_mock() + _restart_ldap_sync_service.reset_mock() + _restart_metrics_service.reset_mock() harness.update_relation_data( rel_id, harness.charm.unit.name, {"tls": ""} ) # Mock some data in the relation to test that it change. @@ -1338,6 +1348,8 @@ def test_update_config(harness): no_peers=False, ) _handle_postgresql_restart_need.assert_called_once() + _restart_ldap_sync_service.assert_called_once() + _restart_metrics_service.assert_called_once() assert "tls" not in harness.get_relation_data( rel_id, harness.charm.unit.name ) # The "tls" flag is set in handle_postgresql_restart_need. @@ -1347,6 +1359,8 @@ def test_update_config(harness): rel_id, harness.charm.unit.name, {"tls": ""} ) # Mock some data in the relation to test that it change. _handle_postgresql_restart_need.reset_mock() + _restart_ldap_sync_service.reset_mock() + _restart_metrics_service.reset_mock() harness.charm.update_config() _handle_postgresql_restart_need.assert_not_called() assert harness.get_relation_data(rel_id, harness.charm.unit.name)["tls"] == "enabled" @@ -1357,6 +1371,8 @@ def test_update_config(harness): ) # Mock some data in the relation to test that it doesn't change. harness.charm.update_config() _handle_postgresql_restart_need.assert_not_called() + _restart_ldap_sync_service.assert_not_called() + _restart_metrics_service.assert_not_called() assert "tls" not in harness.get_relation_data(rel_id, harness.charm.unit.name) From e4c852e2fcd392b0c7dab9d2d5eb9678f419c717 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sinclert=20P=C3=A9rez?= Date: Mon, 21 Apr 2025 14:12:57 +0200 Subject: [PATCH 2/2] [LDAP] Early exit when snap not refreshed --- src/charm.py | 51 ++++++++++++++++++++-------------------- src/utils.py | 19 +++++++++++++++ tests/unit/test_charm.py | 1 + tests/unit/test_utils.py | 20 +++++++++++++++- 4 files changed, 64 insertions(+), 27 deletions(-) diff --git a/src/charm.py b/src/charm.py index ca2fa27fd2..af3bdac712 100755 --- a/src/charm.py +++ b/src/charm.py @@ -112,7 +112,7 @@ from relations.postgresql_provider import PostgreSQLProvider from rotate_logs import RotateLogs from upgrade import PostgreSQLUpgrade, get_postgresql_dependencies_model -from utils import new_password +from utils import new_password, snap_refreshed logger = logging.getLogger(__name__) @@ -1318,11 +1318,8 @@ def _restart_services_after_reboot(self): self._patroni.start_patroni() self.backup.start_stop_pgbackrest_service() - def _restart_metrics_service(self) -> None: + def _restart_metrics_service(self, postgres_snap: snap.Snap) -> None: """Restart the monitoring service if the password was rotated.""" - cache = snap.SnapCache() - postgres_snap = cache[POSTGRESQL_SNAP_NAME] - try: snap_password = postgres_snap.get("exporter.password") except snap.SnapError: @@ -1330,16 +1327,14 @@ def _restart_metrics_service(self) -> None: return None if snap_password != self.get_secret(APP_SCOPE, MONITORING_PASSWORD_KEY): - self._setup_exporter() + self._setup_exporter(postgres_snap) - def _restart_ldap_sync_service(self) -> None: + def _restart_ldap_sync_service(self, postgres_snap: snap.Snap) -> None: """Restart the LDAP sync service in case any configuration changed.""" if not self._patroni.member_started: logger.debug("Restart LDAP sync early exit: Patroni has not started yet") return - cache = snap.SnapCache() - postgres_snap = cache[POSTGRESQL_SNAP_NAME] sync_service = postgres_snap.services["ldap-sync"] if not self.is_primary and sync_service["active"]: @@ -1352,35 +1347,31 @@ def _restart_ldap_sync_service(self) -> None: return if self.is_primary and self.is_ldap_enabled: - self._setup_ldap_sync() + self._setup_ldap_sync(postgres_snap) - def _setup_exporter(self) -> None: + def _setup_exporter(self, postgres_snap: snap.Snap | None = None) -> None: """Set up postgresql_exporter options.""" - cache = snap.SnapCache() - postgres_snap = cache[POSTGRESQL_SNAP_NAME] - - if postgres_snap.revision != next( - filter(lambda snap_package: snap_package[0] == POSTGRESQL_SNAP_NAME, SNAP_PACKAGES) - )[1]["revision"].get(platform.machine()): - logger.debug( - "Early exit _setup_exporter: snap was not refreshed to the right version yet" - ) - return + if postgres_snap is None: + cache = snap.SnapCache() + postgres_snap = cache[POSTGRESQL_SNAP_NAME] postgres_snap.set({ "exporter.user": MONITORING_USER, "exporter.password": self.get_secret(APP_SCOPE, MONITORING_PASSWORD_KEY), }) + if postgres_snap.services[MONITORING_SNAP_SERVICE]["active"] is False: postgres_snap.start(services=[MONITORING_SNAP_SERVICE], enable=True) else: postgres_snap.restart(services=[MONITORING_SNAP_SERVICE]) + self.unit_peer_data.update({"exporter-started": "True"}) - def _setup_ldap_sync(self) -> None: + def _setup_ldap_sync(self, postgres_snap: snap.Snap | None = None) -> None: """Set up postgresql_ldap_sync options.""" - cache = snap.SnapCache() - postgres_snap = cache[POSTGRESQL_SNAP_NAME] + if postgres_snap is None: + cache = snap.SnapCache() + postgres_snap = cache[POSTGRESQL_SNAP_NAME] ldap_params = self.get_ldap_parameters() ldap_url = urlparse(ldap_params["ldapurl"]) @@ -2053,8 +2044,16 @@ def update_config(self, is_creating_backup: bool = False, no_peers: bool = False }) self._handle_postgresql_restart_need(enable_tls) - self._restart_metrics_service() - self._restart_ldap_sync_service() + + cache = snap.SnapCache() + postgres_snap = cache[POSTGRESQL_SNAP_NAME] + + if not snap_refreshed(postgres_snap.revision): + logger.debug("Early exit: snap was not refreshed to the right version yet") + return True + + self._restart_metrics_service(postgres_snap) + self._restart_ldap_sync_service(postgres_snap) return True diff --git a/src/utils.py b/src/utils.py index b3f0e1abad..4f07ec87fe 100644 --- a/src/utils.py +++ b/src/utils.py @@ -3,9 +3,15 @@ """A collection of utility functions that are used in the charm.""" +import platform import secrets import string +from constants import ( + POSTGRESQL_SNAP_NAME, + SNAP_PACKAGES, +) + def new_password() -> str: """Generate a random password string. @@ -16,3 +22,16 @@ def new_password() -> str: choices = string.ascii_letters + string.digits password = "".join([secrets.choice(choices) for i in range(16)]) return password + + +def snap_refreshed(target_rev: str) -> bool: + """Whether the snap was refreshed to the target version.""" + arch = platform.machine() + + for snap_package in SNAP_PACKAGES: + snap_name = snap_package[0] + snap_revs = snap_package[1]["revision"] + if snap_name == POSTGRESQL_SNAP_NAME and target_rev != snap_revs.get(arch): + return False + + return True diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 7909834d39..ee773ed31f 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -1270,6 +1270,7 @@ def test_restart(harness): def test_update_config(harness): with ( patch("subprocess.check_output", return_value=b"C"), + patch("charm.snap_refreshed", return_value=True), patch("charm.snap.SnapCache"), patch( "charm.PostgresqlOperatorCharm._handle_postgresql_restart_need" diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index 6da8995d02..56b46a01ef 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -2,8 +2,10 @@ # See LICENSE file for licensing details. import re +from unittest.mock import patch -from utils import new_password +from constants import POSTGRESQL_SNAP_NAME +from utils import new_password, snap_refreshed def test_new_password(): @@ -16,3 +18,19 @@ def test_new_password(): second_password = new_password() assert re.fullmatch("[a-zA-Z0-9\b]{16}$", second_password) is not None assert second_password != first_password + + +def test_snap_refreshed(): + with patch( + "utils.SNAP_PACKAGES", + [(POSTGRESQL_SNAP_NAME, {"revision": {"aarch64": "100", "x86_64": "100"}})], + ): + assert snap_refreshed("100") is True + assert snap_refreshed("200") is False + + with patch( + "utils.SNAP_PACKAGES", + [(POSTGRESQL_SNAP_NAME, {"revision": {}})], + ): + assert snap_refreshed("100") is False + assert snap_refreshed("200") is False