Skip to content

Commit

Permalink
Merge pull request #1054 from github/timrogers/deprecate-non-standard…
Browse files Browse the repository at this point in the history 
…-aws-environment-variables

Drop support for non-standard, deprecated `AWS_ACCESS_KEY` and `AWS_SECRET_KEY` environment variables
  • Loading branch information
@timrogers
timrogers committed Jul 6, 2023
2 parents b54abd5 + 660e793 commit 5aea393
Show file tree
Hide file tree
Showing 10 changed files with 31 additions and 378 deletions.
1 change: 1 addition & 0 deletions RELEASENOTES.md
View file
Original file line number Diff line number Diff line change
@@ -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 using the `AWS_ACCESS_KEY_ID` and `AWS_SECRET_ACCESS_KEY` variables or command line arguments.
10 changes: 0 additions & 10 deletions src/Octoshift/Services/EnvironmentVariableProvider.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -43,14 +41,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);

Expand Down
74 changes: 0 additions & 74 deletions src/OctoshiftCLI.Tests/bbs2gh/Commands/MigrateRepo/MigrateRepoCommandHandlerTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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<string>())).ReturnsAsync(ARCHIVE_PATH);
_mockAwsApi.Setup(x => x.UploadToBucket(AWS_BUCKET_NAME, ARCHIVE_PATH, It.IsAny<string>())).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<string>(msg => msg.Contains("AWS_ACCESS_KEY"))));
}

[Fact]
public async Task It_Throws_When_Aws_Bucket_Name_Is_Provided_But_No_Aws_Secret_Access_Key()
{
Expand All @@ -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<string>())).ReturnsAsync(ARCHIVE_PATH);
_mockAwsApi.Setup(x => x.UploadToBucket(AWS_BUCKET_NAME, ARCHIVE_PATH, It.IsAny<string>())).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<string>(msg => msg.Contains("AWS_SECRET_KEY"))));
}

[Fact]
public async Task It_Throws_When_Aws_Session_Token_Is_Provided_But_Aws_Region_Is_Not()
{
Expand Down
41 changes: 11 additions & 30 deletions src/OctoshiftCLI.Tests/bbs2gh/Factories/AwsApiFactoryTests.cs
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using FluentAssertions;
using Moq;
using OctoshiftCLI.BbsToGithub.Factories;
using OctoshiftCLI.Services;
Expand All @@ -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<System.ArgumentNullException>();
}

[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<System.ArgumentNullException>();
}
}
178 changes: 0 additions & 178 deletions src/OctoshiftCLI.Tests/gei/Commands/MigrateRepo/MigrateRepoCommandHandlerTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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<bool>())).Returns(sourceGithubPat);
_mockEnvironmentVariableProvider.Setup(m => m.TargetGithubPersonalAccessToken(It.IsAny<bool>())).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<Stream>(), It.IsAny<string>())).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<string>(msg => msg.Contains("AWS_ACCESS_KEY"))));
}

[Fact]
public async Task Ghes_When_Aws_Bucket_Name_Is_Provided_But_No_Aws_Secret_Key_Throws()
{
Expand All @@ -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<bool>())).Returns(sourceGithubPat);
_mockEnvironmentVariableProvider.Setup(m => m.TargetGithubPersonalAccessToken(It.IsAny<bool>())).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<Stream>(), It.IsAny<string>())).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<string>(msg => msg.Contains("AWS_SECRET_KEY"))));
}

[Fact]
public async Task Ghes_When_Aws_Session_Token_Is_Provided_But_No_Aws_Region_Throws()
{
Expand Down
Loading

0 comments on commit 5aea393

Please sign in to comment.