Add rollback-safe NPM certificate create
This commit is contained in:
@@ -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],
|
||||
|
||||
@@ -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:
|
||||
|
||||
+414
-2
@@ -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",
|
||||
|
||||
@@ -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 = {
|
||||
|
||||
Reference in New Issue
Block a user