feat: ingest RackReservation with match-only user resolution#185
Merged
Conversation
Regenerated from drf-extract: adds users.user to legal fields (username) and the RackReservation.user RefInfo (plus the other restored user references). Layers on the existing cabling metadata. Full v4.6.x suite: 426 OK. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
users.user is a match-only reference type: resolved against existing rows by username, never created or updated via ingest. - transformer: _MATCH_ONLY_TYPES guard in _resolve_existing_references. A match-only ref that resolves to an existing row is emitted as a pure reference (parent gets the pk, no change for the node — a users.user change would fail validation anyway since NetBox's User requires a password we never carry). An unresolved match-only ref raises a per-entity deviation instead of a CREATE. - applier: reject any CREATE/UPDATE of a match-only type in _apply_change, covering the direct apply-change-set / bulk-apply path that bypasses the transformer (so a client changeset cannot create or rename a user). Enables dcim.rackreservation ingest (its required user FK). Tests: existing user resolves + applies; unknown/case-mismatch/missing user deviate; direct create/update user changesets rejected; user resolution is idempotent. Suite: 434 OK; ruff clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- transformer: extract _resolve_by_netbox_id + _mark_seen helpers (keeps _resolve_existing_references under the C901 threshold after adding the match-only branches) and add the match-only pure-reference short-circuit to the netbox_id (PK) resolution path for symmetry with find_existing_object. - tests: pin the direct-CREATE rejection to the match-only guard (assert the message + serializer-valid password); directly assert matched users are emitted as pure references (no users.user change); add negative assertions to the case-mismatch test. Dropped the PK-match full-NOOP idempotency test: it is confounded by dcim.rack having no natural-key match when location is null (a pre-existing, unrelated behavior) — the user-resolution idempotency it was meant to prove is covered directly instead. Suite: 434 OK; ruff clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The v4.6.0 matrix leg (largest suite: cabling + rackreservation + coverage + image build) was hitting the 10-minute job timeout; develop already runs it at ~9m45s, so it was one test away from failing regardless of this branch. Raise the timeout to give the growing v4.6.x suite headroom. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
☂️ Python Coverage
Overall Coverage
New Files
Modified Files
|
leoparente
marked this pull request as ready for review
July 21, 2026 13:10
leoparente
requested review from
MicahParks,
davidlanouette,
jajeffries,
ltucker,
marc-barry and
mfiedorowicz
as code owners
July 21, 2026 13:10
jajeffries
approved these changes
Jul 21, 2026
mfiedorowicz
approved these changes
Jul 21, 2026
Move the match-only type registry out of transformer as a private symbol and into common.py as a public constant. It is a shared contract between the transformer (plan path) and applier (direct-apply path), so it lives in the module both already depend on rather than one layer importing the other's private symbol. Unlike plugin_utils.py, common.py is not generated code, so the constant won't be clobbered by a regen. Side effect: removes the last applier -> transformer import edge. No behavior change; v4.6.x suite green (434 tests), ruff clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff3e20db4b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
☂️ Code Coverage
Overall Coverage
New Files
Modified Files
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Makes
dcim.rackreservationingestable by resolving its requireduserFK, andadds match-only resolution for
users.user: users are resolved againstexisting rows by
usernameand are never created or updated via ingest.Previously a rack reservation could not be applied at all (
Field user is required), becauseuserwas not a resolvable reference.What changes
plugin_utils.py) — updated to includeusers.user(username) in the legal fields and the
RackReservation.userreference (plusthe other restored user references).
emitted as a pure reference (the parent gets the user's pk; no change node is
emitted for the user). An unresolved user reference produces a clean
per-entity deviation instead of a create.
_apply_change.This covers the direct
apply-change-set/bulk-applypath (which bypassesthe transformer), so a client-supplied changeset cannot create or rename a user.
Why match-only
Auto-creating or renaming Django auth users from discovery/ingest data is a
privilege/security risk (which password? which permissions?). Users must
pre-exist; a reservation referencing an unknown user deviates cleanly rather than
minting one.
Scope note
Enabling
users.useras a resolvable reference restores it everywhere it isreferenced —
RackReservation.user,Owner.users,JournalEntry.created_by,and the generic-object
*_uservariants — all match-only (never created/updated).Tests (v4.6.x suite green, ruff clean)
no
users.userchange; existing user not duplicated)users.userCREATE and UPDATE changesets → rejected by the applier guard(username unchanged in the DB)
Merge order
Depends on the ingester proto + SDK changes landing first (the regenerated
plugin_utils.pymetadata reflects the newuserreference).