diff --git a/controllers/dpa_controller.go b/controllers/dpa_controller.go index 4db36bca4b..8a9c68cb24 100644 --- a/controllers/dpa_controller.go +++ b/controllers/dpa_controller.go @@ -87,6 +87,7 @@ func (r *DPAReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.R r.ReconcileResticRestoreHelperConfig, r.ValidateBackupStorageLocations, r.ReconcileBackupStorageLocations, + r.ReconcileRegistrySecrets, r.ReconcileRegistries, r.ReconcileRegistrySVCs, r.ReconcileRegistryRoutes, diff --git a/controllers/registry.go b/controllers/registry.go index 41665d8b90..31c634a151 100644 --- a/controllers/registry.go +++ b/controllers/registry.go @@ -410,29 +410,16 @@ func (r *DPAReconciler) getRegistryEnvVars(bsl *velerov1.BackupStorageLocation) } func (r *DPAReconciler) getAWSRegistryEnvVars(bsl *velerov1.BackupStorageLocation, awsEnvVars []corev1.EnvVar) ([]corev1.EnvVar, error) { - // Check for secret name - secretName, secretKey := r.getSecretNameAndKey(bsl.Spec.Credential, oadpv1alpha1.DefaultPluginAWS) - - // fetch secret and error - secret, err := r.getProviderSecret(secretName) - if err != nil { - r.Log.Info(fmt.Sprintf("Error fetching provider secret %s for backupstoragelocation %s/%s", secretName, bsl.Namespace, bsl.Name)) - return nil, err - } - awsProfile := "default" - if value, exists := bsl.Spec.Config[Profile]; exists { - awsProfile = value - } - // parse the secret and get aws access_key and aws secret_key - AWSAccessKey, AWSSecretKey, err := r.parseAWSSecret(secret, secretKey, awsProfile) - if err != nil { - r.Log.Info(fmt.Sprintf("Error parsing provider secret %s for backupstoragelocation %s/%s", secretName, bsl.Namespace, bsl.Name)) - return nil, err - } + // create secret data and fill up the values and return from here for i := range awsEnvVars { if awsEnvVars[i].Name == RegistryStorageS3AccesskeyEnvVarKey { - awsEnvVars[i].Value = AWSAccessKey + awsEnvVars[i].ValueFrom = &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + bsl.Name + "-" + bsl.Spec.Provider + "-registry-secret"}, + Key: "access_key", + }, + } } if awsEnvVars[i].Name == RegistryStorageS3BucketEnvVarKey { @@ -449,7 +436,12 @@ func (r *DPAReconciler) getAWSRegistryEnvVars(bsl *velerov1.BackupStorageLocatio } if awsEnvVars[i].Name == RegistryStorageS3SecretkeyEnvVarKey { - awsEnvVars[i].Value = AWSSecretKey + awsEnvVars[i].ValueFrom = &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + bsl.Name + "-" + bsl.Spec.Provider + "-registry-secret"}, + Key: "secret_key", + }, + } } if awsEnvVars[i].Name == RegistryStorageS3RegionendpointEnvVarKey { @@ -465,38 +457,6 @@ func (r *DPAReconciler) getAWSRegistryEnvVars(bsl *velerov1.BackupStorageLocatio } func (r *DPAReconciler) getAzureRegistryEnvVars(bsl *velerov1.BackupStorageLocation, azureEnvVars []corev1.EnvVar) ([]corev1.EnvVar, error) { - // Check for secret name - secretName, secretKey := r.getSecretNameAndKey(bsl.Spec.Credential, oadpv1alpha1.DefaultPluginMicrosoftAzure) - r.Log.Info(fmt.Sprintf("Azure secret name: %s and secret key: %s", secretName, secretKey)) - - // fetch secret and error - secret, err := r.getProviderSecret(secretName) - if err != nil { - r.Log.Info(fmt.Sprintf("Error fetching provider secret %s for backupstoragelocation %s/%s", secretName, bsl.Namespace, bsl.Name)) - return nil, err - } - - // parse the secret and get azure storage account key - azcreds, err := r.parseAzureSecret(secret, secretKey) - if err != nil { - r.Log.Info(fmt.Sprintf("Error parsing provider secret %s for backupstoragelocation %s/%s", secretName, bsl.Namespace, bsl.Name)) - return nil, err - } - if len(bsl.Spec.Config["storageAccountKeyEnvVar"]) != 0 { - if azcreds.strorageAccountKey == "" { - r.Log.Info("Expecting storageAccountKeyEnvVar value set present in the credentials") - return nil, errors.New("no strorageAccountKey value present in credentials file") - } - } else { - r.Log.Info("Checking for service principal credentials") - if len(azcreds.subscriptionID) == 0 && - len(azcreds.tenantID) == 0 && - len(azcreds.clientID) == 0 && - len(azcreds.clientSecret) == 0 && - len(azcreds.resourceGroup) == 0 { - return nil, errors.New("error finding service principal parameters for the supplied Azure credential") - } - } for i := range azureEnvVars { if azureEnvVars[i].Name == RegistryStorageAzureContainerEnvVarKey { @@ -508,17 +468,37 @@ func (r *DPAReconciler) getAzureRegistryEnvVars(bsl *velerov1.BackupStorageLocat } if azureEnvVars[i].Name == RegistryStorageAzureAccountkeyEnvVarKey { - azureEnvVars[i].Value = azcreds.strorageAccountKey + azureEnvVars[i].ValueFrom = &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + bsl.Name + "-" + bsl.Spec.Provider + "-registry-secret"}, + Key: "storage_account_key", + }, + } } if azureEnvVars[i].Name == RegistryStorageAzureSPNClientIDEnvVarKey { - azureEnvVars[i].Value = azcreds.clientID + azureEnvVars[i].ValueFrom = &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + bsl.Name + "-" + bsl.Spec.Provider + "-registry-secret"}, + Key: "client_id_key", + }, + } } if azureEnvVars[i].Name == RegistryStorageAzureSPNClientSecretEnvVarKey { - azureEnvVars[i].Value = azcreds.clientSecret + azureEnvVars[i].ValueFrom = &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + bsl.Name + "-" + bsl.Spec.Provider + "-registry-secret"}, + Key: "client_secret_key", + }, + } } if azureEnvVars[i].Name == RegistryStorageAzureSPNTenantIDEnvVarKey { - azureEnvVars[i].Value = azcreds.tenantID + azureEnvVars[i].ValueFrom = &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + bsl.Name + "-" + bsl.Spec.Provider + "-registry-secret"}, + Key: "tenant_id_key", + }, + } } } return azureEnvVars, nil @@ -1048,3 +1028,172 @@ func (r *DPAReconciler) updateRegistryConfigMap(registryRouteCM *corev1.ConfigMa return nil } + +func (r *DPAReconciler) ReconcileRegistrySecrets(log logr.Logger) (bool, error) { + dpa := oadpv1alpha1.DataProtectionApplication{} + if err := r.Get(r.Context, r.NamespacedName, &dpa); err != nil { + return false, err + } + + // fetch the bsl instances + bslList := velerov1.BackupStorageLocationList{} + if err := r.List(r.Context, &bslList, &client.ListOptions{ + Namespace: r.NamespacedName.Namespace, + LabelSelector: labels.SelectorFromSet(map[string]string{ + "app.kubernetes.io/component": "bsl", + }), + }); err != nil { + return false, err + } + + // Now for each of these bsl instances, create a registry secret + for _, bsl := range bslList.Items { + secret := corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "oadp-" + bsl.Name + "-" + bsl.Spec.Provider + "-registry-secret", + Namespace: r.NamespacedName.Namespace, + Labels: map[string]string{ + oadpv1alpha1.OadpOperatorLabel: "True", + }, + }, + } + + if !dpa.BackupImages() { + deleteContext := context.Background() + if err := r.Get(deleteContext, types.NamespacedName{ + Name: secret.Name, + Namespace: r.NamespacedName.Namespace, + }, &secret); err != nil { + if k8serror.IsNotFound(err) { + return true, nil + } + return false, err + } + + deleteOptionPropagationForeground := metav1.DeletePropagationForeground + if err := r.Delete(deleteContext, &secret, &client.DeleteOptions{PropagationPolicy: &deleteOptionPropagationForeground}); err != nil { + r.EventRecorder.Event(&secret, corev1.EventTypeNormal, "DeleteRegistrySecretFailed", "Could not delete registry secret:"+err.Error()) + return false, err + } + r.EventRecorder.Event(&secret, corev1.EventTypeNormal, "DeletedRegistrySecret", "Registry secret deleted") + + return true, nil + } + + // Create Secret + op, err := controllerutil.CreateOrUpdate(r.Context, r.Client, &secret, func() error { + // TODO: check for secret status condition errors and respond here + err := r.updateRegistrySecret(&secret, &bsl, &dpa) + + return err + }) + if err != nil { + return false, err + } + if op == controllerutil.OperationResultCreated || op == controllerutil.OperationResultUpdated { + // Trigger event to indicate Secret was created or updated + r.EventRecorder.Event(&secret, + corev1.EventTypeNormal, + "RegistrySecretsReconciled", + fmt.Sprintf("performed %s on secret %s/%s", op, secret.Namespace, secret.Name), + ) + } + } + + return true, nil +} + +func (r *DPAReconciler) updateRegistrySecret(secret *corev1.Secret, bsl *velerov1.BackupStorageLocation, dpa *oadpv1alpha1.DataProtectionApplication) error { + // Setting controller owner reference on the registry secret + err := controllerutil.SetControllerReference(dpa, secret, r.Scheme) + if err != nil { + return err + } + + // when updating the spec fields we update each field individually + // to get around the immutable fields + provider := bsl.Spec.Provider + switch provider { + case AWSProvider: + err = r.populateAWSRegistrySecret(bsl, secret) + case AzureProvider: + err = r.populateAzureRegistrySecret(bsl, secret) + } + + return nil +} + +func (r *DPAReconciler) populateAWSRegistrySecret(bsl *velerov1.BackupStorageLocation, registrySecret *corev1.Secret) error { + // Check for secret name + secretName, secretKey := r.getSecretNameAndKey(bsl.Spec.Credential, oadpv1alpha1.DefaultPluginAWS) + + // fetch secret and error + secret, err := r.getProviderSecret(secretName) + if err != nil { + r.Log.Info(fmt.Sprintf("Error fetching provider secret %s for backupstoragelocation %s/%s", secretName, bsl.Namespace, bsl.Name)) + return err + } + awsProfile := "default" + if value, exists := bsl.Spec.Config[Profile]; exists { + awsProfile = value + } + // parse the secret and get aws access_key and aws secret_key + AWSAccessKey, AWSSecretKey, err := r.parseAWSSecret(secret, secretKey, awsProfile) + if err != nil { + r.Log.Info(fmt.Sprintf("Error parsing provider secret %s for backupstoragelocation %s/%s", secretName, bsl.Namespace, bsl.Name)) + return err + } + + registrySecret.Data = map[string][]byte{ + "access_key": []byte(AWSAccessKey), + "secret_key": []byte(AWSSecretKey), + } + + return nil +} + +func (r *DPAReconciler) populateAzureRegistrySecret(bsl *velerov1.BackupStorageLocation, registrySecret *corev1.Secret) error { + // Check for secret name + secretName, secretKey := r.getSecretNameAndKey(bsl.Spec.Credential, oadpv1alpha1.DefaultPluginMicrosoftAzure) + r.Log.Info(fmt.Sprintf("Azure secret name: %s and secret key: %s", secretName, secretKey)) + + // fetch secret and error + secret, err := r.getProviderSecret(secretName) + if err != nil { + r.Log.Info(fmt.Sprintf("Error fetching provider secret %s for backupstoragelocation %s/%s", secretName, bsl.Namespace, bsl.Name)) + return err + } + + // parse the secret and get azure storage account key + azcreds, err := r.parseAzureSecret(secret, secretKey) + if err != nil { + r.Log.Info(fmt.Sprintf("Error parsing provider secret %s for backupstoragelocation %s/%s", secretName, bsl.Namespace, bsl.Name)) + return err + } + if len(bsl.Spec.Config["storageAccountKeyEnvVar"]) != 0 { + if azcreds.strorageAccountKey == "" { + r.Log.Info("Expecting storageAccountKeyEnvVar value set present in the credentials") + return errors.New("no strorageAccountKey value present in credentials file") + } + } else { + r.Log.Info("Checking for service principal credentials") + if len(azcreds.subscriptionID) == 0 && + len(azcreds.tenantID) == 0 && + len(azcreds.clientID) == 0 && + len(azcreds.clientSecret) == 0 && + len(azcreds.resourceGroup) == 0 { + return errors.New("error finding service principal parameters for the supplied Azure credential") + } + } + + registrySecret.Data = map[string][]byte{ + "storage_account_key": []byte(azcreds.strorageAccountKey), + "subscription_id_key": []byte(azcreds.subscriptionID), + "tenant_id_key": []byte(azcreds.tenantID), + "client_id_key": []byte(azcreds.clientID), + "client_secret_key": []byte(azcreds.clientSecret), + "resource_group_key": []byte(azcreds.resourceGroup), + } + + return nil +} diff --git a/controllers/registry_test.go b/controllers/registry_test.go index cd9526993d..dce726f05c 100644 --- a/controllers/registry_test.go +++ b/controllers/registry_test.go @@ -103,6 +103,26 @@ var ( "AZURE_CLIENT_SECRET=" + testClientSecret + "\n" + "AZURE_RESOURCE_GROUP=" + testResourceGroup), } + awsRegistrySecretData = map[string][]byte{ + "access_key": []byte(testBslAccessKey), + "secret_key": []byte(testBslSecretAccessKey), + } + azureRegistrySecretData = map[string][]byte{ + "client_id_key": []byte(""), + "client_secret_key": []byte(""), + "resource_group_key": []byte(""), + "storage_account_key": []byte(testStoragekey), + "subscription_id_key": []byte(""), + "tenant_id_key": []byte(""), + } + azureRegistrySPSecretData = map[string][]byte{ + "client_id_key": []byte(testClientID), + "client_secret_key": []byte(testClientSecret), + "resource_group_key": []byte(testResourceGroup), + "storage_account_key": []byte(testStoragekey), + "subscription_id_key": []byte(testSubscriptionID), + "tenant_id_key": []byte(testTenantID), + } ) func TestDPAReconciler_buildRegistryDeployment(t *testing.T) { @@ -111,6 +131,7 @@ func TestDPAReconciler_buildRegistryDeployment(t *testing.T) { registryDeployment *appsv1.Deployment bsl *velerov1.BackupStorageLocation secret *corev1.Secret + registrySecret *corev1.Secret dpa *oadpv1alpha1.DataProtectionApplication wantErr bool }{ @@ -155,6 +176,13 @@ func TestDPAReconciler_buildRegistryDeployment(t *testing.T) { }, Data: secretData, }, + registrySecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "oadp-test-bsl-aws-registry-secret", + Namespace: "test-ns", + }, + Data: awsRegistrySecretData, + }, dpa: &oadpv1alpha1.DataProtectionApplication{}, }, { @@ -198,6 +226,13 @@ func TestDPAReconciler_buildRegistryDeployment(t *testing.T) { }, Data: secretData, }, + registrySecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "oadp-test-bsl-aws-registry-secret", + Namespace: "test-ns", + }, + Data: awsRegistrySecretData, + }, dpa: &oadpv1alpha1.DataProtectionApplication{ Spec: oadpv1alpha1.DataProtectionApplicationSpec{ PodAnnotations: map[string]string{ @@ -247,6 +282,13 @@ func TestDPAReconciler_buildRegistryDeployment(t *testing.T) { }, Data: secretData, }, + registrySecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "oadp-test-bsl-aws-registry-secret", + Namespace: "test-ns", + }, + Data: awsRegistrySecretData, + }, dpa: &oadpv1alpha1.DataProtectionApplication{ Spec: oadpv1alpha1.DataProtectionApplicationSpec{ PodAnnotations: map[string]string{ @@ -274,7 +316,7 @@ func TestDPAReconciler_buildRegistryDeployment(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - fakeClient, err := getFakeClientFromObjectsForRegistry(tt.secret, tt.registryDeployment, tt.bsl) + fakeClient, err := getFakeClientFromObjectsForRegistry(tt.secret, tt.registryDeployment, tt.bsl, tt.registrySecret) if err != nil { t.Errorf("error in creating fake client, likely programmer error") } @@ -346,8 +388,13 @@ func TestDPAReconciler_buildRegistryDeployment(t *testing.T) { Value: S3, }, { - Name: RegistryStorageS3AccesskeyEnvVarKey, - Value: testAccessKey, + Name: RegistryStorageS3AccesskeyEnvVarKey, + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry-secret"}, + Key: "access_key", + }, + }, }, { Name: RegistryStorageS3BucketEnvVarKey, @@ -358,8 +405,13 @@ func TestDPAReconciler_buildRegistryDeployment(t *testing.T) { Value: "aws-region", }, { - Name: RegistryStorageS3SecretkeyEnvVarKey, - Value: testSecretAccessKey, + Name: RegistryStorageS3SecretkeyEnvVarKey, + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry-secret"}, + Key: "secret_key", + }, + }, }, { Name: RegistryStorageS3RegionendpointEnvVarKey, @@ -519,6 +571,7 @@ func TestDPAReconciler_getAWSRegistryEnvVars(t *testing.T) { wantRegistryContainerEnvVar []corev1.EnvVar wantProfile string secret *corev1.Secret + registrySecret *corev1.Secret wantErr bool matchProfile bool }{ @@ -551,6 +604,13 @@ func TestDPAReconciler_getAWSRegistryEnvVars(t *testing.T) { }, Data: secretData, }, + registrySecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "oadp-test-bsl-aws-registry-secret", + Namespace: "test-ns", + }, + Data: awsRegistrySecretData, + }, wantProfile: "test-profile", matchProfile: true, }, { @@ -582,6 +642,13 @@ func TestDPAReconciler_getAWSRegistryEnvVars(t *testing.T) { }, Data: secretData, }, + registrySecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "oadp-test-bsl-aws-registry-secret", + Namespace: "test-ns", + }, + Data: awsRegistrySecretData, + }, wantProfile: testBslProfile, matchProfile: true, }, { @@ -613,13 +680,20 @@ func TestDPAReconciler_getAWSRegistryEnvVars(t *testing.T) { }, Data: awsSecretDataWithMissingProfile, }, + registrySecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "oadp-test-bsl-aws-registry-secret", + Namespace: "test-ns", + }, + Data: awsRegistrySecretData, + }, wantProfile: testBslProfile, matchProfile: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - fakeClient, err := getFakeClientFromObjectsForRegistry(tt.secret, tt.bsl) + fakeClient, err := getFakeClientFromObjectsForRegistry(tt.secret, tt.bsl, tt.registrySecret) if err != nil { t.Errorf("error in creating fake client, likely programmer error") } @@ -640,8 +714,13 @@ func TestDPAReconciler_getAWSRegistryEnvVars(t *testing.T) { Value: S3, }, { - Name: RegistryStorageS3AccesskeyEnvVarKey, - Value: testAccessKey, + Name: RegistryStorageS3AccesskeyEnvVarKey, + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry-secret"}, + Key: "access_key", + }, + }, }, { Name: RegistryStorageS3BucketEnvVarKey, @@ -652,8 +731,13 @@ func TestDPAReconciler_getAWSRegistryEnvVars(t *testing.T) { Value: "aws-region", }, { - Name: RegistryStorageS3SecretkeyEnvVarKey, - Value: testSecretAccessKey, + Name: RegistryStorageS3SecretkeyEnvVarKey, + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry-secret"}, + Key: "secret_key", + }, + }, }, { Name: RegistryStorageS3RegionendpointEnvVarKey, @@ -671,8 +755,13 @@ func TestDPAReconciler_getAWSRegistryEnvVars(t *testing.T) { Value: S3, }, { - Name: RegistryStorageS3AccesskeyEnvVarKey, - Value: testBslAccessKey, + Name: RegistryStorageS3AccesskeyEnvVarKey, + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry-secret"}, + Key: "access_key", + }, + }, }, { Name: RegistryStorageS3BucketEnvVarKey, @@ -683,8 +772,13 @@ func TestDPAReconciler_getAWSRegistryEnvVars(t *testing.T) { Value: "aws-region", }, { - Name: RegistryStorageS3SecretkeyEnvVarKey, - Value: testBslSecretAccessKey, + Name: RegistryStorageS3SecretkeyEnvVarKey, + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry-secret"}, + Key: "secret_key", + }, + }, }, { Name: RegistryStorageS3RegionendpointEnvVarKey, @@ -717,6 +811,7 @@ func TestDPAReconciler_getAzureRegistryEnvVars(t *testing.T) { bsl *velerov1.BackupStorageLocation wantRegistryContainerEnvVar []corev1.EnvVar secret *corev1.Secret + registrySecret *corev1.Secret wantErr bool wantProfile string matchProfile bool @@ -750,6 +845,13 @@ func TestDPAReconciler_getAzureRegistryEnvVars(t *testing.T) { }, Data: secretAzureData, }, + registrySecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "oadp-test-bsl-azure-registry-secret", + Namespace: "test-ns", + }, + Data: azureRegistrySecretData, + }, wantProfile: "test-profile", matchProfile: true, }, @@ -781,13 +883,20 @@ func TestDPAReconciler_getAzureRegistryEnvVars(t *testing.T) { }, Data: secretAzureServicePrincipalData, }, + registrySecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "oadp-test-bsl-azure-registry-secret", + Namespace: "test-ns", + }, + Data: azureRegistrySPSecretData, + }, wantProfile: "test-sp-profile", matchProfile: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - fakeClient, err := getFakeClientFromObjectsForRegistry(tt.secret, tt.bsl) + fakeClient, err := getFakeClientFromObjectsForRegistry(tt.secret, tt.bsl, tt.registrySecret) if err != nil { t.Errorf("error in creating fake client, likely programmer error") } @@ -816,24 +925,44 @@ func TestDPAReconciler_getAzureRegistryEnvVars(t *testing.T) { Value: "velero-azure-account", }, { - Name: RegistryStorageAzureAccountkeyEnvVarKey, - Value: "someStorageKey", + Name: RegistryStorageAzureAccountkeyEnvVarKey, + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry-secret"}, + Key: "storage_account_key", + }, + }, }, { Name: RegistryStorageAzureAADEndpointEnvVarKey, Value: "", }, { - Name: RegistryStorageAzureSPNClientIDEnvVarKey, - Value: "", + Name: RegistryStorageAzureSPNClientIDEnvVarKey, + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry-secret"}, + Key: "client_id_key", + }, + }, }, { - Name: RegistryStorageAzureSPNClientSecretEnvVarKey, - Value: "", + Name: RegistryStorageAzureSPNClientSecretEnvVarKey, + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry-secret"}, + Key: "client_secret_key", + }, + }, }, { - Name: RegistryStorageAzureSPNTenantIDEnvVarKey, - Value: "", + Name: RegistryStorageAzureSPNTenantIDEnvVarKey, + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry-secret"}, + Key: "tenant_id_key", + }, + }, }, } if tt.wantProfile == "test-sp-profile" { @@ -851,24 +980,44 @@ func TestDPAReconciler_getAzureRegistryEnvVars(t *testing.T) { Value: "velero-azure-account", }, { - Name: RegistryStorageAzureAccountkeyEnvVarKey, - Value: "someStorageKey", + Name: RegistryStorageAzureAccountkeyEnvVarKey, + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry-secret"}, + Key: "storage_account_key", + }, + }, }, { Name: RegistryStorageAzureAADEndpointEnvVarKey, Value: "", }, { - Name: RegistryStorageAzureSPNClientIDEnvVarKey, - Value: "someClientID", + Name: RegistryStorageAzureSPNClientIDEnvVarKey, + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry-secret"}, + Key: "client_id_key", + }, + }, }, { - Name: RegistryStorageAzureSPNClientSecretEnvVarKey, - Value: "someClientSecret", + Name: RegistryStorageAzureSPNClientSecretEnvVarKey, + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry-secret"}, + Key: "client_secret_key", + }, + }, }, { - Name: RegistryStorageAzureSPNTenantIDEnvVarKey, - Value: "someTenantID", + Name: RegistryStorageAzureSPNTenantIDEnvVarKey, + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry-secret"}, + Key: "tenant_id_key", + }, + }, }, } } @@ -1243,7 +1392,190 @@ func TestDPAReconciler_updateRegistryRouteCM(t *testing.T) { t.Errorf("updateRegistryConfigMap() error = %v, wantErr %v", err, tt.wantErr) } if !reflect.DeepEqual(tt.registryRouteCM, wantRegitryRouteCM) { - t.Errorf("expected bsl labels to be %#v, got %#v", tt.registryRouteCM, wantRegitryRouteCM) + t.Errorf("expected registry CM to be %#v, got %#v", tt.registryRouteCM, wantRegitryRouteCM) + } + }) + } +} + +func TestDPAReconciler_populateAWSRegistrySecret(t *testing.T) { + + tests := []struct { + name string + bsl *velerov1.BackupStorageLocation + registrySecret *corev1.Secret + awsSecret *corev1.Secret + dpa *oadpv1alpha1.DataProtectionApplication + wantErr bool + }{ + { + name: "Given Velero CR and bsl instance, appropriate registry secret is updated for aws case", + wantErr: false, + bsl: &velerov1.BackupStorageLocation{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-bsl", + Namespace: "test-ns", + }, + Spec: velerov1.BackupStorageLocationSpec{ + StorageType: velerov1.StorageType{ + ObjectStorage: &velerov1.ObjectStorageLocation{ + Bucket: "aws-bucket", + }, + }, + Config: map[string]string{ + Region: "aws-region", + S3URL: "https://sr-url-aws-domain.com", + InsecureSkipTLSVerify: "false", + Profile: testBslProfile, + }, + }, + }, + dpa: &oadpv1alpha1.DataProtectionApplication{ + ObjectMeta: metav1.ObjectMeta{ + Name: "Velero-test-CR", + Namespace: "test-ns", + }, + }, + awsSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cloud-credentials", + Namespace: "test-ns", + }, + Data: secretData, + }, + registrySecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "oadp-test-bsl-aws-registry-secret", + Namespace: "test-ns", + Labels: map[string]string{ + oadpv1alpha1.OadpOperatorLabel: "True", + }, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fakeClient, err := getFakeClientFromObjects(tt.awsSecret, tt.dpa) + if err != nil { + t.Errorf("error in creating fake client, likely programmer error") + } + r := &DPAReconciler{ + Client: fakeClient, + Scheme: fakeClient.Scheme(), + Log: logr.Discard(), + Context: newContextForTest(tt.name), + NamespacedName: types.NamespacedName{ + Namespace: tt.bsl.Namespace, + Name: tt.bsl.Name, + }, + EventRecorder: record.NewFakeRecorder(10), + } + wantRegistrySecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry-secret", + Namespace: r.NamespacedName.Namespace, + Labels: map[string]string{ + oadpv1alpha1.OadpOperatorLabel: "True", + }, + }, + Data: awsRegistrySecretData, + } + if err := r.populateAWSRegistrySecret(tt.bsl, tt.registrySecret); (err != nil) != tt.wantErr { + t.Errorf("populateAWSRegistrySecret() error = %v, wantErr %v", err, tt.wantErr) + } + if !reflect.DeepEqual(tt.registrySecret.Data, wantRegistrySecret.Data) { + t.Errorf("expected bsl labels to be %#v, got %#v", tt.registrySecret.Data, wantRegistrySecret.Data) + } + }) + } +} + +func TestDPAReconciler_populateAzureRegistrySecret(t *testing.T) { + tests := []struct { + name string + bsl *velerov1.BackupStorageLocation + registrySecret *corev1.Secret + azureSecret *corev1.Secret + dpa *oadpv1alpha1.DataProtectionApplication + wantErr bool + }{ + { + name: "Given Velero CR and bsl instance, appropriate registry secret is updated for azure case", + wantErr: false, + bsl: &velerov1.BackupStorageLocation{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-bsl", + Namespace: "test-ns", + }, + Spec: velerov1.BackupStorageLocationSpec{ + Provider: AzureProvider, + StorageType: velerov1.StorageType{ + ObjectStorage: &velerov1.ObjectStorageLocation{ + Bucket: "azure-bucket", + }, + }, + Config: map[string]string{ + StorageAccount: "velero-azure-account", + ResourceGroup: testResourceGroup, + RegistryStorageAzureAccountnameEnvVarKey: "velero-azure-account", + "storageAccountKeyEnvVar": "AZURE_STORAGE_ACCOUNT_ACCESS_KEY", + }, + }, + }, + dpa: &oadpv1alpha1.DataProtectionApplication{ + ObjectMeta: metav1.ObjectMeta{ + Name: "Velero-test-CR", + Namespace: "test-ns", + }, + }, + azureSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cloud-credentials-azure", + Namespace: "test-ns", + }, + Data: secretAzureData, + }, + registrySecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "oadp-test-bsl-azure-registry-secret", + Namespace: "test-ns", + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fakeClient, err := getFakeClientFromObjects(tt.azureSecret, tt.dpa) + if err != nil { + t.Errorf("error in creating fake client, likely programmer error") + } + r := &DPAReconciler{ + Client: fakeClient, + Scheme: fakeClient.Scheme(), + Log: logr.Discard(), + Context: newContextForTest(tt.name), + NamespacedName: types.NamespacedName{ + Namespace: tt.bsl.Namespace, + Name: tt.bsl.Name, + }, + EventRecorder: record.NewFakeRecorder(10), + } + wantRegistrySecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry-secret", + Namespace: r.NamespacedName.Namespace, + Labels: map[string]string{ + oadpv1alpha1.OadpOperatorLabel: "True", + }, + }, + Data: azureRegistrySecretData, + } + if err := r.populateAzureRegistrySecret(tt.bsl, tt.registrySecret); (err != nil) != tt.wantErr { + t.Errorf("populateAWSRegistrySecret() error = %v, wantErr %v", err, tt.wantErr) + } + if !reflect.DeepEqual(tt.registrySecret.Data, wantRegistrySecret.Data) { + t.Errorf("expected bsl labels to be %#v, got %#v", tt.registrySecret, wantRegistrySecret.Data) } }) }