From 26968b53947e1265cfd34ba787f4c4a0fe043dc1 Mon Sep 17 00:00:00 2001 From: Pawan Date: Wed, 16 Sep 2020 19:17:48 +0530 Subject: [PATCH] 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 : ...: 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 --- changelogs/unreleased/221-pawanpraka1 | 1 + deploy/yamls/zfsbackup-crd.yaml | 8 ++++++++ deploy/yamls/zfsrestore-crd.yaml | 8 ++++++++ deploy/zfs-operator.yaml | 16 +++++++++++++++ pkg/apis/openebs.io/zfs/v1/zfsbackup.go | 10 ++++----- pkg/apis/openebs.io/zfs/v1/zfsrestore.go | 8 ++++---- pkg/zfs/zfs_util.go | 26 ++++++++++++++++++------ 7 files changed, 62 insertions(+), 15 deletions(-) create mode 100644 changelogs/unreleased/221-pawanpraka1 diff --git a/changelogs/unreleased/221-pawanpraka1 b/changelogs/unreleased/221-pawanpraka1 new file mode 100644 index 0000000..3495df2 --- /dev/null +++ b/changelogs/unreleased/221-pawanpraka1 @@ -0,0 +1 @@ +adding validation for backup and restore diff --git a/deploy/yamls/zfsbackup-crd.yaml b/deploy/yamls/zfsbackup-crd.yaml index 1725170..05eedf6 100644 --- a/deploy/yamls/zfsbackup-crd.yaml +++ b/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 diff --git a/deploy/yamls/zfsrestore-crd.yaml b/deploy/yamls/zfsrestore-crd.yaml index 4726206..c1af836 100644 --- a/deploy/yamls/zfsrestore-crd.yaml +++ b/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 diff --git a/deploy/zfs-operator.yaml b/deploy/zfs-operator.yaml index ac6a94e..4e49ea3 100644 --- a/deploy/zfs-operator.yaml +++ b/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 diff --git a/pkg/apis/openebs.io/zfs/v1/zfsbackup.go b/pkg/apis/openebs.io/zfs/v1/zfsbackup.go index 3fd9513..2a18909 100644 --- a/pkg/apis/openebs.io/zfs/v1/zfsbackup.go +++ b/pkg/apis/openebs.io/zfs/v1/zfsbackup.go @@ -34,8 +34,10 @@ import ( type ZFSBackup struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` - Spec ZFSBackupSpec `json:"spec"` - Status ZFSBackupStatus `json:"status"` + Spec ZFSBackupSpec `json:"spec"` + // +kubebuilder:validation:Required + // +kubebuilder:validation:Enum=Init;Done;Failed;Pending;InProgress;Invalid + Status ZFSBackupStatus `json:"status"` } // ZFSBackupSpec is the spec for a ZFSBackup resource @@ -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" diff --git a/pkg/apis/openebs.io/zfs/v1/zfsrestore.go b/pkg/apis/openebs.io/zfs/v1/zfsrestore.go index 0037395..e832261 100644 --- a/pkg/apis/openebs.io/zfs/v1/zfsrestore.go +++ b/pkg/apis/openebs.io/zfs/v1/zfsrestore.go @@ -29,7 +29,9 @@ 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"` - Status ZFSRestoreStatus `json:"status"` + // +kubebuilder:validation:Required + // +kubebuilder:validation:Enum=Init;Done;Failed;Pending;InProgress;Invalid + Status ZFSRestoreStatus `json:"status"` } // ZFSRestoreSpec is the spec for a ZFSRestore resource @@ -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" diff --git a/pkg/zfs/zfs_util.go b/pkg/zfs/zfs_util.go index 627a15a..213394d 100644 --- a/pkg/zfs/zfs_util.go +++ b/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()