Upfront errors when using the wrong Git creds for PRs

source/Calamari.Tests/ArgoCD/Git/AuthenticatingRepositoryFactoryTests.cs

@@ -2,6 +2,7 @@
 using System.IO;
 using Calamari.ArgoCD.Git;
 using Calamari.ArgoCD.Git.PullRequests;
+using Calamari.Common.Commands;
 using Calamari.Common.Plumbing.FileSystem;
 using Calamari.Integration.Time;
 using Calamari.Testing.Helpers;
@@ -58,7 +59,7 @@ [new GitCredentialDto(httpsUrl, "", "")],
                 repositoryFactory,
                 log);
 
-            using var wrapper = factory.CloneRepository(httpsUrl, branchName.ToFriendlyName());
+            using var wrapper = factory.CloneRepository(httpsUrl, branchName.ToFriendlyName(), requiresPullRequest: false);
             wrapper.Should().NotBeNull();
         }
 
@@ -71,10 +72,34 @@ public void AnonymousCloneWhenNoCredentialsMatch()
                 repositoryFactory,
                 log);
 
-            using var wrapper = factory.CloneRepository(originUrl, branchName.ToFriendlyName());
+            using var wrapper = factory.CloneRepository(originUrl, branchName.ToFriendlyName(), requiresPullRequest: false);
             wrapper.Should().NotBeNull();
             log.Messages.Should().Contain(m => m.FormattedMessage.Contains("No Git credentials found"));
         }
+
+        [Test]
+        public void LogsAnInformationalMessageWhenCloningAnonymously()
+        {
+            var mockRepoFactory = Substitute.For<IRepositoryFactory>();
+            var factory = new AuthenticatingRepositoryFactory([], mockRepoFactory, log);
+
+            factory.CloneRepository("https://github.com/org/repo.git", "main", requiresPullRequest: false);
+
+            mockRepoFactory.Received()
+                           .CloneRepository(Arg.Any<string>(), Arg.Is<IGitConnection>(c => c is AnonymousGitConnection));
+            log.Messages.Should().Contain(m => m.FormattedMessage.Contains("No Git credentials found"));
+        }
+
+        [Test]
+        public void ThrowsWhenCreatingAPullRequestWithoutCredentials()
+        {
+            var mockRepoFactory = Substitute.For<IRepositoryFactory>();
+            var factory = new AuthenticatingRepositoryFactory([], mockRepoFactory, log);
+
+            var act = () => factory.CloneRepository("https://github.com/org/repo.git", "main", requiresPullRequest: true);
+
+            act.Should().Throw<CommandException>().And.Message.Should().Contain("requires Git repository credentials");
+        }
     }
 
     [TestFixture]
@@ -95,12 +120,26 @@ [new SshKeyGitCredentialDto(sshUrl, "git", "private-key", [])],
                 mockRepoFactory,
                 log);
 
-            factory.CloneRepository(sshUrl, branchName.ToFriendlyName());
+            factory.CloneRepository(sshUrl, branchName.ToFriendlyName(), requiresPullRequest: false);
 
             mockRepoFactory.Received()
-                           .CloneRepository(
-                               Arg.Any<string>(),
-                               Arg.Is<IGitConnection>(c => c is SshKeyGitConnection));
+                           .CloneRepository(Arg.Any<string>(), Arg.Is<IGitConnection>(c => c is SshKeyGitConnection));
+        }
+
+        [Test]
+        public void ThrowsWhenCreatingAPullRequestWithAnSshKeyCredential()
+        {
+            const string sshUrl = "ssh://git@github.com/org/repo.git";
+            var mockRepoFactory = Substitute.For<IRepositoryFactory>();
+
+            var factory = new AuthenticatingRepositoryFactory(
+                [new SshKeyGitCredentialDto(sshUrl, "git", "private-key", [])],
+                mockRepoFactory,
+                log);
+
+            var act = () => factory.CloneRepository(sshUrl, branchName.ToFriendlyName(), requiresPullRequest: true);
+
+            act.Should().Throw<CommandException>().And.Message.Should().Contain("SSH key authentication");
         }
 
         [Test]
