diff --git a/README.md b/README.md index 6f63cf0e..28872f6f 100644 --- a/README.md +++ b/README.md @@ -19,6 +19,7 @@ Datadog cli tool to sync resources across organizations. - [Config file](#config-file) - [Cleanup flag](#cleanup-flag) - [Verify DDR Status Flag](#verify-ddr-status-flag) + - [Create users via the v1 API](#create-users-via-the-v1-api) - [State Files](#state-files) - [Supported resources](#supported-resources) - [Best practices](#best-practices) @@ -210,6 +211,13 @@ For example, `ResourceA` and `ResourceB` are imported and synced, followed by de By default all commands check the Datadog Disaster Recovery (DDR) status of both the source and destination organizations before running. This behavior is controlled by the boolean flag `--verify-ddr-status` or the environment variable `DD_VERIFY_DDR_STATUS`. +#### Create users via the v1 API + +The destination enforces user uniqueness on the `handle`, not the email — multiple users may share an email address. The v2 user create endpoint (`POST /api/v2/users`) does not accept a `handle` (it derives one from the email), so users that share an email collapse onto a single handle: the first create takes a handle that may belong to another user, and later creates for that email return an HTTP 409. + +Passing `--use-v1-user-api` (or setting `DD_USE_V1_USER_API=true`) makes `sync` create users via the v1 user endpoint (`POST /api/v1/user`), which accepts an explicit handle, so every user keeps its own handle and no create collides. The flag is off by default. + + #### Running behind an HTTP proxy By default the tool's HTTP client ignores the environment and talks to Datadog directly. To run it behind a proxy, set `--http-client-trust-env true` (or the environment variable `DD_HTTP_CLIENT_TRUST_ENV=true`). When enabled, the underlying HTTP client honors the standard `HTTP_PROXY`, `HTTPS_PROXY`, and `NO_PROXY` environment variables, as well as credentials from `.netrc`. This option is off by default. Note that when enabled, the configured proxy can observe all Datadog API traffic — including the `DD-API-KEY`, `DD-APPLICATION-KEY`, or JWT headers if it terminates TLS — and `.netrc` credentials may be automatically attached for matching hosts, so only enable this for a proxy you trust. diff --git a/datadog_sync/commands/shared/options.py b/datadog_sync/commands/shared/options.py index a519b6ec..1de6ea08 100644 --- a/datadog_sync/commands/shared/options.py +++ b/datadog_sync/commands/shared/options.py @@ -591,6 +591,18 @@ def click_config_file_provider(ctx: Context, opts: CustomOptionClass, value: Non help="Allow self-lockout when syncing restriction policies.", cls=CustomOptionClass, ), + option( + "--use-v1-user-api", + required=False, + envvar=constants.DD_USE_V1_USER_API, + type=bool, + default=False, + show_default=True, + help="Create users via the v1 API (/api/v1/user) instead of v2. v2 cannot set a " + "handle (it derives one from the email), which collides when users share an email; " + "v1 accepts an explicit handle so each user keeps its own. Off by default.", + cls=CustomOptionClass, + ), ] diff --git a/datadog_sync/constants.py b/datadog_sync/constants.py index d62d8485..6c8fefd5 100644 --- a/datadog_sync/constants.py +++ b/datadog_sync/constants.py @@ -29,6 +29,7 @@ DD_SHOW_PROGRESS_BAR = "DD_SHOW_PROGRESS_BAR" DD_VERIFY_SSL_CERTIFICATES = "DD_VERIFY_SSL_CERTIFICATES" DD_ALLOW_PARTIAL_PERMISSIONS_ROLES = "DD_ALLOW_PARTIAL_PERMISSIONS_ROLES" +DD_USE_V1_USER_API = "DD_USE_V1_USER_API" DD_SYNC_JSON = "DD_SYNC_JSON" DD_DATADOG_HOST_OVERRIDE = "DD_DATADOG_HOST_OVERRIDE" diff --git a/datadog_sync/model/users.py b/datadog_sync/model/users.py index 5b4e5a79..be18e046 100644 --- a/datadog_sync/model/users.py +++ b/datadog_sync/model/users.py @@ -4,6 +4,7 @@ # Copyright 2019 Datadog, Inc. from __future__ import annotations +import asyncio from typing import TYPE_CHECKING, Any, Optional, List, Dict, Tuple, cast from datadog_sync.utils.base_resource import BaseResource, ResourceConfig @@ -14,6 +15,17 @@ from datadog_sync.utils.custom_client import CustomClient +class UserRoleAssignmentError(RuntimeError): + """One or more role assignments failed while reconciling a user.""" + + def __init__(self, user: Dict, failed_role_ids: List[str]) -> None: + self.user = user + self.failed_role_ids = tuple(failed_role_ids) + count = len(failed_role_ids) + assignment = "role assignment" if count == 1 else "role assignments" + super().__init__(f"{count} {assignment} failed while reconciling user") + + class Users(BaseResource): resource_type = "users" resource_config = ResourceConfig( @@ -30,7 +42,11 @@ class Users(BaseResource): "attributes.status", "attributes.verified", "attributes.service_account", - "attributes.handle", + # NOTE: attributes.handle is deliberately NOT excluded here. It is the + # user mapping key (resource_mapping_key below) and the payload for the + # v1 user creation, so it must survive prep_resource. It is popped + # manually before the v2 POST/PATCH (v2 treats handle as read-only) and + # kept out of update diffs via deep_diff_config.exclude_regex_paths below. "attributes.icon", "attributes.modified_at", "attributes.mfa_enabled", @@ -40,13 +56,18 @@ class Users(BaseResource): "relationships.org", "relationships.team_roles", ], - resource_mapping_key="attributes.email", + resource_mapping_key="attributes.handle", + # Handle is read-only in v2 and is popped from the create/update payload; + # exclude it from update diffs so a source-vs-destination handle difference + # never drives a spurious PATCH of a field v2 will not accept. + deep_diff_config={"ignore_order": True, "exclude_regex_paths": [r".*\['handle'\]"]}, ) # Additional Users specific attributes pagination_config = PaginationConfig( page_size=500, ) roles_path: str = "/api/v2/roles/{}/users" + user_lookup_retry_delays: Tuple[float, ...] = (1.0, 2.0) async def get_resources(self, client: CustomClient) -> List[Dict]: resp = await client.paginated_request(client.get)( @@ -80,27 +101,152 @@ async def create_resource(self, _id: str, resource: Dict) -> Tuple[str, Dict]: self.config.state.destination[self.resource_type][_id] = self._existing_resources_map[key] return await self.update_resource(_id, resource) + attributes = resource["attributes"] + if self.config.use_v1_user_api: + # v2 create cannot set a handle (it derives one from the email), which + # collapses distinct-handle users that share an email onto one handle. + # v1 accepts an explicit handle, so each user keeps its own. + try: + user = await self._create_via_v1( + attributes.get("handle"), + attributes.get("name"), + attributes.get("email"), + resource.get("relationships", {}).get("roles", {}).get("data", []), + ) + except UserRoleAssignmentError as e: + # Preserve the reconciled UUID and successful memberships so + # downstream resources can still resolve this user. Re-raising + # makes the apply handler count and emit the partial failure. + self.config.state.destination[self.resource_type][_id] = e.user + raise + return _id, user + destination_client = self.config.destination_client - resource["attributes"].pop("disabled", None) + # handle is read-only in v2 (derived from email) and must not be sent. + attributes.pop("disabled", None) + attributes.pop("handle", None) resp = await destination_client.post(self.resource_config.base_path, {"data": resource}) - return _id, resp["data"] + async def _create_via_v1( + self, + handle: Optional[str], + name: Optional[str], + email: Optional[str], + desired_roles: Optional[List[Dict]] = None, + ) -> Dict: + """Create the user via the v1 API, which accepts an explicit handle. + + v2 ``POST /api/v2/users`` cannot set a handle (it is derived from the + email), so users that share an email collapse onto one handle and later + creates 409 — and the v2 "winner" is created with the wrong handle. v1 + ``POST /api/v1/user`` accepts a distinct handle. The v1 response is the + legacy user shape, so re-resolve the v2 UUID by handle and return that + record — state and downstream references (roles, team_memberships) key + on the v2 UUID. + """ + if not handle: + raise ValueError("v1 user creation requires a handle") + + destination_client = self.config.destination_client + await destination_client.post("/api/v1/user", {"handle": handle, "name": name, "email": email}) + + user = await self._get_destination_user_by_handle(handle) + if user is None: + raise ValueError("v1-created user not found by handle after create") + await self._assign_missing_roles(user, desired_roles or []) + return user + + async def _assign_missing_roles(self, user: Dict, desired_roles: List[Dict]) -> None: + """Assign missing roles and keep the reconciled user state accurate.""" + existing_roles = user.setdefault("relationships", {}).setdefault("roles", {}).setdefault("data", []) + existing_role_ids = { + role["id"] for role in existing_roles if isinstance(role, dict) and role.get("id") is not None + } + failed_role_ids = [] + + for role in desired_roles: + if not isinstance(role, dict) or role.get("id") is None: + continue + role_id = role["id"] + if role_id in existing_role_ids: + continue + if await self.add_user_to_role(user["id"], role_id): + existing_roles.append(dict(role)) + existing_role_ids.add(role_id) + else: + failed_role_ids.append(str(role_id)) + + if failed_role_ids: + raise UserRoleAssignmentError(user, failed_role_ids) + + @staticmethod + def _merge_role_state(updated_user: Dict, role_state: Dict) -> Dict: + """Preserve known role memberships when a user PATCH omits them.""" + updated_roles = updated_user.setdefault("relationships", {}).setdefault("roles", {}).setdefault("data", []) + updated_role_ids = { + role["id"] for role in updated_roles if isinstance(role, dict) and role.get("id") is not None + } + for role in role_state.get("relationships", {}).get("roles", {}).get("data", []): + role_id = role.get("id") if isinstance(role, dict) else None + if role_id is not None and role_id not in updated_role_ids: + updated_roles.append(dict(role)) + updated_role_ids.add(role_id) + return updated_user + + async def _get_destination_user_by_handle(self, handle: str) -> Optional[Dict]: + """Return the destination user whose handle matches exactly, or None. + + Transient HTTP errors are already retried by the client's + ``request_with_retry``; this adds a bounded re-query loop with delays to + absorb read-after-write visibility lag after a v1 create. + """ + destination_client = self.config.destination_client + for attempt in range(len(self.user_lookup_retry_delays) + 1): + resp = await destination_client.paginated_request(destination_client.get)( + self.resource_config.base_path, + pagination_config=self.pagination_config, + params={"filter": handle}, + ) + for user in resp: + if user.get("attributes", {}).get("handle") == handle: + return user + if attempt < len(self.user_lookup_retry_delays): + await asyncio.sleep(self.user_lookup_retry_delays[attempt]) + return None + async def update_resource(self, _id: str, resource: Dict) -> Tuple[str, Dict]: destination_client = self.config.destination_client - diff = check_diff(self.resource_config, self.config.state.destination[self.resource_type][_id], resource) + destination_user = self.config.state.destination[self.resource_type][_id] + diff = check_diff(self.resource_config, destination_user, resource) if diff: - await self.update_user_roles(self.config.state.destination[self.resource_type][_id]["id"], diff) - resource["id"] = self.config.state.destination[self.resource_type][_id]["id"] + role_error = None + try: + await self._assign_missing_roles( + destination_user, + resource.get("relationships", {}).get("roles", {}).get("data", []), + ) + except UserRoleAssignmentError as e: + # Continue with unrelated attribute updates, then report the + # partial role failure so the apply handler does not emit success. + role_error = e + + resource["id"] = destination_user["id"] resource.pop("relationships", None) + resource["attributes"].pop("handle", None) resp = await destination_client.patch( - self.resource_config.base_path + f"/{self.config.state.destination[self.resource_type][_id]['id']}", + self.resource_config.base_path + f"/{destination_user['id']}", {"data": resource}, ) + updated_user = self._merge_role_state(resp["data"], destination_user) - return _id, resp["data"] - return _id, self.config.state.destination[self.resource_type][_id] + if role_error is not None: + self.config.state.destination[self.resource_type][_id] = updated_user + role_error.user = updated_user + raise role_error + return _id, updated_user + return _id, destination_user async def delete_resource(self, _id: str) -> None: destination_client = self.config.destination_client @@ -111,31 +257,15 @@ async def delete_resource(self, _id: str) -> None: def connect_id(self, key: str, r_obj: Dict, resource_to_connect: str) -> Optional[List[str]]: return super(Users, self).connect_id(key, r_obj, resource_to_connect) - async def update_user_roles(self, _id, diff): - for k, v in diff.items(): - if k == "iterable_item_added": - for key, value in diff["iterable_item_added"].items(): - if "roles" in key: - await self.add_user_to_role(_id, value["id"]) - # elif k == "iterable_item_removed": - # for key, value in diff["iterable_item_removed"].items(): - # if "roles" in key: - # await self.remove_user_from_role(_id, value["id"]) - elif k == "values_changed": - for key, value in diff["values_changed"].items(): - if "roles" in key: - # await self.remove_user_from_role(_id, value["old_value"]) - new_val = value["new_value"] - role_id = new_val["id"] if isinstance(new_val, dict) else new_val - await self.add_user_to_role(_id, role_id) - - async def add_user_to_role(self, user_id, role_id): + async def add_user_to_role(self, user_id, role_id) -> bool: destination_client = self.config.destination_client payload = {"data": {"id": user_id, "type": "users"}} try: await destination_client.post(self.roles_path.format(role_id), payload) + return True except CustomClientHTTPError as e: self.config.logger.error("error adding user: %s to role %s: %s", user_id, role_id, e) + return False async def remove_user_from_role(self, user_id, role_id): destination_client = self.config.destination_client diff --git a/datadog_sync/utils/configuration.py b/datadog_sync/utils/configuration.py index 75589760..41b39128 100644 --- a/datadog_sync/utils/configuration.py +++ b/datadog_sync/utils/configuration.py @@ -109,6 +109,7 @@ class Configuration(object): max_workers_per_type: Dict[str, int] = field(default_factory=dict) command: str = "" allow_partial_permissions_roles: List[str] = field(default_factory=list) + use_v1_user_api: bool = False resources: Dict[str, BaseResource] = field(default_factory=dict) resources_arg: List[str] = field(default_factory=list) # --id-file: id-targeted import via stdin or file payload. @@ -435,6 +436,7 @@ def build_config(cmd: Command, **kwargs: Optional[Any]) -> Configuration: max_workers_per_type = _parse_max_workers_per_type(max_workers_per_type_raw, known_resource_types) create_global_downtime = kwargs.get("create_global_downtime") validate = kwargs.get("validate") + use_v1_user_api = kwargs.get("use_v1_user_api") or False verify_ddr_status = kwargs.get("verify_ddr_status") backup_before_reset = not kwargs.get("do_not_backup") show_progress_bar = kwargs.get("show_progress_bar") @@ -691,6 +693,7 @@ def build_config(cmd: Command, **kwargs: Optional[Any]) -> Configuration: emit_json=emit_json, command=cmd.value, allow_partial_permissions_roles=allow_partial_permissions_roles, + use_v1_user_api=use_v1_user_api, id_payload=id_payload, max_concurrent_reads=max_concurrent_reads, transient_failure_threshold_pct=transient_failure_threshold_pct, diff --git a/tests/unit/test_users.py b/tests/unit/test_users.py new file mode 100644 index 00000000..74417b99 --- /dev/null +++ b/tests/unit/test_users.py @@ -0,0 +1,482 @@ +# 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 handle-keyed users + the --use-v1-user-api flag. + +The Datadog destination enforces user uniqueness on the ``handle``, not the +``email`` — multiple users may share an email. Keying ``_existing_resources_map`` +by email collapsed distinct-handle users onto one derived handle and caused a +409 Conflict on the second create. These tests cover switching the user mapping +key to the exact-case handle (PR1) and wiring the opt-in --use-v1-user-api flag. + +Tests ``a``/``a2``/``a3``/``f`` are red against ``resource_mapping_key= +"attributes.email"`` and green after the switch to ``"attributes.handle"`` plus +the manual handle pop before the v2 POST. Test ``g`` is a green/green guard that +handle stays excluded from update diffs across the excluded_attributes -> +exclude_regex_paths migration. ``p1``/``p2``/``p3`` guard the flag wiring. +""" + +import asyncio +from unittest.mock import AsyncMock, MagicMock, call, patch + +import pytest +from click.testing import CliRunner + +from datadog_sync.cli import cli +from datadog_sync.constants import Command +from datadog_sync.model.users import UserRoleAssignmentError, Users +from datadog_sync.utils.configuration import build_config +from datadog_sync.utils.resource_utils import CustomClientHTTPError, check_diff + + +def _http_error(status): + """Build a CustomClientHTTPError with the given status code.""" + response = MagicMock() + response.status = status + response.message = "Conflict" if status == 409 else "Error" + return CustomClientHTTPError(response) + + +def _make_user(handle, email, user_id, name="User", roles=None): + """Build a user dict shaped like the v2 GET/create response.""" + return { + "id": user_id, + "type": "users", + "attributes": { + "handle": handle, + "email": email, + "name": name, + "disabled": False, + }, + "relationships": {"roles": {"data": roles or []}}, + } + + +def _base_kwargs(tmp_path): + """Minimal build_config kwargs that avoid network/validation.""" + return dict( + resources="users", + resource_per_file=True, + source_api_key="k", + source_app_key="k", + destination_api_key="k", + destination_app_key="k", + source_api_url="https://example.com", + destination_api_url="https://example.com", + storage_type="local", + source_resources_path=str(tmp_path / "source"), + destination_resources_path=str(tmp_path / "dest"), + max_workers=1, + send_metrics=False, + verify_ddr_status=False, + validate=False, + show_progress_bar=False, + allow_self_lockout=False, + force_missing_dependencies=False, + skip_failed_resource_connections=False, + ) + + +class TestHandleMappingKey: + def test_mapping_key_is_exact_case_handle(self, mock_config): + """a: mapping key is the handle, preserved exact-case (no lowercasing).""" + instance = Users(mock_config) + resource = {"attributes": {"handle": "User-A@example.com", "email": "shared@example.com"}} + assert instance.get_resource_mapping_key(resource) == "User-A@example.com" + + def test_map_keeps_shared_email_distinct_by_handle(self, mock_config): + """a2: two destination users sharing an email but with distinct handles + remain two entries in the map (email keying would collapse to one).""" + instance = Users(mock_config) + dest = [ + _make_user("user-a@example.com", "shared@example.com", "dest-a"), + _make_user("user-b@example.com", "shared@example.com", "dest-b"), + ] + instance.get_resources = AsyncMock(return_value=dest) + asyncio.run(instance.map_existing_resources()) + assert set(instance._existing_resources_map.keys()) == { + "user-a@example.com", + "user-b@example.com", + } + + def test_source_matched_by_handle_not_email(self, mock_config): + """a3: against a pre-existing destination user, a source user is matched + by handle — a same-email/different-handle source is NOT a match.""" + instance = Users(mock_config) + instance.get_resources = AsyncMock( + return_value=[_make_user("user-a@example.com", "shared@example.com", "dest-a")] + ) + asyncio.run(instance.map_existing_resources()) + + same_handle = _make_user("user-a@example.com", "shared@example.com", "src-a") + assert instance.get_resource_mapping_key(same_handle) in instance._existing_resources_map + + diff_handle_same_email = _make_user("user-b@example.com", "shared@example.com", "src-b") + assert instance.get_resource_mapping_key(diff_handle_same_email) not in instance._existing_resources_map + + +class TestV2CreatePayload: + def test_v2_post_body_excludes_handle_and_disabled(self, mock_config): + """f: the v2 create body must not carry read-only handle or disabled.""" + mock_config.use_v1_user_api = False + instance = Users(mock_config) + instance._existing_resources_map = {} + mock_config.destination_client.post = AsyncMock(return_value={"data": {"id": "dest-x", "attributes": {}}}) + src = _make_user("user-a@example.com", "shared@example.com", "src-a") + + asyncio.run(instance.create_resource("src-a", src)) + + mock_config.destination_client.post.assert_called_once() + _, body = mock_config.destination_client.post.call_args.args + attrs = body["data"]["attributes"] + assert "handle" not in attrs + assert "disabled" not in attrs + + def test_v2_patch_body_excludes_handle(self, mock_config): + """f2: the v2 update (PATCH) body must not carry the read-only handle.""" + instance = Users(mock_config) + _id = "src-a" + mock_config.state.destination["users"][_id] = _make_user("user-a@example.com", "shared@example.com", "dest-a") + # A differing name forces a diff -> the PATCH branch. + src = _make_user("user-a@example.com", "shared@example.com", "dest-a", name="New Name") + mock_config.destination_client.patch = AsyncMock(return_value={"data": {"id": "dest-a", "attributes": {}}}) + + asyncio.run(instance.update_resource(_id, src)) + + mock_config.destination_client.patch.assert_called_once() + _, body = mock_config.destination_client.patch.call_args.args + assert "handle" not in body["data"]["attributes"] + + +class TestHandleDiffExclusion: + def test_handle_excluded_from_update_diff(self): + """g: two users differing only by handle produce no diff (guards that + handle stays diff-excluded after moving off excluded_attributes).""" + dest = _make_user("user-a@example.com", "shared@example.com", "same-id") + src = _make_user("user-b@example.com", "shared@example.com", "same-id") + assert not check_diff(Users.resource_config, dest, src) + + +class TestV1UserApiFlagWiring: + def test_build_config_flag_true(self, tmp_path): + """p1: --use-v1-user-api flows from kwargs into Configuration.""" + cfg = build_config(Command.IMPORT, use_v1_user_api=True, **_base_kwargs(tmp_path)) + assert cfg.use_v1_user_api is True + + def test_build_config_flag_default_false(self, tmp_path): + """p1: absent flag defaults to False (guards a typo'd kwarg key).""" + cfg = build_config(Command.IMPORT, **_base_kwargs(tmp_path)) + assert cfg.use_v1_user_api is False + + def test_sync_accepts_v1_flag(self): + """p2: the sync command recognizes the flag (exit 2 == usage error).""" + result = CliRunner(mix_stderr=False).invoke(cli, ["sync", "--use-v1-user-api=true", "--validate=false"]) + assert result.exit_code != 2 + + def test_migrate_accepts_v1_flag(self): + """p3: migrate reuses @sync_options, so it recognizes the flag too.""" + result = CliRunner(mix_stderr=False).invoke(cli, ["migrate", "--use-v1-user-api=true", "--validate=false"]) + assert result.exit_code != 2 + + +def _mock_paginated(config, pages): + """Make destination_client.paginated_request(get)(...) yield ``pages`` in order.""" + inner = AsyncMock(side_effect=pages) + config.destination_client.paginated_request = MagicMock(return_value=inner) + return inner + + +def _post_paths(mock_config): + return [c.args[0] for c in mock_config.destination_client.post.call_args_list] + + +class TestV2CreatePath: + def test_flag_off_uses_v2_not_v1(self, mock_config): + """b: with the flag off, create goes through the v2 endpoint and never + touches the v1 API (preserves the default upstream behavior).""" + mock_config.use_v1_user_api = False + instance = Users(mock_config) + instance._existing_resources_map = {} + mock_config.destination_client.post = AsyncMock(return_value={"data": {"id": "dest-x", "attributes": {}}}) + + asyncio.run(instance.create_resource("src-a", _make_user("user-a@example.com", "shared@example.com", "src-a"))) + + assert _post_paths(mock_config) == ["/api/v2/users"] + + +class TestV1CreatePath: + def test_flag_on_uses_v1_with_handle_not_v2(self, mock_config): + """d1: with the flag on, create posts to /api/v1/user carrying the handle, + never calls the v2 users endpoint, and returns the reconciled v2 UUID.""" + mock_config.use_v1_user_api = True + instance = Users(mock_config) + instance._existing_resources_map = {} + dest_user = _make_user("user-a@example.com", "shared@example.com", "dest-uuid-a") + mock_config.destination_client.post = AsyncMock(return_value={"data": {}}) + _mock_paginated(mock_config, [[dest_user]]) + + _id, r = asyncio.run( + instance.create_resource("src-a", _make_user("user-a@example.com", "shared@example.com", "src-a")) + ) + + v1_calls = [c for c in mock_config.destination_client.post.call_args_list if c.args[0] == "/api/v1/user"] + assert len(v1_calls) == 1 + assert v1_calls[0].args[1]["handle"] == "user-a@example.com" + assert "/api/v2/users" not in _post_paths(mock_config) + assert r["id"] == "dest-uuid-a" + + def test_shared_email_distinct_handles_both_created_via_v1(self, mock_config): + """The core fix. Two users share an email but have distinct handles: one + handle equals the shared email, the other differs. Under v2 the first + create derives its handle from the email and steals the second user's + handle, so the second 409s and can never be created. Via v1 each user is + created with its OWN handle, so both succeed.""" + mock_config.use_v1_user_api = True + instance = Users(mock_config) + instance._existing_resources_map = {} + dest_a = _make_user("abc@example.com", "jsmith@example.com", "dest-a") + dest_b = _make_user("jsmith@example.com", "jsmith@example.com", "dest-b") + mock_config.destination_client.post = AsyncMock(return_value={"data": {}}) + _mock_paginated(mock_config, [[dest_a], [dest_b]]) + + asyncio.run(instance.create_resource("src-a", _make_user("abc@example.com", "jsmith@example.com", "src-a"))) + asyncio.run(instance.create_resource("src-b", _make_user("jsmith@example.com", "jsmith@example.com", "src-b"))) + + v1_handles = [ + c.args[1]["handle"] + for c in mock_config.destination_client.post.call_args_list + if c.args[0] == "/api/v1/user" + ] + assert v1_handles == ["abc@example.com", "jsmith@example.com"] + assert "/api/v2/users" not in _post_paths(mock_config) + + def test_assigns_only_missing_roles_and_returns_updated_state(self, mock_config): + """v1 create assigns mapped roles after resolving the v2 UUID. + + Roles already present on the reconciled user are not posted again, and + successful assignments are reflected in the destination state returned + by create_resource. + """ + mock_config.use_v1_user_api = True + instance = Users(mock_config) + instance._existing_resources_map = {} + existing_role = {"id": "role-dst-existing", "type": "roles"} + missing_role = {"id": "role-dst-missing", "type": "roles"} + dest_user = _make_user( + "user-a@example.com", + "shared@example.com", + "dest-uuid-a", + roles=[existing_role], + ) + source_user = _make_user( + "user-a@example.com", + "shared@example.com", + "src-a", + roles=[existing_role, missing_role], + ) + mock_config.destination_client.post = AsyncMock(return_value={"data": {}}) + _mock_paginated(mock_config, [[dest_user]]) + + _, created = asyncio.run(instance.create_resource("src-a", source_user)) + + assert _post_paths(mock_config) == [ + "/api/v1/user", + "/api/v2/roles/role-dst-missing/users", + ] + assert created["relationships"]["roles"]["data"] == [existing_role, missing_role] + + def test_role_failure_persists_partial_state_and_reports_failure(self, mock_config): + """A failed role assignment is reported after later roles are attempted. + + Only successful assignments are recorded in returned destination state, + leaving failed roles eligible for retry on a later sync. The exception + lets the apply handler count the otherwise-partial create as a failure. + """ + mock_config.use_v1_user_api = True + instance = Users(mock_config) + instance._existing_resources_map = {} + failed_role = {"id": "role-dst-failed", "type": "roles"} + successful_role = {"id": "role-dst-success", "type": "roles"} + dest_user = _make_user("user-a@example.com", "shared@example.com", "dest-uuid-a") + source_user = _make_user( + "user-a@example.com", + "shared@example.com", + "src-a", + roles=[failed_role, successful_role], + ) + + async def post(path, _body): + if path == "/api/v2/roles/role-dst-failed/users": + raise _http_error(403) + return {"data": {}} + + mock_config.destination_client.post = AsyncMock(side_effect=post) + _mock_paginated(mock_config, [[dest_user]]) + + with pytest.raises( + UserRoleAssignmentError, match="1 role assignment failed while reconciling user" + ) as exc_info: + asyncio.run(instance.create_resource("src-a", source_user)) + + assert exc_info.value.failed_role_ids == ("role-dst-failed",) + assert _post_paths(mock_config) == [ + "/api/v1/user", + "/api/v2/roles/role-dst-failed/users", + "/api/v2/roles/role-dst-success/users", + ] + created = mock_config.state.destination["users"]["src-a"] + assert created["relationships"]["roles"]["data"] == [successful_role] + mock_config.logger.error.assert_called_once() + + def test_requires_handle(self, mock_config): + """d0: v1 creation raises when there is no handle to send.""" + mock_config.use_v1_user_api = True + instance = Users(mock_config) + with pytest.raises(ValueError): + asyncio.run(instance._create_via_v1("", "Person Name", "e@example.com")) + + def test_v1_post_failure_reraises(self, mock_config): + """d4: a v1 POST failure propagates so the apply loop counts it failed + and continues (DR-safe).""" + mock_config.use_v1_user_api = True + instance = Users(mock_config) + instance._existing_resources_map = {} + mock_config.destination_client.post = AsyncMock(side_effect=_http_error(400)) + with pytest.raises(CustomClientHTTPError): + asyncio.run( + instance.create_resource("src-a", _make_user("user-a@example.com", "shared@example.com", "src-a")) + ) + + +class TestReconcileByHandle: + def test_no_exact_match_raises(self, mock_config): + """If no exact-handle match is ever found after the v1 create, it raises.""" + mock_config.use_v1_user_api = True + instance = Users(mock_config) + instance._existing_resources_map = {} + mock_config.destination_client.post = AsyncMock(return_value={"data": {}}) + other = _make_user("user-b@example.com", "shared@example.com", "dest-b") + _mock_paginated(mock_config, [[other], [other], [other]]) + with patch("datadog_sync.model.users.asyncio.sleep", new_callable=AsyncMock) as sleep: + with pytest.raises(ValueError): + asyncio.run( + instance.create_resource("src-a", _make_user("user-a@example.com", "shared@example.com", "src-a")) + ) + assert sleep.await_args_list == [call(1.0), call(2.0)] + + def test_requeries_on_empty_then_matches(self, mock_config): + """d3b: read-after-write — an empty first page then a match re-queries + after a bounded delay rather than immediately retrying.""" + instance = Users(mock_config) + match = _make_user("user-a@example.com", "shared@example.com", "dest-uuid-a") + inner = _mock_paginated(mock_config, [[], [match]]) + with patch("datadog_sync.model.users.asyncio.sleep", new_callable=AsyncMock) as sleep: + user = asyncio.run(instance._get_destination_user_by_handle("user-a@example.com")) + assert user["id"] == "dest-uuid-a" + assert inner.call_count == 2 + assert sleep.await_args_list == [call(1.0)] + + def test_selects_exact_case_handle_among_candidates(self, mock_config): + """d5: with multiple filter candidates, only the exact-case handle wins.""" + instance = Users(mock_config) + a = _make_user("user-a@example.com", "shared@example.com", "dest-a") + b = _make_user("user-b@example.com", "shared@example.com", "dest-b") + _mock_paginated(mock_config, [[b, a]]) + user = asyncio.run(instance._get_destination_user_by_handle("user-a@example.com")) + assert user["id"] == "dest-a" + + +class TestUpdatePathRegression: + def test_successful_role_retry_remains_in_persisted_state(self, mock_config): + """A successful role retry is not lost when PATCH omits relationships.""" + instance = Users(mock_config) + successful_role = {"id": "role-dst-success", "type": "roles"} + dest_user = _make_user("user-a@example.com", "shared@example.com", "dest-a") + mock_config.state.destination["users"]["src-a"] = dest_user + instance.add_user_to_role = AsyncMock(return_value=True) + updated_user = { + "id": "dest-a", + "type": "users", + "attributes": { + "handle": "user-a@example.com", + "email": "shared@example.com", + "name": "Updated User", + "disabled": False, + }, + } + mock_config.destination_client.patch = AsyncMock(return_value={"data": updated_user}) + + def source_user(): + return _make_user( + "user-a@example.com", + "shared@example.com", + "src-a", + name="Updated User", + roles=[successful_role], + ) + + asyncio.run(instance._update_resource("src-a", source_user())) + asyncio.run(instance._update_resource("src-a", source_user())) + + stored = mock_config.state.destination["users"]["src-a"] + assert stored["relationships"]["roles"]["data"] == [successful_role] + instance.add_user_to_role.assert_awaited_once_with("dest-a", "role-dst-success") + mock_config.destination_client.patch.assert_awaited_once() + + def test_role_retry_persists_partial_state_and_reports_failure(self, mock_config): + """A later run retries missing roles without reporting full success.""" + instance = Users(mock_config) + failed_role = {"id": "role-dst-failed", "type": "roles"} + successful_role = {"id": "role-dst-success", "type": "roles"} + dest_user = _make_user("user-a@example.com", "shared@example.com", "dest-a") + source_user = _make_user( + "user-a@example.com", + "shared@example.com", + "src-a", + name="Updated User", + roles=[failed_role, successful_role], + ) + mock_config.state.destination["users"]["src-a"] = dest_user + instance.add_user_to_role = AsyncMock(side_effect=[False, True]) + updated_user = { + "id": "dest-a", + "type": "users", + "attributes": { + "handle": "user-a@example.com", + "email": "shared@example.com", + "name": "Updated User", + "disabled": False, + }, + } + mock_config.destination_client.patch = AsyncMock(return_value={"data": updated_user}) + + with pytest.raises(UserRoleAssignmentError) as exc_info: + asyncio.run(instance.update_resource("src-a", source_user)) + + assert exc_info.value.failed_role_ids == ("role-dst-failed",) + assert [call.args for call in instance.add_user_to_role.await_args_list] == [ + ("dest-a", "role-dst-failed"), + ("dest-a", "role-dst-success"), + ] + mock_config.destination_client.patch.assert_awaited_once() + stored = mock_config.state.destination["users"]["src-a"] + assert stored["attributes"]["name"] == "Updated User" + assert stored["relationships"]["roles"]["data"] == [successful_role] + + def test_existing_handle_takes_update_path_no_create(self, mock_config): + """e: an existing destination handle routes to the update path — no v1 or + v2 create, no duplicate.""" + mock_config.use_v1_user_api = True + instance = Users(mock_config) + dest_user = _make_user("user-a@example.com", "shared@example.com", "dest-a") + instance._existing_resources_map = {"user-a@example.com": dest_user} + mock_config.destination_client.post = AsyncMock() + mock_config.destination_client.patch = AsyncMock(return_value={"data": dest_user}) + + _id, r = asyncio.run( + instance.create_resource("src-a", _make_user("user-a@example.com", "shared@example.com", "src-a")) + ) + mock_config.destination_client.post.assert_not_called() + assert r["id"] == "dest-a"