Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

--target-api-url support for migrate repo and org #1214

Merged
merged 14 commits into from
Jan 30, 2024
2 changes: 1 addition & 1 deletion RELEASENOTES.md
Original file line number Diff line number Diff line change
@@ -1 +1 @@

- Add `--target-api-url` to `gh ado2gh migrate-repo`, `gh bbs2gh migrate-repo`, and `gh gei migrate-org` to support newer GitHub migration paths.
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public void Should_Have_Options()
{
_command.Should().NotBeNull();
_command.Name.Should().Be("migrate-repo");
_command.Options.Count.Should().Be(11);
_command.Options.Count.Should().Be(12);

TestHelpers.VerifyCommandOption(_command.Options, "ado-org", true);
TestHelpers.VerifyCommandOption(_command.Options, "ado-team-project", true);
Expand All @@ -48,6 +48,7 @@ public void Should_Have_Options()
TestHelpers.VerifyCommandOption(_command.Options, "ado-pat", false);
TestHelpers.VerifyCommandOption(_command.Options, "github-pat", false);
TestHelpers.VerifyCommandOption(_command.Options, "verbose", false);
TestHelpers.VerifyCommandOption(_command.Options, "target-api-url", false);
}

[Fact]
Expand All @@ -71,5 +72,28 @@ public void It_Uses_Github_Pat_When_Provided()

_mockGithubApiFactory.Verify(m => m.Create(null, githubPat));
}
[Fact]
public void It_Uses_Target_Api_Url_When_Provided()
{
var adoPat = "abc123";
var githubPat = "def456";
var targetApiUrl = "https://api.github.com";

var args = new MigrateRepoCommandArgs
{
AdoOrg = "foo-org",
AdoTeamProject = "blah-tp",
AdoRepo = "some-repo",
GithubOrg = "gh-org",
GithubRepo = "gh-repo",
AdoPat = adoPat,
GithubPat = githubPat,
TargetApiUrl = targetApiUrl
};

_command.BuildHandler(args, _serviceProvider);

_mockGithubApiFactory.Verify(m => m.Create(targetApiUrl, githubPat));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public void Should_Have_Options()
var command = new MigrateRepoCommand();
command.Should().NotBeNull();
command.Name.Should().Be("migrate-repo");
command.Options.Count.Should().Be(30);
command.Options.Count.Should().Be(31);

TestHelpers.VerifyCommandOption(command.Options, "bbs-server-url", true);
TestHelpers.VerifyCommandOption(command.Options, "bbs-project", true);
Expand Down Expand Up @@ -87,6 +87,7 @@ public void Should_Have_Options()
TestHelpers.VerifyCommandOption(command.Options, "verbose", false);
TestHelpers.VerifyCommandOption(command.Options, "keep-archive", false);
TestHelpers.VerifyCommandOption(command.Options, "no-ssl-verify", false);
TestHelpers.VerifyCommandOption(command.Options, "target-api-url", false);
}

[Fact]
Expand Down Expand Up @@ -214,6 +215,27 @@ public void BuildHandler_Creates_GitHub_Api_When_Github_Org_Is_Provided()
_mockGithubApiFactory.Verify(m => m.Create(null, GITHUB_PAT));
}

[Fact]
public void BuildHandler_Uses_Target_Api_Url_When_Provided()
{
// Arrange
var targetApiUrl = "https://api.github.com";
var args = new MigrateRepoCommandArgs
{
GithubOrg = GITHUB_ORG,
GithubPat = GITHUB_PAT,
TargetApiUrl = targetApiUrl
};

// Act
var handler = _command.BuildHandler(args, _mockServiceProvider.Object);

// Assert
handler.Should().NotBeNull();

_mockGithubApiFactory.Verify(m => m.Create(targetApiUrl, GITHUB_PAT));
}

[Fact]
public void BuildHandler_Creates_Bbs_Api_When_Bbs_Server_Url_Is_Provided()
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,42 @@
using FluentAssertions;
using Microsoft.Extensions.DependencyInjection;
using Moq;
using OctoshiftCLI.Contracts;
using OctoshiftCLI.GithubEnterpriseImporter.Commands.MigrateOrg;
using OctoshiftCLI.Services;
using Xunit;

