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.

Sign up for GitHub

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
Merged

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

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
bac1a3f
ado migrate-repo changes
brianaj Jan 29, 2024
169c0c8
bbs migrate repo changes
brianaj Jan 29, 2024
ee49ff9
add migrate org coverage
brianaj Jan 29, 2024
a5a98a0
try and fix tests
brianaj Jan 29, 2024
546800b
copy paste error
brianaj Jan 29, 2024
336cf3d
fix the rest of tests
brianaj Jan 29, 2024
29c4bee
add release note
brianaj Jan 29, 2024
54c56e8
revert handler tests
brianaj Jan 29, 2024
dae236b
Update command tests
brianaj Jan 30, 2024
a9441bd
removed dupe test
brianaj Jan 30, 2024
cf1d3d9
Update RELEASENOTES.md
brianaj Jan 30, 2024
833931b
add migrate org test
brianaj Jan 30, 2024
915aead
fix using ordering -__-
brianaj Jan 30, 2024
1bb6bec
Merge branch 'main' into 1094-support-migrate-org
brianaj Jan 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion RELEASENOTES.md
View file
Open in desktop
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.
26 changes: 25 additions & 1 deletion src/OctoshiftCLI.Tests/ado2gh/Commands/MigrateRepo/MigrateRepoCommandTests.cs
View file
Open in desktop
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));
}
}
}
24 changes: 23 additions & 1 deletion src/OctoshiftCLI.Tests/bbs2gh/Commands/MigrateRepo/MigrateRepoCommandTests.cs
View file
Open in desktop
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
3 changes: 2 additions & 1 deletion src/OctoshiftCLI.Tests/gei/Commands/MigrateOrg/MigrateOrgCommandTests.cs
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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