Merge pull request #391 from ehazlett/ec2-use-single-sg

Create / Re-use existing EC2 security group

README.md

@@ -94,15 +94,17 @@ Create machines on [Amazon Web Services](http://aws.amazon.com).  You will need
 
 Options:
 
- - `--amazonec2-access-key`: Your access key id for the Amazon Web Services API.
- - `--amazonec2-ami`: The AMI ID of the instance to use  Default: `ami-a00461c8`
+ - `--amazonec2-access-key`: **required** Your access key id for the Amazon Web Services API.
+ - `--amazonec2-ami`: The AMI ID of the instance to use  Default: `ami-4ae27e22`
  - `--amazonec2-instance-type`: The instance type to run.  Default: `t2.micro`
  - `--amazonec2-region`: The region to use when launching the instance.  Default: `us-east-1`
  - `--amazonec2-root-size`: The root disk size of the instance (in GB).  Default: `16`
- - `--amazonec2-secret-key`: Your secret access key for the Amazon Web Services API.
+ - `--amazonec2-secret-key`: **required** Your secret access key for the Amazon Web Services API.
+ - `--amazonec2-security-group-name`: AWS VPC security group name. Default: `docker-machine`
  - `--amazonec2-session-token`: Your session token for the Amazon Web Services API.
- - `--amazonec2-vpc-id`: Your VPC ID to launch the instance in.
- - `--amazonec2-zone`: The AWS zone launch the instance in (i.e. one of a,b,c,d,e).
+ - `--amazonec2-subnet-id`: AWS VPC subnet id
+ - `--amazonec2-vpc-id`: **required** Your VPC ID to launch the instance in.
+ - `--amazonec2-zone`: The AWS zone launch the instance in (i.e. one of a,b,c,d,e). Default: `a`
 
 ### Google Compute Engine
 

docs/dockermachine.md

@@ -454,15 +454,17 @@ Create machines on [Amazon Web Services](http://aws.amazon.com).  You will need
 
 Options:
 
- - `--amazonec2-access-key`: Your access key id for the Amazon Web Services API.
- - `--amazonec2-ami`: The AMI ID of the instance to use  Default: `ami-a00461c8`
+ - `--amazonec2-access-key`: **required** Your access key id for the Amazon Web Services API.
+ - `--amazonec2-ami`: The AMI ID of the instance to use  Default: `ami-4ae27e22`
  - `--amazonec2-instance-type`: The instance type to run.  Default: `t2.micro`
  - `--amazonec2-region`: The region to use when launching the instance.  Default: `us-east-1`
  - `--amazonec2-root-size`: The root disk size of the instance (in GB).  Default: `16`
- - `--amazonec2-secret-key`: Your secret access key for the Amazon Web Services API.
+ - `--amazonec2-secret-key`: **required** Your secret access key for the Amazon Web Services API.
+ - `--amazonec2-security-group`: AWS VPC security group name. Default: `docker-machine`
  - `--amazonec2-session-token`: Your session token for the Amazon Web Services API.
- - `--amazonec2-vpc-id`: Your VPC ID to launch the instance in.
- - `--amazonec2-zone`: The AWS zone launch the instance in (i.e. one of a,b,c,d,e).
+ - `--amazonec2-subnet-id`: AWS VPC subnet id
+ - `--amazonec2-vpc-id`: **required** Your VPC ID to launch the instance in.
+ - `--amazonec2-zone`: The AWS zone launch the instance in (i.e. one of a,b,c,d,e). Default: `a`
 
 #### Digital Ocean
 Creates machines on [Digital Ocean](https://www.digitalocean.com/). You need to create a personal access token under "Apps & API" in the Digital Ocean Control Panel and pass that to `docker-machine create` with the `--digitalocean-access-token` option.

drivers/amazonec2/amazonec2.go

@@ -19,38 +19,41 @@ import (
 )
 
 const (
-	driverName          = "amazonec2"
-	defaultRegion       = "us-east-1"
-	defaultAMI          = "ami-4ae27e22"
-	defaultInstanceType = "t2.micro"
-	defaultRootSize     = 16
-	ipRange             = "0.0.0.0/0"
-	dockerConfigDir     = "/etc/docker"
+	driverName               = "amazonec2"
+	defaultRegion            = "us-east-1"
+	defaultAMI               = "ami-4ae27e22"
+	defaultInstanceType      = "t2.micro"
+	defaultRootSize          = 16
+	ipRange                  = "0.0.0.0/0"
+	dockerConfigDir          = "/etc/docker"
+	machineSecurityGroupName = "docker-machine"
+	dockerPort               = 2376
 )
 
 type Driver struct {
-	Id              string
-	AccessKey       string
-	SecretKey       string
-	SessionToken    string
-	Region          string
-	AMI             string
-	SSHKeyID        int
-	KeyName         string
-	InstanceId      string
-	InstanceType    string
-	IPAddress       string
-	MachineName     string
-	SecurityGroupId string
-	ReservationId   string
-	RootSize        int64
-	VpcId           string
-	SubnetId        string
-	Zone            string
-	CaCertPath      string
-	PrivateKeyPath  string
-	storePath       string
-	keyPath         string
+	Id                string
+	AccessKey         string
+	SecretKey         string
+	SessionToken      string
+	Region            string
+	AMI               string
+	SSHKeyID          int
+	KeyName           string
+	InstanceId        string
+	InstanceType      string
+	IPAddress         string
+	MachineName       string
+	SecurityGroupName string
+	SecurityGroupId   string
+	ReservationId     string
+	RootSize          int64
+	VpcId             string
+	SubnetId          string
+	Zone              string
+	CaCertPath        string
+	PrivateKeyPath    string
+	storePath         string
+	keyPath           string
 }
 
 type CreateFlags struct {
@@ -120,6 +123,12 @@ func GetCreateFlags() []cli.Flag {
 			Value:  "",
 			EnvVar: "AWS_SUBNET_ID",
 		},
+		cli.StringFlag{
+			Name:   "amazonec2-security-group",
+			Usage:  "AWS VPC security group",
+			Value:  "docker-machine",
+			EnvVar: "AWS_SECURITY_GROUP",
+		},
 		cli.StringFlag{
 			Name:   "amazonec2-instance-type",
 			Usage:  "AWS instance type",
@@ -149,6 +158,7 @@ func (d *Driver) SetConfigFromFlags(flags drivers.DriverOptions) error {
 	d.InstanceType = flags.String("amazonec2-instance-type")
 	d.VpcId = flags.String("amazonec2-vpc-id")
 	d.SubnetId = flags.String("amazonec2-subnet-id")
+	d.SecurityGroupName = flags.String("amazonec2-security-group")
 	zone := flags.String("amazonec2-zone")
 	d.Zone = zone[:]
 	d.RootSize = int64(flags.Int("amazonec2-root-size"))
@@ -176,12 +186,11 @@ func (d *Driver) Create() error {
 	log.Infof("Launching instance...")
 
 	if err := d.createKeyPair(); err != nil {
-		fmt.Errorf("unable to create key pair: %s", err)
+		return fmt.Errorf("unable to create key pair: %s", err)
 	}
 
-	group, err := d.createSecurityGroup()
-	if err != nil {
-		log.Fatalf("Please make sure you don't have a security group named: %s", d.MachineName)
+	if err := d.configureSecurityGroup(d.SecurityGroupName); err != nil {
+		return err
 	}
 
 	bdm := &amz.BlockDeviceMapping{
@@ -215,7 +224,7 @@ func (d *Driver) Create() error {
 	}
 
 	log.Debugf("launching instance in subnet %s", subnetId)
-	instance, err := d.getClient().RunInstance(d.AMI, d.InstanceType, d.Zone, 1, 1, group.GroupId, d.KeyName, subnetId, bdm)
+	instance, err := d.getClient().RunInstance(d.AMI, d.InstanceType, d.Zone, 1, 1, d.SecurityGroupId, d.KeyName, subnetId, bdm)
 
 	if err != nil {
 		return fmt.Errorf("Error launching instance: %s", err)
@@ -282,7 +291,7 @@ func (d *Driver) GetURL() (string, error) {
 	if ip == "" {
 		return "", nil
 	}
-	return fmt.Sprintf("tcp://%s:2376", ip), nil
+	return fmt.Sprintf("tcp://%s:%d", ip, dockerPort), nil
 }
 
 func (d *Driver) GetIP() (string, error) {
@@ -341,21 +350,6 @@ func (d *Driver) Remove() error {
 	if err := d.terminate(); err != nil {
 		return fmt.Errorf("unable to terminate instance: %s", err)
 	}
-	// wait until terminated so we can remove security group
-	for {
-		st, err := d.GetState()
-		if err != nil {
-			break
-		}
-		if st == state.None {
-			break
-		}
-		time.Sleep(1 * time.Second)
-	}
-
-	if err := d.deleteSecurityGroup(); err != nil {
-		return fmt.Errorf("unable to remove security group: %s", err)
-	}
 
 	// remove keypair
 	if err := d.deleteKeyPair(); err != nil {
@@ -445,8 +439,21 @@ func (d *Driver) updateDriver() error {
 	if err != nil {
 		return err
 	}
-	d.InstanceId = inst.InstanceId
-	d.IPAddress = inst.IpAddress
+	// wait for ipaddress
+	for {
+		i, err := d.getInstance()
+		if err != nil {
+			return err
+		}
+		if i.IpAddress == "" {
+			time.Sleep(1 * time.Second)
+			continue
+		}
+
+		d.InstanceId = inst.InstanceId
+		d.IPAddress = inst.IpAddress
+		break
+	}
 	return nil
 }
 
@@ -518,39 +525,98 @@ func (d *Driver) terminate() error {
 	return nil
 }
 
-func (d *Driver) createSecurityGroup() (*amz.SecurityGroup, error) {
-	log.Debugf("creating security group in %s", d.VpcId)
+func (d *Driver) configureSecurityGroup(groupName string) error {
+	log.Debugf("configuring security group in %s", d.VpcId)
 
-	grpName := d.MachineName
-	group, err := d.getClient().CreateSecurityGroup(grpName, "Docker Machine", d.VpcId)
+	var securityGroup *amz.SecurityGroup
+
+	groups, err := d.getClient().GetSecurityGroups()
 	if err != nil {
-		return nil, err
+		return err
 	}
 
-	d.SecurityGroupId = group.GroupId
-
-	perms := []amz.IpPermission{
-		{
-			Protocol: "tcp",
-			FromPort: 22,
-			ToPort:   22,
-			IpRange:  ipRange,
-		},
-		{
-			Protocol: "tcp",
-			FromPort: 2376,
-			ToPort:   2376,
-			IpRange:  ipRange,
-		},
+	for _, grp := range groups {
+		if grp.GroupName == groupName {
+			log.Debugf("found existing security group (%s) in %s", groupName, d.VpcId)
+			securityGroup = &grp
+			break
+		}
 	}
 
-	log.Debugf("authorizing %s", ipRange)
-
-	if err := d.getClient().AuthorizeSecurityGroup(d.SecurityGroupId, perms); err != nil {
-		return nil, err
+	// if not found, create
+	if securityGroup == nil {
+		log.Debugf("creating security group (%s) in %s", groupName, d.VpcId)
+		group, err := d.getClient().CreateSecurityGroup(groupName, "Docker Machine", d.VpcId)
+		if err != nil {
+			return err
+		}
+		securityGroup = group
+		// wait until created (dat eventual consistency)
+		log.Debugf("waiting for group (%s) to become available", group.GroupId)
+		for {
+			_, err := d.getClient().GetSecurityGroupById(group.GroupId)
+			if err == nil {
+				break
+			}
+			log.Debug(err)
+			time.Sleep(1 * time.Second)
+		}
 	}
 
-	return group, nil
+	d.SecurityGroupId = securityGroup.GroupId
+
+	log.Debugf("configuring security group authorization for %s", ipRange)
+
+	perms := configureSecurityGroupPermissions(securityGroup)
+
+	if len(perms) != 0 {
+		log.Debugf("authorizing group %s with permissions: %v", securityGroup.GroupName, perms)
+		if err := d.getClient().AuthorizeSecurityGroup(d.SecurityGroupId, perms); err != nil {
+			return err
+		}
+
+	}
+
+	return nil
+}
+
+func configureSecurityGroupPermissions(group *amz.SecurityGroup) []amz.IpPermission {
+	hasSshPort := false
+	hasDockerPort := false
+	for _, p := range group.IpPermissions {
+		switch p.FromPort {
+		case 22:
+			hasSshPort = true
+		case dockerPort:
+			hasDockerPort = true
+		}
+	}
+
+	perms := []amz.IpPermission{}
+
+	if !hasSshPort {
+		perm := amz.IpPermission{
+			IpProtocol: "tcp",
+			FromPort:   22,
+			ToPort:     22,
+			IpRange:    ipRange,
+		}
+
+		perms = append(perms, perm)
+	}
+
+	if !hasDockerPort {
+		perm := amz.IpPermission{
+			IpProtocol: "tcp",
+			FromPort:   dockerPort,
+			ToPort:     dockerPort,
+			IpRange:    ipRange,
+		}
+
+		perms = append(perms, perm)
+	}
+
+	return perms
 }
 
 func (d *Driver) deleteSecurityGroup() error {

drivers/amazonec2/amazonec2_test.go

@@ -0,0 +1,94 @@
+package amazonec2
+
+import (
+	"testing"
+
+	"github.com/docker/machine/drivers/amazonec2/amz"
+)
+
+var (
+	securityGroup = amz.SecurityGroup{
+		GroupName: "test-group",
+		GroupId:   "12345",
+		VpcId:     "12345",
+	}
+)
+
+const (
+	testSshPort    = 22
+	testDockerPort = 2376
+)
+
+func TestConfigureSecurityGroupPermissionsEmpty(t *testing.T) {
+	group := securityGroup
+	perms := configureSecurityGroupPermissions(&group)
+	if len(perms) != 2 {
+		t.Fatalf("expected 2 permissions; received %d", len(perms))
+	}
+}
+
+func TestConfigureSecurityGroupPermissionsSshOnly(t *testing.T) {
+	group := securityGroup
+
+	group.IpPermissions = []amz.IpPermission{
+		{
+			IpProtocol: "tcp",
+			FromPort:   testSshPort,
+			ToPort:     testSshPort,
+		},
+	}
+
+	perms := configureSecurityGroupPermissions(&group)
+	if len(perms) != 1 {
+		t.Fatalf("expected 1 permission; received %d", len(perms))
+	}
+
+	receivedPort := perms[0].FromPort
+	if receivedPort != testDockerPort {
+		t.Fatalf("expected permission on port %d; received port %d", testDockerPort, receivedPort)
+	}
+}
+
+func TestConfigureSecurityGroupPermissionsDockerOnly(t *testing.T) {
+	group := securityGroup
+
+	group.IpPermissions = []amz.IpPermission{
+		{
+			IpProtocol: "tcp",
+			FromPort:   testDockerPort,
+			ToPort:     testDockerPort,
+		},
+	}
+
+	perms := configureSecurityGroupPermissions(&group)
+	if len(perms) != 1 {
+		t.Fatalf("expected 1 permission; received %d", len(perms))
+	}
+
+	receivedPort := perms[0].FromPort
+	if receivedPort != testSshPort {
+		t.Fatalf("expected permission on port %d; received port %d", testSshPort, receivedPort)
+	}
+}
+
+func TestConfigureSecurityGroupPermissionsDockerAndSsh(t *testing.T) {
+	group := securityGroup
+
+	group.IpPermissions = []amz.IpPermission{
+		{
+			IpProtocol: "tcp",
+			FromPort:   testSshPort,
+			ToPort:     testSshPort,
+		},
+		{
+			IpProtocol: "tcp",
+			FromPort:   testDockerPort,
+			ToPort:     testDockerPort,
+		},
+	}
+
+	perms := configureSecurityGroupPermissions(&group)
+	if len(perms) != 0 {
+		t.Fatalf("expected 0 permissions; received %d", len(perms))
+	}
+}

drivers/amazonec2/amz/describe_security_groups.go

@@ -1,7 +1,6 @@
 package amz
 
 type DescribeSecurityGroupsResponse struct {
-	RequestId         string `xml:"requestId"`
-	SecurityGroupInfo []struct {
-	} `xml:"securityGroupInfo>item"`
+	RequestId         string          `xml:"requestId"`
+	SecurityGroupInfo []SecurityGroup `xml:"securityGroupInfo>item"`
 }

drivers/amazonec2/amz/ec2.go

@@ -347,7 +347,7 @@ func (e *EC2) AuthorizeSecurityGroup(groupId string, permissions []IpPermission)
 
 	for index, perm := range permissions {
 		n := index + 1 // amazon starts counting from 1 not 0
-		v.Set(fmt.Sprintf("IpPermissions.%d.IpProtocol", n), perm.Protocol)
+		v.Set(fmt.Sprintf("IpPermissions.%d.IpProtocol", n), perm.IpProtocol)
 		v.Set(fmt.Sprintf("IpPermissions.%d.FromPort", n), strconv.Itoa(perm.FromPort))
 		v.Set(fmt.Sprintf("IpPermissions.%d.ToPort", n), strconv.Itoa(perm.ToPort))
 		v.Set(fmt.Sprintf("IpPermissions.%d.IpRanges.1.CidrIp", n), perm.IpRange)
@@ -380,6 +380,42 @@ func (e *EC2) DeleteSecurityGroup(groupId string) error {
 	return nil
 }
 
+func (e *EC2) GetSecurityGroups() ([]SecurityGroup, error) {
+	sgs := []SecurityGroup{}
+	resp, err := e.performStandardAction("DescribeSecurityGroups")
+	if err != nil {
+		return sgs, err
+	}
+	defer resp.Body.Close()
+	contents, err := ioutil.ReadAll(resp.Body)
+	if err != nil {
+		return sgs, fmt.Errorf("Error reading AWS response body: %s", err)
+	}
+
+	unmarshalledResponse := DescribeSecurityGroupsResponse{}
+	if err = xml.Unmarshal(contents, &unmarshalledResponse); err != nil {
+		return sgs, fmt.Errorf("Error unmarshalling AWS response XML: %s", err)
+	}
+
+	sgs = unmarshalledResponse.SecurityGroupInfo
+
+	return sgs, nil
+}
+
+func (e *EC2) GetSecurityGroupById(id string) (*SecurityGroup, error) {
+	groups, err := e.GetSecurityGroups()
+	if err != nil {
+		return nil, err
+	}
+
+	for _, g := range groups {
+		if g.GroupId == id {
+			return &g, nil
+		}
+	}
+	return nil, nil
+}
+
 func (e *EC2) GetSubnets() ([]Subnet, error) {
 	subnets := []Subnet{}
 	resp, err := e.performStandardAction("DescribeSubnets")
@@ -401,6 +437,7 @@ func (e *EC2) GetSubnets() ([]Subnet, error) {
 
 	return subnets, nil
 }
+
 func (e *EC2) GetInstanceState(instanceId string) (state.State, error) {
 	resp, err := e.performInstanceAction(instanceId, "DescribeInstances", nil)
 	if err != nil {

drivers/amazonec2/amz/ip_permission.go

@@ -1,8 +1,8 @@
 package amz
 
 type IpPermission struct {
-	Protocol string
-	FromPort int
-	ToPort   int
-	IpRange  string
+	IpProtocol string `xml:"ipProtocol"`
+	FromPort   int    `xml:"fromPort"`
+	ToPort     int    `xml:"toPort"`
+	IpRange    string `xml:"ipRanges"`
 }

drivers/amazonec2/amz/security_group.go

@@ -12,6 +12,10 @@ type DeleteSecurityGroupResponse struct {
 }
 
 type SecurityGroup struct {
-	GroupId string
-	VpcId   string
+	GroupName           string         `xml:"groupName"`
+	GroupId             string         `xml:"groupId"`
+	VpcId               string         `xml:"vpcId"`
+	OwnerId             string         `xml:"ownerId"`
+	IpPermissions       []IpPermission `xml:"ipPermissions>item,omitempty"`
+	IpPermissionsEgress []IpPermission `xml:"ipPermissionsEgress>item,omitempty"`
 }