Merge pull request #2515 from RomanBednar/OCPBUGS-54897

fix: allow deleting cross-subscription snapshots

Author: andyzhangx

pkg/azurefile/controllerserver.go

@@ -748,7 +748,7 @@ func (d *Driver) DeleteVolume(ctx context.Context, req *csi.DeleteVolumeRequest)
 	if resourceGroupName == "" {
 		resourceGroupName = d.cloud.ResourceGroup
 	}
-	if subsID == "" {
+	if !isValidSubscriptionID(subsID) {
 		subsID = d.cloud.SubscriptionID
 	}
 
@@ -895,7 +895,7 @@ func (d *Driver) CreateSnapshot(ctx context.Context, req *csi.CreateSnapshotRequ
 	if rgName == "" {
 		rgName = d.cloud.ResourceGroup
 	}
-	if subsID == "" {
+	if !isValidSubscriptionID(subsID) {
 		subsID = d.cloud.SubscriptionID
 	}
 
@@ -1033,7 +1033,7 @@ func (d *Driver) DeleteSnapshot(ctx context.Context, req *csi.DeleteSnapshotRequ
 	if len(req.SnapshotId) == 0 {
 		return nil, status.Error(codes.InvalidArgument, "Snapshot ID must be provided")
 	}
-	rgName, accountName, fileShareName, _, _, _, err := GetFileShareInfo(req.SnapshotId) //nolint:dogsled
+	rgName, accountName, fileShareName, _, _, subsID, err := GetFileShareInfo(req.SnapshotId) //nolint:dogsled
 	if fileShareName == "" || err != nil {
 		// According to CSI Driver Sanity Tester, should succeed when an invalid snapshot id is used
 		klog.V(4).Infof("failed to get share url with (%s): %v, returning with success", req.SnapshotId, err)
@@ -1047,7 +1047,10 @@ func (d *Driver) DeleteSnapshot(ctx context.Context, req *csi.DeleteSnapshotRequ
 	if rgName == "" {
 		rgName = d.cloud.ResourceGroup
 	}
-	subsID := d.cloud.SubscriptionID
+	if !isValidSubscriptionID(subsID) {
+		subsID = d.cloud.SubscriptionID
+	}
+
 	mc := metrics.NewMetricContext(azureFileCSIDriverName, "controller_delete_snapshot", rgName, subsID, d.Name)
 	isOperationSucceeded := false
 	defer func() {
@@ -1118,6 +1121,10 @@ func (d *Driver) restoreSnapshot(ctx context.Context, req *csi.CreateVolumeReque
 	if srcAccountName == "" || srcFileShareName == "" || dstFileShareName == "" {
 		return fmt.Errorf("one or more of srcAccountName(%s), srcFileShareName(%s), dstFileShareName(%s) are empty", srcAccountName, srcFileShareName, dstFileShareName)
 	}
+
+	if !isValidSubscriptionID(srcSubscriptionID) {
+		srcSubscriptionID = d.cloud.SubscriptionID
+	}
 	srcAccountSasToken := dstAccountSasToken
 	if srcAccountName != dstAccountName && dstAccountSasToken != "" {
 		srcAccountOptions := &storage.AccountOptions{
@@ -1230,7 +1237,7 @@ func (d *Driver) ControllerExpandVolume(ctx context.Context, req *csi.Controller
 	if resourceGroupName == "" {
 		resourceGroupName = d.cloud.ResourceGroup
 	}
-	if subsID == "" {
+	if !isValidSubscriptionID(subsID) {
 		subsID = d.cloud.SubscriptionID
 	}
 

pkg/azurefile/utils.go

@@ -359,3 +359,7 @@ func getFileServiceURL(accountName, storageEndpointSuffix string) string {
 	}
 	return fmt.Sprintf(serviceURLTemplate, accountName, storageEndpointSuffix)
 }
+
+func isValidSubscriptionID(subsID string) bool {
+	return regexp.MustCompile(`^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`).MatchString(subsID)
+}

pkg/azurefile/utils_test.go

@@ -976,3 +976,55 @@ func TestGetFileServiceURL(t *testing.T) {
 		}
 	}
 }
+
+func TestIsValidSubscriptionID(t *testing.T) {
+	tests := []struct {
+		desc     string
+		subsID   string
+		expected bool
+	}{
+		{
+			desc:     "valid subscription ID",
+			subsID:   "c9d2281e-dcd5-4dfd-9a97-0d50377cdf76",
+			expected: true,
+		},
+		{
+			desc:     "invalid subscription ID - missing sections",
+			subsID:   "123e4567-e89b-12d3-a456",
+			expected: false,
+		},
+		{
+			desc:     "invalid subscription ID - invalid characters",
+			subsID:   "123e4567-e89b-12d3-a456-42661417400z",
+			expected: false,
+		},
+		{
+			desc:     "invalid subscription ID - empty string",
+			subsID:   "",
+			expected: false,
+		},
+		{
+			desc:     "invalid subscription ID - too short",
+			subsID:   "123e4567",
+			expected: false,
+		},
+		{
+			desc:     "invalid subscription ID - too long",
+			subsID:   "123e4567-e89b-12d3-a456-426614174000-extra",
+			expected: false,
+		},
+		{
+			desc:     "invalid subscription ID - timestamp",
+			subsID:   "2025-04-15T11:06:21.0000000Z",
+			expected: false,
+		},
+	}
+
+	for _, test := range tests {
+		result := isValidSubscriptionID(test.subsID)
+		if result != test.expected {
+			t.Errorf("desc: (%s), input: subsID(%s), isValidSubscriptionID returned with bool(%v), not equal to expected(%v)",
+				test.desc, test.subsID, result, test.expected)
+		}
+	}
+}