feat(backup,restore): adding validation for backup and restore

Added a schema validation for backup and restore CR. Also validating
the server address in the backup/restore controller.

Validating the server address as :

^([0-9]+.[0-9]+.[0-9]+.[0-9]+:[0-9]+)$

which is :

<any number>.<any number>.<any number>.<any number>:<any number>

Here we are validating just the format of the IP, not validating that IP should be
correct which  will be little more complex. In any case if IP is not correct,
the zfs send will fail, so no need to do complex validation to validate the
correct IP and port.

Signed-off-by: Pawan <pawan@mayadata.io>

changelogs/unreleased/221-pawanpraka1

@@ -0,0 +1 @@
+adding validation for backup and restore

deploy/yamls/zfsbackup-crd.yaml

@@ -65,6 +65,7 @@ spec:
             backupDest:
               description: BackupDest is the remote address for backup transfer
               minLength: 1
+              pattern: ^([0-9]+.[0-9]+.[0-9]+.[0-9]+:[0-9]+)$
               type: string
             ownerNodeID:
               description: OwnerNodeID is a name of the nodes where the source volume
@@ -90,6 +91,13 @@ spec:
           type: object
         status:
           description: ZFSBackupStatus is to hold status of backup
+          enum:
+          - Init
+          - Done
+          - Failed
+          - Pending
+          - InProgress
+          - Invalid
           type: string
       required:
       - spec

deploy/yamls/zfsrestore-crd.yaml

@@ -55,6 +55,7 @@ spec:
               description: it can be ip:port in case of restore from remote or volumeName
                 in case of local restore
               minLength: 1
+              pattern: ^([0-9]+.[0-9]+.[0-9]+.[0-9]+:[0-9]+)$
               type: string
             volumeName:
               description: volume name to where restore has to be performed
@@ -67,6 +68,13 @@ spec:
           type: object
         status:
           description: ZFSRestoreStatus is to hold result of action.
+          enum:
+          - Init
+          - Done
+          - Failed
+          - Pending
+          - InProgress
+          - Invalid
           type: string
       required:
       - spec

deploy/zfs-operator.yaml

@@ -895,6 +895,7 @@ spec:
             backupDest:
               description: BackupDest is the remote address for backup transfer
               minLength: 1
+              pattern: ^([0-9]+.[0-9]+.[0-9]+.[0-9]+:[0-9]+)$
               type: string
             ownerNodeID:
               description: OwnerNodeID is a name of the nodes where the source volume
@@ -920,6 +921,13 @@ spec:
           type: object
         status:
           description: ZFSBackupStatus is to hold status of backup
+          enum:
+          - Init
+          - Done
+          - Failed
+          - Pending
+          - InProgress
+          - Invalid
           type: string
       required:
       - spec
@@ -993,6 +1001,7 @@ spec:
               description: it can be ip:port in case of restore from remote or volumeName
                 in case of local restore
               minLength: 1
+              pattern: ^([0-9]+.[0-9]+.[0-9]+.[0-9]+:[0-9]+)$
               type: string
             volumeName:
               description: volume name to where restore has to be performed
@@ -1005,6 +1014,13 @@ spec:
           type: object
         status:
           description: ZFSRestoreStatus is to hold result of action.
+          enum:
+          - Init
+          - Done
+          - Failed
+          - Pending
+          - InProgress
+          - Invalid
           type: string
       required:
       - spec

pkg/apis/openebs.io/zfs/v1/zfsbackup.go

@@ -35,6 +35,8 @@ type ZFSBackup struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata,omitempty"`
 	Spec              ZFSBackupSpec `json:"spec"`
+	// +kubebuilder:validation:Required
+	// +kubebuilder:validation:Enum=Init;Done;Failed;Pending;InProgress;Invalid
 	Status ZFSBackupStatus `json:"status"`
 }
 
@@ -61,6 +63,7 @@ type ZFSBackupSpec struct {
 	// BackupDest is the remote address for backup transfer
 	// +kubebuilder:validation:Required
 	// +kubebuilder:validation:MinLength=1
+	// +kubebuilder:validation:Pattern="^([0-9]+.[0-9]+.[0-9]+.[0-9]+:[0-9]+)$"
 	BackupDest string `json:"backupDest"`
 }
 
@@ -69,9 +72,6 @@ type ZFSBackupStatus string
 
 // Status written onto ZFSBackup objects.
 const (
-	// BKPZFSStatusEmpty ensures the create operation is to be done, if import fails.
-	BKPZFSStatusEmpty ZFSBackupStatus = ""
-
 	// BKPZFSStatusDone , backup is completed.
 	BKPZFSStatusDone ZFSBackupStatus = "Done"
 

pkg/apis/openebs.io/zfs/v1/zfsrestore.go

@@ -29,6 +29,8 @@ type ZFSRestore struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata,omitempty"` // set name to restore name + volume name + something like csp tag
 	Spec              ZFSRestoreSpec              `json:"spec"`
+	// +kubebuilder:validation:Required
+	// +kubebuilder:validation:Enum=Init;Done;Failed;Pending;InProgress;Invalid
 	Status ZFSRestoreStatus `json:"status"`
 }
 
