diff --git a/datadog_sync/utils/base_resource.py b/datadog_sync/utils/base_resource.py index 4fead504..5389600a 100644 --- a/datadog_sync/utils/base_resource.py +++ b/datadog_sync/utils/base_resource.py @@ -394,7 +394,11 @@ async def create_resource(self, _id: str, resource: Dict) -> Tuple[str, Dict]: pass async def _create_resource(self, _id: str, resource: Dict) -> None: - _id, r = await self.create_resource(_id, resource) + try: + _id, r = await self.create_resource(_id, resource) + except SkipResource: + self._reconcile_destination_if_absent(_id, resource) + raise self.config.state.destination[self.resource_type][_id] = r @abc.abstractmethod @@ -402,9 +406,50 @@ async def update_resource(self, _id: str, resource: Dict) -> Tuple[str, Dict]: pass async def _update_resource(self, _id: str, resource: Dict) -> None: - _id, r = await self.update_resource(_id, resource) + try: + _id, r = await self.update_resource(_id, resource) + except SkipResource: + self._reconcile_destination_if_absent(_id, resource) + raise self.config.state.destination[self.resource_type][_id] = r + def _reconcile_destination_if_absent(self, _id: str, resource: Dict) -> None: + """Best-effort: record a skipped-but-existing destination resource in state. + + When ``create_resource``/``update_resource`` discovers (via + ``_existing_resources_map``) that a resource already exists on the + destination and raises ``SkipResource`` without writing + ``state.destination``, the bucket view diverges from destination truth: + no state file is persisted, so downstream consumers that trust the bucket + count the id as failed even though the resource is confirmed present via + the live API. + + This reconciles that gap by writing the discovered destination resource + into ``state.destination`` keyed by the source id (the convention used + throughout state.py / models). Insert-if-absent only: an entry already + present is never overwritten, preserving delegate-then-skip resources + that wrote ``state.destination`` before delegating to ``update_resource``. + + Best-effort: any unexpected error during key extraction/lookup is logged + at debug and swallowed so a reconcile failure can never break a skip's + counter/metrics accounting or turn a skip into an unhandled error. + """ + try: + if _id in self.config.state.destination[self.resource_type]: + return + key = self.get_resource_mapping_key(resource) + if key is not None and key in self._existing_resources_map: + self.config.state.destination[self.resource_type][_id] = self._existing_resources_map[key] + except Exception as e: + # Pre-format the message (f-string) rather than passing positional + # %s args: the NDJSON log backend (utils/log.py Log.debug) does not + # interpolate positional args in JSON mode, so %s placeholders would + # be emitted literally. Pre-formatting keeps the diagnostic readable + # in both plain and NDJSON modes. + self.config.logger.debug( + f"destination reconcile skipped for {self.resource_type} {_id}: {e}" + ) + @abc.abstractmethod async def delete_resource(self, _id: str) -> None: pass diff --git a/datadog_sync/utils/resources_handler.py b/datadog_sync/utils/resources_handler.py index d89247c7..f63a5200 100644 --- a/datadog_sync/utils/resources_handler.py +++ b/datadog_sync/utils/resources_handler.py @@ -544,8 +544,19 @@ async def _apply_resource_cb(self, q_item: List) -> None: await sem.acquire() sem_acquired = True - # Run hooks - await r_class._pre_resource_action_hook(_id, resource) + # Run hooks. Reconcile a pre-hook SkipResource the same way the + # _create_resource/_update_resource wrappers do: if the resource is + # present in _existing_resources_map, record it in state.destination + # (insert-if-absent) before re-raising. Without this, a pre-hook skip + # for a resource that already exists on the destination (e.g. an + # immutable or deprecated security monitoring rule whose matching + # destination rule is in the map) would leave no state.destination + # entry -- the same bucket-view divergence this change fixes elsewhere. + try: + await r_class._pre_resource_action_hook(_id, resource) + except SkipResource: + r_class._reconcile_destination_if_absent(_id, resource) + raise connection_result = r_class.connect_resources(_id, resource) empty_binding_escalation = connection_result.empty_binding_escalation diff --git a/tests/unit/test_logs_indexes_skip_reconcile.py b/tests/unit/test_logs_indexes_skip_reconcile.py new file mode 100644 index 00000000..1cee83ab --- /dev/null +++ b/tests/unit/test_logs_indexes_skip_reconcile.py @@ -0,0 +1,93 @@ +# Unless explicitly stated otherwise all files in this repository are licensed +# under the 3-clause BSD style license (see LICENSE). +# This product includes software developed at Datadog (https://www.datadoghq.com/). +# Copyright 2019 Datadog, Inc. + +"""logs_indexes: regression guard + framework safety-net proof. + +logs_indexes' exists-path is already correct (write state.destination, delegate to +update_resource PUT). The framework is a pure no-op for it; the safety-net test +proves the framework WOULD reconcile logs_indexes if its exists-path regressed. +logs_indexes keys _existing_resources_map by name, and import_resource returns +resource["name"] as the source id, so source _id == name == map key. + +All identifiers are obviously synthetic (``index-test``). +""" + +import asyncio +from collections import defaultdict +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from datadog_sync.model.logs_indexes import LogsIndexes +from datadog_sync.utils.resource_utils import SkipResource + + +def _index(name): + return { + "name": name, + "daily_limit": None, + } + + +def _make_logs_indexes(existing_map=None, destination_client=None): + config = MagicMock() + config.destination_client = destination_client or MagicMock() + config.state = MagicMock() + config.state.source = defaultdict(dict) + config.state.destination = defaultdict(dict) + config.logger = MagicMock() + li = LogsIndexes(config=config) + li._existing_resources_map = existing_map or {} + return li, config + + +class TestLogsIndexesSkipReconcile: + def test_existing_resource_create_writes_state_destination(self): + # source _id == name == map key. + _id = "index-test" + dest = _index("index-test") + put_resp = {"name": "index-test", "daily_limit": 42} + destination_client = MagicMock() + destination_client.put = AsyncMock(return_value=put_resp) + li, config = _make_logs_indexes( + existing_map={"index-test": dest}, + destination_client=destination_client, + ) + resource = _index("index-test") + + asyncio.run(li._create_resource(_id, resource)) + + destination_client.put.assert_called_once() + # update_resource does state.destination[_id].update(resp); wrapper rewrites. + assert config.state.destination["logs_indexes"][_id]["daily_limit"] == 42 + + def test_framework_reconciles_if_create_raised_skip(self): + _id = "index-test" + dest = _index("index-test") + li, config = _make_logs_indexes(existing_map={"index-test": dest}) + li.create_resource = AsyncMock(side_effect=SkipResource(_id, "logs_indexes", "exists")) + resource = _index("index-test") + + with pytest.raises(SkipResource): + asyncio.run(li._create_resource(_id, resource)) + + assert config.state.destination["logs_indexes"][_id] == dest + + def test_create_non_existing_still_posts(self): + _id = "index-test" + posted = _index("index-test") + destination_client = MagicMock() + destination_client.post = AsyncMock(return_value=posted) + li, config = _make_logs_indexes( + existing_map={}, + destination_client=destination_client, + ) + resource = _index("index-test") + + out_id, out_r = asyncio.run(li.create_resource(_id, resource)) + + destination_client.post.assert_called_once() + assert out_id == _id + assert out_r == posted diff --git a/tests/unit/test_logs_metrics_skip_reconcile.py b/tests/unit/test_logs_metrics_skip_reconcile.py new file mode 100644 index 00000000..d7e71e9e --- /dev/null +++ b/tests/unit/test_logs_metrics_skip_reconcile.py @@ -0,0 +1,92 @@ +# Unless explicitly stated otherwise all files in this repository are licensed +# under the 3-clause BSD style license (see LICENSE). +# This product includes software developed at Datadog (https://www.datadoghq.com/). +# Copyright 2019 Datadog, Inc. + +"""logs_metrics: regression guard + framework safety-net proof. + +logs_metrics' exists-path is already correct (write state.destination, delegate to +update_resource PATCH). The framework is a pure no-op for it; the safety-net test +proves the framework WOULD reconcile logs_metrics if its exists-path regressed. + +All identifiers are obviously synthetic (``metric-src``, ``metric-dst``). +""" + +import asyncio +from collections import defaultdict +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from datadog_sync.model.logs_metrics import LogsMetrics +from datadog_sync.utils.resource_utils import SkipResource + + +def _metric(metric_id, name=None): + return { + "id": metric_id, + "type": "logs_metrics", + "attributes": {"name": name or metric_id}, + } + + +def _make_logs_metrics(existing_map=None, destination_client=None): + config = MagicMock() + config.destination_client = destination_client or MagicMock() + config.state = MagicMock() + config.state.source = defaultdict(dict) + config.state.destination = defaultdict(dict) + config.logger = MagicMock() + lm = LogsMetrics(config=config) + lm._existing_resources_map = existing_map or {} + return lm, config + + +class TestLogsMetricsSkipReconcile: + def test_existing_resource_create_writes_state_destination(self): + # logs_metrics ids are stable across orgs, so source _id == map key. + _id = "metric-test" + dest = _metric("metric-test") + patched = _metric("metric-test") + patched["attributes"]["marker"] = "patched" + destination_client = MagicMock() + destination_client.patch = AsyncMock(return_value={"data": patched}) + lm, config = _make_logs_metrics( + existing_map={"metric-test": dest}, + destination_client=destination_client, + ) + resource = _metric("metric-test") + + asyncio.run(lm._create_resource(_id, resource)) + + destination_client.patch.assert_called_once() + assert config.state.destination["logs_metrics"][_id] == patched + + def test_framework_reconciles_if_create_raised_skip(self): + _id = "metric-test" + dest = _metric("metric-test") + lm, config = _make_logs_metrics(existing_map={"metric-test": dest}) + lm.create_resource = AsyncMock(side_effect=SkipResource(_id, "logs_metrics", "exists")) + resource = _metric("metric-test") + + with pytest.raises(SkipResource): + asyncio.run(lm._create_resource(_id, resource)) + + assert config.state.destination["logs_metrics"][_id] == dest + + def test_create_non_existing_still_posts(self): + _id = "metric-test" + posted = _metric("metric-test") + destination_client = MagicMock() + destination_client.post = AsyncMock(return_value={"data": posted}) + lm, config = _make_logs_metrics( + existing_map={}, + destination_client=destination_client, + ) + resource = _metric("metric-test") + + out_id, out_r = asyncio.run(lm.create_resource(_id, resource)) + + destination_client.post.assert_called_once() + assert out_id == _id + assert out_r == posted diff --git a/tests/unit/test_metric_tag_configurations_skip_reconcile.py b/tests/unit/test_metric_tag_configurations_skip_reconcile.py new file mode 100644 index 00000000..2677a792 --- /dev/null +++ b/tests/unit/test_metric_tag_configurations_skip_reconcile.py @@ -0,0 +1,142 @@ +# Unless explicitly stated otherwise all files in this repository are licensed +# under the 3-clause BSD style license (see LICENSE). +# This product includes software developed at Datadog (https://www.datadoghq.com/). +# Copyright 2019 Datadog, Inc. + +"""metric_tag_configurations: regression guard, non-existence guard, safety-net proof. + +Two SkipResource classes here must NOT trigger a destination-state write: + (a) "Metric not present on destination" (create_resource POST 400, or + update_resource PATCH 400) -- the metric genuinely does not exist at the + destination, the id is NOT in _existing_resources_map -> framework writes + nothing (no false positive); + (b) the exists-path (id in map) writes state.destination then delegates to + update_resource -- already correct, framework no-ops. + +All identifiers are obviously synthetic (``mtc-test``). +""" + +import asyncio +from collections import defaultdict +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from datadog_sync.model.metric_tag_configurations import MetricTagConfigurations +from datadog_sync.utils.resource_utils import CustomClientHTTPError, SkipResource + + +class _FakeResponse: + def __init__(self, status, message="error"): + self.status = status + self.message = message + + +def _mtc(metric_id): + return { + "id": metric_id, + "type": "manage_tags", + "attributes": {"tags": ["tag:src"]}, + } + + +def _make_mtc(existing_map=None, destination_client=None): + config = MagicMock() + config.destination_client = destination_client or MagicMock() + config.state = MagicMock() + config.state.source = defaultdict(dict) + config.state.destination = defaultdict(dict) + config.logger = MagicMock() + mtc = MetricTagConfigurations(config=config) + mtc._existing_resources_map = existing_map or {} + return mtc, config + + +class TestMetricTagConfigurationsSkipReconcile: + def test_existing_resource_create_writes_state_destination(self): + # id in map -> write state.destination, delegate to update_resource (PATCH). + _id = "mtc-test" + dest = _mtc("mtc-test") + patched = _mtc("mtc-test") + patched["attributes"]["marker"] = "patched" + destination_client = MagicMock() + destination_client.patch = AsyncMock(return_value={"data": patched}) + mtc, config = _make_mtc( + existing_map={"mtc-test": dest}, + destination_client=destination_client, + ) + resource = _mtc("mtc-test") + + asyncio.run(mtc._create_resource(_id, resource)) + + destination_client.patch.assert_called_once() + assert config.state.destination["metric_tag_configurations"][_id] == patched + + def test_metric_not_present_skip_does_not_write(self): + # id NOT in map -> POST -> 400 "metric that does not exist" -> SkipResource. + # Framework must NOT write (genuine non-existence; no false positive). + _id = "mtc-test" + err = CustomClientHTTPError(_FakeResponse(400), "metric that does not exist") + destination_client = MagicMock() + destination_client.post = AsyncMock(side_effect=err) + mtc, config = _make_mtc( + existing_map={}, + destination_client=destination_client, + ) + config.state.source["metric_tag_configurations"][_id] = _mtc("mtc-test") + resource = _mtc("mtc-test") + + with pytest.raises(SkipResource, match="Metric not present"): + asyncio.run(mtc._create_resource(_id, resource)) + + assert config.state.destination["metric_tag_configurations"] == {} + + def test_update_metric_not_present_skip_preserves_state_destination(self): + # update_resource PATCH 400 missing metric -> SkipResource. state.destination + # was already written by the exists-path; insert-if-absent must preserve it. + _id = "mtc-test" + dest = _mtc("mtc-test") + err = CustomClientHTTPError(_FakeResponse(400), "metric that does not exist") + destination_client = MagicMock() + destination_client.patch = AsyncMock(side_effect=err) + mtc, config = _make_mtc( + existing_map={"mtc-test": dest}, + destination_client=destination_client, + ) + config.state.destination["metric_tag_configurations"][_id] = dest + resource = _mtc("mtc-test") + + with pytest.raises(SkipResource, match="Metric not present"): + asyncio.run(mtc._update_resource(_id, resource)) + + assert config.state.destination["metric_tag_configurations"][_id] is dest + + def test_framework_reconciles_if_create_raised_skip(self): + _id = "mtc-test" + dest = _mtc("mtc-test") + mtc, config = _make_mtc(existing_map={"mtc-test": dest}) + mtc.create_resource = AsyncMock(side_effect=SkipResource(_id, "metric_tag_configurations", "exists")) + resource = _mtc("mtc-test") + + with pytest.raises(SkipResource): + asyncio.run(mtc._create_resource(_id, resource)) + + assert config.state.destination["metric_tag_configurations"][_id] == dest + + def test_create_non_existing_still_posts(self): + _id = "mtc-test" + posted = _mtc("mtc-test") + destination_client = MagicMock() + destination_client.post = AsyncMock(return_value={"data": posted}) + mtc, config = _make_mtc( + existing_map={}, + destination_client=destination_client, + ) + config.state.source["metric_tag_configurations"][_id] = _mtc("mtc-test") + resource = _mtc("mtc-test") + + out_id, out_r = asyncio.run(mtc.create_resource(_id, resource)) + + destination_client.post.assert_called_once() + assert out_id == _id + assert out_r == posted diff --git a/tests/unit/test_roles_skip_reconcile.py b/tests/unit/test_roles_skip_reconcile.py new file mode 100644 index 00000000..869ea382 --- /dev/null +++ b/tests/unit/test_roles_skip_reconcile.py @@ -0,0 +1,134 @@ +# Unless explicitly stated otherwise all files in this repository are licensed +# under the 3-clause BSD style license (see LICENSE). +# This product includes software developed at Datadog (https://www.datadoghq.com/). +# Copyright 2019 Datadog, Inc. + +"""roles: destination-state reconciliation behavior on SkipResource paths. + +Roles has no live skip-without-write bug today: every reachable ``SkipResource`` +raise either (a) is a genuine non-existence skip (built-in role absent from the +destination map -> key not in map -> framework correctly writes nothing), or +(b) fires from ``update_resource`` after ``state.destination`` was already +populated (the "already exists" path writes state.destination before delegating) +-> framework insert-if-absent no-ops. These tests pin both invariants and add a +framework safety-net proof showing the wrapper WOULD reconcile roles if its +exists-path ever regressed into a skip-without-write. + +Note: the ``create_resource`` permission-edge SkipResource (inside the +``if role_name not in map`` branch, which then checks ``if role_name in map``) +is unreachable in a single call because ``_existing_resources_map`` is static +during apply -- documented here as a dead-code finding for the audit. + +All identifiers are obviously synthetic (``role-src``, ``role-dst``, ``perm-x``). +""" + +import asyncio +from collections import defaultdict +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from datadog_sync.model.roles import Roles +from datadog_sync.utils.resource_utils import CustomClientHTTPError, SkipResource + + +class _FakeResponse: + def __init__(self, status, message): + self.status = status + self.message = message + + +def _role(name, perm_ids=None, role_id=None): + return { + "type": "roles", + "id": role_id or f"role-id-{name}", + "attributes": {"name": name}, + "relationships": {"permissions": {"data": [{"id": p, "type": "permission"} for p in (perm_ids or [])]}}, + } + + +def _make_roles(existing_map=None, destination_client=None, allow_partial=None): + config = MagicMock() + config.destination_client = destination_client or MagicMock() + config.state = MagicMock() + config.state.source = defaultdict(dict) + config.state.destination = defaultdict(dict) + config.logger = MagicMock() + config.allow_partial_permissions_roles = allow_partial or [] + roles = Roles(config=config) + roles._existing_resources_map = existing_map or {} + return roles, config + + +class TestRolesSkipReconcile: + def test_create_built_in_not_in_map_skip_does_not_write(self): + # Built-in role absent from the destination map -> genuine non-existence + # skip. Framework must NOT invent a destination entry (no false positive). + roles, config = _make_roles(existing_map={}) + _id = "role-src" + resource = _role("Datadog Admin Role", ["perm-x"], role_id=_id) + + with pytest.raises(SkipResource, match="built-in Datadog role"): + asyncio.run(roles._create_resource(_id, resource)) + + assert config.state.destination["roles"] == {} + + def test_update_permission_edge_skip_preserves_state_destination(self): + # Reachable update_resource permission edge: state.destination already + # holds the role; patch 400 -> remove perm-x -> no diff with in-state + # destination -> SkipResource. Insert-if-absent must leave the pre-existing + # entry untouched (same object identity). + _id = "role-src" + in_state = _role("role-src", ["perm-y"], role_id="role-dst") + resource = _role("role-src", ["perm-x", "perm-y"], role_id=_id) + + err = CustomClientHTTPError( + _FakeResponse(400, '{"detail":"invalid UUID [perm-x]"}'), + ) + destination_client = MagicMock() + destination_client.patch = AsyncMock(side_effect=err) + roles, config = _make_roles( + existing_map={"role-src": in_state}, + destination_client=destination_client, + allow_partial=["perm-x"], + ) + config.state.destination["roles"][_id] = in_state + + with pytest.raises(SkipResource, match="already exists at destination"): + asyncio.run(roles._update_resource(_id, resource)) + + # Insert-if-absent: pre-existing entry preserved, not overwritten by map. + assert config.state.destination["roles"][_id] is in_state + + def test_framework_reconciles_if_create_raised_skip(self): + # Safety-net proof: if roles.create_resource ever regressed into a + # skip-without-write with the role present in the map, the framework + # wrapper would reconcile state.destination. + _id = "role-src" + dest_role = _role("role-src", ["perm-y"], role_id="role-dst") + roles, config = _make_roles(existing_map={"role-src": dest_role}) + # Stub create_resource to raise (bypassing the real delegate logic). + roles.create_resource = AsyncMock(side_effect=SkipResource(_id, "roles", "exists")) + resource = _role("role-src", ["perm-y"], role_id=_id) + + with pytest.raises(SkipResource): + asyncio.run(roles._create_resource(_id, resource)) + + assert config.state.destination["roles"][_id] == dest_role + + def test_create_non_existing_role_still_posts(self): + _id = "role-src" + posted = _role("role-src", ["perm-y"], role_id="role-dst") + destination_client = MagicMock() + destination_client.post = AsyncMock(return_value={"data": posted}) + roles, config = _make_roles( + existing_map={}, + destination_client=destination_client, + ) + resource = _role("role-src", ["perm-y"], role_id=_id) + + out_id, out_r = asyncio.run(roles.create_resource(_id, resource)) + + destination_client.post.assert_called_once() + assert out_id == _id + assert out_r == posted diff --git a/tests/unit/test_security_monitoring_skip_reconcile.py b/tests/unit/test_security_monitoring_skip_reconcile.py new file mode 100644 index 00000000..501bb1ae --- /dev/null +++ b/tests/unit/test_security_monitoring_skip_reconcile.py @@ -0,0 +1,218 @@ +# Unless explicitly stated otherwise all files in this repository are licensed +# under the 3-clause BSD style license (see LICENSE). +# This product includes software developed at Datadog (https://www.datadoghq.com/). +# Copyright 2019 Datadog, Inc. + +"""security_monitoring_rules: no-false-positive guards for skip reconciliation. + +Two SkipResource classes here must NOT trigger a destination-state write: + (a) genuine non-existence: a non-default rule POSTs, gets a 400 with a + skip-able "Invalid rule configuration" error, and raises SkipResource + while the rule name is NOT in _existing_resources_map -> framework writes + nothing (the rule genuinely does not exist at the destination); + (b) pre-hook skip: "Default rule does not exist at destination" is raised in + pre_resource_action_hook (before the _create_resource/_update_resource + wrappers), so the framework never sees it -- documented as a boundary. + +The immutable-rule update skip (inside update_resource) is also pinned: +insert-if-absent must preserve a pre-existing state.destination entry. + +All identifiers are obviously synthetic (``rule-src``, ``rule-dst``). +""" + +import asyncio +import json +from collections import defaultdict +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from datadog_sync.model.security_monitoring_rules import SecurityMonitoringRules +from datadog_sync.utils.resource_utils import CustomClientHTTPError, SkipResource + + +class _FakeResponse: + def __init__(self, status, message): + self.status = status + self.message = message + + +def _rule(name, is_default=False, rule_id=None, deprecated=False): + return { + "id": rule_id or f"rule-id-{name}", + "name": name, + "isDefault": is_default, + "isDeprecated": deprecated, + "version": 1, + "queries": [], + "cases": [], + "options": {}, + "message": "synthetic test rule", + "tags": [], + } + + +def _make_rules(existing_map=None, destination_client=None): + config = MagicMock() + config.destination_client = destination_client or MagicMock() + config.state = MagicMock() + config.state.source = defaultdict(dict) + config.state.destination = defaultdict(dict) + config.logger = MagicMock() + rules = SecurityMonitoringRules(config=config) + rules._existing_resources_map = existing_map or {} + return rules, config + + +class TestSecurityMonitoringSkipReconcile: + def test_create_400_invalid_config_skip_does_not_write(self): + # Non-default rule not in map -> POST -> 400 "Invalid rule configuration" + # -> SkipResource. Rule name NOT in map -> framework must NOT write + # (genuine non-existence; no false positive). + _id = "rule-src" + err = CustomClientHTTPError( + _FakeResponse(400, "Bad Request"), + json.dumps({"errors": ["Invalid rule configuration"]}), + ) + destination_client = MagicMock() + destination_client.post = AsyncMock(side_effect=err) + rules, config = _make_rules(existing_map={}, destination_client=destination_client) + resource = _rule("rule-src", is_default=False, rule_id=_id) + + with pytest.raises(SkipResource, match="Invalid rule configuration"): + asyncio.run(rules._create_resource(_id, resource)) + + assert config.state.destination["security_monitoring_rules"] == {} + + def test_pre_hook_default_not_exist_skip_not_reconciled(self): + # "Default rule does not exist at destination" is raised in the pre-hook. + # The handler now reconciles pre-hook skips, but only writes when the key + # is in _existing_resources_map. Here the default rule is NOT in the map + # (it genuinely doesn't exist at the destination), so reconcile no-ops + # and state.destination stays empty. Pins the no-false-positive boundary. + from datadog_sync.utils.resources_handler import ResourcesHandler + + _id = "rule-src" + rules, config = _make_rules(existing_map={}) + resource = _rule("rule-src", is_default=True, rule_id=_id) + config.resources = {"security_monitoring_rules": rules} + config.state.source["security_monitoring_rules"][_id] = resource + + handler = ResourcesHandler(config) + handler.worker = MagicMock() + handler.worker.counter = MagicMock() + handler.sorter = MagicMock() + handler._emit = MagicMock() + + asyncio.run(handler._apply_resource_cb(["security_monitoring_rules", _id])) + + handler.worker.counter.increment_skipped.assert_called_once() + # Key not in map -> reconcile no-ops -> state.destination stays empty. + assert config.state.destination["security_monitoring_rules"] == {} + + def test_pre_hook_immutable_skip_reconciles_when_rule_in_map(self): + # Immutable rule IS present in _existing_resources_map. The pre-hook raises + # "This rule is immutable", but the rule exists on the destination, so the + # handler-level reconcile must record it in state.destination (insert-if- + # absent) to avoid the bucket-view false negative. This is the matching-map + # regression case for the pre-hook skip path. + from datadog_sync.utils.resources_handler import ResourcesHandler + + _id = "rule-src" + dest_rule = _rule( + "Impossible travel event leads to permission enumeration", + is_default=True, + rule_id="rule-dst", + ) + rules, config = _make_rules(existing_map={dest_rule["name"]: dest_rule}) + resource = _rule( + "Impossible travel event leads to permission enumeration", + is_default=True, + rule_id=_id, + ) + config.resources = {"security_monitoring_rules": rules} + config.state.source["security_monitoring_rules"][_id] = resource + + handler = ResourcesHandler(config) + handler.worker = MagicMock() + handler.worker.counter = MagicMock() + handler.sorter = MagicMock() + handler._emit = MagicMock() + + asyncio.run(handler._apply_resource_cb(["security_monitoring_rules", _id])) + + handler.worker.counter.increment_skipped.assert_called_once() + handler.worker.counter.increment_failure.assert_not_called() + # Rule was in the map -> reconciled into state.destination under source id. + assert config.state.destination["security_monitoring_rules"][_id] == dest_rule + + def test_pre_hook_deprecated_skip_reconciles_when_rule_in_map(self): + # Deprecated destination rule IS in the map. The pre-hook raises + # "Cannot update deprecated rules", but the rule exists on the destination, + # so the handler-level reconcile must record it in state.destination. + from datadog_sync.utils.resources_handler import ResourcesHandler + + _id = "rule-src" + dest_rule = _rule("rule-deprecated-test", is_default=False, rule_id="rule-dst", deprecated=True) + rules, config = _make_rules(existing_map={dest_rule["name"]: dest_rule}) + resource = _rule("rule-deprecated-test", is_default=False, rule_id=_id) + config.resources = {"security_monitoring_rules": rules} + config.state.source["security_monitoring_rules"][_id] = resource + + handler = ResourcesHandler(config) + handler.worker = MagicMock() + handler.worker.counter = MagicMock() + handler.sorter = MagicMock() + handler._emit = MagicMock() + + asyncio.run(handler._apply_resource_cb(["security_monitoring_rules", _id])) + + handler.worker.counter.increment_skipped.assert_called_once() + assert config.state.destination["security_monitoring_rules"][_id] == dest_rule + + def test_update_immutable_skip_preserves_state_destination(self): + # Immutable rule is in the map; update_resource raises "This rule is + # immutable". Insert-if-absent must leave the pre-existing entry untouched. + _id = "rule-src" + dest_rule = _rule( + "Impossible travel event leads to permission enumeration", is_default=True, rule_id="rule-dst" + ) + sentinel = _rule("Impossible travel event leads to permission enumeration", is_default=True, rule_id="rule-dst") + sentinel["marker"] = "pre-existing" + rules, config = _make_rules(existing_map={dest_rule["name"]: dest_rule}) + config.state.destination["security_monitoring_rules"][_id] = sentinel + resource = _rule(dest_rule["name"], is_default=True, rule_id=_id) + + with pytest.raises(SkipResource, match="immutable"): + asyncio.run(rules._update_resource(_id, resource)) + + assert config.state.destination["security_monitoring_rules"][_id] is sentinel + + def test_create_non_existing_rule_still_posts(self): + _id = "rule-src" + posted = _rule("rule-src", is_default=False, rule_id="rule-dst") + destination_client = MagicMock() + destination_client.post = AsyncMock(return_value=posted) + rules, config = _make_rules(existing_map={}, destination_client=destination_client) + resource = _rule("rule-src", is_default=False, rule_id=_id) + + out_id, out_r = asyncio.run(rules.create_resource(_id, resource)) + + destination_client.post.assert_called_once() + assert out_id == _id + assert out_r == posted + + def test_framework_reconciles_if_create_raised_skip(self): + # Safety-net proof: if create_resource ever regressed into a + # skip-without-write with the rule present in the map, the framework + # wrapper would reconcile state.destination. + _id = "rule-src" + dest_rule = _rule("rule-src", is_default=False, rule_id="rule-dst") + rules, config = _make_rules(existing_map={"rule-src": dest_rule}) + rules.create_resource = AsyncMock(side_effect=SkipResource(_id, "security_monitoring_rules", "exists")) + resource = _rule("rule-src", is_default=False, rule_id=_id) + + with pytest.raises(SkipResource): + asyncio.run(rules._create_resource(_id, resource)) + + assert config.state.destination["security_monitoring_rules"][_id] == dest_rule diff --git a/tests/unit/test_skip_reconcile_framework.py b/tests/unit/test_skip_reconcile_framework.py new file mode 100644 index 00000000..7ff1a47e --- /dev/null +++ b/tests/unit/test_skip_reconcile_framework.py @@ -0,0 +1,323 @@ +# Unless explicitly stated otherwise all files in this repository are licensed +# under the 3-clause BSD style license (see LICENSE). +# This product includes software developed at Datadog (https://www.datadoghq.com/). +# Copyright 2019 Datadog, Inc. + +"""Framework-level tests for destination-state reconciliation on skip-without-write. + +When a resource's ``create_resource``/``update_resource`` discovers (via +``_existing_resources_map``) that the resource already exists on the destination +and raises ``SkipResource`` without writing ``state.destination``, the bucket +view diverges from destination truth: no state file is persisted, so downstream +consumers that trust the bucket (e.g. a per-id presence summary) count the id as +failed even though the resource is confirmed present via the live API. + +The ``_create_resource``/``_update_resource`` wrappers in ``base_resource.py`` +reconcile this gap: on ``SkipResource`` they look up the resource's mapping key +in ``_existing_resources_map`` and, if found, write the discovered destination +resource into ``state.destination`` (insert-if-absent) before re-raising. These +tests pin that contract and its boundaries. + +All identifiers are obviously synthetic (``team-src``, ``team-dst`` ...). +""" + +import asyncio +from unittest.mock import MagicMock + +import pytest + +from datadog_sync.utils.base_resource import BaseResource, ResourceConfig +from datadog_sync.utils.resource_utils import SkipResource + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _make_resource_class( + resource_config, resource_type_name="test_resource", create_side_effect=None, update_side_effect=None +): + """Create a concrete BaseResource subclass for testing. + + ``create_side_effect``/``update_side_effect`` may be callables returning + ``(_id, resource)`` or raising, mirroring real create/update methods. + """ + + async def get_resources(self, client): + return [] + + async def import_resource(self, _id=None, resource=None): + return _id, resource + + async def pre_resource_action_hook(self, _id, resource): + pass + + async def pre_apply_hook(self): + pass + + async def create_resource(self, _id, resource): + if create_side_effect is not None: + return create_side_effect(_id, resource) + return _id, resource + + async def update_resource(self, _id, resource): + if update_side_effect is not None: + return update_side_effect(_id, resource) + return _id, resource + + async def delete_resource(self, _id): + pass + + return type( + "ConcreteResource", + (BaseResource,), + { + "resource_type": resource_type_name, + "resource_config": resource_config, + "get_resources": get_resources, + "import_resource": import_resource, + "pre_resource_action_hook": pre_resource_action_hook, + "pre_apply_hook": pre_apply_hook, + "create_resource": create_resource, + "update_resource": update_resource, + "delete_resource": delete_resource, + }, + ) + + +def _make_instance(mock_config, resource_config, resource_type="test_resource", **kw): + cls = _make_resource_class(resource_config, resource_type, **kw) + inst = cls(mock_config) + return inst + + +def _raise_skip(_id, resource): + raise SkipResource(_id, "test_resource", "already exists") + + +def _return_resource(_id, resource): + return _id, resource + + +# =========================================================================== +# Cycle 1: core framework wrapper +# =========================================================================== + + +class TestCreateWrapperReconcile: + def test_create_wrapper_reconciles_when_skip_and_key_in_map(self, mock_config): + rc = ResourceConfig(base_path="/test", resource_mapping_key="name", skip_resource_mapping=False) + dest_resource = {"name": "dest-name", "id": "dest-id"} + inst = _make_instance(mock_config, rc, "test_resource", create_side_effect=_raise_skip) + inst._existing_resources_map = {"dest-name": dest_resource} + source_resource = {"name": "dest-name", "id": "src-id"} + + with pytest.raises(SkipResource): + asyncio.run(inst._create_resource("src-id", source_resource)) + + assert mock_config.state.destination["test_resource"]["src-id"] == dest_resource + + def test_update_wrapper_reconciles_when_skip_and_key_in_map(self, mock_config): + rc = ResourceConfig(base_path="/test", resource_mapping_key="name", skip_resource_mapping=False) + dest_resource = {"name": "dest-name", "id": "dest-id"} + inst = _make_instance(mock_config, rc, "test_resource", update_side_effect=_raise_skip) + inst._existing_resources_map = {"dest-name": dest_resource} + source_resource = {"name": "dest-name", "id": "src-id"} + + with pytest.raises(SkipResource): + asyncio.run(inst._update_resource("src-id", source_resource)) + + assert mock_config.state.destination["test_resource"]["src-id"] == dest_resource + + def test_wrapper_reconcile_uses_source_id_as_state_key(self, mock_config): + # The mapping key resolves to a *destination* id, but state.destination + # must be keyed by the *source* _id (the wrapper's _id argument), per the + # "destination state files are keyed by source IDs" convention. + rc = ResourceConfig(base_path="/test", resource_mapping_key="name", skip_resource_mapping=False) + dest_resource = {"name": "dest-name", "id": "dest-id"} + inst = _make_instance(mock_config, rc, "test_resource", create_side_effect=_raise_skip) + inst._existing_resources_map = {"dest-name": dest_resource} + source_resource = {"name": "dest-name", "id": "src-id"} + + with pytest.raises(SkipResource): + asyncio.run(inst._create_resource("src-id", source_resource)) + + assert "src-id" in mock_config.state.destination["test_resource"] + assert "dest-id" not in mock_config.state.destination["test_resource"] + + def test_wrapper_does_not_reconcile_when_key_not_in_map(self, mock_config): + # Genuine non-existence skip: key absent from the map -> no write. + # This is the critical false-positive guard. + rc = ResourceConfig(base_path="/test", resource_mapping_key="name", skip_resource_mapping=False) + inst = _make_instance(mock_config, rc, "test_resource", create_side_effect=_raise_skip) + inst._existing_resources_map = {"other-name": {"name": "other-name"}} + source_resource = {"name": "ghost-name", "id": "src-id"} + + with pytest.raises(SkipResource): + asyncio.run(inst._create_resource("src-id", source_resource)) + + assert mock_config.state.destination["test_resource"] == {} + + def test_wrapper_does_not_reconcile_when_mapping_key_is_none(self, mock_config): + # Opt-out resource (skip_resource_mapping=True, resource_mapping_key=None): + # no write, no crash, re-raises. + rc = ResourceConfig(base_path="/test", resource_mapping_key=None, skip_resource_mapping=True) + inst = _make_instance(mock_config, rc, "test_resource", create_side_effect=_raise_skip) + inst._existing_resources_map = {} + source_resource = {"id": "src-id"} + + with pytest.raises(SkipResource): + asyncio.run(inst._create_resource("src-id", source_resource)) + + assert mock_config.state.destination["test_resource"] == {} + + def test_wrapper_reconciles_empty_string_key_consistent_with_map_existing(self, mock_config): + # map_existing_resources() keeps entries when `key is not None`, so an + # empty-string key IS mappable during discovery. The reconcile must use + # the same `is not None` gate (not truthiness) so an empty-string key + # present in the map is reconciled here too. Pins the consistency fix. + rc = ResourceConfig(base_path="/test", resource_mapping_key="name", skip_resource_mapping=False) + dest_resource = {"name": "", "id": "dest-id"} + inst = _make_instance(mock_config, rc, "test_resource", create_side_effect=_raise_skip) + inst._existing_resources_map = {"": dest_resource} + source_resource = {"name": "", "id": "src-id"} + + with pytest.raises(SkipResource): + asyncio.run(inst._create_resource("src-id", source_resource)) + + assert mock_config.state.destination["test_resource"]["src-id"] == dest_resource + + def test_wrapper_does_not_reconcile_when_entry_already_present(self, mock_config): + # Insert-if-absent: a pre-existing entry must NOT be overwritten on skip. + # Preserves delegate-then-skip resources that wrote state.destination + # before delegating to update_resource. + rc = ResourceConfig(base_path="/test", resource_mapping_key="name", skip_resource_mapping=False) + sentinel = {"name": "dest-name", "id": "dest-id", "marker": "pre-existing"} + inst = _make_instance(mock_config, rc, "test_resource", create_side_effect=_raise_skip) + inst._existing_resources_map = {"dest-name": {"name": "dest-name", "id": "dest-id"}} + mock_config.state.destination["test_resource"]["src-id"] = sentinel + source_resource = {"name": "dest-name", "id": "src-id"} + + with pytest.raises(SkipResource): + asyncio.run(inst._create_resource("src-id", source_resource)) + + assert mock_config.state.destination["test_resource"]["src-id"] == sentinel + + def test_wrapper_does_not_reconcile_on_non_skip_exception(self, mock_config): + rc = ResourceConfig(base_path="/test", resource_mapping_key="name", skip_resource_mapping=False) + + def raise_valueerror(_id, resource): + raise ValueError("boom") + + inst = _make_instance(mock_config, rc, "test_resource", create_side_effect=raise_valueerror) + inst._existing_resources_map = {"dest-name": {"name": "dest-name"}} + source_resource = {"name": "dest-name", "id": "src-id"} + + with pytest.raises(ValueError, match="boom"): + asyncio.run(inst._create_resource("src-id", source_resource)) + + assert mock_config.state.destination["test_resource"] == {} + + def test_wrapper_reconcile_is_best_effort_does_not_break_skip(self, mock_config): + # If get_resource_mapping_key raises an unexpected error, the reconcile + # must not break the skip's accounting — swallow and re-raise SkipResource. + rc = ResourceConfig( + base_path="/test", + resource_mapping_key=lambda r: (_ for _ in ()).throw(RuntimeError("key extraction broke")), + skip_resource_mapping=False, + ) + inst = _make_instance(mock_config, rc, "test_resource", create_side_effect=_raise_skip) + inst._existing_resources_map = {"dest-name": {"name": "dest-name"}} + source_resource = {"name": "dest-name", "id": "src-id"} + + with pytest.raises(SkipResource): + asyncio.run(inst._create_resource("src-id", source_resource)) + + assert mock_config.state.destination["test_resource"] == {} + + def test_wrapper_reconcile_debug_log_is_preformatted(self, mock_config): + # The NDJSON log backend (utils/log.py Log.debug) does not interpolate + # positional %s args in JSON mode, so the reconcile diagnostic must be + # pre-formatted (f-string), not passed as positional args. Verify the + # logged message contains the literal values, not %s placeholders. + rc = ResourceConfig( + base_path="/test", + resource_mapping_key=lambda r: (_ for _ in ()).throw(RuntimeError("key extraction broke")), + skip_resource_mapping=False, + ) + inst = _make_instance(mock_config, rc, "test_resource", create_side_effect=_raise_skip) + inst._existing_resources_map = {"dest-name": {"name": "dest-name"}} + source_resource = {"name": "dest-name", "id": "src-id"} + + with pytest.raises(SkipResource): + asyncio.run(inst._create_resource("src-id", source_resource)) + + mock_config.logger.debug.assert_called_once() + logged_msg = mock_config.logger.debug.call_args.args[0] + assert "%s" not in logged_msg + assert "test_resource" in logged_msg + assert "src-id" in logged_msg + assert "key extraction broke" in logged_msg + + +# =========================================================================== +# Cycle 12: opt-out resources -- no false reconcile +# =========================================================================== + + +class TestOptOutResourceSkip: + def test_opt_out_resource_skip_no_write_no_crash(self, mock_config): + # Opt-out resource (skip_resource_mapping=True, resource_mapping_key=None): + # get_resource_mapping_key returns None -> no write, no crash, re-raises. + # Confirms the framework is inert for the non-mapping resources. + rc = ResourceConfig(base_path="/test", resource_mapping_key=None, skip_resource_mapping=True) + inst = _make_instance(mock_config, rc, "dashboards", create_side_effect=_raise_skip) + inst._existing_resources_map = {} + source_resource = {"id": "dashboard-test"} + + with pytest.raises(SkipResource): + asyncio.run(inst._create_resource("dashboard-test", source_resource)) + + assert mock_config.state.destination["dashboards"] == {} + + +# =========================================================================== +# Cycle 1 case 9: handler accounting unchanged after framework reconcile +# =========================================================================== + + +class TestHandlerAccountingUnchanged: + def test_handler_accounting_unchanged_after_framework_reconcile(self, mock_config): + from datadog_sync.utils.resources_handler import ResourcesHandler + + resource_type = "test_resource" + _id = "src-id" + dest_resource = {"name": "dest-name", "id": "dest-id"} + + # Drive the framework wrapper through a real instance so the + # _create_resource/_update_resource reconcile actually runs (a bare + # MagicMock class stand-in would stub _create_resource directly and + # bypass the wrapper under test). + rc = ResourceConfig(base_path="/test", resource_mapping_key="name", skip_resource_mapping=False) + inst = _make_instance(mock_config, rc, resource_type, create_side_effect=_raise_skip) + inst._existing_resources_map = {"dest-name": dest_resource} + mock_config.resources = {resource_type: inst} + mock_config.state.source[resource_type][_id] = {"name": "dest-name", "id": _id} + + handler = ResourcesHandler(mock_config) + handler.worker = MagicMock() + handler.worker.counter = MagicMock() + handler.sorter = MagicMock() + handler._emit = MagicMock() + + asyncio.run(handler._apply_resource_cb([resource_type, _id])) + + handler.worker.counter.increment_skipped.assert_called_once() + handler.worker.counter.increment_failure.assert_not_called() + handler.worker.counter.increment_success.assert_not_called() + # _emit called with skipped status + emit_args, emit_kwargs = handler._emit.call_args + assert emit_args[:4] == (resource_type, _id, "sync", "skipped") + # And the framework reconciled state.destination + assert mock_config.state.destination[resource_type][_id] == dest_resource diff --git a/tests/unit/test_synthetics_global_variables_skip_reconcile.py b/tests/unit/test_synthetics_global_variables_skip_reconcile.py new file mode 100644 index 00000000..687b0c14 --- /dev/null +++ b/tests/unit/test_synthetics_global_variables_skip_reconcile.py @@ -0,0 +1,94 @@ +# Unless explicitly stated otherwise all files in this repository are licensed +# under the 3-clause BSD style license (see LICENSE). +# This product includes software developed at Datadog (https://www.datadoghq.com/). +# Copyright 2019 Datadog, Inc. + +"""synthetics_global_variables: regression guard + framework safety-net proof. + +The exists-path writes state.destination from the map entry then delegates to +update_resource (PUT). The framework is a pure no-op for it; the safety-net test +proves the framework WOULD reconcile this resource if its exists-path regressed. + +All identifiers are obviously synthetic (``gv-src``, ``gv-dst``). +""" + +import asyncio +from collections import defaultdict +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from datadog_sync.model.synthetics_global_variables import SyntheticsGlobalVariables +from datadog_sync.utils.resource_utils import SkipResource + + +def _gv(name, gv_type="global", gv_id=None, value="v"): + return { + "id": gv_id or f"gv-id-{name}", + "name": name, + "type": gv_type, + "value": {"value": value}, + } + + +def _make_gvs(existing_map=None, destination_client=None, source_client=None): + config = MagicMock() + config.destination_client = destination_client or MagicMock() + config.source_client = source_client or MagicMock() + config.state = MagicMock() + config.state.source = defaultdict(dict) + config.state.destination = defaultdict(dict) + config.logger = MagicMock() + gvs = SyntheticsGlobalVariables(config=config) + gvs._existing_resources_map = existing_map or {} + return gvs, config + + +class TestSyntheticsGlobalVariablesSkipReconcile: + def test_existing_resource_create_writes_state_destination(self): + _id = "gv-src" + dest = _gv("gv-dst", gv_id="gv-dst") + put_resp = {"id": "gv-dst", "name": "gv-dst", "type": "global", "value": {}} + destination_client = MagicMock() + destination_client.put = AsyncMock(return_value=put_resp) + gvs, config = _make_gvs( + existing_map={"gv-dst:global": dest}, + destination_client=destination_client, + ) + # Pre-set value so _inject_secret_value skips the source clear-value fetch. + resource = _gv("gv-dst", gv_id=_id) + + asyncio.run(gvs._create_resource(_id, resource)) + + destination_client.put.assert_called_once() + # update_resource does state.destination[_id].update(resp); wrapper rewrites. + assert config.state.destination["synthetics_global_variables"][_id]["id"] == "gv-dst" + + def test_framework_reconciles_if_create_raised_skip(self): + _id = "gv-src" + dest = _gv("gv-dst", gv_id="gv-dst") + gvs, config = _make_gvs(existing_map={"gv-dst:global": dest}) + gvs.create_resource = AsyncMock(side_effect=SkipResource(_id, "synthetics_global_variables", "exists")) + resource = _gv("gv-dst", gv_id=_id) + + with pytest.raises(SkipResource): + asyncio.run(gvs._create_resource(_id, resource)) + + assert config.state.destination["synthetics_global_variables"][_id] == dest + + def test_create_non_existing_still_posts(self): + _id = "gv-src" + posted = _gv("gv-dst", gv_id="gv-dst") + destination_client = MagicMock() + destination_client.post = AsyncMock(return_value=posted) + gvs, config = _make_gvs( + existing_map={}, + destination_client=destination_client, + ) + resource = _gv("gv-src", gv_id=_id) + + out_id, out_r = asyncio.run(gvs.create_resource(_id, resource)) + + destination_client.post.assert_called_once() + assert out_id == _id + assert out_r == posted diff --git a/tests/unit/test_synthetics_tests_skip_reconcile.py b/tests/unit/test_synthetics_tests_skip_reconcile.py new file mode 100644 index 00000000..ef741cf2 --- /dev/null +++ b/tests/unit/test_synthetics_tests_skip_reconcile.py @@ -0,0 +1,101 @@ +# Unless explicitly stated otherwise all files in this repository are licensed +# under the 3-clause BSD style license (see LICENSE). +# This product includes software developed at Datadog (https://www.datadoghq.com/). +# Copyright 2019 Datadog, Inc. + +"""synthetics_tests: regression guard + framework safety-net proof. + +The exists-path writes state.destination from the map entry (keyed by +metadata.disaster_recovery.source_public_id) then delegates to update_resource. +The framework is a pure no-op for it; the safety-net test proves the framework +WOULD reconcile this resource if its exists-path regressed. Internal helpers +(_replicate_files, _update_test, _replace_variable_public_id) are stubbed to +keep the regression guard focused on the state-destination write contract. + +All identifiers are obviously synthetic (``src-pub-id``, ``dst-pub-id``). +""" + +import asyncio +from collections import defaultdict +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from datadog_sync.model.synthetics_tests import SyntheticsTests +from datadog_sync.utils.resource_utils import SkipResource + + +def _test(public_id, source_public_id=None, test_type="browser"): + return { + "public_id": public_id, + "type": test_type, + "name": f"test-{public_id}", + "metadata": {"disaster_recovery": {"source_public_id": source_public_id or public_id}}, + } + + +def _make_synthetics_tests(existing_map=None, destination_client=None): + config = MagicMock() + config.destination_client = destination_client or MagicMock() + config.state = MagicMock() + config.state.source = defaultdict(dict) + config.state.destination = defaultdict(dict) + config.logger = MagicMock() + st = SyntheticsTests(config=config) + st._existing_resources_map = existing_map or {} + # Stub network/complex internals so the regression guard stays focused. + st._replicate_files = AsyncMock() + st._replace_variable_public_id = MagicMock(return_value=False) + return st, config + + +class TestSyntheticsTestsSkipReconcile: + def test_existing_resource_create_writes_state_destination(self): + # _id is "#"; map keyed by source_public_id. + _id = "src-pub-id#config-1" + dest_test = _test("dst-pub-id", source_public_id="src-pub-id") + updated = _test("dst-pub-id", source_public_id="src-pub-id") + updated["marker"] = "updated" + destination_client = MagicMock() + st, config = _make_synthetics_tests( + existing_map={"src-pub-id": dest_test}, + destination_client=destination_client, + ) + st._update_test = AsyncMock(return_value=updated) + resource = _test("src-pub-id", source_public_id="src-pub-id") + resource["name"] = "test-src-pub-id" + + asyncio.run(st._create_resource(_id, resource)) + + st._update_test.assert_called_once() + assert config.state.destination["synthetics_tests"][_id] == updated + + def test_framework_reconciles_if_create_raised_skip(self): + _id = "src-pub-id#config-1" + dest_test = _test("dst-pub-id", source_public_id="src-pub-id") + st, config = _make_synthetics_tests(existing_map={"src-pub-id": dest_test}) + st.create_resource = AsyncMock(side_effect=SkipResource(_id, "synthetics_tests", "exists")) + resource = _test("src-pub-id", source_public_id="src-pub-id") + + with pytest.raises(SkipResource): + asyncio.run(st._create_resource(_id, resource)) + + assert config.state.destination["synthetics_tests"][_id] == dest_test + + def test_create_non_existing_still_posts(self): + _id = "src-pub-id#config-1" + posted = _test("dst-pub-id", source_public_id="src-pub-id") + destination_client = MagicMock() + st, config = _make_synthetics_tests( + existing_map={}, + destination_client=destination_client, + ) + st._create_test = AsyncMock(return_value=posted) + resource = _test("src-pub-id", source_public_id="src-pub-id") + resource["name"] = "test-src-pub-id" + + out_id, out_r = asyncio.run(st.create_resource(_id, resource)) + + st._create_test.assert_called_once() + assert out_id == _id + assert out_r == posted diff --git a/tests/unit/test_team_memberships_skip_reconcile.py b/tests/unit/test_team_memberships_skip_reconcile.py new file mode 100644 index 00000000..030c6f71 --- /dev/null +++ b/tests/unit/test_team_memberships_skip_reconcile.py @@ -0,0 +1,121 @@ +# Unless explicitly stated otherwise all files in this repository are licensed +# under the 3-clause BSD style license (see LICENSE). +# This product includes software developed at Datadog (https://www.datadoghq.com/). +# Copyright 2019 Datadog, Inc. + +"""team_memberships: destination-state reconciliation on the exists-skip path. + +``team_memberships.create_resource`` discovers (via ``_existing_resources_map`` +keyed by ``team_id:user_id``) that a membership already exists on the +destination and raises ``SkipResource("User is already a member of the team")`` +without writing ``state.destination``. Without framework reconciliation, no +destination state file is ever persisted for that id, so downstream consumers +that trust the bucket count the id as failed even though the membership is +confirmed present via the live API. The framework wrapper now reconciles this. + +All identifiers are obviously synthetic (``team-src``, ``team-dst`` ...). +""" + +import asyncio +from collections import defaultdict +from unittest.mock import MagicMock + +import pytest + +from datadog_sync.model.team_memberships import TeamMemberships +from datadog_sync.utils.resource_utils import SkipResource + + +def _membership(team_id, user_id, membership_id=None): + return { + "type": "team_memberships", + "id": membership_id or f"TeamMembership-{team_id}-{user_id}", + "attributes": {"role": "member"}, + "relationships": { + "team": {"data": {"type": "team", "id": team_id}}, + "user": {"data": {"type": "users", "id": user_id}}, + }, + } + + +def _make_team_memberships(existing_map=None, destination_client=None): + config = MagicMock() + config.destination_client = destination_client or MagicMock() + config.state = MagicMock() + config.state.source = defaultdict(dict) + config.state.destination = defaultdict(dict) + config.logger = MagicMock() + tm = TeamMemberships(config=config) + tm._existing_resources_map = existing_map or {} + return tm, config + + +class TestTeamMembershipsSkipReconcile: + def test_create_skip_reconciles_state_destination(self): + # Post-connect shape: source resource already carries *destination* ids. + existing_dest_member = _membership("team-dst", "user-dst", "TeamMembership-team-dst-user-dst") + tm, config = _make_team_memberships( + existing_map={"team-dst:user-dst": existing_dest_member}, + ) + src_resource = _membership("team-dst", "user-dst", "TeamMembership-team-src-user-src") + _id = "TeamMembership-team-src-user-src" + + with pytest.raises(SkipResource, match="already a member"): + asyncio.run(tm._create_resource(_id, src_resource)) + + assert config.state.destination["team_memberships"][_id] == existing_dest_member + + def test_update_delegating_to_create_reconciles(self): + # update_resource with no in-state destination delegates to create_resource, + # which raises SkipResource on the existing membership. The _update_resource + # wrapper must reconcile. + existing_dest_member = _membership("team-dst", "user-dst", "TeamMembership-team-dst-user-dst") + tm, config = _make_team_memberships( + existing_map={"team-dst:user-dst": existing_dest_member}, + ) + src_resource = _membership("team-dst", "user-dst", "TeamMembership-team-src-user-src") + _id = "TeamMembership-team-src-user-src" + + with pytest.raises(SkipResource): + asyncio.run(tm._update_resource(_id, src_resource)) + + assert config.state.destination["team_memberships"][_id] == existing_dest_member + + def test_update_no_diff_skip_does_not_overwrite(self): + # update_resource finds the membership in-state AND in the map with no diff, + # raising SkipResource("No differences detected"). Insert-if-absent must + # leave the pre-existing entry untouched (same object identity), not replace + # it with the live map entry. + existing_dest_member = _membership("team-dst", "user-dst", "TeamMembership-team-dst-user-dst") + sentinel = _membership("team-dst", "user-dst", "TeamMembership-team-dst-user-dst") + tm, config = _make_team_memberships( + existing_map={"team-dst:user-dst": existing_dest_member}, + ) + _id = "TeamMembership-team-src-user-src" + config.state.destination["team_memberships"][_id] = sentinel + src_resource = _membership("team-dst", "user-dst", "TeamMembership-team-src-user-src") + + with pytest.raises(SkipResource, match="No differences detected"): + asyncio.run(tm._update_resource(_id, src_resource)) + + # Insert-if-absent: the framework must not overwrite the pre-existing entry. + assert config.state.destination["team_memberships"][_id] is sentinel + + def test_create_still_posts_when_not_existing(self): + from unittest.mock import AsyncMock + + posted = _membership("team-dst", "user-dst", "TeamMembership-team-dst-user-dst") + destination_client = MagicMock() + destination_client.post = AsyncMock(return_value={"data": posted}) + tm, config = _make_team_memberships( + existing_map={}, + destination_client=destination_client, + ) + src_resource = _membership("team-dst", "user-dst", "TeamMembership-team-src-user-src") + _id = "TeamMembership-team-src-user-src" + + out_id, out_r = asyncio.run(tm.create_resource(_id, src_resource)) + + destination_client.post.assert_called_once() + assert out_id == _id + assert out_r == posted diff --git a/tests/unit/test_teams_skip_reconcile.py b/tests/unit/test_teams_skip_reconcile.py new file mode 100644 index 00000000..51bd2c94 --- /dev/null +++ b/tests/unit/test_teams_skip_reconcile.py @@ -0,0 +1,92 @@ +# Unless explicitly stated otherwise all files in this repository are licensed +# under the 3-clause BSD style license (see LICENSE). +# This product includes software developed at Datadog (https://www.datadoghq.com/). +# Copyright 2019 Datadog, Inc. + +"""teams: regression guard + framework safety-net proof. + +teams' exists-path is already correct: create_resource writes state.destination +from the map entry then delegates to update_resource (PATCH), so it never raises +SkipResource in the exists-path and the framework is a pure no-op. The safety-net +test proves the framework WOULD reconcile teams if its exists-path regressed. + +All identifiers are obviously synthetic (``team-src``, ``team-dst``). +""" + +import asyncio +from collections import defaultdict +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from datadog_sync.model.teams import Teams +from datadog_sync.utils.resource_utils import SkipResource + + +def _team(name, handle, team_id=None): + return { + "id": team_id or f"team-id-{name}", + "type": "team", + "attributes": {"name": name, "handle": handle}, + } + + +def _make_teams(existing_map=None, destination_client=None): + config = MagicMock() + config.destination_client = destination_client or MagicMock() + config.state = MagicMock() + config.state.source = defaultdict(dict) + config.state.destination = defaultdict(dict) + config.logger = MagicMock() + teams = Teams(config=config) + teams._existing_resources_map = existing_map or {} + return teams, config + + +class TestTeamsSkipReconcile: + def test_existing_resource_create_writes_state_destination(self): + _id = "team-src" + dest_team = _team("team-dst", "team-dst-handle", team_id="team-dst") + updated = _team("team-dst", "team-dst-handle", team_id="team-dst") + updated["attributes"]["marker"] = "patched" + destination_client = MagicMock() + destination_client.patch = AsyncMock(return_value={"data": updated}) + teams, config = _make_teams( + existing_map={"team-dst:team-dst-handle": dest_team}, + destination_client=destination_client, + ) + resource = _team("team-dst", "team-dst-handle", team_id=_id) + + asyncio.run(teams._create_resource(_id, resource)) + + destination_client.patch.assert_called_once() + assert config.state.destination["teams"][_id] == updated + + def test_framework_reconciles_if_create_raised_skip(self): + _id = "team-src" + dest_team = _team("team-dst", "team-dst-handle", team_id="team-dst") + teams, config = _make_teams(existing_map={"team-dst:team-dst-handle": dest_team}) + teams.create_resource = AsyncMock(side_effect=SkipResource(_id, "teams", "exists")) + resource = _team("team-dst", "team-dst-handle", team_id=_id) + + with pytest.raises(SkipResource): + asyncio.run(teams._create_resource(_id, resource)) + + assert config.state.destination["teams"][_id] == dest_team + + def test_create_non_existing_still_posts(self): + _id = "team-src" + posted = _team("team-src", "team-src-handle", team_id="team-dst") + destination_client = MagicMock() + destination_client.post = AsyncMock(return_value={"data": posted}) + teams, config = _make_teams( + existing_map={}, + destination_client=destination_client, + ) + resource = _team("team-src", "team-src-handle", team_id=_id) + + out_id, out_r = asyncio.run(teams.create_resource(_id, resource)) + + destination_client.post.assert_called_once() + assert out_id == _id + assert out_r == posted diff --git a/tests/unit/test_users_skip_reconcile.py b/tests/unit/test_users_skip_reconcile.py new file mode 100644 index 00000000..3aa96302 --- /dev/null +++ b/tests/unit/test_users_skip_reconcile.py @@ -0,0 +1,122 @@ +# Unless explicitly stated otherwise all files in this repository are licensed +# under the 3-clause BSD style license (see LICENSE). +# This product includes software developed at Datadog (https://www.datadoghq.com/). +# Copyright 2019 Datadog, Inc. + +"""users: regression guards + framework safety-net proof. + +Users' exists-path is already correct: ``create_resource`` writes +``state.destination`` from the map entry then delegates to ``update_resource`` +(POST->PATCH), so it never raises ``SkipResource`` in the exists-path and the +framework is a pure no-op for it. The "User is disabled" skip is an *import-time* +skip (raised in ``import_resource``), entirely outside the create/update +wrappers, so the framework cannot and should not reconcile it. + +All identifiers are obviously synthetic (``user-src``, ``user-dst@example.com``). +""" + +import asyncio +from collections import defaultdict +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from datadog_sync.model.users import Users +from datadog_sync.utils.resource_utils import SkipResource + + +def _user(handle, email="user@example.com", user_id=None, disabled=False, service_account=False): + return { + "id": user_id or f"user-id-{handle}", + "type": "users", + "attributes": { + "handle": handle, + "email": email, + "name": f"Test {handle}", + "service_account": service_account, + "disabled": disabled, + }, + "relationships": {}, + } + + +def _make_users(existing_map=None, destination_client=None): + config = MagicMock() + config.destination_client = destination_client or MagicMock() + config.state = MagicMock() + config.state.source = defaultdict(dict) + config.state.destination = defaultdict(dict) + config.logger = MagicMock() + users = Users(config=config) + users._existing_resources_map = existing_map or {} + return users, config + + +class TestUsersSkipReconcile: + def test_existing_user_create_delegates_to_update_and_writes(self): + # Exists-path: key in map -> create_resource writes state.destination + # from the map entry, then delegates to update_resource. With no diff, + # update_resource returns the in-state user (no PATCH). The wrapper then + # writes the returned user. Framework insert-if-absent no-ops because + # create_resource already populated state.destination. + _id = "user-src" + dest_user = _user("user-dst", email="user-dst@example.com", user_id="user-dst") + destination_client = MagicMock() + destination_client.patch = AsyncMock() # should NOT be called (no diff) + users, config = _make_users( + existing_map={"user-dst": dest_user}, + destination_client=destination_client, + ) + # Source resource matches the destination user (handle/service_account + # excluded from diff), so update_resource takes the no-diff return path. + resource = _user("user-dst", email="user-dst@example.com", user_id=_id) + + asyncio.run(users._create_resource(_id, resource)) + + destination_client.patch.assert_not_called() + assert config.state.destination["users"][_id] == dest_user + + def test_disabled_user_import_skip_unrelated_to_framework(self): + # "User is disabled." is raised in import_resource (import path), not in + # the create/update wrappers, so the framework never runs. Document the + # boundary: the skip happens, and state.destination is untouched. + users, config = _make_users(existing_map={"user-dst": _user("user-dst", user_id="user-dst")}) + disabled = _user("user-dst", user_id="user-src", disabled=True) + + with pytest.raises(SkipResource, match="User is disabled"): + asyncio.run(users.import_resource(resource=disabled)) + + assert config.state.destination["users"] == {} + + def test_framework_reconciles_if_create_raised_skip(self): + # Safety-net proof: if users.create_resource ever regressed into a + # skip-without-write with the handle present in the map, the framework + # wrapper would reconcile state.destination. + _id = "user-src" + dest_user = _user("user-dst", email="user-dst@example.com", user_id="user-dst") + users, config = _make_users(existing_map={"user-dst": dest_user}) + users.create_resource = AsyncMock(side_effect=SkipResource(_id, "users", "exists")) + resource = _user("user-dst", email="user-dst@example.com", user_id=_id) + + with pytest.raises(SkipResource): + asyncio.run(users._create_resource(_id, resource)) + + assert config.state.destination["users"][_id] == dest_user + + def test_create_non_existing_user_still_posts(self): + _id = "user-src" + # handle == email avoids the v2 email-backfill second call. + posted = _user("user-src", email="user-src", user_id="user-dst") + destination_client = MagicMock() + destination_client.post = AsyncMock(return_value={"data": posted}) + users, config = _make_users( + existing_map={}, + destination_client=destination_client, + ) + resource = _user("user-src", email="user-src", user_id=_id) + + out_id, out_r = asyncio.run(users.create_resource(_id, resource)) + + destination_client.post.assert_called_once() + assert out_id == _id + assert out_r == posted