provider/cloudstack: enhance security groups and rules (#9645)

* govendor: update go-cloudstack dependency * Separate security groups and rules This commit separates the creation and management of security groups and security group rules. It extends the `icmp` options so you can supply `icmp_type` and `icmp_code` to enbale more specific configs. And it adds lifecycle management of security group rules, so that security groups do not have to be recreated when rules are added or removed. This is particulary helpful since the `cloudstack_instance` cannot update a security group without having to recreate the instance. In CloudStack >= 4.9.0 it is possible to update security groups of existing instances, but as that is just added to the latest version it seems a bit too soon to start using this (causing backwards incompatibility issues for people or service providers running older versions). * Add and update documentation * Add acceptance tests
hashicorp · Oct 27, 2016 · 1619a81 · 1619a81
1 parent 07d10da
commit 1619a81
Show file tree

Hide file tree

Showing 15 changed files with 1,377 additions and 328 deletions.
diff --git a/builtin/providers/cloudstack/provider.go b/builtin/providers/cloudstack/provider.go
@@ -55,6 +55,7 @@ func Provider() terraform.ResourceProvider {
 			"cloudstack_port_forward":         resourceCloudStackPortForward(),
 			"cloudstack_secondary_ipaddress":  resourceCloudStackSecondaryIPAddress(),
 			"cloudstack_security_group":       resourceCloudStackSecurityGroup(),
+			"cloudstack_security_group_rule":  resourceCloudStackSecurityGroupRule(),
 			"cloudstack_ssh_keypair":          resourceCloudStackSSHKeyPair(),
 			"cloudstack_static_nat":           resourceCloudStackStaticNAT(),
 			"cloudstack_template":             resourceCloudStackTemplate(),

diff --git a/builtin/providers/cloudstack/resource_cloudstack_instance.go b/builtin/providers/cloudstack/resource_cloudstack_instance.go
@@ -391,7 +391,8 @@ func resourceCloudStackInstanceUpdate(d *schema.ResourceData, meta interface{})
 	}
 
 	// Attributes that require reboot to update
-	if d.HasChange("name") || d.HasChange("service_offering") || d.HasChange("affinity_group_ids") || d.HasChange("affinity_group_names") || d.HasChange("keypair") || d.HasChange("user_data") {
+	if d.HasChange("name") || d.HasChange("service_offering") || d.HasChange("affinity_group_ids") ||
+		d.HasChange("affinity_group_names") || d.HasChange("keypair") || d.HasChange("user_data") {
 		// Before we can actually make these changes, the virtual machine must be stopped
 		_, err := cs.VirtualMachine.StopVirtualMachine(
 			cs.VirtualMachine.NewStopVirtualMachineParams(d.Id()))
@@ -453,7 +454,16 @@ func resourceCloudStackInstanceUpdate(d *schema.ResourceData, meta interface{})
 				}
 			}
 
+			// Set the new groups
 			p.SetAffinitygroupids(groups)
+
+			// Update the affinity groups
+			_, err = cs.AffinityGroup.UpdateVMAffinityGroup(p)
+			if err != nil {
+				return fmt.Errorf(
+					"Error updating the affinity groups for instance %s: %s", name, err)
+			}
+			d.SetPartial("affinity_group_ids")
 		}
 
 		// Check if the affinity group names have changed and if so, update the names
@@ -467,7 +477,16 @@ func resourceCloudStackInstanceUpdate(d *schema.ResourceData, meta interface{})
 				}
 			}
 
-			p.SetAffinitygroupids(groups)
+			// Set the new groups
+			p.SetAffinitygroupnames(groups)
+
+			// Update the affinity groups
+			_, err = cs.AffinityGroup.UpdateVMAffinityGroup(p)
+			if err != nil {
+				return fmt.Errorf(
+					"Error updating the affinity groups for instance %s: %s", name, err)
+			}
+			d.SetPartial("affinity_group_names")
 		}
 
 		// Check if the keypair has changed and if so, update the keypair
@@ -514,6 +533,7 @@ func resourceCloudStackInstanceUpdate(d *schema.ResourceData, meta interface{})
 	}
 
 	d.Partial(false)
+
 	return resourceCloudStackInstanceRead(d, meta)
 }
 

diff --git a/builtin/providers/cloudstack/resource_cloudstack_network_acl_rule.go b/builtin/providers/cloudstack/resource_cloudstack_network_acl_rule.go
@@ -519,11 +519,7 @@ func resourceCloudStackNetworkACLRuleDelete(d *schema.ResourceData, meta interfa
 	return nil
 }
 
