Repository: docker/compose-cli
Pull request #1064 (2020-12-15 11:44:39)

Only consider public subnets
**What I did**
Add ability to detect "public subnets" and only consider those when building the stack's network configuration.

Note: private subnets (i.e. not associated with an Internet Gateway) don't make sense for Docker ECS integration as service would not be able to pull sidecar images from DockerHub.

**Related issue**
https://github.com/docker/compose-cli/issues/921

/test-ecs

**(not mandatory) A picture of a cute animal, if possible in relation with what you did**
![image](https://user-images.githubusercontent.com/132757/102211022-40bbc080-3ed3-11eb-954a-25b182dfc3ef.png)



Patch:
diff --git a/cli/main.go b/cli/main.go
index 026af7b9..13ea7911 100644
--- a/cli/main.go
+++ b/cli/main.go
@@ -147,8 +147,8 @@ func main() {
 
 	root.PersistentFlags().BoolVarP(&opts.Debug, "debug", "D", false, "Enable debug output in the logs")
 	root.PersistentFlags().StringVarP(&opts.Host, "host", "H", "", "Daemon socket(s) to connect to")
-	opts.AddContextFlags(root.PersistentFlags())
 	opts.AddConfigFlags(root.PersistentFlags())
+	opts.AddContextFlags(root.PersistentFlags())
 	root.Flags().BoolVarP(&opts.Version, "version", "v", false, "Print version information and quit")
 
 	walk(root, func(c *cobra.Command) {
@@ -157,7 +157,6 @@ func main() {
 
 	// populate the opts with the global flags
 	_ = root.PersistentFlags().Parse(os.Args[1:])
-	_ = root.Flags().Parse(os.Args[1:])
 	if opts.Debug {
 		logrus.SetLevel(logrus.DebugLevel)
 	}
diff --git a/cli/mobycli/exec.go b/cli/mobycli/exec.go
index c46637ed..15f87509 100644
--- a/cli/mobycli/exec.go
+++ b/cli/mobycli/exec.go
@@ -22,7 +22,7 @@ import (
 	"os"
 	"os/exec"
 	"os/signal"
-	"regexp"
+	"strings"
 
 	"github.com/spf13/cobra"
 
@@ -112,7 +112,9 @@ func IsDefaultContextCommand(dockerCommand string) bool {
 	if e != nil {
 		fmt.Println(e)
 	}
-	return regexp.MustCompile("Usage:\\s*docker\\s*" + dockerCommand).Match(b)
+	output := string(b)
+	contains := strings.Contains(output, "Usage:\tdocker "+dockerCommand)
+	return contains
 }
 
 // ExecSilent executes a command and do redirect output to stdOut, return output
diff --git a/ecs/aws.go b/ecs/aws.go
index 5cf50fbf..e20fe66a 100644
--- a/ecs/aws.go
+++ b/ecs/aws.go
@@ -40,6 +40,7 @@ type API interface {
 	CheckVPC(ctx context.Context, vpcID string) error
 	GetDefaultVPC(ctx context.Context) (string, error)
 	GetSubNets(ctx context.Context, vpcID string) ([]awsResource, error)
+	IsPublicSubnet(ctx context.Context, vpcID string, subNetID string) (bool, error)
 	GetRoleArn(ctx context.Context, name string) (string, error)
 	StackExists(ctx context.Context, name string) (bool, error)
 	CreateStack(ctx context.Context, name string, region string, template []byte) error
diff --git a/ecs/awsResources.go b/ecs/awsResources.go
index 012b3182..d1ff7cf4 100644
--- a/ecs/awsResources.go
+++ b/ecs/awsResources.go
@@ -185,10 +185,22 @@ func (b *ecsAPIService) parseVPCExtension(ctx context.Context, project *types.Pr
 	if err != nil {
 		return "", nil, err
 	}
-	if len(subNets) < 2 {
-		return "", nil, fmt.Errorf("VPC %s should have at least 2 associated subnets in different availability zones", vpc)
+
+	var publicSubNets []awsResource
+	for _, subNet := range subNets {
+		isPublic, err := b.aws.IsPublicSubnet(ctx, vpc, subNet.ID())
+		if err != nil {
+			return "", nil, err
+		}
+		if isPublic {
+			publicSubNets = append(publicSubNets, subNet)
+		}
+	}
+
+	if len(publicSubNets) < 2 {
+		return "", nil, fmt.Errorf("VPC %s should have at least 2 associated public subnets in different availability zones", vpc)
 	}
-	return vpc, subNets, nil
+	return vpc, publicSubNets, nil
 }
 
 func (b *ecsAPIService) parseLoadBalancerExtension(ctx context.Context, project *types.Project) (awsResource, string, error) {
diff --git a/ecs/aws_mock.go b/ecs/aws_mock.go
index 9d438bf2..486648f4 100644
--- a/ecs/aws_mock.go
+++ b/ecs/aws_mock.go
@@ -6,12 +6,13 @@ package ecs
 
 import (
 	context "context"
+	reflect "reflect"
+
 	cloudformation "github.com/aws/aws-sdk-go/service/cloudformation"
 	ecs "github.com/aws/aws-sdk-go/service/ecs"
 	compose "github.com/docker/compose-cli/api/compose"
 	secrets "github.com/docker/compose-cli/api/secrets"
 	gomock "github.com/golang/mock/gomock"
-	reflect "reflect"
 )
 
 // MockAPI is a mock of API interface
@@ -453,6 +454,21 @@ func (mr *MockAPIMockRecorder) InspectSecret(arg0, arg1 interface{}) *gomock.Cal
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "InspectSecret", reflect.TypeOf((*MockAPI)(nil).InspectSecret), arg0, arg1)
 }
 
+// IsPublicSubnet mocks base method
+func (m *MockAPI) IsPublicSubnet(ctx context.Context, arg0 string, arg1 string) (bool, error) {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "IsPublicSubnet", arg0, arg1)
+	ret0, _ := ret[0].(bool)
+	ret1, _ := ret[1].(error)
+	return ret0, ret1
+}
+
+// IsPublicSubnet indicates an expected call of IsPublicSubnet
+func (mr *MockAPIMockRecorder) IsPublicSubnet(arg0, arg1 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsPublicSubnet", reflect.TypeOf((*MockAPI)(nil).IsPublicSubnet), arg0, arg1)
+}
+
 // ListFileSystems mocks base method
 func (m *MockAPI) ListFileSystems(arg0 context.Context, arg1 map[string]string) ([]awsResource, error) {
 	m.ctrl.T.Helper()
diff --git a/ecs/sdk.go b/ecs/sdk.go
index b44ecbfa..a8e89946 100644
--- a/ecs/sdk.go
+++ b/ecs/sdk.go
@@ -211,6 +211,37 @@ func (s sdk) GetSubNets(ctx context.Context, vpcID string) ([]awsResource, error
 	return ids, nil
 }
 
+func (s sdk) IsPublicSubnet(ctx context.Context, vpcID string, subNetID string) (bool, error) {
+	tables, err := s.EC2.DescribeRouteTablesWithContext(ctx, &ec2.DescribeRouteTablesInput{
+		Filters: []*ec2.Filter{
+			{
+				Name:   aws.String("association.subnet-id"),
+				Values: []*string{aws.String(subNetID)},
+			},
+		},
+	})
+	if err != nil {
+		return false, err
+	}
+	if len(tables.RouteTables) == 0 {
+		// If a subnet is not explicitly associated with any route table, it is implicitly associated with the main route table.
+		// https://docs.aws.amazon.com/cli/latest/reference/ec2/describe-route-tables.html
+		return true, nil
+	}
+	for _, routeTable := range tables.RouteTables {
+		for _, route := range routeTable.Routes {
+			if aws.StringValue(route.State) != "active" {
+				continue
+			}
+			if strings.HasPrefix(aws.StringValue(route.GatewayId), "igw-") {
+				// Connected to an internet Gateway
+				return true, nil
+			}
+		}
+	}
+	return false, nil
+}
+
 func (s sdk) GetRoleArn(ctx context.Context, name string) (string, error) {
 	role, err := s.IAM.GetRoleWithContext(ctx, &iam.GetRoleInput{
 		RoleName: aws.String(name),


Test patch:
diff --git a/ecs/cloudformation_test.go b/ecs/cloudformation_test.go
index 275721c3..0021114a 100644
--- a/ecs/cloudformation_test.go
+++ b/ecs/cloudformation_test.go
@@ -591,6 +591,8 @@ func useDefaultVPC(m *MockAPIMockRecorder) {
 		existingAWSResource{id: "subnet1"},
 		existingAWSResource{id: "subnet2"},
 	}, nil)
+	m.IsPublicSubnet(gomock.Any(), "subnet1").Return(true, nil)
+	m.IsPublicSubnet(gomock.Any(), "subnet2").Return(true, nil)
 }
 
 func useGPU(m *MockAPIMockRecorder) {