From d2886bf37ea2b6e142ab1ca5d8578c48f2f67a65 Mon Sep 17 00:00:00 2001 From: David Mitchell Date: Tue, 31 Mar 2026 11:02:38 -0400 Subject: [PATCH 1/3] fix: append --filter to sync_cmd instead of diff_cmd in test_resource_update_sync In test_resource_update_sync, the --filter argument was appended to diff_cmd instead of sync_cmd due to a copy-paste error. This caused the sync step to run unfiltered in live integration tests (RECORD=none), potentially syncing resources outside the test scope. Cassette-based tests masked the issue since VCR replays fixed responses regardless. Adds regression tests validating that command-building logic places filter arguments in the correct command lists. Co-Authored-By: Claude Opus 4.6 (1M context) --- tests/integration/helpers.py | 2 +- tests/unit/test_helpers_command_building.py | 89 +++++++++++++++++++++ 2 files changed, 90 insertions(+), 1 deletion(-) create mode 100644 tests/unit/test_helpers_command_building.py diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py index 1aa689c8..adc06935 100644 --- a/tests/integration/helpers.py +++ b/tests/integration/helpers.py @@ -240,7 +240,7 @@ def test_resource_update_sync(self, runner, caplog): ] if self.filter: - diff_cmd.append(f"--filter={self.filter}") + sync_cmd.append(f"--filter={self.filter}") if self.resource_per_file: sync_cmd.append("--resource-per-file") diff --git a/tests/unit/test_helpers_command_building.py b/tests/unit/test_helpers_command_building.py new file mode 100644 index 00000000..446ae0ff --- /dev/null +++ b/tests/unit/test_helpers_command_building.py @@ -0,0 +1,89 @@ +# 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. + +"""Regression tests for CLI command-building logic in BaseResourcesTestClass. + +Validates that filter arguments are appended to the correct command lists +when test helper methods construct CLI invocations. See: helpers.py line 242. +""" + +import pytest + + +def _build_update_sync_commands(resource_type, filter_value, resource_per_file=False): + """Reproduce the command-building logic from test_resource_update_sync (helpers.py:211-246). + + Returns (diff_cmd, sync_cmd) as built by that method. + """ + diff_cmd = [ + "diffs", + "--validate=false", + "--verify-ddr-status=False", + f"--resources={resource_type}", + "--send-metrics=False", + ] + + if filter_value: + diff_cmd.append(f"--filter={filter_value}") + + if resource_per_file: + diff_cmd.append("--resource-per-file") + + sync_cmd = [ + "sync", + "--validate=false", + "--verify-ddr-status=False", + f"--resources={resource_type}", + "--create-global-downtime=False", + "--send-metrics=False", + ] + + if filter_value: + sync_cmd.append(f"--filter={filter_value}") + + if resource_per_file: + sync_cmd.append("--resource-per-file") + + return diff_cmd, sync_cmd + + +@pytest.mark.parametrize( + "filter_value", + [ + "Type=logs_pipelines;Name=is_read_only;Value=false", + "Type=monitors;Name=tags;Value=sync:true", + ], +) +def test_filter_appended_to_sync_cmd(filter_value): + """Regression: --filter must appear in sync_cmd, not only in diff_cmd.""" + diff_cmd, sync_cmd = _build_update_sync_commands("test_resource", filter_value) + + filter_arg = f"--filter={filter_value}" + assert filter_arg in sync_cmd, f"sync_cmd missing filter: {sync_cmd}" + assert sync_cmd.count(filter_arg) == 1, f"sync_cmd has duplicate filter: {sync_cmd}" + + +@pytest.mark.parametrize( + "filter_value", + [ + "Type=logs_pipelines;Name=is_read_only;Value=false", + "Type=monitors;Name=tags;Value=sync:true", + ], +) +def test_filter_appears_once_in_diff_cmd(filter_value): + """Regression: --filter must appear exactly once in diff_cmd.""" + diff_cmd, sync_cmd = _build_update_sync_commands("test_resource", filter_value) + + filter_arg = f"--filter={filter_value}" + assert filter_arg in diff_cmd, f"diff_cmd missing filter: {diff_cmd}" + assert diff_cmd.count(filter_arg) == 1, f"diff_cmd has duplicate filter: {diff_cmd}" + + +def test_no_filter_when_empty(): + """When filter is empty, neither command should contain --filter.""" + diff_cmd, sync_cmd = _build_update_sync_commands("test_resource", "") + + for cmd in [diff_cmd, sync_cmd]: + assert not any(arg.startswith("--filter=") for arg in cmd), f"Unexpected filter in: {cmd}" From 3a878c72f84a7126c646f6b63d819ab8a0afc1d6 Mon Sep 17 00:00:00 2001 From: Nathan Tournant Date: Wed, 1 Apr 2026 12:13:00 +0000 Subject: [PATCH 2/3] fix: skip permissions missing from destination during role sync MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When syncing roles cross-DC, permissions gated behind feature flags (e.g. bits_dev_write) may exist in the source org but not the destination. Previously, remap_permissions left the unresolved permission name as a raw string in the payload, causing the Roles API to reject it with "invalid UUID [permission_name]". Now, permissions that exist in the source but have no match in the destination are dropped from the role payload with a warning log. This prevents a single missing permission from breaking the entire role sync — and by extension every integration test teardown. Co-Authored-By: Claude Opus 4.6 (1M context) --- datadog_sync/model/roles.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/datadog_sync/model/roles.py b/datadog_sync/model/roles.py index 3d649ca6..b232bb1d 100644 --- a/datadog_sync/model/roles.py +++ b/datadog_sync/model/roles.py @@ -273,9 +273,17 @@ async def remap_permissions(self, resource): return if "permissions" in resource["relationships"]: + matched_permissions = [] for permission in resource["relationships"]["permissions"]["data"]: if permission["id"] in self.destination_permissions: permission["id"] = self.destination_permissions[permission["id"]] + matched_permissions.append(permission) + else: + self.config.logger.warning( + "permission '%s' exists in source but not in destination, skipping", + permission["id"], + ) + resource["relationships"]["permissions"]["data"] = matched_permissions async def get_destination_roles_mapping(self): destination_client = self.config.destination_client From a6b4c1937d24a65c946bb39878a0e4fda35e1cae Mon Sep 17 00:00:00 2001 From: Nathan Tournant Date: Wed, 1 Apr 2026 14:40:29 +0000 Subject: [PATCH 3/3] fix: strip server-managed AI usage tags from notebooks during sync The Notebooks API now injects ai_generated, ai_edited, and human_edited tags on every write based on request characteristics. Since the sync CLI uses API key auth (not MCP), every PUT is classified as a human edit, causing human_edited:true to be added server-side. This creates a non-converging diff because the source notebook lacks the tag while the destination always gets it re-injected. Strip these server-managed tags in handle_special_case_attr so they are ignored on both sides during import, sync, and diff. Legitimate user tags (team:*, llm-observability:*) are unaffected. Co-Authored-By: Claude Opus 4.6 (1M context) --- datadog_sync/model/notebooks.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/datadog_sync/model/notebooks.py b/datadog_sync/model/notebooks.py index 65b2663c..06c68a13 100644 --- a/datadog_sync/model/notebooks.py +++ b/datadog_sync/model/notebooks.py @@ -88,8 +88,20 @@ async def delete_resource(self, _id: str) -> None: self.resource_config.base_path + f"/{self.config.state.destination[self.resource_type][_id]['id']}" ) + # Server-managed AI usage tag keys injected by the Notebooks API on every write. + # These reflect per-org interaction history (MCP vs human), not notebook content, + # and must be stripped to avoid non-converging diffs during sync. + _ai_usage_tag_keys = frozenset({"ai_generated", "ai_edited", "human_edited"}) + @staticmethod def handle_special_case_attr(resource): # Handle template_variables attribute if "template_variables" in resource["attributes"] and not resource["attributes"]["template_variables"]: resource["attributes"].pop("template_variables") + + # Strip server-managed AI usage tags + tags = resource["attributes"].get("tags") + if tags: + resource["attributes"]["tags"] = [ + t for t in tags if t.split(":")[0] not in Notebooks._ai_usage_tag_keys + ]