Skip to content

Conversation

@pietroalbini
Copy link
Member

This changes the credentials provider used to fetch the AWS credentials from EnvironmentProvider (which just looked at environment variables) to DefaultCredentialsProvider, which looks at:

  1. Environment variables
  2. ~/.aws/credentials
  3. EC2 instance roles

The old behavior is preserved when the environment variable is present, but this will also allow using EC2 instance roles which are going to be implemented on the production server.

A new FORCE_S3 environment variable was also added.

⚠️ ⚠️ I don't have a local environment running so I haven't tested the changes, but the code compiles. Could anyone with a local environment test if everything is ok? ⚠️ ⚠️

r? @Mark-Simulacrum

This changes the credentials provider used to fetch the AWS credentials
from EnvironmentProvider (which just looked at environment variables) to
DefaultCredentialsProvider, which looks at:

1. Environment variables
2. ~/.aws/credentials
3. EC2 instance roles

The old behavior is preserved when the environment variable is present,
but this will also allow using EC2 instance roles which are going to be
implemented on the production server.

A new FORCE_S3 environment variable was also added.
@Mark-Simulacrum
Copy link
Member

Code looks good. I'll probably not have time to test the changes locally for some time but I would loosely recommend that we just merge and deploy since the worst case is a few minutes of downtime as we rebuild after a revert on the server (we can test it without restarting docs.rs in full, too, if we wanted to).

@QuietMisdreavus
Copy link
Contributor

I mean, it should have the same behavior if you don't change the environment variables, right? It seems harmless to deploy in that case, especially if whoever pushes the button watches for issues afterward.

@pietroalbini pietroalbini merged commit e17ec5c into rust-lang:master Oct 8, 2019
@pietroalbini pietroalbini deleted the aws-iam-role branch October 8, 2019 07:58
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 this pull request may close these issues.

3 participants