some refactoring and cleanup

This commit is contained in:
Safwan
2026-07-15 12:33:40 +05:00
parent e9e09e8900
commit 6195dd91e1
12 changed files with 263 additions and 151 deletions
-22
View File
@@ -201,25 +201,3 @@ func (c *Config) IsNamespaceIgnored(namespace string) bool {
func (c *Config) IsGlobalMode() bool {
return len(c.WatchedNamespaces) == 0
}
// ApplyNamespaceScope enforces master-parity semantics: namespace-selector and
// namespaces-to-ignore are only honored in global (all-namespaces) mode. In
// scoped or single-namespace mode the watched set is already explicit, so both
// are cleared. It returns human-readable warnings for any setting it dropped so
// the caller can log them.
func (c *Config) ApplyNamespaceScope() []string {
if c.IsGlobalMode() {
return nil
}
var warnings []string
if len(c.NamespaceSelectors) > 0 {
warnings = append(warnings, "namespace-selector is set but is only honored in global mode; ignoring it")
c.NamespaceSelectors = nil
c.NamespaceSelectorStrings = nil
}
if len(c.IgnoredNamespaces) > 0 {
warnings = append(warnings, "namespaces-to-ignore is set but is only honored in global mode; ignoring it")
c.IgnoredNamespaces = nil
}
return warnings
}
-33
View File
@@ -3,8 +3,6 @@ package config
import (
"testing"
"time"
"k8s.io/apimachinery/pkg/labels"
)
func TestNewDefault(t *testing.T) {
@@ -231,34 +229,3 @@ func TestIsGlobalMode(t *testing.T) {
t.Errorf("non-empty WatchedNamespaces should not be global mode")
}
}
func TestApplyNamespaceScope_GlobalKeepsSettings(t *testing.T) {
c := &Config{
WatchedNamespaces: nil,
IgnoredNamespaces: []string{"kube-system"},
NamespaceSelectors: []labels.Selector{labels.Everything()},
}
warnings := c.ApplyNamespaceScope()
if len(warnings) != 0 {
t.Errorf("global mode should produce no warnings, got %v", warnings)
}
if len(c.IgnoredNamespaces) != 1 || len(c.NamespaceSelectors) != 1 {
t.Errorf("global mode should keep selectors and ignored namespaces")
}
}
func TestApplyNamespaceScope_ScopedClearsSettings(t *testing.T) {
c := &Config{
WatchedNamespaces: []string{"team-a"},
IgnoredNamespaces: []string{"kube-system"},
NamespaceSelectors: []labels.Selector{labels.Everything()},
NamespaceSelectorStrings: []string{"env=prod"},
}
warnings := c.ApplyNamespaceScope()
if len(warnings) != 2 {
t.Errorf("scoped mode should warn about both dropped settings, got %v", warnings)
}
if len(c.IgnoredNamespaces) != 0 || len(c.NamespaceSelectors) != 0 || len(c.NamespaceSelectorStrings) != 0 {
t.Errorf("scoped mode should clear selectors and ignored namespaces")
}
}
+28 -11
View File
@@ -5,6 +5,7 @@ import (
"strings"
"time"
"github.com/go-logr/logr"
"github.com/spf13/pflag"
"github.com/spf13/viper"
"k8s.io/apimachinery/pkg/labels"
@@ -264,11 +265,17 @@ func BindFlags(fs *pflag.FlagSet, cfg *Config) {
_ = v.BindEnv("alert-proxy", "ALERT_PROXY", "ALERT_WEBHOOK_PROXY")
}
// LoggingFlags returns the log format and level from parsed flags/env. The
// caller uses these to configure logging before ApplyFlags runs, so ApplyFlags
// can log warnings through a ready logger.
func LoggingFlags() (format, level string) {
return v.GetString("log-format"), v.GetString("log-level")
}
// ApplyFlags applies flag values from viper to the config struct. Call this
// after parsing flags. It returns any human-readable warnings produced while
// finalizing namespace scope (see ApplyNamespaceScope) so the caller can log
// them once a logger is available.
func ApplyFlags(cfg *Config) ([]string, error) {
// after parsing flags. It finalizes namespace scope and logs any warnings it
// produces through the given logger.
func ApplyFlags(cfg *Config, log logr.Logger) error {
// Boolean flags
cfg.AutoReloadAll = v.GetBool("auto-reload-all")
cfg.SyncAfterRestart = v.GetBool("sync-after-restart")
@@ -367,7 +374,7 @@ func ApplyFlags(cfg *Config) ([]string, error) {
joinedNS := strings.Join(nsSelectors, ",")
selector, err := labels.Parse(joinedNS)
if err != nil {
return nil, fmt.Errorf("invalid selector %q: %w", joinedNS, err)
return fmt.Errorf("invalid selector %q: %w", joinedNS, err)
}
cfg.NamespaceSelectors = []labels.Selector{selector}
}
@@ -375,7 +382,7 @@ func ApplyFlags(cfg *Config) ([]string, error) {
joinedRes := strings.Join(resSelectors, ",")
selector, err := labels.Parse(joinedRes)
if err != nil {
return nil, fmt.Errorf("invalid selector %q: %w", joinedRes, err)
return fmt.Errorf("invalid selector %q: %w", joinedRes, err)
}
cfg.ResourceSelectors = []labels.Selector{selector}
}
@@ -391,11 +398,21 @@ func ApplyFlags(cfg *Config) ([]string, error) {
cfg.LeaderElection.RetryPeriod = 2 * time.Second
}
// Enforce namespace-scope semantics here so the finalized config is
// self-consistent for every caller: selector/ignore lists are only honored
// in global mode. Warnings are returned for the caller to log once logging
// is set up.
return cfg.ApplyNamespaceScope(), nil
// Namespace-selector and namespaces-to-ignore are only honored in global
// mode; in scoped or single-namespace mode the watched set is already
// explicit, so drop them and log where it happens.
if !cfg.IsGlobalMode() {
if len(cfg.NamespaceSelectors) > 0 {
log.Info("namespace-selector is set but is only honored in global mode; ignoring it")
cfg.NamespaceSelectors = nil
cfg.NamespaceSelectorStrings = nil
}
if len(cfg.IgnoredNamespaces) > 0 {
log.Info("namespaces-to-ignore is set but is only honored in global mode; ignoring it")
cfg.IgnoredNamespaces = nil
}
}
return nil
}
// parseBoolString parses a string as a boolean, defaulting to false.
+25 -32
View File
@@ -4,6 +4,7 @@ import (
"strings"
"testing"
"github.com/go-logr/logr"
"github.com/spf13/pflag"
"github.com/spf13/viper"
)
@@ -92,7 +93,7 @@ func TestBindFlags_DefaultValues(t *testing.T) {
t.Fatalf("Parse() error = %v", err)
}
if _, err := ApplyFlags(cfg); err != nil {
if err := ApplyFlags(cfg, logr.Discard()); err != nil {
t.Fatalf("ApplyFlags() error = %v", err)
}
@@ -126,7 +127,7 @@ func TestBindFlags_CustomValues(t *testing.T) {
t.Fatalf("Parse() error = %v", err)
}
if _, err := ApplyFlags(cfg); err != nil {
if err := ApplyFlags(cfg, logr.Discard()); err != nil {
t.Fatalf("ApplyFlags() error = %v", err)
}
@@ -168,7 +169,7 @@ func TestApplyFlags_SecretProviderClassAnnotations(t *testing.T) {
if err := fs.Parse(nil); err != nil {
t.Fatalf("Parse() error = %v", err)
}
if _, err := ApplyFlags(cfg); err != nil {
if err := ApplyFlags(cfg, logr.Discard()); err != nil {
t.Fatalf("ApplyFlags() error = %v", err)
}
defaults := DefaultAnnotations()
@@ -195,7 +196,7 @@ func TestApplyFlags_SecretProviderClassAnnotations(t *testing.T) {
if err := fs.Parse(args); err != nil {
t.Fatalf("Parse() error = %v", err)
}
if _, err := ApplyFlags(cfg); err != nil {
if err := ApplyFlags(cfg, logr.Discard()); err != nil {
t.Fatalf("ApplyFlags() error = %v", err)
}
if cfg.Annotations.SecretProviderClassAuto != "spc.example.com/auto" {
@@ -218,7 +219,7 @@ func TestApplyFlags_ExcludeAnnotations(t *testing.T) {
if err := fs.Parse(nil); err != nil {
t.Fatalf("Parse() error = %v", err)
}
if _, err := ApplyFlags(cfg); err != nil {
if err := ApplyFlags(cfg, logr.Discard()); err != nil {
t.Fatalf("ApplyFlags() error = %v", err)
}
defaults := DefaultAnnotations()
@@ -241,7 +242,7 @@ func TestApplyFlags_ExcludeAnnotations(t *testing.T) {
if err := fs.Parse(args); err != nil {
t.Fatalf("Parse() error = %v", err)
}
if _, err := ApplyFlags(cfg); err != nil {
if err := ApplyFlags(cfg, logr.Discard()); err != nil {
t.Fatalf("ApplyFlags() error = %v", err)
}
if cfg.Annotations.ConfigmapExclude != "cm.example.com/exclude" {
@@ -261,7 +262,7 @@ func TestApplyFlags_IgnoreAnnotation(t *testing.T) {
if err := fs.Parse(nil); err != nil {
t.Fatalf("Parse() error = %v", err)
}
if _, err := ApplyFlags(cfg); err != nil {
if err := ApplyFlags(cfg, logr.Discard()); err != nil {
t.Fatalf("ApplyFlags() error = %v", err)
}
if cfg.Annotations.Ignore != DefaultAnnotations().Ignore {
@@ -276,7 +277,7 @@ func TestApplyFlags_IgnoreAnnotation(t *testing.T) {
if err := fs.Parse([]string{"--ignore-annotation=my.company.com/reloader-ignore"}); err != nil {
t.Fatalf("Parse() error = %v", err)
}
if _, err := ApplyFlags(cfg); err != nil {
if err := ApplyFlags(cfg, logr.Discard()); err != nil {
t.Fatalf("ApplyFlags() error = %v", err)
}
if cfg.Annotations.Ignore != "my.company.com/reloader-ignore" {
@@ -313,7 +314,7 @@ func TestApplyFlags_BooleanStrings(t *testing.T) {
t.Fatalf("Parse() error = %v", err)
}
_, err := ApplyFlags(cfg)
err := ApplyFlags(cfg, logr.Discard())
if (err != nil) != tt.wantErr {
t.Errorf("ApplyFlags() error = %v, wantErr %v", err, tt.wantErr)
return
@@ -343,7 +344,7 @@ func TestApplyFlags_CommaSeparatedLists(t *testing.T) {
t.Fatalf("Parse() error = %v", err)
}
if _, err := ApplyFlags(cfg); err != nil {
if err := ApplyFlags(cfg, logr.Discard()); err != nil {
t.Fatalf("ApplyFlags() error = %v", err)
}
@@ -378,7 +379,7 @@ func TestApplyFlags_Selectors(t *testing.T) {
t.Fatalf("Parse() error = %v", err)
}
if _, err := ApplyFlags(cfg); err != nil {
if err := ApplyFlags(cfg, logr.Discard()); err != nil {
t.Fatalf("ApplyFlags() error = %v", err)
}
@@ -409,7 +410,7 @@ func TestApplyFlags_InvalidSelector(t *testing.T) {
t.Fatalf("Parse() error = %v", err)
}
_, err := ApplyFlags(cfg)
err := ApplyFlags(cfg, logr.Discard())
if err == nil {
t.Error("ApplyFlags() should return error for invalid selector")
}
@@ -461,7 +462,7 @@ func TestApplyFlags_AlertingEnvVars(t *testing.T) {
t.Fatalf("Parse() error = %v", err)
}
if _, err := ApplyFlags(cfg); err != nil {
if err := ApplyFlags(cfg, logr.Discard()); err != nil {
t.Fatalf("ApplyFlags() error = %v", err)
}
@@ -494,7 +495,7 @@ func TestApplyFlags_LegacyProxyEnvVar(t *testing.T) {
t.Fatalf("Parse() error = %v", err)
}
if _, err := ApplyFlags(cfg); err != nil {
if err := ApplyFlags(cfg, logr.Discard()); err != nil {
t.Fatalf("ApplyFlags() error = %v", err)
}
@@ -511,7 +512,7 @@ func TestApplyFlagsCSIIntegration(t *testing.T) {
if err := fs.Parse([]string{"--enable-csi-integration=true"}); err != nil {
t.Fatal(err)
}
if _, err := ApplyFlags(cfg); err != nil {
if err := ApplyFlags(cfg, logr.Discard()); err != nil {
t.Fatal(err)
}
if !cfg.CSIIntegrationEnabled {
@@ -592,7 +593,7 @@ func TestApplyFlags_NamespacesScoped(t *testing.T) {
if err := fs.Parse([]string{"--namespaces=team-a,team-b"}); err != nil {
t.Fatalf("Parse() error = %v", err)
}
if _, err := ApplyFlags(cfg); err != nil {
if err := ApplyFlags(cfg, logr.Discard()); err != nil {
t.Fatalf("ApplyFlags() error = %v", err)
}
@@ -617,7 +618,7 @@ func TestApplyFlags_NamespacesFromEnv(t *testing.T) {
if err := fs.Parse([]string{}); err != nil {
t.Fatalf("Parse() error = %v", err)
}
if _, err := ApplyFlags(cfg); err != nil {
if err := ApplyFlags(cfg, logr.Discard()); err != nil {
t.Fatalf("ApplyFlags() error = %v", err)
}
@@ -636,7 +637,7 @@ func TestApplyFlags_NamespacesGlobal(t *testing.T) {
if err := fs.Parse([]string{}); err != nil {
t.Fatalf("Parse() error = %v", err)
}
if _, err := ApplyFlags(cfg); err != nil {
if err := ApplyFlags(cfg, logr.Discard()); err != nil {
t.Fatalf("ApplyFlags() error = %v", err)
}
@@ -657,7 +658,7 @@ func TestApplyFlags_NamespacesTrimsEmptyEntries(t *testing.T) {
if err := fs.Parse([]string{"--namespaces=team-a, ,team-b,"}); err != nil {
t.Fatalf("Parse() error = %v", err)
}
if _, err := ApplyFlags(cfg); err != nil {
if err := ApplyFlags(cfg, logr.Discard()); err != nil {
t.Fatalf("ApplyFlags() error = %v", err)
}
@@ -682,7 +683,7 @@ func TestApplyFlags_NamespacesAllEmptyIsGlobal(t *testing.T) {
if err := fs.Parse([]string{"--namespaces=, ,"}); err != nil {
t.Fatalf("Parse() error = %v", err)
}
if _, err := ApplyFlags(cfg); err != nil {
if err := ApplyFlags(cfg, logr.Discard()); err != nil {
t.Fatalf("ApplyFlags() error = %v", err)
}
@@ -695,8 +696,8 @@ func TestApplyFlags_NamespacesAllEmptyIsGlobal(t *testing.T) {
}
// ApplyFlags must finalize a self-consistent config: in scoped mode it enforces
// namespace-scope semantics (clears selector/ignore lists) and returns warnings,
// without the caller having to invoke ApplyNamespaceScope separately.
// namespace-scope semantics (clears selector/ignore lists) and logs a warning
// for each dropped setting.
func TestApplyFlags_ScopedClearsSelectorsAndIgnores(t *testing.T) {
resetViper()
cfg := NewDefault()
@@ -710,8 +711,7 @@ func TestApplyFlags_ScopedClearsSelectorsAndIgnores(t *testing.T) {
}); err != nil {
t.Fatalf("Parse() error = %v", err)
}
warnings, err := ApplyFlags(cfg)
if err != nil {
if err := ApplyFlags(cfg, logr.Discard()); err != nil {
t.Fatalf("ApplyFlags() error = %v", err)
}
@@ -721,9 +721,6 @@ func TestApplyFlags_ScopedClearsSelectorsAndIgnores(t *testing.T) {
if len(cfg.IgnoredNamespaces) != 0 {
t.Errorf("scoped mode should clear ignored namespaces, got %v", cfg.IgnoredNamespaces)
}
if len(warnings) != 2 {
t.Errorf("expected 2 scope warnings, got %v", warnings)
}
}
func TestApplyFlags_GlobalKeepsSelectorsNoWarnings(t *testing.T) {
@@ -739,8 +736,7 @@ func TestApplyFlags_GlobalKeepsSelectorsNoWarnings(t *testing.T) {
}); err != nil {
t.Fatalf("Parse() error = %v", err)
}
warnings, err := ApplyFlags(cfg)
if err != nil {
if err := ApplyFlags(cfg, logr.Discard()); err != nil {
t.Fatalf("ApplyFlags() error = %v", err)
}
@@ -750,7 +746,4 @@ func TestApplyFlags_GlobalKeepsSelectorsNoWarnings(t *testing.T) {
if len(cfg.NamespaceSelectors) != 1 || len(cfg.IgnoredNamespaces) != 1 {
t.Errorf("global mode should keep selectors and ignored namespaces")
}
if len(warnings) != 0 {
t.Errorf("global mode should produce no warnings, got %v", warnings)
}
}
+16 -3
View File
@@ -8,6 +8,8 @@ import (
"github.com/go-logr/logr"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/rest"
"sigs.k8s.io/controller-runtime/pkg/client"
"github.com/stakater/Reloader/internal/pkg/config"
@@ -77,10 +79,21 @@ func PublishMetaInfoConfigMap(ctx context.Context, c client.Client, cfg *config.
return publisher.Publish(ctx)
}
// Runnable returns a controller-runtime Runnable that publishes the metadata ConfigMap
// when the manager starts. This ensures the cache is ready before accessing the API.
func Runnable(c client.Client, cfg *config.Config, log logr.Logger) RunnableFunc {
// Runnable returns a controller-runtime Runnable that publishes the meta-info
// ConfigMap when the manager starts. It builds its own uncached client from the
// given rest config and scheme: the ConfigMap lives in Reloader's own namespace,
// which the manager cache does not cover in scoped mode, so a cache-backed client
// cannot read or write it there. Meta-info is internal instance metadata and is
// always published regardless of which resources are watched.
func Runnable(restConfig *rest.Config, scheme *runtime.Scheme, cfg *config.Config, log logr.Logger) RunnableFunc {
return func(ctx context.Context) error {
c, err := client.New(restConfig, client.Options{Scheme: scheme})
if err != nil {
log.Error(err, "Failed to create client for meta info configmap publisher")
// Non-fatal, don't return error to avoid crashing the manager
<-ctx.Done()
return nil
}
if err := PublishMetaInfoConfigMap(ctx, c, cfg, log); err != nil {
log.Error(err, "Failed to create metadata ConfigMap")
// Non-fatal, don't return error to avoid crashing the manager