Issue 589: Add imagePullSecrets for Helm hooks #591
Conversation
### Change log description * Adds imagePullSecrets to the service accounts for the Helm hooks * Follows the same principle as the service account for the operator taking global imagePullSecrets into account ### Purpose of the change Fixes pravega#589 ### What the code does Adds imagePullSecrets to the YAML for the ServiceAccounts for the Helm hooks. This is important when images are pulled from a private registry, e.g. an internal proxy, such as Artifactory, or when using custom images. ### How to verify it Render the templates using the following command: ```console helm template zookeeper charts/zookeeper-operator \ --show-only templates/post-install-upgrade-hooks.yaml \ --show-only templates/pre-delete-hooks.yaml \ --set hooks.serviceAccount.imagePullSecrets={'my-pull-secret'} ``` Signed-off-by: Reinhard Nägele <[email protected]> # Please enter the commit message for your changes. Lines starting # with '#' will be kept; you may remove them yourself if you want to. # An empty message aborts the commit. # # Date: Mon Jan 22 11:52:54 2024 +0100 # # On branch issue-589-pull-creds-for-hook # Your branch and 'origin/issue-589-pull-creds-for-hook' have diverged, # and have 1 and 1 different commits each, respectively. # # Changes to be committed: # modified: charts/zookeeper-operator/templates/post-install-upgrade-hooks.yaml # modified: charts/zookeeper-operator/templates/pre-delete-hooks.yaml # modified: charts/zookeeper-operator/values.yaml #
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #591 +/- ##
=======================================
Coverage 85.91% 85.91%
=======================================
Files 12 12
Lines 1633 1633
=======================================
Hits 1403 1403
Misses 145 145
Partials 85 85 ☔ View full report in Codecov by Sentry. |
|
@jkhalack Would you be able to have a look at this change? It would be really appreciated to get this change added to the helm charts. TIA |
| @@ -57,6 +57,10 @@ tolerations: [] | |||
| annotations: {} | |||
|
|
|||
| hooks: | |||
| ## Optionally specify an array of imagePullSecrets. Will override the global parameter if set | |||
There was a problem hiding this comment.
Please update the comment to explain that the array here is expected to contain secret names only, as we seem to have two different schemas already in the code:
- for
pod.imagePullSecretswe are expecting key-value pair objects
https://github.com/pravega/zookeeper-operator/blob/master/charts/zookeeper/templates/zookeeper.yaml#L110 - for
serviceAccount.imagePullSecretswe are expecting secret names
https://github.com/pravega/zookeeper-operator/blob/master/charts/zookeeper-operator/templates/service_account.yaml#L7
Otherwise I think it is good to go, unless you would think it necessary to update the schema forpod.imagePullSecretsas well (to be able to consumeglobal.imagePullSecretsthere).
@anishakj would you agree that updatingpod.imagePullSecretsschema would be appropriate?
There was a problem hiding this comment.
I tried to be consistent and matched what is already done in the values.yaml. See https://github.com/pravega/zookeeper-operator/blob/master/charts/zookeeper-operator/values.yaml#L30. If you like, I could update the whole chart in order to improve this.
|
@anishakj Could you please help me with the timeline for this change to be released. Appreciate if you could expedite the same. Thanks in advance. |
|
Merged as admin. |
Change log description
Purpose of the change
Fixes #589
What the code does
Adds imagePullSecrets to the YAML for the ServiceAccounts for the Helm hooks. This is important when images are pulled from a private registry, e.g. an internal proxy, such as Artifactory, or when using custom images.
How to verify it
Render the templates using the following command: