BMC interface: add FetchETags and write-only drift detection helpers

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.

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 {

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 <URI>" or "PATCH <URI>" 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...)
+}

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)