Skip to content

OADP-6669: Use CloudStorage creationSecret and config as fallback for BSL #1942

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
wants to merge 2 commits into
base: oadp-dev
Choose a base branch
from
+183 −21
Open

OADP-6669: Use CloudStorage creationSecret and config as fallback for BSL #1942

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
aa93572
feat: Use CloudStorage creationSecret as fallback for BSL credentials
kaovilai Sep 5, 2025
345b342
feat: Add CloudStorage config and region fallback to BSL
kaovilai Sep 5, 2025
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
56 changes: 43 additions & 13 deletions internal/controller/bsl.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,14 +175,50 @@ func (r *DataProtectionApplicationReconciler) ReconcileBackupStorageLocations(lo
return err
}
bsl.Spec.BackupSyncPeriod = bslSpec.CloudStorage.BackupSyncPeriod
bsl.Spec.Config = bslSpec.CloudStorage.Config

// Start with CloudStorage CR's config as base (fallback)
if bucket.Spec.Config != nil {
bsl.Spec.Config = make(map[string]string)
for k, v := range bucket.Spec.Config {
bsl.Spec.Config[k] = v
}
}

// Add region from CloudStorage CR if specified
if bucket.Spec.Region != "" && bsl.Spec.Config == nil {
bsl.Spec.Config = make(map[string]string)
}
if bucket.Spec.Region != "" {
bsl.Spec.Config["region"] = bucket.Spec.Region
}

// Override with DPA's CloudStorageLocation config (higher priority)
for k, v := range bslSpec.CloudStorage.Config {
if bsl.Spec.Config == nil {
bsl.Spec.Config = make(map[string]string)
}
bsl.Spec.Config[k] = v
}

