From f253c5834531a064d4b94b1ea69757609f57ad29 Mon Sep 17 00:00:00 2001
From: Stefan Hipfel <stefan.hipfel@sap.com>
Date: Tue, 1 Sep 2026 14:12:50 +0200
Subject: [PATCH 1/9] Add optional Redfish session token caching to reduce BMC
 audit log spam

Introduce a process-level SessionCache that reuses Redfish X-Auth-Token
across reconcile loops, capping the effective TTL against the BMC-advertised
SessionTimeout, with automatic invalidation and retry on 401. Enable via
--bmc-auth-mode=session-cache and tune with --bmc-session-cache-ttl.

Signed-off-by: Stefan Hipfel <stefan.hipfel@sap.com>
---
 bmc/redfish.go                             |  54 +++--
 bmc/session_cache.go                       | 174 ++++++++++++++++
 bmc/session_cache_test.go                  | 231 +++++++++++++++++++++
 cmd/main.go                                |  46 +++-
 internal/controller/endpoint_controller.go |  10 +-
 internal/controller/suite_test.go          |   2 -
 pkg/bmcutils/bmcutils.go                   |  34 ++-
 7 files changed, 511 insertions(+), 40 deletions(-)
 create mode 100644 bmc/session_cache.go
 create mode 100644 bmc/session_cache_test.go

diff --git a/bmc/redfish.go b/bmc/redfish.go
index f6b20fc45..c362e4405 100644
--- a/bmc/redfish.go
+++ b/bmc/redfish.go
@@ -47,23 +47,19 @@ type Options struct {
 	Endpoint  string
 	Username  string
 	Password  string
-	BasicAuth bool
 
-	// TLS configuration
-	InsecureTLS bool // Skip TLS certificate verification
+	InsecureTLS bool
 
 	ResourcePollingInterval time.Duration
 	ResourcePollingTimeout  time.Duration
 	PowerPollingInterval    time.Duration
 	PowerPollingTimeout     time.Duration
 
-	// AdditionalVendors maps a manufacturer string (as reported by Redfish)
-	// to a factory that wraps the base Redfish client in a vendor-specific
-	// implementation. Entries are merged on top of DefaultVendors() by
-	// NewRedfishBMCClient, so callers only need to supply the extra OEMs
-	// they want to add. Existing built-in manufacturers can be overridden
-	// by registering the same key.
 	AdditionalVendors map[Manufacturer]VendorFactory
+
+	// SessionCache enables Redfish session token caching when non-nil.
+	// Created by the manager at startup and shared across all controllers.
+	SessionCache *SessionCache
 }
 
 // RedfishBaseBMC is the base implementation of the BMC interface for Redfish.
@@ -84,19 +80,33 @@ func (e *InvalidBIOSSettingsError) Error() string {
 	return fmt.Sprintf("Settings Name: %s\nSettings Value: %v\nError: %s", e.SettingName, e.SettingValue, e.Message)
 }
 
