From 89bacc0b154838f20c9184d4745085121e8ddf5d Mon Sep 17 00:00:00 2001
From: Christopher Tiwald <christiwald@gmail.com>
Date: Mon, 4 May 2015 23:43:31 -0400
Subject: [PATCH] aws: error on expndIPPerms(...) if our ports and protocol
 conflict.

Ingress and egress rules given a "-1" protocol don't have ports when
Read out of AWS. This results in hashing problems, as a local
config file might contain port declarations AWS can't ever return.

Rather than making ports optional fields, which carries with it a huge
headache trying to distinguish between zero-value attributes (e.g.
'to_port = 0') and attributes that are simply omitted, simply force the
user to opt-in when using the "-1" protocol. If they choose to use it,
they must now specify "0" for both to_port and from_port. Any other
configuration will error.
---
 .../aws/resource_aws_security_group.go        | 10 +++++++--
 builtin/providers/aws/structure.go            | 21 +++++++++++++++----
 2 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/builtin/providers/aws/resource_aws_security_group.go b/builtin/providers/aws/resource_aws_security_group.go
index 044d0afaa..63bb3439f 100644
--- a/builtin/providers/aws/resource_aws_security_group.go
+++ b/builtin/providers/aws/resource_aws_security_group.go
@@ -406,8 +406,14 @@ func resourceAwsSecurityGroupUpdateRules(
 		os := o.(*schema.Set)
 		ns := n.(*schema.Set)
 
-		remove := expandIPPerms(group, os.Difference(ns).List())
-		add := expandIPPerms(group, ns.Difference(os).List())
+		remove, err := expandIPPerms(group, os.Difference(ns).List())
+		if err != nil {
+			return err
+		}
+		add, err := expandIPPerms(group, ns.Difference(os).List())
+		if err != nil {
+			return err
+		}
 
 		// TODO: We need to handle partial state better in the in-between
 		// in this update.
diff --git a/builtin/providers/aws/structure.go b/builtin/providers/aws/structure.go
index 533780037..269734aa7 100644
--- a/builtin/providers/aws/structure.go
+++ b/builtin/providers/aws/structure.go
@@ -42,10 +42,12 @@ func expandListeners(configured []interface{}) ([]*elb.Listener, error) {
 	return listeners, nil
 }
 
-// Takes the result of flatmap.Expand for an array of ingress/egress
-// security group rules and returns EC2 API compatible objects
+// Takes the result of flatmap.Expand for an array of ingress/egress security
+// group rules and returns EC2 API compatible objects. This function will error
+// if it finds invalid permissions input, namely a protocol of "-1" with either
+// to_port or from_port set to a non-zero value.
 func expandIPPerms(
-	group *ec2.SecurityGroup, configured []interface{}) []*ec2.IPPermission {
+	group *ec2.SecurityGroup, configured []interface{}) ([]*ec2.IPPermission, error) {
 	vpc := group.VPCID != nil
 
 	perms := make([]*ec2.IPPermission, len(configured))
@@ -57,6 +59,17 @@ func expandIPPerms(
 		perm.ToPort = aws.Long(int64(m["to_port"].(int)))
 		perm.IPProtocol = aws.String(m["protocol"].(string))
 
+		// When protocol is "-1", AWS won't store any ports for the
+		// rule, but also won't error if the user specifies ports other
+		// than '0'. Force the user to make a deliberate '0' port
+		// choice when specifying a "-1" protocol, and tell them about
+		// AWS's behavior in the error message.
+		if *perm.IPProtocol == "-1" && (*perm.FromPort != 0 || *perm.ToPort != 0) {
+			return nil, fmt.Errorf(
+				"from_port (%d) and to_port (%d) must both be 0 to use the the 'ALL' \"-1\" protocol!",
+				*perm.FromPort, *perm.ToPort)
+		}
+
 		var groups []string
 		if raw, ok := m["security_groups"]; ok {
 			list := raw.(*schema.Set).List()
@@ -102,7 +115,7 @@ func expandIPPerms(
 		perms[i] = &perm
 	}
 
-	return perms
+	return perms, nil
 }
 
 // Takes the result of flatmap.Expand for an array of parameters and