FIX #1432 open all the ports need for google driver

Signed-off-by: David Gageot <david@gageot.net>

drivers/google/compute_util.go

@@ -127,37 +127,78 @@ func (c *ComputeUtil) firewallRule() (*raw.Firewall, error) {
 	return c.service.Firewalls.Get(c.project, firewallRule).Do()
 }
 
-func (c *ComputeUtil) createFirewallRule() error {
-	log.Infof("Creating firewall rule.")
-	allowed := []*raw.FirewallAllowed{
-		{
-			IPProtocol: "tcp",
-			Ports:      []string{port},
-		},
+func missingOpenedPorts(rule *raw.Firewall, ports []string) []string {
+	missing := []string{}
+	opened := map[string]bool{}
+
+	for _, allowed := range rule.Allowed {
+		for _, allowedPort := range allowed.Ports {
+			opened[allowedPort] = true
+		}
 	}
 
+	for _, port := range ports {
+		if !opened[port] {
+			missing = append(missing, port)
+		}
+	}
+
+	return missing
+}
+
+func (c *ComputeUtil) portsUsed() ([]string, error) {
+	ports := []string{port}
+
 	if c.SwarmMaster {
 		u, err := url.Parse(c.SwarmHost)
 		if err != nil {
-			return fmt.Errorf("error authorizing port for swarm: %s", err)
+			return nil, fmt.Errorf("error authorizing port for swarm: %s", err)
 		}
 
-		parts := strings.Split(u.Host, ":")
-		swarmPort := parts[1]
-		allowed = append(allowed, &raw.FirewallAllowed{
-			IPProtocol: "tcp",
-			Ports:      []string{swarmPort},
-		})
+		swarmPort := strings.Split(u.Host, ":")[1]
+		ports = append(ports, swarmPort)
 	}
 
-	rule := &raw.Firewall{
-		Allowed:      allowed,
-		SourceRanges: []string{"0.0.0.0/0"},
-		TargetTags:   []string{firewallTargetTag},
-		Name:         firewallRule,
+	return ports, nil
+}
+
+func (c *ComputeUtil) createFirewallRule() error {
+	log.Infof("Opening firewall ports.")
+
+	create := false
+	rule, _ := c.firewallRule()
+	if rule == nil {
+		create = true
+		rule = &raw.Firewall{
+			Name:         firewallRule,
+			Allowed:      []*raw.FirewallAllowed{},
+			SourceRanges: []string{"0.0.0.0/0"},
+			TargetTags:   []string{firewallTargetTag},
+		}
+	}
+
+	portsUsed, err := c.portsUsed()
+	if err != nil {
+		return err
+	}
+
+	missingPorts := missingOpenedPorts(rule, portsUsed)
+	if len(missingPorts) == 0 {
+		return nil
+	}
+
+	rule.Allowed = append(rule.Allowed, &raw.FirewallAllowed{
+		IPProtocol: "tcp",
+		Ports:      missingPorts,
+	})
+
+	var op *raw.Operation
+	if create {
+		op, err = c.service.Firewalls.Insert(c.project, rule).Do()
+	} else {
+		op, err = c.service.Firewalls.Update(c.project, firewallRule, rule).Do()
 	}
 
-	op, err := c.service.Firewalls.Insert(c.project, rule).Do()
 	if err != nil {
 		return err
 	}
@@ -174,11 +215,8 @@ func (c *ComputeUtil) instance() (*raw.Instance, error) {
 func (c *ComputeUtil) createInstance(d *Driver) error {
 	log.Infof("Creating instance.")
 
-	// The rule will either exist or be nil in case of an error.
-	if rule, _ := c.firewallRule(); rule == nil {
-		if err := c.createFirewallRule(); err != nil {
-			return err
-		}
+	if err := c.createFirewallRule(); err != nil {
+		return err
 	}
 
 	instance := &raw.Instance{

drivers/google/compute_util_test.go

@@ -4,6 +4,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	raw "google.golang.org/api/compute/v1"
 )
 
 func TestDefaultTag(t *testing.T) {
@@ -23,3 +24,47 @@ func TestAdditionalTags(t *testing.T) {
 
 	assert.Equal(t, []string{"docker-machine", "tag1", "tag2"}, tags)
 }
+
+func TestPortsUsed(t *testing.T) {
+	var tests = []struct {
+		description   string
+		computeUtil   *ComputeUtil
+		expectedPorts []string
+		expectedError error
+	}{
+		{"use docker port", &ComputeUtil{}, []string{"2376"}, nil},
+		{"use docker and swarm port", &ComputeUtil{SwarmMaster: true, SwarmHost: "tcp://host:3376"}, []string{"2376", "3376"}, nil},
+		{"use docker and non default swarm port", &ComputeUtil{SwarmMaster: true, SwarmHost: "tcp://host:4242"}, []string{"2376", "4242"}, nil},
+	}
+
+	for _, test := range tests {
+		ports, err := test.computeUtil.portsUsed()
+
+		assert.Equal(t, test.expectedPorts, ports)
+		assert.Equal(t, test.expectedError, err)
+	}
+}
+
+func TestMissingOpenedPorts(t *testing.T) {
+	var tests = []struct {
+		description     string
+		allowed         []*raw.FirewallAllowed
+		ports           []string
+		expectedMissing []string
+	}{
+		{"no port opened", []*raw.FirewallAllowed{}, []string{"2376"}, []string{"2376"}},
+		{"docker port opened", []*raw.FirewallAllowed{{IPProtocol: "tcp", Ports: []string{"2376"}}}, []string{"2376"}, []string{}},
+		{"missing swarm port", []*raw.FirewallAllowed{{IPProtocol: "tcp", Ports: []string{"2376"}}}, []string{"2376", "3376"}, []string{"3376"}},
+		{"missing docker port", []*raw.FirewallAllowed{{IPProtocol: "tcp", Ports: []string{"3376"}}}, []string{"2376", "3376"}, []string{"2376"}},
+		{"both ports opened", []*raw.FirewallAllowed{{IPProtocol: "tcp", Ports: []string{"2376", "3376"}}}, []string{"2376", "3376"}, []string{}},
+		{"more ports opened", []*raw.FirewallAllowed{{IPProtocol: "tcp", Ports: []string{"2376", "3376", "22", "1024-2048"}}}, []string{"2376", "3376"}, []string{}},
+	}
+
+	for _, test := range tests {
+		firewall := &raw.Firewall{Allowed: test.allowed}
+
+		missingPorts := missingOpenedPorts(firewall, test.ports)
+
+		assert.Equal(t, test.expectedMissing, missingPorts, test.description)
+	}
+}