backend: cmd: Refactor CacheMiddleWare by extracting handler

Extract the request handling logic from CacheMiddleWare into a
separate handleCacheRequest function to reduce cognitive complexity
and function length. This removes the need for the nolint directive
and makes the code more maintainable and testable.

Add documentation comment explaining why ErrHandled is checked and
why we return early when it's encountered. This clarifies that
ErrHandled is a sentinel error indicating the request was already
fully processed during cache invalidation, not an actual error.

Change ErrHandled from fmt.Errorf to errors.New to align with Go
best practices for sentinel errors. Sentinel errors should be created
with errors.New rather than fmt.Errorf to enable proper error
comparison with errors.Is.

Add missing return statement after handleError when LoadFromCache
fails to prevent double writing HTTP responses. This ensures the
middleware doesn't continue processing after an error has been
written to the client.

Author: krrish-sehgal

backend/cmd/headlamp_test.go

@@ -1684,7 +1684,9 @@ func newRealK8sHeadlampConfig(t *testing.T) (*HeadlampConfig, string) {
 
 	if clusterName == "" {
 		clusters := (&HeadlampConfig{
-			HeadlampCFG: &headlampconfig.HeadlampCFG{KubeConfigStore: kubeConfigStore},
+			HeadlampConfig: &headlampconfig.HeadlampConfig{
+				HeadlampCFG: &headlampconfig.HeadlampCFG{KubeConfigStore: kubeConfigStore},
+			},
 		}).getClusters()
 		for _, c := range clusters {
 			if c.Error == "" {
@@ -1699,17 +1701,19 @@ func newRealK8sHeadlampConfig(t *testing.T) (*HeadlampConfig, string) {
 	}
 
 	c := &HeadlampConfig{
-		HeadlampCFG: &headlampconfig.HeadlampCFG{
-			UseInCluster:    false,
-			KubeConfigPath:  kubeConfigPath,
-			KubeConfigStore: kubeConfigStore,
-			CacheEnabled:    true,
-			PluginDir:       pluginDir,
-			UserPluginDir:   userPluginDir,
+		HeadlampConfig: &headlampconfig.HeadlampConfig{
+			HeadlampCFG: &headlampconfig.HeadlampCFG{
+				UseInCluster:    false,
+				KubeConfigPath:  kubeConfigPath,
+				KubeConfigStore: kubeConfigStore,
+				CacheEnabled:    true,
+				PluginDir:       pluginDir,
+				UserPluginDir:   userPluginDir,
+			},
+			Cache:            cache.New[interface{}](),
+			TelemetryConfig:  GetDefaultTestTelemetryConfig(),
+			TelemetryHandler: &telemetry.RequestHandler{},
 		},
-		cache:            cache.New[interface{}](),
-		telemetryConfig:  GetDefaultTestTelemetryConfig(),
-		telemetryHandler: &telemetry.RequestHandler{},
 	}
 
 	return c, clusterName

backend/cmd/server.go

@@ -183,74 +183,85 @@ func GetContextKeyAndKContext(w http.ResponseWriter,
 	return ctx, span, contextKey, kContext, nil
 }
 
-// CacheMiddleWare is Middleware for Caching purpose. It involves generating key for a request,
-// authorizing user , store resource data in cache and returns data if key is present.
-//
-//nolint:gocognit,funlen
-func CacheMiddleWare(c *HeadlampConfig) mux.MiddlewareFunc {
-	return func(next http.Handler) http.Handler {
-		if !c.CacheEnabled {
-			return next
-		}
+// handleCacheRequest processes a request with caching logic.
+func handleCacheRequest(c *HeadlampConfig, next http.Handler, w http.ResponseWriter, r *http.Request) {
+	if k8cache.SkipWebSocket(r, next, w) {
+		return
+	}
 
-		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-			if k8cache.SkipWebSocket(r, next, w) {
-				return
-			}
+	ctx, span, contextKey, kContext, err := GetContextKeyAndKContext(w, r, c)
+	if err != nil {
+		return
+	}
 
-			ctx, span, contextKey, kContext, err := GetContextKeyAndKContext(w, r, c)
-			if err != nil {
-				return
-			}
+	if err := k8cache.HandleNonGETCacheInvalidation(k8sResponseCache, w, r, next, contextKey); err != nil {
+		// ErrHandled is a sentinel error indicating the request was fully
+		// processed during cache invalidation. For non-GET requests
+		// (POST/PUT/DELETE), HandleNonGETCacheInvalidation invalidates the
+		// cache, makes a fresh request to K8s, stores the response, and
+		// writes the response to the client. When ErrHandled is returned,
+		// the request has already been handled and we must return early to
+		// avoid processing the request again or writing duplicate responses.
+		if errors.Is(err, k8cache.ErrHandled) {
+			return
+		}
+
+		c.handleError(w, ctx, span, err, "error while invalidating keys", http.StatusInternalServerError)
 
-			if err := k8cache.HandleNonGETCacheInvalidation(k8sResponseCache, w, r, next, contextKey); err != nil {
-				if errors.Is(err, k8cache.ErrHandled) {
-					return
-				}
+		return
+	}
 
-				c.handleError(w, ctx, span, err, "error while invalidating keys", http.StatusInternalServerError)
+	rcw := k8cache.NewResponseCapture(w)
 
-				return
-			}
+	key, err := k8cache.GenerateKey(r.URL, contextKey)
+	if err != nil {
+		c.handleError(w, ctx, span, err, "failed to generate key ", http.StatusBadRequest)
+		return
+	}
 
-			rcw := k8cache.NewResponseCapture(w)
+	isAllowed, authErr := k8cache.IsAllowed(kContext, r)
+	if authErr != nil {
+		k8cache.ServeFromCacheOrForwardToK8s(k8sResponseCache, isAllowed, next, key, w, r, rcw)
 
-			key, err := k8cache.GenerateKey(r.URL, contextKey)
-			if err != nil {
-				c.handleError(w, ctx, span, err, "failed to generate key ", http.StatusBadRequest)
-				return
-			}
+		return
+	} else if !isAllowed && k8cache.IsAuthBypassURL(r.URL.Path) {
+		_ = k8cache.ReturnAuthErrorResponse(w, r, contextKey)
 
-			isAllowed, authErr := k8cache.IsAllowed(kContext, r)
-			if authErr != nil {
-				k8cache.ServeFromCacheOrForwardToK8s(k8sResponseCache, isAllowed, next, key, w, r, rcw)
+		return
+	}
 
-				return
-			} else if !isAllowed && k8cache.IsAuthBypassURL(r.URL.Path) {
-				_ = k8cache.ReturnAuthErrorResponse(w, r, contextKey)
+	served, err := k8cache.LoadFromCache(k8sResponseCache, isAllowed, key, w, r)
+	if err != nil {
+		c.handleError(w, ctx, span, errors.New(kContext.Error), "failed to load from cache", http.StatusServiceUnavailable)
+		return
+	}
 
-				return
-			}
+	if served {
+		c.TelemetryHandler.RecordEvent(span, "Served from cache")
+		return
+	}
 
-			served, err := k8cache.LoadFromCache(k8sResponseCache, isAllowed, key, w, r)
-			if err != nil {
-				c.handleError(w, ctx, span, errors.New(kContext.Error), "failed to load from cache", http.StatusServiceUnavailable)
-			}
+	k8cache.CheckForChanges(k8sResponseCache, contextKey, *kContext)
 
-			if served {
-				c.TelemetryHandler.RecordEvent(span, "Served from cache")
-				return
-			}
+	next.ServeHTTP(rcw, r)
 
-			k8cache.CheckForChanges(k8sResponseCache, contextKey, *kContext)
+	err = k8cache.StoreK8sResponseInCache(k8sResponseCache, r.URL, rcw, r, key)
+	if err != nil {
+		c.handleError(w, ctx, span, errors.New(kContext.Error), "error while storing into cache", http.StatusBadRequest)
+		return
+	}
+}
 
-			next.ServeHTTP(rcw, r)
+// CacheMiddleWare is Middleware for Caching purpose. It involves generating key for a request,
+// authorizing user , store resource data in cache and returns data if key is present.
+func CacheMiddleWare(c *HeadlampConfig) mux.MiddlewareFunc {
+	return func(next http.Handler) http.Handler {
+		if !c.CacheEnabled {
+			return next
+		}
 
-			err = k8cache.StoreK8sResponseInCache(k8sResponseCache, r.URL, rcw, r, key)
-			if err != nil {
-				c.handleError(w, ctx, span, errors.New(kContext.Error), "error while storing into cache", http.StatusBadRequest)
-				return
-			}
+		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			handleCacheRequest(c, next, w, r)
 		})
 	}
 }

backend/pkg/k8cache/cacheInvalidation.go

@@ -21,6 +21,7 @@ package k8cache
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net/http"
 	"net/http/httptest"
@@ -88,7 +89,7 @@ func HandleNonGETCacheInvalidation(k8scache cache.Cache[string], w http.Response
 }
 
 // ErrHandled indicates the request was fully handled by cache invalidation; middleware must return.
-var ErrHandled = fmt.Errorf("handled by cache invalidation")
+var ErrHandled = errors.New("handled by cache invalidation")
 
 // SkipWebSocket skip all the websocket requests coming from the client/ frontend to ensure
 // real time data updation in the frontend.