From 82edc1d1742c4e7a58ac2cf8d6a02d5358eb61c8 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Thu, 10 Dec 2015 10:33:23 +0100 Subject: [PATCH] FIX #1432 open all the ports need for google driver Signed-off-by: David Gageot --- drivers/google/compute_util.go | 88 +++++++++++++++++++++-------- drivers/google/compute_util_test.go | 45 +++++++++++++++ 2 files changed, 108 insertions(+), 25 deletions(-) diff --git a/drivers/google/compute_util.go b/drivers/google/compute_util.go index 5450cf16..72e3f4e2 100644 --- a/drivers/google/compute_util.go +++ b/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{ diff --git a/drivers/google/compute_util_test.go b/drivers/google/compute_util_test.go index 3bebc302..40f0988c 100644 --- a/drivers/google/compute_util_test.go +++ b/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) + } +}