Various fixes for ec2 provider create command

The create command failed when a default VPC was not
present for the ec2 provider. When the failure occurred there
was also a side effect of dangling security groups that
got created but not cleaned up in the event of failure.

Two additional arguments were added, vpc-id and subnet-id,
to mitigate the failure while also giving users the option
to choose a specific VPC and subnet if more than one VPC
exists. New validation checks that if one of these new arguments
are specified then the other must be as well.

Additional logic was added so that in the event of ec2
instance creation failure any security groups created
will get deleted.

Signed-off-by: Shane Witbeck <[email protected]>

Author: thesurlydev

cmd/create.go

@@ -35,6 +35,8 @@ func init() {
 	createCmd.Flags().StringP("access-token", "a", "", "The access token for your cloud")
 	createCmd.Flags().StringP("access-token-file", "f", "", "Read this file for the access token for your cloud")
 
+	createCmd.Flags().String("vpc-id", "", "The VPC ID to create the exit-node in (EC2)")
+	createCmd.Flags().String("subnet-id", "", "The Subnet ID where the exit-node should be placed (EC2)")
 	createCmd.Flags().String("secret-key", "", "The access token for your cloud (Scaleway, EC2)")
 	createCmd.Flags().String("secret-key-file", "", "Read this file for the access token for your cloud (Scaleway, EC2)")
 	createCmd.Flags().String("organisation-id", "", "Organisation ID (Scaleway)")
@@ -131,6 +133,8 @@ func runCreate(cmd *cobra.Command, _ []string) error {
 	var secretKey string
 	var organisationID string
 	var projectID string
+	var vpcID string
+	var subnetID string
 	if provider == "scaleway" || provider == "ec2" {
 
 		var secretKeyErr error
@@ -145,6 +149,23 @@ func runCreate(cmd *cobra.Command, _ []string) error {
 				return fmt.Errorf("--organisation-id flag must be set")
 			}
 		}
+
+		if provider == "ec2" {
+			vpcID, err = cmd.Flags().GetString("vpc-id")
+			if err != nil {
+				return errors.Wrap(err, "failed to get 'vpc-id' value")
+			}
+
+			subnetID, err = cmd.Flags().GetString("subnet-id")
+			if err != nil {
+				return errors.Wrap(err, "failed to get 'subnet-id' value")
+			}
+
+			if (len(vpcID) == 0 && len(subnetID) > 0) || (len(subnetID) == 0 && len(vpcID) > 0) {
+				return fmt.Errorf("both --vpc-id and --subnet-id must be set")
+			}
+		}
+
 	} else if provider == "gce" || provider == "packet" {
 		projectID, _ = cmd.Flags().GetString("project-id")
 		if len(projectID) == 0 {
@@ -173,7 +194,7 @@ func runCreate(cmd *cobra.Command, _ []string) error {
 
 	userData := makeUserdata(inletsToken, inletsControlPort, remoteTCP)
 
-	hostReq, err := createHost(provider, name, region, zone, projectID, userData, strconv.Itoa(inletsControlPort), pro)
+	hostReq, err := createHost(provider, name, region, zone, projectID, userData, strconv.Itoa(inletsControlPort), vpcID, subnetID, pro)
 	if err != nil {
 		return err
 	}
@@ -274,7 +295,7 @@ func generateAuth() (string, error) {
 	return pwdRes, pwdErr
 }
 
-func createHost(provider, name, region, zone, projectID, userData, inletsPort string, pro bool) (*provision.BasicHost, error) {
+func createHost(provider, name, region, zone, projectID, userData, inletsPort string, vpcID string, subnetID string, pro bool) (*provision.BasicHost, error) {
 	if provider == "digitalocean" {
 		return &provision.BasicHost{
 			Name:       name,
@@ -331,16 +352,27 @@ func createHost(provider, name, region, zone, projectID, userData, inletsPort st
 	} else if provider == "ec2" {
 		// Ubuntu images can be found here https://cloud-images.ubuntu.com/locator/ec2/
 		// Name is used in the OS field so the ami can be lookup up in the region specified
+
+		var additional = map[string]string{
+			"inlets-port": inletsPort,
+			"pro":         fmt.Sprint(pro),
+		}
+
+		if len(vpcID) > 0 {
+			additional["vpc-id"] = vpcID
+		}
+
+		if len(subnetID) > 0 {
+			additional["subnet-id"] = subnetID
+		}
+
 		return &provision.BasicHost{
-			Name:     name,
-			OS:       "ubuntu/images/hvm-ssd/ubuntu-xenial-16.04-amd64-server-20191114",
-			Plan:     "t3.nano",
-			Region:   region,
-			UserData: base64.StdEncoding.EncodeToString([]byte(userData)),
-			Additional: map[string]string{
-				"inlets-port": inletsPort,
-				"pro":         fmt.Sprint(pro),
-			},
+			Name:       name,
+			OS:         "ubuntu/images/hvm-ssd/ubuntu-xenial-16.04-amd64-server-20191114",
+			Plan:       "t3.nano",
+			Region:     region,
+			UserData:   base64.StdEncoding.EncodeToString([]byte(userData)),
+			Additional: additional,
 		}, nil
 	} else if provider == "azure" {
 		// Ubuntu images can be found here https://docs.microsoft.com/en-us/azure/virtual-machines/linux/cli-ps-findimage#list-popular-images

go.sum

@@ -109,6 +109,7 @@ github.com/google/go-cmp v0.3.0 h1:crn/baboCvb5fXaQ0IJ1SGTsTVrWpDsCWC8EGETZijY=
 github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU=
 github.com/google/go-cmp v0.4.0 h1:xsAVV57WRhGj6kEIi8ReJzQlHHqcBYCElAvkovg3B/4=
 github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
+github.com/google/go-cmp v0.5.0 h1:/QaMHBdZ26BB3SSst0Iwl10Epc+xhTquomWX0oZEB6w=
 github.com/google/go-cmp v0.5.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/go-querystring v1.0.0 h1:Xkwi/a1rcvNg1PPYe5vI8GbeBY/jrVuDX5ASuANWTrk=
 github.com/google/go-querystring v1.0.0/go.mod h1:odCYkC5MyYFN7vkCjXpyrEuKhc/BUO6wN/zVPAxq5ck=

pkg/provision/ec2.go

@@ -40,27 +40,44 @@ func (p *EC2Provisioner) Provision(host BasicHost) (*ProvisionedHost, error) {
 	}
 	pro := host.Additional["pro"]
 
-	groupID, name, err := p.creteEC2SecurityGroup(port, pro)
+	var vpcID = host.Additional["vpc-id"]
+	var subnetID = host.Additional["subnet-id"]
+
+	groupID, name, err := p.createEC2SecurityGroup(vpcID, port, pro)
 	if err != nil {
 		return nil, err
 	}
 
+	var networkSpec = ec2.InstanceNetworkInterfaceSpecification{
+		DeviceIndex:              aws.Int64(int64(0)),
+		AssociatePublicIpAddress: aws.Bool(true),
+		DeleteOnTermination:      aws.Bool(true),
+		Groups:                   []*string{groupID},
+	}
+
+	if len(subnetID) > 0 {
+		networkSpec.SubnetId = aws.String(subnetID)
+	}
+
 	runResult, err := p.ec2Provisioner.RunInstances(&ec2.RunInstancesInput{
 		ImageId:      image,
 		InstanceType: aws.String(host.Plan),
 		MinCount:     aws.Int64(1),
 		MaxCount:     aws.Int64(1),
 		UserData:     &host.UserData,
 		NetworkInterfaces: []*ec2.InstanceNetworkInterfaceSpecification{
-			{
-				DeviceIndex:              aws.Int64(int64(0)),
-				AssociatePublicIpAddress: aws.Bool(true),
-				DeleteOnTermination:      aws.Bool(true),
-				Groups:                   []*string{groupID},
-			},
+			&networkSpec,
 		},
 	})
 	if err != nil {
+		// clean up SG if there was an issue provisioning the EC2 instance
+		input := ec2.DeleteSecurityGroupInput {
+			GroupId:   groupID,
+		}
+		_, sgErr := p.ec2Provisioner.DeleteSecurityGroup(&input)
+		if sgErr != nil {
+			return nil, fmt.Errorf("error provisioning ec2 instance: %v; error deleting SG: %v", err, sgErr)
+		}
 		return nil, err
 	}
 
@@ -229,15 +246,21 @@ func (p *EC2Provisioner) lookupID(request HostDeleteRequest) (string, error) {
 	return "", fmt.Errorf("no host with ip: %s", request.IP)
 }
 
-// creteEC2SecurityGroup creates a security group for the exit-node
-func (p *EC2Provisioner) creteEC2SecurityGroup(controlPort int, pro string) (*string, *string, error) {
+// createEC2SecurityGroup creates a security group for the exit-node
+func (p *EC2Provisioner) createEC2SecurityGroup(vpcID string, controlPort int, pro string) (*string, *string, error) {
 	ports := []int{80, 443, controlPort}
 	proPorts := []int{1024, 65535}
 	groupName := "inlets-" + uuid.New().String()
-	group, err := p.ec2Provisioner.CreateSecurityGroup(&ec2.CreateSecurityGroupInput{
+	var input = &ec2.CreateSecurityGroupInput{
 		Description: aws.String("inlets security group"),
 		GroupName:   aws.String(groupName),
-	})
+	}
+
+	if len(vpcID) > 0 {
+		input.VpcId = aws.String(vpcID)
+	}
+
+	group, err := p.ec2Provisioner.CreateSecurityGroup(input)
 	if err != nil {
 		return nil, nil, err
 	}