Skip to content

feat(id-file): enable authn_mappings + team_memberships#594

Merged
michael-richey merged 9 commits into
mainfrom
feature/chun-id-targeted-sync-cli-v2
Jun 11, 2026
Merged

feat(id-file): enable authn_mappings + team_memberships#594
michael-richey merged 9 commits into
mainfrom
feature/chun-id-targeted-sync-cli-v2

Conversation

@michael-richey

@michael-richey michael-richey commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

What

  • add authn_mappings and team_memberships to _ID_FILE_SUPPORTED_TYPES
  • implement team_memberships id-file fan-out in get_resources_by_ids(...) (team_id -> N membership resources)
  • keep the id-file fetch phase side-effect free for team_memberships (no source-state mutation during fetch)
  • reject direct team_memberships.import_resource(_id=...) usage; fan-out is handled by get_resources_by_ids(...)
  • remove now-unused source-state helper methods that were added for fetch-time mutation
  • include focused unit tests for both resource types
  • use asyncio.run(...) in new tests for Python 3.12 compatibility

Why

  • ease batching these resources
  • preserve second-pass import and filter semantics for id-file runs

Validation

  • pytest tests/unit/test_team_memberships_id_file.py tests/unit/test_authn_mappings_id_file.py tests/unit/test_get_resources_by_ids_experiment.py::test_id_file_allowlist_rejects_non_monitors_v1 tests/unit/test_get_resources_by_ids_experiment.py::test_id_file_allowlist_accepts_monitors

@michael-richey

Copy link
Copy Markdown
Collaborator Author

Rationale for team_memberships id-file fan-out (team_id -> team_id:user_id):

  • --id-file provides a team id, but team memberships are naturally a set of rows keyed by (team_id, user_id).
  • The source API is team-scoped (/api/v2/team/{team_id}/memberships), so we must fetch members for that team and expand to per-membership records.
  • State/mapping for this resource already uses composite keys (team_id:user_id) via _team_membership_key, so fan-out preserves existing identity semantics and diff behavior.
  • The implementation does delete-before-write for that team prefix to remove stale memberships for users no longer in the team.
  • Empty team case intentionally raises SkipResource so we don't persist an invalid bare team_id row in state.

@michael-richey michael-richey changed the title [CHUN-11/12] feat(id-file): enable authn_mappings + team_memberships feat(id-file): enable authn_mappings + team_memberships Jun 9, 2026
@michael-richey
michael-richey marked this pull request as ready for review June 9, 2026 20:23
@michael-richey
michael-richey requested a review from a team as a code owner June 9, 2026 20:23
…geted-sync-cli-v2' into feature/chun-id-targeted-sync-cli-v2

# Conflicts:
#	datadog_sync/model/team_memberships.py
#	datadog_sync/utils/import_state.py
#	datadog_sync/utils/state.py
#	tests/unit/test_team_memberships_id_file.py
@riyazsh

riyazsh commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Codex Findings
MAJOR - datadog_sync/model/team_memberships.py#L102: the hand-rolled targeted pagination only stops when len(members_of_team) < page_size, ignoring meta.pagination.total. Reproducer: team has exactly 100 memberships, page 0 returns 100 members with total=100, then the unnecessary page 1 request hits 429/5xx. The code classifies the whole team as transient and drops the 100 complete members. Fix: stop when the fetched count reaches meta.pagination.total or reuse the existing remaining_func; add a regression test that page 1 is not called when total == page_size.
MINOR - datadog_sync/model/team_memberships.py#L150-L151: a valid empty team is reported as skipped/errored even though the method contract says a team ID fans out to “zero or more” memberships. Reproducer: /memberships returns {"data": []} for an existing team; the import emits a skipped outcome for a successful no-op. Fix: return success with an empty list, or emit a distinct success/noop outcome if callers need visibility.
NIT - datadog_sync/utils/configuration.py: the _ID_FILE_SUPPORTED_TYPES docstring still says “Monitors only in v1” after adding authn_mappings and team_memberships. Update that and the stale test wording around “non-monitors” to avoid future reviewer confusion.

@michael-richey
michael-richey merged commit 03cd5f0 into main Jun 11, 2026
11 checks passed
@michael-richey
michael-richey deleted the feature/chun-id-targeted-sync-cli-v2 branch June 11, 2026 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants