don't deprecate connectionString so hard

emgarten · Apr 23, 2024 · 70651ac · 70651ac
1 parent f8516bf
commit 70651ac
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 32 deletions.
diff --git a/doc/client-settings.md b/doc/client-settings.md
@@ -32,12 +32,12 @@ If you need to use a service principal credential type, set the `AZURE_TENANT_ID
 More options can be found in the [Azure.Identity README](https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/identity/Azure.Identity/README.md#environment-variables).
 
 
-| Property | Description |
-| --- | ------ |
-| container | Name of an existing container in the storage account. *[Required]*                                                                           |
-| connectionString | Azure storage connection string. *[Deprecated]*                                                                                              |
-| path | Full URI of the azure storage container. If specified this value will be verified against the container's URI *[Required]*.                  |
-| feedSubPath | Provides a sub directory path within the container where the feed should be added. This allows for multiple feeds within a single container. |
+| Property | Description                                                                                                                                                                               |
+| --- |-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
+| container | Name of an existing container in the storage account. *[Required]*                                                                                                                        |
+| connectionString | Azure storage connection string. Note, either connectionString or path *must* be specified. *[Discouraged]*                                                                               |
+| path | Full URI of the azure storage container. If specified this value will be verified against the container's URI. None, either connectionString or path *must* be specified. *[Encouraged]*. |
+| feedSubPath | Provides a sub directory path within the container where the feed should be added. This allows for multiple feeds within a single container.                                              |
 
 `sleet.json`:
 ```json

diff --git a/src/SleetLib/FileSystem/FileSystemFactory.cs b/src/SleetLib/FileSystem/FileSystemFactory.cs
@@ -78,29 +78,10 @@ public static async Task<ISleetFileSystem> CreateFileSystemAsync(LocalSettings s
                         var connectionString = JsonUtility.GetValueCaseInsensitive(sourceEntry, "connectionString");
                         var container = JsonUtility.GetValueCaseInsensitive(sourceEntry, "container");
 
-                        if (!string.IsNullOrEmpty(connectionString))
-                        {
-                            await log.LogAsync(LogLevel.Warning, "connectionString is deprecated for azure account. Use path instead.");
+                        var blobServiceClient = await GetBlobServiceClient(log, connectionString, pathUri);
 
-                            if (connectionString.Equals(AzureFileSystem.AzureEmptyConnectionString, StringComparison.OrdinalIgnoreCase))
-                            {
-                                throw new ArgumentException("Invalid connectionString for azure account.");
-                            }
-                        }
-
-                        if (pathUri == null)
-                        {
-                            throw new ArgumentException("Path is required for azure account.");
-                        }
-
-                        if (string.IsNullOrEmpty(container))
-                        {
-                            throw new ArgumentException("Missing container for azure account.");
-                        }
-
-                        var blobServiceClient = string.IsNullOrWhiteSpace(connectionString)
-                            ? new BlobServiceClient(new Uri(pathUri.GetLeftPart(UriPartial.Authority)), new DefaultAzureCredential())
-                            : new BlobServiceClient(connectionString);
+                        pathUri ??= blobServiceClient.Uri;
+                        baseUri ??= pathUri;
 
                         result = new AzureFileSystem(cache, pathUri, baseUri, blobServiceClient, container, feedSubPath);
                     }
@@ -248,13 +229,37 @@ public static async Task<ISleetFileSystem> CreateFileSystemAsync(LocalSettings s
             return result;
         }
 
-        private static BlobServiceClient GetBlobServiceClient(
+        private static async Task<BlobServiceClient> GetBlobServiceClient(
+            ILogger log,
             string connectionString,
             Uri pathUri)
         {
-            return !string.IsNullOrEmpty(connectionString) ?
-                new BlobServiceClient(connectionString) :
-                new BlobServiceClient(new Uri(pathUri.GetLeftPart(UriPartial.Authority)), new DefaultAzureCredential());
+            if (pathUri is not null && connectionString is not null)
+            {
+                throw new ArgumentException("path (recommended) and connectionString (discouraged) are mutually exclusive for azure account. Chose one or the other.");
+            }
+
+            if (pathUri is not null)
+            {
+                return new BlobServiceClient(new Uri(pathUri.GetLeftPart(UriPartial.Authority)), new DefaultAzureCredential());
+            }
+
+            if (connectionString is null)
+            {
+                throw new ArgumentException("Missing path (recommended) or connectionString (discouraged) for azure account.");
+            }
+
+            await log.LogAsync(LogLevel.Warning,
+                "connectionString (with access key) is not recommended for azure account. More information here: https://learn.microsoft.com/en-us/azure/storage/common/storage-account-keys-manage?tabs=azure-portal#protect-your-access-keys" + Environment.NewLine +
+                "Use path instead.");
+
+
+            if (connectionString.Equals(AzureFileSystem.AzureEmptyConnectionString, StringComparison.OrdinalIgnoreCase))
+            {
+                throw new ArgumentException("Invalid connectionString for azure account.");
+            }
+
+            return new BlobServiceClient(connectionString);
         }
     }
 }