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

ISSUE-104 - Allow composable-manager to elevated permissions #105

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

ISSUE-104 - Allow composable-manager to elevated permissions #105

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions config/rbac/role.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,15 @@ rules:
- get
- patch
- update
- apiGroups:
- '*'
resources:
- '*'
verbs:
- create
- delete
- get
- list
- patch
- update
- watch
Comment on lines +34 to +45
Copy link
Member

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?

Copy link
Contributor Author

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.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds fine to me!

Copy link
Member

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

Copy link
Contributor Author

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.

Copy link
Contributor Author

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?

Copy link
Member

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!