Merge pull request #904 from buhongw7583c/issue#858

Issue#858 Add Vnet to Storage Account

api/v1alpha1/storageaccount_types.go

@@ -29,6 +29,8 @@ type StorageAccountSpec struct {
 	EnableHTTPSTrafficOnly *bool `json:"supportsHttpsTrafficOnly,omitempty"`
 
 	DataLakeEnabled *bool `json:"dataLakeEnabled,omitempty"`
+
+	NetworkRule *StorageNetworkRuleSet `json:"networkRule,omitempty"`
 }
 
 // StorageAccountSku the SKU of the storage account.
@@ -97,6 +99,43 @@ type StorageAccountList struct {
 	Items           []StorageAccount `json:"items"`
 }
 
+type Bypass string
+
+type StorageNetworkRuleSet struct {
+	// Bypass - Specifies whether traffic is bypassed for Logging/Metrics/AzureServices.
+	//Possible values are any combination of Logging|Metrics|AzureServices (For example, "Logging, Metrics"), or None to bypass none of those traffics.
+	//Possible values include: 'None', 'Logging', 'Metrics', 'AzureServices'
+	Bypass Bypass `json:"bypass,omitempty"`
+	// VirtualNetworkRules - Sets the virtual network rules
+	VirtualNetworkRules *[]VirtualNetworkRule `json:"virtualNetworkRules,omitempty"`
+	// IPRules - Sets the IP ACL rules
+	IPRules *[]IPRule `json:"ipRules,omitempty"`
+	// DefaultAction - Specifies the default action of allow or deny when no other rules match. Possible values include: 'DefaultActionAllow', 'DefaultActionDeny'
+	DefaultAction string `json:"defaultAction,omitempty"`
+}
+
+const (
+
+	// AzureServices ...
+	AzureServices Bypass = "AzureServices"
+	// Logging ...
+	Logging Bypass = "Logging"
+	// Metrics ...
+	Metrics Bypass = "Metrics"
+	// None ...
+	None Bypass = "None"
+)
+
+type VirtualNetworkRule struct {
+	// SubnetId - Resource ID of a subnet, for example: /subscriptions/{subscriptionId}/resourceGroups/{groupName}/providers/Microsoft.Network/virtualNetworks/{vnetName}/subnets/{subnetName}.
+	SubnetId *string `json:"subnetId,omitempty"`
+}
+
+type IPRule struct {
+	// IPAddressOrRange - Specifies the IP or IP range in CIDR format. Only IPV4 address is allowed.
+	IPAddressOrRange *string `json:"ipAddressOrRange,omitempty"`
+}
+
 func init() {
 	SchemeBuilder.Register(&StorageAccount{}, &StorageAccountList{})
 }

config/samples/azure_v1alpha1_storageaccount.yaml

@@ -10,3 +10,12 @@ spec:
   kind: StorageV2
   accessTier: Hot
   supportsHttpsTrafficOnly: true
+# Optional: networkRule
+  networkRule:
+    bypass: AzureServices # Possible values are AzureServices, Metrics, None, Logging
+    defaultAction: Deny # Possible values are Allow, Deny
+    virtualNetworkRules: 
+      - subnetId: /subscriptions/{subscription}/resourceGroups/{resourcegroup}/providers/Microsoft.Network/virtualNetworks/{vnet}/subnets/{subnet}
+    ipRules: #could be an ip range or a ip address
+      - ipAddressOrRange: 2.2.0.0/24
+      - ipAddressOrRange: 2.2.2.1

controllers/storageaccount_controller_test.go

