From 833942db2f97836753261d6222a0d26e7d53223d Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Wed, 1 Jul 2026 15:27:12 -0300 Subject: [PATCH 01/10] feat(tls): add TLS state and workload file primitives Introduce the low-level building blocks that the operator-certificate TLS flow composes on top of, so they can be reviewed on their own, ahead of the manager, events handler and charm wiring that consume them. This covers the peer-relation databag accessors for CA rotation (current-ca/old-ca), the client-facing database-address, and the K8s-shaped peer address set that omits the ip key for parity with the pre-migration K8s charm; the workload file-ownership/mode primitives (user/group and the substrate-specific tls_file_mode, 0o600 on VM and 0o400 on K8s) plus user/group forwarding through write_text so TLS material is chowned correctly on both substrates; a per-substrate tls path; the TLS relation-name constants; and the unit-test harness fixture the later branches' TLS tests depend on. The change is purely additive: the existing charm still constructs unchanged and the current unit suite is unaffected, which keeps this branch a safe, self-contained foundation for the stack. Signed-off-by: Marcelo Henrique Neppel --- single_kernel_postgresql/config/literals.py | 4 ++ .../core/peer_relation.py | 46 +++++++++++++++++++ single_kernel_postgresql/workload/base.py | 34 +++++++++++++- single_kernel_postgresql/workload/k8s.py | 15 ++++++ .../workload/paths/base.py | 6 +++ .../workload/paths/k8s.py | 13 +++++- single_kernel_postgresql/workload/paths/vm.py | 5 ++ single_kernel_postgresql/workload/vm.py | 10 ++++ tests/unit/conftest.py | 37 +++++++++++++++ 9 files changed, 167 insertions(+), 3 deletions(-) create mode 100644 tests/unit/conftest.py diff --git a/single_kernel_postgresql/config/literals.py b/single_kernel_postgresql/config/literals.py index 7bd6e7f..50744c1 100644 --- a/single_kernel_postgresql/config/literals.py +++ b/single_kernel_postgresql/config/literals.py @@ -52,6 +52,10 @@ ## TLS Paths TLS_CA_BUNDLE_FILE = "peer_ca_bundle.pem" +# TLS relation names +TLS_CLIENT_RELATION = "client-certificates" +TLS_PEER_RELATION = "peer-certificates" + # Scopes SCOPES = Literal["app", "unit"] APP_SCOPE = "app" diff --git a/single_kernel_postgresql/core/peer_relation.py b/single_kernel_postgresql/core/peer_relation.py index c03be29..cdb9921 100644 --- a/single_kernel_postgresql/core/peer_relation.py +++ b/single_kernel_postgresql/core/peer_relation.py @@ -100,6 +100,24 @@ def internal_key(self, value: str) -> None: """Set internal private key in the peer relation.""" self.set_secret("internal-key", value) + @property + def current_ca(self) -> str | None: + """Current peer CA (unit secret); part of the peer CA bundle.""" + return self.get_secret("current-ca") + + @current_ca.setter + def current_ca(self, value: str) -> None: + self.set_secret("current-ca", value) + + @property + def old_ca(self) -> str | None: + """Previous peer CA (unit secret); retained for the rotation window.""" + return self.get_secret("old-ca") + + @old_ca.setter + def old_ca(self, value: str) -> None: + self.set_secret("old-ca", value) + @property def ip(self) -> str | None: """Get the unit's IP address from the peer relation data.""" @@ -144,6 +162,13 @@ def database_peers_address(self) -> str | None: return None return self.relation.data[self.unit].get("database-peers-address", None) + @property + def database_address(self) -> str | None: + """Get the client-facing database endpoint address.""" + if not self.relation: + return None + return self.relation.data[self.unit].get("database-address", None) + @property def replication_address(self) -> str | None: """Get the address to be used for replication communication.""" @@ -207,6 +232,27 @@ def data(self) -> MutableMapping[str, str]: return {} return self.relation.data[self.unit] + @property + def peer_addresses_no_ip(self) -> set[str]: + """Peer addresses excluding the ``ip`` databag key (original K8s charm behavior). + + The K8s charm never wrote ``ip`` into the operator peer-cert SANs; it relied on + ``database-peers-address`` + ``replication-address`` + ``replication-offer-address`` + + ``private-address``. The VM charm additionally included ``ip``. This property + exposes the K8s-shaped set so :class:`CharmState` can pick the right one per + substrate without the peer object needing to know the substrate. + """ + peer_addrs: set[str] = set() + if addr := self.database_peers_address: + peer_addrs.add(addr) + if addr := self.replication_address: + peer_addrs.add(addr) + if addr := self.replication_offer_address: + peer_addrs.add(addr) + if addr := self.private_address: + peer_addrs.add(addr) + return peer_addrs + class PostgreSQLApplication(RelationState): """An PostgreSQL Application is the peer application state. diff --git a/single_kernel_postgresql/workload/base.py b/single_kernel_postgresql/workload/base.py index f4e543c..c297ba1 100644 --- a/single_kernel_postgresql/workload/base.py +++ b/single_kernel_postgresql/workload/base.py @@ -41,6 +41,27 @@ def root(self) -> PathProtocol: """Return the root path.""" pass + @property + @abstractmethod + def user(self) -> str: + """The OS user that owns workload files (substrate-specific).""" + pass + + @property + @abstractmethod + def group(self) -> str: + """The OS group that owns workload files (substrate-specific).""" + pass + + @property + def tls_file_mode(self) -> int: + """File mode for TLS material written to disk. + + Defaults to 0o600 (VM); K8s overrides to 0o400 to match the + pre-migration charm. + """ + return 0o600 + @abstractmethod def install(self) -> None: """Install the workload.""" @@ -59,7 +80,12 @@ def workload_present(self) -> bool: pass def write_text( - self, content: str, path: pathops.PathProtocol, mode: int | None = None + self, + content: str, + path: pathops.PathProtocol, + mode: int | None = None, + user: str | None = None, + group: str | None = None, ) -> None: """Write content to a file on disk. @@ -67,18 +93,22 @@ def write_text( content (str): The content to be written. path (pathops.PathProtocol): The file path where the content should be written. mode (int, optional): The mode/permissions to use when writing the file. + user (str, optional): The user to own the file (forwarded to pathops for + substrate-correct chown: os.chown on VM, Pebble push on K8s). + group (str, optional): The group to own the file (forwarded to pathops). Raises: PostgreSQLFileOperationError: If there is an error during the file write operation. """ try: - path.write_text(content, mode=mode) + path.write_text(content, mode=mode, user=user, group=group) except ( FileNotFoundError, LookupError, NotADirectoryError, PermissionError, pathops.PebbleConnectionError, + PebbleError, ValueError, ) as e: raise PostgreSQLFileOperationError(e) from e diff --git a/single_kernel_postgresql/workload/k8s.py b/single_kernel_postgresql/workload/k8s.py index 5bae728..00c91a5 100644 --- a/single_kernel_postgresql/workload/k8s.py +++ b/single_kernel_postgresql/workload/k8s.py @@ -167,6 +167,21 @@ def temp_file( except PostgreSQLFileOperationError as e: logger.warning(f"Failed to delete temporary file {file_path}: {e}") + @property + def user(self) -> str: + """The OS user that owns workload files in the K8s container.""" + return "postgres" + + @property + def group(self) -> str: + """The OS group that owns workload files in the K8s container.""" + return "postgres" + + @property + def tls_file_mode(self) -> int: + """K8s pushes TLS material owner-read-only, matching the original charm.""" + return 0o400 + @property def root(self) -> PathProtocol: """Return the root path for container filesystem. diff --git a/single_kernel_postgresql/workload/paths/base.py b/single_kernel_postgresql/workload/paths/base.py index 4b98794..2f1784a 100644 --- a/single_kernel_postgresql/workload/paths/base.py +++ b/single_kernel_postgresql/workload/paths/base.py @@ -82,3 +82,9 @@ def patroni_logs(self) -> PathProtocol: def pgbackrest_conf(self) -> PathProtocol: """Path to the patroni logs.""" pass + + @property + @abstractmethod + def tls(self) -> PathProtocol: + """Directory where TLS files are written for this substrate.""" + pass diff --git a/single_kernel_postgresql/workload/paths/k8s.py b/single_kernel_postgresql/workload/paths/k8s.py index ffe82ce..1988f46 100644 --- a/single_kernel_postgresql/workload/paths/k8s.py +++ b/single_kernel_postgresql/workload/paths/k8s.py @@ -6,6 +6,7 @@ from single_kernel_postgresql.config.literals import ( K8S_DATA_PATH, + PATRONI_CONF_PATH, PATRONI_LOGS_PATH, PGBACKREST_CONF_PATH, POSTGRESQL_CONF_FILE, @@ -60,7 +61,17 @@ def postgresql_conf(self) -> PathProtocol: @property def patroni_conf(self) -> PathProtocol: - """Path to the patroni configuration file.""" + """Path to the patroni configuration directory.""" + return self.root / PATRONI_CONF_PATH + + @property + def tls(self) -> PathProtocol: + """Directory where TLS files are written on K8s (the data dir Patroni reads from). + + This is the *unversioned* data storage root (``/var/lib/pg/data``), which is + where the charm-rendered patroni.yml references the ``.pem`` files + (``{storage_path}/*.pem``) — NOT the versioned ``data`` subdir (``.../16/main``). + """ return self.root / K8S_DATA_PATH @property diff --git a/single_kernel_postgresql/workload/paths/vm.py b/single_kernel_postgresql/workload/paths/vm.py index 3f5e76b..cc98a3c 100644 --- a/single_kernel_postgresql/workload/paths/vm.py +++ b/single_kernel_postgresql/workload/paths/vm.py @@ -74,6 +74,11 @@ def patroni_conf(self) -> PathProtocol: """Path to the patroni.yaml file.""" return self.snap_current / PATRONI_CONF_PATH + @property + def tls(self) -> PathProtocol: + """Directory where TLS files are written on VM (same as patroni_conf).""" + return self.patroni_conf + @property def patroni_logs(self) -> PathProtocol: """Path to the patroni logs.""" diff --git a/single_kernel_postgresql/workload/vm.py b/single_kernel_postgresql/workload/vm.py index 516a602..a27a6b2 100644 --- a/single_kernel_postgresql/workload/vm.py +++ b/single_kernel_postgresql/workload/vm.py @@ -195,6 +195,16 @@ def temp_file( except OSError as e: raise e + @property + def user(self) -> str: + """The OS user that owns workload files on VM.""" + return "_daemon_" + + @property + def group(self) -> str: + """The OS group that owns workload files on VM.""" + return "_daemon_" + @property def paths(self) -> BasePaths: """Return Workload's paths.""" diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py new file mode 100644 index 0000000..5b15250 --- /dev/null +++ b/tests/unit/conftest.py @@ -0,0 +1,37 @@ +# Copyright 2026 Canonical Ltd. +# See LICENSE file for licensing details. + +from unittest.mock import patch + +import pytest +from ops.testing import Harness +from single_kernel_postgresql.charms import k8s_charm, vm_charm +from single_kernel_postgresql.config.literals import PEER_RELATION + + +@pytest.fixture +def harness(substrate, test_charm_path): + """A begun Harness for the substrate's test charm, with the peer relation added.""" + with open(test_charm_path + "/metadata.yaml") as meta_file: + meta = meta_file.read() + with open(test_charm_path + "/actions.yaml") as actions_file: + actions = actions_file.read() + if substrate == "vm": + harness = Harness(vm_charm.PostgreSQLVMCharm, meta=meta, actions=actions) + else: + harness = Harness(k8s_charm.PostgreSQLK8sCharm, meta=meta, actions=actions) + peer_rel_id = harness.add_relation(PEER_RELATION, "postgresql-single-kernel") + harness.add_relation_unit(peer_rel_id, "postgresql-single-kernel/0") + # Set before begin(): Model.name (K8s namespace) is read by substrate-aware + # state accessors (e.g. common_hosts Service FQDNs). + harness.set_model_name("test-model") + # The workload's versioned paths (K8sPaths) read the major version via + # get_postgresql_version(), which reads refresh_versions.toml from cwd — absent + # in the unit env. Patch it, mirroring tests/unit/test_postgresql.py. + with patch( + "single_kernel_postgresql.workload.base.BaseWorkload.get_postgresql_version", + return_value="16.0", + ): + harness.begin() + yield harness + harness.cleanup() From 6dbc89922f2ff1dddf757517a00ddbe8282c19b8 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Thu, 2 Jul 2026 15:21:23 -0300 Subject: [PATCH 02/10] fix(tls): keep K8s patroni_conf at the data dir (match #172) The TLS stack had overridden the K8s patroni_conf path to /etc/patroni, a vestige of an earlier TLS-hardening lineage. #172's Patroni port renders patroni.yaml at patroni_conf, so it must stay the data storage root (/var/lib/pg/data); the override made a consuming charm run 'patroni /etc/patroni/patroni.yaml' against a config rendered elsewhere. TLS writes its .pem files to the separate 'tls' path (also the data dir) and does not read patroni_conf, so this revert is TLS-safe. Signed-off-by: Marcelo Henrique Neppel --- single_kernel_postgresql/workload/paths/k8s.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/single_kernel_postgresql/workload/paths/k8s.py b/single_kernel_postgresql/workload/paths/k8s.py index 1988f46..e435c47 100644 --- a/single_kernel_postgresql/workload/paths/k8s.py +++ b/single_kernel_postgresql/workload/paths/k8s.py @@ -6,7 +6,6 @@ from single_kernel_postgresql.config.literals import ( K8S_DATA_PATH, - PATRONI_CONF_PATH, PATRONI_LOGS_PATH, PGBACKREST_CONF_PATH, POSTGRESQL_CONF_FILE, @@ -61,8 +60,13 @@ def postgresql_conf(self) -> PathProtocol: @property def patroni_conf(self) -> PathProtocol: - """Path to the patroni configuration directory.""" - return self.root / PATRONI_CONF_PATH + """Path to the patroni configuration directory. + + This is the data storage root (``/var/lib/pg/data``) where the Patroni + config subsystem renders ``patroni.yaml`` — matching #172's Patroni port. + (The TLS ``.pem`` files go to the ``tls`` path, which is the same dir.) + """ + return self.root / K8S_DATA_PATH @property def tls(self) -> PathProtocol: From c6aa083a97b77db11b2f3d970baa50d7f8730965 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Wed, 1 Jul 2026 15:28:03 -0300 Subject: [PATCH 03/10] feat(tls): add substrate-aware CharmState TLS SAN/CN accessors Layer the certificate-SAN and common-name policy onto CharmState, on top of the raw peer-databag accessors from the previous branch, so the substrate-specific certificate identity is reviewable as a unit with its own tests before any manager or handler consumes it. K8s must regain the parity the migration had dropped: common_hosts has to advertise the primary/replicas Service FQDNs and the resolved pod FQDN, and the operator-cert common name has to be the endpoints FQDN (wildcarded past the 64-char CN limit) rather than the VM-style host/address; the peer SAN set must exclude the ip key the original K8s charm never emitted. VM behaviour is left host/address-derived as before. The CharmState charm parameter is also widened to ops.CharmBase so the state object no longer depends on the concrete charm type. These accessors are additive and only read state, so the existing charm keeps constructing unchanged. Signed-off-by: Marcelo Henrique Neppel --- single_kernel_postgresql/core/state.py | 61 +++++++++-- tests/unit/test_tls_client_addrs.py | 50 +++++++++ tests/unit/test_tls_state.py | 138 +++++++++++++++++++++++++ 3 files changed, 243 insertions(+), 6 deletions(-) create mode 100644 tests/unit/test_tls_client_addrs.py create mode 100644 tests/unit/test_tls_state.py diff --git a/single_kernel_postgresql/core/state.py b/single_kernel_postgresql/core/state.py index 8f365e5..e18f16f 100644 --- a/single_kernel_postgresql/core/state.py +++ b/single_kernel_postgresql/core/state.py @@ -8,11 +8,12 @@ import socket from contextlib import suppress from functools import cached_property -from typing import TYPE_CHECKING, Any, get_args +from typing import Any, get_args from data_platform_helpers.advanced_statuses import StatusesState, StatusObject from data_platform_helpers.advanced_statuses.types import Scope as AdvancedStatusesScope from ops import ( + CharmBase, ConfigData, JujuVersion, ModelError, @@ -43,16 +44,13 @@ from single_kernel_postgresql.utils.secret import translate_field_to_secret_key from single_kernel_postgresql.utils.status import format_status -if TYPE_CHECKING: - from single_kernel_postgresql.charms.abstract_charm import AbstractPostgreSQLCharm - class CharmState(Object): """The global PostgreSQL Charm State.""" def __init__( self, - charm: "AbstractPostgreSQLCharm", + charm: CharmBase, substrate: Substrates, ) -> None: """Initialize the CharmState object.""" @@ -245,13 +243,64 @@ def host(self) -> str: @property def common_hosts(self) -> set[str]: """Common hosts to be used in TLS certificate SANs.""" - return {self.host, self.fqdn} if self.fqdn else {self.host} + hosts = {self.host, self.fqdn} if self.fqdn else {self.host} + if self.substrate == Substrates.K8S: + namespace = self.model.name + hosts |= { + f"{self.model.app.name}-primary.{namespace}.svc.cluster.local", + f"{self.model.app.name}-replicas.{namespace}.svc.cluster.local", + # the original K8s charm also included the resolved per-pod FQDN. + socket.getfqdn(), + } + return hosts @property def peer_common_name(self) -> str: """Return the common name for the internally generated peer certificate.""" + if self.substrate == Substrates.K8S: + return self._k8s_cert_common_name return self.peer.database_peers_address or self.host + @property + def client_addresses(self) -> set[str]: + """Client-facing addresses for the operator client certificate SANs.""" + addrs: set[str] = set() + if addr := self.peer.database_address: + addrs.add(addr) + return addrs + + @property + def client_common_name(self) -> str: + """Common name for the operator client certificate.""" + if self.substrate == Substrates.K8S: + return self._k8s_cert_common_name + return self.peer.database_address or self.host + + @property + def _k8s_cert_common_name(self) -> str: + """K8s operator-cert CN: the unit endpoints FQDN, wildcarded if too long. + + Matches the original K8s charm: ``-.-endpoints``, collapsing to + ``*.-endpoints`` when the full FQDN exceeds the 64-char CN limit. The + migration had switched this to the VM-style ``database_address/peers_address or + host``; restore the endpoints-FQDN CN for K8s parity. + """ + full = self._get_hostname_from_unit(unit_name_to_pod_name(self.model.unit.name)) + if len(full) > 64: + return f"*.{self.model.app.name}-endpoints" + return full + + @property + def peer_addresses(self) -> set[str]: + """Peer addresses for the operator peer certificate SANs (substrate-aware). + + K8s excludes the ``ip`` databag key (original K8s charm never added an ``ip`` SAN); + VM keeps it (unchanged from pre-migration behavior). + """ + if self.substrate == Substrates.K8S: + return self.peer.peer_addresses_no_ip + return self.peer.peer_addresses + @property def listen_ips(self) -> list[str]: """Return the IPs to listen on. diff --git a/tests/unit/test_tls_client_addrs.py b/tests/unit/test_tls_client_addrs.py new file mode 100644 index 0000000..bf481b2 --- /dev/null +++ b/tests/unit/test_tls_client_addrs.py @@ -0,0 +1,50 @@ +# Copyright 2026 Canonical Ltd. +# See LICENSE file for licensing details. + +"""Tests for TLS client address and common-name state accessors.""" + + +def _set_unit_db(harness, key, value): + rel_id = harness.model.get_relation("database-peers").id + harness.update_relation_data(rel_id, harness.charm.unit.name, {key: value}) + + +def test_database_address_reads_unit_databag(harness): + _set_unit_db(harness, "database-address", "10.1.2.3") + assert harness.charm.state.peer.database_address == "10.1.2.3" + + +def test_database_address_none_when_unset(harness): + assert harness.charm.state.peer.database_address is None + + +def test_client_addresses_set(harness): + _set_unit_db(harness, "database-address", "10.1.2.3") + assert harness.charm.state.client_addresses == {"10.1.2.3"} + + +def test_client_addresses_empty_when_unset(harness): + assert harness.charm.state.client_addresses == set() + + +def test_client_common_name_prefers_database_address(substrate, harness): + _set_unit_db(harness, "database-address", "10.1.2.3") + state = harness.charm.state + if substrate == "vm": + # VM: CN follows the database-address databag value. + assert state.client_common_name == "10.1.2.3" + else: + # K8s: CN is the endpoints FQDN, not the databag address. + app = state.model.app.name + assert state.client_common_name == f"{app}-0.{app}-endpoints" + + +def test_client_common_name_falls_back_to_host(substrate, harness): + state = harness.charm.state + if substrate == "vm": + # VM: falls back to host when no databag address is set. + assert state.client_common_name == state.host + else: + # K8s: no fallback — always the endpoints FQDN. + app = state.model.app.name + assert state.client_common_name == f"{app}-0.{app}-endpoints" diff --git a/tests/unit/test_tls_state.py b/tests/unit/test_tls_state.py new file mode 100644 index 0000000..3c90734 --- /dev/null +++ b/tests/unit/test_tls_state.py @@ -0,0 +1,138 @@ +# Copyright 2026 Canonical Ltd. +# See LICENSE file for licensing details. + +"""Tests for TLS state accessors on PostgreSQLPeer. + +Operator cert/key are fetched live from the requirer and are NOT held in peer +state; only the peer CA rotation slots (current-ca / old-ca) and the internal +peer material live here. +""" + +import socket +from unittest.mock import patch + +from single_kernel_postgresql.config.enums import Substrates # noqa: F401 (substrate fixture) + + +def _set_unit_db(harness, key, value): + rel_id = harness.model.get_relation("database-peers").id + harness.update_relation_data(rel_id, harness.charm.unit.name, {key: value}) + + +def test_common_hosts_k8s_includes_service_endpoints(substrate, harness): + """On K8s the cert-SAN source must carry the primary/replicas Service DNS.""" + state = harness.charm.state + app = state.model.app.name + namespace = state.model.name + hosts = state.common_hosts + + if substrate == "k8s": + assert f"{app}-primary.{namespace}.svc.cluster.local" in hosts + assert f"{app}-replicas.{namespace}.svc.cluster.local" in hosts + # host/fqdn still present + assert state.host in hosts + assert state.fqdn in hosts + # the resolved per-pod FQDN is also included (parity with the original charm) + assert socket.getfqdn() in hosts + else: + # VM: only host/fqdn — no k8s service DNS leaks in + assert not any(h.endswith(".svc.cluster.local") for h in hosts) + assert hosts == {state.host, state.fqdn} + + +def test_ca_rotation_slots_roundtrip(harness): + peer = harness.charm.state.peer + peer.current_ca = "CA-1" + peer.old_ca = "CA-0" + + assert peer.current_ca == "CA-1" + assert peer.old_ca == "CA-0" + + +def test_unset_ca_slots_are_none(harness): + peer = harness.charm.state.peer + assert peer.current_ca is None + assert peer.old_ca is None + + +# -- G1: substrate-aware cert common name (parity with the original charm) ---- + + +def test_cert_common_name_is_endpoints_fqdn_on_k8s(substrate, harness): + """K8s operator-cert CN must be `-.-endpoints` (original charm parity).""" + state = harness.charm.state + app = state.model.app.name + expected = f"{app}-0.{app}-endpoints" + + if substrate == "k8s": + assert state.client_common_name == expected + assert state.peer_common_name == expected + else: + # VM unchanged: host-derived, not the endpoints FQDN. + assert state.client_common_name != expected + assert state.peer_common_name != expected + + +def test_cert_common_name_wildcards_when_too_long_on_k8s(substrate, harness): + """K8s CN collapses to `*.-endpoints` when the endpoints FQDN exceeds 64 chars.""" + if substrate != "k8s": + return # wildcard rule is K8s-only + + state = harness.charm.state + app = state.model.app.name + # Force the endpoints FQDN past the 64-char CN limit; the suffix stays app-derived. + long_fqdn = "x" * 80 + with patch.object(state, "_get_hostname_from_unit", return_value=long_fqdn): + assert state._k8s_cert_common_name == f"*.{app}-endpoints" + + +def test_cert_common_name_vm_unchanged(substrate, harness): + """VM CN stays host-derived: client reads database-address, peer reads database-peers-address.""" + if substrate != "vm": + return + + _set_unit_db(harness, "database-address", "10.1.2.3") + _set_unit_db(harness, "database-peers-address", "10.4.5.6") + state = harness.charm.state + assert state.client_common_name == "10.1.2.3" + assert state.peer_common_name == "10.4.5.6" + + +# -- G2: substrate-aware peer_addresses (no `ip` SAN on K8s) ----------------- + + +def test_peer_addresses_excludes_ip_on_k8s(substrate, harness): + """K8s peer SAN set must omit the `ip` databag key (original K8s charm never added it).""" + _set_unit_db(harness, "ip", "10.0.0.1") + _set_unit_db(harness, "private-address", "10.0.0.2") + _set_unit_db(harness, "database-peers-address", "10.0.0.3") + + addrs = harness.charm.state.peer_addresses + + if substrate == "k8s": + assert "10.0.0.1" not in addrs # `ip` excluded + assert "10.0.0.2" in addrs # private-address kept + assert "10.0.0.3" in addrs # database-peers-address kept + else: + assert "10.0.0.1" in addrs # VM keeps `ip` + + +def test_peer_addresses_includes_ip_on_vm(substrate, harness): + """VM peer SAN set keeps `ip` (unchanged from pre-migration behavior).""" + if substrate != "vm": + return + + _set_unit_db(harness, "ip", "10.0.0.1") + addrs = harness.charm.state.peer_addresses + assert "10.0.0.1" in addrs # `ip` retained on VM (the G2 regression was K8s-only) + + +def test_charmstate_accepts_plain_charmbase(): + """CharmState must accept any ops.CharmBase, not only AbstractPostgreSQLCharm.""" + import inspect + + import ops + from single_kernel_postgresql.core.state import CharmState + + ann = inspect.signature(CharmState.__init__).parameters["charm"].annotation + assert ann is ops.CharmBase From 65311e9b1ca0f757bc57c269db690a0392463c24 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Tue, 7 Jul 2026 15:48:59 -0300 Subject: [PATCH 04/10] docs(tls): drop migration-history prose from _k8s_cert_common_name docstring The 'migration had switched this to VM-style ...; restore the endpoints-FQDN CN for K8s parity' sentence narrates a completed regression fix and is redundant with the preceding 'Matches the original K8s charm' line. The format, wildcard rule, and parity rationale already carry the load-bearing context; the migration history belongs in the original commit message, not the docstring. Signed-off-by: Marcelo Henrique Neppel --- single_kernel_postgresql/core/state.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/single_kernel_postgresql/core/state.py b/single_kernel_postgresql/core/state.py index e18f16f..432965f 100644 --- a/single_kernel_postgresql/core/state.py +++ b/single_kernel_postgresql/core/state.py @@ -281,9 +281,7 @@ def _k8s_cert_common_name(self) -> str: """K8s operator-cert CN: the unit endpoints FQDN, wildcarded if too long. Matches the original K8s charm: ``-.-endpoints``, collapsing to - ``*.-endpoints`` when the full FQDN exceeds the 64-char CN limit. The - migration had switched this to the VM-style ``database_address/peers_address or - host``; restore the endpoints-FQDN CN for K8s parity. + ``*.-endpoints`` when the full FQDN exceeds the 64-char CN limit. """ full = self._get_hostname_from_unit(unit_name_to_pod_name(self.model.unit.name)) if len(full) > 64: From f78c998cfb92a1a46996849f7db59779e822f168 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Wed, 1 Jul 2026 15:46:13 -0300 Subject: [PATCH 05/10] feat(tls): operator-cert manager, events handler, and wiring Wire the operator-certificate TLSManager and TLS events handler into the lib charm and bump the library version. The manager fetches operator cert/key live from the tls_certificates V4 requirers (constructor-injected by the handler); only the peer CA is tracked in state for rotation. Unit tests for this layer land in the stacked tests PR. Signed-off-by: Marcelo Henrique Neppel --- pyproject.toml | 2 +- .../charms/abstract_charm.py | 12 +- single_kernel_postgresql/events/tls.py | 151 +++++++++++++++++ single_kernel_postgresql/managers/tls.py | 160 +++++++++++++++++- uv.lock | 2 +- 5 files changed, 319 insertions(+), 8 deletions(-) create mode 100644 single_kernel_postgresql/events/tls.py diff --git a/pyproject.toml b/pyproject.toml index 31420e3..bbf204b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ [project] name = "postgresql-charms-single-kernel" description = "Shared and reusable code for PostgreSQL-related charms" -version = "16.3.2" +version = "16.3.3" readme = "README.md" license = {file = "LICENSE"} authors = [ diff --git a/single_kernel_postgresql/charms/abstract_charm.py b/single_kernel_postgresql/charms/abstract_charm.py index 2a18407..671de44 100644 --- a/single_kernel_postgresql/charms/abstract_charm.py +++ b/single_kernel_postgresql/charms/abstract_charm.py @@ -9,6 +9,7 @@ from single_kernel_postgresql.core.state import CharmState from single_kernel_postgresql.events.postgresql import PostgreSQLEventsHandler +from single_kernel_postgresql.events.tls import TLS from single_kernel_postgresql.managers.cluster import ClusterManager from single_kernel_postgresql.managers.config import ConfigManager from single_kernel_postgresql.managers.patroni import PatroniManager @@ -28,8 +29,17 @@ def __init__(self, *args): # State self.state = CharmState(charm=self, substrate=self.substrate) + # TLS events handler owns the two certificate 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) + # Managers - self.tls_manager = TLSManager(state=self.state, workload=self.workload) + self.tls_manager = TLSManager( + state=self.state, + workload=self.workload, + client_certificate=self.tls.client_certificate, + peer_certificate=self.tls.peer_certificate, + ) self.patroni_manager = PatroniManager(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) diff --git a/single_kernel_postgresql/events/tls.py b/single_kernel_postgresql/events/tls.py new file mode 100644 index 0000000..589ee50 --- /dev/null +++ b/single_kernel_postgresql/events/tls.py @@ -0,0 +1,151 @@ +# Copyright 2026 Canonical Ltd. +# See LICENSE file for licensing details. +"""TLS events handler — owns the operator-certificate requirers, delegates to TLSManager.""" + +import logging + +from charmlibs.interfaces.tls_certificates import ( + CertificateRequestAttributes, + TLSCertificatesRequiresV4, +) +from ops import EventSource +from ops.framework import EventBase, Object + +from single_kernel_postgresql.config.exceptions import PostgreSQLFileOperationError +from single_kernel_postgresql.config.literals import ( + PEER_RELATION, + TLS_CLIENT_RELATION, + TLS_PEER_RELATION, +) + +logger = logging.getLogger(__name__) + + +class RefreshTLSCertificatesEvent(EventBase): + """Event emitted to trigger a re-request of TLS certificates with updated SANs.""" + + +class TLS(Object): + """Owns the client/peer certificate requirers and pushes assigned certs to the workload. + + Operator-certificate handler: observes certificate_available and triggers a + file-push via TLSManager. Also owns the refresh_tls_certificates_event that + re-requests certificates whenever SANs change (emitted on peer relation_changed). + + Design notes + ------------ + 1. **Live-fetch model.** Operator cert/key are read live from the requirers + (TLSManager.get_*_tls_files call get_assigned_certificates() on demand) and are + never persisted — matching the pre-port charm. Only the peer CA is tracked in + state (``current-ca`` / ``old-ca``) so the CA bundle can include the previous CA + across a rotation; the requirer holds only the current cert. The manager is + constructor-injected with these requirers and reads from them at call time. + + 2. **CA bundle terminology.** The ``current_ca`` term used in state mirrors the + charm's live operator-CA term. It refers to the most recent peer CA from the TLS + operator, not the internal self-signed CA. + + 3. **SANs and the relation_changed trigger.** The ``relation_changed``-driven + ``refresh_tls_certificates_event`` is a stand-in for the charm's IP-change + trigger. The client/peer SANs in the certificate requests remain inert until + the cluster (address-writer) code migrates and starts writing + ``database-address`` / ``database-peers-address`` keys into the peer databag. + """ + + refresh_tls_certificates_event = EventSource(RefreshTLSCertificatesEvent) + + def __init__(self, charm, state): + super().__init__(charm, key="tls") + self.charm = charm + self.state = state + + client_addresses = self.state.client_addresses + peer_addresses = self.state.peer_addresses + + self.client_certificate = TLSCertificatesRequiresV4( + self.charm, + TLS_CLIENT_RELATION, + certificate_requests=[ + CertificateRequestAttributes( + common_name=self.state.client_common_name, + sans_ip=frozenset(client_addresses), + sans_dns=frozenset({*self.state.common_hosts, *client_addresses}), + ), + ], + refresh_events=[self.refresh_tls_certificates_event], + ) + self.peer_certificate = TLSCertificatesRequiresV4( + self.charm, + TLS_PEER_RELATION, + certificate_requests=[ + CertificateRequestAttributes( + common_name=self.state.peer_common_name, + sans_ip=frozenset(peer_addresses), + sans_dns=frozenset({*self.state.common_hosts, *peer_addresses}), + ), + ], + refresh_events=[self.refresh_tls_certificates_event], + ) + + # The TLS manager is constructor-injected with these requirers (built in + # AbstractPostgreSQLCharm right after this handler); its live-fetch getters + # read cert/key from them. This handler reaches the manager via self.charm. + + 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 + ) + self.framework.observe( + self.charm.on[PEER_RELATION].relation_changed, self._on_peer_relation_changed + ) + + def _on_peer_relation_changed(self, event) -> None: + """Re-request certificates when peer addresses change. + + Fires on any peer-databag change: the address keys this should key on + (``database-address`` / ``database-peers-address``) are only written once + the cluster address-writer code migrates (see design note 3 above), and + the requirer no-ops when the resulting SANs are unchanged. + """ + self.refresh_tls_certificates_event.emit() + + def _push_tls_files(self, event) -> None: + """Guard-then-push helper: defer if the workload is not yet ready. + + Two conditions must hold before files can be written: + 1. The internal CA secret must exist — it is written by the leader on + leader-elected, so non-leaders and early hooks may see it absent. + 2. The workload must accept file writes — on K8s the Pebble container + may not be ready yet, causing PostgreSQLFileOperationError. + """ + if not self.state.application.internal_ca: + logger.debug("Internal CA not yet present; deferring TLS file push.") + event.defer() + return + try: + self.charm.tls_manager.push_tls_files() + except PostgreSQLFileOperationError: + logger.debug("Workload not ready for TLS file write; deferring.") + event.defer() + + def _on_certificate_available(self, event) -> None: + """Push TLS files; the operator client cert/key is read live at push time.""" + self._push_tls_files(event) + + def _on_peer_certificate_available(self, event) -> None: + """Rotate the peer CA if it changed, then push (cert/key read live at push time).""" + certs, _ = self.peer_certificate.get_assigned_certificates() + new_ca = str(certs[0].ca) if certs else None + if new_ca is not None: + self.charm.tls_manager.rotate_peer_ca(new_ca) + else: + self.charm.tls_manager.clear_peer_ca() + self._push_tls_files(event) diff --git a/single_kernel_postgresql/managers/tls.py b/single_kernel_postgresql/managers/tls.py index 8b12614..374bda1 100644 --- a/single_kernel_postgresql/managers/tls.py +++ b/single_kernel_postgresql/managers/tls.py @@ -13,6 +13,7 @@ from charmlibs.interfaces.tls_certificates import ( Certificate, PrivateKey, + TLSCertificatesRequiresV4, generate_ca, generate_certificate, generate_csr, @@ -21,9 +22,13 @@ from data_platform_helpers.advanced_statuses import StatusObject from data_platform_helpers.advanced_statuses.types import Scope as AdvancedStatusesScope -from single_kernel_postgresql.config.exceptions import TlsError +from single_kernel_postgresql.config.exceptions import PostgreSQLFileOperationError, TlsError from single_kernel_postgresql.config.literals import ( APP_SCOPE, + TLS_CA_BUNDLE_FILE, + TLS_CA_FILE, + TLS_CERT_FILE, + TLS_KEY_FILE, ) from single_kernel_postgresql.config.statuses import GeneralStatuses from single_kernel_postgresql.core.state import CharmState @@ -39,8 +44,20 @@ class TLSManager(BaseManager): This manager is responsible for handling TLS configuration operations. """ - def __init__(self, state: CharmState, workload: BaseWorkload): + def __init__( + self, + state: CharmState, + workload: BaseWorkload, + client_certificate: TLSCertificatesRequiresV4, + peer_certificate: TLSCertificatesRequiresV4, + ) -> None: super().__init__(state, workload, "tls_manager") + # Operator-certificate requirers, constructor-injected by events.tls.TLS. + # The getters below fetch cert/key LIVE from the durable relation databag — no + # operator cert/key is persisted to state (matches the pre-port charm). Only the + # peer CA is tracked in state (current-ca / old-ca), for rotation. + self.client_certificate = client_certificate + self.peer_certificate = peer_certificate def configure_internal_peer_ca(self) -> None: """Configure TLS internal peer CA.""" @@ -64,19 +81,22 @@ def generate_internal_peer_cert(self) -> None: csr = generate_csr( private_key, common_name=self.state.peer_common_name, - sans_ip=frozenset(self.state.peer.peer_addresses), + # substrate-aware: K8s excludes the ip SAN (matches the original charm). + sans_ip=frozenset(self.state.peer_addresses), sans_dns=frozenset({ *self.state.common_hosts, # IP address need to be part of the DNS SANs list due to # https://github.com/pgbackrest/pgbackrest/issues/1977. - *self.state.peer.peer_addresses, + *self.state.peer_addresses, }), ) cert = generate_certificate(csr, ca, ca_key, validity=timedelta(days=7300)) self.state.peer.internal_cert = str(cert) self.state.peer.internal_key = str(private_key) - # self.charm.push_tls_files_to_workload() + # NOTE: pushing the internal-peer cert/CA to disk is owned by the config + # subsystem (not yet migrated); operator certs are pushed via + # TLSManager.push_tls_files from the events.tls handler. logger.info( "Internal peer certificate generated. Please use a proper TLS operator if possible." ) @@ -93,6 +113,136 @@ def generate_internal_peer_ca(self) -> None: self.state.set_secret(APP_SCOPE, "internal-ca-key", str(private_key)) self.state.set_secret(APP_SCOPE, "internal-ca", str(ca)) + def rotate_peer_ca(self, ca: str) -> None: + """Track the operator peer CA for rotation (current-ca -> old-ca). + + Only the CA is tracked in state; the operator cert/key are fetched live + from the requirer. + """ + if ca != self.state.peer.current_ca: + if self.state.peer.current_ca: + self.state.peer.old_ca = self.state.peer.current_ca + else: + # Re-enabling after a disable cleared current-ca: nothing is being + # rotated out, so drop any stale old CA left from before the disable. + self.state.peer.remove_secret("old-ca") + self.state.peer.current_ca = ca + + def clear_peer_ca(self) -> None: + """Retire the operator peer CA into old-ca on relation removal.""" + current = self.state.peer.current_ca + if current: + self.state.peer.old_ca = current + self.state.peer.remove_secret("current-ca") + + def get_client_tls_files(self) -> tuple[str | None, str | None, str | None]: + """Return (key, ca, cert) for the operator client cert, fetched live. + + Reads from the requirer's assigned certificate (the durable relation + databag) rather than persisted state — matches the pre-port charm. + """ + key = ca = cert = None + if self.client_certificate is not 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 = str(certs[0].ca) + return key, ca, cert + + def client_tls_files_on_disk(self) -> bool: + """Whether the client TLS files this unit serves are present on disk. + + The reload bridge checks this before enabling TLS in the config so it never + renders ssl:on against files the push has not yet written: on K8s the Pebble + push can defer while the local config render would still succeed. A workload + that cannot be read (container down) counts as not-on-disk, so the caller defers. + """ + tls = self.workload.paths.tls + try: + return all( + self.workload.exists(tls / f) for f in (TLS_KEY_FILE, TLS_CERT_FILE, TLS_CA_FILE) + ) + except PostgreSQLFileOperationError: + return False + + def get_peer_ca_bundle(self) -> str: + """Compose the peer CA bundle: live operator CA, old CA, internal CA. + + The current operator CA is read live from the requirer (pre-port style); + the old CA and internal CA come from state. + """ + operator_ca = "" + if self.peer_certificate is not None: + certs, _ = self.peer_certificate.get_assigned_certificates() + operator_ca = str(certs[0].ca) if certs else "" + cas = [ + operator_ca, + self.state.peer.old_ca, + self.state.get_secret(APP_SCOPE, "internal-ca"), + ] + return "\n".join(ca for ca in cas if ca).strip() + + def get_peer_tls_files(self) -> tuple[str | None, str | None, str | None]: + """Return (key, ca, cert) for the peer certificate, operator cert fetched live. + + Prefers the operator-provided peer material (read live from the requirer, + with the composed CA bundle); falls back to the internally generated peer + material. + """ + key = cert = None + if self.peer_certificate is not None: + certs, private_key = self.peer_certificate.get_assigned_certificates() + if private_key: + key = str(private_key) + if certs: + cert = str(certs[0].certificate) + if not all((key, cert)): + return ( + self.state.peer.internal_key, + self.state.get_secret(APP_SCOPE, "internal-ca"), + self.state.peer.internal_cert, + ) + return key, self.get_peer_ca_bundle(), cert + + def _write_tls_file(self, content: str, path) -> None: + """Write a TLS file with substrate-specific permissions and ownership. + + The mode comes from the workload (VM 0o600, K8s 0o400, matching the + pre-migration charms). Ownership is forwarded through pathops so it works + on both VM (LocalPath uses os.chown) and K8s (ContainerPath uses Pebble push). + """ + self.workload.write_text( + content, + path, + self.workload.tls_file_mode, + user=self.workload.user, + group=self.workload.group, + ) + + def push_tls_files(self) -> None: + """Write the client, peer, and CA-bundle TLS files to the workload.""" + tls = self.workload.paths.tls + + key, ca, cert = self.get_client_tls_files() + if key is not None: + self._write_tls_file(key, tls / TLS_KEY_FILE) + if ca is not None: + self._write_tls_file(ca, tls / TLS_CA_FILE) + if cert is not None: + self._write_tls_file(cert, tls / TLS_CERT_FILE) + + key, ca, cert = self.get_peer_tls_files() + if key is not None: + self._write_tls_file(key, tls / f"peer_{TLS_KEY_FILE}") + if ca is not None: + self._write_tls_file(ca, tls / f"peer_{TLS_CA_FILE}") + if cert is not None: + self._write_tls_file(cert, tls / f"peer_{TLS_CERT_FILE}") + + self._write_tls_file(self.get_peer_ca_bundle(), tls / TLS_CA_BUNDLE_FILE) + def get_statuses( self, scope: AdvancedStatusesScope, recompute: bool = False ) -> list[StatusObject]: diff --git a/uv.lock b/uv.lock index 583ce35..91b9ded 100644 --- a/uv.lock +++ b/uv.lock @@ -982,7 +982,7 @@ wheels = [ [[package]] name = "postgresql-charms-single-kernel" -version = "16.3.2" +version = "16.3.3" source = { editable = "." } dependencies = [ { name = "ops", version = "2.23.2", source = { registry = "https://pypi.org/simple" }, marker = "python_full_version < '3.10'" }, From a1d2178298b7eb77fe644a9ff4a4e9f934b45b55 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Wed, 1 Jul 2026 15:49:17 -0300 Subject: [PATCH 06/10] test(tls): unit tests for the operator-cert manager and events handler Cover TLSManager live-fetch getters, CA rotation, push, and the TLS events handler (certificate_available / relation_broken / defer paths). Stacked on the manager+events PR whose code these exercise. Signed-off-by: Marcelo Henrique Neppel --- tests/unit/test_tls_events.py | 295 ++++++++++++++++++++++++++++++++ tests/unit/test_tls_manager.py | 299 +++++++++++++++++++++++++++++++++ 2 files changed, 594 insertions(+) create mode 100644 tests/unit/test_tls_events.py create mode 100644 tests/unit/test_tls_manager.py diff --git a/tests/unit/test_tls_events.py b/tests/unit/test_tls_events.py new file mode 100644 index 0000000..c04742c --- /dev/null +++ b/tests/unit/test_tls_events.py @@ -0,0 +1,295 @@ +# Copyright 2026 Canonical Ltd. +# See LICENSE file for licensing details. +"""Tests for the TLS events handler (single_kernel_postgresql/events/tls.py). + +The handler is live-fetch: operator cert/key are read from the requirer on demand +(get_assigned_certificates), never persisted. Only the peer CA is tracked in state +(current-ca / old-ca) for rotation, mirroring postgresql-operator/src/relations/tls.py +_on_peer_certificate_available (which stashes current-ca/old-ca and otherwise reads live). +""" + +from types import SimpleNamespace +from unittest.mock import MagicMock, patch + +from single_kernel_postgresql.config.exceptions import PostgreSQLFileOperationError + + +class _FakePrivateKey: + """Minimal stand-in for PrivateKey: str(key) returns the raw PEM string.""" + + def __init__(self, raw: str) -> None: + self._raw = raw + + def __str__(self) -> str: + return self._raw + + +def _fake_assigned(cert, ca, key): + """Mimic TLSCertificatesRequiresV4.get_assigned_certificates() return shape. + + Returns (list[ProviderCertificate], PrivateKey | None). + """ + provider_cert = SimpleNamespace(certificate=cert, ca=ca) + return [provider_cert], _FakePrivateKey(key) + + +def test_handler_is_wired(harness): + tls = harness.charm.tls + assert tls.client_certificate is not None + assert tls.peer_certificate is not None + # the handler wires its requirers onto the manager so the live-fetch getters work + assert harness.charm.tls_manager.client_certificate is tls.client_certificate + assert harness.charm.tls_manager.peer_certificate is tls.peer_certificate + + +def test_client_certificate_available_pushes(harness): + charm = harness.charm + with ( + patch.object(charm.cluster_manager, "configure_system_passwords"), + patch.object(charm.config_manager, "update_config"), + ): + harness.set_leader(True) + + tls = charm.tls + tls.client_certificate.get_assigned_certificates = MagicMock( + return_value=_fake_assigned("CC", "CA", "CK") + ) + charm.tls_manager.push_tls_files = MagicMock() + + tls._on_certificate_available(MagicMock()) + + # no operator client cert/key is persisted; the push reads live + charm.tls_manager.push_tls_files.assert_called_once() + assert charm.tls_manager.get_client_tls_files() == ("CK", "CA", "CC") + + +def test_peer_certificate_available_rotates_ca_and_pushes(harness): + charm = harness.charm + with ( + patch.object(charm.cluster_manager, "configure_system_passwords"), + patch.object(charm.config_manager, "update_config"), + ): + harness.set_leader(True) + + tls = charm.tls + tls.peer_certificate.get_assigned_certificates = MagicMock( + return_value=_fake_assigned("PC", "PCA", "PK") + ) + charm.tls_manager.push_tls_files = MagicMock() + + tls._on_peer_certificate_available(MagicMock()) + + peer = charm.state.peer + # only the CA is tracked in state for rotation; cert/key stay live + assert peer.current_ca == "PCA" + charm.tls_manager.push_tls_files.assert_called_once() + key, _, cert = charm.tls_manager.get_peer_tls_files() + assert (key, cert) == ("PK", "PC") + + +def test_certificate_available_pushes_on_empty(harness): + charm = harness.charm + with ( + patch.object(charm.cluster_manager, "configure_system_passwords"), + patch.object(charm.config_manager, "update_config"), + ): + harness.set_leader(True) + + tls = charm.tls + # requirer reports no assigned cert (relation gone) -> getters return nothing + tls.client_certificate.get_assigned_certificates = MagicMock(return_value=([], None)) + charm.tls_manager.push_tls_files = MagicMock() + + tls._on_certificate_available(MagicMock()) + + assert charm.tls_manager.get_client_tls_files() == (None, None, None) + charm.tls_manager.push_tls_files.assert_called_once() + + +def test_peer_certificate_available_clears_ca_on_empty(harness): + charm = harness.charm + with ( + patch.object(charm.cluster_manager, "configure_system_passwords"), + patch.object(charm.config_manager, "update_config"), + ): + harness.set_leader(True) + + tls = charm.tls + # seed a current CA via a prior rotation, then the relation goes empty + charm.tls_manager.rotate_peer_ca("PCA") + tls.peer_certificate.get_assigned_certificates = MagicMock(return_value=([], None)) + charm.tls_manager.push_tls_files = MagicMock() + + tls._on_peer_certificate_available(MagicMock()) + + peer = charm.state.peer + assert peer.old_ca == "PCA" + assert peer.current_ca is None + charm.tls_manager.push_tls_files.assert_called_once() + + +def test_relation_broken_client_wired(harness): + """relation_broken on TLS_CLIENT_RELATION routes to _on_certificate_available (live push).""" + charm = harness.charm + client_rel_id = harness.add_relation("client-certificates", "tls-provider") + charm.tls_manager.push_tls_files = MagicMock() + harness.remove_relation(client_rel_id) + + # With the relation gone, the live getter reports nothing. + assert charm.tls_manager.get_client_tls_files() == (None, None, None) + + +def test_relation_broken_peer_wired(harness): + """relation_broken on TLS_PEER_RELATION routes to _on_peer_certificate_available (clears CA).""" + charm = harness.charm + # seed a current CA so the broken path has something to retire + charm.tls_manager.rotate_peer_ca("PCA") + + peer_rel_id = harness.add_relation("peer-certificates", "tls-provider") + charm.tls_manager.push_tls_files = MagicMock() + harness.remove_relation(peer_rel_id) + + # The broken handler retired the current CA into old-ca and cleared current. + assert charm.state.peer.current_ca is None + assert charm.state.peer.old_ca == "PCA" + + +def _set_unit_db(harness, key, value): + rel_id = harness.model.get_relation("database-peers").id + harness.update_relation_data(rel_id, harness.charm.unit.name, {key: value}) + + +def test_client_and_peer_requesters_have_distinct_common_names(substrate, harness): + """Client and peer requesters use distinct CNs drawn from different databag keys. + + certificate_requests are baked at init time (before the test updates the databag), so + we verify distinctness via the live state properties (which read directly from the + databag) and confirm both requester objects were constructed. + + On VM the CNs are host-derived (database-address / database-peers-address). On K8s + both CNs collapse to the endpoints FQDN (original charm parity) — still distinct from + each other is not required there, only that both are the endpoints FQDN. + """ + _set_unit_db(harness, "database-address", "10.1.2.3") + _set_unit_db(harness, "database-peers-address", "10.4.5.6") + + state = harness.charm.state + if substrate == "vm": + # VM: client CN reads database-address, peer CN reads database-peers-address. + assert state.client_common_name == "10.1.2.3" + assert state.peer_common_name == "10.4.5.6" + assert state.client_common_name != state.peer_common_name + else: + # K8s: both CNs are the unit endpoints FQDN (operator-cert parity). + app = state.model.app.name + expected = f"{app}-0.{app}-endpoints" + assert state.client_common_name == expected + assert state.peer_common_name == expected + + tls = harness.charm.tls + assert tls.client_certificate is not None + assert tls.peer_certificate is not None + + +def test_refresh_event_defined_and_wired(harness): + """refresh_tls_certificates_event exists; peer relation_changed emits it without error.""" + tls = harness.charm.tls + assert hasattr(tls, "refresh_tls_certificates_event") + + charm = harness.charm + peer_rel_id = harness.model.get_relation("database-peers").id + with ( + patch.object(charm.cluster_manager, "configure_system_passwords", return_value=None), + patch.object(charm.config_manager, "update_config", return_value=None), + ): + harness.update_relation_data( + peer_rel_id, charm.unit.name, {"database-address": "10.9.8.7"} + ) + + +def test_certificate_available_defers_when_internal_ca_absent(harness): + """When internal-ca is not yet set, _on_certificate_available defers and skips push. + + Mirrors postgresql-operator/src/relations/tls.py lines 157-161: the handler must + not attempt file writes before the CA is present (K8s Pebble may not be ready). + """ + tls = harness.charm.tls + tls.client_certificate.get_assigned_certificates = MagicMock( + return_value=_fake_assigned("CC", "CA", "CK") + ) + harness.charm.tls_manager.push_tls_files = MagicMock() + + event = MagicMock() + # internal_ca is None because no leader has set it yet + assert harness.charm.state.application.internal_ca is None + tls._on_certificate_available(event) + + event.defer.assert_called_once() + harness.charm.tls_manager.push_tls_files.assert_not_called() + + +def test_peer_certificate_available_defers_when_internal_ca_absent(harness): + """When internal-ca is not yet set, _on_peer_certificate_available defers and skips push.""" + tls = harness.charm.tls + tls.peer_certificate.get_assigned_certificates = MagicMock( + return_value=_fake_assigned("PC", "PCA", "PK") + ) + harness.charm.tls_manager.push_tls_files = MagicMock() + + event = MagicMock() + assert harness.charm.state.application.internal_ca is None + tls._on_peer_certificate_available(event) + + event.defer.assert_called_once() + harness.charm.tls_manager.push_tls_files.assert_not_called() + + +def test_certificate_available_defers_on_workload_file_error(harness): + """When push_tls_files raises PostgreSQLFileOperationError, the handler defers. + + Mirrors postgresql-operator/src/relations/tls.py lines 162-170: workload + file-write failures (e.g. Pebble not yet ready on K8s) must defer rather + than crash the hook. + """ + charm = harness.charm + with ( + patch.object(charm.cluster_manager, "configure_system_passwords"), + patch.object(charm.config_manager, "update_config"), + ): + harness.set_leader(True) + + tls = charm.tls + tls.client_certificate.get_assigned_certificates = MagicMock( + return_value=_fake_assigned("CC", "CA", "CK") + ) + charm.tls_manager.push_tls_files = MagicMock( + side_effect=PostgreSQLFileOperationError("disk full") + ) + + event = MagicMock() + tls._on_certificate_available(event) + + event.defer.assert_called_once() + + +def test_peer_certificate_available_defers_on_workload_file_error(harness): + """When push_tls_files raises PostgreSQLFileOperationError, the peer handler defers.""" + charm = harness.charm + with ( + patch.object(charm.cluster_manager, "configure_system_passwords"), + patch.object(charm.config_manager, "update_config"), + ): + harness.set_leader(True) + + tls = charm.tls + tls.peer_certificate.get_assigned_certificates = MagicMock( + return_value=_fake_assigned("PC", "PCA", "PK") + ) + charm.tls_manager.push_tls_files = MagicMock( + side_effect=PostgreSQLFileOperationError("pebble not ready") + ) + + event = MagicMock() + tls._on_peer_certificate_available(event) + + event.defer.assert_called_once() diff --git a/tests/unit/test_tls_manager.py b/tests/unit/test_tls_manager.py new file mode 100644 index 0000000..a0cea90 --- /dev/null +++ b/tests/unit/test_tls_manager.py @@ -0,0 +1,299 @@ +# Copyright 2026 Canonical Ltd. +# See LICENSE file for licensing details. + +from datetime import timedelta +from types import SimpleNamespace +from unittest.mock import MagicMock, patch, sentinel + + +class _FakePrivateKey: + """Minimal stand-in for PrivateKey: str(key) returns the raw PEM string.""" + + def __init__(self, raw: str) -> None: + self._raw = raw + + def __str__(self) -> str: + return self._raw + + +def _fake_assigned(cert, ca, key): + """Mimic TLSCertificatesRequiresV4.get_assigned_certificates() -> (list, PrivateKey|None).""" + return [SimpleNamespace(certificate=cert, ca=ca)], _FakePrivateKey(key) + + +def _set_client(mgr, cert, ca, key): + mgr.client_certificate.get_assigned_certificates = MagicMock( + return_value=_fake_assigned(cert, ca, key) + ) + + +def _set_peer(mgr, cert, ca, key): + mgr.peer_certificate.get_assigned_certificates = MagicMock( + return_value=_fake_assigned(cert, ca, key) + ) + + +def _set_empty(requirer): + requirer.get_assigned_certificates = MagicMock(return_value=([], None)) + + +# --- CA rotation (current-ca / old-ca are 16/edge secrets, kept; cert/key are NOT stored) --- + + +def test_rotate_peer_ca_sets_current(harness): + mgr = harness.charm.tls_manager + mgr.rotate_peer_ca("CA1") + + peer = harness.charm.state.peer + assert peer.current_ca == "CA1" + assert peer.old_ca is None + + +def test_rotate_peer_ca_rotates_on_change(harness): + mgr = harness.charm.tls_manager + mgr.rotate_peer_ca("CA1") + mgr.rotate_peer_ca("CA2") + + peer = harness.charm.state.peer + assert peer.current_ca == "CA2" + assert peer.old_ca == "CA1" + + +def test_rotate_peer_ca_no_rotation_when_unchanged(harness): + mgr = harness.charm.tls_manager + mgr.rotate_peer_ca("CA1") + mgr.rotate_peer_ca("CA1") + + peer = harness.charm.state.peer + assert peer.current_ca == "CA1" + assert peer.old_ca is None + + +def test_clear_peer_ca_rotates_current_to_old(harness): + mgr = harness.charm.tls_manager + mgr.rotate_peer_ca("CA") + mgr.clear_peer_ca() + + peer = harness.charm.state.peer + assert peer.old_ca == "CA" + assert peer.current_ca is None + + +def test_rotate_peer_ca_clears_stale_old_ca_on_reenable(harness): + """Re-enabling after a disable must not leave a stale pre-disable CA in the bundle.""" + mgr = harness.charm.tls_manager + mgr.rotate_peer_ca("A") + mgr.rotate_peer_ca("B") # rotate A -> B + assert mgr.state.peer.current_ca == "B" + assert mgr.state.peer.old_ca == "A" + mgr.clear_peer_ca() # disable stashes B as old and clears current + assert mgr.state.peer.current_ca is None + assert mgr.state.peer.old_ca == "B" + mgr.rotate_peer_ca("C") # re-enable with a fresh CA + assert mgr.state.peer.current_ca == "C" + assert mgr.state.peer.old_ca is None + + +# --- operator cert/key are fetched LIVE from the requirer, never persisted --- + + +def test_get_client_tls_files_none_when_absent(harness): + mgr = harness.charm.tls_manager + _set_empty(mgr.client_certificate) + assert mgr.get_client_tls_files() == (None, None, None) + + +def test_get_client_tls_files_returns_live(harness): + mgr = harness.charm.tls_manager + _set_client(mgr, cert="C", ca="A", key="K") + assert mgr.get_client_tls_files() == ("K", "A", "C") + + +def test_get_peer_ca_bundle_composes_live_old_internal(harness): + charm = harness.charm + # leadership is required to write the app-scoped internal-ca secret + with ( + patch.object(charm.cluster_manager, "configure_system_passwords"), + patch.object(charm.config_manager, "update_config"), + ): + harness.set_leader(True) + + mgr = charm.tls_manager + _set_peer(mgr, cert="PC", ca="CUR", key="PK") # live operator CA = CUR + charm.state.peer.old_ca = "OLD" + charm.state.set_secret("app", "internal-ca", "INT") + + assert mgr.get_peer_ca_bundle() == "CUR\nOLD\nINT" + + +def test_get_peer_tls_files_prefers_live_operator(harness): + mgr = harness.charm.tls_manager + _set_peer(mgr, cert="PC", ca="PCA", key="PK") + key, ca, cert = mgr.get_peer_tls_files() + assert (key, cert) == ("PK", "PC") + assert ca == "PCA" # bundle = live operator CA only (no old, no internal here) + + +def test_get_peer_tls_files_falls_back_to_internal(harness): + charm = harness.charm + # leadership is required to write the app-scoped internal-ca secret + with ( + patch.object(charm.cluster_manager, "configure_system_passwords"), + patch.object(charm.config_manager, "update_config"), + ): + harness.set_leader(True) + + mgr = charm.tls_manager + _set_empty(mgr.peer_certificate) + peer = charm.state.peer + peer.internal_key = "IK" + peer.internal_cert = "IC" + charm.state.set_secret("app", "internal-ca", "ICA") + + assert mgr.get_peer_tls_files() == ("IK", "ICA", "IC") + + +def test_push_tls_files_writes_expected_files(harness): + mgr = harness.charm.tls_manager + _set_client(mgr, cert="CC", ca="CA", key="CK") + _set_peer(mgr, cert="PC", ca="PCA", key="PK") + + mgr.workload.write_text = MagicMock() + mgr.push_tls_files() + + written = {call.args[1].name: call.args[0] for call in mgr.workload.write_text.call_args_list} + assert written["key.pem"] == "CK" + assert written["ca.pem"] == "CA" + assert written["cert.pem"] == "CC" + assert written["peer_key.pem"] == "PK" + assert written["peer_cert.pem"] == "PC" + assert written["peer_ca.pem"] == "PCA" + assert written["peer_ca_bundle.pem"] == "PCA" + # all TLS files are written with the workload's substrate-specific mode + # (VM 0o600, K8s 0o400), user, and group + expected_mode = mgr.workload.tls_file_mode + expected_user = mgr.workload.user + expected_group = mgr.workload.group + for call in mgr.workload.write_text.call_args_list: + assert call.args[2] == expected_mode + assert call.kwargs["user"] == expected_user + assert call.kwargs["group"] == expected_group + # every TLS file is written under the substrate-correct TLS dir (paths.tls) + for call in mgr.workload.write_text.call_args_list: + assert call.args[1].parent == mgr.workload.paths.tls + + +def test_tls_manager_constructor_injects_requirers(harness): + """TLSManager takes the two cert requirers at construction (no post-init mutation). + + The manager holds no PostgreSQL client (it never uses one); it reads operator + cert/key live from the constructor-injected requirers. + """ + from single_kernel_postgresql.managers.tls import TLSManager + + client_req = harness.charm.tls.client_certificate + peer_req = harness.charm.tls.peer_certificate + mgr = TLSManager( + harness.charm.state, + harness.charm.tls_manager.workload, + client_certificate=client_req, + peer_certificate=peer_req, + ) + assert mgr.client_certificate is client_req + assert mgr.peer_certificate is peer_req + # the charm-wired manager gets the same requirers the TLS handler built + assert harness.charm.tls_manager.client_certificate is harness.charm.tls.client_certificate + assert harness.charm.tls_manager.peer_certificate is harness.charm.tls.peer_certificate + + +def test_client_tls_files_on_disk(harness): + """Reports presence of the client key/cert/ca files; container errors count as absent.""" + from single_kernel_postgresql.config.exceptions import PostgreSQLFileOperationError + + mgr = harness.charm.tls_manager + with patch.object(mgr.workload, "exists", return_value=True) as _exists: + assert mgr.client_tls_files_on_disk() is True + assert _exists.call_count == 3 + with patch.object(mgr.workload, "exists", side_effect=[True, False]): + assert mgr.client_tls_files_on_disk() is False + with patch.object(mgr.workload, "exists", side_effect=PostgreSQLFileOperationError("down")): + assert mgr.client_tls_files_on_disk() is False + + +def test_generate_internal_peer_ca_stores_secrets(harness): + """generate_internal_peer_ca persists the generated CA cert/key into app state.""" + charm = harness.charm + # leadership is required to write the app-scoped internal-ca secrets + with ( + patch.object(charm.cluster_manager, "configure_system_passwords"), + patch.object(charm.config_manager, "update_config"), + ): + harness.set_leader(True) + + with ( + patch( + "single_kernel_postgresql.managers.tls.generate_private_key", + return_value=sentinel.ca_key, + ), + patch( + "single_kernel_postgresql.managers.tls.generate_ca", + return_value=sentinel.ca, + ), + ): + charm.tls_manager.generate_internal_peer_ca() + + assert charm.state.application.internal_ca_key == str(sentinel.ca_key) + assert charm.state.application.internal_ca == str(sentinel.ca) + + +def test_generate_internal_peer_cert_stores_material(harness): + """generate_internal_peer_cert persists internal key/cert into peer state and pushes nothing. + + Writing the internal peer cert/CA to disk is owned by the (not-yet-migrated) + config subsystem, so generating the material must not touch the workload. + """ + charm = harness.charm + mgr = charm.tls_manager + # leadership is required to write/read the app-scoped internal-ca secrets the + # internal peer cert is signed against + with ( + patch.object(charm.cluster_manager, "configure_system_passwords"), + patch.object(charm.config_manager, "update_config"), + ): + harness.set_leader(True) + charm.state.set_secret("app", "internal-ca-key", "ca-key-content") + charm.state.set_secret("app", "internal-ca", "ca-content") + + # spy on the only file-writing primitive to prove no push happens + mgr.workload.write_text = MagicMock() + with ( + patch( + "single_kernel_postgresql.managers.tls.generate_private_key", + return_value=sentinel.cert_key, + ), + patch( + "single_kernel_postgresql.managers.tls.generate_csr", + return_value=sentinel.cert_csr, + ), + patch( + "single_kernel_postgresql.managers.tls.generate_certificate", + return_value=sentinel.cert, + ) as _generate_certificate, + patch("single_kernel_postgresql.managers.tls.PrivateKey") as _private_key, + patch("single_kernel_postgresql.managers.tls.Certificate") as _certificate, + ): + _private_key.from_string.return_value = sentinel.ca_key + _certificate.from_string.return_value = sentinel.ca_cert + + mgr.generate_internal_peer_cert() + + # the CSR is signed by the stored internal CA (cert + key), valid 20 years + _generate_certificate.assert_called_once_with( + sentinel.cert_csr, sentinel.ca_cert, sentinel.ca_key, validity=timedelta(days=7300) + ) + + # generated material is persisted in peer-unit state (round-trip) + assert charm.state.peer.internal_cert == str(sentinel.cert) + assert charm.state.peer.internal_key == str(sentinel.cert_key) + # writing the internal peer cert/CA to disk is owned by the config subsystem + mgr.workload.write_text.assert_not_called() From c341a63e7508a68959c06c294e1dd7484a9796b3 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Fri, 10 Jul 2026 10:34:56 -0300 Subject: [PATCH 07/10] docs(tls): trim cross-repo refs and overclaim from operator-cert tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 4/4 test docstrings pointed at postgresql-operator/src/relations/tls.py file/line positions and a '16/edge secrets, kept' track label — both stale-prone the moment that repo refactors or the track shifts, with no test catching the drift. Replace with conceptual references to the pre-port charm's defer guards and the state-tracked rotation, which survive refactors; git blame recovers the exact position when needed. Also soften test_relation_broken_client_wired's docstring: it asserts only that removal doesn't crash and the live getter reports absent, not that the event routes to _on_certificate_available (the peer variant verifies routing via the CA side-effect; the client route has no state side-effect to diff on). No test logic changes; suite stays 56/56 green on both substrates. Signed-off-by: Marcelo Henrique Neppel --- tests/unit/test_tls_events.py | 20 ++++++++++++-------- tests/unit/test_tls_manager.py | 2 +- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/tests/unit/test_tls_events.py b/tests/unit/test_tls_events.py index c04742c..4bbbdba 100644 --- a/tests/unit/test_tls_events.py +++ b/tests/unit/test_tls_events.py @@ -4,8 +4,8 @@ The handler is live-fetch: operator cert/key are read from the requirer on demand (get_assigned_certificates), never persisted. Only the peer CA is tracked in state -(current-ca / old-ca) for rotation, mirroring postgresql-operator/src/relations/tls.py -_on_peer_certificate_available (which stashes current-ca/old-ca and otherwise reads live). +(current-ca / old-ca) for rotation, matching the pre-port charm's peer-cert handler +(which stashes current-ca/old-ca and otherwise reads live). """ from types import SimpleNamespace @@ -129,7 +129,12 @@ def test_peer_certificate_available_clears_ca_on_empty(harness): def test_relation_broken_client_wired(harness): - """relation_broken on TLS_CLIENT_RELATION routes to _on_certificate_available (live push).""" + """relation_broken on TLS_CLIENT_RELATION routes to the live-push path without crashing. + + Distinct from the peer variant (which retires the CA): the client route has no + state side-effect to assert, so this smoke-tests that removal doesn't crash and + the live getter reports absent cert material afterward. + """ charm = harness.charm client_rel_id = harness.add_relation("client-certificates", "tls-provider") charm.tls_manager.push_tls_files = MagicMock() @@ -210,8 +215,8 @@ def test_refresh_event_defined_and_wired(harness): def test_certificate_available_defers_when_internal_ca_absent(harness): """When internal-ca is not yet set, _on_certificate_available defers and skips push. - Mirrors postgresql-operator/src/relations/tls.py lines 157-161: the handler must - not attempt file writes before the CA is present (K8s Pebble may not be ready). + The handler must not attempt file writes before the CA is present (K8s Pebble + may not be ready), matching the pre-port charm's defer-before-CA-present guard. """ tls = harness.charm.tls tls.client_certificate.get_assigned_certificates = MagicMock( @@ -247,9 +252,8 @@ def test_peer_certificate_available_defers_when_internal_ca_absent(harness): def test_certificate_available_defers_on_workload_file_error(harness): """When push_tls_files raises PostgreSQLFileOperationError, the handler defers. - Mirrors postgresql-operator/src/relations/tls.py lines 162-170: workload - file-write failures (e.g. Pebble not yet ready on K8s) must defer rather - than crash the hook. + Workload file-write failures (e.g. Pebble not yet ready on K8s) must defer + rather than crash the hook, matching the pre-port charm's defer-on-write-failure guard. """ charm = harness.charm with ( diff --git a/tests/unit/test_tls_manager.py b/tests/unit/test_tls_manager.py index a0cea90..93a0c74 100644 --- a/tests/unit/test_tls_manager.py +++ b/tests/unit/test_tls_manager.py @@ -37,7 +37,7 @@ def _set_empty(requirer): requirer.get_assigned_certificates = MagicMock(return_value=([], None)) -# --- CA rotation (current-ca / old-ca are 16/edge secrets, kept; cert/key are NOT stored) --- +# --- CA rotation (current-ca / old-ca are tracked in state for rotation; cert/key are NOT stored) --- def test_rotate_peer_ca_sets_current(harness): From 7661898ff2434846f9858ec51100dbcde0c5e8b2 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Fri, 10 Jul 2026 11:49:20 -0300 Subject: [PATCH 08/10] test(tls): de-duplicate helpers, stub crypto, strengthen three weak tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lift the requirer-return-shape fakes (FakePrivateKey + fake_assigned) out of the two TLS test files into a shared tests/unit/tls_helpers.py, and add a patch_crypto conftest fixture that stubs generate_private_key/generate_ca with sentinels. The incidental set_leader paths were running real RSA keygen purely to populate internal-ca; the dedicated generate_* tests keep their own sentinels to assert call args, so this only touches paths that need the CA present, not its content. Suite runtime drops 4.2s -> 1.9s. Three tests sat below the patroni/config bar and are tightened, none tautological after the change (each mutation-confirmed to fail when the pinned behavior breaks): - test_tls_manager_wired_to_handler_requirers (was constructor_injects_requirers): drop the two asserts that only verified `self.x = x` plumbing; keep the real wiring asserts (charm injects the handler's requirers into the manager). - test_peer_relation_changed_emits_refresh (was refresh_event_defined_and_wired): drop the hasattr tautology + assertion-less relation_changed smoke; patch TLS.refresh_tls_certificates_event and assert emit() was called once. - test_relation_broken_client_routes_to_live_push (was relation_broken_client_wired): the client route has no CA side-effect, so the old test only proved it didn't crash; now set leader + seed a peer CA, remove the client relation, and assert the push was reached while peer CA rotation stayed untouched — proving the route is the client path, not the peer path. No source or other test changes; the two pre-existing test_patroni_manager.py daemon_reload failures are unrelated (charmlibs.systemd symbol not resolving in this env) and untouched here. Signed-off-by: Marcelo Henrique Neppel --- tests/unit/conftest.py | 24 ++++++++- tests/unit/test_tls_events.py | 90 +++++++++++++++------------------- tests/unit/test_tls_manager.py | 52 +++++--------------- tests/unit/tls_helpers.py | 21 ++++++++ 4 files changed, 96 insertions(+), 91 deletions(-) create mode 100644 tests/unit/tls_helpers.py diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 5b15250..2d519df 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -1,7 +1,7 @@ # Copyright 2026 Canonical Ltd. # See LICENSE file for licensing details. -from unittest.mock import patch +from unittest.mock import patch, sentinel import pytest from ops.testing import Harness @@ -9,6 +9,28 @@ from single_kernel_postgresql.config.literals import PEER_RELATION +@pytest.fixture +def patch_crypto(): + """Stub the tls-crypto generators so leader-elected paths skip real RSA keygen. + + set_leader() fires _on_leader_elected -> configure_internal_peer_ca, which would + otherwise run real generate_private_key/generate_ca. The dedicated generate_* tests + patch with their own sentinels to assert call args; this fixture is for the + incidental paths that only need internal-ca to be present, not its content. + """ + with ( + patch( + "single_kernel_postgresql.managers.tls.generate_private_key", + return_value=sentinel.ca_key, + ), + patch( + "single_kernel_postgresql.managers.tls.generate_ca", + return_value=sentinel.ca, + ), + ): + yield + + @pytest.fixture def harness(substrate, test_charm_path): """A begun Harness for the substrate's test charm, with the peer relation added.""" diff --git a/tests/unit/test_tls_events.py b/tests/unit/test_tls_events.py index 4bbbdba..773a93f 100644 --- a/tests/unit/test_tls_events.py +++ b/tests/unit/test_tls_events.py @@ -8,29 +8,10 @@ (which stashes current-ca/old-ca and otherwise reads live). """ -from types import SimpleNamespace from unittest.mock import MagicMock, patch from single_kernel_postgresql.config.exceptions import PostgreSQLFileOperationError - - -class _FakePrivateKey: - """Minimal stand-in for PrivateKey: str(key) returns the raw PEM string.""" - - def __init__(self, raw: str) -> None: - self._raw = raw - - def __str__(self) -> str: - return self._raw - - -def _fake_assigned(cert, ca, key): - """Mimic TLSCertificatesRequiresV4.get_assigned_certificates() return shape. - - Returns (list[ProviderCertificate], PrivateKey | None). - """ - provider_cert = SimpleNamespace(certificate=cert, ca=ca) - return [provider_cert], _FakePrivateKey(key) +from tls_helpers import fake_assigned def test_handler_is_wired(harness): @@ -42,7 +23,7 @@ def test_handler_is_wired(harness): assert harness.charm.tls_manager.peer_certificate is tls.peer_certificate -def test_client_certificate_available_pushes(harness): +def test_client_certificate_available_pushes(harness, patch_crypto): charm = harness.charm with ( patch.object(charm.cluster_manager, "configure_system_passwords"), @@ -52,7 +33,7 @@ def test_client_certificate_available_pushes(harness): tls = charm.tls tls.client_certificate.get_assigned_certificates = MagicMock( - return_value=_fake_assigned("CC", "CA", "CK") + return_value=fake_assigned("CC", "CA", "CK") ) charm.tls_manager.push_tls_files = MagicMock() @@ -63,7 +44,7 @@ def test_client_certificate_available_pushes(harness): assert charm.tls_manager.get_client_tls_files() == ("CK", "CA", "CC") -def test_peer_certificate_available_rotates_ca_and_pushes(harness): +def test_peer_certificate_available_rotates_ca_and_pushes(harness, patch_crypto): charm = harness.charm with ( patch.object(charm.cluster_manager, "configure_system_passwords"), @@ -73,7 +54,7 @@ def test_peer_certificate_available_rotates_ca_and_pushes(harness): tls = charm.tls tls.peer_certificate.get_assigned_certificates = MagicMock( - return_value=_fake_assigned("PC", "PCA", "PK") + return_value=fake_assigned("PC", "PCA", "PK") ) charm.tls_manager.push_tls_files = MagicMock() @@ -87,7 +68,7 @@ def test_peer_certificate_available_rotates_ca_and_pushes(harness): assert (key, cert) == ("PK", "PC") -def test_certificate_available_pushes_on_empty(harness): +def test_certificate_available_pushes_on_empty(harness, patch_crypto): charm = harness.charm with ( patch.object(charm.cluster_manager, "configure_system_passwords"), @@ -106,7 +87,7 @@ def test_certificate_available_pushes_on_empty(harness): charm.tls_manager.push_tls_files.assert_called_once() -def test_peer_certificate_available_clears_ca_on_empty(harness): +def test_peer_certificate_available_clears_ca_on_empty(harness, patch_crypto): charm = harness.charm with ( patch.object(charm.cluster_manager, "configure_system_passwords"), @@ -128,19 +109,31 @@ def test_peer_certificate_available_clears_ca_on_empty(harness): charm.tls_manager.push_tls_files.assert_called_once() -def test_relation_broken_client_wired(harness): - """relation_broken on TLS_CLIENT_RELATION routes to the live-push path without crashing. +def test_relation_broken_client_routes_to_live_push(harness, patch_crypto): + """relation_broken on TLS_CLIENT_RELATION routes to the live-push path. - Distinct from the peer variant (which retires the CA): the client route has no - state side-effect to assert, so this smoke-tests that removal doesn't crash and - the live getter reports absent cert material afterward. + Distinct from the peer variant (which retires the CA): the client route only pushes, + so routing is verified by the push being reached (internal-ca present via leadership) + while peer CA rotation is left untouched. """ charm = harness.charm - client_rel_id = harness.add_relation("client-certificates", "tls-provider") + # seed a peer CA so we can prove the client route does not touch peer rotation + charm.tls_manager.rotate_peer_ca("PCA") + with ( + patch.object(charm.cluster_manager, "configure_system_passwords"), + patch.object(charm.config_manager, "update_config"), + ): + harness.set_leader(True) # internal-ca present so the route reaches push charm.tls_manager.push_tls_files = MagicMock() + + client_rel_id = harness.add_relation("client-certificates", "tls-provider") harness.remove_relation(client_rel_id) - # With the relation gone, the live getter reports nothing. + # the client broken route reached the live push; peer CA rotation was not touched + charm.tls_manager.push_tls_files.assert_called_once() + assert charm.state.peer.current_ca == "PCA" + assert charm.state.peer.old_ca is None + # with the relation gone, the live getter reports nothing assert charm.tls_manager.get_client_tls_files() == (None, None, None) @@ -196,20 +189,15 @@ def test_client_and_peer_requesters_have_distinct_common_names(substrate, harnes assert tls.peer_certificate is not None -def test_refresh_event_defined_and_wired(harness): - """refresh_tls_certificates_event exists; peer relation_changed emits it without error.""" - tls = harness.charm.tls - assert hasattr(tls, "refresh_tls_certificates_event") +def test_peer_relation_changed_emits_refresh(harness): + """Peer relation_changed emits refresh_tls_certificates_event to re-request certs with updated SANs.""" + from single_kernel_postgresql.events.tls import TLS charm = harness.charm - peer_rel_id = harness.model.get_relation("database-peers").id - with ( - patch.object(charm.cluster_manager, "configure_system_passwords", return_value=None), - patch.object(charm.config_manager, "update_config", return_value=None), - ): - harness.update_relation_data( - peer_rel_id, charm.unit.name, {"database-address": "10.9.8.7"} - ) + with patch.object(TLS, "refresh_tls_certificates_event") as _refresh: + charm.tls._on_peer_relation_changed(MagicMock()) + + _refresh.emit.assert_called_once() def test_certificate_available_defers_when_internal_ca_absent(harness): @@ -220,7 +208,7 @@ def test_certificate_available_defers_when_internal_ca_absent(harness): """ tls = harness.charm.tls tls.client_certificate.get_assigned_certificates = MagicMock( - return_value=_fake_assigned("CC", "CA", "CK") + return_value=fake_assigned("CC", "CA", "CK") ) harness.charm.tls_manager.push_tls_files = MagicMock() @@ -237,7 +225,7 @@ def test_peer_certificate_available_defers_when_internal_ca_absent(harness): """When internal-ca is not yet set, _on_peer_certificate_available defers and skips push.""" tls = harness.charm.tls tls.peer_certificate.get_assigned_certificates = MagicMock( - return_value=_fake_assigned("PC", "PCA", "PK") + return_value=fake_assigned("PC", "PCA", "PK") ) harness.charm.tls_manager.push_tls_files = MagicMock() @@ -249,7 +237,7 @@ def test_peer_certificate_available_defers_when_internal_ca_absent(harness): harness.charm.tls_manager.push_tls_files.assert_not_called() -def test_certificate_available_defers_on_workload_file_error(harness): +def test_certificate_available_defers_on_workload_file_error(harness, patch_crypto): """When push_tls_files raises PostgreSQLFileOperationError, the handler defers. Workload file-write failures (e.g. Pebble not yet ready on K8s) must defer @@ -264,7 +252,7 @@ def test_certificate_available_defers_on_workload_file_error(harness): tls = charm.tls tls.client_certificate.get_assigned_certificates = MagicMock( - return_value=_fake_assigned("CC", "CA", "CK") + return_value=fake_assigned("CC", "CA", "CK") ) charm.tls_manager.push_tls_files = MagicMock( side_effect=PostgreSQLFileOperationError("disk full") @@ -276,7 +264,7 @@ def test_certificate_available_defers_on_workload_file_error(harness): event.defer.assert_called_once() -def test_peer_certificate_available_defers_on_workload_file_error(harness): +def test_peer_certificate_available_defers_on_workload_file_error(harness, patch_crypto): """When push_tls_files raises PostgreSQLFileOperationError, the peer handler defers.""" charm = harness.charm with ( @@ -287,7 +275,7 @@ def test_peer_certificate_available_defers_on_workload_file_error(harness): tls = charm.tls tls.peer_certificate.get_assigned_certificates = MagicMock( - return_value=_fake_assigned("PC", "PCA", "PK") + return_value=fake_assigned("PC", "PCA", "PK") ) charm.tls_manager.push_tls_files = MagicMock( side_effect=PostgreSQLFileOperationError("pebble not ready") diff --git a/tests/unit/test_tls_manager.py b/tests/unit/test_tls_manager.py index 93a0c74..08fa646 100644 --- a/tests/unit/test_tls_manager.py +++ b/tests/unit/test_tls_manager.py @@ -2,34 +2,20 @@ # See LICENSE file for licensing details. from datetime import timedelta -from types import SimpleNamespace from unittest.mock import MagicMock, patch, sentinel - -class _FakePrivateKey: - """Minimal stand-in for PrivateKey: str(key) returns the raw PEM string.""" - - def __init__(self, raw: str) -> None: - self._raw = raw - - def __str__(self) -> str: - return self._raw - - -def _fake_assigned(cert, ca, key): - """Mimic TLSCertificatesRequiresV4.get_assigned_certificates() -> (list, PrivateKey|None).""" - return [SimpleNamespace(certificate=cert, ca=ca)], _FakePrivateKey(key) +from tls_helpers import fake_assigned def _set_client(mgr, cert, ca, key): mgr.client_certificate.get_assigned_certificates = MagicMock( - return_value=_fake_assigned(cert, ca, key) + return_value=fake_assigned(cert, ca, key) ) def _set_peer(mgr, cert, ca, key): mgr.peer_certificate.get_assigned_certificates = MagicMock( - return_value=_fake_assigned(cert, ca, key) + return_value=fake_assigned(cert, ca, key) ) @@ -109,7 +95,7 @@ def test_get_client_tls_files_returns_live(harness): assert mgr.get_client_tls_files() == ("K", "A", "C") -def test_get_peer_ca_bundle_composes_live_old_internal(harness): +def test_get_peer_ca_bundle_composes_live_old_internal(harness, patch_crypto): charm = harness.charm # leadership is required to write the app-scoped internal-ca secret with ( @@ -134,7 +120,7 @@ def test_get_peer_tls_files_prefers_live_operator(harness): assert ca == "PCA" # bundle = live operator CA only (no old, no internal here) -def test_get_peer_tls_files_falls_back_to_internal(harness): +def test_get_peer_tls_files_falls_back_to_internal(harness, patch_crypto): charm = harness.charm # leadership is required to write the app-scoped internal-ca secret with ( @@ -183,27 +169,15 @@ def test_push_tls_files_writes_expected_files(harness): assert call.args[1].parent == mgr.workload.paths.tls -def test_tls_manager_constructor_injects_requirers(harness): - """TLSManager takes the two cert requirers at construction (no post-init mutation). +def test_tls_manager_wired_to_handler_requirers(harness): + """The charm wires the TLS handler's two requirers into TLSManager at construction. - The manager holds no PostgreSQL client (it never uses one); it reads operator - cert/key live from the constructor-injected requirers. + The handler builds the requirers; the charm constructor-injects them into the manager + (no post-init mutation), so the manager's live-fetch getters read the same objects. """ - from single_kernel_postgresql.managers.tls import TLSManager - - client_req = harness.charm.tls.client_certificate - peer_req = harness.charm.tls.peer_certificate - mgr = TLSManager( - harness.charm.state, - harness.charm.tls_manager.workload, - client_certificate=client_req, - peer_certificate=peer_req, - ) - assert mgr.client_certificate is client_req - assert mgr.peer_certificate is peer_req - # the charm-wired manager gets the same requirers the TLS handler built - assert harness.charm.tls_manager.client_certificate is harness.charm.tls.client_certificate - assert harness.charm.tls_manager.peer_certificate is harness.charm.tls.peer_certificate + charm = harness.charm + assert charm.tls_manager.client_certificate is charm.tls.client_certificate + assert charm.tls_manager.peer_certificate is charm.tls.peer_certificate def test_client_tls_files_on_disk(harness): @@ -246,7 +220,7 @@ def test_generate_internal_peer_ca_stores_secrets(harness): assert charm.state.application.internal_ca == str(sentinel.ca) -def test_generate_internal_peer_cert_stores_material(harness): +def test_generate_internal_peer_cert_stores_material(harness, patch_crypto): """generate_internal_peer_cert persists internal key/cert into peer state and pushes nothing. Writing the internal peer cert/CA to disk is owned by the (not-yet-migrated) diff --git a/tests/unit/tls_helpers.py b/tests/unit/tls_helpers.py new file mode 100644 index 0000000..dd146c5 --- /dev/null +++ b/tests/unit/tls_helpers.py @@ -0,0 +1,21 @@ +# Copyright 2026 Canonical Ltd. +# See LICENSE file for licensing details. +"""Shared fakes for the TLS unit tests (the requirer return shape).""" + +from types import SimpleNamespace + + +class FakePrivateKey: + """Minimal stand-in for PrivateKey: str(key) returns the raw PEM string.""" + + def __init__(self, raw: str) -> None: + self._raw = raw + + def __str__(self) -> str: + """Return the raw PEM string.""" + return self._raw + + +def fake_assigned(cert, ca, key): + """Mimic TLSCertificatesRequiresV4.get_assigned_certificates() -> (list, PrivateKey|None).""" + return [SimpleNamespace(certificate=cert, ca=ca)], FakePrivateKey(key) From 7a9b62348ad93f7ff0ccdafe56dc73d9c4fb33e7 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Fri, 10 Jul 2026 12:02:45 -0300 Subject: [PATCH 09/10] test(tls): drop redundant wiring test, cover the TlsError guards MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous round left test_tls_manager_wired_to_handler_requirers asserting only `manager.{client,peer}_certificate is tls.{client,peer}_certificate` — the exact two checks test_handler_is_wired in the events file already makes (plus the not-None construction asserts). The charm-wiring concern has one home (the events file, since wiring is a charm __init__ concern); the manager-side copy added nothing. Delete it. generate_internal_peer_cert had two raise TlsError guards — "No CA key content" and "No CA cert content" — exercised by no test (the stores_material test seeds both secrets). Add pytest.raises coverage for each: the CA-key guard raises before any crypto runs (no leader, no secrets); the CA-cert guard runs after PrivateKey.from_string, so it stubs that and drops only the CA cert. Both mutation-confirmed (removing the guard makes each test fail with DID-NOT-RAISE). Also give test_generate_internal_peer_ca_stores_secrets the patch_crypto fixture and drop its now-redundant inner generate_* patches (patch_crypto stubs the same symbols with the same sentinels for both the set_leader side-effect and the explicit call) — it had been running real RSA during set_leader, then overwriting it with sentinels. No source changes; suite is 58 TLS tests on both substrates. Signed-off-by: Marcelo Henrique Neppel --- tests/unit/test_tls_manager.py | 58 ++++++++++++++++++++-------------- 1 file changed, 35 insertions(+), 23 deletions(-) diff --git a/tests/unit/test_tls_manager.py b/tests/unit/test_tls_manager.py index 08fa646..bd621a0 100644 --- a/tests/unit/test_tls_manager.py +++ b/tests/unit/test_tls_manager.py @@ -4,6 +4,8 @@ from datetime import timedelta from unittest.mock import MagicMock, patch, sentinel +import pytest +from single_kernel_postgresql.config.exceptions import TlsError from tls_helpers import fake_assigned @@ -169,17 +171,6 @@ def test_push_tls_files_writes_expected_files(harness): assert call.args[1].parent == mgr.workload.paths.tls -def test_tls_manager_wired_to_handler_requirers(harness): - """The charm wires the TLS handler's two requirers into TLSManager at construction. - - The handler builds the requirers; the charm constructor-injects them into the manager - (no post-init mutation), so the manager's live-fetch getters read the same objects. - """ - charm = harness.charm - assert charm.tls_manager.client_certificate is charm.tls.client_certificate - assert charm.tls_manager.peer_certificate is charm.tls.peer_certificate - - def test_client_tls_files_on_disk(harness): """Reports presence of the client key/cert/ca files; container errors count as absent.""" from single_kernel_postgresql.config.exceptions import PostgreSQLFileOperationError @@ -194,7 +185,7 @@ def test_client_tls_files_on_disk(harness): assert mgr.client_tls_files_on_disk() is False -def test_generate_internal_peer_ca_stores_secrets(harness): +def test_generate_internal_peer_ca_stores_secrets(harness, patch_crypto): """generate_internal_peer_ca persists the generated CA cert/key into app state.""" charm = harness.charm # leadership is required to write the app-scoped internal-ca secrets @@ -204,17 +195,9 @@ def test_generate_internal_peer_ca_stores_secrets(harness): ): harness.set_leader(True) - with ( - patch( - "single_kernel_postgresql.managers.tls.generate_private_key", - return_value=sentinel.ca_key, - ), - patch( - "single_kernel_postgresql.managers.tls.generate_ca", - return_value=sentinel.ca, - ), - ): - charm.tls_manager.generate_internal_peer_ca() + # patch_crypto stubs the generators for both the set_leader side-effect and this + # explicit call, so no real RSA runs; re-generate to assert the stored values. + charm.tls_manager.generate_internal_peer_ca() assert charm.state.application.internal_ca_key == str(sentinel.ca_key) assert charm.state.application.internal_ca == str(sentinel.ca) @@ -271,3 +254,32 @@ def test_generate_internal_peer_cert_stores_material(harness, patch_crypto): assert charm.state.peer.internal_key == str(sentinel.cert_key) # writing the internal peer cert/CA to disk is owned by the config subsystem mgr.workload.write_text.assert_not_called() + + +def test_generate_internal_peer_cert_raises_without_ca_key(harness): + """Without an internal CA key, generate_internal_peer_cert raises before any crypto runs.""" + mgr = harness.charm.tls_manager + # no leader has set the internal-ca secrets, so the CA key is absent + assert mgr.state.application.internal_ca_key is None + with pytest.raises(TlsError, match="No CA key content"): + mgr.generate_internal_peer_cert() + + +def test_generate_internal_peer_cert_raises_without_ca_cert(harness, patch_crypto): + """With a CA key but no CA cert, generate_internal_peer_cert raises after loading the key.""" + charm = harness.charm + mgr = charm.tls_manager + with ( + patch.object(charm.cluster_manager, "configure_system_passwords"), + patch.object(charm.config_manager, "update_config"), + ): + harness.set_leader(True) # populates internal-ca-key (and internal-ca, dropped next) + charm.state.remove_secret("app", "internal-ca") + assert mgr.state.application.internal_ca_key is not None + assert mgr.state.application.internal_ca is None + + # PrivateKey.from_string runs on the stored key before the CA-cert guard, so stub it + with patch("single_kernel_postgresql.managers.tls.PrivateKey") as _private_key: + _private_key.from_string.return_value = sentinel.ca_key + with pytest.raises(TlsError, match="No CA cert content"): + mgr.generate_internal_peer_cert() From 7742b993bb74b997bc17b9565d1cbae02a73a6d0 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Fri, 10 Jul 2026 12:12:23 -0300 Subject: [PATCH 10/10] test(tls): drop five tests that re-prove already-covered behavior Each dropped test was a second layer over a code path another test already pins; keeping both is the two-layers-that-sort-of-do smell. None of the five guarded a behavior the surviving suite doesn't. - test_rotate_peer_ca_sets_current: first-set + old-ca-None. Covered by no_rotation_when_unchanged's first call (sets current, leaves old None) and the else-branch remove_secret in clears_stale_old_ca_on_reenable. The rotation state machine stays fully covered (rotate, no-op, clear, re-enable-clear). - test_get_client_tls_files_none_when_absent: the absent getter path. Covered by test_certificate_available_pushes_on_empty, which asserts get_client_tls_files == (None, None, None) on an empty requirer. - test_client_and_peer_requesters_have_distinct_common_names: the CN values live in test_tls_state.py (three tests); requester construction is in test_handler_is_wired. - test_peer_certificate_available_defers_when_internal_ca_absent and test_peer_certificate_available_defers_on_workload_file_error: the defer guards live in the shared _push_tls_files. The client variant proves each guard; the peer handler's routing through _push_tls_files is proven by test_peer_certificate_available_rotates_ca_and_pushes. The two surviving defer tests now note they cover the peer path too (shared guard). Suite: 58 -> 48 TLS tests on both substrates. Signed-off-by: Marcelo Henrique Neppel --- tests/unit/test_tls_events.py | 79 ++-------------------------------- tests/unit/test_tls_manager.py | 15 ------- 2 files changed, 3 insertions(+), 91 deletions(-) diff --git a/tests/unit/test_tls_events.py b/tests/unit/test_tls_events.py index 773a93f..54596c0 100644 --- a/tests/unit/test_tls_events.py +++ b/tests/unit/test_tls_events.py @@ -152,43 +152,6 @@ def test_relation_broken_peer_wired(harness): assert charm.state.peer.old_ca == "PCA" -def _set_unit_db(harness, key, value): - rel_id = harness.model.get_relation("database-peers").id - harness.update_relation_data(rel_id, harness.charm.unit.name, {key: value}) - - -def test_client_and_peer_requesters_have_distinct_common_names(substrate, harness): - """Client and peer requesters use distinct CNs drawn from different databag keys. - - certificate_requests are baked at init time (before the test updates the databag), so - we verify distinctness via the live state properties (which read directly from the - databag) and confirm both requester objects were constructed. - - On VM the CNs are host-derived (database-address / database-peers-address). On K8s - both CNs collapse to the endpoints FQDN (original charm parity) — still distinct from - each other is not required there, only that both are the endpoints FQDN. - """ - _set_unit_db(harness, "database-address", "10.1.2.3") - _set_unit_db(harness, "database-peers-address", "10.4.5.6") - - state = harness.charm.state - if substrate == "vm": - # VM: client CN reads database-address, peer CN reads database-peers-address. - assert state.client_common_name == "10.1.2.3" - assert state.peer_common_name == "10.4.5.6" - assert state.client_common_name != state.peer_common_name - else: - # K8s: both CNs are the unit endpoints FQDN (operator-cert parity). - app = state.model.app.name - expected = f"{app}-0.{app}-endpoints" - assert state.client_common_name == expected - assert state.peer_common_name == expected - - tls = harness.charm.tls - assert tls.client_certificate is not None - assert tls.peer_certificate is not None - - def test_peer_relation_changed_emits_refresh(harness): """Peer relation_changed emits refresh_tls_certificates_event to re-request certs with updated SANs.""" from single_kernel_postgresql.events.tls import TLS @@ -205,6 +168,8 @@ def test_certificate_available_defers_when_internal_ca_absent(harness): The handler must not attempt file writes before the CA is present (K8s Pebble may not be ready), matching the pre-port charm's defer-before-CA-present guard. + The defer guard lives in the shared _push_tls_files, so this also covers the peer + handler's path (both route through it). """ tls = harness.charm.tls tls.client_certificate.get_assigned_certificates = MagicMock( @@ -221,27 +186,12 @@ def test_certificate_available_defers_when_internal_ca_absent(harness): harness.charm.tls_manager.push_tls_files.assert_not_called() -def test_peer_certificate_available_defers_when_internal_ca_absent(harness): - """When internal-ca is not yet set, _on_peer_certificate_available defers and skips push.""" - tls = harness.charm.tls - tls.peer_certificate.get_assigned_certificates = MagicMock( - return_value=fake_assigned("PC", "PCA", "PK") - ) - harness.charm.tls_manager.push_tls_files = MagicMock() - - event = MagicMock() - assert harness.charm.state.application.internal_ca is None - tls._on_peer_certificate_available(event) - - event.defer.assert_called_once() - harness.charm.tls_manager.push_tls_files.assert_not_called() - - def test_certificate_available_defers_on_workload_file_error(harness, patch_crypto): """When push_tls_files raises PostgreSQLFileOperationError, the handler defers. Workload file-write failures (e.g. Pebble not yet ready on K8s) must defer rather than crash the hook, matching the pre-port charm's defer-on-write-failure guard. + The guard lives in the shared _push_tls_files, so this also covers the peer handler's path. """ charm = harness.charm with ( @@ -262,26 +212,3 @@ def test_certificate_available_defers_on_workload_file_error(harness, patch_cryp tls._on_certificate_available(event) event.defer.assert_called_once() - - -def test_peer_certificate_available_defers_on_workload_file_error(harness, patch_crypto): - """When push_tls_files raises PostgreSQLFileOperationError, the peer handler defers.""" - charm = harness.charm - with ( - patch.object(charm.cluster_manager, "configure_system_passwords"), - patch.object(charm.config_manager, "update_config"), - ): - harness.set_leader(True) - - tls = charm.tls - tls.peer_certificate.get_assigned_certificates = MagicMock( - return_value=fake_assigned("PC", "PCA", "PK") - ) - charm.tls_manager.push_tls_files = MagicMock( - side_effect=PostgreSQLFileOperationError("pebble not ready") - ) - - event = MagicMock() - tls._on_peer_certificate_available(event) - - event.defer.assert_called_once() diff --git a/tests/unit/test_tls_manager.py b/tests/unit/test_tls_manager.py index bd621a0..4b3ee65 100644 --- a/tests/unit/test_tls_manager.py +++ b/tests/unit/test_tls_manager.py @@ -28,15 +28,6 @@ def _set_empty(requirer): # --- CA rotation (current-ca / old-ca are tracked in state for rotation; cert/key are NOT stored) --- -def test_rotate_peer_ca_sets_current(harness): - mgr = harness.charm.tls_manager - mgr.rotate_peer_ca("CA1") - - peer = harness.charm.state.peer - assert peer.current_ca == "CA1" - assert peer.old_ca is None - - def test_rotate_peer_ca_rotates_on_change(harness): mgr = harness.charm.tls_manager mgr.rotate_peer_ca("CA1") @@ -85,12 +76,6 @@ def test_rotate_peer_ca_clears_stale_old_ca_on_reenable(harness): # --- operator cert/key are fetched LIVE from the requirer, never persisted --- -def test_get_client_tls_files_none_when_absent(harness): - mgr = harness.charm.tls_manager - _set_empty(mgr.client_certificate) - assert mgr.get_client_tls_files() == (None, None, None) - - def test_get_client_tls_files_returns_live(harness): mgr = harness.charm.tls_manager _set_client(mgr, cert="C", ca="A", key="K")