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 `ado2gh migrate repo`, `bbs2gh migrate repo`, and `gei migrate org` to support newer GitHub migration paths.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ public class MigrateRepoCommandHandlerTests
private const string ADO_REPO = "foo-repo";
private const string GITHUB_ORG = "foo-gh-org";
private const string GITHUB_REPO = "gh-repo";
private const string TARGET_API_URL = "https://api.github.com";
Copy link
Collaborator

Choose a reason for hiding this comment

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

I will add a comment just for this line but it applies to all tests for all CLIs.

There is no need to add the target api url here because it's only going to be used inside the Command (not the handler) to create an instance of GithubApi. Instead a new and very simple test needs to be added to the Command tests (e.g. MigrateOrgCommandTests.cs) to make sure if the --target-api-url is provided, the factory is going to receive it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The new tests are going to be similar to this one

private readonly string ADO_REPO_URL = $"https://dev.azure.com/{ADO_ORG}/{ADO_TEAM_PROJECT}/_git/{ADO_REPO}";
private readonly string ADO_TOKEN = Guid.NewGuid().ToString();
private readonly string GITHUB_ORG_ID = Guid.NewGuid().ToString();
Expand Down Expand Up @@ -83,6 +84,7 @@ public async Task Happy_Path()
AdoRepo = ADO_REPO,
GithubOrg = GITHUB_ORG,
GithubRepo = GITHUB_REPO,
TargetApiUrl = TARGET_API_URL,
QueueOnly = true,
};
await _handler.Handle(args);
Expand Down Expand Up @@ -115,6 +117,7 @@ public async Task Ado_Server_Migration()
AdoRepo = ADO_REPO,
GithubOrg = GITHUB_ORG,
GithubRepo = GITHUB_REPO,
TargetApiUrl = TARGET_API_URL,
QueueOnly = true,
AdoServerUrl = ADO_SERVER_URL,
GithubPat = GITHUB_TOKEN,
Expand Down Expand Up @@ -168,6 +171,7 @@ public async Task Skip_Migration_If_Target_Repo_Exists()
AdoRepo = ADO_REPO,
GithubOrg = GITHUB_ORG,
GithubRepo = GITHUB_REPO,
TargetApiUrl = TARGET_API_URL,
QueueOnly = true,
};
await _handler.Handle(args);
Expand Down Expand Up @@ -212,6 +216,7 @@ public async Task Happy_Path_With_Wait()
AdoRepo = ADO_REPO,
GithubOrg = GITHUB_ORG,
GithubRepo = GITHUB_REPO,
TargetApiUrl = TARGET_API_URL,
};
await _handler.Handle(args);

Expand Down Expand Up @@ -242,6 +247,7 @@ await _handler.Invoking(async x => await x.Handle(new MigrateRepoCommandArgs
AdoRepo = ADO_REPO,
GithubOrg = GITHUB_ORG,
GithubRepo = GITHUB_REPO,
TargetApiUrl = TARGET_API_URL,
}))
.Should()
.ThrowAsync<OctoshiftCliException>()
Expand All @@ -268,6 +274,7 @@ public async Task It_Falls_Back_To_Ado_And_Github_Pats_From_Environment_When_Not
AdoRepo = ADO_REPO,
GithubOrg = GITHUB_ORG,
GithubRepo = GITHUB_REPO,
TargetApiUrl = TARGET_API_URL,
};

await _handler.Handle(args);
Expand All @@ -291,6 +298,7 @@ public async Task Sets_Target_Repo_Visibility_When_Specified()
AdoRepo = ADO_REPO,
GithubOrg = GITHUB_ORG,
GithubRepo = GITHUB_REPO,
TargetApiUrl = TARGET_API_URL,
QueueOnly = true,
TargetRepoVisibility = targetRepoVisibility,
};
Expand Down
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 Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ public class MigrateRepoCommandHandlerTests
private const string GITHUB_ORG = "target-org";
private const string GITHUB_REPO = "target-repo";
private const string GITHUB_PAT = "github pat";
private const string TARGET_API_URL = "https://api.github.com";
private const string AWS_BUCKET_NAME = "aws-bucket-name";
private const string AWS_ACCESS_KEY_ID = "aws-access-key-id";
private const string AWS_SECRET_ACCESS_KEY = "aws-secret-access-key";
Expand Down Expand Up @@ -182,6 +183,7 @@ public async Task Happy_Path_Generate_Archive_Ssh_Download_Azure_Upload_And_Inge
GithubOrg = GITHUB_ORG,
GithubRepo = GITHUB_REPO,
GithubPat = GITHUB_PAT,
TargetApiUrl = TARGET_API_URL,
QueueOnly = true,
};
await _handler.Handle(args);
Expand Down Expand Up @@ -227,6 +229,7 @@ public async Task Happy_Path_Generate_Archive_Ssh_Download_Aws_Upload_And_Ingest
GithubOrg = GITHUB_ORG,
GithubRepo = GITHUB_REPO,
GithubPat = GITHUB_PAT,
TargetApiUrl = TARGET_API_URL,
QueueOnly = true,
};
await _handler.Handle(args);
Expand Down Expand Up @@ -269,6 +272,7 @@ public async Task Happy_Path_Full_Flow_Running_On_Bbs_Server()
GithubOrg = GITHUB_ORG,
GithubRepo = GITHUB_REPO,
GithubPat = GITHUB_PAT,
TargetApiUrl = TARGET_API_URL,
BbsSharedHome = bbsSharedHome,
QueueOnly = true,
};
Expand Down Expand Up @@ -327,6 +331,7 @@ public async Task Happy_Path_Full_Flow_Bbs_Credentials_Via_Environment()
GithubOrg = GITHUB_ORG,
GithubRepo = GITHUB_REPO,
GithubPat = GITHUB_PAT,
TargetApiUrl = TARGET_API_URL,
QueueOnly = true,
};
await _handler.Handle(args);
Expand Down Expand Up @@ -367,6 +372,7 @@ public async Task Happy_Path_Deletes_Downloaded_Archive()
GithubOrg = GITHUB_ORG,
GithubRepo = GITHUB_REPO,
GithubPat = GITHUB_PAT,
TargetApiUrl = TARGET_API_URL,
QueueOnly = true,
};
await _handler.Handle(args);
Expand Down Expand Up @@ -398,7 +404,8 @@ public async Task It_Deletes_Downloaded_Archive_Even_If_Upload_Fails()
AzureStorageConnectionString = AZURE_STORAGE_CONNECTION_STRING,
GithubOrg = GITHUB_ORG,
GithubRepo = GITHUB_REPO,
GithubPat = GITHUB_PAT
GithubPat = GITHUB_PAT,
TargetApiUrl = TARGET_API_URL,
};
await _handler.Invoking(async x => await x.Handle(args)).Should().ThrowExactlyAsync<InvalidOperationException>();

