From 5c3db891106eee22e9f09090af95206be743d877 Mon Sep 17 00:00:00 2001
From: "tal.asulin" <tal.asulin@appsflyer.com>
Date: Sat, 21 Dec 2024 00:02:17 +0200
Subject: [PATCH] fix: removing bad memory access in the node selector term

---
 go.mod                                |  2 +-
 internal/controller/pvc_controller.go | 66 +++++++++++++++++++++------
 2 files changed, 52 insertions(+), 16 deletions(-)

diff --git a/go.mod b/go.mod
index 30535d3..ca39a1a 100644
--- a/go.mod
+++ b/go.mod
@@ -15,7 +15,6 @@ require (
 	k8s.io/apimachinery v0.30.0
 	k8s.io/client-go v0.30.0
 	k8s.io/klog/v2 v2.120.1
-	k8s.io/utils v0.0.0-20240423183400-0849a56e8f22
 	sigs.k8s.io/controller-runtime v0.18.0
 )
 
@@ -67,6 +66,7 @@ require (
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 	k8s.io/apiextensions-apiserver v0.30.0 // indirect
 	k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 // indirect
+	k8s.io/utils v0.0.0-20240423183400-0849a56e8f22 // indirect
 	sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
 	sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect
 	sigs.k8s.io/yaml v1.3.0 // indirect
diff --git a/internal/controller/pvc_controller.go b/internal/controller/pvc_controller.go
index 5f3f8c2..45bb1be 100644
--- a/internal/controller/pvc_controller.go
+++ b/internal/controller/pvc_controller.go
@@ -20,7 +20,7 @@ import (
 	"context"
 	"fmt"
 	"github.com/prometheus/client_golang/prometheus"
-	"k8s.io/utils/strings/slices"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"strconv"
 
 	"github.com/AppsFlyer/local-pvc-releaser/internal/exporters"
@@ -124,25 +124,61 @@ func (r *PVCReconciler) CleanPVCS(ctx context.Context, pvcs []*v1.PersistentVolu
 	return nil
 }
 
+// GetLocalPersistentVolumeNodeNames returns the node affinity node name(s) for
+// local PersistentVolumes. nil is returned if the PV does not have any
+// specific node affinity node selector terms and match expressions.
+// PersistentVolume with node affinity has select and match expressions
+// in the form of:
+//
+//	nodeAffinity:
+//	  required:
+//	    nodeSelectorTerms:
+//	    - matchExpressions:
+//	      - key: kubernetes.io/hostname
+//	        operator: In
+//	        values:
+//	        - <node1>
+//	        - <node2>
+//
+// This code block belongs to the K8s official repository of 1.28
+func (r *PVCReconciler) GetLocalPersistentVolumeNodeNames(pv *v1.PersistentVolume) []string {
+	// Ignoring PVs without affinity rules or PVs that already got released
+	if pv == nil || pv.Spec.NodeAffinity == nil || pv.Spec.NodeAffinity.Required == nil || pv.Status.Phase != v1.VolumeBound {
+		return nil
+	}
+
+	var result sets.Set[string]
+	for _, term := range pv.Spec.NodeAffinity.Required.NodeSelectorTerms {
+		var nodes sets.Set[string]
+		for _, matchExpr := range term.MatchExpressions {
+			if matchExpr.Key == v1.LabelHostname && matchExpr.Operator == v1.NodeSelectorOpIn {
+				if nodes == nil {
+					nodes = sets.New(matchExpr.Values...)
+				} else {
+					nodes = nodes.Intersection(sets.New(matchExpr.Values...))
+				}
+			}
+		}
+		result = result.Union(nodes)
+	}
+
+	return sets.List(result)
+}
+
 func (r *PVCReconciler) FilterPVListByNodeName(pvList *v1.PersistentVolumeList, nodeName string) []*v1.PersistentVolume {
 	var relatedPVs []*v1.PersistentVolume
+	var nodeSet []string
 
-	for i := 0; i < len(pvList.Items); i++ {
-		pv := &pvList.Items[i]
-		// Ignoring PVs without affinity rules or PVs that already got released
-		if pv.Spec.NodeAffinity != nil && pv.Spec.NodeAffinity.Required == nil || pv.Status.Phase != v1.VolumeBound {
-			continue
+	for _, pv := range pvList.Items {
+		nodeSet = r.GetLocalPersistentVolumeNodeNames(&pv)
+		// Assuming only one node attachment
+		if nodeSet == nil || len(nodeSet) != 1 {
+			return nil
 		}
 
-		for _, nst := range pv.Spec.NodeAffinity.Required.NodeSelectorTerms {
-			for _, matchEx := range nst.MatchExpressions {
-				if slices.Contains(matchEx.Values, nodeName) {
-					r.Logger.Info(fmt.Sprintf("pv - %s is bounded to node - %s. will be marked for pvc cleanup", pv.Name, nodeName))
-					relatedPVs = append(relatedPVs, pv)
-
-					break
-				}
-			}
+		if nodeSet[0] == nodeName {
+			r.Logger.Info(fmt.Sprintf("pv - %s is bounded to node - %s. will be marked for pvc cleanup", pv.Name, nodeName))
+			relatedPVs = append(relatedPVs, &pv)
 		}
 	}