Reject NAB CR creation calls for orphaned Velero Backup objects

[shubham-pampattiwar]

Scenario:

• User creates NAB CR
• Velero Backup gets created
• User deletes NAB CR (via OC CLI)
• Velero Backup remains orphaned in cluster without any NAB CR mapping
• Now user creates another NAB CR with the same name (In the window of time where NAB-sync-controller does not get the chance to create the NAB CR in-cluster which was deleted by user)
• The user created NAB CR should be blocked from creation

Solution approach:

• Implement a Validating Admission Web-hook to reject the NAB create call from user using the data by setting the informer on Velero Backup objects.

[mpryc]

Would it make sense to add the validation webhook in OADP rather than non admin part ? This could allow such webhook to reject NAB objects when NAB controller is not installed in the cluster (something @shawn-hurley requested in the past).

[mateusoliveira43]

Issue origin #73 (review)

We are still waiting to check how Velero handles this case to apply the same solution, right?

[mpryc]

My opinion is that we don't have to have the NonAdminBackup <-> Backup interaction to be exactly same as Velero Backup. The NonAdminBackup is user facing interface which we provide, so we should make it in a way it won't surprise users.

[mateusoliveira43]

Do not forget about sync controller when implementing this

Discussion https://redhat-internal.slack.com/archives/C064QSN2YR3/p1728312889734609

[shubham-pampattiwar]

Summarizing the slack discussion thread:
In the scenario described in the issue if the actor/user is NAB sync controller then in that case we want the NAB create request to bypass the Validating Admission Webhook. So in order to that we need a way to identify wether the create NAB object request is from the user or the NAB sync controller. The proposed solution here is that we rely on 2 things:

• One is the managed by label
• Second is via checking the userInfo in admission review request, webhook should only be bypassed if the request is coming from the NAC controller service account.

[mateusoliveira43]

Is this issue still valid?

In the past, creating nab called "test", the related velero backup would always have the same name "test-123456" (as discussed here #90)

Now, each related velero backup has a different name (containing UUID)

To avoid this we would need to check UUID is really unique before applying to nonAdmin object here, right?

oadp-non-admin/internal/controller/nonadminbackup_controller.go

Line 544 in 2113181

veleroBackupNACUUID := function.GenerateNacObjectUUID(nab.Namespace, nab.Name)

This is also tracked here #102

[mpryc]

I think we can close this.

Currently there is a major rework of the deletion of NAB implemented that does not require admission webhook.

The diagram presenting how the three paths of deletion of NAB object works:

https://github.com/migtools/oadp-non-admin/blob/master/docs/design/nab_and_nar_status_update.md#status-update-scenarios

With that the only way to leave orphaned Velero Backup would be to remove finalizer from the NAB and call the oc delete on a NAB object.

With standard three deletion methods via spec fields the Velero Backup will get cleanly deleted either by NAC controller (force deletion - leaving s3 storage) or by Velero deletion request (deletion spec, causing s3 storage to be cleaned up).

With oc delete <NAB> the NAB will have condition informing that this will not remove Velero Backup and the user should use spec field instead.