Commit 13fb436
authored
fix(helm): support omitting image.registry in querydoc chart (kagent-dev#2124)
## What
Make the querydoc (doc2vec) chart render a valid image reference when
`image.registry` is left empty, so the registry can be omitted (e.g. to
pull from Docker Hub or a mirror) 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 | default .Chart.AppVersion }}"
```
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`:
```
/kagent-dev/doc2vec/mcp:1.1.10 # 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:
- **Docker Hub.** A bare `org/image` reference resolves to Docker Hub,
so users mirroring the image there want `kagent-dev/doc2vec/mcp:tag`,
not `ghcr.io/...`.
- **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; querydoc
silently breaks with `ImagePullBackOff`, which reads like a bug.
> Note: this is a different issue from the bundled-PostgreSQL
`registry//name:tag`
> double-slash bug (kagent-dev#2120). That helper uses a 3-segment
`registry/repository/name:tag`
> shape; querydoc uses a 2-segment `registry/repository:tag` shape with
no separate
> image name, so the failure here is a leading slash from an empty
registry, not a
> double slash from an empty repository. They are fixed independently.
## Fix
Introduce a `querydoc.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 "querydoc.image" -}}
{{- $img := .Values.image -}}
{{- $parts := compact (list $img.registry $img.repository) -}}
{{- printf "%s:%s" (join "/" $parts) ($img.tag | default .Chart.AppVersion) -}}
{{- end -}}
```
```gotemplate
# templates/deployment.yaml
image: "{{ include "querydoc.image" . }}"
```
Rendered results:
| registry | repository | tag | result |
|---|---|---|---|
| `ghcr.io` | `kagent-dev/doc2vec/mcp` | `1.1.10` |
`ghcr.io/kagent-dev/doc2vec/mcp:1.1.10` (unchanged) |
| `ghcr.io` | `kagent-dev/doc2vec/mcp` | `v2.0.0` |
`ghcr.io/kagent-dev/doc2vec/mcp:v2.0.0` (unchanged) |
| `""` | `kagent-dev/doc2vec/mcp` | `1.1.10` |
`kagent-dev/doc2vec/mcp:1.1.10` (fixed) |
## Tests
Added one case to `helm/tools/querydoc/tests/deployment_test.yaml` for
the empty-registry path (developed test-first). The existing "should
have correct container image" and "should use custom image tag when set"
tests already guard the default and custom-tag paths, so no regression
test was duplicated.
```yaml
- it: should omit empty registry segment in image
template: deployment.yaml
set:
image:
registry: ""
asserts:
- equal:
path: spec.template.spec.containers[0].image
value: kagent-dev/doc2vec/mcp:1.1.10
- notMatchRegex:
path: spec.template.spec.containers[0].image
pattern: "^/"
```
The test fails without this change (`/kagent-dev/doc2vec/mcp:1.1.10`)
and passes with it — revert the `deployment.yaml`/`_helpers.tpl` hunks
to reproduce.
## How to verify
```bash
make helm-tools # generate querydoc Chart.yaml
helm unittest helm/tools/querydoc # full suite green (incl. new case)
helm lint helm/tools/querydoc
# test-independent reproduction:
helm template t helm/tools/querydoc --set image.registry="" | grep image:
# before: /kagent-dev/doc2vec/mcp:1.1.10
# after: kagent-dev/doc2vec/mcp:1.1.10
```
## Scope
Limited to the querydoc chart's image construction. Empty `repository`
is not made valid here, because the repository carries the full image
path and there is no separate image-name segment — 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/querydoc` green; `helm lint
helm/tools/querydoc` clean.
- [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 aee7c8c commit 13fb436
3 files changed
Lines changed: 23 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 | |
|---|---|---|---|
| |||
39 | 39 | | |
40 | 40 | | |
41 | 41 | | |
42 | | - | |
| 42 | + | |
43 | 43 | | |
44 | 44 | | |
45 | 45 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
47 | 47 | | |
48 | 48 | | |
49 | 49 | | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
50 | 63 | | |
51 | 64 | | |
52 | 65 | | |
| |||
0 commit comments