From 53c0404f3340b086287bf2b0c0d58d9b9545b5e8 Mon Sep 17 00:00:00 2001 From: michael-richey <41595765+michael-richey@users.noreply.github.com> Date: Wed, 5 Aug 2026 11:40:51 -0400 Subject: [PATCH] fix(synthetics_tests): remap principals under options.bindings Mobile synthetic tests store access-control bindings under options.bindings. Update principal discovery and remapping to use that documented shape so organization, user, role, and team principals resolve before the destination request. --- datadog_sync/model/synthetics_tests.py | 39 +++++------ tests/unit/test_synthetics_tests.py | 92 +++++++++++++++++--------- 2 files changed, 79 insertions(+), 52 deletions(-) diff --git a/datadog_sync/model/synthetics_tests.py b/datadog_sync/model/synthetics_tests.py index 81d6485c..fdce57e5 100644 --- a/datadog_sync/model/synthetics_tests.py +++ b/datadog_sync/model/synthetics_tests.py @@ -36,7 +36,7 @@ class SyntheticsTests(BaseResource): ], "roles": [ "options.restricted_roles", - "restriction_policy.bindings.principals", + "options.bindings.principals", ], "rum_applications": ["options.rumSettings.applicationId"], "synthetics_mobile_applications": [ @@ -47,8 +47,8 @@ class SyntheticsTests(BaseResource): "mobileApplicationsVersions", "options.mobileApplication.referenceId", ], - "users": ["restriction_policy.bindings.principals"], - "teams": ["restriction_policy.bindings.principals"], + "users": ["options.bindings.principals"], + "teams": ["options.bindings.principals"], }, base_path="/api/v1/synthetics/tests", excluded_attributes=[ @@ -254,12 +254,13 @@ async def pre_resource_action_hook(self, _id, resource: Dict) -> None: "source_status": source_status, } - # org: principals in restriction_policy bindings must be remapped from the + # org: principals in options.bindings must be remapped from the # source org UUID to the destination org UUID; otherwise the destination API # rejects the update with "cross-org principals are not supported". - # Mirrors the pattern in monitors.py. - if self.org_principal and resource.get("restriction_policy"): - for binding in resource["restriction_policy"].get("bindings") or []: + # Mirrors the pattern in monitors.py, but synthetics tests carry this data + # under options.bindings rather than a top-level restriction_policy key. + if self.org_principal and resource.get("options"): + for binding in resource["options"].get("bindings") or []: for i, principal in enumerate(binding.get("principals") or []): if principal.startswith("org:"): binding["principals"][i] = self.org_principal @@ -267,10 +268,10 @@ async def pre_resource_action_hook(self, _id, resource: Dict) -> None: async def pre_apply_hook(self) -> None: # Only fetch destination org UUID when at least one source test carries - # a restriction_policy. Policy-free syncs skip the API call and don't + # options.bindings. Policy-free syncs skip the API call and don't # inherit a failure dependency on /api/v2/current_user. self.org_principal = await self._fetch_destination_org_principal( - has_policy=lambda r: bool(r.get("restriction_policy")), + has_policy=lambda r: bool(r.get("options", {}).get("bindings")), current_user_path=self.current_user_path, ) @@ -429,7 +430,7 @@ def connect_resources(self, _id: str, resource: Dict) -> ResourceConnectionResul All non-access-control connections (private locations, subtests, global variables, rum/mobile apps) keep the generic find_attr/connect_id path. The flat - `options.restricted_roles` list and the `restriction_policy.bindings.principals` + `options.restricted_roles` list and the `options.bindings.principals` composites go through the shared drop-aware filters so permanently-stale references can be dropped (under --drop-unresolvable-principals) while an emptied binding/list still hard-fails as an access-elevation guard. @@ -440,31 +441,27 @@ def connect_resources(self, _id: str, resource: Dict) -> ResourceConnectionResul failed_connections_dict = defaultdict(list) for resource_to_connect, attrs in self.resource_config.resource_connections.items(): for attr_connection in attrs: - if attr_connection in ("options.restricted_roles", "restriction_policy.bindings.principals"): + if attr_connection in ("options.restricted_roles", "options.bindings.principals"): continue # handled by the drop-aware filters below c = find_attr(attr_connection, resource_to_connect, resource, self.connect_id) if c: failed_connections_dict[resource_to_connect].extend(c) empty_risk = False - restriction_policy = resource.get("restriction_policy") - if restriction_policy: - principal_failed, binding_risk = self._filter_stale_binding_principals( - _id, restriction_policy.get("bindings") - ) + options = resource.get("options") + if options: + principal_failed, binding_risk = self._filter_stale_binding_principals(_id, options.get("bindings")) for rt, ids in principal_failed.items(): failed_connections_dict[rt].extend(ids) empty_risk = empty_risk or binding_risk - role_failed, roles_risk = self._filter_stale_flat_roles(_id, resource.get("options"), "restricted_roles") + role_failed, roles_risk = self._filter_stale_flat_roles(_id, options, "restricted_roles") if role_failed: failed_connections_dict["roles"].extend(role_failed) 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]]: @@ -518,7 +515,7 @@ def connect_id(self, key: str, r_obj: Dict, resource_to_connect: str) -> Optiona return [] return super(SyntheticsTests, self).connect_id(key, r_obj, resource_to_connect) elif key == "principals": - # Remap user:/role:/team: principals in restriction_policy bindings. + # Remap user:/role:/team: principals in options.bindings. # org: principals are handled in pre_resource_action_hook before this runs. # Each resource_to_connect pass handles only its type; other types pass through silently. # Mirrors the pattern in monitors.py. diff --git a/tests/unit/test_synthetics_tests.py b/tests/unit/test_synthetics_tests.py index fce54a11..c3098c0b 100644 --- a/tests/unit/test_synthetics_tests.py +++ b/tests/unit/test_synthetics_tests.py @@ -253,9 +253,9 @@ def test_connect_id_rum_app_not_found(self): class TestSyntheticsTestsOrgPrincipalRemap: - """Test suite for restriction_policy org: principal remapping in synthetics_tests. + """Test suite for options.bindings org: principal remapping in synthetics_tests. - Mirrors the tests in test_monitors.py for the same feature. See HAMR-392 Jul8-T15. + Mobile synthetic tests carry access-control bindings under options.bindings. """ def _make_synthetics_tests(self): @@ -265,36 +265,34 @@ def _make_synthetics_tests(self): return SyntheticsTests(mock_config) def test_pre_resource_action_hook_replaces_org_principal(self): - """org: principal in restriction_policy bindings is replaced when org_principal is set.""" + """org: principal in options.bindings is replaced when org_principal is set.""" synthetics_tests = self._make_synthetics_tests() synthetics_tests.org_principal = "org:dest-pub-id" resource = { - "type": "api", + "type": "mobile", "public_id": "abc-123", "status": "live", - "restriction_policy": { - "bindings": [{"principals": ["org:src-pub-id", "user:some-user"], "relation": "editor"}] - }, + "options": {"bindings": [{"principals": ["org:src-pub-id", "user:some-user"], "relation": "editor"}]}, } asyncio.run(synthetics_tests.pre_resource_action_hook("abc-123#12345", resource)) - assert resource["restriction_policy"]["bindings"][0]["principals"][0] == "org:dest-pub-id" - assert resource["restriction_policy"]["bindings"][0]["principals"][1] == "user:some-user" + assert resource["options"]["bindings"][0]["principals"][0] == "org:dest-pub-id" + assert resource["options"]["bindings"][0]["principals"][1] == "user:some-user" def test_pre_resource_action_hook_skips_org_when_no_org_principal(self): """org: principal is left unchanged when org_principal is None.""" synthetics_tests = self._make_synthetics_tests() assert synthetics_tests.org_principal is None resource = { - "type": "api", + "type": "mobile", "public_id": "abc-123", "status": "live", - "restriction_policy": {"bindings": [{"principals": ["org:src-pub-id"], "relation": "editor"}]}, + "options": {"bindings": [{"principals": ["org:src-pub-id"], "relation": "editor"}]}, } asyncio.run(synthetics_tests.pre_resource_action_hook("abc-123#12345", resource)) - assert resource["restriction_policy"]["bindings"][0]["principals"][0] == "org:src-pub-id" + assert resource["options"]["bindings"][0]["principals"][0] == "org:src-pub-id" - def test_pre_resource_action_hook_no_restriction_policy_is_noop(self): - """Resources without restriction_policy are unaffected by the remap step.""" + def test_pre_resource_action_hook_no_bindings_is_noop(self): + """Resources without options.bindings are unaffected by the remap step.""" synthetics_tests = self._make_synthetics_tests() synthetics_tests.org_principal = "org:dest-pub-id" resource = {"type": "api", "public_id": "abc-123", "status": "live"} @@ -302,20 +300,37 @@ def test_pre_resource_action_hook_no_restriction_policy_is_noop(self): # DR metadata is still injected by the existing hook path. assert resource["metadata"]["disaster_recovery"]["source_public_id"] == "abc-123" + def test_pre_resource_action_hook_no_restriction_policy_key_present(self): + """Mobile test bindings are remapped without a top-level restriction_policy key.""" + synthetics_tests = self._make_synthetics_tests() + synthetics_tests.org_principal = "org:dest-pub-id" + resource = { + "type": "mobile", + "public_id": "abc-123", + "status": "live", + "options": { + "restricted_roles": ["role-good"], + "bindings": [{"principals": ["org:src-pub-id"], "relation": "viewer"}], + }, + } + assert "restriction_policy" not in resource + asyncio.run(synthetics_tests.pre_resource_action_hook("abc-123#12345", resource)) + assert resource["options"]["bindings"][0]["principals"][0] == "org:dest-pub-id" + def _seed_source_with_policy(self, synthetics_tests): - """Populate source state with one test that carries a restriction_policy.""" + """Populate source state with one test that carries options.bindings.""" synthetics_tests.config.state.source = { "synthetics_tests": { - "abc-123#1": {"public_id": "abc-123", "restriction_policy": {"bindings": [{"principals": ["org:src"]}]}} + "abc-123#1": {"public_id": "abc-123", "options": {"bindings": [{"principals": ["org:src"]}]}} } } def _seed_source_without_policy(self, synthetics_tests): - """Populate source state with one test lacking a restriction_policy.""" + """Populate source state with one test lacking options.bindings.""" synthetics_tests.config.state.source = {"synthetics_tests": {"abc-123#1": {"public_id": "abc-123"}}} def test_pre_apply_hook_sets_org_principal_on_success(self): - """Source carries a restriction_policy → GET fires → org_principal set.""" + """Source carries options.bindings → GET fires → org_principal set.""" synthetics_tests = self._make_synthetics_tests() self._seed_source_with_policy(synthetics_tests) mock_client = AsyncMock() @@ -339,7 +354,7 @@ def test_pre_apply_hook_leaves_org_principal_none_on_failure(self): assert synthetics_tests.org_principal is None def test_pre_apply_hook_skips_current_user_when_no_policy(self): - """No source test carries a restriction_policy → GET is not called; org_principal stays None.""" + """No source test carries options.bindings → GET is not called; org_principal stays None.""" synthetics_tests = self._make_synthetics_tests() self._seed_source_without_policy(synthetics_tests) mock_client = AsyncMock() @@ -361,8 +376,8 @@ def test_pre_apply_hook_skips_current_user_when_source_empty(self): mock_client.get.assert_not_awaited() -class TestSyntheticsTestsRestrictionPolicyPrincipals: - """Test suite for restriction_policy user:/role:/team: principal remapping. +class TestSyntheticsTestsOptionsBindingPrincipals: + """Test suite for options.bindings user:/role:/team: principal remapping. Mirrors TestMonitorsRestrictionPolicyPrincipals — synthetics_tests must remap prefixed principals via connect_id when resource_connections routes @@ -479,7 +494,7 @@ def test_extract_source_ids_org_excluded(self): class TestSyntheticsTestsConnectResourcesDrop: """connect_resources drop/keep/hard-fail for synthetics_tests' access-control shapes: - flat `options.restricted_roles` and `restriction_policy.bindings.principals` composites. + flat `options.restricted_roles` and `options.bindings.principals` composites. """ def _make_test(self, drop=False, skip_failed=False): @@ -497,38 +512,53 @@ def _make_test(self, drop=False, skip_failed=False): def _seed_valid_role(self, t, src="role-good", dst="role-good-dst"): t.config.state.destination["roles"][src] = {"id": dst} - def test_restriction_policy_flag_on_drops_stale(self): + def test_options_bindings_flag_on_drops_stale(self): t = self._make_test(drop=True) self._seed_valid_role(t) resource = { "public_id": "abc-def-ghi", - "restriction_policy": { - "bindings": [{"principals": ["role:role-good", "role:role-gone"], "relation": "editor"}] - }, + "options": {"bindings": [{"principals": ["role:role-good", "role:role-gone"], "relation": "editor"}]}, } t.connect_resources("abc-def-ghi", resource) # no raise - assert resource["restriction_policy"]["bindings"][0]["principals"] == ["role:role-good-dst"] + assert resource["options"]["bindings"][0]["principals"] == ["role:role-good-dst"] assert t.config.counter.stale_principals_dropped_by_type["synthetics_tests"] == ["abc-def-ghi"] - def test_restriction_policy_flag_off_stale_hard_fails(self): + def test_options_bindings_flag_off_stale_hard_fails(self): t = self._make_test(drop=False) resource = { "public_id": "abc-def-ghi", - "restriction_policy": {"bindings": [{"principals": ["role:role-gone"], "relation": "editor"}]}, + "options": {"bindings": [{"principals": ["role:role-gone"], "relation": "editor"}]}, } with pytest.raises(ResourceConnectionError): t.connect_resources("abc-def-ghi", resource) - def test_restriction_policy_empty_binding_raises_risk(self): + def test_options_bindings_empty_binding_raises_risk(self): t = self._make_test(drop=True) resource = { "public_id": "abc-def-ghi", - "restriction_policy": {"bindings": [{"principals": ["role:role-gone"], "relation": "editor"}]}, + "options": {"bindings": [{"principals": ["role:role-gone"], "relation": "editor"}]}, } with pytest.raises(ResourceConnectionError) as exc_info: t.connect_resources("abc-def-ghi", resource) assert exc_info.value.empty_binding_risk is True + def test_options_bindings_and_restricted_roles_coexist_no_restriction_policy_key(self): + """A mobile test can carry options.restricted_roles and options.bindings together.""" + t = self._make_test(drop=True) + self._seed_valid_role(t, src="role-good", dst="role-good-dst") + resource = { + "public_id": "abc-def-ghi", + "type": "mobile", + "options": { + "restricted_roles": ["role-good"], + "bindings": [{"principals": ["role:role-good"], "relation": "editor"}], + }, + } + assert "restriction_policy" not in resource + t.connect_resources("abc-def-ghi", resource) # no raise + assert resource["options"]["restricted_roles"] == ["role-good-dst"] + assert resource["options"]["bindings"][0]["principals"] == ["role:role-good-dst"] + def test_options_restricted_roles_flat_drops_stale(self): t = self._make_test(drop=True) self._seed_valid_role(t, src="role-good", dst="role-good-dst")