@@ -127,7 +166,7 @@ [new SshKeyGitCredentialDto(sshUrl, "git", "private-key", knownHosts)],
                 mockRepoFactory,
                 log);
 
-            factory.CloneRepository(sshUrl, branchName.ToFriendlyName());
+            factory.CloneRepository(sshUrl, branchName.ToFriendlyName(), requiresPullRequest: false);
 
             mockRepoFactory.Received()
                            .CloneRepository(
@@ -159,7 +198,7 @@ [new GitCredentialDto(httpsUrl, "user", "pass")],
 
             // This will fail to clone (no real repo at this URL) but we can verify it
             // falls through to anonymous because the SCP URL doesn't match the HTTPS URL
-            var act = () => factory.CloneRepository(scpUrl, "main");
+            var act = () => factory.CloneRepository(scpUrl, "main", requiresPullRequest: false);
             act.Should().Throw<Exception>(); // clone failure expected
             log.Messages.Should().Contain(m => m.FormattedMessage.Contains("No Git credentials found"));
         }
@@ -184,11 +223,35 @@ protected void AssertHttpsCredentialTakesPriorityOverSsh(string url)
 
         var factory = new AuthenticatingRepositoryFactory(rawCredentials, mockRepoFactory, log);
 
-        factory.CloneRepository(url, "main");
+        factory.CloneRepository(url, "main", requiresPullRequest: false);
 
         mockRepoFactory.Received()
                        .CloneRepository(
                            Arg.Any<string>(),
                            Arg.Is<IGitConnection>(c => c is HttpsGitConnection));
     }
+
+    [TestFixture]
+    public class UrlNormalizationTests : AuthenticatingRepositoryFactoryTestBase
+    {
+        [Test]
+        public void SchemelessUrlIsNormalizedWithOciSchemeForNonSshCredential()
+        {
+            const string schemelessUrl = "registry.example.com/charts/myapp";
+            const string expectedNormalizedUrl = "oci://registry.example.com/charts/myapp";
+
+            var mockRepoFactory = Substitute.For<IRepositoryFactory>();
+            var factory = new AuthenticatingRepositoryFactory(
+                [new GitCredentialDto(schemelessUrl, "user", "password")],
+                mockRepoFactory,
+                log);
+
+            factory.CloneRepository(schemelessUrl, "main", requiresPullRequest: false);
+
+            mockRepoFactory.Received()
+                           .CloneRepository(
+                               Arg.Any<string>(),
+                               Arg.Is<IGitConnection>(c => c.Url == expectedNormalizedUrl));
+        }
+    }
 }

source/Calamari.Tests/ArgoCD/Git/GitConnectionFactoryTests.cs

