From 833942db2f97836753261d6222a0d26e7d53223d Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Wed, 1 Jul 2026 15:27:12 -0300 Subject: [PATCH 1/2] 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 2/2] 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: