diff --git a/internal/pkg/handler/upgrade.go b/internal/pkg/handler/upgrade.go index a4870403..5570ee2d 100644 --- a/internal/pkg/handler/upgrade.go +++ b/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 diff --git a/pkg/common/common.go b/pkg/common/common.go index de37ad86..25017e3f 100644 --- a/pkg/common/common.go +++ b/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, } } diff --git a/pkg/common/common_test.go b/pkg/common/common_test.go index 532d3adf..ba251ce3 100644 --- a/pkg/common/common_test.go +++ b/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) + } +}