Fix dashboard list logging and restricted monitor filtering

datadog_sync/commands/shared/options.py

@@ -583,7 +583,8 @@ def click_config_file_provider(ctx: Context, opts: CustomOptionClass, value: Non
         is_flag=True,
         default=False,
         show_default=True,
-        help="Filter out monitors whose source payload has a non-empty restricted_roles list. "
+        help="Filter out monitors whose source payload has a non-empty restricted_roles list "
+        "or restriction_policy bindings. "
         "This is an explicit access-control escape hatch for DDR destinations where "
         "role/user activation is not ready yet; filtered monitors are not created or updated.",
         cls=CustomOptionClass,

datadog_sync/model/dashboard_lists.py

@@ -158,10 +158,10 @@ def _drop_integration_dashboards(self, _id: str, resource: Dict) -> None:
         )
         self.config.logger.info(
             "dropping integration dashboards from dashboard list before sync; "
-            "integration dashboard IDs are not portable across orgs",
+            "integration dashboard IDs are not portable across orgs; "
+            f"dropped_dashboard_ids={','.join(dropped)}",
             resource_type=self.resource_type,
             _id=_id,
-            dropped_dashboard_ids=",".join(dropped),
         )
         resource["dashboards"] = portable_dashboards
 

datadog_sync/model/monitors.py

@@ -39,6 +39,7 @@ def _variables_have_group_by(variables) -> bool:
             return True
     return False
 
+
 if TYPE_CHECKING:
     from datadog_sync.utils.custom_client import CustomClient
 
@@ -182,9 +183,7 @@ async def pre_resource_action_hook(self, _id, resource: Dict) -> None:
                 and not _variables_have_group_by(options.get("variables"))
             ):
                 options.pop("notify_by")
-                self.config.logger.info(
-                    f"monitor {_id}: dropped no-op options.notify_by=['*'] on ungrouped query"
-                )
+                self.config.logger.info(f"monitor {_id}: dropped no-op options.notify_by=['*'] on ungrouped query")
 
         # org: principals are remapped here (before connect_resources runs).
         # user:/role:/team: principals are remapped by connect_id via resource_connections paths.
@@ -243,11 +242,11 @@ def filter(self, resource: Dict) -> bool:
         if not super().filter(resource):
             return False
 
