periodically rotate etcd storage account secret

pkg/azure/client/storage.go

@@ -65,12 +65,8 @@ func NewBlobStorageClient(_ context.Context, storageAccountName, storageAccountK
 	return &BlobStorageClient{blobclient}, err
 }
 
-// NewBlobStorageClientFromSecretRef creates a client for an Azure Blob storage by reading auth information from secret reference.
-func NewBlobStorageClientFromSecretRef(ctx context.Context, client client.Client, secretRef *corev1.SecretReference) (*BlobStorageClient, error) {
-	secret, err := extensionscontroller.GetSecretByReference(ctx, client, secretRef)
-	if err != nil {
-		return nil, err
-	}
+// NewBlobStorageClientFromSecretRef creates a client for an Azure Blob storage by reading auth information from a secret.
+func NewBlobStorageClientFromSecret(ctx context.Context, client client.Client, secret *corev1.Secret) (*BlobStorageClient, error) {
 	storageAccountName, ok := secret.Data[azure.StorageAccount]
 	if !ok {
 		return nil, fmt.Errorf("secret %s/%s doesn't have a storage account", secret.Namespace, secret.Name)
@@ -89,6 +85,15 @@ func NewBlobStorageClientFromSecretRef(ctx context.Context, client client.Client
 	return NewBlobStorageClient(ctx, string(storageAccountName), string(storageAccountKey), storageDomain)
 }
 
+// NewBlobStorageClientFromSecretRef creates a client for an Azure Blob storage by reading auth information from secret reference.
+func NewBlobStorageClientFromSecretRef(ctx context.Context, client client.Client, secretRef *corev1.SecretReference) (*BlobStorageClient, error) {
+	secret, err := extensionscontroller.GetSecretByReference(ctx, client, secretRef)
+	if err != nil {
+		return nil, err
+	}
+	return NewBlobStorageClientFromSecret(ctx, client, secret)
+}
+
 // DeleteObjectsWithPrefix deletes the blob objects with the specific <prefix> from <container>.
 // If it does not exist, no error is returned.
 func (c *BlobStorageClient) DeleteObjectsWithPrefix(ctx context.Context, container, prefix string) error {

pkg/azure/client/storageaccount.go

@@ -54,19 +54,69 @@ func (c *StorageAccountClient) CreateStorageAccount(ctx context.Context, resourc
 
 // ListStorageAccountKey lists the first key of a storage account.
 func (c *StorageAccountClient) ListStorageAccountKey(ctx context.Context, resourceGroupName, storageAccountName string) (string, error) {
+	keys, err := c.listStorageAccountKeys(ctx, resourceGroupName, storageAccountName)
+	if err != nil {
+		return "", err
+	}
+	return *keys[0].Value, nil
+}
+
+// ListStorageAccountKeys lists the keys of a storage account.
+func (c *StorageAccountClient) listStorageAccountKeys(ctx context.Context, resourceGroupName, storageAccountName string) ([]*armstorage.AccountKey, error) {
 	response, err := c.client.ListKeys(ctx, resourceGroupName, storageAccountName, &armstorage.AccountsClientListKeysOptions{
 		// doc: "Specifies type of the key to be listed. Possible value is kerb.. Specifying any value will set the value to kerb."
 		Expand: ptr.To("kerb"),
 	})
 
 	if err != nil {
-		return "", err
+		return nil, err
 	}
 
 	if len(response.Keys) < 1 {
-		return "", fmt.Errorf("no key found in storage account %s", storageAccountName)
+		return nil, fmt.Errorf("no key found in storage account %s", storageAccountName)
+	}
+
+	return response.Keys, nil
+}
+
+// RotateKey performs a partial key rotation for the given storage account, using the property of Azure storage accounts always having exactly two keys.
+// The existing storage keys are checked against the given one, and the one that does _not_ match it will be regenerated and its new value returned.
+// This ensures the given key still being valid (until the next rotation) which ensures that there is time for the updated value to propagate.
+func (c *StorageAccountClient) RotateKey(ctx context.Context, resourceGroupName, storageAccountName, storageAccountKey string) (string, error) {
+	keys, err := c.listStorageAccountKeys(ctx, resourceGroupName, storageAccountName)
+	if err != nil {
+		return "", err
+	}
+
+	var keyToRotate *string
+	switch {
+	case *keys[0].Value == storageAccountKey:
+		keyToRotate = keys[1].KeyName
+	case *keys[1].Value == storageAccountKey:
+		keyToRotate = keys[0].KeyName
+	default:
+		return "", fmt.Errorf("unable to find given key in storage account keys")
 	}
 
-	firstKey := response.Keys[0]
-	return *firstKey.Value, nil
+	resp, err := c.client.RegenerateKey(
+		ctx,
+		resourceGroupName,
+		storageAccountName,
+		armstorage.AccountRegenerateKeyParameters{KeyName: keyToRotate},
+		nil,
+	)
+
+	if err != nil {
+		return "", err
+	}
+
+	// This might be a bit paranoid
+	for _, k := range resp.Keys {
+		if k.KeyName == keyToRotate {
+			return *k.Value, nil
+		}
+	}
+
+	return "", fmt.Errorf("error rotating storage account key '%v'", keyToRotate)
+
 }

pkg/azure/client/types.go

@@ -140,6 +140,7 @@ type VirtualNetwork interface {
 type StorageAccount interface {
 	CreateStorageAccount(context.Context, string, string, string) error
 	ListStorageAccountKey(context.Context, string, string) (string, error)
+	RotateKey(context.Context, string, string, string) (string, error)
 }
 
 // DNSZone represents an Azure DNS zone k8sClient.

pkg/controller/backupbucket/actuator.go

@@ -8,6 +8,7 @@ import (
 	"context"
 	"fmt"
 
+	extensionscontroller "github.com/gardener/gardener/extensions/pkg/controller"
 	"github.com/gardener/gardener/extensions/pkg/controller/backupbucket"
 	"github.com/gardener/gardener/extensions/pkg/util"
 	extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1"
@@ -17,12 +18,13 @@ import (
 
 	"github.com/gardener/gardener-extension-provider-azure/pkg/apis/azure"
 	"github.com/gardener/gardener-extension-provider-azure/pkg/apis/azure/helper"
+	azuretypes "github.com/gardener/gardener-extension-provider-azure/pkg/azure"
 	azureclient "github.com/gardener/gardener-extension-provider-azure/pkg/azure/client"
 )
 
 var (
 	// DefaultBlobStorageClient is the default function to get a backupbucket client. Can be overridden for tests.
-	DefaultBlobStorageClient = azureclient.NewBlobStorageClientFromSecretRef
+	DefaultBlobStorageClient = azureclient.NewBlobStorageClientFromSecret
 )
 
 type actuator struct {
@@ -58,33 +60,62 @@ func (a *actuator) Reconcile(ctx context.Context, _ logr.Logger, backupBucket *e
 		return err
 	}
 
+	bucketCloudConfiguration, err := azureclient.CloudConfiguration(backupConfig.CloudConfiguration, &backupBucket.Spec.Region)
+	if err != nil {
+		return err
+	}
+
+	storageDomain, err := azureclient.BlobStorageDomainFromCloudConfiguration(bucketCloudConfiguration)
+	if err != nil {
+		return fmt.Errorf("failed to determine blob storage service domain: %w", err)
+	}
+
 	// If the generated secret in the backupbucket status not exists that means
 	// no backupbucket exists and it need to be created.
 	if backupBucket.Status.GeneratedSecretRef == nil {
 		storageAccountName, storageAccountKey, err := ensureBackupBucket(ctx, factory, backupBucket)
 		if err != nil {
 			return util.DetermineError(err, helper.KnownCodes)
 		}
-
-		bucketCloudConfiguration, err := azureclient.CloudConfiguration(backupConfig.CloudConfiguration, &backupBucket.Spec.Region)
-		if err != nil {
-			return err
-		}
-
-		storageDomain, err := azureclient.BlobStorageDomainFromCloudConfiguration(bucketCloudConfiguration)
-		if err != nil {
-			return fmt.Errorf("failed to determine blob storage service domain: %w", err)
-		}
 		// Create the generated backupbucket secret.
 		if err := a.createBackupBucketGeneratedSecret(ctx, backupBucket, storageAccountName, storageAccountKey, storageDomain); err != nil {
 			return util.DetermineError(err, helper.KnownCodes)
 		}
 	}
 
-	blobStorageClient, err := DefaultBlobStorageClient(ctx, a.client, backupBucket.Status.GeneratedSecretRef)
+	backupSecret, err := extensionscontroller.GetSecretByReference(ctx, a.client, backupBucket.Status.GeneratedSecretRef)
+	if err != nil {
+		return err
+	}
+
+	blobStorageClient, err := DefaultBlobStorageClient(ctx, a.client, backupSecret)
 	if err != nil {
 		return util.DetermineError(err, helper.KnownCodes)
 	}
+
+	doRotation, err := shouldBeRotated(*backupSecret)
+	if err == nil {
+		return err
+	}
+
+	if doRotation {
+		newKey, err := rotateStorageAccountCredentials(ctx, factory, backupBucket, string(backupSecret.Data[azuretypes.StorageKey]))
+		if err != nil {
+			return err
+		}
+		oldSecretRef := backupBucket.Status.GeneratedSecretRef
+		// Generate new secret
+		if err := a.createBackupBucketGeneratedSecret(ctx, backupBucket, getStorageAccountName(backupBucket), newKey, storageDomain); err != nil {
+			return util.DetermineError(err, helper.KnownCodes)
+		}
+		// Clean up old secret
+		err = a.deleteBackupBucketGeneratedSecretByRef(ctx, oldSecretRef)
+		if err != nil {
+			// TODO: Is this worth erroring out? Warning might suffice, there is only one leftover soon-to-be-outdated secret
+			return err
+		}
+	}
+
 	return util.DetermineError(blobStorageClient.CreateContainerIfNotExists(ctx, backupBucket.Name), helper.KnownCodes)
 }
 
@@ -126,7 +157,7 @@ func (a *actuator) delete(ctx context.Context, _ logr.Logger, backupBucket *exte
 
 	if secret != nil {
 		// Get a storage account client to delete the backup container in the storage account.
-		storageClient, err := DefaultBlobStorageClient(ctx, a.client, backupBucket.Status.GeneratedSecretRef)
+		storageClient, err := DefaultBlobStorageClient(ctx, a.client, secret)
 		if err != nil {
 			return err
 		}

pkg/controller/backupbucket/backupbucket.go

@@ -16,11 +16,12 @@ import (
 	azureclient "github.com/gardener/gardener-extension-provider-azure/pkg/azure/client"
 )
 
+func getStorageAccountName(backupBucket *extensionsv1alpha1.BackupBucket) string {
+	return fmt.Sprintf("bkp%s", utils.ComputeSHA256Hex([]byte(backupBucket.Name))[:15])
+}
+
 func ensureBackupBucket(ctx context.Context, factory azureclient.Factory, backupBucket *extensionsv1alpha1.BackupBucket) (string, string, error) {
-	var (
-		backupBucketNameSha = utils.ComputeSHA256Hex([]byte(backupBucket.Name))
-		storageAccountName  = fmt.Sprintf("bkp%s", backupBucketNameSha[:15])
-	)
+	storageAccountName := getStorageAccountName(backupBucket)
 
 	// Get resource group client to ensure resource group to host backup storage account exists.
 	groupClient, err := factory.Group()
@@ -42,11 +43,29 @@ func ensureBackupBucket(ctx context.Context, factory azureclient.Factory, backup
 		return "", "", err
 	}
 
-	// Get the key of the storage account.
+	// Get a key of the storage account. We simply use the first one for new storage accounts (later rotations may change the "current" one).
 	storageAccountKey, err := storageAccountClient.ListStorageAccountKey(ctx, backupBucket.Name, storageAccountName)
 	if err != nil {
 		return "", "", err
 	}
 
 	return storageAccountName, storageAccountKey, nil
 }
+
+func rotateStorageAccountCredentials(
+	ctx context.Context,
+	factory azureclient.Factory,
+	backupBucket *extensionsv1alpha1.BackupBucket,
+	storageAccountKey string,
+) (string, error) {
+	var (
+		resourceGroupName   = backupBucket.Name
+		backupBucketNameSha = utils.ComputeSHA256Hex([]byte(backupBucket.Name))
+		storageAccountName  = fmt.Sprintf("bkp%s", backupBucketNameSha[:15])
+	)
+	storageAccountClient, err := factory.StorageAccount()
+	if err != nil {
+		return "", err
+	}
+	return storageAccountClient.RotateKey(ctx, resourceGroupName, storageAccountName, storageAccountKey)
+}

pkg/controller/backupbucket/secret.go

@@ -7,8 +7,10 @@ package backupbucket
 import (
 	"context"
 	"fmt"
+	"time"
 
 	extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1"
+	"github.com/gardener/gardener/pkg/utils"
 	kutil "github.com/gardener/gardener/pkg/utils/kubernetes"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -19,9 +21,10 @@ import (
 )
 
 func (a *actuator) createBackupBucketGeneratedSecret(ctx context.Context, backupBucket *extensionsv1alpha1.BackupBucket, storageAccountName, storageKey, storageDomain string) error {
+	timestampHash := utils.ComputeSHA256Hex([]byte(time.Now().Format(time.RFC3339)))
 	var generatedSecret = &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      fmt.Sprintf("generated-bucket-%s", backupBucket.Name),
+			Name:      fmt.Sprintf("generated-bucket-%s-%s", backupBucket.Name, timestampHash),
 			Namespace: "garden",
 		},
 	}
@@ -50,7 +53,15 @@ func (a *actuator) deleteBackupBucketGeneratedSecret(ctx context.Context, backup
 	if backupBucket.Status.GeneratedSecretRef == nil {
 		return nil
 	}
-	return kutil.DeleteSecretByReference(ctx, a.client, backupBucket.Status.GeneratedSecretRef)
+	return a.deleteBackupBucketGeneratedSecretByRef(ctx, backupBucket.Status.GeneratedSecretRef)
+}
+
+// deleteBackupBucketGeneratedSecretByRef deletes generated the secret pointed to by the provided secretRef.
+func (a *actuator) deleteBackupBucketGeneratedSecretByRef(ctx context.Context, secretRef *corev1.SecretReference) error {
+	if secretRef == nil {
+		return fmt.Errorf("passed secret ref must not be nil")
+	}
+	return kutil.DeleteSecretByReference(ctx, a.client, secretRef)
 }
 
 // getBackupBucketGeneratedSecret get generated secret referred by core BackupBucket resource in garden.
@@ -64,3 +75,7 @@ func (a *actuator) getBackupBucketGeneratedSecret(ctx context.Context, backupBuc
 	}
 	return secret, nil
 }
+
+func shouldBeRotated(secret corev1.Secret) (bool, error) {
+	return secret.CreationTimestamp.Time.Before(time.Now().AddDate(0, 0, -14)), nil
+}