feat: add ActionHeaders to only include If-Match for same-URI requests#512
Draft
y-isono wants to merge 3 commits intostmcginnis:mainfrom
Draft
feat: add ActionHeaders to only include If-Match for same-URI requests#512y-isono wants to merge 3 commits intostmcginnis:mainfrom
y-isono wants to merge 3 commits intostmcginnis:mainfrom
Conversation
…ailed (stmcginnis#510)" This reverts commit a1180d3.
…ze gzip ETags (stmcginnis#511)" This reverts commit 32021c5.
feat: add ActionHeaders to only include If-Match for same-URI requests
feat: add ActionHeaders to only include If-Match for same-URI requests
Contributor
Author
Test Evidence: ActionHeaders + settingsTargetHardware: Dell PowerEdge R470 (iDRAC10) Test Codemain.go — ActionHeaders integration test (click to expand)// BMC_HOST=<bmc-ip> BMC_USER=<user> BMC_PASS=<pass> go run main.go [--dry-run] [--skip-reset]
package main
import (
"crypto/tls"
"flag"
"fmt"
"net/http"
"os"
"strings"
gofish "github.com/stmcginnis/gofish"
"github.com/stmcginnis/gofish/schemas"
)
func main() {
dryRun := flag.Bool("dry-run", false, "Only show debug info")
skipReset := flag.Bool("skip-reset", false, "Skip Reset test")
flag.Parse()
host := os.Getenv("BMC_HOST")
user := os.Getenv("BMC_USER")
pass := os.Getenv("BMC_PASS")
config := gofish.ClientConfig{
Endpoint: "https://" + host, Username: user, Password: pass, Insecure: true,
HTTPClient: &http.Client{
Transport: &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: true}},
},
}
client, _ := gofish.Connect(config)
defer client.Logout()
service := client.GetService()
systems, _ := service.Systems()
system := systems[0]
// Debug: verify ActionHeaders behavior
// ActionHeaders(ODataID) → includes If-Match
// ActionHeaders(actionURI) → empty (no If-Match)
// Test 1: SetBoot Cd OneTimeBoot (settingsTarget + @odata.etag)
system.SetBoot(&schemas.Boot{
BootSourceOverrideTarget: schemas.CdBootSource,
BootSourceOverrideEnabled: schemas.OnceBootSourceOverrideEnabled,
})
// Test 2: SetBoot PXE OneTimeBoot
system.SetBoot(&schemas.Boot{
BootSourceOverrideTarget: schemas.PxeBootSource,
BootSourceOverrideEnabled: schemas.OnceBootSourceOverrideEnabled,
})
// Test 3: Restore boot settings
system.SetBoot(&schemas.Boot{
BootSourceOverrideTarget: schemas.NoneBootSource,
BootSourceOverrideEnabled: schemas.DisabledBootSourceOverrideEnabled,
})
// Test 4: Reset (ActionHeaders — no If-Match)
system.Reset(schemas.GracefulShutdownResetType) // or OnResetType
// Test 5: VirtualMedia InsertMedia (real ISO)
isoURL := os.Getenv("ISO_URL")
vmList, _ := system.VirtualMedia() // or via Managers
cdVM := vmList[0] // find CD/DVD device
cdVM.InsertMedia(&schemas.VirtualMediaInsertMediaParameters{Image: isoURL})
// Test 6: VirtualMedia EjectMedia
cdVM.EjectMedia()
// Test 7: LogService ClearLog
managers, _ := service.Managers()
logServices, _ := managers[0].LogServices()
logServices[0].ClearLog("") // SEL log
// Test 8: EventService SubmitTestEvent
eventSvc, _ := service.EventService()
eventSvc.SubmitTestEvent(&schemas.EventServiceSubmitTestEventParameters{
EventGroupID: 123, EventID: "Test", EventType: schemas.AlertEventType,
Message: "ActionHeaders test", MessageID: "Test.1.0.Test", Severity: "OK",
})
}Integration Test Results (Dell iDRAC10)All tests passed with no 412 errors. ISO was served from an internal HTTP server.
Unit Test Results |
…ODataID Headers(targetURI) now includes If-Match only when targetURI matches the entity ODataID. This prevents sending If-Match to action POST endpoints (e.g., Reset, InsertMedia) whose URI differs from the resource, which caused 412 Precondition Failed on Dell iDRAC10. Also updates SetBoot to use @Redfish.Settings target URI with @odata.etag for concurrency control on Settings resources. Fixes stmcginnis#516
0512619 to
701a295
Compare
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.
Fixes #516
Entity.Headers()currently includesIf-Matchwith the entity's ETag on all requests, regardless of whether the target URI matches the entity's ODataID. Per RFC 7232 Section 3.1,If-Matchcompares against the target resource, so the entity's ETag does not apply when the target URI differs. This affects action POST endpoints (e.g., Reset, InsertMedia) as well as PATCH operations targeting a different URI (e.g., Settings resources).Additionally, #511 added
@Redfish.Settingssupport toSetBoot(), which this PR preserves. However, the ETag handling should be improved: instead of sanitizing the-gzipsuffix from HTTPEtagheaders, this PR fetches the Settings resource's ETag through the standard entity path, which picks up@odata.etagfrom the JSON body and is not affected by gzip content-encoding.This PR reverts #510 and #511, and replaces them with an improved approach.
1. Headers(targetURI) — URI-aware If-Match (replaces #510)
Instead of retrying on 412 Precondition Failed (#510), this PR extends
Headers()to accept atargetURIparameter:Headers(targetURI)includesIf-Matchonly whentargetURImatches the entity's ODataID. When the target URI differs — whether for action POST (e.g.,/Systems/1/Actions/ComputerSystem.Reset) or PATCH to a Settings resource —If-Matchis correctly omitted.Is412PreconditionFailed()and the 412 retry logic fromPostWithTask()— no longer needed sinceIf-Matchis not sent to different URIs in the first place.Patch(),PostWithResponse(),PostWithTask()) now pass the target URI toHeaders().2. SetBoot settingsTarget improvement (replaces #511)
SetBoot()continues to usesettingsTargetfrom@Redfish.Settings, but the ETag handling is improved:sanitizeETag()— The previous approach stripped-gzipsuffixes from HTTPEtagheaders. Instead,SetBoot()now usesGetObjectto fetch the Settings resource as an Entity, which picks up@odata.etagfrom the JSON body. The JSON@odata.etagis not affected by gzip content-encoding, eliminating the need for suffix handling. When@odata.etagis not present in the JSON body,Entity.Get()falls back to the HTTPEtagheader as before.UpdateBootAttributesApplyAt()— Updated with the sameGetObject+Entity.Headers()approach.Testing
Unit tests: All existing tests pass (
go test ./schemas/). New tests added:TestHeaders(4 cases: same URI, different URI, no ETag, disabled),TestSetBootWithSettings,TestSetBootWithoutSettings.Integration tests on Dell PowerEdge R470 (iDRAC10): Reset, InsertMedia (real ISO), EjectMedia, SetBoot PXE/Cd, SubmitTestEvent — all pass.
References
— 412 fallback
— settingsTarget + sanitizeETag