From 48de8265c568105ded20200a82bbd137d178f6fb Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Fri, 20 Jun 2025 09:00:02 +0100 Subject: [PATCH] Multiple changes to improve performance and reduce complexity. - Optimise the errors and recovery. - Deduplicate code in metadata cache. - Remove unused performance monitoring code. - Simplify session management and settings handling. --- autocleanup.go | 53 +++ cache.go | 32 +- error_recovery.go | 367 +++++++++++++++--- metadata_cache.go | 30 +- metadata_cache_test.go | 8 +- performance_monitoring.go | 676 --------------------------------- performance_monitoring_test.go | 324 ---------------- robustness_test.go | 7 +- security_monitoring.go | 258 +++++++------ security_monitoring_test.go | 66 +--- session.go | 158 ++++---- settings.go | 11 + test_tokens.go | 2 +- 13 files changed, 648 insertions(+), 1344 deletions(-) delete mode 100644 performance_monitoring.go delete mode 100644 performance_monitoring_test.go diff --git a/autocleanup.go b/autocleanup.go index b18b752..5f80f42 100644 --- a/autocleanup.go +++ b/autocleanup.go @@ -2,6 +2,57 @@ package traefikoidc import "time" +// BackgroundTask represents a recurring task that runs in the background +type BackgroundTask struct { + stopChan chan struct{} + taskFunc func() + logger *Logger + name string + interval time.Duration +} + +// NewBackgroundTask creates a new background task +func NewBackgroundTask(name string, interval time.Duration, taskFunc func(), logger *Logger) *BackgroundTask { + return &BackgroundTask{ + name: name, + interval: interval, + stopChan: make(chan struct{}), + taskFunc: taskFunc, + logger: logger, + } +} + +// Start begins the background task execution +func (bt *BackgroundTask) Start() { + go bt.run() +} + +// Stop terminates the background task +func (bt *BackgroundTask) Stop() { + close(bt.stopChan) +} + +// run is the main execution loop for the background task +func (bt *BackgroundTask) run() { + ticker := time.NewTicker(bt.interval) + defer ticker.Stop() + + bt.logger.Debug("Starting background task: %s", bt.name) + + // Run task immediately on startup + bt.taskFunc() + + for { + select { + case <-ticker.C: + bt.taskFunc() + case <-bt.stopChan: + bt.logger.Debug("Stopping background task: %s", bt.name) + return + } + } +} + // autoCleanupRoutine periodically calls the provided cleanup function. // It starts a ticker with the given interval and executes the cleanup function // on each tick. The routine stops gracefully when a signal is received on the @@ -12,6 +63,8 @@ import "time" // - interval: The time duration between cleanup calls. // - stop: A channel used to signal the routine to stop. Receiving any value will terminate the loop. // - cleanup: The function to call periodically for cleanup tasks. +// +// Deprecated: Use BackgroundTask instead. func autoCleanupRoutine(interval time.Duration, stop <-chan struct{}, cleanup func()) { ticker := time.NewTicker(interval) defer ticker.Stop() diff --git a/cache.go b/cache.go index b168305..2939184 100644 --- a/cache.go +++ b/cache.go @@ -26,7 +26,8 @@ type Cache struct { items map[string]CacheItem order *list.List elems map[string]*list.Element - stopCleanup chan struct{} + cleanupTask *BackgroundTask + logger *Logger maxSize int autoCleanupInterval time.Duration mutex sync.RWMutex @@ -36,18 +37,26 @@ type Cache struct { const DefaultMaxSize = 500 // NewCache creates a new empty cache instance with default settings. -// It initializes the internal maps and list, sets the default maximum size, -// and starts the automatic cleanup goroutine. +// It initializes the internal maps and list and sets the default maximum size. func NewCache() *Cache { + return NewCacheWithLogger(nil) +} + +// NewCacheWithLogger creates a new cache with a specified logger +func NewCacheWithLogger(logger *Logger) *Cache { + if logger == nil { + logger = newNoOpLogger() + } + c := &Cache{ items: make(map[string]CacheItem, DefaultMaxSize), order: list.New(), elems: make(map[string]*list.Element, DefaultMaxSize), maxSize: DefaultMaxSize, autoCleanupInterval: 5 * time.Minute, - stopCleanup: make(chan struct{}), + logger: logger, } - go c.startAutoCleanup() + c.startAutoCleanup() return c } @@ -203,15 +212,18 @@ func (c *Cache) removeItem(key string) { } } -// startAutoCleanup starts the background goroutine that automatically calls the Cleanup method +// startAutoCleanup starts the background task that automatically calls the Cleanup method // at the interval specified by c.autoCleanupInterval. -// It uses the autoCleanupRoutine helper function. func (c *Cache) startAutoCleanup() { - autoCleanupRoutine(c.autoCleanupInterval, c.stopCleanup, c.Cleanup) + c.cleanupTask = NewBackgroundTask("cache-cleanup", c.autoCleanupInterval, c.Cleanup, c.logger) + c.cleanupTask.Start() } -// Close stops the automatic cleanup goroutine associated with this cache instance. +// Close stops the automatic cleanup task associated with this cache instance. // It should be called when the cache is no longer needed to prevent resource leaks. func (c *Cache) Close() { - close(c.stopCleanup) + if c.cleanupTask != nil { + c.cleanupTask.Stop() + c.cleanupTask = nil + } } diff --git a/error_recovery.go b/error_recovery.go index 0910bda..2edfd2d 100644 --- a/error_recovery.go +++ b/error_recovery.go @@ -12,6 +12,122 @@ import ( "time" ) +// ErrorRecoveryMechanism defines the common interface for all error recovery mechanisms +type ErrorRecoveryMechanism interface { + // ExecuteWithContext executes a function with error recovery + ExecuteWithContext(ctx context.Context, fn func() error) error + // GetMetrics returns metrics about the error recovery mechanism + GetMetrics() map[string]any + // Reset resets the state of the error recovery mechanism + Reset() + // IsAvailable returns whether the mechanism is available for use + IsAvailable() bool +} + +// BaseRecoveryMechanism provides common functionality for error recovery mechanisms +type BaseRecoveryMechanism struct { + startTime time.Time + lastFailureTime time.Time + lastSuccessTime time.Time + logger *Logger + name string + totalRequests int64 + totalFailures int64 + totalSuccesses int64 + mutex sync.RWMutex +} + +// NewBaseRecoveryMechanism creates a new base recovery mechanism +func NewBaseRecoveryMechanism(name string, logger *Logger) *BaseRecoveryMechanism { + if logger == nil { + logger = newNoOpLogger() + } + + return &BaseRecoveryMechanism{ + name: name, + logger: logger, + startTime: time.Now(), + } +} + +// RecordRequest records a request to the error recovery mechanism +func (b *BaseRecoveryMechanism) RecordRequest() { + atomic.AddInt64(&b.totalRequests, 1) +} + +// RecordSuccess records a successful operation +func (b *BaseRecoveryMechanism) RecordSuccess() { + atomic.AddInt64(&b.totalSuccesses, 1) + + b.mutex.Lock() + defer b.mutex.Unlock() + b.lastSuccessTime = time.Now() +} + +// RecordFailure records a failed operation +func (b *BaseRecoveryMechanism) RecordFailure() { + atomic.AddInt64(&b.totalFailures, 1) + + b.mutex.Lock() + defer b.mutex.Unlock() + b.lastFailureTime = time.Now() +} + +// GetBaseMetrics returns base metrics common to all recovery mechanisms +func (b *BaseRecoveryMechanism) GetBaseMetrics() map[string]any { + b.mutex.RLock() + defer b.mutex.RUnlock() + + metrics := map[string]any{ + "total_requests": atomic.LoadInt64(&b.totalRequests), + "total_failures": atomic.LoadInt64(&b.totalFailures), + "total_successes": atomic.LoadInt64(&b.totalSuccesses), + "uptime_seconds": time.Since(b.startTime).Seconds(), + "name": b.name, + } + + if !b.lastFailureTime.IsZero() { + metrics["last_failure_time"] = b.lastFailureTime.Format(time.RFC3339) + metrics["seconds_since_last_failure"] = time.Since(b.lastFailureTime).Seconds() + } + + if !b.lastSuccessTime.IsZero() { + metrics["last_success_time"] = b.lastSuccessTime.Format(time.RFC3339) + metrics["seconds_since_last_success"] = time.Since(b.lastSuccessTime).Seconds() + } + + // Calculate success rate + if metrics["total_requests"].(int64) > 0 { + successRate := float64(metrics["total_successes"].(int64)) / float64(metrics["total_requests"].(int64)) + metrics["success_rate"] = successRate + } else { + metrics["success_rate"] = 1.0 // Default to 100% if no requests + } + + return metrics +} + +// LogInfo logs an informational message +func (b *BaseRecoveryMechanism) LogInfo(format string, args ...any) { + if b.logger != nil { + b.logger.Infof("%s: "+format, append([]any{b.name}, args...)...) + } +} + +// LogError logs an error message +func (b *BaseRecoveryMechanism) LogError(format string, args ...any) { + if b.logger != nil { + b.logger.Errorf("%s: "+format, append([]any{b.name}, args...)...) + } +} + +// LogDebug logs a debug message +func (b *BaseRecoveryMechanism) LogDebug(format string, args ...any) { + if b.logger != nil { + b.logger.Debugf("%s: "+format, append([]any{b.name}, args...)...) + } +} + // CircuitBreakerState represents the current state of a circuit breaker type CircuitBreakerState int @@ -26,18 +142,12 @@ const ( // CircuitBreaker implements the circuit breaker pattern for external service calls type CircuitBreaker struct { - lastFailureTime time.Time - lastSuccessTime time.Time - logger *Logger - maxFailures int - timeout time.Duration - resetTimeout time.Duration - state CircuitBreakerState - failures int64 - totalRequests int64 - totalFailures int64 - totalSuccesses int64 - mutex sync.RWMutex + *BaseRecoveryMechanism + maxFailures int + timeout time.Duration + resetTimeout time.Duration + state CircuitBreakerState + failures int64 } // CircuitBreakerConfig holds configuration for circuit breakers @@ -59,17 +169,17 @@ func DefaultCircuitBreakerConfig() CircuitBreakerConfig { // NewCircuitBreaker creates a new circuit breaker with the given configuration func NewCircuitBreaker(config CircuitBreakerConfig, logger *Logger) *CircuitBreaker { return &CircuitBreaker{ - maxFailures: config.MaxFailures, - timeout: config.Timeout, - resetTimeout: config.ResetTimeout, - state: CircuitBreakerClosed, - logger: logger, + BaseRecoveryMechanism: NewBaseRecoveryMechanism("circuit-breaker", logger), + maxFailures: config.MaxFailures, + timeout: config.Timeout, + resetTimeout: config.ResetTimeout, + state: CircuitBreakerClosed, } } -// Execute runs the given function with circuit breaker protection -func (cb *CircuitBreaker) Execute(fn func() error) error { - atomic.AddInt64(&cb.totalRequests, 1) +// ExecuteWithContext implements the ErrorRecoveryMechanism interface +func (cb *CircuitBreaker) ExecuteWithContext(ctx context.Context, fn func() error) error { + cb.RecordRequest() // Check if circuit breaker allows the request if !cb.allowRequest() { @@ -81,15 +191,20 @@ func (cb *CircuitBreaker) Execute(fn func() error) error { // Record the result if err != nil { cb.recordFailure() - atomic.AddInt64(&cb.totalFailures, 1) + cb.RecordFailure() return err } cb.recordSuccess() - atomic.AddInt64(&cb.totalSuccesses, 1) + cb.RecordSuccess() return nil } +// Execute is the original method for backward compatibility +func (cb *CircuitBreaker) Execute(fn func() error) error { + return cb.ExecuteWithContext(context.Background(), fn) +} + // allowRequest checks if the circuit breaker allows the request func (cb *CircuitBreaker) allowRequest() bool { cb.mutex.Lock() @@ -125,19 +240,18 @@ func (cb *CircuitBreaker) recordFailure() { defer cb.mutex.Unlock() cb.failures++ - cb.lastFailureTime = time.Now() switch cb.state { case CircuitBreakerClosed: if cb.failures >= int64(cb.maxFailures) { cb.state = CircuitBreakerOpen - cb.logger.Errorf("Circuit breaker opened after %d failures", cb.failures) + cb.LogError("Circuit breaker opened after %d failures", cb.failures) } case CircuitBreakerHalfOpen: // Go back to open state on any failure in half-open cb.state = CircuitBreakerOpen - cb.logger.Errorf("Circuit breaker returned to open state after failure in half-open") + cb.LogError("Circuit breaker returned to open state after failure in half-open") } } @@ -146,14 +260,12 @@ func (cb *CircuitBreaker) recordSuccess() { cb.mutex.Lock() defer cb.mutex.Unlock() - cb.lastSuccessTime = time.Now() - switch cb.state { case CircuitBreakerHalfOpen: // Reset failures and close circuit on success in half-open cb.failures = 0 cb.state = CircuitBreakerClosed - cb.logger.Infof("Circuit breaker closed after successful request in half-open state") + cb.LogInfo("Circuit breaker closed after successful request in half-open state") case CircuitBreakerClosed: // Reset failure count on success @@ -168,20 +280,48 @@ func (cb *CircuitBreaker) GetState() CircuitBreakerState { return cb.state } -// GetMetrics returns circuit breaker metrics +// Reset resets the circuit breaker to its initial state +func (cb *CircuitBreaker) Reset() { + cb.mutex.Lock() + defer cb.mutex.Unlock() + + cb.state = CircuitBreakerClosed + atomic.StoreInt64(&cb.failures, 0) + cb.LogInfo("Circuit breaker has been reset") +} + +// IsAvailable returns whether the circuit breaker is allowing requests +func (cb *CircuitBreaker) IsAvailable() bool { + return cb.allowRequest() +} + +// GetMetrics returns metrics about the circuit breaker func (cb *CircuitBreaker) GetMetrics() map[string]any { cb.mutex.RLock() - defer cb.mutex.RUnlock() + state := cb.state + failures := cb.failures + cb.mutex.RUnlock() - return map[string]any{ - "state": cb.state, - "failures": cb.failures, - "total_requests": atomic.LoadInt64(&cb.totalRequests), - "total_failures": atomic.LoadInt64(&cb.totalFailures), - "total_successes": atomic.LoadInt64(&cb.totalSuccesses), - "last_failure": cb.lastFailureTime, - "last_success": cb.lastSuccessTime, + metrics := cb.GetBaseMetrics() + + // Add circuit breaker specific metrics + stateStr := "unknown" + switch state { + case CircuitBreakerClosed: + stateStr = "closed" + case CircuitBreakerOpen: + stateStr = "open" + case CircuitBreakerHalfOpen: + stateStr = "half-open" } + + metrics["state"] = stateStr + metrics["max_failures"] = cb.maxFailures + metrics["current_failures"] = failures + metrics["timeout_ms"] = cb.timeout.Milliseconds() + metrics["reset_timeout_ms"] = cb.resetTimeout.Milliseconds() + + return metrics } // RetryConfig holds configuration for retry mechanisms @@ -213,20 +353,21 @@ func DefaultRetryConfig() RetryConfig { // RetryExecutor implements retry logic with exponential backoff type RetryExecutor struct { - logger *Logger + *BaseRecoveryMechanism config RetryConfig } // NewRetryExecutor creates a new retry executor func NewRetryExecutor(config RetryConfig, logger *Logger) *RetryExecutor { return &RetryExecutor{ - config: config, - logger: logger, + BaseRecoveryMechanism: NewBaseRecoveryMechanism("retry-executor", logger), + config: config, } } -// Execute runs the given function with retry logic -func (re *RetryExecutor) Execute(ctx context.Context, fn func() error) error { +// ExecuteWithContext implements the ErrorRecoveryMechanism interface +func (re *RetryExecutor) ExecuteWithContext(ctx context.Context, fn func() error) error { + re.RecordRequest() var lastErr error for attempt := 1; attempt <= re.config.MaxAttempts; attempt++ { @@ -234,8 +375,9 @@ func (re *RetryExecutor) Execute(ctx context.Context, fn func() error) error { err := fn() if err == nil { if attempt > 1 { - re.logger.Infof("Operation succeeded on attempt %d", attempt) + re.LogInfo("Operation succeeded on attempt %d", attempt) } + re.RecordSuccess() return nil } @@ -243,30 +385,39 @@ func (re *RetryExecutor) Execute(ctx context.Context, fn func() error) error { // Check if error is retryable if !re.isRetryableError(err) { - re.logger.Debugf("Non-retryable error on attempt %d: %v", attempt, err) + re.LogDebug("Non-retryable error on attempt %d: %v", attempt, err) + re.RecordFailure() return err } // Don't wait after the last attempt if attempt == re.config.MaxAttempts { + re.RecordFailure() break } // Calculate delay with exponential backoff delay := re.calculateDelay(attempt) - re.logger.Debugf("Retrying operation after %v (attempt %d/%d): %v", + re.LogDebug("Retrying operation after %v (attempt %d/%d): %v", delay, attempt, re.config.MaxAttempts, err) // Wait with context cancellation support select { case <-ctx.Done(): + re.RecordFailure() return ctx.Err() case <-time.After(delay): // Continue to next attempt } } - return fmt.Errorf("operation failed after %d attempts: %w", re.config.MaxAttempts, lastErr) + finalErr := fmt.Errorf("operation failed after %d attempts: %w", re.config.MaxAttempts, lastErr) + return finalErr +} + +// Execute runs the given function with retry logic (for backward compatibility) +func (re *RetryExecutor) Execute(ctx context.Context, fn func() error) error { + return re.ExecuteWithContext(ctx, fn) } // isRetryableError checks if an error should trigger a retry @@ -335,6 +486,32 @@ func (re *RetryExecutor) calculateDelay(attempt int) time.Duration { return time.Duration(delay) } +// Reset resets the retry executor state +func (re *RetryExecutor) Reset() { + // Nothing to reset for RetryExecutor + re.LogDebug("Retry executor reset") +} + +// IsAvailable always returns true for RetryExecutor +func (re *RetryExecutor) IsAvailable() bool { + return true +} + +// GetMetrics returns metrics about the retry executor +func (re *RetryExecutor) GetMetrics() map[string]any { + metrics := re.GetBaseMetrics() + + // Add retry executor specific metrics + metrics["max_attempts"] = re.config.MaxAttempts + metrics["initial_delay_ms"] = re.config.InitialDelay.Milliseconds() + metrics["max_delay_ms"] = re.config.MaxDelay.Milliseconds() + metrics["backoff_factor"] = re.config.BackoffFactor + metrics["enable_jitter"] = re.config.EnableJitter + metrics["retryable_errors"] = re.config.RetryableErrors + + return metrics +} + // HTTPError represents an HTTP error with status code type HTTPError struct { Message string @@ -348,10 +525,10 @@ func (e *HTTPError) Error() string { // GracefulDegradation implements graceful degradation patterns type GracefulDegradation struct { + *BaseRecoveryMechanism fallbacks map[string]func() (any, error) healthChecks map[string]func() bool degradedServices map[string]time.Time - logger *Logger config GracefulDegradationConfig mutex sync.RWMutex } @@ -375,11 +552,11 @@ func DefaultGracefulDegradationConfig() GracefulDegradationConfig { // NewGracefulDegradation creates a new graceful degradation manager func NewGracefulDegradation(config GracefulDegradationConfig, logger *Logger) *GracefulDegradation { gd := &GracefulDegradation{ - fallbacks: make(map[string]func() (any, error)), - healthChecks: make(map[string]func() bool), - degradedServices: make(map[string]time.Time), - config: config, - logger: logger, + BaseRecoveryMechanism: NewBaseRecoveryMechanism("graceful-degradation", logger), + fallbacks: make(map[string]func() (any, error)), + healthChecks: make(map[string]func() bool), + degradedServices: make(map[string]time.Time), + config: config, } // Start health check routine @@ -402,10 +579,29 @@ func (gd *GracefulDegradation) RegisterHealthCheck(serviceName string, healthChe gd.healthChecks[serviceName] = healthCheck } +// ExecuteWithContext implements the ErrorRecoveryMechanism interface +func (gd *GracefulDegradation) ExecuteWithContext(ctx context.Context, fn func() error) error { + gd.RecordRequest() + + // Execute with a simple wrapper + _, err := gd.ExecuteWithFallback("default", func() (any, error) { + return nil, fn() + }) + + if err != nil { + gd.RecordFailure() + } else { + gd.RecordSuccess() + } + + return err +} + // ExecuteWithFallback executes a function with fallback support func (gd *GracefulDegradation) ExecuteWithFallback(serviceName string, primary func() (any, error)) (any, error) { // Check if service is degraded if gd.isServiceDegraded(serviceName) { + gd.LogInfo("Service %s is degraded, using fallback", serviceName) return gd.executeFallback(serviceName) } @@ -414,9 +610,11 @@ func (gd *GracefulDegradation) ExecuteWithFallback(serviceName string, primary f if err != nil { // Mark service as degraded gd.markServiceDegraded(serviceName) + gd.LogError("Service %s failed: %v", serviceName, err) // Try fallback if available if gd.config.EnableFallbacks { + gd.LogInfo("Using fallback for service %s", serviceName) return gd.executeFallback(serviceName) } @@ -451,7 +649,7 @@ func (gd *GracefulDegradation) markServiceDegraded(serviceName string) { defer gd.mutex.Unlock() if _, exists := gd.degradedServices[serviceName]; !exists { - gd.logger.Errorf("Service %s marked as degraded", serviceName) + gd.LogError("Service %s marked as degraded", serviceName) } gd.degradedServices[serviceName] = time.Now() @@ -467,18 +665,19 @@ func (gd *GracefulDegradation) executeFallback(serviceName string) (any, error) return nil, fmt.Errorf("no fallback available for service %s", serviceName) } - gd.logger.Infof("Executing fallback for degraded service %s", serviceName) + gd.LogInfo("Executing fallback for degraded service %s", serviceName) return fallback() } // startHealthCheckRoutine starts the background health check routine func (gd *GracefulDegradation) startHealthCheckRoutine() { - ticker := time.NewTicker(gd.config.HealthCheckInterval) - defer ticker.Stop() - - for range ticker.C { - gd.performHealthChecks() - } + healthCheckTask := NewBackgroundTask( + "graceful-degradation-health-check", + gd.config.HealthCheckInterval, + gd.performHealthChecks, + gd.BaseRecoveryMechanism.logger, + ) + healthCheckTask.Start() } // performHealthChecks runs health checks for all registered services @@ -517,6 +716,52 @@ func (gd *GracefulDegradation) GetDegradedServices() []string { return degraded } +// Reset resets the state of all degraded services +func (gd *GracefulDegradation) Reset() { + gd.mutex.Lock() + defer gd.mutex.Unlock() + + // Clear degraded services + gd.degradedServices = make(map[string]time.Time) + gd.LogInfo("Graceful degradation state has been reset") +} + +// IsAvailable returns whether the mechanism is available for use +func (gd *GracefulDegradation) IsAvailable() bool { + return true +} + +// GetMetrics returns metrics about the graceful degradation mechanism +func (gd *GracefulDegradation) GetMetrics() map[string]any { + gd.mutex.RLock() + degradedCount := len(gd.degradedServices) + + // Get the names of degraded services + degradedServices := make([]string, 0, degradedCount) + for service := range gd.degradedServices { + degradedServices = append(degradedServices, service) + } + + // Get total count of registered fallbacks and health checks + fallbackCount := len(gd.fallbacks) + healthCheckCount := len(gd.healthChecks) + gd.mutex.RUnlock() + + // Get base metrics + metrics := gd.GetBaseMetrics() + + // Add graceful degradation specific metrics + metrics["degraded_services_count"] = degradedCount + metrics["degraded_services"] = degradedServices + metrics["registered_fallbacks_count"] = fallbackCount + metrics["registered_health_checks_count"] = healthCheckCount + metrics["health_check_interval_seconds"] = gd.config.HealthCheckInterval.Seconds() + metrics["recovery_timeout_seconds"] = gd.config.RecoveryTimeout.Seconds() + metrics["fallbacks_enabled"] = gd.config.EnableFallbacks + + return metrics +} + // ErrorRecoveryManager coordinates all error recovery mechanisms type ErrorRecoveryManager struct { circuitBreakers map[string]*CircuitBreaker diff --git a/metadata_cache.go b/metadata_cache.go index 895f1f2..3fee567 100644 --- a/metadata_cache.go +++ b/metadata_cache.go @@ -10,19 +10,29 @@ import ( type MetadataCache struct { expiresAt time.Time metadata *ProviderMetadata - stopCleanup chan struct{} + cleanupTask *BackgroundTask + logger *Logger autoCleanupInterval time.Duration mutex sync.RWMutex } // NewMetadataCache creates a new MetadataCache instance. -// It initializes the cache structure and starts the background cleanup goroutine. +// It initializes the cache structure and starts the background cleanup task. func NewMetadataCache() *MetadataCache { + return NewMetadataCacheWithLogger(nil) +} + +// NewMetadataCacheWithLogger creates a new MetadataCache with a specified logger. +func NewMetadataCacheWithLogger(logger *Logger) *MetadataCache { + if logger == nil { + logger = newNoOpLogger() + } + c := &MetadataCache{ autoCleanupInterval: 5 * time.Minute, - stopCleanup: make(chan struct{}), + logger: logger, } - go c.startAutoCleanup() + c.startAutoCleanup() return c } @@ -99,13 +109,17 @@ func (c *MetadataCache) GetMetadata(providerURL string, httpClient *http.Client, return metadata, nil } -// startAutoCleanup starts the background goroutine that periodically calls Cleanup +// startAutoCleanup starts the background task that periodically calls Cleanup // to remove expired metadata from the cache. func (c *MetadataCache) startAutoCleanup() { - autoCleanupRoutine(c.autoCleanupInterval, c.stopCleanup, c.Cleanup) + c.cleanupTask = NewBackgroundTask("metadata-cache-cleanup", c.autoCleanupInterval, c.Cleanup, c.logger) + c.cleanupTask.Start() } -// Close stops the automatic cleanup goroutine associated with this metadata cache. +// Close stops the automatic cleanup task associated with this metadata cache. func (c *MetadataCache) Close() { - close(c.stopCleanup) + if c.cleanupTask != nil { + c.cleanupTask.Stop() + c.cleanupTask = nil + } } diff --git a/metadata_cache_test.go b/metadata_cache_test.go index f9626ac..03bd40f 100644 --- a/metadata_cache_test.go +++ b/metadata_cache_test.go @@ -41,8 +41,8 @@ func TestGetMetadata_Cached(t *testing.T) { mc := &MetadataCache{ metadata: dummyData, expiresAt: time.Now().Add(1 * time.Hour), - stopCleanup: make(chan struct{}), autoCleanupInterval: 5 * time.Minute, + logger: newNoOpLogger(), } // Use NewLogger to create a logger that writes errors only. logger := NewLogger("error") @@ -58,10 +58,10 @@ func TestGetMetadata_Cached(t *testing.T) { func TestMetadataCacheAutoCleanup(t *testing.T) { mc := &MetadataCache{ autoCleanupInterval: 50 * time.Millisecond, - stopCleanup: make(chan struct{}), + logger: newNoOpLogger(), } // Start auto cleanup. - go mc.startAutoCleanup() + mc.startAutoCleanup() mc.mutex.Lock() mc.metadata = &ProviderMetadata{} mc.expiresAt = time.Now().Add(-50 * time.Millisecond) @@ -93,7 +93,7 @@ func TestGetMetadata_FetchError(t *testing.T) { // Case 1: Cache is empty. mc := &MetadataCache{ - stopCleanup: make(chan struct{}), + logger: newNoOpLogger(), } logger := NewLogger("error") metadata, err := mc.GetMetadata("http://example.com", errorClient, logger) diff --git a/performance_monitoring.go b/performance_monitoring.go deleted file mode 100644 index d499d3c..0000000 --- a/performance_monitoring.go +++ /dev/null @@ -1,676 +0,0 @@ -package traefikoidc - -import ( - "maps" - "runtime" - "sync" - "sync/atomic" - "time" -) - -// PerformanceMetrics tracks various performance-related metrics -type PerformanceMetrics struct { - startTime time.Time - logger *Logger - verificationTimes []time.Duration - refreshTimes []time.Duration - validationTimes []time.Duration - memoryUsage int64 - heapSize int64 - successfulVerifications int64 - successfulValidations int64 - successfulRefreshes int64 - failedVerifications int64 - failedValidations int64 - failedRefreshes int64 - avgVerificationTime time.Duration - avgValidationTime time.Duration - avgRefreshTime time.Duration - cacheHits int64 - goroutineCount int64 - memoryPressure int64 - gcPauseTime int64 - tokenRefreshes int64 - heapInUse int64 - verificationErrors int64 - validationErrors int64 - refreshErrors int64 - rateLimitedRequests int64 - activeSessions int64 - sessionCreations int64 - sessionDeletions int64 - cacheMisses int64 - tokenValidations int64 - tokenVerifications int64 - cacheSize int64 - cacheEvictions int64 - timingMutex sync.RWMutex -} - -// NewPerformanceMetrics creates a new performance metrics tracker -func NewPerformanceMetrics(logger *Logger) *PerformanceMetrics { - pm := &PerformanceMetrics{ - startTime: time.Now(), - verificationTimes: make([]time.Duration, 0, 1000), // Keep last 1000 measurements - validationTimes: make([]time.Duration, 0, 1000), - refreshTimes: make([]time.Duration, 0, 1000), - logger: logger, - } - - // Start background metrics collection - go pm.startMetricsCollection() - - return pm -} - -// RecordCacheHit records a cache hit -func (pm *PerformanceMetrics) RecordCacheHit() { - atomic.AddInt64(&pm.cacheHits, 1) -} - -// RecordCacheMiss records a cache miss -func (pm *PerformanceMetrics) RecordCacheMiss() { - atomic.AddInt64(&pm.cacheMisses, 1) -} - -// RecordCacheEviction records a cache eviction -func (pm *PerformanceMetrics) RecordCacheEviction() { - atomic.AddInt64(&pm.cacheEvictions, 1) -} - -// UpdateCacheSize updates the current cache size -func (pm *PerformanceMetrics) UpdateCacheSize(size int64) { - atomic.StoreInt64(&pm.cacheSize, size) -} - -// RecordTokenVerification records a token verification operation -func (pm *PerformanceMetrics) RecordTokenVerification(duration time.Duration, success bool) { - atomic.AddInt64(&pm.tokenVerifications, 1) - - if success { - atomic.AddInt64(&pm.successfulVerifications, 1) - pm.addVerificationTime(duration) - } else { - atomic.AddInt64(&pm.failedVerifications, 1) - atomic.AddInt64(&pm.verificationErrors, 1) - } -} - -// RecordTokenValidation records a token validation operation -func (pm *PerformanceMetrics) RecordTokenValidation(duration time.Duration, success bool) { - atomic.AddInt64(&pm.tokenValidations, 1) - - if success { - atomic.AddInt64(&pm.successfulValidations, 1) - pm.addValidationTime(duration) - } else { - atomic.AddInt64(&pm.failedValidations, 1) - atomic.AddInt64(&pm.validationErrors, 1) - } -} - -// RecordTokenRefresh records a token refresh operation -func (pm *PerformanceMetrics) RecordTokenRefresh(duration time.Duration, success bool) { - atomic.AddInt64(&pm.tokenRefreshes, 1) - - if success { - atomic.AddInt64(&pm.successfulRefreshes, 1) - pm.addRefreshTime(duration) - } else { - atomic.AddInt64(&pm.failedRefreshes, 1) - atomic.AddInt64(&pm.refreshErrors, 1) - } -} - -// RecordRateLimitedRequest records a rate-limited request -func (pm *PerformanceMetrics) RecordRateLimitedRequest() { - atomic.AddInt64(&pm.rateLimitedRequests, 1) -} - -// RecordSessionCreation records a session creation -func (pm *PerformanceMetrics) RecordSessionCreation() { - atomic.AddInt64(&pm.sessionCreations, 1) - atomic.AddInt64(&pm.activeSessions, 1) -} - -// RecordSessionDeletion records a session deletion -func (pm *PerformanceMetrics) RecordSessionDeletion() { - atomic.AddInt64(&pm.sessionDeletions, 1) - atomic.AddInt64(&pm.activeSessions, -1) -} - -// addVerificationTime adds a verification time measurement -func (pm *PerformanceMetrics) addVerificationTime(duration time.Duration) { - pm.timingMutex.Lock() - defer pm.timingMutex.Unlock() - - pm.verificationTimes = append(pm.verificationTimes, duration) - if len(pm.verificationTimes) > 1000 { - pm.verificationTimes = pm.verificationTimes[1:] - } - - pm.updateAverageVerificationTime() -} - -// addValidationTime adds a validation time measurement -func (pm *PerformanceMetrics) addValidationTime(duration time.Duration) { - pm.timingMutex.Lock() - defer pm.timingMutex.Unlock() - - pm.validationTimes = append(pm.validationTimes, duration) - if len(pm.validationTimes) > 1000 { - pm.validationTimes = pm.validationTimes[1:] - } - - pm.updateAverageValidationTime() -} - -// addRefreshTime adds a refresh time measurement -func (pm *PerformanceMetrics) addRefreshTime(duration time.Duration) { - pm.timingMutex.Lock() - defer pm.timingMutex.Unlock() - - pm.refreshTimes = append(pm.refreshTimes, duration) - if len(pm.refreshTimes) > 1000 { - pm.refreshTimes = pm.refreshTimes[1:] - } - - pm.updateAverageRefreshTime() -} - -// updateAverageVerificationTime calculates the average verification time -func (pm *PerformanceMetrics) updateAverageVerificationTime() { - if len(pm.verificationTimes) == 0 { - pm.avgVerificationTime = 0 - return - } - - var total time.Duration - for _, t := range pm.verificationTimes { - total += t - } - pm.avgVerificationTime = total / time.Duration(len(pm.verificationTimes)) -} - -// updateAverageValidationTime calculates the average validation time -func (pm *PerformanceMetrics) updateAverageValidationTime() { - if len(pm.validationTimes) == 0 { - pm.avgValidationTime = 0 - return - } - - var total time.Duration - for _, t := range pm.validationTimes { - total += t - } - pm.avgValidationTime = total / time.Duration(len(pm.validationTimes)) -} - -// updateAverageRefreshTime calculates the average refresh time -func (pm *PerformanceMetrics) updateAverageRefreshTime() { - if len(pm.refreshTimes) == 0 { - pm.avgRefreshTime = 0 - return - } - - var total time.Duration - for _, t := range pm.refreshTimes { - total += t - } - pm.avgRefreshTime = total / time.Duration(len(pm.refreshTimes)) -} - -// startMetricsCollection starts background collection of system metrics -func (pm *PerformanceMetrics) startMetricsCollection() { - ticker := time.NewTicker(30 * time.Second) - defer ticker.Stop() - - for range ticker.C { - pm.collectSystemMetrics() - } -} - -// collectSystemMetrics collects system-level metrics -func (pm *PerformanceMetrics) collectSystemMetrics() { - // Memory statistics - var m runtime.MemStats - runtime.ReadMemStats(&m) - atomic.StoreInt64(&pm.memoryUsage, int64(m.Alloc)) - atomic.StoreInt64(&pm.heapSize, int64(m.HeapSys)) - atomic.StoreInt64(&pm.heapInUse, int64(m.HeapInuse)) - atomic.StoreInt64(&pm.gcPauseTime, int64(m.PauseNs[(m.NumGC+255)%256])) - - // Calculate memory pressure (0-100 scale) - // Based on heap utilization and GC frequency - heapUtilization := float64(m.HeapInuse) / float64(m.HeapSys) - gcFrequency := float64(m.NumGC) / time.Since(pm.startTime).Minutes() - - // Memory pressure calculation - pressure := int64(heapUtilization * 50) // 0-50 based on heap utilization - if gcFrequency > 10 { // High GC frequency indicates pressure - pressure += int64((gcFrequency - 10) * 2) // Add up to 50 more - } - if pressure > 100 { - pressure = 100 - } - atomic.StoreInt64(&pm.memoryPressure, pressure) - - // Goroutine count - atomic.StoreInt64(&pm.goroutineCount, int64(runtime.NumGoroutine())) - - // Log memory pressure warnings - if pressure > 80 { - pm.logger.Errorf("High memory pressure detected: %d%% (heap utilization: %.1f%%, GC frequency: %.1f/min)", - pressure, heapUtilization*100, gcFrequency) - } else if pressure > 60 { - pm.logger.Infof("Moderate memory pressure: %d%% (heap utilization: %.1f%%, GC frequency: %.1f/min)", - pressure, heapUtilization*100, gcFrequency) - } -} - -// GetMetrics returns all current performance metrics -func (pm *PerformanceMetrics) GetMetrics() map[string]any { - pm.timingMutex.RLock() - defer pm.timingMutex.RUnlock() - - // Calculate cache hit ratio - hits := atomic.LoadInt64(&pm.cacheHits) - misses := atomic.LoadInt64(&pm.cacheMisses) - var hitRatio float64 - if hits+misses > 0 { - hitRatio = float64(hits) / float64(hits+misses) - } - - // Calculate error rates - verifications := atomic.LoadInt64(&pm.tokenVerifications) - validations := atomic.LoadInt64(&pm.tokenValidations) - refreshes := atomic.LoadInt64(&pm.tokenRefreshes) - - var verificationErrorRate, validationErrorRate, refreshErrorRate float64 - - if verifications > 0 { - verificationErrorRate = float64(atomic.LoadInt64(&pm.verificationErrors)) / float64(verifications) - } - if validations > 0 { - validationErrorRate = float64(atomic.LoadInt64(&pm.validationErrors)) / float64(validations) - } - if refreshes > 0 { - refreshErrorRate = float64(atomic.LoadInt64(&pm.refreshErrors)) / float64(refreshes) - } - - return map[string]any{ - // Cache metrics - "cache_hits": hits, - "cache_misses": misses, - "cache_hit_ratio": hitRatio, - "cache_evictions": atomic.LoadInt64(&pm.cacheEvictions), - "cache_size": atomic.LoadInt64(&pm.cacheSize), - - // Token operation metrics - "token_verifications": verifications, - "token_validations": validations, - "token_refreshes": refreshes, - "verification_error_rate": verificationErrorRate, - "validation_error_rate": validationErrorRate, - "refresh_error_rate": refreshErrorRate, - - // Success/failure metrics - "successful_verifications": atomic.LoadInt64(&pm.successfulVerifications), - "successful_validations": atomic.LoadInt64(&pm.successfulValidations), - "successful_refreshes": atomic.LoadInt64(&pm.successfulRefreshes), - "failed_verifications": atomic.LoadInt64(&pm.failedVerifications), - "failed_validations": atomic.LoadInt64(&pm.failedValidations), - "failed_refreshes": atomic.LoadInt64(&pm.failedRefreshes), - - // Timing metrics - "avg_verification_time_ms": pm.avgVerificationTime.Milliseconds(), - "avg_validation_time_ms": pm.avgValidationTime.Milliseconds(), - "avg_refresh_time_ms": pm.avgRefreshTime.Milliseconds(), - - // Resource metrics - "memory_usage_bytes": atomic.LoadInt64(&pm.memoryUsage), - "memory_pressure": atomic.LoadInt64(&pm.memoryPressure), - "heap_size_bytes": atomic.LoadInt64(&pm.heapSize), - "heap_inuse_bytes": atomic.LoadInt64(&pm.heapInUse), - "gc_pause_time_ns": atomic.LoadInt64(&pm.gcPauseTime), - "goroutine_count": atomic.LoadInt64(&pm.goroutineCount), - - // Rate limiting metrics - "rate_limited_requests": atomic.LoadInt64(&pm.rateLimitedRequests), - - // Session metrics - "active_sessions": atomic.LoadInt64(&pm.activeSessions), - "sessions_created": atomic.LoadInt64(&pm.sessionCreations), - "sessions_deleted": atomic.LoadInt64(&pm.sessionDeletions), - "session_creations": atomic.LoadInt64(&pm.sessionCreations), - "session_deletions": atomic.LoadInt64(&pm.sessionDeletions), - - // Uptime - "uptime_seconds": time.Since(pm.startTime).Seconds(), - } -} - -// GetDetailedTimingMetrics returns detailed timing statistics -func (pm *PerformanceMetrics) GetDetailedTimingMetrics() map[string]any { - pm.timingMutex.RLock() - defer pm.timingMutex.RUnlock() - - return map[string]any{ - "verification_stats": pm.calculateTimingStats(pm.verificationTimes), - "verification_timing": pm.calculateTimingStats(pm.verificationTimes), - "validation_stats": pm.calculateTimingStats(pm.validationTimes), - "validation_timing": pm.calculateTimingStats(pm.validationTimes), - "refresh_stats": pm.calculateTimingStats(pm.refreshTimes), - "refresh_timing": pm.calculateTimingStats(pm.refreshTimes), - } -} - -// calculateTimingStats calculates statistical metrics for timing data -func (pm *PerformanceMetrics) calculateTimingStats(times []time.Duration) map[string]any { - if len(times) == 0 { - return map[string]any{ - "count": 0, - "min_ms": float64(0), - "max_ms": float64(0), - "avg_ms": float64(0), - "average_ms": float64(0), - "median_ms": float64(0), - "p95_ms": float64(0), - "p99_ms": float64(0), - } - } - - // Sort times for percentile calculations - sortedTimes := make([]time.Duration, len(times)) - copy(sortedTimes, times) - - // Simple bubble sort for small arrays - for i := range sortedTimes { - for j := i + 1; j < len(sortedTimes); j++ { - if sortedTimes[i] > sortedTimes[j] { - sortedTimes[i], sortedTimes[j] = sortedTimes[j], sortedTimes[i] - } - } - } - - // Calculate statistics - min := sortedTimes[0] - max := sortedTimes[len(sortedTimes)-1] - - var total time.Duration - for _, t := range sortedTimes { - total += t - } - avg := total / time.Duration(len(sortedTimes)) - - median := sortedTimes[len(sortedTimes)/2] - p95 := sortedTimes[int(float64(len(sortedTimes))*0.95)] - p99 := sortedTimes[int(float64(len(sortedTimes))*0.99)] - - return map[string]any{ - "count": len(sortedTimes), - "min_ms": float64(min.Nanoseconds()) / 1e6, - "max_ms": float64(max.Nanoseconds()) / 1e6, - "avg_ms": float64(avg.Nanoseconds()) / 1e6, - "average_ms": float64(avg.Nanoseconds()) / 1e6, - "median_ms": float64(median.Nanoseconds()) / 1e6, - "p95_ms": float64(p95.Nanoseconds()) / 1e6, - "p99_ms": float64(p99.Nanoseconds()) / 1e6, - } -} - -// ResourceMonitor tracks resource usage and limits -type ResourceMonitor struct { - cacheSizes map[string]int64 - alertThresholds map[string]float64 - perfMetrics *PerformanceMetrics - logger *Logger - alerts []ResourceAlert - maxMemoryBytes int64 - maxCacheSize int64 - maxSessions int64 - cacheMutex sync.RWMutex - alertsMutex sync.RWMutex -} - -// ResourceAlert represents a resource usage alert -type ResourceAlert struct { - Timestamp time.Time `json:"timestamp"` - Type string `json:"type"` - Message string `json:"message"` - Severity string `json:"severity"` - Threshold float64 `json:"threshold"` - CurrentValue float64 `json:"current_value"` -} - -// NewResourceMonitor creates a new resource monitor -func NewResourceMonitor(perfMetrics *PerformanceMetrics, logger *Logger) *ResourceMonitor { - rm := &ResourceMonitor{ - maxMemoryBytes: 100 * 1024 * 1024, // 100MB default - maxCacheSize: 10000, // 10k items default - maxSessions: 1000, // 1k sessions default - cacheSizes: make(map[string]int64), - alertThresholds: map[string]float64{ - "memory_usage": 0.8, // 80% - "memory_pressure": 0.7, // 70% - "cache_usage": 0.9, // 90% - "session_usage": 0.85, // 85% - "error_rate": 0.1, // 10% - }, - alerts: make([]ResourceAlert, 0), - perfMetrics: perfMetrics, - logger: logger, - } - - // Start monitoring routine - go rm.startMonitoring() - - return rm -} - -// SetMemoryLimit sets the maximum memory usage limit -func (rm *ResourceMonitor) SetMemoryLimit(bytes int64) { - rm.maxMemoryBytes = bytes -} - -// SetCacheLimit sets the maximum cache size limit -func (rm *ResourceMonitor) SetCacheLimit(size int64) { - rm.maxCacheSize = size -} - -// SetSessionLimit sets the maximum session count limit -func (rm *ResourceMonitor) SetSessionLimit(count int64) { - rm.maxSessions = count -} - -// UpdateCacheSize updates the size of a specific cache -func (rm *ResourceMonitor) UpdateCacheSize(cacheName string, size int64) { - rm.cacheMutex.Lock() - defer rm.cacheMutex.Unlock() - rm.cacheSizes[cacheName] = size -} - -// GetCacheSizes returns current cache sizes -func (rm *ResourceMonitor) GetCacheSizes() map[string]int64 { - rm.cacheMutex.RLock() - defer rm.cacheMutex.RUnlock() - - sizes := make(map[string]int64) - maps.Copy(sizes, rm.cacheSizes) - return sizes -} - -// startMonitoring starts the background monitoring routine -func (rm *ResourceMonitor) startMonitoring() { - ticker := time.NewTicker(10 * time.Second) - defer ticker.Stop() - - for range ticker.C { - rm.checkResourceUsage() - } -} - -// checkResourceUsage checks current resource usage against limits -func (rm *ResourceMonitor) checkResourceUsage() { - metrics := rm.perfMetrics.GetMetrics() - - // Check memory usage - if memUsage, ok := metrics["memory_usage_bytes"].(int64); ok { - memUsageRatio := float64(memUsage) / float64(rm.maxMemoryBytes) - if memUsageRatio > rm.alertThresholds["memory_usage"] { - rm.addAlert(ResourceAlert{ - Type: "memory_usage", - Message: "Memory usage exceeds threshold", - Threshold: rm.alertThresholds["memory_usage"], - CurrentValue: memUsageRatio, - Timestamp: time.Now(), - Severity: rm.getSeverity(memUsageRatio, rm.alertThresholds["memory_usage"]), - }) - } - } - - // Check memory pressure - if memPressure, ok := metrics["memory_pressure"].(int64); ok { - pressureRatio := float64(memPressure) / 100.0 // Convert to 0-1 scale - if pressureRatio > rm.alertThresholds["memory_pressure"] { - rm.addAlert(ResourceAlert{ - Type: "memory_pressure", - Message: "Memory pressure exceeds threshold", - Threshold: rm.alertThresholds["memory_pressure"], - CurrentValue: pressureRatio, - Timestamp: time.Now(), - Severity: rm.getSeverity(pressureRatio, rm.alertThresholds["memory_pressure"]), - }) - } - } - - // Check cache usage - if cacheSize, ok := metrics["cache_size"].(int64); ok { - cacheUsageRatio := float64(cacheSize) / float64(rm.maxCacheSize) - if cacheUsageRatio > rm.alertThresholds["cache_usage"] { - rm.addAlert(ResourceAlert{ - Type: "cache_usage", - Message: "Cache usage exceeds threshold", - Threshold: rm.alertThresholds["cache_usage"], - CurrentValue: cacheUsageRatio, - Timestamp: time.Now(), - Severity: rm.getSeverity(cacheUsageRatio, rm.alertThresholds["cache_usage"]), - }) - } - } - - // Check session usage - if activeSessions, ok := metrics["active_sessions"].(int64); ok { - sessionUsageRatio := float64(activeSessions) / float64(rm.maxSessions) - if sessionUsageRatio > rm.alertThresholds["session_usage"] { - rm.addAlert(ResourceAlert{ - Type: "session_usage", - Message: "Active session count exceeds threshold", - Threshold: rm.alertThresholds["session_usage"], - CurrentValue: sessionUsageRatio, - Timestamp: time.Now(), - Severity: rm.getSeverity(sessionUsageRatio, rm.alertThresholds["session_usage"]), - }) - } - } - - // Check error rates - if errorRate, ok := metrics["verification_error_rate"].(float64); ok { - if errorRate > rm.alertThresholds["error_rate"] { - rm.addAlert(ResourceAlert{ - Type: "verification_error_rate", - Message: "Token verification error rate exceeds threshold", - Threshold: rm.alertThresholds["error_rate"], - CurrentValue: errorRate, - Timestamp: time.Now(), - Severity: rm.getSeverity(errorRate, rm.alertThresholds["error_rate"]), - }) - } - } -} - -// getSeverity determines the severity level based on how much the threshold is exceeded -func (rm *ResourceMonitor) getSeverity(currentValue, threshold float64) string { - ratio := currentValue / threshold - if ratio >= 1.5 { - return "critical" - } else if ratio >= 1.2 { - return "high" - } else if ratio >= 1.0 { - return "medium" - } - return "low" -} - -// addAlert adds a new resource alert -func (rm *ResourceMonitor) addAlert(alert ResourceAlert) { - rm.alertsMutex.Lock() - defer rm.alertsMutex.Unlock() - - // Add alert - rm.alerts = append(rm.alerts, alert) - - // Keep only last 100 alerts - if len(rm.alerts) > 100 { - rm.alerts = rm.alerts[1:] - } - - // Log the alert - rm.logger.Errorf("Resource Alert [%s/%s]: %s (%.2f%% > %.2f%%)", - alert.Type, alert.Severity, alert.Message, - alert.CurrentValue*100, alert.Threshold*100) -} - -// GetAlerts returns current resource alerts -func (rm *ResourceMonitor) GetAlerts() []ResourceAlert { - rm.alertsMutex.RLock() - defer rm.alertsMutex.RUnlock() - - alerts := make([]ResourceAlert, len(rm.alerts)) - copy(alerts, rm.alerts) - return alerts -} - -// GetResourceStatus returns current resource status -func (rm *ResourceMonitor) GetResourceStatus() map[string]any { - metrics := rm.perfMetrics.GetMetrics() - cacheSizes := rm.GetCacheSizes() - - status := map[string]any{ - "limits": map[string]any{ - "max_memory_bytes": rm.maxMemoryBytes, - "max_cache_size": rm.maxCacheSize, - "max_sessions": rm.maxSessions, - }, - "thresholds": rm.alertThresholds, - "current": metrics, - "cache_sizes": cacheSizes, - // Add expected keys for tests - "memory_limit": uint64(rm.maxMemoryBytes), - "cache_limit": int(rm.maxCacheSize), - "session_limit": int(rm.maxSessions), - } - - // Calculate usage ratios - if memUsage, ok := metrics["memory_usage_bytes"].(int64); ok { - status["memory_usage_ratio"] = float64(memUsage) / float64(rm.maxMemoryBytes) - } - if memPressure, ok := metrics["memory_pressure"].(int64); ok { - status["memory_pressure_ratio"] = float64(memPressure) / 100.0 - } - if cacheSize, ok := metrics["cache_size"].(int64); ok { - status["cache_usage_ratio"] = float64(cacheSize) / float64(rm.maxCacheSize) - } - if activeSessions, ok := metrics["active_sessions"].(int64); ok { - status["session_usage_ratio"] = float64(activeSessions) / float64(rm.maxSessions) - } - - // Calculate total cache size across all caches - var totalCacheSize int64 - for _, size := range cacheSizes { - totalCacheSize += size - } - status["total_cache_size"] = totalCacheSize - - return status -} diff --git a/performance_monitoring_test.go b/performance_monitoring_test.go deleted file mode 100644 index 05ab4f3..0000000 --- a/performance_monitoring_test.go +++ /dev/null @@ -1,324 +0,0 @@ -package traefikoidc - -import ( - "testing" - "time" -) - -func TestPerformanceMetrics(t *testing.T) { - logger := NewLogger("debug") - metrics := NewPerformanceMetrics(logger) - - t.Run("Record cache operations", func(t *testing.T) { - metrics.RecordCacheHit() - metrics.RecordCacheMiss() - metrics.RecordCacheEviction() - metrics.UpdateCacheSize(100) - - result := metrics.GetMetrics() - - if result["cache_hits"].(int64) != 1 { - t.Errorf("Expected 1 cache hit, got %v", result["cache_hits"]) - } - if result["cache_misses"].(int64) != 1 { - t.Errorf("Expected 1 cache miss, got %v", result["cache_misses"]) - } - if result["cache_evictions"].(int64) != 1 { - t.Errorf("Expected 1 cache eviction, got %v", result["cache_evictions"]) - } - if result["cache_size"].(int64) != 100 { - t.Errorf("Expected cache size 100, got %v", result["cache_size"]) - } - }) - - t.Run("Record token operations", func(t *testing.T) { - start := time.Now() - time.Sleep(10 * time.Millisecond) - metrics.RecordTokenVerification(time.Since(start), true) - - start = time.Now() - time.Sleep(5 * time.Millisecond) - metrics.RecordTokenValidation(time.Since(start), false) - - start = time.Now() - time.Sleep(15 * time.Millisecond) - metrics.RecordTokenRefresh(time.Since(start), true) - - result := metrics.GetMetrics() - - if result["token_verifications"].(int64) != 1 { - t.Errorf("Expected 1 token verification, got %v", result["token_verifications"]) - } - if result["token_validations"].(int64) != 1 { - t.Errorf("Expected 1 token validation, got %v", result["token_validations"]) - } - if result["token_refreshes"].(int64) != 1 { - t.Errorf("Expected 1 token refresh, got %v", result["token_refreshes"]) - } - if result["successful_verifications"].(int64) != 1 { - t.Errorf("Expected 1 successful verification, got %v", result["successful_verifications"]) - } - if result["failed_validations"].(int64) != 1 { - t.Errorf("Expected 1 failed validation, got %v", result["failed_validations"]) - } - }) - - t.Run("Record rate limiting and sessions", func(t *testing.T) { - metrics.RecordRateLimitedRequest() - metrics.RecordSessionCreation() - metrics.RecordSessionDeletion() - - result := metrics.GetMetrics() - - if result["rate_limited_requests"].(int64) != 1 { - t.Errorf("Expected 1 rate limited request, got %v", result["rate_limited_requests"]) - } - if result["sessions_created"].(int64) != 1 { - t.Errorf("Expected 1 session created, got %v", result["sessions_created"]) - } - if result["sessions_deleted"].(int64) != 1 { - t.Errorf("Expected 1 session deleted, got %v", result["sessions_deleted"]) - } - }) - - t.Run("Get detailed timing metrics", func(t *testing.T) { - // Add more timing data - for i := range 5 { - metrics.RecordTokenVerification(time.Duration(i+1)*time.Millisecond, true) - } - - detailed := metrics.GetDetailedTimingMetrics() - - if detailed["verification_stats"] == nil { - t.Error("Expected verification stats to be present") - } - - verificationStats := detailed["verification_stats"].(map[string]any) - if verificationStats["count"].(int) != 6 { // 1 from previous test + 5 new - t.Errorf("Expected 6 verifications, got %v", verificationStats["count"]) - } - }) -} - -func TestResourceMonitor(t *testing.T) { - logger := NewLogger("debug") - metrics := NewPerformanceMetrics(logger) - monitor := NewResourceMonitor(metrics, logger) - - t.Run("Set limits", func(t *testing.T) { - monitor.SetMemoryLimit(100 * 1024 * 1024) // 100MB - monitor.SetCacheLimit(1000) - monitor.SetSessionLimit(500) - - // Should not panic - }) - - t.Run("Get resource status", func(t *testing.T) { - status := monitor.GetResourceStatus() - - if status["memory_limit"] == nil { - t.Error("Expected memory limit to be set") - } - if status["cache_limit"] == nil { - t.Error("Expected cache limit to be set") - } - if status["session_limit"] == nil { - t.Error("Expected session limit to be set") - } - }) - - t.Run("Get alerts", func(t *testing.T) { - alerts := monitor.GetAlerts() - - // Should return empty slice initially - if alerts == nil { - t.Error("Expected alerts slice to be initialized") - } - }) -} - -func TestPerformanceMetricsCalculations(t *testing.T) { - logger := NewLogger("debug") - metrics := NewPerformanceMetrics(logger) - - t.Run("Average calculation", func(t *testing.T) { - // Record multiple operations with known durations - durations := []time.Duration{ - 10 * time.Millisecond, - 20 * time.Millisecond, - 30 * time.Millisecond, - } - - for _, d := range durations { - metrics.RecordTokenVerification(d, true) - } - - detailed := metrics.GetDetailedTimingMetrics() - verificationStats := detailed["verification_stats"].(map[string]any) - - // Average should be 20ms - avgMs := verificationStats["average_ms"].(float64) - if avgMs < 19 || avgMs > 21 { // Allow small variance - t.Errorf("Expected average around 20ms, got %f", avgMs) - } - }) - - t.Run("Min/Max calculation", func(t *testing.T) { - logger := NewLogger("debug") - metrics := NewPerformanceMetrics(logger) // Fresh instance - - durations := []time.Duration{ - 5 * time.Millisecond, - 50 * time.Millisecond, - 25 * time.Millisecond, - } - - for _, d := range durations { - metrics.RecordTokenVerification(d, true) - } - - detailed := metrics.GetDetailedTimingMetrics() - verificationStats := detailed["verification_stats"].(map[string]any) - - minMs := verificationStats["min_ms"].(float64) - maxMs := verificationStats["max_ms"].(float64) - - if minMs < 4 || minMs > 6 { - t.Errorf("Expected min around 5ms, got %f", minMs) - } - if maxMs < 49 || maxMs > 51 { - t.Errorf("Expected max around 50ms, got %f", maxMs) - } - }) -} - -func TestPerformanceMetricsReset(t *testing.T) { - logger := NewLogger("debug") - metrics := NewPerformanceMetrics(logger) - - // Record some data - metrics.RecordCacheHit() - metrics.RecordTokenVerification(10*time.Millisecond, true) - - // Verify data is there - result := metrics.GetMetrics() - if result["cache_hits"].(int64) != 1 { - t.Error("Expected cache hit to be recorded") - } - - // Note: The current implementation doesn't have a reset method, - // but we can test that metrics accumulate correctly - metrics.RecordCacheHit() - result = metrics.GetMetrics() - if result["cache_hits"].(int64) != 2 { - t.Error("Expected cache hits to accumulate") - } -} - -func TestPerformanceMetricsConcurrency(t *testing.T) { - logger := NewLogger("debug") - metrics := NewPerformanceMetrics(logger) - - // Test concurrent access - done := make(chan bool, 10) - - for range 10 { - go func() { - defer func() { done <- true }() - - for range 100 { - metrics.RecordCacheHit() - metrics.RecordTokenVerification(time.Millisecond, true) - } - }() - } - - // Wait for all goroutines to complete - for range 10 { - <-done - } - - result := metrics.GetMetrics() - - // Should have 1000 cache hits (10 goroutines * 100 operations) - if result["cache_hits"].(int64) != 1000 { - t.Errorf("Expected 1000 cache hits, got %v", result["cache_hits"]) - } - - // Should have 1000 token verifications - if result["token_verifications"].(int64) != 1000 { - t.Errorf("Expected 1000 token verifications, got %v", result["token_verifications"]) - } -} - -func TestResourceMonitorLimits(t *testing.T) { - logger := NewLogger("debug") - metrics := NewPerformanceMetrics(logger) - monitor := NewResourceMonitor(metrics, logger) - - t.Run("Memory limit validation", func(t *testing.T) { - // Set a reasonable memory limit - monitor.SetMemoryLimit(50 * 1024 * 1024) // 50MB - - status := monitor.GetResourceStatus() - if status["memory_limit"].(uint64) != 50*1024*1024 { - t.Error("Memory limit not set correctly") - } - }) - - t.Run("Cache limit validation", func(t *testing.T) { - monitor.SetCacheLimit(2000) - - status := monitor.GetResourceStatus() - if status["cache_limit"].(int) != 2000 { - t.Error("Cache limit not set correctly") - } - }) - - t.Run("Session limit validation", func(t *testing.T) { - monitor.SetSessionLimit(1000) - - status := monitor.GetResourceStatus() - if status["session_limit"].(int) != 1000 { - t.Error("Session limit not set correctly") - } - }) -} - -func TestPerformanceMetricsEdgeCases(t *testing.T) { - logger := NewLogger("debug") - metrics := NewPerformanceMetrics(logger) - - t.Run("Zero duration handling", func(t *testing.T) { - metrics.RecordTokenVerification(0, true) - - result := metrics.GetMetrics() - if result["token_verifications"].(int64) != 1 { - t.Error("Should record verification even with zero duration") - } - }) - - t.Run("Very large duration handling", func(t *testing.T) { - largeDuration := time.Hour - metrics.RecordTokenVerification(largeDuration, true) - - detailed := metrics.GetDetailedTimingMetrics() - verificationStats := detailed["verification_stats"].(map[string]any) - - // Should handle large durations without overflow - if verificationStats["max_ms"].(float64) <= 0 { - t.Error("Should handle large durations correctly") - } - }) - - t.Run("Negative cache size handling", func(t *testing.T) { - // This shouldn't happen in practice, but test robustness - metrics.UpdateCacheSize(-1) - - result := metrics.GetMetrics() - // Implementation should handle this gracefully - if result["cache_size"] == nil { - t.Error("Cache size should be present even if negative") - } - }) -} diff --git a/robustness_test.go b/robustness_test.go index 15c3410..fd8497e 100644 --- a/robustness_test.go +++ b/robustness_test.go @@ -651,11 +651,8 @@ func TestErrorRecoveryPatterns(t *testing.T) { // Test recovery from cache corruption t.Run("CacheCorruption", func(t *testing.T) { - // Corrupt the cache by setting invalid data - ts.tOidc.tokenCache.cache.items["corrupted"] = CacheItem{ - Value: "invalid-data", - ExpiresAt: time.Now().Add(time.Hour), - } + // Corrupt the cache by using the Set method to avoid data race + ts.tOidc.tokenCache.cache.Set("corrupted", "invalid-data", time.Hour) // System should handle corrupted cache gracefully validToken, err := createTestJWT(ts.rsaPrivateKey, "RS256", "test-key-id", map[string]any{ diff --git a/security_monitoring.go b/security_monitoring.go index 512c1ca..e8df0d7 100644 --- a/security_monitoring.go +++ b/security_monitoring.go @@ -2,15 +2,57 @@ package traefikoidc import ( "fmt" - "maps" "net" "net/http" "strings" "sync" - "sync/atomic" "time" ) +// SecurityEventType represents different types of security events +type SecurityEventType string + +const ( + // AuthFailure represents an authentication failure event + AuthFailure SecurityEventType = "authentication_failure" + // TokenValidFailure represents a token validation failure event + TokenValidFailure SecurityEventType = "token_validation_failure" + // RateLimitHit represents a rate limit hit event + RateLimitHit SecurityEventType = "rate_limit_hit" + // SuspiciousActivity represents a suspicious activity event + SuspiciousActivity SecurityEventType = "suspicious_activity" +) + +// DefaultSeverity returns the default severity level for a security event type +func (t SecurityEventType) DefaultSeverity() string { + switch t { + case AuthFailure: + return "medium" + case TokenValidFailure: + return "medium" + case RateLimitHit: + return "low" + case SuspiciousActivity: + return "high" + default: + return "medium" + } +} + +// IPFailureType returns the IP failure tracking type for a security event type +func (t SecurityEventType) IPFailureType() string { + switch t { + case AuthFailure: + return "auth_failure" + case TokenValidFailure: + return "token_failure" + case SuspiciousActivity: + return "suspicious" + default: + return "general" + } +} + // SecurityEvent represents a security-related event that should be logged and monitored type SecurityEvent struct { Timestamp time.Time `json:"timestamp"` @@ -25,16 +67,12 @@ type SecurityEvent struct { // SecurityMonitor tracks security events and suspicious activity patterns type SecurityMonitor struct { - ipFailures map[string]*IPFailureTracker - patternDetector *SuspiciousPatternDetector - logger *Logger - eventHandlers []SecurityEventHandler - config SecurityMonitorConfig - authFailures int64 - tokenValidationFails int64 - rateLimitHits int64 - suspiciousRequests int64 - ipMutex sync.RWMutex + ipFailures map[string]*IPFailureTracker + patternDetector *SuspiciousPatternDetector + logger *Logger + eventHandlers []SecurityEventHandler + config SecurityMonitorConfig + ipMutex sync.RWMutex } // IPFailureTracker tracks failures for a specific IP address @@ -93,6 +131,9 @@ func DefaultSecurityMonitorConfig() SecurityMonitorConfig { } } +// cleanupTask holds the BackgroundTask for security cleanup +var cleanupTask *BackgroundTask + // NewSecurityMonitor creates a new security monitor instance func NewSecurityMonitor(config SecurityMonitorConfig, logger *Logger) *SecurityMonitor { sm := &SecurityMonitor{ @@ -104,7 +145,7 @@ func NewSecurityMonitor(config SecurityMonitorConfig, logger *Logger) *SecurityM } // Start cleanup routine - go sm.startCleanupRoutine() + sm.startCleanupRoutine() return sm } @@ -122,29 +163,55 @@ func NewSuspiciousPatternDetector() *SuspiciousPatternDetector { } } -// RecordAuthenticationFailure records an authentication failure event -func (sm *SecurityMonitor) RecordAuthenticationFailure(clientIP, userAgent, requestPath, reason string, details map[string]any) { - atomic.AddInt64(&sm.authFailures, 1) +// RecordSecurityEvent is a generic method to record any type of security event +func (sm *SecurityMonitor) RecordSecurityEvent( + eventType SecurityEventType, + clientIP, userAgent, requestPath string, + message string, + details map[string]any, + trackIPFailure bool) { + // Create event with default values for the event type event := SecurityEvent{ - Type: "authentication_failure", - Severity: "medium", + Type: string(eventType), + Severity: eventType.DefaultSeverity(), Timestamp: time.Now(), ClientIP: clientIP, UserAgent: userAgent, RequestPath: requestPath, - Message: fmt.Sprintf("Authentication failed: %s", reason), + Message: message, Details: details, } - sm.recordIPFailure(clientIP, "auth_failure") + // Track IP failures if requested + if trackIPFailure { + sm.recordIPFailure(clientIP, eventType.IPFailureType()) + } + + // Process the event sm.processSecurityEvent(event) } +// RecordAuthenticationFailure records an authentication failure event +func (sm *SecurityMonitor) RecordAuthenticationFailure(clientIP, userAgent, requestPath, reason string, details map[string]any) { + if details == nil { + details = make(map[string]any) + } + details["reason"] = reason + + sm.RecordSecurityEvent( + AuthFailure, + clientIP, + userAgent, + requestPath, + fmt.Sprintf("Authentication failed: %s", reason), + details, + true, + ) +} + // RecordTokenValidationFailure records a token validation failure func (sm *SecurityMonitor) RecordTokenValidationFailure(clientIP, userAgent, requestPath, reason string, tokenPrefix string) { - atomic.AddInt64(&sm.tokenValidationFails, 1) - details := map[string]any{ "reason": reason, } @@ -152,59 +219,50 @@ func (sm *SecurityMonitor) RecordTokenValidationFailure(clientIP, userAgent, req details["token_prefix"] = tokenPrefix } - event := SecurityEvent{ - Type: "token_validation_failure", - Severity: "medium", - Timestamp: time.Now(), - ClientIP: clientIP, - UserAgent: userAgent, - RequestPath: requestPath, - Message: fmt.Sprintf("Token validation failed: %s", reason), - Details: details, - } - - sm.recordIPFailure(clientIP, "token_failure") - sm.processSecurityEvent(event) + sm.RecordSecurityEvent( + TokenValidFailure, + clientIP, + userAgent, + requestPath, + fmt.Sprintf("Token validation failed: %s", reason), + details, + true, + ) } // RecordRateLimitHit records when rate limiting is triggered func (sm *SecurityMonitor) RecordRateLimitHit(clientIP, userAgent, requestPath string) { - atomic.AddInt64(&sm.rateLimitHits, 1) - - event := SecurityEvent{ - Type: "rate_limit_hit", - Severity: "low", - Timestamp: time.Now(), - ClientIP: clientIP, - UserAgent: userAgent, - RequestPath: requestPath, - Message: "Rate limit exceeded", - Details: map[string]any{ - "limit_type": "token_verification", - }, + details := map[string]any{ + "limit_type": "token_verification", } - sm.recordIPFailure(clientIP, "rate_limit") - sm.processSecurityEvent(event) + sm.RecordSecurityEvent( + RateLimitHit, + clientIP, + userAgent, + requestPath, + "Rate limit exceeded", + details, + true, // Track IP failure for rate limiting + ) } // RecordSuspiciousActivity records suspicious activity that doesn't fit other categories func (sm *SecurityMonitor) RecordSuspiciousActivity(clientIP, userAgent, requestPath, activityType, description string, details map[string]any) { - atomic.AddInt64(&sm.suspiciousRequests, 1) - - event := SecurityEvent{ - Type: "suspicious_activity", - Severity: "high", - Timestamp: time.Now(), - ClientIP: clientIP, - UserAgent: userAgent, - RequestPath: requestPath, - Message: fmt.Sprintf("Suspicious activity detected: %s - %s", activityType, description), - Details: details, + if details == nil { + details = make(map[string]any) } + details["activity_type"] = activityType - sm.recordIPFailure(clientIP, "suspicious") - sm.processSecurityEvent(event) + sm.RecordSecurityEvent( + SuspiciousActivity, + clientIP, + userAgent, + requestPath, + fmt.Sprintf("Suspicious activity detected: %s - %s", activityType, description), + details, + true, + ) } // recordIPFailure tracks failures for a specific IP address @@ -329,30 +387,11 @@ func (sm *SecurityMonitor) AddEventHandler(handler SecurityEventHandler) { sm.eventHandlers = append(sm.eventHandlers, handler) } -// GetSecurityMetrics returns current security metrics +// GetSecurityMetrics returns minimal security metrics +// This is kept for API compatibility but doesn't collect actual metrics func (sm *SecurityMonitor) GetSecurityMetrics() map[string]any { - sm.ipMutex.RLock() - defer sm.ipMutex.RUnlock() - - blockedIPs := 0 - totalTrackedIPs := len(sm.ipFailures) - - for _, tracker := range sm.ipFailures { - tracker.mutex.RLock() - if tracker.IsBlocked && time.Now().Before(tracker.BlockedUntil) { - blockedIPs++ - } - tracker.mutex.RUnlock() - } - return map[string]any{ - "auth_failures": atomic.LoadInt64(&sm.authFailures), - "token_validation_fails": atomic.LoadInt64(&sm.tokenValidationFails), - "rate_limit_hits": atomic.LoadInt64(&sm.rateLimitHits), - "suspicious_requests": atomic.LoadInt64(&sm.suspiciousRequests), - "blocked_ips": blockedIPs, - "tracked_ips": totalTrackedIPs, - "uptime_hours": time.Since(time.Now().Add(-24 * time.Hour)).Hours(), // Placeholder + "tracked_ips": 0, } } @@ -434,11 +473,20 @@ func (spd *SuspiciousPatternDetector) DetectSuspiciousPatterns() []string { // startCleanupRoutine starts the background cleanup routine func (sm *SecurityMonitor) startCleanupRoutine() { - ticker := time.NewTicker(time.Duration(sm.config.CleanupIntervalMinutes) * time.Minute) - defer ticker.Stop() + // Use BackgroundTask abstraction for consistent management + cleanupTask = NewBackgroundTask( + "security-monitor-cleanup", + time.Duration(sm.config.CleanupIntervalMinutes)*time.Minute, + sm.cleanup, + sm.logger) + cleanupTask.Start() +} - for range ticker.C { - sm.cleanup() +// StopCleanupRoutine stops the background cleanup routine +func (sm *SecurityMonitor) StopCleanupRoutine() { + if cleanupTask != nil { + cleanupTask.Stop() + cleanupTask = nil } } @@ -515,34 +563,4 @@ func (h *LoggingSecurityEventHandler) HandleSecurityEvent(event SecurityEvent) { } } -// MetricsSecurityEventHandler tracks security metrics -type MetricsSecurityEventHandler struct { - eventCounts map[string]int64 - mutex sync.RWMutex -} - -// NewMetricsSecurityEventHandler creates a new metrics event handler -func NewMetricsSecurityEventHandler() *MetricsSecurityEventHandler { - return &MetricsSecurityEventHandler{ - eventCounts: make(map[string]int64), - } -} - -// HandleSecurityEvent implements SecurityEventHandler -func (h *MetricsSecurityEventHandler) HandleSecurityEvent(event SecurityEvent) { - h.mutex.Lock() - defer h.mutex.Unlock() - - h.eventCounts[event.Type]++ - h.eventCounts[fmt.Sprintf("%s_%s", event.Type, event.Severity)]++ -} - -// GetMetrics returns the current metrics -func (h *MetricsSecurityEventHandler) GetMetrics() map[string]int64 { - h.mutex.RLock() - defer h.mutex.RUnlock() - - metrics := make(map[string]int64) - maps.Copy(metrics, h.eventCounts) - return metrics -} +// Note: MetricsSecurityEventHandler has been removed as part of metrics cleanup diff --git a/security_monitoring_test.go b/security_monitoring_test.go index e3af06a..a286624 100644 --- a/security_monitoring_test.go +++ b/security_monitoring_test.go @@ -43,42 +43,19 @@ func TestSecurityMonitor(t *testing.T) { }) t.Run("Token validation failure", func(t *testing.T) { + // Just verify the method doesn't panic monitor.RecordTokenValidationFailure("192.168.1.3", "test-agent", "/api", "invalid token", "abc123") - - metrics := monitor.GetSecurityMetrics() - if metrics["token_validation_fails"].(int64) == 0 { - t.Error("Expected token validation failures to be recorded") - } }) t.Run("Rate limit hit", func(t *testing.T) { + // Just verify the method doesn't panic monitor.RecordRateLimitHit("192.168.1.4", "test-agent", "/api") - - metrics := monitor.GetSecurityMetrics() - if metrics["rate_limit_hits"].(int64) == 0 { - t.Error("Expected rate limit hits to be recorded") - } }) t.Run("Suspicious activity", func(t *testing.T) { details := map[string]any{"pattern": "unusual"} + // Just verify the method doesn't panic monitor.RecordSuspiciousActivity("192.168.1.5", "test-agent", "/admin", "unusual pattern", "high frequency requests", details) - - metrics := monitor.GetSecurityMetrics() - if metrics["suspicious_requests"].(int64) == 0 { - t.Error("Expected suspicious activities to be recorded") - } - }) - - t.Run("Get security metrics", func(t *testing.T) { - metrics := monitor.GetSecurityMetrics() - - if metrics["auth_failures"].(int64) == 0 { - t.Error("Expected some authentication failures") - } - if metrics["blocked_ips"] == nil { - t.Error("Expected blocked IPs count to be present") - } }) } @@ -193,24 +170,7 @@ func TestSecurityEventHandlers(t *testing.T) { handler.HandleSecurityEvent(event) }) - t.Run("Metrics security event handler", func(t *testing.T) { - handler := NewMetricsSecurityEventHandler() - - event := SecurityEvent{ - Type: "authentication_failure", - ClientIP: "192.168.1.1", - Timestamp: time.Now(), - Message: "Test failure", - Severity: "medium", - } - - handler.HandleSecurityEvent(event) - - metrics := handler.GetMetrics() - if metrics["authentication_failure"] != 1 { - t.Errorf("Expected 1 authentication failure, got %v", metrics["authentication_failure"]) - } - }) + // Metrics security event handler test removed as part of metrics cleanup } func TestSecurityMonitorEventHandlers(t *testing.T) { @@ -301,7 +261,7 @@ func TestSecurityEventTypes(t *testing.T) { logger := NewLogger("debug") monitor := NewSecurityMonitor(config, logger) - // Test different event types + // Test different event types - just verify they don't panic monitor.RecordAuthenticationFailure("192.168.1.200", "test-agent", "/login", "invalid password", nil) monitor.RecordTokenValidationFailure("192.168.1.200", "test-agent", "/api", "expired token", "abc123") monitor.RecordRateLimitHit("192.168.1.200", "test-agent", "/api") @@ -309,18 +269,6 @@ func TestSecurityEventTypes(t *testing.T) { details := map[string]any{"pattern": "test"} monitor.RecordSuspiciousActivity("192.168.1.200", "test-agent", "/admin", "unusual pattern", "multiple failed logins", details) - metrics := monitor.GetSecurityMetrics() - - if metrics["auth_failures"].(int64) == 0 { - t.Error("Expected authentication failures to be recorded") - } - if metrics["token_validation_fails"].(int64) == 0 { - t.Error("Expected token validation failures to be recorded") - } - if metrics["rate_limit_hits"].(int64) == 0 { - t.Error("Expected rate limit hits to be recorded") - } - if metrics["suspicious_requests"].(int64) == 0 { - t.Error("Expected suspicious activities to be recorded") - } + // Just verify GetSecurityMetrics doesn't panic + _ = monitor.GetSecurityMetrics() } diff --git a/session.go b/session.go index 3957936..6526d20 100644 --- a/session.go +++ b/session.go @@ -508,6 +508,62 @@ func (sd *SessionData) Save(r *http.Request, w http.ResponseWriter) error { return firstErr } +// clearSessionValues clears all values from a session and optionally sets MaxAge to -1 to expire the cookie +// Parameters: +// - session: The session to clear +// - expire: If true, sets MaxAge to -1 to expire the cookie +func clearSessionValues(session *sessions.Session, expire bool) { + if session == nil { + return + } + + // Clear all values + for k := range session.Values { + delete(session.Values, k) + } + + // If expiring, set MaxAge to -1 + if expire { + session.Options.MaxAge = -1 + } +} + +// clearAllSessionData clears values from all session objects (main, token sessions, and chunks) +// Parameters: +// - sd: The SessionData instance containing all sessions +// - r: The HTTP request (needed for chunk clearing) +// - expire: Whether to expire the cookies (set MaxAge to -1) +func (sd *SessionData) clearAllSessionData(r *http.Request, expire bool) { + // Clear main session and token sessions + clearSessionValues(sd.mainSession, expire) + clearSessionValues(sd.accessSession, expire) + clearSessionValues(sd.refreshSession, expire) + clearSessionValues(sd.idTokenSession, expire) + + // If we need to expire cookies, clear token chunks + if expire && r != nil { + sd.clearTokenChunks(r, sd.accessTokenChunks) + sd.clearTokenChunks(r, sd.refreshTokenChunks) + sd.clearTokenChunks(r, sd.idTokenChunks) + } else { + // Just remove the chunks from memory without expiring cookies + for k := range sd.accessTokenChunks { + delete(sd.accessTokenChunks, k) + } + for k := range sd.refreshTokenChunks { + delete(sd.refreshTokenChunks, k) + } + for k := range sd.idTokenChunks { + delete(sd.idTokenChunks, k) + } + } + + // Mark session as dirty if we're changing state + if expire { + sd.dirty = true + } +} + // Clear removes all session data associated with this SessionData instance. // It clears the values map of the main, access, and refresh sessions, sets their MaxAge to -1 // to expire the cookies immediately, and clears any associated token chunk cookies. @@ -539,38 +595,8 @@ func (sd *SessionData) Clear(r *http.Request, w http.ResponseWriter) error { sd.sessionMutex.Lock() defer sd.sessionMutex.Unlock() - sd.dirty = true // Clearing the session means its state is changing and needs to be saved. - - // Clear and expire all sessions. - if sd.mainSession != nil { - sd.mainSession.Options.MaxAge = -1 - for k := range sd.mainSession.Values { - delete(sd.mainSession.Values, k) - } - } - if sd.accessSession != nil { - sd.accessSession.Options.MaxAge = -1 - for k := range sd.accessSession.Values { - delete(sd.accessSession.Values, k) - } - } - if sd.refreshSession != nil { - sd.refreshSession.Options.MaxAge = -1 - for k := range sd.refreshSession.Values { - delete(sd.refreshSession.Values, k) - } - } - if sd.idTokenSession != nil { - sd.idTokenSession.Options.MaxAge = -1 - for k := range sd.idTokenSession.Values { - delete(sd.idTokenSession.Values, k) - } - } - - // Clear chunk sessions. - sd.clearTokenChunks(r, sd.accessTokenChunks) - sd.clearTokenChunks(r, sd.refreshTokenChunks) - sd.clearTokenChunks(r, sd.idTokenChunks) + // Clear all session data and expire cookies + sd.clearAllSessionData(r, true) // Create a guaranteed error when the response writer is set // This is primarily for testing - in production w will often be nil @@ -613,10 +639,7 @@ func (sd *SessionData) returnToPoolSafely() { // - chunks: The map of session chunks (e.g., sd.accessTokenChunks) to clear and expire. func (sd *SessionData) clearTokenChunks(r *http.Request, chunks map[int]*sessions.Session) { for _, session := range chunks { - session.Options.MaxAge = -1 - for k := range session.Values { - delete(session.Values, k) - } + clearSessionValues(session, true) // Clear and expire the chunks } } @@ -711,6 +734,21 @@ func (sd *SessionData) SetAuthenticated(value bool) error { return nil } +// resetSession resets a session by clearing its values and resetting its ID and IsNew flag +// This is specifically for pool reuse preparation +func resetSession(session *sessions.Session) { + if session == nil { + return + } + + // Clear all values + clearSessionValues(session, false) + + // Reset session ID and IsNew flag + session.ID = "" + session.IsNew = true +} + // Reset clears all session data and prepares the SessionData object for reuse. // This method is called when returning objects to the pool to prevent data leakage // between different users/sessions. @@ -718,48 +756,16 @@ func (sd *SessionData) Reset() { sd.sessionMutex.Lock() defer sd.sessionMutex.Unlock() - if sd.mainSession != nil { - for k := range sd.mainSession.Values { - delete(sd.mainSession.Values, k) - } - sd.mainSession.ID = "" - sd.mainSession.IsNew = true - } + // Clear all session data (but don't expire cookies, as this is internal cleanup) + sd.clearAllSessionData(nil, false) - if sd.accessSession != nil { - for k := range sd.accessSession.Values { - delete(sd.accessSession.Values, k) - } - sd.accessSession.ID = "" - sd.accessSession.IsNew = true - } - - if sd.refreshSession != nil { - for k := range sd.refreshSession.Values { - delete(sd.refreshSession.Values, k) - } - sd.refreshSession.ID = "" - sd.refreshSession.IsNew = true - } - - if sd.idTokenSession != nil { - for k := range sd.idTokenSession.Values { - delete(sd.idTokenSession.Values, k) - } - sd.idTokenSession.ID = "" - sd.idTokenSession.IsNew = true - } - - for k := range sd.accessTokenChunks { - delete(sd.accessTokenChunks, k) - } - for k := range sd.refreshTokenChunks { - delete(sd.refreshTokenChunks, k) - } - for k := range sd.idTokenChunks { - delete(sd.idTokenChunks, k) - } + // Reset session state for pool reuse + resetSession(sd.mainSession) + resetSession(sd.accessSession) + resetSession(sd.refreshSession) + resetSession(sd.idTokenSession) + // Reset other state sd.dirty = false sd.inUse = false sd.request = nil diff --git a/settings.go b/settings.go index ffeff65..3dca6d2 100644 --- a/settings.go +++ b/settings.go @@ -469,6 +469,17 @@ func (l *Logger) Errorf(format string, args ...any) { l.logError.Printf(format, args...) } +// newNoOpLogger creates a silent logger that doesn't output anything. +// This is useful for internal components that need a logger instance +// but should not produce any output by default. +func newNoOpLogger() *Logger { + return &Logger{ + logError: log.New(io.Discard, "", 0), + logInfo: log.New(io.Discard, "", 0), + logDebug: log.New(io.Discard, "", 0), + } +} + // handleError logs an error message using the provided logger and sends an HTTP error // response to the client with the specified message and status code. // diff --git a/test_tokens.go b/test_tokens.go index b43ff3d..570a416 100644 --- a/test_tokens.go +++ b/test_tokens.go @@ -158,7 +158,7 @@ func (tt *TestTokens) CreateIncompressibleToken(targetSize int) string { // ~100 bytes per field remainingSize/100, 1) - for i := 0; i < fieldCount; i++ { + for i := range fieldCount { // Generate truly random data for each field randomBytes := make([]byte, 50) rand.Read(randomBytes)