Make unnamed volume names more unique (dotnet#5779)

* Make unnamed volume names more unique - Follow a scheme similar to what we do for persistent containers. Use the first 10 characters of the SHA256 of the app host's physical path as the volume name prefix. - This is a breaking change and the only workaround is the manually specify the volume name. Fixes dotnet#5413 * Keep the application name in the volume name * Fixed test
radical · Sep 21, 2024 · af5fee3 · af5fee3
1 parent eec556f
commit af5fee3
Show file tree

Hide file tree

Showing 11 changed files with 57 additions and 58 deletions.
diff --git a/playground/Redis/Redis.AppHost/Program.cs b/playground/Redis/Redis.AppHost/Program.cs
@@ -1,12 +1,12 @@
 var builder = DistributedApplication.CreateBuilder(args);
 
 var redis = builder.AddRedis("redis")
-    .WithDataVolume("redis-data")
+    .WithDataVolume()
     .WithRedisCommander()
-    .WithRedisInsight(c => c.WithAcceptEula(true));
+    .WithRedisInsight(c => c.WithAcceptEula());
 
 var garnet = builder.AddGarnet("garnet")
-    .WithDataVolume("garnet-data");
+    .WithDataVolume();
 
 var valkey = builder.AddValkey("valkey")
     .WithDataVolume("valkey-data");

diff --git a/src/Aspire.Hosting/DistributedApplicationBuilder.cs b/src/Aspire.Hosting/DistributedApplicationBuilder.cs
@@ -151,12 +151,8 @@ public DistributedApplicationBuilder(DistributedApplicationOptions options)
         var appHostName = options.ProjectName ?? _innerBuilder.Environment.ApplicationName;
         AppHostPath = Path.Join(AppHostDirectory, appHostName);
 
-        var appHostSha = string.Empty;
-        using (var shaHash = SHA256.Create())
-        {
-            var appHostShaBytes = shaHash.ComputeHash(Encoding.UTF8.GetBytes(AppHostPath));
-            appHostSha = Convert.ToHexString(appHostShaBytes);
-        }
+        var appHostShaBytes = SHA256.HashData(Encoding.UTF8.GetBytes(AppHostPath));
+        var appHostSha = Convert.ToHexString(appHostShaBytes);
 
         // Set configuration
         ConfigurePublishingOptions(options);

diff --git a/src/Shared/VolumeNameGenerator.cs b/src/Shared/VolumeNameGenerator.cs
@@ -7,45 +7,63 @@ namespace Aspire.Hosting.Utils;
 
 internal static class VolumeNameGenerator
 {
-    /// <summary>
-    /// Creates a volume name with the form <c>$"{applicationName}-{resourceName}-{suffix}</c>, e.g. <c>"myapplication-postgres-data"</c>.
-    /// </summary>
-    /// <remarks>
-    /// If the application name contains chars that are invalid for a volume name, the prefix <c>"volume-"</c> will be used instead.
-    /// </remarks>
     public static string CreateVolumeName<T>(IResourceBuilder<T> builder, string suffix) where T : IResource
     {
         if (!HasOnlyValidChars(suffix))
         {
             throw new ArgumentException($"The suffix '{suffix}' contains invalid characters. Only [a-zA-Z0-9_.-] are allowed.", nameof(suffix));
         }
 
-        // Create volume name like "myapplication-postgres-data"
-        var applicationName = builder.ApplicationBuilder.Environment.ApplicationName;
+        // Creates a volume name with the form < c > $"{applicationName}-{sha256 of apphost path}-{resourceName}-{suffix}</c>, e.g. <c>"myapplication-a345f2451-postgres-data"</c>.
+        // Create volume name like "{Sanitize(appname).Lower()}-{sha256.Lower()}-postgres-data"
+
+        // Compute a short hash of the content root path to differentiate between multiple AppHost projects with similar volume names
+        var safeApplicationName = Sanitize(builder.ApplicationBuilder.Environment.ApplicationName).ToLowerInvariant();
+        var applicationHash = builder.ApplicationBuilder.Configuration["AppHost:Sha256"]![..10].ToLowerInvariant();
         var resourceName = builder.Resource.Name;
-        return $"{(HasOnlyValidChars(applicationName) ? applicationName : "volume")}-{resourceName}-{suffix}";
+        return $"{safeApplicationName}-{applicationHash}-{resourceName}-{suffix}";
     }
 
-    private static bool HasOnlyValidChars(string name)
+    public static string Sanitize(string name)
     {
-        // According to the error message from docker CLI, volume names must be of form "[a-zA-Z0-9][a-zA-Z0-9_.-]"
-        var nameSpan = name.AsSpan();
-
-        for (var i = 0; i < nameSpan.Length; i++)
+        return string.Create(name.Length, name, static (s, name) =>
         {
-            var c = nameSpan[i];
+            // According to the error message from docker CLI, volume names must be of form "[a-zA-Z0-9][a-zA-Z0-9_.-]"
+            var nameSpan = name.AsSpan();
 
-            if (i == 0 && !(Char.IsAsciiLetter(c) || Char.IsNumber(c)))
+            for (var i = 0; i < nameSpan.Length; i++)
             {
-                // First char must be a letter or number
-                return false;
+                var c = nameSpan[i];
+
+                s[i] = IsValidChar(i, c) ? c : '_';
             }
-            else if (!(Char.IsAsciiLetter(c) || Char.IsNumber(c) || c == '_' || c == '.' || c == '-'))
+        });
+    }
+
+    private static bool HasOnlyValidChars(string value)
+    {
+        for (var i = 0; i < value.Length; i++)
+        {
+            if (!IsValidChar(i, value[i]))
             {
-                // Subsequent chars must be a letter, number, underscore, period, or hyphen
                 return false;
             }
         }
+        return true;
+    }
+
+    private static bool IsValidChar(int i, char c)
+    {
+        if (i == 0 && !(char.IsAsciiLetter(c) || char.IsNumber(c)))
+        {
+            // First char must be a letter or number
+            return false;
+        }
+        else if (!(char.IsAsciiLetter(c) || char.IsNumber(c) || c == '_' || c == '.' || c == '-'))
+        {
+            // Subsequent chars must be a letter, number, underscore, period, or hyphen
+            return false;
+        }
 
         return true;
     }

diff --git a/tests/Aspire.Hosting.Azure.Tests/AzureEventHubsExtensionsTests.cs b/tests/Aspire.Hosting.Azure.Tests/AzureEventHubsExtensionsTests.cs
@@ -55,7 +55,7 @@ public void AzureEventHubsUseEmulatorCallbackWithWithDataVolumeResultsInVolumeAn
 
         // Ignoring the annotation created for the custom Config.json file
         var volumeAnnotation = eventHubs.Resource.Annotations.OfType<ContainerMountAnnotation>().Single(a => !a.Target.Contains("Config.json"));
-        Assert.Equal("Aspire.Hosting.Tests-eh-data", volumeAnnotation.Source);
+        Assert.Equal($"{builder.GetVolumePrefix()}-eh-data", volumeAnnotation.Source);
         Assert.Equal("/data", volumeAnnotation.Target);
         Assert.Equal(ContainerMountType.Volume, volumeAnnotation.Type);
         Assert.False(volumeAnnotation.IsReadOnly);

diff --git a/tests/Aspire.Hosting.Azure.Tests/AzureResourceExtensionsTests.cs b/tests/Aspire.Hosting.Azure.Tests/AzureResourceExtensionsTests.cs
@@ -81,7 +81,7 @@ public void AzureStorageUseEmulatorCallbackWithWithDataVolumeResultsInVolumeAnno
         });
 
         var volumeAnnotation = storage.Resource.Annotations.OfType<ContainerMountAnnotation>().Single();