-func deleteNetworkACLRules(
-	d *schema.ResourceData,
-	meta interface{},
-	rules *schema.Set,
-	ors *schema.Set) error {
+func deleteNetworkACLRules(d *schema.ResourceData, meta interface{}, rules *schema.Set, ors *schema.Set) error {
 	var errs *multierror.Error
 
 	var wg sync.WaitGroup
@@ -559,10 +555,7 @@ func deleteNetworkACLRules(
 	return errs.ErrorOrNil()
 }
 
-func deleteNetworkACLRule(
-	d *schema.ResourceData,
-	meta interface{},
-	rule map[string]interface{}) error {
+func deleteNetworkACLRule(d *schema.ResourceData, meta interface{}, rule map[string]interface{}) error {
 	cs := meta.(*cloudstack.CloudStackClient)
 	uuids := rule["uuids"].(map[string]interface{})
 

diff --git a/builtin/providers/cloudstack/resource_cloudstack_security_group.go b/builtin/providers/cloudstack/resource_cloudstack_security_group.go
@@ -3,7 +3,6 @@ package cloudstack
 import (
 	"fmt"
 	"log"
-	"strconv"
 	"strings"
 
 	"github.com/hashicorp/terraform/helper/schema"
@@ -33,47 +32,9 @@ func resourceCloudStackSecurityGroup() *schema.Resource {
 			"project": &schema.Schema{
 				Type:     schema.TypeString,
 				Optional: true,
+				Computed: true,
 				ForceNew: true,
 			},
-
-			"rules": &schema.Schema{
-				Type:     schema.TypeList,
-				ForceNew: true,
-				Optional: true,
-				Elem: &schema.Resource{
-					Schema: map[string]*schema.Schema{
-						"cidr_list": &schema.Schema{
-							Type:     schema.TypeString,
-							Optional: true,
-							ForceNew: true,
-						},
-
-						"security_group": &schema.Schema{
-							Type:     schema.TypeString,
-							Optional: true,
-							ForceNew: true,
-						},
-
-						"protocol": &schema.Schema{
-							Type:     schema.TypeString,
-							Required: true,
-							ForceNew: true,
-						},
-
-						"ports": &schema.Schema{
-							Type:     schema.TypeString,
-							Optional: true,
-							ForceNew: true,
-						},
-
-						"traffic_type": &schema.Schema{
-							Type:     schema.TypeString,
-							Optional: true,
-							Default:  "ingress",
-						},
-					},
-				},
-			},
 		},
 	}
 }
@@ -98,132 +59,21 @@ func resourceCloudStackSecurityGroupCreate(d *schema.ResourceData, meta interfac
 		return err
 	}
 
-	log.Printf("[DEBUG] Creating security group %s", name)
 	r, err := cs.SecurityGroup.CreateSecurityGroup(p)
 	if err != nil {
-		return err
+		return fmt.Errorf("Error creating security group %s: %s", name, err)
 	}
 
-	log.Printf("[DEBUG] Security group %s successfully created with ID: %s", name, r.Id)
 	d.SetId(r.Id)
 
-	// Create Rules
-	rules := d.Get("rules").([]interface{})
-	for _, r := range rules {
-
-		rule := r.(map[string]interface{})
-
-		m := splitPorts.FindStringSubmatch(rule["ports"].(string))
-		startPort, err := strconv.Atoi(m[1])
-		if err != nil {
-			return err
-		}
-
-		endPort := startPort
-		if m[2] != "" {
-			endPort, err = strconv.Atoi(m[2])
-			if err != nil {
-				return err
-			}
-		}
-
-		traffic := rule["traffic_type"].(string)
-		if traffic == "ingress" {
-			param := cs.SecurityGroup.NewAuthorizeSecurityGroupIngressParams()
-
-			param.SetSecuritygroupid(d.Id())
-			if cidrlist := rule["cidr_list"]; cidrlist != "" {
-				param.SetCidrlist([]string{cidrlist.(string)})
-				log.Printf("[DEBUG] cidr = %v", cidrlist)
-			}
-			if securitygroup := rule["security_group"]; securitygroup != "" {
-				// Get the security group details
-				ag, count, err := cs.SecurityGroup.GetSecurityGroupByName(
-					securitygroup.(string),
-					cloudstack.WithProject(d.Get("project").(string)),
-				)
-				if err != nil {
-					if count == 0 {
-						log.Printf("[DEBUG] Security group %s does not longer exist", d.Get("name").(string))
-						d.SetId("")
-						return nil
-					}
-
-					log.Printf("[DEBUG] Found %v groups matching", count)
-
-					return err
-				}
-				log.Printf("[DEBUG] ag = %v", ag)
-				m := make(map[string]string)
-				m[ag.Account] = ag.Name
-				log.Printf("[DEBUG] m = %v", m)
-				param.SetUsersecuritygrouplist(m)
-			}
-			param.SetStartport(startPort)
-			param.SetEndport(endPort)
-			param.SetProtocol(rule["protocol"].(string))
-
-			log.Printf("[DEBUG] Authorizing Ingress Rule %#v", param)
-			_, err := cs.SecurityGroup.AuthorizeSecurityGroupIngress(param)
-			if err != nil {
-				return err
-			}
-		} else if traffic == "egress" {
-			param := cs.SecurityGroup.NewAuthorizeSecurityGroupEgressParams()
-
-			param.SetSecuritygroupid(d.Id())
-			if cidrlist := rule["cidr_list"]; cidrlist != "" {
-				param.SetCidrlist([]string{cidrlist.(string)})
-				log.Printf("[DEBUG] cidr = %v", cidrlist)
-			}
-			if securitygroup := rule["security_group"]; securitygroup != "" {
-				// Get the security group details
-				ag, count, err := cs.SecurityGroup.GetSecurityGroupByName(
-					securitygroup.(string),
-					cloudstack.WithProject(d.Get("project").(string)),
-				)
-				if err != nil {
-					if count == 0 {
-						log.Printf("[DEBUG] Security group %s does not longer exist", d.Get("name").(string))
-						d.SetId("")
-						return nil
-					}
-
-					log.Printf("[DEBUG] Found %v groups matching", count)
-
-					return err
-				}
-				log.Printf("[DEBUG] ag = %v", ag)
-				m := make(map[string]string)
-				m[ag.Account] = ag.Name
-				log.Printf("[DEBUG] m = %v", m)
-				param.SetUsersecuritygrouplist(m)
-			}
-			param.SetStartport(startPort)
-			param.SetEndport(endPort)
-			param.SetProtocol(rule["protocol"].(string))
-
-			log.Printf("[DEBUG] Authorizing Egress Rule %#v", param)
-			_, err := cs.SecurityGroup.AuthorizeSecurityGroupEgress(param)
-			if err != nil {
-				return err
-			}
-		} else {
-			return fmt.Errorf(
-				"Parameter traffic_type only accepts 'ingress' or 'egress' as values")
-		}
-	}
-
 	return resourceCloudStackSecurityGroupRead(d, meta)
 }
 
 func resourceCloudStackSecurityGroupRead(d *schema.ResourceData, meta interface{}) error {
 	cs := meta.(*cloudstack.CloudStackClient)
 
-	log.Printf("[DEBUG] Retrieving security group %s (ID=%s)", d.Get("name").(string), d.Id())
-
 	// Get the security group details
-	ag, count, err := cs.SecurityGroup.GetSecurityGroupByID(
+	sg, count, err := cs.SecurityGroup.GetSecurityGroupByID(
 		d.Id(),
 		cloudstack.WithProject(d.Get("project").(string)),
 	)
@@ -234,49 +84,14 @@ func resourceCloudStackSecurityGroupRead(d *schema.ResourceData, meta interface{
 			return nil
 		}
 
-		log.Printf("[DEBUG] Found %v groups matching", count)
-
 		return err
 	}
 
 	// Update the config
-	d.Set("name", ag.Name)
-	d.Set("description", ag.Description)
+	d.Set("name", sg.Name)
+	d.Set("description", sg.Description)
 
-	var rules []interface{}
-	for _, r := range ag.Ingressrule {
-		var ports string
-		if r.Startport == r.Endport {
-			ports = strconv.Itoa(r.Startport)
-		} else {
-			ports = fmt.Sprintf("%v-%v", r.Startport, r.Endport)
-		}
-		rule := map[string]interface{}{
-			"cidr_list":      r.Cidr,
-			"ports":          ports,
-			"protocol":       r.Protocol,
-			"traffic_type":   "ingress",
-			"security_group": r.Securitygroupname,
-		}
-		rules = append(rules, rule)
-	}
-	for _, r := range ag.Egressrule {
-		var ports string
-		if r.Startport == r.Endport {
-			ports = strconv.Itoa(r.Startport)
-		} else {
-			ports = fmt.Sprintf("%s-%s", r.Startport, r.Endport)
-		}
-		rule := map[string]interface{}{
-			"cidr_list":      r.Cidr,
-			"ports":          ports,
-			"protocol":       r.Protocol,
-			"traffic_type":   "egress",
-			"security_group": r.Securitygroupname,
-		}
-		rules = append(rules, rule)
-	}
-	d.Set("rules", rules)
+	setValueOrID(d, "project", sg.Project, sg.Projectid)
 
 	return nil
 }