From 4021353a3385c6c0df5feea0f9a6a29b80122bcd Mon Sep 17 00:00:00 2001 From: Michael Foord Date: Wed, 30 Jul 2014 17:52:57 +0300 Subject: [PATCH 1/4] Don't expose state port for openstack provider --- provider/openstack/export_test.go | 4 ++-- provider/openstack/live_test.go | 13 +++++-------- provider/openstack/provider.go | 14 ++++---------- 3 files changed, 11 insertions(+), 20 deletions(-) diff --git a/provider/openstack/export_test.go b/provider/openstack/export_test.go index c507ca3614d..a55492c9087 100644 --- a/provider/openstack/export_test.go +++ b/provider/openstack/export_test.go @@ -296,8 +296,8 @@ func SetUseFloatingIP(e environs.Environ, val bool) { env.ecfg().attrs["use-floating-ip"] = val } -func SetUpGlobalGroup(e environs.Environ, name string, statePort, apiPort int) (nova.SecurityGroup, error) { - return e.(*environ).setUpGlobalGroup(name, statePort, apiPort) +func SetUpGlobalGroup(e environs.Environ, name string, apiPort int) (nova.SecurityGroup, error) { + return e.(*environ).setUpGlobalGroup(name, apiPort) } func EnsureGroup(e environs.Environ, name string, rules []nova.RuleInfo) (nova.SecurityGroup, error) { diff --git a/provider/openstack/live_test.go b/provider/openstack/live_test.go index 4bd6b7c40c7..fd82f5ca046 100644 --- a/provider/openstack/live_test.go +++ b/provider/openstack/live_test.go @@ -178,17 +178,15 @@ func (t *LiveTests) TestSetupGlobalGroupExposesCorrectPorts(c *gc.C) { } cleanup() defer cleanup() - statePort := 12345 // Default 37017 - apiPort := 34567 // Default 17070 - group, err := openstack.SetUpGlobalGroup(t.Env, groupName, statePort, apiPort) + apiPort := 34567 // Default 17070 + group, err := openstack.SetUpGlobalGroup(t.Env, groupName, apiPort) c.Assert(err, gc.IsNil) c.Assert(err, gc.IsNil) - // We default to exporting 22, statePort, apiPort, and icmp/udp/tcp on + // We default to exporting 22, apiPort, and icmp/udp/tcp on // all ports to other machines inside the same group // TODO(jam): 2013-09-18 http://pad.lv/1227142 - // We shouldn't be exposing the API and State ports on all the machines - // that *aren't* hosting the state server. (And once we finish - // client-via-API we can disable the State port as well.) + // We shouldn't be exposing the API port on all the machines + // that *aren't* hosting the state server. stringRules := make([]string, 0, len(group.Rules)) for _, rule := range group.Rules { ruleStr := fmt.Sprintf("%s %d %d %q %q", @@ -203,7 +201,6 @@ func (t *LiveTests) TestSetupGlobalGroupExposesCorrectPorts(c *gc.C) { // We don't care about the ordering, so we sort the result, and compare it. expectedRules := []string{ `tcp 22 22 "0.0.0.0/0" ""`, - fmt.Sprintf(`tcp %d %d "0.0.0.0/0" ""`, statePort, statePort), fmt.Sprintf(`tcp %d %d "0.0.0.0/0" ""`, apiPort, apiPort), fmt.Sprintf(`tcp 1 65535 "" "%s"`, groupName), fmt.Sprintf(`udp 1 65535 "" "%s"`, groupName), diff --git a/provider/openstack/provider.go b/provider/openstack/provider.go index f8441bedba4..b5a543a2ba6 100644 --- a/provider/openstack/provider.go +++ b/provider/openstack/provider.go @@ -959,7 +959,7 @@ func (e *environ) StartInstance(args environs.StartInstanceParams) (instance.Ins } } cfg := e.Config() - groups, err := e.setUpGroups(args.MachineConfig.MachineId, cfg.StatePort(), cfg.APIPort()) + groups, err := e.setUpGroups(args.MachineConfig.MachineId, cfg.APIPort()) if err != nil { return nil, nil, nil, fmt.Errorf("cannot set up groups: %v", err) } @@ -1296,7 +1296,7 @@ func (e *environ) Provider() environs.EnvironProvider { return &providerInstance } -func (e *environ) setUpGlobalGroup(groupName string, statePort, apiPort int) (nova.SecurityGroup, error) { +func (e *environ) setUpGlobalGroup(groupName string, apiPort int) (nova.SecurityGroup, error) { return e.ensureGroup(groupName, []nova.RuleInfo{ { @@ -1305,12 +1305,6 @@ func (e *environ) setUpGlobalGroup(groupName string, statePort, apiPort int) (no ToPort: 22, Cidr: "0.0.0.0/0", }, - { - IPProtocol: "tcp", - FromPort: statePort, - ToPort: statePort, - Cidr: "0.0.0.0/0", - }, { IPProtocol: "tcp", FromPort: apiPort, @@ -1346,8 +1340,8 @@ func (e *environ) setUpGlobalGroup(groupName string, statePort, apiPort int) (no // Note: ideally we'd have a better way to determine group membership so that 2 // people that happen to share an openstack account and name their environment // "openstack" don't end up destroying each other's machines. -func (e *environ) setUpGroups(machineId string, statePort, apiPort int) ([]nova.SecurityGroup, error) { - jujuGroup, err := e.setUpGlobalGroup(e.jujuGroupName(), statePort, apiPort) +func (e *environ) setUpGroups(machineId string, apiPort int) ([]nova.SecurityGroup, error) { + jujuGroup, err := e.setUpGlobalGroup(e.jujuGroupName(), apiPort) if err != nil { return nil, err } From 81a1319e9519e4779a1c0d2728ea11b2f5afbd55 Mon Sep 17 00:00:00 2001 From: Michael Foord Date: Thu, 31 Jul 2014 14:57:41 +0300 Subject: [PATCH 2/4] Do not open state port. ec2 and azure. --- provider/azure/environ.go | 9 --------- provider/azure/environ_test.go | 8 +------- provider/azure/instance_test.go | 6 ------ 3 files changed, 1 insertion(+), 22 deletions(-) diff --git a/provider/azure/environ.go b/provider/azure/environ.go index ec156b276d7..76b53406a4b 100644 --- a/provider/azure/environ.go +++ b/provider/azure/environ.go @@ -778,10 +778,6 @@ func (env *azureEnviron) newOSDisk(sourceImageName string) *gwacl.OSVirtualHardD // getInitialEndpoints returns a slice of the endpoints every instance should have open // (ssh port, etc). func (env *azureEnviron) getInitialEndpoints(stateServer bool) []gwacl.InputEndpoint { - // TODO(axw) either proxy ssh traffic through one of the - // randomly chosen VMs to the internal address, or otherwise - // don't load balance SSH and provide a way of getting the - // local port. cfg := env.Config() endpoints := []gwacl.InputEndpoint{{ LocalPort: 22, @@ -791,11 +787,6 @@ func (env *azureEnviron) getInitialEndpoints(stateServer bool) []gwacl.InputEndp }} if stateServer { endpoints = append(endpoints, []gwacl.InputEndpoint{{ - LocalPort: cfg.StatePort(), - Port: cfg.StatePort(), - Protocol: "tcp", - Name: "stateport", - }, { LocalPort: cfg.APIPort(), Port: cfg.APIPort(), Protocol: "tcp", diff --git a/provider/azure/environ_test.go b/provider/azure/environ_test.go index 1e74614dae9..6f27f0ece6a 100644 --- a/provider/azure/environ_test.go +++ b/provider/azure/environ_test.go @@ -1247,13 +1247,7 @@ func (*environSuite) testNewRole(c *gc.C, stateServer bool) { c.Check(sshEndpoint.Protocol, gc.Equals, "tcp") if stateServer { - // There's also an endpoint for the state (mongodb) port. - stateEndpoint, ok := endpoints[env.Config().StatePort()] - c.Assert(ok, gc.Equals, true) - c.Check(stateEndpoint.LocalPort, gc.Equals, env.Config().StatePort()) - c.Check(stateEndpoint.Protocol, gc.Equals, "tcp") - - // And one for the API port. + // There should be an endpoint for the API port. apiEndpoint, ok := endpoints[env.Config().APIPort()] c.Assert(ok, gc.Equals, true) c.Check(apiEndpoint.LocalPort, gc.Equals, env.Config().APIPort()) diff --git a/provider/azure/instance_test.go b/provider/azure/instance_test.go index b4a3f280113..b7255fd5cfc 100644 --- a/provider/azure/instance_test.go +++ b/provider/azure/instance_test.go @@ -353,11 +353,6 @@ func (s *instanceSuite) testPorts(c *gc.C, maskStateServerPorts bool) { Protocol: "tcp", Name: "test456", Port: 4456, - }, { - LocalPort: s.env.Config().StatePort(), - Protocol: "tcp", - Name: "stateserver", - Port: s.env.Config().StatePort(), }, { LocalPort: s.env.Config().APIPort(), Protocol: "tcp", @@ -380,7 +375,6 @@ func (s *instanceSuite) testPorts(c *gc.C, maskStateServerPorts bool) { {Number: 2123, Protocol: "udp"}, } if !maskStateServerPorts { - expected = append(expected, network.Port{Number: s.env.Config().StatePort(), Protocol: "tcp"}) expected = append(expected, network.Port{Number: s.env.Config().APIPort(), Protocol: "tcp"}) network.SortPorts(expected) } From 319f01e285f97afe0d8c838434a03f094c0478ba Mon Sep 17 00:00:00 2001 From: Michael Foord Date: Thu, 31 Jul 2014 14:58:16 +0300 Subject: [PATCH 3/4] Do not open state port. ec2 and azure. --- provider/ec2/ec2.go | 10 ++-------- provider/ec2/live_test.go | 3 +-- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/provider/ec2/ec2.go b/provider/ec2/ec2.go index e506e787a64..0a9bcd434b3 100644 --- a/provider/ec2/ec2.go +++ b/provider/ec2/ec2.go @@ -604,7 +604,7 @@ func (e *environ) StartInstance(args environs.StartInstanceParams) (instance.Ins } logger.Debugf("ec2 user data; %d bytes", len(userData)) cfg := e.Config() - groups, err := e.setUpGroups(args.MachineConfig.MachineId, cfg.StatePort(), cfg.APIPort()) + groups, err := e.setUpGroups(args.MachineConfig.MachineId, cfg.APIPort()) if err != nil { return nil, nil, nil, fmt.Errorf("cannot set up groups: %v", err) } @@ -1079,7 +1079,7 @@ func (inst *ec2Instance) Ports(machineId string) ([]network.Port, error) { // other instances that might be running on the same EC2 account. In // addition, a specific machine security group is created for each // machine, so that its firewall rules can be configured per machine. -func (e *environ) setUpGroups(machineId string, statePort, apiPort int) ([]ec2.SecurityGroup, error) { +func (e *environ) setUpGroups(machineId string, apiPort int) ([]ec2.SecurityGroup, error) { jujuGroup, err := e.ensureGroup(e.jujuGroupName(), []ec2.IPPerm{ { @@ -1088,12 +1088,6 @@ func (e *environ) setUpGroups(machineId string, statePort, apiPort int) ([]ec2.S ToPort: 22, SourceIPs: []string{"0.0.0.0/0"}, }, - { - Protocol: "tcp", - FromPort: statePort, - ToPort: statePort, - SourceIPs: []string{"0.0.0.0/0"}, - }, { Protocol: "tcp", FromPort: apiPort, diff --git a/provider/ec2/live_test.go b/provider/ec2/live_test.go index 84016632376..502d1164fea 100644 --- a/provider/ec2/live_test.go +++ b/provider/ec2/live_test.go @@ -224,9 +224,8 @@ func (t *LiveTests) TestInstanceGroups(c *gc.C) { // that the unneeded permission that we added earlier // has been deleted). perms := info[0].IPPerms - c.Assert(perms, gc.HasLen, 6) + c.Assert(perms, gc.HasLen, 5) checkPortAllowed(c, perms, 22) // SSH - checkPortAllowed(c, perms, coretesting.FakeConfig()["state-port"].(int)) checkPortAllowed(c, perms, coretesting.FakeConfig()["api-port"].(int)) checkSecurityGroupAllowed(c, perms, groups[0]) From 18a7ffbd412cf93ae346adb0f50b5889b03851f3 Mon Sep 17 00:00:00 2001 From: Michael Foord Date: Thu, 31 Jul 2014 15:48:02 +0300 Subject: [PATCH 4/4] Remove last mention of StatePort in azure provider tests --- provider/azure/environ_test.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/provider/azure/environ_test.go b/provider/azure/environ_test.go index 6f27f0ece6a..7d49645d251 100644 --- a/provider/azure/environ_test.go +++ b/provider/azure/environ_test.go @@ -1158,11 +1158,6 @@ func (s *environSuite) TestInitialPorts(c *gc.C) { // Only role2 should report opened state server ports via the Ports method. dummyRole := *role1 configSetNetwork(&dummyRole).InputEndpoints = &[]gwacl.InputEndpoint{{ - LocalPort: env.Config().StatePort(), - Protocol: "tcp", - Name: "stateserver", - Port: env.Config().StatePort(), - }, { LocalPort: env.Config().APIPort(), Protocol: "tcp", Name: "apiserver", @@ -1177,7 +1172,7 @@ func (s *environSuite) TestInitialPorts(c *gc.C) { for _, port := range ports { portmap[port.Number] = true } - return portmap[env.Config().StatePort()] && portmap[env.Config().APIPort()] + return portmap[env.Config().APIPort()] } c.Check(inst1, gc.Not(jc.Satisfies), reportsStateServerPorts) c.Check(inst2, jc.Satisfies, reportsStateServerPorts)