Add support for ssh git credentials#1917
Conversation
# Conflicts: # source/Calamari.Tests/ArgoCD/Git/GitHttpSmartSubTransportTests.cs
# Conflicts: # source/Calamari.Tests/ArgoCD/Git/RepositoryWrapperTest.cs
# Conflicts: # source/Calamari/CommitToGit/CommitToGitConfigFactory.cs
…enssl1.1-handling # Conflicts: # source/Calamari.Common/Plumbing/Variables/CustomPropertiesLoader.cs # source/Calamari.Contracts/ArgoCD/ArgoCDCustomPropertiesDto.cs # source/Calamari.Tests/ArgoCD/Contracts/ArgoCDCustomPropertiesDtoSerializationTests.cs # source/Calamari.Tests/ArgoCD/Git/AuthenticatingRepositoryFactoryTests.cs # source/Calamari.Tests/ArgoCD/Git/GitCloneSafeUrlTests.cs # source/Calamari.Tests/ArgoCD/Git/PullRequests/GitPullRequestClientResolverTests.cs # source/Calamari.Tests/CommitToGit/CommitToGitConfigFactoryTests.cs # source/Calamari/ArgoCD/Conventions/UpdateArgoCDAppImagesInstallConvention.cs # source/Calamari/ArgoCD/Conventions/UpdateArgoCDApplicationManifestsInstallConvention.cs # source/Calamari/ArgoCD/Git/AuthenticatingRepositoryFactory.cs # source/Calamari/ArgoCD/Git/GitCloneSafeUrl.cs # source/Calamari/ArgoCD/Git/GitConnection.cs # source/Calamari/ArgoCD/Git/IGitCredentialDtoJsonConverter.cs # source/Calamari/ArgoCD/Git/PullRequests/GitVendorPullRequestClientResolver.cs # source/Calamari/ArgoCD/Git/PullRequests/StringExtensionMethods.cs # source/Calamari/ArgoCD/Git/PullRequests/Vendors/AzureDevOps/AzureDevOpsPullRequestClient.cs # source/Calamari/ArgoCD/Git/PullRequests/Vendors/BitBucket/BitBucketPullRequestClient.cs # source/Calamari/ArgoCD/Git/PullRequests/Vendors/GitHub/GitHubPullRequestClient.cs # source/Calamari/ArgoCD/Git/PullRequests/Vendors/GitLab/GitLabPullRequestClient.cs # source/Calamari/ArgoCD/Git/PullRequests/Vendors/GitLab/GitLabPullRequestClientFactory.cs # source/Calamari/ArgoCD/Git/RepositoryFactory.cs # source/Calamari/ArgoCD/Git/RepositoryWrapper.cs # source/Calamari/CommitToGit/CommitToGitConfigFactory.cs
# Conflicts: # source/Calamari.Contracts/ArgoCD/ArgoCDCustomPropertiesDto.cs # source/Calamari.Tests/ArgoCD/Contracts/ArgoCDCustomPropertiesDtoSerializationTests.cs # source/Calamari.Tests/ArgoCD/Git/AuthenticatingRepositoryFactoryTests.cs # source/Calamari/ArgoCD/Git/AuthenticatingRepositoryFactory.cs # source/Calamari/ArgoCD/Git/GitConnection.cs # source/Calamari/ArgoCD/Git/IGitCredentialDtoJsonConverter.cs # source/Calamari/ArgoCD/Git/PullRequests/GitVendorPullRequestClientResolver.cs # source/Calamari/ArgoCD/Git/PullRequests/StringExtensionMethods.cs # source/Calamari/ArgoCD/Git/RepositoryFactory.cs # source/Calamari/ArgoCD/Git/RepositoryWrapper.cs
|
|
||
| internal static class SshHostKeyVerificationBypass | ||
| { | ||
| // TODO(eddy): Implement proper host key verification |
There was a problem hiding this comment.
NB this doesn't include the known hosts handling for ease of review
| using var temporaryFolder = TemporaryDirectory.Create(); | ||
|
|
||
| CredentialsHandler credentialsHandler = (url, usernameFromUrl, types) => new UsernamePasswordCredentials { Username = cloneUsername, Password = clonePassword}; | ||
| LibGit2SharpTransportRegistration.EnsureRegistered(); |
There was a problem hiding this comment.
We enter git usage in a few weird places in the tests, so for now I've just called this method so we get decent errors if the test runner doesn't meet the dependencies.
Normally this gets done in the repo wrapper and it's a bit cleaner.
# Conflicts: # source/Calamari.Contracts/ArgoCD/ArgoCDCustomPropertiesDto.cs # source/Calamari.Contracts/Git/GitCredentials.cs
APErebus
left a comment
There was a problem hiding this comment.
Only a couple of relatively minor questions or comments
| this.gitCredentials = gitCredentials | ||
| .GroupBy(c => c.Url) | ||
| .ToDictionary(g => g.Key, g => g.OfType<GitCredentialDto>().FirstOrDefault<IGitCredentialDto>() ?? g.First()); |
There was a problem hiding this comment.
Are we actually preferencing here like the comment suggests?
There was a problem hiding this comment.
Yeah we are GitCredentialDto is the username/password type (I didn't rename it for contract reasons, but it might be worth revisiting how we'd do that safely).
So the logic is: For each url, get the username password cred. If we don't find that, get any cred (ie. ssh).
This is going to change with https://linear.app/octopus/issue/MD-1898/match-ssh-keys-to-ssh-urls-but-also-pass-in-usernamepasswords-for-api, but it matches the logic in server at the moment.
Probably also worth noting that server shouldn't be sending down multiple creds at the moment, but keeping this logic in place allows us to have confidence that the credential will be selected deterministically if we do start sending multiple credentials per url to calamari.
Adds support for SSH credentials when using git, but does not use them until Server gets the same treatment.
We don't have known host support yet, I'm going to do that separately for PR conciseness. In the meantime, we accept all hosts.
Closes MD-1680
Closes MD-1715