Skip to content

Commit 0056717

Browse files
brianajbrianaj
authored
Merge pull request #1462 from github/brianaj/external-pr-1460
[Copy of 1460] Add path validation and logging for bbs2gh archive operations
2 parents f4e84cd + e5a536a commit 0056717

File tree

4 files changed

+132
-1
lines changed

4 files changed

+132
-1
lines changed

‎RELEASENOTES.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
1+
- bbs2gh : Added validation for `--archive-path` and `--bbs-shared-home` options to fail fast with clear error messages if the provided paths do not exist or are not accessible. Archive path is now logged before upload operations to help with troubleshooting

‎src/Octoshift/Services/FileSystemProvider.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ public class FileSystemProvider
99
{
1010
public virtual bool FileExists(string path) => File.Exists(path);
1111

12+
public virtual bool DirectoryExists(string path) => Directory.Exists(path);
13+
1214
public virtual Task<byte[]> ReadAllBytesAsync(string path) => File.ReadAllBytesAsync(path);
1315

1416
public virtual DirectoryInfo CreateDirectory(string path) => Directory.CreateDirectory(path);

‎src/OctoshiftCLI.Tests/bbs2gh/Commands/MigrateRepo/MigrateRepoCommandHandlerTests.cs‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,10 @@ public MigrateRepoCommandHandlerTests()
7171
_mockFileSystemProvider.Object,
7272
_warningsCountLogger
7373
);
74+
75+
// Default setup for file system operations
76+
_mockFileSystemProvider.Setup(m => m.FileExists(It.IsAny<string>())).Returns(true);
77+
_mockFileSystemProvider.Setup(m => m.DirectoryExists(It.IsAny<string>())).Returns(true);
7478
}
7579

