From 0cfbe10125f9d36c3131a352049d1f156cdeea90 Mon Sep 17 00:00:00 2001 From: Safwan Date: Tue, 14 Jul 2026 11:10:48 +0500 Subject: [PATCH 01/12] 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 | 132 ++++-------------- .../chart/reloader/templates/rolebinding.yaml | 37 ++++- .../kubernetes/chart/reloader/values.yaml | 18 +-- 7 files changed, 219 insertions(+), 151 deletions(-) diff --git a/deployments/kubernetes/chart/reloader/README.md b/deployments/kubernetes/chart/reloader/README.md index 3e7df648..7b453ac4 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 831be8d3..306da2d7 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 8f31764f..bd14dfeb 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 fb6a8bb7..b3ac972e 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 495a3855..28c535d6 100644 --- a/deployments/kubernetes/chart/reloader/templates/role.yaml +++ b/deployments/kubernetes/chart/reloader/templates/role.yaml @@ -1,9 +1,34 @@ -{{- 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 +{{- 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-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 5cf4cf38..7d73b182 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 1e867dc0..15a22bcf 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 02/12] 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 35151f9b..32535215 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 b63611e7..726a6e8b 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 bc6ea0a1..62d1a610 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 784fef10..969ffb59 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 f9c6819a..c7e6b975 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 869bdf00..2c561e16 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 7b671c00..863d1acd 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 03/12] 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 32535215..781d28c5 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 306da2d7..19c65533 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 bd14dfeb..d183cff8 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 b3ac972e..17549358 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 28c535d6..b7b5cd92 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 7d73b182..187c90dd 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 15a22bcf..f236bdf7 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 969ffb59..6680abca 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 c7e6b975..233a6527 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 2c561e16..a6ae3182 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 04/12] 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 19c65533..5656bb4f 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 d183cff8..eb2929e4 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 17549358..da5a1d54 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 05/12] 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 5656bb4f..ec268917 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 eb2929e4..e0a235a2 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 06/12] 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 cef05184..776d5773 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 989bf0ab..9c1e996d 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 af4b7ac3..5961feb4 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 e7f02efa..dab5a8fd 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 5a336514..bce14ee6 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 869aed1d..6e80b55f 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 132c91a6..0d97724d 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 02a1153c..dac03577 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 019df62b..e6edf153 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 f3c0b8fd..60bf129e 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 fdbe98ec..b6a1a195 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 ef55f2bd..08604d61 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 f4cda1cd..839e5e0b 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 44eb539d..e28f8885 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 7a5185ce..0bd4449c 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 39467909..d3599540 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 4ac49cbe..a58da579 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 63fec0bb..b6946ef3 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 ed400e35..88fda766 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 cc94612a..af31a958 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 96e3fb2d..204a3d65 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 2610337f..70a5e7e1 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 69d5163e..c976d746 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 c681cce9..151fe9ae 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 4a7a2b14..74aada0e 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 f7ef5e37..d64e194a 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 e71c86c2..5f8ad506 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 6f758bf4..2fb11c6c 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 d071678a..28052dce 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 07/12] 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 781d28c5..b1f7f7f9 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 ec268917..32d129c9 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 da5a1d54..ff43894a 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 b7b5cd92..0454eadf 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 187c90dd..7d73b182 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 f236bdf7..119bc8e3 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 726a6e8b..c1c7faed 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 62d1a610..218c34d5 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 6680abca..70ba5225 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 233a6527..45d6d2cb 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 6c6a4222..8cd349e1 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 00000000..bcfbd6cb --- /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 477e67784c26e90bb8a531f6ad15e909f5bd8480 Mon Sep 17 00:00:00 2001 From: Safwan Date: Wed, 15 Jul 2026 16:15:14 +0500 Subject: [PATCH 08/12] added reloader-enterprise dependency --- .github/workflows/push-helm-chart.yaml | 14 ++++-- .github/workflows/release-helm-chart.yaml | 4 +- .github/workflows/release.yaml | 5 +- .../kubernetes/chart/reloader/Chart.yaml | 14 ++++-- .../kubernetes/chart/reloader/README.md | 49 +++++++++++++++++++ .../chart/reloader/tests/enterprise_test.yaml | 20 ++++++++ .../kubernetes/chart/reloader/values.yaml | 21 +++++++- 7 files changed, 114 insertions(+), 13 deletions(-) create mode 100644 deployments/kubernetes/chart/reloader/tests/enterprise_test.yaml diff --git a/.github/workflows/push-helm-chart.yaml b/.github/workflows/push-helm-chart.yaml index 0f54f73b..6c937a22 100644 --- a/.github/workflows/push-helm-chart.yaml +++ b/.github/workflows/push-helm-chart.yaml @@ -54,7 +54,7 @@ jobs: - name: Get version for chart from helm repo id: chart_eval run: | - current_chart_version=$(helm search repo stakater/reloader | tail -n 1 | awk '{print $2}') + current_chart_version=$(helm search repo stakater/reloader-v2 | tail -n 1 | awk '{print $2}') echo "CURRENT_CHART_VERSION=$(echo ${current_chart_version})" >> $GITHUB_OUTPUT - name: Get Updated Chart version from Chart.yaml @@ -63,7 +63,10 @@ jobs: with: cmd: yq e '.version' deployments/kubernetes/chart/reloader/Chart.yaml + # Skip the version-increase gate on first publish, when no reloader-v2 + # chart exists yet in the repo (CURRENT_CHART_VERSION is empty). - name: Check Version + if: steps.chart_eval.outputs.CURRENT_CHART_VERSION != '' uses: aleoyakas/check-semver-increased-action@415c9c60054c2442c03478b6dd96a195deac6695 # v1 id: check-version with: @@ -71,7 +74,7 @@ jobs: previous-version: ${{ steps.chart_eval.outputs.CURRENT_CHART_VERSION }} - name: Fail if Helm Chart version isnt updated - if: steps.check-version.outputs.is-version-increased != 'true' + if: steps.chart_eval.outputs.CURRENT_CHART_VERSION != '' && steps.check-version.outputs.is-version-increased != 'true' run: | echo "Helm Chart Version wasnt updated" exit 1 @@ -88,12 +91,15 @@ jobs: - name: Publish Helm chart to ghcr.io run: | + # Pull the enterprise subchart from private GHCR (uses the login above); + # the vendored charts/*.tgz is gitignored, so it isn't present on the runner. + helm dependency build ./deployments/kubernetes/chart/reloader helm package ./deployments/kubernetes/chart/reloader --destination ./packaged-chart helm push ./packaged-chart/*.tgz oci://ghcr.io/stakater/charts rm -rf ./packaged-chart - name: Sign artifacts with Cosign - run: cosign sign --yes ghcr.io/stakater/charts/reloader:${{ steps.new_chart_version.outputs.result }} + run: cosign sign --yes ghcr.io/stakater/charts/reloader-v2:${{ steps.new_chart_version.outputs.result }} - name: Publish Helm chart to gh-pages uses: stefanprodan/helm-gh-pages@0ad2bb377311d61ac04ad9eb6f252fb68e207260 # v1.7.0 @@ -114,7 +120,7 @@ jobs: env: GITHUB_TOKEN: ${{ secrets.PUBLISH_TOKEN }} WITH_V: false - CUSTOM_TAG: chart-v${{ steps.new_chart_version.outputs.result }} + CUSTOM_TAG: reloader-v2-chart-v${{ steps.new_chart_version.outputs.result }} - name: Notify Slack uses: 8398a7/action-slack@77eaa4f1c608a7d68b38af4e3f739dcd8cba273e # v3 diff --git a/.github/workflows/release-helm-chart.yaml b/.github/workflows/release-helm-chart.yaml index afc39ee5..31e7b34c 100644 --- a/.github/workflows/release-helm-chart.yaml +++ b/.github/workflows/release-helm-chart.yaml @@ -3,7 +3,7 @@ name: Release Helm chart on: push: tags: - - "chart-v*" + - "reloader-v2-chart-v*" permissions: contents: write @@ -26,7 +26,7 @@ jobs: run: | gh release create "$tag" \ --repo="$GITHUB_REPOSITORY" \ - --title="Helm chart ${tag#chart-}" \ + --title="Helm chart reloader-v2 ${tag#reloader-v2-chart-v}" \ --generate-notes - name: Notify Slack diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index ee1154b6..5d998917 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -147,7 +147,8 @@ jobs: run: | echo GHCR_IMAGE_REPOSITORY=${{env.REGISTRY}}/$(echo ${{ github.repository }} | tr '[:upper:]' '[:lower:]') >> $GITHUB_ENV - # tag this image as latest as it will be used in plain manifests + # v2 branch: publish a floating `v2` tag instead of `latest` so v2 + # releases never overwrite the v1 `latest` tag used by plain manifests. - name: Build and Push Docker Image to ghcr registry uses: docker/build-push-action@10e90e3645eae34f1e60eeb005ba3a3d33f178e8 # v6 with: @@ -157,7 +158,7 @@ jobs: push: true platforms: linux/amd64,linux/arm,linux/arm64 tags: | - ${{ env.GHCR_IMAGE_REPOSITORY }}:${{ steps.generate_tag.outputs.RELEASE_VERSION }},${{ env.GHCR_IMAGE_REPOSITORY }}:latest + ${{ env.GHCR_IMAGE_REPOSITORY }}:${{ steps.generate_tag.outputs.RELEASE_VERSION }},${{ env.GHCR_IMAGE_REPOSITORY }}:v2 build-args: | VERSION=${{ steps.generate_tag.outputs.RELEASE_VERSION }} COMMIT=${{ github.sha }} diff --git a/deployments/kubernetes/chart/reloader/Chart.yaml b/deployments/kubernetes/chart/reloader/Chart.yaml index ee8bae06..2e11cd6c 100644 --- a/deployments/kubernetes/chart/reloader/Chart.yaml +++ b/deployments/kubernetes/chart/reloader/Chart.yaml @@ -1,8 +1,8 @@ -apiVersion: v1 -name: reloader +apiVersion: v2 +name: reloader-v2 description: Reloader chart that runs on kubernetes -version: 2.2.12 -appVersion: v1.4.17 +version: 2.0.0 +appVersion: v2.0.0 keywords: - Reloader - kubernetes @@ -17,3 +17,9 @@ maintainers: email: rasheed@stakater.com - name: faizanahmad055 email: faizan@stakater.com +dependencies: + - name: reloader-enterprise + alias: enterprise + version: 0.1.0 + repository: oci://ghcr.io/stakater/charts + condition: enterprise.enabled diff --git a/deployments/kubernetes/chart/reloader/README.md b/deployments/kubernetes/chart/reloader/README.md index 7b453ac4..5a00198e 100644 --- a/deployments/kubernetes/chart/reloader/README.md +++ b/deployments/kubernetes/chart/reloader/README.md @@ -170,6 +170,55 @@ helm uninstall {{RELEASE_NAME}} -n {{NAMESPACE}} ### Deprecation Notice - `serviceMonitor` will be removed in future releases in favor of `PodMonitor` +## Enterprise mode + +Reloader Enterprise adds a console, gateway, and cache (dragonfly) alongside the +operator. It ships as an optional subchart gated by `enterprise.enabled`. + +Enabling it requires **four** changes, because the operator image is swapped manually: + +1. Turn on the subchart and set the component hostnames: + + ```yaml + enterprise: + enabled: true + global: + consoleHost: reloader-enterprise-console.apps.example.com + gatewayHost: reloader-enterprise-gateway.apps.example.com + ``` + +2. Swap the operator image to the enterprise image: + + ```yaml + image: + repository: ghcr.io/stakater/reloader-enterprise + tag: v0.1.0 + ``` + +3. Provide an image pull secret — the enterprise operator, console, and gateway + images are in **private GHCR**: + + ```bash + kubectl create secret docker-registry saap-dockerconfigjson \ + --docker-server=ghcr.io \ + --docker-username= --docker-password= \ + -n + ``` + + ```yaml + global: + imagePullSecrets: + - name: saap-dockerconfigjson + ``` + +4. Deeper enterprise component settings pass through under `enterprise.console.*`, + `enterprise.gateway.*`, and `enterprise.dragonfly.*` (see the reloader-enterprise + chart values for the full list). Values you do not override fall back to the + subchart defaults. + +When `enterprise.enabled=false` (the default) none of the enterprise components are +rendered and the operator uses the community image. + ## Release Process _Helm chart versioning_: The Reloader Helm chart is maintained in this repository. The Helm chart has its own semantic versioning. Helm charts and code releases are separate artifacts and separately versioned. Manifest making strategy relies on Kustomize. The Reloader Helm chart manages the two artifacts with these two fields: diff --git a/deployments/kubernetes/chart/reloader/tests/enterprise_test.yaml b/deployments/kubernetes/chart/reloader/tests/enterprise_test.yaml new file mode 100644 index 00000000..7ea2da1c --- /dev/null +++ b/deployments/kubernetes/chart/reloader/tests/enterprise_test.yaml @@ -0,0 +1,20 @@ +suite: Enterprise image swap +templates: + - deployment.yaml +tests: + - it: uses the community image by default + asserts: + - equal: + path: spec.template.spec.containers[0].image + value: ghcr.io/stakater/reloader:v2.0.0 + - it: uses the enterprise image when the image block is overridden + set: + enterprise: + enabled: true + image: + repository: ghcr.io/stakater/reloader-enterprise + tag: v2.0.0 + asserts: + - equal: + path: spec.template.spec.containers[0].image + value: ghcr.io/stakater/reloader-enterprise:v2.0.0 diff --git a/deployments/kubernetes/chart/reloader/values.yaml b/deployments/kubernetes/chart/reloader/values.yaml index 119bc8e3..d4bd113a 100644 --- a/deployments/kubernetes/chart/reloader/values.yaml +++ b/deployments/kubernetes/chart/reloader/values.yaml @@ -10,6 +10,11 @@ global: #imagePullSecrets: # - my-pull-secret + ## Required when enterprise.enabled=true (shared with the enterprise subchart) + consoleHost: "" + gatewayHost: "" + gatewayScheme: https + kubernetes: host: https://kubernetes.default @@ -19,10 +24,24 @@ fullnameOverride: "" image: name: stakater/reloader repository: ghcr.io/stakater/reloader - tag: v1.4.19 + tag: v2.0.0 # digest: sha256:1234567 pullPolicy: IfNotPresent +# Reloader Enterprise components (console + gateway + dragonfly cache). +# When enabled, ALSO set image.repository/image.tag to the enterprise operator +# image and provide global.imagePullSecrets — see the chart README "Enterprise mode". +enterprise: + enabled: false + console: + enabled: true + gateway: + enabled: true + dragonfly: + enabled: true + dragonfly: + fullnameOverride: reloader-enterprise-dragonfly + reloader: autoReloadAll: false isArgoRollouts: false From 15ffe9d788cc27bb42bac7058b52eb4cee398cb7 Mon Sep 17 00:00:00 2001 From: Safwan Date: Wed, 15 Jul 2026 17:00:28 +0500 Subject: [PATCH 09/12] some placeholders for ease of access --- deployments/kubernetes/chart/reloader/values.yaml | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/deployments/kubernetes/chart/reloader/values.yaml b/deployments/kubernetes/chart/reloader/values.yaml index d4bd113a..fce643d3 100644 --- a/deployments/kubernetes/chart/reloader/values.yaml +++ b/deployments/kubernetes/chart/reloader/values.yaml @@ -35,10 +35,25 @@ enterprise: enabled: false console: enabled: true + # deployment: + # image: + # tag: "" + # OpenShift: router auto-creates a Route from the Ingress; this annotation + # sets edge TLS termination on that Route. + # ingress: + # annotations: + # route.openshift.io/termination: edge gateway: enabled: true dragonfly: enabled: true + # deployment: + # image: + # Gateway images are published v-prefixed (WITH_V: true); latest is v0.0.23. + # tag: "" + # ingress: + # annotations: + # route.openshift.io/termination: edge dragonfly: fullnameOverride: reloader-enterprise-dragonfly From 80032eb380ad8730481973fe762cdc7db09b6e4c Mon Sep 17 00:00:00 2001 From: Safwan Date: Fri, 17 Jul 2026 12:03:38 +0500 Subject: [PATCH 10/12] resolved comments --- .../kubernetes/chart/reloader/README.md | 2 +- .../chart/reloader/templates/_helpers.tpl | 65 +++-- .../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, 561 insertions(+), 28 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 7b453ac4..6eb65d21 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 32d129c9..b2c6186e 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,7 +181,15 @@ Expects the root context ($) as its argument. - create - update - patch -{{- if .Values.reloader.namespaces }} +{{- if and (include "reloader-isScoped" .) .Values.reloader.enableHA }} + - apiGroups: + - "coordination.k8s.io" + resources: + - leases + verbs: + - create + - get + - update - apiGroups: - "" - "events.k8s.io" @@ -156,16 +199,6 @@ Expects the root context ($) as its argument. - create - patch - update -{{- if .Values.reloader.enableHA }} - - apiGroups: - - "coordination.k8s.io" - resources: - - leases - verbs: - - create - - get - - update -{{- end }} {{- end }} {{- end -}} diff --git a/deployments/kubernetes/chart/reloader/templates/deployment.yaml b/deployments/kubernetes/chart/reloader/templates/deployment.yaml index ff43894a..d7b9a673 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 0454eadf..4333e73c 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 7d73b182..7965973b 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 00000000..04579b16 --- /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 00000000..0585cdeb --- /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 b3d2695b..e3f72543 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 52dc6f00..f4772c44 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 bcfbd6cb..7f2a1031 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) + }) +}) From e7396030d35fe522f0d43425375d0190a0746ac0 Mon Sep 17 00:00:00 2001 From: Safwan Date: Fri, 17 Jul 2026 13:04:29 +0500 Subject: [PATCH 11/12] resolved comments and updated readme --- .github/workflows/pull_request-helm.yaml | 7 +++++-- .../kubernetes/chart/reloader/README.md | 17 ++++++++++++----- .../chart/reloader/templates/NOTES.txt | 10 ++++++++++ .../chart/reloader/tests/enterprise_test.yaml | 18 +++++++++++++----- .../kubernetes/chart/reloader/values.yaml | 7 +++++-- 5 files changed, 45 insertions(+), 14 deletions(-) diff --git a/.github/workflows/pull_request-helm.yaml b/.github/workflows/pull_request-helm.yaml index 7639c2c3..0a31f4ef 100644 --- a/.github/workflows/pull_request-helm.yaml +++ b/.github/workflows/pull_request-helm.yaml @@ -73,7 +73,7 @@ jobs: - name: Get version for chart from helm repo id: chart_eval run: | - current_chart_version=$(helm search repo stakater/reloader | tail -n 1 | awk '{print $2}') + current_chart_version=$(helm search repo stakater/reloader-v2 | tail -n 1 | awk '{print $2}') echo "CURRENT_CHART_VERSION=$(echo ${current_chart_version})" >> $GITHUB_OUTPUT - name: Get Updated Chart version from Chart.yaml @@ -82,7 +82,10 @@ jobs: with: cmd: yq e '.version' deployments/kubernetes/chart/reloader/Chart.yaml + # Skip the version-increase gate on first publish, when no reloader-v2 + # chart exists yet in the repo (CURRENT_CHART_VERSION is empty). - name: Check Version + if: steps.chart_eval.outputs.CURRENT_CHART_VERSION != '' uses: aleoyakas/check-semver-increased-action@415c9c60054c2442c03478b6dd96a195deac6695 # v1 id: check-version with: @@ -90,7 +93,7 @@ jobs: previous-version: ${{ steps.chart_eval.outputs.CURRENT_CHART_VERSION }} - name: Fail if Helm Chart version isnt updated - if: steps.check-version.outputs.is-version-increased != 'true' + if: steps.chart_eval.outputs.CURRENT_CHART_VERSION != '' && steps.check-version.outputs.is-version-increased != 'true' run: | echo "Helm Chart Version wasnt updated" exit 1 diff --git a/deployments/kubernetes/chart/reloader/README.md b/deployments/kubernetes/chart/reloader/README.md index 4a7ba5e3..05e86076 100644 --- a/deployments/kubernetes/chart/reloader/README.md +++ b/deployments/kubernetes/chart/reloader/README.md @@ -10,13 +10,13 @@ helm repo add stakater https://stakater.github.io/stakater-charts helm repo update -helm install stakater/reloader # For helm3 add --generate-name flag or set the release name +helm install stakater/reloader-v2 # For helm3 add --generate-name flag or set the release name -helm install {{RELEASE_NAME}} stakater/reloader -n {{NAMESPACE}} --set reloader.watchGlobally=false # By default, Reloader watches in all namespaces. To watch in single namespace, set watchGlobally=false +helm install {{RELEASE_NAME}} stakater/reloader-v2 -n {{NAMESPACE}} --set reloader.watchGlobally=false # By default, Reloader watches in all namespaces. To watch in single namespace, set watchGlobally=false -helm install stakater/reloader --set reloader.watchGlobally=false --namespace test --generate-name # Install Reloader in `test` namespace which will only watch `Deployments`, `Daemonsets` `Statefulsets` and `Rollouts` in `test` namespace. +helm install stakater/reloader-v2 --set reloader.watchGlobally=false --namespace test --generate-name # Install Reloader in `test` namespace which will only watch `Deployments`, `Daemonsets` `Statefulsets` and `Rollouts` in `test` namespace. -helm install stakater/reloader --set reloader.ignoreJobs=true --set reloader.ignoreCronJobs=true --generate-name # Install Reloader ignoring Jobs and CronJobs from reload monitoring +helm install stakater/reloader-v2 --set reloader.ignoreJobs=true --set reloader.ignoreCronJobs=true --generate-name # Install Reloader ignoring Jobs and CronJobs from reload monitoring ``` ## Uninstalling @@ -192,9 +192,16 @@ Enabling it requires **four** changes, because the operator image is swapped man ```yaml image: repository: ghcr.io/stakater/reloader-enterprise - tag: v0.1.0 + tag: v0.0.43 # enterprise operator image tag (v-prefixed); use the latest release ``` + > **Version note:** three different numbers are in play here, don't mix them up: + > - `image.tag` above is the **enterprise operator image** tag (e.g. `v0.0.43`). + > - the `reloader-enterprise` **subchart** version (`0.1.0`, in `Chart.yaml` + > dependencies) is the Helm chart version, not an image tag. + > - the console and gateway images have their own tags again, set under + > `enterprise.console.*` / `enterprise.gateway.*`. + 3. Provide an image pull secret — the enterprise operator, console, and gateway images are in **private GHCR**: diff --git a/deployments/kubernetes/chart/reloader/templates/NOTES.txt b/deployments/kubernetes/chart/reloader/templates/NOTES.txt index d437a4f2..be10fd61 100644 --- a/deployments/kubernetes/chart/reloader/templates/NOTES.txt +++ b/deployments/kubernetes/chart/reloader/templates/NOTES.txt @@ -5,3 +5,13 @@ {{ .Values.reloader.custom_annotations.secret | default "secret.reloader.stakater.com/reload" }}: "foo-secret" - After successful installation, your pods will get rolling updates when a change in data of configmap or secret will happen. +{{- if .Values.enterprise.enabled }} +{{- $usingCommunityImage := ternary (eq .Values.image.name "stakater/reloader") (eq .Values.image.repository "ghcr.io/stakater/reloader") (not (empty .Values.global.imageRegistry)) }} +{{- if $usingCommunityImage }} + + WARNING: enterprise.enabled=true but the operator image still points at the + community image. The operator will run, but without enterprise features. To use + the enterprise operator, override {{ if .Values.global.imageRegistry }}image.name{{ else }}image.repository{{ end }}/image.tag — see the + chart README "Enterprise mode". +{{- end }} +{{- end }} diff --git a/deployments/kubernetes/chart/reloader/tests/enterprise_test.yaml b/deployments/kubernetes/chart/reloader/tests/enterprise_test.yaml index 7ea2da1c..cdfc6a5d 100644 --- a/deployments/kubernetes/chart/reloader/tests/enterprise_test.yaml +++ b/deployments/kubernetes/chart/reloader/tests/enterprise_test.yaml @@ -7,14 +7,22 @@ tests: - equal: path: spec.template.spec.containers[0].image value: ghcr.io/stakater/reloader:v2.0.0 - - it: uses the enterprise image when the image block is overridden + # The operator image is swapped manually via image.*; enterprise.enabled only + # toggles the subchart and must NOT change the operator image on its own. + - it: enterprise.enabled alone does not change the operator image set: enterprise: enabled: true - image: - repository: ghcr.io/stakater/reloader-enterprise - tag: v2.0.0 asserts: - equal: path: spec.template.spec.containers[0].image - value: ghcr.io/stakater/reloader-enterprise:v2.0.0 + value: ghcr.io/stakater/reloader:v2.0.0 + - it: uses the enterprise image when the image block is overridden + set: + image: + repository: ghcr.io/stakater/reloader-enterprise + tag: test-tag + asserts: + - equal: + path: spec.template.spec.containers[0].image + value: ghcr.io/stakater/reloader-enterprise:test-tag diff --git a/deployments/kubernetes/chart/reloader/values.yaml b/deployments/kubernetes/chart/reloader/values.yaml index fce643d3..3d0e47eb 100644 --- a/deployments/kubernetes/chart/reloader/values.yaml +++ b/deployments/kubernetes/chart/reloader/values.yaml @@ -29,8 +29,11 @@ image: pullPolicy: IfNotPresent # Reloader Enterprise components (console + gateway + dragonfly cache). -# When enabled, ALSO set image.repository/image.tag to the enterprise operator -# image and provide global.imagePullSecrets — see the chart README "Enterprise mode". +# When enabled, ALSO swap the operator image to the enterprise image and provide +# global.imagePullSecrets — see the chart README "Enterprise mode". +# NOTE: if global.imageRegistry is set, the operator image resolves from +# global.imageRegistry + image.name (image.repository is ignored); in that mode +# override image.name, not image.repository. enterprise: enabled: false console: From de62f06f96e8092c91df0b1b20dae9190eb0d932 Mon Sep 17 00:00:00 2001 From: Safwan Date: Mon, 20 Jul 2026 19:41:41 +0500 Subject: [PATCH 12/12] update tests --- .../chart/reloader/tests/namespaces_rbac_test.yaml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/deployments/kubernetes/chart/reloader/tests/namespaces_rbac_test.yaml b/deployments/kubernetes/chart/reloader/tests/namespaces_rbac_test.yaml index 0585cdeb..47b0c02f 100644 --- a/deployments/kubernetes/chart/reloader/tests/namespaces_rbac_test.yaml +++ b/deployments/kubernetes/chart/reloader/tests/namespaces_rbac_test.yaml @@ -82,7 +82,7 @@ tests: of: RoleBinding - equal: path: roleRef.name - value: reloader-reloader-role + value: reloader-reloader-v2-role - equal: path: subjects[0].namespace value: reloader-ns @@ -97,7 +97,7 @@ tests: template: role.yaml documentSelector: path: metadata.name - value: reloader-reloader-metadata-role + value: reloader-reloader-v2-metadata-role asserts: - equal: path: metadata.namespace @@ -119,7 +119,7 @@ tests: template: role.yaml documentSelector: path: metadata.name - value: reloader-reloader-metadata-role + value: reloader-reloader-v2-metadata-role asserts: - contains: path: rules @@ -143,7 +143,7 @@ tests: template: role.yaml documentSelector: path: metadata.name - value: reloader-reloader-metadata-role + value: reloader-reloader-v2-metadata-role asserts: - notContains: path: rules @@ -194,7 +194,7 @@ tests: template: role.yaml - equal: path: metadata.name - value: reloader-reloader-metadata-role + value: reloader-reloader-v2-metadata-role template: role.yaml documentIndex: 0