namespace OctoshiftCLI.Tests.GithubEnterpriseImporter.Commands.MigrateOrg
{
public class MigrateOrgCommandTests
{
private readonly Mock<ITargetGithubApiFactory> _mockGithubApiFactory = new();
private readonly Mock<OctoLogger> _mockOctoLogger = TestHelpers.CreateMock<OctoLogger>();
private readonly Mock<EnvironmentVariableProvider> _mockEnvironmentVariableProvider = TestHelpers.CreateMock<EnvironmentVariableProvider>();
private readonly Mock<WarningsCountLogger> _warningsCountLogger = TestHelpers.CreateMock<WarningsCountLogger>();

private readonly ServiceProvider _serviceProvider;
private readonly MigrateOrgCommand _command = new();

public MigrateOrgCommandTests()
{
var serviceCollection = new ServiceCollection();
serviceCollection
.AddSingleton(_mockOctoLogger.Object)
.AddSingleton(_mockGithubApiFactory.Object)
.AddSingleton(_mockEnvironmentVariableProvider.Object)
.AddSingleton(_warningsCountLogger.Object);

_serviceProvider = serviceCollection.BuildServiceProvider();
}

[Fact]
public void Should_Have_Options()
{
var command = new MigrateOrgCommand();
command.Should().NotBeNull();
command.Name.Should().Be("migrate-org");
command.Options.Count.Should().Be(7);
command.Options.Count.Should().Be(8);

TestHelpers.VerifyCommandOption(command.Options, "github-source-org", true);
TestHelpers.VerifyCommandOption(command.Options, "github-target-org", true);
Expand All @@ -21,6 +45,29 @@ public void Should_Have_Options()
TestHelpers.VerifyCommandOption(command.Options, "github-source-pat", false);
TestHelpers.VerifyCommandOption(command.Options, "github-target-pat", false);
TestHelpers.VerifyCommandOption(command.Options, "verbose", false);
TestHelpers.VerifyCommandOption(command.Options, "target-api-url", false);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A similar test to It_Uses_Target_Api_Url_When_Provided also needs to be added for this command too.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay will add tomorrow, thanks for the quick feedback!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ArinGhazarian added in 833931b, also note I was following the gei spec which originally added this flag, hence why I added test in handler to begin with but thanks for catching it 👍🏾 https://github.com/github/gh-gei/blob/main/src/OctoshiftCLI.Tests/gei/Commands/MigrateRepo/MigrateRepoCommandTests.cs

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah a lot has changed in the CLI since then mostly because the underlying System.CommandLine nuget package pas early beta. Later on as the package became more mature, we changed the way we created the handlers to keep the handles' constructor very simple and easy to test and also keep the concerns as separate as possible so for each command we have the arg, the command and the handler, each one doing a specific thing. IMO the most interesting thing we did was to not inject factories into handlers and delegate the creation of a handler to the command via BuildHanlder and that's where all the creation logic goes.

}

[Fact]
public void It_Uses_Target_Api_Url_When_Provided()
{
var githubSourcePat = "abc123";
var githubTargetPat = "def456";
var targetApiUrl = "https://api.github.com";

var args = new MigrateOrgCommandArgs
{
GithubSourceOrg = "source-org",
GithubSourcePat = githubSourcePat,
GithubTargetOrg = "target-org",
GithubTargetEnterprise = "target-enterprise",
GithubTargetPat = githubTargetPat,
TargetApiUrl = targetApiUrl
};

_command.BuildHandler(args, _serviceProvider);

_mockGithubApiFactory.Verify(m => m.Create(targetApiUrl, githubTargetPat));
}
}
}
7 changes: 6 additions & 1 deletion src/ado2gh/Commands/MigrateRepo/MigrateRepoCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ public MigrateRepoCommand() : base(
AddOption(AdoPat);
AddOption(GithubPat);
AddOption(Verbose);
AddOption(TargetApiUrl);
}

public Option<string> AdoOrg { get; } = new("--ado-org")
Expand Down Expand Up @@ -61,6 +62,10 @@ public MigrateRepoCommand() : base(
{
Description = "The visibility of the target repo. Defaults to private. Valid values are public, private, or internal."
};
public Option<string> TargetApiUrl { get; } = new("--target-api-url")
{
Description = "The URL of the target API, if not migrating to github.com. Defaults to https://api.github.com"
};
public Option<string> AdoPat { get; } = new("--ado-pat");
public Option<string> GithubPat { get; } = new("--github-pat");
public Option<bool> Verbose { get; } = new("--verbose");
Expand All @@ -79,7 +84,7 @@ public override MigrateRepoCommandHandler BuildHandler(MigrateRepoCommandArgs ar

var log = sp.GetRequiredService<OctoLogger>();
var githubApiFactory = sp.GetRequiredService<ITargetGithubApiFactory>();
var githubApi = githubApiFactory.Create(targetPersonalAccessToken: args.GithubPat);
var githubApi = githubApiFactory.Create(args.TargetApiUrl, args.GithubPat);
var environmentVariableProvider = sp.GetRequiredService<EnvironmentVariableProvider>();
var warningsCountLogger = sp.GetRequiredService<WarningsCountLogger>();

Expand Down
1 change: 1 addition & 0 deletions src/ado2gh/Commands/MigrateRepo/MigrateRepoCommandArgs.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,6 @@ public class MigrateRepoCommandArgs : CommandArgs
public string AdoPat { get; set; }
[Secret]
public string GithubPat { get; set; }
public string TargetApiUrl { get; set; }
}
}
8 changes: 6 additions & 2 deletions src/bbs2gh/Commands/MigrateRepo/MigrateRepoCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ public MigrateRepoCommand() : base(
AddOption(Verbose);
AddOption(KeepArchive);
AddOption(NoSslVerify);
AddOption(TargetApiUrl);
}

