AWS auto-discovery #2459

merged 9 commits into from Nov 3, 2016


None yet

6 participants

kyhavlov commented Nov 1, 2016 edited

Reworked #2039 to add EC2 discovery to -retry-join along with restructuring the config and adding multiple auth methods.

There's still some question of what to do about CI for the integration test (TestDiscoverEC2Hosts); for now I just wrote it to rely on the setup generated by our terraform AWS module and added a ConsulRole = Server tag to the instances in that. In Travis where the env vars aren't set, it just gets skipped until we decide what to do there.

jlambert121 and others added some commits May 12, 2016
@jlambert121 jlambert121 allow automatically discovering aws ec2 consul servers 9ece75f
@kyhavlov kyhavlov Merge branch 'master' of into aws_autodis…
@kyhavlov kyhavlov vendor: Add aws-go-sdk 0036eb9
@kyhavlov kyhavlov Fixed up config structure for EC2 discovery 043e689
@kyhavlov kyhavlov Add testing around EC2 discovery config 9c75e69
+ config := c.EC2Discovery
+ awsConfig := &aws.Config{
+ Region: aws.String(config.Region),
+ Credentials: credentials.NewStaticCredentials(config.AccessKeyID, config.SecretAccessKey, ""),
jhmartin Nov 1, 2016 Contributor

Doesn't this preclude use of the very useful IAM instance credentials? While statically defining the credentials is a useful option, IAM roles remove the management overhead of rotating those keys.

kyhavlov Nov 1, 2016 Contributor

Thanks for reminding me - I had meant to look into adding that after the first pass at this. It's definitely the more natural option when using an already AWS-specific feature

jhmartin Nov 2, 2016 Contributor

If you use the default credential provider it'll accept it from any of the environment, cred-file, and IAM role.

kyhavlov Nov 2, 2016 edited Contributor

I ended up using a ChainProvider to match the order in which Terraform's AWS Provider looks for credentials

kyhavlov added some commits Nov 2, 2016
@kyhavlov kyhavlov Support more forms of EC2 authentication 468bf73
@kyhavlov kyhavlov Move EC2 discovery logic into retryJoin for robustness d4d6e2b
- Credentials: credentials.NewStaticCredentials(config.AccessKeyID, config.SecretAccessKey, ""),
+ Region: aws.String(config.Region),
+ Credentials: credentials.NewChainCredentials(
+ []credentials.Provider{
xose Nov 2, 2016

Can we please get support for ECS Task Roles here? This would allow Consul to fetch credentials meant specifically for it rather than using the instance role when running as a Docker container.

Sample code:

kyhavlov Nov 2, 2016 Contributor

Sure, thanks for pointing this out.

@kyhavlov kyhavlov Add support for ECS task roles as an auth mechanism f3efab5
+ return nil, err
+ }
+ servers := make([]string, 0)
slackpad Nov 3, 2016 Contributor

Style nit - I'd just say var servers []string since you aren't setting a size.

+ logger.Printf("[INFO] agent: Discovered %d servers from EC2...", len(servers))
+ }
+ servers = append(config.RetryJoin, servers...)
slackpad Nov 3, 2016 Contributor

This will work fine, but I'd flip this to keep the servers so far in front, so servers = append(servers, config.RetryJoin...).


Looks awesome - noted some nits and the args should show up in the documentation in alphabetical order for both sections; they are a little out of place. Otherwise good to merge.

@kyhavlov kyhavlov Small tweaks to docs and syntax 1de39d2
@kyhavlov kyhavlov merged commit 0737985 into master Nov 3, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
@kyhavlov kyhavlov deleted the f-aws-autodiscovery branch Nov 3, 2016
CpuID commented Nov 11, 2016

This is pretty cool :) I currently take this a step further via a separate binary right now (to use with Docker + a bash wrapper) called ecs-discoverer, which detects which EC2 instances are running a specific Amazon ECS Task (based on an ECS Service Name).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment