Merge pull request #1193 from nikolauspschuetz/fix/reload-annotation-invalid-regex-panic

fix: prevent panic on invalid regex in reload annotation

internal/pkg/handler/upgrade.go

@@ -310,6 +310,10 @@ func upgradeResource(clients kube.Clients, config common.Config, upgradeFuncs ca
 	podAnnotations := upgradeFuncs.PodAnnotationsFunc(resource)
 	result := common.ShouldReload(config, upgradeFuncs.ResourceType, annotations, podAnnotations, common.GetCommandLineOptions())
 
+	for _, reloadErr := range result.Errors {
+		logrus.Errorf("Skipping invalid reload annotation on %s '%s' in namespace '%s': %v", upgradeFuncs.ResourceType, resourceName, config.Namespace, reloadErr)
+	}
+
 	if !result.ShouldReload {
 		logrus.Debugf("No changes detected in '%s' of type '%s' in namespace '%s'", config.ResourceName, config.Type, config.Namespace)
 		return false, nil

pkg/common/common.go

@@ -2,6 +2,7 @@ package common
 
 import (
 	"context"
+	"fmt"
 	"os"
 	"regexp"
 	"strconv"
@@ -22,6 +23,7 @@ type Map map[string]string
 type ReloadCheckResult struct {
 	ShouldReload bool
 	AutoReload   bool
+	Errors       []error
 }
 
 // ReloaderOptions contains all configurable options for the Reloader controller.
@@ -273,14 +275,20 @@ func ShouldReload(config Config, resourceType string, annotations Map, podAnnota
 		}
 	}
 
+	var regexErrors []error
 	values := strings.Split(annotationValue, ",")
 	for _, value := range values {
 		value = strings.TrimSpace(value)
-		re := regexp.MustCompile("^" + value + "$")
+		re, err := regexp.Compile("^" + value + "$")
+		if err != nil {
+			regexErrors = append(regexErrors, fmt.Errorf("invalid regex %q in reload annotation %q: %w", value, config.Annotation, err))
+			continue
+		}
 		if re.Match([]byte(config.ResourceName)) {
 			return ReloadCheckResult{
 				ShouldReload: true,
 				AutoReload:   false,
+				Errors:       regexErrors,
 			}
 		}
 	}
@@ -291,6 +299,7 @@ func ShouldReload(config Config, resourceType string, annotations Map, podAnnota
 			return ReloadCheckResult{
 				ShouldReload: true,
 				AutoReload:   true,
+				Errors:       regexErrors,
 			}
 		}
 	}
@@ -301,11 +310,13 @@ func ShouldReload(config Config, resourceType string, annotations Map, podAnnota
 		return ReloadCheckResult{
 			ShouldReload: true,
 			AutoReload:   true,
+			Errors:       regexErrors,
 		}
 	}
 
 	return ReloadCheckResult{
 		ShouldReload: false,
+		Errors:       regexErrors,
 	}
 }
 

pkg/common/common_test.go

@@ -222,3 +222,56 @@ func TestShouldReload_IssueRBACPermissionFixed(t *testing.T) {
 		})
 	}
 }
+
+// A malformed regex in a named reload annotation must not panic the operator.
+// Regression test: previously regexp.MustCompile("^"+value+"$") panicked on an
+// invalid pattern, crashing Reloader cluster-wide (no recover on the worker).
+func TestShouldReload_InvalidRegexAnnotation_DoesNotPanic(t *testing.T) {
+	config := Config{
+		ResourceName: "app-config",
+		Annotation:   "secret.reloader.stakater.com/reload",
+	}
+	annotations := Map{
+		// unbalanced bracket => invalid regex
+		"secret.reloader.stakater.com/reload": "app-config[",
+	}
+	opts := &ReloaderOptions{
+		ReloaderAutoAnnotation: "reloader.stakater.com/auto",
+	}
+
+	// Before the fix this panicked inside ShouldReload.
+	result := ShouldReload(config, "Deployment", annotations, Map{}, opts)
+
+	if result.ShouldReload {
+		t.Errorf("Expected ShouldReload=false for an invalid regex pattern, got=%v", result.ShouldReload)
+	}
+	if len(result.Errors) != 1 {
+		t.Errorf("Expected 1 surfaced regex error, got=%d: %v", len(result.Errors), result.Errors)
+	}
+}
+
+// When a named reload annotation holds several comma-separated patterns, a
+// single malformed one is skipped while a valid one still matches, and the
+// skipped pattern's error is surfaced on the result.
+func TestShouldReload_InvalidRegexAnnotation_SkipsMalformedPattern(t *testing.T) {
+	config := Config{
+		ResourceName: "app-config",
+		Annotation:   "secret.reloader.stakater.com/reload",
+	}
+	annotations := Map{
+		// first pattern is invalid (unbalanced bracket), second matches
+		"secret.reloader.stakater.com/reload": "bad[,app-config",
+	}
+	opts := &ReloaderOptions{
+		ReloaderAutoAnnotation: "reloader.stakater.com/auto",
+	}
+
+	result := ShouldReload(config, "Deployment", annotations, Map{}, opts)
+
+	if !result.ShouldReload {
+		t.Errorf("Expected ShouldReload=true from the valid pattern, got=%v", result.ShouldReload)
+	}
+	if len(result.Errors) != 1 {
+		t.Errorf("Expected the malformed pattern to surface 1 error, got=%d: %v", len(result.Errors), result.Errors)
+	}
+}