mirror of
https://github.com/lukaszraczylo/traefikoidc.git
synced 2026-10-16 04:29:15 +00:00
feat(core): refactor linters config and improve code quality (#119)
- [x] Reorganize golangci-lint configuration with documented disable reasons - [x] Simplify errcheck and revive linter rules with targeted exclusions - [x] Pre-compile regex patterns in input_validation.go for performance - [x] Fix type assertions in memory_shard.go and resp.go with safety checks - [x] Replace string comparison with EqualFold for case-insensitive matching - [x] Fix loop variable captures in jwk.go and logout.go - [x] Change high goroutine log level from Info to Debug in autocleanup.go - [x] Replace deprecated "cancelled" spelling with "canceled" throughout - [x] Add nolint annotations for intentional unused parameters - [x] Improve comment formatting for deprecated functions - [x] Fix comment spelling: "marshalling" → "marshaling" - [x] Refactor provider warnings formatting in internal/providers/warnings.go - [x] Simplify metrics summary building in internal/recovery/metrics.go - [x] Pre-allocate slice in error_recovery.go GetDegradedServices - [x] Refactor context cancellation checks in redis.go
This commit is contained in:
+9
-5
@@ -241,9 +241,11 @@ func (s *cacheShard) evictLRULocked() bool {
|
||||
|
||||
element := s.lruList.Back()
|
||||
if element != nil {
|
||||
item := element.Value.(*memoryCacheItem)
|
||||
s.deleteItemLocked(item)
|
||||
return true
|
||||
item, ok := element.Value.(*memoryCacheItem)
|
||||
if ok {
|
||||
s.deleteItemLocked(item)
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
@@ -267,8 +269,10 @@ func (s *cacheShard) getOldestAccessTime() time.Time {
|
||||
|
||||
element := s.lruList.Back()
|
||||
if element != nil {
|
||||
item := element.Value.(*memoryCacheItem)
|
||||
return item.accessedAt
|
||||
item, ok := element.Value.(*memoryCacheItem)
|
||||
if ok {
|
||||
return item.accessedAt
|
||||
}
|
||||
}
|
||||
return time.Time{}
|
||||
}
|
||||
|
||||
Vendored
+2
-2
@@ -345,7 +345,7 @@ func (r *RedisBackend) prefixKey(key string) string {
|
||||
|
||||
// executeWithRetry executes a Redis operation with exponential backoff retry logic.
|
||||
// It checks context cancellation at multiple points to ensure fast abort when the
|
||||
// caller's context is cancelled (e.g., due to request timeout).
|
||||
// caller's context is canceled (e.g., due to request timeout).
|
||||
func (r *RedisBackend) executeWithRetry(ctx context.Context, operation func(*RedisConn) error) error {
|
||||
maxRetries := 3
|
||||
baseDelay := 50 * time.Millisecond // Reduced from 100ms to fail faster
|
||||
@@ -377,7 +377,7 @@ func (r *RedisBackend) executeWithRetry(ctx context.Context, operation func(*Red
|
||||
err = operation(conn)
|
||||
r.pool.Put(conn)
|
||||
|
||||
// Check context after operation - if cancelled, don't bother retrying
|
||||
// Check context after operation - if canceled, don't bother retrying
|
||||
if ctx.Err() != nil {
|
||||
return ctx.Err()
|
||||
}
|
||||
|
||||
+1
-1
@@ -201,7 +201,7 @@ func TestConnectionPool_ContextCancellation(t *testing.T) {
|
||||
conn, err := pool.Get(context.Background())
|
||||
require.NoError(t, err)
|
||||
|
||||
// Try to get another with cancelled context
|
||||
// Try to get another with canceled context
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
cancel() // Cancel immediately
|
||||
|
||||
|
||||
Vendored
+2
-2
@@ -44,7 +44,7 @@ type RESPWriter struct {
|
||||
|
||||
// NewRESPWriter creates a new RESP writer from the pool (memory optimized)
|
||||
func NewRESPWriter(w io.Writer) *RESPWriter {
|
||||
writer := writerPool.Get().(*RESPWriter)
|
||||
writer, _ := writerPool.Get().(*RESPWriter)
|
||||
writer.w = w
|
||||
return writer
|
||||
}
|
||||
@@ -80,7 +80,7 @@ type RESPReader struct {
|
||||
|
||||
// NewRESPReader creates a new RESP reader from the pool (memory optimized)
|
||||
func NewRESPReader(r io.Reader) *RESPReader {
|
||||
reader := readerPool.Get().(*RESPReader)
|
||||
reader, _ := readerPool.Get().(*RESPReader)
|
||||
reader.r.Reset(r)
|
||||
return reader
|
||||
}
|
||||
|
||||
+1
-1
@@ -87,7 +87,7 @@ func (s *SingleflightCache) GetOrFetch(ctx context.Context, key string, fetcher
|
||||
// If successful, store in cache
|
||||
if call.err == nil && call.val != nil {
|
||||
// Use a background context for cache storage to ensure it completes
|
||||
// even if the original context is cancelled
|
||||
// even if the original context is canceled
|
||||
storeCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
|
||||
_ = s.backend.Set(storeCtx, key, call.val, call.ttl)
|
||||
cancel()
|
||||
|
||||
Vendored
+1
-1
@@ -232,7 +232,7 @@ func (m *Manager) Close() error {
|
||||
|
||||
var firstErr error
|
||||
|
||||
if err := m.tokenCache.Close(); err != nil && firstErr == nil {
|
||||
if err := m.tokenCache.Close(); err != nil {
|
||||
firstErr = err
|
||||
}
|
||||
if err := m.metadataCache.Close(); err != nil && firstErr == nil {
|
||||
|
||||
@@ -397,7 +397,7 @@ func (wp *WorkerPool) Submit(task func()) error {
|
||||
}
|
||||
|
||||
// worker is the main worker routine
|
||||
func (wp *WorkerPool) worker(id int) {
|
||||
func (wp *WorkerPool) worker(_ int) {
|
||||
defer wp.workerWg.Done()
|
||||
|
||||
for {
|
||||
|
||||
@@ -173,7 +173,7 @@ func (m *FeatureManager) LoadFromEnv() {
|
||||
for name, flag := range flags {
|
||||
envVar := "FEATURE_" + name
|
||||
if value := os.Getenv(envVar); value != "" {
|
||||
enabled := strings.ToLower(value) == "true" || value == "1"
|
||||
enabled := strings.EqualFold(value, "true") || value == "1"
|
||||
flag.enabled.Store(enabled)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -40,7 +40,7 @@ func (p *AWSCognitoProvider) BuildAuthParams(baseParams url.Values, scopes []str
|
||||
// Remove offline_access scope as Cognito doesn't use it (case-insensitive)
|
||||
var filteredScopes []string
|
||||
for _, scope := range scopes {
|
||||
if strings.ToLower(scope) != ScopeOfflineAccess {
|
||||
if !strings.EqualFold(scope, ScopeOfflineAccess) {
|
||||
filteredScopes = append(filteredScopes, scope)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -18,16 +18,17 @@ func GetProviderWarnings(providerType ProviderType) []ProviderWarning {
|
||||
|
||||
switch providerType {
|
||||
case ProviderTypeGitHub:
|
||||
warnings = append(warnings, ProviderWarning{
|
||||
ProviderType: ProviderTypeGitHub,
|
||||
Level: "warning",
|
||||
Message: "GitHub uses OAuth 2.0, not OpenID Connect. ID tokens are not available. Use access tokens for API calls only.",
|
||||
})
|
||||
warnings = append(warnings, ProviderWarning{
|
||||
ProviderType: ProviderTypeGitHub,
|
||||
Level: "info",
|
||||
Message: "GitHub OAuth apps do not support refresh tokens. Users will need to re-authenticate when tokens expire.",
|
||||
})
|
||||
warnings = append(warnings,
|
||||
ProviderWarning{
|
||||
ProviderType: ProviderTypeGitHub,
|
||||
Level: "warning",
|
||||
Message: "GitHub uses OAuth 2.0, not OpenID Connect. ID tokens are not available. Use access tokens for API calls only.",
|
||||
},
|
||||
ProviderWarning{
|
||||
ProviderType: ProviderTypeGitHub,
|
||||
Level: "info",
|
||||
Message: "GitHub OAuth apps do not support refresh tokens. Users will need to re-authenticate when tokens expire.",
|
||||
})
|
||||
|
||||
case ProviderTypeAuth0:
|
||||
warnings = append(warnings, ProviderWarning{
|
||||
|
||||
@@ -116,7 +116,7 @@ func (re *RetryExecutor) ExecuteWithContext(ctx context.Context, fn func() error
|
||||
// Continue to next attempt
|
||||
case <-ctx.Done():
|
||||
re.RecordFailure()
|
||||
return fmt.Errorf("retry cancelled: %w", ctx.Err())
|
||||
return fmt.Errorf("retry canceled: %w", ctx.Err())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -301,7 +301,7 @@ func (rm *RecoveryMetrics) GetAllMetrics() map[string]interface{} {
|
||||
}
|
||||
}
|
||||
|
||||
allMetrics["summary"] = map[string]interface{}{
|
||||
summary := map[string]interface{}{
|
||||
"totalMechanisms": len(rm.mechanisms),
|
||||
"totalRequests": totalRequests,
|
||||
"totalSuccesses": totalSuccesses,
|
||||
@@ -310,8 +310,9 @@ func (rm *RecoveryMetrics) GetAllMetrics() map[string]interface{} {
|
||||
|
||||
if totalRequests > 0 {
|
||||
successRate := float64(totalSuccesses) / float64(totalRequests) * 100
|
||||
allMetrics["summary"].(map[string]interface{})["overallSuccessRate"] = fmt.Sprintf("%.2f%%", successRate)
|
||||
summary["overallSuccessRate"] = fmt.Sprintf("%.2f%%", successRate)
|
||||
}
|
||||
allMetrics["summary"] = summary
|
||||
|
||||
return allMetrics
|
||||
}
|
||||
|
||||
@@ -223,7 +223,7 @@ func TestRetryExecutor_ExecuteWithContext_ContextCancelled(t *testing.T) {
|
||||
wg.Wait()
|
||||
|
||||
if execErr == nil {
|
||||
t.Error("Expected error when context is cancelled")
|
||||
t.Error("Expected error when context is canceled")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -240,7 +240,7 @@ func TestRetryExecutor_ExecuteWithContext_ContextCancelledBeforeStart(t *testing
|
||||
})
|
||||
|
||||
if err == nil {
|
||||
t.Error("Expected error when context is already cancelled")
|
||||
t.Error("Expected error when context is already canceled")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -282,7 +282,7 @@ func TestRetryExecutor_isRetryableError(t *testing.T) {
|
||||
{name: "timeout", err: errors.New("TIMEOUT"), expected: true}, // case insensitive
|
||||
{name: "EOF", err: errors.New("EOF"), expected: false},
|
||||
{name: "random error", err: errors.New("something else"), expected: false},
|
||||
{name: "context cancelled", err: context.Canceled, expected: false},
|
||||
{name: "context canceled", err: context.Canceled, expected: false},
|
||||
{name: "context deadline exceeded", err: context.DeadlineExceeded, expected: false},
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user