Skip to content

Commit 95deede

Browse files
authored
egress: stop Helm clobbering serge's runtime allowlist additions (#53)
## Problem The `serge-egress` tinyproxy filter is a single `.data.filter` string written by **two** parties: - **Helm** seeds the base allowlist (`taskExecution.kubernetes.egress.allowDomains`: GitHub + the HF LLM router). - **serge** patches the same ConfigMap at runtime to add configured LLM provider hosts — `reviewbot/k8s_sandbox.py:sync_egress_allowlist`, which is additive and non-destructive (reads the current filter, appends only missing hosts). Under **server-side apply** the two managers can't share ownership of one atomic string. The next `helm upgrade` re-applies the base-only value onto serge's augmented value and fails: ``` UPGRADE FAILED: conflict occurred while applying object serge/serge-egress /v1, Kind=ConfigMap: Apply failed with 1 conflict: conflict with "OpenAPI-Generator" using v1: .data.filter ``` This blocks **every** serge deploy once serge has added a provider host live (e.g. `api.anthropic.com`, `api.openai.com`). ## Fix (chart-only) Render `.data.filter` as the **union of the live filter (read via `lookup`) and the declared base domains**: - Helm still **guarantees the base allowlist is always present** (appends any base domain that's missing). - Helm **re-emits serge's runtime additions verbatim**, so its applied value equals the live value in steady state → **no SSA conflict**. - No serge code change, no image rebuild. ### Caveat `lookup` returns nothing under `helm template` / `--dry-run` (no cluster read), so a **dry-run renders only the base domains**. The real `helm upgrade` reads the live filter and preserves serge's additions. This is documented in a comment in the template. ## Testing - `helm template … --show-only templates/egress-proxy.yaml` → renders the base 3 domains, no errors (expected: `lookup` empty under `template`).
1 parent bb3d598 commit 95deede

1 file changed

Lines changed: 28 additions & 0 deletions

File tree

deploy/helm/templates/egress-proxy.yaml

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,35 @@ data:
4242
PidFile "/tmp/tinyproxy.pid"
4343
LogLevel Info
4444
filter: |
45+
{{- /*
46+
Helm's job here is only to GUARANTEE the base allowlist (allowDomains) is
47+
always present. serge itself patches this same ConfigMap at runtime to add
48+
configured LLM provider hosts (reviewbot/k8s_sandbox.py:sync_egress_allowlist),
49+
additively and at any time. Because .data.filter is a single string, Helm
50+
and serge would otherwise fight over its ownership under server-side apply:
51+
Helm re-applying the base-only value onto serge's augmented value conflicts.
52+
53+
So on upgrade we read the LIVE filter via `lookup`, re-emit it verbatim, and
54+
union the base domains in (appending any that are missing). This preserves
55+
serge's runtime additions and makes Helm's applied value equal the live
56+
value in steady state -> no SSA conflict.
57+
58+
NOTE: `lookup` returns nothing under `helm template` / `--dry-run` (no cluster
59+
read), so a dry-run renders ONLY the base domains. The real `helm upgrade`
60+
reads the live filter and preserves serge's additions.
61+
*/}}
62+
{{- $out := list }}
63+
{{- $existing := lookup "v1" "ConfigMap" .Release.Namespace $name }}
64+
{{- if $existing }}
65+
{{- range (splitList "\n" (get ($existing.data | default dict) "filter" | default "")) }}
66+
{{- $line := trim . }}
67+
{{- if and $line (not (has $line $out)) }}{{ $out = append $out $line }}{{ end }}
68+
{{- end }}
69+
{{- end }}
4570
{{- range $egress.allowDomains }}
71+
{{- if not (has . $out) }}{{ $out = append $out . }}{{ end }}
72+
{{- end }}
73+
{{- range $out }}
4674
{{ . }}
4775
{{- end }}
4876
---

0 commit comments

Comments
 (0)