Cannot delete BackupEntries which use the secret from a core.BackupBucket.status.generatedSecretRef reference

[plkokanov]

How to categorize this issue?

/kind bug
/platform azure

What happened:
Currently it is not possible to delete extensions.BackupEntries which have the following data in the secret specified by their .spec.secretRef field:

data:
  storageAccount: <account>
  storageKey: <key>

Trying to delete such a BackupEntry returns the following error:

'Error deleting BackupEntry: secret garden/entry-<backup-entry-name>
      doesn''t have a subscription ID'

What you expected to happen:
extensions.BackupEntries to be deleted successfully

How to reproduce it (as minimally and precisely as possible):

1. Create a shoot cluster with provider azure
2. Delete the shoot cluster
3. Make sure that the core.BackupEntry for the shoot is deleted as well - the grace period for it's deletion should be 0 or it should be manually deleted
4. Observe that the core.BackupEntry deletion fails, because its associated extensions.BackupEntry cannot be deleted with:

   'Error deleting BackupEntry: secret garden/entry-<backup-entry-name>
         doesn''t have a subscription ID'
   

Anything else we need to know?:
The problem seems to come from this validation:

gardener-extension-provider-azure/pkg/internal/auth.go

Lines 69 to 72 in 5c5b346

	subscriptionID, ok := getSecretDataValue(secret, azure.SubscriptionIDKey, altSubscriptionIDIDKey)
	if !ok {
	return nil, fmt.Errorf("secret %s/%s doesn't have a subscription ID", secret.Namespace, secret.Name)
	}

This code is called when instantiating a factory which is then used to get an azure storage client to delete the backup object associated with the extensions.BackupEntry here:

gardener-extension-provider-azure/pkg/controller/backupentry/actuator.go

Lines 46 to 50 in 5c5b346

	func (a *actuator) Delete(ctx context.Context, _ logr.Logger, backupEntry *extensionsv1alpha1.BackupEntry) error {
	factory, err := azureclient.NewAzureClientFactory(ctx, a.client, backupEntry.Spec.SecretRef)
	if err != nil {
	return util.DetermineError(err, helper.KnownCodes)
	}

gardener-extension-provider-azure/pkg/azure/client/factory.go

Lines 38 to 42 in 5c5b346

	func NewAzureClientFactory(ctx context.Context, client client.Client, secretRef corev1.SecretReference) (Factory, error) {
	auth, err := internal.GetClientAuthData(ctx, client, secretRef, false)
	if err != nil {
	return nil, err
	}

gardener-extension-provider-azure/pkg/internal/auth.go

Lines 50 to 56 in 5c5b346

	func GetClientAuthData(ctx context.Context, c client.Client, secretRef corev1.SecretReference, allowDNSKeys bool) (*ClientAuth, error) {
	secret, err := extensionscontroller.GetSecretByReference(ctx, c, &secretRef)
	if err != nil {
	return nil, err
	}
	return NewClientAuthDataFromSecret(secret, allowDNSKeys)

So this actually expects the data in the secret reference to be of the following format:

data:
  clientID: <client-id>
  clientSecret: <client-secret>
  subscriptionID: <subscription-id>
  tenantID: <tenant-id>

---

Previously (before #739) we did not use the secret reference when instantiating a factory for the azure client:

gardener-extension-provider-azure/pkg/controller/backupentry/actuator.go

Lines 48 to 49 in 417843d

	func (a *actuator) Delete(ctx context.Context, _ logr.Logger, backupEntry *extensionsv1alpha1.BackupEntry) error {
	factory := azureclient.NewAzureClientFactory(a.client)

gardener-extension-provider-azure/pkg/azure/client/factory.go

Lines 32 to 36 in 417843d

	func NewAzureClientFactory(client client.Client) Factory {
	return AzureFactory{
	client: client,
	}
	}

Instantiating only the storage client with the secret ref was ok, because the storage client actually only needs the data that is already present in the secret:

data:
  storageAccount: <account>
  storageKey: <key>

---

Essentially there are two different types of provider auth secret data:
(1):

data:
  storageAccount: <account>
  storageKey: <key>

and (2)

data:
  clientID: <client-id>
  clientSecret: <client-secret>
  subscriptionID: <subscription-id>
  tenantID: <tenant-id>

The secret data in (1) is inside the secret specified in the core.BackupBucket.status.generatedSecretRef field, whereas the secret data in (2) is inside the secret specified in the core.BackupBucket.spec.secretRef field.
When a core.BackupEntry is reconciled, the BackupEntry controller in gardenlet checks if the corresponding core.BackupBucket has a .status.generatedSecretRef field, and if that is the case, it will copy the corresponding secret to the seed cluster and set that as the extensions.BackupEntry.spec.secretRef.
If a .status.generatedSecretRef field does not exist, the BackupEntry controller will instead copy the core.BackupBucket.spec.secretRef to the seed cluster and set that to the extensions.BackupEntry.spec.secretRef field.
This can be seen in the following code:
https://github.com/gardener/gardener/blob/cff77d588fa928a950ce1459f2c856a2d9bee2e9/pkg/gardenlet/controller/backupentry/reconciler.go#L651-L663
https://github.com/gardener/gardener/blob/cff77d588fa928a950ce1459f2c856a2d9bee2e9/pkg/gardenlet/controller/backupentry/reconciler.go#L170-L173
https://github.com/gardener/gardener/blob/cff77d588fa928a950ce1459f2c856a2d9bee2e9/pkg/gardenlet/controller/backupentry/reconciler.go#L332-L339
https://github.com/gardener/gardener/blob/cff77d588fa928a950ce1459f2c856a2d9bee2e9/pkg/gardenlet/controller/backupentry/reconciler.go#L665-L675

---

Environment:

• Gardener version (if relevant):
• Extension version: v1.41.1 however it should be present since v1.40.0
• Kubernetes version (use kubectl version):
• Cloud provider or hardware configuration:
• Others:

[ialidzhikov]

@kon-angelo can we close this issue after #795 is merged?

[kon-angelo]

@ialidzhikov This should be closed yes