Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Database Download Trait does not respect AWS credentials in environment variables #222

Closed
apotek opened this issue Oct 10, 2024 · 1 comment · Fixed by #223
Closed

Database Download Trait does not respect AWS credentials in environment variables #222

apotek opened this issue Oct 10, 2024 · 1 comment · Fixed by #223
Assignees

Comments

@apotek
Copy link
Contributor

apotek commented Oct 10, 2024

Description

The Database Download Trait uses the AsyncAWS library to do S3 database backup downloads.

According to the library, AsyncAWS respects environment variables set, for authentication.

But Usher does not allow for this and forces use of credentials stored in $HOME/.aws/credentials

$awsConfigDirPath = getenv('HOME') . '/.aws';
$awsConfigFilePath = "$awsConfigDirPath/credentials";
if (!is_dir($awsConfigDirPath) || !file_exists($awsConfigFilePath)) {
$result = $this->configureAwsCredentials($awsConfigDirPath, $awsConfigFilePath);
if ($result->wasCancelled()) {
return Result::cancelled();
}
}

According to their documentation, the following sources, in order, should be consulted for credential configuration:

  1. Hard-Coded Configuration
  2. Environment Variables
  3. WebIdentity
  4. Credential and Configuration Files
  5. ECS Container Credentials
  6. EC2 Instance Metadata

In a perfect world, we should support all of these, in this order. Perhaps the perfect world is achievable and Usher just gets out of the way, but if not, we can at least, minimally, allow for 1, 2, and 4, in that order.

This means that, before we run lines 35-42 of the above quoted code, we should:

  1. Check to see if the client is already configured with authentication values.
  2. Check to see if the required environment variables are set, and
  3. Only if the above two conditions are false, continue to our current logic.
@apotek apotek self-assigned this Oct 29, 2024
apotek added a commit that referenced this issue Oct 30, 2024
Do not assume credential file creation will have valid credentials.
Begin to move away from deprecated ->io constructs.

Resolves #222
@apotek apotek linked a pull request Oct 30, 2024 that will close this issue
@apotek
Copy link
Contributor Author

apotek commented Oct 30, 2024

Tested all tugboat commands in Tugboat and they work. https://github.com/ChromaticHQ/benz-platform/pull/6220

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant