feat: make operator and data-plane image registry configurable - #464
WentingWu666666 wants to merge 2 commits into
Conversation
Introduce a single registry-prefix model so the same operator binary and
Helm chart can be repointed at a different registry (public upstream, or a
private/mirrored registry) without a code change.
Helm:
- Add image.registry prefix and relative per-component repositories.
- Add documentdb.imageRef helper implementing the canonical Docker/
containerd host-detection rule: a repository whose first path segment
looks like a host (contains '.' or ':', or equals localhost) is used
verbatim and the registry prefix is ignored; otherwise the prefix is
prepended. Empty tag yields a bare repository.
- Nest gateway/documentdb pull policies under image.{gateway,documentdb}
and set explicit IfNotPresent defaults; add image.otelCollector and
image.postgres seams (default to the operator's built-in images).
- Pass data-plane image repositories and optional otel/postgres refs to
the operator via environment variables.
Operator:
- Add env-backed image-source getters (ExtensionImageRepo, GatewayImageRepo,
OtelCollectorImage, PostgresImage) with compiled-in fallbacks.
- Extract DEFAULT_DOCUMENTDB_TAG and simplify ResolveComponentImage to
compose repo:tag from a single default tag, so a registry override flows
through to the default image as well.
Default rendering is byte-identical to the previous ghcr.io references, so
existing deployments are unaffected.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: e346b0ad-0a96-4275-ad25-875de64e4b21
Signed-off-by: Wenting Wu <wentingwu@microsoft.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
A critical release-workflow incompatibility and unresolved image-reference, registry, and pull-policy compatibility issues remain.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (3)
What changed in this PR
This PR adds configurable registry and image-source handling for the operator and data-plane images through Helm values and operator environment variables.
Changes:
- Adds host-aware registry and repository composition.
- Adds configurable data-plane, OTel, and PostgreSQL image sources.
- Updates image resolution, pull policies, and Helm test coverage.
| File | Summary |
|---|---|
operator/src/internal/utils/util.go |
Image-source getters and resolution logic; design documentation is now stale. |
operator/src/internal/utils/image_source_test.go |
Tests image-source defaults and overrides. |
operator/src/internal/utils/constants.go |
Adds image constants and default tag; release workflow parsing must be updated. |
operator/src/internal/product/profile.go |
Uses a default-tag model for image resolution. |
operator/src/internal/product/profile_test.go |
Updates profile assertions. |
operator/src/internal/product/documentdb.go |
Applies configurable image and PostgreSQL resolution. |
operator/src/internal/product/documentdb_test.go |
Tests image overrides and resolution. |
operator/src/internal/cnpg/cnpg_cluster.go |
Uses the configurable OTel image. |
operator/documentdb-helm-chart/values.yaml |
Defines registry, repository, and pull-policy settings; compatibility and single-registry behavior need attention. |
operator/documentdb-helm-chart/tests/09_operator_deployment_test.yaml |
Adds image configuration and pull-policy coverage. |
operator/documentdb-helm-chart/templates/09_documentdb_operator.yaml |
Wires operator images and environment variables; full data-plane references and legacy pull-policy values need handling. |
operator/documentdb-helm-chart/templates/03_documentdb_wal_replica.yaml |
Uses registry-aware image rendering. |
operator/documentdb-helm-chart/templates/02_documentdb_sidecar_injector.yaml |
Uses registry-aware image rendering. |
operator/documentdb-helm-chart/templates/_helpers.tpl |
Implements canonical image-reference composition. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| // DEFAULT_DOCUMENTDB_IMAGE is the extension image used in ImageVolume mode. | ||
| DEFAULT_DOCUMENTDB_IMAGE = DOCUMENTDB_EXTENSION_IMAGE_REPO + ":0.117.0" | ||
| DEFAULT_DOCUMENTDB_IMAGE = DOCUMENTDB_EXTENSION_IMAGE_REPO + ":" + DEFAULT_DOCUMENTDB_TAG |
There was a problem hiding this comment.
Fixed in ef9c7b4. The release workflow now reads and rewrites DEFAULT_DOCUMENTDB_TAG (the single remaining version literal) instead of the DEFAULT_DOCUMENTDB_IMAGE line, which is now composed from repo + tag. Both the detect step (regex) and the update step (sed) were updated; verified the detection returns 0.117.0 and the bump rewrites the tag const.
| {{- if .Values.image.documentdb.repository }} | ||
| - name: DOCUMENTDB_EXTENSION_IMAGE_REPO | ||
| value: "{{ include "documentdb.imageRef" (dict "registry" .Values.image.registry "repo" .Values.image.documentdb.repository "tag" "") }}" | ||
| {{- end }} | ||
| {{- if .Values.image.gateway.repository }} | ||
| - name: GATEWAY_IMAGE_REPO | ||
| value: "{{ include "documentdb.imageRef" (dict "registry" .Values.image.registry "repo" .Values.image.gateway.repository "tag" "") }}" |
There was a problem hiding this comment.
Fixed in ef9c7b4, taking a stricter approach than composing a second path. Since a repository is defined as host/path only (the tag comes from the component tag field, or documentDbVersion for data-plane images), the documentdb.imageRef helper now rejects any repository whose final path segment already carries a tag (:) or digest (@) with an actionable error. All images (control-plane, data-plane repo env, otel, postgres) flow through this one helper, so this closes the double-tag hole everywhere without adding priority rules. Host ports like localhost:5000/repo are unaffected (only the last segment is inspected). Added helm-unittest cases for the embedded-tag and embedded-digest rejections.
| pullPolicy: IfNotPresent | ||
| gateway: | ||
| repository: documentdb/documentdb-kubernetes-operator/gateway | ||
| pullPolicy: IfNotPresent |
There was a problem hiding this comment.
Acknowledged. Since the operator is in public preview, we're accepting this as a deliberate (preview-only) breaking change rather than carrying dual keys. To avoid it being silent, it's now documented under a new "Breaking Changes" entry in CHANGELOG (ef9c7b4): gatewayImagePullPolicy / documentDbImagePullPolicy → image.gateway.pullPolicy / image.documentdb.pullPolicy, with migration guidance.
…rd, changelog
- release_documentdb_images.yml: detect and bump DEFAULT_DOCUMENTDB_TAG
instead of the now-composed DEFAULT_DOCUMENTDB_IMAGE literal, so the
automated database-image version bump keeps working after the tag
constant was factored out.
- documentdb.imageRef helper: reject a repository that already carries a
tag or digest (':' or '@' in the final path segment) with an actionable
error, so an embedded tag can no longer produce a double-tagged image
reference on any component. Repositories are host/path only.
- CHANGELOG: document the configurable image registry feature and the
breaking rename of the pull-policy values (gatewayImagePullPolicy /
documentDbImagePullPolicy -> image.{gateway,documentdb}.pullPolicy).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: e346b0ad-0a96-4275-ad25-875de64e4b21
Signed-off-by: Wenting Wu <wentingwu@microsoft.com>
| value: "{{ .Values.image.gateway.pullPolicy }}" | ||
| {{- end }} | ||
| {{- if .Values.documentDbImagePullPolicy }} | ||
| {{- if .Values.image.documentdb.pullPolicy }} |
There was a problem hiding this comment.
An invalid pull policy such as Sometimes renders successfully. Both consumers then silently fall back to their defaults, so the deployment does not use the policy the user requested. Can Helm reject values other than Always, IfNotPresent, and Never with a clear error?
| {{- $first := (splitList "/" .repo) | first -}} | ||
| {{- $full := .repo -}} | ||
| {{- if not (or (contains "." $first) (contains ":" $first) (eq $first "localhost")) -}} | ||
| {{- $full = printf "%s/%s" .registry .repo -}} |
There was a problem hiding this comment.
An empty image.registry or operator repository renders an invalid image such as /documentdb/...:0.3.0 or ghcr.io/:0.3.0. Helm reports success and the failure appears later at pod startup. Can we fail rendering with the name of the empty setting?
documentdb-triage-tool
Bot
added
CI/CD
documentation
|
🤖 Auto-triaged by documentdb-triage-tool. Applied: Reasoningcomponent from path globs (controllers, test, ci, docs); effort from diff stats (436+55 LOC, 16 files); LLM: Multi-file change spanning Helm chart values/helpers, operator env-var plumbing, and new Helm unit tests to enable configurable image registries for air-gapped/mirrored deployments. If a label is wrong, remove it manually and ping |
What
Make the operator and data-plane container images registry-configurable so the same operator binary and Helm chart can be repointed at a different registry (a public registry upstream, or a private/mirrored registry) without a code change. Default rendering is byte-identical to today's
ghcr.io/...references, so existing deployments are unaffected.Why
Downstream consumers (private mirrors, air-gapped/regulated environments) need to source every image from a single registry they control, without forking the chart or rebuilding the operator.
How
Single prefix + relative repository + verbatim clause.
values.yamlgainsimage.registry(defaultghcr.io) and relative per-component repositories.documentdb.imageRefHelm helper implements the canonical Docker/containerd host-detection rule: if a repository's first path segment looks like a host (contains.or:, or equalslocalhost) it is used verbatim and the registry prefix is ignored; otherwise the prefix is prepended. An empty tag yields a bare repository. This lets you either re-home everything via oneimage.registry, or override any single component with a full ref.image.{gateway,documentdb}.pullPolicywith explicitIfNotPresentdefaults; addedimage.otelCollector/image.postgresseams (default to the operator's built-in images; postgres empty defers to CloudNativePG).Operator side:
ExtensionImageRepo,GatewayImageRepo,OtelCollectorImage,PostgresImage) with compiled-in fallbacks.DEFAULT_DOCUMENTDB_TAGand simplifiedResolveComponentImageto composerepo:tagfrom a single default tag, so a registry override flows through to the default image as well.Testing
go build ./...,go vet,gofmt, fullgo test ./...— all pass.helm lintclean,helm unittest98/98 (added registry-prefix, full-ref verbatim,localhost/localhost:portverbatim, data-plane env, and pull-policy default/empty cases).helm templateverified: defaults byte-identical to upstreamghcr.io/...:0.3.0; a registry override re-homes all images; per-component full refs are used verbatim.