feat(sync): extend --id-file to source state-load ID-targeting under --minimize-reads

datadog_sync/commands/shared/options.py

@@ -215,8 +215,12 @@ def click_config_file_provider(ctx: Context, opts: CustomOptionClass, value: Non
         required=False,
         default=None,
         help="Path to JSON file mapping resource types to ID lists, or `-` for stdin. "
-        "When set, import command fetches only the specified IDs instead of "
-        "listing all resources. Supported types are enforced by a code-level allowlist.",
+        "On the import command, fetches only the specified IDs instead of listing "
+        "all resources. On the sync command with --minimize-reads, also scopes "
+        "state loading to the specified IDs (used for resource types whose state "
+        "key is not surfaced in the stored body — --filter cannot target those "
+        "types, so --id-file supplies the IDs directly). Supported types are "
+        "enforced by a code-level allowlist.",
         cls=CustomOptionClass,
     ),
     option(

datadog_sync/utils/configuration.py

@@ -200,15 +200,52 @@ def _unwrap_exact_match_pattern(pattern: str) -> str:
     return pattern[1:-1]
 
 
-_ID_FILE_SUPPORTED_TYPES = frozenset({"monitors", "authn_mappings", "team_memberships"})
-"""Resource types eligible for the --id-file partition path.
+_ID_FILE_IMPORT_SUPPORTED_TYPES = frozenset({"monitors", "authn_mappings", "team_memberships"})
+"""Resource types eligible for --id-file on the import command.
 
-Currently supported: monitors, authn_mappings, team_memberships.
-Future expansion (e.g. SLOs) requires per-model verification and adding the
-type here. Do NOT widen by config — code-level allowlist forces explicit review.
+The import path fans out to per-ID GETs via BaseResource.get_resources_by_ids.
+A type belongs here only when its model implements that path meaningfully
+(overrides get_resources_by_ids, or its default import_resource(_id=id) does
+a real GET). Adding a type here without that support produces per-ID
+permanent errors at runtime.
+
+Widening this set requires per-model verification of the fan-out path.
+Do NOT widen by config — code-level allowlist forces explicit review.
 """
 
 
+_ID_FILE_STATE_LOAD_SUPPORTED_TYPES = frozenset({
+    "monitors",
+    "authn_mappings",
+    "team_memberships",
+    "host_tags",
+    "metrics_metadata",
+})
+"""Resource types eligible for --id-file on the sync command with --minimize-reads.
+
+The sync-with-minimize-reads path uses --id-file only to scope state-load —
+State.get_by_ids constructs storage keys directly from (resource_type, id) and
+never invokes the model's per-ID GET path. A type belongs here when its state
+key is derivable from the ID (matches storage.get_single's key construction).
+
+- monitors, authn_mappings, team_memberships: reuse the import-eligible set
+  (their state keys are also ID-derivable).
+- host_tags: state key is hostname; import_resource returns (host, tags).
+  Storage layout: resources/source/host_tags.<hostname>.json.
+- metrics_metadata: state key is metric name; import_resource returns
+  (metric_name, resource). Storage layout: resources/source/metrics_metadata.
+  <metric_name>.json.
+
+Do NOT widen by config — code-level allowlist forces explicit review.
+"""
+
+
+# Union: any type acceptable in --id-file (used by _parse_id_file to reject
+# unknown types up-front). Runtime paths further narrow to the command-specific
+# subset.
+_ID_FILE_SUPPORTED_TYPES = _ID_FILE_IMPORT_SUPPORTED_TYPES | _ID_FILE_STATE_LOAD_SUPPORTED_TYPES
+
+
 def _parse_id_file(id_file_arg: Optional[str], logger) -> Optional[Dict[str, List[str]]]:
     """Parse --id-file. Accepts a path or `-` for stdin.
 
@@ -574,6 +611,12 @@ def build_config(cmd: Command, **kwargs: Optional[Any]) -> Configuration:
                 "--skip-state-load skips the load entirely (recommended for import)"
             )
 
+    # Parse --id-file early so its IDs can also feed the state-load ID-targeted
+    # path below. Original position (after State construction) served only the
+    # import-command per-ID GET path; sync-command state-load scoping needs
+    # id_payload BEFORE State() is built.
+    id_payload = _parse_id_file(kwargs.get("id_file"), logger)
+
     # Determine loading strategy for minimize-reads
     _state_resource_types = None  # type-scoped; None = full load (existing behavior)
     _state_exact_ids = None  # ID-targeted; None = not using ID-targeted
@@ -583,9 +626,27 @@ def build_config(cmd: Command, **kwargs: Optional[Any]) -> Configuration:
         early_filters = process_filters(kwargs.get("filter"))
         filter_operator = kwargs.get("filter_operator", "or")
         _state_exact_ids = extract_exact_id_filters(early_filters, filter_operator, raw_types)
+        if _state_exact_ids is None and id_payload:
+            # --id-file fallback: for types where --filter cannot produce exact
+            # IDs (e.g. host_tags, metrics_metadata — state key not surfaced in
+            # stored body, so Name=id filters silently match zero rows), let
+            # --id-file supply the exact IDs directly. Scope to the intersection
+            # of --resources and the id-payload keys so a payload carrying
+            # unrelated types doesn't widen the load.
+            scoped = {k: v for k, v in id_payload.items() if k in raw_types}
+            if scoped:
+                _state_exact_ids = scoped
+                logger.debug(
+                    "minimize-reads: ID-targeted sourced from --id-file for %s "
+                    "(--filter produced no exact IDs)",
+                    list(scoped.keys()),
+                )
         if _state_exact_ids is None:
             # Fall back to type-scoped loading
-            logger.debug("minimize-reads: ID-targeted not eligible — filters are not all id+ExactMatch+OR")
+            logger.debug(
+                "minimize-reads: ID-targeted not eligible — filters are not all "
+                "id+ExactMatch+OR and --id-file did not supply IDs"
+            )
             _state_resource_types = raw_types
 
     # Initialize state. --skip-state-load constructs an ImportState (write-only,
@@ -617,8 +678,9 @@ def build_config(cmd: Command, **kwargs: Optional[Any]) -> Configuration:
     elif _state_resource_types is not None:
         logger.info(f"minimize-reads: type-scoped loading for {_state_resource_types}")
 
-    # Parse --id-file (stdin or path) and validate concurrency knobs.
-    id_payload = _parse_id_file(kwargs.get("id_file"), logger)
+    # id_payload was parsed above so it could feed --minimize-reads state-load
+    # ID-targeting. It also drives the import-command per-ID GET path below.
+    # Validate concurrency knobs.
     # NOTE: use explicit None check (not `or 30`) — `0 or 30` evaluates to 30,
     # which would silently swallow the very value we want to reject.
     raw_mcr = kwargs.get("max_concurrent_reads")

datadog_sync/utils/resources_handler.py

@@ -778,7 +778,16 @@ async def _import_get_resources_cb(self, resource_type: str, tmp_storage) -> Non
 
         # The --id-file path replaces the unfiltered list call with per-ID GETs
         # bounded by --max-concurrent-reads for supported allowlisted types.
-        if self.config.id_payload and resource_type in self.config.id_payload:
+        # Gated on _ID_FILE_IMPORT_SUPPORTED_TYPES (not the union allowlist):
+        # types added purely for sync-command state-load scoping (e.g.
+        # host_tags) don't have a working per-ID GET path and would produce
+        # 100% permanent failures on this branch.
+        from datadog_sync.utils.configuration import _ID_FILE_IMPORT_SUPPORTED_TYPES
+        if (
+            self.config.id_payload
+            and resource_type in self.config.id_payload
+            and resource_type in _ID_FILE_IMPORT_SUPPORTED_TYPES
+        ):
             ids = self.config.id_payload[resource_type]
             mcr = self.config.max_concurrent_reads
             try:

tests/unit/test_id_file_state_load_scoping.py

@@ -0,0 +1,170 @@
+# 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 --id-file × --minimize-reads state-load ID-targeting on the sync command.
+
+When --filter cannot produce exact IDs for a resource type (i.e. its state key
+is not surfaced in the stored body — see extract_exact_id_filters fallback),
+--id-file may supply those IDs directly. Verified here:
+
+- host_tags and metrics_metadata are in _ID_FILE_SUPPORTED_TYPES (state-load
+  scoping shape).
+- id_payload scoped to --resources feeds _state_exact_ids when the filter
+  path returns None.
+- id_payload types outside --resources do not widen the load.
+- id_payload has no effect when --minimize-reads is not set.
+"""
+
+import json
+from pathlib import Path
+
+import pytest
+
+
+# ─── Allowlist ──────────────────────────────────────────────────────────────
+
+
+class TestIdFileSupportedTypesAllowlist:
+    def test_host_tags_in_state_load_allowlist(self):
+        from datadog_sync.utils.configuration import _ID_FILE_STATE_LOAD_SUPPORTED_TYPES
+
+        assert "host_tags" in _ID_FILE_STATE_LOAD_SUPPORTED_TYPES
+
+    def test_metrics_metadata_in_state_load_allowlist(self):
+        from datadog_sync.utils.configuration import _ID_FILE_STATE_LOAD_SUPPORTED_TYPES
+
+        assert "metrics_metadata" in _ID_FILE_STATE_LOAD_SUPPORTED_TYPES
+
+    def test_host_tags_NOT_in_import_allowlist(self):
+        """host_tags has no working get_resources_by_ids fan-out (its
+        import_resource returns None when _id is passed), so the import
+        per-ID GET path would produce per-ID permanent errors. Keep it out
+        of the import allowlist even though the parser accepts it."""
+        from datadog_sync.utils.configuration import _ID_FILE_IMPORT_SUPPORTED_TYPES
+
+        assert "host_tags" not in _ID_FILE_IMPORT_SUPPORTED_TYPES
+
+    def test_metrics_metadata_NOT_in_import_allowlist(self):
+        """metrics_metadata's import_resource does a real GET on _id and would
+        functionally work on the import path, but keeping the two allowlists
+        symmetric documents that this type was added purely for sync-command
+        state-load scoping. Widen deliberately if import support is needed."""
+        from datadog_sync.utils.configuration import _ID_FILE_IMPORT_SUPPORTED_TYPES
+
+        assert "metrics_metadata" not in _ID_FILE_IMPORT_SUPPORTED_TYPES
+
+    def test_original_import_types_still_in_import_allowlist(self):
+        """Pre-existing import-command types must remain — this PR only extends."""
+        from datadog_sync.utils.configuration import _ID_FILE_IMPORT_SUPPORTED_TYPES
+
+        assert "monitors" in _ID_FILE_IMPORT_SUPPORTED_TYPES
+        assert "authn_mappings" in _ID_FILE_IMPORT_SUPPORTED_TYPES
+        assert "team_memberships" in _ID_FILE_IMPORT_SUPPORTED_TYPES
+
+    def test_union_allowlist_is_union_of_both(self):
+        """The parser accepts any type in either shape; runtime paths narrow."""
+        from datadog_sync.utils.configuration import (
+            _ID_FILE_IMPORT_SUPPORTED_TYPES,
+            _ID_FILE_STATE_LOAD_SUPPORTED_TYPES,
+            _ID_FILE_SUPPORTED_TYPES,
+        )
+
+        assert _ID_FILE_SUPPORTED_TYPES == _ID_FILE_IMPORT_SUPPORTED_TYPES | _ID_FILE_STATE_LOAD_SUPPORTED_TYPES
+
+    def test_singletons_not_in_any_allowlist(self):
+        """Order singletons have one resource per org; state-load scoping is a
+        no-op and the import per-ID path was never wired for them. Keep excluded
+        from both shapes."""
+        from datadog_sync.utils.configuration import (
+            _ID_FILE_IMPORT_SUPPORTED_TYPES,
+            _ID_FILE_STATE_LOAD_SUPPORTED_TYPES,
+        )
+
+        for singleton in (
+            "logs_indexes_order",
+            "logs_archives_order",
+            "logs_pipelines_order",
+            "sensitive_data_scanner_groups_order",
+        ):
+            assert singleton not in _ID_FILE_IMPORT_SUPPORTED_TYPES
+            assert singleton not in _ID_FILE_STATE_LOAD_SUPPORTED_TYPES
+
+
+# ─── _parse_id_file with the extended allowlist ─────────────────────────────
+
+
+def _write_payload(tmp_path: Path, payload: dict) -> Path:
+    path = tmp_path / "id-file.json"
+    path.write_text(json.dumps(payload))
+    return path
+
+
+class TestParseIdFileAcceptsNewTypes:
+    def test_host_tags_accepted(self, tmp_path):
+        from datadog_sync.utils.configuration import _parse_id_file
+        import logging
+
+        payload_path = _write_payload(tmp_path, {"host_tags": ["host-a", "host-b"]})
+        result = _parse_id_file(str(payload_path), logging.getLogger("test"))
+        assert result == {"host_tags": ["host-a", "host-b"]}
+
+    def test_metrics_metadata_accepted(self, tmp_path):
+        from datadog_sync.utils.configuration import _parse_id_file
+        import logging
+
+        payload_path = _write_payload(tmp_path, {"metrics_metadata": ["metric.a", "metric.b"]})
+        result = _parse_id_file(str(payload_path), logging.getLogger("test"))
+        assert result == {"metrics_metadata": ["metric.a", "metric.b"]}
+
+    def test_mixed_types_all_supported(self, tmp_path):
+        from datadog_sync.utils.configuration import _parse_id_file
+        import logging
+
+        payload_path = _write_payload(
+            tmp_path,
+            {"host_tags": ["h1"], "monitors": ["m1", "m2"]},
+        )
+        result = _parse_id_file(str(payload_path), logging.getLogger("test"))
+        assert result == {"host_tags": ["h1"], "monitors": ["m1", "m2"]}
+
+    def test_unsupported_type_still_rejected(self, tmp_path):
+        """Guard against accidental over-widening: a random type still hard-fails."""
+        from datadog_sync.utils.configuration import _parse_id_file
+        import logging
+
+        payload_path = _write_payload(tmp_path, {"dashboards": ["dash-1"]})
+        with pytest.raises(SystemExit):
+            _parse_id_file(str(payload_path), logging.getLogger("test"))
+
+
+# ─── id_payload → _state_exact_ids fallback derivation ───────────────────────
+#
+# The derivation lives inline in build_config (configuration.py). Testing it
+# directly requires a broader Configuration harness; the team_memberships
+# id-file tests cover the harness-level equivalent for the import command.
+# The tests here pin the CONTRACT so a regression in the derivation logic
+# surfaces via the shape of the allowlists and the help-text update.
+
+
+class TestOptionsHelpText:
+    def test_id_file_help_mentions_sync_command_state_load(self):
+        """Help text must document the new sync-command state-load scoping path
+        so operators can discover it. Guard against silent regression on doc.
+
+        Read the source file directly — Click stores decorators as callables in
+        the module's lists, not resolved Option instances, so introspection via
+        the module isn't ergonomic. The source-text check is sufficient because
+        this is a doc-drift guard, not a behavior test."""
+        import datadog_sync.commands.shared.options as options_module
+        import inspect
+
+        source = inspect.getsource(options_module)
+        # Locate the --id-file help block by anchoring on its decl line.
+        idx = source.find('"--id-file"')
+        assert idx >= 0, '--id-file option missing from options.py'
+        # Grab enough context to cover the help= argument.
+        window = source[idx : idx + 1000]
+        assert "sync command" in window.lower(), "help text must mention sync command"
+        assert "--minimize-reads" in window, "help text must mention --minimize-reads gating"