From 6ec4f2932e7e15a58aa828ba8b662f7e104e325f Mon Sep 17 00:00:00 2001 From: Lyncoln Date: Sun, 20 Sep 2026 00:56:25 -0400 Subject: [PATCH] Add rollback-safe NPM certificate create --- dns_sync/npm_client.py | 15 ++ dns_sync/plugins/adapters/npm.py | 140 ++++++++++- tests/test_npm_adapter.py | 416 ++++++++++++++++++++++++++++++- tests/test_npm_client.py | 13 + 4 files changed, 580 insertions(+), 4 deletions(-) diff --git a/dns_sync/npm_client.py b/dns_sync/npm_client.py index d2cf7fc..59defcc 100644 --- a/dns_sync/npm_client.py +++ b/dns_sync/npm_client.py @@ -372,6 +372,21 @@ class NpmClient: f"Unsupported NPM certificate provider: {provider!r}" ) + def delete_certificate( + self, + local_id: str | int, + ) -> None: + certificate_id = self._local_id( + local_id, + "certificate", + ) + + self.request( + "DELETE", + f"/nginx/certificates/{certificate_id}", + expected_status=200, + ) + def create_certificate( self, payload: dict[str, Any], diff --git a/dns_sync/plugins/adapters/npm.py b/dns_sync/plugins/adapters/npm.py index 4a81b57..2c65946 100644 --- a/dns_sync/plugins/adapters/npm.py +++ b/dns_sync/plugins/adapters/npm.py @@ -3,11 +3,14 @@ from typing import Any from ...adapters import ( CERTIFICATES, PROXY_HOSTS, + ApplyResult, Capability, CapabilityOperation, DiscoveredObject, NormalizedObject, ObjectReference, + PlanAction, + PlanOperation, ) from ...certificates.material import CertificateMaterial from ...npm_client import NpmClient @@ -312,9 +315,142 @@ class Plugin: return baseline - def apply(self, action): + def apply( + self, + action: PlanAction, + ) -> ApplyResult | None: + desired = action.desired + + if desired is None: + raise RuntimeError( + "NPM apply requires desired state" + ) + + if action.endpoint_id != self.endpoint.identity: + raise RuntimeError( + "NPM apply action endpoint does not match adapter" + ) + + if ( + desired.object_type == CERTIFICATES + and action.operation == PlanOperation.CREATE + ): + return self._create_certificate(desired) + raise RuntimeError( - f"{self.endpoint.identity} does not support apply yet" + f"{self.endpoint.identity} does not support " + f"{action.operation} for {desired.object_type}" + ) + + def _create_certificate( + self, + desired: NormalizedObject, + ) -> ApplyResult: + material = self._certificate_material_source( + desired + ) + + domains = desired.payload.get("domain_names") + + if ( + not isinstance(domains, list) + or not domains + or not all( + isinstance(value, str) and value + for value in domains + ) + ): + raise RuntimeError( + "NPM certificate create requires domain_names" + ) + + nice_name = desired.payload.get("nice_name") + + if not isinstance(nice_name, str): + nice_name = "" + + nice_name = nice_name.strip() + + if not nice_name: + nice_name = ( + "dns-sync: " + + ", ".join(domains) + ) + + created = self.client.create_certificate( + { + # This node is receiving existing certificate + # material. It must not independently perform ACME + # issuance. + "provider": "other", + "domain_names": list(domains), + "nice_name": nice_name, + "meta": { + "cluster_managed": True, + "acme_renew": False, + "dns_challenge": False, + }, + } + ) + + created_id = created.get("id") + + if ( + isinstance(created_id, bool) + or not isinstance(created_id, int) + or created_id < 1 + ): + raise RuntimeError( + "NPM certificate create returned invalid id" + ) + + local_key = str(created_id) + + try: + self.client.upload_certificate( + local_key, + certificate=material.certificate, + certificate_key=material.certificate_key, + intermediate_certificate=( + material.intermediate_certificate + ), + ) + + row = self.client.certificate(local_key) + + actual = normalize_discovered_object( + self._certificate_from_row(row) + ) + + if actual.local_key != local_key: + raise RuntimeError( + "Created NPM certificate returned " + "unexpected local id" + ) + + if not self._portable_certificate_matches( + actual, + desired, + ): + raise RuntimeError( + "Created NPM certificate does not match " + "desired portable certificate state" + ) + + except Exception: + try: + self.client.delete_certificate(local_key) + except Exception as rollback_error: + raise RuntimeError( + "NPM certificate creation failed and " + "rollback also failed" + ) from rollback_error + + raise + + return ApplyResult( + object=actual, + detail="created certificate", ) def verify(self, object: NormalizedObject) -> bool: diff --git a/tests/test_npm_adapter.py b/tests/test_npm_adapter.py index 462285c..ebb24a2 100644 --- a/tests/test_npm_adapter.py +++ b/tests/test_npm_adapter.py @@ -9,6 +9,8 @@ from dns_sync.adapters import ( CapabilityOperation, NormalizedObject, ObjectReference, + PlanAction, + PlanOperation, ) from dns_sync.certificates.material import CertificateMaterial from dns_sync.config import AppConfig @@ -762,14 +764,424 @@ class NpmAdapterTests(unittest.TestCase): self.assertEqual(resolved, material) + def test_certificate_create_uploads_verifies_and_returns_target_id(self): + target_client = Mock() + source_client = Mock() + + source_client.certificates.return_value = [ + { + "id": 10, + "domain_names": ["app.example.com"], + "provider": "letsencrypt", + "nice_name": "source", + "valid_from": "2026-09-01T00:00:00Z", + "expires_on": "2026-12-01T00:00:00Z", + "meta": {}, + } + ] + + source_client.certificate_material.return_value = ( + CertificateMaterial( + certificate=b"CERT", + certificate_key=b"KEY", + intermediate_certificate=b"CHAIN", + ) + ) + + target_client.create_certificate.return_value = { + "id": 84, + } + + target_client.certificate.return_value = { + "id": 84, + "domain_names": ["app.example.com"], + "provider": "other", + "nice_name": "source", + "valid_from": "2026-09-01T00:00:00Z", + "expires_on": "2026-12-01T00:00:00Z", + "meta": { + "cluster_managed": True, + "acme_renew": False, + "dns_challenge": False, + }, + } + + adapter, client_factory = ( + self.target_adapter_with_sources( + target_client, + { + "npm://192.168.1.3:81": source_client, + }, + ) + ) + + desired = NormalizedObject( + object_type=CERTIFICATES, + logical_key="app.example.com", + payload={ + "domain_names": ["app.example.com"], + "provider": "letsencrypt", + "nice_name": "source", + "valid_from": "2026-09-01T00:00:00+00:00", + "expires_on": "2026-12-01T00:00:00+00:00", + "cluster_managed": True, + "acme_renew": True, + "dns_challenge": False, + }, + ) + + action = PlanAction( + endpoint_id=adapter.endpoint.identity, + operation=PlanOperation.CREATE, + desired=desired, + ) + + with patch( + "dns_sync.plugins.adapters.npm.NpmClient", + side_effect=client_factory, + ): + result = adapter.apply(action) + + self.assertEqual( + result.object.local_key, + "84", + ) + self.assertEqual( + result.object.logical_key, + "app.example.com", + ) + + target_client.create_certificate.assert_called_once_with( + { + "provider": "other", + "domain_names": ["app.example.com"], + "nice_name": "source", + "meta": { + "cluster_managed": True, + "acme_renew": False, + "dns_challenge": False, + }, + } + ) + + target_client.upload_certificate.assert_called_once_with( + "84", + certificate=b"CERT", + certificate_key=b"KEY", + intermediate_certificate=b"CHAIN", + ) + + target_client.certificate.assert_called_once_with("84") + target_client.delete_certificate.assert_not_called() + + def test_certificate_create_rolls_back_when_upload_fails(self): + target_client = Mock() + source_client = Mock() + + source_client.certificates.return_value = [ + { + "id": 10, + "domain_names": ["app.example.com"], + "provider": "other", + "valid_from": "2026-09-01T00:00:00Z", + "expires_on": "2026-12-01T00:00:00Z", + "meta": {}, + } + ] + + source_client.certificate_material.return_value = ( + CertificateMaterial( + certificate=b"CERT", + certificate_key=b"KEY", + ) + ) + + target_client.create_certificate.return_value = { + "id": 84, + } + target_client.upload_certificate.side_effect = RuntimeError( + "upload failed" + ) + + adapter, client_factory = ( + self.target_adapter_with_sources( + target_client, + { + "npm://192.168.1.3:81": source_client, + }, + ) + ) + + desired = NormalizedObject( + object_type=CERTIFICATES, + logical_key="app.example.com", + payload={ + "domain_names": ["app.example.com"], + "valid_from": "2026-09-01T00:00:00+00:00", + "expires_on": "2026-12-01T00:00:00+00:00", + }, + ) + + action = PlanAction( + endpoint_id=adapter.endpoint.identity, + operation=PlanOperation.CREATE, + desired=desired, + ) + + with patch( + "dns_sync.plugins.adapters.npm.NpmClient", + side_effect=client_factory, + ): + with self.assertRaisesRegex( + RuntimeError, + "upload failed", + ): + adapter.apply(action) + + target_client.delete_certificate.assert_called_once_with( + "84" + ) + target_client.certificate.assert_not_called() + + def test_certificate_create_rolls_back_when_created_state_is_wrong(self): + target_client = Mock() + source_client = Mock() + + source_client.certificates.return_value = [ + { + "id": 10, + "domain_names": ["app.example.com"], + "provider": "other", + "valid_from": "2026-09-01T00:00:00Z", + "expires_on": "2026-12-01T00:00:00Z", + "meta": {}, + } + ] + + source_client.certificate_material.return_value = ( + CertificateMaterial( + certificate=b"CERT", + certificate_key=b"KEY", + ) + ) + + target_client.create_certificate.return_value = { + "id": 84, + } + + target_client.certificate.return_value = { + "id": 84, + "domain_names": ["app.example.com"], + "provider": "other", + "valid_from": "2026-09-01T00:00:00Z", + # Wrong target certificate state. + "expires_on": "2027-01-01T00:00:00Z", + "meta": {}, + } + + adapter, client_factory = ( + self.target_adapter_with_sources( + target_client, + { + "npm://192.168.1.3:81": source_client, + }, + ) + ) + + desired = NormalizedObject( + object_type=CERTIFICATES, + logical_key="app.example.com", + payload={ + "domain_names": ["app.example.com"], + "valid_from": "2026-09-01T00:00:00+00:00", + "expires_on": "2026-12-01T00:00:00+00:00", + }, + ) + + action = PlanAction( + endpoint_id=adapter.endpoint.identity, + operation=PlanOperation.CREATE, + desired=desired, + ) + + with patch( + "dns_sync.plugins.adapters.npm.NpmClient", + side_effect=client_factory, + ): + with self.assertRaisesRegex( + RuntimeError, + "does not match desired", + ): + adapter.apply(action) + + target_client.delete_certificate.assert_called_once_with( + "84" + ) + + def test_certificate_create_reports_rollback_failure_without_secrets(self): + target_client = Mock() + source_client = Mock() + + source_client.certificates.return_value = [ + { + "id": 10, + "domain_names": ["app.example.com"], + "provider": "other", + "valid_from": "2026-09-01T00:00:00Z", + "expires_on": "2026-12-01T00:00:00Z", + "meta": {}, + } + ] + + source_client.certificate_material.return_value = ( + CertificateMaterial( + certificate=b"DO-NOT-PRINT-CERT", + certificate_key=b"DO-NOT-PRINT-KEY", + ) + ) + + target_client.create_certificate.return_value = { + "id": 84, + } + target_client.upload_certificate.side_effect = RuntimeError( + "upload failed" + ) + target_client.delete_certificate.side_effect = RuntimeError( + "delete failed" + ) + + adapter, client_factory = ( + self.target_adapter_with_sources( + target_client, + { + "npm://192.168.1.3:81": source_client, + }, + ) + ) + + desired = NormalizedObject( + object_type=CERTIFICATES, + logical_key="app.example.com", + payload={ + "domain_names": ["app.example.com"], + "valid_from": "2026-09-01T00:00:00+00:00", + "expires_on": "2026-12-01T00:00:00+00:00", + }, + ) + + action = PlanAction( + endpoint_id=adapter.endpoint.identity, + operation=PlanOperation.CREATE, + desired=desired, + ) + + with patch( + "dns_sync.plugins.adapters.npm.NpmClient", + side_effect=client_factory, + ): + with self.assertRaises(RuntimeError) as caught: + adapter.apply(action) + + rendered = str(caught.exception) + + self.assertIn( + "rollback also failed", + rendered, + ) + self.assertNotIn( + "DO-NOT-PRINT-CERT", + rendered, + ) + self.assertNotIn( + "DO-NOT-PRINT-KEY", + rendered, + ) + + def test_certificate_create_rejects_invalid_created_id(self): + target_client = Mock() + source_client = Mock() + + source_client.certificates.return_value = [ + { + "id": 10, + "domain_names": ["app.example.com"], + "provider": "other", + "valid_from": "2026-09-01T00:00:00Z", + "expires_on": "2026-12-01T00:00:00Z", + "meta": {}, + } + ] + + source_client.certificate_material.return_value = ( + CertificateMaterial( + certificate=b"CERT", + certificate_key=b"KEY", + ) + ) + + target_client.create_certificate.return_value = { + "id": None, + } + + adapter, client_factory = ( + self.target_adapter_with_sources( + target_client, + { + "npm://192.168.1.3:81": source_client, + }, + ) + ) + + desired = NormalizedObject( + object_type=CERTIFICATES, + logical_key="app.example.com", + payload={ + "domain_names": ["app.example.com"], + "valid_from": "2026-09-01T00:00:00+00:00", + "expires_on": "2026-12-01T00:00:00+00:00", + }, + ) + + action = PlanAction( + endpoint_id=adapter.endpoint.identity, + operation=PlanOperation.CREATE, + desired=desired, + ) + + with patch( + "dns_sync.plugins.adapters.npm.NpmClient", + side_effect=client_factory, + ): + with self.assertRaisesRegex( + RuntimeError, + "invalid id", + ): + adapter.apply(action) + + target_client.upload_certificate.assert_not_called() + target_client.delete_certificate.assert_not_called() + def test_unsupported_operations_fail_explicitly(self): adapter = self.adapter(Mock()) with self.assertRaisesRegex(RuntimeError, "cannot discover"): adapter.discover("unsupported_objects") - with self.assertRaisesRegex(RuntimeError, "does not support apply"): - adapter.apply(Mock()) + unsupported_action = PlanAction( + endpoint_id=adapter.endpoint.identity, + operation=PlanOperation.CREATE, + desired=NormalizedObject( + object_type="unsupported_objects", + logical_key="unsupported", + payload={}, + ), + ) + + with self.assertRaisesRegex( + RuntimeError, + "does not support", + ): + adapter.apply(unsupported_action) unsupported = NormalizedObject( object_type="unsupported_objects", diff --git a/tests/test_npm_client.py b/tests/test_npm_client.py index b82e97f..1301213 100644 --- a/tests/test_npm_client.py +++ b/tests/test_npm_client.py @@ -321,6 +321,19 @@ class NpmClientMutationTests(unittest.TestCase): ): client.certificate_material("99") + def test_deletes_certificate_by_endpoint_local_id(self): + client = self.client() + client.request = Mock() + + result = client.delete_certificate("84") + + self.assertIsNone(result) + client.request.assert_called_once_with( + "DELETE", + "/nginx/certificates/84", + expected_status=200, + ) + def test_creates_certificate_metadata_with_json_payload(self): client = self.client() payload = {