Fix for #89 - Use custom UUID for NAB Object

api/v1alpha1/nonadminbackup_types.go

@@ -51,9 +51,9 @@ type VeleroBackup struct {
 	// +optional
 	Status *velerov1.BackupStatus `json:"status,omitempty"`
 
-	// name references the Velero backup by it's name.
+	// nameuuid references the Velero Backup object by it's label containing same NameUUID.
 	// +optional
-	Name string `json:"name,omitempty"`
+	NameUUID string `json:"nameuuid,omitempty"`
 
 	// namespace references the Namespace in which Velero backup exists.
 	// +optional

config/crd/bases/nac.oadp.openshift.io_nonadminbackups.yaml

@@ -612,13 +612,14 @@ spec:
                 description: VeleroBackup contains information of the related Velero
                   backup object.
                 properties:
-                  name:
-                    description: name references the Velero backup by it's name.
-                    type: string
                   namespace:
                     description: namespace references the Namespace in which Velero
                       backup exists.
                     type: string
+                  nameuuid:
+                    description: nameuuid references the Velero Backup object by it's
+                      label containing same NameUUID.
+                    type: string
                   status:
                     description: status captures the current status of the Velero
                       backup.

docs/design/Non_Admin_Controller_design.md

@@ -139,17 +139,17 @@ This design intends to enable non-admin users the ability to perform Backup and
       backupSpec: {}
     ```
     - **NAB controller reconciles on this NAB CR:** The NonAdminBackup controller continuously reconciles the NonAdminBackup object's desired state with the actual state in the cluster.
-    - **NAB controller validates the NAB CR and then creates a corresponding Velero Backup CR:** When the NonAdminBackup controller detects a new or modified NonAdminBackup object, it creates or updates a corresponding Velero Backup object within the OADP Namespace using the information provided in the `backupSpec` field of the NonAdminBackup object. The resulting Backup object is named as `nab-<namespace>-<hash>`, where the `<namespace>` is the NonAdminBackup namespace and the `<hash>` is computed from the original NonAdminBackup name. The resulting Backup object is labeled and annotated with the following additional metadata:
+    - **NAB controller validates the NAB CR and then creates a corresponding Velero Backup CR:** When the NonAdminBackup controller detects a new or modified NonAdminBackup object, it creates or updates a corresponding Velero Backup object within the OADP Namespace using the information provided in the `backupSpec` field of the NonAdminBackup object. The resulting Backup object is named as `<namespace>-<name>-<hash>`, where the `<namespace>` is the NonAdminBackup namespace, `<name>` is the NonAdminBackup name and the `<hash>` is the generated UUID (version 4). If the resulting Backup object name is too long the name is adapted to 63 characters limit strpping first characters from name and then from namespace. The resulting Backup object is labeled and annotated with the following additional metadata:
 
     ```yaml
     metadata:
       annotations:
         openshift.io/oadp-nab-origin-name: <NonAdminBackup name>
         openshift.io/oadp-nab-origin-namespace: <NonAdminBackup Namespace>
-        openshift.io/oadp-nab-origin-uuid: <NonAdminBackup UUID>
       labels:
         app.kubernetes.io/managed-by: <OADP NonAdminController id>
         openshift.io/oadp: 'True'
+        openshift.io/oadp-nab-origin-nameuuid: <NonAdminBackup's NameUUID from Status>
     ```
     - **Velero runs Backup**: Velero executes the backup operation based on the configuration specified in the Velero Backup object. Velero updates the status of the Velero Backup object to reflect the outcome of the backup process.
     - **Reconcile loop updates NonAdminBackup object Status**: Upon detecting changes in the status of the Velero Backup object, the NonAdminBackup controller's reconciliation loop updates the Status field of the corresponding NonAdminBackup object with the updated status from the Velero Backup object.
@@ -162,17 +162,17 @@ This design intends to enable non-admin users the ability to perform Backup and
 - **Backup Sync controller syncs the Non-admin Backup CRs:** The Backup-Sync controller ensures that the NS relevant backups are synced and Non-admin backup CRs exists for non-admin users to refer them for restore operations.
 - **Non-Admin user creates the Non-Admin restore CR:** The user creates NonAdminRestore custom resource object in the Namespace on which the restore will run within the Kubernetes cluster. The `NonAdminRestore` schema has the `restoreSpec`, which is the same as `Restore` CR from the `velero.io/v1` apiVersion. Note that the non-admin user will use non-admin backup name (and not Velero backup name) in non-admin restore spec.
 - **NAR controller reconciles on this NAR CR:** The NonAdminRestore controller continuously reconciles the NonAdminRestore object's desired state with the actual state in the cluster.
-- **NAR controller validates the NAR CR and then creates a corresponding Velero Restore CR:** When the NonAdminRestore controller detects a new or modified NonAdminRestore object, it creates the corresponding Velero Restore object within the OADP Namespace using the information provided in the `restoreSpec` field of the NonAdminRestore object. The resulting Restore object is named as `nar-<namespace>-<hash>`, where the `<namespace>` is the NonAdminRestore namespace and the `<hash>` is computed from the original NonAdminRestore name. The resulting Restore object is labeled and annotated with the following additional metadata:
+- **NAR controller validates the NAR CR and then creates a corresponding Velero Restore CR:** When the NonAdminRestore controller detects a new or modified NonAdminRestore object, it creates the corresponding Velero Restore object within the OADP Namespace using the information provided in the `restoreSpec` field of the NonAdminRestore object. The resulting Restore object is named as `<namespace>-<name>-<hash>`, where the `<namespace>` is the NonAdminRestore namespace, `<name>` is the NonAdminRestore name and the `<hash>` is the generated UUID (version 4). If the resulting Restore object name is too long the name is adapted to 63 characters limit strpping first characters from name and then from namespace. The resulting Restore object is labeled and annotated with the following additional metadata:
 
   ```yaml
   metadata:
     annotations:
       openshift.io/oadp-nar-origin-name: <NonAdminRestore name>
       openshift.io/oadp-nar-origin-namespace: <NonAdminRestore Namespace>
-      openshift.io/oadp-nar-origin-uuid: <NonAdminRestore UUID>
     labels:
       app.kubernetes.io/managed-by: <OADP NonAdminController id>
       openshift.io/oadp: 'True'
+      openshift.io/oadp-nar-origin-nameuuid: <NonAdminRestore's NameUUID from Status>
   ```
 - **Velero runs Restore**: Velero executes the restore operation based on the configuration specified in the Velero Restore object. Velero updates the status of the Velero Restore object to reflect the outcome of the restore process.
 - **Reconcile loop updates NonAdminRestore object Status**: Upon detecting changes in the status of the Velero Restore object, the NonAdminRestore controller's reconciliation loop updates the Status field of the corresponding NonAdminRestore object with the updated status from the Velero Restore object.

docs/design/nab_and_nar_status_update.md

@@ -56,8 +56,8 @@ Those are are the possible values for `NonAdminCondition`:
 
 NonAdminBackup/NonAdminRestore `status` contains reference to the related Velero Backup/Restore.
 
-NonAdminBackup `status.veleroBackup` contains `name`, `namespace` and `status`.
-- `status.veleroBackup.name` represents the name of the `VeleroBackup` object.
+NonAdminBackup `status.veleroBackup` contains `nameuuid`, `namespace` and `status`.
+- `status.veleroBackup.nameuuid` field stores generated unique UUID of the `VeleroBackup` object. The same UUID is also stored as the label value `openshift.io/oadp-nab-origin-nameuuid` within the created `VeleroBackup` object.
 - `status.veleroBackup.namespace` represents the namespace in which the `VeleroBackup` object was created.
 - `status.veleroBackup.status` field is a copy of the `VeleroBackup` object status.
 
@@ -76,10 +76,10 @@ status:
 velero backup describe -n openshift-adp nab-nacproject-c3499c2729730a
 ```
 
-Similarly, NonAdminRestore `status.veleroRestore` contains `name`, `namespace` and `status`.
-- `status.veleroRestore.name` represents the name of the `veleroRestore` object.
+Similarly, NonAdminRestore `status.veleroRestore` contains `nameuuid`, `namespace` and `status`.
+- `status.veleroRestore.nameuuid` field stores generated unique UUID of the `VeleroRestore` object. The same UUID is also stored as the label value `openshift.io/oadp-nar-origin-nameuuid` within the created `VeleroRestore` object.
 - `status.veleroRestore.namespace` represents the namespace in which the `veleroRestore` object was created.
-- `status.veleroRestore.status` field is a copy of the `VeleroBackup` object status.
+- `status.veleroRestore.status` field is a copy of the `VeleroRestore` object status.
 
 ## Example
 
@@ -91,7 +91,7 @@ Object passed validation and Velero `Backup` object was created, but there was a
 ```yaml
 status:
   veleroBackup:
-    name: nab-nacproject-83fc04a2fd253d
+    nameuuid: nonadmin-test-86b8d92b-66b2-11e4-8a2d-42010af06f3f
     namespace: openshift-adp
     status:
       expiration: '2024-05-16T08:12:11Z'
@@ -135,24 +135,33 @@ reconcileExit1[\Requeue: true, nil/]
 reconcileExit1 ==> question(is NonAdminBackup Spec valid?) == yes ==> reconcileStart2
 question == no ==> reconcileStartInvalid1
 
+
+
 reconcileStart2[/Reconcile start\] ==>
 
 conditionsAcceptedTrue["`status.conditions[Accepted] to **True**`"] -. Requeue: false, err .- reconcileStart2
+
 conditionsAcceptedTrue ==>
 
 reconcileExit2[\Requeue: true, nil/] ==>
 
 reconcileStart3[/Reconcile start\] ==>
 
-createVeleroBackup([Create Velero Backup]) -. Requeue: false, err .- reconcileStart3
+setVeleroBackupUUID([Set status.veleroBackup.nameUUID]) -. Requeue: false, err .- reconcileStart3
+
+setVeleroBackupUUID ==>
+
+reconcileStart4[/Reconcile start\] ==>
+
+createVeleroBackup([Create Velero Backup]) -. Requeue: false, err .- reconcileStart4
 createVeleroBackup ==>
 
 phaseCreated["`
 status.phase: **Created**
 status.conditions[Queued] to **True**
 status.conditions.veleroBackup.name
 status.conditions.veleroBackup.namespace
-`"] -. Requeue: false, err .- reconcileStart3
+`"] -. Requeue: false, err .- reconcileStart4
 phaseCreated ==>
 
 reconcileExit4[\Requeue: false, nil/]

internal/common/constant/constant.go

@@ -17,6 +17,8 @@ limitations under the License.
 // Package constant contains all common constants used in the project
 package constant
 
+import "k8s.io/apimachinery/pkg/util/validation"
+
 // Common labels for objects manipulated by the Non Admin Controller
 // Labels should be used to identify the NAC object
 // Annotations on the other hand should be used to define ownership
@@ -28,7 +30,8 @@ const (
 	ManagedByLabelValue          = "oadp-nac-controller" // TODO why not use same project name as in PROJECT file?
 	NabOriginNameAnnotation      = "openshift.io/oadp-nab-origin-name"
 	NabOriginNamespaceAnnotation = "openshift.io/oadp-nab-origin-namespace"
-	NabOriginUUIDAnnotation      = "openshift.io/oadp-nab-origin-uuid"
+	NabOriginNameUUIDLabel       = "openshift.io/oadp-nab-origin-nameuuid"
+	NarOriginNameUUIDLabel       = "openshift.io/oadp-nar-origin-nameuuid"
 )
 
 // Common environment variables for the Non Admin Controller
@@ -39,9 +42,12 @@ const (
 // EmptyString defines a constant for the empty string
 const EmptyString = ""
 
+// NameDelimiter defines character that is used to separate name parts
+const NameDelimiter = "-"
+
 // TrueString defines a constant for the True string
 const TrueString = "True"
 
-// VeleroBackupNamePrefix represents the prefix for the object name generated
-// by the NonAdminController
-const VeleroBackupNamePrefix = "nab"
+// MaximumNacObjectNameLength represents Generated Non Admin Object Name and
+// must be below 63 characters, because it's used within object Label Value
+const MaximumNacObjectNameLength = validation.DNS1123LabelMaxLength

internal/common/function/function.go

@@ -19,12 +19,13 @@ package function
 
 import (
 	"context"
-	"crypto/sha256"
-	"encoding/hex"
 	"fmt"
 
 	"github.com/go-logr/logr"
+	"github.com/google/uuid"
+	velerov1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/validation"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -47,7 +48,6 @@ func GetNonAdminBackupAnnotations(objectMeta metav1.ObjectMeta) map[string]strin
 	return map[string]string{
 		constant.NabOriginNamespaceAnnotation: objectMeta.Namespace,
 		constant.NabOriginNameAnnotation:      objectMeta.Name,
-		constant.NabOriginUUIDAnnotation:      string(objectMeta.UID),
 	}
 }
 
@@ -77,38 +77,101 @@ func ValidateBackupSpec(nonAdminBackup *nacv1alpha1.NonAdminBackup) error {
 	return nil
 }
 
-// GenerateVeleroBackupName generates a Velero backup name based on the provided namespace and NonAdminBackup name.
-// It calculates a hash of the NonAdminBackup name and combines it with the namespace and a prefix to create the Velero backup name.
-// If the resulting name exceeds the maximum Kubernetes name length, it truncates the namespace to fit within the limit.
-func GenerateVeleroBackupName(namespace, nabName string) string {
-	// Calculate a hash of the name
-	hasher := sha256.New()
-	_, err := hasher.Write([]byte(nabName))
-	if err != nil {
-		return ""
+// GenerateNacObjectNameWithUUID generates a unique name based on the provided namespace and object origin name.
+// It includes a UUID suffix. If the name exceeds the maximum length, it truncates nacName first, then namespace.
+func GenerateNacObjectNameWithUUID(namespace, nacName string) string {
+	// Generate UUID suffix
+	uuidSuffix := uuid.New().String()
+
+	// Build the initial name based on the presence of namespace and nacName
+	nacObjectName := uuidSuffix
+	if len(nacName) > 0 {
+		nacObjectName = nacName + constant.NameDelimiter + nacObjectName
+	}
+	if len(namespace) > 0 {
+		nacObjectName = namespace + constant.NameDelimiter + nacObjectName
+	}
+
+	if len(nacObjectName) > constant.MaximumNacObjectNameLength {
+		// Calculate remaining length after UUID
+		remainingLength := constant.MaximumNacObjectNameLength - len(uuidSuffix)
+
+		delimeterLength := len(constant.NameDelimiter)
+
+		// Subtract two delimiter lengths to avoid a corner case where the namespace
+		// and delimiters leave no space for any part of nabName
+		if len(namespace) > remainingLength-delimeterLength-delimeterLength {
+			namespace = namespace[:remainingLength-delimeterLength-delimeterLength]
+			nacObjectName = namespace + constant.NameDelimiter + uuidSuffix
+		} else {
+			remainingLength = remainingLength - len(namespace) - delimeterLength - delimeterLength
+			nacName = nacName[:remainingLength]
+			nacObjectName = uuidSuffix
+			if len(nacName) > 0 {
+				nacObjectName = nacName + constant.NameDelimiter + nacObjectName
+			}
+			if len(namespace) > 0 {
+				nacObjectName = namespace + constant.NameDelimiter + nacObjectName
+			}
+		}
+	}
+
+	return nacObjectName
+}
+
+// ListLabeledObjectsInNamespace retrieves a list of Kubernetes objects in a specified namespace
+// that match a given label key-value pair.
+func ListLabeledObjectsInNamespace(ctx context.Context, clientInstance client.Client, namespace string, labelKey string, labelValue string, objectList client.ObjectList) error {
+	// Validate input parameters
+	if namespace == constant.EmptyString || labelKey == constant.EmptyString || labelValue == constant.EmptyString {
+		return fmt.Errorf("invalid input: namespace, labelKey, and labelValue must not be empty")
 	}
 
-	const hashLength = 14
-	nameHash := hex.EncodeToString(hasher.Sum(nil))[:hashLength] // Take first 14 chars
+	labelSelector := labels.SelectorFromSet(labels.Set{labelKey: labelValue})
 
-	usedLength := hashLength + len(constant.VeleroBackupNamePrefix) + len("--")
-	maxNamespaceLength := validation.DNS1123SubdomainMaxLength - usedLength
-	// Ensure the name is within the character limit
-	if len(namespace) > maxNamespaceLength {
-		// Truncate the namespace if necessary
-		return fmt.Sprintf("%s-%s-%s", constant.VeleroBackupNamePrefix, namespace[:maxNamespaceLength], nameHash)
+	// Attempt to list objects with the specified label
+	if err := clientInstance.List(ctx, objectList, &client.ListOptions{
+		LabelSelector: labelSelector,
+		Namespace:     namespace,
+	}); err != nil {
+		return fmt.Errorf("failed to list objects in namespace '%s': %w", namespace, err)
 	}
 
-	return fmt.Sprintf("%s-%s-%s", constant.VeleroBackupNamePrefix, namespace, nameHash)
+	return nil
+}
+
+// GetVeleroBackupByLabel retrieves a VeleroBackup object based on a specified label within a given namespace.
+// It returns the VeleroBackup only when exactly one object is found, throws an error if multiple backups are found,
+// or returns nil if no matches are found.
+func GetVeleroBackupByLabel(ctx context.Context, clientInstance client.Client, namespace string, labelValue string) (*velerov1.Backup, error) {
+	veleroBackupList := &velerov1.BackupList{}
+
+	// Call the generic ListLabeledObjectsInNamespace function
+	if err := ListLabeledObjectsInNamespace(ctx, clientInstance, namespace, constant.NabOriginNameUUIDLabel, labelValue, veleroBackupList); err != nil {
+		return nil, err
+	}
+
+	switch len(veleroBackupList.Items) {
+	case 0:
+		return nil, nil // No matching VeleroBackup found
+	case 1:
+		return &veleroBackupList.Items[0], nil // Found 1 matching VeleroBackup
+	default:
+		return nil, fmt.Errorf("multiple VeleroBackup objects found with label %s=%s in namespace '%s'", constant.NabOriginNameUUIDLabel, labelValue, namespace)
+	}
 }
 
 // CheckVeleroBackupMetadata return true if Velero Backup object has required Non Admin labels and annotations, false otherwise
 func CheckVeleroBackupMetadata(obj client.Object) bool {
-	labels := obj.GetLabels()
-	if !checkLabelValue(labels, constant.OadpLabel, constant.OadpLabelValue) {
+	objLabels := obj.GetLabels()
+	if !checkLabelValue(objLabels, constant.OadpLabel, constant.OadpLabelValue) {
 		return false
 	}
-	if !checkLabelValue(labels, constant.ManagedByLabel, constant.ManagedByLabelValue) {
+	if !checkLabelValue(objLabels, constant.ManagedByLabel, constant.ManagedByLabelValue) {
+		return false
+	}
+
+	if !checkLabelValueIsValid(objLabels, constant.NabOriginNameUUIDLabel) {
 		return false
 	}
 
@@ -119,16 +182,12 @@ func CheckVeleroBackupMetadata(obj client.Object) bool {
 	if !checkAnnotationValueIsValid(annotations, constant.NabOriginNameAnnotation) {
 		return false
 	}
-	// TODO what is a valid uuid?
-	if !checkAnnotationValueIsValid(annotations, constant.NabOriginUUIDAnnotation) {
-		return false
-	}
 
 	return true
 }
 
-func checkLabelValue(labels map[string]string, key string, value string) bool {
-	got, exists := labels[key]
+func checkLabelValue(objLabels map[string]string, key string, value string) bool {
+	got, exists := objLabels[key]
 	if !exists {
 		return false
 	}
@@ -141,7 +200,16 @@ func checkAnnotationValueIsValid(annotations map[string]string, key string) bool
 		return false
 	}
 	length := len(value)
-	return length > 0 && length < validation.DNS1123SubdomainMaxLength
+	return length > 0
+}
+
+func checkLabelValueIsValid(objLabels map[string]string, key string) bool {
+	value, exists := objLabels[key]
+	if !exists {
+		return false
+	}
+	length := len(value)
+	return length > 0 && length < validation.DNS1123LabelMaxLength
 }
 
 // GetLogger return a logger from input ctx, with additional key/value pairs being