Skip to content

Commit

Permalink
feat: add containerNamePrefix in storage class
Browse files Browse the repository at this point in the history
  • Loading branch information
andyzhangx committed Apr 10, 2022
1 parent cc21fb1 commit c69edcd
Show file tree
Hide file tree
Showing 6 changed files with 135 additions and 9 deletions.
3 changes: 2 additions & 1 deletion docs/driver-parameters.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ location | Azure location | `eastus`, `westus`, etc. | No | if empty, driver wil
resourceGroup | Azure resource group name | existing resource group name | No | if empty, driver will use the same resource group name as current k8s cluster
storageAccount | specify Azure storage account name| STORAGE_ACCOUNT_NAME | - No for blobfuse mount </br> - Yes for NFSv3 mount | - For blobfuse mount: if empty, driver will find a suitable storage account that matches `skuName` in the same resource group; if a storage account name is provided, storage account must exist. </br> - For NFSv3 mount, storage account name must be provided
protocol | specify blobfuse mount or NFSv3 mount | `fuse`, `nfs` | No | `fuse`
containerName | specify the existing container name | existing container name | No | if empty, driver will create a new container name, starting with `pvc-fuse` for blobfuse or `pvc-nfs` for NFSv3
containerName | specify the existing container(directory) name | existing container name | No | if empty, driver will create a new container name, starting with `pvc-fuse` for blobfuse or `pvc-nfs` for NFSv3
containerNamePrefix | specify Azure storage directory prefix created by driver | can only contain lowercase letters, numbers, hyphens, and length should be less than 21 | No |
server | specify Azure storage account server address | existing server address, e.g. `accountname.privatelink.blob.core.windows.net` | No | if empty, driver will use default `accountname.blob.core.windows.net` or other sovereign cloud account address
allowBlobPublicAccess | Allow or disallow public access to all blobs or containers for storage account created by driver | `true`,`false` | No | `false`
storageEndpointSuffix | specify Azure storage endpoint suffix | `core.windows.net` | No | if empty, driver will use default storage endpoint suffix according to cloud environment, e.g. `core.windows.net`
Expand Down
27 changes: 23 additions & 4 deletions pkg/blob/blob.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ const (
secretNameField = "secretname"
secretNamespaceField = "secretnamespace"
containerNameField = "containername"
containerNamePrefixField = "containernameprefix"
storeAccountKeyField = "storeaccountkey"
isHnsEnabledField = "ishnsenabled"
getAccountKeyFromSecretField = "getaccountkeyfromsecret"
Expand Down Expand Up @@ -278,11 +279,9 @@ func getValidContainerName(volumeName, protocol string) string {
// now we set as 63 for maximum container name length
// todo: get cluster name
containerName = k8sutil.GenerateVolumeName(fmt.Sprintf("pvc-%s", protocol), uuid.NewUUID().String(), 63)
klog.Warningf("the requested volume name (%q) is invalid, so it is regenerated as (%q)", volumeName, containerName)
klog.Warningf("requested volume name (%s) is invalid, regenerated as (%q)", volumeName, containerName)
}
containerName = strings.Replace(containerName, "--", "-", -1)

return containerName
return strings.Replace(containerName, "--", "-", -1)
}

func checkContainerNameBeginAndEnd(containerName string) bool {
Expand Down Expand Up @@ -563,6 +562,26 @@ func isSupportedProtocol(protocol string) bool {
return false
}

// container names can contain only lowercase letters, numbers, and hyphens,
// and must begin and end with a letter or a number
func isSupportedContainerNamePrefix(prefix string) bool {
if prefix == "" {
return true
}
if len(prefix) > 20 {
return false
}
if prefix[0] == '-' {
return false
}
for _, v := range prefix {
if v != '-' && (v < '0' || v > '9') && (v < 'a' || v > 'z') {
return false
}
}
return true
}

// get storage account from secrets map
func getStorageAccount(secrets map[string]string) (string, string, error) {
if secrets == nil {
Expand Down
47 changes: 47 additions & 0 deletions pkg/blob/blob_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -816,3 +816,50 @@ func TestAppendDefaultMountOptions(t *testing.T) {
}
}
}

func TestIsSupportedContainerNamePrefix(t *testing.T) {
tests := []struct {
prefix string
expectedResult bool
}{
{
prefix: "",
expectedResult: true,
},
{
prefix: "ext3",
expectedResult: true,
},
{
prefix: "ext-2",
expectedResult: true,
},
{
prefix: "-xfs",
expectedResult: false,
},
{
prefix: "Absdf",
expectedResult: false,
},
{
prefix: "tooooooooooooooooooooooooolong",
expectedResult: false,
},
{
prefix: "+invalid",
expectedResult: false,
},
{
prefix: " invalidspace",
expectedResult: false,
},
}

for _, test := range tests {
result := isSupportedContainerNamePrefix(test.prefix)
if result != test.expectedResult {
t.Errorf("isSupportedContainerNamePrefix(%s) returned with %v, not equal to %v", test.prefix, result, test.expectedResult)
}
}
}
17 changes: 15 additions & 2 deletions pkg/blob/controllerserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
if parameters == nil {
parameters = make(map[string]string)
}
var storageAccountType, subsID, resourceGroup, location, account, containerName, protocol, customTags, secretName, secretNamespace, pvcNamespace string
var storageAccountType, subsID, resourceGroup, location, account, containerName, containerNamePrefix, protocol, customTags, secretName, secretNamespace, pvcNamespace string
var isHnsEnabled *bool
var vnetResourceGroup, vnetName, subnetName string
// set allowBlobPublicAccess as false by default
Expand All @@ -94,6 +94,8 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
resourceGroup = v
case containerNameField:
containerName = v
case containerNamePrefixField:
containerNamePrefix = v
case protocolField:
protocol = v
case tagsField:
Expand Down Expand Up @@ -170,6 +172,13 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
return nil, status.Errorf(codes.InvalidArgument, "protocol(%s) is not supported, supported protocol list: %v", protocol, supportedProtocolList)
}

if containerName != "" && containerNamePrefix != "" {
return nil, status.Errorf(codes.InvalidArgument, "containerName(%s) and containerNamePrefix(%s) could not be specified together", containerName, containerNamePrefix)
}
if !isSupportedContainerNamePrefix(containerNamePrefix) {
return nil, status.Errorf(codes.InvalidArgument, "containerNamePrefix(%s) can only contain lowercase letters, numbers, hyphens, and length should be less than 21", containerNamePrefix)
}

enableHTTPSTrafficOnly := true
accountKind := string(storage.KindStorageV2)
var (
Expand Down Expand Up @@ -268,7 +277,11 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)

validContainerName := containerName
if validContainerName == "" {
validContainerName = getValidContainerName(volName, protocol)
validContainerName = volName
if containerNamePrefix != "" {
validContainerName = containerNamePrefix + "-" + volName
}
validContainerName = getValidContainerName(validContainerName, protocol)
parameters[containerNameField] = validContainerName
}

Expand Down
45 changes: 45 additions & 0 deletions pkg/blob/controllerserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,51 @@ func TestCreateVolume(t *testing.T) {
}
},
},
{
name: "containerName and containerNamePrefix could not be specified together",
testFunc: func(t *testing.T) {
d := NewFakeDriver()
d.cloud = &azure.Cloud{}
mp := make(map[string]string)
mp[containerNameField] = "containerName"
mp[containerNamePrefixField] = "containerNamePrefix"
req := &csi.CreateVolumeRequest{
Name: "unit-test",
VolumeCapabilities: stdVolumeCapabilities,
Parameters: mp,
}
d.Cap = []*csi.ControllerServiceCapability{
controllerServiceCapability,
}
_, err := d.CreateVolume(context.Background(), req)
expectedErr := status.Errorf(codes.InvalidArgument, "containerName(containerName) and containerNamePrefix(containerNamePrefix) could not be specified together")
if !reflect.DeepEqual(err, expectedErr) {
t.Errorf("actualErr: (%v), expectedErr: (%v)", err, expectedErr)
}
},
},
{
name: "invalid containerNamePrefix",
testFunc: func(t *testing.T) {
d := NewFakeDriver()
d.cloud = &azure.Cloud{}
mp := make(map[string]string)
mp[containerNamePrefixField] = "UpperCase"
req := &csi.CreateVolumeRequest{
Name: "unit-test",
VolumeCapabilities: stdVolumeCapabilities,
Parameters: mp,
}
d.Cap = []*csi.ControllerServiceCapability{
controllerServiceCapability,
}
_, err := d.CreateVolume(context.Background(), req)
expectedErr := status.Errorf(codes.InvalidArgument, "containerNamePrefix(UpperCase) can only contain lowercase letters, numbers, hyphens, and length should be less than 21")
if !reflect.DeepEqual(err, expectedErr) {
t.Errorf("actualErr: (%v), expectedErr: (%v)", err, expectedErr)
}
},
},
{
name: "tags error",
testFunc: func(t *testing.T) {
Expand Down
5 changes: 3 additions & 2 deletions test/e2e/dynamic_provisioning_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,9 @@ var _ = ginkgo.Describe("[blob-csi-e2e] Dynamic Provisioning", func() {
CSIDriver: testDriver,
Pods: pods,
StorageClassParameters: map[string]string{
"skuName": "Standard_LRS",
"secretNamespace": "default",
"skuName": "Standard_LRS",
"secretNamespace": "default",
"containerNamePrefix": "nameprefix",
},
}
test.Run(cs, ns)
Expand Down

0 comments on commit c69edcd

Please sign in to comment.