-        if getattr(self.config, "skip_monitors_with_restricted_roles", False) is True and resource.get(
-            "restricted_roles"
+        if getattr(self.config, "skip_monitors_with_restricted_roles", False) is True and _has_access_restrictions(
+            resource
         ):
             self.config.logger.info(
-                "filtering monitor with restricted_roles because --skip-monitors-with-restricted-roles is enabled",
+                "filtering monitor with access restrictions because --skip-monitors-with-restricted-roles is enabled",
                 resource_type=self.resource_type,
                 _id=str(resource.get("id", "")),
             )
@@ -293,9 +292,7 @@ def connect_resources(self, _id: str, resource: Dict) -> ResourceConnectionResul
         empty_risk = empty_risk or roles_risk
 
         return ResourceConnectionResult(
-            empty_binding_escalation=self._raise_connection_error_if_any(
-                _id, failed_connections_dict, empty_risk
-            )
+            empty_binding_escalation=self._raise_connection_error_if_any(_id, failed_connections_dict, empty_risk)
         )
 
     def connect_id(self, key: str, r_obj: Dict, resource_to_connect: str) -> Optional[List[str]]:
@@ -390,6 +387,20 @@ def extract_source_ids(self, key: str, r_obj: Dict, resource_to_connect: str) ->
         return super(Monitors, self).extract_source_ids(key, r_obj, resource_to_connect)
 
 
+def _has_access_restrictions(resource: Dict) -> bool:
+    if resource.get("restricted_roles"):
+        return True
+
+    restriction_policy = resource.get("restriction_policy")
+    if not isinstance(restriction_policy, dict):
+        return False
+
+    for binding in restriction_policy.get("bindings") or []:
+        if isinstance(binding, dict) and binding.get("principals"):
+            return True
+    return False
+
+
 _MONITOR_LOG_QUERY_MAX_CHARS = 2000
 _MONITOR_LOG_REASON_MAX_CHARS = 200
 _MONITOR_APPLICATION_ID_RE = re.compile(r"@application\.id:[A-Za-z0-9\-]+")

tests/unit/test_dashboard_lists.py

@@ -26,11 +26,17 @@ def _make_dashboard_lists() -> DashboardLists:
     return DashboardLists(config)
 
 
+class _SignatureCheckedLogger:
+    def __init__(self):
+        self.info_calls = []
+
+    def info(self, msg: str, *arg, _id: str = "", resource_type: str = "") -> None:
+        self.info_calls.append({"msg": msg, "args": arg, "_id": _id, "resource_type": resource_type})
+
+
 def test_pre_resource_action_hook_drops_integration_dashboards_before_apply():
     dashboard_lists = _make_dashboard_lists()
-    dashboard_lists.config.state.destination["dashboards"]["dash-src"] = {
-        "id": "dash-dst"
-    }
+    dashboard_lists.config.state.destination["dashboards"]["dash-src"] = {"id": "dash-dst"}
     resource = {
         "id": 510887,
         "dashboards": [
@@ -48,11 +54,37 @@ def test_pre_resource_action_hook_drops_integration_dashboards_before_apply():
     ]
 
 
-def test_connect_resources_ignores_unmapped_integration_dashboards():
+def test_drop_integration_dashboards_uses_supported_logger_kwargs():
     dashboard_lists = _make_dashboard_lists()
-    dashboard_lists.config.state.destination["dashboards"]["dash-src"] = {
-        "id": "dash-dst"
+    logger = _SignatureCheckedLogger()
+    dashboard_lists.config.logger = logger
+    resource = {
+        "id": 510887,
+        "dashboards": [
+            {"id": "dash-src", "type": "custom_timeboard"},
+            {"id": "62", "type": "integration_timeboard"},
+            {"id": "30516", "type": "integration_timeboard"},
+        ],
     }
+
+    asyncio.run(dashboard_lists.pre_resource_action_hook("510887", resource))
+
+    assert resource["dashboards"] == [{"id": "dash-src", "type": "custom_timeboard"}]
+    assert logger.info_calls == [
+        {
+            "msg": "dropping integration dashboards from dashboard list before sync; "
+            "integration dashboard IDs are not portable across orgs; "
+            "dropped_dashboard_ids=30516,62",
+            "args": (),
+            "_id": "510887",
+            "resource_type": "dashboard_lists",
+        }
+    ]
+
+
+def test_connect_resources_ignores_unmapped_integration_dashboards():
+    dashboard_lists = _make_dashboard_lists()
+    dashboard_lists.config.state.destination["dashboards"]["dash-src"] = {"id": "dash-dst"}
     resource = {
         "id": 510887,
         "dashboards": [
@@ -125,9 +157,7 @@ def test_update_dash_list_items_drops_integration_dashboards_from_payload():
         dashboard_lists.dash_list_items_path.format("dst-list"),
         {"dashboards": [{"id": "dash-dst", "type": "custom_timeboard"}]},
     )
-    assert dashboard_list == {
-        "dashboards": [{"id": "dash-dst", "type": "custom_timeboard"}]
-    }
+    assert dashboard_list == {"dashboards": [{"id": "dash-dst", "type": "custom_timeboard"}]}
 
 
 def _http_error(status: int) -> CustomClientHTTPError:
@@ -156,30 +186,18 @@ def test_500_on_items_fetch_propagates(self):
         classifies it as http_5xx (transient) — counted as failure, logged
         at WARNING, no exit-code poisoning, no incomplete state written."""
         dashboard_lists = _make_dashboard_lists()
-        dashboard_lists.config.source_client.get = AsyncMock(
-            side_effect=_http_error(500)
-        )
+        dashboard_lists.config.source_client.get = AsyncMock(side_effect=_http_error(500))
 
         with pytest.raises(CustomClientHTTPError):
-            asyncio.run(
-                dashboard_lists.import_resource(
-                    resource={"id": "42", "name": "my-list"}
-                )
-            )
+            asyncio.run(dashboard_lists.import_resource(resource={"id": "42", "name": "my-list"}))
 
     def test_503_on_items_fetch_propagates(self):
         """Any 5xx propagates — same treatment as 500."""
         dashboard_lists = _make_dashboard_lists()
-        dashboard_lists.config.source_client.get = AsyncMock(
-            side_effect=_http_error(503)
-        )
+        dashboard_lists.config.source_client.get = AsyncMock(side_effect=_http_error(503))
 
         with pytest.raises(CustomClientHTTPError):
-            asyncio.run(
-                dashboard_lists.import_resource(
-                    resource={"id": "42", "name": "my-list"}
-                )
-            )
+            asyncio.run(dashboard_lists.import_resource(resource={"id": "42", "name": "my-list"}))
 
     def test_items_fetch_success_populates_dashboards(self):
         """Happy path: items endpoint returns dashboard IDs that are
@@ -193,8 +211,6 @@ async def fake_get(path, **kwargs):
 
         dashboard_lists.config.source_client.get = AsyncMock(side_effect=fake_get)
 
-        _id, resource = asyncio.run(
-            dashboard_lists.import_resource(resource={"id": "42", "name": "my-list"})
-        )
+        _id, resource = asyncio.run(dashboard_lists.import_resource(resource={"id": "42", "name": "my-list"}))
 
         assert resource["dashboards"] == [{"id": "dash-1", "type": "custom_timeboard"}]

tests/unit/test_monitors.py

@@ -143,15 +143,52 @@ def test_restricted_roles_filtered_when_flag_enabled(self):
         assert monitors.filter(resource) is False
         monitors.config.logger.info.assert_called_once()
 
+    def test_restriction_policy_principals_allowed_by_default(self):
+        monitors = self._make_monitors()
+        resource = {
+            "id": 269290576,
+            "restriction_policy": {
+                "bindings": [
+                    {
+                        "relation": "editor",
+                        "principals": ["user:source-user"],
+                    }
+                ]
+            },
+        }
+
+        assert monitors.filter(resource) is True
+
+    def test_restriction_policy_principals_filtered_when_flag_enabled(self):
+        monitors = self._make_monitors(skip_restricted=True)
+        resource = {
+            "id": 269290576,
+            "restriction_policy": {
+                "bindings": [
+                    {
+                        "relation": "editor",
+                        "principals": ["user:source-user"],
+                    }
+                ]
+            },
+        }
+
+        assert monitors.filter(resource) is False
+        monitors.config.logger.info.assert_called_once()
+
     @pytest.mark.parametrize(
         "resource",
         [
             {"id": 1},
             {"id": 2, "restricted_roles": []},
             {"id": 3, "restricted_roles": None},
+            {"id": 4, "restriction_policy": None},
+            {"id": 5, "restriction_policy": {}},
+            {"id": 6, "restriction_policy": {"bindings": []}},
+            {"id": 7, "restriction_policy": {"bindings": [{"relation": "editor", "principals": []}]}},
         ],
     )
-    def test_empty_or_missing_restricted_roles_are_not_filtered(self, resource):
+    def test_empty_or_missing_access_restrictions_are_not_filtered(self, resource):
         monitors = self._make_monitors(skip_restricted=True)
 
         assert monitors.filter(resource) is True
@@ -403,7 +440,7 @@ def test_notify_by_star_preserved_on_log_alert_grouped_query(self):
         resource = {
             "id": 14,
             "type": "log alert",
-            "query": "logs(\"service:foo\").index(\"*\").rollup(\"count\").by(\"host\").last(\"5m\") > 0",
+            "query": 'logs("service:foo").index("*").rollup("count").by("host").last("5m") > 0',
             "options": {"notify_by": ["*"]},
         }
         asyncio.run(monitors.pre_resource_action_hook("14", resource))