From 5d78122cc13813c6f7ae4007af8ba98d951ec765 Mon Sep 17 00:00:00 2001 From: Michael Richey Date: Tue, 1 Sep 2026 15:01:11 -0400 Subject: [PATCH 1/2] feat(import): allow dashboards in --id-file Add 'dashboards' to _ID_FILE_IMPORT_SUPPORTED_TYPES so the import command accepts --id-file payloads selecting specific dashboards by ID. This lets callers split a large dashboards set into multiple sync-cli subprocesses (importing a subset of IDs per invocation) instead of one monolithic run that can time out before finishing on orgs with tens of thousands of dashboards. Dashboards.import_resource(_id=...) already performs a real GET to /api/v1/dashboard/{id} for the full body (widgets are omitted by the LIST endpoint) and short-circuits when the caller passes a body already carrying widgets, so the id-file path's get_resources_by_ids -> import_resource(_id) -> queue-handler _import_resource(resource=body) sequence does exactly one GET per dashboard with no double-fetch. Verified via tests/unit/test_dashboards_id_file.py. Three existing tests used 'dashboards' as their example of an *unsupported* type in --id-file; updated them to 'notebooks' (genuinely unsupported) to preserve their intent. --- datadog_sync/utils/configuration.py | 18 ++- tests/unit/test_dashboards_id_file.py | 149 ++++++++++++++++++ .../test_get_resources_by_ids_experiment.py | 7 +- tests/unit/test_id_file_state_load_scoping.py | 2 +- .../test_id_file_subprocess_experiment.py | 8 +- 5 files changed, 176 insertions(+), 8 deletions(-) create mode 100644 tests/unit/test_dashboards_id_file.py diff --git a/datadog_sync/utils/configuration.py b/datadog_sync/utils/configuration.py index 9934458bf..ae4518944 100644 --- a/datadog_sync/utils/configuration.py +++ b/datadog_sync/utils/configuration.py @@ -253,7 +253,7 @@ def _unwrap_exact_match_pattern(pattern: str) -> str: return _regex_literal_from_exact_match_body(pattern[1:-1]) -_ID_FILE_IMPORT_SUPPORTED_TYPES = frozenset({"monitors", "authn_mappings", "team_memberships"}) +_ID_FILE_IMPORT_SUPPORTED_TYPES = frozenset({"monitors", "authn_mappings", "team_memberships", "dashboards"}) """Resource types eligible for --id-file on the import command. The import path fans out to per-ID GETs via BaseResource.get_resources_by_ids. @@ -264,6 +264,14 @@ def _unwrap_exact_match_pattern(pattern: str) -> str: Widening this set requires per-model verification of the fan-out path. Do NOT widen by config — code-level allowlist forces explicit review. + +dashboards: Dashboards.import_resource(_id=...) performs a real GET to +/api/v1/dashboard/{id} for the full body (widgets are omitted by the LIST +endpoint). The model also short-circuits when the caller passes a body +already carrying widgets, so the id-file path's get_resources_by_ids -> +import_resource(_id=...) -> queue-handler _import_resource(resource=body) +sequence does exactly one GET per dashboard (no double-fetch). Verified via +tests/unit/test_dashboards_id_file.py. """ @@ -273,6 +281,7 @@ def _unwrap_exact_match_pattern(pattern: str) -> str: "team_memberships", "host_tags", "metrics_metadata", + "dashboards", }) """Resource types eligible for --id-file on the sync command with --minimize-reads. @@ -288,6 +297,13 @@ def _unwrap_exact_match_pattern(pattern: str) -> str: - metrics_metadata: state key is metric name; import_resource returns (metric_name, resource). Storage layout: resources/source/metrics_metadata. .json. +- dashboards: state key is the dashboard id (e.g. abc-def-ghi). Storage + layout: resources/source/dashboards..json. ID-derivable, so + State.get_by_ids constructs the correct key. Added alongside the import + allowlist entry so the union (_ID_FILE_SUPPORTED_TYPES) accepts dashboards + on both paths by design rather than incidentally via the import set. + (metric_name, resource). Storage layout: resources/source/metrics_metadata. + .json. Do NOT widen by config — code-level allowlist forces explicit review. """ diff --git a/tests/unit/test_dashboards_id_file.py b/tests/unit/test_dashboards_id_file.py new file mode 100644 index 000000000..ab3a4582a --- /dev/null +++ b/tests/unit/test_dashboards_id_file.py @@ -0,0 +1,149 @@ +# 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. + +"""Tests for dashboards support in _ID_FILE_IMPORT_SUPPORTED_TYPES. + +Pins that 'dashboards' is accepted by --id-file and that the per-ID GET +path used by get_resources_by_ids (import --id-file) does exactly one GET +per dashboard, with the prefetched-body short-circuit preventing a +double-fetch on queue-handler re-entry. +""" + +import asyncio +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from datadog_sync.model.dashboards import Dashboards +from datadog_sync.utils.configuration import ( + _ID_FILE_IMPORT_SUPPORTED_TYPES, + _ID_FILE_SUPPORTED_TYPES, +) +from datadog_sync.utils.resource_utils import CustomClientHTTPError, SkipResource + + +class TestDashboardsIDFileSupport: + """Tests for dashboards support in the --id-file allowlist.""" + + def test_dashboards_in_id_file_import_supported_types(self): + """'dashboards' must be present in _ID_FILE_IMPORT_SUPPORTED_TYPES so + that `import --id-file=- < {"dashboards": [...]}` is accepted.""" + assert "dashboards" in _ID_FILE_IMPORT_SUPPORTED_TYPES, ( + "dashboards must be in _ID_FILE_IMPORT_SUPPORTED_TYPES. " + "If this fails, id-file import support for dashboards is missing." + ) + + def test_dashboards_in_id_file_supported_types_union(self): + """'dashboards' must also be in the union allowlist consulted by + _parse_id_file (rejects unknown types up-front).""" + assert "dashboards" in _ID_FILE_SUPPORTED_TYPES + + def test_dashboards_in_state_load_supported_types(self): + """Adding dashboards to the import allowlist also adds it to the + union (_ID_FILE_SUPPORTED_TYPES), which _parse_id_file consults for + *both* import and sync --minimize-reads. Since the state-load path + scopes by --resources intersection rather than by + _ID_FILE_STATE_LOAD_SUPPORTED_TYPES, dashboards must be explicitly + in the state-load set too — otherwise it's accepted incidentally via + the union without the ID-derivability verification the set exists to + enforce. Dashboards' state key is dashboards..json (ID-derivable), + so it qualifies.""" + from datadog_sync.utils.configuration import _ID_FILE_STATE_LOAD_SUPPORTED_TYPES + + assert "dashboards" in _ID_FILE_STATE_LOAD_SUPPORTED_TYPES, ( + "dashboards is in _ID_FILE_IMPORT_SUPPORTED_TYPES, so the union " + "accepts it on the sync state-load path too; it must be explicitly " + "in _ID_FILE_STATE_LOAD_SUPPORTED_TYPES (state key is ID-derivable)." + ) + + def test_import_resource_id_does_real_get(self): + """import_resource(_id=...) performs a GET to /api/v1/dashboard/{id} + and returns the body — the per-ID fan-out path used by + get_resources_by_ids on id-file import runs.""" + mock_config = MagicMock() + mock_client = AsyncMock() + body = { + "id": "abc-def-ghi", + "title": "test-dashboard", + "widgets": [{"definition": {"type": "timeseries"}}], + } + mock_client.get.return_value = body + mock_config.source_client = mock_client + dashboards = Dashboards(mock_config) + + _id, resource = asyncio.run(dashboards.import_resource(_id="abc-def-ghi")) + + mock_client.get.assert_awaited_once() + call_path = mock_client.get.call_args[0][0] + assert call_path == "/api/v1/dashboard/abc-def-ghi", ( + f"import_resource(_id=...) must GET /api/v1/dashboard/{{id}}; got {call_path!r}" + ) + assert _id == "abc-def-ghi" + assert resource == body + + def test_import_resource_id_short_circuits_when_caller_supplies_full_body(self): + """When the queue handler re-enters import_resource with the body + already fetched by get_resources_by_ids (detected by 'widgets' + presence), the model must NOT issue a second GET — otherwise id-file + import doubles rate-limit pressure on /api/v1/dashboard/{id}.""" + mock_config = MagicMock() + mock_client = AsyncMock() + mock_config.source_client = mock_client + dashboards = Dashboards(mock_config) + full_body = { + "id": "abc-def-ghi", + "title": "prefetched", + "widgets": [{"definition": {"type": "timeseries"}}], + } + + _id, resource = asyncio.run(dashboards.import_resource(resource=full_body)) + + mock_client.get.assert_not_awaited() + assert _id == "abc-def-ghi" + assert resource == full_body + + def test_import_resource_id_403_raises_skip_resource(self): + """A 403 on the per-ID GET raises SkipResource so the id-file path + buckets it as 'skipped' (matching get_resources_by_ids' handling) + rather than aborting the whole import.""" + mock_config = MagicMock() + mock_client = AsyncMock() + resp = MagicMock() + resp.status = 403 + mock_client.get.side_effect = CustomClientHTTPError(resp, message="Forbidden") + mock_config.source_client = mock_client + dashboards = Dashboards(mock_config) + + with pytest.raises(SkipResource): + asyncio.run(dashboards.import_resource(_id="abc-def-ghi")) + + def test_import_resource_id_propagates_non_403_errors(self): + """A 500 on the per-ID GET propagates (get_resources_by_ids classifies + 5xx as transient and retries; the model must not swallow it).""" + mock_config = MagicMock() + mock_client = AsyncMock() + resp = MagicMock() + resp.status = 500 + mock_client.get.side_effect = CustomClientHTTPError(resp, message="boom") + mock_config.source_client = mock_client + dashboards = Dashboards(mock_config) + + with pytest.raises(CustomClientHTTPError): + asyncio.run(dashboards.import_resource(_id="abc-def-ghi")) + + def test_import_resource_id_without_explicit_id_uses_resource_id(self): + """import_resource() with only a resource dict derives the id from + resource['id'] (the legacy list-path shape) — pins that both call + shapes produce a consistent _id.""" + mock_config = MagicMock() + mock_client = AsyncMock() + body = {"id": "abc-def-ghi", "title": "t", "widgets": []} + mock_client.get.return_value = body + mock_config.source_client = mock_client + dashboards = Dashboards(mock_config) + + _id, _ = asyncio.run(dashboards.import_resource(_id=None, resource={"id": "abc-def-ghi"})) + + assert _id == "abc-def-ghi" diff --git a/tests/unit/test_get_resources_by_ids_experiment.py b/tests/unit/test_get_resources_by_ids_experiment.py index e669c4bc3..cc47b8b5c 100644 --- a/tests/unit/test_get_resources_by_ids_experiment.py +++ b/tests/unit/test_get_resources_by_ids_experiment.py @@ -318,9 +318,10 @@ def test_id_file_allowlist_rejects_unsupported_type(tmp_path, monkeypatch): from datadog_sync.utils.configuration import _parse_id_file from unittest.mock import MagicMock - # Write a payload with an unsupported type + # Write a payload with an unsupported type ('notebooks' is not in + # _ID_FILE_SUPPORTED_TYPES; 'dashboards' is now supported). payload_path = tmp_path / "ids.json" - payload_path.write_text(json.dumps({"dashboards": ["abc-def-ghi"]})) + payload_path.write_text(json.dumps({"notebooks": ["abc-def-ghi"]})) logger = MagicMock() # _parse_id_file calls sys.exit(1) on validation failure. Patch to raise instead. @@ -329,7 +330,7 @@ def test_id_file_allowlist_rejects_unsupported_type(tmp_path, monkeypatch): assert excinfo.value.code == 1 # logger.error should have been called with a message naming the unsupported type error_calls = [str(call) for call in logger.error.call_args_list] - assert any("dashboards" in c for c in error_calls), error_calls + assert any("notebooks" in c for c in error_calls), error_calls def test_id_file_allowlist_accepts_monitors(tmp_path): diff --git a/tests/unit/test_id_file_state_load_scoping.py b/tests/unit/test_id_file_state_load_scoping.py index 09b1df626..1ae70cad8 100644 --- a/tests/unit/test_id_file_state_load_scoping.py +++ b/tests/unit/test_id_file_state_load_scoping.py @@ -134,7 +134,7 @@ def test_unsupported_type_still_rejected(self, tmp_path): from datadog_sync.utils.configuration import _parse_id_file import logging - payload_path = _write_payload(tmp_path, {"dashboards": ["dash-1"]}) + payload_path = _write_payload(tmp_path, {"notebooks": ["nb-1"]}) with pytest.raises(SystemExit): _parse_id_file(str(payload_path), logging.getLogger("test")) diff --git a/tests/unit/test_id_file_subprocess_experiment.py b/tests/unit/test_id_file_subprocess_experiment.py index e8ecb629f..0c836adfe 100644 --- a/tests/unit/test_id_file_subprocess_experiment.py +++ b/tests/unit/test_id_file_subprocess_experiment.py @@ -390,7 +390,9 @@ def test_subprocess_stdin_payload_parsed(tmp_path): @pytest.mark.experiment_subprocess def test_subprocess_unsupported_type_in_id_file(tmp_path): """--id-file with unsupported type errors at config-build.""" - payload = json.dumps({"dashboards": ["abc-def-ghi"]}) + # 'notebooks' is not in _ID_FILE_IMPORT_SUPPORTED_TYPES; 'dashboards' is + # now supported, so it can no longer serve as the unsupported-type example. + payload = json.dumps({"notebooks": ["abc-def-ghi"]}) source_dir = tmp_path / "source" # Use any URL — the subprocess shouldn't even reach the network. rc, stdout, stderr = _run_sync_cli( @@ -402,8 +404,8 @@ def test_subprocess_unsupported_type_in_id_file(tmp_path): assert rc == 1, f"expected exit 1, got {rc}\nSTDERR:\n{stderr.decode(errors='replace')}" combined = (stdout + stderr).decode(errors="replace") assert ( - "dashboards" in combined and "not supported" in combined.lower() - ), f"expected error message mentioning 'dashboards' and 'not supported', got:\n{combined}" + "notebooks" in combined and "not supported" in combined.lower() + ), f"expected error message mentioning 'notebooks' and 'not supported', got:\n{combined}" # --- --id-file requires --resources --- From e772c46b2e9f16644989100f3153999e37baa2ee Mon Sep 17 00:00:00 2001 From: Michael Richey Date: Tue, 1 Sep 2026 15:42:47 -0400 Subject: [PATCH 2/2] docs(id-file): clarify dashboard sync scope --- datadog_sync/utils/configuration.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/datadog_sync/utils/configuration.py b/datadog_sync/utils/configuration.py index ae4518944..ef7c8ffc5 100644 --- a/datadog_sync/utils/configuration.py +++ b/datadog_sync/utils/configuration.py @@ -112,12 +112,10 @@ class Configuration(object): allow_partial_permissions_roles: List[str] = field(default_factory=list) resources: Dict[str, BaseResource] = field(default_factory=dict) resources_arg: List[str] = field(default_factory=list) - # --id-file: id-targeted import via stdin or file payload. - # Supported resource types are explicitly allowlisted in - # _ID_FILE_SUPPORTED_TYPES (currently monitors, authn_mappings, - # team_memberships). Other types use the legacy list-everything path. - # Future expansion (e.g. SLOs) is mechanical via - # BaseResource.get_resources_by_ids inheritance. + # --id-file: id-targeted import and sync state-load scoping via stdin or + # file payload. Supported resource types are explicitly allowlisted in + # _ID_FILE_SUPPORTED_TYPES; runtime paths narrow that union to the + # command-specific import and state-load sets. id_payload: Optional[Dict[str, List[str]]] = None max_concurrent_reads: int = 30 transient_failure_threshold_pct: int = 5 @@ -302,8 +300,6 @@ def _unwrap_exact_match_pattern(pattern: str) -> str: State.get_by_ids constructs the correct key. Added alongside the import allowlist entry so the union (_ID_FILE_SUPPORTED_TYPES) accepts dashboards on both paths by design rather than incidentally via the import set. - (metric_name, resource). Storage layout: resources/source/metrics_metadata. - .json. Do NOT widen by config — code-level allowlist forces explicit review. """