From a1f5d9869ebb97db4a6a4bb20154b77373e44a00 Mon Sep 17 00:00:00 2001 From: Andrew Dodds Date: Tue, 8 Sep 2026 14:44:49 +0100 Subject: [PATCH] feat(bmc): add FetchETags and write-only drift detection helpers Signed-off-by: Andrew Dodds --- bmc/bmc.go | 5 ++ bmc/mock/server/server.go | 127 ++++++++++++++++++++++++++-- bmc/oem_helpers.go | 103 ++++++++++++++++++++--- bmc/redfish.go | 5 ++ bmc/redfish_dell.go | 171 +++++++++++++++++++++++++++++--------- bmc/redfish_hpe.go | 10 ++- bmc/redfish_lenovo.go | 10 ++- bmc/redfish_local.go | 40 ++++++++- 8 files changed, 402 insertions(+), 69 deletions(-) diff --git a/bmc/bmc.go b/bmc/bmc.go index b56645067..af9a6a77c 100644 --- a/bmc/bmc.go +++ b/bmc/bmc.go @@ -122,6 +122,9 @@ type BMCSettingsManager interface { // CheckBMCAttributes checks if the BMC attributes are valid and returns whether a reset is required. CheckBMCAttributes(ctx context.Context, UUID string, attrs schemas.SettingsAttributes) (reset bool, err error) + + // FetchETags returns the current ETag for each URI, or nil if ETags are unavailable for this vendor. + FetchETags(ctx context.Context, uris []string) (map[string]string, error) } // FirmwareUpdater drives BIOS and BMC firmware upgrades. @@ -401,6 +404,8 @@ type ApplyResult struct { // ETag from the response header or follow-up GET. // May be a real ETag (e.g. "W/\"abc\"") or a body hash (prefixed "hash:sha256:"). ETag string + // IsPost is true when the value was applied via HTTP POST (ephemeral resource). + IsPost bool } // GetBMCAttributeValuesRequest bundles the inputs for GetBMCAttributeValues. diff --git a/bmc/mock/server/server.go b/bmc/mock/server/server.go index da62ca821..456fefa34 100644 --- a/bmc/mock/server/server.go +++ b/bmc/mock/server/server.go @@ -14,6 +14,7 @@ import ( "net/http" "path" "slices" + "strconv" "strings" "sync" "time" @@ -202,6 +203,7 @@ type MockServer struct { handler http.Handler mu sync.RWMutex overrides map[string]any + etags map[string]string // file path → current ETag value upgradeGen int64 // incremented on each SimpleUpdate to cancel stale goroutines dellJobGen int64 // incremented on each Dell repository install to cancel stale goroutines dellRepoState dellRepoUpdateState // GetRepoBasedUpdateList pending-package simulation state (see handleDellGetRepoBasedUpdateList) @@ -252,6 +254,7 @@ func NewMockServer(log logr.Logger, addr string, opts ...Option) *MockServer { addr: addr, log: log, overrides: make(map[string]any), + etags: make(map[string]string), upgradedResources: make(map[string]string), accounts: loadAccountsFromEmbedded(), // onCreate hooks run after a new collection member is stored. @@ -398,8 +401,13 @@ func (s *MockServer) handleGet(w http.ResponseWriter, r *http.Request) { if hasOverride { copied = deepCopyAny(cached) } + etag := s.etags[filePath] s.mu.RUnlock() + if etag != "" { + w.Header().Set("ETag", etag) + } + if hasOverride { s.writeJSON(w, http.StatusOK, copied) return @@ -544,6 +552,17 @@ func (s *MockServer) handlePatch(w http.ResponseWriter, r *http.Request) { return } + // Validate If-Match before any side effects. + if ifMatch := r.Header.Get("If-Match"); ifMatch != "" && ifMatch != "*" { + s.mu.RLock() + current := s.etags[filePath] + s.mu.RUnlock() + if current != "" && current != ifMatch { + http.Error(w, "Precondition Failed", http.StatusPreconditionFailed) + return + } + } + if err := s.applyBiosSettings(r.URL.Path, update); err != nil { s.handleError(w, r, err) return @@ -556,16 +575,24 @@ func (s *MockServer) handlePatch(w http.ResponseWriter, r *http.Request) { mergeJSON(base, update) - // Keep the authentication store in sync when Password is updated via PATCH. - if newPwd, ok := update["Password"].(string); ok && newPwd != "" { - if username, ok := base["UserName"].(string); ok && username != "" { - s.mu.Lock() - s.accounts[username] = newPwd - s.mu.Unlock() + // Extract password update for atomic commit inside checkIfMatchAndSave. + var newPwd, pwdUsername string + if p, ok := update["Password"].(string); ok && p != "" { + if u, ok := base["UserName"].(string); ok && u != "" { + newPwd = p + pwdUsername = u } } - s.saveResource(filePath, base) + newETag, ok := s.checkIfMatchAndSave(filePath, r.Header.Get("If-Match"), base, pwdUsername, newPwd) + if !ok { + http.Error(w, "Precondition Failed", http.StatusPreconditionFailed) + return + } + if newETag != "" { + w.Header().Set("ETag", newETag) + } + w.WriteHeader(http.StatusNoContent) } @@ -1188,6 +1215,25 @@ func (s *MockServer) GetBMCSettingAttr(managerID string) map[string]any { return attrs } +// SetBMCSettingAttr overwrites a single BMC attribute without bumping the ETag. +// Use in tests to simulate out-of-band drift invisible to the ETag fast-path. +func (s *MockServer) SetBMCSettingAttr(managerID, key string, value any) { + filePath := fmt.Sprintf("data/Managers/%s/index.json", managerID) + s.mu.Lock() + defer s.mu.Unlock() + resource, err := s.loadResourceLocked(filePath) + if err != nil { + return + } + attrs, ok := resource[attributesKey].(map[string]any) + if !ok { + attrs = make(map[string]any) + resource[attributesKey] = attrs + } + attrs[key] = value + s.overrides[filePath] = resource +} + // ResetBMCSettings resets the BMC attribute state on the server to defaults, // clearing both current and pending attributes. managerID is the folder name under data/Managers/ (e.g. "BMC"). func (s *MockServer) ResetBMCSettings(managerID string) { @@ -1414,6 +1460,29 @@ func (s *MockServer) saveResource(filePath string, data map[string]any) { s.overrides[filePath] = data } +// checkIfMatchAndSave validates If-Match, writes data, and bumps the ETag atomically. +// Returns the new ETag and true on success; returns "", false when If-Match is stale. +// If username and password are non-empty, s.accounts is updated in the same critical section. +func (s *MockServer) checkIfMatchAndSave(filePath, ifMatch string, data map[string]any, username, password string) (string, bool) { + s.mu.Lock() + defer s.mu.Unlock() + if ifMatch != "" && ifMatch != "*" { + if current := s.etags[filePath]; current != "" && current != ifMatch { + return "", false + } + } + s.overrides[filePath] = data + if username != "" && password != "" { + s.accounts[username] = password + } + if etag, ok := s.etags[filePath]; ok && etag != "" { + newETag := bumpETag(etag) + s.etags[filePath] = newETag + return newETag, true + } + return "", true +} + func (s *MockServer) isLocked(resource map[string]any) bool { locked, _ := resource["resourceLock"].(string) return locked == "Locked" @@ -1512,6 +1581,50 @@ func (s *MockServer) ResetAccounts() { s.accounts = loadAccountsFromEmbedded() } +// SetResourceETag sets the ETag for the resource at urlPath. +func (s *MockServer) SetResourceETag(urlPath, etag string) { + fp := resolvePath(urlPath) + s.mu.Lock() + defer s.mu.Unlock() + s.etags[fp] = etag +} + +// GetResourceETag returns the current ETag for the resource at urlPath, or "". +func (s *MockServer) GetResourceETag(urlPath string) string { + fp := resolvePath(urlPath) + s.mu.RLock() + defer s.mu.RUnlock() + return s.etags[fp] +} + +// bumpETag increments the numeric suffix of an ETag (e.g. W/"v1" → W/"v2"). +func bumpETag(etag string) string { + weak := strings.HasPrefix(etag, "W/") + prefix := "" + if weak { + prefix = "W/" + } + inner := strings.TrimPrefix(etag, "W/") + inner = strings.Trim(inner, "\"") + // Hyphen-delimited suffix. + if idx := strings.LastIndexByte(inner, '-'); idx >= 0 { + if n, err := strconv.ParseInt(inner[idx+1:], 10, 64); err == nil { + return fmt.Sprintf("%s\"%s-%d\"", prefix, inner[:idx], n+1) + } + } + // Trailing decimal sequence (no delimiter). + i := len(inner) + for i > 0 && inner[i-1] >= '0' && inner[i-1] <= '9' { + i-- + } + if i < len(inner) { + if n, err := strconv.ParseInt(inner[i:], 10, 64); err == nil { + return fmt.Sprintf("%s\"%s%d\"", prefix, inner[:i], n+1) + } + } + return fmt.Sprintf("%s\"%s-1\"", prefix, inner) +} + // Start starts the mock server and stops on ctx cancellation. func (s *MockServer) Start(ctx context.Context) error { if s.handler == nil { diff --git a/bmc/oem_helpers.go b/bmc/oem_helpers.go index 599c1d205..0429b0ee6 100644 --- a/bmc/oem_helpers.go +++ b/bmc/oem_helpers.go @@ -182,22 +182,34 @@ func httpBasedGetBMCSettingAttribute(c schemas.Client, attributes map[string]str if isSubMap(respData, dataMap) { result[key] = data } else { - result[key] = string(respRawBody) + // All desired fields null: write-only semantics — return nil. + allNull := len(dataMap) > 0 + for k := range dataMap { + if v, ok := respData[k]; !ok || v != nil { + allNull = false + break + } + } + if allNull { + result[key] = nil + } else { + result[key] = string(respRawBody) + } } } return result, errors.Join(errs...) } -// httpBasedUpdateBMCAttributes applies BMC attributes via HTTP POST/PATCH. -// Shared by HPE and Lenovo which use "POST " or "PATCH " format attributes. -func httpBasedUpdateBMCAttributes(c schemas.Client, attrs schemas.SettingsAttributes, applyTime schemas.SettingsApplyTime) error { +// httpBasedUpdateBMCAttributes applies BMC attributes via HTTP POST/PATCH, returning URI+ETag per key. +func httpBasedUpdateBMCAttributes(c schemas.Client, attrs schemas.SettingsAttributes, applyTime schemas.SettingsApplyTime) (map[string]ApplyResult, error) { if applyTime != schemas.ImmediateSettingsApplyTime { - return fmt.Errorf("does not support scheduled apply time for BMC attributes") + return nil, fmt.Errorf("does not support scheduled apply time for BMC attributes") } if c == nil { - return fmt.Errorf("failed to get client from gofish service") + return nil, fmt.Errorf("failed to get client from gofish service") } okCodes := []int{http.StatusOK, http.StatusAccepted, http.StatusNoContent, http.StatusCreated} + results := make(map[string]ApplyResult, len(attrs)) var errs []error for attr, value := range attrs { parts := strings.Fields(attr) @@ -213,14 +225,13 @@ func httpBasedUpdateBMCAttributes(c schemas.Client, attrs schemas.SettingsAttrib var err error jsonBytes, err = json.Marshal(value) if err != nil { - errs = append(errs, fmt.Errorf("failed to marshal spec data for url %s: %w\nbody: %v", parts[1], err, value)) + errs = append(errs, fmt.Errorf("failed to marshal spec data for url %s: %w\nbody: %v", url, err, value)) continue } } valueMap := map[string]any{} - err := json.Unmarshal(jsonBytes, &valueMap) - if err != nil { - errs = append(errs, fmt.Errorf("failed to unmarshal spec data for url %s: %w\nbody: %v", parts[1], err, value)) + if err := json.Unmarshal(jsonBytes, &valueMap); err != nil { + errs = append(errs, fmt.Errorf("failed to unmarshal spec data for url %s: %w\nbody: %v", url, err, value)) continue } switch parts[0] { @@ -230,25 +241,60 @@ func httpBasedUpdateBMCAttributes(c schemas.Client, attrs schemas.SettingsAttrib errs = append(errs, fmt.Errorf("failed to POST attribute %s to URL %s: %w", attr, url, err)) continue } + resp.Body.Close() // nolint: errcheck if !slices.Contains(okCodes, resp.StatusCode) { errs = append(errs, fmt.Errorf("failed to POST attribute %s: received status code %d", attr, resp.StatusCode)) continue } + resourceURI := resp.Header.Get("Location") + if resourceURI == "" { + resourceURI = url + } + results[attr] = ApplyResult{ + URI: resourceURI, + ETag: resp.Header.Get("ETag"), + IsPost: true, + } + case http.MethodPatch: - resp, err := c.Patch(url, valueMap) + var ifMatchHeader map[string]string + getResp, getErr := c.Get(url) + if getErr != nil { + errs = append(errs, fmt.Errorf("failed to GET ETag for PATCH %s: %w", url, getErr)) + continue + } + getResp.Body.Close() // nolint: errcheck + if getResp.StatusCode < http.StatusOK || getResp.StatusCode >= http.StatusMultipleChoices { + errs = append(errs, fmt.Errorf("failed to GET ETag for PATCH %s: status %d", url, getResp.StatusCode)) + continue + } + if etag := getResp.Header.Get("ETag"); etag != "" { + ifMatchHeader = map[string]string{"If-Match": etag} + } + resp, err := c.PatchWithHeaders(url, valueMap, ifMatchHeader) if err != nil { errs = append(errs, fmt.Errorf("failed to PATCH attribute %s to URL %s: %w", attr, url, err)) continue } + resp.Body.Close() // nolint: errcheck if !slices.Contains(okCodes, resp.StatusCode) { errs = append(errs, fmt.Errorf("failed to PATCH attribute %s: received status code %d", attr, resp.StatusCode)) continue } + appliedETag := resp.Header.Get("ETag") + if appliedETag == "" { + if getResp, err := c.Get(url); err == nil { + getResp.Body.Close() // nolint: errcheck + appliedETag = getResp.Header.Get("ETag") + } + } + results[attr] = ApplyResult{URI: url, ETag: appliedETag} + default: errs = append(errs, fmt.Errorf("unsupported HTTP method %s for attribute %s", parts[0], attr)) } } - return errors.Join(errs...) + return results, errors.Join(errs...) } // isSubMap checks if sub is a subset of main (recursively for nested maps). @@ -296,6 +342,10 @@ func checkAttributes( if entryAttribute.ResetRequired { reset = true } + // Password attrs: value is opaque and cannot be validated against registry bounds — skip. + if entryAttribute.Type == schemas.PasswordAttributeType { + continue + } switch entryAttribute.Type { case schemas.IntegerAttributeType: if _, ok := value.(int); !ok { @@ -418,3 +468,32 @@ func checkPendingComponentUpgrade(ctx context.Context, base *RedfishBaseBMC, com return false, nil } + +// httpFetchETags issues a GET for each URI and returns URI → ETag. Shared by HPE and Lenovo. +func httpFetchETags(c schemas.Client, uris []string) (map[string]string, error) { + if c == nil { + return nil, fmt.Errorf("failed to get client for FetchETags") + } + result := make(map[string]string, len(uris)) + var errs []error + for _, uri := range uris { + resp, err := c.Get(uri) + if err != nil { + // 404: POST-created resource gone — treat as empty, not an error. + var redfishErr *schemas.Error + if errors.As(err, &redfishErr) && redfishErr.HTTPReturnedStatusCode == http.StatusNotFound { + result[uri] = "" + continue + } + errs = append(errs, fmt.Errorf("FetchETags GET %s: %w", uri, err)) + continue + } + resp.Body.Close() // nolint: errcheck + if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices { + errs = append(errs, fmt.Errorf("FetchETags GET %s: unexpected status %d", uri, resp.StatusCode)) + continue + } + result[uri] = resp.Header.Get("ETag") + } + return result, errors.Join(errs...) +} diff --git a/bmc/redfish.go b/bmc/redfish.go index 09e0af317..71cd23680 100644 --- a/bmc/redfish.go +++ b/bmc/redfish.go @@ -590,6 +590,11 @@ func (r *RedfishBaseBMC) SetBMCAttributesImmediately(_ context.Context, _ string return nil, fmt.Errorf("BMC attribute operations not supported for manufacturer %q", r.manufacturer) } +// FetchETags returns nil; callers must treat nil as "ETags unavailable, fall back to full value-map GET". +func (r *RedfishBaseBMC) FetchETags(_ context.Context, _ []string) (map[string]string, error) { + return nil, nil +} + // SetBootOrder sets bios boot order func (r *RedfishBaseBMC) SetBootOrder(ctx context.Context, systemURI string, bootOrder []string) error { system, err := r.getSystemFromUri(ctx, systemURI) diff --git a/bmc/redfish_dell.go b/bmc/redfish_dell.go index e01eb89ec..4359cc9be 100644 --- a/bmc/redfish_dell.go +++ b/bmc/redfish_dell.go @@ -211,14 +211,43 @@ func (r *DellRedfishBMC) GetBMCAttributeValues(ctx context.Context, req GetBMCAt return nil, nil } + genericAttrs := map[string]string{} + dellAttrNames := map[string]string{} + for k, v := range attributes { + if parts := strings.Fields(k); len(parts) == 2 && + (parts[0] == http.MethodPost || parts[0] == http.MethodPatch) { + genericAttrs[k] = v + } else { + dellAttrNames[k] = v + } + } + + result := make(schemas.SettingsAttributes) + + if len(genericAttrs) > 0 { + manager, err := r.getManagerForOEM() + if err != nil { + return nil, err + } + genericResult, err := httpBasedGetBMCSettingAttribute(manager.GetClient(), genericAttrs) + maps.Copy(result, genericResult) + if err != nil { + return result, err + } + } + + if len(dellAttrNames) == 0 { + return result, nil + } + manager, err := r.getManagerForOEM() if err != nil { - return nil, err + return result, err } bmcDellAttributes, err := r.getCurrentBMCSettingAttribute(manager) if err != nil { - return nil, err + return result, err } var mergedBMCAttributes = make(schemas.SettingsAttributes) @@ -227,7 +256,7 @@ func (r *DellRedfishBMC) GetBMCAttributeValues(ctx context.Context, req GetBMCAt if _, ok := mergedBMCAttributes[k]; !ok { mergedBMCAttributes[k] = v } else { - return nil, + return result, fmt.Errorf("duplicate attributes in BMC settings are not supported duplicate key %v. in attribute %v", k, bmcDellAttributes) } @@ -236,15 +265,14 @@ func (r *DellRedfishBMC) GetBMCAttributeValues(ctx context.Context, req GetBMCAt filteredAttr, err := r.getFilteredBMCRegistryAttributes(manager, false, false) if err != nil { - return nil, err + return result, err } if len(filteredAttr) == 0 { - return nil, fmt.Errorf("'ManagerAttributeRegistry' not found") + return result, fmt.Errorf("'ManagerAttributeRegistry' not found") } - result := make(schemas.SettingsAttributes, len(attributes)) var errs []error - for name := range attributes { + for name := range dellAttrNames { var entry schemas.Attributes var ok bool if entry, ok = filteredAttr[name]; !ok { @@ -358,18 +386,47 @@ func (r *DellRedfishBMC) SetBMCAttributesImmediately(ctx context.Context, bmcUUI return nil, nil } + genericAttrs := schemas.SettingsAttributes{} + dellAttrs := schemas.SettingsAttributes{} + for key, value := range attributes { + if parts := strings.Fields(key); len(parts) == 2 && + (parts[0] == http.MethodPost || parts[0] == http.MethodPatch) { + genericAttrs[key] = value + } else { + dellAttrs[key] = value + } + } + + results := make(map[string]ApplyResult) + + if len(genericAttrs) > 0 { + manager, err := r.getManagerForOEM() + if err != nil { + return results, err + } + genericResults, err := httpBasedUpdateBMCAttributes(manager.GetClient(), genericAttrs, schemas.ImmediateSettingsApplyTime) + maps.Copy(results, genericResults) + if err != nil { + return results, err + } + } + + if len(dellAttrs) == 0 { + return results, nil + } + manager, err := r.getManagerForOEM() if err != nil { - return nil, err + return results, err } bmcAttrValues, err := r.getCurrentBMCSettingAttribute(manager) if err != nil { - return nil, err + return results, err } payloads := make(map[string]schemas.SettingsAttributes, len(bmcAttrValues)) - for key, value := range attributes { + for key, value := range dellAttrs { for _, eachAttr := range bmcAttrValues { if _, ok := eachAttr.Attributes[key]; ok { target := eachAttr.Settings.SettingsObject @@ -377,7 +434,7 @@ func (r *DellRedfishBMC) SetBMCAttributesImmediately(ctx context.Context, bmcUUI target = eachAttr.SourceURI } if target == "" { - return nil, fmt.Errorf("attribute '%v' has no target endpoint to patch", key) + return results, fmt.Errorf("attribute '%v' has no target endpoint to patch", key) } if data, ok := payloads[target]; ok { data[key] = value @@ -393,47 +450,79 @@ func (r *DellRedfishBMC) SetBMCAttributesImmediately(ctx context.Context, bmcUUI if len(payloads) > 0 { var errs []error for settingPath, payload := range payloads { - etag, err := func() (string, error) { - resp, err := manager.GetClient().Get(settingPath) - if err != nil { - return "", err - } - defer resp.Body.Close() // nolint: errcheck - return resp.Header.Get("ETag"), nil - }() - if err != nil { - errs = append(errs, fmt.Errorf("failed to get Etag for %v: %w", settingPath, err)) - continue - } - - data := map[string]any{"Attributes": payload} - data["@Redfish.SettingsApplyTime"] = map[string]string{"ApplyTime": string(schemas.ImmediateSettingsApplyTime)} - var header = make(map[string]string) - if etag != "" { - header["If-Match"] = etag - } - - err = func() error { - resp, err := manager.GetClient().PatchWithHeaders(settingPath, data, header) - if err != nil { - return err - } - defer resp.Body.Close() // nolint: errcheck - return nil - }() + patchETag, err := r.dellPatchPayload(manager.GetClient(), settingPath, payload) if err != nil { errs = append(errs, fmt.Errorf("failed to patch settings at %v: %w", settingPath, err)) continue } + for key := range payload { + results[key] = ApplyResult{URI: settingPath, ETag: patchETag} + } } if len(errs) > 0 { - return nil, fmt.Errorf("some settings failed to apply %v", errs) + return results, fmt.Errorf("some settings failed to apply %v", errs) } } + + return results, nil +} + +// dellPatchPayload applies a single Dell DellAttributes payload via PATCH and returns the post-update ETag. +func (r *DellRedfishBMC) dellPatchPayload(c schemas.Client, settingPath string, payload schemas.SettingsAttributes) (string, error) { + resp, err := c.Get(settingPath) + if err != nil { + return "", fmt.Errorf("failed to get ETag for %v: %w", settingPath, err) + } + resp.Body.Close() // nolint: errcheck + if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices { + return "", fmt.Errorf("GET %s returned status %d", settingPath, resp.StatusCode) + } + etag := resp.Header.Get("ETag") + + data := map[string]any{"Attributes": payload} + data["@Redfish.SettingsApplyTime"] = map[string]string{"ApplyTime": string(schemas.ImmediateSettingsApplyTime)} + var header = make(map[string]string) + if etag != "" { + header["If-Match"] = etag + } + + patchResp, err := c.PatchWithHeaders(settingPath, data, header) + if err != nil { + return "", err + } + defer patchResp.Body.Close() // nolint: errcheck + if patchResp.StatusCode < http.StatusOK || patchResp.StatusCode >= http.StatusMultipleChoices { + return "", fmt.Errorf("PATCH %s returned status %d", settingPath, patchResp.StatusCode) + } + if e := patchResp.Header.Get("ETag"); e != "" { + return e, nil + } + getResp, err := c.Get(settingPath) + if err != nil { + return "", nil + } + defer getResp.Body.Close() // nolint: errcheck + return getResp.Header.Get("ETag"), nil +} + +// FetchETags returns nil; Dell DellAttributes are a monolithic resource with no per-attribute ETag signal. +func (r *DellRedfishBMC) FetchETags(_ context.Context, _ []string) (map[string]string, error) { return nil, nil } func (r *DellRedfishBMC) CheckBMCAttributes(ctx context.Context, bmcUUID string, attrs schemas.SettingsAttributes) (bool, error) { + dellAttrs := schemas.SettingsAttributes{} + for k, v := range attrs { + if parts := strings.Fields(k); len(parts) == 2 && + (parts[0] == http.MethodPost || parts[0] == http.MethodPatch) { + continue + } + dellAttrs[k] = v + } + if len(dellAttrs) == 0 { + return false, nil + } + manager, err := r.getManagerForOEM() if err != nil { return false, err @@ -454,7 +543,7 @@ func (r *DellRedfishBMC) CheckBMCAttributes(ctx context.Context, bmcUUID string, if len(filteredAttr) == 0 { return false, nil } - return checkAttributes(attrs, filteredAttr) + return checkAttributes(dellAttrs, filteredAttr) } func (r *DellRedfishBMC) dellBuildRequestBody(parameters *schemas.UpdateServiceSimpleUpdateParameters) *SimpleUpdateRequestBody { diff --git a/bmc/redfish_hpe.go b/bmc/redfish_hpe.go index 0e725c274..9a14da377 100644 --- a/bmc/redfish_hpe.go +++ b/bmc/redfish_hpe.go @@ -34,18 +34,22 @@ func (r *HPERedfishBMC) GetBMCPendingAttributeValues(_ context.Context, _ string return schemas.SettingsAttributes{}, nil } -func (r *HPERedfishBMC) SetBMCAttributesImmediately(ctx context.Context, _ string, attributes schemas.SettingsAttributes) (map[string]ApplyResult, error) { +func (r *HPERedfishBMC) SetBMCAttributesImmediately(_ context.Context, _ string, attributes schemas.SettingsAttributes) (map[string]ApplyResult, error) { if len(attributes) == 0 { return nil, nil } - err := httpBasedUpdateBMCAttributes(r.client.GetService().GetClient(), attributes, schemas.ImmediateSettingsApplyTime) - return nil, err + return httpBasedUpdateBMCAttributes(r.client.GetService().GetClient(), attributes, schemas.ImmediateSettingsApplyTime) } func (r *HPERedfishBMC) CheckBMCAttributes(_ context.Context, _ string, _ schemas.SettingsAttributes) (bool, error) { return false, nil } +// FetchETags returns the current ETag for each URI. HPE iLO exposes ETags on BIOS settings resources. +func (r *HPERedfishBMC) FetchETags(_ context.Context, uris []string) (map[string]string, error) { + return httpFetchETags(r.client.GetService().GetClient(), uris) +} + // CreateEventSubscription overrides the base implementation to omit DeliveryRetryPolicy. // HPE iLO firmware does not support the DeliveryRetryPolicy property in EventDestination // POST requests and returns: "PropertyNotWritableOrUnknown: DeliveryRetryPolicy is not writable or unknown" diff --git a/bmc/redfish_lenovo.go b/bmc/redfish_lenovo.go index 283ca437c..be1995f87 100644 --- a/bmc/redfish_lenovo.go +++ b/bmc/redfish_lenovo.go @@ -40,18 +40,22 @@ func (r *LenovoRedfishBMC) GetBMCPendingAttributeValues(_ context.Context, _ str return schemas.SettingsAttributes{}, nil } -func (r *LenovoRedfishBMC) SetBMCAttributesImmediately(ctx context.Context, _ string, attributes schemas.SettingsAttributes) (map[string]ApplyResult, error) { +func (r *LenovoRedfishBMC) SetBMCAttributesImmediately(_ context.Context, _ string, attributes schemas.SettingsAttributes) (map[string]ApplyResult, error) { if len(attributes) == 0 { return nil, nil } - err := httpBasedUpdateBMCAttributes(r.client.GetService().GetClient(), attributes, schemas.ImmediateSettingsApplyTime) - return nil, err + return httpBasedUpdateBMCAttributes(r.client.GetService().GetClient(), attributes, schemas.ImmediateSettingsApplyTime) } func (r *LenovoRedfishBMC) CheckBMCAttributes(_ context.Context, _ string, _ schemas.SettingsAttributes) (bool, error) { return false, nil } +// FetchETags returns the current ETag for each URI. Lenovo XCC exposes ETags on BIOS Pending resources. +func (r *LenovoRedfishBMC) FetchETags(_ context.Context, uris []string) (map[string]string, error) { + return httpFetchETags(r.client.GetService().GetClient(), uris) +} + // --- Firmware upgrade overrides --- func (r *LenovoRedfishBMC) lenovoBuildRequestBody(parameters *schemas.UpdateServiceSimpleUpdateParameters) *SimpleUpdateRequestBody { diff --git a/bmc/redfish_local.go b/bmc/redfish_local.go index fdd2d6707..3f6b1feee 100644 --- a/bmc/redfish_local.go +++ b/bmc/redfish_local.go @@ -7,6 +7,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "io" "net/http" @@ -66,8 +67,7 @@ func (r *RedfishLocalBMC) GetBiosUpgradeTask(ctx context.Context, _ string, task return getUpgradeTask(ctx, r.RedfishBaseBMC, taskURI, localParseTaskDetails) } -// SetBMCAttributesImmediately sets BMC attributes via HTTP PATCH to the BMC Settings endpoint. -// Navigates from the manager's @Redfish.Settings.SettingsObject link, mirroring the Dell pattern. +// SetBMCAttributesImmediately sets BMC attributes via HTTP PATCH and returns URI+ETag per key. func (r *RedfishLocalBMC) SetBMCAttributesImmediately(ctx context.Context, bmcUUID string, attributes schemas.SettingsAttributes) (map[string]ApplyResult, error) { if len(attributes) == 0 { return nil, nil @@ -94,7 +94,13 @@ func (r *RedfishLocalBMC) SetBMCAttributesImmediately(ctx context.Context, bmcUU if resp.StatusCode != http.StatusNoContent && resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusAccepted { return nil, fmt.Errorf("PATCH %s returned status %d", managerData.Settings.SettingsObject, resp.StatusCode) } - return nil, nil + + etag := resp.Header.Get("ETag") + results := make(map[string]ApplyResult, len(attributes)) + for key := range attributes { + results[key] = ApplyResult{URI: managerData.Settings.SettingsObject, ETag: etag} + } + return results, nil } // GetBMCAttributeValues retrieves specific BMC attribute values via HTTP from the BMC manager. @@ -189,6 +195,34 @@ func (r *RedfishLocalBMC) CheckBMCAttributes(ctx context.Context, UUID string, a return checkAttributes(attrs, filtered) } +// FetchETags returns the ETag for each URI. A 404 stores an empty string to support ephemeral POST resources. +func (r *RedfishLocalBMC) FetchETags(ctx context.Context, uris []string) (map[string]string, error) { + if len(uris) == 0 { + return nil, nil + } + manager, err := r.GetManager("") + if err != nil { + return nil, fmt.Errorf("failed to get manager for ETag fetch: %w", err) + } + c := manager.GetClient() + result := make(map[string]string, len(uris)) + for _, uri := range uris { + resp, err := c.Get(uri) + if err != nil { + // 404: POST-created resource gone — treat as empty, not an error. + var redfishErr *schemas.Error + if errors.As(err, &redfishErr) && redfishErr.HTTPReturnedStatusCode == http.StatusNotFound { + result[uri] = "" + continue + } + return result, fmt.Errorf("failed to GET %s for ETag: %w", uri, err) + } + result[uri] = resp.Header.Get("ETag") + _ = resp.Body.Close() + } + return result, nil +} + // UpgradeBMCVersion initiates a BMC upgrade. func (r *RedfishLocalBMC) UpgradeBMCVersion(ctx context.Context, _ string, params *schemas.UpdateServiceSimpleUpdateParameters) (string, bool, error) { return upgradeVersion(ctx, r.RedfishBaseBMC, params, localBuildBMCRequestBody, localExtractTaskMonitorURI)