-
Notifications
You must be signed in to change notification settings - Fork 17
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
chore: add typing to templates.ts #1602
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1602 +/- ##
==========================================
+ Coverage 81.24% 81.31% +0.07%
==========================================
Files 42 43 +1
Lines 1962 1970 +8
Branches 406 426 +20
==========================================
+ Hits 1594 1602 +8
+ Misses 366 340 -26
- Partials 2 28 +26
|
@@ -52,6 +80,8 @@ export function genPkgJSON(opts: InitOptions, pgkVerOverride?: string) { | |||
}, | |||
includedFiles: [], | |||
env: pgkVerOverride ? testEnv : {}, | |||
rbac: [], | |||
rbacMode: RbacMode.SCOPED, |
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 lack context here, but it seems like rbacMode
is the equivalent to setting a default value here. In that case, I'm assuming we ought to default to the use of scoped
instead of admin
.
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.
ah sorry, I missed this.
So, in short, in prod, without a doubt you are right, no reason for anyone to use cluster-admin, no way someone is going to need to control every single object in a cluster.
There is a more philosophical conversation to be had around starting a project (and Pepr's origin), figuring out how to do the Kubernetes calls that you need, then adding the appropriate RBAC after you figure everything out.
The intent of the project is that anyone can build an operator or controller. We increase the barrier of entry if we expect them to know RBAC at the beginning.
Currently, after initializing a new Pepr Module, the ServiceAccount is bound to a ClusterRole that has cluster-admin privs.
If we were going to change a default behavior there would need to be a longer convo, and warnings to potential users.
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.
nice job!
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.
For now lets keep RBAC mode wide open since this would be a major change. Other than that it is all good.
Description
Adds typing to
templates.ts
End to End Test:
(See Pepr Excellent Examples)
Related Issue
Relates to #1364
Type of change
Checklist before merging