AWS auto-discovery #2459
AWS auto-discovery #2459
Conversation
| config := c.EC2Discovery | ||
| awsConfig := &aws.Config{ | ||
| Region: aws.String(config.Region), | ||
| Credentials: credentials.NewStaticCredentials(config.AccessKeyID, config.SecretAccessKey, ""), |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
If you use the default credential provider it'll accept it from any of the environment, cred-file, and IAM role.
There was a problem hiding this comment.
I ended up using a ChainProvider to match the order in which Terraform's AWS Provider looks for credentials
kyhavlov
force-pushed
the
f-aws-autodiscovery
branch
from
a061df8 to
468bf73
Compare
| Credentials: credentials.NewStaticCredentials(config.AccessKeyID, config.SecretAccessKey, ""), | ||
| Region: aws.String(config.Region), | ||
| Credentials: credentials.NewChainCredentials( | ||
| []credentials.Provider{ |
There was a problem hiding this comment.
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: https://github.com/aws/aws-sdk-go/blob/master/aws/defaults/defaults.go#L102
There was a problem hiding this comment.
Sure, thanks for pointing this out.
| return nil, err | ||
| } | ||
|
|
||
| servers := make([]string, 0) |
There was a problem hiding this comment.
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...) |
There was a problem hiding this comment.
This will work fine, but I'd flip this to keep the servers so far in front, so servers = append(servers, config.RetryJoin...).
|
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). |
Reworked #2039 to add EC2 discovery to
-retry-joinalong 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 = Servertag 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.