@@ -0,0 +1,70 @@
+using Calamari.ArgoCD.Git;
+using FluentAssertions;
+using NUnit.Framework;
+using ArgoCdPasswordCredential = Octopus.Calamari.Contracts.ArgoCD.GitCredentialDto;
+using ArgoCdSshKeyCredential = Octopus.Calamari.Contracts.ArgoCD.SshKeyGitCredentialDto;
+using GitCredential = Octopus.Calamari.Contracts.Git.IGitCredentialDto;
+using GitPasswordCredential = Octopus.Calamari.Contracts.Git.UsernamePasswordGitCredentialDto;
+using GitSshKeyCredential = Octopus.Calamari.Contracts.Git.SshKeyGitCredentialDto;
+using GitSshKnownHostDto = Octopus.Calamari.Contracts.Git.SshKnownHostDto;
+
+namespace Calamari.Tests.ArgoCD.Git;
+
+[TestFixture]
+public class GitConnectionFactoryTests
+{
+    static readonly GitReference Reference = GitBranchName.CreateFromFriendlyName("main");
+    const string HttpsUrl = "https://github.com/org/repo.git";
+    const string SshUrl = "ssh://git@github.com/org/repo.git";
+
+    [Test]
+    public void ArgoCdSshKeyCredentialIsMappedToSshKeyGitConnection()
+    {
+        var connection = GitConnectionFactory.Create(new ArgoCdSshKeyCredential(SshUrl, "git", "private-key", []), SshUrl, Reference);
+
+        connection.Should().BeEquivalentTo(new SshKeyGitConnection("git", "private-key", SshUrl, Reference, []));
+    }
+
+    [Test]
+    public void ArgoCdUsernamePasswordCredentialIsMappedToUsernamePasswordGitConnection()
+    {
+        var connection = GitConnectionFactory.Create(new ArgoCdPasswordCredential(HttpsUrl, "user", "password"), HttpsUrl, Reference);
+
+        connection.Should().BeEquivalentTo(new UsernamePasswordGitConnection("user", "password", HttpsUrl, Reference), options => options.Excluding(c => c.Uri));
+    }
+
+    [Test]
+    public void SshKeyCredentialIsMappedToSshKeyGitConnection()
+    {
+        GitSshKnownHostDto[] knownHosts =
+        [
+            new ("github.com", "AAAAB3NzaC1yc2EAAAADAQABAAABAQ=="),
+            new ("bitbucket.org", "AAAAC3NzaC1lZDI1NTE5AAAAIA=="),
+        ];
+        var connection = GitConnectionFactory.Create(new GitSshKeyCredential("cred", SshUrl, "git", "private-key", knownHosts), SshUrl, Reference);
+
+
+        SshKnownHost[] expectedKnownHosts =
+        [
+            new("github.com", "AAAAB3NzaC1yc2EAAAADAQABAAABAQ"),
+            new("bitbucket.org", "AAAAC3NzaC1lZDI1NTE5AAAAIA=="),
+        ];
+        connection.Should().BeEquivalentTo(new SshKeyGitConnection("git", "private-key", SshUrl, Reference, expectedKnownHosts));
+    }
+
+    [Test]
+    public void UsernamePasswordCredentialIsMappedToUsernamePasswordGitConnection()
+    {
+        var connection = GitConnectionFactory.Create(new GitPasswordCredential("cred", HttpsUrl, "user", "password"), HttpsUrl, Reference);
+
+        connection.Should().BeEquivalentTo(new UsernamePasswordGitConnection("user", "password", HttpsUrl, Reference), options => options.Excluding(c => c.Uri));
+    }
+
+    [Test]
+    public void NoCredentialIsMappedToAnonymousGitConnection()
+    {
+        var connection = GitConnectionFactory.Create((GitCredential)null, HttpsUrl, Reference);
+
+        connection.Should().BeEquivalentTo(new AnonymousGitConnection(HttpsUrl, Reference), options => options.Excluding(c => c.Uri));
+    }
+}

source/Calamari.Tests/CommitToGit/CommitToGitConfigFactoryTests.cs

@@ -53,6 +53,34 @@ public void CreateRepositoryConfig_UsesUsernameAndPasswordFromLoadedProperties()
         httpsGitConnection.Uri.Value.Should().Be(new Uri("https://example.invalid/repo.git"));
     }
 
