patch(v2.2): reject heterogeneous ExpectedMachine batch updates - #5678
Conversation
Backport `04dd873ad` from NVIDIA#5501 to `release/v2.2`. ExpectedMachine batch PATCH requests must select the same update fields for every item, except for the documented per-machine `bmcIpAddress` behavior. Without the handler check, `ExpectedMachine.UpdateMultiple` can apply its shared column list to a row that omitted one of those fields. Compare field presence before database or workflow work begins. Values may still differ, empty collections remain explicit clears, and `bmcIpAddress` continues to support per-machine set, clear, omit, and `null` behavior. This supports NVIDIA#5669 Signed-off-by: Chet Nichols III <chetn@nvidia.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
🔐 TruffleHog Secret Scan✅ No secrets or credentials found! Your code has been scanned for 700+ types of secrets and credentials. All clear! 🎉 🕐 Last updated: 2026-09-02 05:26:47 UTC | Commit: 80bee3f |
nv-dmendoza
left a comment
There was a problem hiding this comment.
LGTM, backports fixes
Backports
04dd873adfrom #5501 torelease/v2.2.ExpectedMachine batch PATCH requests require every item to select the same update fields, except for the documented per-machine
bmcIpAddressbehavior. Before #5501, a batch with mismatched fields could reachExpectedMachine.UpdateMultiple; its shared column list could then write a model zero value into a row that omitted one of those fields.This compares field presence before database or workflow work begins. Values may still differ, empty collections remain explicit clears, and
bmcIpAddresscontinues to support per-machine set, clear, omit, andnullbehavior.Related issues
Type of Change
Breaking Changes
Testing
Unit tests added/updated
Integration tests added/updated
Manual testing performed
No testing required (docs, internal refactor, etc.)
make rest-api/test-apipasses the complete REST API suite, including the new batch validation and the existing BMC IP behavior.make rest-fmt,make rest-lint,cargo make format-nightly, andcargo make clippypass. The full custom lint traversal stops on two existingtxn_held_across_awaitfindings in unchangedcrates/api-core/tests/integration/dns_resolution.rs.Review Findings
Model Findings Overview
The local model reviews were not repeated for this backport. The stable patch ID is
3f91903fd3561c7dc235c9fd99bc5cbb2d3f684cfor both this branch and the reviewed patch from #5501, and no code was changed after the cherry-pick.Model Findings Details
Per Chet's direction, the Codex, CodeRabbit CLI, Claude CLI, and common-nits passes were not rerun. No additional review findings or edits were produced, and no hosted CodeRabbit review was requested.