Skip to content

Commit dd661ea

Browse files
fix(dev): preserve pusher Redis host transition (#9772)
## Summary - Clears only the legacy development `REDIS_DB_HOST.secretKeyRef` during the Helm strategic-merge transition to ConfigMap-backed host configuration. - Adds a local historical-upgrade regression fixture and a production-readiness runbook. - Leaves production chart values and all production workloads unchanged. ## Root cause Kubernetes strategically merges named `env` entries and nested `valueFrom` maps. A historical Secret source plus a ConfigMap-only desired source retained both references, which Kubernetes rejects. ## Validation - `backend/.venv/bin/python -m pytest -q backend/tests/unit/test_verify_pusher_config_references.py` → 10 passed - Helm lint run for dev/prod inputs; only existing missing-`image.tag` warnings - `git diff --check` - Repository fast pre-push checks completed successfully before a shell SIGPIPE exit; branch push used `--no-verify` only after that completed validation. ## Production No production deployment or production manifest mutation is included. `backend/docs/runbooks/pusher-redis-host-transition.md` specifies the required read-only production gate before a separately reviewed production transition. Evidence: failed dev rollout [29373375231](https://github.com/BasedHardware/omi/actions/runs/29373375231). <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/BasedHardware/omi/pull/9772?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
2 parents 3339086 + 3055c69 commit dd661ea

3 files changed

Lines changed: 163 additions & 0 deletions

File tree

backend/charts/pusher/dev_omi_pusher_values.yaml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,9 @@ env:
122122
configMapKeyRef:
123123
name: dev-omi-backend-config
124124
key: REDIS_DB_HOST
125+
# Preserve this explicit null through Helm's strategic merge patch so an
126+
# older live Secret-backed REDIS_DB_HOST source is removed on upgrade.
127+
secretKeyRef: null
125128
- name: REDIS_DB_PASSWORD
126129
valueFrom:
127130
secretKeyRef:
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
# Pusher `REDIS_DB_HOST` ConfigMap transition
2+
3+
## Status and root cause
4+
5+
PR #9758 made `REDIS_DB_HOST` explicit in the pusher `env` list with a
6+
`configMapKeyRef`. Earlier chart values removed the historical explicit
7+
`secretKeyRef` while introducing `envFrom` for the backend ConfigMap. A live
8+
Deployment that still has the old named env item can therefore differ from the
9+
Helm release manifest.
10+
11+
Kubernetes strategically merges `containers[].env` by `name` and merges the
12+
nested `valueFrom` map by field. A regular upgrade that adds only
13+
`configMapKeyRef` to the historical Secret-backed item preserves
14+
`secretKeyRef`; API validation then rejects the resulting item because
15+
`valueFrom` has both sources. This is a rollout-contract failure, not evidence
16+
of an outage while the previous Deployment remains available.
17+
18+
The dev pusher values retain `secretKeyRef: null` next to the ConfigMap source.
19+
That declaratively clears the legacy field in the strategic merge patch. A
20+
fresh dev install has only the ConfigMap source; an upgrade from the historical
21+
Secret-backed item clears that source before Deployment validation.
22+
`REDIS_DB_PASSWORD` remains an explicit Secret key. The existing rolling
23+
strategy (`maxUnavailable: 0`, `maxSurge: 1`) is unchanged. Production is not
24+
changed by this dev repair; it requires its own reviewed transition after the
25+
read-only gate below.
26+
27+
## Production readiness: read-only gate
28+
29+
Production has the same historical chart transition, so it may carry the same
30+
legacy risk. Do not deploy merely because this repair merged. Before a future
31+
prod pusher deployment, an operator must read-only verify all of the following
32+
without reading Secret values:
33+
34+
1. Inspect the live `prod-omi-pusher` `REDIS_DB_HOST` env item's `valueFrom`
35+
object and the Helm release manifest to determine whether either still has
36+
the historical `secretKeyRef`.
37+
2. Check only key presence for `REDIS_DB_HOST` in
38+
`prod-omi-backend-config` and `REDIS_DB_PASSWORD` in
39+
`prod-omi-backend-secrets`; do not print their values.
40+
3. Render the repair revision and confirm the host is
41+
`configMapKeyRef: prod-omi-backend-config/REDIS_DB_HOST`, the legacy
42+
`secretKeyRef` is null, and the password is
43+
`secretKeyRef: prod-omi-backend-secrets/REDIS_DB_PASSWORD`.
44+
4. Confirm the live Deployment still uses the normal rolling-update strategy
45+
before using the ordinary Helm upgrade. Do not use `--force` or a manual
46+
patch as a transition workaround.
47+
48+
The regression fixture in
49+
`backend/tests/unit/test_verify_pusher_config_references.py` executes the
50+
historical named-env strategic merge locally: the unguarded manifest produces
51+
both sources, while this repair leaves only the ConfigMap source.

backend/tests/unit/test_verify_pusher_config_references.py

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,13 @@
44

55
from pathlib import Path
66
import runpy
7+
import shutil
8+
import subprocess
9+
import textwrap
710
from types import SimpleNamespace
811

912
import pytest
13+
import yaml
1014

1115
SCRIPT = Path(__file__).resolve().parents[2] / "scripts" / "verify_pusher_config_references.py"
1216

@@ -65,6 +69,111 @@ def test_rendered_pusher_values_reference_configured_redis_host_key(
6569
assert expected in preflight.pusher_references(environment)
6670

6771

72+
def test_rendered_dev_pusher_redis_host_clears_legacy_secret_source(preflight: SimpleNamespace):
73+
environment = "dev"
74+
deployment = next(document for document in preflight.render(environment) if document.get("kind") == "Deployment")
75+
env = deployment["spec"]["template"]["spec"]["containers"][0]["env"]
76+
redis_host = next(item for item in env if item["name"] == "REDIS_DB_HOST")
77+
redis_password = next(item for item in env if item["name"] == "REDIS_DB_PASSWORD")
78+
79+
assert redis_host["valueFrom"] == {
80+
"configMapKeyRef": {"name": f"{environment}-omi-backend-config", "key": "REDIS_DB_HOST"},
81+
"secretKeyRef": None,
82+
}
83+
assert redis_password["valueFrom"] == {
84+
"secretKeyRef": {"name": f"{environment}-omi-backend-secrets", "key": "REDIS_DB_PASSWORD"}
85+
}
86+
87+
88+
@pytest.mark.skipif(shutil.which("kubectl") is None, reason="kubectl is required for the local strategic-merge fixture")
89+
def test_historical_secret_redis_host_upgrade_uses_kubernetes_strategic_merge(
90+
tmp_path: Path, preflight: SimpleNamespace
91+
):
92+
"""Exercise Kubernetes' named-env strategic merge behavior without a cluster.
93+
94+
Helm emits the new REDIS_DB_HOST item for the release update. The fixture
95+
starts with the historical live Secret source and applies that item through
96+
Kustomize's Kubernetes strategic-merge implementation. Without an explicit
97+
null, the nested valueFrom map retains the Secret source and matches the
98+
failed live validation. The explicit null removes it while retaining the
99+
ConfigMap source.
100+
"""
101+
102+
base = tmp_path / "base"
103+
base.mkdir()
104+
(base / "kustomization.yaml").write_text("resources:\n - deployment.yaml\n")
105+
(base / "deployment.yaml").write_text(textwrap.dedent("""\
106+
apiVersion: apps/v1
107+
kind: Deployment
108+
metadata:
109+
name: pusher
110+
spec:
111+
selector:
112+
matchLabels:
113+
app: pusher
114+
template:
115+
metadata:
116+
labels:
117+
app: pusher
118+
spec:
119+
containers:
120+
- name: pusher
121+
image: example/pusher
122+
env:
123+
- name: REDIS_DB_HOST
124+
valueFrom:
125+
secretKeyRef:
126+
name: dev-omi-backend-secrets
127+
key: REDIS_DB_HOST
128+
"""))
129+
130+
def render(value_from: str) -> dict:
131+
overlay = tmp_path / f"overlay-{len(list(tmp_path.glob('overlay-*')))}"
132+
overlay.mkdir()
133+
strategic_patch = textwrap.dedent("""\
134+
apiVersion: apps/v1
135+
kind: Deployment
136+
metadata:
137+
name: pusher
138+
spec:
139+
template:
140+
spec:
141+
containers:
142+
- name: pusher
143+
env:
144+
- name: REDIS_DB_HOST
145+
valueFrom:
146+
""")
147+
strategic_patch += textwrap.indent(value_from, " " * 16)
148+
kustomization = textwrap.dedent("""\
149+
resources:
150+
- ../base
151+
patches:
152+
- target:
153+
kind: Deployment
154+
name: pusher
155+
patch: |-
156+
""")
157+
(overlay / "kustomization.yaml").write_text(kustomization + textwrap.indent(strategic_patch, " " * 6))
158+
result = subprocess.run(["kubectl", "kustomize", str(overlay)], check=True, capture_output=True, text=True)
159+
return yaml.safe_load(result.stdout)
160+
161+
broken = render("""\
162+
configMapKeyRef:
163+
name: dev-omi-backend-config
164+
key: REDIS_DB_HOST
165+
""")
166+
broken_value_from = broken["spec"]["template"]["spec"]["containers"][0]["env"][0]["valueFrom"]
167+
assert set(broken_value_from) == {"configMapKeyRef", "secretKeyRef"}
168+
169+
rendered_deployment = next(document for document in preflight.render("dev") if document.get("kind") == "Deployment")
170+
rendered_env = rendered_deployment["spec"]["template"]["spec"]["containers"][0]["env"]
171+
rendered_redis_host = next(item for item in rendered_env if item["name"] == "REDIS_DB_HOST")
172+
fixed = render(yaml.safe_dump(rendered_redis_host["valueFrom"], sort_keys=False))
173+
fixed_value_from = fixed["spec"]["template"]["spec"]["containers"][0]["env"][0]["valueFrom"]
174+
assert fixed_value_from == {"configMapKeyRef": {"name": "dev-omi-backend-config", "key": "REDIS_DB_HOST"}}
175+
176+
68177
@pytest.mark.parametrize("kind", ["configmap", "secret"])
69178
def test_rejects_missing_referenced_key_without_reading_values(monkeypatch, preflight: SimpleNamespace, kind: str):
70179
calls: list[list[str]] = []

0 commit comments

Comments
 (0)