@@ -10,18 +10,15 @@ import (
 	"testing"
 
 	azurev1alpha1 "github.com/Azure/azure-service-operator/api/v1alpha1"
-
 	"github.com/Azure/go-autorest/autorest/to"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-func TestStorageControllerHappyPath(t *testing.T) {
+func TestStorageControllerHappyPathWithoutNetworkRule(t *testing.T) {
 	t.Parallel()
 	defer PanicRecover(t)
 	ctx := context.Background()
-
 	StorageAccountName := GenerateAlphaNumTestResourceName("sadev")
-
 	// Create the ResourceGroup object and expect the Reconcile to be created
 	saInstance := &azurev1alpha1.StorageAccount{
 		ObjectMeta: metav1.ObjectMeta{
@@ -39,10 +36,8 @@ func TestStorageControllerHappyPath(t *testing.T) {
 			EnableHTTPSTrafficOnly: to.BoolPtr(true),
 		},
 	}
-
 	// create rg
 	EnsureInstance(ctx, t, tc, saInstance)
-
 	// delete rg
 	EnsureDelete(ctx, t, tc, saInstance)
 }

controllers/suite_test.go

@@ -745,9 +745,9 @@ func setup() error {
 
 	log.Println("Creating SA:", storageAccountName)
 	// Create the Storage Account and Container
-	_, _ = storageAccountManager.CreateStorage(context.Background(), resourceGroupName, storageAccountName, resourcegroupLocation, azurev1alpha1.StorageAccountSku{
+	_, _, _ = storageAccountManager.CreateStorage(context.Background(), resourceGroupName, storageAccountName, resourcegroupLocation, azurev1alpha1.StorageAccountSku{
 		Name: "Standard_LRS",
-	}, "Storage", map[string]*string{}, "", nil, nil)
+	}, "Storage", map[string]*string{}, "", nil, nil, nil)
 
 	// Storage account needs to be in "Suceeded" state
 	// for container create to succeed

pkg/errhelp/errors.go

@@ -54,7 +54,10 @@ const (
 	RequestDisallowedByPolicy           = "RequestDisallowedByPolicy"
 	ServiceBusy                         = "ServiceBusy"
 	NameNotAvailable                    = "NameNotAvailable"
-	PublicIPIdleTimeoutIsOutOfRange     = "PublicIPIdleTimeoutIsOutOfRange"
+
+	NetworkAclsValidationFailure = "NetworkAclsValidationFailure"
+
+	PublicIPIdleTimeoutIsOutOfRange = "PublicIPIdleTimeoutIsOutOfRange"
 )
 
 func NewAzureError(err error) error {

pkg/resourcemanager/mock/storages/storageaccount.go

@@ -50,7 +50,7 @@ func (manager *mockStorageManager) CreateStorage(ctx context.Context, groupName
 	kind azurev1alpha1.StorageAccountKind,
 	tags map[string]*string,
 	accessTier azurev1alpha1.StorageAccountAccessTier,
-	enableHTTPsTrafficOnly *bool, dataLakeEnabled *bool) (result storage.Account, err error) {
+	enableHTTPsTrafficOnly *bool, dataLakeEnabled *bool, networkRule *azurev1alpha1.NetworkRuleSet) (result storage.Account, err error) {
 	s := storageResource{
 		resourceGroupName:  groupName,
 		storageAccountName: storageAccountName,

pkg/resourcemanager/storages/storageaccount/storageaccount.go

@@ -7,8 +7,10 @@ import (
 	"context"
 	"errors"
 	"log"
+	"strings"
 
 	"github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2019-04-01/storage"
+	"github.com/Azure/azure-service-operator/api/v1alpha1"
 	azurev1alpha1 "github.com/Azure/azure-service-operator/api/v1alpha1"
 	"github.com/Azure/azure-service-operator/pkg/resourcemanager/config"
 	"github.com/Azure/azure-service-operator/pkg/resourcemanager/iam"
@@ -18,6 +20,56 @@ import (
 
 type azureStorageManager struct{}
 
+// ParseNetworkPolicy - helper function to parse network policies from Kubernetes spec
+func ParseNetworkPolicy(ruleSet *v1alpha1.StorageNetworkRuleSet) storage.NetworkRuleSet {
+
+	bypass := storage.AzureServices
+	switch ruleSet.Bypass {
+	case "AzureServices":
+		bypass = storage.AzureServices
+	case "None":
+		bypass = storage.None
+	case "Logging":
+		bypass = storage.Logging
+	case "Metrics":
+		bypass = storage.Metrics
+	}
+
+	defaultAction := storage.DefaultActionDeny
+	if strings.EqualFold(ruleSet.DefaultAction, "allow") {
+		defaultAction = storage.DefaultActionAllow
+	}
+
+	var ipInstances []storage.IPRule
+	if ruleSet.IPRules != nil {
+		for _, i := range *ruleSet.IPRules {
+			ipmask := i.IPAddressOrRange
+			ipInstances = append(ipInstances, storage.IPRule{
+				IPAddressOrRange: ipmask,
+				Action:           storage.Allow,
+			})
+		}
+	}
+
+	var vnetInstances []storage.VirtualNetworkRule
+	if ruleSet.VirtualNetworkRules != nil {
+		for _, i := range *ruleSet.VirtualNetworkRules {
+			vnetID := i.SubnetId
+			vnetInstances = append(vnetInstances, storage.VirtualNetworkRule{
+				VirtualNetworkResourceID: vnetID,
+				Action:                   storage.Allow,
+			})
+		}
+	}
+
+	return storage.NetworkRuleSet{
+		Bypass:              bypass,
+		DefaultAction:       defaultAction,
+		IPRules:             &ipInstances,
+		VirtualNetworkRules: &vnetInstances,
+	}
+}
+
 func getStoragesClient() storage.AccountsClient {
 	storagesClient := storage.NewAccountsClientWithBaseURI(config.BaseURI(), config.SubscriptionID())
 	a, err := iam.GetResourceManagementAuthorizer()
@@ -30,14 +82,15 @@ func getStoragesClient() storage.AccountsClient {
 }
 
 // CreateStorage creates a new storage account
-func (_ *azureStorageManager) CreateStorage(ctx context.Context, groupName string,
+func (_ *azureStorageManager) CreateStorage(ctx context.Context,
+	groupName string,
 	storageAccountName string,
 	location string,
 	sku azurev1alpha1.StorageAccountSku,
 	kind azurev1alpha1.StorageAccountKind,
 	tags map[string]*string,
 	accessTier azurev1alpha1.StorageAccountAccessTier,
-	enableHTTPsTrafficOnly *bool, dataLakeEnabled *bool) (result storage.Account, err error) {
+	enableHTTPsTrafficOnly *bool, dataLakeEnabled *bool, networkRule *storage.NetworkRuleSet) (pollingURL string, result storage.Account, err error) {
 
 	storagesClient := getStoragesClient()
 
@@ -46,16 +99,19 @@ func (_ *azureStorageManager) CreateStorage(ctx context.Context, groupName strin
 	checkAccountParams := storage.AccountCheckNameAvailabilityParameters{Name: &storageAccountName, Type: &storageType}
 	checkNameResult, err := storagesClient.CheckNameAvailability(ctx, checkAccountParams)
 	if err != nil {
-		return result, err
+		return "", result, err
 	}
 	if dataLakeEnabled == to.BoolPtr(true) && kind != "StorageV2" {
-		return result, errors.New("unable to create datalake enabled storage account")
+		err = errors.New("unable to create datalake enabled storage account")
+		return
 	}
 	if *checkNameResult.NameAvailable == false {
 		if checkNameResult.Reason == storage.AccountNameInvalid {
-			return result, errors.New("AccountNameInvalid")
+			err = errors.New("AccountNameInvalid")
+			return
 		} else if checkNameResult.Reason == storage.AlreadyExists {
-			return result, errors.New("AlreadyExists")
+			err = errors.New("AlreadyExists")
+			return
 		}
 	}
 
@@ -73,16 +129,19 @@ func (_ *azureStorageManager) CreateStorage(ctx context.Context, groupName strin
 			AccessTier:             sAccessTier,
 			EnableHTTPSTrafficOnly: enableHTTPsTrafficOnly,
 			IsHnsEnabled:           dataLakeEnabled,
+			NetworkRuleSet:         networkRule,
 		},
 	}
 
 	//log.Println(fmt.Sprintf("creating storage '%s' in resource group '%s' and location: %v", storageAccountName, groupName, location))
 	future, err := storagesClient.Create(ctx, groupName, storageAccountName, params)
 	if err != nil {
-		return result, err
+		return "", result, err
 	}
 
-	return future.Result(storagesClient)
+	result, err = future.Result(storagesClient)
+
+	return future.PollingURL(), result, err
 
 }
 

pkg/resourcemanager/storages/storageaccount/storageaccount_manager.go

@@ -18,14 +18,15 @@ func New() *azureStorageManager {
 }
 
 type StorageManager interface {
-	CreateStorage(ctx context.Context, groupName string,
+	CreateStorage(ctx context.Context,
+		groupName string,
 		storageAccountName string,
 		location string,
 		sku azurev1alpha1.StorageAccountSku,
 		kind azurev1alpha1.StorageAccountKind,
 		tags map[string]*string,
 		accessTier azurev1alpha1.StorageAccountAccessTier,
-		enableHTTPsTrafficOnly *bool, dataLakeEnabled *bool) (result storage.Account, err error)
+		enableHTTPsTrafficOnly *bool, dataLakeEnabled *bool, networkRule *storage.NetworkRuleSet) (pollingURL string, result storage.Account, err error)
 
 	// Get gets the description of the specified storage account.
 	// Parameters:

pkg/resourcemanager/storages/storageaccount/storageaccount_reconcile.go

@@ -7,10 +7,13 @@ import (
 	"context"
 	"fmt"
 
+	"github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2019-04-01/storage"
 	azurev1alpha1 "github.com/Azure/azure-service-operator/api/v1alpha1"
 	"github.com/Azure/azure-service-operator/pkg/errhelp"
 	"github.com/Azure/azure-service-operator/pkg/helpers"
 	"github.com/Azure/azure-service-operator/pkg/resourcemanager"
+	"github.com/Azure/azure-service-operator/pkg/resourcemanager/pollclient"
+
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 )
@@ -31,7 +34,13 @@ func (sa *azureStorageManager) Ensure(ctx context.Context, obj runtime.Object, o
 	accessTier := instance.Spec.AccessTier
 	enableHTTPSTrafficOnly := instance.Spec.EnableHTTPSTrafficOnly
 	dataLakeEnabled := instance.Spec.DataLakeEnabled
+	pollURL := instance.Status.PollingURL
+
+	networkAcls := storage.NetworkRuleSet{}
 
+	if instance.Spec.NetworkRule != nil {
+		networkAcls = ParseNetworkPolicy(instance.Spec.NetworkRule)
+	}
 	// convert kube labels to expected tag format
 	labels := map[string]*string{}
 	for k, v := range instance.GetLabels() {
@@ -44,11 +53,33 @@ func (sa *azureStorageManager) Ensure(ctx context.Context, obj runtime.Object, o
 	if err != nil {
 		instance.Status.Message = err.Error()
 		instance.Status.State = "NotReady"
+
+		// handle failures in the async operation
+		if pollURL != "" {
+			pClient := pollclient.NewPollClient()
+			res, err := pClient.Get(ctx, pollURL)
+			azerr := errhelp.NewAzureErrorAzureError(err)
+			if err != nil {
+				if azerr.Type == errhelp.NetworkAclsValidationFailure {
+					instance.Status.Message = "Unable to provision Azure Storage Account due to error: " + errhelp.StripErrorIDs(err)
+					instance.Status.Provisioning = false
+					instance.Status.Provisioned = false
+					return true, nil
+				}
+				return false, err
+			}
+
+			if res.Status == "Failed" {
+				instance.Status.Message = res.Error.Error()
+				instance.Status.Provisioning = false
+				return true, nil
+			}
+		}
 	} else {
 		instance.Status.State = string(stor.ProvisioningState)
 
 		hash = helpers.Hash256(instance.Spec)
-		if instance.Status.SpecHash == hash && instance.Status.Provisioned {
+		if instance.Status.SpecHash == hash && (instance.Status.Provisioned || instance.Status.FailedProvisioning) {
 			instance.Status.RequestedAt = nil
 			return true, nil
 		}
@@ -60,13 +91,14 @@ func (sa *azureStorageManager) Ensure(ctx context.Context, obj runtime.Object, o
 		instance.Status.Provisioning = false
 		instance.Status.SpecHash = hash
 		instance.Status.ResourceId = *stor.ID
+		instance.Status.PollingURL = ""
 		return true, nil
 	}
 
 	instance.Status.Provisioning = true
 	instance.Status.Provisioned = false
 
-	_, err = sa.CreateStorage(ctx, groupName, name, location, sku, kind, labels, accessTier, enableHTTPSTrafficOnly, dataLakeEnabled)
+	pollURL, _, err = sa.CreateStorage(ctx, groupName, name, location, sku, kind, labels, accessTier, enableHTTPSTrafficOnly, dataLakeEnabled, &networkAcls)
 	if err != nil {
 		instance.Status.Message = err.Error()
 		azerr := errhelp.NewAzureErrorAzureError(err)
@@ -105,14 +137,19 @@ func (sa *azureStorageManager) Ensure(ctx context.Context, obj runtime.Object, o
 
 			if azerr.Type == errhelp.AsyncOpIncompleteError {
 				instance.Status.Provisioning = true
+				instance.Status.PollingURL = pollURL
 			}
 			return false, nil
 		}
 
 		stop := []string{
 			errhelp.AccountNameInvalid,
+			errhelp.NetworkAclsValidationFailure,
 		}
 		if helpers.ContainsString(stop, azerr.Type) {
+			instance.Status.Message = "Unable to provision Azure Storage Account due to error: " + errhelp.StripErrorIDs(err)
+			instance.Status.Provisioning = false
+			instance.Status.Provisioned = false
 			return true, nil
 		}