Expand Down Expand Up @@ -431,6 +438,7 @@ public async Task Happy_Path_Does_Not_Throw_If_Fails_To_Delete_Downloaded_Archiv
GithubOrg = GITHUB_ORG,
GithubRepo = GITHUB_REPO,
GithubPat = GITHUB_PAT,
TargetApiUrl = TARGET_API_URL,
QueueOnly = true,
};
await _handler.Handle(args);
Expand Down Expand Up @@ -459,6 +467,7 @@ public async Task Dont_Generate_Archive_If_Target_Repo_Exists()
GithubOrg = GITHUB_ORG,
GithubRepo = GITHUB_REPO,
GithubPat = GITHUB_PAT,
TargetApiUrl = TARGET_API_URL,
QueueOnly = true,
};

Expand Down Expand Up @@ -488,6 +497,7 @@ public async Task Uses_GitHub_Pat_When_Provided_As_Option()
GithubOrg = GITHUB_ORG,
GithubRepo = GITHUB_REPO,
GithubPat = githubPat,
TargetApiUrl = TARGET_API_URL,
QueueOnly = true,
};
await _handler.Handle(args);
Expand Down Expand Up @@ -522,6 +532,7 @@ public async Task Skip_Migration_If_Target_Repo_Exists()
ArchiveUrl = ARCHIVE_URL,
GithubOrg = GITHUB_ORG,
GithubRepo = GITHUB_REPO,
TargetApiUrl = TARGET_API_URL,
QueueOnly = true,
};
await _handler.Handle(args);
Expand All @@ -547,6 +558,7 @@ await _handler.Invoking(async x => await x.Handle(new MigrateRepoCommandArgs
ArchiveUrl = ARCHIVE_URL,
GithubOrg = GITHUB_ORG,
GithubRepo = GITHUB_REPO,
TargetApiUrl = TARGET_API_URL,
QueueOnly = true,
}))
.Should()
Expand Down Expand Up @@ -598,6 +610,7 @@ public async Task Uses_Archive_Path_If_Provided()
ArchivePath = ARCHIVE_PATH,
GithubOrg = GITHUB_ORG,
GithubRepo = GITHUB_REPO,
TargetApiUrl = TARGET_API_URL,
AzureStorageConnectionString = AZURE_STORAGE_CONNECTION_STRING,
QueueOnly = true,
};
Expand Down Expand Up @@ -667,6 +680,7 @@ public async Task It_Does_Not_Set_The_Archive_Path_When_Archive_Path_Is_Provided
AzureStorageConnectionString = AZURE_STORAGE_CONNECTION_STRING,
GithubOrg = GITHUB_ORG,
GithubRepo = GITHUB_REPO,
TargetApiUrl = TARGET_API_URL,
QueueOnly = true,
};
await _handler.Handle(args);
Expand All @@ -686,6 +700,7 @@ public async Task It_Does_Not_Set_The_Archive_Path_When_Archive_Url_Is_Provided(
AzureStorageConnectionString = AZURE_STORAGE_CONNECTION_STRING,
GithubOrg = GITHUB_ORG,
GithubRepo = GITHUB_REPO,
TargetApiUrl = TARGET_API_URL,
QueueOnly = true,
};
await _handler.Handle(args);
Expand Down
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
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ public class MigrateOrgCommandHandlerTests
private const string TARGET_ORG = "foo-target-org";
private const string SOURCE_PAT = "foo-source-pat";
private const string TARGET_PAT = "foo-target-pat";
private const string TARGET_API_URL = "https://api.github.com";

public MigrateOrgCommandHandlerTests()
{
Expand Down Expand Up @@ -119,6 +120,7 @@ public async Task Happy_Path_PAT_In_Env()
GithubSourceOrg = SOURCE_ORG,
GithubTargetOrg = TARGET_ORG,
GithubTargetEnterprise = TARGET_ENTERPRISE,
TargetApiUrl = TARGET_API_URL,
};
await _handler.Handle(args);

Expand Down Expand Up @@ -155,6 +157,7 @@ public async Task Queue_Only()
GithubTargetOrg = TARGET_ORG,
GithubTargetEnterprise = TARGET_ENTERPRISE,
GithubTargetPat = TARGET_PAT,
TargetApiUrl = TARGET_API_URL,
QueueOnly = true,
};
await _handler.Handle(args);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ 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 +21,7 @@ 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.

}
}
}
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