fix: prevent nil pointer dereference and silent error handling in overcommit logic

- Return safe no-op values (1.0, 1.0) when errors occur instead of
  continuing with zero values or crashing on nil pointer dereference
- Remove unused getNamespaceYAML function and YAML roundtrip
- Properly handle all error paths in getNamespaceOvercommit and
  checkOvercommitType to avoid mutating pods with incorrect values
- Use direct client.Get for namespace instead of YAML marshal/unmarshal

Author: enriqueavindi

pkg/overcommit/calculate_values_from_labels.go

@@ -7,27 +7,24 @@ package overcommit
 
 import (
 	"context"
-	"fmt"
 
 	"github.com/InditexTech/k8s-overcommit-operator/internal/metrics"
 	"github.com/InditexTech/k8s-overcommit-operator/internal/utils"
 	corev1 "k8s.io/api/core/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/yaml"
+	k8sclient "sigs.k8s.io/controller-runtime/pkg/client"
 )
 
-// GetLabelValue gets the value of a label in the pod or in the namespace of the pod
+// getNamespaceOvercommit gets the overcommit values from the namespace label or falls back to the default class.
+// Returns (1.0, 1.0) as safe no-op values when any error occurs to avoid mutating pods incorrectly.
 func getNamespaceOvercommit(ctx context.Context, pod *corev1.Pod, client client.Client, label, ownerName, ownerKind string) (float64, float64) {
 	// Get the namespace of the pod
 	namespaceName := pod.ObjectMeta.Namespace
 	var ns corev1.Namespace
-	namespace, err := getNamespaceYAML(ctx, namespaceName, client)
+	err := client.Get(ctx, k8sclient.ObjectKey{Name: namespaceName}, &ns)
 	if err != nil {
-		podlog.Error(err, "Error getting the namespace yaml", "namespace", namespaceName)
-	}
-	err = yaml.Unmarshal([]byte(namespace), &ns)
-	if err != nil {
-		podlog.Error(err, "Error unmarshaling the namespace", "namespace", namespaceName)
+		podlog.Error(err, "Error getting the namespace", "namespace", namespaceName)
+		return 1.0, 1.0
 	}
 
 	// Check if the overcommit class label is in the namespace
@@ -36,53 +33,33 @@ func getNamespaceOvercommit(ctx context.Context, pod *corev1.Pod, client client.
 		overcommitClass, err := utils.GetOvercommitClassSpec(ctx, val, client)
 		if err != nil {
 			podlog.Error(err, "Error getting the overcommit class", "overcommitClassLabel", val)
+			return 1.0, 1.0
 		}
 		metrics.K8sOvercommitPodMutated.WithLabelValues(val, ownerKind, ownerName, pod.Namespace).Inc()
 		return overcommitClass.CpuOvercommit, overcommitClass.MemoryOvercommit
-	} else {
-		podlog.Info("Overcommit class not found in the namespace, using the default", "namespace", ns.Name)
-		defaultClass, error := utils.GetDefaultSpec(client)
-		if error != nil {
-			podlog.Error(error, "Error getting the default overcommit class")
-		}
-		metrics.K8sOvercommitPodMutated.WithLabelValues("default", ownerKind, ownerName, pod.Namespace).Inc()
-		return defaultClass.CpuOvercommit, defaultClass.MemoryOvercommit
-	}
-}
-
-// getNamespaceYAML gets the YAML of a namespace using the ServiceAccount token
-func getNamespaceYAML(ctx context.Context, namespaceName string, k8sClient client.Client) (string, error) {
-	// Create a variable to store the Namespace object
-	var namespace corev1.Namespace
-
-	// Get the Namespace object from the API server
-	err := k8sClient.Get(ctx, client.ObjectKey{
-		Name: namespaceName,
-	}, &namespace)
-	if err != nil {
-		return "", fmt.Errorf("error getting the namespace: %v", err)
 	}
 
-	// Convert the Namespace object to YAML
-	nsYAML, err := yaml.Marshal(namespace)
+	podlog.Info("Overcommit class not found in the namespace, using the default", "namespace", ns.Name)
+	defaultClass, err := utils.GetDefaultSpec(client)
 	if err != nil {
-		return "", fmt.Errorf("error converting the namespace to YAML: %v", err)
+		podlog.Error(err, "Error getting the default overcommit class")
+		return 1.0, 1.0
 	}
-
-	return string(nsYAML), nil
+	metrics.K8sOvercommitPodMutated.WithLabelValues("default", ownerKind, ownerName, pod.Namespace).Inc()
+	return defaultClass.CpuOvercommit, defaultClass.MemoryOvercommit
 }
 
 func checkOvercommitType(ctx context.Context, pod corev1.Pod, client client.Client) (float64, float64) {
-	var cpuValue float64
-	var memoryValue float64
 	ownerName, ownerKind, err := utils.GetPodOwner(ctx, client, &pod)
 	if err != nil {
 		podlog.Error(err, "Error getting the pod owner")
+		// Non-fatal: continue with empty owner info
 	}
 
 	label, err := utils.GetOvercommitLabel(ctx, client)
 	if err != nil {
 		podlog.Error(err, "Error getting the overcommit label")
+		return 1.0, 1.0
 	}
 	//  Check if the pod has the overcommit class label
 	value, exists := pod.Labels[label]
@@ -96,17 +73,14 @@ func checkOvercommitType(ctx context.Context, pod corev1.Pod, client client.Clie
 		overcommitClass, err := utils.GetOvercommitClassSpec(ctx, value, client)
 		if err != nil {
 			podlog.Error(err, "Error getting the overcommit class", "overcommitClassLabel", value)
-			// Overcommit class not found or some error
-			cpuValue, memoryValue = getNamespaceOvercommit(ctx, &pod, client, label, ownerName, ownerKind)
-		} else {
-			cpuValue, memoryValue = overcommitClass.CpuOvercommit, overcommitClass.MemoryOvercommit
+			// Overcommit class not found or some error, fall back to namespace/default
+			return getNamespaceOvercommit(ctx, &pod, client, label, ownerName, ownerKind)
 		}
 		metrics.K8sOvercommitPodMutated.WithLabelValues(value, ownerKind, ownerName, pod.Namespace).Inc()
-	} else {
-		// Overcommit class not found, checking the overcommit labels
-		podlog.Info("Overcommit class label not found in pod, checking the namespace")
-		cpuValue, memoryValue = getNamespaceOvercommit(ctx, &pod, client, label, ownerName, ownerKind)
-
+		return overcommitClass.CpuOvercommit, overcommitClass.MemoryOvercommit
 	}
-	return cpuValue, memoryValue
+
+	// Overcommit class not found, checking the namespace
+	podlog.Info("Overcommit class label not found in pod, checking the namespace")
+	return getNamespaceOvercommit(ctx, &pod, client, label, ownerName, ownerKind)
 }