From 0cfbe10125f9d36c3131a352049d1f156cdeea90 Mon Sep 17 00:00:00 2001 From: Safwan Date: Tue, 14 Jul 2026 11:10:48 +0500 Subject: [PATCH 1/8] resync v2 chart with master --- .../kubernetes/chart/reloader/README.md | 3 +- .../chart/reloader/templates/_helpers.tpl | 130 ++++++++++++++++++ .../chart/reloader/templates/clusterrole.yaml | 7 +- .../chart/reloader/templates/deployment.yaml | 43 ++---- .../chart/reloader/templates/role.yaml | 130 ++++-------------- .../chart/reloader/templates/rolebinding.yaml | 37 ++++- .../kubernetes/chart/reloader/values.yaml | 18 +-- 7 files changed, 218 insertions(+), 150 deletions(-) diff --git a/deployments/kubernetes/chart/reloader/README.md b/deployments/kubernetes/chart/reloader/README.md index 3e7df6481..7b453ac47 100644 --- a/deployments/kubernetes/chart/reloader/README.md +++ b/deployments/kubernetes/chart/reloader/README.md @@ -56,11 +56,12 @@ helm uninstall {{RELEASE_NAME}} -n {{NAMESPACE}} | `reloader.reloadOnDelete` | Enable reload on delete events. Valid value are either `true` or `false` | boolean | `false` | | `reloader.syncAfterRestart` | Enable sync after Reloader restarts for **Add** events, works only when reloadOnCreate is `true`. Valid value are either `true` or `false` | boolean | `false` | | `reloader.reloadStrategy` | Strategy to trigger resource restart, set to either `default`, `env-vars` or `annotations` | enumeration | `default` | -| `reloader.ignoreNamespaces` | List of comma separated namespaces to ignore, if multiple are provided, they are combined with the AND operator | string | `""` | +| `reloader.ignoreNamespaces` | List of comma separated namespaces to ignore, if multiple are provided, they are combined with the AND operator. Only honored when `reloader.watchGlobally` is `true`; in single-namespace and scoped (`reloader.namespaces`) modes the watched set is already explicit and this value is ignored. | string | `""` | | `reloader.namespaceSelector` | List of comma separated k8s label selectors for namespaces selection. The parameter only used when `reloader.watchGlobally` is `true`. See [LIST and WATCH filtering](https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#list-and-watch-filtering) for more details on label-selector | string | `""` | | `reloader.resourceLabelSelector` | List of comma separated label selectors, if multiple are provided they are combined with the AND operator | string | `""` | | `reloader.logFormat` | Set type of log format. Value could be either `json` or `""` | string | `""` | | `reloader.watchGlobally` | Allow Reloader to watch in all namespaces (`true`) or just in a single namespace (`false`) | boolean | `true` | +| `reloader.namespaces` | Explicit namespaces to watch (scoped mode). When non-empty and `reloader.watchGlobally` is `false`, Reloader watches exactly these namespaces and the chart creates a namespace-scoped Role + RoleBinding in each (no ClusterRole). The release namespace is always included automatically. Accepts either a YAML list (`["team-a","team-b"]`) or a comma-separated string (`"team-a,team-b"`). | list/string | `[]` | | `reloader.enableHA` | Enable leadership election allowing you to run multiple replicas | boolean | `false` | | `reloader.enablePProf` | Enables pprof for profiling | boolean | `false` | | `reloader.pprofAddr` | Address to start pprof server on | string | `:6060` | diff --git a/deployments/kubernetes/chart/reloader/templates/_helpers.tpl b/deployments/kubernetes/chart/reloader/templates/_helpers.tpl index 831be8d30..306da2d72 100644 --- a/deployments/kubernetes/chart/reloader/templates/_helpers.tpl +++ b/deployments/kubernetes/chart/reloader/templates/_helpers.tpl @@ -88,6 +88,136 @@ Create the namespace selector if it does not watch globally {{- end -}} {{- end -}} +{{/* +Effective set of namespaces to watch in scoped mode: the release namespace +(always included so the meta-info ConfigMap, HA leases and events keep working) +plus the user-supplied reloader.namespaces, deduped and sorted. +Returns a JSON-encoded list; consumers use mustFromJson to iterate. +*/}} +{{- define "reloader-watchNamespaces" -}} +{{- $relNs := .Values.namespace | default .Release.Namespace -}} +{{- $ns := .Values.reloader.namespaces | default list -}} +{{- if kindIs "string" $ns -}} +{{- $ns = splitList "," $ns -}} +{{- end -}} +{{- $clean := list -}} +{{- range $ns -}} +{{- $t := . | toString | trim -}} +{{- if $t -}} +{{- $clean = append $clean $t -}} +{{- end -}} +{{- end -}} +{{- $all := concat (list $relNs) $clean | uniq | sortAlpha -}} +{{- $all | toJson -}} +{{- end -}} + +{{/* +Comma-joined form of reloader-watchNamespaces, for the --namespaces CLI flag. +*/}} +{{- define "reloader-watchNamespaces-csv" -}} +{{- include "reloader-watchNamespaces" . | mustFromJson | join "," -}} +{{- end -}} + +{{/* +The namespaced RBAC rules granted to Reloader in every watched namespace. +Shared between the single-namespace Role and the per-namespace scoped Roles so +the rule set is defined once. Expects the root context ($) as its argument. +*/}} +{{- define "reloader-namespaced-rules" }} + - apiGroups: + - "" + resources: +{{- if .Values.reloader.ignoreSecrets }}{{- else }} + - secrets +{{- end }} +{{- if .Values.reloader.ignoreConfigMaps }}{{- else }} + - configmaps +{{- end }} + verbs: + - list + - get + - watch +{{- if and (.Capabilities.APIVersions.Has "apps.openshift.io/v1") (.Values.reloader.isOpenshift) }} + - apiGroups: + - "apps.openshift.io" + - "" + resources: + - deploymentconfigs + verbs: + - list + - get + - update + - patch +{{- end }} +{{- if and (.Capabilities.APIVersions.Has "argoproj.io/v1alpha1") (.Values.reloader.isArgoRollouts) }} + - apiGroups: + - "argoproj.io" + - "" + resources: + - rollouts + verbs: + - list + - get + - update + - patch +{{- end }} + - apiGroups: + - "apps" + resources: + - deployments + - daemonsets + - statefulsets + verbs: + - list + - get + - update + - patch + - apiGroups: + - "batch" + resources: + - cronjobs + verbs: + - list + - get + - apiGroups: + - "batch" + resources: + - jobs + verbs: + - create + - delete + - list + - get +{{- if .Values.reloader.enableHA }} + - apiGroups: + - "coordination.k8s.io" + resources: + - leases + verbs: + - create + - get + - update +{{- end}} +{{- if .Values.reloader.enableCSIIntegration }} + - apiGroups: + - "secrets-store.csi.x-k8s.io" + resources: + - secretproviderclasspodstatuses + - secretproviderclasses + verbs: + - list + - get + - watch +{{- end}} + - apiGroups: + - "" + resources: + - events + verbs: + - create + - patch +{{- end -}} + {{/* Normalizes global.imagePullSecrets to a list of objects with name fields. Supports both of these in values.yaml: diff --git a/deployments/kubernetes/chart/reloader/templates/clusterrole.yaml b/deployments/kubernetes/chart/reloader/templates/clusterrole.yaml index 8f31764fa..bd14dfeb7 100644 --- a/deployments/kubernetes/chart/reloader/templates/clusterrole.yaml +++ b/deployments/kubernetes/chart/reloader/templates/clusterrole.yaml @@ -56,12 +56,12 @@ rules: {{- if and (.Capabilities.APIVersions.Has "argoproj.io/v1alpha1") (.Values.reloader.isArgoRollouts) }} - apiGroups: - "argoproj.io" + - "" resources: - rollouts verbs: - list - get - - watch - update - patch {{- end }} @@ -76,7 +76,6 @@ rules: - get - update - patch - - watch {{- if .Values.reloader.ignoreCronJobs }}{{- else }} - apiGroups: - "batch" @@ -85,9 +84,6 @@ rules: verbs: - list - get - - watch - - update - - patch {{- end }} {{- if .Values.reloader.ignoreJobs }}{{- else }} - apiGroups: @@ -99,7 +95,6 @@ rules: - delete - list - get - - watch {{- end}} {{- if .Values.reloader.enableHA }} - apiGroups: diff --git a/deployments/kubernetes/chart/reloader/templates/deployment.yaml b/deployments/kubernetes/chart/reloader/templates/deployment.yaml index fb6a8bb72..b3ac972e1 100644 --- a/deployments/kubernetes/chart/reloader/templates/deployment.yaml +++ b/deployments/kubernetes/chart/reloader/templates/deployment.yaml @@ -144,7 +144,7 @@ spec: fieldPath: {{ $value | quote}} {{- end }} {{- end }} - {{- if eq .Values.reloader.watchGlobally false }} + {{- if and (eq .Values.reloader.watchGlobally false) (not .Values.reloader.namespaces) }} - name: KUBERNETES_NAMESPACE valueFrom: fieldRef: @@ -176,12 +176,10 @@ spec: ports: - name: http containerPort: 9090 - - name: health - containerPort: 8080 livenessProbe: httpGet: - path: /healthz - port: health + path: /live + port: http timeoutSeconds: {{ .Values.reloader.deployment.livenessProbe.timeoutSeconds | default "5" }} failureThreshold: {{ .Values.reloader.deployment.livenessProbe.failureThreshold | default "5" }} periodSeconds: {{ .Values.reloader.deployment.livenessProbe.periodSeconds | default "10" }} @@ -189,8 +187,8 @@ spec: initialDelaySeconds: {{ .Values.reloader.deployment.livenessProbe.initialDelaySeconds | default "10" }} readinessProbe: httpGet: - path: /readyz - port: health + path: /metrics + port: http timeoutSeconds: {{ .Values.reloader.deployment.readinessProbe.timeoutSeconds | default "5" }} failureThreshold: {{ .Values.reloader.deployment.readinessProbe.failureThreshold | default "5" }} periodSeconds: {{ .Values.reloader.deployment.readinessProbe.periodSeconds | default "10" }} @@ -215,7 +213,7 @@ spec: {{- . | toYaml | nindent 10 }} {{- end }} {{- end }} - {{- if or (.Values.reloader.logFormat) (.Values.reloader.logLevel) (.Values.reloader.ignoreSecrets) (.Values.reloader.ignoreNamespaces) (include "reloader-namespaceSelector" .) (.Values.reloader.resourceLabelSelector) (.Values.reloader.ignoreConfigMaps) (.Values.reloader.custom_annotations) (eq .Values.reloader.isArgoRollouts true) (eq .Values.reloader.reloadOnCreate true) (eq .Values.reloader.reloadOnDelete true) (ne .Values.reloader.reloadStrategy "default") (.Values.reloader.enableHA) (.Values.reloader.autoReloadAll) (.Values.reloader.ignoreJobs) (.Values.reloader.ignoreCronJobs) (.Values.reloader.enableCSIIntegration)}} + {{- if or (.Values.reloader.logFormat) (.Values.reloader.logLevel) (.Values.reloader.ignoreSecrets) (and .Values.reloader.ignoreNamespaces .Values.reloader.watchGlobally) (.Values.reloader.namespaces) (include "reloader-namespaceSelector" .) (.Values.reloader.resourceLabelSelector) (.Values.reloader.ignoreConfigMaps) (.Values.reloader.custom_annotations) (eq .Values.reloader.isArgoRollouts true) (eq .Values.reloader.reloadOnCreate true) (eq .Values.reloader.reloadOnDelete true) (ne .Values.reloader.reloadStrategy "default") (.Values.reloader.enableHA) (.Values.reloader.autoReloadAll) (.Values.reloader.ignoreJobs) (.Values.reloader.ignoreCronJobs) (.Values.reloader.enableCSIIntegration)}} args: {{- if .Values.reloader.logFormat }} - "--log-format={{ .Values.reloader.logFormat }}" @@ -236,11 +234,14 @@ spec: {{- else if .Values.reloader.ignoreCronJobs }} - "--ignored-workload-types=cronjobs" {{- end }} - {{- if .Values.reloader.ignoreNamespaces }} + {{- if .Values.reloader.namespaces }} + - "--namespaces={{ include "reloader-watchNamespaces-csv" . }}" + {{- end }} + {{- if and .Values.reloader.ignoreNamespaces .Values.reloader.watchGlobally }} - "--namespaces-to-ignore={{ .Values.reloader.ignoreNamespaces }}" {{- end }} {{- if (include "reloader-namespaceSelector" .) }} - - "--namespace-selector={{ include "reloader-namespaceSelector" . }}" + - "--namespace-selector=\"{{ include "reloader-namespaceSelector" . }}\"" {{- end }} {{- if .Values.reloader.resourceLabelSelector }} - "--resource-label-selector={{ .Values.reloader.resourceLabelSelector }}" @@ -252,7 +253,7 @@ spec: {{- end }} {{- end }} {{- if .Values.reloader.enableCSIIntegration }} - - "--enable-csi-integration" + - "--enable-csi-integration=true" {{- end }} {{- if .Values.reloader.custom_annotations }} {{- if .Values.reloader.custom_annotations.configmap }} @@ -274,26 +275,6 @@ spec: {{- if .Values.reloader.custom_annotations.configmap_auto }} - "--configmap-auto-annotation" - "{{ .Values.reloader.custom_annotations.configmap_auto }}" - {{- end }} - {{- if .Values.reloader.custom_annotations.configmap_exclude }} - - "--configmap-exclude-annotation" - - "{{ .Values.reloader.custom_annotations.configmap_exclude }}" - {{- end }} - {{- if .Values.reloader.custom_annotations.secret_exclude }} - - "--secret-exclude-annotation" - - "{{ .Values.reloader.custom_annotations.secret_exclude }}" - {{- end }} - {{- if .Values.reloader.custom_annotations.secretproviderclass }} - - "--secretproviderclass-annotation" - - "{{ .Values.reloader.custom_annotations.secretproviderclass }}" - {{- end }} - {{- if .Values.reloader.custom_annotations.secretproviderclass_auto }} - - "--secretproviderclass-auto-annotation" - - "{{ .Values.reloader.custom_annotations.secretproviderclass_auto }}" - {{- end }} - {{- if .Values.reloader.custom_annotations.secretproviderclass_exclude }} - - "--secretproviderclass-exclude-annotation" - - "{{ .Values.reloader.custom_annotations.secretproviderclass_exclude }}" {{- end }} {{- if .Values.reloader.custom_annotations.search }} - "--auto-search-annotation" diff --git a/deployments/kubernetes/chart/reloader/templates/role.yaml b/deployments/kubernetes/chart/reloader/templates/role.yaml index 495a38552..28c535d62 100644 --- a/deployments/kubernetes/chart/reloader/templates/role.yaml +++ b/deployments/kubernetes/chart/reloader/templates/role.yaml @@ -1,9 +1,34 @@ +{{- if and .Values.reloader.watchGlobally .Values.reloader.namespaces }} +{{- fail "reloader.namespaces is set but reloader.watchGlobally is true; set reloader.watchGlobally=false to use scoped namespace mode." }} +{{- end }} {{- if and (not (.Values.reloader.watchGlobally)) (.Values.reloader.rbac.enabled) }} -{{- if (.Capabilities.APIVersions.Has "rbac.authorization.k8s.io/v1") }} -apiVersion: rbac.authorization.k8s.io/v1 -{{ else }} -apiVersion: rbac.authorization.k8s.io/v1beta1 +{{- $apiVersion := "rbac.authorization.k8s.io/v1" }} +{{- if not (.Capabilities.APIVersions.Has "rbac.authorization.k8s.io/v1") }} +{{- $apiVersion = "rbac.authorization.k8s.io/v1beta1" }} {{- end }} +{{- if .Values.reloader.namespaces }} +{{- range $ns := (include "reloader-watchNamespaces" . | mustFromJson) }} +apiVersion: {{ $apiVersion }} +kind: Role +metadata: + annotations: +{{ include "reloader-helm3.annotations" $ | indent 4 }} + labels: +{{ include "reloader-labels.chart" $ | indent 4 }} +{{- if $.Values.reloader.rbac.labels }} +{{ tpl (toYaml $.Values.reloader.rbac.labels) $ | indent 4 }} +{{- end }} +{{- if $.Values.reloader.matchLabels }} +{{ tpl (toYaml $.Values.reloader.matchLabels) $ | indent 4 }} +{{- end }} + name: {{ template "reloader-fullname" $ }}-role + namespace: {{ $ns }} +rules: +{{- include "reloader-namespaced-rules" $ }} +--- +{{- end }} +{{- else }} +apiVersion: {{ $apiVersion }} kind: Role metadata: annotations: @@ -19,103 +44,8 @@ metadata: name: {{ template "reloader-fullname" . }}-role namespace: {{ .Values.namespace | default .Release.Namespace }} rules: - - apiGroups: - - "" - resources: -{{- if .Values.reloader.ignoreSecrets }}{{- else }} - - secrets +{{- include "reloader-namespaced-rules" . }} {{- end }} -{{- if .Values.reloader.ignoreConfigMaps }}{{- else }} - - configmaps -{{- end }} - verbs: - - list - - get - - watch -{{- if and (.Capabilities.APIVersions.Has "apps.openshift.io/v1") (.Values.reloader.isOpenshift) }} - - apiGroups: - - "apps.openshift.io" - - "" - resources: - - deploymentconfigs - verbs: - - list - - get - - update - - patch -{{- end }} -{{- if and (.Capabilities.APIVersions.Has "argoproj.io/v1alpha1") (.Values.reloader.isArgoRollouts) }} - - apiGroups: - - "argoproj.io" - resources: - - rollouts - verbs: - - list - - get - - watch - - update - - patch -{{- end }} - - apiGroups: - - "apps" - resources: - - deployments - - daemonsets - - statefulsets - verbs: - - list - - get - - update - - patch - - watch - - apiGroups: - - "batch" - resources: - - cronjobs - verbs: - - list - - get - - watch - - update - - patch - - apiGroups: - - "batch" - resources: - - jobs - verbs: - - create - - delete - - list - - get - - watch -{{- if .Values.reloader.enableHA }} - - apiGroups: - - "coordination.k8s.io" - resources: - - leases - verbs: - - create - - get - - update -{{- end}} -{{- if .Values.reloader.enableCSIIntegration }} - - apiGroups: - - "secrets-store.csi.x-k8s.io" - resources: - - secretproviderclasspodstatuses - - secretproviderclasses - verbs: - - list - - get - - watch -{{- end}} - - apiGroups: - - "" - resources: - - events - verbs: - - create - - patch {{- end }} --- diff --git a/deployments/kubernetes/chart/reloader/templates/rolebinding.yaml b/deployments/kubernetes/chart/reloader/templates/rolebinding.yaml index 5cf4cf38b..7d73b1822 100644 --- a/deployments/kubernetes/chart/reloader/templates/rolebinding.yaml +++ b/deployments/kubernetes/chart/reloader/templates/rolebinding.yaml @@ -1,9 +1,37 @@ {{- if and (not (.Values.reloader.watchGlobally)) (.Values.reloader.rbac.enabled) }} -{{- if (.Capabilities.APIVersions.Has "rbac.authorization.k8s.io/v1") }} -apiVersion: rbac.authorization.k8s.io/v1 -{{ else }} -apiVersion: rbac.authorization.k8s.io/v1beta1 +{{- $apiVersion := "rbac.authorization.k8s.io/v1" }} +{{- if not (.Capabilities.APIVersions.Has "rbac.authorization.k8s.io/v1") }} +{{- $apiVersion = "rbac.authorization.k8s.io/v1beta1" }} {{- end }} +{{- if .Values.reloader.namespaces }} +{{- range $ns := (include "reloader-watchNamespaces" . | mustFromJson) }} +apiVersion: {{ $apiVersion }} +kind: RoleBinding +metadata: + annotations: +{{ include "reloader-helm3.annotations" $ | indent 4 }} + labels: +{{ include "reloader-labels.chart" $ | indent 4 }} +{{- if $.Values.reloader.rbac.labels }} +{{ tpl (toYaml $.Values.reloader.rbac.labels) $ | indent 4 }} +{{- end }} +{{- if $.Values.reloader.matchLabels }} +{{ tpl (toYaml $.Values.reloader.matchLabels) $ | indent 4 }} +{{- end }} + name: {{ template "reloader-fullname" $ }}-role-binding + namespace: {{ $ns }} +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: {{ template "reloader-fullname" $ }}-role +subjects: + - kind: ServiceAccount + name: {{ template "reloader-serviceAccountName" $ }} + namespace: {{ $.Values.namespace | default $.Release.Namespace }} +--- +{{- end }} +{{- else }} +apiVersion: {{ $apiVersion }} kind: RoleBinding metadata: annotations: @@ -27,6 +55,7 @@ subjects: name: {{ template "reloader-serviceAccountName" . }} namespace: {{ .Values.namespace | default .Release.Namespace }} {{- end }} +{{- end }} --- {{- if .Values.reloader.rbac.enabled }} diff --git a/deployments/kubernetes/chart/reloader/values.yaml b/deployments/kubernetes/chart/reloader/values.yaml index 1e867dc0c..15a22bcf8 100644 --- a/deployments/kubernetes/chart/reloader/values.yaml +++ b/deployments/kubernetes/chart/reloader/values.yaml @@ -19,7 +19,7 @@ fullnameOverride: "" image: name: stakater/reloader repository: ghcr.io/stakater/reloader - tag: v1.4.17 + tag: v1.4.19 # digest: sha256:1234567 pullPolicy: IfNotPresent @@ -45,11 +45,18 @@ reloader: logFormat: "" # json logLevel: info # Log level to use (trace, debug, info, warning, error, fatal and panic) watchGlobally: true + # Scoped mode: explicit list of namespaces to watch. When non-empty (and watchGlobally + # is false), Reloader watches exactly these namespaces and the chart creates a namespace + # scoped Role + RoleBinding in each one — no ClusterRole is created. The release namespace + # is always included automatically. Leave empty ([]) for the default single-namespace or + # global behavior controlled by watchGlobally. + # Accepts either a YAML list (e.g. ["team-a", "team-b"]) or a comma-separated string + # (e.g. "team-a,team-b") + namespaces: [] # Set to true to enable leadership election allowing you to run multiple replicas enableHA: false # Set to true to enable pprof for profiling enablePProf: false - # Set to true to enable CSI / SecretProviderClass integration enableCSIIntegration: false # Address to start pprof server on. Default is ":6060" pprofAddr: ":6060" @@ -136,7 +143,7 @@ reloader: labels: provider: stakater group: com.stakater.platform - version: v1.4.14 + version: v1.4.19 # Support for extra environment variables. env: # Open supports Key value pair as environment variables. @@ -216,11 +223,6 @@ reloader: # custom_annotations: # configmap: "my.company.com/configmap" # secret: "my.company.com/secret" - # configmap_exclude: "my.company.com/configmap-exclude" - # secret_exclude: "my.company.com/secret-exclude" - # secretproviderclass: "my.company.com/secretproviderclass" - # secretproviderclass_auto: "my.company.com/secretproviderclass-auto" - # secretproviderclass_exclude: "my.company.com/secretproviderclass-exclude" # ignore: "my.company.com/reloader-ignore" custom_annotations: {} From 0ef47cdc4d2e37e3b8ca3a149a9bf256e8dc06be Mon Sep 17 00:00:00 2001 From: Safwan Date: Tue, 14 Jul 2026 13:09:14 +0500 Subject: [PATCH 2/8] added watch namespaces support --- cmd/reloader/main.go | 12 ++- internal/pkg/config/config.go | 40 +++++++-- internal/pkg/config/config_test.go | 44 +++++++++ internal/pkg/config/flags.go | 40 +++++++-- internal/pkg/config/flags_test.go | 113 +++++++++++++++++++++++- internal/pkg/controller/manager.go | 28 ++++-- internal/pkg/controller/manager_test.go | 19 ++++ 7 files changed, 270 insertions(+), 26 deletions(-) diff --git a/cmd/reloader/main.go b/cmd/reloader/main.go index 35151f9ba..325352157 100644 --- a/cmd/reloader/main.go +++ b/cmd/reloader/main.go @@ -81,10 +81,16 @@ func run(cmd *cobra.Command, args []string) error { log.Info("Starting Reloader") - if cfg.WatchedNamespace != "" { - log.Info("watching single namespace", "namespace", cfg.WatchedNamespace) - } else { + // Enforce master-parity scope semantics before reconcilers/manager read the + // config: selector and ignore lists are only honored in global mode. + for _, w := range cfg.ApplyNamespaceScope() { + log.Info(w) + } + + if cfg.IsGlobalMode() { log.Info("watching all namespaces") + } else { + log.Info("watching scoped namespaces", "namespaces", cfg.WatchedNamespaces) } if len(cfg.NamespaceSelectors) > 0 { diff --git a/internal/pkg/config/config.go b/internal/pkg/config/config.go index b63611e79..726a6e8b2 100644 --- a/internal/pkg/config/config.go +++ b/internal/pkg/config/config.go @@ -54,10 +54,10 @@ type Config struct { EnablePProf bool `json:"enablePProf"` PProfAddr string `json:"pprofAddr,omitempty"` - Alerting AlertingConfig `json:"alerting"` - LeaderElection LeaderElectionConfig `json:"leaderElection"` - WatchedNamespace string `json:"watchedNamespace,omitempty"` - SyncPeriod time.Duration `json:"syncPeriod"` + Alerting AlertingConfig `json:"alerting"` + LeaderElection LeaderElectionConfig `json:"leaderElection"` + WatchedNamespaces []string `json:"watchedNamespaces,omitempty"` + SyncPeriod time.Duration `json:"syncPeriod"` } // AnnotationConfig holds customizable annotation keys. @@ -137,8 +137,8 @@ func NewDefault() *Config { RetryPeriod: 2 * time.Second, ReleaseOnCancel: true, }, - WatchedNamespace: "", - SyncPeriod: 0, + WatchedNamespaces: []string{}, + SyncPeriod: 0, } } @@ -195,3 +195,31 @@ func (c *Config) IsNamespaceIgnored(namespace string) bool { } return false } + +// IsGlobalMode reports whether Reloader watches all namespaces. Global mode is +// the absence of an explicit watched-namespace list. +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 +} diff --git a/internal/pkg/config/config_test.go b/internal/pkg/config/config_test.go index bc6ea0a11..62d1a610d 100644 --- a/internal/pkg/config/config_test.go +++ b/internal/pkg/config/config_test.go @@ -3,6 +3,8 @@ package config import ( "testing" "time" + + "k8s.io/apimachinery/pkg/labels" ) func TestNewDefault(t *testing.T) { @@ -218,3 +220,45 @@ func TestConfig_IsNamespaceIgnored(t *testing.T) { ) } } + +func TestIsGlobalMode(t *testing.T) { + c := &Config{WatchedNamespaces: nil} + if !c.IsGlobalMode() { + t.Errorf("empty WatchedNamespaces should be global mode") + } + c.WatchedNamespaces = []string{"team-a"} + if c.IsGlobalMode() { + 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") + } +} diff --git a/internal/pkg/config/flags.go b/internal/pkg/config/flags.go index 784fef108..969ffb59d 100644 --- a/internal/pkg/config/flags.go +++ b/internal/pkg/config/flags.go @@ -223,10 +223,11 @@ func BindFlags(fs *pflag.FlagSet, cfg *Config) { "Annotation to indicate when a deployment was paused by Reloader", ) - // Watched namespace (for single-namespace mode) - fs.String( - "watch-namespace", cfg.WatchedNamespace, - "Namespace to watch (empty for all namespaces)", + // Watched namespaces (scoped mode). Empty means watch all namespaces; + // KUBERNETES_NAMESPACE (env) is used as a single-namespace fallback. + fs.StringSlice( + "namespaces", nil, + "explicit list of namespaces to watch (scoped mode; creates no ClusterRole)", ) // Alerting @@ -294,9 +295,16 @@ func ApplyFlags(cfg *Config) error { cfg.MetricsAddr = v.GetString("metrics-addr") cfg.HealthAddr = v.GetString("health-addr") cfg.PProfAddr = v.GetString("pprof-addr") - cfg.WatchedNamespace = v.GetString("watch-namespace") - if cfg.WatchedNamespace == "" { - cfg.WatchedNamespace = v.GetString("KUBERNETES_NAMESPACE") + // Namespace scope: an explicit --namespaces list takes precedence (scoped + // mode); otherwise fall back to KUBERNETES_NAMESPACE for single-namespace + // mode; an empty result means global (all-namespaces) mode. + // Trim and drop empty entries from the slice to prevent empty strings from + // being treated as "watch all namespaces" by controller-runtime. + cfg.WatchedNamespaces = trimAndDropEmptyStrings(v.GetStringSlice("namespaces")) + if len(cfg.WatchedNamespaces) == 0 { + if ns := v.GetString("KUBERNETES_NAMESPACE"); ns != "" { + cfg.WatchedNamespaces = []string{ns} + } } // Leader election @@ -412,3 +420,21 @@ func splitAndTrim(s string) []string { } return result } + +// trimAndDropEmptyStrings trims whitespace from each string in a slice and drops empty entries. +func trimAndDropEmptyStrings(ss []string) []string { + if len(ss) == 0 { + return nil + } + result := make([]string, 0, len(ss)) + for _, s := range ss { + s = strings.TrimSpace(s) + if s != "" { + result = append(result, s) + } + } + if len(result) == 0 { + return nil + } + return result +} diff --git a/internal/pkg/config/flags_test.go b/internal/pkg/config/flags_test.go index f9c6819a2..c7e6b975d 100644 --- a/internal/pkg/config/flags_test.go +++ b/internal/pkg/config/flags_test.go @@ -65,7 +65,7 @@ func TestBindFlags(t *testing.T) { "ignore-annotation", "pause-deployment-annotation", "pause-deployment-time-annotation", - "watch-namespace", + "namespaces", "alert-on-reload", "alert-webhook-url", "alert-sink", @@ -582,3 +582,114 @@ func TestSplitAndTrim(t *testing.T) { ) } } + +func TestApplyFlags_NamespacesScoped(t *testing.T) { + resetViper() + cfg := NewDefault() + fs := pflag.NewFlagSet("test", pflag.ContinueOnError) + BindFlags(fs, cfg) + + if err := fs.Parse([]string{"--namespaces=team-a,team-b"}); err != nil { + t.Fatalf("Parse() error = %v", err) + } + if err := ApplyFlags(cfg); err != nil { + t.Fatalf("ApplyFlags() error = %v", err) + } + + if len(cfg.WatchedNamespaces) != 2 { + t.Fatalf("WatchedNamespaces length = %d, want 2", len(cfg.WatchedNamespaces)) + } + if cfg.WatchedNamespaces[0] != "team-a" || cfg.WatchedNamespaces[1] != "team-b" { + t.Errorf("WatchedNamespaces = %v", cfg.WatchedNamespaces) + } + if cfg.IsGlobalMode() { + t.Errorf("explicit namespaces should not be global mode") + } +} + +func TestApplyFlags_NamespacesFromEnv(t *testing.T) { + resetViper() + t.Setenv("KUBERNETES_NAMESPACE", "single-ns") + cfg := NewDefault() + fs := pflag.NewFlagSet("test", pflag.ContinueOnError) + BindFlags(fs, cfg) + + if err := fs.Parse([]string{}); err != nil { + t.Fatalf("Parse() error = %v", err) + } + if err := ApplyFlags(cfg); err != nil { + t.Fatalf("ApplyFlags() error = %v", err) + } + + if len(cfg.WatchedNamespaces) != 1 || cfg.WatchedNamespaces[0] != "single-ns" { + t.Errorf("WatchedNamespaces = %v, want [single-ns]", cfg.WatchedNamespaces) + } +} + +func TestApplyFlags_NamespacesGlobal(t *testing.T) { + resetViper() + t.Setenv("KUBERNETES_NAMESPACE", "") + cfg := NewDefault() + fs := pflag.NewFlagSet("test", pflag.ContinueOnError) + BindFlags(fs, cfg) + + if err := fs.Parse([]string{}); err != nil { + t.Fatalf("Parse() error = %v", err) + } + if err := ApplyFlags(cfg); err != nil { + t.Fatalf("ApplyFlags() error = %v", err) + } + + if len(cfg.WatchedNamespaces) != 0 { + t.Errorf("WatchedNamespaces = %v, want empty (global)", cfg.WatchedNamespaces) + } + if !cfg.IsGlobalMode() { + t.Errorf("no namespaces and no env should be global mode") + } +} + +func TestApplyFlags_NamespacesTrimsEmptyEntries(t *testing.T) { + resetViper() + cfg := NewDefault() + fs := pflag.NewFlagSet("test", pflag.ContinueOnError) + BindFlags(fs, cfg) + + if err := fs.Parse([]string{"--namespaces=team-a, ,team-b,"}); err != nil { + t.Fatalf("Parse() error = %v", err) + } + if err := ApplyFlags(cfg); err != nil { + t.Fatalf("ApplyFlags() error = %v", err) + } + + if len(cfg.WatchedNamespaces) != 2 { + t.Fatalf("WatchedNamespaces length = %d, want 2", len(cfg.WatchedNamespaces)) + } + if cfg.WatchedNamespaces[0] != "team-a" || cfg.WatchedNamespaces[1] != "team-b" { + t.Errorf("WatchedNamespaces = %v, want [team-a team-b]", cfg.WatchedNamespaces) + } + if cfg.IsGlobalMode() { + t.Errorf("trimmed namespaces should not be global mode") + } +} + +func TestApplyFlags_NamespacesAllEmptyIsGlobal(t *testing.T) { + resetViper() + t.Setenv("KUBERNETES_NAMESPACE", "") + cfg := NewDefault() + fs := pflag.NewFlagSet("test", pflag.ContinueOnError) + BindFlags(fs, cfg) + + if err := fs.Parse([]string{"--namespaces=, ,"}); err != nil { + t.Fatalf("Parse() error = %v", err) + } + if err := ApplyFlags(cfg); err != nil { + t.Fatalf("ApplyFlags() error = %v", err) + } + + if len(cfg.WatchedNamespaces) != 0 { + t.Errorf("WatchedNamespaces = %v, want empty (global)", cfg.WatchedNamespaces) + } + if !cfg.IsGlobalMode() { + t.Errorf("all-empty namespaces should be global mode") + } +} diff --git a/internal/pkg/controller/manager.go b/internal/pkg/controller/manager.go index 869bdf001..2c561e16e 100644 --- a/internal/pkg/controller/manager.go +++ b/internal/pkg/controller/manager.go @@ -45,6 +45,20 @@ func AddOptionalSchemes(argoRolloutsEnabled, deploymentConfigEnabled, csiEnabled } } +// buildDefaultNamespaces returns the controller-runtime cache namespace scoping +// for the given watched namespaces. An empty input yields nil, meaning the cache +// watches all namespaces. +func buildDefaultNamespaces(namespaces []string) map[string]cache.Config { + if len(namespaces) == 0 { + return nil + } + out := make(map[string]cache.Config, len(namespaces)) + for _, ns := range namespaces { + out[ns] = cache.Config{} + } + return out +} + // ManagerOptions contains options for creating a new Manager. type ManagerOptions struct { Config *config.Config @@ -79,13 +93,11 @@ func NewManager(opts ManagerOptions) (ctrl.Manager, error) { RetryPeriod: &le.RetryPeriod, } - if cfg.WatchedNamespace != "" { + if nsScope := buildDefaultNamespaces(cfg.WatchedNamespaces); nsScope != nil { mgrOpts.Cache = cache.Options{ - DefaultNamespaces: map[string]cache.Config{ - cfg.WatchedNamespace: {}, - }, + DefaultNamespaces: nsScope, } - opts.Log.Info("namespace filtering enabled", "namespace", cfg.WatchedNamespace) + opts.Log.Info("namespace filtering enabled", "namespaces", cfg.WatchedNamespaces) } mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), mgrOpts) @@ -130,11 +142,9 @@ func NewManagerWithRestConfig(opts ManagerOptions, restConfig *rest.Config) (ctr RetryPeriod: &le.RetryPeriod, } - if cfg.WatchedNamespace != "" { + if nsScope := buildDefaultNamespaces(cfg.WatchedNamespaces); nsScope != nil { mgrOpts.Cache = cache.Options{ - DefaultNamespaces: map[string]cache.Config{ - cfg.WatchedNamespace: {}, - }, + DefaultNamespaces: nsScope, } } diff --git a/internal/pkg/controller/manager_test.go b/internal/pkg/controller/manager_test.go index 7b671c003..863d1acd3 100644 --- a/internal/pkg/controller/manager_test.go +++ b/internal/pkg/controller/manager_test.go @@ -9,6 +9,25 @@ import ( clientgoscheme "k8s.io/client-go/kubernetes/scheme" ) +func TestBuildDefaultNamespaces(t *testing.T) { + if got := buildDefaultNamespaces(nil); got != nil { + t.Errorf("empty input should return nil, got %v", got) + } + if got := buildDefaultNamespaces([]string{}); got != nil { + t.Errorf("empty slice should return nil, got %v", got) + } + + got := buildDefaultNamespaces([]string{"team-a", "team-b", "team-c"}) + if len(got) != 3 { + t.Fatalf("expected 3 entries, got %d", len(got)) + } + for _, ns := range []string{"team-a", "team-b", "team-c"} { + if _, ok := got[ns]; !ok { + t.Errorf("missing namespace %q in %v", ns, got) + } + } +} + func TestAddOptionalSchemesRegistersCSI(t *testing.T) { // Reset to a clean scheme for the test. runtimeScheme = runtime.NewScheme() From faf488fcd7f0eff350ccc96c0b0b804655ebfa56 Mon Sep 17 00:00:00 2001 From: Safwan Date: Tue, 14 Jul 2026 14:14:55 +0500 Subject: [PATCH 3/8] added support for multiple watch namespaces --- cmd/reloader/main.go | 9 +- .../chart/reloader/templates/_helpers.tpl | 22 ++-- .../chart/reloader/templates/clusterrole.yaml | 1 - .../chart/reloader/templates/deployment.yaml | 22 +++- .../chart/reloader/templates/role.yaml | 2 +- .../chart/reloader/templates/rolebinding.yaml | 2 +- .../kubernetes/chart/reloader/values.yaml | 5 + internal/pkg/config/flags.go | 18 ++-- internal/pkg/config/flags_test.go | 101 ++++++++++++++---- internal/pkg/controller/manager.go | 52 +++------ 10 files changed, 154 insertions(+), 80 deletions(-) diff --git a/cmd/reloader/main.go b/cmd/reloader/main.go index 325352157..781d28c56 100644 --- a/cmd/reloader/main.go +++ b/cmd/reloader/main.go @@ -54,7 +54,8 @@ func newReloaderCommand() *cobra.Command { } func run(cmd *cobra.Command, args []string) error { - if err := config.ApplyFlags(cfg); err != nil { + scopeWarnings, err := config.ApplyFlags(cfg) + if err != nil { return fmt.Errorf("applying flags: %w", err) } @@ -81,9 +82,9 @@ func run(cmd *cobra.Command, args []string) error { log.Info("Starting Reloader") - // Enforce master-parity scope semantics before reconcilers/manager read the - // config: selector and ignore lists are only honored in global mode. - for _, w := range cfg.ApplyNamespaceScope() { + // Namespace-scope semantics are enforced in ApplyFlags; surface any warnings + // it produced now that logging is configured. + for _, w := range scopeWarnings { log.Info(w) } diff --git a/deployments/kubernetes/chart/reloader/templates/_helpers.tpl b/deployments/kubernetes/chart/reloader/templates/_helpers.tpl index 306da2d72..19c65533d 100644 --- a/deployments/kubernetes/chart/reloader/templates/_helpers.tpl +++ b/deployments/kubernetes/chart/reloader/templates/_helpers.tpl @@ -89,13 +89,12 @@ Create the namespace selector if it does not watch globally {{- end -}} {{/* -Effective set of namespaces to watch in scoped mode: the release namespace -(always included so the meta-info ConfigMap, HA leases and events keep working) -plus the user-supplied reloader.namespaces, deduped and sorted. +Namespaces to watch in scoped mode: exactly the user-supplied reloader.namespaces, +trimmed, de-duped and sorted. The release namespace is intentionally NOT added here +— Reloader watches only what the user asked for (an empty result means global mode). Returns a JSON-encoded list; consumers use mustFromJson to iterate. */}} {{- define "reloader-watchNamespaces" -}} -{{- $relNs := .Values.namespace | default .Release.Namespace -}} {{- $ns := .Values.reloader.namespaces | default list -}} {{- if kindIs "string" $ns -}} {{- $ns = splitList "," $ns -}} @@ -107,8 +106,7 @@ Returns a JSON-encoded list; consumers use mustFromJson to iterate. {{- $clean = append $clean $t -}} {{- end -}} {{- end -}} -{{- $all := concat (list $relNs) $clean | uniq | sortAlpha -}} -{{- $all | toJson -}} +{{- $clean | uniq | sortAlpha | toJson -}} {{- end -}} {{/* @@ -118,6 +116,18 @@ Comma-joined form of reloader-watchNamespaces, for the --namespaces CLI flag. {{- include "reloader-watchNamespaces" . | mustFromJson | join "," -}} {{- end -}} +{{/* +Namespaces that need namespaced RBAC in scoped mode: the watched namespaces plus +the release namespace, so leader-election leases, the meta-info ConfigMap and +events keep working there even though it is not watched for reloads. +Returns a JSON-encoded list; consumers use mustFromJson to iterate. +*/}} +{{- define "reloader-rbacNamespaces" -}} +{{- $relNs := .Values.namespace | default .Release.Namespace -}} +{{- $watch := include "reloader-watchNamespaces" . | mustFromJson -}} +{{- concat (list $relNs) $watch | uniq | sortAlpha | toJson -}} +{{- end -}} + {{/* The namespaced RBAC rules granted to Reloader in every watched namespace. Shared between the single-namespace Role and the per-namespace scoped Roles so diff --git a/deployments/kubernetes/chart/reloader/templates/clusterrole.yaml b/deployments/kubernetes/chart/reloader/templates/clusterrole.yaml index bd14dfeb7..d183cff89 100644 --- a/deployments/kubernetes/chart/reloader/templates/clusterrole.yaml +++ b/deployments/kubernetes/chart/reloader/templates/clusterrole.yaml @@ -56,7 +56,6 @@ rules: {{- if and (.Capabilities.APIVersions.Has "argoproj.io/v1alpha1") (.Values.reloader.isArgoRollouts) }} - apiGroups: - "argoproj.io" - - "" resources: - rollouts verbs: diff --git a/deployments/kubernetes/chart/reloader/templates/deployment.yaml b/deployments/kubernetes/chart/reloader/templates/deployment.yaml index b3ac972e1..175493584 100644 --- a/deployments/kubernetes/chart/reloader/templates/deployment.yaml +++ b/deployments/kubernetes/chart/reloader/templates/deployment.yaml @@ -241,7 +241,7 @@ spec: - "--namespaces-to-ignore={{ .Values.reloader.ignoreNamespaces }}" {{- end }} {{- if (include "reloader-namespaceSelector" .) }} - - "--namespace-selector=\"{{ include "reloader-namespaceSelector" . }}\"" + - "--namespace-selector={{ include "reloader-namespaceSelector" . }}" {{- end }} {{- if .Values.reloader.resourceLabelSelector }} - "--resource-label-selector={{ .Values.reloader.resourceLabelSelector }}" @@ -275,6 +275,26 @@ spec: {{- if .Values.reloader.custom_annotations.configmap_auto }} - "--configmap-auto-annotation" - "{{ .Values.reloader.custom_annotations.configmap_auto }}" + {{- end }} + {{- if .Values.reloader.custom_annotations.configmap_exclude }} + - "--configmap-exclude-annotation" + - "{{ .Values.reloader.custom_annotations.configmap_exclude }}" + {{- end }} + {{- if .Values.reloader.custom_annotations.secret_exclude }} + - "--secret-exclude-annotation" + - "{{ .Values.reloader.custom_annotations.secret_exclude }}" + {{- end }} + {{- if .Values.reloader.custom_annotations.secretproviderclass }} + - "--secretproviderclass-annotation" + - "{{ .Values.reloader.custom_annotations.secretproviderclass }}" + {{- end }} + {{- if .Values.reloader.custom_annotations.secretproviderclass_auto }} + - "--secretproviderclass-auto-annotation" + - "{{ .Values.reloader.custom_annotations.secretproviderclass_auto }}" + {{- end }} + {{- if .Values.reloader.custom_annotations.secretproviderclass_exclude }} + - "--secretproviderclass-exclude-annotation" + - "{{ .Values.reloader.custom_annotations.secretproviderclass_exclude }}" {{- end }} {{- if .Values.reloader.custom_annotations.search }} - "--auto-search-annotation" diff --git a/deployments/kubernetes/chart/reloader/templates/role.yaml b/deployments/kubernetes/chart/reloader/templates/role.yaml index 28c535d62..b7b5cd929 100644 --- a/deployments/kubernetes/chart/reloader/templates/role.yaml +++ b/deployments/kubernetes/chart/reloader/templates/role.yaml @@ -7,7 +7,7 @@ {{- $apiVersion = "rbac.authorization.k8s.io/v1beta1" }} {{- end }} {{- if .Values.reloader.namespaces }} -{{- range $ns := (include "reloader-watchNamespaces" . | mustFromJson) }} +{{- range $ns := (include "reloader-rbacNamespaces" . | mustFromJson) }} apiVersion: {{ $apiVersion }} kind: Role metadata: diff --git a/deployments/kubernetes/chart/reloader/templates/rolebinding.yaml b/deployments/kubernetes/chart/reloader/templates/rolebinding.yaml index 7d73b1822..187c90ddb 100644 --- a/deployments/kubernetes/chart/reloader/templates/rolebinding.yaml +++ b/deployments/kubernetes/chart/reloader/templates/rolebinding.yaml @@ -4,7 +4,7 @@ {{- $apiVersion = "rbac.authorization.k8s.io/v1beta1" }} {{- end }} {{- if .Values.reloader.namespaces }} -{{- range $ns := (include "reloader-watchNamespaces" . | mustFromJson) }} +{{- range $ns := (include "reloader-rbacNamespaces" . | mustFromJson) }} apiVersion: {{ $apiVersion }} kind: RoleBinding metadata: diff --git a/deployments/kubernetes/chart/reloader/values.yaml b/deployments/kubernetes/chart/reloader/values.yaml index 15a22bcf8..f236bdf71 100644 --- a/deployments/kubernetes/chart/reloader/values.yaml +++ b/deployments/kubernetes/chart/reloader/values.yaml @@ -223,6 +223,11 @@ reloader: # custom_annotations: # configmap: "my.company.com/configmap" # secret: "my.company.com/secret" + # configmap_exclude: "my.company.com/configmap-exclude" + # secret_exclude: "my.company.com/secret-exclude" + # secretproviderclass: "my.company.com/secretproviderclass" + # secretproviderclass_auto: "my.company.com/secretproviderclass-auto" + # secretproviderclass_exclude: "my.company.com/secretproviderclass-exclude" # ignore: "my.company.com/reloader-ignore" custom_annotations: {} diff --git a/internal/pkg/config/flags.go b/internal/pkg/config/flags.go index 969ffb59d..6680abcaf 100644 --- a/internal/pkg/config/flags.go +++ b/internal/pkg/config/flags.go @@ -264,9 +264,11 @@ func BindFlags(fs *pflag.FlagSet, cfg *Config) { _ = v.BindEnv("alert-proxy", "ALERT_PROXY", "ALERT_WEBHOOK_PROXY") } -// ApplyFlags applies flag values from viper to the config struct. -// Call this after parsing flags. -func ApplyFlags(cfg *Config) error { +// 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) { // Boolean flags cfg.AutoReloadAll = v.GetBool("auto-reload-all") cfg.SyncAfterRestart = v.GetBool("sync-after-restart") @@ -365,7 +367,7 @@ func ApplyFlags(cfg *Config) error { joinedNS := strings.Join(nsSelectors, ",") selector, err := labels.Parse(joinedNS) if err != nil { - return fmt.Errorf("invalid selector %q: %w", joinedNS, err) + return nil, fmt.Errorf("invalid selector %q: %w", joinedNS, err) } cfg.NamespaceSelectors = []labels.Selector{selector} } @@ -373,7 +375,7 @@ func ApplyFlags(cfg *Config) error { joinedRes := strings.Join(resSelectors, ",") selector, err := labels.Parse(joinedRes) if err != nil { - return fmt.Errorf("invalid selector %q: %w", joinedRes, err) + return nil, fmt.Errorf("invalid selector %q: %w", joinedRes, err) } cfg.ResourceSelectors = []labels.Selector{selector} } @@ -389,7 +391,11 @@ func ApplyFlags(cfg *Config) error { cfg.LeaderElection.RetryPeriod = 2 * time.Second } - return nil + // 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 } // parseBoolString parses a string as a boolean, defaulting to false. diff --git a/internal/pkg/config/flags_test.go b/internal/pkg/config/flags_test.go index c7e6b975d..233a65272 100644 --- a/internal/pkg/config/flags_test.go +++ b/internal/pkg/config/flags_test.go @@ -92,7 +92,7 @@ func TestBindFlags_DefaultValues(t *testing.T) { t.Fatalf("Parse() error = %v", err) } - if err := ApplyFlags(cfg); err != nil { + if _, err := ApplyFlags(cfg); err != nil { t.Fatalf("ApplyFlags() error = %v", err) } @@ -126,7 +126,7 @@ func TestBindFlags_CustomValues(t *testing.T) { t.Fatalf("Parse() error = %v", err) } - if err := ApplyFlags(cfg); err != nil { + if _, err := ApplyFlags(cfg); err != nil { t.Fatalf("ApplyFlags() error = %v", err) } @@ -168,7 +168,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); err != nil { t.Fatalf("ApplyFlags() error = %v", err) } defaults := DefaultAnnotations() @@ -195,7 +195,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); err != nil { t.Fatalf("ApplyFlags() error = %v", err) } if cfg.Annotations.SecretProviderClassAuto != "spc.example.com/auto" { @@ -218,7 +218,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); err != nil { t.Fatalf("ApplyFlags() error = %v", err) } defaults := DefaultAnnotations() @@ -241,7 +241,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); err != nil { t.Fatalf("ApplyFlags() error = %v", err) } if cfg.Annotations.ConfigmapExclude != "cm.example.com/exclude" { @@ -261,7 +261,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); err != nil { t.Fatalf("ApplyFlags() error = %v", err) } if cfg.Annotations.Ignore != DefaultAnnotations().Ignore { @@ -276,7 +276,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); err != nil { t.Fatalf("ApplyFlags() error = %v", err) } if cfg.Annotations.Ignore != "my.company.com/reloader-ignore" { @@ -313,7 +313,7 @@ func TestApplyFlags_BooleanStrings(t *testing.T) { t.Fatalf("Parse() error = %v", err) } - err := ApplyFlags(cfg) + _, err := ApplyFlags(cfg) if (err != nil) != tt.wantErr { t.Errorf("ApplyFlags() error = %v, wantErr %v", err, tt.wantErr) return @@ -343,7 +343,7 @@ func TestApplyFlags_CommaSeparatedLists(t *testing.T) { t.Fatalf("Parse() error = %v", err) } - if err := ApplyFlags(cfg); err != nil { + if _, err := ApplyFlags(cfg); err != nil { t.Fatalf("ApplyFlags() error = %v", err) } @@ -378,7 +378,7 @@ func TestApplyFlags_Selectors(t *testing.T) { t.Fatalf("Parse() error = %v", err) } - if err := ApplyFlags(cfg); err != nil { + if _, err := ApplyFlags(cfg); err != nil { t.Fatalf("ApplyFlags() error = %v", err) } @@ -409,7 +409,7 @@ func TestApplyFlags_InvalidSelector(t *testing.T) { t.Fatalf("Parse() error = %v", err) } - err := ApplyFlags(cfg) + _, err := ApplyFlags(cfg) if err == nil { t.Error("ApplyFlags() should return error for invalid selector") } @@ -461,7 +461,7 @@ func TestApplyFlags_AlertingEnvVars(t *testing.T) { t.Fatalf("Parse() error = %v", err) } - if err := ApplyFlags(cfg); err != nil { + if _, err := ApplyFlags(cfg); err != nil { t.Fatalf("ApplyFlags() error = %v", err) } @@ -494,7 +494,7 @@ func TestApplyFlags_LegacyProxyEnvVar(t *testing.T) { t.Fatalf("Parse() error = %v", err) } - if err := ApplyFlags(cfg); err != nil { + if _, err := ApplyFlags(cfg); err != nil { t.Fatalf("ApplyFlags() error = %v", err) } @@ -511,7 +511,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); err != nil { t.Fatal(err) } if !cfg.CSIIntegrationEnabled { @@ -592,7 +592,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); err != nil { t.Fatalf("ApplyFlags() error = %v", err) } @@ -617,7 +617,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); err != nil { t.Fatalf("ApplyFlags() error = %v", err) } @@ -636,7 +636,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); err != nil { t.Fatalf("ApplyFlags() error = %v", err) } @@ -657,7 +657,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); err != nil { t.Fatalf("ApplyFlags() error = %v", err) } @@ -682,7 +682,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); err != nil { t.Fatalf("ApplyFlags() error = %v", err) } @@ -693,3 +693,64 @@ func TestApplyFlags_NamespacesAllEmptyIsGlobal(t *testing.T) { t.Errorf("all-empty namespaces should be global mode") } } + +// 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. +func TestApplyFlags_ScopedClearsSelectorsAndIgnores(t *testing.T) { + resetViper() + cfg := NewDefault() + fs := pflag.NewFlagSet("test", pflag.ContinueOnError) + BindFlags(fs, cfg) + + if err := fs.Parse([]string{ + "--namespaces=team-a", + "--namespace-selector=env=prod", + "--namespaces-to-ignore=kube-system", + }); err != nil { + t.Fatalf("Parse() error = %v", err) + } + warnings, err := ApplyFlags(cfg) + if err != nil { + t.Fatalf("ApplyFlags() error = %v", err) + } + + if len(cfg.NamespaceSelectors) != 0 || len(cfg.NamespaceSelectorStrings) != 0 { + t.Errorf("scoped mode should clear namespace selectors, got %v", cfg.NamespaceSelectorStrings) + } + 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) { + resetViper() + t.Setenv("KUBERNETES_NAMESPACE", "") + cfg := NewDefault() + fs := pflag.NewFlagSet("test", pflag.ContinueOnError) + BindFlags(fs, cfg) + + if err := fs.Parse([]string{ + "--namespace-selector=env=prod", + "--namespaces-to-ignore=kube-system", + }); err != nil { + t.Fatalf("Parse() error = %v", err) + } + warnings, err := ApplyFlags(cfg) + if err != nil { + t.Fatalf("ApplyFlags() error = %v", err) + } + + if !cfg.IsGlobalMode() { + t.Fatalf("no --namespaces should be global mode") + } + 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) + } +} diff --git a/internal/pkg/controller/manager.go b/internal/pkg/controller/manager.go index 2c561e16e..a6ae3182e 100644 --- a/internal/pkg/controller/manager.go +++ b/internal/pkg/controller/manager.go @@ -66,9 +66,18 @@ type ManagerOptions struct { Collectors *metrics.Collectors } -// NewManager creates a new controller-runtime manager with the given options. -// This follows controller-runtime and operator-sdk conventions for leader election. +// NewManager creates a new controller-runtime manager using the ambient cluster +// configuration (in-cluster or kubeconfig). It follows controller-runtime and +// operator-sdk conventions for leader election. func NewManager(opts ManagerOptions) (ctrl.Manager, error) { + return NewManagerWithRestConfig(opts, ctrl.GetConfigOrDie()) +} + +// NewManagerWithRestConfig creates a new controller-runtime manager with the +// given rest.Config. NewManager delegates here; tests can call it directly with +// a pre-existing cluster configuration (set cfg.MetricsAddr/HealthAddr to "0" to +// disable those servers and avoid port conflicts). +func NewManagerWithRestConfig(opts ManagerOptions, restConfig *rest.Config) (ctrl.Manager, error) { cfg := opts.Config le := cfg.LeaderElection @@ -100,7 +109,7 @@ func NewManager(opts ManagerOptions) (ctrl.Manager, error) { opts.Log.Info("namespace filtering enabled", "namespaces", cfg.WatchedNamespaces) } - mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), mgrOpts) + mgr, err := ctrl.NewManager(restConfig, mgrOpts) if err != nil { return nil, fmt.Errorf("creating manager: %w", err) } @@ -119,43 +128,6 @@ func NewManager(opts ManagerOptions) (ctrl.Manager, error) { return mgr, nil } -// NewManagerWithRestConfig creates a new controller-runtime manager with the given rest.Config. -// This is useful for testing where you have a pre-existing cluster configuration. -func NewManagerWithRestConfig(opts ManagerOptions, restConfig *rest.Config) (ctrl.Manager, error) { - cfg := opts.Config - le := cfg.LeaderElection - - mgrOpts := ctrl.Options{ - Scheme: runtimeScheme, - Metrics: ctrlmetrics.Options{ - BindAddress: "0", // Disable metrics server in tests - }, - HealthProbeBindAddress: "0", // Disable health probes in tests - - // Leader election configuration - LeaderElection: cfg.EnableHA, - LeaderElectionID: le.LockName, - LeaderElectionNamespace: le.Namespace, - LeaderElectionReleaseOnCancel: le.ReleaseOnCancel, - LeaseDuration: &le.LeaseDuration, - RenewDeadline: &le.RenewDeadline, - RetryPeriod: &le.RetryPeriod, - } - - if nsScope := buildDefaultNamespaces(cfg.WatchedNamespaces); nsScope != nil { - mgrOpts.Cache = cache.Options{ - DefaultNamespaces: nsScope, - } - } - - mgr, err := ctrl.NewManager(restConfig, mgrOpts) - if err != nil { - return nil, fmt.Errorf("creating manager: %w", err) - } - - return mgr, nil -} - // SetupReconcilers sets up all reconcilers with the manager. func SetupReconcilers(mgr ctrl.Manager, cfg *config.Config, log logr.Logger, collectors *metrics.Collectors) error { registry := workload.NewRegistry( From 4e24cbca07e3cbd7ce0261ae344308e11c6ea285 Mon Sep 17 00:00:00 2001 From: Safwan Date: Tue, 14 Jul 2026 16:08:43 +0500 Subject: [PATCH 4/8] fixes in chart --- .../kubernetes/chart/reloader/templates/_helpers.tpl | 7 ++++++- .../chart/reloader/templates/clusterrole.yaml | 6 ++++++ .../chart/reloader/templates/deployment.yaml | 10 ++++++---- 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/deployments/kubernetes/chart/reloader/templates/_helpers.tpl b/deployments/kubernetes/chart/reloader/templates/_helpers.tpl index 19c65533d..5656bb4f7 100644 --- a/deployments/kubernetes/chart/reloader/templates/_helpers.tpl +++ b/deployments/kubernetes/chart/reloader/templates/_helpers.tpl @@ -162,12 +162,12 @@ the rule set is defined once. Expects the root context ($) as its argument. {{- if and (.Capabilities.APIVersions.Has "argoproj.io/v1alpha1") (.Values.reloader.isArgoRollouts) }} - apiGroups: - "argoproj.io" - - "" resources: - rollouts verbs: - list - get + - watch - update - patch {{- end }} @@ -180,6 +180,7 @@ the rule set is defined once. Expects the root context ($) as its argument. verbs: - list - get + - watch - update - patch - apiGroups: @@ -189,6 +190,9 @@ the rule set is defined once. Expects the root context ($) as its argument. verbs: - list - get + - watch + - update + - patch - apiGroups: - "batch" resources: @@ -198,6 +202,7 @@ the rule set is defined once. Expects the root context ($) as its argument. - delete - list - get + - watch {{- if .Values.reloader.enableHA }} - apiGroups: - "coordination.k8s.io" diff --git a/deployments/kubernetes/chart/reloader/templates/clusterrole.yaml b/deployments/kubernetes/chart/reloader/templates/clusterrole.yaml index d183cff89..eb2929e4d 100644 --- a/deployments/kubernetes/chart/reloader/templates/clusterrole.yaml +++ b/deployments/kubernetes/chart/reloader/templates/clusterrole.yaml @@ -61,6 +61,7 @@ rules: verbs: - list - get + - watch - update - patch {{- end }} @@ -73,6 +74,7 @@ rules: verbs: - list - get + - watch - update - patch {{- if .Values.reloader.ignoreCronJobs }}{{- else }} @@ -83,6 +85,9 @@ rules: verbs: - list - get + - watch + - update + - patch {{- end }} {{- if .Values.reloader.ignoreJobs }}{{- else }} - apiGroups: @@ -94,6 +99,7 @@ rules: - delete - list - get + - watch {{- end}} {{- if .Values.reloader.enableHA }} - apiGroups: diff --git a/deployments/kubernetes/chart/reloader/templates/deployment.yaml b/deployments/kubernetes/chart/reloader/templates/deployment.yaml index 175493584..da5a1d540 100644 --- a/deployments/kubernetes/chart/reloader/templates/deployment.yaml +++ b/deployments/kubernetes/chart/reloader/templates/deployment.yaml @@ -176,10 +176,12 @@ spec: ports: - name: http containerPort: 9090 + - name: health + containerPort: 8080 livenessProbe: httpGet: - path: /live - port: http + path: /healthz + port: health timeoutSeconds: {{ .Values.reloader.deployment.livenessProbe.timeoutSeconds | default "5" }} failureThreshold: {{ .Values.reloader.deployment.livenessProbe.failureThreshold | default "5" }} periodSeconds: {{ .Values.reloader.deployment.livenessProbe.periodSeconds | default "10" }} @@ -187,8 +189,8 @@ spec: initialDelaySeconds: {{ .Values.reloader.deployment.livenessProbe.initialDelaySeconds | default "10" }} readinessProbe: httpGet: - path: /metrics - port: http + path: /readyz + port: health timeoutSeconds: {{ .Values.reloader.deployment.readinessProbe.timeoutSeconds | default "5" }} failureThreshold: {{ .Values.reloader.deployment.readinessProbe.failureThreshold | default "5" }} periodSeconds: {{ .Values.reloader.deployment.readinessProbe.periodSeconds | default "10" }} From 35ff2ea89b2768aa28584bf3ae9c0ed0974b94ec Mon Sep 17 00:00:00 2001 From: Safwan Date: Tue, 14 Jul 2026 18:59:31 +0500 Subject: [PATCH 5/8] fix for events warning in test logs --- deployments/kubernetes/chart/reloader/templates/_helpers.tpl | 2 ++ .../kubernetes/chart/reloader/templates/clusterrole.yaml | 2 ++ 2 files changed, 4 insertions(+) diff --git a/deployments/kubernetes/chart/reloader/templates/_helpers.tpl b/deployments/kubernetes/chart/reloader/templates/_helpers.tpl index 5656bb4f7..ec268917a 100644 --- a/deployments/kubernetes/chart/reloader/templates/_helpers.tpl +++ b/deployments/kubernetes/chart/reloader/templates/_helpers.tpl @@ -226,11 +226,13 @@ the rule set is defined once. Expects the root context ($) as its argument. {{- end}} - apiGroups: - "" + - "events.k8s.io" resources: - events verbs: - create - patch + - update {{- end -}} {{/* diff --git a/deployments/kubernetes/chart/reloader/templates/clusterrole.yaml b/deployments/kubernetes/chart/reloader/templates/clusterrole.yaml index eb2929e4d..e0a235a2f 100644 --- a/deployments/kubernetes/chart/reloader/templates/clusterrole.yaml +++ b/deployments/kubernetes/chart/reloader/templates/clusterrole.yaml @@ -124,9 +124,11 @@ rules: {{- end}} - apiGroups: - "" + - "events.k8s.io" resources: - events verbs: - create - patch + - update {{- end }} From e9e09e890037712043443e89d0b257ad59235c40 Mon Sep 17 00:00:00 2001 From: Safwan Date: Tue, 14 Jul 2026 19:44:25 +0500 Subject: [PATCH 6/8] fix the race condition based on master --- test/e2e/advanced/multi_container_test.go | 18 ++- test/e2e/advanced/regex_test.go | 27 +++- test/e2e/annotations/auto_reload_test.go | 48 ++++-- test/e2e/annotations/combination_test.go | 72 ++++++--- test/e2e/annotations/exclude_test.go | 40 +++-- test/e2e/annotations/pause_period_test.go | 24 ++- test/e2e/annotations/resource_ignore_test.go | 16 +- test/e2e/annotations/search_match_test.go | 46 ++++-- test/e2e/argo/rollout_test.go | 9 +- test/e2e/core/reference_methods_test.go | 88 +++++++--- test/e2e/core/workloads_test.go | 159 ++++++++++++++----- test/e2e/csi/csi_test.go | 44 +++-- test/e2e/flags/auto_reload_all_test.go | 16 +- test/e2e/flags/ignore_resources_test.go | 32 +++- test/e2e/flags/ignored_workloads_test.go | 24 ++- test/e2e/flags/namespace_ignore_test.go | 16 +- test/e2e/flags/namespace_selector_test.go | 16 +- test/e2e/flags/reload_on_create_test.go | 24 ++- test/e2e/flags/reload_on_delete_test.go | 24 ++- test/e2e/flags/resource_selector_test.go | 16 +- test/e2e/flags/watch_globally_test.go | 24 ++- test/e2e/utils/workload_adapter.go | 15 ++ test/e2e/utils/workload_argo.go | 12 ++ test/e2e/utils/workload_cronjob.go | 11 ++ test/e2e/utils/workload_daemonset.go | 12 ++ test/e2e/utils/workload_deployment.go | 12 ++ test/e2e/utils/workload_job.go | 11 ++ test/e2e/utils/workload_openshift.go | 12 ++ test/e2e/utils/workload_statefulset.go | 12 ++ 29 files changed, 682 insertions(+), 198 deletions(-) diff --git a/test/e2e/advanced/multi_container_test.go b/test/e2e/advanced/multi_container_test.go index cef051846..776d5773f 100644 --- a/test/e2e/advanced/multi_container_test.go +++ b/test/e2e/advanced/multi_container_test.go @@ -47,13 +47,18 @@ var _ = Describe("Multi-Container Tests", Serial, func() { err = adapter.WaitReady(ctx, testNamespace, deploymentName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the ConfigMap") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"shared-key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment with multiple containers should be reloaded") }) @@ -81,13 +86,18 @@ var _ = Describe("Multi-Container Tests", Serial, func() { err = adapter.WaitReady(ctx, testNamespace, deploymentName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the first ConfigMap") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key1": "updated1"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should be reloaded when first container's ConfigMap changes") }) diff --git a/test/e2e/advanced/regex_test.go b/test/e2e/advanced/regex_test.go index 989bf0ab8..9c1e996d9 100644 --- a/test/e2e/advanced/regex_test.go +++ b/test/e2e/advanced/regex_test.go @@ -53,13 +53,18 @@ var _ = Describe("Regex Pattern Tests", func() { err = adapter.WaitReady(ctx, testNamespace, deploymentName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the matching ConfigMap") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, matchingCM, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should be reloaded when matching ConfigMap changes") }) @@ -87,14 +92,19 @@ var _ = Describe("Regex Pattern Tests", func() { err = adapter.WaitReady(ctx, testNamespace, deploymentName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the non-matching ConfigMap") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, nonMatchingCM, map[string]string{"other": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying Deployment was NOT reloaded (pattern mismatch)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "Deployment should NOT reload when non-matching ConfigMap changes") }) @@ -120,13 +130,18 @@ var _ = Describe("Regex Pattern Tests", func() { err = adapter.WaitReady(ctx, testNamespace, deploymentName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the matching Secret") err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, matchingSecret, map[string]string{"password": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should be reloaded when matching Secret changes") }) diff --git a/test/e2e/annotations/auto_reload_test.go b/test/e2e/annotations/auto_reload_test.go index af4b7ac3f..5961feb42 100644 --- a/test/e2e/annotations/auto_reload_test.go +++ b/test/e2e/annotations/auto_reload_test.go @@ -47,12 +47,16 @@ var _ = Describe("Auto Reload Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap data") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment with auto=true should have been reloaded") }) @@ -75,12 +79,16 @@ var _ = Describe("Auto Reload Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the Secret data") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, secretName, map[string]string{"password": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment with auto=true should have been reloaded for Secret change") }) @@ -108,12 +116,16 @@ var _ = Describe("Auto Reload Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"config": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment with auto=true should have been reloaded for ConfigMap change") }) @@ -145,12 +157,16 @@ var _ = Describe("Auto Reload Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"config": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should have been reloaded for ConfigMap change") }) @@ -180,12 +196,16 @@ var _ = Describe("Auto Reload Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the Secret") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, secretName, map[string]string{"secret": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should have been reloaded for Secret change") }) @@ -221,12 +241,16 @@ var _ = Describe("Auto Reload Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the second ConfigMap (auto-detected)") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName2, map[string]string{"key2": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should have been reloaded for auto-detected ConfigMap change") }) diff --git a/test/e2e/annotations/combination_test.go b/test/e2e/annotations/combination_test.go index e7f02efa5..dab5a8fd5 100644 --- a/test/e2e/annotations/combination_test.go +++ b/test/e2e/annotations/combination_test.go @@ -61,12 +61,16 @@ var _ = Describe("Combination Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the auto-detected ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should reload when auto-detected ConfigMap changes") }) @@ -95,12 +99,16 @@ var _ = Describe("Combination Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the explicitly listed ConfigMap (not mounted)") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName2, map[string]string{"extra": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should reload when explicitly listed ConfigMap changes") }) @@ -129,12 +137,16 @@ var _ = Describe("Combination Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the explicitly listed Secret") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, secretName2, map[string]string{"api-key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should reload when explicitly listed Secret changes") }) @@ -166,13 +178,17 @@ var _ = Describe("Combination Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the excluded ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName2, map[string]string{"excluded": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying Deployment was NOT reloaded (negative test)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "Deployment should NOT reload when excluded ConfigMap changes") }) @@ -202,12 +218,16 @@ var _ = Describe("Combination Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the non-excluded ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should reload when non-excluded ConfigMap changes") }) @@ -237,13 +257,17 @@ var _ = Describe("Combination Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the excluded Secret") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, secretName2, map[string]string{"excluded": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying Deployment was NOT reloaded (negative test)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "Deployment should NOT reload when excluded Secret changes") }) @@ -270,12 +294,16 @@ var _ = Describe("Combination Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the second ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName2, map[string]string{"key2": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should reload when any of the listed ConfigMaps changes") }) @@ -300,12 +328,16 @@ var _ = Describe("Combination Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the first Secret") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, secretName, map[string]string{"key1": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should reload when any of the listed Secrets changes") }) @@ -333,12 +365,16 @@ var _ = Describe("Combination Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the Secret") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, secretName, map[string]string{"password": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should reload when Secret changes with both annotations present") }) diff --git a/test/e2e/annotations/exclude_test.go b/test/e2e/annotations/exclude_test.go index 5a336514a..bce14ee6b 100644 --- a/test/e2e/annotations/exclude_test.go +++ b/test/e2e/annotations/exclude_test.go @@ -66,13 +66,17 @@ var _ = Describe("Exclude Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the excluded ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying Deployment was NOT reloaded (excluded ConfigMap)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "Deployment should NOT reload when excluded ConfigMap changes") }) @@ -103,12 +107,16 @@ var _ = Describe("Exclude Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the non-excluded ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName2, map[string]string{"key2": "updated2"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should reload when non-excluded ConfigMap changes") }) @@ -141,13 +149,17 @@ var _ = Describe("Exclude Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the excluded Secret") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, secretName, map[string]string{"password": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying Deployment was NOT reloaded (excluded Secret)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "Deployment should NOT reload when excluded Secret changes") }) @@ -178,12 +190,16 @@ var _ = Describe("Exclude Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the non-excluded Secret") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, secretName2, map[string]string{"password2": "updated2"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should reload when non-excluded Secret changes") }) @@ -225,13 +241,17 @@ var _ = Describe("Exclude Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the excluded ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying workload was NOT reloaded (excluded ConfigMap)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "%s should NOT reload with exclude on pod template", workloadType) }, diff --git a/test/e2e/annotations/pause_period_test.go b/test/e2e/annotations/pause_period_test.go index 869aed1dc..6e80b55ff 100644 --- a/test/e2e/annotations/pause_period_test.go +++ b/test/e2e/annotations/pause_period_test.go @@ -49,12 +49,16 @@ var _ = Describe("Pause Period Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap data") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should have been reloaded") @@ -83,12 +87,16 @@ var _ = Describe("Pause Period Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap data") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should have been reloaded") @@ -124,12 +132,16 @@ var _ = Describe("Pause Period Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap data") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should have been reloaded") diff --git a/test/e2e/annotations/resource_ignore_test.go b/test/e2e/annotations/resource_ignore_test.go index 132c91a65..0d97724d6 100644 --- a/test/e2e/annotations/resource_ignore_test.go +++ b/test/e2e/annotations/resource_ignore_test.go @@ -50,13 +50,17 @@ var _ = Describe("Resource Ignore Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap data") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying Deployment was NOT reloaded (negative test)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "Deployment should NOT reload when ConfigMap has ignore=true") }) @@ -80,13 +84,17 @@ var _ = Describe("Resource Ignore Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the Secret data") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, secretName, map[string]string{"password": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying Deployment was NOT reloaded (negative test)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "Deployment should NOT reload when Secret has ignore=true") }) diff --git a/test/e2e/annotations/search_match_test.go b/test/e2e/annotations/search_match_test.go index 02a1153ca..dac035774 100644 --- a/test/e2e/annotations/search_match_test.go +++ b/test/e2e/annotations/search_match_test.go @@ -50,12 +50,16 @@ var _ = Describe("Search and Match Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap data") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment with search annotation should reload when ConfigMap has match annotation") }) @@ -78,13 +82,17 @@ var _ = Describe("Search and Match Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap data") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying Deployment was NOT reloaded (negative test)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "Deployment should NOT reload when ConfigMap lacks match annotation") }) @@ -106,13 +114,17 @@ var _ = Describe("Search and Match Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap data") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying Deployment was NOT reloaded (negative test)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "Deployment without search annotation should NOT reload even when ConfigMap has match") }) @@ -149,18 +161,24 @@ var _ = Describe("Search and Match Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap data") + // Capture the reload-annotation baselines before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload1, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + priorReload2, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName2, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for first Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload1, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment with search annotation should reload") By("Verifying second Deployment was NOT reloaded") - reloaded2, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName2, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded2, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName2, + utils.AnnotationLastReloadedFrom, priorReload2, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded2).To(BeFalse(), "Deployment without search annotation should NOT reload") }) @@ -196,12 +214,16 @@ var _ = Describe("Search and Match Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s should reload with search annotation on pod template", workloadType) }, diff --git a/test/e2e/argo/rollout_test.go b/test/e2e/argo/rollout_test.go index 019df62b8..e6edf153c 100644 --- a/test/e2e/argo/rollout_test.go +++ b/test/e2e/argo/rollout_test.go @@ -47,13 +47,18 @@ var _ = Describe("Argo Rollout Strategy Tests", func() { err = adapter.WaitReady(ctx, testNamespace, rolloutName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, rolloutName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the ConfigMap") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Rollout to be reloaded with annotation") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, rolloutName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, rolloutName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Argo Rollout should be reloaded with default rollout strategy") }) diff --git a/test/e2e/core/reference_methods_test.go b/test/e2e/core/reference_methods_test.go index f3c0b8fd5..60bf129ef 100644 --- a/test/e2e/core/reference_methods_test.go +++ b/test/e2e/core/reference_methods_test.go @@ -58,13 +58,17 @@ var _ = Describe("Reference Method Tests", func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the ConfigMap") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"config_key": "updated_value"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s with valueFrom.configMapKeyRef should reload", workloadType) }, @@ -107,13 +111,17 @@ var _ = Describe("Reference Method Tests", func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the Secret") err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, secretName, map[string]string{"secret_key": "updated_secret"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s with valueFrom.secretKeyRef should reload", workloadType) }, @@ -154,13 +162,17 @@ var _ = Describe("Reference Method Tests", func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the ConfigMap") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"config.yaml": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s with projected ConfigMap volume should reload", workloadType) }, @@ -196,13 +208,17 @@ var _ = Describe("Reference Method Tests", func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the Secret") err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, secretName, map[string]string{"credentials": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s with projected Secret volume should reload", workloadType) }, @@ -246,13 +262,17 @@ var _ = Describe("Reference Method Tests", func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the ConfigMap") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"config.yaml": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s should reload when ConfigMap in mixed projected volume changes", workloadType) }, @@ -296,13 +316,17 @@ var _ = Describe("Reference Method Tests", func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the Secret") err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, secretName, map[string]string{"credentials": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s should reload when Secret in mixed projected volume changes", workloadType) }, @@ -343,13 +367,17 @@ var _ = Describe("Reference Method Tests", func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the ConfigMap") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"INIT_VAR": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s with init container ConfigMap should reload", workloadType) }, @@ -385,13 +413,17 @@ var _ = Describe("Reference Method Tests", func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the Secret") err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, secretName, map[string]string{"INIT_SECRET": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s with init container Secret should reload", workloadType) }, @@ -429,13 +461,17 @@ var _ = Describe("Reference Method Tests", func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the ConfigMap") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"config.yaml": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s with init container ConfigMap volume should reload", workloadType) }, @@ -471,13 +507,17 @@ var _ = Describe("Reference Method Tests", func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the Secret") err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, secretName, map[string]string{"credentials": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s with init container Secret volume should reload", workloadType) }, @@ -520,13 +560,17 @@ var _ = Describe("Reference Method Tests", func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the ConfigMap") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"auto_config_key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s with auto=true and valueFrom should reload", workloadType) }, diff --git a/test/e2e/core/workloads_test.go b/test/e2e/core/workloads_test.go index fdbe98ec4..b6a1a1954 100644 --- a/test/e2e/core/workloads_test.go +++ b/test/e2e/core/workloads_test.go @@ -64,13 +64,17 @@ var _ = Describe("Workload Reload Tests", Serial, func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the ConfigMap data") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, - utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s should have been reloaded", workloadType) }, @@ -106,13 +110,17 @@ var _ = Describe("Workload Reload Tests", Serial, func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the Secret data") err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, secretName, map[string]string{"password": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, - utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s should have been reloaded", workloadType) }, @@ -149,13 +157,17 @@ var _ = Describe("Workload Reload Tests", Serial, func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the ConfigMap data") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s with auto=true should have been reloaded", workloadType) }, @@ -192,14 +204,18 @@ var _ = Describe("Workload Reload Tests", Serial, func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating only the ConfigMap labels (no data change)") err = utils.UpdateConfigMapLabels(ctx, kubeClient, testNamespace, configMapName, map[string]string{"new-label": "new-value"}) Expect(err).NotTo(HaveOccurred()) By("Verifying workload was NOT reloaded (negative test)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "%s should NOT reload when only ConfigMap labels change", workloadType) }, @@ -235,14 +251,18 @@ var _ = Describe("Workload Reload Tests", Serial, func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating only the Secret labels (no data change)") err = utils.UpdateSecretLabels(ctx, kubeClient, testNamespace, secretName, map[string]string{"new-label": "new-value"}) Expect(err).NotTo(HaveOccurred()) By("Verifying workload was NOT reloaded (negative test)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "%s should NOT reload when only Secret labels change", workloadType) }, @@ -369,13 +389,17 @@ var _ = Describe("Workload Reload Tests", Serial, func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the ConfigMap data") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"config.yaml": "setting: updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, - utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s with volume-mounted ConfigMap should have been reloaded", workloadType) }, @@ -410,13 +434,17 @@ var _ = Describe("Workload Reload Tests", Serial, func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the Secret data") err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, secretName, map[string]string{"credentials.yaml": "secret: updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, - utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s with volume-mounted Secret should have been reloaded", workloadType) }, @@ -451,14 +479,18 @@ var _ = Describe("Workload Reload Tests", Serial, func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the ConfigMap data") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying workload is NOT reloaded (negative test)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, - utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "%s without Reloader annotation should NOT be reloaded", workloadType) }, @@ -507,13 +539,19 @@ var _ = Describe("Workload Reload Tests", Serial, func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger: Reloader can react + // to the ConfigMap update before WaitReloaded captures its baseline, which would + // otherwise record the already-reloaded value and time out (TOCTOU race). + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the second ConfigMap") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName2, map[string]string{"key2": "updated-value2"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s should reload when second ConfigMap changes", workloadType) }, @@ -556,13 +594,18 @@ var _ = Describe("Workload Reload Tests", Serial, func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the TOCTOU + // race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the second Secret") err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, secretName2, map[string]string{"key2": "updated-value2"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s should reload when second Secret changes", workloadType) }, @@ -598,13 +641,17 @@ var _ = Describe("Workload Reload Tests", Serial, func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("First update to ConfigMap") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "v2"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for first reload") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue()) @@ -666,13 +713,17 @@ var _ = Describe("Workload Reload Tests", Serial, func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the Secret") err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, secretName, map[string]string{"secret": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s should reload when Secret changes", workloadType) }, @@ -708,14 +759,18 @@ var _ = Describe("Workload Reload Tests", Serial, func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the ConfigMap data") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying workload is NOT reloaded (auto=false)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "%s with auto=false should NOT be reloaded", workloadType) }, @@ -759,13 +814,17 @@ var _ = Describe("Workload Reload Tests", Serial, func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the ConfigMap") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s should reload with pod template annotation", workloadType) }, @@ -801,13 +860,17 @@ var _ = Describe("Workload Reload Tests", Serial, func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the Secret") err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, secretName, map[string]string{"password": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s should reload with pod template annotation", workloadType) }, @@ -843,13 +906,17 @@ var _ = Describe("Workload Reload Tests", Serial, func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the ConfigMap") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s with auto=true on pod template should reload", workloadType) }, @@ -886,13 +953,17 @@ var _ = Describe("Workload Reload Tests", Serial, func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the ConfigMap") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s should reload with annotations on both locations", workloadType) }, @@ -932,14 +1003,18 @@ var _ = Describe("Workload Reload Tests", Serial, func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the Secret (not the ConfigMap)") err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, secretName, map[string]string{"password": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying workload was NOT reloaded (negative test)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "%s should NOT reload when updating different resource than annotated", workloadType) }, @@ -1016,8 +1091,7 @@ var _ = Describe("Workload Reload Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to have STAKATER_ env var") - found, err := adapter.WaitEnvVar(ctx, testNamespace, workloadName, utils.StakaterEnvVarPrefix, - utils.ReloadTimeout) + found, err := adapter.WaitEnvVarFrom(ctx, testNamespace, workloadName, utils.StakaterEnvVarPrefix, "", utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(found).To(BeTrue(), "%s should have STAKATER_ env var after ConfigMap change", workloadType) }, @@ -1061,8 +1135,7 @@ var _ = Describe("Workload Reload Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to have STAKATER_ env var") - found, err := adapter.WaitEnvVar(ctx, testNamespace, workloadName, utils.StakaterEnvVarPrefix, - utils.ReloadTimeout) + found, err := adapter.WaitEnvVarFrom(ctx, testNamespace, workloadName, utils.StakaterEnvVarPrefix, "", utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(found).To(BeTrue(), "%s should have STAKATER_ env var after Secret change", workloadType) }, @@ -1109,8 +1182,7 @@ var _ = Describe("Workload Reload Tests", Serial, func() { By("Verifying workload does NOT have STAKATER_ env var") time.Sleep(utils.NegativeTestWait) - found, err := adapter.WaitEnvVar(ctx, testNamespace, workloadName, utils.StakaterEnvVarPrefix, - utils.ShortTimeout) + found, err := adapter.WaitEnvVarFrom(ctx, testNamespace, workloadName, utils.StakaterEnvVarPrefix, "", utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(found).To(BeFalse(), "%s should NOT have STAKATER_ env var for label-only change", workloadType) }, @@ -1154,8 +1226,7 @@ var _ = Describe("Workload Reload Tests", Serial, func() { By("Verifying workload does NOT have STAKATER_ env var") time.Sleep(utils.NegativeTestWait) - found, err := adapter.WaitEnvVar(ctx, testNamespace, workloadName, utils.StakaterEnvVarPrefix, - utils.ShortTimeout) + found, err := adapter.WaitEnvVarFrom(ctx, testNamespace, workloadName, utils.StakaterEnvVarPrefix, "", utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(found).To(BeFalse(), "%s should NOT have STAKATER_ env var for label-only change", workloadType) }, diff --git a/test/e2e/csi/csi_test.go b/test/e2e/csi/csi_test.go index ef55f2bdf..08604d615 100644 --- a/test/e2e/csi/csi_test.go +++ b/test/e2e/csi/csi_test.go @@ -72,6 +72,11 @@ var _ = Describe("CSI SecretProviderClass Tests", Label("csi"), Serial, func() { Expect(err).NotTo(HaveOccurred()) GinkgoWriter.Printf("Initial SPCPS version: %s\n", initialVersion) + // Capture the reload-annotation baseline before the trigger: Reloader reacts to the + // same SPCPS update the test waits on below, so it may reload before WaitReloaded runs. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the Vault secret") err = utils.UpdateVaultSecret( ctx, kubeClient, restConfig, vaultSecretPath, map[string]string{"api_key": "updated-value-v2"}) @@ -83,9 +88,9 @@ var _ = Describe("CSI SecretProviderClass Tests", Label("csi"), Serial, func() { GinkgoWriter.Println("CSI driver synced new secret version") By("Waiting for Deployment to be reloaded by Reloader") - reloaded, err := adapter.WaitReloaded( + reloaded, err := adapter.WaitReloadedFrom( ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout, ) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should have been reloaded after Vault secret change") @@ -124,6 +129,10 @@ var _ = Describe("CSI SecretProviderClass Tests", Label("csi"), Serial, func() { By("First update to Vault secret") initialVersion, _ := utils.GetSPCPSVersion(ctx, csiClient, testNamespace, spcpsName) + // Capture the baseline before the trigger to avoid racing Reloader's own reaction + // to the SPCPS update below. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateVaultSecret( ctx, kubeClient, restConfig, vaultSecretPath, map[string]string{"password": "pass-v2"}) Expect(err).NotTo(HaveOccurred()) @@ -133,9 +142,9 @@ var _ = Describe("CSI SecretProviderClass Tests", Label("csi"), Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Waiting for first reload") - reloaded, err := adapter.WaitReloaded( + reloaded, err := adapter.WaitReloadedFrom( ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout, ) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue()) @@ -207,6 +216,11 @@ var _ = Describe("CSI SecretProviderClass Tests", Label("csi"), Serial, func() { err = adapter.WaitReady(ctx, testNamespace, deploymentName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the ConfigMap (should NOT trigger reload)") err = utils.UpdateConfigMap( ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) @@ -214,9 +228,9 @@ var _ = Describe("CSI SecretProviderClass Tests", Label("csi"), Serial, func() { By("Verifying Deployment was NOT reloaded for ConfigMap change") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded( + reloaded, err := adapter.WaitReloadedFrom( ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout, ) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "SPC auto annotation should not trigger reload for ConfigMap changes") @@ -230,6 +244,11 @@ var _ = Describe("CSI SecretProviderClass Tests", Label("csi"), Serial, func() { By("Getting SPCPS version before Vault update") initialVersion, _ := utils.GetSPCPSVersion(ctx, csiClient, testNamespace, spcpsName) + // Capture the baseline before the trigger to avoid racing Reloader's own reaction + // to the SPCPS update below. + priorReload, err = adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the Vault secret (should trigger reload)") err = utils.UpdateVaultSecret( ctx, kubeClient, restConfig, vaultSecretPath, map[string]string{"token": "token-v2"}) @@ -240,9 +259,9 @@ var _ = Describe("CSI SecretProviderClass Tests", Label("csi"), Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Verifying Deployment WAS reloaded for Vault secret change") - reloaded, err = adapter.WaitReloaded( + reloaded, err = adapter.WaitReloadedFrom( ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout, ) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "SPC auto annotation should trigger reload for Vault secret changes") @@ -281,15 +300,20 @@ var _ = Describe("CSI SecretProviderClass Tests", Label("csi"), Serial, func() { err = adapter.WaitReady(ctx, testNamespace, deploymentName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the ConfigMap (should trigger reload with auto=true)") err = utils.UpdateConfigMap( ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying Deployment WAS reloaded for ConfigMap change") - reloaded, err := adapter.WaitReloaded( + reloaded, err := adapter.WaitReloadedFrom( ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout, ) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Combined auto=true should trigger reload for ConfigMap changes") diff --git a/test/e2e/flags/auto_reload_all_test.go b/test/e2e/flags/auto_reload_all_test.go index f4cda1cd9..839e5e0b1 100644 --- a/test/e2e/flags/auto_reload_all_test.go +++ b/test/e2e/flags/auto_reload_all_test.go @@ -65,12 +65,16 @@ var _ = Describe("Auto Reload All Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, autoNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, autoNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded (autoReloadAll=true)") - reloaded, err := adapter.WaitReloaded(ctx, autoNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, autoNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment without annotations should reload when autoReloadAll=true") }) @@ -93,13 +97,17 @@ var _ = Describe("Auto Reload All Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, autoNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, autoNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying Deployment was NOT reloaded (auto=false overrides autoReloadAll)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, autoNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, autoNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "Deployment with auto=false should NOT reload even with autoReloadAll=true") }) diff --git a/test/e2e/flags/ignore_resources_test.go b/test/e2e/flags/ignore_resources_test.go index 44eb539df..e28f88858 100644 --- a/test/e2e/flags/ignore_resources_test.go +++ b/test/e2e/flags/ignore_resources_test.go @@ -69,13 +69,17 @@ var _ = Describe("Ignore Resources Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Updating the Secret") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, ignoreNS, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateSecretFromStrings(ctx, kubeClient, ignoreNS, secretName, map[string]string{"password": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying Deployment was NOT reloaded (ignoreSecrets=true)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, ignoreNS, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, ignoreNS, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "Deployment should NOT reload when ignoreSecrets=true") }) @@ -98,12 +102,16 @@ var _ = Describe("Ignore Resources Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, ignoreNS, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, ignoreNS, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded (ConfigMap should still work)") - reloaded, err := adapter.WaitReloaded(ctx, ignoreNS, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, ignoreNS, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "ConfigMap changes should still trigger reload with ignoreSecrets=true") }) @@ -146,13 +154,17 @@ var _ = Describe("Ignore Resources Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, ignoreNS, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, ignoreNS, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying Deployment was NOT reloaded (ignoreConfigMaps=true)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, ignoreNS, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, ignoreNS, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "Deployment should NOT reload when ignoreConfigMaps=true") }) @@ -175,12 +187,16 @@ var _ = Describe("Ignore Resources Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Updating the Secret") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, ignoreNS, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateSecretFromStrings(ctx, kubeClient, ignoreNS, secretName, map[string]string{"password": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded (Secret should still work)") - reloaded, err := adapter.WaitReloaded(ctx, ignoreNS, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, ignoreNS, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Secret changes should still trigger reload with ignoreConfigMaps=true") }) diff --git a/test/e2e/flags/ignored_workloads_test.go b/test/e2e/flags/ignored_workloads_test.go index 7a5185ce5..0bd4449c5 100644 --- a/test/e2e/flags/ignored_workloads_test.go +++ b/test/e2e/flags/ignored_workloads_test.go @@ -64,13 +64,17 @@ var _ = Describe("Ignored Workloads Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := cronJobAdapter.GetPodTemplateAnnotation(ctx, ignoreNS, cronJobName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, ignoreNS, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying CronJob was NOT reloaded (ignoreCronJobs=true)") time.Sleep(utils.NegativeTestWait) - reloaded, err := cronJobAdapter.WaitReloaded(ctx, ignoreNS, cronJobName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := cronJobAdapter.WaitReloadedFrom(ctx, ignoreNS, cronJobName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "CronJob should NOT reload when ignoreCronJobs=true") }) @@ -98,12 +102,16 @@ var _ = Describe("Ignored Workloads Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := deploymentAdapter.GetPodTemplateAnnotation(ctx, ignoreNS, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, ignoreNS, configMapName, map[string]string{"key": "updated-deploy"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded (Deployment should still work)") - reloaded, err := deploymentAdapter.WaitReloaded(ctx, ignoreNS, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := deploymentAdapter.WaitReloadedFrom(ctx, ignoreNS, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should still reload with ignoreCronJobs=true") }) @@ -143,13 +151,17 @@ var _ = Describe("Ignored Workloads Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := cronJobAdapter.GetPodTemplateAnnotation(ctx, ignoreNS, cronJobName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, ignoreNS, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying CronJob was NOT reloaded") time.Sleep(utils.NegativeTestWait) - reloaded, err := cronJobAdapter.WaitReloaded(ctx, ignoreNS, cronJobName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := cronJobAdapter.WaitReloadedFrom(ctx, ignoreNS, cronJobName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "CronJob should NOT reload when ignoreCronJobs=true and ignoreJobs=true") }) diff --git a/test/e2e/flags/namespace_ignore_test.go b/test/e2e/flags/namespace_ignore_test.go index 394679091..d35995409 100644 --- a/test/e2e/flags/namespace_ignore_test.go +++ b/test/e2e/flags/namespace_ignore_test.go @@ -73,13 +73,17 @@ var _ = Describe("Namespace Ignore Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, ignoredNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, ignoredNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying Deployment was NOT reloaded (ignored namespace)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, ignoredNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, ignoredNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "Deployment in ignored namespace should NOT be reloaded") }) @@ -102,12 +106,16 @@ var _ = Describe("Namespace Ignore Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, watchedNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, watchedNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, watchedNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, watchedNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment in non-ignored namespace should be reloaded") }) diff --git a/test/e2e/flags/namespace_selector_test.go b/test/e2e/flags/namespace_selector_test.go index 4ac49cbed..a58da5795 100644 --- a/test/e2e/flags/namespace_selector_test.go +++ b/test/e2e/flags/namespace_selector_test.go @@ -74,12 +74,16 @@ var _ = Describe("Namespace Selector Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, matchingNS, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, matchingNS, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, matchingNS, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, matchingNS, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment in matching namespace should be reloaded") }) @@ -102,13 +106,17 @@ var _ = Describe("Namespace Selector Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, nonMatchingNS, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, nonMatchingNS, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying Deployment was NOT reloaded (non-matching namespace)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, nonMatchingNS, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, nonMatchingNS, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "Deployment in non-matching namespace should NOT be reloaded") }) diff --git a/test/e2e/flags/reload_on_create_test.go b/test/e2e/flags/reload_on_create_test.go index 63fec0bb3..b6946ef31 100644 --- a/test/e2e/flags/reload_on_create_test.go +++ b/test/e2e/flags/reload_on_create_test.go @@ -60,13 +60,17 @@ var _ = Describe("Reload On Create Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Creating the ConfigMap that the Deployment references") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, createNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) _, err = utils.CreateConfigMap(ctx, kubeClient, createNamespace, configMapName, map[string]string{"key": "value"}, nil) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded (reloadOnCreate=true)") - reloaded, err := adapter.WaitReloaded(ctx, createNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, createNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should reload when referenced ConfigMap is created") }) @@ -86,13 +90,17 @@ var _ = Describe("Reload On Create Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Creating the Secret that the Deployment references") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, createNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) _, err = utils.CreateSecretFromStrings(ctx, kubeClient, createNamespace, secretName, map[string]string{"password": "secret"}, nil) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded (reloadOnCreate=true)") - reloaded, err := adapter.WaitReloaded(ctx, createNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, createNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should reload when referenced Secret is created") }) @@ -127,14 +135,18 @@ var _ = Describe("Reload On Create Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Creating the ConfigMap that the Deployment references") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, createNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) _, err = utils.CreateConfigMap(ctx, kubeClient, createNamespace, configMapName, map[string]string{"key": "value"}, nil) Expect(err).NotTo(HaveOccurred()) By("Verifying Deployment was NOT reloaded (reloadOnCreate=false)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, createNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, createNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "Deployment should NOT reload on create when reloadOnCreate=false") }) diff --git a/test/e2e/flags/reload_on_delete_test.go b/test/e2e/flags/reload_on_delete_test.go index ed400e35f..88fda7663 100644 --- a/test/e2e/flags/reload_on_delete_test.go +++ b/test/e2e/flags/reload_on_delete_test.go @@ -65,12 +65,16 @@ var _ = Describe("Reload On Delete Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Deleting the ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, deleteNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.DeleteConfigMap(ctx, kubeClient, deleteNamespace, configMapName) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded (reloadOnDelete=true)") - reloaded, err := adapter.WaitReloaded(ctx, deleteNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, deleteNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should reload when referenced ConfigMap is deleted") }) @@ -94,12 +98,16 @@ var _ = Describe("Reload On Delete Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Deleting the Secret") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, deleteNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.DeleteSecret(ctx, kubeClient, deleteNamespace, secretName) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded (reloadOnDelete=true)") - reloaded, err := adapter.WaitReloaded(ctx, deleteNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, deleteNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should reload when referenced Secret is deleted") }) @@ -139,13 +147,17 @@ var _ = Describe("Reload On Delete Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Deleting the ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, deleteNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.DeleteConfigMap(ctx, kubeClient, deleteNamespace, configMapName) Expect(err).NotTo(HaveOccurred()) By("Verifying Deployment was NOT reloaded (reloadOnDelete=false)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, deleteNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, deleteNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "Deployment should NOT reload on delete when reloadOnDelete=false") }) diff --git a/test/e2e/flags/resource_selector_test.go b/test/e2e/flags/resource_selector_test.go index cc94612a8..af31a958c 100644 --- a/test/e2e/flags/resource_selector_test.go +++ b/test/e2e/flags/resource_selector_test.go @@ -70,12 +70,16 @@ var _ = Describe("Resource Label Selector Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Updating the labeled ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, resourceNS, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, resourceNS, matchingCM, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, resourceNS, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, resourceNS, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should be reloaded when labeled ConfigMap changes") }) @@ -98,13 +102,17 @@ var _ = Describe("Resource Label Selector Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Updating the unlabeled ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, resourceNS, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, resourceNS, nonMatchingCM, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying Deployment was NOT reloaded (unlabeled ConfigMap)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, resourceNS, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, resourceNS, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "Deployment should NOT reload when unlabeled ConfigMap changes") }) diff --git a/test/e2e/flags/watch_globally_test.go b/test/e2e/flags/watch_globally_test.go index 96e3fb2d5..204a3d657 100644 --- a/test/e2e/flags/watch_globally_test.go +++ b/test/e2e/flags/watch_globally_test.go @@ -68,12 +68,16 @@ var _ = Describe("Watch Globally Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded (same namespace should work)") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment in Reloader's namespace should reload with watchGlobally=false") }) @@ -96,13 +100,17 @@ var _ = Describe("Watch Globally Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap in the other namespace") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, otherNS, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, otherNS, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying Deployment was NOT reloaded (different namespace with watchGlobally=false)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, otherNS, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, otherNS, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "Deployment in other namespace should NOT reload with watchGlobally=false") }) @@ -151,12 +159,16 @@ var _ = Describe("Watch Globally Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, globalNS, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, globalNS, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded (watchGlobally=true)") - reloaded, err := adapter.WaitReloaded(ctx, globalNS, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, globalNS, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment in any namespace should reload with watchGlobally=true") }) diff --git a/test/e2e/utils/workload_adapter.go b/test/e2e/utils/workload_adapter.go index 2610337fe..70a5e7e17 100644 --- a/test/e2e/utils/workload_adapter.go +++ b/test/e2e/utils/workload_adapter.go @@ -66,12 +66,27 @@ type WorkloadAdapter interface { // WaitReloaded waits for the workload to have the reload annotation. // Returns true if the annotation was found, false if timeout occurred. + // It captures the baseline annotation value at call time, which races with Reloader + // if the reload trigger happened earlier — prefer WaitReloadedFrom in that case. WaitReloaded(ctx context.Context, namespace, name, annotationKey string, timeout time.Duration) (bool, error) + // WaitReloadedFrom waits for the reload annotation to be present with a value different + // from priorValue. Capture priorValue (via GetPodTemplateAnnotation) BEFORE performing + // the change that triggers the reload; capturing it afterwards can observe the already + // reloaded value and then wait for a further change that never comes. + WaitReloadedFrom(ctx context.Context, namespace, name, annotationKey, priorValue string, timeout time.Duration) (bool, error) + // WaitEnvVar waits for the workload to have a STAKATER_ env var (for envvars strategy). // Returns true if the env var was found, false if timeout occurred. + // It captures the baseline env var value at call time, which races with Reloader + // if the reload trigger happened earlier — prefer WaitEnvVarFrom in that case. WaitEnvVar(ctx context.Context, namespace, name, prefix string, timeout time.Duration) (bool, error) + // WaitEnvVarFrom waits for a STAKATER_ env var whose value differs from priorValue. + // Capture priorValue BEFORE performing the change that triggers the reload + // (an empty priorValue means the env var is expected to appear). + WaitEnvVarFrom(ctx context.Context, namespace, name, prefix, priorValue string, timeout time.Duration) (bool, error) + // SupportsEnvVarStrategy returns true if the workload supports env var reload strategy. // CronJob does not support this as it uses job creation instead. SupportsEnvVarStrategy() bool diff --git a/test/e2e/utils/workload_argo.go b/test/e2e/utils/workload_argo.go index 69d5163e7..c976d7462 100644 --- a/test/e2e/utils/workload_argo.go +++ b/test/e2e/utils/workload_argo.go @@ -58,6 +58,12 @@ func (a *ArgoRolloutAdapter) WaitReady(ctx context.Context, namespace, name stri // Captures the current annotation value first to avoid false positives from prior reloads. func (a *ArgoRolloutAdapter) WaitReloaded(ctx context.Context, namespace, name, annotationKey string, timeout time.Duration) (bool, error) { priorValue, _ := a.GetPodTemplateAnnotation(ctx, namespace, name, annotationKey) + return a.WaitReloadedFrom(ctx, namespace, name, annotationKey, priorValue, timeout) +} + +// WaitReloadedFrom waits for the reload annotation to be present with a value different from +// priorValue, which the caller captured before triggering the reload. +func (a *ArgoRolloutAdapter) WaitReloadedFrom(ctx context.Context, namespace, name, annotationKey, priorValue string, timeout time.Duration) (bool, error) { watchFunc := func(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { return a.rolloutsClient.ArgoprojV1alpha1().Rollouts(namespace).Watch(ctx, opts) } @@ -72,6 +78,12 @@ func (a *ArgoRolloutAdapter) WaitEnvVar(ctx context.Context, namespace, name, pr if r, err := a.rolloutsClient.ArgoprojV1alpha1().Rollouts(namespace).Get(ctx, name, metav1.GetOptions{}); err == nil { priorValue = GetEnvVarValueByPrefix(r.Spec.Template.Spec.Containers, prefix) } + return a.WaitEnvVarFrom(ctx, namespace, name, prefix, priorValue, timeout) +} + +// WaitEnvVarFrom waits for a STAKATER_ env var whose value differs from priorValue, which the +// caller captured before triggering the reload. +func (a *ArgoRolloutAdapter) WaitEnvVarFrom(ctx context.Context, namespace, name, prefix, priorValue string, timeout time.Duration) (bool, error) { watchFunc := func(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { return a.rolloutsClient.ArgoprojV1alpha1().Rollouts(namespace).Watch(ctx, opts) } diff --git a/test/e2e/utils/workload_cronjob.go b/test/e2e/utils/workload_cronjob.go index c681cce96..151fe9ae9 100644 --- a/test/e2e/utils/workload_cronjob.go +++ b/test/e2e/utils/workload_cronjob.go @@ -50,6 +50,12 @@ func (a *CronJobAdapter) WaitReady(ctx context.Context, namespace, name string, // Captures the current annotation value first to avoid false positives from prior reloads. func (a *CronJobAdapter) WaitReloaded(ctx context.Context, namespace, name, annotationKey string, timeout time.Duration) (bool, error) { priorValue, _ := a.GetPodTemplateAnnotation(ctx, namespace, name, annotationKey) + return a.WaitReloadedFrom(ctx, namespace, name, annotationKey, priorValue, timeout) +} + +// WaitReloadedFrom waits for the reload annotation to be present with a value different from +// priorValue, which the caller captured before triggering the reload. +func (a *CronJobAdapter) WaitReloadedFrom(ctx context.Context, namespace, name, annotationKey, priorValue string, timeout time.Duration) (bool, error) { watchFunc := func(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { return a.client.BatchV1().CronJobs(namespace).Watch(ctx, opts) } @@ -62,6 +68,11 @@ func (a *CronJobAdapter) WaitEnvVar(ctx context.Context, namespace, name, prefix return false, ErrUnsupportedOperation } +// WaitEnvVarFrom returns an error because CronJobs don't support env var reload strategy. +func (a *CronJobAdapter) WaitEnvVarFrom(ctx context.Context, namespace, name, prefix, priorValue string, timeout time.Duration) (bool, error) { + return false, ErrUnsupportedOperation +} + // SupportsEnvVarStrategy returns false as CronJobs don't support env var reload strategy. func (a *CronJobAdapter) SupportsEnvVarStrategy() bool { return false diff --git a/test/e2e/utils/workload_daemonset.go b/test/e2e/utils/workload_daemonset.go index 4a7a2b14e..74aada0ee 100644 --- a/test/e2e/utils/workload_daemonset.go +++ b/test/e2e/utils/workload_daemonset.go @@ -50,6 +50,12 @@ func (a *DaemonSetAdapter) WaitReady(ctx context.Context, namespace, name string // Captures the current annotation value first to avoid false positives from prior reloads. func (a *DaemonSetAdapter) WaitReloaded(ctx context.Context, namespace, name, annotationKey string, timeout time.Duration) (bool, error) { priorValue, _ := a.GetPodTemplateAnnotation(ctx, namespace, name, annotationKey) + return a.WaitReloadedFrom(ctx, namespace, name, annotationKey, priorValue, timeout) +} + +// WaitReloadedFrom waits for the reload annotation to be present with a value different from +// priorValue, which the caller captured before triggering the reload. +func (a *DaemonSetAdapter) WaitReloadedFrom(ctx context.Context, namespace, name, annotationKey, priorValue string, timeout time.Duration) (bool, error) { watchFunc := func(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { return a.client.AppsV1().DaemonSets(namespace).Watch(ctx, opts) } @@ -64,6 +70,12 @@ func (a *DaemonSetAdapter) WaitEnvVar(ctx context.Context, namespace, name, pref if ds, err := a.client.AppsV1().DaemonSets(namespace).Get(ctx, name, metav1.GetOptions{}); err == nil { priorValue = GetEnvVarValueByPrefix(ds.Spec.Template.Spec.Containers, prefix) } + return a.WaitEnvVarFrom(ctx, namespace, name, prefix, priorValue, timeout) +} + +// WaitEnvVarFrom waits for a STAKATER_ env var whose value differs from priorValue, which the +// caller captured before triggering the reload. +func (a *DaemonSetAdapter) WaitEnvVarFrom(ctx context.Context, namespace, name, prefix, priorValue string, timeout time.Duration) (bool, error) { watchFunc := func(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { return a.client.AppsV1().DaemonSets(namespace).Watch(ctx, opts) } diff --git a/test/e2e/utils/workload_deployment.go b/test/e2e/utils/workload_deployment.go index f7ef5e37a..d64e194a1 100644 --- a/test/e2e/utils/workload_deployment.go +++ b/test/e2e/utils/workload_deployment.go @@ -51,6 +51,12 @@ func (a *DeploymentAdapter) WaitReady(ctx context.Context, namespace, name strin // does not cause a false positive — the condition triggers only when the value changes. func (a *DeploymentAdapter) WaitReloaded(ctx context.Context, namespace, name, annotationKey string, timeout time.Duration) (bool, error) { priorValue, _ := a.GetPodTemplateAnnotation(ctx, namespace, name, annotationKey) + return a.WaitReloadedFrom(ctx, namespace, name, annotationKey, priorValue, timeout) +} + +// WaitReloadedFrom waits for the reload annotation to be present with a value different from +// priorValue, which the caller captured before triggering the reload. +func (a *DeploymentAdapter) WaitReloadedFrom(ctx context.Context, namespace, name, annotationKey, priorValue string, timeout time.Duration) (bool, error) { watchFunc := func(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { return a.client.AppsV1().Deployments(namespace).Watch(ctx, opts) } @@ -66,6 +72,12 @@ func (a *DeploymentAdapter) WaitEnvVar(ctx context.Context, namespace, name, pre if d, err := a.client.AppsV1().Deployments(namespace).Get(ctx, name, metav1.GetOptions{}); err == nil { priorValue = GetEnvVarValueByPrefix(d.Spec.Template.Spec.Containers, prefix) } + return a.WaitEnvVarFrom(ctx, namespace, name, prefix, priorValue, timeout) +} + +// WaitEnvVarFrom waits for a STAKATER_ env var whose value differs from priorValue, which the +// caller captured before triggering the reload. +func (a *DeploymentAdapter) WaitEnvVarFrom(ctx context.Context, namespace, name, prefix, priorValue string, timeout time.Duration) (bool, error) { watchFunc := func(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { return a.client.AppsV1().Deployments(namespace).Watch(ctx, opts) } diff --git a/test/e2e/utils/workload_job.go b/test/e2e/utils/workload_job.go index e71c86c25..5f8ad5065 100644 --- a/test/e2e/utils/workload_job.go +++ b/test/e2e/utils/workload_job.go @@ -54,11 +54,22 @@ func (a *JobAdapter) WaitReloaded(ctx context.Context, namespace, name, annotati return false, ErrUnsupportedOperation } +// WaitReloadedFrom returns an error because Jobs are recreated, not updated. +// Use the Recreatable interface (GetOriginalUID + WaitRecreated) instead. +func (a *JobAdapter) WaitReloadedFrom(ctx context.Context, namespace, name, annotationKey, priorValue string, timeout time.Duration) (bool, error) { + return false, ErrUnsupportedOperation +} + // WaitEnvVar returns an error because Jobs don't support env var reload strategy. func (a *JobAdapter) WaitEnvVar(ctx context.Context, namespace, name, prefix string, timeout time.Duration) (bool, error) { return false, ErrUnsupportedOperation } +// WaitEnvVarFrom returns an error because Jobs don't support env var reload strategy. +func (a *JobAdapter) WaitEnvVarFrom(ctx context.Context, namespace, name, prefix, priorValue string, timeout time.Duration) (bool, error) { + return false, ErrUnsupportedOperation +} + // WaitRecreated waits for the Job to be recreated with a different UID using watches. func (a *JobAdapter) WaitRecreated(ctx context.Context, namespace, name, originalUID string, timeout time.Duration) (string, bool, error) { watchFunc := func(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { diff --git a/test/e2e/utils/workload_openshift.go b/test/e2e/utils/workload_openshift.go index 6f758bf43..2fb11c6c6 100644 --- a/test/e2e/utils/workload_openshift.go +++ b/test/e2e/utils/workload_openshift.go @@ -60,6 +60,12 @@ func (a *DeploymentConfigAdapter) WaitReady(ctx context.Context, namespace, name // Captures the current annotation value first to avoid false positives from prior reloads. func (a *DeploymentConfigAdapter) WaitReloaded(ctx context.Context, namespace, name, annotationKey string, timeout time.Duration) (bool, error) { priorValue, _ := a.GetPodTemplateAnnotation(ctx, namespace, name, annotationKey) + return a.WaitReloadedFrom(ctx, namespace, name, annotationKey, priorValue, timeout) +} + +// WaitReloadedFrom waits for the reload annotation to be present with a value different from +// priorValue, which the caller captured before triggering the reload. +func (a *DeploymentConfigAdapter) WaitReloadedFrom(ctx context.Context, namespace, name, annotationKey, priorValue string, timeout time.Duration) (bool, error) { watchFunc := func(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { return a.openshiftClient.AppsV1().DeploymentConfigs(namespace).Watch(ctx, opts) } @@ -74,6 +80,12 @@ func (a *DeploymentConfigAdapter) WaitEnvVar(ctx context.Context, namespace, nam if dc, err := a.openshiftClient.AppsV1().DeploymentConfigs(namespace).Get(ctx, name, metav1.GetOptions{}); err == nil && dc.Spec.Template != nil { priorValue = GetEnvVarValueByPrefix(dc.Spec.Template.Spec.Containers, prefix) } + return a.WaitEnvVarFrom(ctx, namespace, name, prefix, priorValue, timeout) +} + +// WaitEnvVarFrom waits for a STAKATER_ env var whose value differs from priorValue, which the +// caller captured before triggering the reload. +func (a *DeploymentConfigAdapter) WaitEnvVarFrom(ctx context.Context, namespace, name, prefix, priorValue string, timeout time.Duration) (bool, error) { watchFunc := func(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { return a.openshiftClient.AppsV1().DeploymentConfigs(namespace).Watch(ctx, opts) } diff --git a/test/e2e/utils/workload_statefulset.go b/test/e2e/utils/workload_statefulset.go index d071678a4..28052dce3 100644 --- a/test/e2e/utils/workload_statefulset.go +++ b/test/e2e/utils/workload_statefulset.go @@ -50,6 +50,12 @@ func (a *StatefulSetAdapter) WaitReady(ctx context.Context, namespace, name stri // Captures the current annotation value first to avoid false positives from prior reloads. func (a *StatefulSetAdapter) WaitReloaded(ctx context.Context, namespace, name, annotationKey string, timeout time.Duration) (bool, error) { priorValue, _ := a.GetPodTemplateAnnotation(ctx, namespace, name, annotationKey) + return a.WaitReloadedFrom(ctx, namespace, name, annotationKey, priorValue, timeout) +} + +// WaitReloadedFrom waits for the reload annotation to be present with a value different from +// priorValue, which the caller captured before triggering the reload. +func (a *StatefulSetAdapter) WaitReloadedFrom(ctx context.Context, namespace, name, annotationKey, priorValue string, timeout time.Duration) (bool, error) { watchFunc := func(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { return a.client.AppsV1().StatefulSets(namespace).Watch(ctx, opts) } @@ -64,6 +70,12 @@ func (a *StatefulSetAdapter) WaitEnvVar(ctx context.Context, namespace, name, pr if sts, err := a.client.AppsV1().StatefulSets(namespace).Get(ctx, name, metav1.GetOptions{}); err == nil { priorValue = GetEnvVarValueByPrefix(sts.Spec.Template.Spec.Containers, prefix) } + return a.WaitEnvVarFrom(ctx, namespace, name, prefix, priorValue, timeout) +} + +// WaitEnvVarFrom waits for a STAKATER_ env var whose value differs from priorValue, which the +// caller captured before triggering the reload. +func (a *StatefulSetAdapter) WaitEnvVarFrom(ctx context.Context, namespace, name, prefix, priorValue string, timeout time.Duration) (bool, error) { watchFunc := func(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { return a.client.AppsV1().StatefulSets(namespace).Watch(ctx, opts) } From 6195dd91e14fe6e9f64d70bf90ccf10bdf1d12b3 Mon Sep 17 00:00:00 2001 From: Safwan Date: Wed, 15 Jul 2026 12:33:40 +0500 Subject: [PATCH 7/8] some refactoring and cleanup --- cmd/reloader/main.go | 37 +++--- .../chart/reloader/templates/_helpers.tpl | 61 ++++++++-- .../chart/reloader/templates/deployment.yaml | 1 + .../chart/reloader/templates/role.yaml | 16 +-- .../chart/reloader/templates/rolebinding.yaml | 2 +- .../kubernetes/chart/reloader/values.yaml | 12 +- internal/pkg/config/config.go | 22 ---- internal/pkg/config/config_test.go | 33 ----- internal/pkg/config/flags.go | 39 ++++-- internal/pkg/config/flags_test.go | 57 ++++----- internal/pkg/metadata/publisher.go | 19 ++- test/e2e/flags/watch_namespaces_test.go | 115 ++++++++++++++++++ 12 files changed, 263 insertions(+), 151 deletions(-) create mode 100644 test/e2e/flags/watch_namespaces_test.go diff --git a/cmd/reloader/main.go b/cmd/reloader/main.go index 781d28c56..b1f7f7f9b 100644 --- a/cmd/reloader/main.go +++ b/cmd/reloader/main.go @@ -54,8 +54,15 @@ func newReloaderCommand() *cobra.Command { } func run(cmd *cobra.Command, args []string) error { - scopeWarnings, err := config.ApplyFlags(cfg) + // Configure logging first so ApplyFlags can surface namespace-scope warnings + // through a ready logger instead of returning them to the caller. + log, err := configureLogging(config.LoggingFlags()) if err != nil { + return fmt.Errorf("configuring logging: %w", err) + } + controllerruntime.SetLogger(log) + + if err := config.ApplyFlags(cfg, log); err != nil { return fmt.Errorf("applying flags: %w", err) } @@ -73,21 +80,8 @@ func run(cmd *cobra.Command, args []string) error { } } - log, err := configureLogging(cfg.LogFormat, cfg.LogLevel) - if err != nil { - return fmt.Errorf("configuring logging: %w", err) - } - - controllerruntime.SetLogger(log) - log.Info("Starting Reloader") - // Namespace-scope semantics are enforced in ApplyFlags; surface any warnings - // it produced now that logging is configured. - for _, w := range scopeWarnings { - log.Info(w) - } - if cfg.IsGlobalMode() { log.Info("watching all namespaces") } else { @@ -156,14 +150,13 @@ func run(cmd *cobra.Command, args []string) error { return fmt.Errorf("setting up reconcilers: %w", err) } - // Skip metadata publisher when ConfigMaps are ignored (no RBAC permissions) - if !cfg.IsResourceIgnored("configmaps") { - if err := mgr.Add(metadata.Runnable(mgr.GetClient(), cfg, log)); err != nil { - log.Error(err, "Failed to add metadata publisher") - // Non-fatal, continue starting - } - } else { - log.Info("skipping metadata publisher (configmaps ignored)") + // Meta-info is internal instance metadata and is always published. The + // publisher builds its own uncached client (see metadata.Runnable) because + // the ConfigMap lives in Reloader's own namespace, which the manager cache + // does not cover in scoped mode. + if err := mgr.Add(metadata.Runnable(mgr.GetConfig(), mgr.GetScheme(), cfg, log)); err != nil { + log.Error(err, "Failed to add metadata publisher") + // Non-fatal, continue starting } if cfg.EnablePProf { diff --git a/deployments/kubernetes/chart/reloader/templates/_helpers.tpl b/deployments/kubernetes/chart/reloader/templates/_helpers.tpl index ec268917a..32d129c94 100644 --- a/deployments/kubernetes/chart/reloader/templates/_helpers.tpl +++ b/deployments/kubernetes/chart/reloader/templates/_helpers.tpl @@ -117,15 +117,56 @@ Comma-joined form of reloader-watchNamespaces, for the --namespaces CLI flag. {{- end -}} {{/* -Namespaces that need namespaced RBAC in scoped mode: the watched namespaces plus -the release namespace, so leader-election leases, the meta-info ConfigMap and -events keep working there even though it is not watched for reloads. -Returns a JSON-encoded list; consumers use mustFromJson to iterate. +Fails the render on an inconsistent namespace configuration: reloader.namespaces +(scoped mode) requires reloader.watchGlobally=false. Included from deployment.yaml +so it is validated once regardless of which templates render. */}} -{{- define "reloader-rbacNamespaces" -}} -{{- $relNs := .Values.namespace | default .Release.Namespace -}} -{{- $watch := include "reloader-watchNamespaces" . | mustFromJson -}} -{{- concat (list $relNs) $watch | uniq | sortAlpha | toJson -}} +{{- define "reloader-validate-namespaces" -}} +{{- if and .Values.reloader.watchGlobally .Values.reloader.namespaces -}} +{{- fail "reloader.namespaces is set but reloader.watchGlobally is true; set reloader.watchGlobally=false to use scoped namespace mode." -}} +{{- end -}} +{{- end -}} + +{{/* +RBAC rules Reloader needs in its own (release) namespace, independent of the +watched namespaces. Reloader publishes an internal meta-info ConfigMap there in +every mode, so configmap write access is always granted. In scoped mode the +release namespace is not covered by the watch RBAC, so leader-election events +(and leases under HA) are granted here too; in global/single mode those are +already covered by the ClusterRole or the single-namespace Role. +Expects the root context ($) as its argument. +*/}} +{{- define "reloader-release-rules" }} + - apiGroups: + - "" + resources: + - configmaps + verbs: + - get + - create + - update + - patch +{{- if .Values.reloader.namespaces }} + - apiGroups: + - "" + - "events.k8s.io" + resources: + - events + verbs: + - create + - patch + - update +{{- if .Values.reloader.enableHA }} + - apiGroups: + - "coordination.k8s.io" + resources: + - leases + verbs: + - create + - get + - update +{{- end }} +{{- end }} {{- end -}} {{/* @@ -183,6 +224,7 @@ the rule set is defined once. Expects the root context ($) as its argument. - watch - update - patch +{{- if .Values.reloader.ignoreCronJobs }}{{- else }} - apiGroups: - "batch" resources: @@ -193,6 +235,8 @@ the rule set is defined once. Expects the root context ($) as its argument. - watch - update - patch +{{- end }} +{{- if .Values.reloader.ignoreJobs }}{{- else }} - apiGroups: - "batch" resources: @@ -203,6 +247,7 @@ the rule set is defined once. Expects the root context ($) as its argument. - list - get - watch +{{- end }} {{- if .Values.reloader.enableHA }} - apiGroups: - "coordination.k8s.io" diff --git a/deployments/kubernetes/chart/reloader/templates/deployment.yaml b/deployments/kubernetes/chart/reloader/templates/deployment.yaml index da5a1d540..ff43894a5 100644 --- a/deployments/kubernetes/chart/reloader/templates/deployment.yaml +++ b/deployments/kubernetes/chart/reloader/templates/deployment.yaml @@ -1,3 +1,4 @@ +{{- include "reloader-validate-namespaces" . -}} apiVersion: apps/v1 kind: Deployment metadata: diff --git a/deployments/kubernetes/chart/reloader/templates/role.yaml b/deployments/kubernetes/chart/reloader/templates/role.yaml index b7b5cd929..0454eadff 100644 --- a/deployments/kubernetes/chart/reloader/templates/role.yaml +++ b/deployments/kubernetes/chart/reloader/templates/role.yaml @@ -1,13 +1,10 @@ -{{- if and .Values.reloader.watchGlobally .Values.reloader.namespaces }} -{{- fail "reloader.namespaces is set but reloader.watchGlobally is true; set reloader.watchGlobally=false to use scoped namespace mode." }} -{{- end }} {{- if and (not (.Values.reloader.watchGlobally)) (.Values.reloader.rbac.enabled) }} {{- $apiVersion := "rbac.authorization.k8s.io/v1" }} {{- if not (.Capabilities.APIVersions.Has "rbac.authorization.k8s.io/v1") }} {{- $apiVersion = "rbac.authorization.k8s.io/v1beta1" }} {{- end }} {{- if .Values.reloader.namespaces }} -{{- range $ns := (include "reloader-rbacNamespaces" . | mustFromJson) }} +{{- range $ns := (include "reloader-watchNamespaces" . | mustFromJson) }} apiVersion: {{ $apiVersion }} kind: Role metadata: @@ -67,14 +64,5 @@ metadata: name: {{ template "reloader-fullname" . }}-metadata-role namespace: {{ .Values.namespace | default .Release.Namespace }} rules: - - apiGroups: - - "" - resources: - - configmaps - verbs: - - list - - get - - watch - - create - - update +{{- include "reloader-release-rules" . }} {{- end }} \ No newline at end of file diff --git a/deployments/kubernetes/chart/reloader/templates/rolebinding.yaml b/deployments/kubernetes/chart/reloader/templates/rolebinding.yaml index 187c90ddb..7d73b1822 100644 --- a/deployments/kubernetes/chart/reloader/templates/rolebinding.yaml +++ b/deployments/kubernetes/chart/reloader/templates/rolebinding.yaml @@ -4,7 +4,7 @@ {{- $apiVersion = "rbac.authorization.k8s.io/v1beta1" }} {{- end }} {{- if .Values.reloader.namespaces }} -{{- range $ns := (include "reloader-rbacNamespaces" . | mustFromJson) }} +{{- range $ns := (include "reloader-watchNamespaces" . | mustFromJson) }} apiVersion: {{ $apiVersion }} kind: RoleBinding metadata: diff --git a/deployments/kubernetes/chart/reloader/values.yaml b/deployments/kubernetes/chart/reloader/values.yaml index f236bdf71..119bc8e33 100644 --- a/deployments/kubernetes/chart/reloader/values.yaml +++ b/deployments/kubernetes/chart/reloader/values.yaml @@ -45,11 +45,13 @@ reloader: logFormat: "" # json logLevel: info # Log level to use (trace, debug, info, warning, error, fatal and panic) watchGlobally: true - # Scoped mode: explicit list of namespaces to watch. When non-empty (and watchGlobally - # is false), Reloader watches exactly these namespaces and the chart creates a namespace - # scoped Role + RoleBinding in each one — no ClusterRole is created. The release namespace - # is always included automatically. Leave empty ([]) for the default single-namespace or - # global behavior controlled by watchGlobally. + # Scoped mode: explicit list of namespaces to watch. When non-empty you must also set + # watchGlobally=false. Reloader watches exactly these namespaces and the chart creates a + # namespaced Role + RoleBinding in each one — no ClusterRole is created. Reloader's own + # (release) namespace is NOT watched for reloads; it only receives a small metadata Role + # for the internal meta-info ConfigMap and (under HA) leader-election leases/events. + # Leave empty ([]) for the default single-namespace or global behavior controlled by + # watchGlobally. # Accepts either a YAML list (e.g. ["team-a", "team-b"]) or a comma-separated string # (e.g. "team-a,team-b") namespaces: [] diff --git a/internal/pkg/config/config.go b/internal/pkg/config/config.go index 726a6e8b2..c1c7faed4 100644 --- a/internal/pkg/config/config.go +++ b/internal/pkg/config/config.go @@ -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 -} diff --git a/internal/pkg/config/config_test.go b/internal/pkg/config/config_test.go index 62d1a610d..218c34d5c 100644 --- a/internal/pkg/config/config_test.go +++ b/internal/pkg/config/config_test.go @@ -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") - } -} diff --git a/internal/pkg/config/flags.go b/internal/pkg/config/flags.go index 6680abcaf..70ba52258 100644 --- a/internal/pkg/config/flags.go +++ b/internal/pkg/config/flags.go @@ -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. diff --git a/internal/pkg/config/flags_test.go b/internal/pkg/config/flags_test.go index 233a65272..45d6d2cb3 100644 --- a/internal/pkg/config/flags_test.go +++ b/internal/pkg/config/flags_test.go @@ -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) - } } diff --git a/internal/pkg/metadata/publisher.go b/internal/pkg/metadata/publisher.go index 6c6a42221..8cd349e15 100644 --- a/internal/pkg/metadata/publisher.go +++ b/internal/pkg/metadata/publisher.go @@ -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 diff --git a/test/e2e/flags/watch_namespaces_test.go b/test/e2e/flags/watch_namespaces_test.go new file mode 100644 index 000000000..bcfbd6cbd --- /dev/null +++ b/test/e2e/flags/watch_namespaces_test.go @@ -0,0 +1,115 @@ +package flags + +import ( + "fmt" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/stakater/Reloader/test/e2e/utils" +) + +var _ = Describe("Watch Namespaces (scoped mode) Flag Tests", Serial, func() { + var ( + deploymentName string + configMapName string + watchedNS string + unwatchedNS string + adapter *utils.DeploymentAdapter + ) + + BeforeEach(func() { + deploymentName = utils.RandName("deploy") + configMapName = utils.RandName("cm") + watchedNS = "watched-" + utils.RandName("ns") + unwatchedNS = "unwatched-" + utils.RandName("ns") + adapter = utils.NewDeploymentAdapter(kubeClient) + + // The watched namespace must exist before install: in scoped mode the + // chart creates a Role/RoleBinding in it. + Expect(utils.CreateNamespace(ctx, kubeClient, watchedNS)).To(Succeed()) + Expect(utils.CreateNamespace(ctx, kubeClient, unwatchedNS)).To(Succeed()) + + err := deployReloaderWithFlags(map[string]string{ + "reloader.watchGlobally": "false", + "reloader.namespaces": fmt.Sprintf("{%s}", watchedNS), + }) + Expect(err).NotTo(HaveOccurred()) + + Expect(waitForReloaderReady()).To(Succeed()) + }) + + AfterEach(func() { + _ = utils.DeleteDeployment(ctx, kubeClient, watchedNS, deploymentName) + _ = utils.DeleteConfigMap(ctx, kubeClient, watchedNS, configMapName) + _ = utils.DeleteDeployment(ctx, kubeClient, unwatchedNS, deploymentName) + _ = utils.DeleteConfigMap(ctx, kubeClient, unwatchedNS, configMapName) + _ = undeployReloader() + _ = utils.DeleteNamespace(ctx, kubeClient, watchedNS) + _ = utils.DeleteNamespace(ctx, kubeClient, unwatchedNS) + }) + + It("should reload workloads in a watched namespace", func() { + By("Creating a ConfigMap in the watched namespace") + _, err := utils.CreateConfigMap(ctx, kubeClient, watchedNS, configMapName, + map[string]string{"key": "initial"}, nil) + Expect(err).NotTo(HaveOccurred()) + + By("Creating a Deployment in the watched namespace with auto annotation") + _, err = utils.CreateDeployment(ctx, kubeClient, watchedNS, deploymentName, + utils.WithConfigMapEnvFrom(configMapName), + utils.WithAnnotations(utils.BuildAutoTrueAnnotation()), + ) + Expect(err).NotTo(HaveOccurred()) + + By("Waiting for Deployment to be ready") + Expect(adapter.WaitReady(ctx, watchedNS, deploymentName, utils.WorkloadReadyTimeout)).To(Succeed()) + + By("Updating the ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, watchedNS, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + err = utils.UpdateConfigMap(ctx, kubeClient, watchedNS, configMapName, map[string]string{"key": "updated"}) + Expect(err).NotTo(HaveOccurred()) + + By("Waiting for Deployment to be reloaded (watched namespace should work)") + reloaded, err := adapter.WaitReloadedFrom(ctx, watchedNS, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) + Expect(err).NotTo(HaveOccurred()) + Expect(reloaded).To(BeTrue(), "Deployment in a watched namespace should reload") + }) + + It("should NOT reload workloads in an unwatched namespace", func() { + By("Creating a ConfigMap in an unwatched namespace") + _, err := utils.CreateConfigMap(ctx, kubeClient, unwatchedNS, configMapName, + map[string]string{"key": "initial"}, nil) + Expect(err).NotTo(HaveOccurred()) + + By("Creating a Deployment in an unwatched namespace with auto annotation") + _, err = utils.CreateDeployment(ctx, kubeClient, unwatchedNS, deploymentName, + utils.WithConfigMapEnvFrom(configMapName), + utils.WithAnnotations(utils.BuildAutoTrueAnnotation()), + ) + Expect(err).NotTo(HaveOccurred()) + + By("Waiting for Deployment to be ready") + Expect(adapter.WaitReady(ctx, unwatchedNS, deploymentName, utils.WorkloadReadyTimeout)).To(Succeed()) + + By("Updating the ConfigMap in the unwatched namespace") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, unwatchedNS, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + err = utils.UpdateConfigMap(ctx, kubeClient, unwatchedNS, configMapName, map[string]string{"key": "updated"}) + Expect(err).NotTo(HaveOccurred()) + + By("Verifying Deployment was NOT reloaded (namespace not in --namespaces)") + time.Sleep(utils.NegativeTestWait) + reloaded, err := adapter.WaitReloadedFrom(ctx, unwatchedNS, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) + Expect(err).NotTo(HaveOccurred()) + Expect(reloaded).To(BeFalse(), "Deployment in an unwatched namespace should NOT reload") + }) +}) From 80032eb380ad8730481973fe762cdc7db09b6e4c Mon Sep 17 00:00:00 2001 From: Safwan Date: Fri, 17 Jul 2026 12:03:38 +0500 Subject: [PATCH 8/8] resolved comments --- .../kubernetes/chart/reloader/README.md | 2 +- .../chart/reloader/templates/_helpers.tpl | 63 +++-- .../chart/reloader/templates/deployment.yaml | 12 +- .../chart/reloader/templates/role.yaml | 2 +- .../chart/reloader/templates/rolebinding.yaml | 2 +- .../reloader/tests/namespaces_flags_test.yaml | 134 ++++++++++ .../reloader/tests/namespaces_rbac_test.yaml | 234 ++++++++++++++++++ internal/pkg/config/validation.go | 14 ++ internal/pkg/config/validation_test.go | 34 +++ test/e2e/flags/watch_namespaces_test.go | 90 +++++++ 10 files changed, 560 insertions(+), 27 deletions(-) create mode 100644 deployments/kubernetes/chart/reloader/tests/namespaces_flags_test.yaml create mode 100644 deployments/kubernetes/chart/reloader/tests/namespaces_rbac_test.yaml diff --git a/deployments/kubernetes/chart/reloader/README.md b/deployments/kubernetes/chart/reloader/README.md index 7b453ac47..6eb65d216 100644 --- a/deployments/kubernetes/chart/reloader/README.md +++ b/deployments/kubernetes/chart/reloader/README.md @@ -61,7 +61,7 @@ helm uninstall {{RELEASE_NAME}} -n {{NAMESPACE}} | `reloader.resourceLabelSelector` | List of comma separated label selectors, if multiple are provided they are combined with the AND operator | string | `""` | | `reloader.logFormat` | Set type of log format. Value could be either `json` or `""` | string | `""` | | `reloader.watchGlobally` | Allow Reloader to watch in all namespaces (`true`) or just in a single namespace (`false`) | boolean | `true` | -| `reloader.namespaces` | Explicit namespaces to watch (scoped mode). When non-empty and `reloader.watchGlobally` is `false`, Reloader watches exactly these namespaces and the chart creates a namespace-scoped Role + RoleBinding in each (no ClusterRole). The release namespace is always included automatically. Accepts either a YAML list (`["team-a","team-b"]`) or a comma-separated string (`"team-a,team-b"`). | list/string | `[]` | +| `reloader.namespaces` | Explicit namespaces to watch (scoped mode). When non-empty and `reloader.watchGlobally` is `false`, Reloader watches exactly these namespaces and the chart creates a namespace-scoped Role + RoleBinding in each (no ClusterRole). The release namespace is not watched for reloads unless you list it explicitly; the chart only grants it a minimal Role for Reloader's internal meta-info ConfigMap (and leader-election in HA). Accepts either a YAML list (`["team-a","team-b"]`) or a comma-separated string (`"team-a,team-b"`). | list/string | `[]` | | `reloader.enableHA` | Enable leadership election allowing you to run multiple replicas | boolean | `false` | | `reloader.enablePProf` | Enables pprof for profiling | boolean | `false` | | `reloader.pprofAddr` | Address to start pprof server on | string | `:6060` | diff --git a/deployments/kubernetes/chart/reloader/templates/_helpers.tpl b/deployments/kubernetes/chart/reloader/templates/_helpers.tpl index 32d129c94..b2c6186e8 100644 --- a/deployments/kubernetes/chart/reloader/templates/_helpers.tpl +++ b/deployments/kubernetes/chart/reloader/templates/_helpers.tpl @@ -91,7 +91,10 @@ Create the namespace selector if it does not watch globally {{/* Namespaces to watch in scoped mode: exactly the user-supplied reloader.namespaces, trimmed, de-duped and sorted. The release namespace is intentionally NOT added here -— Reloader watches only what the user asked for (an empty result means global mode). +— Reloader watches only what the user asked for. An empty result is not necessarily +global mode: with watchGlobally=false it becomes single-namespace mode (the release +namespace, injected via --namespaces by reloader-effectiveNamespaces-csv); only with +watchGlobally=true does empty mean watch-all. Returns a JSON-encoded list; consumers use mustFromJson to iterate. */}} {{- define "reloader-watchNamespaces" -}} @@ -116,13 +119,45 @@ Comma-joined form of reloader-watchNamespaces, for the --namespaces CLI flag. {{- include "reloader-watchNamespaces" . | mustFromJson | join "," -}} {{- end -}} +{{/* +The effective watched namespaces for the --namespaces CLI flag — the single +chart-side source of truth for watch scope, so the binary never has to fall back +to the KUBERNETES_NAMESPACE env: + - scoped mode -> the cleaned reloader.namespaces list + - single-namespace mode -> the release namespace (watchGlobally=false, no list) + - global mode -> empty (no --namespaces flag; watch all) +Returns a comma-joined string ("" in global mode). +*/}} +{{- define "reloader-effectiveNamespaces-csv" -}} +{{- $watch := include "reloader-watchNamespaces" . | mustFromJson -}} +{{- if $watch -}} +{{- $watch | join "," -}} +{{- else if not .Values.reloader.watchGlobally -}} +{{- .Values.namespace | default .Release.Namespace -}} +{{- end -}} +{{- end -}} + +{{/* +Whether Reloader runs in scoped mode. This is the single source of truth for the +scoped-vs-global decision: it is true only when the cleaned watch list +(reloader-watchNamespaces) is non-empty. Gate on this rather than the raw +.Values.reloader.namespaces, which is truthy even for values like " , " that trim +to an empty list (those must fall through to global/single-namespace mode). +Returns "true" (truthy) or "" (falsy). +*/}} +{{- define "reloader-isScoped" -}} +{{- if include "reloader-watchNamespaces" . | mustFromJson -}} +true +{{- end -}} +{{- end -}} + {{/* Fails the render on an inconsistent namespace configuration: reloader.namespaces (scoped mode) requires reloader.watchGlobally=false. Included from deployment.yaml so it is validated once regardless of which templates render. */}} {{- define "reloader-validate-namespaces" -}} -{{- if and .Values.reloader.watchGlobally .Values.reloader.namespaces -}} +{{- if and .Values.reloader.watchGlobally (include "reloader-isScoped" .) -}} {{- fail "reloader.namespaces is set but reloader.watchGlobally is true; set reloader.watchGlobally=false to use scoped namespace mode." -}} {{- end -}} {{- end -}} @@ -131,9 +166,9 @@ so it is validated once regardless of which templates render. RBAC rules Reloader needs in its own (release) namespace, independent of the watched namespaces. Reloader publishes an internal meta-info ConfigMap there in every mode, so configmap write access is always granted. In scoped mode the -release namespace is not covered by the watch RBAC, so leader-election events -(and leases under HA) are granted here too; in global/single mode those are -already covered by the ClusterRole or the single-namespace Role. +release namespace is not covered by the watch RBAC, so under HA the leader-election +leases and the events it emits are granted here too; in global/single mode those +are already covered by the ClusterRole or the single-namespace Role. Expects the root context ($) as its argument. */}} {{- define "reloader-release-rules" }} @@ -146,27 +181,25 @@ Expects the root context ($) as its argument. - create - update - patch -{{- if .Values.reloader.namespaces }} +{{- if and (include "reloader-isScoped" .) .Values.reloader.enableHA }} - apiGroups: - - "" - - "events.k8s.io" + - "coordination.k8s.io" resources: - - events + - leases verbs: - create - - patch + - get - update -{{- if .Values.reloader.enableHA }} - apiGroups: - - "coordination.k8s.io" + - "" + - "events.k8s.io" resources: - - leases + - events verbs: - create - - get + - patch - update {{- end }} -{{- end }} {{- end -}} {{/* diff --git a/deployments/kubernetes/chart/reloader/templates/deployment.yaml b/deployments/kubernetes/chart/reloader/templates/deployment.yaml index ff43894a5..d7b9a6739 100644 --- a/deployments/kubernetes/chart/reloader/templates/deployment.yaml +++ b/deployments/kubernetes/chart/reloader/templates/deployment.yaml @@ -144,12 +144,6 @@ spec: fieldRef: fieldPath: {{ $value | quote}} {{- end }} - {{- end }} - {{- if and (eq .Values.reloader.watchGlobally false) (not .Values.reloader.namespaces) }} - - name: KUBERNETES_NAMESPACE - valueFrom: - fieldRef: - fieldPath: metadata.namespace {{- end }} - name: RELOADER_NAMESPACE @@ -216,7 +210,7 @@ spec: {{- . | toYaml | nindent 10 }} {{- end }} {{- end }} - {{- if or (.Values.reloader.logFormat) (.Values.reloader.logLevel) (.Values.reloader.ignoreSecrets) (and .Values.reloader.ignoreNamespaces .Values.reloader.watchGlobally) (.Values.reloader.namespaces) (include "reloader-namespaceSelector" .) (.Values.reloader.resourceLabelSelector) (.Values.reloader.ignoreConfigMaps) (.Values.reloader.custom_annotations) (eq .Values.reloader.isArgoRollouts true) (eq .Values.reloader.reloadOnCreate true) (eq .Values.reloader.reloadOnDelete true) (ne .Values.reloader.reloadStrategy "default") (.Values.reloader.enableHA) (.Values.reloader.autoReloadAll) (.Values.reloader.ignoreJobs) (.Values.reloader.ignoreCronJobs) (.Values.reloader.enableCSIIntegration)}} + {{- if or (.Values.reloader.logFormat) (.Values.reloader.logLevel) (.Values.reloader.ignoreSecrets) (and .Values.reloader.ignoreNamespaces .Values.reloader.watchGlobally) (include "reloader-effectiveNamespaces-csv" .) (include "reloader-namespaceSelector" .) (.Values.reloader.resourceLabelSelector) (.Values.reloader.ignoreConfigMaps) (.Values.reloader.custom_annotations) (eq .Values.reloader.isArgoRollouts true) (eq .Values.reloader.reloadOnCreate true) (eq .Values.reloader.reloadOnDelete true) (ne .Values.reloader.reloadStrategy "default") (.Values.reloader.enableHA) (.Values.reloader.autoReloadAll) (.Values.reloader.ignoreJobs) (.Values.reloader.ignoreCronJobs) (.Values.reloader.enableCSIIntegration)}} args: {{- if .Values.reloader.logFormat }} - "--log-format={{ .Values.reloader.logFormat }}" @@ -237,8 +231,8 @@ spec: {{- else if .Values.reloader.ignoreCronJobs }} - "--ignored-workload-types=cronjobs" {{- end }} - {{- if .Values.reloader.namespaces }} - - "--namespaces={{ include "reloader-watchNamespaces-csv" . }}" + {{- if (include "reloader-effectiveNamespaces-csv" .) }} + - "--namespaces={{ include "reloader-effectiveNamespaces-csv" . }}" {{- end }} {{- if and .Values.reloader.ignoreNamespaces .Values.reloader.watchGlobally }} - "--namespaces-to-ignore={{ .Values.reloader.ignoreNamespaces }}" diff --git a/deployments/kubernetes/chart/reloader/templates/role.yaml b/deployments/kubernetes/chart/reloader/templates/role.yaml index 0454eadff..4333e73c3 100644 --- a/deployments/kubernetes/chart/reloader/templates/role.yaml +++ b/deployments/kubernetes/chart/reloader/templates/role.yaml @@ -3,7 +3,7 @@ {{- if not (.Capabilities.APIVersions.Has "rbac.authorization.k8s.io/v1") }} {{- $apiVersion = "rbac.authorization.k8s.io/v1beta1" }} {{- end }} -{{- if .Values.reloader.namespaces }} +{{- if (include "reloader-isScoped" .) }} {{- range $ns := (include "reloader-watchNamespaces" . | mustFromJson) }} apiVersion: {{ $apiVersion }} kind: Role diff --git a/deployments/kubernetes/chart/reloader/templates/rolebinding.yaml b/deployments/kubernetes/chart/reloader/templates/rolebinding.yaml index 7d73b1822..7965973b2 100644 --- a/deployments/kubernetes/chart/reloader/templates/rolebinding.yaml +++ b/deployments/kubernetes/chart/reloader/templates/rolebinding.yaml @@ -3,7 +3,7 @@ {{- if not (.Capabilities.APIVersions.Has "rbac.authorization.k8s.io/v1") }} {{- $apiVersion = "rbac.authorization.k8s.io/v1beta1" }} {{- end }} -{{- if .Values.reloader.namespaces }} +{{- if (include "reloader-isScoped" .) }} {{- range $ns := (include "reloader-watchNamespaces" . | mustFromJson) }} apiVersion: {{ $apiVersion }} kind: RoleBinding diff --git a/deployments/kubernetes/chart/reloader/tests/namespaces_flags_test.yaml b/deployments/kubernetes/chart/reloader/tests/namespaces_flags_test.yaml new file mode 100644 index 000000000..04579b167 --- /dev/null +++ b/deployments/kubernetes/chart/reloader/tests/namespaces_flags_test.yaml @@ -0,0 +1,134 @@ +suite: Namespace watch flags & validation +templates: + - deployment.yaml +release: + name: reloader + namespace: reloader-ns +tests: + # ---------------------------------------------------------------- scoped mode + - it: scoped mode passes the watched namespaces via --namespaces + set: + reloader: + watchGlobally: false + namespaces: + - team-a + - team-b + asserts: + - contains: + path: spec.template.spec.containers[0].args + content: "--namespaces=team-a,team-b" + + - it: namespaces are de-duplicated and sorted for the --namespaces flag + set: + reloader: + watchGlobally: false + namespaces: + - team-b + - team-a + - team-a + asserts: + - contains: + path: spec.template.spec.containers[0].args + content: "--namespaces=team-a,team-b" + + - it: a comma-separated string value is accepted for reloader.namespaces + set: + reloader: + watchGlobally: false + namespaces: "team-a,team-b" + asserts: + - contains: + path: spec.template.spec.containers[0].args + content: "--namespaces=team-a,team-b" + + # ----------------------------------------------------------- single-ns mode + - it: single-namespace mode passes the release namespace via --namespaces + set: + reloader: + watchGlobally: false + asserts: + - contains: + path: spec.template.spec.containers[0].args + content: "--namespaces=reloader-ns" + + - it: whitespace-only namespaces fall through to single-namespace mode + set: + reloader: + watchGlobally: false + namespaces: + - " " + - " " + asserts: + - contains: + path: spec.template.spec.containers[0].args + content: "--namespaces=reloader-ns" + + - it: KUBERNETES_NAMESPACE env is no longer injected in single-namespace mode + set: + reloader: + watchGlobally: false + asserts: + - notContains: + path: spec.template.spec.containers[0].env + content: + name: KUBERNETES_NAMESPACE + any: true + + # ---------------------------------------------------------------- global mode + - it: global mode does not pass a --namespaces flag + set: + reloader: + watchGlobally: true + logLevel: info + asserts: + - notContains: + path: spec.template.spec.containers[0].args + content: "--namespaces=reloader-ns" + - notContains: + path: spec.template.spec.containers[0].args + content: "--namespaces=team-a" + + - it: namespaces-to-ignore is only emitted in global mode + set: + reloader: + watchGlobally: true + ignoreNamespaces: "kube-system" + asserts: + - contains: + path: spec.template.spec.containers[0].args + content: "--namespaces-to-ignore=kube-system" + + - it: namespaces-to-ignore is dropped in single-namespace mode + set: + reloader: + watchGlobally: false + ignoreNamespaces: "kube-system" + asserts: + - notContains: + path: spec.template.spec.containers[0].args + content: "--namespaces-to-ignore=kube-system" + + # ------------------------------------------------------------- validation + - it: fails to render when reloader.namespaces is set but watchGlobally is true + set: + reloader: + watchGlobally: true + namespaces: + - team-a + asserts: + - failedTemplate: + errorMessage: "reloader.namespaces is set but reloader.watchGlobally is true; set reloader.watchGlobally=false to use scoped namespace mode." + + - it: does not fail validation for whitespace-only namespaces with watchGlobally true + set: + reloader: + watchGlobally: true + namespaces: + - " " + asserts: + # whitespace trims to an empty watch list, so this is global mode, not scoped -> renders fine + - hasDocuments: + count: 1 + - notContains: + path: spec.template.spec.containers[0].args + content: "--namespaces=reloader-ns" diff --git a/deployments/kubernetes/chart/reloader/tests/namespaces_rbac_test.yaml b/deployments/kubernetes/chart/reloader/tests/namespaces_rbac_test.yaml new file mode 100644 index 000000000..0585cdebb --- /dev/null +++ b/deployments/kubernetes/chart/reloader/tests/namespaces_rbac_test.yaml @@ -0,0 +1,234 @@ +suite: Namespace-scoped RBAC +templates: + - role.yaml + - rolebinding.yaml + - clusterrole.yaml +release: + name: reloader + namespace: reloader-ns +tests: + # ---------------------------------------------------------------- scoped mode + - it: scoped mode renders a Role + RoleBinding per watched namespace plus the metadata role, and no ClusterRole + set: + reloader: + watchGlobally: false + namespaces: + - team-a + - team-b + asserts: + # 2 watched Roles + 1 metadata Role + - hasDocuments: + count: 3 + template: role.yaml + - hasDocuments: + count: 3 + template: rolebinding.yaml + - hasDocuments: + count: 0 + template: clusterrole.yaml + + - it: scoped mode grants the namespaced workload rules in a watched namespace + set: + reloader: + watchGlobally: false + namespaces: + - team-a + - team-b + template: role.yaml + documentSelector: + path: metadata.namespace + value: team-a + asserts: + - isKind: + of: Role + - equal: + path: rules[0].apiGroups[0] + value: "" + - contains: + path: rules[0].resources + content: secrets + - contains: + path: rules[0].resources + content: configmaps + - contains: + path: rules + content: + apiGroups: + - "apps" + resources: + - deployments + - daemonsets + - statefulsets + verbs: + - list + - get + - watch + - update + - patch + + - it: scoped RoleBinding in a watched namespace binds the release ServiceAccount to the namespaced Role + set: + reloader: + watchGlobally: false + namespaces: + - team-a + - team-b + template: rolebinding.yaml + documentSelector: + path: metadata.namespace + value: team-b + asserts: + - isKind: + of: RoleBinding + - equal: + path: roleRef.name + value: reloader-reloader-role + - equal: + path: subjects[0].namespace + value: reloader-ns + + # ------------------------------------------------------------- metadata role + - it: metadata role is created in the release namespace with configmap write in scoped mode + set: + reloader: + watchGlobally: false + namespaces: + - team-a + template: role.yaml + documentSelector: + path: metadata.name + value: reloader-reloader-metadata-role + asserts: + - equal: + path: metadata.namespace + value: reloader-ns + - contains: + path: rules[0].resources + content: configmaps + - contains: + path: rules[0].verbs + content: create + + - it: metadata role also grants leases and events in the release namespace under HA + scoped mode + set: + reloader: + watchGlobally: false + enableHA: true + namespaces: + - team-a + template: role.yaml + documentSelector: + path: metadata.name + value: reloader-reloader-metadata-role + asserts: + - contains: + path: rules + content: + apiGroups: + - "coordination.k8s.io" + resources: + - leases + verbs: + - create + - get + - update + + - it: metadata role does NOT grant leases when HA is disabled in scoped mode + set: + reloader: + watchGlobally: false + enableHA: false + namespaces: + - team-a + template: role.yaml + documentSelector: + path: metadata.name + value: reloader-reloader-metadata-role + asserts: + - notContains: + path: rules + content: + apiGroups: + - "coordination.k8s.io" + resources: + - leases + verbs: + - create + - get + - update + + # ----------------------------------------------------------- single-ns mode + - it: single-namespace mode (watchGlobally=false, no list) renders one Role in the release namespace and no ClusterRole + set: + reloader: + watchGlobally: false + asserts: + # 1 single Role + 1 metadata Role + - hasDocuments: + count: 2 + template: role.yaml + - hasDocuments: + count: 0 + template: clusterrole.yaml + - equal: + path: metadata.namespace + value: reloader-ns + template: role.yaml + documentIndex: 0 + + # ---------------------------------------------------------------- global mode + - it: global mode renders a ClusterRole and only the metadata Role (no watched Roles) + set: + reloader: + watchGlobally: true + asserts: + - hasDocuments: + count: 1 + template: clusterrole.yaml + - isKind: + of: ClusterRole + template: clusterrole.yaml + # only the metadata role remains from role.yaml + - hasDocuments: + count: 1 + template: role.yaml + - equal: + path: metadata.name + value: reloader-reloader-metadata-role + template: role.yaml + documentIndex: 0 + + # -------------------------------------------------------- input normalization + - it: whitespace-only namespaces fall through to single-namespace mode (no watched Roles) + set: + reloader: + watchGlobally: false + namespaces: + - " " + - " " + asserts: + # cleaned list is empty -> single Role + metadata Role, both in release ns + - hasDocuments: + count: 2 + template: role.yaml + - equal: + path: metadata.namespace + value: reloader-ns + template: role.yaml + documentIndex: 0 + + - it: rbac disabled renders no Roles, RoleBindings, or ClusterRole + set: + reloader: + watchGlobally: false + rbac: + enabled: false + namespaces: + - team-a + asserts: + - hasDocuments: + count: 0 + template: role.yaml + - hasDocuments: + count: 0 + template: rolebinding.yaml diff --git a/internal/pkg/config/validation.go b/internal/pkg/config/validation.go index b3d2695b7..e3f725432 100644 --- a/internal/pkg/config/validation.go +++ b/internal/pkg/config/validation.go @@ -5,6 +5,7 @@ import ( "strings" "k8s.io/apimachinery/pkg/labels" + apivalidation "k8s.io/apimachinery/pkg/util/validation" "github.com/stakater/Reloader/internal/pkg/workload" ) @@ -97,6 +98,19 @@ func (c *Config) Validate() error { ) } + // Watched namespaces must be valid DNS-1123 labels; an invalid entry would + // otherwise fail deep inside the controller-runtime cache with an opaque error. + for _, ns := range c.WatchedNamespaces { + if msgs := apivalidation.IsDNS1123Label(ns); len(msgs) > 0 { + errs = append( + errs, ValidationError{ + Field: "WatchedNamespaces", + Message: fmt.Sprintf("invalid namespace %q: %s", ns, strings.Join(msgs, "; ")), + }, + ) + } + } + c.IgnoredResources = normalizeToLower(c.IgnoredResources) // Normalize ignored workloads to canonical Kind values (e.g., "cronjobs" -> "CronJob") diff --git a/internal/pkg/config/validation_test.go b/internal/pkg/config/validation_test.go index 52dc6f000..f4772c44d 100644 --- a/internal/pkg/config/validation_test.go +++ b/internal/pkg/config/validation_test.go @@ -194,6 +194,40 @@ func TestConfig_Validate_InvalidIgnoredWorkload(t *testing.T) { } } +func TestConfig_Validate_WatchedNamespaces(t *testing.T) { + tests := []struct { + name string + namespaces []string + wantErr bool + }{ + {"nil is valid (global mode)", nil, false}, + {"empty is valid (global mode)", []string{}, false}, + {"single valid label", []string{"team-a"}, false}, + {"multiple valid labels", []string{"team-a", "team-b", "kube-system"}, false}, + {"uppercase is invalid", []string{"Team-A"}, true}, + {"underscore is invalid", []string{"team_a"}, true}, + {"trailing dash is invalid", []string{"team-"}, true}, + {"one invalid among valid", []string{"team-a", "Bad_NS!"}, true}, + } + + for _, tt := range tests { + t.Run( + tt.name, func(t *testing.T) { + cfg := NewDefault() + cfg.WatchedNamespaces = tt.namespaces + + err := cfg.Validate() + if (err != nil) != tt.wantErr { + t.Fatalf("Validate() error = %v, wantErr %v", err, tt.wantErr) + } + if tt.wantErr && !strings.Contains(err.Error(), "WatchedNamespaces") { + t.Errorf("error should mention WatchedNamespaces, got: %v", err) + } + }, + ) + } +} + func TestConfig_Validate_MultipleErrors(t *testing.T) { cfg := NewDefault() cfg.ReloadStrategy = "invalid" diff --git a/test/e2e/flags/watch_namespaces_test.go b/test/e2e/flags/watch_namespaces_test.go index bcfbd6cbd..7f2a10315 100644 --- a/test/e2e/flags/watch_namespaces_test.go +++ b/test/e2e/flags/watch_namespaces_test.go @@ -113,3 +113,93 @@ var _ = Describe("Watch Namespaces (scoped mode) Flag Tests", Serial, func() { Expect(reloaded).To(BeFalse(), "Deployment in an unwatched namespace should NOT reload") }) }) + +var _ = Describe("Watch Multiple Namespaces (scoped mode) Flag Tests", Serial, func() { + var ( + deploymentName string + configMapName string + watchedA string + watchedB string + unwatchedNS string + adapter *utils.DeploymentAdapter + ) + + BeforeEach(func() { + deploymentName = utils.RandName("deploy") + configMapName = utils.RandName("cm") + watchedA = "watched-a-" + utils.RandName("ns") + watchedB = "watched-b-" + utils.RandName("ns") + unwatchedNS = "unwatched-" + utils.RandName("ns") + adapter = utils.NewDeploymentAdapter(kubeClient) + + // Both watched namespaces must exist before install: in scoped mode the + // chart creates a Role/RoleBinding in each. + Expect(utils.CreateNamespace(ctx, kubeClient, watchedA)).To(Succeed()) + Expect(utils.CreateNamespace(ctx, kubeClient, watchedB)).To(Succeed()) + Expect(utils.CreateNamespace(ctx, kubeClient, unwatchedNS)).To(Succeed()) + + err := deployReloaderWithFlags(map[string]string{ + "reloader.watchGlobally": "false", + "reloader.namespaces": fmt.Sprintf("{%s,%s}", watchedA, watchedB), + }) + Expect(err).NotTo(HaveOccurred()) + + Expect(waitForReloaderReady()).To(Succeed()) + }) + + AfterEach(func() { + for _, ns := range []string{watchedA, watchedB, unwatchedNS} { + _ = utils.DeleteDeployment(ctx, kubeClient, ns, deploymentName) + _ = utils.DeleteConfigMap(ctx, kubeClient, ns, configMapName) + } + _ = undeployReloader() + for _, ns := range []string{watchedA, watchedB, unwatchedNS} { + _ = utils.DeleteNamespace(ctx, kubeClient, ns) + } + }) + + It("should reload workloads across all watched namespaces but not an unwatched one", func() { + // Set up an annotated Deployment + ConfigMap in every namespace (both + // watched ones and the unwatched control). + allNS := []string{watchedA, watchedB, unwatchedNS} + for _, ns := range allNS { + By("Creating a ConfigMap and Deployment in " + ns) + _, err := utils.CreateConfigMap(ctx, kubeClient, ns, configMapName, + map[string]string{"key": "initial"}, nil) + Expect(err).NotTo(HaveOccurred()) + _, err = utils.CreateDeployment(ctx, kubeClient, ns, deploymentName, + utils.WithConfigMapEnvFrom(configMapName), + utils.WithAnnotations(utils.BuildAutoTrueAnnotation()), + ) + Expect(err).NotTo(HaveOccurred()) + Expect(adapter.WaitReady(ctx, ns, deploymentName, utils.WorkloadReadyTimeout)).To(Succeed()) + } + + // Capture reload baselines before triggering, then update every ConfigMap. + priorReload := map[string]string{} + for _, ns := range allNS { + pv, err := adapter.GetPodTemplateAnnotation(ctx, ns, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + priorReload[ns] = pv + } + for _, ns := range allNS { + By("Updating the ConfigMap in " + ns) + Expect(utils.UpdateConfigMap(ctx, kubeClient, ns, configMapName, + map[string]string{"key": "updated"})).To(Succeed()) + } + + By("Verifying workloads in BOTH watched namespaces reloaded") + for _, ns := range []string{watchedA, watchedB} { + reloaded, err := adapter.WaitReloadedFrom(ctx, ns, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload[ns], utils.ReloadTimeout) + Expect(err).NotTo(HaveOccurred()) + Expect(reloaded).To(BeTrue(), "Deployment in watched namespace %s should reload", ns) + } + + By("Verifying the workload in the unwatched namespace did NOT reload") + reloaded, err := adapter.WaitReloadedFrom(ctx, unwatchedNS, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload[unwatchedNS], utils.ShortTimeout) + Expect(err).NotTo(HaveOccurred()) + Expect(reloaded).To(BeFalse(), "Deployment in unwatched namespace %s should NOT reload", unwatchedNS) + }) +})