-
Notifications
You must be signed in to change notification settings - Fork 137
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
Add RBAC for tink-controller and tink-server #610
Conversation
c5bac71
to
57bcea0
Compare
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.
Thank you so much for this, awesome work! Just a few minor changes
- configmaps | ||
verbs: | ||
- get | ||
- list | ||
- watch | ||
- create | ||
- update | ||
- patch | ||
- delete |
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.
Why are full config map permissions required in the namespace?
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 role is auto-generated by kubebuilder. I am not sure if all these permissions are really required during leader-election or not. Will check and update here
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.
Alright I just tested and looks like it needs all the permissions on cm except for delete
Codecov Report
@@ Coverage Diff @@
## main #610 +/- ##
=======================================
Coverage 44.37% 44.37%
=======================================
Files 61 61
Lines 3491 3491
=======================================
Hits 1549 1549
Misses 1858 1858
Partials 84 84
Continue to review full report at Codecov.
|
c58ea83
to
56183d0
Compare
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.
LGTM!
@mmlb, can you check this out? approve the workflow run? |
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.
The workflow is green and the changes LGTM. Will wait for @mmlb's review though.
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.
LGTM
Signed-off-by: Abhinav Pandey <abhinavmpandey08@gmail.com>
b48d0da
to
83240cd
Compare
Description
This PR adds all deployment and RBAC manifests and
kustomization.yaml
s fortink-controller
andtink-server
.Why is this needed
This is needed to help users deploy the kubified tink services in their Kubernetes environment
This is what the generated manifest looks like after running
kustomize build config/default
https://gist.github.com/abhinavmpandey08/ed00314e4b738cacbad67ea9e345ec2aHow Has This Been Tested?
This has been tested by applying the manifests on a KinD cluster and verifying that
tink-controller
andtink-server
both work as intended.How are existing users impacted? What migration steps/scripts do we need?
No user impact
Checklist:
I have: