Commit 6c94eeb
authored
[DPE-10609] Unit tests for the operator-cert manager + handler (4/4) (#175)
* 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 <marcelo.neppel@canonical.com>
* 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 <marcelo.neppel@canonical.com>
* 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>
* 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 <marcelo.neppel@canonical.com>
* 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 <marcelo.neppel@canonical.com>
* docs(tls): trim cross-repo refs and overclaim from operator-cert tests
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>
* test(tls): de-duplicate helpers, stub crypto, strengthen three weak tests
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>
* test(tls): drop redundant wiring test, cover the TlsError guards
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>
* 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 <marcelo.neppel@canonical.com>
---------
Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>1 parent 834c90e commit 6c94eeb
4 files changed
Lines changed: 528 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | 3 | | |
4 | | - | |
| 4 | + | |
5 | 5 | | |
6 | 6 | | |
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
11 | 11 | | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
12 | 34 | | |
13 | 35 | | |
14 | 36 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
0 commit comments