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.

Sign up for GitHub

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

Jump to bottom

Adding ability to add 'extraManifests' #212

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Adding ability to add 'extraManifests' #212

wants to merge 3 commits into from

Conversation

JunaidChaudry
Copy link
Contributor

@JunaidChaudry JunaidChaudry commented Mar 9, 2023 •
edited
Loading

All Submissions:

  • Have you opened an Issue before filing this PR?
  • Have you signed our CLA?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Put closes #XXXX in your comment to auto-close the issue that your PR fixes (if such).

@JunaidChaudry JunaidChaudry mentioned this pull request Mar 9, 2023
Closed
@JunaidChaudry
Copy link
Contributor Author

JunaidChaudry commented Mar 9, 2023 •
edited
Loading

Closes #213

@JunaidChaudry JunaidChaudry force-pushed the feature/community-operator-supports-extraManifests branch from 229817b to 6c9f51e Compare March 9, 2023 18:41
@JunaidChaudry
Copy link
Contributor Author

Can a reviewer kindly review this?

@igor-karpukhin
Copy link
Collaborator

cc @slaskawi @irajdeep

@JunaidChaudry
Copy link
Contributor Author

JunaidChaudry commented May 25, 2023 •
edited
Loading

@slaskawi Can you please review this one as well?

The feature aligns with many other opensource helm charts (e.g. grafana, prometheus, argo-cd, etc.), to support extra k8s manifests using the values files.
This allows the end user to add any additional files needed to support their helm deployment. A nice use case now a days is to mount secrets on the fly by leveraging the Secrets Store CSI Driver..

@JunaidChaudry JunaidChaudry force-pushed the feature/community-operator-supports-extraManifests branch from d330cc9 to 9fd8540 Compare May 25, 2023 14:00
slaskawi
slaskawi reviewed Jul 5, 2023
View reviewed changes
Copy link
Contributor

@slaskawi slaskawi left a comment

Choose a reason for hiding this comment

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

Overall, I'm fine with extraObjects. We're seen this approach in Grafana and Argo, and we see it useful.

I'm not sure about adding annotations on Service Accounts. Could you please provide us examples why it's necessary and extract this into a separate PR?

charts/community-operator/templates/database_roles.yaml Show resolved Hide resolved
@slaskawi slaskawi self-assigned this Jul 5, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Sep 4, 2023

This PR is being marked stale because it has been open for 60 days with no activity. Please update the PR or ask for a fresh review.

@github-actions github-actions bot added the Stale label Sep 4, 2023
@slaskawi
Copy link
Contributor

slaskawi commented Sep 4, 2023

@JunaidChaudry Could you please check my comment and rebase this PR?

@github-actions github-actions bot removed the Stale label Sep 5, 2023
@JunaidChaudry
Copy link
Contributor Author

JunaidChaudry commented Sep 21, 2023 •
edited
Loading

@slaskawi Apologies for the VERY long delay.

I have rebased with main, and please also check out my comment around serviceaccount annotations, and why I think it should be included.

@github-actions GitHub Actions
Copy link
Contributor

This PR is being marked stale because it has been open for 60 days with no activity. Please update the PR or ask for a fresh review.

@github-actions github-actions bot added the Stale label Nov 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@slaskawi slaskawi slaskawi left review comments

@irajdeep irajdeep Awaiting requested review from irajdeep

@tibulca tibulca Awaiting requested review from tibulca tibulca is a code owner

@priyolahiri priyolahiri Awaiting requested review from priyolahiri

@mircea-cosbuc mircea-cosbuc Awaiting requested review from mircea-cosbuc mircea-cosbuc is a code owner

@igor-karpukhin igor-karpukhin Awaiting requested review from igor-karpukhin igor-karpukhin is a code owner

@fabritsius fabritsius Awaiting requested review from fabritsius

@Sugar-pack Sugar-pack Awaiting requested review from Sugar-pack

@helderjs helderjs Awaiting requested review from helderjs helderjs is a code owner

@lsierant lsierant Awaiting requested review from lsierant lsierant is a code owner

@josvazg josvazg Awaiting requested review from josvazg josvazg is a code owner

@roothorp roothorp Awaiting requested review from roothorp roothorp is a code owner

@nammn nammn Awaiting requested review from nammn nammn is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

@slaskawi slaskawi

Labels
Stale
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@JunaidChaudry @igor-karpukhin @slaskawi