Merge pull request #427 from rasmus/better-docker-credentials

Improve parsing of docker.io credentials
rasmus · Oct 22, 2024 · 1c3245e · 1c3245e
2 parents cddd6a6 + 0bc7539
commit 1c3245e
Show file tree

Hide file tree

Showing 8 changed files with 66 additions and 13 deletions.
diff --git a/.github/workflows/pull-requests.yml b/.github/workflows/pull-requests.yml
@@ -44,7 +44,7 @@ jobs:
       run: dotnet publish -o bake-it Source/Bake
 
     - name: Run Bake
-      run: bake-it/bake run --build-version 0.28.$GITHUB_RUN_NUMBER
+      run: bake-it/bake run --build-version 0.29.$GITHUB_RUN_NUMBER
 
     - name: Upload test results
       uses: actions/upload-artifact@v3

diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml
@@ -43,7 +43,7 @@ jobs:
       run: dotnet publish -o bake-it Source/Bake
 
     - name: Run Bake
-      run: bake-it/bake run --convention=Release --build-version 0.28.$GITHUB_RUN_NUMBER --destination="nuget>github,nuget,release>github"
+      run: bake-it/bake run --convention=Release --build-version 0.29.$GITHUB_RUN_NUMBER --destination="nuget>github,nuget,release>github"
 
     - name: Upload test results
       uses: actions/upload-artifact@v3

diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
@@ -1,3 +1,7 @@
+# 0.29-beta
+
+* Fix: Incorrect parsing of Docker Hub image names, e.g. `rasmus/debug`
+
 # 0.28-beta
 
 * Feature: Allow setting timeouts for both ingredient gathering as well as composing

diff --git a/Source/Bake.Tests/UnitTests/Core/CredentialsTests.cs b/Source/Bake.Tests/UnitTests/Core/CredentialsTests.cs
@@ -20,11 +20,8 @@
 // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 // SOFTWARE.
 
-using System;
-using System.Collections.Generic;
-using System.Threading;
-using System.Threading.Tasks;
 using Bake.Core;
+using Bake.Services;
 using Bake.Tests.Helpers;
 using FluentAssertions;
 using NUnit.Framework;
@@ -36,7 +33,7 @@ public class CredentialsTests : TestFor<Credentials>
         [TestCase(
             "http://localhost:5555/v3/index.json",
             "bake_credentials_nuget_localhost_apikey", "acd0b30512ac4fa39f62eb7a61fcf56c")]
-        public async Task GetNuGetApiKeyAsync(
+        public async Task GetNuGetApiKey(
             string url,
             string environmentKey,
             string expectedCredentials)
@@ -56,5 +53,38 @@ public async Task GetNuGetApiKeyAsync(
             // Assert
             credentials.Should().Be(expectedCredentials);
         }
+
+        [TestCase(
+            "docker.io/rasmus/debug",
+            "dockerhub")]
+        [TestCase(
+            "artifactory.example.io/rasmus/debug",
+            "bake_credentials_docker_artifactory_example_io")]
+        public async Task GetDockerLogin(
+            string containerTagStr,
+            string environmentKeyPrefix)
+        {
+            // Arrange
+            var username = A<string>();
+            var password = A<string>();
+            Inject<IDefaults>(A<Defaults>());
+            Inject<IEnvironmentVariables>(new TestEnvironmentVariables(new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase)
+            {
+                [$"{environmentKeyPrefix}_username"] = username,
+                [$"{environmentKeyPrefix}_password"] = password
+            }));
+            var containerTagParser = new ContainerTagParser();
+            containerTagParser.TryParse(containerTagStr, out var containerTag).Should().BeTrue();
+
+            // Act
+            var credentials = await Sut.TryGetDockerLoginAsync(
+                containerTag!,
+                CancellationToken.None);
+
+            // Assert
+            credentials!.Username.Should().Be(username);
+            credentials!.Password.Should().Be(password);
+        }
+
     }
 }
