[helm] Enable pulling from private Docker registry#2692
[helm] Enable pulling from private Docker registry#2692xx789633 merged 2 commits intoapache:mainfrom
Conversation
affo
force-pushed
the
la-pull-secrets
branch
from
4ac5e0d to
8cd11e5
Compare
helm/templates/_helpers.tpl
Outdated
| Image name | ||
| */}} | ||
| {{- define "fluss.image" -}} | ||
| {{- $image := printf "%s:%s" .Values.image.repository .Values.image.tag }} |
There was a problem hiding this comment.
If a user omits image.tag or sets it to empty, this would produce an invalid image reference like apache/fluss:. Maybe we need to add a fallback here.
helm/templates/_helpers.tpl
Outdated
| Image name | ||
| */}} | ||
| {{- define "fluss.image" -}} | ||
| {{- $image := printf "%s:%s" .Values.image.repository .Values.image.tag }} |
There was a problem hiding this comment.
If a user provides a purely numeric tag (e.g. tag: 1.0), printf "%s" in Helm may output apache/fluss:%!s(float64=1).
There was a problem hiding this comment.
what a catch! Partially addressed 🤝
affo
force-pushed
the
la-pull-secrets
branch
from
8cd11e5 to
8020252
Compare
|
@xx789633 the latest changes now address your doubts 🤝
For tag formatting, unfortunately, it is not possible to discriminate Wrapping the tag in quotes in YAML is left to the user 👍 I checked this behavior and it would be the same in Bitnami's charts. |
|
Looks good to me. |
|
LGTM overall. One more suggestion: Could you add documentation in |
Added instructions for using a private Docker registry and included image values reference.
|
I have appended a commit to document the private docker registry. Merging.... |
|
@xx789633 thanks for promptly acting on this one! |
4 participants
Purpose
Linked issue: close #2691
Brief change log
images.registryimage.pullSecretsMakes it possible to use an alternate registry and use pull secrets to pull the image if the registry is private.
I favored the approach of adding pull secrets directly to STS and not in the service account for bigger flexibility as the user can still link another service account to pods while pull secrets are attached to pods.
Tests
No test introduced.
Would be good to add tests once this PR lands (introduces
helm unittest+ github workflow).API and Format
NO
Documentation
No new feature, it is already documented in the README.