Merge pull request #2545 from dgageot/1432-google-update-firewall

Google driver - Update firewall open ports

drivers/google/compute_util.go

@@ -9,7 +9,6 @@ import (
 	"time"
 
 	"github.com/docker/machine/libmachine/log"
-	"github.com/docker/machine/libmachine/ssh"
 	raw "google.golang.org/api/compute/v1"
 
 	"errors"
@@ -82,7 +81,7 @@ func (c *ComputeUtil) diskType() string {
 	return apiURL + c.project + "/zones/" + c.zone + "/diskTypes/" + c.diskTypeURL
 }
 
-// disk returns the gce Disk.
+// disk returns the persistent disk attached to the vm.
 func (c *ComputeUtil) disk() (*raw.Disk, error) {
 	return c.service.Disks.Get(c.project, c.zone, c.diskName()).Do()
 }
@@ -94,6 +93,7 @@ func (c *ComputeUtil) deleteDisk() error {
 	if err != nil {
 		return err
 	}
+
 	log.Infof("Waiting for disk to delete.")
 	return c.waitForRegionalOp(op.Name)
 }
@@ -127,47 +127,82 @@ 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{
+func missingOpenedPorts(rule *raw.Firewall, ports []string) []string {
+	missing := []string{}
+	opened := map[string]bool{}
 
-		{
-			IPProtocol: "tcp",
-			Ports: []string{
-				port,
-			},
-		},
+	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,
-		},
+
+	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},
 		}
-	op, err := c.service.Firewalls.Insert(c.project, rule).Do()
+	}
+
+	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()
+	}
+
+	if err != nil {
+		return err
+	}
+
 	return c.waitForGlobalOp(op.Name)
 }
 
@@ -179,12 +214,10 @@ func (c *ComputeUtil) instance() (*raw.Instance, error) {
 // createInstance creates a GCE VM instance.
 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
 	}
-	}
 
 	instance := &raw.Instance{
 		Name:        c.instanceName,
@@ -329,30 +362,13 @@ func (c *ComputeUtil) startInstance() error {
 	return c.waitForRegionalOp(op.Name)
 }
 
-func (c *ComputeUtil) executeCommands(commands []string, ip, sshKeyPath string) error {
-	for _, command := range commands {
-		auth := &ssh.Auth{
-			Keys: []string{sshKeyPath},
-		}
-
-		client, err := ssh.NewClient(c.userName, ip, 22, auth)
-		if err != nil {
-			return err
-		}
-
-		if _, err := client.Output(command); err != nil {
-			return err
-		}
-	}
-	return nil
-}
-
 func (c *ComputeUtil) waitForOp(opGetter func() (*raw.Operation, error)) error {
 	for {
 		op, err := opGetter()
 		if err != nil {
 			return err
 		}
+
 		log.Debugf("operation %q status: %s", op.Name, op.Status)
 		if op.Status == "DONE" {
 			if op.Error != nil {
@@ -365,13 +381,14 @@ func (c *ComputeUtil) waitForOp(opGetter func() (*raw.Operation, error)) error {
 	return nil
 }
 
-// waitForOp waits for the GCE Operation to finish.
+// waitForOp waits for the operation to finish.
 func (c *ComputeUtil) waitForRegionalOp(name string) error {
 	return c.waitForOp(func() (*raw.Operation, error) {
 		return c.service.ZoneOperations.Get(c.project, c.zone, name).Do()
 	})
 }
 
+// waitForGlobalOp waits for the global operation to finish.
 func (c *ComputeUtil) waitForGlobalOp(name string) error {
 	return c.waitForOp(func() (*raw.Operation, error) {
 		return c.service.GlobalOperations.Get(c.project, name).Do()

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)
+	}
+}

drivers/google/google_test.go

@@ -8,7 +8,7 @@ import (
 )
 
 func TestSetConfigFromFlags(t *testing.T) {
-	driver := NewDriver("default", "path")
+	driver := NewDriver("", "")
 
 	checkFlags := &drivers.CheckDriverOptions{
 		FlagsValues: map[string]interface{}{

libmachine/provision/ubuntu_systemd.go

@@ -141,7 +141,7 @@ func (provisioner *UbuntuSystemdProvisioner) Provision(swarmOptions swarm.Option
 		}
 	}
 
-	log.Debug("installing docker")
+	log.Info("Installing Docker...")
 	if err := installDockerGeneric(provisioner, engineOptions.InstallURL); err != nil {
 		return err
 	}