moving BlobStorageSecretsRepository to use UploadAsync instead of OpenWriteAsync

Author: brettsam

src/WebJobs.Script.WebHost/Security/KeyManagement/BlobStorageSecretsRepository.cs

@@ -5,9 +5,9 @@
 using System.Collections.Generic;
 using System.IO;
 using System.Threading.Tasks;
+using Azure;
 using Azure.Storage.Blobs;
 using Azure.Storage.Blobs.Models;
-using Azure.Storage.Blobs.Specialized;
 using Microsoft.Azure.WebJobs.Script.WebHost.Diagnostics.Extensions;
 using Microsoft.Extensions.Logging;
 
@@ -187,11 +187,22 @@ private string GetSecretsBlobPath(ScriptSecretsType secretsType, string function
 
         private async Task WriteToBlobAsync(string blobPath, string secretsContent)
         {
-            BlockBlobClient secretBlobClient = Container.GetBlockBlobClient(blobPath);
-            using (StreamWriter writer = new StreamWriter(await secretBlobClient.OpenWriteAsync(true)))
+            BlobClient secretBlobClient = Container.GetBlobClient(blobPath);
+            BlobUploadOptions uploadOptions = new BlobUploadOptions();
+
+            if (await secretBlobClient.ExistsAsync())
+            {
+                // Return a 412 if another write beats us to updating the file.
+                BlobProperties properties = await secretBlobClient.GetPropertiesAsync();
+                uploadOptions.Conditions = new BlobRequestConditions { IfMatch = properties.ETag };
+            }
+            else
             {
-                await writer.WriteAsync(secretsContent);
+                // Return a 409 if another write beats us to creating the file.
+                uploadOptions.Conditions = new BlobRequestConditions { IfNoneMatch = ETag.All };
             }
+
+            await secretBlobClient.UploadAsync(BinaryData.FromString(secretsContent), uploadOptions);
         }
 
         protected virtual void LogErrorMessage(string operation)

test/WebJobs.Script.Tests.Integration/Host/SecretsRepositoryTests.cs

@@ -7,6 +7,7 @@
 using System.IO;
 using System.Linq;
 using System.Threading.Tasks;
+using Azure;
 using Microsoft.Azure.KeyVault;
 using Microsoft.Azure.KeyVault.Models;
 using Microsoft.Azure.Services.AppAuthentication;
@@ -369,6 +370,133 @@ public async Task GetSecretSnapshots_ReturnsExpected(SecretsRepositoryType repos
             }
         }
 