+    [Test]
+    public void CreateRepositoryConfig_WhenCreatingPullRequestWithSshKeyCredential_Throws()
+    {
+        variables.GetFlag(SpecialVariables.Action.Git.PullRequest.Create).Returns(true);
+        loader.Load<CommitToGitCustomPropertiesDto>()
+              .Returns(new CommitToGitCustomPropertiesDto(new SshKeyGitCredentialDto("MyCred", "https://example.invalid/repo.git", "git", "private-key", [])));
+
+        var deployment = new RunningDeployment(null, variables);
+
+        var act = () => factory.CreateRepositoryConfig(deployment, loader);
+
+        act.Should().Throw<CommandException>().And.Message.Should().Contain("SSH key authentication");
+    }
+
+    [Test]
+    public void CreateRepositoryConfig_WhenCreatingPullRequestWithoutCredentials_Throws()
+    {
+        variables.GetFlag(SpecialVariables.Action.Git.PullRequest.Create).Returns(true);
+        loader.Load<CommitToGitCustomPropertiesDto>()
+              .Returns(new CommitToGitCustomPropertiesDto(null));
+
+        var deployment = new RunningDeployment(null, variables);
+
+        var act = () => factory.CreateRepositoryConfig(deployment, loader);
+
+        act.Should().Throw<CommandException>().And.Message.Should().Contain("requires Git repository credentials");
+    }
+
     [Test]
     public void CreateRepositoryConfig_WhenDestinationPathIsMissing_DefaultsToEmptyString()
     {

source/Calamari/ArgoCD/Git/AuthenticatingRepositoryFactory.cs

@@ -1,6 +1,7 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using Calamari.Common.Commands;
 using Calamari.Common.Plumbing.Logging;
 using Octopus.Calamari.Contracts.ArgoCD;
 
@@ -26,39 +27,28 @@ public AuthenticatingRepositoryFactory(
         this.log = log;
     }
 
-    public RepositoryWrapper CloneRepository(string requestedUrl, string targetRevision)
+    public RepositoryWrapper CloneRepository(string requestedUrl, string targetRevision, bool requiresPullRequest)
     {
         var gitCredential = gitCredentials.GetValueOrDefault(requestedUrl);
-        switch (gitCredential)
+        var cloneUrl = gitCredential is SshKeyGitCredentialDto ? requestedUrl : GitCloneSafeUrl.ConvertToUriString(requestedUrl);
+        var gitConnection = GitConnectionFactory.Create(gitCredential, cloneUrl, GitReference.CreateFromString(targetRevision));
+
+        if (requiresPullRequest)
         {
-            case GitCredentialDto passwordCredential:
-            {
-                var gitConnection = new HttpsGitConnection(passwordCredential.Username, passwordCredential.Password, GitCloneSafeUrl.ConvertToUriString(requestedUrl), GitReference.CreateFromString(targetRevision));
-                return repositoryFactory.CloneRepository(UniqueRepoNameGenerator.Generate(), gitConnection);
-            }
-            case SshKeyGitCredentialDto sshCredential:
-            {
-                var sshConnection = new SshKeyGitConnection(
-                    sshCredential.Username,
-                    sshCredential.PrivateKey,
-                    requestedUrl,
-                    GitReference.CreateFromString(targetRevision),
-                    sshCredential.KnownHosts.Select(kh => new SshKnownHost(kh.Host, kh.PublicKey)).ToArray());
-                return repositoryFactory.CloneRepository(UniqueRepoNameGenerator.Generate(), sshConnection);
-            }
-            case null:
+            switch (gitConnection)
             {
-                log.Info($"No Git credentials found for: '{requestedUrl}', will attempt to clone repository anonymously.");
-                break;
-            }
-            default:
-            {
-                log.Warn($"An unrecognised credential type '{gitCredential.GetType().Name}' was found for '{requestedUrl}'. Ignoring the credentials and attempting an anonymous clone.");
-                break;
+                case SshKeyGitConnection:
+                    throw new CommandException("Creating PRs is not possible when using SSH key authentication, please use a username and password instead");
+                case AnonymousGitConnection:
+                    throw new CommandException("Creating a pull request requires Git repository credentials, but none were provided. Please configure a username and password.");
             }
         }
 
-        var anonGitConnection = new HttpsGitConnection(null, null, GitCloneSafeUrl.ConvertToUriString(requestedUrl), GitReference.CreateFromString(targetRevision));
-        return repositoryFactory.CloneRepository(UniqueRepoNameGenerator.Generate(), anonGitConnection);
+        if (gitConnection is AnonymousGitConnection)
+        {
+            log.Info($"No Git credentials found for: '{requestedUrl}', will attempt to clone repository anonymously.");
+        }
+
+        return repositoryFactory.CloneRepository(UniqueRepoNameGenerator.Generate(), gitConnection);
     }
-}
+}

