Re: [PR] feat(ingress-controller): add imagePullSecrets support with global fallback [apisix-helm-chart]
ljiljanakozic commented on PR #916: URL: https://github.com/apache/apisix-helm-chart/pull/916#issuecomment-4565012902 Any plans regarding new version of helm chart that includes this request? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] feat(ingress-controller): add imagePullSecrets support with global fallback [apisix-helm-chart]
Hathoute commented on PR #916: URL: https://github.com/apache/apisix-helm-chart/pull/916#issuecomment-4242513125 @shreemaan-abhishek can we have a new release of ingress-controller (to include this) ? thanks -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] feat(ingress-controller): add imagePullSecrets support with global fallback [apisix-helm-chart]
Baoyuantop merged PR #916: URL: https://github.com/apache/apisix-helm-chart/pull/916 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] feat(ingress-controller): add imagePullSecrets support with global fallback [apisix-helm-chart]
ArnaudCHT commented on code in PR #916:
URL: https://github.com/apache/apisix-helm-chart/pull/916#discussion_r2982269836
##
charts/apisix-ingress-controller/values.yaml:
##
@@ -52,6 +52,7 @@ deployment:
nodeSelector: {}
tolerations: []
affinity: {}
+ imagePullSecrets: []
Review Comment:
Hello @Baoyuantop,
Thanks for the review! I've updated the template to align with the main
apisix chart — `imagePullSecrets` now accepts a string array and renders using
`range` + `- name: {{ . }}`.
Here are the test cases:
```bash
# Test with global.imagePullSecrets
helm template . -s templates/deployment.yaml \
--set 'global.imagePullSecrets[0]=global-registry-secret'
# Test with deployment.imagePullSecrets (local override)
helm template . -s templates/deployment.yaml \
--set 'deployment.imagePullSecrets[0]=my-registry-secret'
# Test priority: deployment takes precedence over global
helm template . -s templates/deployment.yaml \
--set 'global.imagePullSecrets[0]=global-registry-secret' \
--set 'deployment.imagePullSecrets[0]=my-registry-secret'
# Test standalone (no imagePullSecrets)
helm template . -s templates/deployment.yaml
```
All cases render correctly. Let me know if anything else needs to be
adjusted.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] feat(ingress-controller): add imagePullSecrets support with global fallback [apisix-helm-chart]
Baoyuantop commented on code in PR #916:
URL: https://github.com/apache/apisix-helm-chart/pull/916#discussion_r2981876238
##
charts/apisix-ingress-controller/values.yaml:
##
@@ -52,6 +52,7 @@ deployment:
nodeSelector: {}
tolerations: []
affinity: {}
+ imagePullSecrets: []
Review Comment:
The data format of `global.imagePullSecrets` in the main apisix chart is a
string array:
```
# charts/apisix/values.yaml
global:
imagePullSecrets: []
# - my-registry-secrets
# - other-registry-secrets
```
The rendering logic in the corresponding template is:
```
# charts/apisix/templates/deployment.yaml
{{- with .Values.global.imagePullSecrets }}
imagePullSecrets:
{{- range $.Values.global.imagePullSecrets }}
- name: {{ . }}
{{- end }}
{{- end }}
```
However, the expected format of `deployment.imagePullSecrets` in this PR is
an array of objects (Kubernetes native format):
```
# PR testing example
deployment:
imagePullSecrets:
- name: my-registry-secret
```
Risk: When used as a child chart (passed from the parent chart's
`global.imagePullSecrets`), if the parent chart's `global.imagePullSecrets`
uses the main apisix chart's string array format (e.g., `["my-secret"]`), this
PR's `toYaml` will render an invalid `imagePullSecrets` structure (a string
instead of a `{name: ...}` object), causing Deployment creation to fail.
It is recommended to maintain consistency with the main apisix chart, accept
string arrays, and render them in the template using range + - name: {{ . }}.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] feat(ingress-controller): add imagePullSecrets support with global fallback [apisix-helm-chart]
juzhiyuan commented on PR #916: URL: https://github.com/apache/apisix-helm-chart/pull/916#issuecomment-4109722821 cc @Baoyuantop -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] feat(ingress-controller): add imagePullSecrets support with global fallback [apisix-helm-chart]
martin-schulze-e2m commented on PR #916: URL: https://github.com/apache/apisix-helm-chart/pull/916#issuecomment-4109459073 @AlinsRan can you please give this another look? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] feat(ingress-controller): add imagePullSecrets support with global fallback [apisix-helm-chart]
martin-schulze-e2m commented on PR #916: URL: https://github.com/apache/apisix-helm-chart/pull/916#issuecomment-4038910255 @AlinsRan could you please approve the workflow run? Hopefully we don't need another round. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] feat(ingress-controller): add imagePullSecrets support with global fallback [apisix-helm-chart]
martin-schulze-e2m commented on code in PR #916:
URL: https://github.com/apache/apisix-helm-chart/pull/916#discussion_r2905851654
##
charts/apisix-ingress-controller/README.md:
##
@@ -133,7 +133,8 @@ The same for container level, you need to set:
| deployment.annotations | object | `{}` | Add annotations to Apache APISIX
ingress controller resource |
| deployment.image.pullPolicy | string | `"IfNotPresent"` | |
| deployment.image.repository | string | `"apache/apisix-ingress-controller"`
| |
-| deployment.image.tag | string | `"2.0.1"` | |
+| deployment.image.tag | string | `"2.0.0-rc5"` | |
Review Comment:
The CI tests failed due to mismatches between the committed README.md and
the helm-docs generated one. This change should fix the CI error:
```suggestion
| deployment.image.tag | string | `"2.0.1"` | |
```
Alternatively, run `helm-docs` locally and commit that @ArnaudCHT.
##
charts/apisix-ingress-controller/README.md:
##
@@ -133,7 +133,8 @@ The same for container level, you need to set:
| deployment.annotations | object | `{}` | Add annotations to Apache APISIX
ingress controller resource |
| deployment.image.pullPolicy | string | `"IfNotPresent"` | |
| deployment.image.repository | string | `"apache/apisix-ingress-controller"`
| |
-| deployment.image.tag | string | `"2.0.1"` | |
+| deployment.image.tag | string | `"2.0.0-rc5"` | |
Review Comment:
The CI tests failed due to mismatches between the committed README.md and
the helm-docs generated one. This change should fix the CI error:
```suggestion
| deployment.image.tag | string | `"2.0.1"` | |
```
Alternatively, you can run `helm-docs` locally and commit that @ArnaudCHT.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] feat(ingress-controller): add imagePullSecrets support with global fallback [apisix-helm-chart]
martin-schulze-e2m commented on code in PR #916:
URL: https://github.com/apache/apisix-helm-chart/pull/916#discussion_r2905851654
##
charts/apisix-ingress-controller/README.md:
##
@@ -133,7 +133,8 @@ The same for container level, you need to set:
| deployment.annotations | object | `{}` | Add annotations to Apache APISIX
ingress controller resource |
| deployment.image.pullPolicy | string | `"IfNotPresent"` | |
| deployment.image.repository | string | `"apache/apisix-ingress-controller"`
| |
-| deployment.image.tag | string | `"2.0.1"` | |
+| deployment.image.tag | string | `"2.0.0-rc5"` | |
Review Comment:
The CI tests failed due to mismatches between the committed README.md and
the helm-docs generated one. This change should fix the CI error:
```suggestion
| deployment.image.tag | string | `"2.0.1"` | |
```
Alternatively, run `helm-docs` locally @ArnaudCHT and commit that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] feat(ingress-controller): add imagePullSecrets support with global fallback [apisix-helm-chart]
martin-schulze-e2m commented on code in PR #916:
URL: https://github.com/apache/apisix-helm-chart/pull/916#discussion_r2905851654
##
charts/apisix-ingress-controller/README.md:
##
@@ -133,7 +133,8 @@ The same for container level, you need to set:
| deployment.annotations | object | `{}` | Add annotations to Apache APISIX
ingress controller resource |
| deployment.image.pullPolicy | string | `"IfNotPresent"` | |
| deployment.image.repository | string | `"apache/apisix-ingress-controller"`
| |
-| deployment.image.tag | string | `"2.0.1"` | |
+| deployment.image.tag | string | `"2.0.0-rc5"` | |
Review Comment:
The CI tests failed due to mismatches between the committed README.md and
the helm-docs generated one
```suggestion
| deployment.image.tag | string | `"2.0.1"` | |
```
Alternatively, run `helm-docs` locally @ArnaudCHT
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] feat(ingress-controller): add imagePullSecrets support with global fallback [apisix-helm-chart]
martin-schulze-e2m commented on code in PR #916:
URL: https://github.com/apache/apisix-helm-chart/pull/916#discussion_r2905851654
##
charts/apisix-ingress-controller/README.md:
##
@@ -133,7 +133,8 @@ The same for container level, you need to set:
| deployment.annotations | object | `{}` | Add annotations to Apache APISIX
ingress controller resource |
| deployment.image.pullPolicy | string | `"IfNotPresent"` | |
| deployment.image.repository | string | `"apache/apisix-ingress-controller"`
| |
-| deployment.image.tag | string | `"2.0.1"` | |
+| deployment.image.tag | string | `"2.0.0-rc5"` | |
Review Comment:
```suggestion
| deployment.image.tag | string | `"2.0.1"` | |
```
Alternatively, run `helm-docs` locally
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] feat(ingress-controller): add imagePullSecrets support with global fallback [apisix-helm-chart]
ArnaudCHT commented on PR #916: URL: https://github.com/apache/apisix-helm-chart/pull/916#issuecomment-3996186283 > @ArnaudCHT Please resolve the conflict. done -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] feat(ingress-controller): add imagePullSecrets support with global fallback [apisix-helm-chart]
AlinsRan commented on PR #916: URL: https://github.com/apache/apisix-helm-chart/pull/916#issuecomment-3994779720 @ArnaudCHT Please resolve the conflict. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