-        Assert.Equal("Aspire.Hosting.Tests-storage-data", volumeAnnotation.Source);
+        Assert.Equal($"{builder.GetVolumePrefix()}-storage-data", volumeAnnotation.Source);
         Assert.Equal("/data", volumeAnnotation.Target);
         Assert.Equal(ContainerMountType.Volume, volumeAnnotation.Type);
         Assert.Equal(isReadOnly ?? false, volumeAnnotation.IsReadOnly);

diff --git a/tests/Aspire.Hosting.Garnet.Tests/AddGarnetTests.cs b/tests/Aspire.Hosting.Garnet.Tests/AddGarnetTests.cs
@@ -129,7 +129,7 @@ public void WithDataVolumeAddsVolumeAnnotation(bool? isReadOnly)
 
         var volumeAnnotation = garnet.Resource.Annotations.OfType<ContainerMountAnnotation>().Single();
 
-        Assert.Equal("Aspire.Hosting.Tests-myGarnet-data", volumeAnnotation.Source);
+        Assert.Equal($"{builder.GetVolumePrefix()}-myGarnet-data", volumeAnnotation.Source);
         Assert.Equal("/data", volumeAnnotation.Target);
         Assert.Equal(ContainerMountType.Volume, volumeAnnotation.Type);
         Assert.Equal(isReadOnly ?? false, volumeAnnotation.IsReadOnly);

diff --git a/tests/Aspire.Hosting.Keycloak.Tests/KeycloakResourceBuilderTests.cs b/tests/Aspire.Hosting.Keycloak.Tests/KeycloakResourceBuilderTests.cs
@@ -52,7 +52,7 @@ public void WithDataVolumeAddsVolumeAnnotation()
 
         var volumeAnnotation = keycloak.Resource.Annotations.OfType<ContainerMountAnnotation>().Single();
 
-        Assert.Equal($"Aspire.Hosting.Tests-{resourceName}-data", volumeAnnotation.Source);
+        Assert.Equal($"{builder.GetVolumePrefix()}-{resourceName}-data", volumeAnnotation.Source);
         Assert.Equal("/opt/keycloak/data", volumeAnnotation.Target);
         Assert.Equal(ContainerMountType.Volume, volumeAnnotation.Type);
         Assert.False(volumeAnnotation.IsReadOnly);

