OADP-4001 Self-Service #89043
OADP-4001 Self-Service #89043
Conversation
|
/label OADP |
openshift-ci
bot
added
OADP
openshift-ci
bot
added
size/L
openshift-ci
bot
added
size/XXL
shdeshpa07
force-pushed
the
OADP-4001-Self-Service
branch
from
dc07fa0 to
d87b95c
Compare
shdeshpa07
force-pushed
the
OADP-4001-Self-Service
branch
from
e672e74 to
568298f
Compare
|
@anarnold97 - Requesting your review please and thanks. |
|
@shdeshpa07 AWESOME work here Shruti! There are some things to cover in office hours so we're very clearly communicating the changes. Mostly just minor things to change/reword. I would like discuss and think about the non-admin use case section with you and the team in office hours. THANK YOU!! @shubham-pampattiwar @mpryc as you folks have time, please jump in. This will most likely take a few iterations |
@weshayutin - Thank you for the review. I am on the feedback :). |
|
@weshayutin @mpryc @shubham-pampattiwar @PrasadJoshi12 Can I please request a review for the self-service PR. I have re-worked the sections based on our discussion in office hours yesterday. I am no longer delineating FSB/DM/CSI backups separately. I have also included a section of NADR. Here is the preview again: Self-Service cluster admin use cases Self-Service namespace admin use cases Thanks! |
backup_and_restore/application_backup_and_restore/oadp-self-service/oadp-self-service.adoc
Outdated
Show resolved
Hide resolved
modules/oadp-self-service-troubleshoot-controller-not-found.adoc
Outdated
Show resolved
Hide resolved
|
This is looking GREAT!!! https://github.com/migtools/oadp-non-admin/blob/8a0600cf9dc4944cf1aa3886e61010d0d92d3822/docs/draft_production_docs/oadp-self-service.md#restricted-nonadminbackups Perhaps we want a self-service reference section? |
|
|
||
| Namespace admin users can: | ||
|
|
||
| * Create and manage backups of their authorized namespaces. |
There was a problem hiding this comment.
@mpryc A qq as per current design we only allow backing up a single namespace at a time, right ? Do you think we should mentioned that here.
Signed-off-by: Shruti Deshpande <shdeshpa@redhat.com>
shdeshpa07
force-pushed
the
OADP-4001-Self-Service
branch
from
e7c973b to
18e34b2
Compare
|
@shdeshpa07: all tests passed! Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
/label peer-review-needed |
openshift-ci
bot
added
the
peer-review-needed
|
/label peer-review-in-progress |
openshift-ci
bot
added
the
peer-review-in-progress
There was a problem hiding this comment.
I have left some initial comments regarding some of the repeated issues in the PR. This is not a complete peer review. I did not make it all the way through the PR, and I did not review for grammar, minimalism, or more subtle style guide and markup issues. As a result, I am not putting the peer-review-done label on this.
PTAL at the comments and look for places where they can be applied elsewhere in the doc. Also, please read over the style guide links and consider how to apply the guidance. Please address the consistent issues before putting this PR back in the queue.
In the future, I would recommend chunking this work into several smaller PRs. 2000 lines of net-new writing takes a long time to review, so please be patient with the peer review squad. For a PR of this size, it might take a few rounds of peer review before it is ready for merge. I would expect that the next reviewer might need a full working day or several to perform a complete review.
If you have questions, I am most responsive on Slack. Feel free to DM me or start a thread in the #team-ocp-docs channel.
/remove-label peer-review-needed
/remove-label peer-review-in-progress
Also, if you don't have Vale set up in your IDE or terminal, I recommend asking for help in the #project-vale-at-red-hat Slack channel.
|
|
||
| toc::[] | ||
|
|
||
| As a cluster administrator, following are some use cases where you: |
There was a problem hiding this comment.
See the IBM Style Guide for lead-in wording on lists: https://www.ibm.com/docs/en/ibm-style?topic=format-lists#lead-in-wording. List lead-ins must be grammatically complete sentences.
This comment applies throughout the doc.
|
|
||
| toc::[] | ||
|
|
||
| Following sections can help you troubleshoot common errors, when using {oadp-short} Self-Service. |
There was a problem hiding this comment.
See Minimalism principle 1: Customer focus and action orientation. Avoid self-referential language.
|
|
||
|
|
||
| [role="_additional-resources"] | ||
| .Additional resources |
There was a problem hiding this comment.
These additional resources links seem to apply to the whole assembly and are not directly related to the proceeding module. If that is true, this needs to be an H2 heading. See the repo guidelines for more information.
|
|
||
| :_mod-docs-content-type: CONCEPT | ||
| [id="oadp-self-service-about-nabsl_{context}"] | ||
| = About `NonAdminBackupStorageLocation` CR |
There was a problem hiding this comment.
You can't use backticks or any other markup in headings. This comment applies throughout the PR.
https://github.com/openshift/openshift-docs/blob/main/contributing_to_docs/doc_guidelines.adoc#assemblymodule-titles-and-section-headings
| [id="oadp-self-service-about-nabsl_{context}"] | ||
| = About `NonAdminBackupStorageLocation` CR | ||
|
|
||
| A `NonAdminBackupStorageLocation` (NABSL) is a custom resource that a namespace admin user can create to store the backup data. |
There was a problem hiding this comment.
Avoid passive voice.
| A `NonAdminBackupStorageLocation` (NABSL) is a custom resource that a namespace admin user can create to store the backup data. | |
| Namespace administrators can create a `NonAdminBackupStorageLocation` (NABSL) custom resource to store the backup data. |
| In the example DPA, you have: | ||
|
|
||
| * Added the `enforceBSLSpec` section as shown in annotation *1*. | ||
| * Enforced the `config` section of a NABSL to use an {aws-short} S3 bucket in the us-west-2 region as shown in annotation *2*. | ||
| * Enforced the `objectStorage` section of a NABSL to use a company bucket named `my-company-bucket` as shown in annotation *3*. |
There was a problem hiding this comment.
This way of using code callouts is incorrect. You can use code callouts or create a description/definition list. But both options have specific style rules that need to be followed. See https://redhat-documentation.github.io/modular-docs/#creating-reference-modules
There was a problem hiding this comment.
See the previous comment regarding lists and reference modules.
|
|
||
| The {oadp-short} Self-Service feature has the following new custom resources (CRs) to enable the backup and restore operations for a namespace admin user: | ||
|
|
||
| * `NonAdminController` (NAC): Controls and orchestrates the Self-Service operations. |
There was a problem hiding this comment.
This should be formatted as a description/definition list.
| * `NonAdminController` (NAC): Controls and orchestrates the Self-Service operations. | |
| `NonAdminController` (NAC):: Controls and orchestrates the Self-Service operations. |
There was a problem hiding this comment.
This comment applies throughout the doc.
| * The cluster administrator has installed the {oadp-short} Operator. | ||
| * The cluster administrator has configured the `DataProtectionApplication` (DPA) to enable {oadp-short} Self-Service. | ||
| * The cluster administrator has created a namespace for you and has authorized you to operate from that namespace. | ||
| * Optionally, you can create and use a `NonAdminBackupStorageLocation` (NABSL) CR to store the backup data. If you do not use a NABSL CR, then the backup is stored in the default backup storage location configured in the DPA. |
There was a problem hiding this comment.
See the ISG on Optional and conditional steps: https://www.ibm.com/docs/en/ibm-style?topic=format-procedures#indicating-optional-and-conditional-steps
That said, it doesn't really make sense that to have an optional prereq. I would find another way to describe this environment constraint.
|
|
||
| . To create a `NonAdminBackup` CR, create a YAML manifest file with the following configuration: | ||
| + | ||
| .Example `NonAdminBackup` |
There was a problem hiding this comment.
| .Example `NonAdminBackup` | |
| .Example `NonAdminBackup` CR |
All specific API object references (ie, anything in backticks) must be followed by a noun. This comment applies throughout the doc.
openshift-ci
bot
removed
peer-review-needed
|
|
||
| In the earlier versions of {oadp-full}, you needed the `cluster-admin` role to perform {oadp-short} operations such as backing up and restoring an application, creating a backup storage location, and so on. For more details on cluster roles, see "Default cluster roles". | ||
|
|
||
| From {oadp-short} 1.5.0 onward, you do not need the `cluster-admin` role to perform the backup and restore operations. You can use {oadp-short} with the namespace `admin` role. The namespace `admin` role has administrator access only to the namespace the user is assigned to. |
There was a problem hiding this comment.
Should we add a note that you still need the cluster admin to install OADP Operator ? Installation of OADP Operator is not supported for non-admin users
| * `nonadminrestores.oadp.openshift.io` | ||
| * `nonadmindownloadrequests.oadp.openshift.io` | ||
|
|
||
| A namespace admin user cannot edit the `NonAdminBackupStorageLocation` (NABSL) CR. They can only create a new NABSL. |
There was a problem hiding this comment.
This is not accurate. OADP/NAC cannot directly control this. If the namespace admin has edit access, they can edit the NABSL CR.
| resources: | ||
| - nonadminbackups | ||
| - nonadminrestores | ||
| - nonadminbackupstoragelocations |
There was a problem hiding this comment.
We should add NADR CR too
|
|
||
| * To ensure secure backup and restore, Self-Service automatically excludes the following resources from being backed up or restored: | ||
|
|
||
| ** Security Context Constraints (SCCs) |
There was a problem hiding this comment.
Lets add codeblocks for all the resources in this list, just like PriorityClasses
| ** Security Context Constraints (SCCs) | ||
| ** Cluster roles | ||
| ** Cluster role bindings | ||
| ** Custom resource definitions (CRDs) |
There was a problem hiding this comment.
Also add NS scoped Non-Admin CRs:
alwaysExcludedNamespacedResources = []string{
nacv1alpha1.NonAdminBackups,
nacv1alpha1.NonAdminRestores,
nacv1alpha1.NonAdminBackupStorageLocations,
}
alwaysExcludedClusterResources = []string{
"securitycontextconstraints",
"clusterroles",
"clusterrolebindings",
"priorityclasses",
"customresourcedefinitions",
"virtualmachineclusterinstancetypes",
"virtualmachineclusterpreferences",
}
| |=== | ||
| |*Value* |*Description* | ||
| |`New`|NAB/NAR CR creation request is accepted by the NAC, but it has not yet been validated by the NAC. | ||
| |`BackingOff`|NAB/NAR CR is invalidated by the NAC because of an invalid NAB/NAR `spec`. The NAC fails to reconcile the CRs. |
There was a problem hiding this comment.
This is incorrect, lets remove this statement: The NAC fails to reconcile the CRs.
|
|
||
| You can create a NABSL CR by using one of the following workflows: | ||
|
|
||
| * *Administrator creation workflow*: In this workflow, the cluster administrator creates the NABSL CR for the namespace admin user. The administrator then sets the NABSL in the `spec.backupstoragelocation` section of the DPA. |
There was a problem hiding this comment.
This seems inaccurate "The administrator then sets the NABSL in the spec.backupstoragelocation section of the DPA." @mpryc please check.
There was a problem hiding this comment.
The admin can create default BSL in the oadp namespace or create for the users NABSL in the users namespace.
In the second scenario the user must then reference that NABSL in the Non Admin Backup object. The above doc is not correct.
| ==== | ||
| For security purposes, it is recommended to use either the administrator creation or the administrator approval workflow. The automatic approval workflow is less secure as it does not require administrator review. | ||
|
|
||
| Also note that, a namespace admin user cannot edit the NABSL CR. |
There was a problem hiding this comment.
I would reframe to say that "Edit functionality is not sipported for NABSL CR" or something similar.
| [id="oadp-self-service-admin-spec-enforce-nab_{context}"] | ||
| = Self-Service administrator spec enforcement for NAB | ||
|
|
||
| As a cluster administrator, you can enforce the following fields for a `NonAdminBackup` (NAB) CR: |
There was a problem hiding this comment.
IMO we should change the approach here, we should document what spec fields are non-enforceable by the admins and the special cases etc. Velero API keeps evolving per release, documenting allowlist is a difficult task.
There was a problem hiding this comment.
@shubham-pampattiwar wdyt about us creating a kbase article that we can update and perhaps that is something the docs could point to. I'm not 100% sure the docs team is allowed to point at a kbase, it's worth asking. If we do think it's going to be very dynamic release to release I would suggest we exclude it from the production docs and ONLY have a kbase article.
| [id="oadp-self-service-namespace-permissions_{context}"] | ||
| = {oadp-short} Self-Service namespace permissions | ||
|
|
||
| As a cluster administrator, ensure that a namespace admin user has editor roles assigned for the following list of objects in their namespace. These objects ensure that a namespace admin user can perform the backup and restore operations in their namespace. |
There was a problem hiding this comment.
A reference link to the defined cluster user roles would be appropriate here:
https://docs.redhat.com/en/documentation/openshift_container_platform/4.18/html/authentication_and_authorization/using-rbac#authorization-overview_using-rbac
noting the admin role..
Also noting that a cluster-admin can define their own specifications to users that have equivilant rights as a project or namespace admin. @mail2nadeem92 FYI please add flavor if I did not cover it
Jira
OADP Self-Service feature
Version
Preview
Self-Service
Self-Service admin use cases
Self-Service non-admin use cases
Self-Service troubleshooting
QE Review