Commit aee7c8c
authored
fix(helm): support omitting image.registry in grafana-mcp chart (#2125)
## What
Make the grafana-mcp chart render a valid image reference when
`image.registry` is left empty, so the registry can be omitted (e.g. to
pull from a mirror or let the runtime resolve the default registry)
without producing a leading-slash, unpullable reference.
## Why
The image is built inline in `templates/deployment.yaml`:
```gotemplate
image: "{{ .Values.image.registry }}/{{ .Values.image.repository }}:{{ .Values.image.tag }}"
```
The `/` between registry and repository is written unconditionally. If
`image.registry` is set to `""`, the result has a leading slash and is
not a valid image reference, so the pod enters `ImagePullBackOff`:
```
/grafana:latest # invalid reference
```
There is currently no way to omit the registry and let the container
runtime resolve the repository against its default registry / a
configured mirror.
Omitting the registry is a configuration users legitimately need:
- **Air-gapped / mirrored environments.** Registry mirrors are
configured at the container runtime, which rewrites the host — so chart
values should carry only `repository:tag`.
- **Registry rewriting.** Admission controllers (e.g. Kyverno) prepend
an internal registry, so users leave `registry: ""`.
- **Consistency.** Other charts let you omit the registry; grafana-mcp
silently breaks with `ImagePullBackOff`, which reads like a bug.
> Note: the default `registry: mcp` is intentional and unchanged —
`mcp/grafana` resolves to `docker.io/mcp/grafana` (Docker Hub's `mcp`
namespace). This PR only makes an *empty* registry valid; the default
path still renders `mcp/grafana:latest`.
> This is the same 2-segment `registry/repository:tag` leading-slash
shape as the querydoc chart (#2124), and distinct from the
bundled-PostgreSQL `registry//name:tag` double-slash bug (#2120), which
uses a 3-segment shape. All fixed independently.
## Fix
Introduce a `grafana-mcp.image` helper (the chart currently has none)
that builds the path from non-empty segments via `compact` + `join`, and
call it from the deployment:
```gotemplate
{{- define "grafana-mcp.image" -}}
{{- $img := .Values.image -}}
{{- $parts := compact (list $img.registry $img.repository) -}}
{{- printf "%s:%s" (join "/" $parts) $img.tag -}}
{{- end -}}
```
```gotemplate
# templates/deployment.yaml
image: "{{ include "grafana-mcp.image" . }}"
```
Rendered results:
| registry | repository | tag | result |
|---|---|---|---|
| `mcp` | `grafana` | `latest` | `mcp/grafana:latest` (unchanged) |
| `""` | `grafana` | `latest` | `grafana:latest` (fixed) |
## Tests
Added two cases to `helm/tools/grafana-mcp/tests/deployment_test.yaml`
(developed test-first): a default-image regression guard (the suite had
no image assertion), and the empty-registry path.
```yaml
- it: should render the default container image
template: deployment.yaml
asserts:
- equal:
path: spec.template.spec.containers[0].image
value: mcp/grafana:latest
- it: should omit empty registry segment in image
template: deployment.yaml
set:
image:
registry: ""
asserts:
- equal:
path: spec.template.spec.containers[0].image
value: grafana:latest
- notMatchRegex:
path: spec.template.spec.containers[0].image
pattern: "^/"
```
The empty-registry test fails without this change (`/grafana:latest`)
and passes with it — revert the `deployment.yaml`/`_helpers.tpl` hunks
to reproduce.
## How to verify
```bash
make helm-tools # generate grafana-mcp Chart.yaml
helm unittest helm/tools/grafana-mcp # full suite green (incl. new cases)
helm lint helm/tools/grafana-mcp
# test-independent reproduction:
helm template t helm/tools/grafana-mcp --set image.registry="" | grep image:
# before: /grafana:latest
# after: grafana:latest
```
## Scope
Limited to the grafana-mcp chart's image construction. Empty
`repository` is not made valid here — omitting it leaves no image to
pull. This PR only makes `image.registry` optional.
## Checklist
- [x] Commit is DCO signed-off (`git commit -s`).
- [x] `helm unittest helm/tools/grafana-mcp` green; `helm lint
helm/tools/grafana-mcp` clean.
- [x] Default image still renders `mcp/grafana:latest` (regression
guard).
- [x] Diff limited to `templates/_helpers.tpl`,
`templates/deployment.yaml`, and `tests/deployment_test.yaml` (generated
chart files excluded).
- [x] Test fails without the fix, passes with it.
Signed-off-by: Mike Spinks <mikespinks@gmail.com>1 parent 841e807 commit aee7c8c
3 files changed
Lines changed: 30 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
67 | 67 | | |
68 | 68 | | |
69 | 69 | | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
35 | 35 | | |
36 | 36 | | |
37 | 37 | | |
38 | | - | |
| 38 | + | |
39 | 39 | | |
40 | 40 | | |
41 | 41 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
100 | 100 | | |
101 | 101 | | |
102 | 102 | | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
0 commit comments