7680
[Fact]
@@ -960,5 +964,116 @@ public async Task Sets_Target_Repo_Visibility()
960964
targetRepoVisibility
961965
));
962966
}
967+
968+
[Fact]
969+
public async Task It_Throws_When_Archive_Path_Does_Not_Exist()
970+
{
971+
const string nonExistentArchivePath = "/path/to/nonexistent/archive.tar";
972+
_mockFileSystemProvider.Setup(m => m.FileExists(nonExistentArchivePath)).Returns(false);
973+
974+
await _handler.Invoking(async x => await x.Handle(new MigrateRepoCommandArgs
975+
{
976+
ArchivePath = nonExistentArchivePath,
977+
GithubOrg = GITHUB_ORG,
978+
GithubRepo = GITHUB_REPO,
979+
AzureStorageConnectionString = AZURE_STORAGE_CONNECTION_STRING
980+
}))
981+
.Should()
982+
.ThrowAsync<OctoshiftCliException>()
983+
.WithMessage($"*--archive-path*{nonExistentArchivePath}*");
984+
}
985+
986+
[Fact]
987+
public async Task It_Throws_When_Bbs_Shared_Home_Does_Not_Exist_When_Running_On_Bitbucket_Instance()
988+
{
989+
const string nonExistentBbsSharedHome = "/nonexistent/shared/home";
990+
_mockBbsApi.Setup(x => x.StartExport(BBS_PROJECT, BBS_REPO)).ReturnsAsync(BBS_EXPORT_ID);
991+
_mockBbsApi.Setup(x => x.GetExport(BBS_EXPORT_ID)).ReturnsAsync(("COMPLETED", "The export is complete", 100));
992+
_mockFileSystemProvider.Setup(m => m.DirectoryExists(nonExistentBbsSharedHome)).Returns(false);
993+
994+
await _handler.Invoking(async x => await x.Handle(new MigrateRepoCommandArgs
995+
{
996+
BbsServerUrl = BBS_SERVER_URL,
997+
BbsUsername = BBS_USERNAME,
998+
BbsPassword = BBS_PASSWORD,
999+
BbsProject = BBS_PROJECT,
1000+
BbsRepo = BBS_REPO,
1001+
BbsSharedHome = nonExistentBbsSharedHome
1002+
}))
1003+
.Should()
1004+
.ThrowAsync<OctoshiftCliException>()
1005+
.WithMessage($"*--bbs-shared-home*{nonExistentBbsSharedHome}*");
1006+
}
1007+
1008+
[Fact]
1009+
public async Task It_Does_Not_Validate_Bbs_Shared_Home_When_Using_Ssh()
1010+
{
1011+
const string nonExistentBbsSharedHome = "/nonexistent/shared/home";
1012+
_mockBbsApi.Setup(x => x.StartExport(BBS_PROJECT, BBS_REPO)).ReturnsAsync(BBS_EXPORT_ID);
1013+
_mockBbsApi.Setup(x => x.GetExport(BBS_EXPORT_ID)).ReturnsAsync(("COMPLETED", "The export is complete", 100));
1014+
_mockBbsArchiveDownloader.Setup(x => x.Download(BBS_EXPORT_ID, It.IsAny<string>())).ReturnsAsync(ARCHIVE_PATH);
1015+
_mockFileSystemProvider.Setup(m => m.DirectoryExists(nonExistentBbsSharedHome)).Returns(false);
1016+
1017+
var args = new MigrateRepoCommandArgs
1018+
{
1019+
BbsServerUrl = BBS_SERVER_URL,
1020+
BbsUsername = BBS_USERNAME,
1021+
BbsPassword = BBS_PASSWORD,
1022+
BbsProject = BBS_PROJECT,
1023+
BbsRepo = BBS_REPO,
1024+
BbsSharedHome = nonExistentBbsSharedHome,
1025+
SshUser = SSH_USER,
1026+
SshPrivateKey = PRIVATE_KEY
1027+
};
1028+
1029+
await _handler.Invoking(x => x.Handle(args))
1030+
.Should()
1031+
.NotThrowAsync();
1032+
}
1033+
1034+
[Fact]
1035+
public async Task It_Does_Not_Validate_Bbs_Shared_Home_When_Using_Smb()
1036+
{
1037+
const string nonExistentBbsSharedHome = "/nonexistent/shared/home";
1038+
_mockBbsApi.Setup(x => x.StartExport(BBS_PROJECT, BBS_REPO)).ReturnsAsync(BBS_EXPORT_ID);
1039+
_mockBbsApi.Setup(x => x.GetExport(BBS_EXPORT_ID)).ReturnsAsync(("COMPLETED", "The export is complete", 100));
1040+
_mockBbsArchiveDownloader.Setup(x => x.Download(BBS_EXPORT_ID, It.IsAny<string>())).ReturnsAsync(ARCHIVE_PATH);
1041+
_mockFileSystemProvider.Setup(m => m.DirectoryExists(nonExistentBbsSharedHome)).Returns(false);
1042+
_mockEnvironmentVariableProvider.Setup(m => m.SmbPassword(It.IsAny<bool>())).Returns(SMB_PASSWORD);
1043+
1044+
var args = new MigrateRepoCommandArgs
1045+
{
1046+
BbsServerUrl = BBS_SERVER_URL,
1047+
BbsUsername = BBS_USERNAME,
1048+
BbsPassword = BBS_PASSWORD,
1049+
BbsProject = BBS_PROJECT,
1050+
BbsRepo = BBS_REPO,
1051+
BbsSharedHome = nonExistentBbsSharedHome,
1052+
SmbUser = SMB_USER
1053+
};
1054+
1055+
await _handler.Invoking(x => x.Handle(args))
1056+
.Should()
1057+
.NotThrowAsync();
1058+
}
1059+
1060+
[Fact]
1061+
public async Task It_Logs_Archive_Path_Before_Upload()
1062+
{
1063+
_mockFileSystemProvider.Setup(m => m.FileExists(ARCHIVE_PATH)).Returns(true);
1064+
_mockAzureApi.Setup(x => x.UploadToBlob(It.IsAny<string>(), It.IsAny<FileStream>())).ReturnsAsync(new Uri(ARCHIVE_URL));
1065+
1066+
var args = new MigrateRepoCommandArgs
1067+
{
1068+
ArchivePath = ARCHIVE_PATH,
1069+
AzureStorageConnectionString = AZURE_STORAGE_CONNECTION_STRING,
1070+
GithubOrg = GITHUB_ORG,
1071+
GithubRepo = GITHUB_REPO,
1072+
QueueOnly = true,
1073+
};
1074+
1075+
await _handler.Handle(args);
1076+
_mockOctoLogger.Verify(m => m.LogInformation($"Archive path: {ARCHIVE_PATH}"), Times.Once);
1077+
}
9631078
}
9641079
}

‎src/bbs2gh/Commands/MigrateRepo/MigrateRepoCommandHandler.cs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,8 @@ public async Task Handle(MigrateRepoCommandArgs args)
9191
args.ArchivePath = GetSourceExportArchiveAbsolutePath(args.BbsSharedHome, exportId);
9292
}
9393

94+
_log.LogInformation($"Archive path: {args.ArchivePath}");
95+
9496
try
9597
{
9698
if (args.UseGithubStorage)
@@ -339,6 +341,18 @@ private void ValidateOptions(MigrateRepoCommandArgs args)
339341
{
340342
throw new OctoshiftCliException("Both --smb-user and --smb-password (or SMB_PASSWORD env. variable) must be specified for SMB download.");
341343
}
344+
345+
// Validate --bbs-shared-home if running on Bitbucket instance (not using SSH/SMB)
346+
if (!args.ShouldDownloadArchive() && args.BbsSharedHome.HasValue() && !_fileSystemProvider.DirectoryExists(args.BbsSharedHome))
347+
{
348+
throw new OctoshiftCliException($"The path provided for --bbs-shared-home does not exist or is not accessible: {args.BbsSharedHome}");
349+
}
350+
}
351+
352+
// Validate --archive-path if provided
353+
if (args.ArchivePath.HasValue() && !_fileSystemProvider.FileExists(args.ArchivePath))
354+
{
355+
throw new OctoshiftCliException($"The archive file provided with --archive-path does not exist or is not accessible: {args.ArchivePath}");
342356
}
343357

344358
if (args.ShouldUploadArchive())

0 commit comments

Comments
 (0)