Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow multiple imagePullSecrets in the helm chart #4589

Closed
AlessioCasco opened this issue Oct 31, 2023 · 10 comments · Fixed by #4656
Closed

Allow multiple imagePullSecrets in the helm chart #4589

AlessioCasco opened this issue Oct 31, 2023 · 10 comments · Fixed by #4656
Assignees
Labels
backlog Pull requests/issues that are backlog items good first issue Issues identified as good for first-time contributors proposal An issue that proposes a feature request
Milestone

Comments

@AlessioCasco
Copy link
Contributor

Hello everyone, I consider this a feature request, do let me know if this is more of a bug in your opinion.

Is your feature request related to a problem? Please describe.

imagePullSecretName can only be set as a string currently. We need the ability to configure more than one imagePullSecrets in our ServiceAccount. The reason is that we use a sidecar that has to be pulled from a private repository.

Describe the solution you'd like

  • Rename values key imagePullSecretName to imagePullSecretsNames
  • Change it to be an array
  • Iterate within it into the controller-serviceaccount.yaml so the ServiceAccount can have more than one entry under imagePullSecrets.
    I can fork the repo and submit a PR if you are happy to.

Describe alternatives you've considered

I don't have other solutions, to be honest.

@AlessioCasco AlessioCasco added the proposal An issue that proposes a feature request label Oct 31, 2023
Copy link

Hi @AlessioCasco thanks for reporting!

Be sure to check out the docs and the Contributing Guidelines while you wait for a human to take a look at this 🙂

Cheers!

@shaun-nx
Copy link
Contributor

shaun-nx commented Nov 6, 2023

Hi @AlessioCasco this does sound like a really good addition. I want to be mindful that this will change how imagePullSecrets are defined in both our values.yaml as well as when using the --set command. I'd like to run this one by our team first before saying "Yes, please make a PR".

Thanks!

@shaun-nx shaun-nx self-assigned this Nov 6, 2023
@brianehlert
Copy link
Collaborator

To prevent introducing an absolute breaking change.
Is there a way to support both imagePullSecretName and imagePullSecretsNames for a period of multiple releases?
I am aware of customers that implement this in production and are dependent on it and wish to give them a period toin-place move to the new setting.

@shaun-nx shaun-nx added backlog Pull requests/issues that are backlog items good first issue Issues identified as good for first-time contributors labels Nov 8, 2023
@shaun-nx
Copy link
Contributor

shaun-nx commented Nov 9, 2023

Hi @AlessioCasco we think making a PR for this would be great if you are still happy to.
To the point that @brianehlert made, to prevent introducing any breaking changes, we'd like to maintain the exiting imagePullSecretName field while also introducing the new imagePullSecretNames field.

@AlessioCasco
Copy link
Contributor Author

Hello

Sorry for my late reply on this. happy to submit a PR.

Thanks

@shaun-nx
Copy link
Contributor

No problem @AlessioCasco! We appreciate you contributing! 😄

@AlessioCasco
Copy link
Contributor Author

Hi! @shaun-nx
I have a PR ready, one question before I submit it: Shall we remove any reference to the imagePullSecretName in the docs? This is so new people will start using the new imagePullSecretsNames even tho the old parameter still works.
What do you think?

@shaun-nx
Copy link
Contributor

@AlessioCasco Hmm, that's a very good question.
I would keep both. In the future we would like to phase out the current one in favour of using the list.
I'll bring this up with the team today. For now I would submit the PR for the new field with documentation on the new field and keep the documentation for the old field as well.

@AlessioCasco
Copy link
Contributor Author

Ok cool! Will do

@AlessioCasco
Copy link
Contributor Author

PR created #4656

@brianehlert brianehlert added this to the v3.4.0 milestone Nov 28, 2023
@shaun-nx shaun-nx linked a pull request Nov 30, 2023 that will close this issue
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog Pull requests/issues that are backlog items good first issue Issues identified as good for first-time contributors proposal An issue that proposes a feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants