diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 6da37fb47..f154344c5 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -1 +1,2 @@ -- Added support for configurable multipart upload chunk size for GitHub-owned storage uploads via `GITHUB_OWNED_STORAGE_MULTIPART_MEBIBYTES` environment variable (minimum 5 MiB, default 100 MiB) to improve upload reliability in environments with proxies or slow connections \ No newline at end of file +- Fixed issue where alert migration commands (migrate-code-scanning-alerts, migrate-secret-alerts) required GH_PAT environment variable even when GitHub tokens were provided via command-line arguments (--github-source-pat, --github-target-pat). These commands now work correctly with CLI-only token authentication. +- Added support for configurable multipart upload chunk size for GitHub-owned storage uploads via `GITHUB_OWNED_STORAGE_MULTIPART_MEBIBYTES` environment variable (minimum 5 MiB, default 100 MiB) to improve upload reliability in environments with proxies or slow connections diff --git a/src/OctoshiftCLI.Tests/gei/Commands/MigrateCodeScanningAlerts/MigrateCodeScanningAlertsCommandTests.cs b/src/OctoshiftCLI.Tests/gei/Commands/MigrateCodeScanningAlerts/MigrateCodeScanningAlertsCommandTests.cs index 2ebbb75d2..e4e036733 100644 --- a/src/OctoshiftCLI.Tests/gei/Commands/MigrateCodeScanningAlerts/MigrateCodeScanningAlertsCommandTests.cs +++ b/src/OctoshiftCLI.Tests/gei/Commands/MigrateCodeScanningAlerts/MigrateCodeScanningAlertsCommandTests.cs @@ -49,7 +49,7 @@ public void Should_Have_Options() } [Fact] - public void Source_Pat_Should_Default_To_Target_Pat() + public void Should_Pass_Target_Pat_From_Cli_Args_To_Factory() { var targetToken = "target-token"; @@ -66,4 +66,25 @@ public void Source_Pat_Should_Default_To_Target_Pat() _mockCodeScanningAlertServiceFactory.Verify(m => m.Create(It.IsAny(), targetToken, It.IsAny(), targetToken, It.IsAny())); } + + [Fact] + public void Should_Pass_Source_Pat_From_Cli_Args_To_Factory() + { + var sourceToken = "source-token"; + var targetToken = "target-token"; + + var args = new MigrateCodeScanningAlertsCommandArgs() + { + SourceOrg = "source-org", + SourceRepo = "source-repo", + TargetOrg = "target-org", + TargetRepo = "target-repo", + GithubSourcePat = sourceToken, + GithubTargetPat = targetToken, + }; + + _command.BuildHandler(args, _serviceProvider); + + _mockCodeScanningAlertServiceFactory.Verify(m => m.Create(It.IsAny(), sourceToken, It.IsAny(), targetToken, It.IsAny())); + } } diff --git a/src/OctoshiftCLI.Tests/gei/Commands/MigrateSecretAlerts/MigrateSecretAlertsCommandTests.cs b/src/OctoshiftCLI.Tests/gei/Commands/MigrateSecretAlerts/MigrateSecretAlertsCommandTests.cs index da2f68d12..d1b7914a1 100644 --- a/src/OctoshiftCLI.Tests/gei/Commands/MigrateSecretAlerts/MigrateSecretAlertsCommandTests.cs +++ b/src/OctoshiftCLI.Tests/gei/Commands/MigrateSecretAlerts/MigrateSecretAlertsCommandTests.cs @@ -49,7 +49,7 @@ public void Should_Have_Options() } [Fact] - public void Source_Pat_Should_Default_To_Target_Pat() + public void Should_Pass_Target_Pat_From_Cli_Args_To_Factory() { var targetToken = "target-token"; @@ -66,4 +66,25 @@ public void Source_Pat_Should_Default_To_Target_Pat() _mockSecretScanningAlertServiceFactory.Verify(m => m.Create(It.IsAny(), targetToken, It.IsAny(), targetToken, It.IsAny())); } + + [Fact] + public void Should_Pass_Source_Pat_From_Cli_Args_To_Factory() + { + var sourceToken = "source-token"; + var targetToken = "target-token"; + + var args = new MigrateSecretAlertsCommandArgs() + { + SourceOrg = "source-org", + SourceRepo = "source-repo", + TargetOrg = "target-org", + TargetRepo = "target-repo", + GithubSourcePat = sourceToken, + GithubTargetPat = targetToken, + }; + + _command.BuildHandler(args, _serviceProvider); + + _mockSecretScanningAlertServiceFactory.Verify(m => m.Create(It.IsAny(), sourceToken, It.IsAny(), targetToken, It.IsAny())); + } } diff --git a/src/gei/Commands/MigrateCodeScanningAlerts/MigrateCodeScanningAlertsCommand.cs b/src/gei/Commands/MigrateCodeScanningAlerts/MigrateCodeScanningAlertsCommand.cs index c5019b464..17cd7d455 100644 --- a/src/gei/Commands/MigrateCodeScanningAlerts/MigrateCodeScanningAlertsCommand.cs +++ b/src/gei/Commands/MigrateCodeScanningAlerts/MigrateCodeScanningAlertsCommand.cs @@ -78,10 +78,7 @@ public override MigrateCodeScanningAlertsCommandHandler BuildHandler(MigrateCode throw new ArgumentNullException(nameof(sp)); } - var environmentVariableProvider = sp.GetRequiredService(); - args.GithubSourcePat ??= environmentVariableProvider.SourceGithubPersonalAccessToken(false); - args.GithubTargetPat ??= environmentVariableProvider.TargetGithubPersonalAccessToken(); - + // The factory handles environment variable resolution if (args.GithubSourcePat.IsNullOrWhiteSpace()) { args.GithubSourcePat = args.GithubTargetPat; diff --git a/src/gei/Commands/MigrateSecretAlerts/MigrateSecretAlertsCommand.cs b/src/gei/Commands/MigrateSecretAlerts/MigrateSecretAlertsCommand.cs index 7e2da37ba..c1998ecb9 100644 --- a/src/gei/Commands/MigrateSecretAlerts/MigrateSecretAlertsCommand.cs +++ b/src/gei/Commands/MigrateSecretAlerts/MigrateSecretAlertsCommand.cs @@ -78,10 +78,7 @@ public override MigrateSecretAlertsCommandHandler BuildHandler(MigrateSecretAler throw new ArgumentNullException(nameof(sp)); } - var environmentVariableProvider = sp.GetRequiredService(); - args.GithubSourcePat ??= environmentVariableProvider.SourceGithubPersonalAccessToken(false); - args.GithubTargetPat ??= environmentVariableProvider.TargetGithubPersonalAccessToken(); - + // The factory handles environment variable resolution if (args.GithubSourcePat.IsNullOrWhiteSpace()) { args.GithubSourcePat = args.GithubTargetPat; diff --git a/src/gei/Factories/CodeScanningAlertServiceFactory.cs b/src/gei/Factories/CodeScanningAlertServiceFactory.cs index c07d23563..2535bd4dc 100644 --- a/src/gei/Factories/CodeScanningAlertServiceFactory.cs +++ b/src/gei/Factories/CodeScanningAlertServiceFactory.cs @@ -25,13 +25,27 @@ public CodeScanningAlertServiceFactory( public virtual CodeScanningAlertService Create(string sourceApi, string sourceToken, string targetApi, string targetToken, bool sourceApiNoSsl = false) { - sourceToken ??= _environmentVariableProvider.SourceGithubPersonalAccessToken(); - targetToken ??= _environmentVariableProvider.TargetGithubPersonalAccessToken(); + // Only resolve from environment if tokens are explicitly null + // Use consistent throwIfNotFound=false to prevent exceptions when CLI args are preferred + sourceToken ??= _environmentVariableProvider.SourceGithubPersonalAccessToken(false); + + targetToken ??= _environmentVariableProvider.TargetGithubPersonalAccessToken(false); + + // Validate that we have tokens after all resolution attempts + if (string.IsNullOrWhiteSpace(sourceToken)) + { + throw new OctoshiftCliException("Source GitHub Personal Access Token is required. Provide it via --github-source-pat argument or GH_SOURCE_PAT/GH_PAT environment variable."); + } + + if (string.IsNullOrWhiteSpace(targetToken)) + { + throw new OctoshiftCliException("Target GitHub Personal Access Token is required. Provide it via --github-target-pat argument or GH_PAT environment variable."); + } var sourceGithubApi = sourceApiNoSsl - ? _sourceGithubApiFactory.CreateClientNoSsl(sourceApi, sourceToken) - : _sourceGithubApiFactory.Create(sourceApi, sourceToken); + ? _sourceGithubApiFactory.CreateClientNoSsl(sourceApi, null, sourceToken) + : _sourceGithubApiFactory.Create(sourceApi, null, sourceToken); - return new(sourceGithubApi, _targetGithubApiFactory.Create(targetApi, targetToken), _octoLogger); + return new(sourceGithubApi, _targetGithubApiFactory.Create(targetApi, null, targetToken), _octoLogger); } } diff --git a/src/gei/Factories/SecretScanningAlertServiceFactory.cs b/src/gei/Factories/SecretScanningAlertServiceFactory.cs index 84f74c3a9..2486ab101 100644 --- a/src/gei/Factories/SecretScanningAlertServiceFactory.cs +++ b/src/gei/Factories/SecretScanningAlertServiceFactory.cs @@ -25,13 +25,27 @@ public SecretScanningAlertServiceFactory( public virtual SecretScanningAlertService Create(string sourceApi, string sourceToken, string targetApi, string targetToken, bool sourceApiNoSsl = false) { - sourceToken ??= _environmentVariableProvider.SourceGithubPersonalAccessToken(); - targetToken ??= _environmentVariableProvider.TargetGithubPersonalAccessToken(); + // Only resolve from environment if tokens are explicitly null + // Use consistent throwIfNotFound=false to prevent exceptions when CLI args are preferred + sourceToken ??= _environmentVariableProvider.SourceGithubPersonalAccessToken(false); + + targetToken ??= _environmentVariableProvider.TargetGithubPersonalAccessToken(false); + + // Validate that we have tokens after all resolution attempts + if (string.IsNullOrWhiteSpace(sourceToken)) + { + throw new OctoshiftCliException("Source GitHub Personal Access Token is required. Provide it via --github-source-pat argument or GH_SOURCE_PAT/GH_PAT environment variable."); + } + + if (string.IsNullOrWhiteSpace(targetToken)) + { + throw new OctoshiftCliException("Target GitHub Personal Access Token is required. Provide it via --github-target-pat argument or GH_PAT environment variable."); + } var sourceGithubApi = sourceApiNoSsl - ? _sourceGithubApiFactory.CreateClientNoSsl(sourceApi, sourceToken) - : _sourceGithubApiFactory.Create(sourceApi, sourceToken); + ? _sourceGithubApiFactory.CreateClientNoSsl(sourceApi, null, sourceToken) + : _sourceGithubApiFactory.Create(sourceApi, null, sourceToken); - return new(sourceGithubApi, _targetGithubApiFactory.Create(targetApi, targetToken), _octoLogger); + return new(sourceGithubApi, _targetGithubApiFactory.Create(targetApi, null, targetToken), _octoLogger); } }