From a036312705d6956b6f9511565e1f3ac17860db5a Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Thu, 2 Jul 2026 15:46:59 -0300 Subject: [PATCH 1/3] feat(tls): migrate TLS to the single-kernel library Consume the single-kernel library's operator-certificate TLS handler (events.tls.TLS) and TLSManager instead of the charm-side src/relations/tls.py, stacked on the DPE-10062 Patroni/cluster port (#1788). The TLS events handler owns the two certificate requirers and is constructor-injected into TLSManager, whose live-fetch getters read cert/key from them. A charm-side reload bridge (_reload_tls_after_push) reloads PostgreSQL after the lib handler stores+pushes certs; it also fires on relation_broken so detaching the TLS operator re-renders Patroni with TLS disabled. Removes src/relations/tls.py and push_tls_files_to_workload (now owned by the lib), routes internal-cert regeneration through _regenerate_internal_cert, redirects the operator-cert getters to tls_manager, and pins the library to the TLS stack tip via archive URL (16.3.2 is not yet released). Signed-off-by: Marcelo Henrique Neppel --- poetry.lock | 43 ++-- pyproject.toml | 2 +- src/charm.py | 128 +++++++---- src/relations/postgresql_provider.py | 2 +- src/relations/tls.py | 259 --------------------- src/relations/watcher.py | 2 +- tests/unit/test_charm.py | 49 +--- tests/unit/test_tls.py | 321 ++++++++++++++------------- 8 files changed, 284 insertions(+), 522 deletions(-) delete mode 100644 src/relations/tls.py diff --git a/poetry.lock b/poetry.lock index e8098b5a4e..f8f0cf2f33 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1865,37 +1865,40 @@ testing = ["coverage", "pytest", "pytest-benchmark"] [[package]] name = "postgresql-charms-single-kernel" -version = "16.3.1" +version = "16.3.2" description = "Shared and reusable code for PostgreSQL-related charms" optional = false -python-versions = "<4.0,>=3.8" +python-versions = ">=3.8,<4.0" groups = ["main", "integration"] files = [ - {file = "postgresql_charms_single_kernel-16.3.1-py3-none-any.whl", hash = "sha256:a1c3f32fb396e69421471d64f54a43968f134fe5538708fabe37139a9fae832e"}, - {file = "postgresql_charms_single_kernel-16.3.1.tar.gz", hash = "sha256:48e028e873b00c877fd36a25ff6c1913a40d559cb4124aae888edd1262cdce3a"}, + {file = "5aeebec.zip", hash = "sha256:76c2e468de1169912416fffaa51b474f79223b5caee16207c88ec71902d7ee6f"}, ] [package.dependencies] -charm-refresh = {version = "*", optional = true, markers = "python_full_version >= \"3.12.0\" and extra == \"postgresql\""} -charmlibs-interfaces-tls-certificates = {version = ">=1.8.3", optional = true, markers = "python_full_version >= \"3.12.0\" and extra == \"postgresql\""} -charmlibs-pathops = {version = ">=1.0.1", optional = true, markers = "python_full_version >= \"3.12.0\" and extra == \"postgresql\""} -charmlibs-rollingops = {version = ">=1.1.1", optional = true, markers = "python_full_version >= \"3.12.0\" and extra == \"postgresql\""} -charmlibs-snap = {version = ">=1.0.1", optional = true, markers = "python_full_version >= \"3.12.0\" and extra == \"vm\""} -data-platform-helpers = {version = ">=0.1.7", optional = true, markers = "python_full_version >= \"3.12.0\" and extra == \"postgresql\""} -httpx = {version = "*", optional = true, markers = "python_full_version >= \"3.12.0\" and extra == \"postgresql\""} -jinja2 = {version = ">=3.1.6", optional = true, markers = "python_full_version >= \"3.12.0\" and extra == \"postgresql\""} +charm-refresh = {version = "*", optional = true, markers = "python_version >= \"3.12\" and extra == \"postgresql\""} +charmlibs-interfaces-tls-certificates = {version = ">=1.8.3", optional = true, markers = "python_version >= \"3.12\" and extra == \"postgresql\""} +charmlibs-pathops = {version = ">=1.0.1", optional = true, markers = "python_version >= \"3.12\" and extra == \"postgresql\""} +charmlibs-rollingops = {version = ">=1.1.1", optional = true, markers = "python_version >= \"3.12\" and extra == \"postgresql\""} +charmlibs-snap = {version = ">=1.0.1", optional = true, markers = "python_version >= \"3.12\" and extra == \"vm\""} +data-platform-helpers = {version = ">=0.1.7", optional = true, markers = "python_version >= \"3.12\" and extra == \"postgresql\""} +httpx = {version = "*", optional = true, markers = "python_version >= \"3.12\" and extra == \"postgresql\""} +jinja2 = {version = ">=3.1.6", optional = true, markers = "python_version >= \"3.12\" and extra == \"postgresql\""} ops = ">=2.0.0" -psutil = {version = ">=7.2.2", optional = true, markers = "python_full_version >= \"3.12.0\" and extra == \"vm\""} -pydantic = {version = ">=2.0", optional = true, markers = "python_full_version >= \"3.12.0\" and extra == \"postgresql\""} -pysyncobj = {version = ">=0.3.15", optional = true, markers = "python_full_version >= \"3.12.0\" and extra == \"vm\""} -requests = {version = "*", optional = true, markers = "python_full_version >= \"3.12.0\" and extra == \"postgresql\""} +psutil = {version = ">=7.2.2", optional = true, markers = "python_version >= \"3.12\" and extra == \"vm\""} +pydantic = {version = ">=2.0", optional = true, markers = "python_version >= \"3.12\" and extra == \"postgresql\""} +pysyncobj = {version = ">=0.3.15", optional = true, markers = "python_version >= \"3.12\" and extra == \"vm\""} +requests = {version = "*", optional = true, markers = "python_version >= \"3.12\" and extra == \"postgresql\""} tenacity = ">=9.0.0" -tomli = {version = "*", optional = true, markers = "python_full_version >= \"3.12.0\" and extra == \"postgresql\""} +tomli = {version = "*", optional = true, markers = "python_version >= \"3.12\" and extra == \"postgresql\""} [package.extras] db-driver = ["psycopg2 (>=2.9.10)"] -postgresql = ["charm-refresh ; python_full_version >= \"3.12.0\"", "charmlibs-interfaces-tls-certificates (>=1.8.3) ; python_full_version >= \"3.12.0\"", "charmlibs-pathops (>=1.0.1) ; python_full_version >= \"3.12.0\"", "charmlibs-rollingops (>=1.1.1) ; python_full_version >= \"3.12.0\"", "data-platform-helpers (>=0.1.7) ; python_full_version >= \"3.12.0\"", "httpx ; python_full_version >= \"3.12.0\"", "jinja2 (>=3.1.6) ; python_full_version >= \"3.12.0\"", "pydantic (>=2.0) ; python_full_version >= \"3.12.0\"", "requests ; python_full_version >= \"3.12.0\"", "tomli ; python_full_version >= \"3.12.0\""] -vm = ["charmlibs-snap (>=1.0.1) ; python_full_version >= \"3.12.0\"", "psutil (>=7.2.2) ; python_full_version >= \"3.12.0\"", "pysyncobj (>=0.3.15) ; python_full_version >= \"3.12.0\""] +postgresql = ["charm-refresh ; python_version >= \"3.12\"", "charmlibs-interfaces-tls-certificates (>=1.8.3) ; python_version >= \"3.12\"", "charmlibs-pathops (>=1.0.1) ; python_version >= \"3.12\"", "charmlibs-rollingops (>=1.1.1) ; python_version >= \"3.12\"", "data-platform-helpers (>=0.1.7) ; python_version >= \"3.12\"", "httpx ; python_version >= \"3.12\"", "jinja2 (>=3.1.6) ; python_version >= \"3.12\"", "pydantic (>=2.0) ; python_version >= \"3.12\"", "requests ; python_version >= \"3.12\"", "tomli ; python_version >= \"3.12\""] +vm = ["charmlibs-snap (>=1.0.1) ; python_version >= \"3.12\"", "psutil (>=7.2.2) ; python_version >= \"3.12\"", "pysyncobj (>=0.3.15) ; python_version >= \"3.12\""] + +[package.source] +type = "url" +url = "https://github.com/canonical/postgresql-single-kernel-library/archive/5aeebec.zip" [[package]] name = "prompt-toolkit" @@ -3193,4 +3196,4 @@ h11 = ">=0.16.0,<1" [metadata] lock-version = "2.1" python-versions = ">=3.12,<4.0" -content-hash = "71742651a72b3bcf766b893c729952fb30c097d13a1e08e4ccf9f86d111a9d0c" +content-hash = "e49d6ddac7ca3a2d13571479e983cc73c99b15b4244ae803052c7e6d4c44d00e" diff --git a/pyproject.toml b/pyproject.toml index 441969572b..2fb0dacd1c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -21,7 +21,7 @@ charm-refresh = "^3.1.0.2" charmlibs-snap = "^1.0.1" charmlibs-systemd = "^1.0.0.post0" charmlibs-interfaces-tls-certificates = "^1.8.3" -postgresql-charms-single-kernel = {extras = ["postgresql", "vm"], version = "16.3.1"} +postgresql-charms-single-kernel = {url = "https://github.com/canonical/postgresql-single-kernel-library/archive/5aeebec.zip", extras = ["postgresql", "vm"]} [tool.poetry.group.charm-libs.dependencies] # data_platform_libs/v0/data_interfaces.py diff --git a/src/charm.py b/src/charm.py index 0b9626453f..614b0329cd 100755 --- a/src/charm.py +++ b/src/charm.py @@ -109,10 +109,8 @@ SNAP_USER, SPI_MODULE, SYSTEM_USERS, - TLS_CA_BUNDLE_FILE, - TLS_CA_FILE, - TLS_CERT_FILE, - TLS_KEY_FILE, + TLS_CLIENT_RELATION, + TLS_PEER_RELATION, TRACING_PROTOCOL, TRACING_RELATION_NAME, UNIT_SCOPE, @@ -121,12 +119,13 @@ ) from single_kernel_postgresql.core.config import CharmConfig from single_kernel_postgresql.core.state import CharmState +from single_kernel_postgresql.events.tls import TLS from single_kernel_postgresql.events.tls_transfer import TLSTransfer from single_kernel_postgresql.managers.cluster import ClusterManager from single_kernel_postgresql.managers.config import ConfigManager from single_kernel_postgresql.managers.patroni import PatroniManager from single_kernel_postgresql.managers.tls import TLSManager -from single_kernel_postgresql.utils import label2name, new_password, render_file +from single_kernel_postgresql.utils import label2name, new_password from single_kernel_postgresql.utils.postgresql import ( ACCESS_GROUP_IDENTITY, ACCESS_GROUPS, @@ -155,7 +154,6 @@ ) from constants import ( MONITORING_SNAP_SERVICE, - PATRONI_CONF_PATH, PGBACKREST_MONITORING_SNAP_SERVICE, POSTGRESQL_DATA_DIR, RAFT_PARTNER_PREFIX, @@ -167,7 +165,6 @@ from ldap import PostgreSQLLDAP from relations.async_replication import PostgreSQLAsyncReplication from relations.postgresql_provider import PostgreSQLProvider -from relations.tls import TLS from relations.watcher import PostgreSQLWatcherRelation from rotate_logs import RotateLogs @@ -379,11 +376,10 @@ def __init__(self, *args): # TODO switch to the abstract class base # State - self.state = CharmState(charm=self, substrate=self.substrate) # type: ignore + self.state = CharmState(charm=self, substrate=self.substrate) # Managers self.patroni_manager = PatroniManager(state=self.state, workload=self.workload) - self.tls_manager = TLSManager(state=self.state, workload=self.workload) self.cluster_manager = ClusterManager(state=self.state, workload=self.workload) self.config_manager = ConfigManager(state=self.state, workload=self.workload) @@ -421,7 +417,31 @@ def __init__(self, *args): self.postgresql_client_relation = PostgreSQLProvider(self) self.backup = PostgreSQLBackups(self, "s3-parameters") self.ldap = PostgreSQLLDAP(self, "ldap") - self.tls = TLS(self, PEER_RELATION) + # TLS events handler owns the two cert requirers; build it before the TLS + # manager so the manager can constructor-inject them for its live-fetch getters. + self.tls = TLS(self, self.state, self.workload) + self.tls_manager = TLSManager( + state=self.state, + workload=self.workload, + client_certificate=self.tls.client_certificate, + peer_certificate=self.tls.peer_certificate, + ) + # Bridge the lib TLS handler's requirer events back into a PostgreSQL reload: + # the lib handler stores+pushes certs on certificate_available, then we reload. + # Also fires on relation_broken so detaching the TLS operator re-renders Patroni + # with TLS disabled. + self.framework.observe( + self.tls.client_certificate.on.certificate_available, self._reload_tls_after_push + ) + self.framework.observe( + self.tls.peer_certificate.on.certificate_available, self._reload_tls_after_push + ) + self.framework.observe( + self.on[TLS_CLIENT_RELATION].relation_broken, self._reload_tls_after_push + ) + self.framework.observe( + self.on[TLS_PEER_RELATION].relation_broken, self._reload_tls_after_push + ) self.tls_transfer = TLSTransfer(self, PEER_RELATION) self.async_replication = PostgreSQLAsyncReplication(self) self.watcher_offer = PostgreSQLWatcherRelation(self) @@ -498,6 +518,49 @@ def substrate(self) -> Substrates: """ return Substrates.VM + def _reload_tls_after_push(self, event) -> None: + """Reload PostgreSQL after the lib TLS handler stores+pushes certs. + + Also fires on the TLS relations' relation_broken so detaching the TLS + operator re-renders Patroni with TLS disabled and reloads. + + Mirror the handler's readiness guard: when the internal CA is absent the + handler defers its push (no files on disk), so skip the reload to avoid + rendering ssl:on against missing TLS files on an already-running unit. + + A transient config-apply failure (Patroni API unreachable, member not + started) defers and retries rather than leaving stale TLS state or failing + the hook. + """ + if not self.get_secret(APP_SCOPE, "internal-ca"): + return + # Don't enable TLS in the config until the lib has written the cert files to + # disk (its push can defer while this local render would still succeed, which + # would start Patroni ssl:on against missing files). + if self.is_tls_enabled and not self.tls_manager.client_tls_files_on_disk(): + event.defer() + return + try: + if not self.update_config(): + event.defer() + except Exception: + logger.exception("TLS reload (update_config) failed; deferring") + event.defer() + + def _regenerate_internal_cert(self, *, reload: bool = True) -> None: + """Generate the internal peer cert, push it to the workload, and (optionally) reload. + + reload=False is used at cluster bootstrap: the leader renders patroni.yml on + leader-elected and each replica renders it in _on_peer_relation_changed just + before starting Patroni, so a reload here would be redundant. The internal peer + cert does not toggle ssl in the config -- only the operator/client cert does, via + is_tls_enabled -- so skipping the reload cannot leave a stale ssl setting. + """ + self.tls_manager.generate_internal_peer_cert() + self.tls_manager.push_tls_files() + if reload: + self.update_config() + def _check_and_update_internal_cert(self) -> None: """Check if the internal cert CN matches the unit IP and regenerate if needed.""" try: @@ -509,7 +572,7 @@ def _check_and_update_internal_cert(self) -> None: != self.state.unit_ip ) ): - self.tls.generate_internal_peer_cert() + self._regenerate_internal_cert() except Exception: logger.exception("Unable to check or update internal cert") @@ -1548,7 +1611,7 @@ def is_standby_cluster(self) -> bool: @property def is_tls_enabled(self) -> bool: """Return whether TLS is enabled.""" - return all(self.tls.get_client_tls_files()) + return all(self.tls_manager.get_client_tls_files()) @property def _peer_members_ips(self) -> set[str]: @@ -1759,7 +1822,7 @@ def _on_leader_elected(self, event: LeaderElectedEvent) -> None: # noqa: C901 logger.debug("On leader elected failed to reconfigure cluster.") if not self.get_secret(APP_SCOPE, "internal-ca"): - self.tls.generate_internal_peer_ca() + self.tls_manager.generate_internal_peer_ca() self.update_config() # Don't update connection endpoints in the first time this event run for @@ -1946,7 +2009,7 @@ def _on_start(self, event: StartEvent) -> None: event.defer() return if not self.get_secret(UNIT_SCOPE, "internal-cert"): - self.tls.generate_internal_peer_cert() + self._regenerate_internal_cert(reload=False) self.unit_peer_data.update({"ip": self.state.unit_ip}) self._ensure_storage_layout() @@ -2497,10 +2560,12 @@ def _update_certificate(self) -> None: # Request the certificate only if there is already one. If there isn't, # the certificate will be generated in the relation joined event when # relating to the TLS Certificates Operator. - if all(self.tls.get_client_tls_files()) or all(self.tls.get_peer_tls_files()): + if all(self.tls_manager.get_client_tls_files()) or all( + self.tls_manager.get_peer_tls_files() + ): self.tls.refresh_tls_certificates_event.emit() if self.get_secret(UNIT_SCOPE, "internal-cert"): - self.tls.generate_internal_peer_cert() + self._regenerate_internal_cert() @property def is_blocked(self) -> bool: @@ -2582,37 +2647,6 @@ def _peers(self) -> Relation | None: """ return self.model.get_relation(PEER_RELATION) - def push_tls_files_to_workload(self) -> bool: - """Move TLS files to the PostgreSQL storage path and enable TLS.""" - key, ca, cert = self.tls.get_client_tls_files() - if key is not None: - render_file(Substrates.VM, f"{PATRONI_CONF_PATH}/{TLS_KEY_FILE}", key, 0o600) - if ca is not None: - render_file(Substrates.VM, f"{PATRONI_CONF_PATH}/{TLS_CA_FILE}", ca, 0o600) - if cert is not None: - render_file(Substrates.VM, f"{PATRONI_CONF_PATH}/{TLS_CERT_FILE}", cert, 0o600) - - key, ca, cert = self.tls.get_peer_tls_files() - if key is not None: - render_file(Substrates.VM, f"{PATRONI_CONF_PATH}/peer_{TLS_KEY_FILE}", key, 0o600) - if ca is not None: - render_file(Substrates.VM, f"{PATRONI_CONF_PATH}/peer_{TLS_CA_FILE}", ca, 0o600) - if cert is not None: - render_file(Substrates.VM, f"{PATRONI_CONF_PATH}/peer_{TLS_CERT_FILE}", cert, 0o600) - - render_file( - Substrates.VM, - f"{PATRONI_CONF_PATH}/{TLS_CA_BUNDLE_FILE}", - self.tls.get_peer_ca_bundle(), - 0o600, - ) - - try: - return self.update_config() - except Exception: - logger.exception("TLS files failed to push. Error in config update") - return False - def push_ca_file_into_workload(self, secret_name: str) -> bool: """Move CA certificates file into the PostgreSQL storage path.""" certs = self.get_secret(UNIT_SCOPE, secret_name) diff --git a/src/relations/postgresql_provider.py b/src/relations/postgresql_provider.py index 7b5f0bd699..a0e47d79aa 100644 --- a/src/relations/postgresql_provider.py +++ b/src/relations/postgresql_provider.py @@ -485,7 +485,7 @@ def update_endpoints(self, event: DatabaseRequestedEvent | None = None) -> None: tls = "True" if self.charm.is_tls_enabled else "False" ca = None if tls == "True": - _, ca, _ = self.charm.tls.get_client_tls_files() + _, ca, _ = self.charm.tls_manager.get_client_tls_files() if not ca: ca = "" diff --git a/src/relations/tls.py b/src/relations/tls.py deleted file mode 100644 index 2c3bc92b5a..0000000000 --- a/src/relations/tls.py +++ /dev/null @@ -1,259 +0,0 @@ -# Copyright 2022 Canonical Ltd. -# See LICENSE file for licensing details. - -"""TLS Handler.""" - -import logging -import socket -from datetime import timedelta -from typing import TYPE_CHECKING - -from charmlibs.interfaces.tls_certificates import ( - Certificate, - CertificateRequestAttributes, - PrivateKey, - TLSCertificatesRequiresV4, - generate_ca, - generate_certificate, - generate_csr, - generate_private_key, -) -from ops import ( - EventSource, -) -from ops.framework import EventBase, Object -from ops.pebble import ConnectionError as PebbleConnectionError -from ops.pebble import PathError, ProtocolError -from single_kernel_postgresql.config.literals import ( - APP_SCOPE, - UNIT_SCOPE, -) -from tenacity import RetryError - -if TYPE_CHECKING: - from charm import PostgresqlOperatorCharm - -logger = logging.getLogger(__name__) -SCOPE = "unit" -TLS_CLIENT_RELATION = "client-certificates" -TLS_PEER_RELATION = "peer-certificates" - - -class RefreshTLSCertificatesEvent(EventBase): - """Event for refreshing TLS certificates.""" - - -class TlsError(Exception): - """TLS implementation internal exception.""" - - -class TLS(Object): - """In this class we manage certificates relation.""" - - refresh_tls_certificates_event = EventSource(RefreshTLSCertificatesEvent) - - def _get_client_addrs(self) -> set[str]: - client_addrs = set() - if addr := self.charm.unit_peer_data.get("database-address"): - client_addrs.add(addr) - return client_addrs - - def _get_peer_addrs(self) -> set[str]: - peer_addrs = set() - if addr := self.charm.unit_peer_data.get("database-peers-address"): - peer_addrs.add(addr) - if addr := self.charm.unit_peer_data.get("replication-address"): - peer_addrs.add(addr) - if addr := self.charm.unit_peer_data.get("replication-offer-address"): - peer_addrs.add(addr) - if addr := self.charm.unit_peer_data.get( - "ip", self.charm.unit_peer_data.get("private-address") - ): - peer_addrs.add(addr) - return peer_addrs - - def _get_common_name(self) -> str: - return self.charm.unit_peer_data.get("database-address") or self.host - - def _get_peer_common_name(self) -> str: - return self.charm.unit_peer_data.get("database-peers-address") or self.host - - def __init__(self, charm: "PostgresqlOperatorCharm", peer_relation: str): - super().__init__(charm, "client-relations") - self.charm = charm - self.peer_relation = peer_relation - unit_id = self.charm.unit.name.split("/")[1] - self.host = f"{self.charm.app.name}-{unit_id}" - if self.charm.unit_peer_data: - client_addresses = self._get_client_addrs() - peer_addresses = self._get_peer_addrs() - else: - client_addresses = set() - peer_addresses = set() - self.common_hosts = {self.host} - if fqdn := socket.getfqdn(): - self.common_hosts.add(fqdn) - - self.client_certificate = TLSCertificatesRequiresV4( - self.charm, - TLS_CLIENT_RELATION, - certificate_requests=[ - CertificateRequestAttributes( - common_name=self._get_common_name(), - sans_ip=frozenset(client_addresses), - sans_dns=frozenset({ - *self.common_hosts, - # IP address need to be part of the DNS SANs list due to - # https://github.com/pgbackrest/pgbackrest/issues/1977. - *client_addresses, - }), - ), - ], - refresh_events=[self.refresh_tls_certificates_event], - ) - self.peer_certificate = TLSCertificatesRequiresV4( - self.charm, - TLS_PEER_RELATION, - certificate_requests=[ - CertificateRequestAttributes( - common_name=self._get_peer_common_name(), - sans_ip=frozenset(self._get_peer_addrs()), - sans_dns=frozenset({ - *self.common_hosts, - # IP address need to be part of the DNS SANs list due to - # https://github.com/pgbackrest/pgbackrest/issues/1977. - *peer_addresses, - }), - ), - ], - refresh_events=[self.refresh_tls_certificates_event], - ) - - self.framework.observe( - self.client_certificate.on.certificate_available, self._on_certificate_available - ) - self.framework.observe( - self.peer_certificate.on.certificate_available, self._on_peer_certificate_available - ) - - self.framework.observe( - self.charm.on[TLS_CLIENT_RELATION].relation_broken, self._on_certificate_available - ) - self.framework.observe( - self.charm.on[TLS_PEER_RELATION].relation_broken, self._on_peer_certificate_available - ) - - def _on_peer_certificate_available(self, event: EventBase) -> None: - certs, _ = self.peer_certificate.get_assigned_certificates() - new_ca = str(certs[0].ca) if certs else None - current_ca = self.charm.get_secret(UNIT_SCOPE, "current-ca") - # Stash the CAs in case of rotation - if new_ca != current_ca: - self.charm.set_secret(UNIT_SCOPE, "current-ca", new_ca) - self.charm.set_secret(UNIT_SCOPE, "old-ca", current_ca) - self._on_certificate_available(event) - - def _on_certificate_available(self, event: EventBase) -> None: - if not self.charm.get_secret(APP_SCOPE, "internal-ca"): - logger.debug("Charm not ready yet") - event.defer() - return - try: - if not self.charm.push_tls_files_to_workload(): - logger.debug("Cannot push TLS certificates at this moment") - event.defer() - return - except (PebbleConnectionError, PathError, ProtocolError, RetryError) as e: - logger.error("Cannot push TLS certificates: %r", e) - event.defer() - return - - def get_client_tls_files(self) -> tuple[str | None, str | None, str | None]: - """Prepare TLS files in special PostgreSQL way. - - PostgreSQL needs three files: - — CA file should have a full chain. - — Key file should have private key. - — Certificate file should have certificate without certificate chain. - """ - ca_file = None - cert = None - key = None - certs, private_key = self.client_certificate.get_assigned_certificates() - if private_key: - key = str(private_key) - if certs: - cert = str(certs[0].certificate) - ca_file = str(certs[0].ca) - return key, ca_file, cert - - def get_peer_tls_files(self) -> tuple[str | None, str | None, str | None]: - """Prepare TLS files in special PostgreSQL way. - - PostgreSQL needs three files: - — CA file should have a full chain. - — Key file should have private key. - — Certificate file should have certificate without certificate chain. - """ - ca_file = None - cert = None - key = None - certs, private_key = self.peer_certificate.get_assigned_certificates() - if private_key: - key = str(private_key) - if certs: - cert = str(certs[0].certificate) - ca_file = str(certs[0].ca) - if not all((key, ca_file, cert)): - key = self.charm.get_secret(UNIT_SCOPE, "internal-key") - cert = self.charm.get_secret(UNIT_SCOPE, "internal-cert") - ca_file = self.charm.get_secret(APP_SCOPE, "internal-ca") - return key, ca_file, cert - - def get_peer_ca_bundle(self) -> str: - """Get bundled CA certs.""" - certs, _ = self.peer_certificate.get_assigned_certificates() - operator_ca = str(certs[0].ca) if certs else "" - old_operator_ca = self.charm.get_secret(UNIT_SCOPE, "old-ca") or "" - internal_ca = self.charm.get_secret(APP_SCOPE, "internal-ca") or "" - return "\n".join((operator_ca, old_operator_ca, internal_ca)).strip() - - def generate_internal_peer_ca(self) -> None: - """Generate internal peer CA using the tls lib.""" - private_key = generate_private_key() - ca = generate_ca( - private_key, - common_name=f"{self.charm.app.name}-{self.charm.model.uuid}", - validity=timedelta(days=7300), - ) - logger.warning("Internal peer CA generated. Please use a proper TLS operator if possible.") - self.charm.set_secret(APP_SCOPE, "internal-ca-key", str(private_key)) - self.charm.set_secret(APP_SCOPE, "internal-ca", str(ca)) - - def generate_internal_peer_cert(self) -> None: - """Generate internal peer certificate using the tls lib.""" - if not (ca_key_secret := self.charm.get_secret(APP_SCOPE, "internal-ca-key")): - raise TlsError("No CA key content.") - ca_key = PrivateKey.from_string(ca_key_secret) - if not (ca_secret := self.charm.get_secret(APP_SCOPE, "internal-ca")): - raise TlsError("No CA cert content.") - ca = Certificate.from_string(ca_secret) - private_key = generate_private_key() - csr = generate_csr( - private_key, - common_name=self._get_peer_common_name(), - sans_ip=frozenset(self._get_peer_addrs()), - sans_dns=frozenset({ - *self.common_hosts, - # IP address need to be part of the DNS SANs list due to - # https://github.com/pgbackrest/pgbackrest/issues/1977. - *self._get_peer_addrs(), - }), - ) - cert = generate_certificate(csr, ca, ca_key, validity=timedelta(days=7300)) - self.charm.set_secret(UNIT_SCOPE, "internal-key", str(private_key)) - self.charm.set_secret(UNIT_SCOPE, "internal-cert", str(cert)) - self.charm.push_tls_files_to_workload() - logger.info( - "Internal peer certificate generated. Please use a proper TLS operator if possible." - ) diff --git a/src/relations/watcher.py b/src/relations/watcher.py index 9cc92a3bef..4f46e5f760 100644 --- a/src/relations/watcher.py +++ b/src/relations/watcher.py @@ -419,7 +419,7 @@ def _update_relation_data(self, relation: Relation) -> None: "raft-secret-id": secret_id, "raft-partner-addrs": json.dumps(pg_endpoints), "raft-port": str(RAFT_PORT), - "patroni-cas": self.charm.tls.get_peer_ca_bundle(), + "patroni-cas": self.charm.tls_manager.get_peer_ca_bundle(), "standby-clusters": json.dumps(self._get_standby_clusters()), "tls-enabled": "true" if self.charm.is_tls_enabled else "false", }) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 7a9db219e5..9e6c31b7a8 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -188,7 +188,7 @@ def test_on_leader_elected(harness): ) as _primary_endpoint, patch("charm.PostgresqlOperatorCharm.update_config") as _update_config, patch("charm.PostgresqlOperatorCharm._reconfigure_cluster"), - patch("charm.TLS.generate_internal_peer_cert"), + patch("charm.TLSManager.generate_internal_peer_cert"), ): # Assert that there is no password in the peer relation. assert harness.charm._peers.data[harness.charm.app].get("operator-password", None) is None @@ -444,7 +444,7 @@ def test_on_start_bootstrap_failure(harness): patch("charm.PostgresqlOperatorCharm._ensure_storage_layout"), patch("charm.PostgresqlOperatorCharm._check_detached_storage"), patch("charm.PostgresqlOperatorCharm.get_secret"), - patch("charm.TLS.generate_internal_peer_cert"), + patch("charm.TLSManager.generate_internal_peer_cert"), patch("charm.PatroniManager.bootstrap_cluster") as _bootstrap_cluster, patch("charm.PostgresqlOperatorCharm.update_config"), patch("charm.start_raft_observer"), @@ -475,7 +475,7 @@ def test_on_start_create_user_error(harness): patch("charm.PostgresqlOperatorCharm._ensure_storage_layout"), patch("charm.PostgresqlOperatorCharm._check_detached_storage"), patch("charm.PostgresqlOperatorCharm.get_secret"), - patch("charm.TLS.generate_internal_peer_cert"), + patch("charm.TLSManager.generate_internal_peer_cert"), patch("charm.PatroniManager.bootstrap_cluster") as _bootstrap_cluster, patch( "charm.PatroniManager.member_started", @@ -532,7 +532,7 @@ def test_on_start_success(harness): patch("charm.PostgresqlOperatorCharm._ensure_storage_layout"), patch("charm.PostgresqlOperatorCharm._check_detached_storage"), patch("charm.PostgresqlOperatorCharm.get_secret"), - patch("charm.TLS.generate_internal_peer_cert"), + patch("charm.TLSManager.generate_internal_peer_cert"), patch("charm.PatroniManager.bootstrap_cluster") as _bootstrap_cluster, patch( "charm.PatroniManager.member_started", @@ -661,7 +661,7 @@ def test_on_start_replica(harness): return_value=True, ) as _is_storage_attached, patch("charm.PostgresqlOperatorCharm.get_secret"), - patch("charm.TLS.generate_internal_peer_cert"), + patch("charm.TLSManager.generate_internal_peer_cert"), patch("charm.start_raft_observer"), ): _get_postgresql_version.return_value = "16.6" @@ -720,7 +720,7 @@ def test_on_start_no_patroni_member(harness): ) as _is_storage_attached, patch("charm.PostgresqlOperatorCharm.get_available_memory") as _get_available_memory, patch("charm.PostgresqlOperatorCharm.get_secret"), - patch("charm.TLS.generate_internal_peer_cert"), + patch("charm.TLSManager.generate_internal_peer_cert"), patch("charm.PostgreSQLProvider.get_username_mapping", return_value={}), patch("charm.PostgreSQLProvider.get_databases_prefix_mapping", return_value={}), patch("charm.start_raft_observer"), @@ -2344,7 +2344,7 @@ def test_reconfigure_cluster(harness): def test_update_certificate(harness): with ( - patch("charm.TLS.get_client_tls_files") as _get_client_tls_files, + patch("charm.TLSManager.get_client_tls_files") as _get_client_tls_files, patch("charm.TLS.refresh_tls_certificates_event") as _refresh_tls_certificates_event, ): # If there is no current TLS files, _request_certificate should be called @@ -2402,41 +2402,6 @@ def test_update_member_ip(harness): _update_certificate.assert_called_once() -def test_push_tls_files_to_workload(harness): - with ( - patch("charm.PostgresqlOperatorCharm.update_config") as _update_config, - patch("charm.render_file") as _render_file, - patch("charm.TLS.get_client_tls_files") as _get_client_tls_files, - patch("charm.TLS.get_peer_tls_files") as _get_peer_tls_files, - patch( - "charm.PostgresqlOperatorCharm.get_secret", return_value="internal_ca" - ) as _get_secret, - ): - _get_client_tls_files.side_effect = [ - ("key", "ca", "cert"), - ("key", "ca", None), - ("key", None, "cert"), - (None, "ca", "cert"), - ] - _get_peer_tls_files.side_effect = [ - ("key", "ca", "cert"), - ("key", "ca", None), - ("key", None, "cert"), - (None, "ca", "cert"), - ] - _update_config.side_effect = [True, False, False, False] - - # Test when all TLS files are available. - assert harness.charm.push_tls_files_to_workload() - assert _render_file.call_count == 7 - - # Test when not all TLS files are available. - for _ in range(3): - _render_file.reset_mock() - assert not (harness.charm.push_tls_files_to_workload()) - assert _render_file.call_count == 5 - - def test_push_ca_file_into_workload(harness): with ( patch("charm.PostgresqlOperatorCharm.update_config") as _update_config, diff --git a/tests/unit/test_tls.py b/tests/unit/test_tls.py index c8ce927080..8a67b73028 100644 --- a/tests/unit/test_tls.py +++ b/tests/unit/test_tls.py @@ -1,172 +1,191 @@ # Copyright 2022 Canonical Ltd. # See LICENSE file for licensing details. -from datetime import timedelta -from unittest.mock import Mock, patch, sentinel +"""TLS wiring tests for the single-kernel (lib) TLS handler. + +The charm consumes the library TLS handler (single_kernel_postgresql.events.tls.TLS) +and TLS manager (single_kernel_postgresql.managers.tls.TLSManager) instead of its +own removed src/relations/tls.py. These tests exercise the lib-backed wiring: +state-backed cert storage via TLSManager, and the charm's reload bridge that calls +update_config after the handler stores+pushes certificates. +""" + +from unittest.mock import Mock, PropertyMock, patch import pytest from ops.testing import Harness -from single_kernel_postgresql.config.literals import PEER_RELATION +from single_kernel_postgresql.config.literals import ( + PEER_RELATION, + TLS_CLIENT_RELATION, + TLS_PEER_RELATION, +) +from single_kernel_postgresql.events.tls import TLS +from single_kernel_postgresql.managers.tls import TLSManager from charm import PostgresqlOperatorCharm @pytest.fixture(autouse=True) -def harness(): - with patch("relations.tls.socket") as _socket: - _socket.getfqdn.return_value = "fqdn" - harness = Harness(PostgresqlOperatorCharm) - - # Set up the initial relation and hooks. - peer_rel_id = harness.add_relation(PEER_RELATION, "postgresql") - harness.add_relation_unit(peer_rel_id, "postgresql/0") - harness.begin() - yield harness - harness.cleanup() +def _platform_machine(monkeypatch): + # refresh_versions.toml keys snap revisions by x86_64/aarch64; ensure the charm + # constructs regardless of the host arch (e.g. arm64 dev laptops). + monkeypatch.setattr("platform.machine", lambda: "x86_64") -def test_generate_internal_peer_cert(harness): - with ( - patch( - "relations.tls.generate_private_key", return_value=sentinel.cert_key - ) as _generate_private_key, - patch("relations.tls.generate_csr", return_value=sentinel.cert_csr) as _generate_csr, - patch( - "relations.tls.generate_certificate", return_value=sentinel.cert - ) as _generate_certificate, - patch("relations.tls.PrivateKey") as _private_key, - patch("relations.tls.Certificate") as _certificate, - patch("charm.PostgresqlOperatorCharm.set_secret") as _set_secret, - patch("charm.PostgresqlOperatorCharm.get_secret", return_value="secret value"), - patch( - "charm.PostgresqlOperatorCharm.push_tls_files_to_workload" - ) as _push_tls_files_to_workload, - ): - _private_key.from_string.return_value = sentinel.ca_key - _certificate.from_string.return_value = sentinel.ca_cert - - harness.charm.tls.generate_internal_peer_cert() - - _generate_csr.assert_called_once_with( - sentinel.cert_key, - common_name="postgresql-0", - sans_ip=frozenset(), - sans_dns=frozenset({"postgresql-0", "fqdn"}), - ) - _generate_certificate.assert_called_once_with( - sentinel.cert_csr, sentinel.ca_cert, sentinel.ca_key, validity=timedelta(days=7300) - ) - assert _set_secret.call_count == 2 - _set_secret.assert_any_call("unit", "internal-key", str(sentinel.cert_key)) - _set_secret.assert_any_call("unit", "internal-cert", str(sentinel.cert)) - _push_tls_files_to_workload.assert_called_once_with() - - -def test_get_client_tls_files(harness): - with patch( - "relations.tls.TLSCertificatesRequiresV4.get_assigned_certificates" - ) as _get_assigned_certificates: - cert_mock = Mock() - cert_mock.certificate = sentinel.certificate - cert_mock.ca = sentinel.ca - _get_assigned_certificates.return_value = ([cert_mock], sentinel.private_key) - - assert harness.charm.tls.get_client_tls_files() == ( - "sentinel.private_key", - "sentinel.ca", - "sentinel.certificate", - ) - - _get_assigned_certificates.return_value = (None, None) - assert harness.charm.tls.get_client_tls_files() == (None, None, None) - - -def test_get_peer_tls_files(harness): - with ( - patch( - "relations.tls.TLSCertificatesRequiresV4.get_assigned_certificates" - ) as _get_assigned_certificates, - patch( - "charm.PostgresqlOperatorCharm.get_secret", return_value=sentinel.secret - ) as _get_secret, - ): - cert_mock = Mock() - cert_mock.certificate = sentinel.certificate - cert_mock.ca = sentinel.ca - _get_assigned_certificates.return_value = ([cert_mock], sentinel.private_key) - - assert harness.charm.tls.get_peer_tls_files() == ( - "sentinel.private_key", - "sentinel.ca", - "sentinel.certificate", - ) - assert not _get_secret.called - - _get_assigned_certificates.return_value = (None, None) - assert harness.charm.tls.get_peer_tls_files() == ( - sentinel.secret, - sentinel.secret, - sentinel.secret, - ) - assert _get_secret.call_count == 3 - _get_secret.assert_any_call("unit", "internal-key") - _get_secret.assert_any_call("unit", "internal-cert") - _get_secret.assert_any_call("app", "internal-ca") - - -def test_on_client_certificate_available(harness): +@pytest.fixture(autouse=True) +def harness(_platform_machine): + harness = Harness(PostgresqlOperatorCharm) + peer_rel_id = harness.add_relation(PEER_RELATION, "postgresql") + harness.add_relation_unit(peer_rel_id, "postgresql/0") + harness.begin() + yield harness + harness.cleanup() + + +def test_tls_handler_is_lib_backed(harness): + """The charm wires the lib TLS handler + manager (not the removed relations.tls).""" + charm = harness.charm + assert isinstance(charm.tls, TLS) + assert isinstance(charm.tls_manager, TLSManager) + # The handler owns the operator client/peer requirers and the refresh event. + assert hasattr(charm.tls, "client_certificate") + assert hasattr(charm.tls, "peer_certificate") + assert hasattr(charm.tls, "refresh_tls_certificates_event") + # The removed method must not resurface anywhere. + assert not hasattr(charm, "push_tls_files_to_workload") + + +def test_is_tls_enabled_reflects_tls_manager(harness): + """is_tls_enabled is driven by TLSManager.get_client_tls_files(), not the handler.""" + with patch("charm.TLSManager.get_client_tls_files") as _get_client_tls_files: + _get_client_tls_files.return_value = (None, None, None) + assert harness.charm.is_tls_enabled is False + + _get_client_tls_files.return_value = ("key", "ca", "cert") + assert harness.charm.is_tls_enabled is True + + +def _observers_for(harness, bound_event): + """Return method names observing the given bound event, in registration order.""" + emitter_path = bound_event.emitter.handle.path + event_kind = bound_event.event_kind + return [ + method + for (_obs_path, method, e_path, e_kind) in harness.framework._observers + if e_path == emitter_path and e_kind == event_kind + ] + + +def test_reload_bridge_wired_after_handler_on_client_certificate(harness): + """The reload bridge observes the same certificate_available event as the handler. + + The lib handler's store+push observer must be registered BEFORE the charm's + reload bridge so that, when the event fires, certs are stored+pushed first and + the reload (update_config) runs afterwards (ops calls observers in order). + """ + methods = _observers_for( + harness, harness.charm.tls.client_certificate.on.certificate_available + ) + assert "_on_certificate_available" in methods, methods + assert "_reload_tls_after_push" in methods, methods + # Handler (store+push) before bridge (reload). + assert methods.index("_on_certificate_available") < methods.index("_reload_tls_after_push") + + +def test_reload_bridge_wired_after_handler_on_peer_certificate(harness): + """The reload bridge also observes the peer certificate_available event.""" + methods = _observers_for(harness, harness.charm.tls.peer_certificate.on.certificate_available) + assert "_on_peer_certificate_available" in methods, methods + assert "_reload_tls_after_push" in methods, methods + assert methods.index("_on_peer_certificate_available") < methods.index( + "_reload_tls_after_push" + ) + + +def test_reload_bridge_calls_update_config(harness): + """_reload_tls_after_push delegates to update_config when internal-ca is present.""" + with harness.hooks_disabled(): + harness.set_leader(True) + harness.charm.set_secret("app", "internal-ca", "ca-content") + + with patch("charm.PostgresqlOperatorCharm.update_config") as _update_config: + harness.charm._reload_tls_after_push(Mock()) + _update_config.assert_called_once_with() + + +def test_reload_bridge_skips_update_config_without_internal_ca(harness): + """_reload_tls_after_push is a no-op when internal-ca is absent (defer path). + + The lib TLS handler defers its push when the internal CA isn't present yet + (no files written to disk). The bridge must not call update_config in that + case, or it would render ssl:on against TLS files that don't exist yet. + """ + with patch("charm.PostgresqlOperatorCharm.update_config") as _update_config: + harness.charm._reload_tls_after_push(Mock()) + _update_config.assert_not_called() + + +def test_reload_bridge_wired_on_tls_relation_broken(harness): + """Detaching the TLS operator (relation_broken) must trigger the reload bridge. + + Regression test: the VM charm previously observed only certificate_available, so + removing a TLS relation left Patroni ssl:on against cleared certs. Both TLS + relations' relation_broken must reach the reload bridge (parity with K8s). + """ + for relation in (TLS_CLIENT_RELATION, TLS_PEER_RELATION): + methods = _observers_for(harness, harness.charm.on[relation].relation_broken) + assert "_reload_tls_after_push" in methods, (relation, methods) + + +def test_reload_bridge_defers_when_update_config_not_ready(harness): + """A transient config-apply failure must defer (retry), not leave stale TLS state.""" + with harness.hooks_disabled(): + harness.set_leader(True) + harness.charm.set_secret("app", "internal-ca", "ca-content") + + event = Mock() + with patch("charm.PostgresqlOperatorCharm.update_config", return_value=False): + harness.charm._reload_tls_after_push(event) + event.defer.assert_called_once() + + +def test_reload_bridge_defers_when_tls_files_not_yet_on_disk(harness): + """ssl:on must not be rendered until the lib has pushed the cert files to disk.""" + with harness.hooks_disabled(): + harness.set_leader(True) + harness.charm.set_secret("app", "internal-ca", "ca-content") + event = Mock() with ( - patch("charm.PostgresqlOperatorCharm.get_secret") as _get_secret, - patch( - "charm.PostgresqlOperatorCharm.push_tls_files_to_workload" - ) as _push_tls_files_to_workload, + patch("charm.PostgresqlOperatorCharm.update_config") as _uc, + # is_tls_enabled -> True via the live-fetch getter + patch("charm.TLSManager.get_client_tls_files", return_value=("K", "CA", "C")), + patch.object(harness.charm.tls_manager, "client_tls_files_on_disk", return_value=False), ): - # Defers if internal CA is not ready yet - _get_secret.return_value = None - event_mock = Mock() - - harness.charm.tls._on_certificate_available(event_mock) - - event_mock.defer.assert_called_once_with() - assert not _push_tls_files_to_workload.called - event_mock.reset_mock() - - # Defers if can't push - _get_secret.return_value = sentinel.secret - _push_tls_files_to_workload.return_value = False + harness.charm._reload_tls_after_push(event) + event.defer.assert_called_once() + _uc.assert_not_called() - harness.charm.tls._on_certificate_available(event_mock) - event_mock.defer.assert_called_once_with() - _push_tls_files_to_workload.assert_called_once_with() - event_mock.reset_mock() - - -def test_on_peer_certificate_available(harness): +def test_internal_cert_path_pushes_and_reloads(harness): + """_check_and_update_internal_cert generates, pushes, and reloads on CN mismatch.""" with ( + patch("charm.CharmState.unit_ip", new_callable=PropertyMock) as _unit_ip, patch( - "relations.tls.TLSCertificatesRequiresV4.get_assigned_certificates" - ) as _get_assigned_certificates, - patch("relations.tls.TLS._on_certificate_available") as _on_certificate_available, - patch("charm.PostgresqlOperatorCharm.get_secret") as _get_secret, - patch("charm.PostgresqlOperatorCharm.set_secret") as _set_secret, + "charm.PostgresqlOperatorCharm.get_secret", + return_value="-----BEGIN CERTIFICATE-----", + ), + patch("charm.load_pem_x509_certificate") as _load_cert, + patch("charm.TLSManager.generate_internal_peer_cert") as _generate, + patch("charm.TLSManager.push_tls_files") as _push, + patch("charm.PostgresqlOperatorCharm.update_config") as _update_config, ): - # Same ca - cert_mock = Mock() - cert_mock.ca = sentinel.ca - _get_assigned_certificates.return_value = ([cert_mock], None) - _get_secret.return_value = "sentinel.ca" - event_mock = Mock() - - harness.charm.tls._on_peer_certificate_available(event_mock) - - _on_certificate_available.assert_called_once_with(event_mock) - assert not _set_secret.called - - # Different ca - _get_secret.return_value = sentinel.old_ca + _unit_ip.return_value = "1.2.3.4" + # Make the cert CN differ from the unit IP to force regeneration. + attr = Mock() + attr.value = "9.9.9.9" + _load_cert.return_value.subject.get_attributes_for_oid.return_value = [attr] - harness.charm.tls._on_peer_certificate_available(event_mock) + harness.charm._check_and_update_internal_cert() - assert _set_secret.call_count == 2 - _set_secret.assert_any_call("unit", "current-ca", "sentinel.ca") - _set_secret.assert_any_call("unit", "old-ca", sentinel.old_ca) + _generate.assert_called_once_with() + _push.assert_called_once_with() + _update_config.assert_called_once_with() From ab21c96a23002878b4cbd7b88ad4c4ade73b9a99 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Fri, 3 Jul 2026 17:37:42 -0300 Subject: [PATCH 2/3] build(deps): bump single-kernel lib to the 16.3.3 stack tip The TLS lib stack was rebased onto current 16/edge and re-bumped to 16.3.3 because 16/edge had meanwhile shipped its own 16.3.2, and the rebase dropped the dead workload parameter from the TLS events handler constructor, so the pin and the construction call move together. Signed-off-by: Marcelo Henrique Neppel --- poetry.lock | 11 ++++++----- pyproject.toml | 2 +- src/charm.py | 2 +- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/poetry.lock b/poetry.lock index f8f0cf2f33..a0bb056c8c 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1865,13 +1865,13 @@ testing = ["coverage", "pytest", "pytest-benchmark"] [[package]] name = "postgresql-charms-single-kernel" -version = "16.3.2" +version = "16.3.3" description = "Shared and reusable code for PostgreSQL-related charms" optional = false python-versions = ">=3.8,<4.0" groups = ["main", "integration"] files = [ - {file = "5aeebec.zip", hash = "sha256:76c2e468de1169912416fffaa51b474f79223b5caee16207c88ec71902d7ee6f"}, + {file = "5a9fa05.zip", hash = "sha256:0081d9b0a413bb8122d788054c175aa459611b817232542d4c01d6aaf7e8c5e3"}, ] [package.dependencies] @@ -1880,6 +1880,7 @@ charmlibs-interfaces-tls-certificates = {version = ">=1.8.3", optional = true, m charmlibs-pathops = {version = ">=1.0.1", optional = true, markers = "python_version >= \"3.12\" and extra == \"postgresql\""} charmlibs-rollingops = {version = ">=1.1.1", optional = true, markers = "python_version >= \"3.12\" and extra == \"postgresql\""} charmlibs-snap = {version = ">=1.0.1", optional = true, markers = "python_version >= \"3.12\" and extra == \"vm\""} +charmlibs-systemd = {version = ">=1.0.0.post0", optional = true, markers = "python_version >= \"3.12\" and extra == \"vm\""} data-platform-helpers = {version = ">=0.1.7", optional = true, markers = "python_version >= \"3.12\" and extra == \"postgresql\""} httpx = {version = "*", optional = true, markers = "python_version >= \"3.12\" and extra == \"postgresql\""} jinja2 = {version = ">=3.1.6", optional = true, markers = "python_version >= \"3.12\" and extra == \"postgresql\""} @@ -1894,11 +1895,11 @@ tomli = {version = "*", optional = true, markers = "python_version >= \"3.12\" a [package.extras] db-driver = ["psycopg2 (>=2.9.10)"] postgresql = ["charm-refresh ; python_version >= \"3.12\"", "charmlibs-interfaces-tls-certificates (>=1.8.3) ; python_version >= \"3.12\"", "charmlibs-pathops (>=1.0.1) ; python_version >= \"3.12\"", "charmlibs-rollingops (>=1.1.1) ; python_version >= \"3.12\"", "data-platform-helpers (>=0.1.7) ; python_version >= \"3.12\"", "httpx ; python_version >= \"3.12\"", "jinja2 (>=3.1.6) ; python_version >= \"3.12\"", "pydantic (>=2.0) ; python_version >= \"3.12\"", "requests ; python_version >= \"3.12\"", "tomli ; python_version >= \"3.12\""] -vm = ["charmlibs-snap (>=1.0.1) ; python_version >= \"3.12\"", "psutil (>=7.2.2) ; python_version >= \"3.12\"", "pysyncobj (>=0.3.15) ; python_version >= \"3.12\""] +vm = ["charmlibs-snap (>=1.0.1) ; python_version >= \"3.12\"", "charmlibs-systemd (>=1.0.0.post0) ; python_version >= \"3.12\"", "psutil (>=7.2.2) ; python_version >= \"3.12\"", "pysyncobj (>=0.3.15) ; python_version >= \"3.12\""] [package.source] type = "url" -url = "https://github.com/canonical/postgresql-single-kernel-library/archive/5aeebec.zip" +url = "https://github.com/canonical/postgresql-single-kernel-library/archive/5a9fa05.zip" [[package]] name = "prompt-toolkit" @@ -3196,4 +3197,4 @@ h11 = ">=0.16.0,<1" [metadata] lock-version = "2.1" python-versions = ">=3.12,<4.0" -content-hash = "e49d6ddac7ca3a2d13571479e983cc73c99b15b4244ae803052c7e6d4c44d00e" +content-hash = "b84f2901f3732eba7616a6fe7c249d00b6de70efb92630990ca53e4e22b17a3d" diff --git a/pyproject.toml b/pyproject.toml index 2fb0dacd1c..521c86338d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -21,7 +21,7 @@ charm-refresh = "^3.1.0.2" charmlibs-snap = "^1.0.1" charmlibs-systemd = "^1.0.0.post0" charmlibs-interfaces-tls-certificates = "^1.8.3" -postgresql-charms-single-kernel = {url = "https://github.com/canonical/postgresql-single-kernel-library/archive/5aeebec.zip", extras = ["postgresql", "vm"]} +postgresql-charms-single-kernel = {url = "https://github.com/canonical/postgresql-single-kernel-library/archive/5a9fa05.zip", extras = ["postgresql", "vm"]} [tool.poetry.group.charm-libs.dependencies] # data_platform_libs/v0/data_interfaces.py diff --git a/src/charm.py b/src/charm.py index 614b0329cd..7151466c71 100755 --- a/src/charm.py +++ b/src/charm.py @@ -419,7 +419,7 @@ def __init__(self, *args): self.ldap = PostgreSQLLDAP(self, "ldap") # TLS events handler owns the two cert requirers; build it before the TLS # manager so the manager can constructor-inject them for its live-fetch getters. - self.tls = TLS(self, self.state, self.workload) + self.tls = TLS(self, self.state) self.tls_manager = TLSManager( state=self.state, workload=self.workload, From 2c02f639d8b1547d2f82e1fe04337ea00155eb3e Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Fri, 3 Jul 2026 17:39:22 -0300 Subject: [PATCH 3/3] test(tls): assert relation_broken bridge ordering and exception defer The relation_broken bridge registrations reference only self.on[...], so an __init__ reorder above the lib handler's construction would silently run the reload before the lib clears state and pushes files; the prior test asserted membership without order. The bridge's except-and-defer branch also had no coverage, so a refactor could drop the broad guard without failing CI. Signed-off-by: Marcelo Henrique Neppel --- tests/unit/test_tls.py | 37 ++++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_tls.py b/tests/unit/test_tls.py index 8a67b73028..0b38b69b07 100644 --- a/tests/unit/test_tls.py +++ b/tests/unit/test_tls.py @@ -130,10 +130,24 @@ def test_reload_bridge_wired_on_tls_relation_broken(harness): Regression test: the VM charm previously observed only certificate_available, so removing a TLS relation left Patroni ssl:on against cleared certs. Both TLS relations' relation_broken must reach the reload bridge (parity with K8s). + + The ordering is also asserted: unlike the certificate_available wiring, these + registrations reference only ``self.on[...]``, so moving them above the lib + handler's construction in __init__ would silently reverse the order and run + the reload before the lib clears state and pushes files. """ - for relation in (TLS_CLIENT_RELATION, TLS_PEER_RELATION): + for relation, lib_handler in ( + (TLS_CLIENT_RELATION, "_on_certificate_available"), + (TLS_PEER_RELATION, "_on_peer_certificate_available"), + ): methods = _observers_for(harness, harness.charm.on[relation].relation_broken) + assert lib_handler in methods, (relation, methods) assert "_reload_tls_after_push" in methods, (relation, methods) + # Lib handler (clear+push) before charm bridge (reload). + assert methods.index(lib_handler) < methods.index("_reload_tls_after_push"), ( + relation, + methods, + ) def test_reload_bridge_defers_when_update_config_not_ready(harness): @@ -165,6 +179,27 @@ def test_reload_bridge_defers_when_tls_files_not_yet_on_disk(harness): _uc.assert_not_called() +def test_reload_bridge_defers_when_update_config_raises(harness): + """A raising update_config must be caught and deferred, not fail the hook. + + The bridge mirrors the original charm's broad push-failure guard: a transient + Patroni/render failure defers and retries rather than propagating out of the + observer. + """ + with harness.hooks_disabled(): + harness.set_leader(True) + harness.charm.set_secret("app", "internal-ca", "ca-content") + + event = Mock() + with patch( + "charm.PostgresqlOperatorCharm.update_config", + side_effect=RuntimeError("patroni render failed"), + ): + # Must not raise. + harness.charm._reload_tls_after_push(event) + event.defer.assert_called_once_with() + + def test_internal_cert_path_pushes_and_reloads(harness): """_check_and_update_internal_cert generates, pushes, and reloads on CN mismatch.""" with (