+        [Theory]
+        [InlineData(SecretsRepositoryType.BlobStorage)]
+        [InlineData(SecretsRepositoryType.BlobStorageSas)]
+        public async Task BlobRepository_WriteAsync_DoesNot_ClearBlobContents(SecretsRepositoryType repositoryType)
+        {
+            using (var directory = new TempDirectory())
+            {
+                await _fixture.TestInitialize(repositoryType, directory.Path);
+
+                ScriptSecrets testSecrets = new HostSecrets()
+                {
+                    MasterKey = new Key("master", "test"),
+                    FunctionKeys = new List<Key>() { new Key(KeyName, "test") },
+                    SystemKeys = new List<Key>() { new Key(KeyName, "test") }
+                };
+
+                string testFunctionName = "host";
+
+                var target = _fixture.GetNewSecretRepository();
+
+                // Set up initial secrets.
+                await _fixture.WriteSecret(testFunctionName, testSecrets);
+
+                // Perform a write and read similtaneously. Previously, our usage of OpenWriteAsync
+                // would erase the content of the blob while writing, resulting in null secrets from the
+                // read.
+                Task writeTask = target.WriteAsync(ScriptSecretsType.Host, testFunctionName, testSecrets);
+                HostSecrets secretsContent = await target.ReadAsync(ScriptSecretsType.Host, testFunctionName) as HostSecrets;
+
+                await writeTask;
+
+                Assert.Equal(secretsContent.MasterKey.Name, "master");
+                Assert.Equal(secretsContent.MasterKey.Value, "test");
+                Assert.Equal(secretsContent.FunctionKeys[0].Name, KeyName);
+                Assert.Equal(secretsContent.FunctionKeys[0].Value, "test");
+                Assert.Equal(secretsContent.SystemKeys[0].Name, KeyName);
+                Assert.Equal(secretsContent.SystemKeys[0].Value, "test");
+            }
+        }
+
+        [Theory]
+        [InlineData(SecretsRepositoryType.BlobStorage)]
+        [InlineData(SecretsRepositoryType.BlobStorageSas)]
+        public async Task BlobRepository_SimultaneousWrites_Throws_PreconditionFailed(SecretsRepositoryType repositoryType)
+        {
+            using (var directory = new TempDirectory())
+            {
+                await _fixture.TestInitialize(repositoryType, directory.Path);
+
+                HostSecrets testSecrets = new HostSecrets()
+                {
+                    MasterKey = new Key("master", "test"),
+                    FunctionKeys = new List<Key>() { new Key(KeyName, "test") },
+                    SystemKeys = new List<Key>() { new Key(KeyName, "test") }
+                };
+
+                string testFunctionName = "host";
+
+                var target = _fixture.GetNewSecretRepository();
+
+                // Set up initial secrets.
+                await _fixture.WriteSecret(testFunctionName, testSecrets);
+                HostSecrets secretsContent = await target.ReadAsync(ScriptSecretsType.Host, testFunctionName) as HostSecrets;
+                Assert.Equal("test", secretsContent.FunctionKeys.Single().Value);
+
+                testSecrets.FunctionKeys.Single().Value = "changed";
+
+                // Simultaneous writes will result in one of the writes being discarded due to
+                // non-matching ETag.
+                Task writeTask1 = target.WriteAsync(ScriptSecretsType.Host, testFunctionName, testSecrets);
+                Task writeTask2 = target.WriteAsync(ScriptSecretsType.Host, testFunctionName, testSecrets);
+
+                var ex = await Assert.ThrowsAsync<RequestFailedException>(() => Task.WhenAll(writeTask1, writeTask2));
+
+                // Ensure the write went through.
+                secretsContent = await target.ReadAsync(ScriptSecretsType.Host, testFunctionName) as HostSecrets;
+                Assert.Equal("changed", secretsContent.FunctionKeys.Single().Value);
+
+                Assert.Equal("ConditionNotMet", ex.ErrorCode);
+                Assert.Equal(412, ex.Status);
+                Assert.True(writeTask1.IsCompletedSuccessfully || writeTask2.IsCompletedSuccessfully,
+                    "One of the write operations should have completed successfully.");
+            }
+        }
+
+        [Theory]
+        [InlineData(SecretsRepositoryType.BlobStorage)]
+        [InlineData(SecretsRepositoryType.BlobStorageSas)]
+        public async Task BlobRepository_SimultaneousCreates_Throws_Conflict(SecretsRepositoryType repositoryType)
+        {
+            using (var directory = new TempDirectory())
+            {
+                await _fixture.TestInitialize(repositoryType, directory.Path);
+
+                HostSecrets testSecrets = new HostSecrets()
+                {
+                    MasterKey = new Key("master", "test"),
+                    FunctionKeys = new List<Key>() { new Key(KeyName, "test") },
+                    SystemKeys = new List<Key>() { new Key(KeyName, "test") }
+                };
+
+                string testFunctionName = "host";
+
+                var target = _fixture.GetNewSecretRepository();
+
+                // Ensure nothing is there.                
+                HostSecrets secretsContent = await target.ReadAsync(ScriptSecretsType.Host, testFunctionName) as HostSecrets;
+                Assert.Null(secretsContent);
+
+                // Simultaneous creates will result in one of the writes being discarded due to
+                // non-matching ETag.
+                Task writeTask1 = target.WriteAsync(ScriptSecretsType.Host, testFunctionName, testSecrets);
+                Task writeTask2 = target.WriteAsync(ScriptSecretsType.Host, testFunctionName, testSecrets);
+
+                var ex = await Assert.ThrowsAsync<RequestFailedException>(() => Task.WhenAll(writeTask1, writeTask2));
+
+                // Ensure the write went through.
+                secretsContent = await target.ReadAsync(ScriptSecretsType.Host, testFunctionName) as HostSecrets;
+                Assert.Equal("test", secretsContent.FunctionKeys.Single().Value);
+
+                Assert.Equal("BlobAlreadyExists", ex.ErrorCode);
+                Assert.Equal(409, ex.Status);
+                Assert.True(writeTask1.IsCompletedSuccessfully || writeTask2.IsCompletedSuccessfully,
+                    "One of the write operations should have completed successfully.");
+            }
+        }
+
         public class Fixture : IDisposable
         {
             public Fixture()