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 45de1f19..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,18 +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, err := regexp.Compile("^" + value + "$") if err != nil { - logrus.Errorf("Invalid regex %q in reload annotation %q on resource '%s' of type '%s'; skipping this pattern: %v", value, config.Annotation, config.ResourceName, config.Type, err) + 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, } } } @@ -295,6 +299,7 @@ func ShouldReload(config Config, resourceType string, annotations Map, podAnnota return ReloadCheckResult{ ShouldReload: true, AutoReload: true, + Errors: regexErrors, } } } @@ -305,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 0bbdab54..ba251ce3 100644 --- a/pkg/common/common_test.go +++ b/pkg/common/common_test.go @@ -245,4 +245,33 @@ func TestShouldReload_InvalidRegexAnnotation_DoesNotPanic(t *testing.T) { 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) + } }