Skip to content

fix(synthetics_tests): remap mobile test principals under options.bindings #658

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
michael-richey merged 1 commit into from
Aug 5, 2026
+79 −52
Merged

fix(synthetics_tests): remap mobile test principals under options.bindings #658

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 18 additions & 21 deletions datadog_sync/model/synthetics_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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": [
Expand All @@ -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=[
Expand Down Expand Up @@ -254,23 +254,24 @@ 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
break

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,
)

Expand Down Expand Up @@ -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.
Expand All @@ -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]]:
Expand Down Expand Up @@ -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.
Expand Down
92 changes: 61 additions & 31 deletions tests/unit/test_synthetics_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -265,57 +265,72 @@ 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"}
asyncio.run(synthetics_tests.pre_resource_action_hook("abc-123#12345", resource))
# 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()
Expand All @@ -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()
Expand All @@ -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
Expand Down Expand Up @@ -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):
Expand All @@ -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")
Expand Down
Loading