-
Notifications
You must be signed in to change notification settings - Fork 7
NAR CLI support #44
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
NAR CLI support #44
Conversation
Looks great. Do we know if the logs command has any issues similar to NAB? |
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.
Does the NAR flow work as expected with a NAB?
Is it possible to specify namespaces in restores? I don't see the create option, but there is a includenamespaceresource field so wondering about that
The flow of NAR work with NABs, the flag IncludeNamespaceScopedResources is only supposed to be in NABs, and includednamespaces and other namespace spec fields in NARs are not admin enforceable and restricted so specifying namespace should not be allowed I believe. I changed the code to remove the fields not allowed in NAR such as the one mentioned. |
cmd/non-admin/backup/describe.go
Outdated
} | ||
|
||
// NonAdminDescribeBackupDetailed provides a Velero-style detailed output format | ||
func NonAdminDescribeBackupDetailed(cmd *cobra.Command, kbClient kbclient.Client, nab *nacv1alpha1.NonAdminBackup, userNamespace string, ctx context.Context) error { |
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.
Hmm here is thought, Instead of implementing NonAdminDescribeBackupDetailed
from scratch, consider leveraging Velero's existing describe logic and filtering the output for non-admin users. Like get the describe output then stream it to a filtering function and then present the refined output for non-admin users. WDYGT ? @Joeavaikath @NicholasYancey (We will have significantly less code to maintain 😉)
Let me know your thoughts on this.
Please resolve conflicts |
Continued by #57 |
No description provided.