diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 70892eba5..734c6b517 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -1,3 +1,4 @@ +- __BREAKING CHANGE:__ Require the AWS region to always be specified with the `--aws-region` argument or `AWS_REGION` environment variable if using AWS S3 for blob storage. Previously, this was optional (with a warning) if you weren't specifying an AWS session token. - __BREAKING CHANGE:__ Drop support for deprecated `AWS_ACCESS_KEY` and `AWS_SECRET_KEY` environment variables in `gh gei` and `gh bbs2gh`. The AWS S3 credentials can now only be configured using the industry-standard `AWS_ACCESS_KEY_ID` and `AWS_SECRET_ACCESS_KEY` variables or command line arguments. - __BREAKING CHANGE__: Require the Bitbucket Server URL, project key and repo to always be provided for `bbs2gh migrate-repo`, even if using the upload-and-migrate (`--archive-path`) or migrate-only (`--archive-url`) flows - Increase timeouts in archive uploads to AWS to prevent timeouts during large uploads diff --git a/src/Octoshift/Services/AwsApi.cs b/src/Octoshift/Services/AwsApi.cs index 31b55f95e..2df028dda 100644 --- a/src/Octoshift/Services/AwsApi.cs +++ b/src/Octoshift/Services/AwsApi.cs @@ -13,10 +13,11 @@ namespace OctoshiftCLI.Services; public class AwsApi : IDisposable { private const int AUTHORIZATION_TIMEOUT_IN_HOURS = 48; - private static readonly RegionEndpoint DefaultRegionEndpoint = RegionEndpoint.USEast1; private readonly ITransferUtility _transferUtility; + public AwsApi(ITransferUtility transferUtility) => _transferUtility = transferUtility; + #pragma warning disable CA2000 public AwsApi(string awsAccessKeyId, string awsSecretAccessKey, string awsRegion = null, string awsSessionToken = null) : this(new TransferUtility(BuildAmazonS3Client(awsAccessKeyId, awsSecretAccessKey, awsRegion, awsSessionToken))) @@ -24,16 +25,10 @@ public AwsApi(string awsAccessKeyId, string awsSecretAccessKey, string awsRegion { } - internal AwsApi(ITransferUtility transferUtility) => _transferUtility = transferUtility; - private static AmazonS3Client BuildAmazonS3Client(string awsAccessKeyId, string awsSecretAccessKey, string awsRegion, string awsSessionToken) { - var regionEndpoint = DefaultRegionEndpoint; - if (awsRegion.HasValue()) - { - regionEndpoint = GetRegionEndpoint(awsRegion); - AWSConfigsS3.UseSignatureVersion4 = true; - } + var regionEndpoint = awsRegion.IsNullOrWhiteSpace() ? null : GetRegionEndpoint(awsRegion); + AWSConfigsS3.UseSignatureVersion4 = true; var creds = awsSessionToken.HasValue() ? (AWSCredentials)new SessionAWSCredentials(awsAccessKeyId, awsSecretAccessKey, awsSessionToken) diff --git a/src/OctoshiftCLI.IntegrationTests/BbsToGithub.cs b/src/OctoshiftCLI.IntegrationTests/BbsToGithub.cs index 8be54cbd7..9935cc2c1 100644 --- a/src/OctoshiftCLI.IntegrationTests/BbsToGithub.cs +++ b/src/OctoshiftCLI.IntegrationTests/BbsToGithub.cs @@ -17,6 +17,7 @@ public sealed class BbsToGithub : IDisposable private const string SSH_KEY_FILE = "ssh_key.pem"; private const string AWS_BUCKET_NAME = "github-dev"; + private const string AWS_REGION = "us-east-1"; private readonly ITestOutputHelper _output; private readonly OctoLogger _logger; @@ -115,7 +116,7 @@ await retryPolicy.Retry(async () => { _tokens.Add("AWS_ACCESS_KEY_ID", Environment.GetEnvironmentVariable("AWS_ACCESS_KEY_ID")); _tokens.Add("AWS_SECRET_ACCESS_KEY", Environment.GetEnvironmentVariable("AWS_SECRET_ACCESS_KEY")); - archiveUploadOptions = $" --aws-bucket-name {AWS_BUCKET_NAME}"; + archiveUploadOptions = $" --aws-bucket-name {AWS_BUCKET_NAME} --aws-region {AWS_REGION}"; } await _targetHelper.RunBbsCliMigration( diff --git a/src/OctoshiftCLI.Tests/bbs2gh/Commands/MigrateRepo/MigrateRepoCommandHandlerTests.cs b/src/OctoshiftCLI.Tests/bbs2gh/Commands/MigrateRepo/MigrateRepoCommandHandlerTests.cs index 3e9c36ac7..bd2757ef8 100644 --- a/src/OctoshiftCLI.Tests/bbs2gh/Commands/MigrateRepo/MigrateRepoCommandHandlerTests.cs +++ b/src/OctoshiftCLI.Tests/bbs2gh/Commands/MigrateRepo/MigrateRepoCommandHandlerTests.cs @@ -34,6 +34,7 @@ public class MigrateRepoCommandHandlerTests private const string AWS_ACCESS_KEY_ID = "aws-access-key-id"; private const string AWS_SECRET_ACCESS_KEY = "aws-secret-access-key"; private const string AWS_SESSION_TOKEN = "aws-session-token"; + private const string AWS_REGION = "eu-west-1"; private const string AZURE_STORAGE_CONNECTION_STRING = "azure-storage-connection-string"; private const string BBS_HOST = "our-bbs-server.com"; @@ -214,6 +215,7 @@ public async Task Happy_Path_Generate_Archive_Ssh_Download_Aws_Upload_And_Ingest AwsBucketName = AWS_BUCKET_NAME, AwsAccessKey = AWS_ACCESS_KEY_ID, AwsSecretKey = AWS_SECRET_ACCESS_KEY, + AwsRegion = AWS_REGION, GithubOrg = GITHUB_ORG, GithubRepo = GITHUB_REPO, GithubPat = GITHUB_PAT, @@ -672,6 +674,7 @@ public async Task Uses_Aws_If_Credentials_Are_Passed() AwsAccessKey = AWS_ACCESS_KEY_ID, AwsSecretKey = AWS_SECRET_ACCESS_KEY, AwsBucketName = AWS_BUCKET_NAME, + AwsRegion = AWS_REGION, QueueOnly = true, }; @@ -755,7 +758,7 @@ await _handler.Invoking(async x => await x.Handle(new MigrateRepoCommandArgs } [Fact] - public async Task It_Throws_When_Aws_Session_Token_Is_Provided_But_Aws_Region_Is_Not() + public async Task It_Throws_When_Aws_Bucket_Name_Is_Provided_But_No_Aws_Region() { await _handler.Invoking(async x => await x.Handle(new MigrateRepoCommandArgs { @@ -769,7 +772,7 @@ await _handler.Invoking(async x => await x.Handle(new MigrateRepoCommandArgs })) .Should() .ThrowAsync() - .WithMessage("*--aws-region*AWS_REGION*--aws-session-token*AWS_SESSION_TOKEN*"); + .WithMessage("Either --aws-region or AWS_REGION environment variable must be set."); } [Fact] diff --git a/src/OctoshiftCLI.Tests/gei/Commands/MigrateRepo/MigrateRepoCommandHandlerTests.cs b/src/OctoshiftCLI.Tests/gei/Commands/MigrateRepo/MigrateRepoCommandHandlerTests.cs index 5b6c322af..050196acd 100644 --- a/src/OctoshiftCLI.Tests/gei/Commands/MigrateRepo/MigrateRepoCommandHandlerTests.cs +++ b/src/OctoshiftCLI.Tests/gei/Commands/MigrateRepo/MigrateRepoCommandHandlerTests.cs @@ -1119,6 +1119,7 @@ public async Task It_Uses_Aws_If_Arguments_Are_Included() var awsAccessKeyId = "awsAccessKeyId"; var awsSecretAccessKey = "awsSecretAccessKey"; var awsBucketName = "awsBucketName"; + var awsRegion = "eu-west-1"; var archiveUrl = $"https://s3.amazonaws.com/{awsBucketName}/archive.tar"; _mockTargetGithubApi.Setup(x => x.GetOrganizationId(TARGET_ORG).Result).Returns(githubOrgId); @@ -1181,6 +1182,7 @@ public async Task It_Uses_Aws_If_Arguments_Are_Included() AwsBucketName = awsBucketName, AwsAccessKey = awsAccessKeyId, AwsSecretKey = awsSecretAccessKey, + AwsRegion = awsRegion, Wait = true }; @@ -1250,7 +1252,7 @@ await _handler.Invoking(async x => await x.Handle(new MigrateRepoCommandArgs } [Fact] - public async Task Ghes_When_Aws_Session_Token_Is_Provided_But_No_Aws_Region_Throws() + public async Task Ghes_When_Aws_Bucket_Name_Is_Provided_But_No_Aws_Region_Throws() { _mockGhesVersionChecker.Setup(m => m.AreBlobCredentialsRequired(GHES_API_URL)).ReturnsAsync(true); @@ -1268,7 +1270,7 @@ await _handler.Invoking(async x => await x.Handle(new MigrateRepoCommandArgs })) .Should() .ThrowAsync() - .WithMessage("*--aws-region*AWS_REGION*--aws-session-token*AWS_SESSION_TOKEN*"); + .WithMessage("Either --aws-region or AWS_REGION environment variable must be set."); } [Fact] diff --git a/src/bbs2gh/Commands/GenerateScript/GenerateScriptCommand.cs b/src/bbs2gh/Commands/GenerateScript/GenerateScriptCommand.cs index 487cbc161..00861b1c6 100644 --- a/src/bbs2gh/Commands/GenerateScript/GenerateScriptCommand.cs +++ b/src/bbs2gh/Commands/GenerateScript/GenerateScriptCommand.cs @@ -106,8 +106,7 @@ public GenerateScriptCommand() : base( public Option AwsRegion { get; } = new( name: "--aws-region", description: "If using AWS, the AWS region. If not provided, it will be read from AWS_REGION environment variable. " + - "Defaults to us-east-1 if neither the argument nor the environment variable is set. " + - "In a future release, you will be required to set an AWS region if using AWS S3 as your blob storage provider."); + "Required if using AWS."); public Option Verbose { get; } = new("--verbose"); diff --git a/src/bbs2gh/Commands/MigrateRepo/MigrateRepoCommand.cs b/src/bbs2gh/Commands/MigrateRepo/MigrateRepoCommand.cs index 4f493429a..162e83de2 100644 --- a/src/bbs2gh/Commands/MigrateRepo/MigrateRepoCommand.cs +++ b/src/bbs2gh/Commands/MigrateRepo/MigrateRepoCommand.cs @@ -116,8 +116,7 @@ public MigrateRepoCommand() : base( public Option AwsRegion { get; } = new( name: "--aws-region", description: "If using AWS, the AWS region. If not provided, it will be read from AWS_REGION environment variable. " + - "Defaults to us-east-1 if neither the argument nor the environment variable is set. " + - "In a future release, you will be required to set an AWS region if using AWS S3 as your blob storage provider."); + "Required if using AWS."); public Option GithubOrg { get; } = new("--github-org"); diff --git a/src/bbs2gh/Commands/MigrateRepo/MigrateRepoCommandHandler.cs b/src/bbs2gh/Commands/MigrateRepo/MigrateRepoCommandHandler.cs index b8e02cbac..0db133ba5 100644 --- a/src/bbs2gh/Commands/MigrateRepo/MigrateRepoCommandHandler.cs +++ b/src/bbs2gh/Commands/MigrateRepo/MigrateRepoCommandHandler.cs @@ -244,9 +244,6 @@ private async Task ImportArchive(MigrateRepoCommandArgs args, string archiveUrl private string GetAwsRegion(MigrateRepoCommandArgs args) => args.AwsRegion.HasValue() ? args.AwsRegion : _environmentVariableProvider.AwsRegion(false); - private string GetAwsSessionToken(MigrateRepoCommandArgs args) => - args.AwsSessionToken.HasValue() ? args.AwsSessionToken : _environmentVariableProvider.AwsSessionToken(false); - private string GetAzureStorageConnectionString(MigrateRepoCommandArgs args) => args.AzureStorageConnectionString.HasValue() ? args.AzureStorageConnectionString : _environmentVariableProvider.AzureStorageConnectionString(false); @@ -325,16 +322,9 @@ private void ValidateUploadOptions(MigrateRepoCommandArgs args) throw new OctoshiftCliException("Either --aws-secret-key or AWS_SECRET_ACCESS_KEY environment variable must be set."); } - if (GetAwsSessionToken(args).HasValue() && GetAwsRegion(args).IsNullOrWhiteSpace()) - { - throw new OctoshiftCliException( - "--aws-region or AWS_REGION environment variable must be provided with --aws-session-token or AWS_SESSION_TOKEN environment variable."); - } - - if (!GetAwsRegion(args).HasValue()) + if (GetAwsRegion(args).IsNullOrWhiteSpace()) { - _log.LogWarning("Specifying an AWS region with the --aws-region argument or AWS_REGION environment variable is currently not required, " + - "but will be required in a future release. Defaulting to us-east-1."); + throw new OctoshiftCliException("Either --aws-region or AWS_REGION environment variable must be set."); } } } diff --git a/src/bbs2gh/Factories/AwsApiFactory.cs b/src/bbs2gh/Factories/AwsApiFactory.cs index 058b67536..c657d83de 100644 --- a/src/bbs2gh/Factories/AwsApiFactory.cs +++ b/src/bbs2gh/Factories/AwsApiFactory.cs @@ -16,7 +16,7 @@ public virtual AwsApi Create(string awsRegion = null, string awsAccessKeyId = nu awsAccessKeyId ??= _environmentVariableProvider.AwsAccessKeyId(); awsSecretAccessKey ??= _environmentVariableProvider.AwsSecretAccessKey(); awsSessionToken ??= _environmentVariableProvider.AwsSessionToken(false); - awsRegion ??= _environmentVariableProvider.AwsRegion(false); + awsRegion ??= _environmentVariableProvider.AwsRegion(); return new AwsApi(awsAccessKeyId, awsSecretAccessKey, awsRegion, awsSessionToken); } diff --git a/src/gei/Commands/GenerateScript/GenerateScriptCommand.cs b/src/gei/Commands/GenerateScript/GenerateScriptCommand.cs index 8468af18f..265bf622a 100644 --- a/src/gei/Commands/GenerateScript/GenerateScriptCommand.cs +++ b/src/gei/Commands/GenerateScript/GenerateScriptCommand.cs @@ -85,8 +85,7 @@ public GenerateScriptCommand() : base( public Option AwsRegion { get; } = new("--aws-region") { Description = "If using AWS, the AWS region. If not provided, it will be read from AWS_REGION environment variable. " + - "Defaults to us-east-1 if neither the argument nor the environment variable is set. " + - "In a future release, you will be required to set an AWS region if using AWS S3 as your blob storage provider." + "Required if using AWS." }; public Option Verbose { get; } = new("--verbose"); diff --git a/src/gei/Commands/MigrateRepo/MigrateRepoCommand.cs b/src/gei/Commands/MigrateRepo/MigrateRepoCommand.cs index 235f2dbaa..eb1f1572f 100644 --- a/src/gei/Commands/MigrateRepo/MigrateRepoCommand.cs +++ b/src/gei/Commands/MigrateRepo/MigrateRepoCommand.cs @@ -97,8 +97,7 @@ public MigrateRepoCommand() : base( public Option AwsRegion { get; } = new("--aws-region") { Description = "If using AWS, the AWS region. If not provided, it will be read from AWS_REGION environment variable. " + - "Defaults to us-east-1 if neither the argument nor the environment variable is set. " + - "In a future release, you will be required to set an AWS region if using AWS S3 as your blob storage provider." + "Required if using AWS." }; public Option NoSslVerify { get; } = new("--no-ssl-verify") { diff --git a/src/gei/Commands/MigrateRepo/MigrateRepoCommandHandler.cs b/src/gei/Commands/MigrateRepo/MigrateRepoCommandHandler.cs index d572630ac..9f209e186 100644 --- a/src/gei/Commands/MigrateRepo/MigrateRepoCommandHandler.cs +++ b/src/gei/Commands/MigrateRepo/MigrateRepoCommandHandler.cs @@ -373,16 +373,9 @@ private void ValidateGHESOptions(MigrateRepoCommandArgs args, bool cloudCredenti throw new OctoshiftCliException("Either --aws-secret-key or AWS_SECRET_ACCESS_KEY environment variable must be set."); } - if (GetAwsSessionToken(args).HasValue() && GetAwsRegion(args).IsNullOrWhiteSpace()) + if (GetAwsRegion(args).IsNullOrWhiteSpace()) { - throw new OctoshiftCliException( - "--aws-region or AWS_REGION environment variable must be provided with --aws-session-token or AWS_SESSION_TOKEN environment variable."); - } - - if (!GetAwsRegion(args).HasValue()) - { - _log.LogWarning("Specifying an AWS region with the --aws-region argument or AWS_REGION environment variable is currently not required, " + - "but will be required in a future release. Defaulting to us-east-1."); + throw new OctoshiftCliException("Either --aws-region or AWS_REGION environment variable must be set."); } } else if (new[] { args.AwsAccessKey, args.AwsSecretKey, args.AwsSessionToken, args.AwsRegion }.Any(x => x.HasValue())) @@ -397,9 +390,6 @@ private void ValidateGHESOptions(MigrateRepoCommandArgs args, bool cloudCredenti private string GetAwsRegion(MigrateRepoCommandArgs args) => args.AwsRegion.HasValue() ? args.AwsRegion : _environmentVariableProvider.AwsRegion(false); - private string GetAwsSessionToken(MigrateRepoCommandArgs args) => - args.AwsSessionToken.HasValue() ? args.AwsSessionToken : _environmentVariableProvider.AwsSessionToken(false); - private string GetAzureStorageConnectionString(MigrateRepoCommandArgs args) => args.AzureStorageConnectionString.HasValue() ? args.AzureStorageConnectionString : _environmentVariableProvider.AzureStorageConnectionString(false); diff --git a/src/gei/Factories/AwsApiFactory.cs b/src/gei/Factories/AwsApiFactory.cs index 356659fe9..cb71759d2 100644 --- a/src/gei/Factories/AwsApiFactory.cs +++ b/src/gei/Factories/AwsApiFactory.cs @@ -16,7 +16,7 @@ public virtual AwsApi Create(string awsRegion = null, string awsAccessKeyId = nu awsAccessKeyId ??= _environmentVariableProvider.AwsAccessKeyId(); awsSecretAccessKey ??= _environmentVariableProvider.AwsSecretAccessKey(); awsSessionToken ??= _environmentVariableProvider.AwsSessionToken(false); - awsRegion ??= _environmentVariableProvider.AwsRegion(false); + awsRegion ??= _environmentVariableProvider.AwsRegion(); return new AwsApi(awsAccessKeyId, awsSecretAccessKey, awsRegion, awsSessionToken); }