source/Calamari/ArgoCD/Git/GitConnection.cs

@@ -2,7 +2,6 @@
 
 using System;
 using System.Collections.Generic;
-using System.Linq;
 using Calamari.Common.Commands;
 
 namespace Calamari.ArgoCD.Git
@@ -59,6 +58,12 @@ static Uri ParseAsHttpsUri(string repositoryUrl)
         }
     }
 
+    public class AnonymousGitConnection(string url, GitReference gitReference)
+        : HttpsGitConnection(null, null, url, gitReference);
+
+    public class UsernamePasswordGitConnection(string username, string password, string url, GitReference gitReference)
+        : HttpsGitConnection(username, password, url, gitReference);
+
     public record SshKeyGitConnection(
         string? Username,
         string PrivateKey,

source/Calamari/ArgoCD/Git/GitConnectionFactory.cs

@@ -0,0 +1,61 @@
+#nullable enable
+
+using System;
+using System.Linq;
+using Calamari.Common.Commands;
+using Octopus.Calamari.Contracts.Git;
+using ArgoCdIGitCredentialDto = Octopus.Calamari.Contracts.ArgoCD.IGitCredentialDto;
+using ArgoCdGitCredentialDto = Octopus.Calamari.Contracts.ArgoCD.GitCredentialDto;
+using ArgoCdSshKeyGitCredentialDto = Octopus.Calamari.Contracts.ArgoCD.SshKeyGitCredentialDto;
+
+namespace Calamari.ArgoCD.Git;
+
+public static class GitConnectionFactory
+{
+    // Converts Argo credentials into the common type
+    // ideally we migrate the Argo code path to use the common type from server and we can delete this
+    public static IGitConnection Create(ArgoCdIGitCredentialDto? credential, string url, GitReference gitReference)
+    {
+        return credential switch
+        {
+            ArgoCdGitCredentialDto password => Create(
+                // ArgoCD credentials don't pass a name
+                new UsernamePasswordGitCredentialDto(string.Empty, password.Url, password.Username, password.Password),
+                url,
+                gitReference
+            ),
+            ArgoCdSshKeyGitCredentialDto ssh => Create(
+                new SshKeyGitCredentialDto(
+                    // ArgoCD credentials don't pass a name
+                    string.Empty,
+                    ssh.Url,
+                    ssh.Username,
+                    ssh.PrivateKey,
+                    ssh.KnownHosts.Select(kh => new SshKnownHostDto(kh.Host, kh.PublicKey)).ToArray()),
+                url,
+                gitReference
+            ),
+            null => Create((IGitCredentialDto?)null, url, gitReference),
+            _ => throw new CommandException($"An unrecognised credential type '{credential.GetType().Name}' was found for '{url}'"),
+        };
+    }
+
+    public static IGitConnection Create(IGitCredentialDto? credential, string url, GitReference gitReference)
+    {
+        return credential switch
+        {
+            UsernamePasswordGitCredentialDto password
+                => new UsernamePasswordGitConnection(password.Username, password.Password, url, gitReference),
+            SshKeyGitCredentialDto ssh
+                => new SshKeyGitConnection(
+                    ssh.Username,
+                    ssh.PrivateKey,
+                    url,
+                    gitReference,
+                    ssh.KnownHosts.Select(kh => new SshKnownHost(kh.Host, kh.PublicKey)).ToArray()
+                ),
+            null => new AnonymousGitConnection(url, gitReference),
+            _ => throw new CommandException($"An unrecognised credential type '{credential.GetType().Name}' was found for '{url}'")
+        };
+    }
+}