@@ -46,6 +48,7 @@ type ZFSRestoreSpec struct {
 	// it can be ip:port in case of restore from remote or volumeName in case of local restore
 	// +kubebuilder:validation:Required
 	// +kubebuilder:validation:MinLength=1
+	// +kubebuilder:validation:Pattern="^([0-9]+.[0-9]+.[0-9]+.[0-9]+:[0-9]+)$"
 	RestoreSrc string `json:"restoreSrc"`
 }
 
@@ -54,9 +57,6 @@ type ZFSRestoreStatus string
 
 // Status written onto CStrorRestore object.
 const (
-	// RSTZFSStatusEmpty ensures the create operation is to be done, if import fails.
-	RSTZFSStatusEmpty ZFSRestoreStatus = ""
-
 	// RSTZFSStatusDone , restore operation is completed.
 	RSTZFSStatusDone ZFSRestoreStatus = "Done"
 

pkg/zfs/zfs_util.go

@@ -297,11 +297,14 @@ func buildVolumeResizeArgs(vol *apis.ZFSVolume) []string {
 }
 
 // builldVolumeBackupArgs returns volume send command for sending the zfs volume
-func buildVolumeBackupArgs(bkp *apis.ZFSBackup, vol *apis.ZFSVolume) []string {
+func buildVolumeBackupArgs(bkp *apis.ZFSBackup, vol *apis.ZFSVolume) ([]string, error) {
 	var ZFSVolArg []string
 	backupDest := bkp.Spec.BackupDest
 
 	bkpAddr := strings.Split(backupDest, ":")
+	if len(bkpAddr) != 2 {
+		return ZFSVolArg, fmt.Errorf("zfs: invalid backup server address %s", backupDest)
+	}
 
 	curSnap := vol.Spec.PoolName + "/" + vol.Name + "@" + bkp.Spec.SnapName
 
@@ -319,24 +322,28 @@ func buildVolumeBackupArgs(bkp *apis.ZFSBackup, vol *apis.ZFSVolume) []string {
 
 	ZFSVolArg = append(ZFSVolArg, "-c", cmd)
 
-	return ZFSVolArg
+	return ZFSVolArg, nil
 }
 
 // builldVolumeRestoreArgs returns volume recv command for receiving the zfs volume
-func buildVolumeRestoreArgs(rstr *apis.ZFSRestore, vol *apis.ZFSVolume) []string {
+func buildVolumeRestoreArgs(rstr *apis.ZFSRestore, vol *apis.ZFSVolume) ([]string, error) {
 	var ZFSVolArg []string
 	restoreSrc := rstr.Spec.RestoreSrc
 
 	volume := vol.Spec.PoolName + "/" + vol.Name
 
 	rstrAddr := strings.Split(restoreSrc, ":")
+	if len(rstrAddr) != 2 {
+		return ZFSVolArg, fmt.Errorf("zfs: invalid restore server address %s", restoreSrc)
+	}
+
 	source := "nc -w 3 " + rstrAddr[0] + " " + rstrAddr[1] + " | "
 
 	cmd := source + ZFSVolCmd + " " + ZFSRecvArg + " -F " + volume
 
 	ZFSVolArg = append(ZFSVolArg, "-c", cmd)
 
-	return ZFSVolArg
+	return ZFSVolArg, nil
 }
 
 // builldVolumeDestroyArgs returns volume destroy command along with attributes as a string array
@@ -692,7 +699,10 @@ func CreateBackup(bkp *apis.ZFSBackup) error {
 		return err
 	}
 
-	args := buildVolumeBackupArgs(bkp, vol)
+	args, err := buildVolumeBackupArgs(bkp, vol)
+	if err != nil {
+		return err
+	}
 	cmd := exec.Command("bash", args...)
 	out, err := cmd.CombinedOutput()
 
@@ -741,7 +751,11 @@ func CreateRestore(rstr *apis.ZFSRestore) error {
 		return err
 	}
 	volume := vol.Spec.PoolName + "/" + vol.Name
-	args := buildVolumeRestoreArgs(rstr, vol)
+	args, err := buildVolumeRestoreArgs(rstr, vol)
+	if err != nil {
+		return err
+	}
+
 	cmd := exec.Command("bash", args...)
 	out, err := cmd.CombinedOutput()