Skip to content

Fixes #38914 - Use ETag-aware patch_if_match for Redfish operations#930

Merged
stejskalleos merged 1 commit intotheforeman:developfrom
spesnova717:feature/redfish-etag-support
Mar 9, 2026
Merged

Fixes #38914 - Use ETag-aware patch_if_match for Redfish operations#930
stejskalleos merged 1 commit intotheforeman:developfrom
spesnova717:feature/redfish-etag-support

Conversation

@spesnova717
Copy link
Copy Markdown
Contributor

Summary

Some Redfish implementations reject PATCH requests without If-Match.
This change updates boot device operations to use
patch_if_match, enabling ETag-based conditional PATCH when needed.

Problem

Certain BMC implementations require the If-Match header
with ETag values for PATCH operations.
The previous implementation using system.patch
did not include this header, causing operations to fail on some hardware.

Solution

Changed PATCH operations in modules/bmc/redfish.rb to use patch_if_match,
which automatically includes the If-Match header with the appropriate ETag value when available.

@spesnova717 spesnova717 force-pushed the feature/redfish-etag-support branch from 9b48f9f to 1519582 Compare February 25, 2026 05:56
@spesnova717 spesnova717 marked this pull request as ready for review February 25, 2026 06:04
@spesnova717
Copy link
Copy Markdown
Contributor Author

Hi @ekohl ,

redfish_client 0.8.0 has now been officially released with ETag support included.
I've updated the dependency to use the released 0.8.0 version instead of the forked branch.

Thank you as always for your review.

ekohl
ekohl previously requested changes Feb 25, 2026
Some Redfish implementations reject PATCH requests without If-Match.
This change updates boot device operations to use
patch_if_match, enabling ETag-based conditional PATCH when needed.
Copy link
Copy Markdown
Contributor

@stejskalleos stejskalleos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🍏 LGTM

  • Scope of introduced changes is clear and does what it should
  • Well covered by the tests

My testing

This time, my testing abilities were limited to the redfishMockupServer.

curl --user admin:changeme \
  -X PUT \
  -H "Content-Type: application/json" \
  -d '{}' \
  "http://smart-proxy.local.lan/bmc/:redfish_host/chassis/config/bootdevice/pxe"

=>

{
  "action": "bios",
  "result": {
    "device": "bios",
    "reboot": null,
    "persistent": null
  }
}

and logs from the redfish mockup server:

"GET /redfish/v1/Systems/RackServer-3 HTTP/1.1"
   PATCH: Content: type=application/json and params={}
   PATCH: Data: {'Boot': {'BootSourceOverrideTarget': 'BiosSetup', 'BootSourceOverrideEnabled': 'Once'}}
application/json

The overall result looks good, and looking at the coverage in the redfish_client PR, I can say we can merge, even without tests on a real setup.

I'll keep the PR open until Monday to give the community time to comment. If there are no objections on Monday, I will merge the PR.

@stejskalleos stejskalleos dismissed ekohl’s stale review March 9, 2026 11:30

The requested change has been addressed.

@stejskalleos stejskalleos merged commit a5780bc into theforeman:develop Mar 9, 2026
10 checks passed
@stejskalleos
Copy link
Copy Markdown
Contributor

Thanks @spesnova717

@spesnova717
Copy link
Copy Markdown
Contributor Author

Thanks @stejskalleos for the testing and for merging this!
I really appreciate the verification with the redfishMockupServer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants