From f43404fab4540f51e061b8cc78ef93c9d2ba32f5 Mon Sep 17 00:00:00 2001 From: Tim Rogers Date: Tue, 4 Jul 2023 17:59:00 -0700 Subject: [PATCH 1/6] Drop support for non-standard, deprecated `AWS_ACCESS_KEY` and `AWS_SECRET_KEY` environment variables Up until #855, we used non-standard environment variables for configuring AWS S3 credentials. With #855, we *added* support for the standard `AWS_ACCESS_KEY_ID` and `AWS_SECRET_ACCESS_KEY` variables, but maintained support for the old names, printing a warning when they're used. We've had that behaviour for 4 months now, and it's time to cut the old behaviour. With this change, only the new environment variable names will be accepted, and trying to use the old names will result in your argument not being recognised. Closes #856. --- .../Services/EnvironmentVariableProvider.cs | 8 - .../MigrateRepoCommandHandlerTests.cs | 74 -------- .../bbs2gh/Factories/AwsApiFactoryTests.cs | 41 ++-- .../MigrateRepoCommandHandlerTests.cs | 178 ------------------ .../gei/Factories/AwsApiFactoryTests.cs | 41 ++-- .../MigrateRepo/MigrateRepoCommandHandler.cs | 22 +-- src/bbs2gh/Factories/AwsApiFactory.cs | 10 +- .../MigrateRepo/MigrateRepoCommandHandler.cs | 22 +-- src/gei/Factories/AwsApiFactory.cs | 10 +- 9 files changed, 30 insertions(+), 376 deletions(-) diff --git a/src/Octoshift/Services/EnvironmentVariableProvider.cs b/src/Octoshift/Services/EnvironmentVariableProvider.cs index 8f22d50ac..07fb0561a 100644 --- a/src/Octoshift/Services/EnvironmentVariableProvider.cs +++ b/src/Octoshift/Services/EnvironmentVariableProvider.cs @@ -43,14 +43,6 @@ public virtual string AwsSecretAccessKey(bool throwIfNotFound = true) => public virtual string AwsAccessKeyId(bool throwIfNotFound = true) => GetSecret(AWS_ACCESS_KEY_ID, throwIfNotFound); - [Obsolete("AwsSecretKey is deprecated, please use AwsSecretAccessKey instead.")] - public virtual string AwsSecretKey(bool throwIfNotFound = true) => - GetSecret(AWS_SECRET_KEY, throwIfNotFound); - - [Obsolete("AwsAccessKey is deprecated, please use AwsAccessKeyId instead.")] - public virtual string AwsAccessKey(bool throwIfNotFound = true) => - GetSecret(AWS_ACCESS_KEY, throwIfNotFound); - public virtual string AwsSessionToken(bool throwIfNotFound = true) => GetSecret(AWS_SESSION_TOKEN, throwIfNotFound); diff --git a/src/OctoshiftCLI.Tests/bbs2gh/Commands/MigrateRepo/MigrateRepoCommandHandlerTests.cs b/src/OctoshiftCLI.Tests/bbs2gh/Commands/MigrateRepo/MigrateRepoCommandHandlerTests.cs index 26555c5d0..3e9c36ac7 100644 --- a/src/OctoshiftCLI.Tests/bbs2gh/Commands/MigrateRepo/MigrateRepoCommandHandlerTests.cs +++ b/src/OctoshiftCLI.Tests/bbs2gh/Commands/MigrateRepo/MigrateRepoCommandHandlerTests.cs @@ -33,8 +33,6 @@ public class MigrateRepoCommandHandlerTests 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"; - private const string AWS_ACCESS_KEY = "aws-access-key"; - private const string AWS_SECRET_KEY = "aws-secret-key"; private const string AWS_SESSION_TOKEN = "aws-session-token"; private const string AZURE_STORAGE_CONNECTION_STRING = "azure-storage-connection-string"; @@ -740,42 +738,6 @@ await _handler.Invoking(async x => await x.Handle(new MigrateRepoCommandArgs .WithMessage("*--aws-access-key*AWS_ACCESS_KEY_ID*"); } - [Fact] - public async Task It_Does_Not_Throw_When_Aws_Bucket_Name_Is_Provided_And_Can_Fallback_To_Aws_Access_Key_Environment_Variable() - { - // Arrange - _mockBbsApi.Setup(x => x.StartExport(BBS_PROJECT, BBS_REPO)).ReturnsAsync(BBS_EXPORT_ID); - _mockBbsApi.Setup(x => x.GetExport(BBS_EXPORT_ID)).ReturnsAsync(("COMPLETED", "The export is complete", 100)); - _mockBbsArchiveDownloader.Setup(x => x.Download(BBS_EXPORT_ID, It.IsAny())).ReturnsAsync(ARCHIVE_PATH); - _mockAwsApi.Setup(x => x.UploadToBucket(AWS_BUCKET_NAME, ARCHIVE_PATH, It.IsAny())).ReturnsAsync(ARCHIVE_URL); - _mockGithubApi.Setup(x => x.GetOrganizationId(GITHUB_ORG).Result).Returns(GITHUB_ORG_ID); - _mockGithubApi.Setup(x => x.CreateBbsMigrationSource(GITHUB_ORG_ID).Result).Returns(MIGRATION_SOURCE_ID); -#pragma warning disable CS0618 - _mockEnvironmentVariableProvider.Setup(x => x.AwsAccessKey(false)).Returns(AWS_ACCESS_KEY); -#pragma warning restore CS0618 - - // Act, Assert - var args = new MigrateRepoCommandArgs - { - BbsServerUrl = BBS_SERVER_URL, - BbsUsername = BBS_USERNAME, - BbsPassword = BBS_PASSWORD, - BbsProject = BBS_PROJECT, - BbsRepo = BBS_REPO, - SshUser = SSH_USER, - SshPrivateKey = PRIVATE_KEY, - AwsBucketName = AWS_BUCKET_NAME, - AwsSecretKey = AWS_SECRET_ACCESS_KEY, - GithubOrg = GITHUB_ORG, - GithubRepo = GITHUB_REPO, - GithubPat = GITHUB_PAT, - QueueOnly = true, - }; - await _handler.Invoking(async x => await x.Handle(args)).Should().NotThrowAsync(); - - _mockOctoLogger.Verify(m => m.LogWarning(It.Is(msg => msg.Contains("AWS_ACCESS_KEY")))); - } - [Fact] public async Task It_Throws_When_Aws_Bucket_Name_Is_Provided_But_No_Aws_Secret_Access_Key() { @@ -792,42 +754,6 @@ await _handler.Invoking(async x => await x.Handle(new MigrateRepoCommandArgs .WithMessage("*--aws-secret-key*AWS_SECRET_ACCESS_KEY*"); } - [Fact] - public async Task It_Does_Not_Throw_When_Aws_Bucket_Name_Is_Provided_And_Can_Fallback_To_Aws_Secret_Key_Environment_Variable() - { - // Arrange - _mockBbsApi.Setup(x => x.StartExport(BBS_PROJECT, BBS_REPO)).ReturnsAsync(BBS_EXPORT_ID); - _mockBbsApi.Setup(x => x.GetExport(BBS_EXPORT_ID)).ReturnsAsync(("COMPLETED", "The export is complete", 100)); - _mockBbsArchiveDownloader.Setup(x => x.Download(BBS_EXPORT_ID, It.IsAny())).ReturnsAsync(ARCHIVE_PATH); - _mockAwsApi.Setup(x => x.UploadToBucket(AWS_BUCKET_NAME, ARCHIVE_PATH, It.IsAny())).ReturnsAsync(ARCHIVE_URL); - _mockGithubApi.Setup(x => x.GetOrganizationId(GITHUB_ORG).Result).Returns(GITHUB_ORG_ID); - _mockGithubApi.Setup(x => x.CreateBbsMigrationSource(GITHUB_ORG_ID).Result).Returns(MIGRATION_SOURCE_ID); -#pragma warning disable CS0618 - _mockEnvironmentVariableProvider.Setup(x => x.AwsSecretKey(false)).Returns(AWS_SECRET_KEY); -#pragma warning restore CS0618 - - // Act, Assert - var args = new MigrateRepoCommandArgs - { - BbsServerUrl = BBS_SERVER_URL, - BbsUsername = BBS_USERNAME, - BbsPassword = BBS_PASSWORD, - BbsProject = BBS_PROJECT, - BbsRepo = BBS_REPO, - SshUser = SSH_USER, - SshPrivateKey = PRIVATE_KEY, - AwsBucketName = AWS_BUCKET_NAME, - AwsAccessKey = AWS_ACCESS_KEY_ID, - GithubOrg = GITHUB_ORG, - GithubRepo = GITHUB_REPO, - GithubPat = GITHUB_PAT, - QueueOnly = true, - }; - await _handler.Invoking(async x => await x.Handle(args)).Should().NotThrowAsync(); - - _mockOctoLogger.Verify(m => m.LogWarning(It.Is(msg => msg.Contains("AWS_SECRET_KEY")))); - } - [Fact] public async Task It_Throws_When_Aws_Session_Token_Is_Provided_But_Aws_Region_Is_Not() { diff --git a/src/OctoshiftCLI.Tests/bbs2gh/Factories/AwsApiFactoryTests.cs b/src/OctoshiftCLI.Tests/bbs2gh/Factories/AwsApiFactoryTests.cs index 20584dbf2..8a5c4a100 100644 --- a/src/OctoshiftCLI.Tests/bbs2gh/Factories/AwsApiFactoryTests.cs +++ b/src/OctoshiftCLI.Tests/bbs2gh/Factories/AwsApiFactoryTests.cs @@ -1,3 +1,4 @@ +using FluentAssertions; using Moq; using OctoshiftCLI.BbsToGithub.Factories; using OctoshiftCLI.Services; @@ -16,40 +17,20 @@ public AwsApiFactoryTests() } [Fact] - public void It_Falls_Back_To_Aws_Access_Key_Environment_Variable_If_Aws_Access_Key_Id_Is_Not_Set() + public void It_Errors_If_Aws_Access_Key_Id_Is_Not_Provided_Or_Set_In_Environment_Variable() { - // Arrange - const string awsAccessKey = "AWS_ACCESS_KEY"; - const string awsRegion = "us-east-2"; -#pragma warning disable CS0618 - _mockEnvironmentVariableProvider.Setup(m => m.AwsAccessKey(false)).Returns(awsAccessKey); -#pragma warning restore CS0618 - - // Act - _awsApiFactory.Create(awsRegion, null, "aws-secret-access-key", "aws-session-token"); - - // Assert -#pragma warning disable CS0618 - _mockEnvironmentVariableProvider.Verify(m => m.AwsAccessKey(false), Times.Once); -#pragma warning restore CS0618 + // Act, Assert + _awsApiFactory.Invoking(x => x.Create("us-east-2", null, "aws-secret-access-key", "aws-session-token")) + .Should() + .ThrowExactly(); } [Fact] - public void It_Falls_Back_To_Aws_Secret_Key_Environment_Variable_If_Aws_Secret_Access_Key_Is_Not_Set() + public void It_Errors_If_Aws_Secret_Access_Key_Is_Not_Set_Or_Set_In_Environment_Variable() { - // Arrange - const string awsSecretKey = "AWS_SECRET_KEY"; - const string awsRegion = "us-east-2"; -#pragma warning disable CS0618 - _mockEnvironmentVariableProvider.Setup(m => m.AwsSecretKey(false)).Returns(awsSecretKey); -#pragma warning restore CS0618 - - // Act - _awsApiFactory.Create(awsRegion, "aws-access-key-id", null, "aws-session-token"); - - // Assert -#pragma warning disable CS0618 - _mockEnvironmentVariableProvider.Verify(m => m.AwsSecretKey(false), Times.Once); -#pragma warning restore CS0618 + // Act, Assert + _awsApiFactory.Invoking(x => x.Create("us-east-2", "aws-access-key-id", null, "aws-session-token")) + .Should() + .ThrowExactly(); } } diff --git a/src/OctoshiftCLI.Tests/gei/Commands/MigrateRepo/MigrateRepoCommandHandlerTests.cs b/src/OctoshiftCLI.Tests/gei/Commands/MigrateRepo/MigrateRepoCommandHandlerTests.cs index 9cfe3d5ee..5b6c322af 100644 --- a/src/OctoshiftCLI.Tests/gei/Commands/MigrateRepo/MigrateRepoCommandHandlerTests.cs +++ b/src/OctoshiftCLI.Tests/gei/Commands/MigrateRepo/MigrateRepoCommandHandlerTests.cs @@ -38,9 +38,7 @@ public class MigrateRepoCommandHandlerTests private const string GITHUB_SOURCE_PAT = "github-source-pat"; private const string AWS_BUCKET_NAME = "aws-bucket-name"; private const string AWS_ACCESS_KEY_ID = "aws-access-key-id"; - private const string AWS_ACCESS_KEY = "AWS_ACCESS_KEY"; private const string AWS_SECRET_ACCESS_KEY = "aws-secret-access-key"; - private const string AWS_SECRET_KEY = "AWS_SECRET_KEY"; private const string AWS_SESSION_TOKEN = "aws-session-token"; private const string AWS_REGION = "aws-region"; @@ -1231,94 +1229,6 @@ await _handler.Invoking(async x => await x.Handle(new MigrateRepoCommandArgs .WithMessage("*--aws-access-key*"); } - [Fact] - public async Task Ghes_When_Aws_Bucket_Name_Is_Provided_And_Can_Fallback_To_Aws_Access_Key_Environment_Variable_Does_Not_Throw() - { - var githubOrgId = Guid.NewGuid().ToString(); - var migrationSourceId = Guid.NewGuid().ToString(); - var sourceGithubPat = Guid.NewGuid().ToString(); - var targetGithubPat = Guid.NewGuid().ToString(); - var githubRepoUrl = $"https://myghes/{SOURCE_ORG}/{SOURCE_REPO}"; - var migrationId = Guid.NewGuid().ToString(); - var gitArchiveId = 1; - var metadataArchiveId = 2; - - var gitArchiveUrl = $"https://example.com/{gitArchiveId}"; - var metadataArchiveUrl = $"https://example.com/{metadataArchiveId}"; - var gitArchiveContent = new byte[] { 1, 2, 3, 4, 5 }; - var metadataArchiveContent = new byte[] { 6, 7, 8, 9, 10 }; - - var archiveUrl = $"https://s3.amazonaws.com/{AWS_BUCKET_NAME}/archive.tar"; - - _mockTargetGithubApi.Setup(x => x.GetOrganizationId(TARGET_ORG).Result).Returns(githubOrgId); - _mockTargetGithubApi.Setup(x => x.CreateGhecMigrationSource(githubOrgId).Result).Returns(migrationSourceId); - _mockTargetGithubApi - .Setup(x => x.StartMigration( - migrationSourceId, - githubRepoUrl, - githubOrgId, - TARGET_REPO, - sourceGithubPat, - targetGithubPat, - archiveUrl, - archiveUrl, - false, - null, - false).Result) - .Returns(migrationId); - _mockTargetGithubApi.Setup(x => x.GetMigration(migrationId).Result).Returns((State: RepositoryMigrationStatus.Succeeded, TARGET_REPO, "", "")); - _mockTargetGithubApi.Setup(x => x.DoesOrgExist(TARGET_ORG).Result).Returns(true); - - _mockSourceGithubApi.Setup(x => x.StartGitArchiveGeneration(SOURCE_ORG, SOURCE_REPO).Result).Returns(gitArchiveId); - _mockSourceGithubApi.Setup(x => x.StartMetadataArchiveGeneration(SOURCE_ORG, SOURCE_REPO, false, false).Result).Returns(metadataArchiveId); - _mockSourceGithubApi.Setup(x => x.GetArchiveMigrationStatus(SOURCE_ORG, gitArchiveId).Result).Returns(ArchiveMigrationStatus.Exported); - _mockSourceGithubApi.Setup(x => x.GetArchiveMigrationStatus(SOURCE_ORG, metadataArchiveId).Result).Returns(ArchiveMigrationStatus.Exported); - _mockSourceGithubApi.Setup(x => x.GetArchiveMigrationUrl(SOURCE_ORG, gitArchiveId).Result).Returns(gitArchiveUrl); - _mockSourceGithubApi.Setup(x => x.GetArchiveMigrationUrl(SOURCE_ORG, metadataArchiveId).Result).Returns(metadataArchiveUrl); - - _mockHttpDownloadService.Setup(x => x.DownloadToBytes(gitArchiveUrl).Result).Returns(gitArchiveContent); - _mockHttpDownloadService.Setup(x => x.DownloadToBytes(metadataArchiveUrl).Result).Returns(metadataArchiveContent); - - _mockEnvironmentVariableProvider.Setup(m => m.SourceGithubPersonalAccessToken(It.IsAny())).Returns(sourceGithubPat); - _mockEnvironmentVariableProvider.Setup(m => m.TargetGithubPersonalAccessToken(It.IsAny())).Returns(targetGithubPat); -#pragma warning disable CS0618 - _mockEnvironmentVariableProvider.Setup(m => m.AwsAccessKey(false)).Returns(AWS_ACCESS_KEY); -#pragma warning restore CS0618 - - _mockAwsApi.Setup(m => m.UploadToBucket(AWS_BUCKET_NAME, It.IsAny(), It.IsAny())).ReturnsAsync(archiveUrl); - - _mockGhesVersionChecker.Setup(m => m.AreBlobCredentialsRequired(GHES_API_URL)).ReturnsAsync(true); - - var handler = new MigrateRepoCommandHandler( - _mockOctoLogger.Object, - _mockSourceGithubApi.Object, - _mockTargetGithubApi.Object, - _mockEnvironmentVariableProvider.Object, - _mockAzureApi.Object, - _mockAwsApi.Object, - _mockHttpDownloadService.Object, - _mockFileSystemProvider.Object, - _mockGhesVersionChecker.Object, - _retryPolicy); - - // Act, Assert - var args = new MigrateRepoCommandArgs - { - GithubSourceOrg = SOURCE_ORG, - SourceRepo = SOURCE_REPO, - GithubTargetOrg = TARGET_ORG, - TargetRepo = TARGET_REPO, - TargetApiUrl = TARGET_API_URL, - GhesApiUrl = GHES_API_URL, - AwsBucketName = AWS_BUCKET_NAME, - AwsSecretKey = AWS_SECRET_ACCESS_KEY, - Wait = true - }; - await handler.Invoking(async x => await x.Handle(args)).Should().NotThrowAsync(); - - _mockOctoLogger.Verify(m => m.LogWarning(It.Is(msg => msg.Contains("AWS_ACCESS_KEY")))); - } - [Fact] public async Task Ghes_When_Aws_Bucket_Name_Is_Provided_But_No_Aws_Secret_Key_Throws() { @@ -1339,94 +1249,6 @@ await _handler.Invoking(async x => await x.Handle(new MigrateRepoCommandArgs .WithMessage("*--aws-secret-key*"); } - [Fact] - public async Task Ghes_When_Aws_Bucket_Name_Is_Provided_And_Can_Fallback_To_Aws_Secret_Key_Environment_Variable_Does_Not_Throw() - { - var githubOrgId = Guid.NewGuid().ToString(); - var migrationSourceId = Guid.NewGuid().ToString(); - var sourceGithubPat = Guid.NewGuid().ToString(); - var targetGithubPat = Guid.NewGuid().ToString(); - var githubRepoUrl = $"https://myghes/{SOURCE_ORG}/{SOURCE_REPO}"; - var migrationId = Guid.NewGuid().ToString(); - var gitArchiveId = 1; - var metadataArchiveId = 2; - - var gitArchiveUrl = $"https://example.com/{gitArchiveId}"; - var metadataArchiveUrl = $"https://example.com/{metadataArchiveId}"; - var gitArchiveContent = new byte[] { 1, 2, 3, 4, 5 }; - var metadataArchiveContent = new byte[] { 6, 7, 8, 9, 10 }; - - var archiveUrl = $"https://s3.amazonaws.com/{AWS_BUCKET_NAME}/archive.tar"; - - _mockTargetGithubApi.Setup(x => x.GetOrganizationId(TARGET_ORG).Result).Returns(githubOrgId); - _mockTargetGithubApi.Setup(x => x.CreateGhecMigrationSource(githubOrgId).Result).Returns(migrationSourceId); - _mockTargetGithubApi - .Setup(x => x.StartMigration( - migrationSourceId, - githubRepoUrl, - githubOrgId, - TARGET_REPO, - sourceGithubPat, - targetGithubPat, - archiveUrl, - archiveUrl, - false, - null, - false).Result) - .Returns(migrationId); - _mockTargetGithubApi.Setup(x => x.GetMigration(migrationId).Result).Returns((State: RepositoryMigrationStatus.Succeeded, TARGET_REPO, "", "")); - _mockTargetGithubApi.Setup(x => x.DoesOrgExist(TARGET_ORG).Result).Returns(true); - - _mockSourceGithubApi.Setup(x => x.StartGitArchiveGeneration(SOURCE_ORG, SOURCE_REPO).Result).Returns(gitArchiveId); - _mockSourceGithubApi.Setup(x => x.StartMetadataArchiveGeneration(SOURCE_ORG, SOURCE_REPO, false, false).Result).Returns(metadataArchiveId); - _mockSourceGithubApi.Setup(x => x.GetArchiveMigrationStatus(SOURCE_ORG, gitArchiveId).Result).Returns(ArchiveMigrationStatus.Exported); - _mockSourceGithubApi.Setup(x => x.GetArchiveMigrationStatus(SOURCE_ORG, metadataArchiveId).Result).Returns(ArchiveMigrationStatus.Exported); - _mockSourceGithubApi.Setup(x => x.GetArchiveMigrationUrl(SOURCE_ORG, gitArchiveId).Result).Returns(gitArchiveUrl); - _mockSourceGithubApi.Setup(x => x.GetArchiveMigrationUrl(SOURCE_ORG, metadataArchiveId).Result).Returns(metadataArchiveUrl); - - _mockHttpDownloadService.Setup(x => x.DownloadToBytes(gitArchiveUrl).Result).Returns(gitArchiveContent); - _mockHttpDownloadService.Setup(x => x.DownloadToBytes(metadataArchiveUrl).Result).Returns(metadataArchiveContent); - - _mockEnvironmentVariableProvider.Setup(m => m.SourceGithubPersonalAccessToken(It.IsAny())).Returns(sourceGithubPat); - _mockEnvironmentVariableProvider.Setup(m => m.TargetGithubPersonalAccessToken(It.IsAny())).Returns(targetGithubPat); -#pragma warning disable CS0618 - _mockEnvironmentVariableProvider.Setup(m => m.AwsSecretKey(false)).Returns(AWS_SECRET_KEY); -#pragma warning restore CS0618 - - _mockAwsApi.Setup(m => m.UploadToBucket(AWS_BUCKET_NAME, It.IsAny(), It.IsAny())).ReturnsAsync(archiveUrl); - - _mockGhesVersionChecker.Setup(m => m.AreBlobCredentialsRequired(GHES_API_URL)).ReturnsAsync(true); - - var handler = new MigrateRepoCommandHandler( - _mockOctoLogger.Object, - _mockSourceGithubApi.Object, - _mockTargetGithubApi.Object, - _mockEnvironmentVariableProvider.Object, - _mockAzureApi.Object, - _mockAwsApi.Object, - _mockHttpDownloadService.Object, - _mockFileSystemProvider.Object, - _mockGhesVersionChecker.Object, - _retryPolicy); - - // Act, Assert - var args = new MigrateRepoCommandArgs - { - GithubSourceOrg = SOURCE_ORG, - SourceRepo = SOURCE_REPO, - GithubTargetOrg = TARGET_ORG, - TargetRepo = TARGET_REPO, - TargetApiUrl = TARGET_API_URL, - GhesApiUrl = GHES_API_URL, - AwsBucketName = AWS_BUCKET_NAME, - AwsAccessKey = AWS_ACCESS_KEY, - Wait = true - }; - await handler.Invoking(async x => await x.Handle(args)).Should().NotThrowAsync(); - - _mockOctoLogger.Verify(m => m.LogWarning(It.Is(msg => msg.Contains("AWS_SECRET_KEY")))); - } - [Fact] public async Task Ghes_When_Aws_Session_Token_Is_Provided_But_No_Aws_Region_Throws() { diff --git a/src/OctoshiftCLI.Tests/gei/Factories/AwsApiFactoryTests.cs b/src/OctoshiftCLI.Tests/gei/Factories/AwsApiFactoryTests.cs index 5aab96250..9256e147f 100644 --- a/src/OctoshiftCLI.Tests/gei/Factories/AwsApiFactoryTests.cs +++ b/src/OctoshiftCLI.Tests/gei/Factories/AwsApiFactoryTests.cs @@ -1,3 +1,4 @@ +using FluentAssertions; using Moq; using OctoshiftCLI.GithubEnterpriseImporter.Factories; using OctoshiftCLI.Services; @@ -16,40 +17,20 @@ public AwsApiFactoryTests() } [Fact] - public void It_Falls_Back_To_Aws_Access_Key_Environment_Variable_If_Aws_Access_Key_Id_Is_Not_Set() + public void It_Errors_If_Aws_Access_Key_Id_Is_Not_Provided_Or_Set_In_Environment_Variable() { - // Arrange - const string awsAccessKey = "AWS_ACCESS_KEY"; - const string awsRegion = "us-east-2"; -#pragma warning disable CS0618 - _mockEnvironmentVariableProvider.Setup(m => m.AwsAccessKey(false)).Returns(awsAccessKey); -#pragma warning restore CS0618 - - // Act - _awsApiFactory.Create(awsRegion, null, "aws-secret-access-key", "aws-session-token"); - - // Assert -#pragma warning disable CS0618 - _mockEnvironmentVariableProvider.Verify(m => m.AwsAccessKey(false), Times.Once); -#pragma warning restore CS0618 + // Act, Assert + _awsApiFactory.Invoking(x => x.Create("us-east-2", null, "aws-secret-access-key", "aws-session-token")) + .Should() + .ThrowExactly(); } [Fact] - public void It_Falls_Back_To_Aws_Secret_Key_Environment_Variable_If_Aws_Secret_Access_Key_Is_Not_Set() + public void It_Errors_If_Aws_Secret_Access_Key_Is_Not_Set_Or_Set_In_Environment_Variable() { - // Arrange - const string awsSecretKey = "AWS_SECRET_KEY"; - const string awsRegion = "us-east-2"; -#pragma warning disable CS0618 - _mockEnvironmentVariableProvider.Setup(m => m.AwsSecretKey(false)).Returns(awsSecretKey); -#pragma warning restore CS0618 - - // Act - _awsApiFactory.Create(awsRegion, "aws-access-key-id", null, "aws-session-token"); - - // Assert -#pragma warning disable CS0618 - _mockEnvironmentVariableProvider.Verify(m => m.AwsSecretKey(false), Times.Once); -#pragma warning restore CS0618 + // Act, Assert + _awsApiFactory.Invoking(x => x.Create("us-east-2", "aws-access-key-id", null, "aws-session-token")) + .Should() + .ThrowExactly(); } } diff --git a/src/bbs2gh/Commands/MigrateRepo/MigrateRepoCommandHandler.cs b/src/bbs2gh/Commands/MigrateRepo/MigrateRepoCommandHandler.cs index 033d89b32..b8e02cbac 100644 --- a/src/bbs2gh/Commands/MigrateRepo/MigrateRepoCommandHandler.cs +++ b/src/bbs2gh/Commands/MigrateRepo/MigrateRepoCommandHandler.cs @@ -317,30 +317,12 @@ private void ValidateUploadOptions(MigrateRepoCommandArgs args) { if (!GetAwsAccessKey(args).HasValue()) { -#pragma warning disable CS0618 - if (_environmentVariableProvider.AwsAccessKey(false).HasValue()) -#pragma warning restore CS0618 - { - _log.LogWarning("AWS_ACCESS_KEY environment variable is deprecated and will be removed in future releases. Please consider using AWS_ACCESS_KEY_ID environment variable instead."); - } - else - { - throw new OctoshiftCliException("Either --aws-access-key or AWS_ACCESS_KEY_ID environment variable must be set."); - } + throw new OctoshiftCliException("Either --aws-access-key or AWS_ACCESS_KEY_ID environment variable must be set."); } if (!GetAwsSecretKey(args).HasValue()) { -#pragma warning disable CS0618 - if (_environmentVariableProvider.AwsSecretKey(false).HasValue()) -#pragma warning restore CS0618 - { - _log.LogWarning("AWS_SECRET_KEY environment variable is deprecated and will be removed in future releases. Please consider using AWS_SECRET_ACCESS_KEY environment variable instead."); - } - else - { - throw new OctoshiftCliException("Either --aws-secret-key or AWS_SECRET_ACCESS_KEY environment variable must be set."); - } + throw new OctoshiftCliException("Either --aws-secret-key or AWS_SECRET_ACCESS_KEY environment variable must be set."); } if (GetAwsSessionToken(args).HasValue() && GetAwsRegion(args).IsNullOrWhiteSpace()) diff --git a/src/bbs2gh/Factories/AwsApiFactory.cs b/src/bbs2gh/Factories/AwsApiFactory.cs index 900e7b837..f49952a98 100644 --- a/src/bbs2gh/Factories/AwsApiFactory.cs +++ b/src/bbs2gh/Factories/AwsApiFactory.cs @@ -13,14 +13,8 @@ public AwsApiFactory(EnvironmentVariableProvider environmentVariableProvider) public virtual AwsApi Create(string awsRegion = null, string awsAccessKeyId = null, string awsSecretAccessKey = null, string awsSessionToken = null) { -#pragma warning disable CS0618 - awsAccessKeyId ??= _environmentVariableProvider.AwsAccessKeyId(false) - ?? _environmentVariableProvider.AwsAccessKey(false) - ?? _environmentVariableProvider.AwsAccessKeyId(); - awsSecretAccessKey ??= _environmentVariableProvider.AwsSecretAccessKey(false) - ?? _environmentVariableProvider.AwsSecretKey(false) - ?? _environmentVariableProvider.AwsSecretAccessKey(); -#pragma warning restore CS0618 + awsAccessKeyId ??= _environmentVariableProvider.AwsAccessKeyId(false); + awsSecretAccessKey ??= _environmentVariableProvider.AwsSecretAccessKey(false); awsSessionToken ??= _environmentVariableProvider.AwsSessionToken(false); awsRegion ??= _environmentVariableProvider.AwsRegion(false); diff --git a/src/gei/Commands/MigrateRepo/MigrateRepoCommandHandler.cs b/src/gei/Commands/MigrateRepo/MigrateRepoCommandHandler.cs index a1ec187b9..d572630ac 100644 --- a/src/gei/Commands/MigrateRepo/MigrateRepoCommandHandler.cs +++ b/src/gei/Commands/MigrateRepo/MigrateRepoCommandHandler.cs @@ -365,30 +365,12 @@ private void ValidateGHESOptions(MigrateRepoCommandArgs args, bool cloudCredenti { if (!GetAwsAccessKey(args).HasValue()) { -#pragma warning disable CS0618 - if (_environmentVariableProvider.AwsAccessKey(false).HasValue()) -#pragma warning restore CS0618 - { - _log.LogWarning("AWS_ACCESS_KEY environment variable is deprecated and will be removed in future releases. Please consider using AWS_ACCESS_KEY_ID environment variable instead."); - } - else - { - throw new OctoshiftCliException("Either --aws-access-key or AWS_ACCESS_KEY_ID environment variable must be set."); - } + throw new OctoshiftCliException("Either --aws-access-key or AWS_ACCESS_KEY_ID environment variable must be set."); } if (!GetAwsSecretKey(args).HasValue()) { -#pragma warning disable CS0618 - if (_environmentVariableProvider.AwsSecretKey(false).HasValue()) -#pragma warning restore CS0618 - { - _log.LogWarning("AWS_SECRET_KEY environment variable is deprecated and will be removed in future releases. Please consider using AWS_SECRET_ACCESS_KEY environment variable instead."); - } - else - { - throw new OctoshiftCliException("Either --aws-secret-key or AWS_SECRET_ACCESS_KEY environment variable must be set."); - } + throw new OctoshiftCliException("Either --aws-secret-key or AWS_SECRET_ACCESS_KEY environment variable must be set."); } if (GetAwsSessionToken(args).HasValue() && GetAwsRegion(args).IsNullOrWhiteSpace()) diff --git a/src/gei/Factories/AwsApiFactory.cs b/src/gei/Factories/AwsApiFactory.cs index 3b77d57ce..47c88e231 100644 --- a/src/gei/Factories/AwsApiFactory.cs +++ b/src/gei/Factories/AwsApiFactory.cs @@ -13,14 +13,8 @@ public AwsApiFactory(EnvironmentVariableProvider environmentVariableProvider) public virtual AwsApi Create(string awsRegion = null, string awsAccessKeyId = null, string awsSecretAccessKey = null, string awsSessionToken = null) { -#pragma warning disable CS0618 - awsAccessKeyId ??= _environmentVariableProvider.AwsAccessKeyId(false) - ?? _environmentVariableProvider.AwsAccessKey(false) - ?? _environmentVariableProvider.AwsAccessKeyId(); - awsSecretAccessKey ??= _environmentVariableProvider.AwsSecretAccessKey(false) - ?? _environmentVariableProvider.AwsSecretKey(false) - ?? _environmentVariableProvider.AwsSecretAccessKey(); -#pragma warning restore CS0618 + awsAccessKeyId ??= _environmentVariableProvider.AwsAccessKeyId(false); + awsSecretAccessKey ??= _environmentVariableProvider.AwsSecretAccessKey(false); awsSessionToken ??= _environmentVariableProvider.AwsSessionToken(false); awsRegion ??= _environmentVariableProvider.AwsRegion(false); From 56d353cfea5ed98acf537d6444d4e84f424b30bb Mon Sep 17 00:00:00 2001 From: Tim Rogers Date: Tue, 4 Jul 2023 18:03:02 -0700 Subject: [PATCH 2/6] Update release notes --- RELEASENOTES.md | 1 + 1 file changed, 1 insertion(+) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 8b1378917..23faad33f 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -1 +1,2 @@ +- 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 be configured the `AWS_ACCESS_KEY_ID` and `AWS_SECRET_ACCESS_KEY` variables or command line arguments. \ No newline at end of file From bb3c45aa895eb5ce67d94de115288ccee3a22908 Mon Sep 17 00:00:00 2001 From: Tim Rogers Date: Tue, 4 Jul 2023 18:11:34 -0700 Subject: [PATCH 3/6] Remove unused constants --- src/Octoshift/Services/EnvironmentVariableProvider.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Octoshift/Services/EnvironmentVariableProvider.cs b/src/Octoshift/Services/EnvironmentVariableProvider.cs index 07fb0561a..a5f3a30cc 100644 --- a/src/Octoshift/Services/EnvironmentVariableProvider.cs +++ b/src/Octoshift/Services/EnvironmentVariableProvider.cs @@ -10,8 +10,6 @@ public class EnvironmentVariableProvider private const string AZURE_STORAGE_CONNECTION_STRING = "AZURE_STORAGE_CONNECTION_STRING"; 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_ACCESS_KEY = "AWS_ACCESS_KEY"; - private const string AWS_SECRET_KEY = "AWS_SECRET_KEY"; private const string AWS_SESSION_TOKEN = "AWS_SESSION_TOKEN"; private const string AWS_REGION = "AWS_REGION"; private const string BBS_USERNAME = "BBS_USERNAME"; From aaec575701a89f3531ea6af19fed8d35769df439 Mon Sep 17 00:00:00 2001 From: Tim Rogers Date: Wed, 5 Jul 2023 19:34:58 +0100 Subject: [PATCH 4/6] Update src/bbs2gh/Factories/AwsApiFactory.cs Co-authored-by: Dylan Smith --- src/bbs2gh/Factories/AwsApiFactory.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/bbs2gh/Factories/AwsApiFactory.cs b/src/bbs2gh/Factories/AwsApiFactory.cs index f49952a98..058b67536 100644 --- a/src/bbs2gh/Factories/AwsApiFactory.cs +++ b/src/bbs2gh/Factories/AwsApiFactory.cs @@ -13,8 +13,8 @@ public AwsApiFactory(EnvironmentVariableProvider environmentVariableProvider) public virtual AwsApi Create(string awsRegion = null, string awsAccessKeyId = null, string awsSecretAccessKey = null, string awsSessionToken = null) { - awsAccessKeyId ??= _environmentVariableProvider.AwsAccessKeyId(false); - awsSecretAccessKey ??= _environmentVariableProvider.AwsSecretAccessKey(false); + awsAccessKeyId ??= _environmentVariableProvider.AwsAccessKeyId(); + awsSecretAccessKey ??= _environmentVariableProvider.AwsSecretAccessKey(); awsSessionToken ??= _environmentVariableProvider.AwsSessionToken(false); awsRegion ??= _environmentVariableProvider.AwsRegion(false); From 9e0ac2ee31197e8a74821c718b7aa9035cc24d3d Mon Sep 17 00:00:00 2001 From: Tim Rogers Date: Wed, 5 Jul 2023 19:35:06 +0100 Subject: [PATCH 5/6] Update src/gei/Factories/AwsApiFactory.cs Co-authored-by: Dylan Smith --- src/gei/Factories/AwsApiFactory.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/gei/Factories/AwsApiFactory.cs b/src/gei/Factories/AwsApiFactory.cs index 47c88e231..356659fe9 100644 --- a/src/gei/Factories/AwsApiFactory.cs +++ b/src/gei/Factories/AwsApiFactory.cs @@ -13,8 +13,8 @@ public AwsApiFactory(EnvironmentVariableProvider environmentVariableProvider) public virtual AwsApi Create(string awsRegion = null, string awsAccessKeyId = null, string awsSecretAccessKey = null, string awsSessionToken = null) { - awsAccessKeyId ??= _environmentVariableProvider.AwsAccessKeyId(false); - awsSecretAccessKey ??= _environmentVariableProvider.AwsSecretAccessKey(false); + awsAccessKeyId ??= _environmentVariableProvider.AwsAccessKeyId(); + awsSecretAccessKey ??= _environmentVariableProvider.AwsSecretAccessKey(); awsSessionToken ??= _environmentVariableProvider.AwsSessionToken(false); awsRegion ??= _environmentVariableProvider.AwsRegion(false); From 660e7934b86baeb6c28537968705217dbdfe785e Mon Sep 17 00:00:00 2001 From: Tim Rogers Date: Wed, 5 Jul 2023 19:35:13 +0100 Subject: [PATCH 6/6] Update RELEASENOTES.md Co-authored-by: Dylan Smith --- RELEASENOTES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 23faad33f..08ecf8a1f 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -1,2 +1,2 @@ -- 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 be configured the `AWS_ACCESS_KEY_ID` and `AWS_SECRET_ACCESS_KEY` variables or command line arguments. \ No newline at end of file +- 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 be configured using the `AWS_ACCESS_KEY_ID` and `AWS_SECRET_ACCESS_KEY` variables or command line arguments. \ No newline at end of file