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

Proposal: Improved AWS configuration. #102

Merged
merged 14 commits into from
Feb 10, 2021
Merged

Conversation

dougal
Copy link
Collaborator

@dougal dougal commented May 15, 2020

Summary

Proposed change to Propono configuration, to allow more flexible AWS configuration, and defaults which match those of the aws-sdk.

PR is currently a docs-only change, to encourage discussion, with code changes to follow after.

What problems does this solve?

Currently, AWS for Propono has two authentication options:

  1. Passing in keys, which requires the keys to be available to the application, or
  2. IAM roles (which are broken, see Handle use_iam_profile with the new aws-sdk gem #90 and New IAM profiles pattern #96)

Proposed changes would:

  1. By default require no AWS configuration. AWS gems would use their own defaults of either credentials in ~/.aws/credentials or an IAM Role (if on EC2, ECS, etc).
  2. Add a new configuration option aws_options, into which any options common to both SQS and SNS could be passed. Examples of these are region, access_key_id, and secret_access_key.
  3. Add sqs_options and sns_options to configure per-service options. An example of where separate configuration might be useful is setting endpoints for development work. See Allow configuration of SNS and SQS endpoints. #95.
  4. Allow advanced authentication methods, such as Aws::AssumeRoleCredentials.
  5. Encourage better practice, by not requiring keys which could potentially be committed source control.

What about existing configuration options?

Currently, Propono provides access_key, secret_key, and queue_region.

Short term, remove these options from documentation, and have them continue to work silently. Optionally display a deprecation notice for each usage. This would allow current users to continue to use their existing authentication setup without modification.

On release of the next major version, remove these options entirely.

README.md Outdated Show resolved Hide resolved
@iHiD
Copy link
Owner

iHiD commented May 15, 2020

Yeah - this sounds sensible to me. Thanks. Sorry for not looking at the other PRs before.

Allow advanced authentication methods, such as Aws::AssumeRoleCredentials.

Where does this fit in? What would an example look like to do this?

Co-authored-by: Jeremy Walker <jez.walker@gmail.com>
@dougal
Copy link
Collaborator Author

dougal commented May 15, 2020

Yeah - this sounds sensible to me. Thanks. Sorry for not looking at the other PRs before.

Allow advanced authentication methods, such as Aws::AssumeRoleCredentials.

Where does this fit in? What would an example look like to do this?

An example of this is you have role X, which has access to use Y on a different account, for access to resources in that account.

See docs here: https://docs.aws.amazon.com/sdk-for-ruby/v3/developer-guide/setup-config.html#aws-ruby-sdk-credentials-access-token

Full example:

role_credentials = Aws::AssumeRoleCredentials.new(
  client: Aws::STS::Client.new,
  role_arn: "linked::account::arn",
  role_session_name: "session-name"
)

client = Propono::Client.new do |config|
  config.aws_options = {
    credentials: role_credentials       
  }
end

@iHiD
Copy link
Owner

iHiD commented May 15, 2020

Gotcha. Cool. Yep - this makes sense. Do it with a bump to the major version and I'll ship it.

@dougal dougal marked this pull request as draft May 15, 2020 19:49
@dougal dougal marked this pull request as ready for review February 10, 2021 17:26
@dougal
Copy link
Collaborator Author

dougal commented Feb 10, 2021

I accidentally made this draft almost a year ago, then forgot about it.

Is good-to-go. Travis failure is unrelated. I have made an issue for this: #103

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.

2 participants