// Handle enableSharedConfig from CloudStorage CR
if bucket.Spec.EnableSharedConfig != nil && *bucket.Spec.EnableSharedConfig {
if bsl.Spec.Config == nil {
bsl.Spec.Config = map[string]string{}
}
bsl.Spec.Config["enableSharedConfig"] = "true"
}
bsl.Spec.Credential = bslSpec.CloudStorage.Credential
// Use DPA's CloudStorage credential if provided, otherwise fallback to CloudStorage's creationSecret
if bslSpec.CloudStorage.Credential != nil {
bsl.Spec.Credential = bslSpec.CloudStorage.Credential
} else {
// Use CloudStorage's creationSecret as the BSL credential
bsl.Spec.Credential = &corev1.SecretKeySelector{
LocalObjectReference: corev1.LocalObjectReference{
Name: bucket.Spec.CreationSecret.Name,
},
Key: bucket.Spec.CreationSecret.Key,
}
}
bsl.Spec.Default = bslSpec.CloudStorage.Default
bsl.Spec.ObjectStorage = &velerov1.ObjectStorageLocation{
Bucket: bucket.Spec.Name,
Expand Down Expand Up @@ -537,21 +573,15 @@ func (r *DataProtectionApplicationReconciler) ensureSecretDataExists(bsl *oadpv1

// Get secret details from either CloudStorage or Velero
if bsl.CloudStorage != nil {
// Make sure credentials are specified.
if bsl.CloudStorage.Credential == nil {
return fmt.Errorf("must provide a valid credential secret")
}
if bsl.CloudStorage.Credential.Name == "" {
return fmt.Errorf("must provide a valid credential secret name")
}
// Check if user specified empty credential key
if bsl.CloudStorage.Credential.Key == "" {
return fmt.Errorf("must provide a valid credential secret key")
}
// Get credentials - this will fallback to CloudStorage CR if needed
secretName, secretKey, err = r.getSecretNameAndKeyFromCloudStorage(bsl.CloudStorage)
if err != nil {
return err
}
// If still no secret found, it means CloudStorage CR doesn't exist or has no credentials
if secretName == "" {
return fmt.Errorf("must provide credentials either in DPA or CloudStorage CR")
}

// Get provider type from CloudStorage
if bsl.CloudStorage.CloudStorageRef.Name != "" {
Expand Down
125 changes: 118 additions & 7 deletions internal/controller/bsl_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2953,6 +2953,7 @@ func TestDPAReconciler_ReconcileBackupStorageLocations(t *testing.T) {
},
Spec: velerov1.BackupStorageLocationSpec{
Provider: "aws",
Config: map[string]string{"region": "test-region"},
StorageType: velerov1.StorageType{
ObjectStorage: &velerov1.ObjectStorageLocation{
Bucket: "test-bucket",
Expand Down Expand Up @@ -3027,6 +3028,7 @@ func TestDPAReconciler_ReconcileBackupStorageLocations(t *testing.T) {
Provider: "aws",
Config: map[string]string{
"enableSharedConfig": "true",
"region": "us-east-1",
},
StorageType: velerov1.StorageType{
ObjectStorage: &velerov1.ObjectStorageLocation{
Expand All @@ -3043,6 +3045,88 @@ func TestDPAReconciler_ReconcileBackupStorageLocations(t *testing.T) {
},
},
},
{
name: "CloudStorage with config and region fallback",
objects: []client.Object{
&oadpv1alpha1.DataProtectionApplication{
ObjectMeta: metav1.ObjectMeta{
Name: "test-dpa",
Namespace: "test-ns",
},
Spec: oadpv1alpha1.DataProtectionApplicationSpec{
BackupLocations: []oadpv1alpha1.BackupLocation{
{
CloudStorage: &oadpv1alpha1.CloudStorageLocation{
CloudStorageRef: corev1.LocalObjectReference{
Name: "config-fallback-cs",
},
Credential: &corev1.SecretKeySelector{
LocalObjectReference: corev1.LocalObjectReference{
Name: "cloud-credentials",
},
Key: "credentials",
},
Config: map[string]string{
"profile": "custom-profile", // This should override CloudStorage's config
},
},
},
},
},
},
&corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "cloud-credentials",
Namespace: "test-ns",
},
Data: map[string][]byte{"credentials": {}},
},
&oadpv1alpha1.CloudStorage{
ObjectMeta: metav1.ObjectMeta{
Name: "config-fallback-cs",
Namespace: "test-ns",
},
Spec: oadpv1alpha1.CloudStorageSpec{
Provider: oadpv1alpha1.AWSBucketProvider,
Name: "config-test-bucket",
Region: "us-west-2",
Config: map[string]string{
"profile": "default",
"s3ForcePathStyle": "true",
"serverSideEncryption": "AES256",
},
},
},
},
want: true,
wantErr: false,
wantBSL: velerov1.BackupStorageLocation{
ObjectMeta: metav1.ObjectMeta{
Name: "test-dpa-1",
Namespace: "test-ns",
},
Spec: velerov1.BackupStorageLocationSpec{
Provider: "aws",
Config: map[string]string{
"region": "us-west-2", // From CloudStorage CR
"profile": "custom-profile", // Overridden by DPA
"s3ForcePathStyle": "true", // From CloudStorage CR
"serverSideEncryption": "AES256", // From CloudStorage CR
},
StorageType: velerov1.StorageType{
ObjectStorage: &velerov1.ObjectStorageLocation{
Bucket: "config-test-bucket",
},
},
Credential: &corev1.SecretKeySelector{
LocalObjectReference: corev1.LocalObjectReference{
Name: "cloud-credentials",
},
Key: "credentials",
},
},
},
},
{
name: "CloudStorage with Azure provider",
objects: []client.Object{
Expand Down Expand Up @@ -3105,6 +3189,7 @@ func TestDPAReconciler_ReconcileBackupStorageLocations(t *testing.T) {
Config: map[string]string{
"storageAccount": "mystorageaccount",
"resourceGroup": "myresourcegroup",
"region": "eastus",
},
StorageType: velerov1.StorageType{
ObjectStorage: &velerov1.ObjectStorageLocation{
Expand Down Expand Up @@ -3282,7 +3367,7 @@ func TestDPAReconciler_ReconcileBackupStorageLocations(t *testing.T) {
},
Spec: velerov1.BackupStorageLocationSpec{
Provider: "aws",
Config: map[string]string(nil),
Config: map[string]string{"region": "us-west-2"},
StorageType: velerov1.StorageType{
ObjectStorage: &velerov1.ObjectStorageLocation{
Bucket: "aws-bucket-1",
Expand Down Expand Up @@ -3355,7 +3440,7 @@ func TestDPAReconciler_ReconcileBackupStorageLocations(t *testing.T) {
},
Spec: velerov1.BackupStorageLocationSpec{
Provider: "aws",
Config: map[string]string(nil),
Config: map[string]string{"region": "eu-west-1"},
StorageType: velerov1.StorageType{
ObjectStorage: &velerov1.ObjectStorageLocation{
Bucket: "sync-test-bucket",
Expand Down Expand Up @@ -4564,7 +4649,7 @@ AZURE_CLOUD_NAME=AzurePublicCloud`),
wantErr: false,
},
{
name: "CloudStorage without credentials",
name: "CloudStorage without credentials - should use CloudStorage's creationSecret",
dpa: &oadpv1alpha1.DataProtectionApplication{
ObjectMeta: metav1.ObjectMeta{
Name: "test-dpa",
Expand All @@ -4584,8 +4669,34 @@ AZURE_CLOUD_NAME=AzurePublicCloud`),
Credential: nil,
},
},
wantErr: true,
errMsg: "must provide a valid credential secret",
objects: []client.Object{
&oadpv1alpha1.CloudStorage{
ObjectMeta: metav1.ObjectMeta{
Name: "no-cred-cs",
Namespace: "test-ns",
},
Spec: oadpv1alpha1.CloudStorageSpec{
Name: "test-bucket",
Provider: oadpv1alpha1.AWSBucketProvider,
CreationSecret: corev1.SecretKeySelector{
LocalObjectReference: corev1.LocalObjectReference{
Name: "cloud-creds",
},
Key: "cloud",
},
},
},
},
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "cloud-creds",
Namespace: "test-ns",
},
Data: map[string][]byte{
"cloud": []byte("[default]\naws_access_key_id=test\naws_secret_access_key=test"),
},
},
wantErr: false, // Should succeed using CloudStorage's creationSecret
},
{
name: "CloudStorage with empty credential name",
Expand Down Expand Up @@ -4613,7 +4724,7 @@ AZURE_CLOUD_NAME=AzurePublicCloud`),
},
},
wantErr: true,
errMsg: "must provide a valid credential secret name",
errMsg: "Secret key specified in CloudStorage cannot be empty",
},
{
name: "CloudStorage with empty credential key",
Expand Down Expand Up @@ -4642,7 +4753,7 @@ AZURE_CLOUD_NAME=AzurePublicCloud`),
},
},
wantErr: true,
errMsg: "must provide a valid credential secret key",
errMsg: "Secret key specified in CloudStorage cannot be empty",
},
{
name: "CloudStorage not found",
Expand Down
9 changes: 9 additions & 0 deletions internal/controller/registry.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,15 @@ func (r *DataProtectionApplicationReconciler) getSecretNameAndKeyFromCloudStorag
err := r.verifySecretContent(secretName, secretKey)
return secretName, secretKey, err
}

// If no credential is specified, fallback to CloudStorage's creationSecret
if cloudStorage.CloudStorageRef.Name != "" {
bucket := &oadpv1alpha1.CloudStorage{}
if err := r.Get(r.Context, client.ObjectKey{Namespace: r.dpa.Namespace, Name: cloudStorage.CloudStorageRef.Name}, bucket); err == nil {
return bucket.Spec.CreationSecret.Name, bucket.Spec.CreationSecret.Key, nil
}
}

return "", "", nil
}

Expand Down
5 changes: 5 additions & 0 deletions internal/controller/registry_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,11 @@ func TestDPAReconciler_getSecretNameAndKeyFromCloudStorage(t *testing.T) {
Log: logr.Discard(),
Context: newContextForTest(),
EventRecorder: record.NewFakeRecorder(10),
dpa: &oadpv1alpha1.DataProtectionApplication{
ObjectMeta: metav1.ObjectMeta{
Namespace: "test-ns",
},
},
}

if tt.wantProfile == "aws-cloud-cred" {
Expand Down
9 changes: 8 additions & 1 deletion internal/controller/validator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -557,12 +557,19 @@ func TestDPAReconciler_ValidateDataProtectionCR(t *testing.T) {
Namespace: "test-ns",
},
Spec: oadpv1alpha1.CloudStorageSpec{
Name: "test-bucket",
Provider: "aws",
CreationSecret: corev1.SecretKeySelector{
LocalObjectReference: corev1.LocalObjectReference{
Name: "", // Empty secret name
},
Key: "cloud",
},
},
},
},
wantErr: true,
messageErr: "must provide a valid credential secret",
messageErr: "must provide credentials either in DPA or CloudStorage CR",
},
{
name: "given valid DPA CR bucket BSL configured and AWS Default Plugin with secret",
Expand Down