Non-admin Restore for OADP CLI #113
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add create and get subcommands for non-admin restores following the established pattern from backup commands. Phase 1 provides core functionality for creating restores from backups and viewing restore status, enabling users to perform restore operations through both noun-verb and verb-noun command syntax. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Joseph <jvaikath@redhat.com>
Add three new commands to complete the non-admin restore functionality: - describe: Display detailed restore information with optional --details flag for RestoreResults, RestoreItemOperations, and RestoreResourceList - logs: Stream restore operation logs using NonAdminDownloadRequest - delete: Delete restore CRs with --confirm and --all flags All commands support both noun-verb (restore describe) and verb-noun (describe restore) orderings, work with 'na' shorthand, and use the 'kubectl oadp' prefix in examples. Key features: - --details flag for additional restore information (describe only) - --request-timeout flag for download operations (describe and logs) - --confirm flag to skip confirmation prompts (delete only) - --all flag to delete all restores in namespace (delete only) - Color-coded phase display (green/yellow/red) - Restore-specific fields: backup name, namespace mappings, restore PVs - Comprehensive test coverage with 51 passing tests Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Joseph <jvaikath@redhat.com>
Closed
Signed-off-by: Joseph <jvaikath@redhat.com>
Joeavaikath
force-pushed
the
nar-phase1
branch
from
617af3b to
1e4164d
Compare
kaovilai
reviewed
Feb 5, 2026
Comment on lines
73
to
93
| type CreateOptions struct { | ||
| Name string | ||
| BackupName string | ||
| IncludeNamespaces flag.StringArray | ||
| ExcludeNamespaces flag.StringArray | ||
| IncludeResources flag.StringArray | ||
| ExcludeResources flag.StringArray | ||
| NamespaceMappings flag.Map | ||
| Labels flag.Map | ||
| Annotations flag.Map | ||
| Selector flag.LabelSelector | ||
| OrSelector flag.OrLabelSelector | ||
| RestoreVolumes flag.OptionalBool | ||
| PreserveNodePorts flag.OptionalBool | ||
| IncludeClusterResources flag.OptionalBool | ||
| ExistingResourcePolicy string | ||
| ItemOperationTimeout time.Duration | ||
| ResourceModifierConfigMap string | ||
| client kbclient.WithWatch | ||
| currentNamespace string | ||
| } |
Member
There was a problem hiding this comment.
Out of curiosity, I tried importing "github.com/vmware-tanzu/velero/pkg/cmd/cli/restore"
type CreateOptions struct {
restore.CreateOptions
Name string
client kbclient.WithWatch
currentNamespace string
}
func NewCreateOptions() *CreateOptions {
return &CreateOptions{
CreateOptions: restore.CreateOptions{
Labels: flag.NewMap(),
Annotations: flag.NewMap(),
NamespaceMappings: flag.NewMap(),
},
}
}Seems to compile. Could be anti-pattern tho..
Anyway this would add writesparsefiles and other settings which is in non-admin CRD. Thoughts on why some settings are missing?
Contributor
Author
There was a problem hiding this comment.
This seems like a neater way to do it
The reason I was writing it up from scratch was to exclude stuff non-admin would not use
This approach seems nicer, let me see if I can piggyback w/ some control
Contributor
Author
There was a problem hiding this comment.
Decided to only implement mvp flags, have READMEs around flags that are supported by nonadmin for backup and restore
If that looks fine can get this in
Fixes three issues with nonadmin commands: 1. Delete command hanging: Excluded all nonadmin commands from the output wrapper in cmd/root.go. The wrapper was capturing stdout which prevented interactive prompts (like delete confirmation) from reaching the terminal. Since we have full control over nonadmin command output, wrapping is unnecessary. 2. Backup create naming: Changed to require a name (cobra.ExactArgs(1)). The controller requires backup names to be specified upfront. 3. Restore create naming: Made name optional (cobra.MaximumNArgs(1)). When no name is provided, uses GenerateName with "restore-" prefix for auto-generation by the API server. Updated examples and builder to support this pattern. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Joseph <jvaikath@redhat.com>
Updated test expectations to match the MVP implementation: Backup tests: - Removed --assume-yes flag (no longer needed with simplified flow) - Updated create flags test to reflect MVP flag list with correct order - Added comment noting these are MVP-only flags Restore tests: - Removed --namespace-mappings (restricted for non-admin users) - Added --selector and --or-selector (MVP label selection) - Updated create flags test to reflect MVP implementation - Updated examples to show selector usage instead of namespace-mappings These changes ensure tests validate the actual MVP implementation rather than the full Velero feature set. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Joseph <jvaikath@redhat.com>
Added detailed documentation covering the OADP self-service feature and the CLI MVP implementation: 1. cmd/non-admin/backup/README.md: - Complete MVP flag reference (13 flags) - Restricted flags table with API reasons - Future enhancement flags categorized by type - Architecture notes on struct embedding pattern - Examples for common backup scenarios 2. cmd/non-admin/restore/README.md: - Complete MVP flag reference (7 flags) - Restricted flags table with API reasons - Future enhancement flags categorized by type - Architecture notes matching backup pattern - Examples for common restore scenarios 3. docs/oadp-self-service.md: - Overview of OADP self-service architecture - NAB/NAR/NABSL/NADR glossary and workflows - Cluster administrator setup instructions - User permissions and workflow examples - Admin enforceable spec fields reference - Security considerations - CLI MVP approach and implementation details The documentation provides clear guidance on: - Which flags are available in MVP vs future enhancements - Why certain flags are restricted (API limitations) - How the struct embedding pattern enables future growth - Complete alignment between CLI and API capabilities Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Joseph <jvaikath@redhat.com>
Bind factory flags to expose -n/--namespace support for admin Velero and NABSL commands. Nonadmin commands continue using kubeconfig context for security isolation. Changes: - Add f.BindFlags() to enable namespace flag on admin commands - Add namespace_flag_test.go to verify flag visibility - Fix restore test that incorrectly checked for "required" keyword Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Joseph <jvaikath@redhat.com>
kaovilai
reviewed
Feb 6, 2026
| flags.IntVar(&o.ParallelFilesUpload, "parallel-files-upload", 0, "Number of files uploads simultaneously when running a backup. This is only applicable for the kopia uploader") | ||
| // NAB-specific control flag | ||
| flags.BoolVarP(&o.Force, "force", "f", o.Force, "Force creation without specifying a storage location (uses admin defaults).") | ||
| flags.BoolVarP(&o.AssumeYes, "assume-yes", "y", o.AssumeYes, "Assume yes to all prompts and run non-interactively.") |
Contributor
Author
There was a problem hiding this comment.
was overkill
was used if you wanted to not explicitly answer yes after supplying --force with non-admin create commands
--force is used when you want to just use the default rather than supplying a storage-location (NABSL)
kaovilai
approved these changes
Feb 6, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Why the changes were made
Major documentation addition (+921 lines):
Aligned tests with MVP implementation:
Critical bug fixes (-302/+104 lines):
Completed restore functionality (+1,181 lines):
Foundation for restore commands (+1,000 lines):