Surface invalid-regex errors to the call site with workload context

Per review: instead of logging the skipped pattern inside ShouldReload
(which lacks workload identity), collect the compile errors on
ReloadCheckResult.Errors and log them at the upgrade call site, where the
resource name, type, and namespace are known. Add a test for the
multi-value case where one comma-separated pattern is malformed and a
valid one still matches.
This commit is contained in:
Nikolaus Schuetz
2026-08-01 21:21:02 -07:00
parent 54e6b1c44f
commit 4471a45591
3 changed files with 41 additions and 1 deletions
+4
View File
@@ -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
+8 -1
View File
@@ -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,
}
}
+29
View File
@@ -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)
}
}