Repository: kubernetes-sigs/cloud-provider-azure
Pull request #701 (2021-07-14 10:15:56)

feat: support networkAccessPolicy
<!--  Thanks for sending a pull request!  Here are some tips for you:

1. If this is your first time, please read our contributor guidelines: https://git.k8s.io/community/contributors/guide#your-first-contribution and developer guide https://git.k8s.io/community/contributors/devel/development.md#development-guide
2. Please label this pull request according to what type of issue you are addressing, especially if this is a release targeted pull request. For reference on required PR/issue labels, read here:
https://git.k8s.io/community/contributors/devel/release.md#issue-kind-label
3. Ensure you have added or ran the appropriate tests for your PR: https://git.k8s.io/community/contributors/devel/testing.md
4. If you want *faster* PR reviews, read how: https://git.k8s.io/community/contributors/guide/pull-requests.md#best-practices-for-faster-reviews
5. Follow the instructions for writing a release note: https://git.k8s.io/community/contributors/guide/release-notes.md
6. If the PR is unfinished, see how to mark it: https://git.k8s.io/community/contributors/guide/pull-requests.md#marking-unfinished-pull-requests
-->

**What type of PR is this?**
/kind feature

**What this PR does / why we need it**:
feat: support networkAccessPolicy

**Which issue(s) this PR fixes**:
<!-- 
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
_If PR is about `failing-tests or flakes`, please post the related issues/tests in a comment and do not use `Fixes`_*
-->
Fixes #

**Special notes for your reviewer**:
/hold

**Release note**:
```
feat: support networkAccessPolicy
```


k8s-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes-sigs/cloud-provider-azure/pull/701#" title="Author self-approved">andyzhangx</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes-sigs%2Fcloud-provider-azure).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/kubernetes-sigs/cloud-provider-azure/blob/master/OWNERS)~~ [andyzhangx]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
k8s-ci-robot: @andyzhangx: The following test **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-cloud-provider-azure-check | 8ae6747609b8ead24a0ff99ac3e8a7e2b879ad33 | [link](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_cloud-provider-azure/701/pull-cloud-provider-azure-check/1415253727920525312) | `/test pull-cloud-provider-azure-check`

