From 4ab6d2043a225413e13505d2e4afaee992602e7c Mon Sep 17 00:00:00 2001 From: hobu <37413937+buhongw7583c@users.noreply.github.com> Date: Wed, 8 Apr 2020 06:42:59 +0800 Subject: [PATCH 01/15] issue#858#WIPaddVnetStorageAccount --- api/v1alpha1/storage_types.go | 42 +++++++++++ config/samples/azure_v1alpha1_storage.yaml | 18 ++++- .../storages/storageaccount/storage.go | 69 ++++++++++++++++++- .../storageaccount/storage_manager.go | 4 +- .../storageaccount/storage_reconcile.go | 3 +- 5 files changed, 130 insertions(+), 6 deletions(-) diff --git a/api/v1alpha1/storage_types.go b/api/v1alpha1/storage_types.go index 682c81ffc95..457582ab32e 100644 --- a/api/v1alpha1/storage_types.go +++ b/api/v1alpha1/storage_types.go @@ -29,6 +29,8 @@ type StorageSpec struct { EnableHTTPSTrafficOnly *bool `json:"supportsHttpsTrafficOnly,omitempty"` DataLakeEnabled *bool `json:"dataLakeEnabled,omitempty"` + + NetworkRule *StorageNetworkRuleSet `json:"networkRule,omitempty"` } // Sku the SKU of the storage account. @@ -96,6 +98,46 @@ type StorageList struct { Items []Storage `json:"items"` } +type Bypass string +type Action 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 + IPRule *[]IPRule `json:"ipRules,omitempty"` + // DefaultAction - Specifies the default action of allow or deny when no other rules match. Possible values include: 'DefaultActionAllow', 'DefaultActionDeny' + DefaultAction DefaultAction `json:"defaultAction,omitempty"` +} + +// DefaultAction defined as a string + +const ( + + // AzureServices ... + AzureServices Bypass = "AzureServices" + // Logging ... + Logging Bypass = "Logging" + // Metrics ... + Metrics Bypass = "Metrics" + // None ... + None Bypass = "None" +) + +type VirtualNetworkRule struct { + // VirtualNetworkResourceID - Resource ID of a subnet, for example: /subscriptions/{subscriptionId}/resourceGroups/{groupName}/providers/Microsoft.Network/virtualNetworks/{vnetName}/subnets/{subnetName}. + VirtualNetworkResourceID *string `json:"id,omitempty"` + Action Action `json:"action,omitempty"` +} + +type IPRule struct { + // IPAddressOrRange - Specifies the IP or IP range in CIDR format. Only IPV4 address is allowed. + IPAddressOrRange *string `json:"value,omitempty"` + Action Action `json:"action,omitempty"` +} + func init() { SchemeBuilder.Register(&Storage{}, &StorageList{}) } diff --git a/config/samples/azure_v1alpha1_storage.yaml b/config/samples/azure_v1alpha1_storage.yaml index fd2f6690787..a3eeb567636 100644 --- a/config/samples/azure_v1alpha1_storage.yaml +++ b/config/samples/azure_v1alpha1_storage.yaml @@ -1,12 +1,24 @@ apiVersion: azure.microsoft.com/v1alpha1 kind: Storage metadata: - name: storagesample1908ayzkj + name: storagesamplehong222 spec: location: westus resourceGroup: resourcegroup-azure-operators sku: name: Standard_RAGRS kind: StorageV2 - accessTier: Hot - supportsHttpsTrafficOnly: true + properties: + accessTier: Hot + supportsHttpsTrafficOnly: true + +# Optional: networkRule is only supported on "Standard" tier namespace + networkRule: + bypass: AzureServices + defaultAction: Allow + virtualNetworkRules: + - subnetID: /subscriptions/08daa385-27fa-477a-b556-a9ead8b270d9/resourceGroups/resourcegroup-azure-operator/providers/Microsoft.Network/virtualNetworks/virtualnetwork-sample/subnets/test1 +# ipRules: +# - ipMask: 10.0.0.0/24 +# - ipMask: 2.2.0.0/24 + diff --git a/pkg/resourcemanager/storages/storageaccount/storage.go b/pkg/resourcemanager/storages/storageaccount/storage.go index b43cdd1475b..ec74ca48720 100644 --- a/pkg/resourcemanager/storages/storageaccount/storage.go +++ b/pkg/resourcemanager/storages/storageaccount/storage.go @@ -9,6 +9,7 @@ import ( "log" "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 +19,63 @@ import ( type azureStorageManager struct{} +// ParseNetworkPolicy - helper function to parse network policies from Kubernetes spec +func ParseNetworkPolicy(ruleSet *v1alpha1.StorageNetworkRuleSet) storage.NetworkRuleSet { + var bypass storage.Bypass + + switch ruleSet.Bypass { + case "AzureServices": + bypass = storage.AzureServices + case "None": + bypass = storage.None + case "Logging": + bypass = storage.Logging + case "Metrics": + bypass = storage.Metrics + default: + bypass = storage.AzureServices + } + + var defaultAction storage.DefaultAction + switch ruleSet.DefaultAction { + case "Allow": + defaultAction = storage.DefaultActionAllow + case "Deny": + defaultAction = storage.DefaultActionDeny + default: + defaultAction = storage.DefaultActionDeny + } + + var ipInstances []storage.IPRule + if ruleSet.IPRule != nil { + for _, i := range *ruleSet.IPRule { + subnetID := i.IPAddressOrRange + ipInstances = append(ipInstances, storage.IPRule{ + IPAddressOrRange: subnetID, + Action: storage.Allow, + }) + } + } + + var vnetInstances []storage.VirtualNetworkRule + if ruleSet.VirtualNetworkRules != nil { + for _, i := range *ruleSet.VirtualNetworkRules { + ventID := i.VirtualNetworkResourceID + vnetInstances = append(vnetInstances, storage.VirtualNetworkRule{ + VirtualNetworkResourceID: ventID, + 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,7 +88,8 @@ func getStoragesClient() storage.AccountsClient { } // CreateStorage creates a new storage account -func (_ *azureStorageManager) CreateStorage(ctx context.Context, groupName string, +func (_ *azureStorageManager) CreateStorage(ctx context.Context, instance *v1alpha1.Storage, + groupName string, storageAccountName string, location string, sku azurev1alpha1.StorageSku, @@ -63,6 +122,13 @@ func (_ *azureStorageManager) CreateStorage(ctx context.Context, groupName strin sKind := storage.Kind(kind) sAccessTier := storage.AccessTier(accessTier) + var networkAcls storage.NetworkRuleSet + if instance.Spec.NetworkRule != nil { + networkAcls = ParseNetworkPolicy(instance.Spec.NetworkRule) + } else { + networkAcls = storage.NetworkRuleSet{} + } + params := storage.AccountCreateParameters{ Location: to.StringPtr(location), Sku: &sSku, @@ -73,6 +139,7 @@ func (_ *azureStorageManager) CreateStorage(ctx context.Context, groupName strin AccessTier: sAccessTier, EnableHTTPSTrafficOnly: enableHTTPsTrafficOnly, IsHnsEnabled: dataLakeEnabled, + NetworkRuleSet: &networkAcls, }, } diff --git a/pkg/resourcemanager/storages/storageaccount/storage_manager.go b/pkg/resourcemanager/storages/storageaccount/storage_manager.go index 80437e7b729..08b7c6dba4e 100644 --- a/pkg/resourcemanager/storages/storageaccount/storage_manager.go +++ b/pkg/resourcemanager/storages/storageaccount/storage_manager.go @@ -7,6 +7,7 @@ import ( "context" "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" "github.com/Azure/go-autorest/autorest" @@ -18,7 +19,8 @@ func New() *azureStorageManager { } type StorageManager interface { - CreateStorage(ctx context.Context, groupName string, + CreateStorage(ctx context.Context, instance *v1alpha1.Storage, + groupName string, storageAccountName string, location string, sku azurev1alpha1.StorageSku, diff --git a/pkg/resourcemanager/storages/storageaccount/storage_reconcile.go b/pkg/resourcemanager/storages/storageaccount/storage_reconcile.go index 55ebed67f06..aff0bccadc8 100644 --- a/pkg/resourcemanager/storages/storageaccount/storage_reconcile.go +++ b/pkg/resourcemanager/storages/storageaccount/storage_reconcile.go @@ -11,6 +11,7 @@ import ( "github.com/Azure/azure-service-operator/pkg/errhelp" "github.com/Azure/azure-service-operator/pkg/helpers" "github.com/Azure/azure-service-operator/pkg/resourcemanager" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" ) @@ -66,7 +67,7 @@ func (sa *azureStorageManager) Ensure(ctx context.Context, obj runtime.Object, o instance.Status.Provisioning = true instance.Status.Provisioned = false - _, err = sa.CreateStorage(ctx, groupName, name, location, sku, kind, labels, accessTier, enableHTTPSTrafficOnly, dataLakeEnabled) + _, err = sa.CreateStorage(ctx, instance, groupName, name, location, sku, kind, labels, accessTier, enableHTTPSTrafficOnly, dataLakeEnabled) if err != nil { instance.Status.Message = err.Error() azerr := errhelp.NewAzureErrorAzureError(err) From 24b8092ad671823b0b71e71b45e2ea3480b67871 Mon Sep 17 00:00:00 2001 From: hobu <37413937+buhongw7583c@users.noreply.github.com> Date: Wed, 8 Apr 2020 21:42:54 +0800 Subject: [PATCH 02/15] issue#858AddVentStorage0.2 --- api/v1alpha1/storage_types.go | 19 +++--- config/samples/azure_v1alpha1_storage.yaml | 18 +++--- controllers/storage_controller_test.go | 61 +++++++++++++++++++ controllers/suite_test.go | 2 +- pkg/resourcemanager/mock/storages/storage.go | 2 +- .../storageaccount/storage_manager.go | 5 +- .../storageaccount/storage_reconcile.go | 8 ++- 7 files changed, 89 insertions(+), 26 deletions(-) diff --git a/api/v1alpha1/storage_types.go b/api/v1alpha1/storage_types.go index 457582ab32e..a61c370f1d9 100644 --- a/api/v1alpha1/storage_types.go +++ b/api/v1alpha1/storage_types.go @@ -99,21 +99,20 @@ type StorageList struct { } type Bypass string -type Action 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 - 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 - IPRule *[]IPRule `json:"ipRules,omitempty"` + 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 DefaultAction `json:"defaultAction,omitempty"` + DefaultAction string `json:"defaultAction,omitempty"` } -// DefaultAction defined as a string - const ( // AzureServices ... @@ -127,15 +126,13 @@ const ( ) type VirtualNetworkRule struct { - // VirtualNetworkResourceID - Resource ID of a subnet, for example: /subscriptions/{subscriptionId}/resourceGroups/{groupName}/providers/Microsoft.Network/virtualNetworks/{vnetName}/subnets/{subnetName}. - VirtualNetworkResourceID *string `json:"id,omitempty"` - Action Action `json:"action,omitempty"` + // 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:"value,omitempty"` - Action Action `json:"action,omitempty"` + IPAddressOrRange *string `json:"ipAddressOrRange,omitempty"` } func init() { diff --git a/config/samples/azure_v1alpha1_storage.yaml b/config/samples/azure_v1alpha1_storage.yaml index a3eeb567636..02b25809087 100644 --- a/config/samples/azure_v1alpha1_storage.yaml +++ b/config/samples/azure_v1alpha1_storage.yaml @@ -1,7 +1,7 @@ apiVersion: azure.microsoft.com/v1alpha1 kind: Storage metadata: - name: storagesamplehong222 + name: storagesamplegfdsa spec: location: westus resourceGroup: resourcegroup-azure-operators @@ -11,14 +11,14 @@ spec: properties: accessTier: Hot supportsHttpsTrafficOnly: true - -# Optional: networkRule is only supported on "Standard" tier namespace +# Optional: networkRule networkRule: - bypass: AzureServices - defaultAction: Allow + bypass: AzureServices # Possible values are AzureServices, Metrics, None, Logging + defaultAction: Allow # Possible values are Allow, Deny virtualNetworkRules: - - subnetID: /subscriptions/08daa385-27fa-477a-b556-a9ead8b270d9/resourceGroups/resourcegroup-azure-operator/providers/Microsoft.Network/virtualNetworks/virtualnetwork-sample/subnets/test1 -# ipRules: -# - ipMask: 10.0.0.0/24 -# - ipMask: 2.2.0.0/24 + - subnetId: /subscriptions/08daa385-27fa-477a-b556-a9ead8b270d9/resourceGroups/resourcegroup-azure-operator/providers/Microsoft.Network/virtualNetworks/virtualnetwork-sample/subnets/test1 + ipRules: + - ipAddressOrRange: 2.2.0.0/24 + - ipAddressOrRange: 2.2.2.1 + diff --git a/controllers/storage_controller_test.go b/controllers/storage_controller_test.go index f4a6c00f9a7..b8ce7f1fbaf 100644 --- a/controllers/storage_controller_test.go +++ b/controllers/storage_controller_test.go @@ -10,6 +10,7 @@ import ( "testing" azurev1alpha1 "github.com/Azure/azure-service-operator/api/v1alpha1" + config "github.com/Azure/azure-service-operator/pkg/resourcemanager/config" "github.com/Azure/go-autorest/autorest/to" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -46,3 +47,63 @@ func TestStorageControllerHappyPath(t *testing.T) { // delete rg EnsureDelete(ctx, t, tc, saInstance) } + +func TestStorageControllerHappyPathWithNetworkRule(t *testing.T) { + t.Parallel() + defer PanicRecover(t) + ctx := context.Background() + + StorageAccountName := GenerateAlphaNumTestResourceName("cndev") + + rgName := tc.resourceGroupName + rgLocation := tc.resourceGroupLocation + VNetName := GenerateTestResourceNameWithRandom("vnet", 10) + subnetName := "subnet-test" + + subnetID := "/subscriptions/" + config.SubscriptionID() + "/resourceGroups/" + rgName + "/providers/Microsoft.Network/virtualNetworks/" + VNetName + "/subnets/" + subnetName + vnetRules := []azurev1alpha1.VirtualNetworkRule{ + { + SubnetId: &subnetID, + }, + } + ipAddress := "1.1.1.1" + ipRange := "2.2.2.2/24" + ipRules := []azurev1alpha1.IPRule{ + { + IPAddressOrRange: &ipAddress, + }, + { + IPAddressOrRange: &ipRange, + }, + } + + // Create the ResourceGroup object and expect the Reconcile to be created + cnInstance := &azurev1alpha1.Storage{ + ObjectMeta: metav1.ObjectMeta{ + Name: StorageAccountName, + Namespace: "default", + }, + Spec: azurev1alpha1.StorageSpec{ + Location: rgLocation, + ResourceGroup: rgName, + Sku: azurev1alpha1.StorageSku{ + Name: "Standard_RAGRS", + }, + Kind: "StorageV2", + AccessTier: "Hot", + EnableHTTPSTrafficOnly: to.BoolPtr(true), + NetworkRule: &azurev1alpha1.StorageNetworkRuleSet{ + Bypass: "AzureServices", + VirtualNetworkRules: &vnetRules, + IPRules: &ipRules, + DefaultAction: "Deny", + }, + }, + } + + // create rg + EnsureInstance(ctx, t, tc, cnInstance) + + // delete rg + EnsureDelete(ctx, t, tc, cnInstance) +} diff --git a/controllers/suite_test.go b/controllers/suite_test.go index 63f50e48416..048ad379567 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -706,7 +706,7 @@ func setup() error { // Create the Storage Account and Container _, _ = storageAccountManager.CreateStorage(context.Background(), resourceGroupName, storageAccountName, resourcegroupLocation, azurev1alpha1.StorageSku{ 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 diff --git a/pkg/resourcemanager/mock/storages/storage.go b/pkg/resourcemanager/mock/storages/storage.go index 92cc51d207f..7c4d0be5b12 100644 --- a/pkg/resourcemanager/mock/storages/storage.go +++ b/pkg/resourcemanager/mock/storages/storage.go @@ -50,7 +50,7 @@ func (manager *mockStorageManager) CreateStorage(ctx context.Context, groupName kind azurev1alpha1.StorageKind, tags map[string]*string, accessTier azurev1alpha1.StorageAccessTier, - 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, diff --git a/pkg/resourcemanager/storages/storageaccount/storage_manager.go b/pkg/resourcemanager/storages/storageaccount/storage_manager.go index 08b7c6dba4e..3fdd74fcd1d 100644 --- a/pkg/resourcemanager/storages/storageaccount/storage_manager.go +++ b/pkg/resourcemanager/storages/storageaccount/storage_manager.go @@ -7,7 +7,6 @@ import ( "context" "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" "github.com/Azure/go-autorest/autorest" @@ -19,7 +18,7 @@ func New() *azureStorageManager { } type StorageManager interface { - CreateStorage(ctx context.Context, instance *v1alpha1.Storage, + CreateStorage(ctx context.Context, groupName string, storageAccountName string, location string, @@ -27,7 +26,7 @@ type StorageManager interface { kind azurev1alpha1.StorageKind, tags map[string]*string, accessTier azurev1alpha1.StorageAccessTier, - enableHTTPsTrafficOnly *bool, dataLakeEnabled *bool) (result storage.Account, err error) + enableHTTPsTrafficOnly *bool, dataLakeEnabled *bool, networkRule *storage.NetworkRuleSet) (result storage.Account, err error) // Get gets the description of the specified storage account. // Parameters: diff --git a/pkg/resourcemanager/storages/storageaccount/storage_reconcile.go b/pkg/resourcemanager/storages/storageaccount/storage_reconcile.go index aff0bccadc8..74929a12ce1 100644 --- a/pkg/resourcemanager/storages/storageaccount/storage_reconcile.go +++ b/pkg/resourcemanager/storages/storageaccount/storage_reconcile.go @@ -7,6 +7,7 @@ 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" @@ -33,6 +34,11 @@ func (sa *azureStorageManager) Ensure(ctx context.Context, obj runtime.Object, o enableHTTPSTrafficOnly := instance.Spec.EnableHTTPSTrafficOnly dataLakeEnabled := instance.Spec.DataLakeEnabled + 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() { @@ -67,7 +73,7 @@ func (sa *azureStorageManager) Ensure(ctx context.Context, obj runtime.Object, o instance.Status.Provisioning = true instance.Status.Provisioned = false - _, err = sa.CreateStorage(ctx, instance, groupName, name, location, sku, kind, labels, accessTier, enableHTTPSTrafficOnly, dataLakeEnabled) + _, 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) From 12c04f3034777e5db199a7b325af9ab4834ed7ca Mon Sep 17 00:00:00 2001 From: hobu <37413937+buhongw7583c@users.noreply.github.com> Date: Thu, 9 Apr 2020 07:24:57 +0800 Subject: [PATCH 03/15] issue#858#fixYamlfile --- api/v1alpha1/storage_types.go | 2 + config/samples/azure_v1alpha1_storage.yaml | 24 ++++++------ .../storages/storageaccount/storage.go | 39 +++++++------------ 3 files changed, 26 insertions(+), 39 deletions(-) diff --git a/api/v1alpha1/storage_types.go b/api/v1alpha1/storage_types.go index a61c370f1d9..1c7b83ceaff 100644 --- a/api/v1alpha1/storage_types.go +++ b/api/v1alpha1/storage_types.go @@ -31,6 +31,8 @@ type StorageSpec struct { DataLakeEnabled *bool `json:"dataLakeEnabled,omitempty"` NetworkRule *StorageNetworkRuleSet `json:"networkRule,omitempty"` + + //Properties StorageAccountProperties `json:"properties,omitempty"` } // Sku the SKU of the storage account. diff --git a/config/samples/azure_v1alpha1_storage.yaml b/config/samples/azure_v1alpha1_storage.yaml index 02b25809087..30f34270af0 100644 --- a/config/samples/azure_v1alpha1_storage.yaml +++ b/config/samples/azure_v1alpha1_storage.yaml @@ -7,18 +7,16 @@ spec: resourceGroup: resourcegroup-azure-operators sku: name: Standard_RAGRS - kind: StorageV2 - properties: - accessTier: Hot - supportsHttpsTrafficOnly: true + kind: BlobStorage + accessTier: Hot + supportsHttpsTrafficOnly: true # Optional: networkRule - networkRule: - bypass: AzureServices # Possible values are AzureServices, Metrics, None, Logging - defaultAction: Allow # Possible values are Allow, Deny - virtualNetworkRules: - - subnetId: /subscriptions/08daa385-27fa-477a-b556-a9ead8b270d9/resourceGroups/resourcegroup-azure-operator/providers/Microsoft.Network/virtualNetworks/virtualnetwork-sample/subnets/test1 - ipRules: - - ipAddressOrRange: 2.2.0.0/24 - - ipAddressOrRange: 2.2.2.1 - + 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 diff --git a/pkg/resourcemanager/storages/storageaccount/storage.go b/pkg/resourcemanager/storages/storageaccount/storage.go index ec74ca48720..0b45d1d69bd 100644 --- a/pkg/resourcemanager/storages/storageaccount/storage.go +++ b/pkg/resourcemanager/storages/storageaccount/storage.go @@ -7,6 +7,7 @@ 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" @@ -21,8 +22,8 @@ type azureStorageManager struct{} // ParseNetworkPolicy - helper function to parse network policies from Kubernetes spec func ParseNetworkPolicy(ruleSet *v1alpha1.StorageNetworkRuleSet) storage.NetworkRuleSet { - var bypass storage.Bypass + bypass := storage.AzureServices switch ruleSet.Bypass { case "AzureServices": bypass = storage.AzureServices @@ -32,26 +33,19 @@ func ParseNetworkPolicy(ruleSet *v1alpha1.StorageNetworkRuleSet) storage.Network bypass = storage.Logging case "Metrics": bypass = storage.Metrics - default: - bypass = storage.AzureServices } - var defaultAction storage.DefaultAction - switch ruleSet.DefaultAction { - case "Allow": + defaultAction := storage.DefaultActionDeny + if strings.ToLower(ruleSet.DefaultAction) == "allow" { defaultAction = storage.DefaultActionAllow - case "Deny": - defaultAction = storage.DefaultActionDeny - default: - defaultAction = storage.DefaultActionDeny } var ipInstances []storage.IPRule - if ruleSet.IPRule != nil { - for _, i := range *ruleSet.IPRule { - subnetID := i.IPAddressOrRange + if ruleSet.IPRules != nil { + for _, i := range *ruleSet.IPRules { + ipmask := i.IPAddressOrRange ipInstances = append(ipInstances, storage.IPRule{ - IPAddressOrRange: subnetID, + IPAddressOrRange: ipmask, Action: storage.Allow, }) } @@ -60,9 +54,9 @@ func ParseNetworkPolicy(ruleSet *v1alpha1.StorageNetworkRuleSet) storage.Network var vnetInstances []storage.VirtualNetworkRule if ruleSet.VirtualNetworkRules != nil { for _, i := range *ruleSet.VirtualNetworkRules { - ventID := i.VirtualNetworkResourceID + vnetID := i.SubnetId vnetInstances = append(vnetInstances, storage.VirtualNetworkRule{ - VirtualNetworkResourceID: ventID, + VirtualNetworkResourceID: vnetID, Action: storage.Allow, }) } @@ -88,7 +82,7 @@ func getStoragesClient() storage.AccountsClient { } // CreateStorage creates a new storage account -func (_ *azureStorageManager) CreateStorage(ctx context.Context, instance *v1alpha1.Storage, +func (_ *azureStorageManager) CreateStorage(ctx context.Context, groupName string, storageAccountName string, location string, @@ -96,7 +90,7 @@ func (_ *azureStorageManager) CreateStorage(ctx context.Context, instance *v1alp kind azurev1alpha1.StorageKind, tags map[string]*string, accessTier azurev1alpha1.StorageAccessTier, - enableHTTPsTrafficOnly *bool, dataLakeEnabled *bool) (result storage.Account, err error) { + enableHTTPsTrafficOnly *bool, dataLakeEnabled *bool, networkRule *storage.NetworkRuleSet) (result storage.Account, err error) { storagesClient := getStoragesClient() @@ -122,13 +116,6 @@ func (_ *azureStorageManager) CreateStorage(ctx context.Context, instance *v1alp sKind := storage.Kind(kind) sAccessTier := storage.AccessTier(accessTier) - var networkAcls storage.NetworkRuleSet - if instance.Spec.NetworkRule != nil { - networkAcls = ParseNetworkPolicy(instance.Spec.NetworkRule) - } else { - networkAcls = storage.NetworkRuleSet{} - } - params := storage.AccountCreateParameters{ Location: to.StringPtr(location), Sku: &sSku, @@ -139,7 +126,7 @@ func (_ *azureStorageManager) CreateStorage(ctx context.Context, instance *v1alp AccessTier: sAccessTier, EnableHTTPSTrafficOnly: enableHTTPsTrafficOnly, IsHnsEnabled: dataLakeEnabled, - NetworkRuleSet: &networkAcls, + NetworkRuleSet: networkRule, }, } From 400a0dd6af9c1aff528e6814868cd5ae1b6fa607 Mon Sep 17 00:00:00 2001 From: hobu <37413937+buhongw7583c@users.noreply.github.com> Date: Thu, 9 Apr 2020 20:14:55 +0800 Subject: [PATCH 04/15] #issue858#Controllertest --- controllers/storage_controller_test.go | 57 +++++++++++++++++++------- 1 file changed, 43 insertions(+), 14 deletions(-) diff --git a/controllers/storage_controller_test.go b/controllers/storage_controller_test.go index b8ce7f1fbaf..03bd01edc57 100644 --- a/controllers/storage_controller_test.go +++ b/controllers/storage_controller_test.go @@ -7,22 +7,23 @@ package controllers import ( "context" + "strings" "testing" azurev1alpha1 "github.com/Azure/azure-service-operator/api/v1alpha1" config "github.com/Azure/azure-service-operator/pkg/resourcemanager/config" "github.com/Azure/go-autorest/autorest/to" + "github.com/stretchr/testify/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" ) -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.Storage{ ObjectMeta: metav1.ObjectMeta{ @@ -40,25 +41,47 @@ func TestStorageControllerHappyPath(t *testing.T) { EnableHTTPSTrafficOnly: to.BoolPtr(true), }, } - // create rg EnsureInstance(ctx, t, tc, saInstance) - // delete rg EnsureDelete(ctx, t, tc, saInstance) } - func TestStorageControllerHappyPathWithNetworkRule(t *testing.T) { t.Parallel() defer PanicRecover(t) ctx := context.Background() + assert := assert.New(t) + var err error - StorageAccountName := GenerateAlphaNumTestResourceName("cndev") + StorageAccountName := GenerateTestResourceNameWithRandom("storage", 10) rgName := tc.resourceGroupName rgLocation := tc.resourceGroupLocation - VNetName := GenerateTestResourceNameWithRandom("vnet", 10) - subnetName := "subnet-test" + VNetName := GenerateTestResourceNameWithRandom("svnet", 10) + subnetName := "subnet-storage-test" + + // Create a VNET as prereq for the test + + VNetSubNetInstance := azurev1alpha1.VNetSubnets{ + SubnetName: subnetName, + SubnetAddressPrefix: "110.1.0.0/16", + } + + // Create a VNET + VNetInstance := &azurev1alpha1.VirtualNetwork{ + ObjectMeta: metav1.ObjectMeta{ + Name: VNetName, + Namespace: "default", + }, + Spec: azurev1alpha1.VirtualNetworkSpec{ + Location: rgLocation, + ResourceGroup: rgName, + AddressSpace: "110.0.0.0/8", + Subnets: []azurev1alpha1.VNetSubnets{VNetSubNetInstance}, + }, + } + + EnsureInstance(ctx, t, tc, VNetInstance) subnetID := "/subscriptions/" + config.SubscriptionID() + "/resourceGroups/" + rgName + "/providers/Microsoft.Network/virtualNetworks/" + VNetName + "/subnets/" + subnetName vnetRules := []azurev1alpha1.VirtualNetworkRule{ @@ -77,7 +100,7 @@ func TestStorageControllerHappyPathWithNetworkRule(t *testing.T) { }, } - // Create the ResourceGroup object and expect the Reconcile to be created + // Create the storage account object and expect the Reconcile to be created cnInstance := &azurev1alpha1.Storage{ ObjectMeta: metav1.ObjectMeta{ Name: StorageAccountName, @@ -101,9 +124,15 @@ func TestStorageControllerHappyPathWithNetworkRule(t *testing.T) { }, } - // create rg - EnsureInstance(ctx, t, tc, cnInstance) + err = tc.k8sClient.Create(ctx, cnInstance) + assert.Equal(nil, err, "create StorageAccount in k8s") + + storageAccountNamespacedName := types.NamespacedName{Name: StorageAccountName, Namespace: "default"} + + // Wait for the APIMgmtAPI instance to be written to k8s + assert.Eventually(func() bool { + err = tc.k8sClient.Get(ctx, storageAccountNamespacedName, cnInstance) + return strings.Contains(cnInstance.Status.Message, successMsg) + }, tc.timeout, tc.retry, "awaiting storageAccount instance creation") - // delete rg - EnsureDelete(ctx, t, tc, cnInstance) } From 182a66a623374a4ee8ba2405b743f77421a90936 Mon Sep 17 00:00:00 2001 From: jananivMS Date: Thu, 9 Apr 2020 17:45:38 -0600 Subject: [PATCH 05/15] updates --- controllers/storage_controller_test.go | 18 ++++-------------- .../storages/storageaccount/storage.go | 3 +++ .../storageaccount/storage_reconcile.go | 3 +++ 3 files changed, 10 insertions(+), 14 deletions(-) diff --git a/controllers/storage_controller_test.go b/controllers/storage_controller_test.go index 03bd01edc57..c45e1d557af 100644 --- a/controllers/storage_controller_test.go +++ b/controllers/storage_controller_test.go @@ -7,16 +7,13 @@ package controllers import ( "context" - "strings" "testing" azurev1alpha1 "github.com/Azure/azure-service-operator/api/v1alpha1" config "github.com/Azure/azure-service-operator/pkg/resourcemanager/config" "github.com/Azure/go-autorest/autorest/to" - "github.com/stretchr/testify/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" ) func TestStorageControllerHappyPathWithoutNetworkRule(t *testing.T) { @@ -50,10 +47,8 @@ func TestStorageControllerHappyPathWithNetworkRule(t *testing.T) { t.Parallel() defer PanicRecover(t) ctx := context.Background() - assert := assert.New(t) - var err error - StorageAccountName := GenerateTestResourceNameWithRandom("storage", 10) + StorageAccountName := GenerateAlphaNumTestResourceName("sanet") rgName := tc.resourceGroupName rgLocation := tc.resourceGroupLocation @@ -124,15 +119,10 @@ func TestStorageControllerHappyPathWithNetworkRule(t *testing.T) { }, } - err = tc.k8sClient.Create(ctx, cnInstance) - assert.Equal(nil, err, "create StorageAccount in k8s") + EnsureInstance(ctx, t, tc, cnInstance) - storageAccountNamespacedName := types.NamespacedName{Name: StorageAccountName, Namespace: "default"} + // Delete instance - // Wait for the APIMgmtAPI instance to be written to k8s - assert.Eventually(func() bool { - err = tc.k8sClient.Get(ctx, storageAccountNamespacedName, cnInstance) - return strings.Contains(cnInstance.Status.Message, successMsg) - }, tc.timeout, tc.retry, "awaiting storageAccount instance creation") + EnsureDelete(ctx, t, tc, cnInstance) } diff --git a/pkg/resourcemanager/storages/storageaccount/storage.go b/pkg/resourcemanager/storages/storageaccount/storage.go index 0b45d1d69bd..5cb2a5eb29b 100644 --- a/pkg/resourcemanager/storages/storageaccount/storage.go +++ b/pkg/resourcemanager/storages/storageaccount/storage.go @@ -8,6 +8,7 @@ import ( "errors" "log" "strings" + "time" "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2019-04-01/storage" "github.com/Azure/azure-service-operator/api/v1alpha1" @@ -136,6 +137,8 @@ func (_ *azureStorageManager) CreateStorage(ctx context.Context, return result, err } + time.Sleep(2 * time.Second) + return future.Result(storagesClient) } diff --git a/pkg/resourcemanager/storages/storageaccount/storage_reconcile.go b/pkg/resourcemanager/storages/storageaccount/storage_reconcile.go index 74929a12ce1..3a91ad05ceb 100644 --- a/pkg/resourcemanager/storages/storageaccount/storage_reconcile.go +++ b/pkg/resourcemanager/storages/storageaccount/storage_reconcile.go @@ -76,6 +76,7 @@ func (sa *azureStorageManager) Ensure(ctx context.Context, obj runtime.Object, o _, err = sa.CreateStorage(ctx, groupName, name, location, sku, kind, labels, accessTier, enableHTTPSTrafficOnly, dataLakeEnabled, &networkAcls) if err != nil { instance.Status.Message = err.Error() + fmt.Println(err.Error()) azerr := errhelp.NewAzureErrorAzureError(err) instance.Status.Provisioning = false @@ -118,8 +119,10 @@ func (sa *azureStorageManager) Ensure(ctx context.Context, obj runtime.Object, o stop := []string{ errhelp.AccountNameInvalid, + errhelp.NetworkAclsValidationFailure, } if helpers.ContainsString(stop, azerr.Type) { + instance.Status.Provisioning = false return true, nil } From 4a89de5b0f76eef42087a7c235b004d400eec21c Mon Sep 17 00:00:00 2001 From: hobu <37413937+buhongw7583c@users.noreply.github.com> Date: Fri, 10 Apr 2020 13:49:11 +0800 Subject: [PATCH 06/15] #issue858#StorageControllertest#HandleAsyncError --- controllers/storage_controller_test.go | 9 ++++--- controllers/suite_test.go | 2 +- pkg/errhelp/errors.go | 1 + .../storages/storageaccount/storage.go | 20 ++++++++------- .../storageaccount/storage_manager.go | 2 +- .../storageaccount/storage_reconcile.go | 25 +++++++++++++++++-- 6 files changed, 42 insertions(+), 17 deletions(-) diff --git a/controllers/storage_controller_test.go b/controllers/storage_controller_test.go index c45e1d557af..8fc67df0a91 100644 --- a/controllers/storage_controller_test.go +++ b/controllers/storage_controller_test.go @@ -12,6 +12,7 @@ import ( azurev1alpha1 "github.com/Azure/azure-service-operator/api/v1alpha1" config "github.com/Azure/azure-service-operator/pkg/resourcemanager/config" + "github.com/Azure/azure-service-operator/pkg/errhelp" "github.com/Azure/go-autorest/autorest/to" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -43,7 +44,7 @@ func TestStorageControllerHappyPathWithoutNetworkRule(t *testing.T) { // delete rg EnsureDelete(ctx, t, tc, saInstance) } -func TestStorageControllerHappyPathWithNetworkRule(t *testing.T) { +func TestStorageControllerFailurePathWithNetworkRule(t *testing.T) { t.Parallel() defer PanicRecover(t) ctx := context.Background() @@ -75,7 +76,7 @@ func TestStorageControllerHappyPathWithNetworkRule(t *testing.T) { Subnets: []azurev1alpha1.VNetSubnets{VNetSubNetInstance}, }, } - + //Create VNet EnsureInstance(ctx, t, tc, VNetInstance) subnetID := "/subscriptions/" + config.SubscriptionID() + "/resourceGroups/" + rgName + "/providers/Microsoft.Network/virtualNetworks/" + VNetName + "/subnets/" + subnetName @@ -119,10 +120,10 @@ func TestStorageControllerHappyPathWithNetworkRule(t *testing.T) { }, } - EnsureInstance(ctx, t, tc, cnInstance) + //Due to the service endpoint cannot be automatically created, can only verify the failure result + EnsureInstanceWithResult(ctx, t, tc, cnInstance, errhelp.NetworkAclsValidationFailure, false) // Delete instance - EnsureDelete(ctx, t, tc, cnInstance) } diff --git a/controllers/suite_test.go b/controllers/suite_test.go index a9d3ac01976..36db6ea89ae 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -705,7 +705,7 @@ func setup() error { log.Println("Creating SA:", storageAccountName) // Create the Storage Account and Container - _, _ = storageAccountManager.CreateStorage(context.Background(), resourceGroupName, storageAccountName, resourcegroupLocation, azurev1alpha1.StorageSku{ + _, _, _ = storageAccountManager.CreateStorage(context.Background(), resourceGroupName, storageAccountName, resourcegroupLocation, azurev1alpha1.StorageSku{ Name: "Standard_LRS", }, "Storage", map[string]*string{}, "", nil, nil, nil) diff --git a/pkg/errhelp/errors.go b/pkg/errhelp/errors.go index f459b077182..b8ffd15f4dd 100644 --- a/pkg/errhelp/errors.go +++ b/pkg/errhelp/errors.go @@ -53,6 +53,7 @@ const ( RequestDisallowedByPolicy = "RequestDisallowedByPolicy" ServiceBusy = "ServiceBusy" NameNotAvailable = "NameNotAvailable" + NetworkAclsValidationFailure = "NetworkAclsValidationFailure" ) func NewAzureError(err error) error { diff --git a/pkg/resourcemanager/storages/storageaccount/storage.go b/pkg/resourcemanager/storages/storageaccount/storage.go index 5cb2a5eb29b..24e0aa06445 100644 --- a/pkg/resourcemanager/storages/storageaccount/storage.go +++ b/pkg/resourcemanager/storages/storageaccount/storage.go @@ -8,7 +8,6 @@ import ( "errors" "log" "strings" - "time" "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2019-04-01/storage" "github.com/Azure/azure-service-operator/api/v1alpha1" @@ -91,7 +90,7 @@ func (_ *azureStorageManager) CreateStorage(ctx context.Context, kind azurev1alpha1.StorageKind, tags map[string]*string, accessTier azurev1alpha1.StorageAccessTier, - enableHTTPsTrafficOnly *bool, dataLakeEnabled *bool, networkRule *storage.NetworkRuleSet) (result storage.Account, err error) { + enableHTTPsTrafficOnly *bool, dataLakeEnabled *bool, networkRule *storage.NetworkRuleSet) (pollingURL string, result storage.Account, err error) { storagesClient := getStoragesClient() @@ -100,16 +99,19 @@ func (_ *azureStorageManager) CreateStorage(ctx context.Context, 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 } } @@ -134,12 +136,12 @@ func (_ *azureStorageManager) CreateStorage(ctx context.Context, //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 } - time.Sleep(2 * time.Second) + result, err = future.Result(storagesClient) - return future.Result(storagesClient) + return future.PollingURL(), result, err } diff --git a/pkg/resourcemanager/storages/storageaccount/storage_manager.go b/pkg/resourcemanager/storages/storageaccount/storage_manager.go index 3fdd74fcd1d..6bd272928a6 100644 --- a/pkg/resourcemanager/storages/storageaccount/storage_manager.go +++ b/pkg/resourcemanager/storages/storageaccount/storage_manager.go @@ -26,7 +26,7 @@ type StorageManager interface { kind azurev1alpha1.StorageKind, tags map[string]*string, accessTier azurev1alpha1.StorageAccessTier, - enableHTTPsTrafficOnly *bool, dataLakeEnabled *bool, networkRule *storage.NetworkRuleSet) (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: diff --git a/pkg/resourcemanager/storages/storageaccount/storage_reconcile.go b/pkg/resourcemanager/storages/storageaccount/storage_reconcile.go index 3a91ad05ceb..a46a1e0d0d5 100644 --- a/pkg/resourcemanager/storages/storageaccount/storage_reconcile.go +++ b/pkg/resourcemanager/storages/storageaccount/storage_reconcile.go @@ -12,6 +12,7 @@ import ( "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" @@ -33,6 +34,7 @@ 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{} @@ -51,11 +53,26 @@ 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) + if err != 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 } @@ -67,13 +84,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, &networkAcls) + pollURL, _, err = sa.CreateStorage(ctx, groupName, name, location, sku, kind, labels, accessTier, enableHTTPSTrafficOnly, dataLakeEnabled, &networkAcls) if err != nil { instance.Status.Message = err.Error() fmt.Println(err.Error()) @@ -113,6 +131,7 @@ 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 } @@ -122,7 +141,9 @@ func (sa *azureStorageManager) Ensure(ctx context.Context, obj runtime.Object, o 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 } From 9c0587b421835708e734a4fc4daba18255ee09ad Mon Sep 17 00:00:00 2001 From: hobu <37413937+buhongw7583c@users.noreply.github.com> Date: Fri, 10 Apr 2020 20:05:14 +0800 Subject: [PATCH 07/15] #issue858#conflict resolve --- controllers/storage_controller_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/controllers/storage_controller_test.go b/controllers/storage_controller_test.go index 8fc67df0a91..cca3e843209 100644 --- a/controllers/storage_controller_test.go +++ b/controllers/storage_controller_test.go @@ -124,6 +124,7 @@ func TestStorageControllerFailurePathWithNetworkRule(t *testing.T) { EnsureInstanceWithResult(ctx, t, tc, cnInstance, errhelp.NetworkAclsValidationFailure, false) // Delete instance + EnsureDelete(ctx, t, tc, cnInstance) } From 68614704e26dc1f4806e3ee0ebc13b047694f30b Mon Sep 17 00:00:00 2001 From: hobu <37413937+buhongw7583c@users.noreply.github.com> Date: Fri, 10 Apr 2020 20:41:46 +0800 Subject: [PATCH 08/15] #issue858#updatecontroller test --- controllers/storage_controller_test.go | 84 -------------------------- 1 file changed, 84 deletions(-) diff --git a/controllers/storage_controller_test.go b/controllers/storage_controller_test.go index cca3e843209..d12b2b04cdf 100644 --- a/controllers/storage_controller_test.go +++ b/controllers/storage_controller_test.go @@ -44,87 +44,3 @@ func TestStorageControllerHappyPathWithoutNetworkRule(t *testing.T) { // delete rg EnsureDelete(ctx, t, tc, saInstance) } -func TestStorageControllerFailurePathWithNetworkRule(t *testing.T) { - t.Parallel() - defer PanicRecover(t) - ctx := context.Background() - - StorageAccountName := GenerateAlphaNumTestResourceName("sanet") - - rgName := tc.resourceGroupName - rgLocation := tc.resourceGroupLocation - VNetName := GenerateTestResourceNameWithRandom("svnet", 10) - subnetName := "subnet-storage-test" - - // Create a VNET as prereq for the test - - VNetSubNetInstance := azurev1alpha1.VNetSubnets{ - SubnetName: subnetName, - SubnetAddressPrefix: "110.1.0.0/16", - } - - // Create a VNET - VNetInstance := &azurev1alpha1.VirtualNetwork{ - ObjectMeta: metav1.ObjectMeta{ - Name: VNetName, - Namespace: "default", - }, - Spec: azurev1alpha1.VirtualNetworkSpec{ - Location: rgLocation, - ResourceGroup: rgName, - AddressSpace: "110.0.0.0/8", - Subnets: []azurev1alpha1.VNetSubnets{VNetSubNetInstance}, - }, - } - //Create VNet - EnsureInstance(ctx, t, tc, VNetInstance) - - subnetID := "/subscriptions/" + config.SubscriptionID() + "/resourceGroups/" + rgName + "/providers/Microsoft.Network/virtualNetworks/" + VNetName + "/subnets/" + subnetName - vnetRules := []azurev1alpha1.VirtualNetworkRule{ - { - SubnetId: &subnetID, - }, - } - ipAddress := "1.1.1.1" - ipRange := "2.2.2.2/24" - ipRules := []azurev1alpha1.IPRule{ - { - IPAddressOrRange: &ipAddress, - }, - { - IPAddressOrRange: &ipRange, - }, - } - - // Create the storage account object and expect the Reconcile to be created - cnInstance := &azurev1alpha1.Storage{ - ObjectMeta: metav1.ObjectMeta{ - Name: StorageAccountName, - Namespace: "default", - }, - Spec: azurev1alpha1.StorageSpec{ - Location: rgLocation, - ResourceGroup: rgName, - Sku: azurev1alpha1.StorageSku{ - Name: "Standard_RAGRS", - }, - Kind: "StorageV2", - AccessTier: "Hot", - EnableHTTPSTrafficOnly: to.BoolPtr(true), - NetworkRule: &azurev1alpha1.StorageNetworkRuleSet{ - Bypass: "AzureServices", - VirtualNetworkRules: &vnetRules, - IPRules: &ipRules, - DefaultAction: "Deny", - }, - }, - } - - //Due to the service endpoint cannot be automatically created, can only verify the failure result - EnsureInstanceWithResult(ctx, t, tc, cnInstance, errhelp.NetworkAclsValidationFailure, false) - - // Delete instance - - EnsureDelete(ctx, t, tc, cnInstance) - -} From c0fdecbe389c2f1b95fc569f9ea6a058a4bfc401 Mon Sep 17 00:00:00 2001 From: hobu <37413937+buhongw7583c@users.noreply.github.com> Date: Fri, 10 Apr 2020 20:58:43 +0800 Subject: [PATCH 09/15] #issue858#remove unused code --- api/v1alpha1/storage_types.go | 2 -- controllers/storage_controller_test.go | 3 --- 2 files changed, 5 deletions(-) diff --git a/api/v1alpha1/storage_types.go b/api/v1alpha1/storage_types.go index 1c7b83ceaff..a61c370f1d9 100644 --- a/api/v1alpha1/storage_types.go +++ b/api/v1alpha1/storage_types.go @@ -31,8 +31,6 @@ type StorageSpec struct { DataLakeEnabled *bool `json:"dataLakeEnabled,omitempty"` NetworkRule *StorageNetworkRuleSet `json:"networkRule,omitempty"` - - //Properties StorageAccountProperties `json:"properties,omitempty"` } // Sku the SKU of the storage account. diff --git a/controllers/storage_controller_test.go b/controllers/storage_controller_test.go index d12b2b04cdf..4b853ae7210 100644 --- a/controllers/storage_controller_test.go +++ b/controllers/storage_controller_test.go @@ -10,9 +10,6 @@ import ( "testing" azurev1alpha1 "github.com/Azure/azure-service-operator/api/v1alpha1" - config "github.com/Azure/azure-service-operator/pkg/resourcemanager/config" - - "github.com/Azure/azure-service-operator/pkg/errhelp" "github.com/Azure/go-autorest/autorest/to" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) From 09c8772faed0173b58b87fde1bf360c20c7ad3b0 Mon Sep 17 00:00:00 2001 From: Hong Bu <37413937+buhongw7583c@users.noreply.github.com> Date: Tue, 14 Apr 2020 17:10:22 +0800 Subject: [PATCH 10/15] Update config/samples/azure_v1alpha1_storage.yaml Co-Authored-By: Erin Corson --- config/samples/azure_v1alpha1_storage.yaml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/config/samples/azure_v1alpha1_storage.yaml b/config/samples/azure_v1alpha1_storage.yaml index 30f34270af0..52241e9ee0b 100644 --- a/config/samples/azure_v1alpha1_storage.yaml +++ b/config/samples/azure_v1alpha1_storage.yaml @@ -1,7 +1,7 @@ apiVersion: azure.microsoft.com/v1alpha1 kind: Storage metadata: - name: storagesamplegfdsa + name: storagesample spec: location: westus resourceGroup: resourcegroup-azure-operators @@ -19,4 +19,3 @@ spec: ipRules: #could be an ip range or a ip address - ipAddressOrRange: 2.2.0.0/24 - ipAddressOrRange: 2.2.2.1 - From 5e9f8e9d879d02a53778f5a9f9bb3a9e3103f8b4 Mon Sep 17 00:00:00 2001 From: Hong Bu <37413937+buhongw7583c@users.noreply.github.com> Date: Tue, 14 Apr 2020 17:11:12 +0800 Subject: [PATCH 11/15] Update pkg/resourcemanager/storages/storageaccount/storage.go Co-Authored-By: Erin Corson --- pkg/resourcemanager/storages/storageaccount/storage.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/resourcemanager/storages/storageaccount/storage.go b/pkg/resourcemanager/storages/storageaccount/storage.go index 24e0aa06445..e09fda9c314 100644 --- a/pkg/resourcemanager/storages/storageaccount/storage.go +++ b/pkg/resourcemanager/storages/storageaccount/storage.go @@ -36,7 +36,7 @@ func ParseNetworkPolicy(ruleSet *v1alpha1.StorageNetworkRuleSet) storage.Network } defaultAction := storage.DefaultActionDeny - if strings.ToLower(ruleSet.DefaultAction) == "allow" { + if strings.EqualFold(ruleSet.DefaultAction, "allow") { defaultAction = storage.DefaultActionAllow } From 45c69890e48c002e796cc47f0e579c221340af1f Mon Sep 17 00:00:00 2001 From: hobu <37413937+buhongw7583c@users.noreply.github.com> Date: Wed, 15 Apr 2020 11:06:47 +0800 Subject: [PATCH 12/15] #issue858#changePerComments --- controllers/suite_test.go | 2 +- pkg/resourcemanager/mock/storages/storageaccount.go | 2 +- .../storages/storageaccount/storageaccount.go | 2 +- .../storages/storageaccount/storageaccount_manager.go | 2 +- .../storages/storageaccount/storageaccount_reconcile.go | 7 +++++++ 5 files changed, 11 insertions(+), 4 deletions(-) diff --git a/controllers/suite_test.go b/controllers/suite_test.go index 10524dd8241..f3fb0123e48 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -745,7 +745,7 @@ func setup() error { log.Println("Creating SA:", storageAccountName) // Create the Storage Account and Container - _, _, _ = storageAccountManager.CreateStorage(context.Background(), resourceGroupName, storageAccountName, resourcegroupLocation, azurev1alpha1.StorageSku{ + _, _, _ = storageAccountManager.CreateStorage(context.Background(), resourceGroupName, storageAccountName, resourcegroupLocation, azurev1alpha1.StorageAccountSku{ Name: "Standard_LRS", }, "Storage", map[string]*string{}, "", nil, nil, nil) diff --git a/pkg/resourcemanager/mock/storages/storageaccount.go b/pkg/resourcemanager/mock/storages/storageaccount.go index d3962b954f9..48a939f73a4 100644 --- a/pkg/resourcemanager/mock/storages/storageaccount.go +++ b/pkg/resourcemanager/mock/storages/storageaccount.go @@ -49,7 +49,7 @@ func (manager *mockStorageManager) CreateStorage(ctx context.Context, groupName sku azurev1alpha1.StorageAccountSku, kind azurev1alpha1.StorageAccountKind, tags map[string]*string, - accessTier azurev1alpha1.StorageAccessTier, + accessTier azurev1alpha1.StorageAccountAccessTier, enableHTTPsTrafficOnly *bool, dataLakeEnabled *bool, networkRule *azurev1alpha1.NetworkRuleSet) (result storage.Account, err error) { s := storageResource{ resourceGroupName: groupName, diff --git a/pkg/resourcemanager/storages/storageaccount/storageaccount.go b/pkg/resourcemanager/storages/storageaccount/storageaccount.go index 3b80ca5d7dc..e2e75b353d6 100644 --- a/pkg/resourcemanager/storages/storageaccount/storageaccount.go +++ b/pkg/resourcemanager/storages/storageaccount/storageaccount.go @@ -89,7 +89,7 @@ func (_ *azureStorageManager) CreateStorage(ctx context.Context, sku azurev1alpha1.StorageAccountSku, kind azurev1alpha1.StorageAccountKind, tags map[string]*string, - accessTier azurev1alpha1.StorageAccessTier, + accessTier azurev1alpha1.StorageAccountAccessTier, enableHTTPsTrafficOnly *bool, dataLakeEnabled *bool, networkRule *storage.NetworkRuleSet) (pollingURL string, result storage.Account, err error) { storagesClient := getStoragesClient() diff --git a/pkg/resourcemanager/storages/storageaccount/storageaccount_manager.go b/pkg/resourcemanager/storages/storageaccount/storageaccount_manager.go index a1d19888d04..5ef05d8e7d2 100644 --- a/pkg/resourcemanager/storages/storageaccount/storageaccount_manager.go +++ b/pkg/resourcemanager/storages/storageaccount/storageaccount_manager.go @@ -25,7 +25,7 @@ type StorageManager interface { sku azurev1alpha1.StorageAccountSku, kind azurev1alpha1.StorageAccountKind, tags map[string]*string, - accessTier azurev1alpha1.StorageAccessTier, + accessTier azurev1alpha1.StorageAccountAccessTier, enableHTTPsTrafficOnly *bool, dataLakeEnabled *bool, networkRule *storage.NetworkRuleSet) (pollingURL string, result storage.Account, err error) // Get gets the description of the specified storage account. diff --git a/pkg/resourcemanager/storages/storageaccount/storageaccount_reconcile.go b/pkg/resourcemanager/storages/storageaccount/storageaccount_reconcile.go index 445d967958e..45efda9e48c 100644 --- a/pkg/resourcemanager/storages/storageaccount/storageaccount_reconcile.go +++ b/pkg/resourcemanager/storages/storageaccount/storageaccount_reconcile.go @@ -58,7 +58,14 @@ func (sa *azureStorageManager) Ensure(ctx context.Context, obj runtime.Object, o 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 } From 03459caf249b1bbebb3cff1721622e5795ae6369 Mon Sep 17 00:00:00 2001 From: hobu <37413937+buhongw7583c@users.noreply.github.com> Date: Wed, 15 Apr 2020 11:45:56 +0800 Subject: [PATCH 13/15] #updateyaml --- config/samples/azure_v1alpha1_storageaccount.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/samples/azure_v1alpha1_storageaccount.yaml b/config/samples/azure_v1alpha1_storageaccount.yaml index 9be18126eda..77deb93e411 100644 --- a/config/samples/azure_v1alpha1_storageaccount.yaml +++ b/config/samples/azure_v1alpha1_storageaccount.yaml @@ -1,7 +1,7 @@ apiVersion: azure.microsoft.com/v1alpha1 kind: StorageAccount metadata: - name: storagesample + name: storagesample777 spec: location: westus resourceGroup: resourcegroup-azure-operators From f050ac36a8460b04039a48f852bc9d5c6a78180e Mon Sep 17 00:00:00 2001 From: hobu <37413937+buhongw7583c@users.noreply.github.com> Date: Wed, 15 Apr 2020 11:46:57 +0800 Subject: [PATCH 14/15] #updateyamle --- config/samples/azure_v1alpha1_storageaccount.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/samples/azure_v1alpha1_storageaccount.yaml b/config/samples/azure_v1alpha1_storageaccount.yaml index 77deb93e411..39b590fe276 100644 --- a/config/samples/azure_v1alpha1_storageaccount.yaml +++ b/config/samples/azure_v1alpha1_storageaccount.yaml @@ -1,7 +1,7 @@ apiVersion: azure.microsoft.com/v1alpha1 kind: StorageAccount metadata: - name: storagesample777 + name: storageaccountsample777 spec: location: westus resourceGroup: resourcegroup-azure-operators From 1e62a4a2ec8e5f8c473571c742d52ca5c78e9a8b Mon Sep 17 00:00:00 2001 From: hobu <37413937+buhongw7583c@users.noreply.github.com> Date: Thu, 16 Apr 2020 08:49:23 +0800 Subject: [PATCH 15/15] #issue858#removeWrongFile --- config/default/manager_image_patch.yaml-e | 50 ----------------------- 1 file changed, 50 deletions(-) delete mode 100644 config/default/manager_image_patch.yaml-e diff --git a/config/default/manager_image_patch.yaml-e b/config/default/manager_image_patch.yaml-e deleted file mode 100644 index 9c62eb69973..00000000000 --- a/config/default/manager_image_patch.yaml-e +++ /dev/null @@ -1,50 +0,0 @@ -apiVersion: apps/v1 -kind: Deployment -metadata: - name: controller-manager - namespace: system -spec: - template: - spec: - containers: - # Change the value of image field below to your controller image URL - - image: controller:latest - name: manager - env: - - name: AZURE_CLIENT_ID - valueFrom: - secretKeyRef: - name: azureoperatorsettings - key: AZURE_CLIENT_ID - optional: true - - name: AZURE_CLIENT_SECRET - valueFrom: - secretKeyRef: - name: azureoperatorsettings - key: AZURE_CLIENT_SECRET - optional: true - - name: AZURE_TENANT_ID - valueFrom: - secretKeyRef: - name: azureoperatorsettings - key: AZURE_TENANT_ID - - name: AZURE_SUBSCRIPTION_ID - valueFrom: - secretKeyRef: - name: azureoperatorsettings - key: AZURE_SUBSCRIPTION_ID - - name: AZURE_USE_MI - valueFrom: - secretKeyRef: - name: azureoperatorsettings - key: AZURE_USE_MI - optional: true - - name: AZURE_OPERATOR_KEYVAULT - valueFrom: - secretKeyRef: - name: azureoperatorsettings - key: AZURE_OPERATOR_KEYVAULT - optional: true - #requeue after time in seconds" - - name: REQUEUE_AFTER - value: "30"