Skip to content

Require region in BSL validation when s3Url is set #2109

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kaovilai wants to merge 1 commit into
base: oadp-dev
Choose a base branch
from
Open

Require region in BSL validation when s3Url is set #2109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions internal/controller/bsl.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -527,12 +527,16 @@ func (r *DataProtectionApplicationReconciler) validateAWSBackupStorageLocation(b
}

// BSL region is required when
// - s3Url is set, because the user is pointing to a non-AWS S3-compatible service
// where region auto-discovery via AWS API is not valid
// - s3ForcePathStyle is true, because some velero processes requires region to be set and is not auto-discoverable when s3ForcePathStyle is true
// imagestream backup in openshift-velero-plugin now uses the same method to discover region as the rest of the velero codebase
// - even when s3ForcePathStyle is false, some aws bucket regions may not be discoverable and the user has to set it manually
if (bslSpec.Config == nil || len(bslSpec.Config[Region]) == 0) &&
(bslSpec.Config != nil && bslSpec.Config[S3ForcePathStyle] == "true" || !aws.BucketRegionIsDiscoverable(bslSpec.ObjectStorage.Bucket)) {
return fmt.Errorf("region for AWS backupstoragelocation not automatically discoverable. Please set the region in the backupstoragelocation config")
(bslSpec.Config != nil && len(bslSpec.Config[S3URL]) > 0 ||
bslSpec.Config != nil && bslSpec.Config[S3ForcePathStyle] == "true" ||
!aws.BucketRegionIsDiscoverable(bslSpec.ObjectStorage.Bucket)) {
return fmt.Errorf("region for AWS backupstoragelocation is required. Please set the region in the backupstoragelocation config")
}

//TODO: Add minio, noobaa, local storage validations
Expand Down
125 changes: 125 additions & 0 deletions internal/controller/bsl_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1459,6 +1459,131 @@ func TestDPAReconciler_ValidateBackupStorageLocations(t *testing.T) {
want: true,
wantErr: false,
},
{
name: "BSL with s3Url and no region expect to fail",
dpa: &oadpv1alpha1.DataProtectionApplication{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: "test-ns",
},
Spec: oadpv1alpha1.DataProtectionApplicationSpec{
Configuration: &oadpv1alpha1.ApplicationConfig{
Velero: &oadpv1alpha1.VeleroConfig{},
},
BackupImages: ptr.To(false),
BackupLocations: []oadpv1alpha1.BackupLocation{
{
Velero: &velerov1.BackupStorageLocationSpec{
Provider: "aws",
Config: map[string]string{
S3URL: "https://s3.us-south.cloud-object-storage.appdomain.cloud",
},
StorageType: velerov1.StorageType{
ObjectStorage: &velerov1.ObjectStorageLocation{
Bucket: DiscoverableBucket,
Prefix: "prefix",
},
},
Default: true,
},
},
},
},
},
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "cloud-credentials",
Namespace: "test-ns",
},
Data: map[string][]byte{"cloud": []byte("[default]\naws_access_key_id=AKIAIOSFODNN7EXAMPLE\naws_secret_access_key=wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY")},
},
want: false,
wantErr: true,
},
{
name: "BSL with s3Url and region set expect to succeed",
dpa: &oadpv1alpha1.DataProtectionApplication{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: "test-ns",
},
Spec: oadpv1alpha1.DataProtectionApplicationSpec{
Configuration: &oadpv1alpha1.ApplicationConfig{
Velero: &oadpv1alpha1.VeleroConfig{},
},
BackupImages: ptr.To(false),
BackupLocations: []oadpv1alpha1.BackupLocation{
{
Velero: &velerov1.BackupStorageLocationSpec{
Provider: "aws",
Config: map[string]string{
S3URL: "https://s3.us-south.cloud-object-storage.appdomain.cloud",
Region: "us-south",
},
StorageType: velerov1.StorageType{
ObjectStorage: &velerov1.ObjectStorageLocation{
Bucket: "ibm-cos-bucket",
Prefix: "prefix",
},
},
Default: true,
},
},
},
},
},
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "cloud-credentials",
Namespace: "test-ns",
},
Data: map[string][]byte{"cloud": []byte("[default]\naws_access_key_id=AKIAIOSFODNN7EXAMPLE\naws_secret_access_key=wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY")},
},
want: true,
wantErr: false,
},
{
name: "BSL with s3Url and s3ForcePathStyle but no region expect to fail",
dpa: &oadpv1alpha1.DataProtectionApplication{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: "test-ns",
},
Spec: oadpv1alpha1.DataProtectionApplicationSpec{
Configuration: &oadpv1alpha1.ApplicationConfig{
Velero: &oadpv1alpha1.VeleroConfig{},
},
BackupImages: ptr.To(false),
BackupLocations: []oadpv1alpha1.BackupLocation{
{
Velero: &velerov1.BackupStorageLocationSpec{
Provider: "aws",
Config: map[string]string{
S3URL: "https://s3.us-south.cloud-object-storage.appdomain.cloud",
S3ForcePathStyle: "true",
},
StorageType: velerov1.StorageType{
ObjectStorage: &velerov1.ObjectStorageLocation{
Bucket: "ibm-cos-bucket",
Prefix: "prefix",
},
},
Default: true,
},
},
},
},
},
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "cloud-credentials",
Namespace: "test-ns",
},
Data: map[string][]byte{"cloud": []byte("[default]\naws_access_key_id=AKIAIOSFODNN7EXAMPLE\naws_secret_access_key=wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY")},
},
want: false,
wantErr: true,
},
{
name: "CloudStorage with different providers - AWS",
dpa: &oadpv1alpha1.DataProtectionApplication{
Expand Down