[Full PR test history](https://prow.k8s.io/pr-history?org=kubernetes-sigs&repo=cloud-provider-azure&pr=701). [Your PR dashboard](https://prow.k8s.io/pr?query=is%3Apr%20state%3Aopen%20author%3Aandyzhangx). Please help us cut down on flakes by [linking to](https://git.k8s.io/community/contributors/devel/sig-testing/flaky-tests.md#filing-issues-for-flaky-tests) an [open issue](https://github.com/kubernetes-sigs/cloud-provider-azure/issues?q=is:issue+is:open) when you hit one in your PR.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
coveralls: 
[![Coverage Status](https://coveralls.io/builds/41358793/badge)](https://coveralls.io/builds/41358793)

Coverage increased (+0.01%) to 79.553% when pulling **8ae6747609b8ead24a0ff99ac3e8a7e2b879ad33 on andyzhangx:networkAccessPolicy** into **a972a9825ba4bf07ce733234e25427f0c96abab9 on kubernetes-sigs:master**.


Patch:
diff --git a/.golangci.yml b/.golangci.yml
index 39c9edd66..5f8e8c35f 100644
--- a/.golangci.yml
+++ b/.golangci.yml
@@ -9,7 +9,7 @@ linters:
     - errcheck
 #    - errorlint
     - goconst
-    - gocyclo
+#    - gocyclo
     - gofmt
     - goimports
     - golint
diff --git a/pkg/provider/azure_managedDiskController.go b/pkg/provider/azure_managedDiskController.go
index 9f3c6c7c8..548c3e09d 100644
--- a/pkg/provider/azure_managedDiskController.go
+++ b/pkg/provider/azure_managedDiskController.go
@@ -74,6 +74,12 @@ type ManagedDiskOptions struct {
 	LogicalSectorSize int32
 	// SkipGetDiskOperation indicates whether skip GetDisk operation(mainly due to throttling)
 	SkipGetDiskOperation bool
+	// NetworkAccessPolicy - Possible values include: 'AllowAll', 'AllowPrivate', 'DenyAll'
+	NetworkAccessPolicy compute.NetworkAccessPolicy
+	// DiskAccessID - ARM id of the DiskAccess resource for using private endpoints on disks.
+	DiskAccessID *string
+	// BurstingEnabled - Set to true to enable bursting beyond the provisioned performance target of the disk.
+	BurstingEnabled *bool
 }
 
 //CreateManagedDisk : create managed disk
@@ -110,8 +116,23 @@ func (c *ManagedDiskController) CreateManagedDisk(options *ManagedDiskOptions) (
 		return "", err
 	}
 	diskProperties := compute.DiskProperties{
-		DiskSizeGB:   &diskSizeGB,
-		CreationData: &creationData,
+		DiskSizeGB:      &diskSizeGB,
+		CreationData:    &creationData,
+		BurstingEnabled: options.BurstingEnabled,
+	}
+
+	if options.NetworkAccessPolicy != "" {
+		diskProperties.NetworkAccessPolicy = options.NetworkAccessPolicy
+		if options.NetworkAccessPolicy == compute.AllowPrivate {
+			if options.DiskAccessID == nil {
+				return "", fmt.Errorf("DiskAccessID should not be empty when NetworkAccessPolicy is AllowPrivate")
+			}
+			diskProperties.DiskAccessID = options.DiskAccessID
+		} else {
+			if options.DiskAccessID != nil {
+				return "", fmt.Errorf("DiskAccessID(%s) must be empty when NetworkAccessPolicy(%s) is not AllowPrivate", *options.DiskAccessID, options.NetworkAccessPolicy)
+			}
+		}
 	}
 
 	if diskSku == compute.UltraSSDLRS {


Test patch:
diff --git a/pkg/provider/azure_managedDiskController_test.go b/pkg/provider/azure_managedDiskController_test.go
index e627d3bbe..b00d80622 100644
--- a/pkg/provider/azure_managedDiskController_test.go
+++ b/pkg/provider/azure_managedDiskController_test.go
@@ -59,6 +59,8 @@ func TestCreateManagedDisk(t *testing.T) {
 		diskIOPSReadWrite   string
 		diskMBPSReadWrite   string
 		diskEncryptionSetID string
+		networkAccessPolicy compute.NetworkAccessPolicy
+		diskAccessID        *string
 		expectedDiskID      string
 		existedDisk         compute.Disk
 		expectedErr         bool
@@ -153,6 +155,53 @@ func TestCreateManagedDisk(t *testing.T) {
 			expectedErr:         true,
 			expectedErrMsg:      fmt.Errorf("AzureDisk - DiskMBpsReadWrite parameter is only applicable in UltraSSD_LRS disk type"),
 		},
+		{
+			desc:                "correct NetworkAccessPolicy(DenyAll) setting",
+			diskID:              disk1ID,
+			diskName:            disk1Name,
+			storageAccountType:  compute.StandardLRS,
+			diskEncryptionSetID: goodDiskEncryptionSetID,
+			networkAccessPolicy: compute.DenyAll,
+			expectedDiskID:      disk1ID,
+			existedDisk:         compute.Disk{ID: to.StringPtr(disk1ID), Name: to.StringPtr(disk1Name), DiskProperties: &compute.DiskProperties{Encryption: &compute.Encryption{DiskEncryptionSetID: &goodDiskEncryptionSetID, Type: compute.EncryptionTypeEncryptionAtRestWithCustomerKey}, ProvisioningState: to.StringPtr("Succeeded")}, Tags: testTags},
+			expectedErr:         false,
+		},
+		{
+			desc:                "correct NetworkAccessPolicy(AllowAll) setting",
+			diskID:              disk1ID,
+			diskName:            disk1Name,
+			storageAccountType:  compute.StandardLRS,
+			diskEncryptionSetID: goodDiskEncryptionSetID,
+			networkAccessPolicy: compute.AllowAll,
+			expectedDiskID:      disk1ID,
+			existedDisk:         compute.Disk{ID: to.StringPtr(disk1ID), Name: to.StringPtr(disk1Name), DiskProperties: &compute.DiskProperties{Encryption: &compute.Encryption{DiskEncryptionSetID: &goodDiskEncryptionSetID, Type: compute.EncryptionTypeEncryptionAtRestWithCustomerKey}, ProvisioningState: to.StringPtr("Succeeded")}, Tags: testTags},
+			expectedErr:         false,
+		},
+		{
+			desc:                "DiskAccessID should not be empty when NetworkAccessPolicy is AllowPrivate",
+			diskID:              disk1ID,
+			diskName:            disk1Name,
+			storageAccountType:  compute.StandardLRS,
+			diskEncryptionSetID: goodDiskEncryptionSetID,
+			networkAccessPolicy: compute.AllowPrivate,
+			expectedDiskID:      "",
+			existedDisk:         compute.Disk{ID: to.StringPtr(disk1ID), Name: to.StringPtr(disk1Name), DiskProperties: &compute.DiskProperties{Encryption: &compute.Encryption{DiskEncryptionSetID: &goodDiskEncryptionSetID, Type: compute.EncryptionTypeEncryptionAtRestWithCustomerKey}, ProvisioningState: to.StringPtr("Succeeded")}, Tags: testTags},
+			expectedErr:         true,
+			expectedErrMsg:      fmt.Errorf("DiskAccessID should not be empty when NetworkAccessPolicy is AllowPrivate"),
+		},
+		{
+			desc:                "DiskAccessID(%s) must be empty when NetworkAccessPolicy(%s) is not AllowPrivate",
+			diskID:              disk1ID,
+			diskName:            disk1Name,
+			storageAccountType:  compute.StandardLRS,
+			diskEncryptionSetID: goodDiskEncryptionSetID,
+			networkAccessPolicy: compute.AllowAll,
+			diskAccessID:        to.StringPtr("diskAccessID"),
+			expectedDiskID:      "",
+			existedDisk:         compute.Disk{ID: to.StringPtr(disk1ID), Name: to.StringPtr(disk1Name), DiskProperties: &compute.DiskProperties{Encryption: &compute.Encryption{DiskEncryptionSetID: &goodDiskEncryptionSetID, Type: compute.EncryptionTypeEncryptionAtRestWithCustomerKey}, ProvisioningState: to.StringPtr("Succeeded")}, Tags: testTags},
+			expectedErr:         true,
+			expectedErrMsg:      fmt.Errorf("DiskAccessID(diskAccessID) must be empty when NetworkAccessPolicy(AllowAll) is not AllowPrivate"),
+		},
 	}
 
 	for i, test := range testCases {
@@ -169,6 +218,8 @@ func TestCreateManagedDisk(t *testing.T) {
 			DiskMBpsReadWrite:   test.diskMBPSReadWrite,
 			DiskEncryptionSetID: test.diskEncryptionSetID,
 			MaxShares:           maxShare,
+			NetworkAccessPolicy: test.networkAccessPolicy,
+			DiskAccessID:        test.diskAccessID,
 		}
 
 		mockDisksClient := testCloud.DisksClient.(*mockdiskclient.MockInterface)