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

Support for ECS credentials provider #1033

Closed
wants to merge 2 commits into from

Conversation

fabienrenaud
Copy link

@fabienrenaud fabienrenaud commented Jul 2, 2017

This addresses issue #259

Adds the ECS credentials provider to the credentials chain before the EC2RoleProvider when AWS_CREDENTIALS_RELATIVE_UR environment variable is defined.

The code introduced merely mimics aws' defaults: https://github.com/aws/aws-sdk-go/blob/master/aws/defaults/defaults.go#L114

Added 1 new unit test.
I did not find acceptance tests for functions in auth_helpers.go so I believe I don't need to write any for this change...

Instructions to build a fully working terraform binary with this plugin would be appreciated.

I do not know how to point my local terraform checkout to use my locally built terraform-provider-aws. Everything is in my ~/go dir and my GOPATH is configured to point ~/go/bin as instructed. govendor faq didn't help. So far, I've tested the new terraform binary by manually copying auth_helpers.go to the terraform project. I ran the new binary in a container on ECS and it did work:

2017/07/01 21:13:11 [INFO] AWS EC2 instance detected via default metadata API endpoint, EC2RoleProvider added to the auth chain
2017/07/01 21:13:11 [INFO] AWS Auth provider used: "CredentialsEndpointProvider"

and got me through making calls to dynamodb and run most of terraform plan (which failed before this change because the instance-profile hasn't enough permissions). But later in the execution, terraform seems to using an older version of the plugin and therefore falls back to the instance-profile role:

2017/07/02 18:12:03 [DEBUG] plugin: terraform-provider-aws_v0.1.2_x4: 2017/07/02 18:12:03 [INFO] No assume_role block read from configuration
2017/07/02 18:12:03 [DEBUG] plugin: terraform-provider-aws_v0.1.2_x4: 2017/07/02 18:12:03 [INFO] Building AWS region structure
2017/07/02 18:12:03 [DEBUG] plugin: terraform-provider-aws_v0.1.2_x4: 2017/07/02 18:12:03 [INFO] Building AWS auth structure
2017/07/02 18:12:03 [DEBUG] plugin: terraform-provider-aws_v0.1.2_x4: 2017/07/02 18:12:03 [INFO] Setting AWS metadata API timeout to 100ms
2017/07/02 18:12:03 [DEBUG] root.ecs_service- [...]
2017/07/02 18:12:03 [DEBUG] plugin: terraform-provider-aws_v0.1.2_x4: 2017/07/02 18:12:03 [INFO] AWS EC2 instance detected via default metadata API endpoint, EC2RoleProvider added to the auth chain

Feel free to take over this change.

@radeksimko radeksimko added the enhancement Requests to existing resources that expand the functionality or scope. label Jul 3, 2017
@fabienrenaud fabienrenaud changed the title [WIP] Support for ECS credentials provider Support for ECS credentials provider Jul 10, 2017
@radeksimko
Copy link
Member

Hi @fabienrenaud
firstly I really apologise I missed your PR before reviewing #1425

The good news is that the next AWS provider release is going to include the functionality.

I appreciate the time you invested into this, especially into the attached tests. I'm willing to review & merge a patch adding tests, but I think that better approach would be to use AWS_CONTAINER_CREDENTIALS_FULL_URI instead of custom ENV variable.

Also, it's usually better practice to just rely on the SDK to do its job in regards to defaults and avoid duplicating any logic and/or constants, where possible - in this case 169.254.170.2.

Let me know what you think.

@radeksimko radeksimko closed this Sep 11, 2017
@ghost
Copy link

ghost commented Apr 11, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked and limited conversation to collaborators Apr 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Requests to existing resources that expand the functionality or scope.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants