Document NADR (NonAdminDownloadRequest) usage - #373
Conversation
Add comprehensive documentation for the NonAdminDownloadRequest (NADR) feature, which was previously undocumented despite being fully implemented. NADR lets non-admin users download logs, backup contents, and other information related to their NonAdminBackup and NonAdminRestore operations through secure, time-limited signed URLs. - docs/nadr_usage.md: usage guide covering supported download kinds, creating/monitoring requests, downloading via signed URLs, and troubleshooting - README.md: link the NADR workflow into the main user journey - hack/nadr-download.sh: helper script automating request creation, waiting for processing, and downloading the signed URL - hack/samples/downloads/: OpenShift templates for common download scenarios (backup logs, backup contents, backup resource list, restore logs) - config/samples/oadp_v1alpha1_nonadmindownloadrequest.yaml: document all supported target.kind values Fixes migtools#253. Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
📝 WalkthroughWalkthroughChangesNADR download workflow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant nadr-download.sh
participant Kubernetes API
participant Download URL
Operator->>nadr-download.sh: provide target and download options
nadr-download.sh->>Kubernetes API: verify target and create NonAdminDownloadRequest
nadr-download.sh->>Kubernetes API: wait for Processed condition
Kubernetes API-->>nadr-download.sh: return phase and signed URL
nadr-download.sh->>Download URL: download archive
nadr-download.sh-->>Operator: report download result
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR documents the existing NonAdminDownloadRequest (NADR) feature and provides ready-to-use examples/tools to help non-admin users request and download backup/restore artifacts via signed, time-limited URLs.
Changes:
- Add a comprehensive NADR usage guide describing supported target kinds, phases/conditions, and download workflows.
- Update the main README to link NADR into the NAC user flow and show example commands.
- Add an automation script plus OpenShift template samples for common download scenarios.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Adds a NADR walkthrough (create request, wait, fetch URL, download) and links to the full guide + script. |
| docs/nadr_usage.md | New detailed NADR usage guide with examples, phases/conditions, and troubleshooting. |
| hack/nadr-download.sh | New helper script to create NADR, wait for processing, and download artifacts via wget/curl. |
| hack/samples/downloads/README.md | Documents how to use the new OpenShift template samples. |
| hack/samples/downloads/backup-logs.yaml | Template to create a NADR for backup logs. |
| hack/samples/downloads/backup-contents.yaml | Template to create a NADR for backup contents. |
| hack/samples/downloads/backup-resource-list.yaml | Template to create a NADR for backup resource lists. |
| hack/samples/downloads/restore-logs.yaml | Template to create a NADR for restore logs. |
| config/samples/oadp_v1alpha1_nonadmindownloadrequest.yaml | Expands the sample with documented target.kind options and adds a component label. |
Suppressed comments (1)
README.md:106
- The script invocation example uses
<backup-name>/<namespace>placeholders, while the YAML example above uses<NonAdminBackup-name>/<non-admin-user-namespace>. Aligning the placeholders makes it clearer that-nexpects the NonAdminBackup name and-nsexpects the user namespace.
./hack/nadr-download.sh -k BackupLog -n <backup-name> -ns <namespace>
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@hack/nadr-download.sh`:
- Around line 129-142: Replace the substring-based KIND condition in the
backup/restore existence-check block with an exact case allowlist for the
documented backup and restore kinds, including their supported casing. Reject
unsupported values such as BackupTypo with an error and exit before any oc get
call, while preserving the existing nab path for backup kinds and nar path for
restore kinds.
- Around line 146-160: Ensure NADR creation rejects an existing request name
instead of reapplying it: in hack/nadr-download.sh lines 146-160, change the
manifest submission to oc create -f -. Update hack/samples/downloads/README.md
lines 24-28, 34-37, and 43-46 to use oc create -f - and tell users to choose a
new request name for each download; update docs/nadr_usage.md lines 102-104 and
README.md lines 79-89 to use oc create, with the fixed-name example also using
oc create -f -.
In `@hack/samples/downloads/README.md`:
- Line 70: Update the NADR Usage Guide link in the README to use
../../../docs/nadr_usage.md so it resolves to the repository-level
documentation.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 59b42218-9c9b-417a-832f-973d029e4a4e
📒 Files selected for processing (9)
README.mdconfig/samples/oadp_v1alpha1_nonadmindownloadrequest.yamldocs/nadr_usage.mdhack/nadr-download.shhack/samples/downloads/README.mdhack/samples/downloads/backup-contents.yamlhack/samples/downloads/backup-logs.yamlhack/samples/downloads/backup-resource-list.yamlhack/samples/downloads/restore-logs.yaml
- Fix broken NADR Usage Guide relative link from hack/samples/downloads/ and hack/nadr-download.sh's own comment path - Use a single <non-admin-user-namespace> placeholder consistently in the README example instead of mixing it with <namespace> - nadr-download.sh: validate --kind against an exact allowlist of documented target kinds instead of a loose substring match - nadr-download.sh: use 'oc create' instead of 'oc apply' when creating the NonAdminDownloadRequest, and same for the sample README/doc examples, so re-running against an existing request name fails instead of silently reusing a stale processed request - nadr-download.sh: capture the download command's exit status explicitly instead of relying on $? after 'set -e', which made the success/failure branch unreachable on a failed download Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kaovilai, sseago The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/lgtm |
Why the changes were made
The NonAdminDownloadRequest (NADR) feature lets non-admin users download logs, backup contents, and other information related to their NonAdminBackup and NonAdminRestore operations through secure, time-limited signed URLs. It was previously undocumented despite being fully implemented.
This re-opens #307 (closed) with the same content, rebased onto current
oadp-dev. Verified against current source (internal/controller/nonadmindownloadrequest_controller.go,api/v1alpha1/nonadmindownloadrequest_types.go) that the documentedtarget.kindvalues, phases, and condition types are still accurate/up to date.Adds:
docs/nadr_usage.md- usage guide covering supported download kinds, creating/monitoring requests, downloading via signed URLs, and troubleshootingREADME.md- links the NADR workflow into the main user journeyhack/nadr-download.sh- helper script automating request creation, waiting for processing, and downloading the signed URLhack/samples/downloads/- OpenShift templates for common download scenarios (backup logs, backup contents, backup resource list, restore logs)config/samples/oadp_v1alpha1_nonadmindownloadrequest.yaml- documents all supportedtarget.kindvaluesFixes #253.
How to test the changes made
Mostly documentation, but also adds an executable helper script (
hack/nadr-download.sh) and OpenShift templates (hack/samples/downloads/), so those need their own validation:hack/nadr-download.sh: syntax-checked withbash -n, and argument parsing/validation smoke-tested (-h, missing required args, unsupported--kindvalue all exit/print as expected). Full end-to-end download still needs a real cluster with OADP + NAC installed: create a NonAdminBackup using a NonAdminBackupStorageLocation, then run the script against it and confirm the signed URL download succeeds.hack/samples/downloads/*.yaml: OpenShiftTemplateobjects, validate withoc process -f <file>.yaml -p NAMESPACE=... -p BACKUP_NAME=... --dry-run=client.docs/nadr_usage.md/README.md: follow the examples against a live cluster to confirm commands and links are correct.Note
Responses generated with Claude