fix: resolve clientmanager lint regressions

Signed-off-by: Moritz Johner <beller.moritz@googlemail.com>

runtime/clientmanager/manager.go

@@ -47,6 +47,13 @@ const (
 	errClusterStoreMismatch  = "using cluster store %q is not allowed from namespace %q: denied by spec.condition"
 	errClusterProviderDenied = "using ClusterProvider %q is not allowed from namespace %q: denied by spec.conditions"
 	errV2ProvidersDisabled   = "v2 provider support is disabled, refusing %s %q (enable with --enable-v2-providers)"
+
+	providerMetricsLabel        = "provider"
+	clusterProviderMetricsLabel = "cluster-provider"
+	cacheInvalidationGeneration = "generation_change"
+	cacheInvalidationMismatch   = "store_mismatch"
+	v2ProviderCacheKeyType      = "v2-provider"
+	v2ClusterProviderCacheKey   = "v2-cluster-provider"
 )
 
 var (
@@ -136,6 +143,26 @@ type v2ProviderConfig struct {
 	kindStr           string // "Provider" or "ClusterProvider"
 }
 
+func providerMetricsLabelForScope(isClusterScoped bool) string {
+	if isClusterScoped {
+		return clusterProviderMetricsLabel
+	}
+
+	return providerMetricsLabel
+}
+
+func providerMetricsLabelForKey(key clientKey) string {
+	if key.v2ProviderName == "" {
+		return "unknown"
+	}
+
+	if key.v2ProviderNamespace == "" {
+		return clusterProviderMetricsLabel
+	}
+
+	return providerMetricsLabel
+}
+
 // NewManager constructs a new manager with defaults.
 func NewManager(ctrlClient client.Client, controllerClass string, enableFloodgate bool) *Manager {
 	log := ctrl.Log.WithName("clientmanager")
@@ -229,9 +256,9 @@ func (m *Manager) Get(ctx context.Context, storeRef esv1.SecretStoreRef, namespa
 // It handles caching, connection pooling, and client lifecycle for both Provider and ClusterProvider.
 func (m *Manager) getOrCreateV2Client(ctx context.Context, cfg v2ProviderConfig, authNamespace string) (esv1.SecretsClient, error) {
 	// Determine cache key type based on resource type
-	cacheKeyType := "v2-provider"
+	cacheKeyType := v2ProviderCacheKeyType
 	if cfg.isClusterScoped {
-		cacheKeyType = "v2-cluster-provider"
+		cacheKeyType = v2ClusterProviderCacheKey
 	}
 
 	// Create cache key
@@ -250,11 +277,7 @@ func (m *Manager) getOrCreateV2Client(ctx context.Context, cfg v2ProviderConfig,
 				"authNamespace", authNamespace,
 				"generation", cfg.generation)
 			// Record cache hit
-			providerType := "provider"
-			if cfg.isClusterScoped {
-				providerType = "cluster-provider"
-			}
-			clientManagerMetrics.RecordCacheHit(providerType)
+			clientManagerMetrics.RecordCacheHit(providerMetricsLabelForScope(cfg.isClusterScoped))
 			return cached.client, nil
 		}
 		// Cache is stale, invalidate
@@ -264,11 +287,7 @@ func (m *Manager) getOrCreateV2Client(ctx context.Context, cfg v2ProviderConfig,
 			"oldGeneration", cached.v2ProviderGeneration,
 			"newGeneration", cfg.generation)
 		// Record cache invalidation
-		providerType := "provider"
-		if cfg.isClusterScoped {
-			providerType = "cluster-provider"
-		}
-		clientManagerMetrics.RecordCacheInvalidation(providerType, "generation_change")
+		clientManagerMetrics.RecordCacheInvalidation(providerMetricsLabelForScope(cfg.isClusterScoped), cacheInvalidationGeneration)
 		delete(m.clientMap, cacheKey)
 	}
 
@@ -312,10 +331,10 @@ func (m *Manager) getOrCreateV2Client(ctx context.Context, cfg v2ProviderConfig,
 
 	// Convert ProviderReference to protobuf format
 	providerRef := &pb.ProviderReference{
-		ApiVersion: cfg.config.ProviderRef.APIVersion,
-		Kind:       cfg.config.ProviderRef.Kind,
-		Name:       cfg.config.ProviderRef.Name,
-		Namespace:  cfg.config.ProviderRef.Namespace,
+		ApiVersion:   cfg.config.ProviderRef.APIVersion,
+		Kind:         cfg.config.ProviderRef.Kind,
+		Name:         cfg.config.ProviderRef.Name,
+		Namespace:    cfg.config.ProviderRef.Namespace,
 		StoreRefKind: cfg.kindStr,
 	}
 
@@ -439,15 +458,7 @@ func (m *Manager) getStoredClient(ctx context.Context, storeProvider esv1.Provid
 			"provider", fmt.Sprintf("%T", storeProvider),
 			"store", storeName)
 		// Record cache hit
-		providerType := "unknown"
-		if idx.v2ProviderName != "" {
-			if idx.v2ProviderNamespace == "" {
-				providerType = "cluster-provider"
-			} else {
-				providerType = "provider"
-			}
-		}
-		clientManagerMetrics.RecordCacheHit(providerType)
+		clientManagerMetrics.RecordCacheHit(providerMetricsLabelForKey(idx))
 		return val.client
 	}
 	m.log.V(1).Info("cleaning up client",
@@ -459,16 +470,11 @@ func (m *Manager) getStoredClient(ctx context.Context, storeProvider esv1.Provid
 	delete(m.clientMap, idx)
 
 	// Record cache invalidation
-	providerType := "unknown"
-	reason := "store_mismatch"
+	providerType := providerMetricsLabelForKey(idx)
+	reason := cacheInvalidationMismatch
 	if idx.v2ProviderName != "" {
-		if idx.v2ProviderNamespace == "" {
-			providerType = "cluster-provider"
-		} else {
-			providerType = "provider"
-		}
 		if val.store.GetObjectMeta().Generation != store.GetGeneration() {
-			reason = "generation_change"
+			reason = cacheInvalidationGeneration
 		}
 	}
 	clientManagerMetrics.RecordCacheInvalidation(providerType, reason)
@@ -523,7 +529,7 @@ func (m *Manager) Close(ctx context.Context) error {
 	// Close v1 provider clients (they don't use the pool)
 	for key, val := range m.clientMap {
 		// Only close v1 clients; v2 clients are managed by the pool
-		if key.providerType != "v2-provider" && key.providerType != "v2-cluster-provider" {
+		if key.providerType != v2ProviderCacheKeyType && key.providerType != v2ClusterProviderCacheKey {
 			err := val.client.Close(ctx)
 			if err != nil {
 				errs = append(errs, err.Error())

runtime/clientmanager/manager_test.go

@@ -879,8 +879,8 @@ func TestGetV2ProviderInvalidatesGenerationCacheAndReleasesPoolReferences(t *tes
 
 	provider := &esv1.Provider{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      "provider",
-			Namespace: manifestNamespace,
+			Name:       "provider",
+			Namespace:  manifestNamespace,
 			Generation: 1,
 		},
 		Spec: esv1.ProviderSpec{
@@ -1193,9 +1193,7 @@ func installGlobalV2ConnectionPoolForTest(t *testing.T) *prometheus.Registry {
 		HealthCheckInterval: 10 * time.Millisecond,
 	})
 
-	var once sync.Once
-	once.Do(func() {})
-	globalV2ConnectionPoolOnce = once
+	globalV2ConnectionPoolOnce.Do(func() {})
 
 	registry := prometheus.NewRegistry()
 	require.NoError(t, providergrpc.RegisterMetrics(registry))

runtime/clientmanager/metrics.go

@@ -17,6 +17,7 @@ limitations under the License.
 package clientmanager
 
 import (
+	"errors"
 	"fmt"
 
 	"github.com/prometheus/client_golang/prometheus"
@@ -24,7 +25,7 @@ import (
 )
 
 var (
-	// ClientManager Gauges
+	// ClientManager gauges.
 	clientsCachedTotal = prometheus.NewGaugeVec(
 		prometheus.GaugeOpts{
 			Name: "clientmanager_clients_cached_total",
@@ -33,7 +34,7 @@ var (
 		[]string{"provider_type"},
 	)
 
-	// ClientManager Counters
+	// ClientManager counters.
 	cacheHitsTotal = prometheus.NewCounterVec(
 		prometheus.CounterOpts{
 			Name: "clientmanager_cache_hits_total",
@@ -51,42 +52,42 @@ var (
 	)
 )
 
-// ClientManagerMetrics interface for testability
-type ClientManagerMetrics interface {
+// Metrics provides test hooks for client manager metrics.
+type Metrics interface {
 	RecordCacheHit(providerType string)
 	RecordCacheMiss(providerType string)
-	RecordCacheInvalidation(providerType string, reason string)
+	RecordCacheInvalidation(providerType, reason string)
 	UpdateCachedClients(providerType string, count int)
 }
 
-// defaultClientManagerMetrics implements ClientManagerMetrics using Prometheus
-type defaultClientManagerMetrics struct{}
+// defaultMetrics implements Metrics using Prometheus.
+type defaultMetrics struct{}
 
-// RecordCacheHit records a cache hit
-func (m *defaultClientManagerMetrics) RecordCacheHit(providerType string) {
+// RecordCacheHit records a cache hit.
+func (m *defaultMetrics) RecordCacheHit(providerType string) {
 	cacheHitsTotal.WithLabelValues(providerType).Inc()
 }
 
-// RecordCacheMiss records a cache miss
-func (m *defaultClientManagerMetrics) RecordCacheMiss(providerType string) {
+// RecordCacheMiss records a cache miss.
+func (m *defaultMetrics) RecordCacheMiss(_ string) {
 	// Cache misses are implicit - we don't track them separately
 	// The absence of a hit implies a miss
 }
 
-// RecordCacheInvalidation records a cache invalidation
-func (m *defaultClientManagerMetrics) RecordCacheInvalidation(providerType string, reason string) {
+// RecordCacheInvalidation records a cache invalidation.
+func (m *defaultMetrics) RecordCacheInvalidation(providerType, reason string) {
 	cacheInvalidationsTotal.WithLabelValues(providerType, reason).Inc()
 }
 
-// UpdateCachedClients updates the total cached clients gauge
-func (m *defaultClientManagerMetrics) UpdateCachedClients(providerType string, count int) {
+// UpdateCachedClients updates the total cached clients gauge.
+func (m *defaultMetrics) UpdateCachedClients(providerType string, count int) {
 	clientsCachedTotal.WithLabelValues(providerType).Set(float64(count))
 }
 
-// Global instance
-var clientManagerMetrics ClientManagerMetrics = &defaultClientManagerMetrics{}
+// Global instance.
+var clientManagerMetrics Metrics = &defaultMetrics{}
 
-// RegisterMetrics registers all client manager metrics with the controller-runtime metrics registry
+// RegisterMetrics registers all client manager metrics with the controller-runtime metrics registry.
 func RegisterMetrics() error {
 	collectors := []prometheus.Collector{
 		clientsCachedTotal,
@@ -96,8 +97,8 @@ func RegisterMetrics() error {
 
 	for _, collector := range collectors {
 		if err := metrics.Registry.Register(collector); err != nil {
-			// Check if already registered
-			if _, ok := err.(prometheus.AlreadyRegisteredError); ok {
+			var alreadyRegistered prometheus.AlreadyRegisteredError
+			if errors.As(err, &alreadyRegistered) {
 				continue
 			}
 			return fmt.Errorf("failed to register clientmanager metric: %w", err)
@@ -106,18 +107,17 @@ func RegisterMetrics() error {
 
 	// Initialize metrics with zero values so they appear in /metrics output
 	// This ensures metrics are visible even before any cache operations occur
-	for _, providerType := range []string{"provider", "cluster-provider"} {
+	for _, providerType := range []string{providerMetricsLabel, clusterProviderMetricsLabel} {
 		clientsCachedTotal.WithLabelValues(providerType).Set(0)
 		cacheHitsTotal.WithLabelValues(providerType).Add(0)
-		cacheInvalidationsTotal.WithLabelValues(providerType, "generation_change").Add(0)
-		cacheInvalidationsTotal.WithLabelValues(providerType, "store_mismatch").Add(0)
+		cacheInvalidationsTotal.WithLabelValues(providerType, cacheInvalidationGeneration).Add(0)
+		cacheInvalidationsTotal.WithLabelValues(providerType, cacheInvalidationMismatch).Add(0)
 	}
 
 	return nil
 }
 
-// GetClientManagerMetrics returns the client manager metrics instance (for testing)
-func GetClientManagerMetrics() ClientManagerMetrics {
+// GetClientManagerMetrics returns the client manager metrics instance for tests.
+func GetClientManagerMetrics() Metrics {
 	return clientManagerMetrics
 }
-