pkg/storage/azure: configure private network once

pkg/storage/azure/azure.go

@@ -14,6 +14,7 @@ import (
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore"
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore/cloud"
 	"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
+	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/storage/armstorage"
 	"github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2019-06-01/storage"
 	"github.com/Azure/azure-storage-blob-go/azblob"
 	"github.com/Azure/go-autorest/autorest"
@@ -559,6 +560,25 @@ func (d *driver) StorageChanged(cr *imageregistryv1.Config) bool {
 	return !reflect.DeepEqual(cr.Status.Storage.Azure, cr.Spec.Storage.Azure)
 }
 
+// isStorageAccountPrivate gets a storage account and returns true if public
+// network access is disabled, or false if public network access is enabled.
+// Public network access is enabled by default in Azure. In case of any
+// unexpected behaviour this function will return false.
+func (d *driver) isStorageAccountPrivate(azclient *azureclient.Client, accountName string) bool {
+	account, err := azclient.GetStorageAccount(d.Context, accountName)
+	if err != nil {
+		return false
+	}
+	if account.Properties == nil {
+		return false
+	}
+	publicNetworkAccess := account.Properties.PublicNetworkAccess
+	if publicNetworkAccess == nil {
+		return false
+	}
+	return *publicNetworkAccess == armstorage.PublicNetworkAccessDisabled
+}
+
 func (d *driver) assurePrivateAccount(cfg *Azure, infra *configv1.Infrastructure, tagset map[string]*string, accountName string) (string, error) {
 	if d.Config.PrivateStorageAccount == nil {
 		// user did not request private storage account setup - skip.
@@ -590,15 +610,14 @@ func (d *driver) assurePrivateAccount(cfg *Azure, infra *configv1.Infrastructure
 		return "", err
 	}
 
-	exists, err := azclient.PrivateEndpointExists(d.Context, privateEndpointName)
-	if err != nil {
-		return "", err
-	}
-	if exists {
-		return "", nil
+	// the last step in this function is to disable public network for the
+	// storage account - if we already did that, then none of the steps
+	// below need to be executed.
+	if d.isStorageAccountPrivate(azclient, accountName) {
+		return privateEndpointName, nil
 	}
 
-	klog.V(2).Info("configuring private networking for storage account...")
+	klog.V(4).Infof("configuring private endpoint %q for storage account...", privateEndpointName)
 	pe, err := azclient.CreatePrivateEndpoint(
 		d.Context,
 		&azureclient.PrivateEndpointCreateOptions{
@@ -612,11 +631,19 @@ func (d *driver) assurePrivateAccount(cfg *Azure, infra *configv1.Infrastructure
 	if err != nil {
 		return "", err
 	}
-	// TODO: do we need to check if private dns is already configured?
+	klog.V(4).Info("private endpoint configured")
+
+	klog.V(4).Info("configuring private DNS...")
 	if err := azclient.ConfigurePrivateDNS(d.Context, pe, d.Config.PrivateStorageAccount.VNetName, accountName); err != nil {
 		return privateEndpointName, err
 	}
-	klog.V(2).Info("private endpoint configured")
+	klog.V(4).Info("private DNS configured")
+
+	klog.V(4).Infof("disabling public network access for storage account %q...", accountName)
+	if err := azclient.UpdateStorageAccountNetworkAccess(d.Context, accountName, false); err != nil {
+		return privateEndpointName, err
+	}
+	klog.V(4).Info("storage account public network access disabled, the storage account is now private!")
 
 	return privateEndpointName, nil
 }

pkg/storage/azure/azureclient/azureclient.go

@@ -129,7 +129,21 @@ func New(opts *Options) (*Client, error) {
 	}, nil
 }
 
+func (c *Client) GetStorageAccount(ctx context.Context, accountName string) (armstorage.Account, error) {
+	// TODO: reuse client
+	client, err := armstorage.NewAccountsClient(c.subscriptionID, c.creds, c.clientOpts)
+	if err != nil {
+		return armstorage.Account{}, fmt.Errorf("failed to create accounts client: %q", err)
+	}
+	resp, err := client.GetProperties(ctx, c.resourceGroupName, accountName, nil)
+	if err != nil {
+		return armstorage.Account{}, fmt.Errorf("failed to get storage account: %q", err)
+	}
+	return resp.Account, nil
+}
+
 func (c *Client) UpdateStorageAccountNetworkAccess(ctx context.Context, accountName string, allowPublicAccess bool) error {
+	// TODO: reuse client
 	client, err := armstorage.NewAccountsClient(c.subscriptionID, c.creds, c.clientOpts)
 	if err != nil {
 		return fmt.Errorf("failed to create accounts client: %q", err)
@@ -143,8 +157,7 @@ func (c *Client) UpdateStorageAccountNetworkAccess(ctx context.Context, accountN
 			PublicNetworkAccess: &publicNetworkAccess,
 		},
 	}
-	_, err := client.Update(ctx, c.resourceGroupName, accountName, params, nil)
-	if err != nil {
+	if _, err := client.Update(ctx, c.resourceGroupName, accountName, params, nil); err != nil {
 		return fmt.Errorf("failed to updated storage account network access: %q", err)
 	}
 	return nil