ECS: Redid security group generation for LBs

ecs/awsResources.go

@@ -51,6 +51,9 @@ func (r *awsResources) serviceSecurityGroups(service types.ServiceConfig) []stri
 	for net := range service.Networks {
 		groups = append(groups, r.securityGroups[net])
 	}
+	if len(service.Ports) > 0 {
+		groups = append(groups, r.securityGroups[serviceIngressSecGroupName(service.Name, false)])
+	}
 	return groups
 }
 
@@ -330,6 +333,7 @@ func (b *ecsAPIService) ensureCluster(r *awsResources, project *types.Project, t
 
 func (b *ecsAPIService) ensureNetworks(r *awsResources, project *types.Project, template *cloudformation.Template) {
 	if r.securityGroups == nil {
+		// TODO NITZ change the size hint?
 		r.securityGroups = make(map[string]string, len(project.Networks))
 	}
 	for name, net := range project.Networks {
@@ -353,6 +357,27 @@ func (b *ecsAPIService) ensureNetworks(r *awsResources, project *types.Project,
 
 		r.securityGroups[name] = cloudformation.Ref(securityGroup)
 	}
+
+	for _, service := range project.Services {
+		if len(service.Ports) == 0 {
+			continue
+		}
+		lbSecurityGroup := serviceIngressSecGroupName(service.Name, true)
+		template.Resources[lbSecurityGroup] = &ec2.SecurityGroup{
+			GroupDescription: fmt.Sprintf("%s Security Group for service %s ingress, LB side", project.Name, service.Name),
+			VpcId:            r.vpc,
+			Tags:             serviceTags(project, service),
+		}
+		r.securityGroups[lbSecurityGroup] = cloudformation.Ref(lbSecurityGroup)
+
+		serviceSecurityGroup := serviceIngressSecGroupName(service.Name, false)
+		template.Resources[serviceSecurityGroup] = &ec2.SecurityGroup{
+			GroupDescription: fmt.Sprintf("%s Security Group for service %s ingress, Service side", project.Name, service.Name),
+			VpcId:            r.vpc,
+			Tags:             serviceTags(project, service),
+		}
+		r.securityGroups[serviceSecurityGroup] = cloudformation.Ref(serviceSecurityGroup)
+	}
 }
 
 func (b *ecsAPIService) ensureVolumes(r *awsResources, project *types.Project, template *cloudformation.Template) error {
@@ -465,9 +490,9 @@ func (b *ecsAPIService) ensureLoadBalancer(r *awsResources, project *types.Proje
 
 func (r *awsResources) getLoadBalancerSecurityGroups(project *types.Project) []string {
 	securityGroups := []string{}
-	for name, network := range project.Networks {
-		if !network.Internal {
-			securityGroups = append(securityGroups, r.securityGroups[name])
+	for _, service := range project.Services {
+		if len(service.Ports) > 0 {
+			securityGroups = append(securityGroups, r.securityGroups[serviceIngressSecGroupName(service.Name, true)])
 		}
 	}
 	return securityGroups

ecs/cloudformation.go

@@ -197,11 +197,15 @@ func (b *ecsAPIService) createService(project *types.Project, service types.Serv
 		dependsOn []string
 		serviceLB []ecs.Service_LoadBalancer
 	)
-	for _, port := range service.Ports {
-		for net := range service.Networks {
-			b.createIngress(service, net, port, template, resources)
-		}
 
+	for _, port := range service.Ports {
+		lbSecurityGroupName := serviceIngressSecGroupName(service.Name, true)
+		serviceSecurityGroupName := serviceIngressSecGroupName(service.Name, false)
+		// outside to ingress
+		b.createIngress(service, lbSecurityGroupName, port, template, resources)
+		// ingress to service
+		b.createCrossGroupIngress(service, lbSecurityGroupName, serviceSecurityGroupName, port, template, resources)
+		// TODO attach new secgroup to this service
 		protocol := strings.ToUpper(port.Protocol)
 		if resources.loadBalancerType == elbv2.LoadBalancerTypeEnumApplication {
 			// we don't set Https as a certificate must be specified for HTTPS listeners
@@ -293,6 +297,22 @@ func (b *ecsAPIService) createIngress(service types.ServiceConfig, net string, p
 	}
 }
 
+func (b *ecsAPIService) createCrossGroupIngress(service types.ServiceConfig, source string, dest string, port types.ServicePortConfig, template *cloudformation.Template, resources awsResources) {
+	protocol := strings.ToUpper(port.Protocol)
+	if protocol == "" {
+		protocol = allProtocols
+	}
+	ingress := fmt.Sprintf("%s%d%sIngress", normalizeResourceName(source), port.Target, normalizeResourceName(dest))
+	template.Resources[ingress] = &ec2.SecurityGroupIngress{
+		SourceSecurityGroupId: resources.securityGroups[source],
+		Description:           fmt.Sprintf("LB connectivity on %d", port.Target),
+		GroupId:               resources.securityGroups[dest],
+		FromPort:              int(port.Target),
+		IpProtocol:            protocol,
+		ToPort:                int(port.Target),
+	}
+}
+
 func (b *ecsAPIService) createSecret(project *types.Project, name string, s types.SecretConfig, template *cloudformation.Template) error {
 	if s.External.External {
 		return nil
@@ -534,6 +554,16 @@ func networkResourceName(network string) string {
 	return fmt.Sprintf("%sNetwork", normalizeResourceName(network))
 }
 
+func serviceIngressSecGroupName(service string, isLBSide bool) string {
+	var header string
+	if isLBSide {
+		header = "LB"
+	} else {
+		header = "Service"
+	}
+	return fmt.Sprintf("%s%sIngressSecurityGroup", normalizeResourceName(service), header)
+}
+
 func serviceResourceName(service string) string {
 	return fmt.Sprintf("%sService", normalizeResourceName(service))
 }

ecs/cloudformation_test.go

@@ -51,6 +51,17 @@ func TestSimpleConvert(t *testing.T) {
 	golden.Assert(t, result, expected)
 }
 
+func TestSlightlyComplexConvert(t *testing.T) {
+	bytes, err := ioutil.ReadFile("testdata/input/slightly-complex-service.yaml")
+	assert.NilError(t, err)
+	template := convertYaml(t, string(bytes), nil, useDefaultVPC)
+	resultAsJSON, err := marshall(template, "yaml")
+	assert.NilError(t, err)
+	result := fmt.Sprintf("%s\n", string(resultAsJSON))
+	expected := "slightly-complex-cloudformation-conversion.golden"
+	golden.Assert(t, result, expected)
+}
+
 func TestLogging(t *testing.T) {
 	template := convertYaml(t, `
 services:

ecs/testdata/input/slightly-complex-service.yaml

@@ -0,0 +1,8 @@
+services:
+  entrance:
+    image: nginx
+    ports:
+      - "80:80"
+
+  sensitive:
+    image: httpd

ecs/testdata/simple-cloudformation-conversion.golden

@@ -13,16 +13,6 @@ Resources:
       - Key: com.docker.compose.project
         Value: TestSimpleConvert
     Type: AWS::ECS::Cluster
-  Default80Ingress:
-    Properties:
-      CidrIp: 0.0.0.0/0
-      Description: simple:80/tcp on default network
-      FromPort: 80
-      GroupId:
-        Ref: DefaultNetwork
-      IpProtocol: TCP
-      ToPort: 80
-    Type: AWS::EC2::SecurityGroupIngress
   DefaultNetwork:
     Properties:
       GroupDescription: TestSimpleConvert Security Group for default network
@@ -46,7 +36,7 @@ Resources:
     Properties:
       Scheme: internet-facing
       SecurityGroups:
-      - Ref: DefaultNetwork
+      - Ref: SimpleLBIngressSecurityGroup
       Subnets:
       - subnet1
       - subnet2
@@ -59,6 +49,38 @@ Resources:
     Properties:
       LogGroupName: /docker-compose/TestSimpleConvert
     Type: AWS::Logs::LogGroup
+  SimpleLBIngressSecurityGroup:
+    Properties:
+      GroupDescription: TestSimpleConvert Security Group for service simple ingress,
+        LB side
+      Tags:
+      - Key: com.docker.compose.project
+        Value: TestSimpleConvert
+      - Key: com.docker.compose.service
+        Value: simple
+      VpcId: vpc-123
+    Type: AWS::EC2::SecurityGroup
+  SimpleLBIngressSecurityGroup80Ingress:
+    Properties:
+      CidrIp: 0.0.0.0/0
+      Description: simple:80/tcp on SimpleLBIngressSecurityGroup network
+      FromPort: 80
+      GroupId:
+        Ref: SimpleLBIngressSecurityGroup
+      IpProtocol: TCP
+      ToPort: 80
+    Type: AWS::EC2::SecurityGroupIngress
+  SimpleLBIngressSecurityGroup80SimpleServiceIngressSecurityGroupIngress:
+    Properties:
+      Description: LB connectivity on 80
+      FromPort: 80
+      GroupId:
+        Ref: SimpleServiceIngressSecurityGroup
+      IpProtocol: TCP
+      SourceSecurityGroupId:
+        Ref: SimpleLBIngressSecurityGroup
+      ToPort: 80
+    Type: AWS::EC2::SecurityGroupIngress
   SimpleService:
     DependsOn:
     - SimpleTCP80Listener
@@ -84,6 +106,7 @@ Resources:
           AssignPublicIp: ENABLED
           SecurityGroups:
           - Ref: DefaultNetwork
+          - Ref: SimpleServiceIngressSecurityGroup
           Subnets:
           - subnet1
           - subnet2
@@ -117,6 +140,17 @@ Resources:
       NamespaceId:
         Ref: CloudMap
     Type: AWS::ServiceDiscovery::Service
+  SimpleServiceIngressSecurityGroup:
+    Properties:
+      GroupDescription: TestSimpleConvert Security Group for service simple ingress,
+        Service side
+      Tags:
+      - Key: com.docker.compose.project
+        Value: TestSimpleConvert
+      - Key: com.docker.compose.service
+        Value: simple
+      VpcId: vpc-123
+    Type: AWS::EC2::SecurityGroup
   SimpleTCP80Listener:
     Properties:
       DefaultActions:

ecs/testdata/slightly-complex-cloudformation-conversion.golden

@@ -0,0 +1,354 @@
+AWSTemplateFormatVersion: 2010-09-09
+Resources:
+  CloudMap:
+    Properties:
+      Description: Service Map for Docker Compose project TestSlightlyComplexConvert
+      Name: TestSlightlyComplexConvert.local
+      Vpc: vpc-123
+    Type: AWS::ServiceDiscovery::PrivateDnsNamespace
+  Cluster:
+    Properties:
+      ClusterName: TestSlightlyComplexConvert
+      Tags:
+      - Key: com.docker.compose.project
+        Value: TestSlightlyComplexConvert
+    Type: AWS::ECS::Cluster
+  DefaultNetwork:
+    Properties:
+      GroupDescription: TestSlightlyComplexConvert Security Group for default network
+      Tags:
+      - Key: com.docker.compose.project
+        Value: TestSlightlyComplexConvert
+      - Key: com.docker.compose.network
+        Value: TestSlightlyComplexConvert_default
+      VpcId: vpc-123
+    Type: AWS::EC2::SecurityGroup
+  DefaultNetworkIngress:
+    Properties:
+      Description: Allow communication within network default
+      GroupId:
+        Ref: DefaultNetwork
+      IpProtocol: "-1"
+      SourceSecurityGroupId:
+        Ref: DefaultNetwork
+    Type: AWS::EC2::SecurityGroupIngress
+  EntranceLBIngressSecurityGroup:
+    Properties:
+      GroupDescription: TestSlightlyComplexConvert Security Group for service entrance
+        ingress, LB side
+      Tags:
+      - Key: com.docker.compose.project
+        Value: TestSlightlyComplexConvert
+      - Key: com.docker.compose.service
+        Value: entrance
+      VpcId: vpc-123
+    Type: AWS::EC2::SecurityGroup
+  EntranceLBIngressSecurityGroup80EntranceServiceIngressSecurityGroupIngress:
+    Properties:
+      Description: LB connectivity on 80
+      FromPort: 80
+      GroupId:
+        Ref: EntranceServiceIngressSecurityGroup
+      IpProtocol: TCP
+      SourceSecurityGroupId:
+        Ref: EntranceLBIngressSecurityGroup
+      ToPort: 80
+    Type: AWS::EC2::SecurityGroupIngress
+  EntranceLBIngressSecurityGroup80Ingress:
+    Properties:
+      CidrIp: 0.0.0.0/0
+      Description: entrance:80/tcp on EntranceLBIngressSecurityGroup network
+      FromPort: 80
+      GroupId:
+        Ref: EntranceLBIngressSecurityGroup
+      IpProtocol: TCP
+      ToPort: 80
+    Type: AWS::EC2::SecurityGroupIngress
+  EntranceService:
+    DependsOn:
+    - EntranceTCP80Listener
+    Properties:
+      Cluster:
+        Fn::GetAtt:
+        - Cluster
+        - Arn
+      DeploymentConfiguration:
+        MaximumPercent: 200
+        MinimumHealthyPercent: 100
+      DeploymentController:
+        Type: ECS
+      DesiredCount: 1
+      LaunchType: FARGATE
+      LoadBalancers:
+      - ContainerName: entrance
+        ContainerPort: 80
+        TargetGroupArn:
+          Ref: EntranceTCP80TargetGroup
+      NetworkConfiguration:
+        AwsvpcConfiguration:
+          AssignPublicIp: ENABLED
+          SecurityGroups:
+          - Ref: DefaultNetwork
+          - Ref: EntranceServiceIngressSecurityGroup
+          Subnets:
+          - subnet1
+          - subnet2
+      PlatformVersion: 1.4.0
+      PropagateTags: SERVICE
+      SchedulingStrategy: REPLICA
+      ServiceRegistries:
+      - RegistryArn:
+          Fn::GetAtt:
+          - EntranceServiceDiscoveryEntry
+          - Arn
+      Tags:
+      - Key: com.docker.compose.project
+        Value: TestSlightlyComplexConvert
+      - Key: com.docker.compose.service
+        Value: entrance
+      TaskDefinition:
+        Ref: EntranceTaskDefinition
+    Type: AWS::ECS::Service
+  EntranceServiceDiscoveryEntry:
+    Properties:
+      Description: '"entrance" service discovery entry in Cloud Map'
+      DnsConfig:
+        DnsRecords:
+        - TTL: 60
+          Type: A
+        RoutingPolicy: MULTIVALUE
+      HealthCheckCustomConfig:
+        FailureThreshold: 1
+      Name: entrance
+      NamespaceId:
+        Ref: CloudMap
+    Type: AWS::ServiceDiscovery::Service
+  EntranceServiceIngressSecurityGroup:
+    Properties:
+      GroupDescription: TestSlightlyComplexConvert Security Group for service entrance
+        ingress, Service side
+      Tags:
+      - Key: com.docker.compose.project
+        Value: TestSlightlyComplexConvert
+      - Key: com.docker.compose.service
+        Value: entrance
+      VpcId: vpc-123
+    Type: AWS::EC2::SecurityGroup
+  EntranceTCP80Listener:
+    Properties:
+      DefaultActions:
+      - ForwardConfig:
+          TargetGroups:
+          - TargetGroupArn:
+              Ref: EntranceTCP80TargetGroup
+        Type: forward
+      LoadBalancerArn:
+        Ref: LoadBalancer
+      Port: 80
+      Protocol: HTTP
+    Type: AWS::ElasticLoadBalancingV2::Listener
+  EntranceTCP80TargetGroup:
+    Properties:
+      Port: 80
+      Protocol: HTTP
+      Tags:
+      - Key: com.docker.compose.project
+        Value: TestSlightlyComplexConvert
+      TargetType: ip
+      VpcId: vpc-123
+    Type: AWS::ElasticLoadBalancingV2::TargetGroup
+  EntranceTaskDefinition:
+    Properties:
+      ContainerDefinitions:
+      - Command:
+        - .compute.internal
+        - TestSlightlyComplexConvert.local
+        Essential: false
+        Image: docker/ecs-searchdomain-sidecar:1.0
+        LogConfiguration:
+          LogDriver: awslogs
+          Options:
+            awslogs-group:
+              Ref: LogGroup
+            awslogs-region:
+              Ref: AWS::Region
+            awslogs-stream-prefix: TestSlightlyComplexConvert
+        Name: Entrance_ResolvConf_InitContainer
+      - DependsOn:
+        - Condition: SUCCESS
+          ContainerName: Entrance_ResolvConf_InitContainer
+        Essential: true
+        Image: nginx
+        LinuxParameters: {}
+        LogConfiguration:
+          LogDriver: awslogs
+          Options:
+            awslogs-group:
+              Ref: LogGroup
+            awslogs-region:
+              Ref: AWS::Region
+            awslogs-stream-prefix: TestSlightlyComplexConvert
+        Name: entrance
+        PortMappings:
+        - ContainerPort: 80
+          HostPort: 80
+          Protocol: tcp
+      Cpu: "256"
+      ExecutionRoleArn:
+        Ref: EntranceTaskExecutionRole
+      Family: TestSlightlyComplexConvert-entrance
+      Memory: "512"
+      NetworkMode: awsvpc
+      RequiresCompatibilities:
+      - FARGATE
+    Type: AWS::ECS::TaskDefinition
+  EntranceTaskExecutionRole:
+    Properties:
+      AssumeRolePolicyDocument:
+        Statement:
+        - Action:
+          - sts:AssumeRole
+          Condition: {}
+          Effect: Allow
+          Principal:
+            Service: ecs-tasks.amazonaws.com
+        Version: 2012-10-17
+      ManagedPolicyArns:
+      - arn:aws:iam::aws:policy/service-role/AmazonECSTaskExecutionRolePolicy
+      - arn:aws:iam::aws:policy/AmazonEC2ContainerRegistryReadOnly
+      Tags:
+      - Key: com.docker.compose.project
+        Value: TestSlightlyComplexConvert
+      - Key: com.docker.compose.service
+        Value: entrance
+    Type: AWS::IAM::Role
+  LoadBalancer:
+    Properties:
+      Scheme: internet-facing
+      SecurityGroups:
+      - Ref: EntranceLBIngressSecurityGroup
+      Subnets:
+      - subnet1
+      - subnet2
+      Tags:
+      - Key: com.docker.compose.project
+        Value: TestSlightlyComplexConvert
+      Type: application
+    Type: AWS::ElasticLoadBalancingV2::LoadBalancer
+  LogGroup:
+    Properties:
+      LogGroupName: /docker-compose/TestSlightlyComplexConvert
+    Type: AWS::Logs::LogGroup
+  SensitiveService:
+    Properties:
+      Cluster:
+        Fn::GetAtt:
+        - Cluster
+        - Arn
+      DeploymentConfiguration:
+        MaximumPercent: 200
+        MinimumHealthyPercent: 100
+      DeploymentController:
+        Type: ECS
+      DesiredCount: 1
+      LaunchType: FARGATE
+      NetworkConfiguration:
+        AwsvpcConfiguration:
+          AssignPublicIp: ENABLED
+          SecurityGroups:
+          - Ref: DefaultNetwork
+          Subnets:
+          - subnet1
+          - subnet2
+      PlatformVersion: 1.4.0
+      PropagateTags: SERVICE
+      SchedulingStrategy: REPLICA
+      ServiceRegistries:
+      - RegistryArn:
+          Fn::GetAtt:
+          - SensitiveServiceDiscoveryEntry
+          - Arn
+      Tags:
+      - Key: com.docker.compose.project
+        Value: TestSlightlyComplexConvert
+      - Key: com.docker.compose.service
+        Value: sensitive
+      TaskDefinition:
+        Ref: SensitiveTaskDefinition
+    Type: AWS::ECS::Service
+  SensitiveServiceDiscoveryEntry:
+    Properties:
+      Description: '"sensitive" service discovery entry in Cloud Map'
+      DnsConfig:
+        DnsRecords:
+        - TTL: 60
+          Type: A
+        RoutingPolicy: MULTIVALUE
+      HealthCheckCustomConfig:
+        FailureThreshold: 1
+      Name: sensitive
+      NamespaceId:
+        Ref: CloudMap
+    Type: AWS::ServiceDiscovery::Service
+  SensitiveTaskDefinition:
+    Properties:
+      ContainerDefinitions:
+      - Command:
+        - .compute.internal
+        - TestSlightlyComplexConvert.local
+        Essential: false
+        Image: docker/ecs-searchdomain-sidecar:1.0
+        LogConfiguration:
+          LogDriver: awslogs
+          Options:
+            awslogs-group:
+              Ref: LogGroup
+            awslogs-region:
+              Ref: AWS::Region
+            awslogs-stream-prefix: TestSlightlyComplexConvert
+        Name: Sensitive_ResolvConf_InitContainer
+      - DependsOn:
+        - Condition: SUCCESS
+          ContainerName: Sensitive_ResolvConf_InitContainer
+        Essential: true
+        Image: httpd
+        LinuxParameters: {}
+        LogConfiguration:
+          LogDriver: awslogs
+          Options:
+            awslogs-group:
+              Ref: LogGroup
+            awslogs-region:
+              Ref: AWS::Region
+            awslogs-stream-prefix: TestSlightlyComplexConvert
+        Name: sensitive
+      Cpu: "256"
+      ExecutionRoleArn:
+        Ref: SensitiveTaskExecutionRole
+      Family: TestSlightlyComplexConvert-sensitive
+      Memory: "512"
+      NetworkMode: awsvpc
+      RequiresCompatibilities:
+      - FARGATE
+    Type: AWS::ECS::TaskDefinition
+  SensitiveTaskExecutionRole:
+    Properties:
+      AssumeRolePolicyDocument:
+        Statement:
+        - Action:
+          - sts:AssumeRole
+          Condition: {}
+          Effect: Allow
+          Principal:
+            Service: ecs-tasks.amazonaws.com
+        Version: 2012-10-17
+      ManagedPolicyArns:
+      - arn:aws:iam::aws:policy/service-role/AmazonECSTaskExecutionRolePolicy
+      - arn:aws:iam::aws:policy/AmazonEC2ContainerRegistryReadOnly
+      Tags:
+      - Key: com.docker.compose.project
+        Value: TestSlightlyComplexConvert
+      - Key: com.docker.compose.service
+        Value: sensitive
+    Type: AWS::IAM::Role
+