Skip to content

fix(GPP-burgerportaal): honour 0 for sitemapCacheDurationHours and downloadTimeoutMinutes - #71

Open
xiduzo wants to merge 3 commits into
mainfrom
fix/render-zero-valued-settings
Open

fix(GPP-burgerportaal): honour 0 for sitemapCacheDurationHours and downloadTimeoutMinutes#71
xiduzo wants to merge 3 commits into
mainfrom
fix/render-zero-valued-settings

Conversation

@xiduzo

@xiduzo xiduzo commented Aug 11, 2026

Copy link
Copy Markdown

Wat is er aan de hand

charts/GPP-burgerportaal/templates/configmap.yaml rendert deze twee settings op truthiness:

{{- if .Values.settings.sitemapCacheDurationHours }}
SITEMAP_CACHE_DURATION_HOURS: {{ ... }}
{{- end }}

In Go templates is 0 falsy. Dus juist de waarde die je op een testomgeving wilt zetten — sitemapCacheDurationHours: 0 om de sitemap-outputcache uit te zetten — verdwijnt stilletjes uit de ConfigMap. De app valt dan terug op zijn eigen default van 23 uur.

Hetzelfde geldt voor downloadTimeoutMinutes.

Hoe dit zich uitte

In de e2e-pipeline (kind + deze charts) bouwde de burgerportaal de sitemap één keer, vroeg in de run terwijl de stack nog leeg was, en serveerde die lege XML de rest van de suite uit de cache:

45 x "Serving response from cache."
 1 x "Sending HTTP request"      <- één enkele ODRC-call in de hele run

Gevolg: elk scenario dat een document seedt en daarna in de sitemap opzoekt viel om tegen een sitemap met 0 entries, terwijl ODRC 20-40 documenten als gepubliceerd + isGereedVoorPublicatie rapporteerde. Dat leest als een propagatie- of timingbug in de tests, en dat is het niet.

De fix

Renderen op "is de waarde gezet?" in plaats van op truthiness:

{{- if not (kindIs "invalid" .Values.settings.sitemapCacheDurationHours) }}

Geverifieerd met helm template:

waarde voor na
sitemapCacheDurationHours: 0 env var afwezig SITEMAP_CACHE_DURATION_HOURS: "0"
default (23) "23" "23"
expliciet null afwezig afwezig

helm lint charts/GPP-burgerportaal is schoon.

Chart version

gpp-burgerportaal gaat naar 2.0.1 — de release-workflow publiceert een chart alleen als de versie wijzigt, dus zonder bump bereikt de fix niemand.

Follow-up (bewust niet in deze PR)

charts/GPP-stack/Chart.yaml pint gpp-burgerportaal: 2.0.0, dus consumers van de umbrella krijgen deze fix nog niet. Ik heb die bump er niet in gelaten: chart-releaser resolvet de dependencies van gpp-stack uit de gepubliceerde index, en 2.0.1 bestaat daar pas nadat deze merge zelf gepubliceerd is — een bump in dezelfde PR kan de release-run dus laten falen. Beter een losse bump nadat 2.0.1 er staat.

🤖 Generated with Claude Code

xiduzo and others added 3 commits August 11, 2026 13:24
…imeout

`{{- if .Values.settings.sitemapCacheDurationHours }}` renders on truthiness, and
0 is falsy in Go templates — so the one value a testomgeving most wants to set,
`sitemapCacheDurationHours: 0` to disable the sitemap output cache, was silently
dropped from the ConfigMap. The app then fell back to its own 23h default.

Effect in the e2e pipeline: the burgerportaal built the sitemap once, early in the
run while the stack was still empty, and served that empty XML from its output
cache for the rest of the suite — 45 sitemap requests, 45 "Serving response from
cache", exactly one outgoing ODRC call. Every seed-then-look-up scenario then
failed against a sitemap holding 0 entries while ODRC reported 20-40 documents as
gepubliceerd + isGereedVoorPublicatie, which read like a propagation bug and is
not one.

Render on "is it set?" instead. Same fix for downloadTimeoutMinutes, which has the
identical flaw. Verified with `helm template ... -f dev/values/odbp.yaml`:
SITEMAP_CACHE_DURATION_HOURS is absent before and "0" after.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The release workflow publishes a chart only when its version changes, so the
ConfigMap fix above needs a patch bump to reach anyone.
@xiduzo
xiduzo requested a review from felixcicatt August 11, 2026 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant