Skip to content

Commit 7742b99

Browse files
committed
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>
1 parent 7a9b623 commit 7742b99

2 files changed

Lines changed: 3 additions & 91 deletions

File tree

tests/unit/test_tls_events.py

Lines changed: 3 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -152,43 +152,6 @@ def test_relation_broken_peer_wired(harness):
152152
assert charm.state.peer.old_ca == "PCA"
153153

154154

155-
def _set_unit_db(harness, key, value):
156-
rel_id = harness.model.get_relation("database-peers").id
157-
harness.update_relation_data(rel_id, harness.charm.unit.name, {key: value})
158-
159-
160-
def test_client_and_peer_requesters_have_distinct_common_names(substrate, harness):
161-
"""Client and peer requesters use distinct CNs drawn from different databag keys.
162-
163-
certificate_requests are baked at init time (before the test updates the databag), so
164-
we verify distinctness via the live state properties (which read directly from the
165-
databag) and confirm both requester objects were constructed.
166-
167-
On VM the CNs are host-derived (database-address / database-peers-address). On K8s
168-
both CNs collapse to the endpoints FQDN (original charm parity) — still distinct from
169-
each other is not required there, only that both are the endpoints FQDN.
170-
"""
171-
_set_unit_db(harness, "database-address", "10.1.2.3")
172-
_set_unit_db(harness, "database-peers-address", "10.4.5.6")
173-
174-
state = harness.charm.state
175-
if substrate == "vm":
176-
# VM: client CN reads database-address, peer CN reads database-peers-address.
177-
assert state.client_common_name == "10.1.2.3"
178-
assert state.peer_common_name == "10.4.5.6"
179-
assert state.client_common_name != state.peer_common_name
180-
else:
181-
# K8s: both CNs are the unit endpoints FQDN (operator-cert parity).
182-
app = state.model.app.name
183-
expected = f"{app}-0.{app}-endpoints"
184-
assert state.client_common_name == expected
185-
assert state.peer_common_name == expected
186-
187-
tls = harness.charm.tls
188-
assert tls.client_certificate is not None
189-
assert tls.peer_certificate is not None
190-
191-
192155
def test_peer_relation_changed_emits_refresh(harness):
193156
"""Peer relation_changed emits refresh_tls_certificates_event to re-request certs with updated SANs."""
194157
from single_kernel_postgresql.events.tls import TLS
@@ -205,6 +168,8 @@ def test_certificate_available_defers_when_internal_ca_absent(harness):
205168
206169
The handler must not attempt file writes before the CA is present (K8s Pebble
207170
may not be ready), matching the pre-port charm's defer-before-CA-present guard.
171+
The defer guard lives in the shared _push_tls_files, so this also covers the peer
172+
handler's path (both route through it).
208173
"""
209174
tls = harness.charm.tls
210175
tls.client_certificate.get_assigned_certificates = MagicMock(
@@ -221,27 +186,12 @@ def test_certificate_available_defers_when_internal_ca_absent(harness):
221186
harness.charm.tls_manager.push_tls_files.assert_not_called()
222187

223188

224-
def test_peer_certificate_available_defers_when_internal_ca_absent(harness):
225-
"""When internal-ca is not yet set, _on_peer_certificate_available defers and skips push."""
226-
tls = harness.charm.tls
227-
tls.peer_certificate.get_assigned_certificates = MagicMock(
228-
return_value=fake_assigned("PC", "PCA", "PK")
229-
)
230-
harness.charm.tls_manager.push_tls_files = MagicMock()
231-
232-
event = MagicMock()
233-
assert harness.charm.state.application.internal_ca is None
234-
tls._on_peer_certificate_available(event)
235-
236-
event.defer.assert_called_once()
237-
harness.charm.tls_manager.push_tls_files.assert_not_called()
238-
239-
240189
def test_certificate_available_defers_on_workload_file_error(harness, patch_crypto):
241190
"""When push_tls_files raises PostgreSQLFileOperationError, the handler defers.
242191
243192
Workload file-write failures (e.g. Pebble not yet ready on K8s) must defer
244193
rather than crash the hook, matching the pre-port charm's defer-on-write-failure guard.
194+
The guard lives in the shared _push_tls_files, so this also covers the peer handler's path.
245195
"""
246196
charm = harness.charm
247197
with (
@@ -262,26 +212,3 @@ def test_certificate_available_defers_on_workload_file_error(harness, patch_cryp
262212
tls._on_certificate_available(event)
263213

264214
event.defer.assert_called_once()
265-
266-
267-
def test_peer_certificate_available_defers_on_workload_file_error(harness, patch_crypto):
268-
"""When push_tls_files raises PostgreSQLFileOperationError, the peer handler defers."""
269-
charm = harness.charm
270-
with (
271-
patch.object(charm.cluster_manager, "configure_system_passwords"),
272-
patch.object(charm.config_manager, "update_config"),
273-
):
274-
harness.set_leader(True)
275-
276-
tls = charm.tls
277-
tls.peer_certificate.get_assigned_certificates = MagicMock(
278-
return_value=fake_assigned("PC", "PCA", "PK")
279-
)
280-
charm.tls_manager.push_tls_files = MagicMock(
281-
side_effect=PostgreSQLFileOperationError("pebble not ready")
282-
)
283-
284-
event = MagicMock()
285-
tls._on_peer_certificate_available(event)
286-
287-
event.defer.assert_called_once()

tests/unit/test_tls_manager.py

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,6 @@ def _set_empty(requirer):
2828
# --- CA rotation (current-ca / old-ca are tracked in state for rotation; cert/key are NOT stored) ---
2929

3030

31-
def test_rotate_peer_ca_sets_current(harness):
32-
mgr = harness.charm.tls_manager
33-
mgr.rotate_peer_ca("CA1")
34-
35-
peer = harness.charm.state.peer
36-
assert peer.current_ca == "CA1"
37-
assert peer.old_ca is None
38-
39-
4031
def test_rotate_peer_ca_rotates_on_change(harness):
4132
mgr = harness.charm.tls_manager
4233
mgr.rotate_peer_ca("CA1")
@@ -85,12 +76,6 @@ def test_rotate_peer_ca_clears_stale_old_ca_on_reenable(harness):
8576
# --- operator cert/key are fetched LIVE from the requirer, never persisted ---
8677

8778

88-
def test_get_client_tls_files_none_when_absent(harness):
89-
mgr = harness.charm.tls_manager
90-
_set_empty(mgr.client_certificate)
91-
assert mgr.get_client_tls_files() == (None, None, None)
92-
93-
9479
def test_get_client_tls_files_returns_live(harness):
9580
mgr = harness.charm.tls_manager
9681
_set_client(mgr, cert="C", ca="A", key="K")

0 commit comments

Comments
 (0)