Design For OADP CLI MVP #1797

NicholasYancey · 2025-06-24T16:29:21Z

Why the changes were made

Need the kubectl plugin design document peer reviewed

How to test the changes made

Please read

openshift-ci · 2025-06-24T16:29:43Z

Hi @NicholasYancey. Thanks for your PR.

I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

kaovilai · 2025-06-24T20:37:18Z

docs/design/kubectl-oadp.md

+oc oadp backup create 
+oc oadp backup logs
+oc oadp restore create
+oc oadp restore logs 


Would be nice to have some markdown code decorations for the code here.

Suggested change

oc oadp backup create

oc oadp backup logs

oc oadp restore create

oc oadp restore logs

```sh

oc oadp backup create

oc oadp backup logs

oc oadp restore create

oc oadp restore logs

```

docs/design/kubectl-oadp.md

sseago · 2025-06-29T23:55:27Z

docs/design/kubectl-oadp.md

+
+## Goals
+- Customers can create backups and restores
+- A non-cluster admin can create Non-Admin-Backups (NAB)


Would the non-admin backup/restore API calls be different? Since the CRDs are different, I imagine those would need different APIs here as well.

@sseago you are correct, however because we are limited in time based on the internship timeframe. This design is only going to cover NAB's.

Is the CLI compatibility across different OADP versions in-scope ?

How you will tell the user that the CRD is not available on the system for Non Admin use case if the user tries to use this CLI against older OADP deployment. This is of course only for NAB due to time constrains.

@weshayutin Oh, wait -- this is for NABs and not Velero backups? Oh, that wasn't clear from the mention of backups and restores. In that case, we need to be clear by referencing NABs and NARs and not Backups and Restores elsewhere.

Changed doc to make it more clear to the reader

Co-authored-by: Copilot <[email protected]>

weshayutin · 2025-06-30T17:10:53Z

@shubham-pampattiwar @mpryc please provide a review today, tomorrow so we can get this through.

docs/design/kubectl-oadp.md

shubham-pampattiwar · 2025-07-01T06:50:05Z

docs/design/kubectl-oadp.md

+
+## Non-Goals
+- Non-Admin-Restore due to time constraints
+


Lets add a use-case section too

docs/design/kubectl-oadp.md

mpryc

Clear design with small parts for further improvement (not a must, but nice to have).

docs/design/kubectl-oadp.md

mpryc · 2025-07-01T09:07:43Z

docs/design/kubectl-oadp.md

+
+## Goals
+- Customers can create backups and restores
+- A non-cluster admin can create Non-Admin-Backups (NAB)


Is the CLI compatibility across different OADP versions in-scope ?

How you will tell the user that the CRD is not available on the system for Non Admin use case if the user tries to use this CLI against older OADP deployment. This is of course only for NAB due to time constrains.

docs/design/kubectl-oadp.md

mpryc · 2025-07-01T09:11:55Z

docs/design/kubectl-oadp.md

+## High-Level Design
+Creating a kubectl plugin (kubectl-oadp) will be a good solution to the problem at hand. It will be able to create/delete backups and restores. Non-cluster admin will be able to create NABs without the need for cluster admin to do it for them. 
+
+## Detailed Design


What plugin installation process will look like?

I will leave it out of the design doc for now, but using krew for installations would be nice in the future

weshayutin · 2025-07-02T14:15:20Z

LET'S GO, move move move!

Copilot

Pull Request Overview

This PR adds a design document for the kubectl-oadp CLI plugin, outlining its purpose, architecture, and usage.

Defines goals, non-goals, and use cases for both regular and non-admin backups/restores
Presents high-level and detailed design with Go code snippets and CLI examples
Covers security considerations, compatibility, and future work

Comments suppressed due to low confidence (2)

docs/design/kubectl-oadp.md:20

Align this CLI usage with the examples below—use kubectl oadp nonadmin backup create [backupName] instead of kubectl oadp create nonadmin backup [backupname].

Creating a kubectl plugin (kubectl-oadp) will be a good solution to the problem at hand. It will be able to create/delete backups and restores. Non-cluster admin will be able to create NABs without the need for cluster admin to do it for them. A way to distinguish between creating either NABs or regular backups would be in the cli. For intstance, if you would like to create a NAB, you would have to do kubectl oadp create nonadmin backup [backupname].

docs/design/kubectl-oadp.md:72

Fix the misspelling in the error message to 'Unauthorized Access'.

The security for the plugin is controlled by OpenShift RBAC, which allows cluster admins to control permissions of users below it. This is untilized to allows users to only be allowed to use the commands that they have permission to use by their namespace. The plugin would also create error statements like "Unautherized Access" whenever a user who does not have the right permissions tries to do unautherized commands.

docs/design/kubectl-oadp.md

shubham-pampattiwar

Some NITs and suggestions here and there.
Overall looks good ! Awesome job @NicholasYancey @Joeavaikath !

docs/design/kubectl-oadp.md

Co-authored-by: Copilot <[email protected]>

Co-authored-by: Tiger Kaovilai <[email protected]>

openshift-ci · 2025-07-02T20:10:47Z

@NicholasYancey: 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.

weshayutin

Nice work here. It takes time and work to thoroughly communicate details with regards to your intentions, plans and how you intend to implement an idea. If this design were not time constrained I suspect we would have gone a few more rounds of updates and reviews.

