Climate-REF / Climate-REF/climate-ref-aft

Provider Secrets never lose a removed key, because they are rendered via stringData

Open
#44 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
0
Forks
0
Avg merge
1d 4h
Merged PRs (30d)
14

Description

## Summary

`ref.providerSecret` renders provider environment into `Secret.stringData`. Helm can add and update keys there, but it can never remove one, so every key ever set on a provider stays in the Secret forever. A key dropped from `defaults.env` in a chart release is still injected into workers on existing installs.

This is currently breaking our deployment. Chart 0.5.0 and 0.5.1 shipped:

```yaml
defaults:
env:
CELERY_ACCEPT_CONTENT: |
["json", "pickle"]
```

0.5.2 removed it. On upgrade to 0.5.3 the key was still in every provider Secret that predated 0.5.2, and every worker crashlooped:

```
SerializerNotInstalled: No encoder/decoder installed for ["json"
```

Celery reads `accept_content` from the environment as a comma separated string, so `["json", "pickle"]` splits into `["json` and ` "pickle"]` and neither names a serialiser. The commented example in 0.5.3 documents the correct form as `"json,ref-json,pickle"`, so the 0.5.0 value was never valid for its own format.

## Cause

`helm/templates/_helpers.tpl`:

```gotemplate
{{- define "ref.providerSecret" -}}
apiVersion: v1
kind: Secret
metadata:
name: {{ include "ref.fullname" .root }}-{{ .provider }}
...
stringData:
{{- tpl (toYaml .spec.env) .root | nindent 2 }}
{{- end -}}
```

`stringData` is write only. The API server folds it into `data` and drops it from the stored object. So on upgrade Helm's three way merge sees the key gone from the rendered manifest and emits a patch setting `stringData.CELERY_ACCEPT_CONTENT` to null, against a live object where that field does not exist. The patch is a no op and `data.CELERY_ACCEPT_CONTENT` survives.

## Evidence

Helm's own v78 release record renders `ref-pmp` with 17 keys and no `CELERY_ACCEPT_CONTENT`. The live Secret has it, plus `CELERY_WORKER_MAX_TASKS_PER_CHILD` which we removed from our own values two commits earlier.

The split is exact. Two worker instances created new under 0.5.3 have clean Secrets and run. Every instance predating 0.5.2 carries the dead key and crashloops. Same chart, same release, same reconcile.

| secret | stale keys | pods |
|---|---|---|
| `ref-esmvaltool` | both | 0/5 crashlooping |
| `ref-pmp` | both | 3/26 crashlooping |
| `ref-ilamb` | both | 0/1 crashlooping |
| `ref-orchestrator` | `CELERY_ACCEPT_CONTENT` | running |
| `ref-esmvaltool-small` | none | 1/1 healthy |
| `ref-ilamb-small` | none | 1/1 healthy |

## Suggested fix

Render `data` with explicit base64, so the removal patch targets a field that exists:

```gotemplate
data:
{{- range $k, $v := .spec.env }}
{{ $k }}: {{ tpl (toString $v) $.root | b64enc | quote }}
{{- end }}
```

`tpl` still runs, so `{{ include "ref.brokerUrl" . }}` keeps working. `toString` also makes the coercion explicit, which is what would have caught the block scalar in 0.5.0.

Two related points:

- Until this lands, any release that drops a key from `defaults.env` needs an upgrade note telling operators to delete the provider Secrets so Helm recreates them.
- Worth checking whether anything else still sets `CELERY_ACCEPT_CONTENT` in the list form.

## Workaround

```
kubectl -n delete secret ref-orchestrator ref-esmvaltool ref-pmp ref-ilamb
kubectl -n rollout restart deploy/ref-orchestrator deploy/ref-esmvaltool deploy/ref-pmp deploy/ref-ilamb
```

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in helm/templates/_helpers.tpl at the ref.providerSecret definition and inspect how provider environment keys are rendered into the Secret. Verify the rendered Secret uses fields that let Helm remove keys absent from .spec.env, preserves tpl evaluation, and coerces values as described; confirm upgrades remove CELERY_ACCEPT_CONTENT and other dropped keys without requiring Secret deletion.

Written by the indexing model from the issue text.

Assessment

Tech stack
helm, kubernetes
Domain
devops, infrastructure
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
74/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.