diff --git a/cmd/reloader/main.go b/cmd/reloader/main.go index 35151f9ba..b1f7f7f9b 100644 --- a/cmd/reloader/main.go +++ b/cmd/reloader/main.go @@ -54,7 +54,15 @@ func newReloaderCommand() *cobra.Command { } func run(cmd *cobra.Command, args []string) error { - if err := config.ApplyFlags(cfg); err != nil { + // 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) } @@ -72,19 +80,12 @@ 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") - if cfg.WatchedNamespace != "" { - log.Info("watching single namespace", "namespace", cfg.WatchedNamespace) - } else { + if cfg.IsGlobalMode() { log.Info("watching all namespaces") + } else { + log.Info("watching scoped namespaces", "namespaces", cfg.WatchedNamespaces) } if len(cfg.NamespaceSelectors) > 0 { @@ -149,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/README.md b/deployments/kubernetes/chart/reloader/README.md index 3e7df6481..6eb65d216 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 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 831be8d30..b2c6186e8 100644 --- a/deployments/kubernetes/chart/reloader/templates/_helpers.tpl +++ b/deployments/kubernetes/chart/reloader/templates/_helpers.tpl @@ -88,6 +88,231 @@ Create the namespace selector if it does not watch globally {{- end -}} {{- end -}} +{{/* +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 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" -}} +{{- $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 -}} +{{- $clean | uniq | sortAlpha | toJson -}} +{{- end -}} + +{{/* +Comma-joined form of reloader-watchNamespaces, for the --namespaces CLI flag. +*/}} +{{- define "reloader-watchNamespaces-csv" -}} +{{- 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 (include "reloader-isScoped" .) -}} +{{- 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 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" }} + - apiGroups: + - "" + resources: + - configmaps + verbs: + - get + - create + - update + - patch +{{- if and (include "reloader-isScoped" .) .Values.reloader.enableHA }} + - apiGroups: + - "coordination.k8s.io" + resources: + - leases + verbs: + - create + - get + - update + - apiGroups: + - "" + - "events.k8s.io" + resources: + - events + verbs: + - create + - patch + - update +{{- end }} +{{- 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 + - watch + - update + - patch +{{- end }} + - apiGroups: + - "apps" + resources: + - deployments + - daemonsets + - statefulsets + verbs: + - list + - get + - watch + - update + - patch +{{- if .Values.reloader.ignoreCronJobs }}{{- else }} + - apiGroups: + - "batch" + resources: + - cronjobs + verbs: + - list + - get + - watch + - update + - patch +{{- end }} +{{- if .Values.reloader.ignoreJobs }}{{- else }} + - apiGroups: + - "batch" + resources: + - jobs + verbs: + - create + - delete + - list + - get + - watch +{{- end }} +{{- 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: + - "" + - "events.k8s.io" + resources: + - events + verbs: + - create + - patch + - update +{{- end -}} + {{/* Normalizes global.imagePullSecrets to a list of objects with name fields. Supports both of these in values.yaml: diff --git a/deployments/kubernetes/chart/reloader/templates/clusterrole.yaml b/deployments/kubernetes/chart/reloader/templates/clusterrole.yaml index 8f31764fa..e0a235a2f 100644 --- a/deployments/kubernetes/chart/reloader/templates/clusterrole.yaml +++ b/deployments/kubernetes/chart/reloader/templates/clusterrole.yaml @@ -74,9 +74,9 @@ rules: verbs: - list - get + - watch - update - patch - - watch {{- if .Values.reloader.ignoreCronJobs }}{{- else }} - apiGroups: - "batch" @@ -124,9 +124,11 @@ rules: {{- end}} - apiGroups: - "" + - "events.k8s.io" resources: - events verbs: - create - patch + - update {{- end }} diff --git a/deployments/kubernetes/chart/reloader/templates/deployment.yaml b/deployments/kubernetes/chart/reloader/templates/deployment.yaml index fb6a8bb72..d7b9a6739 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: @@ -143,12 +144,6 @@ spec: fieldRef: fieldPath: {{ $value | quote}} {{- end }} - {{- end }} - {{- if eq .Values.reloader.watchGlobally false }} - - name: KUBERNETES_NAMESPACE - valueFrom: - fieldRef: - fieldPath: metadata.namespace {{- end }} - name: RELOADER_NAMESPACE @@ -215,7 +210,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) (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 }}" @@ -236,7 +231,10 @@ spec: {{- else if .Values.reloader.ignoreCronJobs }} - "--ignored-workload-types=cronjobs" {{- end }} - {{- if .Values.reloader.ignoreNamespaces }} + {{- 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 }}" {{- end }} {{- if (include "reloader-namespaceSelector" .) }} @@ -252,7 +250,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 }} diff --git a/deployments/kubernetes/chart/reloader/templates/role.yaml b/deployments/kubernetes/chart/reloader/templates/role.yaml index 495a38552..4333e73c3 100644 --- a/deployments/kubernetes/chart/reloader/templates/role.yaml +++ b/deployments/kubernetes/chart/reloader/templates/role.yaml @@ -1,9 +1,31 @@ {{- 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 (include "reloader-isScoped" .) }} +{{- 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 +41,8 @@ metadata: name: {{ template "reloader-fullname" . }}-role namespace: {{ .Values.namespace | default .Release.Namespace }} 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 - - watch - - update - - patch +{{- include "reloader-namespaced-rules" . }} {{- 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 }} --- @@ -137,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 5cf4cf38b..7965973b2 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 (include "reloader-isScoped" .) }} +{{- 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/tests/namespaces_flags_test.yaml b/deployments/kubernetes/chart/reloader/tests/namespaces_flags_test.yaml new file mode 100644 index 000000000..04579b167 --- /dev/null +++ b/deployments/kubernetes/chart/reloader/tests/namespaces_flags_test.yaml @@ -0,0 +1,134 @@ +suite: Namespace watch flags & validation +templates: + - deployment.yaml +release: + name: reloader + namespace: reloader-ns +tests: + # ---------------------------------------------------------------- scoped mode + - it: scoped mode passes the watched namespaces via --namespaces + set: + reloader: + watchGlobally: false + namespaces: + - team-a + - team-b + asserts: + - contains: + path: spec.template.spec.containers[0].args + content: "--namespaces=team-a,team-b" + + - it: namespaces are de-duplicated and sorted for the --namespaces flag + set: + reloader: + watchGlobally: false + namespaces: + - team-b + - team-a + - team-a + asserts: + - contains: + path: spec.template.spec.containers[0].args + content: "--namespaces=team-a,team-b" + + - it: a comma-separated string value is accepted for reloader.namespaces + set: + reloader: + watchGlobally: false + namespaces: "team-a,team-b" + asserts: + - contains: + path: spec.template.spec.containers[0].args + content: "--namespaces=team-a,team-b" + + # ----------------------------------------------------------- single-ns mode + - it: single-namespace mode passes the release namespace via --namespaces + set: + reloader: + watchGlobally: false + asserts: + - contains: + path: spec.template.spec.containers[0].args + content: "--namespaces=reloader-ns" + + - it: whitespace-only namespaces fall through to single-namespace mode + set: + reloader: + watchGlobally: false + namespaces: + - " " + - " " + asserts: + - contains: + path: spec.template.spec.containers[0].args + content: "--namespaces=reloader-ns" + + - it: KUBERNETES_NAMESPACE env is no longer injected in single-namespace mode + set: + reloader: + watchGlobally: false + asserts: + - notContains: + path: spec.template.spec.containers[0].env + content: + name: KUBERNETES_NAMESPACE + any: true + + # ---------------------------------------------------------------- global mode + - it: global mode does not pass a --namespaces flag + set: + reloader: + watchGlobally: true + logLevel: info + asserts: + - notContains: + path: spec.template.spec.containers[0].args + content: "--namespaces=reloader-ns" + - notContains: + path: spec.template.spec.containers[0].args + content: "--namespaces=team-a" + + - it: namespaces-to-ignore is only emitted in global mode + set: + reloader: + watchGlobally: true + ignoreNamespaces: "kube-system" + asserts: + - contains: + path: spec.template.spec.containers[0].args + content: "--namespaces-to-ignore=kube-system" + + - it: namespaces-to-ignore is dropped in single-namespace mode + set: + reloader: + watchGlobally: false + ignoreNamespaces: "kube-system" + asserts: + - notContains: + path: spec.template.spec.containers[0].args + content: "--namespaces-to-ignore=kube-system" + + # ------------------------------------------------------------- validation + - it: fails to render when reloader.namespaces is set but watchGlobally is true + set: + reloader: + watchGlobally: true + namespaces: + - team-a + asserts: + - failedTemplate: + errorMessage: "reloader.namespaces is set but reloader.watchGlobally is true; set reloader.watchGlobally=false to use scoped namespace mode." + + - it: does not fail validation for whitespace-only namespaces with watchGlobally true + set: + reloader: + watchGlobally: true + namespaces: + - " " + asserts: + # whitespace trims to an empty watch list, so this is global mode, not scoped -> renders fine + - hasDocuments: + count: 1 + - notContains: + path: spec.template.spec.containers[0].args + content: "--namespaces=reloader-ns" diff --git a/deployments/kubernetes/chart/reloader/tests/namespaces_rbac_test.yaml b/deployments/kubernetes/chart/reloader/tests/namespaces_rbac_test.yaml new file mode 100644 index 000000000..0585cdebb --- /dev/null +++ b/deployments/kubernetes/chart/reloader/tests/namespaces_rbac_test.yaml @@ -0,0 +1,234 @@ +suite: Namespace-scoped RBAC +templates: + - role.yaml + - rolebinding.yaml + - clusterrole.yaml +release: + name: reloader + namespace: reloader-ns +tests: + # ---------------------------------------------------------------- scoped mode + - it: scoped mode renders a Role + RoleBinding per watched namespace plus the metadata role, and no ClusterRole + set: + reloader: + watchGlobally: false + namespaces: + - team-a + - team-b + asserts: + # 2 watched Roles + 1 metadata Role + - hasDocuments: + count: 3 + template: role.yaml + - hasDocuments: + count: 3 + template: rolebinding.yaml + - hasDocuments: + count: 0 + template: clusterrole.yaml + + - it: scoped mode grants the namespaced workload rules in a watched namespace + set: + reloader: + watchGlobally: false + namespaces: + - team-a + - team-b + template: role.yaml + documentSelector: + path: metadata.namespace + value: team-a + asserts: + - isKind: + of: Role + - equal: + path: rules[0].apiGroups[0] + value: "" + - contains: + path: rules[0].resources + content: secrets + - contains: + path: rules[0].resources + content: configmaps + - contains: + path: rules + content: + apiGroups: + - "apps" + resources: + - deployments + - daemonsets + - statefulsets + verbs: + - list + - get + - watch + - update + - patch + + - it: scoped RoleBinding in a watched namespace binds the release ServiceAccount to the namespaced Role + set: + reloader: + watchGlobally: false + namespaces: + - team-a + - team-b + template: rolebinding.yaml + documentSelector: + path: metadata.namespace + value: team-b + asserts: + - isKind: + of: RoleBinding + - equal: + path: roleRef.name + value: reloader-reloader-role + - equal: + path: subjects[0].namespace + value: reloader-ns + + # ------------------------------------------------------------- metadata role + - it: metadata role is created in the release namespace with configmap write in scoped mode + set: + reloader: + watchGlobally: false + namespaces: + - team-a + template: role.yaml + documentSelector: + path: metadata.name + value: reloader-reloader-metadata-role + asserts: + - equal: + path: metadata.namespace + value: reloader-ns + - contains: + path: rules[0].resources + content: configmaps + - contains: + path: rules[0].verbs + content: create + + - it: metadata role also grants leases and events in the release namespace under HA + scoped mode + set: + reloader: + watchGlobally: false + enableHA: true + namespaces: + - team-a + template: role.yaml + documentSelector: + path: metadata.name + value: reloader-reloader-metadata-role + asserts: + - contains: + path: rules + content: + apiGroups: + - "coordination.k8s.io" + resources: + - leases + verbs: + - create + - get + - update + + - it: metadata role does NOT grant leases when HA is disabled in scoped mode + set: + reloader: + watchGlobally: false + enableHA: false + namespaces: + - team-a + template: role.yaml + documentSelector: + path: metadata.name + value: reloader-reloader-metadata-role + asserts: + - notContains: + path: rules + content: + apiGroups: + - "coordination.k8s.io" + resources: + - leases + verbs: + - create + - get + - update + + # ----------------------------------------------------------- single-ns mode + - it: single-namespace mode (watchGlobally=false, no list) renders one Role in the release namespace and no ClusterRole + set: + reloader: + watchGlobally: false + asserts: + # 1 single Role + 1 metadata Role + - hasDocuments: + count: 2 + template: role.yaml + - hasDocuments: + count: 0 + template: clusterrole.yaml + - equal: + path: metadata.namespace + value: reloader-ns + template: role.yaml + documentIndex: 0 + + # ---------------------------------------------------------------- global mode + - it: global mode renders a ClusterRole and only the metadata Role (no watched Roles) + set: + reloader: + watchGlobally: true + asserts: + - hasDocuments: + count: 1 + template: clusterrole.yaml + - isKind: + of: ClusterRole + template: clusterrole.yaml + # only the metadata role remains from role.yaml + - hasDocuments: + count: 1 + template: role.yaml + - equal: + path: metadata.name + value: reloader-reloader-metadata-role + template: role.yaml + documentIndex: 0 + + # -------------------------------------------------------- input normalization + - it: whitespace-only namespaces fall through to single-namespace mode (no watched Roles) + set: + reloader: + watchGlobally: false + namespaces: + - " " + - " " + asserts: + # cleaned list is empty -> single Role + metadata Role, both in release ns + - hasDocuments: + count: 2 + template: role.yaml + - equal: + path: metadata.namespace + value: reloader-ns + template: role.yaml + documentIndex: 0 + + - it: rbac disabled renders no Roles, RoleBindings, or ClusterRole + set: + reloader: + watchGlobally: false + rbac: + enabled: false + namespaces: + - team-a + asserts: + - hasDocuments: + count: 0 + template: role.yaml + - hasDocuments: + count: 0 + template: rolebinding.yaml diff --git a/deployments/kubernetes/chart/reloader/values.yaml b/deployments/kubernetes/chart/reloader/values.yaml index 1e867dc0c..119bc8e33 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,20 @@ 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 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: [] # 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 +145,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. diff --git a/internal/pkg/config/config.go b/internal/pkg/config/config.go index b63611e79..c1c7faed4 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,9 @@ 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 +} diff --git a/internal/pkg/config/config_test.go b/internal/pkg/config/config_test.go index bc6ea0a11..218c34d5c 100644 --- a/internal/pkg/config/config_test.go +++ b/internal/pkg/config/config_test.go @@ -218,3 +218,14 @@ 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") + } +} diff --git a/internal/pkg/config/flags.go b/internal/pkg/config/flags.go index 784fef108..70ba52258 100644 --- a/internal/pkg/config/flags.go +++ b/internal/pkg/config/flags.go @@ -5,6 +5,7 @@ import ( "strings" "time" + "github.com/go-logr/logr" "github.com/spf13/pflag" "github.com/spf13/viper" "k8s.io/apimachinery/pkg/labels" @@ -223,10 +224,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 @@ -263,9 +265,17 @@ 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 { +// 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 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") @@ -294,9 +304,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 @@ -381,6 +398,20 @@ func ApplyFlags(cfg *Config) error { cfg.LeaderElection.RetryPeriod = 2 * time.Second } + // 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 } @@ -412,3 +443,21 @@ func splitAndTrim(s string) []string { } return result } + +// trimAndDropEmptyStrings trims whitespace from each string in a slice and drops empty entries. +func trimAndDropEmptyStrings(ss []string) []string { + if len(ss) == 0 { + return nil + } + result := make([]string, 0, len(ss)) + for _, s := range ss { + s = strings.TrimSpace(s) + if s != "" { + result = append(result, s) + } + } + if len(result) == 0 { + return nil + } + return result +} diff --git a/internal/pkg/config/flags_test.go b/internal/pkg/config/flags_test.go index f9c6819a2..45d6d2cb3 100644 --- a/internal/pkg/config/flags_test.go +++ b/internal/pkg/config/flags_test.go @@ -4,6 +4,7 @@ import ( "strings" "testing" + "github.com/go-logr/logr" "github.com/spf13/pflag" "github.com/spf13/viper" ) @@ -65,7 +66,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", @@ -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 { @@ -582,3 +583,167 @@ 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, logr.Discard()); 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, logr.Discard()); 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, logr.Discard()); 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, logr.Discard()); 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, logr.Discard()); 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") + } +} + +// ApplyFlags must finalize a self-consistent config: in scoped mode it enforces +// namespace-scope semantics (clears selector/ignore lists) and logs a warning +// for each dropped setting. +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) + } + if err := ApplyFlags(cfg, logr.Discard()); 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) + } +} + +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) + } + if err := ApplyFlags(cfg, logr.Discard()); 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") + } +} diff --git a/internal/pkg/config/validation.go b/internal/pkg/config/validation.go index b3d2695b7..e3f725432 100644 --- a/internal/pkg/config/validation.go +++ b/internal/pkg/config/validation.go @@ -5,6 +5,7 @@ import ( "strings" "k8s.io/apimachinery/pkg/labels" + apivalidation "k8s.io/apimachinery/pkg/util/validation" "github.com/stakater/Reloader/internal/pkg/workload" ) @@ -97,6 +98,19 @@ func (c *Config) Validate() error { ) } + // Watched namespaces must be valid DNS-1123 labels; an invalid entry would + // otherwise fail deep inside the controller-runtime cache with an opaque error. + for _, ns := range c.WatchedNamespaces { + if msgs := apivalidation.IsDNS1123Label(ns); len(msgs) > 0 { + errs = append( + errs, ValidationError{ + Field: "WatchedNamespaces", + Message: fmt.Sprintf("invalid namespace %q: %s", ns, strings.Join(msgs, "; ")), + }, + ) + } + } + c.IgnoredResources = normalizeToLower(c.IgnoredResources) // Normalize ignored workloads to canonical Kind values (e.g., "cronjobs" -> "CronJob") diff --git a/internal/pkg/config/validation_test.go b/internal/pkg/config/validation_test.go index 52dc6f000..f4772c44d 100644 --- a/internal/pkg/config/validation_test.go +++ b/internal/pkg/config/validation_test.go @@ -194,6 +194,40 @@ func TestConfig_Validate_InvalidIgnoredWorkload(t *testing.T) { } } +func TestConfig_Validate_WatchedNamespaces(t *testing.T) { + tests := []struct { + name string + namespaces []string + wantErr bool + }{ + {"nil is valid (global mode)", nil, false}, + {"empty is valid (global mode)", []string{}, false}, + {"single valid label", []string{"team-a"}, false}, + {"multiple valid labels", []string{"team-a", "team-b", "kube-system"}, false}, + {"uppercase is invalid", []string{"Team-A"}, true}, + {"underscore is invalid", []string{"team_a"}, true}, + {"trailing dash is invalid", []string{"team-"}, true}, + {"one invalid among valid", []string{"team-a", "Bad_NS!"}, true}, + } + + for _, tt := range tests { + t.Run( + tt.name, func(t *testing.T) { + cfg := NewDefault() + cfg.WatchedNamespaces = tt.namespaces + + err := cfg.Validate() + if (err != nil) != tt.wantErr { + t.Fatalf("Validate() error = %v, wantErr %v", err, tt.wantErr) + } + if tt.wantErr && !strings.Contains(err.Error(), "WatchedNamespaces") { + t.Errorf("error should mention WatchedNamespaces, got: %v", err) + } + }, + ) + } +} + func TestConfig_Validate_MultipleErrors(t *testing.T) { cfg := NewDefault() cfg.ReloadStrategy = "invalid" diff --git a/internal/pkg/controller/manager.go b/internal/pkg/controller/manager.go index 869bdf001..a6ae3182e 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 @@ -52,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 @@ -79,16 +102,14 @@ 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) + mgr, err := ctrl.NewManager(restConfig, mgrOpts) if err != nil { return nil, fmt.Errorf("creating manager: %w", err) } @@ -107,45 +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 cfg.WatchedNamespace != "" { - mgrOpts.Cache = cache.Options{ - DefaultNamespaces: map[string]cache.Config{ - cfg.WatchedNamespace: {}, - }, - } - } - - 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( diff --git a/internal/pkg/controller/manager_test.go b/internal/pkg/controller/manager_test.go index 7b671c003..863d1acd3 100644 --- a/internal/pkg/controller/manager_test.go +++ b/internal/pkg/controller/manager_test.go @@ -9,6 +9,25 @@ import ( clientgoscheme "k8s.io/client-go/kubernetes/scheme" ) +func TestBuildDefaultNamespaces(t *testing.T) { + if got := buildDefaultNamespaces(nil); got != nil { + t.Errorf("empty input should return nil, got %v", got) + } + if got := buildDefaultNamespaces([]string{}); got != nil { + t.Errorf("empty slice should return nil, got %v", got) + } + + got := buildDefaultNamespaces([]string{"team-a", "team-b", "team-c"}) + if len(got) != 3 { + t.Fatalf("expected 3 entries, got %d", len(got)) + } + for _, ns := range []string{"team-a", "team-b", "team-c"} { + if _, ok := got[ns]; !ok { + t.Errorf("missing namespace %q in %v", ns, got) + } + } +} + func TestAddOptionalSchemesRegistersCSI(t *testing.T) { // Reset to a clean scheme for the test. runtimeScheme = runtime.NewScheme() diff --git a/internal/pkg/metadata/publisher.go b/internal/pkg/metadata/publisher.go index 6c6a42221..8cd349e15 100644 --- a/internal/pkg/metadata/publisher.go +++ b/internal/pkg/metadata/publisher.go @@ -8,6 +8,8 @@ import ( "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/stakater/Reloader/internal/pkg/config" @@ -77,10 +79,21 @@ func PublishMetaInfoConfigMap(ctx context.Context, c client.Client, cfg *config. return publisher.Publish(ctx) } -// Runnable returns a controller-runtime Runnable that publishes the metadata ConfigMap -// when the manager starts. This ensures the cache is ready before accessing the API. -func Runnable(c client.Client, cfg *config.Config, log logr.Logger) RunnableFunc { +// Runnable returns a controller-runtime Runnable that publishes the meta-info +// ConfigMap when the manager starts. It builds its own uncached client from the +// given rest config and scheme: the ConfigMap lives in Reloader's own namespace, +// which the manager cache does not cover in scoped mode, so a cache-backed client +// cannot read or write it there. Meta-info is internal instance metadata and is +// always published regardless of which resources are watched. +func Runnable(restConfig *rest.Config, scheme *runtime.Scheme, cfg *config.Config, log logr.Logger) RunnableFunc { return func(ctx context.Context) error { + c, err := client.New(restConfig, client.Options{Scheme: scheme}) + if err != nil { + log.Error(err, "Failed to create client for meta info configmap publisher") + // Non-fatal, don't return error to avoid crashing the manager + <-ctx.Done() + return nil + } if err := PublishMetaInfoConfigMap(ctx, c, cfg, log); err != nil { log.Error(err, "Failed to create metadata ConfigMap") // Non-fatal, don't return error to avoid crashing the manager diff --git a/test/e2e/advanced/multi_container_test.go b/test/e2e/advanced/multi_container_test.go index cef051846..776d5773f 100644 --- a/test/e2e/advanced/multi_container_test.go +++ b/test/e2e/advanced/multi_container_test.go @@ -47,13 +47,18 @@ var _ = Describe("Multi-Container Tests", Serial, func() { err = adapter.WaitReady(ctx, testNamespace, deploymentName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the ConfigMap") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"shared-key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment with multiple containers should be reloaded") }) @@ -81,13 +86,18 @@ var _ = Describe("Multi-Container Tests", Serial, func() { err = adapter.WaitReady(ctx, testNamespace, deploymentName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the first ConfigMap") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key1": "updated1"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should be reloaded when first container's ConfigMap changes") }) diff --git a/test/e2e/advanced/regex_test.go b/test/e2e/advanced/regex_test.go index 989bf0ab8..9c1e996d9 100644 --- a/test/e2e/advanced/regex_test.go +++ b/test/e2e/advanced/regex_test.go @@ -53,13 +53,18 @@ var _ = Describe("Regex Pattern Tests", func() { err = adapter.WaitReady(ctx, testNamespace, deploymentName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the matching ConfigMap") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, matchingCM, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should be reloaded when matching ConfigMap changes") }) @@ -87,14 +92,19 @@ var _ = Describe("Regex Pattern Tests", func() { err = adapter.WaitReady(ctx, testNamespace, deploymentName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the non-matching ConfigMap") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, nonMatchingCM, map[string]string{"other": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying Deployment was NOT reloaded (pattern mismatch)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "Deployment should NOT reload when non-matching ConfigMap changes") }) @@ -120,13 +130,18 @@ var _ = Describe("Regex Pattern Tests", func() { err = adapter.WaitReady(ctx, testNamespace, deploymentName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the matching Secret") err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, matchingSecret, map[string]string{"password": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should be reloaded when matching Secret changes") }) diff --git a/test/e2e/annotations/auto_reload_test.go b/test/e2e/annotations/auto_reload_test.go index af4b7ac3f..5961feb42 100644 --- a/test/e2e/annotations/auto_reload_test.go +++ b/test/e2e/annotations/auto_reload_test.go @@ -47,12 +47,16 @@ var _ = Describe("Auto Reload Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap data") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment with auto=true should have been reloaded") }) @@ -75,12 +79,16 @@ var _ = Describe("Auto Reload Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the Secret data") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, secretName, map[string]string{"password": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment with auto=true should have been reloaded for Secret change") }) @@ -108,12 +116,16 @@ var _ = Describe("Auto Reload Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"config": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment with auto=true should have been reloaded for ConfigMap change") }) @@ -145,12 +157,16 @@ var _ = Describe("Auto Reload Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"config": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should have been reloaded for ConfigMap change") }) @@ -180,12 +196,16 @@ var _ = Describe("Auto Reload Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the Secret") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, secretName, map[string]string{"secret": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should have been reloaded for Secret change") }) @@ -221,12 +241,16 @@ var _ = Describe("Auto Reload Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the second ConfigMap (auto-detected)") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName2, map[string]string{"key2": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should have been reloaded for auto-detected ConfigMap change") }) diff --git a/test/e2e/annotations/combination_test.go b/test/e2e/annotations/combination_test.go index e7f02efa5..dab5a8fd5 100644 --- a/test/e2e/annotations/combination_test.go +++ b/test/e2e/annotations/combination_test.go @@ -61,12 +61,16 @@ var _ = Describe("Combination Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the auto-detected ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should reload when auto-detected ConfigMap changes") }) @@ -95,12 +99,16 @@ var _ = Describe("Combination Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the explicitly listed ConfigMap (not mounted)") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName2, map[string]string{"extra": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should reload when explicitly listed ConfigMap changes") }) @@ -129,12 +137,16 @@ var _ = Describe("Combination Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the explicitly listed Secret") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, secretName2, map[string]string{"api-key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should reload when explicitly listed Secret changes") }) @@ -166,13 +178,17 @@ var _ = Describe("Combination Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the excluded ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName2, map[string]string{"excluded": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying Deployment was NOT reloaded (negative test)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "Deployment should NOT reload when excluded ConfigMap changes") }) @@ -202,12 +218,16 @@ var _ = Describe("Combination Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the non-excluded ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should reload when non-excluded ConfigMap changes") }) @@ -237,13 +257,17 @@ var _ = Describe("Combination Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the excluded Secret") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, secretName2, map[string]string{"excluded": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying Deployment was NOT reloaded (negative test)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "Deployment should NOT reload when excluded Secret changes") }) @@ -270,12 +294,16 @@ var _ = Describe("Combination Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the second ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName2, map[string]string{"key2": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should reload when any of the listed ConfigMaps changes") }) @@ -300,12 +328,16 @@ var _ = Describe("Combination Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the first Secret") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, secretName, map[string]string{"key1": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should reload when any of the listed Secrets changes") }) @@ -333,12 +365,16 @@ var _ = Describe("Combination Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the Secret") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, secretName, map[string]string{"password": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should reload when Secret changes with both annotations present") }) diff --git a/test/e2e/annotations/exclude_test.go b/test/e2e/annotations/exclude_test.go index 5a336514a..bce14ee6b 100644 --- a/test/e2e/annotations/exclude_test.go +++ b/test/e2e/annotations/exclude_test.go @@ -66,13 +66,17 @@ var _ = Describe("Exclude Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the excluded ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying Deployment was NOT reloaded (excluded ConfigMap)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "Deployment should NOT reload when excluded ConfigMap changes") }) @@ -103,12 +107,16 @@ var _ = Describe("Exclude Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the non-excluded ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName2, map[string]string{"key2": "updated2"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should reload when non-excluded ConfigMap changes") }) @@ -141,13 +149,17 @@ var _ = Describe("Exclude Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the excluded Secret") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, secretName, map[string]string{"password": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying Deployment was NOT reloaded (excluded Secret)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "Deployment should NOT reload when excluded Secret changes") }) @@ -178,12 +190,16 @@ var _ = Describe("Exclude Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the non-excluded Secret") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, secretName2, map[string]string{"password2": "updated2"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should reload when non-excluded Secret changes") }) @@ -225,13 +241,17 @@ var _ = Describe("Exclude Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the excluded ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying workload was NOT reloaded (excluded ConfigMap)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "%s should NOT reload with exclude on pod template", workloadType) }, diff --git a/test/e2e/annotations/pause_period_test.go b/test/e2e/annotations/pause_period_test.go index 869aed1dc..6e80b55ff 100644 --- a/test/e2e/annotations/pause_period_test.go +++ b/test/e2e/annotations/pause_period_test.go @@ -49,12 +49,16 @@ var _ = Describe("Pause Period Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap data") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should have been reloaded") @@ -83,12 +87,16 @@ var _ = Describe("Pause Period Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap data") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should have been reloaded") @@ -124,12 +132,16 @@ var _ = Describe("Pause Period Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap data") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should have been reloaded") diff --git a/test/e2e/annotations/resource_ignore_test.go b/test/e2e/annotations/resource_ignore_test.go index 132c91a65..0d97724d6 100644 --- a/test/e2e/annotations/resource_ignore_test.go +++ b/test/e2e/annotations/resource_ignore_test.go @@ -50,13 +50,17 @@ var _ = Describe("Resource Ignore Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap data") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying Deployment was NOT reloaded (negative test)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "Deployment should NOT reload when ConfigMap has ignore=true") }) @@ -80,13 +84,17 @@ var _ = Describe("Resource Ignore Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the Secret data") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, secretName, map[string]string{"password": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying Deployment was NOT reloaded (negative test)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "Deployment should NOT reload when Secret has ignore=true") }) diff --git a/test/e2e/annotations/search_match_test.go b/test/e2e/annotations/search_match_test.go index 02a1153ca..dac035774 100644 --- a/test/e2e/annotations/search_match_test.go +++ b/test/e2e/annotations/search_match_test.go @@ -50,12 +50,16 @@ var _ = Describe("Search and Match Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap data") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment with search annotation should reload when ConfigMap has match annotation") }) @@ -78,13 +82,17 @@ var _ = Describe("Search and Match Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap data") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying Deployment was NOT reloaded (negative test)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "Deployment should NOT reload when ConfigMap lacks match annotation") }) @@ -106,13 +114,17 @@ var _ = Describe("Search and Match Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap data") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying Deployment was NOT reloaded (negative test)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "Deployment without search annotation should NOT reload even when ConfigMap has match") }) @@ -149,18 +161,24 @@ var _ = Describe("Search and Match Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap data") + // Capture the reload-annotation baselines before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload1, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + priorReload2, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName2, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for first Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload1, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment with search annotation should reload") By("Verifying second Deployment was NOT reloaded") - reloaded2, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName2, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded2, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName2, + utils.AnnotationLastReloadedFrom, priorReload2, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded2).To(BeFalse(), "Deployment without search annotation should NOT reload") }) @@ -196,12 +214,16 @@ var _ = Describe("Search and Match Annotation Tests", func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s should reload with search annotation on pod template", workloadType) }, diff --git a/test/e2e/argo/rollout_test.go b/test/e2e/argo/rollout_test.go index 019df62b8..e6edf153c 100644 --- a/test/e2e/argo/rollout_test.go +++ b/test/e2e/argo/rollout_test.go @@ -47,13 +47,18 @@ var _ = Describe("Argo Rollout Strategy Tests", func() { err = adapter.WaitReady(ctx, testNamespace, rolloutName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, rolloutName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the ConfigMap") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Rollout to be reloaded with annotation") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, rolloutName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, rolloutName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Argo Rollout should be reloaded with default rollout strategy") }) diff --git a/test/e2e/core/reference_methods_test.go b/test/e2e/core/reference_methods_test.go index f3c0b8fd5..60bf129ef 100644 --- a/test/e2e/core/reference_methods_test.go +++ b/test/e2e/core/reference_methods_test.go @@ -58,13 +58,17 @@ var _ = Describe("Reference Method Tests", func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the ConfigMap") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"config_key": "updated_value"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s with valueFrom.configMapKeyRef should reload", workloadType) }, @@ -107,13 +111,17 @@ var _ = Describe("Reference Method Tests", func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the Secret") err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, secretName, map[string]string{"secret_key": "updated_secret"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s with valueFrom.secretKeyRef should reload", workloadType) }, @@ -154,13 +162,17 @@ var _ = Describe("Reference Method Tests", func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the ConfigMap") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"config.yaml": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s with projected ConfigMap volume should reload", workloadType) }, @@ -196,13 +208,17 @@ var _ = Describe("Reference Method Tests", func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the Secret") err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, secretName, map[string]string{"credentials": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s with projected Secret volume should reload", workloadType) }, @@ -246,13 +262,17 @@ var _ = Describe("Reference Method Tests", func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the ConfigMap") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"config.yaml": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s should reload when ConfigMap in mixed projected volume changes", workloadType) }, @@ -296,13 +316,17 @@ var _ = Describe("Reference Method Tests", func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the Secret") err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, secretName, map[string]string{"credentials": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s should reload when Secret in mixed projected volume changes", workloadType) }, @@ -343,13 +367,17 @@ var _ = Describe("Reference Method Tests", func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the ConfigMap") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"INIT_VAR": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s with init container ConfigMap should reload", workloadType) }, @@ -385,13 +413,17 @@ var _ = Describe("Reference Method Tests", func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the Secret") err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, secretName, map[string]string{"INIT_SECRET": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s with init container Secret should reload", workloadType) }, @@ -429,13 +461,17 @@ var _ = Describe("Reference Method Tests", func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the ConfigMap") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"config.yaml": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s with init container ConfigMap volume should reload", workloadType) }, @@ -471,13 +507,17 @@ var _ = Describe("Reference Method Tests", func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the Secret") err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, secretName, map[string]string{"credentials": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s with init container Secret volume should reload", workloadType) }, @@ -520,13 +560,17 @@ var _ = Describe("Reference Method Tests", func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the ConfigMap") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"auto_config_key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s with auto=true and valueFrom should reload", workloadType) }, diff --git a/test/e2e/core/workloads_test.go b/test/e2e/core/workloads_test.go index fdbe98ec4..b6a1a1954 100644 --- a/test/e2e/core/workloads_test.go +++ b/test/e2e/core/workloads_test.go @@ -64,13 +64,17 @@ var _ = Describe("Workload Reload Tests", Serial, func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the ConfigMap data") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, - utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s should have been reloaded", workloadType) }, @@ -106,13 +110,17 @@ var _ = Describe("Workload Reload Tests", Serial, func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the Secret data") err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, secretName, map[string]string{"password": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, - utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s should have been reloaded", workloadType) }, @@ -149,13 +157,17 @@ var _ = Describe("Workload Reload Tests", Serial, func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the ConfigMap data") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s with auto=true should have been reloaded", workloadType) }, @@ -192,14 +204,18 @@ var _ = Describe("Workload Reload Tests", Serial, func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating only the ConfigMap labels (no data change)") err = utils.UpdateConfigMapLabels(ctx, kubeClient, testNamespace, configMapName, map[string]string{"new-label": "new-value"}) Expect(err).NotTo(HaveOccurred()) By("Verifying workload was NOT reloaded (negative test)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "%s should NOT reload when only ConfigMap labels change", workloadType) }, @@ -235,14 +251,18 @@ var _ = Describe("Workload Reload Tests", Serial, func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating only the Secret labels (no data change)") err = utils.UpdateSecretLabels(ctx, kubeClient, testNamespace, secretName, map[string]string{"new-label": "new-value"}) Expect(err).NotTo(HaveOccurred()) By("Verifying workload was NOT reloaded (negative test)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "%s should NOT reload when only Secret labels change", workloadType) }, @@ -369,13 +389,17 @@ var _ = Describe("Workload Reload Tests", Serial, func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the ConfigMap data") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"config.yaml": "setting: updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, - utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s with volume-mounted ConfigMap should have been reloaded", workloadType) }, @@ -410,13 +434,17 @@ var _ = Describe("Workload Reload Tests", Serial, func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the Secret data") err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, secretName, map[string]string{"credentials.yaml": "secret: updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, - utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s with volume-mounted Secret should have been reloaded", workloadType) }, @@ -451,14 +479,18 @@ var _ = Describe("Workload Reload Tests", Serial, func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the ConfigMap data") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying workload is NOT reloaded (negative test)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, - utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "%s without Reloader annotation should NOT be reloaded", workloadType) }, @@ -507,13 +539,19 @@ var _ = Describe("Workload Reload Tests", Serial, func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger: Reloader can react + // to the ConfigMap update before WaitReloaded captures its baseline, which would + // otherwise record the already-reloaded value and time out (TOCTOU race). + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the second ConfigMap") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName2, map[string]string{"key2": "updated-value2"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s should reload when second ConfigMap changes", workloadType) }, @@ -556,13 +594,18 @@ var _ = Describe("Workload Reload Tests", Serial, func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the TOCTOU + // race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the second Secret") err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, secretName2, map[string]string{"key2": "updated-value2"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s should reload when second Secret changes", workloadType) }, @@ -598,13 +641,17 @@ var _ = Describe("Workload Reload Tests", Serial, func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("First update to ConfigMap") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "v2"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for first reload") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue()) @@ -666,13 +713,17 @@ var _ = Describe("Workload Reload Tests", Serial, func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the Secret") err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, secretName, map[string]string{"secret": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s should reload when Secret changes", workloadType) }, @@ -708,14 +759,18 @@ var _ = Describe("Workload Reload Tests", Serial, func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the ConfigMap data") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying workload is NOT reloaded (auto=false)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "%s with auto=false should NOT be reloaded", workloadType) }, @@ -759,13 +814,17 @@ var _ = Describe("Workload Reload Tests", Serial, func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the ConfigMap") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s should reload with pod template annotation", workloadType) }, @@ -801,13 +860,17 @@ var _ = Describe("Workload Reload Tests", Serial, func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the Secret") err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, secretName, map[string]string{"password": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s should reload with pod template annotation", workloadType) }, @@ -843,13 +906,17 @@ var _ = Describe("Workload Reload Tests", Serial, func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the ConfigMap") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s with auto=true on pod template should reload", workloadType) }, @@ -886,13 +953,17 @@ var _ = Describe("Workload Reload Tests", Serial, func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the ConfigMap") err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "%s should reload with annotations on both locations", workloadType) }, @@ -932,14 +1003,18 @@ var _ = Describe("Workload Reload Tests", Serial, func() { err = adapter.WaitReady(ctx, testNamespace, workloadName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the Secret (not the ConfigMap)") err = utils.UpdateSecretFromStrings(ctx, kubeClient, testNamespace, secretName, map[string]string{"password": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying workload was NOT reloaded (negative test)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, workloadName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, workloadName, utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "%s should NOT reload when updating different resource than annotated", workloadType) }, @@ -1016,8 +1091,7 @@ var _ = Describe("Workload Reload Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to have STAKATER_ env var") - found, err := adapter.WaitEnvVar(ctx, testNamespace, workloadName, utils.StakaterEnvVarPrefix, - utils.ReloadTimeout) + found, err := adapter.WaitEnvVarFrom(ctx, testNamespace, workloadName, utils.StakaterEnvVarPrefix, "", utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(found).To(BeTrue(), "%s should have STAKATER_ env var after ConfigMap change", workloadType) }, @@ -1061,8 +1135,7 @@ var _ = Describe("Workload Reload Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Waiting for workload to have STAKATER_ env var") - found, err := adapter.WaitEnvVar(ctx, testNamespace, workloadName, utils.StakaterEnvVarPrefix, - utils.ReloadTimeout) + found, err := adapter.WaitEnvVarFrom(ctx, testNamespace, workloadName, utils.StakaterEnvVarPrefix, "", utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(found).To(BeTrue(), "%s should have STAKATER_ env var after Secret change", workloadType) }, @@ -1109,8 +1182,7 @@ var _ = Describe("Workload Reload Tests", Serial, func() { By("Verifying workload does NOT have STAKATER_ env var") time.Sleep(utils.NegativeTestWait) - found, err := adapter.WaitEnvVar(ctx, testNamespace, workloadName, utils.StakaterEnvVarPrefix, - utils.ShortTimeout) + found, err := adapter.WaitEnvVarFrom(ctx, testNamespace, workloadName, utils.StakaterEnvVarPrefix, "", utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(found).To(BeFalse(), "%s should NOT have STAKATER_ env var for label-only change", workloadType) }, @@ -1154,8 +1226,7 @@ var _ = Describe("Workload Reload Tests", Serial, func() { By("Verifying workload does NOT have STAKATER_ env var") time.Sleep(utils.NegativeTestWait) - found, err := adapter.WaitEnvVar(ctx, testNamespace, workloadName, utils.StakaterEnvVarPrefix, - utils.ShortTimeout) + found, err := adapter.WaitEnvVarFrom(ctx, testNamespace, workloadName, utils.StakaterEnvVarPrefix, "", utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(found).To(BeFalse(), "%s should NOT have STAKATER_ env var for label-only change", workloadType) }, diff --git a/test/e2e/csi/csi_test.go b/test/e2e/csi/csi_test.go index ef55f2bdf..08604d615 100644 --- a/test/e2e/csi/csi_test.go +++ b/test/e2e/csi/csi_test.go @@ -72,6 +72,11 @@ var _ = Describe("CSI SecretProviderClass Tests", Label("csi"), Serial, func() { Expect(err).NotTo(HaveOccurred()) GinkgoWriter.Printf("Initial SPCPS version: %s\n", initialVersion) + // Capture the reload-annotation baseline before the trigger: Reloader reacts to the + // same SPCPS update the test waits on below, so it may reload before WaitReloaded runs. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the Vault secret") err = utils.UpdateVaultSecret( ctx, kubeClient, restConfig, vaultSecretPath, map[string]string{"api_key": "updated-value-v2"}) @@ -83,9 +88,9 @@ var _ = Describe("CSI SecretProviderClass Tests", Label("csi"), Serial, func() { GinkgoWriter.Println("CSI driver synced new secret version") By("Waiting for Deployment to be reloaded by Reloader") - reloaded, err := adapter.WaitReloaded( + reloaded, err := adapter.WaitReloadedFrom( ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout, ) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should have been reloaded after Vault secret change") @@ -124,6 +129,10 @@ var _ = Describe("CSI SecretProviderClass Tests", Label("csi"), Serial, func() { By("First update to Vault secret") initialVersion, _ := utils.GetSPCPSVersion(ctx, csiClient, testNamespace, spcpsName) + // Capture the baseline before the trigger to avoid racing Reloader's own reaction + // to the SPCPS update below. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateVaultSecret( ctx, kubeClient, restConfig, vaultSecretPath, map[string]string{"password": "pass-v2"}) Expect(err).NotTo(HaveOccurred()) @@ -133,9 +142,9 @@ var _ = Describe("CSI SecretProviderClass Tests", Label("csi"), Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Waiting for first reload") - reloaded, err := adapter.WaitReloaded( + reloaded, err := adapter.WaitReloadedFrom( ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout, ) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue()) @@ -207,6 +216,11 @@ var _ = Describe("CSI SecretProviderClass Tests", Label("csi"), Serial, func() { err = adapter.WaitReady(ctx, testNamespace, deploymentName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the ConfigMap (should NOT trigger reload)") err = utils.UpdateConfigMap( ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) @@ -214,9 +228,9 @@ var _ = Describe("CSI SecretProviderClass Tests", Label("csi"), Serial, func() { By("Verifying Deployment was NOT reloaded for ConfigMap change") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded( + reloaded, err := adapter.WaitReloadedFrom( ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout, ) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "SPC auto annotation should not trigger reload for ConfigMap changes") @@ -230,6 +244,11 @@ var _ = Describe("CSI SecretProviderClass Tests", Label("csi"), Serial, func() { By("Getting SPCPS version before Vault update") initialVersion, _ := utils.GetSPCPSVersion(ctx, csiClient, testNamespace, spcpsName) + // Capture the baseline before the trigger to avoid racing Reloader's own reaction + // to the SPCPS update below. + priorReload, err = adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the Vault secret (should trigger reload)") err = utils.UpdateVaultSecret( ctx, kubeClient, restConfig, vaultSecretPath, map[string]string{"token": "token-v2"}) @@ -240,9 +259,9 @@ var _ = Describe("CSI SecretProviderClass Tests", Label("csi"), Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Verifying Deployment WAS reloaded for Vault secret change") - reloaded, err = adapter.WaitReloaded( + reloaded, err = adapter.WaitReloadedFrom( ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout, ) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "SPC auto annotation should trigger reload for Vault secret changes") @@ -281,15 +300,20 @@ var _ = Describe("CSI SecretProviderClass Tests", Label("csi"), Serial, func() { err = adapter.WaitReady(ctx, testNamespace, deploymentName, utils.WorkloadReadyTimeout) Expect(err).NotTo(HaveOccurred()) + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) + By("Updating the ConfigMap (should trigger reload with auto=true)") err = utils.UpdateConfigMap( ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying Deployment WAS reloaded for ConfigMap change") - reloaded, err := adapter.WaitReloaded( + reloaded, err := adapter.WaitReloadedFrom( ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout, ) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Combined auto=true should trigger reload for ConfigMap changes") diff --git a/test/e2e/flags/auto_reload_all_test.go b/test/e2e/flags/auto_reload_all_test.go index f4cda1cd9..839e5e0b1 100644 --- a/test/e2e/flags/auto_reload_all_test.go +++ b/test/e2e/flags/auto_reload_all_test.go @@ -65,12 +65,16 @@ var _ = Describe("Auto Reload All Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, autoNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, autoNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded (autoReloadAll=true)") - reloaded, err := adapter.WaitReloaded(ctx, autoNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, autoNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment without annotations should reload when autoReloadAll=true") }) @@ -93,13 +97,17 @@ var _ = Describe("Auto Reload All Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, autoNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, autoNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying Deployment was NOT reloaded (auto=false overrides autoReloadAll)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, autoNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, autoNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "Deployment with auto=false should NOT reload even with autoReloadAll=true") }) diff --git a/test/e2e/flags/ignore_resources_test.go b/test/e2e/flags/ignore_resources_test.go index 44eb539df..e28f88858 100644 --- a/test/e2e/flags/ignore_resources_test.go +++ b/test/e2e/flags/ignore_resources_test.go @@ -69,13 +69,17 @@ var _ = Describe("Ignore Resources Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Updating the Secret") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, ignoreNS, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateSecretFromStrings(ctx, kubeClient, ignoreNS, secretName, map[string]string{"password": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying Deployment was NOT reloaded (ignoreSecrets=true)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, ignoreNS, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, ignoreNS, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "Deployment should NOT reload when ignoreSecrets=true") }) @@ -98,12 +102,16 @@ var _ = Describe("Ignore Resources Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, ignoreNS, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, ignoreNS, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded (ConfigMap should still work)") - reloaded, err := adapter.WaitReloaded(ctx, ignoreNS, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, ignoreNS, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "ConfigMap changes should still trigger reload with ignoreSecrets=true") }) @@ -146,13 +154,17 @@ var _ = Describe("Ignore Resources Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, ignoreNS, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, ignoreNS, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying Deployment was NOT reloaded (ignoreConfigMaps=true)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, ignoreNS, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, ignoreNS, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "Deployment should NOT reload when ignoreConfigMaps=true") }) @@ -175,12 +187,16 @@ var _ = Describe("Ignore Resources Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Updating the Secret") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, ignoreNS, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateSecretFromStrings(ctx, kubeClient, ignoreNS, secretName, map[string]string{"password": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded (Secret should still work)") - reloaded, err := adapter.WaitReloaded(ctx, ignoreNS, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, ignoreNS, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Secret changes should still trigger reload with ignoreConfigMaps=true") }) diff --git a/test/e2e/flags/ignored_workloads_test.go b/test/e2e/flags/ignored_workloads_test.go index 7a5185ce5..0bd4449c5 100644 --- a/test/e2e/flags/ignored_workloads_test.go +++ b/test/e2e/flags/ignored_workloads_test.go @@ -64,13 +64,17 @@ var _ = Describe("Ignored Workloads Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := cronJobAdapter.GetPodTemplateAnnotation(ctx, ignoreNS, cronJobName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, ignoreNS, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying CronJob was NOT reloaded (ignoreCronJobs=true)") time.Sleep(utils.NegativeTestWait) - reloaded, err := cronJobAdapter.WaitReloaded(ctx, ignoreNS, cronJobName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := cronJobAdapter.WaitReloadedFrom(ctx, ignoreNS, cronJobName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "CronJob should NOT reload when ignoreCronJobs=true") }) @@ -98,12 +102,16 @@ var _ = Describe("Ignored Workloads Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := deploymentAdapter.GetPodTemplateAnnotation(ctx, ignoreNS, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, ignoreNS, configMapName, map[string]string{"key": "updated-deploy"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded (Deployment should still work)") - reloaded, err := deploymentAdapter.WaitReloaded(ctx, ignoreNS, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := deploymentAdapter.WaitReloadedFrom(ctx, ignoreNS, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should still reload with ignoreCronJobs=true") }) @@ -143,13 +151,17 @@ var _ = Describe("Ignored Workloads Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := cronJobAdapter.GetPodTemplateAnnotation(ctx, ignoreNS, cronJobName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, ignoreNS, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying CronJob was NOT reloaded") time.Sleep(utils.NegativeTestWait) - reloaded, err := cronJobAdapter.WaitReloaded(ctx, ignoreNS, cronJobName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := cronJobAdapter.WaitReloadedFrom(ctx, ignoreNS, cronJobName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "CronJob should NOT reload when ignoreCronJobs=true and ignoreJobs=true") }) diff --git a/test/e2e/flags/namespace_ignore_test.go b/test/e2e/flags/namespace_ignore_test.go index 394679091..d35995409 100644 --- a/test/e2e/flags/namespace_ignore_test.go +++ b/test/e2e/flags/namespace_ignore_test.go @@ -73,13 +73,17 @@ var _ = Describe("Namespace Ignore Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, ignoredNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, ignoredNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying Deployment was NOT reloaded (ignored namespace)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, ignoredNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, ignoredNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "Deployment in ignored namespace should NOT be reloaded") }) @@ -102,12 +106,16 @@ var _ = Describe("Namespace Ignore Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, watchedNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, watchedNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, watchedNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, watchedNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment in non-ignored namespace should be reloaded") }) diff --git a/test/e2e/flags/namespace_selector_test.go b/test/e2e/flags/namespace_selector_test.go index 4ac49cbed..a58da5795 100644 --- a/test/e2e/flags/namespace_selector_test.go +++ b/test/e2e/flags/namespace_selector_test.go @@ -74,12 +74,16 @@ var _ = Describe("Namespace Selector Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, matchingNS, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, matchingNS, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, matchingNS, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, matchingNS, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment in matching namespace should be reloaded") }) @@ -102,13 +106,17 @@ var _ = Describe("Namespace Selector Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, nonMatchingNS, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, nonMatchingNS, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying Deployment was NOT reloaded (non-matching namespace)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, nonMatchingNS, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, nonMatchingNS, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "Deployment in non-matching namespace should NOT be reloaded") }) diff --git a/test/e2e/flags/reload_on_create_test.go b/test/e2e/flags/reload_on_create_test.go index 63fec0bb3..b6946ef31 100644 --- a/test/e2e/flags/reload_on_create_test.go +++ b/test/e2e/flags/reload_on_create_test.go @@ -60,13 +60,17 @@ var _ = Describe("Reload On Create Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Creating the ConfigMap that the Deployment references") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, createNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) _, err = utils.CreateConfigMap(ctx, kubeClient, createNamespace, configMapName, map[string]string{"key": "value"}, nil) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded (reloadOnCreate=true)") - reloaded, err := adapter.WaitReloaded(ctx, createNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, createNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should reload when referenced ConfigMap is created") }) @@ -86,13 +90,17 @@ var _ = Describe("Reload On Create Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Creating the Secret that the Deployment references") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, createNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) _, err = utils.CreateSecretFromStrings(ctx, kubeClient, createNamespace, secretName, map[string]string{"password": "secret"}, nil) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded (reloadOnCreate=true)") - reloaded, err := adapter.WaitReloaded(ctx, createNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, createNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should reload when referenced Secret is created") }) @@ -127,14 +135,18 @@ var _ = Describe("Reload On Create Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Creating the ConfigMap that the Deployment references") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, createNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) _, err = utils.CreateConfigMap(ctx, kubeClient, createNamespace, configMapName, map[string]string{"key": "value"}, nil) Expect(err).NotTo(HaveOccurred()) By("Verifying Deployment was NOT reloaded (reloadOnCreate=false)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, createNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, createNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "Deployment should NOT reload on create when reloadOnCreate=false") }) diff --git a/test/e2e/flags/reload_on_delete_test.go b/test/e2e/flags/reload_on_delete_test.go index ed400e35f..88fda7663 100644 --- a/test/e2e/flags/reload_on_delete_test.go +++ b/test/e2e/flags/reload_on_delete_test.go @@ -65,12 +65,16 @@ var _ = Describe("Reload On Delete Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Deleting the ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, deleteNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.DeleteConfigMap(ctx, kubeClient, deleteNamespace, configMapName) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded (reloadOnDelete=true)") - reloaded, err := adapter.WaitReloaded(ctx, deleteNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, deleteNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should reload when referenced ConfigMap is deleted") }) @@ -94,12 +98,16 @@ var _ = Describe("Reload On Delete Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Deleting the Secret") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, deleteNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.DeleteSecret(ctx, kubeClient, deleteNamespace, secretName) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded (reloadOnDelete=true)") - reloaded, err := adapter.WaitReloaded(ctx, deleteNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, deleteNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should reload when referenced Secret is deleted") }) @@ -139,13 +147,17 @@ var _ = Describe("Reload On Delete Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Deleting the ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, deleteNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.DeleteConfigMap(ctx, kubeClient, deleteNamespace, configMapName) Expect(err).NotTo(HaveOccurred()) By("Verifying Deployment was NOT reloaded (reloadOnDelete=false)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, deleteNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, deleteNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "Deployment should NOT reload on delete when reloadOnDelete=false") }) diff --git a/test/e2e/flags/resource_selector_test.go b/test/e2e/flags/resource_selector_test.go index cc94612a8..af31a958c 100644 --- a/test/e2e/flags/resource_selector_test.go +++ b/test/e2e/flags/resource_selector_test.go @@ -70,12 +70,16 @@ var _ = Describe("Resource Label Selector Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Updating the labeled ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, resourceNS, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, resourceNS, matchingCM, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded") - reloaded, err := adapter.WaitReloaded(ctx, resourceNS, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, resourceNS, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment should be reloaded when labeled ConfigMap changes") }) @@ -98,13 +102,17 @@ var _ = Describe("Resource Label Selector Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Updating the unlabeled ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, resourceNS, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, resourceNS, nonMatchingCM, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying Deployment was NOT reloaded (unlabeled ConfigMap)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, resourceNS, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, resourceNS, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "Deployment should NOT reload when unlabeled ConfigMap changes") }) diff --git a/test/e2e/flags/watch_globally_test.go b/test/e2e/flags/watch_globally_test.go index 96e3fb2d5..204a3d657 100644 --- a/test/e2e/flags/watch_globally_test.go +++ b/test/e2e/flags/watch_globally_test.go @@ -68,12 +68,16 @@ var _ = Describe("Watch Globally Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, testNamespace, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, testNamespace, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded (same namespace should work)") - reloaded, err := adapter.WaitReloaded(ctx, testNamespace, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, testNamespace, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment in Reloader's namespace should reload with watchGlobally=false") }) @@ -96,13 +100,17 @@ var _ = Describe("Watch Globally Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap in the other namespace") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, otherNS, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, otherNS, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Verifying Deployment was NOT reloaded (different namespace with watchGlobally=false)") time.Sleep(utils.NegativeTestWait) - reloaded, err := adapter.WaitReloaded(ctx, otherNS, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ShortTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, otherNS, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ShortTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeFalse(), "Deployment in other namespace should NOT reload with watchGlobally=false") }) @@ -151,12 +159,16 @@ var _ = Describe("Watch Globally Flag Tests", Serial, func() { Expect(err).NotTo(HaveOccurred()) By("Updating the ConfigMap") + // Capture the reload-annotation baseline before the trigger to avoid the + // TOCTOU race where Reloader reloads before WaitReloaded records its baseline. + priorReload, err := adapter.GetPodTemplateAnnotation(ctx, globalNS, deploymentName, utils.AnnotationLastReloadedFrom) + Expect(err).NotTo(HaveOccurred()) err = utils.UpdateConfigMap(ctx, kubeClient, globalNS, configMapName, map[string]string{"key": "updated"}) Expect(err).NotTo(HaveOccurred()) By("Waiting for Deployment to be reloaded (watchGlobally=true)") - reloaded, err := adapter.WaitReloaded(ctx, globalNS, deploymentName, - utils.AnnotationLastReloadedFrom, utils.ReloadTimeout) + reloaded, err := adapter.WaitReloadedFrom(ctx, globalNS, deploymentName, + utils.AnnotationLastReloadedFrom, priorReload, utils.ReloadTimeout) Expect(err).NotTo(HaveOccurred()) Expect(reloaded).To(BeTrue(), "Deployment in any namespace should reload with watchGlobally=true") }) diff --git a/test/e2e/flags/watch_namespaces_test.go b/test/e2e/flags/watch_namespaces_test.go new file mode 100644 index 000000000..7f2a10315 --- /dev/null +++ b/test/e2e/flags/watch_namespaces_test.go @@ -0,0 +1,205 @@ +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") + }) +}) + +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) + }) +}) diff --git a/test/e2e/utils/workload_adapter.go b/test/e2e/utils/workload_adapter.go index 2610337fe..70a5e7e17 100644 --- a/test/e2e/utils/workload_adapter.go +++ b/test/e2e/utils/workload_adapter.go @@ -66,12 +66,27 @@ type WorkloadAdapter interface { // WaitReloaded waits for the workload to have the reload annotation. // Returns true if the annotation was found, false if timeout occurred. + // It captures the baseline annotation value at call time, which races with Reloader + // if the reload trigger happened earlier — prefer WaitReloadedFrom in that case. WaitReloaded(ctx context.Context, namespace, name, annotationKey string, timeout time.Duration) (bool, error) + // WaitReloadedFrom waits for the reload annotation to be present with a value different + // from priorValue. Capture priorValue (via GetPodTemplateAnnotation) BEFORE performing + // the change that triggers the reload; capturing it afterwards can observe the already + // reloaded value and then wait for a further change that never comes. + WaitReloadedFrom(ctx context.Context, namespace, name, annotationKey, priorValue string, timeout time.Duration) (bool, error) + // WaitEnvVar waits for the workload to have a STAKATER_ env var (for envvars strategy). // Returns true if the env var was found, false if timeout occurred. + // It captures the baseline env var value at call time, which races with Reloader + // if the reload trigger happened earlier — prefer WaitEnvVarFrom in that case. WaitEnvVar(ctx context.Context, namespace, name, prefix string, timeout time.Duration) (bool, error) + // WaitEnvVarFrom waits for a STAKATER_ env var whose value differs from priorValue. + // Capture priorValue BEFORE performing the change that triggers the reload + // (an empty priorValue means the env var is expected to appear). + WaitEnvVarFrom(ctx context.Context, namespace, name, prefix, priorValue string, timeout time.Duration) (bool, error) + // SupportsEnvVarStrategy returns true if the workload supports env var reload strategy. // CronJob does not support this as it uses job creation instead. SupportsEnvVarStrategy() bool diff --git a/test/e2e/utils/workload_argo.go b/test/e2e/utils/workload_argo.go index 69d5163e7..c976d7462 100644 --- a/test/e2e/utils/workload_argo.go +++ b/test/e2e/utils/workload_argo.go @@ -58,6 +58,12 @@ func (a *ArgoRolloutAdapter) WaitReady(ctx context.Context, namespace, name stri // Captures the current annotation value first to avoid false positives from prior reloads. func (a *ArgoRolloutAdapter) WaitReloaded(ctx context.Context, namespace, name, annotationKey string, timeout time.Duration) (bool, error) { priorValue, _ := a.GetPodTemplateAnnotation(ctx, namespace, name, annotationKey) + return a.WaitReloadedFrom(ctx, namespace, name, annotationKey, priorValue, timeout) +} + +// WaitReloadedFrom waits for the reload annotation to be present with a value different from +// priorValue, which the caller captured before triggering the reload. +func (a *ArgoRolloutAdapter) WaitReloadedFrom(ctx context.Context, namespace, name, annotationKey, priorValue string, timeout time.Duration) (bool, error) { watchFunc := func(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { return a.rolloutsClient.ArgoprojV1alpha1().Rollouts(namespace).Watch(ctx, opts) } @@ -72,6 +78,12 @@ func (a *ArgoRolloutAdapter) WaitEnvVar(ctx context.Context, namespace, name, pr if r, err := a.rolloutsClient.ArgoprojV1alpha1().Rollouts(namespace).Get(ctx, name, metav1.GetOptions{}); err == nil { priorValue = GetEnvVarValueByPrefix(r.Spec.Template.Spec.Containers, prefix) } + return a.WaitEnvVarFrom(ctx, namespace, name, prefix, priorValue, timeout) +} + +// WaitEnvVarFrom waits for a STAKATER_ env var whose value differs from priorValue, which the +// caller captured before triggering the reload. +func (a *ArgoRolloutAdapter) WaitEnvVarFrom(ctx context.Context, namespace, name, prefix, priorValue string, timeout time.Duration) (bool, error) { watchFunc := func(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { return a.rolloutsClient.ArgoprojV1alpha1().Rollouts(namespace).Watch(ctx, opts) } diff --git a/test/e2e/utils/workload_cronjob.go b/test/e2e/utils/workload_cronjob.go index c681cce96..151fe9ae9 100644 --- a/test/e2e/utils/workload_cronjob.go +++ b/test/e2e/utils/workload_cronjob.go @@ -50,6 +50,12 @@ func (a *CronJobAdapter) WaitReady(ctx context.Context, namespace, name string, // Captures the current annotation value first to avoid false positives from prior reloads. func (a *CronJobAdapter) WaitReloaded(ctx context.Context, namespace, name, annotationKey string, timeout time.Duration) (bool, error) { priorValue, _ := a.GetPodTemplateAnnotation(ctx, namespace, name, annotationKey) + return a.WaitReloadedFrom(ctx, namespace, name, annotationKey, priorValue, timeout) +} + +// WaitReloadedFrom waits for the reload annotation to be present with a value different from +// priorValue, which the caller captured before triggering the reload. +func (a *CronJobAdapter) WaitReloadedFrom(ctx context.Context, namespace, name, annotationKey, priorValue string, timeout time.Duration) (bool, error) { watchFunc := func(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { return a.client.BatchV1().CronJobs(namespace).Watch(ctx, opts) } @@ -62,6 +68,11 @@ func (a *CronJobAdapter) WaitEnvVar(ctx context.Context, namespace, name, prefix return false, ErrUnsupportedOperation } +// WaitEnvVarFrom returns an error because CronJobs don't support env var reload strategy. +func (a *CronJobAdapter) WaitEnvVarFrom(ctx context.Context, namespace, name, prefix, priorValue string, timeout time.Duration) (bool, error) { + return false, ErrUnsupportedOperation +} + // SupportsEnvVarStrategy returns false as CronJobs don't support env var reload strategy. func (a *CronJobAdapter) SupportsEnvVarStrategy() bool { return false diff --git a/test/e2e/utils/workload_daemonset.go b/test/e2e/utils/workload_daemonset.go index 4a7a2b14e..74aada0ee 100644 --- a/test/e2e/utils/workload_daemonset.go +++ b/test/e2e/utils/workload_daemonset.go @@ -50,6 +50,12 @@ func (a *DaemonSetAdapter) WaitReady(ctx context.Context, namespace, name string // Captures the current annotation value first to avoid false positives from prior reloads. func (a *DaemonSetAdapter) WaitReloaded(ctx context.Context, namespace, name, annotationKey string, timeout time.Duration) (bool, error) { priorValue, _ := a.GetPodTemplateAnnotation(ctx, namespace, name, annotationKey) + return a.WaitReloadedFrom(ctx, namespace, name, annotationKey, priorValue, timeout) +} + +// WaitReloadedFrom waits for the reload annotation to be present with a value different from +// priorValue, which the caller captured before triggering the reload. +func (a *DaemonSetAdapter) WaitReloadedFrom(ctx context.Context, namespace, name, annotationKey, priorValue string, timeout time.Duration) (bool, error) { watchFunc := func(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { return a.client.AppsV1().DaemonSets(namespace).Watch(ctx, opts) } @@ -64,6 +70,12 @@ func (a *DaemonSetAdapter) WaitEnvVar(ctx context.Context, namespace, name, pref if ds, err := a.client.AppsV1().DaemonSets(namespace).Get(ctx, name, metav1.GetOptions{}); err == nil { priorValue = GetEnvVarValueByPrefix(ds.Spec.Template.Spec.Containers, prefix) } + return a.WaitEnvVarFrom(ctx, namespace, name, prefix, priorValue, timeout) +} + +// WaitEnvVarFrom waits for a STAKATER_ env var whose value differs from priorValue, which the +// caller captured before triggering the reload. +func (a *DaemonSetAdapter) WaitEnvVarFrom(ctx context.Context, namespace, name, prefix, priorValue string, timeout time.Duration) (bool, error) { watchFunc := func(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { return a.client.AppsV1().DaemonSets(namespace).Watch(ctx, opts) } diff --git a/test/e2e/utils/workload_deployment.go b/test/e2e/utils/workload_deployment.go index f7ef5e37a..d64e194a1 100644 --- a/test/e2e/utils/workload_deployment.go +++ b/test/e2e/utils/workload_deployment.go @@ -51,6 +51,12 @@ func (a *DeploymentAdapter) WaitReady(ctx context.Context, namespace, name strin // does not cause a false positive — the condition triggers only when the value changes. func (a *DeploymentAdapter) WaitReloaded(ctx context.Context, namespace, name, annotationKey string, timeout time.Duration) (bool, error) { priorValue, _ := a.GetPodTemplateAnnotation(ctx, namespace, name, annotationKey) + return a.WaitReloadedFrom(ctx, namespace, name, annotationKey, priorValue, timeout) +} + +// WaitReloadedFrom waits for the reload annotation to be present with a value different from +// priorValue, which the caller captured before triggering the reload. +func (a *DeploymentAdapter) WaitReloadedFrom(ctx context.Context, namespace, name, annotationKey, priorValue string, timeout time.Duration) (bool, error) { watchFunc := func(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { return a.client.AppsV1().Deployments(namespace).Watch(ctx, opts) } @@ -66,6 +72,12 @@ func (a *DeploymentAdapter) WaitEnvVar(ctx context.Context, namespace, name, pre if d, err := a.client.AppsV1().Deployments(namespace).Get(ctx, name, metav1.GetOptions{}); err == nil { priorValue = GetEnvVarValueByPrefix(d.Spec.Template.Spec.Containers, prefix) } + return a.WaitEnvVarFrom(ctx, namespace, name, prefix, priorValue, timeout) +} + +// WaitEnvVarFrom waits for a STAKATER_ env var whose value differs from priorValue, which the +// caller captured before triggering the reload. +func (a *DeploymentAdapter) WaitEnvVarFrom(ctx context.Context, namespace, name, prefix, priorValue string, timeout time.Duration) (bool, error) { watchFunc := func(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { return a.client.AppsV1().Deployments(namespace).Watch(ctx, opts) } diff --git a/test/e2e/utils/workload_job.go b/test/e2e/utils/workload_job.go index e71c86c25..5f8ad5065 100644 --- a/test/e2e/utils/workload_job.go +++ b/test/e2e/utils/workload_job.go @@ -54,11 +54,22 @@ func (a *JobAdapter) WaitReloaded(ctx context.Context, namespace, name, annotati return false, ErrUnsupportedOperation } +// WaitReloadedFrom returns an error because Jobs are recreated, not updated. +// Use the Recreatable interface (GetOriginalUID + WaitRecreated) instead. +func (a *JobAdapter) WaitReloadedFrom(ctx context.Context, namespace, name, annotationKey, priorValue string, timeout time.Duration) (bool, error) { + return false, ErrUnsupportedOperation +} + // WaitEnvVar returns an error because Jobs don't support env var reload strategy. func (a *JobAdapter) WaitEnvVar(ctx context.Context, namespace, name, prefix string, timeout time.Duration) (bool, error) { return false, ErrUnsupportedOperation } +// WaitEnvVarFrom returns an error because Jobs don't support env var reload strategy. +func (a *JobAdapter) WaitEnvVarFrom(ctx context.Context, namespace, name, prefix, priorValue string, timeout time.Duration) (bool, error) { + return false, ErrUnsupportedOperation +} + // WaitRecreated waits for the Job to be recreated with a different UID using watches. func (a *JobAdapter) WaitRecreated(ctx context.Context, namespace, name, originalUID string, timeout time.Duration) (string, bool, error) { watchFunc := func(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { diff --git a/test/e2e/utils/workload_openshift.go b/test/e2e/utils/workload_openshift.go index 6f758bf43..2fb11c6c6 100644 --- a/test/e2e/utils/workload_openshift.go +++ b/test/e2e/utils/workload_openshift.go @@ -60,6 +60,12 @@ func (a *DeploymentConfigAdapter) WaitReady(ctx context.Context, namespace, name // Captures the current annotation value first to avoid false positives from prior reloads. func (a *DeploymentConfigAdapter) WaitReloaded(ctx context.Context, namespace, name, annotationKey string, timeout time.Duration) (bool, error) { priorValue, _ := a.GetPodTemplateAnnotation(ctx, namespace, name, annotationKey) + return a.WaitReloadedFrom(ctx, namespace, name, annotationKey, priorValue, timeout) +} + +// WaitReloadedFrom waits for the reload annotation to be present with a value different from +// priorValue, which the caller captured before triggering the reload. +func (a *DeploymentConfigAdapter) WaitReloadedFrom(ctx context.Context, namespace, name, annotationKey, priorValue string, timeout time.Duration) (bool, error) { watchFunc := func(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { return a.openshiftClient.AppsV1().DeploymentConfigs(namespace).Watch(ctx, opts) } @@ -74,6 +80,12 @@ func (a *DeploymentConfigAdapter) WaitEnvVar(ctx context.Context, namespace, nam if dc, err := a.openshiftClient.AppsV1().DeploymentConfigs(namespace).Get(ctx, name, metav1.GetOptions{}); err == nil && dc.Spec.Template != nil { priorValue = GetEnvVarValueByPrefix(dc.Spec.Template.Spec.Containers, prefix) } + return a.WaitEnvVarFrom(ctx, namespace, name, prefix, priorValue, timeout) +} + +// WaitEnvVarFrom waits for a STAKATER_ env var whose value differs from priorValue, which the +// caller captured before triggering the reload. +func (a *DeploymentConfigAdapter) WaitEnvVarFrom(ctx context.Context, namespace, name, prefix, priorValue string, timeout time.Duration) (bool, error) { watchFunc := func(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { return a.openshiftClient.AppsV1().DeploymentConfigs(namespace).Watch(ctx, opts) } diff --git a/test/e2e/utils/workload_statefulset.go b/test/e2e/utils/workload_statefulset.go index d071678a4..28052dce3 100644 --- a/test/e2e/utils/workload_statefulset.go +++ b/test/e2e/utils/workload_statefulset.go @@ -50,6 +50,12 @@ func (a *StatefulSetAdapter) WaitReady(ctx context.Context, namespace, name stri // Captures the current annotation value first to avoid false positives from prior reloads. func (a *StatefulSetAdapter) WaitReloaded(ctx context.Context, namespace, name, annotationKey string, timeout time.Duration) (bool, error) { priorValue, _ := a.GetPodTemplateAnnotation(ctx, namespace, name, annotationKey) + return a.WaitReloadedFrom(ctx, namespace, name, annotationKey, priorValue, timeout) +} + +// WaitReloadedFrom waits for the reload annotation to be present with a value different from +// priorValue, which the caller captured before triggering the reload. +func (a *StatefulSetAdapter) WaitReloadedFrom(ctx context.Context, namespace, name, annotationKey, priorValue string, timeout time.Duration) (bool, error) { watchFunc := func(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { return a.client.AppsV1().StatefulSets(namespace).Watch(ctx, opts) } @@ -64,6 +70,12 @@ func (a *StatefulSetAdapter) WaitEnvVar(ctx context.Context, namespace, name, pr if sts, err := a.client.AppsV1().StatefulSets(namespace).Get(ctx, name, metav1.GetOptions{}); err == nil { priorValue = GetEnvVarValueByPrefix(sts.Spec.Template.Spec.Containers, prefix) } + return a.WaitEnvVarFrom(ctx, namespace, name, prefix, priorValue, timeout) +} + +// WaitEnvVarFrom waits for a STAKATER_ env var whose value differs from priorValue, which the +// caller captured before triggering the reload. +func (a *StatefulSetAdapter) WaitEnvVarFrom(ctx context.Context, namespace, name, prefix, priorValue string, timeout time.Duration) (bool, error) { watchFunc := func(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { return a.client.AppsV1().StatefulSets(namespace).Watch(ctx, opts) }