Skip to content

[DPE-10609] Unit tests for the operator-cert manager + handler (4/4)#175

Merged
marceloneppel merged 11 commits into
16/edgefrom
tls-4-tests
Jul 11, 2026
Merged

[DPE-10609] Unit tests for the operator-cert manager + handler (4/4)#175
marceloneppel merged 11 commits into
16/edgefrom
tls-4-tests

Conversation

@marceloneppel

@marceloneppel marceloneppel commented Jul 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Issue

Part 4/4 of the operator-certificate TLS stack. Parts 1–3 landed the TLS state primitives, the substrate-aware SAN/CN accessors, and the TLSManager + TLS events handler + charm wiring. This PR adds the unit tests for that manager and handler.

Solution

24 unit-test functions (48 with the VM/K8s substrate parametrization), covering only behaviors not already pinned elsewhere:

  • tests/unit/test_tls_manager.py: the CA-rotation state machine (rotate_peer_ca / clear_peer_ca, including the stale-old-ca clear on re-enable), the live-fetch getters and internal-material fallback, the CA-bundle composition, push_tls_files file/mode/ownership mapping, client_tls_files_on_disk, internal CA/cert generation, and the two TlsError guards on generate_internal_peer_cert.
  • tests/unit/test_tls_events.py: the TLS handler — certificate_available / relation_broken routing (client vs peer), CA rotation on peer-cert, the refresh_tls_certificates_event emit on peer relation_changed, and the two shared _push_tls_files defer guards (internal-ca absent, workload write error).
  • tests/unit/tls_helpers.py: shared FakePrivateKey / fake_assigned requirer-return-shape fakes.
  • tests/unit/conftest.py: a patch_crypto fixture that stubs generate_private_key/generate_ca with sentinels for the incidental set_leader paths (real RSA removed); the dedicated generate tests keep their own sentinels to assert call args.

Checklist

  • I have added or updated any relevant documentation.
  • I have cleaned any remaining cloud resources from my accounts.

This was referenced Jul 1, 2026
Closed
Closed
@marceloneppel
marceloneppel force-pushed the tls-3-manager-events branch from 90e7728 to ab4b0c8 Compare July 2, 2026 17:46
@marceloneppel
marceloneppel force-pushed the tls-3-manager-events branch from ab4b0c8 to d59f0ac Compare July 2, 2026 18:23
@marceloneppel
feat(tls): add TLS state and workload file primitives
833942d 
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 <marcelo.neppel@canonical.com>
@marceloneppel
fix(tls): keep K8s patroni_conf at the data dir (match #172)
6dbc899 
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 <marcelo.neppel@canonical.com>
@marceloneppel
feat(tls): add substrate-aware CharmState TLS SAN/CN accessors
c6aa083 
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>
@marceloneppel
marceloneppel force-pushed the tls-3-manager-events branch from d59f0ac to 1f84b9c Compare July 3, 2026 20:27
@marceloneppel
docs(tls): drop migration-history prose from _k8s_cert_common_name do…
65311e9 
…cstring

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>
@marceloneppel marceloneppel changed the title test(tls): unit tests for the operator-cert manager + handler (4/4) [DPE-10609] Unit tests for the operator-cert manager + handler (4/4) Jul 7, 2026
@marceloneppel
feat(tls): operator-cert manager, events handler, and wiring
f78c998 
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 <marcelo.neppel@canonical.com>
@marceloneppel
marceloneppel force-pushed the tls-3-manager-events branch from 1f84b9c to f78c998 Compare July 7, 2026 19:28
@marceloneppel
test(tls): unit tests for the operator-cert manager and events handler
a1d2178 
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 <marcelo.neppel@canonical.com>
Base automatically changed from tls-3-manager-events to 16/edge July 10, 2026 12:28
@marceloneppel
Merge 16/edge into tls-4-tests
83f4333 
Bring tls-4-tests up to date with 16/edge (now carries squashed 1/4 #165,
2/4 #177, 3/4 #178). Content-neutral: 16/edge's 1/4-3/4 content is already
present via the unsquashed commits, so the merge only adds ancestry.

Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
@marceloneppel
docs(tls): trim cross-repo refs and overclaim from operator-cert tests
c341a63 
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 <marcelo.neppel@canonical.com>
@marceloneppel
test(tls): de-duplicate helpers, stub crypto, strengthen three weak t…
7661898 
…ests

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 <marcelo.neppel@canonical.com>
@marceloneppel
test(tls): drop redundant wiring test, cover the TlsError guards
7a9b623 
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 <marcelo.neppel@canonical.com>
@marceloneppel
test(tls): drop five tests that re-prove already-covered behavior
7742b99 
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 <marcelo.neppel@canonical.com>
@marceloneppel
marceloneppel marked this pull request as ready for review July 10, 2026 16:02
@marceloneppel
marceloneppel requested a review from a team as a code owner July 10, 2026 16:02
@marceloneppel
marceloneppel requested review from carlcsaposs-canonical, dragomirp and taurus-forever and removed request for a team July 10, 2026 16:02
This was referenced Jul 11, 2026
Draft
Draft
@marceloneppel
marceloneppel merged commit 6c94eeb into 16/edge Jul 11, 2026
8 of 9 checks passed
@marceloneppel
marceloneppel deleted the tls-4-tests branch July 11, 2026 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@taurus-forever taurus-forever taurus-forever approved these changes
@dragomirp dragomirp dragomirp approved these changes
@carlcsaposs-canonical carlcsaposs-canonical Awaiting requested review from carlcsaposs-canonical carlcsaposs-canonical is a code owner automatically assigned from canonical/data-postgresql
@juju-charm-bot juju-charm-bot Awaiting requested review from juju-charm-bot juju-charm-bot is a code owner automatically assigned from canonical/data-postgresql

Assignees

No one assigned

Labels

not bug or enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@marceloneppel @taurus-forever @dragomirp