-
Notifications
You must be signed in to change notification settings - Fork 55
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 provided ServiceAccount documentation to drafts #1232
Conversation
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
1862242
to
b59e7f8
Compare
There are some locations where it would make sense to add hyperlinks to this doc, but I'm not sure how we want to handle that while it's in the drafts directory so I didn't add any. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1232 +/- ##
==========================================
- Coverage 76.53% 76.37% -0.17%
==========================================
Files 40 40
Lines 2340 2341 +1
==========================================
- Hits 1791 1788 -3
- Misses 392 394 +2
- Partials 157 159 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
While OLMv0 provides many mechanisms for permission configuration via OperatorGroups, | ||
or by manipulating CatalogSources, cluster administrators must be aware of these options | ||
and actually implement them. If no ServiceAccount is explicitly specified for installing | ||
and upgrading operators, then cluster-admin is used by default. This can pose security risks | ||
by providing more permissions than are actually required for the management of any specific bundle. |
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.
Not sure we really need to mention OLMv0 here - what do others think?
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 definitely see an argument for certain OLMv0 callouts on specific notable changes. And this is certainly one of those.
But maybe we should just have a separate doc that highlights all of those things in one place?
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.
That sounds like a good idea. So do you think I should remove that mention here and open a ticket to create a different page outlining the differences between OLMv0/v1?
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 +1 for a separate "changes from OLMv0" type documentation item
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 agree with this, we can have a separate document for changes from OLMv0 and not mention OLMv0 in this doc.
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.
Maybe we can rephrase as below:
OLMv1 differs from its predecessor and requires you to specify the permissions required to install cluster extensions upfront. It does not provider cluster admin privileges by default. It gives the control to the cluster administrator to specify the exact permissions required for the management of any specific bundle.A ServiceAccount needs to be explicitly specified for installing and upgrading operators else will face errors deploying your cluster extension.
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 added the following to my doc:
OLMv1 does not provide cluster admin privileges by default for installing cluster extensions. It depends on the cluster extension developer to specify the permissions required for the management of any specific bundle. A ServiceAccount needs to be explicitly specified for installing and upgrading operators else will face errors when deploying your cluster extension.
b59e7f8
to
9c10a83
Compare
9c10a83
to
1deb4fe
Compare
I have reworked most of the doc, integrating everyone's feedback. Ready for another look. |
1deb4fe
to
a5c198a
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.
One minor nit, but other than that it LGTM!
Signed-off-by: Tayler Geiger <[email protected]>
a5c198a
to
d921582
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
d09f325
Description
Closes #977
Reviewer Checklist