-
-
Notifications
You must be signed in to change notification settings - Fork 120
feat!: harmonize service account binding #363
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
base: main
Are you sure you want to change the base?
Conversation
0ef820d to
7d495fb
Compare
7d495fb to
c127a42
Compare
d3adb5
left a comment
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.
Left a few comments. I like the idea of adding a semantic value like this, but I feel the way it currently is introduces more confusion than clarity. I expanded upon this in one of the threads I opened below.
4662e88 to
a6b974e
Compare
a6b974e to
6d404a8
Compare
6d404a8 to
0c15cb1
Compare
0c15cb1 to
de41a4f
Compare
d3adb5
left a comment
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.
Final set of comments, I promise. Reminder to add a unit test for it if you do take the suggestion. Otherwise, LGTM.
|
@aslafy-z lets finalize and get this merged |
Signed-off-by: GitHub <noreply@github.com>
fb0c8ba to
0efceb9
Compare
|
@karl-johan-grahn @d3adb5 Can you please give a look? Thank you! |
|
@aslafy-z Can you please resolve the conflicts? |
|
@d3adb5 can you plz review it again and approve it? |
d3adb5
left a comment
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! Sorry for the delay in coming back to this.
|
Do make sure to do a major version bump afterwards! |
|
@rasheedamir @karl-johan-grahn Can you please also review? This PR needs a second approval. |
Closes #320
Closes #361
BREAKING: Rename
rbac.serviceAccount.enabledfield torbac.serviceAccount.create.Fix inconsistencies in serviceAccount binding across
CronJob,Job, andDeploymenttemplates by introducing a newapplication.serviceAccountNametemplate:rbac.serviceAccount.createrbac.serviceAccount.nametrue""(include "application.name" .)true"foo""foo"false"""default"false"bar""bar"Add field
automountServiceAccountTokenin pod specs controlled byrbac.enabled.Document missing Role and RoleBinding
rbac.annotationsandrbac.additionalLabelsfields.