Autocorrect image names if they are correctable (#108)

* correct input container image names if they are correctable

* another test

* propose warning code

* remove unused pattern

Author: baronfel

Microsoft.NET.Build.Containers/ContainerHelpers.cs

@@ -9,6 +9,11 @@ public static class ContainerHelpers
 
     private static Regex imageNameRegex = new Regex("^[a-z0-9]+([._-][a-z0-9]+)*(/[a-z0-9]+([._-][a-z0-9]+)*)*$");
 
+    /// <summary>
+    /// Matches if the string is not lowercase or numeric, or ., _, or -.
+    /// </summary>
+    private static Regex imageNameCharacters = new Regex("[^a-zA-Z0-9._-]");
+
     /// <summary>
     /// Given some "fully qualified" image name (e.g. mcr.microsoft.com/dotnet/runtime), return
     /// a valid UriBuilder. This means appending 'https' if the URI is not absolute, otherwise UriBuilder will throw.
@@ -39,8 +44,8 @@ public static bool IsValidRegistry(string registryName)
     {
         // No scheme prefixed onto the registry
         if (string.IsNullOrEmpty(registryName) ||
-            (!registryName.StartsWith("http://") && 
-             !registryName.StartsWith("https://") && 
+            (!registryName.StartsWith("http://") &&
+             !registryName.StartsWith("https://") &&
              !registryName.StartsWith("docker://")))
         {
             return false;
@@ -89,9 +94,9 @@ public static bool IsValidImageTag(string imageTag)
     /// <param name="containerName"></param>
     /// <param name="containerTag"></param>
     /// <returns>True if the parse was successful. When false is returned, all out vars are set to empty strings.</returns>
-    public static bool TryParseFullyQualifiedContainerName(string fullyQualifiedContainerName, 
-                                                            [NotNullWhen(true)] out string? containerRegistry, 
-                                                            [NotNullWhen(true)] out string? containerName, 
+    public static bool TryParseFullyQualifiedContainerName(string fullyQualifiedContainerName,
+                                                            [NotNullWhen(true)] out string? containerRegistry,
+                                                            [NotNullWhen(true)] out string? containerName,
                                                             [NotNullWhen(true)] out string? containerTag)
     {
         Uri? uri = ContainerImageToUri(fullyQualifiedContainerName);
@@ -115,4 +120,27 @@ public static bool TryParseFullyQualifiedContainerName(string fullyQualifiedCont
         containerTag = indexOfColon == -1 ? "" : image.Substring(indexOfColon + 1);
         return true;
     }
+
+    /// <summary>
+    /// Checks if a given container image name adheres to the image name spec. If not, and recoverable, then normalizes invalid characters.
+    /// </summary>
+    public static bool NormalizeImageName(string containerImageName, [NotNullWhen(false)] out string? normalizedImageName)
+    {
+        if (IsValidImageName(containerImageName))
+        {
+            normalizedImageName = null;
+            return true;
+        }
+        else
+        {
+            if (Char.IsUpper(containerImageName, 0))
+            {
+                containerImageName = Char.ToLowerInvariant(containerImageName[0]) + containerImageName[1..];
+            } else if (!Char.IsLetterOrDigit(containerImageName, 0)) {
+                throw new ArgumentException("The first character of the image name must be a lowercase letter or a digit.");
+            }
+            normalizedImageName = imageNameCharacters.Replace(containerImageName, "-");
+            return false;
+        }
+    }
 }

Microsoft.NET.Build.Containers/ParseContainerProperties.cs

@@ -62,11 +62,6 @@ public ParseContainerProperties()
 
     public override bool Execute()
     {
-        if (!ContainerHelpers.IsValidImageName(ContainerImageName))
-        {
-            Log.LogError($"Invalid {nameof(ContainerImageName)}: {0}", ContainerImageName);
-            return !Log.HasLoggedErrors;
-        }
 
         if (!string.IsNullOrEmpty(ContainerImageTag) && !ContainerHelpers.IsValidImageTag(ContainerImageTag))
         {
@@ -106,11 +101,29 @@ public override bool Execute()
             return !Log.HasLoggedErrors;
         }
 
+        try
+        {
+            if (!ContainerHelpers.NormalizeImageName(ContainerImageName, out string? normalizedImageName))
+            {
+                Log.LogWarning(null, "CONTAINER001", null, null, 0, 0, 0, 0, $"{nameof(ContainerImageName)} was not a valid container image name, it was normalized to {normalizedImageName}");
+                NewContainerImageName = normalizedImageName;
+            }
+            else
+            {
+                // name was valid already
+                NewContainerImageName = ContainerImageName;
+            }
+        }
+        catch (ArgumentException)
+        {
+            Log.LogError($"Invalid {nameof(ContainerImageName)}: {{0}}", ContainerImageName);
+            return !Log.HasLoggedErrors;
+        }
+
         ParsedContainerRegistry = outputReg;
         ParsedContainerImage = outputImage;
         ParsedContainerTag = outputTag;
         NewContainerRegistry = registryToUse;
-        NewContainerImageName = ContainerImageName;
         NewContainerTag = ContainerImageTag;
 
         if (BuildEngine != null)

Test.Microsoft.NET.Build.Containers.Filesystem/TargetsTests.cs

@@ -76,12 +76,26 @@ public void CanSetEntrypointArgsToUseAppHost(bool useAppHost, params string[] en
             ["UseAppHost"] = useAppHost.ToString()
         });
         Assert.IsTrue(project.Build("ComputeContainerConfig"));
+        var computedEntrypointArgs = project.GetItems("ContainerEntrypoint").Select(i => i.EvaluatedInclude).ToArray();
+        foreach (var (First, Second) in entrypointArgs.Zip(computedEntrypointArgs))
         {
-            var computedEntrypointArgs = project.GetItems("ContainerEntrypoint").Select(i => i.EvaluatedInclude).ToArray();
-            foreach (var (First, Second) in entrypointArgs.Zip(computedEntrypointArgs))
-            {
-                Assert.AreEqual(First, Second);
-            }
+            Assert.AreEqual(First, Second);
         }
     }
+
+    [DataRow("WebApplication44", "webApplication44", true)]
+    [DataRow("friendly-suspicious-alligator", "friendly-suspicious-alligator", true)]
+    [DataRow("*friendly-suspicious-alligator", "", false)]
+    [DataRow("web/app2+7", "web-app2-7", true)]
+    [TestMethod]
+    public void CanNormalizeInputContainerNames(string projectName, string expectedContainerImageName, bool shouldPass)
+    {
+        var project = InitProject(new()
+        {
+            ["AssemblyName"] = projectName
+        });
+        var instance = project.CreateProjectInstance(global::Microsoft.Build.Execution.ProjectInstanceSettings.None);
+        Assert.AreEqual(shouldPass, instance.Build(new[]{"ComputeContainerConfig"}, null, null, out var outputs), "Build should have succeeded");
+        Assert.AreEqual(expectedContainerImageName, instance.GetPropertyValue("ContainerImageName"));
+    }
 }