diff --git a/tests/Aspire.Hosting.Redis.Tests/AddRedisTests.cs b/tests/Aspire.Hosting.Redis.Tests/AddRedisTests.cs
@@ -268,7 +268,7 @@ public void WithDataVolumeAddsVolumeAnnotation(bool? isReadOnly)
 
         var volumeAnnotation = redis.Resource.Annotations.OfType<ContainerMountAnnotation>().Single();
 
-        Assert.Equal("Aspire.Hosting.Tests-myRedis-data", volumeAnnotation.Source);
+        Assert.Equal($"{builder.GetVolumePrefix()}-myRedis-data", volumeAnnotation.Source);
         Assert.Equal("/data", volumeAnnotation.Target);
         Assert.Equal(ContainerMountType.Volume, volumeAnnotation.Type);
         Assert.Equal(isReadOnly ?? false, volumeAnnotation.IsReadOnly);

diff --git a/tests/Aspire.Hosting.Tests/Utils/TestDistributedApplicationBuilder.cs b/tests/Aspire.Hosting.Tests/Utils/TestDistributedApplicationBuilder.cs
@@ -40,6 +40,10 @@ public static TestDistributedApplicationBuilder Create(DistributedApplicationOpe
         return Create(args);
     }
 
+    // Returns the unique prefix used for volumes from unnamed volumes this builder
+    public string GetVolumePrefix() =>
+        $"{VolumeNameGenerator.Sanitize(Environment.ApplicationName).ToLowerInvariant()}-{Configuration["AppHost:Sha256"]!.ToLowerInvariant()[..10]}";
+
     public static TestDistributedApplicationBuilder Create(params string[] args)
     {
         return new TestDistributedApplicationBuilder(options => options.Args = args);

diff --git a/tests/Aspire.Hosting.Tests/Utils/VolumeNameGeneratorTests.cs b/tests/Aspire.Hosting.Tests/Utils/VolumeNameGeneratorTests.cs
@@ -8,37 +8,18 @@ namespace Aspire.Hosting.Tests.Utils;
 
 public class VolumeNameGeneratorTests
 {
-    [Theory]
-    [InlineData("ACustomButValidAppName")]
-    [InlineData("0123ThisIsValidToo")]
-    [InlineData("This_Is_Valid_Too")]
-    [InlineData("This.Is.Valid.Too")]
-    [InlineData("This-Is-Valid-Too")]
-    [InlineData("This_Is.Valid-Too")]
-    [InlineData("This_0Is.1Valid-2Too")]
-    [InlineData("This_---.....---___Is_Valid_Too")]
-    public void UsesApplicationNameAsPrefixIfCharsAreValid(string applicationName)
+    [Fact]
+    public void VolumeGeneratorUsesUniqueName()
     {
         var builder = DistributedApplication.CreateBuilder();
-        builder.Environment.ApplicationName = applicationName;
-        var resource = builder.AddResource(new TestResource("myresource"));
 
-        var volumeName = CreateVolumeName(resource, "data");
-
-        Assert.Equal($"{builder.Environment.ApplicationName}-{resource.Resource.Name}-data", volumeName);
-    }
+        var volumePrefix = $"{Sanitize(builder.Environment.ApplicationName).ToLowerInvariant()}-{builder.Configuration["AppHost:Sha256"]!.ToLowerInvariant()[..10]}";
 
-    [Theory]
-    [MemberData(nameof(InvalidNameParts))]
-    public void UsesVolumeAsPrefixIfApplicationNameCharsAreInvalid(string applicationName)
-    {
-        var builder = DistributedApplication.CreateBuilder();
-        builder.Environment.ApplicationName = applicationName;
         var resource = builder.AddResource(new TestResource("myresource"));
 
         var volumeName = CreateVolumeName(resource, "data");
 
-        Assert.Equal($"volume-{resource.Resource.Name}-data", volumeName);
+        Assert.Equal($"{volumePrefix}-{resource.Resource.Name}-data", volumeName);
     }
 
     [Theory]

diff --git a/tests/Aspire.Hosting.Valkey.Tests/AddValkeyTests.cs b/tests/Aspire.Hosting.Valkey.Tests/AddValkeyTests.cs
@@ -129,7 +129,7 @@ public void WithDataVolumeAddsVolumeAnnotation(bool? isReadOnly)
 
         var volumeAnnotation = valkey.Resource.Annotations.OfType<ContainerMountAnnotation>().Single();
 
-        Assert.Equal("Aspire.Hosting.Tests-myValkey-data", volumeAnnotation.Source);
+        Assert.Equal($"{builder.GetVolumePrefix()}-myValkey-data", volumeAnnotation.Source);
         Assert.Equal("/data", volumeAnnotation.Target);
         Assert.Equal(ContainerMountType.Volume, volumeAnnotation.Type);
         Assert.Equal(isReadOnly ?? false, volumeAnnotation.IsReadOnly);