Skip to content

Commit

Permalink
validator and type updates
Browse files Browse the repository at this point in the history
  • Loading branch information
@kaovilai
kaovilai committed Aug 11, 2023
1 parent 4148c32 commit 27cccab
Show file tree
Hide file tree
Showing 6 changed files with 123 additions and 13 deletions.
8 changes: 5 additions & 3 deletions api/v1alpha1/cloud_storage_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

// CloudStorage types are APIs for automatic bucket creation at cloud providers if defined name do not exists.

//+kubebuilder:object:root=true
//+kubebuilder:subresource:status

Expand All @@ -19,9 +21,9 @@ type CloudStorage struct {
type CloudStorageProvider string

const (
AWSBucketProvider CloudStorageProvider = "aws"
AzureBucketProvider CloudStorageProvider = "azure"
GCPBucketProvider CloudStorageProvider = "gcp"
AWSBucketProvider CloudStorageProvider = CloudStorageProvider(DefaultPluginAWS)
AzureBucketProvider CloudStorageProvider = CloudStorageProvider(DefaultPluginMicrosoftAzure)
GCPBucketProvider CloudStorageProvider = CloudStorageProvider(DefaultPluginGCP)
)

type CloudStorageSpec struct {
Expand Down
11 changes: 11 additions & 0 deletions api/v1alpha1/oadp_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ type ApplicationConfig struct {
Restic *ResticConfig `json:"restic,omitempty"`
}

// CloudStorageLocation defines BackupStorageLocation using bucket referenced by CloudStorage CR.
type CloudStorageLocation struct {
CloudStorageRef corev1.LocalObjectReference `json:"cloudStorageRef"`

Expand All @@ -171,6 +172,16 @@ type CloudStorageLocation struct {
// +optional
// +nullable
BackupSyncPeriod *metav1.Duration `json:"backupSyncPeriod,omitempty"`

// Prefix and CACert are copied from velero/pkg/apis/v1/backupstoragelocation_types.go under ObjectStorageLocation

// Prefix is the path inside a bucket to use for Velero storage. Optional.
// +optional
Prefix string `json:"prefix,omitempty"`

// CACert defines a CA bundle to use when verifying TLS connections to the provider.
// +optional
CACert []byte `json:"caCert,omitempty"`
}

// BackupLocation defines the configuration for the DPA backup storage
Expand Down
8 changes: 8 additions & 0 deletions bundle/manifests/oadp.openshift.io_dataprotectionapplications.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,16 @@ spec:
description: BackupLocation defines the configuration for the DPA backup storage
properties:
bucket:
description: CloudStorageLocation defines BackupStorageLocation using bucket referenced by CloudStorage CR.
properties:
backupSyncPeriod:
description: backupSyncPeriod defines how frequently to sync backup API objects from object storage. A value of 0 disables sync.
nullable: true
type: string
caCert:
description: CACert defines a CA bundle to use when verifying TLS connections to the provider.
format: byte
type: string
cloudStorageRef:
description: LocalObjectReference contains enough information to let you locate the referenced object inside the same namespace.
properties:
Expand Down Expand Up @@ -76,6 +81,9 @@ spec:
default:
description: default indicates this location is the default backup storage location.
type: boolean
prefix:
description: Prefix is the path inside a bucket to use for Velero storage. Optional.
type: string
required:
- cloudStorageRef
type: object
Expand Down
8 changes: 8 additions & 0 deletions config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,16 @@ spec:
description: BackupLocation defines the configuration for the DPA backup storage
properties:
bucket:
description: CloudStorageLocation defines BackupStorageLocation using bucket referenced by CloudStorage CR.
properties:
backupSyncPeriod:
description: backupSyncPeriod defines how frequently to sync backup API objects from object storage. A value of 0 disables sync.
nullable: true
type: string
caCert:
description: CACert defines a CA bundle to use when verifying TLS connections to the provider.
format: byte
type: string
cloudStorageRef:
description: LocalObjectReference contains enough information to let you locate the referenced object inside the same namespace.
properties:
Expand Down Expand Up @@ -76,6 +81,9 @@ spec:
default:
description: default indicates this location is the default backup storage location.
type: boolean
prefix:
description: Prefix is the path inside a bucket to use for Velero storage. Optional.
type: string
required:
- cloudStorageRef
type: object
Expand Down
24 changes: 14 additions & 10 deletions controllers/validator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,20 +37,24 @@ func (r *DPAReconciler) ValidateDataProtectionCR(log logr.Logger) (bool, error)
return false, errors.New("backupImages needs to be set to false when noDefaultBackupLocation is set")
}

if len(dpa.Spec.BackupLocations) > 0 {
for _, location := range dpa.Spec.BackupLocations {
// check for velero BSL config or cloud storage config
if location.Velero == nil && location.CloudStorage == nil {
return false, errors.New("BackupLocation must have velero or bucket configuration")
for _, location := range dpa.Spec.BackupLocations {
// check for velero BSL config or cloud storage config
if location.Velero == nil && location.CloudStorage == nil {
return false, errors.New("BackupLocation must have velero or bucket configuration")
}
if location.Velero != nil && location.Velero.ObjectStorage != nil && location.Velero.ObjectStorage.Prefix == "" && dpa.BackupImages() {
return false, errors.New("BackupLocation must have velero prefix when backupImages is set to true")
}
if location.CloudStorage != nil {
if location.CloudStorage.Prefix == "" && dpa.BackupImages() {
return false, errors.New("BackupLocation must have cloud storage prefix when backupImages is set to true")
}
}
}

if len(dpa.Spec.SnapshotLocations) > 0 {
for _, location := range dpa.Spec.SnapshotLocations {
if location.Velero == nil {
return false, errors.New("snapshotLocation velero configuration cannot be nil")
}
for _, location := range dpa.Spec.SnapshotLocations {
if location.Velero == nil {
return false, errors.New("snapshotLocation velero configuration cannot be nil")
}
}

Expand Down
77 changes: 77 additions & 0 deletions controllers/validator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ func TestDPAReconciler_ValidateDataProtectionCR(t *testing.T) {
},
},
},
BackupImages: pointer.Bool(false),
},
},
objects: []client.Object{
Expand Down Expand Up @@ -276,6 +277,7 @@ func TestDPAReconciler_ValidateDataProtectionCR(t *testing.T) {
},
},
},
BackupImages: pointer.Bool(false),
},
},
objects: []client.Object{
Expand Down Expand Up @@ -335,6 +337,7 @@ func TestDPAReconciler_ValidateDataProtectionCR(t *testing.T) {
},
},
},
BackupImages: pointer.Bool(false),
},
},
objects: []client.Object{
Expand Down Expand Up @@ -385,6 +388,7 @@ func TestDPAReconciler_ValidateDataProtectionCR(t *testing.T) {
},
},
},
BackupImages: pointer.Bool(false),
},
},
objects: []client.Object{},
Expand Down Expand Up @@ -429,6 +433,7 @@ func TestDPAReconciler_ValidateDataProtectionCR(t *testing.T) {
},
},
},
BackupImages: pointer.Bool(false),
},
},
objects: []client.Object{},
Expand Down Expand Up @@ -466,6 +471,7 @@ func TestDPAReconciler_ValidateDataProtectionCR(t *testing.T) {
},
},
},
BackupImages: pointer.Bool(false),
},
},
objects: []client.Object{
Expand Down Expand Up @@ -513,6 +519,7 @@ func TestDPAReconciler_ValidateDataProtectionCR(t *testing.T) {
},
},
},
BackupImages: pointer.Bool(false),
},
},
objects: []client.Object{
Expand Down Expand Up @@ -968,6 +975,76 @@ func TestDPAReconciler_ValidateDataProtectionCR(t *testing.T) {
wantErr: false,
want: true,
},
{
name: "If DPA CR has CloudStorageLocation without Prefix defined with backupImages enabled, error case",
dpa: &oadpv1alpha1.DataProtectionApplication{
ObjectMeta: metav1.ObjectMeta{
Name: "test-DPA-CR",
Namespace: "test-ns",
},
Spec: oadpv1alpha1.DataProtectionApplicationSpec{
BackupLocations: []oadpv1alpha1.BackupLocation{
{
CloudStorage: &oadpv1alpha1.CloudStorageLocation{
CloudStorageRef: corev1.LocalObjectReference{
Name: "testing",
},
Prefix: "",
},
},
},
Configuration: &oadpv1alpha1.ApplicationConfig{
Velero: &oadpv1alpha1.VeleroConfig{
DefaultPlugins: []oadpv1alpha1.DefaultPlugin{
},
},
},
BackupImages: pointer.Bool(true),
},
},
objects: []client.Object{
},
wantErr: true,
want: false,
},
{
name: "If DPA CR has CloudStorageLocation with Prefix defined with backupImages enabled, no error case",
dpa: &oadpv1alpha1.DataProtectionApplication{
ObjectMeta: metav1.ObjectMeta{
Name: "test-DPA-CR",
Namespace: "test-ns",
},
Spec: oadpv1alpha1.DataProtectionApplicationSpec{
BackupLocations: []oadpv1alpha1.BackupLocation{
{
CloudStorage: &oadpv1alpha1.CloudStorageLocation{
CloudStorageRef: corev1.LocalObjectReference{
Name: "testing",
},
Prefix: "some-prefix",
},
},
},
Configuration: &oadpv1alpha1.ApplicationConfig{
Velero: &oadpv1alpha1.VeleroConfig{
DefaultPlugins: []oadpv1alpha1.DefaultPlugin{
},
},
},
BackupImages: pointer.Bool(true),
},
},
objects: []client.Object{
&oadpv1alpha1.CloudStorage{
ObjectMeta: metav1.ObjectMeta{
Name: "testing",
Namespace: "test-ns",
},
},
},
wantErr: false,
want: true,
},
}
for _, tt := range tests {
tt.objects = append(tt.objects, tt.dpa)
Expand Down

0 comments on commit 27cccab

Please sign in to comment.