-// newRedfishBaseBMCClient creates a new RedfishBaseBMC with the given connection details (internal use only).
 func newRedfishBaseBMCClient(ctx context.Context, options Options) (*RedfishBaseBMC, error) {
-	clientConfig := gofish.ClientConfig{
-		Endpoint:  options.Endpoint,
-		Username:  options.Username,
-		Password:  options.Password,
-		Insecure:  options.InsecureTLS,
-		BasicAuth: options.BasicAuth,
-	}
-	client, err := gofish.ConnectContext(ctx, clientConfig)
+	var client *gofish.APIClient
+	var err error
+
+	if options.SessionCache != nil {
+		session, sessionErr := options.SessionCache.GetOrCreate(ctx, options)
+		if sessionErr != nil {
+			return nil, sessionErr
+		}
+		client, err = gofish.ConnectContext(ctx, gofish.ClientConfig{
+			Endpoint: options.Endpoint,
+			Session:  session,
+			Insecure: options.InsecureTLS,
+		})
+	} else {
+		client, err = gofish.ConnectContext(ctx, gofish.ClientConfig{
+			Endpoint:  options.Endpoint,
+			Username:  options.Username,
+			Password:  options.Password,
+			Insecure:  options.InsecureTLS,
+			BasicAuth: true,
+		})
+	}
 	if err != nil {
 		return nil, err
 	}
+
 	bmc := &RedfishBaseBMC{client: client}
 	if options.ResourcePollingInterval == 0 {
 		options.ResourcePollingInterval = DefaultResourcePollingInterval
@@ -176,11 +186,13 @@ func (r *RedfishBaseBMC) Manufacturer() Manufacturer {
 	return Manufacturer(r.manufacturer)
 }
 
-// Logout closes the BMC client connection by logging out
+// Logout closes the BMC client connection. When session caching is enabled
+// the session is owned by the cache, so Logout is a no-op.
 func (r *RedfishBaseBMC) Logout() {
-	if r.client != nil {
-		r.client.Logout()
+	if r.client == nil || r.options.SessionCache != nil {
+		return
 	}
+	r.client.Logout()
 }
 
 // PowerOn powers on the system using Redfish.
diff --git a/bmc/session_cache.go b/bmc/session_cache.go
new file mode 100644
index 000000000..994a2853f
--- /dev/null
+++ b/bmc/session_cache.go
@@ -0,0 +1,174 @@
+// SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and IronCore contributors
+// SPDX-License-Identifier: Apache-2.0
+
+package bmc
+
+import (
+	"context"
+	"errors"
+	"maps"
+	"net/http"
+	"sync"
+	"time"
+
+	"github.com/stmcginnis/gofish"
+	"github.com/stmcginnis/gofish/schemas"
+)
+
+// SessionCacheKey identifies a cached Redfish session by endpoint and username.
+type SessionCacheKey struct {
+	Endpoint string
+	Username string
+}
+
+type sessionCacheEntry struct {
+	mu        sync.Mutex
+	session   *gofish.Session
+	expiresAt time.Time
+}
+
+// SessionCache holds live Redfish session tokens keyed by endpoint+username.
+type SessionCache struct {
+	mu      sync.Mutex
+	entries map[SessionCacheKey]*sessionCacheEntry
+	ttl     time.Duration
+}
+
+// NewSessionCache returns a SessionCache with the given idle TTL.
+func NewSessionCache(ttl time.Duration) *SessionCache {
+	return &SessionCache{
+		entries: make(map[SessionCacheKey]*sessionCacheEntry),
+		ttl:     ttl,
+	}
+}
+
+// GetOrCreate returns a valid Redfish session for the given options, reusing a
+// cached token if one exists and has not expired.
+func (c *SessionCache) GetOrCreate(ctx context.Context, opts Options) (*gofish.Session, error) {
+	if c == nil || c.ttl == 0 {
+		session, _, err := c.createSession(ctx, opts)
+		return session, err
+	}
+
+	key := SessionCacheKey{Endpoint: opts.Endpoint, Username: opts.Username}
+
+	c.mu.Lock()
+	entry, ok := c.entries[key]
+	if !ok {
+		entry = &sessionCacheEntry{}
+		c.entries[key] = entry
+	}
+	c.mu.Unlock()
+
+	entry.mu.Lock()
+	defer entry.mu.Unlock()
+
+	if entry.session != nil && time.Now().Before(entry.expiresAt) {
+		return entry.session, nil
+	}
+
+	session, bmcTTL, err := c.createSession(ctx, opts)
+	if err != nil {
+		return nil, err
+	}
+	ttl := c.ttl
+	if bmcTTL > 0 && bmcTTL < ttl {
+		ttl = bmcTTL
+	}
+	entry.session = session
+	entry.expiresAt = time.Now().Add(ttl)
+	return session, nil
+}
+
+// Invalidate evicts the cached session for the given key so the next call to
+// GetOrCreate creates a fresh one.
+func (c *SessionCache) Invalidate(key SessionCacheKey) {
+	if c == nil {
+		return
+	}
+	c.mu.Lock()
+	entry, ok := c.entries[key]
+	c.mu.Unlock()
+	if !ok {
+		return
+	}
+	entry.mu.Lock()
+	entry.session = nil
+	entry.expiresAt = time.Time{}
+	entry.mu.Unlock()
+}
+
+// Close deletes all live server-side Redfish sessions and clears the cache.
+// Should be called from the manager shutdown hook.
+func (c *SessionCache) Close() {
+	if c == nil {
+		return
+	}
+	c.mu.Lock()
+	entries := make(map[SessionCacheKey]*sessionCacheEntry, len(c.entries))
+	maps.Copy(entries, c.entries)
+	c.entries = make(map[SessionCacheKey]*sessionCacheEntry)
+	c.mu.Unlock()
+
+	for key, entry := range entries {
+		entry.mu.Lock()
+		sess := entry.session
+		entry.session = nil
+		entry.mu.Unlock()
+
+		if sess == nil || sess.ID == "" {
+			continue
+		}
+		httpClient := &http.Client{}
+		req, err := http.NewRequest(http.MethodDelete, key.Endpoint+sess.ID, nil)
+		if err != nil {
+			continue
+		}
+		req.Header.Set("X-Auth-Token", sess.Token)
+		resp, err := httpClient.Do(req)
+		if err == nil {
+			_ = resp.Body.Close()
+		}
+	}
+}
+
+// createSession establishes a new Redfish session, queries the BMC's advertised
+// SessionTimeout to cap the cache TTL, then drops the transient client without
+// calling Logout (which would immediately delete the session we just created).
+func (c *SessionCache) createSession(ctx context.Context, opts Options) (*gofish.Session, time.Duration, error) {
+	client, err := gofish.ConnectContext(ctx, gofish.ClientConfig{
+		Endpoint: opts.Endpoint,
+		Username: opts.Username,
+		Password: opts.Password,
+		Insecure: opts.InsecureTLS,
+	})
+	if err != nil {
+		return nil, 0, err
+	}
+	session, err := client.GetSession()
+	if err != nil {
+		client.Logout()
+		return nil, 0, err
+	}
+
+	var bmcTTL time.Duration
+	if ss, err := client.Service.SessionService(); err == nil && ss.SessionTimeout > 0 {
+		bmcTTL = time.Duration(ss.SessionTimeout) * time.Second
+	}
+
+	client.HTTPClient.CloseIdleConnections()
+	return session, bmcTTL, nil
+}
+
+// IsSessionExpiredError reports whether err is an HTTP 401 from the BMC,
+// indicating the cached session token was invalidated server-side.
+func IsSessionExpiredError(err error) bool {
+	if err == nil {
+		return false
+	}
+	var redfishErr *schemas.Error
+	if !errors.As(err, &redfishErr) {
+		return false
+	}
+	return redfishErr.HTTPReturnedStatusCode == http.StatusUnauthorized
+}
diff --git a/bmc/session_cache_test.go b/bmc/session_cache_test.go
new file mode 100644
index 000000000..2e683789a
--- /dev/null
+++ b/bmc/session_cache_test.go
@@ -0,0 +1,231 @@
+// SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and IronCore contributors
+// SPDX-License-Identifier: Apache-2.0
+
+package bmc
+
+import (
+	"net/http"
+	"sync"
+	"time"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	"github.com/stmcginnis/gofish"
+)
+
+// seedSession is a sentinel gofish.Session used in tests that need a non-nil cached session.
+var seedSession = &gofish.Session{ID: "/redfish/v1/SessionService/Sessions/abc", Token: "test-token-123"}
+
+var _ = Describe("SessionCache", func() {
+	Describe("NewSessionCache", func() {
+		It("returns a non-nil cache with the given TTL", func() {
+			cache := NewSessionCache(10 * time.Minute)
+			Expect(cache).NotTo(BeNil())
+			Expect(cache.ttl).To(Equal(10 * time.Minute))
+		})
+	})
+
+	Describe("Invalidate", func() {
+		It("is a no-op on a nil cache", func() {
+			var cache *SessionCache
+			Expect(func() { cache.Invalidate(SessionCacheKey{}) }).NotTo(Panic())
+		})
+
+		It("clears a cached entry", func() {
+			cache := NewSessionCache(10 * time.Minute)
+			key := SessionCacheKey{Endpoint: "https://bmc.test", Username: "admin"}
+
+			cache.mu.Lock()
+			entry := &sessionCacheEntry{
+				session:   seedSession,
+				expiresAt: time.Now().Add(10 * time.Minute),
+			}
+			cache.entries[key] = entry
+			cache.mu.Unlock()
+
+			cache.Invalidate(key)
+
+			entry.mu.Lock()
+			defer entry.mu.Unlock()
+			Expect(entry.session).To(BeNil())
+			Expect(entry.expiresAt.IsZero()).To(BeTrue())
+		})
+
+		It("is a no-op for unknown keys", func() {
+			cache := NewSessionCache(10 * time.Minute)
+			Expect(func() {
+				cache.Invalidate(SessionCacheKey{Endpoint: "https://unknown", Username: "x"})
+			}).NotTo(Panic())
+		})
+	})
+
+	Describe("Close", func() {
+		It("is a no-op on a nil cache", func() {
+			var cache *SessionCache
+			Expect(func() { cache.Close() }).NotTo(Panic())
+		})
+
+		It("empties the entries map", func() {
+			cache := NewSessionCache(10 * time.Minute)
+			key := SessionCacheKey{Endpoint: "https://bmc.test", Username: "admin"}
+
+			cache.mu.Lock()
+			cache.entries[key] = &sessionCacheEntry{session: seedSession, expiresAt: time.Now().Add(time.Minute)}
+			cache.mu.Unlock()
+
+			// Close attempts a DELETE to clean up the session but there is no live server;
+			// it should not panic. The entries map must be cleared regardless.
+			cache.Close()
+
+			cache.mu.Lock()
+			defer cache.mu.Unlock()
+			Expect(cache.entries).To(BeEmpty())
+		})
+	})
+
+	Describe("cache-hit logic (internal state)", func() {
+		It("a seeded entry within TTL is treated as a cache hit", func() {
+			cache := NewSessionCache(10 * time.Minute)
+			key := SessionCacheKey{Endpoint: "https://bmc.test", Username: "admin"}
+
+			cache.mu.Lock()
+			entry := &sessionCacheEntry{
+				session:   seedSession,
+				expiresAt: time.Now().Add(10 * time.Minute),
+			}
+			cache.entries[key] = entry
+			cache.mu.Unlock()
+
+			entry.mu.Lock()
+			hit := entry.session != nil && time.Now().Before(entry.expiresAt)
+			sess := entry.session
+			entry.mu.Unlock()
+
+			Expect(hit).To(BeTrue())
+			Expect(sess).To(Equal(seedSession))
+		})
+
+		It("an expired entry is treated as a cache miss", func() {
+			cache := NewSessionCache(10 * time.Minute)
+			key := SessionCacheKey{Endpoint: "https://bmc.test", Username: "admin"}
+
+			cache.mu.Lock()
+			entry := &sessionCacheEntry{
+				session:   seedSession,
+				expiresAt: time.Now().Add(-1 * time.Second), // already expired
+			}
+			cache.entries[key] = entry
+			cache.mu.Unlock()
+
+			entry.mu.Lock()
+			miss := entry.session == nil || !time.Now().Before(entry.expiresAt)
+			entry.mu.Unlock()
+
+			Expect(miss).To(BeTrue(), "expired entry should be a cache miss")
+		})
+	})
+
+	Describe("BMC TTL capping", func() {
+		It("uses the configured TTL when it is shorter than the BMC timeout", func() {
+			configured := 10 * time.Minute
+			bmcTTL := 30 * time.Minute
+			ttl := configured
+			if bmcTTL > 0 && bmcTTL < ttl {
+				ttl = bmcTTL
+			}
+			Expect(ttl).To(Equal(configured))
+		})
+
+		It("caps to the BMC timeout when it is shorter than the configured TTL", func() {
+			configured := 30 * time.Minute
+			bmcTTL := 10 * time.Minute
+			ttl := configured
+			if bmcTTL > 0 && bmcTTL < ttl {
+				ttl = bmcTTL
+			}
+			Expect(ttl).To(Equal(bmcTTL))
+		})
+
+		It("ignores a zero BMC timeout (not advertised)", func() {
+			configured := 10 * time.Minute
+			var bmcTTL time.Duration // zero: BMC did not advertise timeout
+			ttl := configured
+			if bmcTTL > 0 && bmcTTL < ttl {
+				ttl = bmcTTL
+			}
+			Expect(ttl).To(Equal(configured))
+		})
+	})
+
+	Describe("concurrent access", func() {
+		It("serialises concurrent reads for the same key without data races", func() {
+			cache := NewSessionCache(10 * time.Minute)
+			key := SessionCacheKey{Endpoint: "https://bmc.test", Username: "admin"}
+
+			cache.mu.Lock()
+			cache.entries[key] = &sessionCacheEntry{
+				session:   seedSession,
+				expiresAt: time.Now().Add(10 * time.Minute),
+			}
+			cache.mu.Unlock()
+
+			const goroutines = 20
+			var wg sync.WaitGroup
+			wg.Add(goroutines)
+			for range goroutines {
+				go func() {
+					defer wg.Done()
+					cache.mu.Lock()
+					entry := cache.entries[key]
+					cache.mu.Unlock()
+					entry.mu.Lock()
+					_ = entry.session
+					entry.mu.Unlock()
+				}()
+			}
+			wg.Wait()
+			// No race detector violation → correct locking.
+		})
+
+		It("concurrent Invalidate and read do not race", func() {
+			cache := NewSessionCache(10 * time.Minute)
+			key := SessionCacheKey{Endpoint: "https://bmc.test", Username: "admin"}
+
+			cache.mu.Lock()
+			cache.entries[key] = &sessionCacheEntry{
+				session:   seedSession,
+				expiresAt: time.Now().Add(10 * time.Minute),
+			}
+			cache.mu.Unlock()
+
+			var wg sync.WaitGroup
+			wg.Add(2)
+			go func() {
+				defer wg.Done()
+				cache.Invalidate(key)
+			}()
+			go func() {
+				defer wg.Done()
+				cache.mu.Lock()
+				entry, ok := cache.entries[key]
+				cache.mu.Unlock()
+				if ok {
+					entry.mu.Lock()
+					_ = entry.session
+					entry.mu.Unlock()
+				}
+			}()
+			wg.Wait()
+		})
+	})
+
+	Describe("IsSessionExpiredError", func() {
+		It("returns false for nil", func() {
+			Expect(IsSessionExpiredError(nil)).To(BeFalse())
+		})
+
+		It("returns false for a non-Redfish error", func() {
+			Expect(IsSessionExpiredError(http.ErrNoCookie)).To(BeFalse())
+		})
+	})
+})
diff --git a/cmd/main.go b/cmd/main.go
index e8197f056..bb8ae6261 100644
--- a/cmd/main.go
+++ b/cmd/main.go
@@ -107,6 +107,8 @@ func main() { // nolint: gocyclo
 		dnsRecordTemplatePath              string
 		defaultFailedAutoRetryCount        int
 		skipMigrations                     bool
+		bmcAuthMode                        string
+		bmcSessionCacheTTL                 time.Duration
 	)
 
 	flag.IntVar(&serverMaxConcurrentReconciles, "server-max-concurrent-reconciles", 5,
@@ -182,6 +184,15 @@ func main() { // nolint: gocyclo
 	flag.IntVar(&defaultFailedAutoRetryCount, "default-failed-auto-retry-count", 0,
 		"The default number of auto retries for a CRD when it fails. 0 for no retries.")
 	flag.BoolVar(&skipMigrations, "skip-migrations", false, "Whether to skip any migration before start or not.")
+	flag.StringVar(&bmcAuthMode, "bmc-auth-mode", "basic",
+		"Authentication mode for Redfish BMC connections. "+
+			"'basic': HTTP Basic Auth on every request (default). "+
+			"'session-cache': reuse Redfish session tokens across reconciles (requires --bmc-session-cache-ttl).")
+	flag.DurationVar(&bmcSessionCacheTTL, "bmc-session-cache-ttl", 25*time.Minute,
+		"Maximum idle TTL for cached Redfish session tokens (used with --bmc-auth-mode=session-cache). "+
+			"The effective TTL is min(this value, BMC-advertised SessionTimeout) — the BMC is queried "+
+			"on each cache miss and its SessionTimeout caps the value automatically. "+
+			"0 disables the cache.")
 
 	opts := zap.Options{
 		Development: true,
@@ -429,12 +440,41 @@ func main() { // nolint: gocyclo
 		mgr = migrationutils.WrapManager(migrator, mgr)
 	}
 
+	var sessionCache *bmc.SessionCache
+	switch bmcAuthMode {
+	case "session-cache":
+		if bmcSessionCacheTTL == 0 {
+			setupLog.Error(nil, "--bmc-session-cache-ttl must be > 0 when --bmc-auth-mode=session-cache")
+			os.Exit(1)
+		}
+		sessionCache = bmc.NewSessionCache(bmcSessionCacheTTL)
+		if err := mgr.Add(manager.RunnableFunc(func(ctx context.Context) error {
+			<-ctx.Done()
+			sessionCache.Close()
+			return nil
+		})); err != nil {
+			setupLog.Error(err, "Failed to register session cache shutdown")
+			os.Exit(1)
+		}
+		setupLog.Info("Redfish session cache enabled", "ttl", bmcSessionCacheTTL)
+	case "basic", "":
+		// default: basic auth, no session cache
+	default:
+		setupLog.Error(nil, "Invalid --bmc-auth-mode value. Must be 'basic' or 'session-cache'", "value", bmcAuthMode)
+		os.Exit(1)
+	}
+
+	bmcBaseOptions := bmc.Options{
+		SessionCache: sessionCache,
+	}
+
 	if err = (&controller.EndpointReconciler{
 		Client:             mgr.GetClient(),
 		Scheme:             mgr.GetScheme(),
 		MACPrefixes:        macPRefixes,
 		DefaultProtocol:    effectiveProtocol,
 		SkipCertValidation: effectiveSkipCert,
+		BMCOptions:         bmcBaseOptions,
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "Failed to create controller", "controller", "endpoint")
 		os.Exit(1)
@@ -458,9 +498,7 @@ func main() { // nolint: gocyclo
 		DNSRecordTemplate:      dnsRecordTemplate,
 		Conditions:             conditionutils.NewAccessor(conditionutils.AccessorOptions{}),
 		SSHResetTimeout:        sshResetTimeout,
-		BMCOptions: bmc.Options{
-			BasicAuth: true,
-		},
+		BMCOptions: bmcBaseOptions,
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "Failed to create controller", "controller", "bmc")
 		os.Exit(1)
@@ -486,7 +524,7 @@ func main() { // nolint: gocyclo
 		Conditions:              conditionutils.NewAccessor(conditionutils.AccessorOptions{}),
 		DiscoveryIgnitionPath:   discoveryIgnitionPath,
 		BMCOptions: bmc.Options{
-			BasicAuth:               true,
+			SessionCache:            sessionCache,
 			PowerPollingInterval:    powerPollingInterval,
 			PowerPollingTimeout:     powerPollingTimeout,
 			ResourcePollingInterval: resourcePollingInterval,
diff --git a/internal/controller/endpoint_controller.go b/internal/controller/endpoint_controller.go
index cb436bbff..f3a581c73 100644
--- a/internal/controller/endpoint_controller.go
+++ b/internal/controller/endpoint_controller.go
@@ -95,12 +95,10 @@ func (r *EndpointReconciler) reconcile(ctx context.Context, endpoint *metalv1alp
 				return ctrl.Result{}, fmt.Errorf("no default credentials present for BMC %s", endpoint.Spec.MACAddress)
 			}
 
-			bmcOptions := bmc.Options{
-				BasicAuth:   true,
-				Username:    m.DefaultCredentials[0].Username,
-				Password:    m.DefaultCredentials[0].Password,
-				InsecureTLS: r.SkipCertValidation,
-			}
+			bmcOptions := r.BMCOptions
+			bmcOptions.Username = m.DefaultCredentials[0].Username
+			bmcOptions.Password = m.DefaultCredentials[0].Password
+			bmcOptions.InsecureTLS = r.SkipCertValidation
 
 			protocolScheme := bmcutils.GetProtocolScheme(m.ProtocolScheme, r.DefaultProtocol)
 			bmcOptions.Endpoint = fmt.Sprintf("%s://%s", protocolScheme, net.JoinHostPort(endpoint.Spec.IP.String(), fmt.Sprintf("%d", m.Port)))
diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go
index e83a1fc50..8405e9e72 100644
--- a/internal/controller/suite_test.go
+++ b/internal/controller/suite_test.go
@@ -200,7 +200,6 @@ func SetupTest(redfishMockServers []netip.AddrPort) *corev1.Namespace {
 				ResourcePollingTimeout:  200 * time.Millisecond,
 				PowerPollingInterval:    50 * time.Millisecond,
 				PowerPollingTimeout:     200 * time.Millisecond,
-				BasicAuth:               true,
 			},
 		}).SetupWithManager(k8sManager)).To(Succeed())
 
@@ -227,7 +226,6 @@ func SetupTest(redfishMockServers []netip.AddrPort) *corev1.Namespace {
 				ResourcePollingTimeout:  200 * time.Millisecond,
 				PowerPollingInterval:    50 * time.Millisecond,
 				PowerPollingTimeout:     200 * time.Millisecond,
-				BasicAuth:               true,
 			},
 			DiscoveryTimeout:      30 * time.Second, // Set a short discovery timeout for testing
 			DiscoveryIgnitionPath: filepath.Join("..", "..", "config", "manager", "ignition-template.yaml"),
diff --git a/pkg/bmcutils/bmcutils.go b/pkg/bmcutils/bmcutils.go
index b84a30fd6..637b22636 100644
--- a/pkg/bmcutils/bmcutils.go
+++ b/pkg/bmcutils/bmcutils.go
@@ -216,6 +216,33 @@ func CreateBMCClient(
 	bmcOptions bmc.Options,
 	skipCertValidation bool,
 	opts ...CreateBMCClientOption,
+) (bmc.BMC, error) {
+	// Resolve the endpoint and credentials up-front so the cache key is
+	// available before and after the first attempt.
+	bmcOptions.Endpoint = fmt.Sprintf("%s://%s", protocolScheme, net.JoinHostPort(address, fmt.Sprintf("%d", port)))
+	var err error
+	bmcOptions.Username, bmcOptions.Password, err = GetBMCCredentialsFromSecret(bmcSecret)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get credentials from BMC secret: %w", err)
+	}
+	bmcOptions.InsecureTLS = skipCertValidation
+
+	bmcClient, err := doCreateBMCClient(ctx, bmcProtocol, bmcOptions, opts...)
+	if err != nil && bmc.IsSessionExpiredError(err) && bmcOptions.SessionCache != nil {
+		// Cached session was rejected by the BMC (e.g. server-side expiry). Invalidate
+		// and retry once with a fresh session.
+		key := bmc.SessionCacheKey{Endpoint: bmcOptions.Endpoint, Username: bmcOptions.Username}
+		bmcOptions.SessionCache.Invalidate(key)
+		bmcClient, err = doCreateBMCClient(ctx, bmcProtocol, bmcOptions, opts...)
+	}
+	return bmcClient, err
+}
+
+func doCreateBMCClient(
+	ctx context.Context,
+	bmcProtocol metalv1alpha1.ProtocolName,
+	bmcOptions bmc.Options,
+	opts ...CreateBMCClientOption,
 ) (bmc.BMC, error) {
 	var bmcClient bmc.BMC
 	var err error
@@ -225,13 +252,6 @@ func CreateBMCClient(
 		o(cfg)
 	}
 
-	bmcOptions.Endpoint = fmt.Sprintf("%s://%s", protocolScheme, net.JoinHostPort(address, fmt.Sprintf("%d", port)))
-	bmcOptions.Username, bmcOptions.Password, err = GetBMCCredentialsFromSecret(bmcSecret)
-	if err != nil {
-		return nil, fmt.Errorf("failed to get credentials from BMC secret: %w", err)
-	}
-	bmcOptions.InsecureTLS = skipCertValidation
-
 	log := ctrl.LoggerFrom(ctx)
 	log.V(1).Info("Creating BMC client", "Protocol", bmcProtocol, "Address", bmcOptions.Endpoint, "Username", bmcOptions.Username, "cfg", cfg)
 

From a1a370a2a2f7478e72fa719c309c51a593582090 Mon Sep 17 00:00:00 2001
From: Stefan Hipfel <stefan.hipfel@sap.com>
Date: Tue, 1 Sep 2026 14:18:46 +0200
Subject: [PATCH 2/9] Fix gofmt formatting

Signed-off-by: Stefan Hipfel <stefan.hipfel@sap.com>
---
 bmc/redfish.go | 6 +++---
 cmd/main.go    | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/bmc/redfish.go b/bmc/redfish.go
index c362e4405..9212654ea 100644
--- a/bmc/redfish.go
+++ b/bmc/redfish.go
@@ -44,9 +44,9 @@ const (
 
 // Options contain the options for the BMC redfish client.
 type Options struct {
-	Endpoint  string
-	Username  string
-	Password  string
+	Endpoint string
+	Username string
+	Password string
 
 	InsecureTLS bool
 
diff --git a/cmd/main.go b/cmd/main.go
index bb8ae6261..02f607b7b 100644
--- a/cmd/main.go
+++ b/cmd/main.go
@@ -498,7 +498,7 @@ func main() { // nolint: gocyclo
 		DNSRecordTemplate:      dnsRecordTemplate,
 		Conditions:             conditionutils.NewAccessor(conditionutils.AccessorOptions{}),
 		SSHResetTimeout:        sshResetTimeout,
-		BMCOptions: bmcBaseOptions,
+		BMCOptions:             bmcBaseOptions,
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "Failed to create controller", "controller", "bmc")
 		os.Exit(1)

From 20d5c1f6e4c8dcf41d19382214334216ff3b0231 Mon Sep 17 00:00:00 2001
From: Stefan Hipfel <stefan.hipfel@sap.com>
Date: Tue, 1 Sep 2026 14:42:21 +0200
Subject: [PATCH 3/9] Fix session cache: reject non-positive TTL and add
 shutdown DELETE timeout and TLS config

- NewSessionCache panics on non-positive TTL; cmd/main.go validates with <= 0
- sessionCacheEntry stores insecureTLS so Close() can build a matching TLS config
- Close() uses a 10s per-request timeout to avoid blocking manager shutdown

Signed-off-by: Stefan Hipfel <stefan.hipfel@sap.com>
---
 bmc/session_cache.go      | 32 ++++++++++++++++++++++----------
 bmc/session_cache_test.go |  8 ++++++++
 cmd/main.go               |  6 +++---
 3 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/bmc/session_cache.go b/bmc/session_cache.go
index 994a2853f..87ac4cd79 100644
--- a/bmc/session_cache.go
+++ b/bmc/session_cache.go
@@ -5,6 +5,7 @@ package bmc
 
 import (
 	"context"
+	"crypto/tls"
 	"errors"
 	"maps"
 	"net/http"
@@ -22,9 +23,10 @@ type SessionCacheKey struct {
 }
 
 type sessionCacheEntry struct {
-	mu        sync.Mutex
-	session   *gofish.Session
-	expiresAt time.Time
+	mu          sync.Mutex
+	session     *gofish.Session
+	expiresAt   time.Time
+	insecureTLS bool
 }
 
 // SessionCache holds live Redfish session tokens keyed by endpoint+username.
@@ -35,7 +37,11 @@ type SessionCache struct {
 }
 
 // NewSessionCache returns a SessionCache with the given idle TTL.
+// ttl must be positive.
 func NewSessionCache(ttl time.Duration) *SessionCache {
+	if ttl <= 0 {
+		panic("bmc: NewSessionCache requires a positive TTL")
+	}
 	return &SessionCache{
 		entries: make(map[SessionCacheKey]*sessionCacheEntry),
 		ttl:     ttl,
@@ -45,11 +51,6 @@ func NewSessionCache(ttl time.Duration) *SessionCache {
 // GetOrCreate returns a valid Redfish session for the given options, reusing a
 // cached token if one exists and has not expired.
 func (c *SessionCache) GetOrCreate(ctx context.Context, opts Options) (*gofish.Session, error) {
-	if c == nil || c.ttl == 0 {
-		session, _, err := c.createSession(ctx, opts)
-		return session, err
-	}
-
 	key := SessionCacheKey{Endpoint: opts.Endpoint, Username: opts.Username}
 
 	c.mu.Lock()
@@ -77,6 +78,7 @@ func (c *SessionCache) GetOrCreate(ctx context.Context, opts Options) (*gofish.S
 	}
 	entry.session = session
 	entry.expiresAt = time.Now().Add(ttl)
+	entry.insecureTLS = opts.InsecureTLS
 	return session, nil
 }
 
@@ -113,15 +115,24 @@ func (c *SessionCache) Close() {
 	for key, entry := range entries {
 		entry.mu.Lock()
 		sess := entry.session
+		insecureTLS := entry.insecureTLS
 		entry.session = nil
 		entry.mu.Unlock()
 
 		if sess == nil || sess.ID == "" {
 			continue
 		}
-		httpClient := &http.Client{}
-		req, err := http.NewRequest(http.MethodDelete, key.Endpoint+sess.ID, nil)
+		//nolint:gosec
+		httpClient := &http.Client{
+			Timeout: 10 * time.Second,
+			Transport: &http.Transport{
+				TLSClientConfig: &tls.Config{InsecureSkipVerify: insecureTLS},
+			},
+		}
+		ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+		req, err := http.NewRequestWithContext(ctx, http.MethodDelete, key.Endpoint+sess.ID, nil)
 		if err != nil {
+			cancel()
 			continue
 		}
 		req.Header.Set("X-Auth-Token", sess.Token)
@@ -129,6 +140,7 @@ func (c *SessionCache) Close() {
 		if err == nil {
 			_ = resp.Body.Close()
 		}
+		cancel()
 	}
 }
 
diff --git a/bmc/session_cache_test.go b/bmc/session_cache_test.go
index 2e683789a..1e280b72a 100644
--- a/bmc/session_cache_test.go
+++ b/bmc/session_cache_test.go
@@ -23,6 +23,14 @@ var _ = Describe("SessionCache", func() {
 			Expect(cache).NotTo(BeNil())
 			Expect(cache.ttl).To(Equal(10 * time.Minute))
 		})
+
+		It("panics for a zero TTL", func() {
+			Expect(func() { NewSessionCache(0) }).To(Panic())
+		})
+
+		It("panics for a negative TTL", func() {
+			Expect(func() { NewSessionCache(-1 * time.Second) }).To(Panic())
+		})
 	})
 
 	Describe("Invalidate", func() {
diff --git a/cmd/main.go b/cmd/main.go
index 02f607b7b..14fcd7a72 100644
--- a/cmd/main.go
+++ b/cmd/main.go
@@ -192,7 +192,7 @@ func main() { // nolint: gocyclo
 		"Maximum idle TTL for cached Redfish session tokens (used with --bmc-auth-mode=session-cache). "+
 			"The effective TTL is min(this value, BMC-advertised SessionTimeout) — the BMC is queried "+
 			"on each cache miss and its SessionTimeout caps the value automatically. "+
-			"0 disables the cache.")
+			"Must be positive.")
 
 	opts := zap.Options{
 		Development: true,
@@ -443,8 +443,8 @@ func main() { // nolint: gocyclo
 	var sessionCache *bmc.SessionCache
 	switch bmcAuthMode {
 	case "session-cache":
-		if bmcSessionCacheTTL == 0 {
-			setupLog.Error(nil, "--bmc-session-cache-ttl must be > 0 when --bmc-auth-mode=session-cache")
+		if bmcSessionCacheTTL <= 0 {
+			setupLog.Error(nil, "--bmc-session-cache-ttl must be positive when --bmc-auth-mode=session-cache")
 			os.Exit(1)
 		}
 		sessionCache = bmc.NewSessionCache(bmcSessionCacheTTL)

From ae6f51ec9c785a4065e4ad1373ccd265a3be92f8 Mon Sep 17 00:00:00 2001
From: Stefan Hipfel <stefan.hipfel@sap.com>
Date: Thu, 3 Sep 2026 09:47:36 +0200
Subject: [PATCH 4/9] Address review feedback: error return, 403 retry, options
 asymmetry, orphan note

- NewSessionCache returns (cache, error) instead of panicking
- IsSessionExpiredError now also matches HTTP 403 (Forbidden)
- bmcBaseOptions includes polling fields so all three reconcilers share one source
- --bmc-session-cache-ttl help text notes orphaned sessions on unclean exit

Signed-off-by: Stefan Hipfel <stefan.hipfel@sap.com>
---
 bmc/session_cache.go      | 17 ++++++++++-------
 bmc/session_cache_test.go | 36 ++++++++++++++++++++++++------------
 cmd/main.go               | 24 ++++++++++++++----------
 3 files changed, 48 insertions(+), 29 deletions(-)

diff --git a/bmc/session_cache.go b/bmc/session_cache.go
index 87ac4cd79..d9d50a685 100644
--- a/bmc/session_cache.go
+++ b/bmc/session_cache.go
@@ -7,6 +7,7 @@ import (
 	"context"
 	"crypto/tls"
 	"errors"
+	"fmt"
 	"maps"
 	"net/http"
 	"sync"
@@ -37,15 +38,15 @@ type SessionCache struct {
 }
 
 // NewSessionCache returns a SessionCache with the given idle TTL.
-// ttl must be positive.
-func NewSessionCache(ttl time.Duration) *SessionCache {
+// Returns an error if ttl is not positive.
+func NewSessionCache(ttl time.Duration) (*SessionCache, error) {
 	if ttl <= 0 {
-		panic("bmc: NewSessionCache requires a positive TTL")
+		return nil, fmt.Errorf("bmc: session cache TTL must be positive, got %v", ttl)
 	}
 	return &SessionCache{
 		entries: make(map[SessionCacheKey]*sessionCacheEntry),
 		ttl:     ttl,
-	}
+	}, nil
 }
 
 // GetOrCreate returns a valid Redfish session for the given options, reusing a
@@ -172,8 +173,9 @@ func (c *SessionCache) createSession(ctx context.Context, opts Options) (*gofish
 	return session, bmcTTL, nil
 }
 
-// IsSessionExpiredError reports whether err is an HTTP 401 from the BMC,
-// indicating the cached session token was invalidated server-side.
+// IsSessionExpiredError reports whether err indicates the BMC rejected the
+// cached session token. Matches HTTP 401 (Unauthorized) and 403 (Forbidden),
+// which are the most common responses for an expired or revoked token.
 func IsSessionExpiredError(err error) bool {
 	if err == nil {
 		return false
@@ -182,5 +184,6 @@ func IsSessionExpiredError(err error) bool {
 	if !errors.As(err, &redfishErr) {
 		return false
 	}
-	return redfishErr.HTTPReturnedStatusCode == http.StatusUnauthorized
+	return redfishErr.HTTPReturnedStatusCode == http.StatusUnauthorized ||
+		redfishErr.HTTPReturnedStatusCode == http.StatusForbidden
 }
diff --git a/bmc/session_cache_test.go b/bmc/session_cache_test.go
index 1e280b72a..9f5ea1c19 100644
--- a/bmc/session_cache_test.go
+++ b/bmc/session_cache_test.go
@@ -16,20 +16,32 @@ import (
 // seedSession is a sentinel gofish.Session used in tests that need a non-nil cached session.
 var seedSession = &gofish.Session{ID: "/redfish/v1/SessionService/Sessions/abc", Token: "test-token-123"}
 
+// mustNewSessionCache wraps NewSessionCache for tests where a valid TTL is always passed.
+func mustNewSessionCache(ttl time.Duration) *SessionCache {
+	c, err := NewSessionCache(ttl)
+	if err != nil {
+		panic(err)
+	}
+	return c
+}
+
 var _ = Describe("SessionCache", func() {
 	Describe("NewSessionCache", func() {
 		It("returns a non-nil cache with the given TTL", func() {
-			cache := NewSessionCache(10 * time.Minute)
+			cache, err := NewSessionCache(10 * time.Minute)
+			Expect(err).NotTo(HaveOccurred())
 			Expect(cache).NotTo(BeNil())
 			Expect(cache.ttl).To(Equal(10 * time.Minute))
 		})
 
-		It("panics for a zero TTL", func() {
-			Expect(func() { NewSessionCache(0) }).To(Panic())
+		It("returns an error for a zero TTL", func() {
+			_, err := NewSessionCache(0)
+			Expect(err).To(HaveOccurred())
 		})
 
-		It("panics for a negative TTL", func() {
-			Expect(func() { NewSessionCache(-1 * time.Second) }).To(Panic())
+		It("returns an error for a negative TTL", func() {
+			_, err := NewSessionCache(-1 * time.Second)
+			Expect(err).To(HaveOccurred())
 		})
 	})
 
@@ -40,7 +52,7 @@ var _ = Describe("SessionCache", func() {
 		})
 
 		It("clears a cached entry", func() {
-			cache := NewSessionCache(10 * time.Minute)
+			cache := mustNewSessionCache(10 * time.Minute)
 			key := SessionCacheKey{Endpoint: "https://bmc.test", Username: "admin"}
 
 			cache.mu.Lock()
@@ -60,7 +72,7 @@ var _ = Describe("SessionCache", func() {
 		})
 
 		It("is a no-op for unknown keys", func() {
-			cache := NewSessionCache(10 * time.Minute)
+			cache := mustNewSessionCache(10 * time.Minute)
 			Expect(func() {
 				cache.Invalidate(SessionCacheKey{Endpoint: "https://unknown", Username: "x"})
 			}).NotTo(Panic())
@@ -74,7 +86,7 @@ var _ = Describe("SessionCache", func() {
 		})
 
 		It("empties the entries map", func() {
-			cache := NewSessionCache(10 * time.Minute)
+			cache := mustNewSessionCache(10 * time.Minute)
 			key := SessionCacheKey{Endpoint: "https://bmc.test", Username: "admin"}
 
 			cache.mu.Lock()
@@ -93,7 +105,7 @@ var _ = Describe("SessionCache", func() {
 
 	Describe("cache-hit logic (internal state)", func() {
 		It("a seeded entry within TTL is treated as a cache hit", func() {
-			cache := NewSessionCache(10 * time.Minute)
+			cache := mustNewSessionCache(10 * time.Minute)
 			key := SessionCacheKey{Endpoint: "https://bmc.test", Username: "admin"}
 
 			cache.mu.Lock()
@@ -114,7 +126,7 @@ var _ = Describe("SessionCache", func() {
 		})
 
 		It("an expired entry is treated as a cache miss", func() {
-			cache := NewSessionCache(10 * time.Minute)
+			cache := mustNewSessionCache(10 * time.Minute)
 			key := SessionCacheKey{Endpoint: "https://bmc.test", Username: "admin"}
 
 			cache.mu.Lock()
@@ -167,7 +179,7 @@ var _ = Describe("SessionCache", func() {
 
 	Describe("concurrent access", func() {
 		It("serialises concurrent reads for the same key without data races", func() {
-			cache := NewSessionCache(10 * time.Minute)
+			cache := mustNewSessionCache(10 * time.Minute)
 			key := SessionCacheKey{Endpoint: "https://bmc.test", Username: "admin"}
 
 			cache.mu.Lock()
@@ -196,7 +208,7 @@ var _ = Describe("SessionCache", func() {
 		})
 
 		It("concurrent Invalidate and read do not race", func() {
-			cache := NewSessionCache(10 * time.Minute)
+			cache := mustNewSessionCache(10 * time.Minute)
 			key := SessionCacheKey{Endpoint: "https://bmc.test", Username: "admin"}
 
 			cache.mu.Lock()
diff --git a/cmd/main.go b/cmd/main.go
index 14fcd7a72..68eda371f 100644
--- a/cmd/main.go
+++ b/cmd/main.go
@@ -192,6 +192,8 @@ func main() { // nolint: gocyclo
 		"Maximum idle TTL for cached Redfish session tokens (used with --bmc-auth-mode=session-cache). "+
 			"The effective TTL is min(this value, BMC-advertised SessionTimeout) — the BMC is queried "+
 			"on each cache miss and its SessionTimeout caps the value automatically. "+
+			"Sessions are deleted on clean shutdown; an unclean exit (OOM kill, eviction) may leave "+
+			"orphaned sessions on the BMC until the BMC-side timeout expires. "+
 			"Must be positive.")
 
 	opts := zap.Options{
@@ -447,7 +449,11 @@ func main() { // nolint: gocyclo
 			setupLog.Error(nil, "--bmc-session-cache-ttl must be positive when --bmc-auth-mode=session-cache")
 			os.Exit(1)
 		}
-		sessionCache = bmc.NewSessionCache(bmcSessionCacheTTL)
+		sessionCache, err = bmc.NewSessionCache(bmcSessionCacheTTL)
+		if err != nil {
+			setupLog.Error(err, "Failed to create session cache")
+			os.Exit(1)
+		}
 		if err := mgr.Add(manager.RunnableFunc(func(ctx context.Context) error {
 			<-ctx.Done()
 			sessionCache.Close()
@@ -465,7 +471,11 @@ func main() { // nolint: gocyclo
 	}
 
 	bmcBaseOptions := bmc.Options{
-		SessionCache: sessionCache,
+		SessionCache:            sessionCache,
+		PowerPollingInterval:    powerPollingInterval,
+		PowerPollingTimeout:     powerPollingTimeout,
+		ResourcePollingInterval: resourcePollingInterval,
+		ResourcePollingTimeout:  resourcePollingTimeout,
 	}
 
 	if err = (&controller.EndpointReconciler{
@@ -523,14 +533,8 @@ func main() { // nolint: gocyclo
 		MaxConcurrentReconciles: serverMaxConcurrentReconciles,
 		Conditions:              conditionutils.NewAccessor(conditionutils.AccessorOptions{}),
 		DiscoveryIgnitionPath:   discoveryIgnitionPath,
-		BMCOptions: bmc.Options{
-			SessionCache:            sessionCache,
-			PowerPollingInterval:    powerPollingInterval,
-			PowerPollingTimeout:     powerPollingTimeout,
-			ResourcePollingInterval: resourcePollingInterval,
-			ResourcePollingTimeout:  resourcePollingTimeout,
-		},
-		DiscoveryTimeout: discoveryTimeout,
+		BMCOptions:              bmcBaseOptions,
+		DiscoveryTimeout:        discoveryTimeout,
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "Failed to create controller", "controller", "server")
 		os.Exit(1)

From 146908803117490c07be3f889a91fc7e3cd7a483 Mon Sep 17 00:00:00 2001
From: Stefan Hipfel <stefan.hipfel@sap.com>
Date: Wed, 9 Sep 2026 15:05:50 +0200
Subject: [PATCH 5/9] Address CodeRabbit review feedback on session cache

- Extract effectiveTTL helper and use it in GetOrCreate
- Add CheckRedirect to Close HTTP client to prevent token leakage via redirects
- Call CloseIdleConnections in Logout when session cache owns the session
- Fix mustNewSessionCache to handle error return from NewSessionCache
- Add NewSessionCache error tests for zero/negative TTL
- Replace duplicated TTL capping test logic with effectiveTTL calls
- Add IsSessionExpiredError positive test cases (401, 403, 500, wrapped)
- Fix log message style in cmd/main.go to use active voice

Signed-off-by: Stefan Hipfel <stefan.hipfel@sap.com>
---
 bmc/redfish.go            |  8 +++--
 bmc/session_cache.go      | 19 ++++++++---
 bmc/session_cache_test.go | 69 +++++++++++++++++++++------------------
 cmd/main.go               |  2 +-
 4 files changed, 58 insertions(+), 40 deletions(-)

diff --git a/bmc/redfish.go b/bmc/redfish.go
index 9212654ea..dd025cb56 100644
--- a/bmc/redfish.go
+++ b/bmc/redfish.go
@@ -187,9 +187,13 @@ func (r *RedfishBaseBMC) Manufacturer() Manufacturer {
 }
 
 // Logout closes the BMC client connection. When session caching is enabled
-// the session is owned by the cache, so Logout is a no-op.
+// the session is owned by the cache, so Logout only closes idle connections.
 func (r *RedfishBaseBMC) Logout() {
-	if r.client == nil || r.options.SessionCache != nil {
+	if r.client == nil {
+		return
+	}
+	if r.options.SessionCache != nil {
+		r.client.HTTPClient.CloseIdleConnections()
 		return
 	}
 	r.client.Logout()
diff --git a/bmc/session_cache.go b/bmc/session_cache.go
index d9d50a685..9e85e0966 100644
--- a/bmc/session_cache.go
+++ b/bmc/session_cache.go
@@ -49,6 +49,15 @@ func NewSessionCache(ttl time.Duration) (*SessionCache, error) {
 	}, nil
 }
 
+// effectiveTTL returns the shorter of the configured TTL and the BMC-advertised
+// timeout. A zero or negative bmcTTL means the BMC did not advertise a timeout.
+func effectiveTTL(configured, bmcTTL time.Duration) time.Duration {
+	if bmcTTL > 0 && bmcTTL < configured {
+		return bmcTTL
+	}
+	return configured
+}
+
 // GetOrCreate returns a valid Redfish session for the given options, reusing a
 // cached token if one exists and has not expired.
 func (c *SessionCache) GetOrCreate(ctx context.Context, opts Options) (*gofish.Session, error) {
@@ -73,12 +82,8 @@ func (c *SessionCache) GetOrCreate(ctx context.Context, opts Options) (*gofish.S
 	if err != nil {
 		return nil, err
 	}
-	ttl := c.ttl
-	if bmcTTL > 0 && bmcTTL < ttl {
-		ttl = bmcTTL
-	}
 	entry.session = session
-	entry.expiresAt = time.Now().Add(ttl)
+	entry.expiresAt = time.Now().Add(effectiveTTL(c.ttl, bmcTTL))
 	entry.insecureTLS = opts.InsecureTLS
 	return session, nil
 }
@@ -129,6 +134,10 @@ func (c *SessionCache) Close() {
 			Transport: &http.Transport{
 				TLSClientConfig: &tls.Config{InsecureSkipVerify: insecureTLS},
 			},
+			// Prevent X-Auth-Token from following redirects to another host.
+			CheckRedirect: func(_ *http.Request, _ []*http.Request) error {
+				return http.ErrUseLastResponse
+			},
 		}
 		ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
 		req, err := http.NewRequestWithContext(ctx, http.MethodDelete, key.Endpoint+sess.ID, nil)
diff --git a/bmc/session_cache_test.go b/bmc/session_cache_test.go
index 9f5ea1c19..448d0c273 100644
--- a/bmc/session_cache_test.go
+++ b/bmc/session_cache_test.go
@@ -4,6 +4,7 @@
 package bmc
 
 import (
+	"fmt"
 	"net/http"
 	"sync"
 	"time"
@@ -11,14 +12,15 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"github.com/stmcginnis/gofish"
+	"github.com/stmcginnis/gofish/schemas"
 )
 
 // seedSession is a sentinel gofish.Session used in tests that need a non-nil cached session.
 var seedSession = &gofish.Session{ID: "/redfish/v1/SessionService/Sessions/abc", Token: "test-token-123"}
 
-// mustNewSessionCache wraps NewSessionCache for tests where a valid TTL is always passed.
-func mustNewSessionCache(ttl time.Duration) *SessionCache {
-	c, err := NewSessionCache(ttl)
+// mustNewSessionCache creates a SessionCache with a 10-minute TTL for use in tests.
+func mustNewSessionCache() *SessionCache {
+	c, err := NewSessionCache(10 * time.Minute)
 	if err != nil {
 		panic(err)
 	}
@@ -52,7 +54,7 @@ var _ = Describe("SessionCache", func() {
 		})
 
 		It("clears a cached entry", func() {
-			cache := mustNewSessionCache(10 * time.Minute)
+			cache := mustNewSessionCache()
 			key := SessionCacheKey{Endpoint: "https://bmc.test", Username: "admin"}
 
 			cache.mu.Lock()
@@ -72,7 +74,7 @@ var _ = Describe("SessionCache", func() {
 		})
 
 		It("is a no-op for unknown keys", func() {
-			cache := mustNewSessionCache(10 * time.Minute)
+			cache := mustNewSessionCache()
 			Expect(func() {
 				cache.Invalidate(SessionCacheKey{Endpoint: "https://unknown", Username: "x"})
 			}).NotTo(Panic())
@@ -86,7 +88,7 @@ var _ = Describe("SessionCache", func() {
 		})
 
 		It("empties the entries map", func() {
-			cache := mustNewSessionCache(10 * time.Minute)
+			cache := mustNewSessionCache()
 			key := SessionCacheKey{Endpoint: "https://bmc.test", Username: "admin"}
 
 			cache.mu.Lock()
@@ -105,7 +107,7 @@ var _ = Describe("SessionCache", func() {
 
 	Describe("cache-hit logic (internal state)", func() {
 		It("a seeded entry within TTL is treated as a cache hit", func() {
-			cache := mustNewSessionCache(10 * time.Minute)
+			cache := mustNewSessionCache()
 			key := SessionCacheKey{Endpoint: "https://bmc.test", Username: "admin"}
 
 			cache.mu.Lock()
@@ -126,7 +128,7 @@ var _ = Describe("SessionCache", func() {
 		})
 
 		It("an expired entry is treated as a cache miss", func() {
-			cache := mustNewSessionCache(10 * time.Minute)
+			cache := mustNewSessionCache()
 			key := SessionCacheKey{Endpoint: "https://bmc.test", Username: "admin"}
 
 			cache.mu.Lock()
@@ -145,41 +147,23 @@ var _ = Describe("SessionCache", func() {
 		})
 	})
 
-	Describe("BMC TTL capping", func() {
+	Describe("effectiveTTL", func() {
 		It("uses the configured TTL when it is shorter than the BMC timeout", func() {
-			configured := 10 * time.Minute
-			bmcTTL := 30 * time.Minute
-			ttl := configured
-			if bmcTTL > 0 && bmcTTL < ttl {
-				ttl = bmcTTL
-			}
-			Expect(ttl).To(Equal(configured))
+			Expect(effectiveTTL(10*time.Minute, 30*time.Minute)).To(Equal(10 * time.Minute))
 		})
 
 		It("caps to the BMC timeout when it is shorter than the configured TTL", func() {
-			configured := 30 * time.Minute
-			bmcTTL := 10 * time.Minute
-			ttl := configured
-			if bmcTTL > 0 && bmcTTL < ttl {
-				ttl = bmcTTL
-			}
-			Expect(ttl).To(Equal(bmcTTL))
+			Expect(effectiveTTL(30*time.Minute, 10*time.Minute)).To(Equal(10 * time.Minute))
 		})
 
 		It("ignores a zero BMC timeout (not advertised)", func() {
-			configured := 10 * time.Minute
-			var bmcTTL time.Duration // zero: BMC did not advertise timeout
-			ttl := configured
-			if bmcTTL > 0 && bmcTTL < ttl {
-				ttl = bmcTTL
-			}
-			Expect(ttl).To(Equal(configured))
+			Expect(effectiveTTL(10*time.Minute, 0)).To(Equal(10 * time.Minute))
 		})
 	})
 
 	Describe("concurrent access", func() {
 		It("serialises concurrent reads for the same key without data races", func() {
-			cache := mustNewSessionCache(10 * time.Minute)
+			cache := mustNewSessionCache()
 			key := SessionCacheKey{Endpoint: "https://bmc.test", Username: "admin"}
 
 			cache.mu.Lock()
@@ -208,7 +192,7 @@ var _ = Describe("SessionCache", func() {
 		})
 
 		It("concurrent Invalidate and read do not race", func() {
-			cache := mustNewSessionCache(10 * time.Minute)
+			cache := mustNewSessionCache()
 			key := SessionCacheKey{Endpoint: "https://bmc.test", Username: "admin"}
 
 			cache.mu.Lock()
@@ -247,5 +231,26 @@ var _ = Describe("SessionCache", func() {
 		It("returns false for a non-Redfish error", func() {
 			Expect(IsSessionExpiredError(http.ErrNoCookie)).To(BeFalse())
 		})
+
+		It("returns true for a Redfish 401 error", func() {
+			err := &schemas.Error{HTTPReturnedStatusCode: http.StatusUnauthorized}
+			Expect(IsSessionExpiredError(err)).To(BeTrue())
+		})
+
+		It("returns true for a Redfish 403 error", func() {
+			err := &schemas.Error{HTTPReturnedStatusCode: http.StatusForbidden}
+			Expect(IsSessionExpiredError(err)).To(BeTrue())
+		})
+
+		It("returns false for a Redfish 500 error", func() {
+			err := &schemas.Error{HTTPReturnedStatusCode: http.StatusInternalServerError}
+			Expect(IsSessionExpiredError(err)).To(BeFalse())
+		})
+
+		It("returns true when wrapped in another error", func() {
+			redfishErr := &schemas.Error{HTTPReturnedStatusCode: http.StatusUnauthorized}
+			wrapped := fmt.Errorf("wrapped: %w", redfishErr)
+			Expect(IsSessionExpiredError(wrapped)).To(BeTrue())
+		})
 	})
 })
diff --git a/cmd/main.go b/cmd/main.go
index 68eda371f..12f01b8a4 100644
--- a/cmd/main.go
+++ b/cmd/main.go
@@ -446,7 +446,7 @@ func main() { // nolint: gocyclo
 	switch bmcAuthMode {
 	case "session-cache":
 		if bmcSessionCacheTTL <= 0 {
-			setupLog.Error(nil, "--bmc-session-cache-ttl must be positive when --bmc-auth-mode=session-cache")
+			setupLog.Error(nil, "BMC session cache TTL must be positive", "flag", "--bmc-session-cache-ttl")
 			os.Exit(1)
 		}
 		sessionCache, err = bmc.NewSessionCache(bmcSessionCacheTTL)

From b1b22509ae3bbc526eed9bee7072dcceffd718c9 Mon Sep 17 00:00:00 2001
From: Stefan Hipfel <stefan.hipfel@sap.com>
Date: Mon, 14 Sep 2026 15:53:00 +0200
Subject: [PATCH 6/9] Fix session cache correctness issues found in code review

- Propagate 401/403 errors from getSystemManufacturer() in
  NewRedfishBMCClient so the Invalidate-and-retry guard in
  CreateBMCClient is actually reachable on token expiry. Non-auth
  errors (e.g. no systems during endpoint discovery) still fall back
  to the base implementation.

- Add a closed flag to SessionCache. Close() sets it before copying
  entries; GetOrCreate() checks it both before inserting a new entry
  and after acquiring the entry lock, closing the TOCTOU window that
  could otherwise leak orphaned server-side sessions on shutdown.

- Change Invalidate() to delete(c.entries, key) instead of zeroing
  the entry fields, preventing unbounded map growth in long-running
  deployments where BMC endpoints cycle in and out.

- Extract CreateBMCClientFromOptions() from CreateBMCClient() and
  route the EndpointReconciler's ProtocolRedfish path through it so
  session expiry during endpoint discovery is recovered via the same
  invalidate-and-retry mechanism as other controllers.

- Remove the unreachable factory==nil guard after the vendor map
  lookup; DefaultVendors() never produces nil values and the
  AdditionalVendors loop hard-errors before inserting nil.

Signed-off-by: Stefan Hipfel <stefan.hipfel@sap.com>
---
 bmc/redfish.go                             | 11 +++++-----
 bmc/session_cache.go                       | 24 ++++++++++++++--------
 bmc/session_cache_test.go                  | 10 ++++-----
 internal/controller/endpoint_controller.go |  2 +-
 pkg/bmcutils/bmcutils.go                   | 14 +++++++++++++
 5 files changed, 41 insertions(+), 20 deletions(-)

diff --git a/bmc/redfish.go b/bmc/redfish.go
index dd025cb56..5158d7581 100644
--- a/bmc/redfish.go
+++ b/bmc/redfish.go
@@ -139,8 +139,12 @@ func NewRedfishBMCClient(ctx context.Context, options Options) (BMC, error) {
 
 	manufacturer, err := base.getSystemManufacturer()
 	if err != nil {
-		// If we can't determine the manufacturer (e.g. no systems yet during
-		// endpoint discovery), fall back to the base implementation.
+		// Propagate authentication errors so the caller can invalidate the session
+		// cache and retry. For other errors (e.g. no systems yet during endpoint
+		// discovery), fall back to the base implementation.
+		if IsSessionExpiredError(err) {
+			return nil, err
+		}
 		return base, nil
 	}
 	base.manufacturer = manufacturer
@@ -153,9 +157,6 @@ func NewRedfishBMCClient(ctx context.Context, options Options) (BMC, error) {
 		vendors[k] = v
 	}
 	if factory, ok := vendors[Manufacturer(manufacturer)]; ok {
-		if factory == nil {
-			return nil, fmt.Errorf("nil vendor factory registered for manufacturer %q", manufacturer)
-		}
 		client := factory(base)
 		if client == nil {
 			return nil, fmt.Errorf("vendor factory for manufacturer %q returned nil", manufacturer)
diff --git a/bmc/session_cache.go b/bmc/session_cache.go
index 9e85e0966..291d4d0c0 100644
--- a/bmc/session_cache.go
+++ b/bmc/session_cache.go
@@ -35,6 +35,7 @@ type SessionCache struct {
 	mu      sync.Mutex
 	entries map[SessionCacheKey]*sessionCacheEntry
 	ttl     time.Duration
+	closed  bool
 }
 
 // NewSessionCache returns a SessionCache with the given idle TTL.
@@ -64,6 +65,10 @@ func (c *SessionCache) GetOrCreate(ctx context.Context, opts Options) (*gofish.S
 	key := SessionCacheKey{Endpoint: opts.Endpoint, Username: opts.Username}
 
 	c.mu.Lock()
+	if c.closed {
+		c.mu.Unlock()
+		return nil, fmt.Errorf("bmc: session cache is closed")
+	}
 	entry, ok := c.entries[key]
 	if !ok {
 		entry = &sessionCacheEntry{}
@@ -74,6 +79,15 @@ func (c *SessionCache) GetOrCreate(ctx context.Context, opts Options) (*gofish.S
 	entry.mu.Lock()
 	defer entry.mu.Unlock()
 
+	// Re-check closed under the entry lock: Close may have run between the outer
+	// lock release and acquiring the entry lock, leaving this entry orphaned.
+	c.mu.Lock()
+	if c.closed {
+		c.mu.Unlock()
+		return nil, fmt.Errorf("bmc: session cache is closed")
+	}
+	c.mu.Unlock()
+
 	if entry.session != nil && time.Now().Before(entry.expiresAt) {
 		return entry.session, nil
 	}
@@ -95,15 +109,8 @@ func (c *SessionCache) Invalidate(key SessionCacheKey) {
 		return
 	}
 	c.mu.Lock()
-	entry, ok := c.entries[key]
+	delete(c.entries, key)
 	c.mu.Unlock()
-	if !ok {
-		return
-	}
-	entry.mu.Lock()
-	entry.session = nil
-	entry.expiresAt = time.Time{}
-	entry.mu.Unlock()
 }
 
 // Close deletes all live server-side Redfish sessions and clears the cache.
@@ -113,6 +120,7 @@ func (c *SessionCache) Close() {
 		return
 	}
 	c.mu.Lock()
+	c.closed = true
 	entries := make(map[SessionCacheKey]*sessionCacheEntry, len(c.entries))
 	maps.Copy(entries, c.entries)
 	c.entries = make(map[SessionCacheKey]*sessionCacheEntry)
diff --git a/bmc/session_cache_test.go b/bmc/session_cache_test.go
index 448d0c273..8a5634f8d 100644
--- a/bmc/session_cache_test.go
+++ b/bmc/session_cache_test.go
@@ -58,19 +58,17 @@ var _ = Describe("SessionCache", func() {
 			key := SessionCacheKey{Endpoint: "https://bmc.test", Username: "admin"}
 
 			cache.mu.Lock()
-			entry := &sessionCacheEntry{
+			cache.entries[key] = &sessionCacheEntry{
 				session:   seedSession,
 				expiresAt: time.Now().Add(10 * time.Minute),
 			}
-			cache.entries[key] = entry
 			cache.mu.Unlock()
 
 			cache.Invalidate(key)
 
-			entry.mu.Lock()
-			defer entry.mu.Unlock()
-			Expect(entry.session).To(BeNil())
-			Expect(entry.expiresAt.IsZero()).To(BeTrue())
+			cache.mu.Lock()
+			defer cache.mu.Unlock()
+			Expect(cache.entries).NotTo(HaveKey(key))
 		})
 
 		It("is a no-op for unknown keys", func() {
diff --git a/internal/controller/endpoint_controller.go b/internal/controller/endpoint_controller.go
index f3a581c73..785ec2e1f 100644
--- a/internal/controller/endpoint_controller.go
+++ b/internal/controller/endpoint_controller.go
@@ -106,7 +106,7 @@ func (r *EndpointReconciler) reconcile(ctx context.Context, endpoint *metalv1alp
 			switch m.Protocol {
 			case metalv1alpha1.ProtocolRedfish:
 				log.V(1).Info("Creating client for BMC", "Address", bmcOptions.Endpoint)
-				bmcClient, err := bmc.NewRedfishBMCClient(ctx, bmcOptions)
+				bmcClient, err := bmcutils.CreateBMCClientFromOptions(ctx, metalv1alpha1.ProtocolName(m.Protocol), bmcOptions)
 				if err != nil {
 					return ctrl.Result{}, fmt.Errorf("failed to create BMC client: %w", err)
 				}
diff --git a/pkg/bmcutils/bmcutils.go b/pkg/bmcutils/bmcutils.go
index 637b22636..89712bd9f 100644
--- a/pkg/bmcutils/bmcutils.go
+++ b/pkg/bmcutils/bmcutils.go
@@ -227,6 +227,20 @@ func CreateBMCClient(
 	}
 	bmcOptions.InsecureTLS = skipCertValidation
 
+	return CreateBMCClientFromOptions(ctx, bmcProtocol, bmcOptions, opts...)
+}
+
+// CreateBMCClientFromOptions creates a BMC client from fully-populated options.
+// Unlike CreateBMCClient, it does not resolve credentials from a secret — the
+// caller must set Endpoint, Username, Password, and InsecureTLS on bmcOptions.
+// When session caching is enabled and the first attempt fails with a session-expired
+// error, it invalidates the cache entry and retries once with a fresh session.
+func CreateBMCClientFromOptions(
+	ctx context.Context,
+	bmcProtocol metalv1alpha1.ProtocolName,
+	bmcOptions bmc.Options,
+	opts ...CreateBMCClientOption,
+) (bmc.BMC, error) {
 	bmcClient, err := doCreateBMCClient(ctx, bmcProtocol, bmcOptions, opts...)
 	if err != nil && bmc.IsSessionExpiredError(err) && bmcOptions.SessionCache != nil {
 		// Cached session was rejected by the BMC (e.g. server-side expiry). Invalidate

From 2e59d860c1037c24067077ec73b4a29b3ff5e625 Mon Sep 17 00:00:00 2001
From: Stefan Hipfel <stefan.hipfel@sap.com>
Date: Mon, 5 Oct 2026 14:27:21 +0200
Subject: [PATCH 7/9] Introduce bmc.Dialer to decouple session caching from
 callers

Adds a Dialer interface with two implementations:
- DirectDialer: basic auth, fresh connection per call
- PoolDialer: reuses cached Redfish session tokens via SessionCache

Controllers receive a Dialer and never interact with the cache
directly, so all Redfish protocol variants go through the same
auth path. PoolDialer also installs a retryRoundTripper that
transparently recovers from BMC-side session eviction (e.g. BMC
restart) without surfacing a 401/403 to the caller.

Signed-off-by: Stefan Hipfel <stefan.hipfel@sap.com>
---
 bmc/dialer.go                              | 103 +++++++++++++++++++++
 cmd/main.go                                |   6 +-
 internal/controller/bmc_controller.go      |   3 +-
 internal/controller/endpoint_controller.go |   3 +-
 internal/controller/server_controller.go   |  14 ++-
 internal/controller/suite_test.go          |   3 +
 pkg/bmcutils/bmcutils.go                   |  35 ++++---
 7 files changed, 144 insertions(+), 23 deletions(-)
 create mode 100644 bmc/dialer.go

diff --git a/bmc/dialer.go b/bmc/dialer.go
new file mode 100644
index 000000000..49e2a3428
--- /dev/null
+++ b/bmc/dialer.go
@@ -0,0 +1,103 @@
+// SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and IronCore contributors
+// SPDX-License-Identifier: Apache-2.0
+
+package bmc
+
+import (
+	"context"
+	"net/http"
+	"sync/atomic"
+
+	"github.com/stmcginnis/gofish"
+)
+
+// Dialer creates BMC connections.
+type Dialer interface {
+	Dial(ctx context.Context, o Options) (BMC, error)
+}
+
+// DirectDialer creates a fresh basic-auth BMC connection on every Dial call.
+type DirectDialer struct{}
+
+func (DirectDialer) Dial(ctx context.Context, o Options) (BMC, error) {
+	return NewRedfishBMCClient(ctx, o)
+}
+
+// PoolDialer dials using a shared SessionCache, reusing tokens across calls.
+// On 401/403 it invalidates the cached session and retries once.
+type PoolDialer struct {
+	cache *SessionCache
+}
+
+func NewPoolDialer(cache *SessionCache) *PoolDialer {
+	return &PoolDialer{cache: cache}
+}
+
+type redfishClientHolder interface {
+	Client() *gofish.APIClient
+}
+
+func (p *PoolDialer) Dial(ctx context.Context, o Options) (BMC, error) {
+	o.SessionCache = p.cache
+
+	bmcClient, err := NewRedfishBMCClient(ctx, o)
+	if err != nil {
+		if IsSessionExpiredError(err) {
+			p.cache.Invalidate(SessionCacheKey{Endpoint: o.Endpoint, Username: o.Username})
+			bmcClient, err = NewRedfishBMCClient(ctx, o)
+		}
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	if holder, ok := bmcClient.(redfishClientHolder); ok {
+		httpClient := holder.Client().HTTPClient
+		base := httpClient.Transport
+		if base == nil {
+			base = http.DefaultTransport
+		}
+		httpClient.Transport = &retryRoundTripper{
+			base:  base,
+			cache: p.cache,
+			key:   SessionCacheKey{Endpoint: o.Endpoint, Username: o.Username},
+			opts:  o,
+		}
+	}
+
+	return bmcClient, nil
+}
+
+// retryRoundTripper recovers from BMC-side session eviction (e.g. BMC restart)
+// without surfacing a 401/403 to the caller.
+type retryRoundTripper struct {
+	base    http.RoundTripper
+	cache   *SessionCache
+	key     SessionCacheKey
+	opts    Options
+	retried atomic.Bool
+}
+
+func (r *retryRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
+	resp, err := r.base.RoundTrip(req)
+	if err != nil {
+		return resp, err
+	}
+	if resp.StatusCode != http.StatusUnauthorized && resp.StatusCode != http.StatusForbidden {
+		return resp, nil
+	}
+	if !r.retried.CompareAndSwap(false, true) {
+		return resp, nil
+	}
+	_ = resp.Body.Close()
+
+	r.cache.Invalidate(r.key)
+	session, sessionErr := r.cache.GetOrCreate(req.Context(), r.opts)
+	if sessionErr != nil {
+		return nil, sessionErr
+	}
+
+	retryReq := req.Clone(req.Context())
+	retryReq.Header.Set("X-Auth-Token", session.Token)
+	return r.base.RoundTrip(retryReq)
+}
diff --git a/cmd/main.go b/cmd/main.go
index 12f01b8a4..e3726dea6 100644
--- a/cmd/main.go
+++ b/cmd/main.go
@@ -443,6 +443,7 @@ func main() { // nolint: gocyclo
 	}
 
 	var sessionCache *bmc.SessionCache
+	var bmcDialer bmc.Dialer = bmc.DirectDialer{}
 	switch bmcAuthMode {
 	case "session-cache":
 		if bmcSessionCacheTTL <= 0 {
@@ -462,6 +463,7 @@ func main() { // nolint: gocyclo
 			setupLog.Error(err, "Failed to register session cache shutdown")
 			os.Exit(1)
 		}
+		bmcDialer = bmc.NewPoolDialer(sessionCache)
 		setupLog.Info("Redfish session cache enabled", "ttl", bmcSessionCacheTTL)
 	case "basic", "":
 		// default: basic auth, no session cache
@@ -471,7 +473,6 @@ func main() { // nolint: gocyclo
 	}
 
 	bmcBaseOptions := bmc.Options{
-		SessionCache:            sessionCache,
 		PowerPollingInterval:    powerPollingInterval,
 		PowerPollingTimeout:     powerPollingTimeout,
 		ResourcePollingInterval: resourcePollingInterval,
@@ -485,6 +486,7 @@ func main() { // nolint: gocyclo
 		DefaultProtocol:    effectiveProtocol,
 		SkipCertValidation: effectiveSkipCert,
 		BMCOptions:         bmcBaseOptions,
+		Dialer:             bmcDialer,
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "Failed to create controller", "controller", "endpoint")
 		os.Exit(1)
@@ -509,6 +511,7 @@ func main() { // nolint: gocyclo
 		Conditions:             conditionutils.NewAccessor(conditionutils.AccessorOptions{}),
 		SSHResetTimeout:        sshResetTimeout,
 		BMCOptions:             bmcBaseOptions,
+		Dialer:                 bmcDialer,
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "Failed to create controller", "controller", "bmc")
 		os.Exit(1)
@@ -534,6 +537,7 @@ func main() { // nolint: gocyclo
 		Conditions:              conditionutils.NewAccessor(conditionutils.AccessorOptions{}),
 		DiscoveryIgnitionPath:   discoveryIgnitionPath,
 		BMCOptions:              bmcBaseOptions,
+		Dialer:                  bmcDialer,
 		DiscoveryTimeout:        discoveryTimeout,
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "Failed to create controller", "controller", "server")
diff --git a/internal/controller/bmc_controller.go b/internal/controller/bmc_controller.go
index 2f5a3911b..b4311b48c 100644
--- a/internal/controller/bmc_controller.go
+++ b/internal/controller/bmc_controller.go
@@ -47,6 +47,7 @@ type BMCReconciler struct {
 	// BMCFailureResetDelay defines the duration after which a BMC will be reset upon repeated connection failures.
 	BMCFailureResetDelay time.Duration
 	BMCOptions           bmc.Options
+	Dialer               bmc.Dialer
 	ManagerNamespace     string
 	// BMCResetWaitTime defines the duration to wait after a BMC reset before attempting reconciliation again.
 	BMCResetWaitTime time.Duration
@@ -120,7 +121,7 @@ func (r *BMCReconciler) reconcile(ctx context.Context, bmcObj *metalv1alpha1.BMC
 			RequeueAfter: r.BMCClientRetryInterval,
 		}, nil
 	}
-	bmcClient, err := bmcutils.GetBMCClientFromBMC(ctx, r.Client, bmcObj, r.DefaultProtocol, r.SkipCertValidation, r.BMCOptions, bmcutils.BMCConnectivityCheckOption)
+	bmcClient, err := bmcutils.GetBMCClientFromBMC(ctx, r.Client, bmcObj, r.DefaultProtocol, r.SkipCertValidation, r.BMCOptions, r.Dialer, bmcutils.BMCConnectivityCheckOption)
 	if err != nil {
 		if r.shouldResetBMC(bmcObj) {
 			log.V(1).Info("BMC needs reset, resetting", "BMC", bmcObj.Name)
diff --git a/internal/controller/endpoint_controller.go b/internal/controller/endpoint_controller.go
index 785ec2e1f..7e4f03340 100644
--- a/internal/controller/endpoint_controller.go
+++ b/internal/controller/endpoint_controller.go
@@ -38,6 +38,7 @@ type EndpointReconciler struct {
 	DefaultProtocol    metalv1alpha1.ProtocolScheme
 	SkipCertValidation bool
 	BMCOptions         bmc.Options
+	Dialer             bmc.Dialer
 }
 
 // +kubebuilder:rbac:groups=metal.ironcore.dev,resources=bmcs,verbs=get;list;watch;create;update;patch;delete
@@ -106,7 +107,7 @@ func (r *EndpointReconciler) reconcile(ctx context.Context, endpoint *metalv1alp
 			switch m.Protocol {
 			case metalv1alpha1.ProtocolRedfish:
 				log.V(1).Info("Creating client for BMC", "Address", bmcOptions.Endpoint)
-				bmcClient, err := bmcutils.CreateBMCClientFromOptions(ctx, metalv1alpha1.ProtocolName(m.Protocol), bmcOptions)
+				bmcClient, err := bmcutils.CreateBMCClientFromOptions(ctx, metalv1alpha1.ProtocolName(m.Protocol), bmcOptions, r.Dialer)
 				if err != nil {
 					return ctrl.Result{}, fmt.Errorf("failed to create BMC client: %w", err)
 				}
diff --git a/internal/controller/server_controller.go b/internal/controller/server_controller.go
index 66a68e4b4..171af9d37 100644
--- a/internal/controller/server_controller.go
+++ b/internal/controller/server_controller.go
@@ -100,6 +100,7 @@ type ServerReconciler struct {
 	EnforcePowerOff         bool
 	ResyncInterval          time.Duration
 	BMCOptions              bmc.Options
+	Dialer                  bmc.Dialer
 	DiscoveryTimeout        time.Duration
 	MaxConcurrentReconciles int
 	Conditions              *conditionutils.Accessor
@@ -194,7 +195,18 @@ func (r *ServerReconciler) reconcile(ctx context.Context, server *metalv1alpha1.
 		}
 	}
 
-	bmcClient, err := bmcutils.GetBMCClientForServer(ctx, r.Client, server, r.DefaultProtocol, r.SkipCertValidation, r.BMCOptions, bmcutils.WithRegistryURL(r.RegistryURL))
+	// Clear the deprecated spec.power field.
+	// TODO: remove this part once the spec field is gone
+	if server.Spec.Power != "" {
+		serverBase := server.DeepCopy()
+		server.Spec.Power = ""
+		if err := r.Patch(ctx, server, client.MergeFrom(serverBase)); err != nil {
+			return ctrl.Result{}, fmt.Errorf("failed to clear deprecated spec.power field: %w", err)
+		}
+		log.V(1).Info("Cleared deprecated spec.power field")
+	}
+
+	bmcClient, err := bmcutils.GetBMCClientForServer(ctx, r.Client, server, r.DefaultProtocol, r.SkipCertValidation, r.BMCOptions, r.Dialer, bmcutils.WithRegistryURL(r.RegistryURL))
 	if err != nil {
 		if errors.As(err, &bmcutils.BMCUnAvailableError{}) {
 			log.V(1).Info("BMC is not available, skipping", "BMC", server.Spec.BMCRef.Name, "Server", server.Name, "error", err)
diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go
index 8405e9e72..5ed3e6936 100644
--- a/internal/controller/suite_test.go
+++ b/internal/controller/suite_test.go
@@ -179,6 +179,7 @@ func SetupTest(redfishMockServers []netip.AddrPort) *corev1.Namespace {
 			MACPrefixes:        prefixDB,
 			DefaultProtocol:    metalv1alpha1.HTTPProtocolScheme,
 			SkipCertValidation: true,
+			Dialer:             bmc.DirectDialer{},
 		}).SetupWithManager(k8sManager)).To(Succeed())
 
 		dnsTemplate, err := dns.LoadTemplate("../../test/data/dns_record_template.yaml")
@@ -195,6 +196,7 @@ func SetupTest(redfishMockServers []netip.AddrPort) *corev1.Namespace {
 			SSHResetTimeout:        1 * time.Second,
 			DNSRecordTemplate:      dnsTemplate,
 			Conditions:             accessor,
+			Dialer:                 bmc.DirectDialer{},
 			BMCOptions: bmc.Options{
 				ResourcePollingInterval: 50 * time.Millisecond,
 				ResourcePollingTimeout:  200 * time.Millisecond,
@@ -221,6 +223,7 @@ func SetupTest(redfishMockServers []netip.AddrPort) *corev1.Namespace {
 			EnforceFirstBoot:        true,
 			MaxConcurrentReconciles: 5,
 			Conditions:              accessor,
+			Dialer:                  bmc.DirectDialer{},
 			BMCOptions: bmc.Options{
 				ResourcePollingInterval: 50 * time.Millisecond,
 				ResourcePollingTimeout:  200 * time.Millisecond,
diff --git a/pkg/bmcutils/bmcutils.go b/pkg/bmcutils/bmcutils.go
index 89712bd9f..b80d9a006 100644
--- a/pkg/bmcutils/bmcutils.go
+++ b/pkg/bmcutils/bmcutils.go
@@ -124,7 +124,7 @@ func GetBMCAddressForBMC(ctx context.Context, c client.Client, bmcObj *metalv1al
 
 const DefaultKubeNamespace = "default"
 
-func GetBMCClientForServer(ctx context.Context, c client.Client, server *metalv1alpha1.Server, defaultProtocol metalv1alpha1.ProtocolScheme, skipCertValidation bool, options bmc.Options, opts ...CreateBMCClientOption) (bmc.BMC, error) {
+func GetBMCClientForServer(ctx context.Context, c client.Client, server *metalv1alpha1.Server, defaultProtocol metalv1alpha1.ProtocolScheme, skipCertValidation bool, options bmc.Options, dialer bmc.Dialer, opts ...CreateBMCClientOption) (bmc.BMC, error) {
 	if server.Spec.BMCRef != nil {
 		b := &metalv1alpha1.BMC{}
 		bmcName := server.Spec.BMCRef.Name
@@ -136,7 +136,7 @@ func GetBMCClientForServer(ctx context.Context, c client.Client, server *metalv1
 		for i, o := range opts {
 			anyOpts[i] = o
 		}
-		return GetBMCClientFromBMC(ctx, c, b, defaultProtocol, skipCertValidation, options, anyOpts...)
+		return GetBMCClientFromBMC(ctx, c, b, defaultProtocol, skipCertValidation, options, dialer, anyOpts...)
 	}
 
 	if server.Spec.BMC != nil {
@@ -157,6 +157,7 @@ func GetBMCClientForServer(ctx context.Context, c client.Client, server *metalv1
 			bmcSecret,
 			options,
 			skipCertValidation,
+			dialer,
 			opts...,
 		)
 	}
@@ -164,7 +165,7 @@ func GetBMCClientForServer(ctx context.Context, c client.Client, server *metalv1
 	return nil, fmt.Errorf("server %s has neither a BMCRef nor a BMC configured", server.Name)
 }
 
-func GetBMCClientFromBMC(ctx context.Context, c client.Client, bmcObj *metalv1alpha1.BMC, defaultProtocol metalv1alpha1.ProtocolScheme, skipCertValidation bool, options bmc.Options, opts ...any) (bmc.BMC, error) {
+func GetBMCClientFromBMC(ctx context.Context, c client.Client, bmcObj *metalv1alpha1.BMC, defaultProtocol metalv1alpha1.ProtocolScheme, skipCertValidation bool, options bmc.Options, dialer bmc.Dialer, opts ...any) (bmc.BMC, error) {
 	var address string
 	var bmcClientOpts []BMCClientOptions
 	var createOpts []CreateBMCClientOption
@@ -201,7 +202,7 @@ func GetBMCClientFromBMC(ctx context.Context, c client.Client, bmcObj *metalv1al
 	}
 
 	protocolScheme := GetProtocolScheme(bmcObj.Spec.Protocol.Scheme, defaultProtocol)
-	bmcClient, err := CreateBMCClient(ctx, c, protocolScheme, bmcObj.Spec.Protocol.Name, address, bmcObj.Spec.Protocol.Port, bmcSecret, options, skipCertValidation, createOpts...)
+	bmcClient, err := CreateBMCClient(ctx, c, protocolScheme, bmcObj.Spec.Protocol.Name, address, bmcObj.Spec.Protocol.Port, bmcSecret, options, skipCertValidation, dialer, createOpts...)
 	return bmcClient, err
 }
 
@@ -215,10 +216,9 @@ func CreateBMCClient(
 	bmcSecret *metalv1alpha1.BMCSecret,
 	bmcOptions bmc.Options,
 	skipCertValidation bool,
+	dialer bmc.Dialer,
 	opts ...CreateBMCClientOption,
 ) (bmc.BMC, error) {
-	// Resolve the endpoint and credentials up-front so the cache key is
-	// available before and after the first attempt.
 	bmcOptions.Endpoint = fmt.Sprintf("%s://%s", protocolScheme, net.JoinHostPort(address, fmt.Sprintf("%d", port)))
 	var err error
 	bmcOptions.Username, bmcOptions.Password, err = GetBMCCredentialsFromSecret(bmcSecret)
@@ -227,37 +227,34 @@ func CreateBMCClient(
 	}
 	bmcOptions.InsecureTLS = skipCertValidation
 
-	return CreateBMCClientFromOptions(ctx, bmcProtocol, bmcOptions, opts...)
+	return CreateBMCClientFromOptions(ctx, bmcProtocol, bmcOptions, dialer, opts...)
 }
 
 // CreateBMCClientFromOptions creates a BMC client from fully-populated options.
 // Unlike CreateBMCClient, it does not resolve credentials from a secret — the
 // caller must set Endpoint, Username, Password, and InsecureTLS on bmcOptions.
-// When session caching is enabled and the first attempt fails with a session-expired
-// error, it invalidates the cache entry and retries once with a fresh session.
+// The provided Dialer is used for Redfish connections; other protocols use their
+// own constructors directly.
 func CreateBMCClientFromOptions(
 	ctx context.Context,
 	bmcProtocol metalv1alpha1.ProtocolName,
 	bmcOptions bmc.Options,
+	dialer bmc.Dialer,
 	opts ...CreateBMCClientOption,
 ) (bmc.BMC, error) {
-	bmcClient, err := doCreateBMCClient(ctx, bmcProtocol, bmcOptions, opts...)
-	if err != nil && bmc.IsSessionExpiredError(err) && bmcOptions.SessionCache != nil {
-		// Cached session was rejected by the BMC (e.g. server-side expiry). Invalidate
-		// and retry once with a fresh session.
-		key := bmc.SessionCacheKey{Endpoint: bmcOptions.Endpoint, Username: bmcOptions.Username}
-		bmcOptions.SessionCache.Invalidate(key)
-		bmcClient, err = doCreateBMCClient(ctx, bmcProtocol, bmcOptions, opts...)
-	}
-	return bmcClient, err
+	return doCreateBMCClient(ctx, bmcProtocol, bmcOptions, dialer, opts...)
 }
 
 func doCreateBMCClient(
 	ctx context.Context,
 	bmcProtocol metalv1alpha1.ProtocolName,
 	bmcOptions bmc.Options,
+	dialer bmc.Dialer,
 	opts ...CreateBMCClientOption,
 ) (bmc.BMC, error) {
+	if dialer == nil {
+		dialer = bmc.DirectDialer{}
+	}
 	var bmcClient bmc.BMC
 	var err error
 
@@ -271,7 +268,7 @@ func doCreateBMCClient(
 
 	switch bmcProtocol {
 	case metalv1alpha1.ProtocolRedfish:
-		bmcClient, err = bmc.NewRedfishBMCClient(ctx, bmcOptions)
+		bmcClient, err = dialer.Dial(ctx, bmcOptions)
 		if err != nil {
 			return nil, err
 		}

From 49f70e4e9a432414e9cc7f24c5397c10538c20e0 Mon Sep 17 00:00:00 2001
From: Stefan Hipfel <stefan.hipfel@sap.com>
Date: Mon, 5 Oct 2026 14:29:28 +0200
Subject: [PATCH 8/9] Rename PoolDialer to SessionDialer

SessionDialer expresses intent (session-based auth) rather than the
internal mechanism (pooling), and pairs more naturally with DirectDialer.

Signed-off-by: Stefan Hipfel <stefan.hipfel@sap.com>
---
 bmc/dialer.go | 10 +++++-----
 cmd/main.go   |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/bmc/dialer.go b/bmc/dialer.go
index 49e2a3428..88d5adc00 100644
--- a/bmc/dialer.go
+++ b/bmc/dialer.go
@@ -23,21 +23,21 @@ func (DirectDialer) Dial(ctx context.Context, o Options) (BMC, error) {
 	return NewRedfishBMCClient(ctx, o)
 }
 
-// PoolDialer dials using a shared SessionCache, reusing tokens across calls.
+// SessionDialer dials using a shared SessionCache, reusing tokens across calls.
 // On 401/403 it invalidates the cached session and retries once.
-type PoolDialer struct {
+type SessionDialer struct {
 	cache *SessionCache
 }
 
-func NewPoolDialer(cache *SessionCache) *PoolDialer {
-	return &PoolDialer{cache: cache}
+func NewSessionDialer(cache *SessionCache) *SessionDialer {
+	return &SessionDialer{cache: cache}
 }
 
 type redfishClientHolder interface {
 	Client() *gofish.APIClient
 }
 
-func (p *PoolDialer) Dial(ctx context.Context, o Options) (BMC, error) {
+func (p *SessionDialer) Dial(ctx context.Context, o Options) (BMC, error) {
 	o.SessionCache = p.cache
 
 	bmcClient, err := NewRedfishBMCClient(ctx, o)
diff --git a/cmd/main.go b/cmd/main.go
index e3726dea6..f859f5529 100644
--- a/cmd/main.go
+++ b/cmd/main.go
@@ -463,7 +463,7 @@ func main() { // nolint: gocyclo
 			setupLog.Error(err, "Failed to register session cache shutdown")
 			os.Exit(1)
 		}
-		bmcDialer = bmc.NewPoolDialer(sessionCache)
+		bmcDialer = bmc.NewSessionDialer(sessionCache)
 		setupLog.Info("Redfish session cache enabled", "ttl", bmcSessionCacheTTL)
 	case "basic", "":
 		// default: basic auth, no session cache

From 04cb8406c8031a87cdebaa4c51b41b7989223559 Mon Sep 17 00:00:00 2001
From: Stefan Hipfel <stefan.hipfel@sap.com>
Date: Tue, 13 Oct 2026 13:39:32 +0200
Subject: [PATCH 9/9] Refresh request body from GetBody on session retry to
 avoid replaying a consumed body

Signed-off-by: Stefan Hipfel <stefan.hipfel@sap.com>
---
 bmc/dialer.go                            | 11 +++++++++++
 internal/controller/server_controller.go | 11 -----------
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/bmc/dialer.go b/bmc/dialer.go
index 88d5adc00..c73287e2f 100644
--- a/bmc/dialer.go
+++ b/bmc/dialer.go
@@ -5,6 +5,7 @@ package bmc
 
 import (
 	"context"
+	"fmt"
 	"net/http"
 	"sync/atomic"
 
@@ -99,5 +100,15 @@ func (r *retryRoundTripper) RoundTrip(req *http.Request) (*http.Response, error)
 
 	retryReq := req.Clone(req.Context())
 	retryReq.Header.Set("X-Auth-Token", session.Token)
+	if req.Body != nil && req.Body != http.NoBody {
+		if req.GetBody == nil {
+			return nil, fmt.Errorf("cannot retry request with non-rewindable body")
+		}
+		freshBody, bodyErr := req.GetBody()
+		if bodyErr != nil {
+			return nil, fmt.Errorf("getting fresh request body for retry: %w", bodyErr)
+		}
+		retryReq.Body = freshBody
+	}
 	return r.base.RoundTrip(retryReq)
 }
diff --git a/internal/controller/server_controller.go b/internal/controller/server_controller.go
index 171af9d37..e3469ab73 100644
--- a/internal/controller/server_controller.go
+++ b/internal/controller/server_controller.go
@@ -195,17 +195,6 @@ func (r *ServerReconciler) reconcile(ctx context.Context, server *metalv1alpha1.
 		}
 	}
 
-	// Clear the deprecated spec.power field.
-	// TODO: remove this part once the spec field is gone
-	if server.Spec.Power != "" {
-		serverBase := server.DeepCopy()
-		server.Spec.Power = ""
-		if err := r.Patch(ctx, server, client.MergeFrom(serverBase)); err != nil {
-			return ctrl.Result{}, fmt.Errorf("failed to clear deprecated spec.power field: %w", err)
-		}
-		log.V(1).Info("Cleared deprecated spec.power field")
-	}
-
 	bmcClient, err := bmcutils.GetBMCClientForServer(ctx, r.Client, server, r.DefaultProtocol, r.SkipCertValidation, r.BMCOptions, r.Dialer, bmcutils.WithRegistryURL(r.RegistryURL))
 	if err != nil {
 		if errors.As(err, &bmcutils.BMCUnAvailableError{}) {