CF template for freegeoip fargate stack #27259
Conversation
WIP - minimal ELB stack, minimal fargate stack WIP - first working version Remove unneeded file
uponthesun
force-pushed
the
geocoder-fargate
branch
from
8da08d5
to
e83770c
Compare
aws/cloudformation/geocoder.yml
Outdated
| VpcId: !ImportValue VPC | ||
| SecurityGroupIngress: | ||
| - | ||
| IpProtocol: 'tcp' |
There was a problem hiding this comment.
In general you don't need to quote YAML values unless you're using special characters in a string or need a number/boolean to be cast as a string, I find it more readable to usually leave unnecessary quotes off.
aws/cloudformation/geocoder.yml
Outdated
| Tags: | ||
| - | ||
| Key: 'Name' | ||
| Value: !Join [' ', ['ECS', !Ref 'EcsClusterName', '-', 'ECS SecurityGroup']] |
aws/cloudformation/geocoder.yml
Outdated
| Properties: | ||
| LogGroupName: !Join ['-', [ECSLogGroup, !Ref 'AWS::StackName']] | ||
| RetentionInDays: 14 | ||
| taskdefinition: |
There was a problem hiding this comment.
nit: consistent case-style for all Resources/Outputs (e.g., TaskDefinition)
There was a problem hiding this comment.
Agreed. Oddly enough this is from the AWS template snippets: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/quickref-ecs.html
aws/cloudformation/geocoder.yml
Outdated
| Value: !Join [' ', ['ECS', !Ref 'EcsClusterName', '-', 'ECS SecurityGroup']] | ||
| - | ||
| Key: 'Description' | ||
| Value: !Join [' ', ['Created for ECS cluster', !Ref 'EcsClusterName']] |
There was a problem hiding this comment.
Not sure what benefit these 'description' tags are providing (they seem redundant with the Name tags) so can probably be removed (unless Description tag is significant in the console, I don't think it is).
There was a problem hiding this comment.
Both name and description show up in the console apparently. Agreed that the description is redundant, will remove.
aws/cloudformation/geocoder.yml
Outdated
| Default: geocoder-default | ||
| EcsPort: | ||
| Type: String | ||
| Description: Optional - Security Group port to open on ECS instances - defaults to port 80 |
There was a problem hiding this comment.
The 'optional' and 'defaults to' parts of the description text are redundant- see how this currently renders in the console view:
| Value: !Join [' ', ['Created for ECS cluster', !Ref 'EcsClusterName']] | ||
| EcsElasticLoadBalancer: | ||
| Type: AWS::ElasticLoadBalancingV2::LoadBalancer | ||
| Properties: |
There was a problem hiding this comment.
This resource could use a readable Name Property, in addition to the Name tag. (The current stack is using an auto-generated name freeg-EcsEl-XXXXXXXXXXXX which isn't ideal).
| Value: !Join [' ', ['Created for ECS cluster', !Ref 'EcsClusterName']] | ||
| DefaultTargetGroup: | ||
| Type: AWS::ElasticLoadBalancingV2::TargetGroup | ||
| Properties: |
There was a problem hiding this comment.
This could also use a specific Name Property for better readability.
I was able to create a stack from this template in the dev@ account and make a request to the ELB from an EC2 instance. Manual steps done beforehand: