Skip to content

Commit c6d806a

Browse files
andyzhangxandyzhangx
authored
Merge pull request #668 from andyzhangx/volumeid-ns
chore: append secretNamespace to volumeid
2 parents 38de342 + 427f266 commit c6d806a

File tree

4 files changed

+86
-54
lines changed

4 files changed

+86
-54
lines changed

‎pkg/blob/blob.go

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ const (
4747
DefaultDriverName = "blob.csi.azure.com"
4848
blobCSIDriverName = "blob_csi_driver"
4949
separator = "#"
50-
volumeIDTemplate = "%s#%s#%s"
50+
volumeIDTemplate = "%s#%s#%s#%s#%s"
5151
secretNameTemplate = "azure-storage-account-%s-secret"
5252
serverNameField = "server"
5353
storageEndpointSuffixField = "storageendpointsuffix"
@@ -256,14 +256,20 @@ func (d *Driver) Run(endpoint, kubeconfig string, testBool bool) {
256256
}
257257

258258
// GetContainerInfo get container info according to volume id, e.g.
259-
// input: "rg#f5713de20cde511e8ba4900#pvc-fuse-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41#uuid"
260-
// output: rg, f5713de20cde511e8ba4900, pvc-fuse-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41
261-
func GetContainerInfo(id string) (string, string, string, error) {
259+
// input: "rg#f5713de20cde511e8ba4900#containerName#uuid"
260+
// output: rg, f5713de20cde511e8ba4900, containerName
261+
// input: "rg#f5713de20cde511e8ba4900#containerName#uuid#namespace"
262+
// output: rg, f5713de20cde511e8ba4900, containerName, namespace
263+
func GetContainerInfo(id string) (string, string, string, string, error) {
262264
segments := strings.Split(id, separator)
263265
if len(segments) < 3 {
264-
return "", "", "", fmt.Errorf("error parsing volume id: %q, should at least contain two #", id)
266+
return "", "", "", "", fmt.Errorf("error parsing volume id: %q, should at least contain two #", id)
265267
}
266-
return segments[0], segments[1], segments[2], nil
268+
var secretNamespace string
269+
if len(segments) > 4 {
270+
secretNamespace = segments[4]
271+
}
272+
return segments[0], segments[1], segments[2], secretNamespace, nil
267273
}
268274

269275
// A container name must be a valid DNS name, conforming to the following naming rules:
@@ -306,7 +312,7 @@ func isSASToken(key string) bool {
306312

307313
// GetAuthEnv return <accountName, containerName, authEnv, error>
308314
func (d *Driver) GetAuthEnv(ctx context.Context, volumeID, protocol string, attrib, secrets map[string]string) (string, string, string, string, []string, error) {
309-
rgName, accountName, containerName, err := GetContainerInfo(volumeID)
315+
rgName, accountName, containerName, secretNamespace, err := GetContainerInfo(volumeID)
310316
if err != nil {
311317
// ignore volumeID parsing error
312318
klog.V(2).Info("parsing volumeID(%s) return with error: %v", volumeID, err)
@@ -318,7 +324,6 @@ func (d *Driver) GetAuthEnv(ctx context.Context, volumeID, protocol string, attr
318324
accountKey string
319325
accountSasToken string
320326
secretName string
321-
secretNamespace string
322327
pvcNamespace string
323328
keyVaultURL string
324329
keyVaultSecretName string
@@ -514,7 +519,7 @@ func (d *Driver) GetStorageAccountAndContainer(ctx context.Context, volumeID str
514519
} else {
515520
if len(secrets) == 0 {
516521
var rgName string
517-
rgName, accountName, containerName, err = GetContainerInfo(volumeID)
522+
rgName, accountName, containerName, _, err = GetContainerInfo(volumeID)
518523
if err != nil {
519524
return "", "", "", "", err
520525
}

‎pkg/blob/blob_test.go

Lines changed: 66 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -168,62 +168,88 @@ func TestRun(t *testing.T) {
168168

169169
func TestGetContainerInfo(t *testing.T) {
170170
tests := []struct {
171-
options string
172-
expected1 string
173-
expected2 string
174-
expected3 string
175-
expected4 error
171+
volumeID string
172+
rg string
173+
account string
174+
container string
175+
namespace string
176+
expectedError error
176177
}{
177178
{
178-
options: "rg#f5713de20cde511e8ba4900#pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41#uuid",
179-
expected1: "rg",
180-
expected2: "f5713de20cde511e8ba4900",
181-
expected3: "pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41",
182-
expected4: nil,
179+
volumeID: "rg#f5713de20cde511e8ba4900#container#uuid#namespace",
180+
rg: "rg",
181+
account: "f5713de20cde511e8ba4900",
182+
container: "container",
183+
namespace: "namespace",
184+
expectedError: nil,
183185
},
184186
{
185-
options: "rg#f5713de20cde511e8ba4900#pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41#",
186-
expected1: "rg",
187-
expected2: "f5713de20cde511e8ba4900",
188-
expected3: "pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41",
189-
expected4: nil,
187+
volumeID: "rg#f5713de20cde511e8ba4900#container##namespace",
188+
rg: "rg",
189+
account: "f5713de20cde511e8ba4900",
190+
container: "container",
191+
namespace: "namespace",
192+
expectedError: nil,
190193
},
191194
{
192-
options: "rg#f5713de20cde511e8ba4900#pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41",
193-
expected1: "rg",
194-
expected2: "f5713de20cde511e8ba4900",
195-
expected3: "pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41",
196-
expected4: nil,
195+
volumeID: "rg#f5713de20cde511e8ba4900#container##",
196+
rg: "rg",
197+
account: "f5713de20cde511e8ba4900",
198+
container: "container",
199+
namespace: "",
200+
expectedError: nil,
197201
},
198202
{
199-
options: "rg#f5713de20cde511e8ba4900",
200-
expected1: "",
201-
expected2: "",
202-
expected3: "",
203-
expected4: fmt.Errorf("error parsing volume id: \"rg#f5713de20cde511e8ba4900\", should at least contain two #"),
203+
volumeID: "rg#f5713de20cde511e8ba4900#pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41#uuid",
204+
rg: "rg",
205+
account: "f5713de20cde511e8ba4900",
206+
container: "pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41",
207+
expectedError: nil,
204208
},
205209
{
206-
options: "rg",
207-
expected1: "",
208-
expected2: "",
209-
expected3: "",
210-
expected4: fmt.Errorf("error parsing volume id: \"rg\", should at least contain two #"),
210+
volumeID: "rg#f5713de20cde511e8ba4900#pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41#",
211+
rg: "rg",
212+
account: "f5713de20cde511e8ba4900",
213+
container: "pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41",
214+
expectedError: nil,
211215
},
212216
{
213-
options: "",
214-
expected1: "",
215-
expected2: "",
216-
expected3: "",
217-
expected4: fmt.Errorf("error parsing volume id: \"\", should at least contain two #"),
217+
volumeID: "rg#f5713de20cde511e8ba4900#pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41",
218+
rg: "rg",
219+
account: "f5713de20cde511e8ba4900",
220+
container: "pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41",
221+
expectedError: nil,
222+
},
223+
{
224+
volumeID: "rg#f5713de20cde511e8ba4900",
225+
rg: "",
226+
account: "",
227+
container: "",
228+
expectedError: fmt.Errorf("error parsing volume id: \"rg#f5713de20cde511e8ba4900\", should at least contain two #"),
229+
},
230+
{
231+
volumeID: "rg",
232+
rg: "",
233+
account: "",
234+
container: "",
235+
expectedError: fmt.Errorf("error parsing volume id: \"rg\", should at least contain two #"),
236+
},
237+
{
238+
volumeID: "",
239+
rg: "",
240+
account: "",
241+
container: "",
242+
expectedError: fmt.Errorf("error parsing volume id: \"\", should at least contain two #"),
218243
},
219244
}
220245

221246
for _, test := range tests {
222-
result1, result2, result3, result4 := GetContainerInfo(test.options)
223-
if !reflect.DeepEqual(result1, test.expected1) || !reflect.DeepEqual(result2, test.expected2) ||
224-
!reflect.DeepEqual(result3, test.expected3) || !reflect.DeepEqual(result4, test.expected4) {
225-
t.Errorf("input: %q, GetContainerInfo result1: %q, expected1: %q, result2: %q, expected2: %q, result3: %q, expected3: %q, result4: %q, expected4: %q", test.options, result1, test.expected1, result2, test.expected2,
226-
result3, test.expected3, result4, test.expected4)
247+
rg, account, container, ns, err := GetContainerInfo(test.volumeID)
248+
if !reflect.DeepEqual(rg, test.rg) || !reflect.DeepEqual(account, test.account) ||
249+
!reflect.DeepEqual(container, test.container) || !reflect.DeepEqual(err, test.expectedError) ||
250+
!reflect.DeepEqual(ns, test.namespace) {
251+
t.Errorf("input: %q, GetContainerInfo rg: %q, rg: %q, account: %q, account: %q, container: %q, container: %q, namespace: %q, namespace: %q, err: %q, expectedError: %q", test.volumeID, rg, test.rg, account, test.account,
252+
container, test.container, ns, test.namespace, err, test.expectedError)
227253
}
228254
}
229255
}

‎pkg/blob/controllerserver.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -322,12 +322,13 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
322322
}
323323
}
324324

325-
volumeID = fmt.Sprintf(volumeIDTemplate, resourceGroup, accountName, validContainerName)
325+
var uuid string
326326
if containerName != "" {
327327
// add volume name as suffix to differentiate volumeID since "containerName" is specified
328328
// not necessary for dynamic container name creation since volumeID already contains volume name
329-
volumeID = volumeID + "#" + volName
329+
uuid = volName
330330
}
331+
volumeID = fmt.Sprintf(volumeIDTemplate, resourceGroup, accountName, validContainerName, uuid, secretNamespace)
331332
klog.V(2).Infof("create container %s on storage account %s successfully", validContainerName, accountName)
332333

333334
isOperationSucceeded = true
@@ -358,7 +359,7 @@ func (d *Driver) DeleteVolume(ctx context.Context, req *csi.DeleteVolumeRequest)
358359
}
359360
defer d.volumeLocks.Release(volumeID)
360361

361-
if _, _, _, err := GetContainerInfo(volumeID); err != nil {
362+
if _, _, _, _, err := GetContainerInfo(volumeID); err != nil {
362363
// According to CSI Driver Sanity Tester, should succeed when an invalid volume id is used
363364
klog.Errorf("GetContainerInfo(%s) in DeleteVolume failed with error: %v", volumeID, err)
364365
return &csi.DeleteVolumeResponse{}, nil
@@ -427,7 +428,7 @@ func (d *Driver) ValidateVolumeCapabilities(ctx context.Context, req *csi.Valida
427428
return nil, status.Error(codes.InvalidArgument, err.Error())
428429
}
429430

430-
resourceGroupName, accountName, containerName, err := GetContainerInfo(volumeID)
431+
resourceGroupName, accountName, containerName, _, err := GetContainerInfo(volumeID)
431432
if err != nil {
432433
klog.Errorf("GetContainerInfo(%s) in ValidateVolumeCapabilities failed with error: %v", volumeID, err)
433434
return nil, status.Error(codes.NotFound, err.Error())

‎test/e2e/testsuites/pre_provisioned_existing_credentials_tester.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ type PreProvisionedExistingCredentialsTest struct {
3939
func (t *PreProvisionedExistingCredentialsTest) Run(client clientset.Interface, namespace *v1.Namespace) {
4040
for _, pod := range t.Pods {
4141
for n, volume := range pod.Volumes {
42-
resourceGroupName, accountName, containerName, err := blob.GetContainerInfo(volume.VolumeID)
42+
resourceGroupName, accountName, containerName, _, err := blob.GetContainerInfo(volume.VolumeID)
4343
if err != nil {
4444
framework.ExpectNoError(err, fmt.Sprintf("Error GetContainerInfo from volumeID(%s): %v", volume.VolumeID, err))
4545
return

0 commit comments

Comments
 (0)