-
Notifications
You must be signed in to change notification settings - Fork 21
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
ISSUE-104 - Allow composable-manager to elevated permissions #105
base: main
Are you sure you want to change the base?
Conversation
- apiGroups: | ||
- '*' | ||
resources: | ||
- '*' | ||
verbs: | ||
- create | ||
- delete | ||
- get | ||
- list | ||
- patch | ||
- update | ||
- watch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a hard one, bcs it allows cluster-wide privilege escalation. Wdyt about better docs or a custom target that includes this as a patch rather than a default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also struggled with this question. I think a middle ground might be to parametrize the roles and lock it behind an enable: true
option. The documentation would be part of the values.yaml
so that you don't have to seek it out as a special case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds fine to me!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lhriley are you planning to work on it? Otherwise I'd close the PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had planned to, but I don't have the bandwidth at the moment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was taking a look at this, and I'm actually not sure how to approach it using kustomize
. If this was helm
, it would be simple. Any thoughts? Or should I close this and let someone else come up with something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy to accept a helm chart as additional deployment method!
Allow the
composable-manager-role
rights to manage cluster resources.Resolves: #104