Clean up credentials related code

[mateusoliveira43]

Credentials are used by BackupStorageLocation (BSL) and VolumeSnapshotLocation (VSL)

• confirm that no other part of OADP uses credentials code
• should controllers/registry.go be deleted/moved to pkg/credentials/credentials.go?

in controllers/bsl.go

• oadp-operator/controllers/bsl.go

  Lines 72 to 74 in 91e1aac

  	if bslSpec.CloudStorage.Credential.LocalObjectReference.Name == "" {
  	return false, fmt.Errorf("must provide a valid credential secret name")
  	}

  is duplication of

  oadp-operator/controllers/bsl.go

  Line 415 in 91e1aac

  if bsl.Velero.Credential.Name == "" {

• oadp-operator/controllers/bsl.go

  Lines 81 to 83 in 91e1aac

  	if bslSpec.CloudStorage != nil && bslSpec.Velero != nil {
  	return false, fmt.Errorf("must choose one of bucket or velero")
  	}

  is duplication of

  oadp-operator/controllers/bsl.go

  Line 386 in 91e1aac

  if bsl.CloudStorage != nil && bsl.Velero != nil {

• these functions all have duplication
  

  oadp-operator/controllers/bsl.go

  Line 268 in 91e1aac

  func (r *DPAReconciler) validateAWSBackupStorageLocation(bslSpec velerov1.BackupStorageLocationSpec, dpa *oadpv1alpha1.DataProtectionApplication) error {

  
  

  oadp-operator/controllers/bsl.go

  Line 299 in 91e1aac

  func (r *DPAReconciler) validateAzureBackupStorageLocation(bslSpec velerov1.BackupStorageLocationSpec, dpa *oadpv1alpha1.DataProtectionApplication) error {

  
  

  oadp-operator/controllers/bsl.go

  Line 330 in 91e1aac

  func (r *DPAReconciler) validateGCPBackupStorageLocation(bslSpec velerov1.BackupStorageLocationSpec, dpa *oadpv1alpha1.DataProtectionApplication) error {

  move it to this function

  oadp-operator/controllers/bsl.go

  Line 361 in 91e1aac

  func (r *DPAReconciler) validateProviderPluginAndSecret(bslSpec velerov1.BackupStorageLocationSpec, dpa *oadpv1alpha1.DataProtectionApplication) error {

  (but remove secret validation from it, it already done by other part of the code)

• should this only be called if !(dpa.Spec.Configuration.Velero.HasFeatureFlag("no-secret"))?

  oadp-operator/controllers/bsl.go

  Lines 123 to 127 in 91e1aac

  	secretName, _ := r.getSecretNameAndKeyforBackupLocation(bslSpec)
  	_, err := r.UpdateCredentialsSecretLabels(secretName, dpa.Namespace, dpa.Name)
  	if err != nil {
  	return false, err
  	}

• remove validation from this function, it was done previously

  oadp-operator/controllers/bsl.go

  Line 194 in 91e1aac

  func (r *DPAReconciler) UpdateCredentialsSecretLabels(secretName string, namespace string, dpaName string) (bool, error) {

in controllers/registry.go

• are not these duplication from api/v1alpha1/oadp_types.go?

  oadp-operator/controllers/registry.go

  Line 55 in 91e1aac

  const (

• this function should not patch secret every time

  oadp-operator/controllers/registry.go

  Line 216 in 91e1aac

  func (r *DPAReconciler) getProviderSecret(secretName string) (corev1.Secret, error) {

  move patching code to validation part
• delete this code

  oadp-operator/controllers/registry.go

  Lines 264 to 270 in 91e1aac

  	if _, ok := bslSpec.Config["credentialsFile"]; ok {
  	if secretName, secretKey, err :=
  	credentials.GetSecretNameKeyFromCredentialsFileConfigString(bslSpec.Config["credentialsFile"]); err == nil {
  	r.Log.Info(fmt.Sprintf("credentialsFile secret: %s, key: %s", secretName, secretKey))
  	return secretName, secretKey
  	}
  	}

  and docs/developer/testing/ MULTI_CLOUD_TESTING_UPDATES.md file
• verify credential function should check all cases

// add doc comments!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
func (r *DPAReconciler) verifyCredential(credential *corev1.SecretKeySelector, provider oadpv1alpha1.DefaultPlugin, location string) error {
	var credentialName string
	var credentialKey string

	if credential != nil {
		// Check if user specified empty credential name
		if credential.Name == "" {
			return fmt.Errorf("credential name specified in %s cannot be empty", location)
		} else {
			credentialName = credential.Name
		}
		// Check if user specified empty credential key
		if credential.Key == "" {
			return fmt.Errorf("credential key specified in %s cannot be empty", location)
		} else {
			credentialKey = credential.Key
		}
	} else {
		if provider != "" {
			// Assume default values
			credentialName = credentials.PluginSpecificFields[provider].SecretName
			credentialKey = credentials.PluginSpecificFields[provider].PluginSecretKey
		} else {
			// cloud storage case
			return fmt.Errorf("must provide a valid credential secret")
		}
	}

	secret, err := r.getProviderSecret(credentialName)
	if err != nil {
		return err
	}
	// need???
	// if secret.Name == "" {
	// 	return false, errors.New("secret not found")
	// }
	data, foundKey := secret.Data[credentialKey]
	if !foundKey || len(data) == 0 {
		return fmt.Errorf("Secret name %s is missing data for key %s", credentialName, credentialKey)
	}
	return nil
}

in controllers/validator.go

• delete this function

  oadp-operator/controllers/validator.go

  Line 74 in 91e1aac

  func (r *DPAReconciler) ValidateVeleroPlugins(log logr.Logger) (bool, error) {

  credentials should have been already validated previously

in controllers/vsl.go

• duplication from controllers/registry.go

  oadp-operator/controllers/vsl.go

  Line 18 in 91e1aac

  const (

• duplication from function pluginExistsInVeleroCR in controllers/bsl.go

  oadp-operator/controllers/vsl.go

  Line 256 in 91e1aac

  func containsPlugin(d []oadpv1alpha1.DefaultPlugin, value string) bool {

in pkg/credentials/credentials.go

• delete

  oadp-operator/pkg/credentials/credentials.go

  Line 77 in 91e1aac

  func GetSecretNameKeyFromCredentialsFileConfigString(credentialsFile string) (string, string, error) {

• delete

  oadp-operator/pkg/credentials/credentials.go

  Line 358 in 91e1aac

  func GetSecretNameAndKey(bslSpec *velerov1.BackupStorageLocationSpec, plugin oadpv1alpha1.DefaultPlugin) (string, string) {

  duplication from registry.go
• remove duplication in

  oadp-operator/pkg/credentials/credentials.go

  Line 389 in 91e1aac

  func GetSecretNameKeyConfigProviderForBackupLocation(blspec oadpv1alpha1.BackupLocation, namespace string) (string, string, string, map[string]string, error) {

[kaovilai]

related issue: #921

[openshift-bot]

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

[mateusoliveira43]

/lifecycle frozen