public Option<string> BbsServerUrl { get; } = new(
Expand Down Expand Up @@ -182,7 +183,10 @@ public MigrateRepoCommand() : base(
public Option<bool> KeepArchive { get; } = new(
name: "--keep-archive",
description: "Keeps the downloaded export archive after successfully uploading it. By default, it will be automatically deleted.");

public Option<string> TargetApiUrl { get; } = new("--target-api-url")
{
Description = "The URL of the target API, if not migrating to github.com. Defaults to https://api.github.com"
};
public Option<bool> NoSslVerify { get; } = new(
name: "--no-ssl-verify",
description: "Disables SSL verification when communicating with your Bitbucket Server/Data Center instance. All other migration steps will continue to verify SSL. " +
Expand Down Expand Up @@ -212,7 +216,7 @@ public override MigrateRepoCommandHandler BuildHandler(MigrateRepoCommandArgs ar
if (args.GithubOrg.HasValue())
{
var githubApiFactory = sp.GetRequiredService<GithubApiFactory>();
githubApi = githubApiFactory.Create(null, args.GithubPat);
githubApi = githubApiFactory.Create(args.TargetApiUrl, args.GithubPat);
}

if (args.BbsServerUrl.HasValue())
Expand Down
1 change: 1 addition & 0 deletions src/bbs2gh/Commands/MigrateRepo/MigrateRepoCommandArgs.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ public class MigrateRepoCommandArgs : CommandArgs
public string GithubPat { get; set; }
public bool QueueOnly { get; set; }
public string TargetRepoVisibility { get; set; }
public string TargetApiUrl { get; set; }
public bool Kerberos { get; set; }

public string BbsServerUrl { get; set; }
Expand Down
7 changes: 6 additions & 1 deletion src/gei/Commands/MigrateOrg/MigrateOrgCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ public MigrateOrgCommand() : base(
AddOption(GithubSourceOrg);
AddOption(GithubTargetOrg);
AddOption(GithubTargetEnterprise);
AddOption(TargetApiUrl);

AddOption(GithubSourcePat);
AddOption(GithubTargetPat);
Expand All @@ -40,6 +41,10 @@ public MigrateOrgCommand() : base(
};
public Option<string> GithubSourcePat { get; } = new("--github-source-pat");
public Option<string> GithubTargetPat { get; } = new("--github-target-pat");
public Option<string> TargetApiUrl { get; } = new("--target-api-url")
{
Description = "The URL of the target API, if not migrating to github.com. Defaults to https://api.github.com"
};
public Option<bool> QueueOnly { get; } = new("--queue-only")
{
Description = "Only queues the migration, does not wait for it to finish. Use the wait-for-migration command to subsequently wait for it to finish and view the status."
Expand All @@ -62,7 +67,7 @@ public override MigrateOrgCommandHandler BuildHandler(MigrateOrgCommandArgs args
var environmentVariableProvider = sp.GetRequiredService<EnvironmentVariableProvider>();

var targetGithubApiFactory = sp.GetRequiredService<ITargetGithubApiFactory>();
var targetGithubApi = targetGithubApiFactory.Create(targetPersonalAccessToken: args.GithubTargetPat);
var targetGithubApi = targetGithubApiFactory.Create(args.TargetApiUrl, args.GithubTargetPat);

return new MigrateOrgCommandHandler(log, targetGithubApi, environmentVariableProvider);
}
Expand Down
1 change: 1 addition & 0 deletions src/gei/Commands/MigrateOrg/MigrateOrgCommandArgs.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ public class MigrateOrgCommandArgs : CommandArgs
public string GithubSourcePat { get; set; }
[Secret]
public string GithubTargetPat { get; set; }
public string TargetApiUrl { get; set; }

public override void Validate(OctoLogger log)
{
Expand Down
Loading