make azure storage account private

CHANGELOG.md

@@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- Secure Azure Storage Account by making them private.
 - Update Kyverno PolicyException to v2beta1.
 
 ## [0.9.0] - 2024-10-03

internal/controller/bucket_controller.go

@@ -110,7 +110,12 @@ func (r BucketReconciler) reconcileNormal(ctx context.Context, objectStorageServ
 			return ctrl.Result{}, errors.WithStack(err)
 		}
 	} else {
-		logger.Info("Bucket exists and you already own it.")
+		logger.Info("Bucket exists and you already own it, let's update it")
+		err = objectStorageService.UpdateBucket(ctx, bucket)
+		if err != nil {
+			logger.Error(err, "Bucket could not be updated")
+			return ctrl.Result{}, errors.WithStack(err)
+		}
 	}
 
 	logger.Info("Configuring bucket settings")

internal/pkg/service/objectstorage/cloud/aws/s3.go

@@ -64,6 +64,11 @@ func (s S3ObjectStorageAdapter) CreateBucket(ctx context.Context, bucket *v1alph
 	return err
 }
 
+// UpdateBucket does nothing as we cannot update an s3 bucket
+func (s S3ObjectStorageAdapter) UpdateBucket(ctx context.Context, bucket *v1alpha1.Bucket) error {
+	return nil
+}
+
 func (s S3ObjectStorageAdapter) DeleteBucket(ctx context.Context, bucket *v1alpha1.Bucket) error {
 	// First we need to empty the bucket
 	paginator := s3.NewListObjectsV2Paginator(s.s3Client, &s3.ListObjectsV2Input{

internal/pkg/service/objectstorage/cloud/azure/storage.go

@@ -33,7 +33,6 @@ type AzureObjectStorageAdapter struct {
 	logger                   logr.Logger
 	cluster                  AzureCluster
 	client                   client.Client
-	listStorageAccountName   []string
 }
 
 // NewAzureStorageService creates a new instance of AzureObjectStorageAdapter.
@@ -42,7 +41,6 @@ type AzureObjectStorageAdapter struct {
 // The logger is used for logging purposes.
 // The cluster represents the Azure cluster.
 // The client is the Kubernetes client used for interacting with the Kubernetes API.
-// The listStorageAccountName is a list of storage account names.
 func NewAzureStorageService(storageAccountClient *armstorage.AccountsClient, blobContainerClient *armstorage.BlobContainersClient, managementPoliciesClient *armstorage.ManagementPoliciesClient, logger logr.Logger, cluster AzureCluster, client client.Client) AzureObjectStorageAdapter {
 	return AzureObjectStorageAdapter{
 		storageAccountClient:     storageAccountClient,
@@ -51,7 +49,6 @@ func NewAzureStorageService(storageAccountClient *armstorage.AccountsClient, blo
 		logger:                   logger,
 		cluster:                  cluster,
 		client:                   client,
-		listStorageAccountName:   []string{},
 	}
 }
 
@@ -121,41 +118,46 @@ func (s AzureObjectStorageAdapter) CreateBucket(ctx context.Context, bucket *v1a
 	if err != nil {
 		return err
 	}
-	// If Storage Account does not exists, we need to create it first
-	if !existsStorageAccount {
-		// Create Storage Account
-		pollerResp, err := s.storageAccountClient.BeginCreate(
-			ctx,
-			s.cluster.GetResourceGroup(),
-			storageAccountName,
-			armstorage.AccountCreateParameters{
-				Kind: to.Ptr(armstorage.KindBlobStorage),
-				SKU: &armstorage.SKU{
-					Name: to.Ptr(armstorage.SKUNameStandardLRS),
-				},
-				Location: to.Ptr(s.cluster.GetRegion()),
-				Properties: &armstorage.AccountPropertiesCreateParameters{
-					AccessTier: to.Ptr(armstorage.AccessTierHot),
-					Encryption: &armstorage.Encryption{
-						Services: &armstorage.EncryptionServices{
-							Blob: &armstorage.EncryptionService{
-								KeyType: to.Ptr(armstorage.KeyTypeAccount),
-								Enabled: to.Ptr(true),
-							},
+
+	// Create or Update Storage Account
+	pollerResp, err := s.storageAccountClient.BeginCreate(
+		ctx,
+		s.cluster.GetResourceGroup(),
+		storageAccountName,
+		armstorage.AccountCreateParameters{
+			Kind: to.Ptr(armstorage.KindBlobStorage),
+			SKU: &armstorage.SKU{
+				Name: to.Ptr(armstorage.SKUNameStandardLRS),
+			},
+			Location: to.Ptr(s.cluster.GetRegion()),
+			Properties: &armstorage.AccountPropertiesCreateParameters{
+				AllowSharedKeyAccess: to.Ptr(true),
+				AccessTier:           to.Ptr(armstorage.AccessTierHot),
+				Encryption: &armstorage.Encryption{
+					Services: &armstorage.EncryptionServices{
+						Blob: &armstorage.EncryptionService{
+							KeyType: to.Ptr(armstorage.KeyTypeAccount),
+							Enabled: to.Ptr(true),
 						},
-						KeySource: to.Ptr(armstorage.KeySourceMicrosoftStorage),
 					},
-					EnableHTTPSTrafficOnly: to.Ptr(true),
+					KeySource: to.Ptr(armstorage.KeySourceMicrosoftStorage),
 				},
-			}, nil)
-		if err != nil {
-			return err
-		}
-		_, err = pollerResp.PollUntilDone(ctx, nil)
-		if err != nil {
-			return err
-		}
+				EnableHTTPSTrafficOnly: to.Ptr(true),
+				PublicNetworkAccess:    to.Ptr(armstorage.PublicNetworkAccessDisabled),
+			},
+		}, nil)
+	if err != nil {
+		return err
+	}
+	_, err = pollerResp.PollUntilDone(ctx, nil)
+	if err != nil {
+		return err
+	}
+
+	if !existsStorageAccount {
 		s.logger.Info(fmt.Sprintf("Storage Account %s created", storageAccountName))
+	} else {
+		s.logger.Info(fmt.Sprintf("Storage Account %s updated", storageAccountName))
 	}
 
 	// Create Storage Container
@@ -187,43 +189,49 @@ func (s AzureObjectStorageAdapter) CreateBucket(ctx context.Context, bucket *v1a
 	if err != nil {
 		return fmt.Errorf("unable to retrieve access keys from storage account %s", storageAccountName)
 	}
-	// Then, we retrieve the Access Key for 'key1'
-	foundKey1 := false
-	for _, k := range listKeys.Keys {
-		if *k.KeyName == "key1" {
-			foundKey1 = true
-			// Finally, we create the Secret into the bucket namespace
-			secret := &v1.Secret{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      bucket.Spec.Name,
-					Namespace: bucket.Namespace,
-					Labels: map[string]string{
-						"giantswarm.io/managed-by": "object-storage-operator",
-					},
-					Finalizers: []string{
-						v1alpha1.AzureSecretFinalizer,
-					},
-				},
-				Data: map[string][]byte{
+
+	secret := v1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      bucket.Spec.Name,
+			Namespace: bucket.Namespace,
+			Labels: map[string]string{
+				"giantswarm.io/managed-by": "object-storage-operator",
+			},
+			Finalizers: []string{
+				v1alpha1.AzureSecretFinalizer,
+			},
+		},
+	}
+
+	_, err = controllerutil.CreateOrUpdate(ctx, s.client, &secret, func() error {
+		// Then, we retrieve the Access Key for 'key1'
+		for _, k := range listKeys.Keys {
+			if *k.KeyName == "key1" {
+				// Finally, we create the Secret into the bucket namespace
+				secret.Data = map[string][]byte{
 					"accountName": []byte(storageAccountName),
 					"accountKey":  []byte(*k.Value),
-				},
-			}
-			err := s.client.Create(ctx, secret)
-			if err != nil {
-				return err
+				}
+				return nil
 			}
-			s.logger.Info(fmt.Sprintf("created secret %s", bucket.Spec.Name))
-			break
 		}
-	}
-	if !foundKey1 {
+
 		return fmt.Errorf("unable to retrieve access keys 'key1' from storage account %s", storageAccountName)
+	})
+
+	if err != nil {
+		return err
 	}
 
+	s.logger.Info(fmt.Sprintf("created secret %s", bucket.Spec.Name))
 	return nil
 }
 
+// UpdateBucket creates or updates the Storage Account if it not exists AND the Storage Container
+func (s AzureObjectStorageAdapter) UpdateBucket(ctx context.Context, bucket *v1alpha1.Bucket) error {
+	return s.CreateBucket(ctx, bucket)
+}
+
 // DeleteBucket deletes the Storage Account (Storage Container will be deleted by cascade)
 // Here, we decided to have a Storage Account dedicated to a Storage Container (relation 1 - 1)
 // We want to prevent the Storage Account from being used by anyone
@@ -394,17 +402,9 @@ func (s AzureObjectStorageAdapter) setTags(ctx context.Context, bucket *v1alpha1
 }
 
 // getStorageAccountName returns the storage account name for the given bucket name.
-// It sanitizes the bucket name and checks if it already exists in the list of storage account names.
-// If it exists, it returns the sanitized name. Otherwise, it adds the sanitized name to the list and returns it.
+// It sanitizes the bucket name and returns it.
 func (s *AzureObjectStorageAdapter) getStorageAccountName(bucketName string) string {
-	sanitizeName := sanitizeAlphanumeric24(bucketName)
-	for _, name := range s.listStorageAccountName {
-		if sanitizeName == name {
-			return sanitizeName
-		}
-	}
-	s.listStorageAccountName = append(s.listStorageAccountName, sanitizeName)
-	return sanitizeName
+	return sanitizeAlphanumeric24(bucketName)
 }
 
 // sanitizeAlphanumeric24 sanitizes the given name by removing any non-alphanumeric characters and truncating it to a maximum length of 24 characters.

internal/pkg/service/objectstorage/cloud/azure/storage_test.go

@@ -42,55 +42,3 @@ func Test_sanitizeAlphanumeric24(t *testing.T) {
 		})
 	}
 }
-
-func Test_getStorageAccountName(t *testing.T) {
-	testCases := []struct {
-		name              string
-		bucketName        string
-		listStorageNames  []string
-		expectedResult    string
-		expectedListNames []string
-	}{
-		{
-			name:              "case 0: bucket name exists in list",
-			bucketName:        "giantswarm-glippy-loki",
-			listStorageNames:  []string{"giantswarmglippyloki", "giantswarmverylonginstal"},
-			expectedResult:    "giantswarmglippyloki",
-			expectedListNames: []string{"giantswarmglippyloki", "giantswarmverylonginstal"},
-		},
-		{
-			name:              "case 1: bucket name does not exist in list",
-			bucketName:        "giantswarm-verylonginstallationname-loki",
-			listStorageNames:  []string{"giantswarmglippyloki"},
-			expectedResult:    "giantswarmverylonginstal",
-			expectedListNames: []string{"giantswarmglippyloki", "giantswarmverylonginstal"},
-		},
-		{
-			name:              "case 2: empty list",
-			bucketName:        "giantswarm-1111-loki",
-			listStorageNames:  []string{},
-			expectedResult:    "giantswarm1111loki",
-			expectedListNames: []string{"giantswarm1111loki"},
-		},
-	}
-
-	for i, tc := range testCases {
-		t.Run(strconv.Itoa(i), func(t *testing.T) {
-			t.Log(tc.name)
-
-			adapter := &AzureObjectStorageAdapter{
-				listStorageAccountName: tc.listStorageNames,
-			}
-
-			result := adapter.getStorageAccountName(tc.bucketName)
-
-			if result != tc.expectedResult {
-				t.Fatalf("Expected result: %s, but got: %s", tc.expectedResult, result)
-			}
-
-			if !cmp.Equal(adapter.listStorageAccountName, tc.expectedListNames) {
-				t.Fatalf("\n\n%s\n", cmp.Diff(tc.expectedListNames, adapter.listStorageAccountName))
-			}
-		})
-	}
-}

internal/pkg/service/objectstorage/service.go

@@ -11,6 +11,7 @@ type ObjectStorageService interface {
 	// Configure all bucket related configurations (Tags, ...)
 	ConfigureBucket(ctx context.Context, bucket *v1alpha1.Bucket) error
 	CreateBucket(ctx context.Context, bucket *v1alpha1.Bucket) error
+	UpdateBucket(ctx context.Context, bucket *v1alpha1.Bucket) error
 	DeleteBucket(ctx context.Context, bucket *v1alpha1.Bucket) error
 	// Exists checks whether a bucket exists in the current account.
 	ExistsBucket(ctx context.Context, bucket *v1alpha1.Bucket) (bool, error)