Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue#858 Add Vnet to Storage Account #904

Merged
merged 36 commits into from
Apr 17, 2020
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
4ab6d20
issue#858#WIPaddVnetStorageAccount
buhongw7583c Apr 7, 2020
a13cc11
Merge upstream to current branch
buhongw7583c Apr 7, 2020
24b8092
issue#858AddVentStorage0.2
buhongw7583c Apr 8, 2020
d3e62e7
Merge branch 'master' of https://github.com/Azure/azure-service-opera…
buhongw7583c Apr 8, 2020
12c04f3
issue#858#fixYamlfile
buhongw7583c Apr 8, 2020
c15eaba
Merge branch 'master' of https://github.com/Azure/azure-service-opera…
buhongw7583c Apr 8, 2020
569f067
Merge branch 'master' into issue#858
buhongw7583c Apr 9, 2020
d681d4c
Merge branch 'master' into issue#858
buhongw7583c Apr 9, 2020
f7e8d6a
:Merge branch 'master' of https://github.com/Azure/azure-service-oper…
buhongw7583c Apr 9, 2020
400a0dd
#issue858#Controllertest
buhongw7583c Apr 9, 2020
3bb8cb0
:1Merge branch 'issue#858' of https://github.com/buhongw7583c/azure-s…
buhongw7583c Apr 9, 2020
182a66a
updates
jananivMS Apr 9, 2020
d5a30c2
Merge pull request #1 from jananivMS/hongstortest
buhongw7583c Apr 10, 2020
4a89de5
#issue858#StorageControllertest#HandleAsyncError
buhongw7583c Apr 10, 2020
b7bbc9f
Merge branch 'master' into issue#858
buhongw7583c Apr 10, 2020
ea92543
Merge branch 'master' of https://github.com/Azure/azure-service-opera…
buhongw7583c Apr 10, 2020
9c0587b
#issue858#conflict resolve
buhongw7583c Apr 10, 2020
5721b5b
Merge branch 'issue#858' of https://github.com/buhongw7583c/azure-ser…
buhongw7583c Apr 10, 2020
6861470
#issue858#updatecontroller test
buhongw7583c Apr 10, 2020
c0fdecb
#issue858#remove unused code
buhongw7583c Apr 10, 2020
b8ffe66
Merge branch 'master' into issue#858
WilliamMortlMicrosoft Apr 13, 2020
09c8772
Update config/samples/azure_v1alpha1_storage.yaml
buhongw7583c Apr 14, 2020
5e9f8e9
Update pkg/resourcemanager/storages/storageaccount/storage.go
buhongw7583c Apr 14, 2020
6bf9286
Merge branch 'issue#858' of https://github.com/buhongw7583c/azure-ser…
buhongw7583c Apr 14, 2020
230792f
Merge branch 'master' of https://github.com/Azure/azure-service-opera…
buhongw7583c Apr 15, 2020
45c6989
#issue858#changePerComments
buhongw7583c Apr 15, 2020
45af4d4
Merge branch 'buhongw7583c-issue#858'
buhongw7583c Apr 15, 2020
03459ca
#updateyaml
buhongw7583c Apr 15, 2020
f050ac3
#updateyamle
buhongw7583c Apr 15, 2020
4ce17b4
Merge branch 'master' into issue#858
frodopwns Apr 15, 2020
4f8c974
Merge branch 'master' into issue#858
frodopwns Apr 15, 2020
509b7e9
Merge branch 'master' into issue#858
buhongw7583c Apr 16, 2020
b33cd2c
::Merge branch 'master' of https://github.com/Azure/azure-service-ope…
buhongw7583c Apr 16, 2020
1e62a4a
#issue858#removeWrongFile
buhongw7583c Apr 16, 2020
7327240
Merge branch 'issue#858' of https://github.com/buhongw7583c/azure-ser…
buhongw7583c Apr 16, 2020
7371d2d
Merge branch 'master' into issue#858
frodopwns Apr 16, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions api/v1alpha1/storage_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -96,6 +98,43 @@ type StorageList struct {
Items []Storage `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(&Storage{}, &StorageList{})
}
Expand Down
14 changes: 12 additions & 2 deletions config/samples/azure_v1alpha1_storage.yaml
Original file line number Diff line number Diff line change
@@ -1,12 +1,22 @@
apiVersion: azure.microsoft.com/v1alpha1
kind: Storage
metadata:
name: storagesample1908ayzkj
name: storagesamplegfdsa
buhongw7583c marked this conversation as resolved.
Show resolved Hide resolved
spec:
location: westus
resourceGroup: resourcegroup-azure-operators
sku:
name: Standard_RAGRS
kind: StorageV2
kind: BlobStorage
frodopwns marked this conversation as resolved.
Show resolved Hide resolved
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

7 changes: 1 addition & 6 deletions controllers/storage_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.Storage{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -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)
}
4 changes: 2 additions & 2 deletions controllers/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.StorageSku{
_, _, _ = 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
Expand Down
5 changes: 4 additions & 1 deletion pkg/errhelp/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,10 @@ const (
RequestDisallowedByPolicy = "RequestDisallowedByPolicy"
ServiceBusy = "ServiceBusy"
NameNotAvailable = "NameNotAvailable"
PublicIPIdleTimeoutIsOutOfRange = "PublicIPIdleTimeoutIsOutOfRange"

NetworkAclsValidationFailure = "NetworkAclsValidationFailure"

PublicIPIdleTimeoutIsOutOfRange = "PublicIPIdleTimeoutIsOutOfRange"
)

func NewAzureError(err error) error {
Expand Down
2 changes: 1 addition & 1 deletion pkg/resourcemanager/mock/storages/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
75 changes: 67 additions & 8 deletions pkg/resourcemanager/storages/storageaccount/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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.ToLower(ruleSet.DefaultAction) == "allow" {
buhongw7583c marked this conversation as resolved.
Show resolved Hide resolved
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()
Expand All @@ -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.StorageSku,
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) (pollingURL string, result storage.Account, err error) {

storagesClient := getStoragesClient()

Expand All @@ -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
}
}

Expand All @@ -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

}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.StorageSku,
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) (pollingURL string, result storage.Account, err error)

// Get gets the description of the specified storage account.
// Parameters:
Expand Down
35 changes: 33 additions & 2 deletions pkg/resourcemanager/storages/storageaccount/storage_reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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() {
Expand All @@ -44,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
frodopwns marked this conversation as resolved.
Show resolved Hide resolved
}

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
}
Expand All @@ -60,15 +84,17 @@ 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()
fmt.Println(err.Error())
buhongw7583c marked this conversation as resolved.
Show resolved Hide resolved
azerr := errhelp.NewAzureErrorAzureError(err)
instance.Status.Provisioning = false

Expand Down Expand Up @@ -105,14 +131,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
}

Expand Down