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

feat: Adds possibility to set image pull secrets in all charts #204

Merged

Conversation

luflow
Copy link
Contributor

@luflow luflow commented Aug 4, 2024

Adds possibility of image pull secrets to all charts

on connect: global
on secrets injector: in the injector scope

closes #203

@luflow
Copy link
Contributor Author

luflow commented Aug 4, 2024

Also fixed some whitespace issues in the two templates I touched.

@luflow
Copy link
Contributor Author

luflow commented Aug 5, 2024

@edif2008 Fixed the linting errors

Copy link
Member

@edif2008 edif2008 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much for your contribution and adding this enhancement! 😊

I've left a couple of small comments and once they're addressed, I will approve this PR.

charts/secrets-injector/templates/deployment.yaml Outdated Show resolved Hide resolved
charts/connect/templates/connect-deployment.yaml Outdated Show resolved Hide resolved
charts/connect/values.yaml Outdated Show resolved Hide resolved
@edif2008
Copy link
Member

I've also noticed just now that your branch seems to be in conflict with the latest main. What I recommend is to update your branch with the latest main while you address the comments.

@luflow luflow force-pushed the feature/image-pull-secrets-configurable branch 2 times, most recently from bd87082 to 1da8285 Compare August 21, 2024 04:53
@luflow
Copy link
Contributor Author

luflow commented Aug 21, 2024

@edif2008 thx for your review, fixed all your comments :)

@volodymyrZotov
Copy link
Contributor

Hi @luflow 👋. I merged another one PR #170 earlier today, which caused the conflicts with your one 🤦‍♂️. I solved them, but looks like there is a protection rule on your forked repo, so I could not push the changes into your branch.

I'd appreciate if you have a chance resolve the conflicts once again. Or give us permission to push to your branch, so we can help you with that. And finally merge you PR 😃

Sorry for the inconvenience. I appreciate the efforts and the time you put into making this contribution! 👍

@luflow
Copy link
Contributor Author

luflow commented Aug 28, 2024

@edif2008 @volodymyrZotov Done :)

@luflow
Copy link
Contributor Author

luflow commented Sep 7, 2024

@edif2008 @volodymyrZotov anything else needed? :) Looking forward to the merge 🤩

@volodymyrZotov volodymyrZotov merged commit afd7ff7 into 1Password:main Sep 9, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Please add a way to set imagePullSecrets to deployments
3 participants