Skip to content

Disable If-Match on SimpleUpdate action POST to prevent 412 from Dell Idrac#1020

Merged
afritzler merged 1 commit into
ironcore-dev:mainfrom
atd9876:fix/simple-update-etag-412
Jul 16, 2026
Merged

Disable If-Match on SimpleUpdate action POST to prevent 412 from Dell Idrac#1020
afritzler merged 1 commit into
ironcore-dev:mainfrom
atd9876:fix/simple-update-etag-412

Conversation

@atd9876

@atd9876 atd9876 commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Proposed Changes

Dell iDRAC returns @odata.etag on the UpdateService resource from schema version v1_15_0 onwards (confirmed on R770 v1_15_0 and R6715 v1_16_0). gofish's Entity.PostWithResponse automatically includes that ETag as an If-Match header on any POST. SimpleUpdate is an action endpoint, not a resource modification, so the BMC rejects If-Match with 412 Precondition Failed regardless of whether the ETag value is current.

  • call DisableEtagMatch(true) on the UpdateService entity before calling PostWithResponse in the shared upgradeVersion helper. This applies to all vendors (Dell, HPE, Lenovo, local) since they all go through the same code path.
  • add @odata.etag to the mock UpdateService fixture and enforce the correct 412 behaviour in handleSimpleUpdate so that the test suite would catch any future regression.

Fixes #1015

Summary by CodeRabbit

  • Bug Fixes

    • Improved firmware upgrade compatibility with Redfish services that enforce ETag-based request validation.
    • Firmware upgrade requests now avoid sending an unnecessary conditional-match header, preventing precondition failures during updates.
    • Update service handling now correctly reports precondition failures when request validation is required.
  • Tests

    • Added coverage to verify successful firmware upgrades against ETag-aware Redfish services.

… Dell iDRAC

Dell iDRAC returns @odata.etag on the UpdateService resource from schema
version v1_15_0 onwards (confirmed on R770 v1_15_0 and R6715 v1_16_0).
gofish's Entity.PostWithResponse automatically includes that ETag as an
If-Match header on any POST. SimpleUpdate is an action endpoint, not a
resource modification, so the BMC rejects If-Match with 412 Precondition
Failed regardless of whether the ETag value is current.

Fix: call DisableEtagMatch(true) on the UpdateService entity before
calling PostWithResponse in the shared upgradeVersion helper. This
applies to all vendors (Dell, HPE, Lenovo, local) since they all go
through the same code path.

Also add @odata.etag to the mock UpdateService fixture and enforce the
correct 412 behaviour in handleSimpleUpdate so that the test suite would
catch any future regression.

Fixes ironcore-dev#1015

Signed-off-by: Andrew Dodds <andrew.dodds@sap.com>
@atd9876
atd9876 requested a review from a team as a code owner July 15, 2026 15:27
@github-actions github-actions Bot added the bug Something isn't working label Jul 15, 2026
@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The UpdateService mock now exposes an ETag and rejects conditional SimpleUpdate requests. The firmware upgrade path disables ETag matching before posting, with a regression test verifying that no If-Match header is sent.

Changes

ETag handling

Layer / File(s) Summary
Mock UpdateService ETag contract
bmc/mock/server/data/UpdateService/index.json, bmc/mock/server/server.go
The mock resource includes @odata.etag, and SimpleUpdate returns HTTP 412 when If-Match is present.
Upgrade request ETag suppression
bmc/oem_helpers.go, bmc/oem_helpers_test.go
upgradeVersion disables ETag matching before posting to SimpleUpdate, with an in-memory Redfish test verifying the request succeeds without If-Match.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: nagadeesh-nagaraja

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The code changes implement the issue's required fix by disabling ETag matching before SimpleUpdate and updating the mock/test coverage.
Out of Scope Changes check ✅ Passed The added mock fixture, handler behavior, and test support are directly tied to the SimpleUpdate If-Match regression.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title clearly summarizes the main change: disabling If-Match on SimpleUpdate POSTs to avoid Dell iDRAC 412 errors.
Description check ✅ Passed The description matches the template with a Proposed Changes section and a Fixes reference, and it explains the key behavioral change.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@afritzler afritzler changed the title fix: disable If-Match on SimpleUpdate action POST to prevent 412 from Dell Idrac Disable If-Match on SimpleUpdate action POST to prevent 412 from Dell Idrac Jul 15, 2026
@afritzler
afritzler merged commit 7bc956a into ironcore-dev:main Jul 16, 2026
16 checks passed
@github-project-automation github-project-automation Bot moved this to Done in Roadmap Jul 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Dell iDRAC: BIOSVersion (and BMCVersion) upgrade fails permanently with 412 PreconditionFailed — ETag not sent on SimpleUpdate POST

2 participants