diff --git a/bmc/redfish.go b/bmc/redfish.go index 09e0af317..0038b7f57 100644 --- a/bmc/redfish.go +++ b/bmc/redfish.go @@ -44,26 +44,22 @@ const ( // Options contain the options for the BMC redfish client. type Options struct { - Endpoint string - Username string - Password string - BasicAuth bool + Endpoint string + Username string + Password string - // 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..d9d50a685 --- /dev/null +++ b/bmc/session_cache.go @@ -0,0 +1,189 @@ +// SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and IronCore contributors +// SPDX-License-Identifier: Apache-2.0 + +package bmc + +import ( + "context" + "crypto/tls" + "errors" + "fmt" + "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 + insecureTLS bool +} + +// 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. +// Returns an error if ttl is not positive. +func NewSessionCache(ttl time.Duration) (*SessionCache, error) { + if ttl <= 0 { + 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 +// cached token if one exists and has not expired. +func (c *SessionCache) GetOrCreate(ctx context.Context, opts Options) (*gofish.Session, error) { + 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) + entry.insecureTLS = opts.InsecureTLS + 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 + insecureTLS := entry.insecureTLS + entry.session = nil + entry.mu.Unlock() + + if sess == nil || sess.ID == "" { + continue + } + //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) + resp, err := httpClient.Do(req) + if err == nil { + _ = resp.Body.Close() + } + cancel() + } +} + +// 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 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 + } + var redfishErr *schemas.Error + if !errors.As(err, &redfishErr) { + return false + } + return redfishErr.HTTPReturnedStatusCode == http.StatusUnauthorized || + redfishErr.HTTPReturnedStatusCode == http.StatusForbidden +} diff --git a/bmc/session_cache_test.go b/bmc/session_cache_test.go new file mode 100644 index 000000000..dfd7e14ea --- /dev/null +++ b/bmc/session_cache_test.go @@ -0,0 +1,251 @@ +// 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"} + +// 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) + } + return c +} + +var _ = Describe("SessionCache", func() { + Describe("NewSessionCache", func() { + It("returns a non-nil cache with the given TTL", func() { + cache, err := NewSessionCache(10 * time.Minute) + Expect(err).NotTo(HaveOccurred()) + Expect(cache).NotTo(BeNil()) + Expect(cache.ttl).To(Equal(10 * time.Minute)) + }) + + It("returns an error for a zero TTL", func() { + _, err := NewSessionCache(0) + Expect(err).To(HaveOccurred()) + }) + + It("returns an error for a negative TTL", func() { + _, err := NewSessionCache(-1 * time.Second) + Expect(err).To(HaveOccurred()) + }) + }) + + 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 := mustNewSessionCache() + 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 := mustNewSessionCache() + 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 := mustNewSessionCache() + 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 := mustNewSessionCache() + 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 := mustNewSessionCache() + 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 := mustNewSessionCache() + 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 := mustNewSessionCache() + 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 6cf565416..3d119f367 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -107,6 +107,8 @@ func main() { // nolint: gocyclo serverClaimMaxConcurrentReconciles int dnsRecordTemplatePath string defaultFailedAutoRetryCount int + bmcAuthMode string + bmcSessionCacheTTL time.Duration ) flag.IntVar(&serverMaxConcurrentReconciles, "server-max-concurrent-reconciles", 5, @@ -183,6 +185,17 @@ func main() { // nolint: gocyclo "Path to the DNS record template file used for creating DNS records for Servers.") 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.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. "+ + "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{ Development: true, @@ -410,12 +423,48 @@ func main() { // nolint: gocyclo ctrlmetrics.Registry.MustRegister(serverCollector) setupLog.Info("Registered custom server metrics collector") + var sessionCache *bmc.SessionCache + switch bmcAuthMode { + case "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, 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() + 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, + PowerPollingInterval: powerPollingInterval, + PowerPollingTimeout: powerPollingTimeout, + ResourcePollingInterval: resourcePollingInterval, + ResourcePollingTimeout: resourcePollingTimeout, + } 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) @@ -440,9 +489,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) @@ -467,14 +514,8 @@ func main() { // nolint: gocyclo MaxConcurrentReconciles: serverMaxConcurrentReconciles, Conditions: conditionutils.NewAccessor(conditionutils.AccessorOptions{}), DiscoveryIgnitionPath: discoveryIgnitionPath, - BMCOptions: bmc.Options{ - BasicAuth: true, - 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) @@ -512,7 +553,7 @@ func main() { // nolint: gocyclo ResyncInterval: maintenanceResyncInterval, Conditions: conditionutils.NewAccessor(conditionutils.AccessorOptions{}), BMCOptions: bmc.Options{ - BasicAuth: true, + SessionCache: sessionCache, PowerPollingInterval: powerPollingInterval, PowerPollingTimeout: powerPollingTimeout, ResourcePollingInterval: resourcePollingInterval, @@ -533,7 +574,7 @@ func main() { // nolint: gocyclo ResyncInterval: maintenanceResyncInterval, Conditions: conditionutils.NewAccessor(conditionutils.AccessorOptions{}), BMCOptions: bmc.Options{ - BasicAuth: true, + SessionCache: sessionCache, PowerPollingInterval: powerPollingInterval, PowerPollingTimeout: powerPollingTimeout, ResourcePollingInterval: resourcePollingInterval, @@ -553,7 +594,7 @@ func main() { // nolint: gocyclo SkipCertValidation: effectiveSkipCert, Conditions: conditionutils.NewAccessor(conditionutils.AccessorOptions{}), BMCOptions: bmc.Options{ - BasicAuth: true, + SessionCache: sessionCache, PowerPollingInterval: powerPollingInterval, PowerPollingTimeout: powerPollingTimeout, ResourcePollingInterval: resourcePollingInterval, @@ -573,7 +614,7 @@ func main() { // nolint: gocyclo ResyncInterval: maintenanceResyncInterval, Conditions: conditionutils.NewAccessor(conditionutils.AccessorOptions{}), BMCOptions: bmc.Options{ - BasicAuth: true, + SessionCache: sessionCache, PowerPollingInterval: powerPollingInterval, PowerPollingTimeout: powerPollingTimeout, ResourcePollingInterval: resourcePollingInterval, @@ -622,7 +663,7 @@ func main() { // nolint: gocyclo DefaultProtocol: effectiveProtocol, SkipCertValidation: effectiveSkipCert, BMCOptions: bmc.Options{ - BasicAuth: true, + SessionCache: sessionCache, PowerPollingInterval: powerPollingInterval, PowerPollingTimeout: powerPollingTimeout, ResourcePollingInterval: resourcePollingInterval, diff --git a/internal/controller/bmcuser_controller.go b/internal/controller/bmcuser_controller.go index c01142900..779a0eaee 100644 --- a/internal/controller/bmcuser_controller.go +++ b/internal/controller/bmcuser_controller.go @@ -370,10 +370,10 @@ func (r *BMCUserReconciler) bmcConnectionTest(ctx context.Context, secret *metal return false, fmt.Errorf("failed to create BMC client: %w", err) } defer bmcClient.Logout() - // With BasicAuth, ConnectContext only stores credentials without making an + // With BasicAuth (no session cache), ConnectContext only stores credentials without making an // authenticated request (it skips CreateSession). Probe an authenticated // endpoint here so that invalid/rotated credentials are detected. - if r.BMCOptions.BasicAuth { + if r.BMCOptions.SessionCache == nil { if _, err := bmcClient.GetAccountService(); err != nil { var httpErr *schemas.Error if errors.As(err, &httpErr) && (httpErr.HTTPReturnedStatusCode == 401 || httpErr.HTTPReturnedStatusCode == 403) { diff --git a/internal/controller/endpoint_controller.go b/internal/controller/endpoint_controller.go index a8ff3af52..a7d13689f 100644 --- a/internal/controller/endpoint_controller.go +++ b/internal/controller/endpoint_controller.go @@ -88,12 +88,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 8a6027735..78a632293 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -203,7 +203,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()) @@ -230,7 +229,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"), @@ -267,7 +265,6 @@ func SetupTest(redfishMockServers []netip.AddrPort) *corev1.Namespace { ResourcePollingTimeout: 200 * time.Millisecond, PowerPollingInterval: 50 * time.Millisecond, PowerPollingTimeout: 200 * time.Millisecond, - BasicAuth: true, }, TimeoutExpiry: 6 * time.Second, }).SetupWithManager(k8sManager)).To(Succeed()) @@ -285,7 +282,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()) @@ -308,7 +304,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()) @@ -325,7 +320,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()) @@ -357,7 +351,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()) 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)