Merge pull request #21 from super-phenix/fix-security

chore(security): ensure nothing can crash the operator

Author: SkalaNetworks

cmd/main.go

@@ -5,6 +5,7 @@ import (
 	"flag"
 	"os"
 	"os/signal"
+	"regexp"
 
 	"github.com/super-phenix/volume-replicator/internal/k8s"
 	"github.com/super-phenix/volume-replicator/internal/replicator"
@@ -16,17 +17,25 @@ func main() {
 	ctx, cancel := signal.NotifyContext(context.Background(), os.Interrupt)
 	defer cancel()
 
-	var kubeconfig, namespace string
+	var kubeconfig, namespace, exclusionRegexStr string
 	flag.StringVar(&kubeconfig, "kubeconfig", "", "path to kubeconfig file")
 	flag.StringVar(&namespace, "namespace", os.Getenv("NAMESPACE"), "deployment namespace")
-	flag.StringVar(&replicator.ExclusionRegex, "exclusion-regex", os.Getenv("EXCLUSION_REGEX"), "regex to exclude PVCs from replication")
+	flag.StringVar(&exclusionRegexStr, "exclusion-regex", os.Getenv("EXCLUSION_REGEX"), "regex to exclude PVCs from replication")
 	klog.InitFlags(nil)
 	flag.Parse()
 
 	if namespace == "" {
 		klog.Fatalf("must provide the namespace in which the controller is running through --namespace")
 	}
 
+	if exclusionRegexStr != "" {
+		var err error
+		replicator.ExclusionRegex, err = regexp.Compile(exclusionRegexStr)
+		if err != nil {
+			klog.Fatalf("failed to compile exclusion regex: %s", err.Error())
+		}
+	}
+
 	if err := k8s.Load(kubeconfig); err != nil {
 		klog.Fatalf("failed to load kubernetes configuration: %s", err.Error())
 	}

internal/replicator/informers.go

@@ -72,7 +72,18 @@ func (c *Controller) createPvcInformer(factory informers.SharedInformerFactory)
 			c.pvcUpdate(newObj.(*corev1.PersistentVolumeClaim))
 		},
 		DeleteFunc: func(obj any) {
-			c.pvcUpdate(obj.(*corev1.PersistentVolumeClaim))
+			pvc, ok := obj.(*corev1.PersistentVolumeClaim)
+			if !ok {
+				tombstone, ok := obj.(cache.DeletedFinalStateUnknown)
+				if !ok {
+					return
+				}
+				pvc, ok = tombstone.Obj.(*corev1.PersistentVolumeClaim)
+				if !ok {
+					return
+				}
+			}
+			c.pvcUpdate(pvc)
 		},
 	})
 }
@@ -87,7 +98,18 @@ func (c *Controller) createVolumeReplicationInformer(factory dynamicinformer.Dyn
 			c.volumeReplicationUpdate(oldObj.(*unstructured.Unstructured), newObj.(*unstructured.Unstructured))
 		},
 		DeleteFunc: func(obj any) {
-			c.volumeReplicationCreateOrDelete(obj.(*unstructured.Unstructured))
+			vr, ok := obj.(*unstructured.Unstructured)
+			if !ok {
+				tombstone, ok := obj.(cache.DeletedFinalStateUnknown)
+				if !ok {
+					return
+				}
+				vr, ok = tombstone.Obj.(*unstructured.Unstructured)
+				if !ok {
+					return
+				}
+			}
+			c.volumeReplicationCreateOrDelete(vr)
 		},
 	})
 }

internal/replicator/utils.go

@@ -15,7 +15,7 @@ import (
 	"k8s.io/klog/v2"
 )
 
-var ExclusionRegex string
+var ExclusionRegex *regexp.Regexp
 
 // isVolumeReplicationCorrect verifies if the definition of a VolumeReplication conforms to its originating PVC
 func isVolumeReplicationCorrect(pvc *corev1.PersistentVolumeClaim, vr *unstructured.Unstructured) bool {
@@ -197,17 +197,10 @@ func isNamespacePaused(namespace string) bool {
 // pvcNameMatchesExclusion returns whether a PVC has a name matching the exclusion regex
 func pvcNameMatchesExclusion(pvc *corev1.PersistentVolumeClaim) bool {
 	// If no regex is provided, return that it doesn't match
-	// This is to avoid Go matching "" as "everything matches"
-	if ExclusionRegex == "" {
+	if ExclusionRegex == nil {
 		return false
 	}
 
 	// Match the user-provided regex
-	match, err := regexp.MatchString(ExclusionRegex, pvc.Name)
-	if err != nil {
-		klog.Errorf("failed to parse exclusion regex: %s", err.Error())
-		return false
-	}
-
-	return match
+	return ExclusionRegex.MatchString(pvc.Name)
 }

internal/replicator/utils_test.go

@@ -2,6 +2,7 @@ package replicator
 
 import (
 	"fmt"
+	"regexp"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -940,7 +941,19 @@ func TestPvcNameMatchesExclusion(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			ExclusionRegex = tt.exclusionRegex
+			if tt.exclusionRegex != "" {
+				var err error
+				ExclusionRegex, err = regexp.Compile(tt.exclusionRegex)
+				if tt.name != "Invalid regex" {
+					require.NoError(t, err)
+				} else {
+					require.Error(t, err)
+					ExclusionRegex = nil // Simulate what happens when it fails (though main would exit)
+				}
+			} else {
+				ExclusionRegex = nil
+			}
+
 			pvc := &corev1.PersistentVolumeClaim{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: tt.pvcName,

internal/replicator/vrc_test.go

@@ -2,6 +2,7 @@ package replicator
 
 import (
 	"fmt"
+	"regexp"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -211,11 +212,11 @@ func TestGetVolumeReplicationClass(t *testing.T) {
 
 			// Set ExclusionRegex specifically for each test case
 			if tt.name == "Excluded by name regex" {
-				ExclusionRegex = "exclude-.*"
+				ExclusionRegex = regexp.MustCompile("exclude-.*")
 			} else {
-				ExclusionRegex = ""
+				ExclusionRegex = nil
 			}
-			defer func() { ExclusionRegex = "" }()
+			defer func() { ExclusionRegex = nil }()
 
 			if tt.namespace != nil {
 				err := NamespaceInformer.Informer().GetIndexer().Add(tt.namespace)