feat: support networkAccessPolicy

kubernetes-sigs · Jul 14, 2021 · 764a466 · 764a466
1 parent 6e6efcd
commit 764a466
Show file tree

Hide file tree

Showing 7 changed files with 126 additions and 24 deletions.
diff --git a/docs/driver-parameters.md b/docs/driver-parameters.md
@@ -20,7 +20,8 @@ tags | azure disk [tags](https://docs.microsoft.com/en-us/azure/azure-resource-m
 diskEncryptionSetID | ResourceId of the disk encryption set to use for [enabling encryption at rest](https://docs.microsoft.com/en-us/azure/virtual-machines/windows/disk-encryption) | format: `/subscriptions/{subs-id}/resourceGroups/{rg-name}/providers/Microsoft.Compute/diskEncryptionSets/{diskEncryptionSet-name}` | No | ""
 writeAcceleratorEnabled | [Write Accelerator on Azure Disks](https://docs.microsoft.com/azure/virtual-machines/windows/how-to-enable-write-accelerator) | `true`, `false` | No | ""
 perfProfile | [disk device performance profile](../docs/enhancements/feat-add-ability-to-tune-azuredisk-performance-parameters.md) | `none`, `basic` | No | `none`
-
+networkAccessPolicy | NetworkAccessPolicy property to prevent anybody from generating the SAS URI for a disk or a snapshot | `AllowAll`, `DenyAll`, `AllowPrivate` | No | `AllowAll`
+diskAccessID | ARM id of the DiskAccess resource for using private endpoints on disks | | No  | ``
 
  - disk created by dynamic provisioning
    - disk name format(example): `pvc-e132d37f-9e8f-434a-b599-15a4ab211b39`

diff --git a/pkg/azuredisk/azure_common.go b/pkg/azuredisk/azure_common.go
@@ -46,6 +46,19 @@ var (
 	lunPathRE = regexp.MustCompile(`/dev(?:.*)/disk/azure/scsi(?:.*)/lun(.+)`)
 )
 
+func normalizeNetworkAccessPolicy(networkAccessPolicy string) (compute.NetworkAccessPolicy, error) {
+	if networkAccessPolicy == "" {
+		return compute.AllowAll, nil
+	}
+	policy := compute.NetworkAccessPolicy(networkAccessPolicy)
+	for _, s := range compute.PossibleNetworkAccessPolicyValues() {
+		if policy == s {
+			return policy, nil
+		}
+	}
+	return "", fmt.Errorf("azureDisk - %s is not supported NetworkAccessPolicy. Supported values are %s", networkAccessPolicy, compute.PossibleNetworkAccessPolicyValues())
+}
+
 func normalizeStorageAccountType(storageAccountType, cloud string, disableAzureStackCloud bool) (compute.DiskStorageAccountTypes, error) {
 	if storageAccountType == "" {
 		if IsAzureStackCloud(cloud, disableAzureStackCloud) {

diff --git a/pkg/azuredisk/azure_common_test.go b/pkg/azuredisk/azure_common_test.go
@@ -249,6 +249,52 @@ func TestNormalizeCachingMode(t *testing.T) {
 		assert.Equal(t, err, test.expectedErr, fmt.Sprintf("error msg: %v", err))
 	}
 }
+
+func TestNormalizeNetworkAccessPolicy(t *testing.T) {
+	tests := []struct {
+		networkAccessPolicy         string
+		expectedNetworkAccessPolicy compute.NetworkAccessPolicy
+		expectError                 bool
+	}{
+		{
+			networkAccessPolicy:         "",
+			expectedNetworkAccessPolicy: compute.AllowAll,
+			expectError:                 false,
+		},
+		{
+			networkAccessPolicy:         "AllowAll",
+			expectedNetworkAccessPolicy: compute.AllowAll,
+			expectError:                 false,
+		},
+		{
+			networkAccessPolicy:         "DenyAll",
+			expectedNetworkAccessPolicy: compute.DenyAll,
+			expectError:                 false,
+		},
+		{
+			networkAccessPolicy:         "AllowPrivate",
+			expectedNetworkAccessPolicy: compute.AllowPrivate,
+			expectError:                 false,
+		},
+		{
+			networkAccessPolicy:         "allowAll",
+			expectedNetworkAccessPolicy: compute.NetworkAccessPolicy(""),
+			expectError:                 true,
+		},
+		{
+			networkAccessPolicy:         "invalid",
+			expectedNetworkAccessPolicy: compute.NetworkAccessPolicy(""),
+			expectError:                 true,
+		},
+	}
+
+	for _, test := range tests {
+		result, err := normalizeNetworkAccessPolicy(test.networkAccessPolicy)
+		assert.Equal(t, result, test.expectedNetworkAccessPolicy)
+		assert.Equal(t, err != nil, test.expectError, fmt.Sprintf("error msg: %v", err))
+	}
+}
+
 func TestGetDiskLUN(t *testing.T) {
 	tests := []struct {
 		deviceInfo  string

diff --git a/pkg/azuredisk/azuredisk.go b/pkg/azuredisk/azuredisk.go
@@ -78,22 +78,24 @@ const (
 	// LUN lun number
 	LUN = "LUN"
 
-	cachingModeField        = "cachingmode"
-	storageAccountTypeField = "storageaccounttype"
-	skuNameField            = "skuname"
-	locationField           = "location"
-	resourceGroupField      = "resourcegroup"
-	diskIOPSReadWriteField  = "diskiopsreadwrite"
-	diskMBPSReadWriteField  = "diskmbpsreadwrite"
-	diskNameField           = "diskname"
-	desIDField              = "diskencryptionsetid"
-	tagsField               = "tags"
-	maxSharesField          = "maxshares"
-	incrementalField        = "incremental"
-	logicalSectorSizeField  = "logicalsectorsize"
-	fsTypeField             = "fstype"
-	kindField               = "kind"
-	perfProfileField        = "perfprofile"
+	cachingModeField         = "cachingmode"
+	storageAccountTypeField  = "storageaccounttype"
+	skuNameField             = "skuname"
+	locationField            = "location"
+	resourceGroupField       = "resourcegroup"
+	diskIOPSReadWriteField   = "diskiopsreadwrite"
+	diskMBPSReadWriteField   = "diskmbpsreadwrite"
+	diskNameField            = "diskname"
+	desIDField               = "diskencryptionsetid"
+	tagsField                = "tags"
+	maxSharesField           = "maxshares"
+	incrementalField         = "incremental"
+	logicalSectorSizeField   = "logicalsectorsize"
+	fsTypeField              = "fstype"
+	kindField                = "kind"
+	perfProfileField         = "perfprofile"
+	networkAccessPolicyField = "networkaccesspolicy"
+	diskAccessIDField        = "diskaccessid"
 
 	WellKnownTopologyKey = "topology.kubernetes.io/zone"
 	throttlingKey        = "throttlingKey"

diff --git a/pkg/azuredisk/controllerserver.go b/pkg/azuredisk/controllerserver.go
@@ -129,6 +129,8 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
 		diskEncryptionSetID     string
 		customTags              string
 		writeAcceleratorEnabled string
+		netAccessPolicy         string
+		diskAccessID            string
 		maxShares               int
 	)
 
@@ -189,6 +191,10 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
 			if !optimization.IsValidPerfProfile(v) {
 				return nil, status.Error(codes.InvalidArgument, fmt.Sprintf("Perf profile %s is not supported. Supported tuning modes are none and basic.", v))
 			}
+		case networkAccessPolicyField:
+			netAccessPolicy = v
+		case diskAccessIDField:
+			diskAccessID = v
 		default:
 			return nil, fmt.Errorf("invalid parameter %s in storage class", k)
 		}
@@ -229,6 +235,11 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
 		return nil, err
 	}
 
+	networkAccessPolicy, err := normalizeNetworkAccessPolicy(netAccessPolicy)
+	if err != nil {
+		return nil, err
+	}
+
 	requirement := req.GetAccessibilityRequirements()
 	diskZone := pickAvailabilityZone(requirement, d.cloud.Location)
 	accessibleTopology := []*csi.Topology{}
@@ -330,8 +341,12 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
 		DiskEncryptionSetID: diskEncryptionSetID,
 		MaxShares:           int32(maxShares),
 		LogicalSectorSize:   int32(logicalSectorSize),
+		NetworkAccessPolicy: networkAccessPolicy,
 	}
 	volumeOptions.SkipGetDiskOperation = d.isGetDiskThrottled()
+	if diskAccessID != "" {
+		volumeOptions.DiskAccessID = &diskAccessID
+	}
 	diskURI, err := d.cloud.CreateManagedDisk(volumeOptions)
 	if err != nil {
 		if strings.Contains(err.Error(), NotFound) {

diff --git a/pkg/azuredisk/controllerserver_v2.go b/pkg/azuredisk/controllerserver_v2.go
@@ -91,6 +91,8 @@ func (d *DriverV2) CreateVolume(ctx context.Context, req *csi.CreateVolumeReques
 		diskEncryptionSetID     string
 		customTags              string
 		writeAcceleratorEnabled string
+		netAccessPolicy         string
+		diskAccessID            string
 		maxShares               int
 	)
 
@@ -151,6 +153,10 @@ func (d *DriverV2) CreateVolume(ctx context.Context, req *csi.CreateVolumeReques
 			if !optimization.IsValidPerfProfile(v) {
 				return nil, status.Error(codes.InvalidArgument, fmt.Sprintf("Perf profile %s is not supported. Supported tuning modes are none and basic.", v))
 			}
+		case networkAccessPolicyField:
+			netAccessPolicy = v
+		case diskAccessIDField:
+			diskAccessID = v
 		default:
 			return nil, fmt.Errorf("invalid parameter %s in storage class", k)
 		}
@@ -191,6 +197,11 @@ func (d *DriverV2) CreateVolume(ctx context.Context, req *csi.CreateVolumeReques
 		return nil, err
 	}
 
+	networkAccessPolicy, err := normalizeNetworkAccessPolicy(netAccessPolicy)
+	if err != nil {
+		return nil, err
+	}
+
 	selectedAvailabilityZone := pickAvailabilityZone(req.GetAccessibilityRequirements(), d.cloud.Location)
 
 	if ok, err := d.checkDiskCapacity(ctx, resourceGroup, diskName, requestGiB); !ok {
@@ -268,6 +279,7 @@ func (d *DriverV2) CreateVolume(ctx context.Context, req *csi.CreateVolumeReques
 		DiskEncryptionSetID: diskEncryptionSetID,
 		MaxShares:           int32(maxShares),
 		LogicalSectorSize:   int32(logicalSectorSize),
+		NetworkAccessPolicy: networkAccessPolicy,
 	}
 	diskURI, err := d.cloud.CreateManagedDisk(volumeOptions)
 	if err != nil {

diff --git a/test/e2e/dynamic_provisioning_test.go b/test/e2e/dynamic_provisioning_test.go
@@ -123,9 +123,11 @@ func (t *dynamicProvisioningTestSuite) defineTests(isMultiZone bool) {
 			},
 		}
 		test := testsuites.DynamicallyProvisionedCmdVolumeTest{
-			CSIDriver:              testDriver,
-			Pods:                   pods,
-			StorageClassParameters: map[string]string{"skuName": "Standard_LRS"},
+			CSIDriver: testDriver,
+			Pods:      pods,
+			StorageClassParameters: map[string]string{
+				"skuName": "Standard_LRS",
+			},
 		}
 
 		if isMultiZone && !isUsingInTreeVolumePlugin {
@@ -138,7 +140,10 @@ func (t *dynamicProvisioningTestSuite) defineTests(isMultiZone bool) {
 			}
 		}
 		if !isUsingInTreeVolumePlugin && supportsZRS {
-			test.StorageClassParameters = map[string]string{"skuName": "StandardSSD_ZRS"}
+			test.StorageClassParameters = map[string]string{
+				"skuName":             "StandardSSD_ZRS",
+				"networkAccessPolicy": "AllowAll",
+			}
 		}
 		if isUsingInTreeVolumePlugin {
 			// cover case: https://github.com/kubernetes/kubernetes/issues/103433
@@ -167,7 +172,8 @@ func (t *dynamicProvisioningTestSuite) defineTests(isMultiZone bool) {
 		}
 
 		scParameters := map[string]string{
-			"skuName": "Standard_LRS",
+			"skuName":             "Standard_LRS",
+			"networkAccessPolicy": "DenyAll",
 		}
 		test := testsuites.DynamicallyProvisionedVolumeSubpathTester{
 			CSIDriver:              testDriver,
@@ -466,7 +472,10 @@ func (t *dynamicProvisioningTestSuite) defineTests(isMultiZone bool) {
 			},
 		}
 		if !isUsingInTreeVolumePlugin && supportsZRS {
-			test.StorageClassParameters = map[string]string{"skuName": "StandardSSD_ZRS"}
+			test.StorageClassParameters = map[string]string{
+				"skuName":             "StandardSSD_ZRS",
+				"networkAccessPolicy": "DenyAll",
+			}
 		}
 		test.Run(cs, ns)
 	})
@@ -504,7 +513,11 @@ func (t *dynamicProvisioningTestSuite) defineTests(isMultiZone bool) {
 			},
 		}
 		if !isUsingInTreeVolumePlugin && supportsZRS {
-			test.StorageClassParameters = map[string]string{"skuName": "StandardSSD_ZRS", "fsType": "xfs"}
+			test.StorageClassParameters = map[string]string{
+				"skuName":             "StandardSSD_ZRS",
+				"fsType":              "xfs",
+				"networkAccessPolicy": "DenyAll",
+			}
 		}
 		test.Run(cs, ns)
 	})