Skip to content
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

Ensure non-admin backups are namespace scoped #49

Closed
weshayutin opened this issue Apr 23, 2024 · 8 comments
Closed

Ensure non-admin backups are namespace scoped #49

weshayutin opened this issue Apr 23, 2024 · 8 comments
Assignees

Comments

@weshayutin
Copy link
Contributor

With a valid namespace set and credentials a non-admin user is able to take a backup of cr's outside the scope of the user's namespace(s).

apiVersion: nac.oadp.openshift.io/v1alpha1
kind: NonAdminBackup
metadata:
  name: empty-backup-spec
  namespace: mysql-persistent
spec:
  backupSpec: {}

results in:

status:
  conditions:
  - lastTransitionTime: "2024-04-23T18:48:23Z"
    message: backup accepted
    reason: BackupAccepted
    status: "True"
    type: Accepted
  - lastTransitionTime: "2024-04-23T18:48:33Z"
    message: Created Velero Backup object
    reason: BackupScheduled
    status: "True"
    type: Queued
  phase: Created
  veleroBackupName: nab-mysql-persistent-e33e97f12f2303
  veleroBackupNamespace: openshift-adp
  veleroBackupStatus:
    backupItemOperationsAttempted: 62
    backupItemOperationsCompleted: 60
    csiVolumeSnapshotsAttempted: 31
    csiVolumeSnapshotsCompleted: 24
    expiration: "2024-05-23T18:48:33Z"
    formatVersion: 1.1.0
    phase: WaitingForPluginOperations
    progress:
      itemsBackedUp: 8241
      totalItems: 8241
    startTimestamp: "2024-04-23T18:48:33Z"
    version: 1

A properly namespaced backup should have only 83 items in this case.

status:
  conditions:
  - lastTransitionTime: "2024-04-23T18:38:54Z"
    message: backup accepted
    reason: BackupAccepted
    status: "True"
    type: Accepted
  - lastTransitionTime: "2024-04-23T18:39:04Z"
    message: Created Velero Backup object
    reason: BackupScheduled
    status: "True"
    type: Queued
  phase: Created
  veleroBackupName: nab-mysql-persistent-a0a611237fcdb4
  veleroBackupNamespace: openshift-adp
  veleroBackupStatus:
    backupItemOperationsAttempted: 2
    backupItemOperationsCompleted: 2
    completionTimestamp: "2024-04-23T18:39:17Z"
    csiVolumeSnapshotsAttempted: 1
    csiVolumeSnapshotsCompleted: 1
    expiration: "2024-05-23T18:39:04Z"
    formatVersion: 1.1.0
    phase: Completed
    progress:
      itemsBackedUp: 83
      totalItems: 83
    startTimestamp: "2024-04-23T18:39:04Z"
    version: 1
@shubham-pampattiwar
Copy link
Member

This will be part of the backupspec control list: #37

@mpryc
Copy link
Collaborator

mpryc commented Apr 23, 2024

Thanks for that. The function to perform other checks is already defined, just need more control of what's permitted:

https://github.com/migtools/oadp-non-admin/pull/13/files#diff-de455907ed1c5413fa6248ebea483af559585f8f6277a7c8b370813b9bb254f9R81

@mateusoliveira43
Copy link
Contributor

@shubham-pampattiwar I think this is not #37 responsibility. There, admin user can, for example, restrict type of backups allowed. But checking if non admin user has permissions over the requested items to be back upped, should be NAC responsibility, not ❓

@mpryc
Copy link
Collaborator

mpryc commented Apr 24, 2024

It is controller responsibility so the user should never create VeleroBackup pointing to a namespace different then NonAdminBackup

@mateusoliveira43
Copy link
Contributor

@mpryc could non admin user has more than one namespace, and then ask NonAdminBackup to back up all labels, and expect NAC to back up all objects which have that label across all its namespaces ❓ Or this is out of scope ❓

@mpryc
Copy link
Collaborator

mpryc commented Apr 24, 2024

Very much out of scope. The user may create automation to gather all the objects with the labels and create NAB per namespace where the objects resides.

The objects from outside of the namespace can and will be backed up if velero decides to (e.g. non namespace objects that are required during backup).

@mateusoliveira43
Copy link
Contributor

@mpryc is this fixed?

@mpryc
Copy link
Collaborator

mpryc commented May 14, 2024

@mateusoliveira43 yes with the merge of #56

@mpryc mpryc closed this as completed May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Merged / Ready for build
Development

No branches or pull requests

4 participants