openshift-ci · 2025-07-02T20:40:31Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kaovilai, NicholasYancey, shubham-pampattiwar, weshayutin

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [kaovilai,shubham-pampattiwar]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

sseago · 2025-07-03T15:23:52Z

/lgtm

* DNM Design For OADP CLI NVP * Crucial edits to Design Doc * Update docs/design/kubectl-oadp.md Co-authored-by: Copilot <[email protected]> * Small edits to Design Doc * Much needed changes to Design Doc * Update docs/design/kubectl-oadp.md Co-authored-by: Copilot <[email protected]> * Update docs/design/kubectl-oadp.md Co-authored-by: Copilot <[email protected]> * Update docs/design/kubectl-oadp.md Co-authored-by: Copilot <[email protected]> * Update docs/design/kubectl-oadp.md Co-authored-by: Copilot <[email protected]> * Update docs/design/kubectl-oadp.md Co-authored-by: Tiger Kaovilai <[email protected]> * Finishing touches to Design Doc * Update docs/design/kubectl-oadp.md Co-authored-by: Tiger Kaovilai <[email protected]> --------- Co-authored-by: Nicholas Yancey <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: Tiger Kaovilai <[email protected]>

DNM Design For OADP CLI NVP

a18be57

openshift-ci bot requested review from kaovilai and sseago June 24, 2025 16:29

openshift-ci bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jun 24, 2025

weshayutin added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jun 24, 2025

kaovilai reviewed Jun 24, 2025

View reviewed changes

weshayutin added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 24, 2025

Crucial edits to Design Doc

12379c0

kaovilai requested a review from Copilot June 25, 2025 22:51

This comment was marked as outdated.

Sign in to view

sseago reviewed Jun 29, 2025

View reviewed changes

Update docs/design/kubectl-oadp.md

9d2de8c

Co-authored-by: Copilot <[email protected]>

Small edits to Design Doc

5fd59fc

shubham-pampattiwar reviewed Jul 1, 2025

View reviewed changes

mpryc reviewed Jul 1, 2025

View reviewed changes

weshayutin changed the title ~~DNM Design For OADP CLI NVP~~ Design For OADP CLI NVP Jul 2, 2025

weshayutin changed the title ~~Design For OADP CLI NVP~~ Design For OADP CLI MVP Jul 2, 2025

kaovilai requested a review from Copilot July 2, 2025 14:42

This comment was marked as outdated.

Sign in to view

Much needed changes to Design Doc

79db996

kaovilai requested a review from Copilot July 2, 2025 17:08

Copilot AI reviewed Jul 2, 2025

View reviewed changes

docs/design/kubectl-oadp.md Outdated Show resolved Hide resolved

docs/design/kubectl-oadp.md Outdated Show resolved Hide resolved

docs/design/kubectl-oadp.md Outdated Show resolved Hide resolved

docs/design/kubectl-oadp.md Outdated Show resolved Hide resolved

Joeavaikath reviewed Jul 2, 2025

View reviewed changes

docs/design/kubectl-oadp.md Outdated Show resolved Hide resolved

kaovilai reviewed Jul 2, 2025

View reviewed changes

docs/design/kubectl-oadp.md Outdated Show resolved Hide resolved

docs/design/kubectl-oadp.md Outdated Show resolved Hide resolved

Joeavaikath reviewed Jul 2, 2025

View reviewed changes

docs/design/kubectl-oadp.md Show resolved Hide resolved

shubham-pampattiwar reviewed Jul 2, 2025

View reviewed changes

NicholasYancey and others added 7 commits July 2, 2025 14:00

Update docs/design/kubectl-oadp.md

f0dea65

Co-authored-by: Copilot <[email protected]>

Update docs/design/kubectl-oadp.md

0cbdcb6

Co-authored-by: Copilot <[email protected]>

Update docs/design/kubectl-oadp.md

6414563

Co-authored-by: Copilot <[email protected]>

Update docs/design/kubectl-oadp.md

2c699c8

Co-authored-by: Copilot <[email protected]>

Update docs/design/kubectl-oadp.md

c40344d

Co-authored-by: Tiger Kaovilai <[email protected]>

Finishing touches to Design Doc

c67cf83

Update docs/design/kubectl-oadp.md

ff3da2f

Co-authored-by: Tiger Kaovilai <[email protected]>

shubham-pampattiwar approved these changes Jul 2, 2025

View reviewed changes

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 2, 2025

kaovilai approved these changes Jul 2, 2025

View reviewed changes

weshayutin approved these changes Jul 2, 2025

View reviewed changes

weshayutin removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 3, 2025

openshift-ci bot assigned sseago Jul 3, 2025

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 3, 2025

weshayutin merged commit b3e7f02 into openshift:master Jul 3, 2025
4 of 6 checks passed

Design For OADP CLI MVP #1797

Design For OADP CLI MVP #1797

Uh oh!

Conversation

NicholasYancey commented Jun 24, 2025

Why the changes were made

How to test the changes made

Uh oh!

openshift-ci bot commented Jun 24, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

weshayutin commented Jun 30, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mpryc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

weshayutin commented Jul 2, 2025

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shubham-pampattiwar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

openshift-ci bot commented Jul 2, 2025

Uh oh!

weshayutin left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Jul 2, 2025