Skip to content

Commit 0b4577e

Browse files
[DPE-10607] Substrate-aware CharmState SAN/CN accessors (2/4) (#177)
* 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 <marcelo.neppel@canonical.com> * 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 <marcelo.neppel@canonical.com> --------- Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
1 parent f5bfe4d commit 0b4577e

3 files changed

Lines changed: 241 additions & 6 deletions

File tree

single_kernel_postgresql/core/state.py

Lines changed: 53 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,12 @@
88
import socket
99
from contextlib import suppress
1010
from functools import cached_property
11-
from typing import TYPE_CHECKING, Any, get_args
11+
from typing import Any, get_args
1212

1313
from data_platform_helpers.advanced_statuses import StatusesState, StatusObject
1414
from data_platform_helpers.advanced_statuses.types import Scope as AdvancedStatusesScope
1515
from ops import (
16+
CharmBase,
1617
ConfigData,
1718
JujuVersion,
1819
ModelError,
@@ -43,16 +44,13 @@
4344
from single_kernel_postgresql.utils.secret import translate_field_to_secret_key
4445
from single_kernel_postgresql.utils.status import format_status
4546

46-
if TYPE_CHECKING:
47-
from single_kernel_postgresql.charms.abstract_charm import AbstractPostgreSQLCharm
48-
4947

5048
class CharmState(Object):
5149
"""The global PostgreSQL Charm State."""
5250

5351
def __init__(
5452
self,
55-
charm: "AbstractPostgreSQLCharm",
53+
charm: CharmBase,
5654
substrate: Substrates,
5755
) -> None:
5856
"""Initialize the CharmState object."""
@@ -245,13 +243,62 @@ def host(self) -> str:
245243
@property
246244
def common_hosts(self) -> set[str]:
247245
"""Common hosts to be used in TLS certificate SANs."""
248-
return {self.host, self.fqdn} if self.fqdn else {self.host}
246+
hosts = {self.host, self.fqdn} if self.fqdn else {self.host}
247+
if self.substrate == Substrates.K8S:
248+
namespace = self.model.name
249+
hosts |= {
250+
f"{self.model.app.name}-primary.{namespace}.svc.cluster.local",
251+
f"{self.model.app.name}-replicas.{namespace}.svc.cluster.local",
252+
# the original K8s charm also included the resolved per-pod FQDN.
253+
socket.getfqdn(),
254+
}
255+
return hosts
249256

250257
@property
251258
def peer_common_name(self) -> str:
252259
"""Return the common name for the internally generated peer certificate."""
260+
if self.substrate == Substrates.K8S:
261+
return self._k8s_cert_common_name
253262
return self.peer.database_peers_address or self.host
254263

264+
@property
265+
def client_addresses(self) -> set[str]:
266+
"""Client-facing addresses for the operator client certificate SANs."""
267+
addrs: set[str] = set()
268+
if addr := self.peer.database_address:
269+
addrs.add(addr)
270+
return addrs
271+
272+
@property
273+
def client_common_name(self) -> str:
274+
"""Common name for the operator client certificate."""
275+
if self.substrate == Substrates.K8S:
276+
return self._k8s_cert_common_name
277+
return self.peer.database_address or self.host
278+
279+
@property
280+
def _k8s_cert_common_name(self) -> str:
281+
"""K8s operator-cert CN: the unit endpoints FQDN, wildcarded if too long.
282+
283+
Matches the original K8s charm: ``<app>-<unit_id>.<app>-endpoints``, collapsing to
284+
``*.<app>-endpoints`` when the full FQDN exceeds the 64-char CN limit.
285+
"""
286+
full = self._get_hostname_from_unit(unit_name_to_pod_name(self.model.unit.name))
287+
if len(full) > 64:
288+
return f"*.{self.model.app.name}-endpoints"
289+
return full
290+
291+
@property
292+
def peer_addresses(self) -> set[str]:
293+
"""Peer addresses for the operator peer certificate SANs (substrate-aware).
294+
295+
K8s excludes the ``ip`` databag key (original K8s charm never added an ``ip`` SAN);
296+
VM keeps it (unchanged from pre-migration behavior).
297+
"""
298+
if self.substrate == Substrates.K8S:
299+
return self.peer.peer_addresses_no_ip
300+
return self.peer.peer_addresses
301+
255302
@property
256303
def listen_ips(self) -> list[str]:
257304
"""Return the IPs to listen on.
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
# Copyright 2026 Canonical Ltd.
2+
# See LICENSE file for licensing details.
3+
4+
"""Tests for TLS client address and common-name state accessors."""
5+
6+
7+
def _set_unit_db(harness, key, value):
8+
rel_id = harness.model.get_relation("database-peers").id
9+
harness.update_relation_data(rel_id, harness.charm.unit.name, {key: value})
10+
11+
12+
def test_database_address_reads_unit_databag(harness):
13+
_set_unit_db(harness, "database-address", "10.1.2.3")
14+
assert harness.charm.state.peer.database_address == "10.1.2.3"
15+
16+
17+
def test_database_address_none_when_unset(harness):
18+
assert harness.charm.state.peer.database_address is None
19+
20+
21+
def test_client_addresses_set(harness):
22+
_set_unit_db(harness, "database-address", "10.1.2.3")
23+
assert harness.charm.state.client_addresses == {"10.1.2.3"}
24+
25+
26+
def test_client_addresses_empty_when_unset(harness):
27+
assert harness.charm.state.client_addresses == set()
28+
29+
30+
def test_client_common_name_prefers_database_address(substrate, harness):
31+
_set_unit_db(harness, "database-address", "10.1.2.3")
32+
state = harness.charm.state
33+
if substrate == "vm":
34+
# VM: CN follows the database-address databag value.
35+
assert state.client_common_name == "10.1.2.3"
36+
else:
37+
# K8s: CN is the endpoints FQDN, not the databag address.
38+
app = state.model.app.name
39+
assert state.client_common_name == f"{app}-0.{app}-endpoints"
40+
41+
42+
def test_client_common_name_falls_back_to_host(substrate, harness):
43+
state = harness.charm.state
44+
if substrate == "vm":
45+
# VM: falls back to host when no databag address is set.
46+
assert state.client_common_name == state.host
47+
else:
48+
# K8s: no fallback — always the endpoints FQDN.
49+
app = state.model.app.name
50+
assert state.client_common_name == f"{app}-0.{app}-endpoints"

tests/unit/test_tls_state.py

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
# Copyright 2026 Canonical Ltd.
2+
# See LICENSE file for licensing details.
3+
4+
"""Tests for TLS state accessors on PostgreSQLPeer.
5+
6+
Operator cert/key are fetched live from the requirer and are NOT held in peer
7+
state; only the peer CA rotation slots (current-ca / old-ca) and the internal
8+
peer material live here.
9+
"""
10+
11+
import socket
12+
from unittest.mock import patch
13+
14+
from single_kernel_postgresql.config.enums import Substrates # noqa: F401 (substrate fixture)
15+
16+
17+
def _set_unit_db(harness, key, value):
18+
rel_id = harness.model.get_relation("database-peers").id
19+
harness.update_relation_data(rel_id, harness.charm.unit.name, {key: value})
20+
21+
22+
def test_common_hosts_k8s_includes_service_endpoints(substrate, harness):
23+
"""On K8s the cert-SAN source must carry the primary/replicas Service DNS."""
24+
state = harness.charm.state
25+
app = state.model.app.name
26+
namespace = state.model.name
27+
hosts = state.common_hosts
28+
29+
if substrate == "k8s":
30+
assert f"{app}-primary.{namespace}.svc.cluster.local" in hosts
31+
assert f"{app}-replicas.{namespace}.svc.cluster.local" in hosts
32+
# host/fqdn still present
33+
assert state.host in hosts
34+
assert state.fqdn in hosts
35+
# the resolved per-pod FQDN is also included (parity with the original charm)
36+
assert socket.getfqdn() in hosts
37+
else:
38+
# VM: only host/fqdn — no k8s service DNS leaks in
39+
assert not any(h.endswith(".svc.cluster.local") for h in hosts)
40+
assert hosts == {state.host, state.fqdn}
41+
42+
43+
def test_ca_rotation_slots_roundtrip(harness):
44+
peer = harness.charm.state.peer
45+
peer.current_ca = "CA-1"
46+
peer.old_ca = "CA-0"
47+
48+
assert peer.current_ca == "CA-1"
49+
assert peer.old_ca == "CA-0"
50+
51+
52+
def test_unset_ca_slots_are_none(harness):
53+
peer = harness.charm.state.peer
54+
assert peer.current_ca is None
55+
assert peer.old_ca is None
56+
57+
58+
# -- G1: substrate-aware cert common name (parity with the original charm) ----
59+
60+
61+
def test_cert_common_name_is_endpoints_fqdn_on_k8s(substrate, harness):
62+
"""K8s operator-cert CN must be `<app>-<unit_id>.<app>-endpoints` (original charm parity)."""
63+
state = harness.charm.state
64+
app = state.model.app.name
65+
expected = f"{app}-0.{app}-endpoints"
66+
67+
if substrate == "k8s":
68+
assert state.client_common_name == expected
69+
assert state.peer_common_name == expected
70+
else:
71+
# VM unchanged: host-derived, not the endpoints FQDN.
72+
assert state.client_common_name != expected
73+
assert state.peer_common_name != expected
74+
75+
76+
def test_cert_common_name_wildcards_when_too_long_on_k8s(substrate, harness):
77+
"""K8s CN collapses to `*.<app>-endpoints` when the endpoints FQDN exceeds 64 chars."""
78+
if substrate != "k8s":
79+
return # wildcard rule is K8s-only
80+
81+
state = harness.charm.state
82+
app = state.model.app.name
83+
# Force the endpoints FQDN past the 64-char CN limit; the suffix stays app-derived.
84+
long_fqdn = "x" * 80
85+
with patch.object(state, "_get_hostname_from_unit", return_value=long_fqdn):
86+
assert state._k8s_cert_common_name == f"*.{app}-endpoints"
87+
88+
89+
def test_cert_common_name_vm_unchanged(substrate, harness):
90+
"""VM CN stays host-derived: client reads database-address, peer reads database-peers-address."""
91+
if substrate != "vm":
92+
return
93+
94+
_set_unit_db(harness, "database-address", "10.1.2.3")
95+
_set_unit_db(harness, "database-peers-address", "10.4.5.6")
96+
state = harness.charm.state
97+
assert state.client_common_name == "10.1.2.3"
98+
assert state.peer_common_name == "10.4.5.6"
99+
100+
101+
# -- G2: substrate-aware peer_addresses (no `ip` SAN on K8s) -----------------
102+
103+
104+
def test_peer_addresses_excludes_ip_on_k8s(substrate, harness):
105+
"""K8s peer SAN set must omit the `ip` databag key (original K8s charm never added it)."""
106+
_set_unit_db(harness, "ip", "10.0.0.1")
107+
_set_unit_db(harness, "private-address", "10.0.0.2")
108+
_set_unit_db(harness, "database-peers-address", "10.0.0.3")
109+
110+
addrs = harness.charm.state.peer_addresses
111+
112+
if substrate == "k8s":
113+
assert "10.0.0.1" not in addrs # `ip` excluded
114+
assert "10.0.0.2" in addrs # private-address kept
115+
assert "10.0.0.3" in addrs # database-peers-address kept
116+
else:
117+
assert "10.0.0.1" in addrs # VM keeps `ip`
118+
119+
120+
def test_peer_addresses_includes_ip_on_vm(substrate, harness):
121+
"""VM peer SAN set keeps `ip` (unchanged from pre-migration behavior)."""
122+
if substrate != "vm":
123+
return
124+
125+
_set_unit_db(harness, "ip", "10.0.0.1")
126+
addrs = harness.charm.state.peer_addresses
127+
assert "10.0.0.1" in addrs # `ip` retained on VM (the G2 regression was K8s-only)
128+
129+
130+
def test_charmstate_accepts_plain_charmbase():
131+
"""CharmState must accept any ops.CharmBase, not only AbstractPostgreSQLCharm."""
132+
import inspect
133+
134+
import ops
135+
from single_kernel_postgresql.core.state import CharmState
136+
137+
ann = inspect.signature(CharmState.__init__).parameters["charm"].annotation
138+
assert ann is ops.CharmBase

0 commit comments

Comments
 (0)