diff --git a/Source/Bake.Tests/UnitTests/Services/ContainerTagParserTests.cs b/Source/Bake.Tests/UnitTests/Services/ContainerTagParserTests.cs
@@ -31,6 +31,12 @@ namespace Bake.Tests.UnitTests.Services
 {
     public class ContainerTagParserTests : TestFor<ContainerTagParser>
     {
+        [TestCase(
+            "rasmus/debug:latest",
+            "",
+            "rasmus",
+            "debug",
+            "latest")]
         [TestCase(
             "registry",
             "",
@@ -49,6 +55,12 @@ public class ContainerTagParserTests : TestFor<ContainerTagParser>
             "path",
             "image",
             "latest")]
+        [TestCase(
+            "127.0.0.1/path/image:2",
+            "127.0.0.1",
+            "path",
+            "image",
+            "2")]
         [TestCase(
             "localhost/path/image:2",
             "localhost",

diff --git a/Source/Bake/Core/Credentials.cs b/Source/Bake/Core/Credentials.cs
@@ -155,18 +155,18 @@ public async Task<string> TryGetOctopusDeployApiKeyAsync(
         {
             var environmentVariables = await _environmentVariables.GetAsync(cancellationToken);
 
-            if (containerTag.HostAndPort.StartsWith("ghcr.io/") &&
+            if (string.Equals(containerTag.HostAndPort, "ghcr.io", StringComparison.OrdinalIgnoreCase) &&
                 environmentVariables.TryGetValue("github_token", out var githubToken))
             {
                 return new DockerLogin(
-                    containerTag.HostAndPort.Split('/', StringSplitOptions.RemoveEmptyEntries)[1],
+                    containerTag.HostAndPort,
                     githubToken,
                     "ghcr.io");
             }
 
             var possibilities = new List<(string, string)>();
 
-            if (string.IsNullOrEmpty(containerTag.HostAndPort))
+            if (string.IsNullOrEmpty(containerTag.HostAndPort) || string.Equals(containerTag.HostAndPort, "docker.io", StringComparison.OrdinalIgnoreCase))
             {
                 possibilities.Add(("dockerhub", string.Empty));
             }
@@ -183,6 +183,15 @@ public async Task<string> TryGetOctopusDeployApiKeyAsync(
                 var username = environmentVariables.TryGetValue($"{e}_username", out var u) ? u : string.Empty;
                 var password = environmentVariables.TryGetValue($"{e}_password", out var p) ? p : string.Empty;
 
+                if (!string.IsNullOrEmpty(username) && !string.IsNullOrEmpty(password))
+                {
+                    _logger.LogInformation($"Did not find any Docker credentials at '{e}_(username/password)'");
+                }
+                else
+                {
+                    _logger.LogInformation($"Found Docker credentials at '{e}_(username/password)'");
+                }
+
                 return !string.IsNullOrEmpty(username) && !string.IsNullOrEmpty(password)
                     ? new DockerLogin(username, password, s)
                     : null;

diff --git a/Source/Bake/Services/ContainerTagParser.cs b/Source/Bake/Services/ContainerTagParser.cs
@@ -30,7 +30,7 @@ public class ContainerTagParser : IContainerTagParser
     {
         private static readonly Regex ImageParser = new(
             @"^
-                (?<hostAndPort>[a-z\-0-9\.]+(:[0-9]+){0,1}/){0,1}
+                (?<hostAndPort>([a-z\-0-9]+\.[a-z\-0-9\.]+|localhost)(:[0-9]+){0,1}/){0,1}
                 (?<path>[a-z\-0-9/]+/){0,1}
                 (?<name>[a-z\-0-9]+)
                 (:(?<label>[a-z\-0-9\.]+)){0,1}

diff --git a/Source/Bake/ValueObjects/ContainerTag.cs b/Source/Bake/ValueObjects/ContainerTag.cs
@@ -20,8 +20,6 @@
 // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 // SOFTWARE.
 
-using System;
-
 namespace Bake.ValueObjects
 {
     public class ContainerTag