From 2503f0b01d0531f5811d412d7cfa51d2409b153c Mon Sep 17 00:00:00 2001
From: Joe Topjian <joe@topjian.net>
Date: Wed, 30 Dec 2015 18:22:24 +0000
Subject: [PATCH] provider/openstack: Ensure valid Security Group Rule
 attribute combination

This commit ensures that a valid combination of security group rule attributes
is set before creating the security group.
---
 .../resource_openstack_compute_secgroup_v2.go | 43 +++++++++++++++++++
 .../r/compute_secgroup_v2.html.markdown       | 17 ++++----
 2 files changed, 52 insertions(+), 8 deletions(-)

diff --git a/builtin/providers/openstack/resource_openstack_compute_secgroup_v2.go b/builtin/providers/openstack/resource_openstack_compute_secgroup_v2.go
index e3d281b2e117..556208e92ac0 100644
--- a/builtin/providers/openstack/resource_openstack_compute_secgroup_v2.go
+++ b/builtin/providers/openstack/resource_openstack_compute_secgroup_v2.go
@@ -93,6 +93,12 @@ func resourceComputeSecGroupV2Create(d *schema.ResourceData, meta interface{}) e
 		return fmt.Errorf("Error creating OpenStack compute client: %s", err)
 	}
 
+	// Before creating the security group, make sure all rules are valid.
+	if err := checkSecGroupV2RulesForErrors(d); err != nil {
+		return err
+	}
+
+	// If all rules are valid, proceed with creating the security gruop.
 	createOpts := secgroups.CreateOpts{
 		Name:        d.Get("name").(string),
 		Description: d.Get("description").(string),
@@ -106,6 +112,7 @@ func resourceComputeSecGroupV2Create(d *schema.ResourceData, meta interface{}) e
 
 	d.SetId(sg.ID)
 
+	// Now that the security group has been created, iterate through each rule and create it
 	createRuleOptsList := resourceSecGroupRulesV2(d)
 	for _, createRuleOpts := range createRuleOptsList {
 		_, err := secgroups.CreateRule(computeClient, createRuleOpts).Extract()
@@ -251,6 +258,42 @@ func resourceSecGroupRuleCreateOptsV2(d *schema.ResourceData, rawRule interface{
 	}
 }
 
+func checkSecGroupV2RulesForErrors(d *schema.ResourceData) error {
+	rawRules := d.Get("rule").(*schema.Set).List()
+	for _, rawRule := range rawRules {
+		rawRuleMap := rawRule.(map[string]interface{})
+
+		// only one of cidr, from_group_id, or self can be set
+		cidr := rawRuleMap["cidr"].(string)
+		groupId := rawRuleMap["from_group_id"].(string)
+		self := rawRuleMap["self"].(bool)
+		errorMessage := fmt.Errorf("Only one of cidr, from_group_id, or self can be set.")
+
+		// if cidr is set, from_group_id and self cannot be set
+		if cidr != "" {
+			if groupId != "" || self {
+				return errorMessage
+			}
+		}
+
+		// if from_group_id is set, cidr and self cannot be set
+		if groupId != "" {
+			if cidr != "" || self {
+				return errorMessage
+			}
+		}
+
+		// if self is set, cidr and from_group_id cannot be set
+		if self {
+			if cidr != "" || groupId != "" {
+				return errorMessage
+			}
+		}
+	}
+
+	return nil
+}
+
 func resourceSecGroupRuleV2(d *schema.ResourceData, rawRule interface{}) secgroups.Rule {
 	rawRuleMap := rawRule.(map[string]interface{})
 	return secgroups.Rule{
diff --git a/website/source/docs/providers/openstack/r/compute_secgroup_v2.html.markdown b/website/source/docs/providers/openstack/r/compute_secgroup_v2.html.markdown
index e7d88ead76b8..2005c9aea02b 100644
--- a/website/source/docs/providers/openstack/r/compute_secgroup_v2.html.markdown
+++ b/website/source/docs/providers/openstack/r/compute_secgroup_v2.html.markdown
@@ -62,17 +62,18 @@ range to open. Changing this creates a new security group rule.
 * `ip_protocol` - (Required) The protocol type that will be allowed. Changing
 this creates a new security group rule.
 
-* `cidr` - (Optional) Required if `from_group_id` is empty. The IP range that
-will be the source of network traffic to the security group. Use 0.0.0.0./0
-to allow all IP addresses. Changing this creates a new security group rule.
+* `cidr` - (Optional) Required if `from_group_id` or `self` is empty. The IP range
+that will be the source of network traffic to the security group. Use 0.0.0.0/0
+to allow all IP addresses. Changing this creates a new security group rule. Cannot
+be combined with `from_group_id` or `self`.
 
-* `from_group_id` - (Optional) Required if `cidr` is empty. The ID of a group
-from which to forward traffic to the parent group. Changing
-this creates a new security group rule.
+* `from_group_id` - (Optional) Required if `cidr` or `self` is empty. The ID of a
+group from which to forward traffic to the parent group. Changing this creates a
+new security group rule. Cannot be combined with `cidr` or `self`.
 
 * `self` - (Optional) Required if `cidr` and `from_group_id` is empty. If true,
-the security group itself will be added as a source to this ingress rule. `cidr`
-and `from_group_id` will be ignored if either are set while `self` is true.
+the security group itself will be added as a source to this ingress rule. Cannot
+be combined with `cidr` or `from_group_id`.
 
 ## Attributes Reference