Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 non-admin controller design #18
Add non-admin controller design #18
Changes from 3 commits
edf3b31
d6ddfe9
8dd58cd
815de7f
8f902f2
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
here "has" would be better than "requires"?
suppose someone tries to use OADP in non admin way (without NAC). Installs it in a non admin namespace. the non admin user can only create things in that namespace, but it can tell OADP to backup/restore anything (in any namespace) in the cluster, right?
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 think either is fine, currently we advertise OADP functionalities as cluster admin tasks so that way requires makes sense.
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.
Should we mention the use case of that BSL in the other operations such as Backup? How this will be used?
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.
added the use-case
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 isn't clear to me. One instance of NAC for one OADP operator, and above we have the multiple OADP:NAC installations, so does it mean we can only scale the NAC by scaling OADP?
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.
OADP Operator is NS scoped operator, so you can have multiple OADP Operator installations in the cluster. But each of the OADP Operator will have one NAC controller instance if NAC is enabled. So we need to ensure that if there are multiple OADP/NAC installations in one particular cluster then no 2 NAC controllers should serve once single non-admin application/user 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.
Also not clear - NAC will create namespace per instance? Or it's OADP namespace to which the NAC controller(s) get installed, or if I think what it is:
The non-admin namespaces on which NAC controllers will operate....
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.
Yes your understanding that
The non-admin namespaces on which NAC controllers will operate....
is correctThere 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.
If the user has admin rights to a particular namespace, the RBAC is not required right?
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.
yeah makes sense, added a note.