Merge branch 'pr-5939'

* pr-5939:
  Update acc tests
  provider/aws: Allow `aws_redshift_security_group` ingress rules to change

builtin/providers/aws/resource_aws_redshift_security_group.go

@@ -20,6 +20,7 @@ func resourceAwsRedshiftSecurityGroup() *schema.Resource {
 	return &schema.Resource{
 		Create: resourceAwsRedshiftSecurityGroupCreate,
 		Read:   resourceAwsRedshiftSecurityGroupRead,
+		Update: resourceAwsRedshiftSecurityGroupUpdate,
 		Delete: resourceAwsRedshiftSecurityGroupDelete,
 
 		Schema: map[string]*schema.Schema{
@@ -40,7 +41,6 @@ func resourceAwsRedshiftSecurityGroup() *schema.Resource {
 			"ingress": &schema.Schema{
 				Type:     schema.TypeSet,
 				Required: true,
-				ForceNew: true,
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
 						"cidr": &schema.Schema{
@@ -151,6 +151,55 @@ func resourceAwsRedshiftSecurityGroupRead(d *schema.ResourceData, meta interface
 	return nil
 }
 
+func resourceAwsRedshiftSecurityGroupUpdate(d *schema.ResourceData, meta interface{}) error {
+	conn := meta.(*AWSClient).redshiftconn
+
+	if d.HasChange("ingress") {
+		o, n := d.GetChange("ingress")
+		if o == nil {
+			o = new(schema.Set)
+		}
+		if n == nil {
+			n = new(schema.Set)
+		}
+
+		os := o.(*schema.Set)
+		ns := n.(*schema.Set)
+
+		removeIngressRules, err := expandRedshiftSGRevokeIngress(os.Difference(ns).List())
+		if err != nil {
+			return err
+		}
+		if len(removeIngressRules) > 0 {
+			for _, r := range removeIngressRules {
+				r.ClusterSecurityGroupName = aws.String(d.Id())
+
+				_, err := conn.RevokeClusterSecurityGroupIngress(&r)
+				if err != nil {
+					return err
+				}
+			}
+		}
+
+		addIngressRules, err := expandRedshiftSGAuthorizeIngress(ns.Difference(os).List())
+		if err != nil {
+			return err
+		}
+		if len(addIngressRules) > 0 {
+			for _, r := range addIngressRules {
+				r.ClusterSecurityGroupName = aws.String(d.Id())
+
+				_, err := conn.AuthorizeClusterSecurityGroupIngress(&r)
+				if err != nil {
+					return err
+				}
+			}
+		}
+
+	}
+	return resourceAwsRedshiftSecurityGroupRead(d, meta)
+}
+
 func resourceAwsRedshiftSecurityGroupDelete(d *schema.ResourceData, meta interface{}) error {
 	conn := meta.(*AWSClient).redshiftconn
 
@@ -290,3 +339,59 @@ func resourceAwsRedshiftSecurityGroupStateRefreshFunc(
 		return v, "authorized", nil
 	}
 }
+
+func expandRedshiftSGAuthorizeIngress(configured []interface{}) ([]redshift.AuthorizeClusterSecurityGroupIngressInput, error) {
+	var ingress []redshift.AuthorizeClusterSecurityGroupIngressInput
+
+	// Loop over our configured parameters and create
+	// an array of aws-sdk-go compatabile objects
+	for _, pRaw := range configured {
+		data := pRaw.(map[string]interface{})
+
+		i := redshift.AuthorizeClusterSecurityGroupIngressInput{}
+
+		if v, ok := data["cidr"]; ok {
+			i.CIDRIP = aws.String(v.(string))
+		}
+
+		if v, ok := data["security_group_name"]; ok {
+			i.EC2SecurityGroupName = aws.String(v.(string))
+		}
+
+		if v, ok := data["security_group_owner_id"]; ok {
+			i.EC2SecurityGroupOwnerId = aws.String(v.(string))
+		}
+
+		ingress = append(ingress, i)
+	}
+
+	return ingress, nil
+}
+
+func expandRedshiftSGRevokeIngress(configured []interface{}) ([]redshift.RevokeClusterSecurityGroupIngressInput, error) {
+	var ingress []redshift.RevokeClusterSecurityGroupIngressInput
+
+	// Loop over our configured parameters and create
+	// an array of aws-sdk-go compatabile objects
+	for _, pRaw := range configured {
+		data := pRaw.(map[string]interface{})
+
+		i := redshift.RevokeClusterSecurityGroupIngressInput{}
+
+		if v, ok := data["cidr"]; ok {
+			i.CIDRIP = aws.String(v.(string))
+		}
+
+		if v, ok := data["security_group_name"]; ok {
+			i.EC2SecurityGroupName = aws.String(v.(string))
+		}
+
+		if v, ok := data["security_group_owner_id"]; ok {
+			i.EC2SecurityGroupOwnerId = aws.String(v.(string))
+		}
+
+		ingress = append(ingress, i)
+	}
+
+	return ingress, nil
+}

builtin/providers/aws/resource_aws_redshift_security_group_test.go

@@ -37,6 +37,44 @@ func TestAccAWSRedshiftSecurityGroup_ingressCidr(t *testing.T) {
 	})
 }
 
+func TestAccAWSRedshiftSecurityGroup_updateIngressCidr(t *testing.T) {
+	var v redshift.ClusterSecurityGroup
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckAWSRedshiftSecurityGroupDestroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: testAccAWSRedshiftSecurityGroupConfig_ingressCidr,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSRedshiftSecurityGroupExists("aws_redshift_security_group.bar", &v),
+					resource.TestCheckResourceAttr(
+						"aws_redshift_security_group.bar", "ingress.#", "1"),
+				),
+			},
+
+			resource.TestStep{
+				Config: testAccAWSRedshiftSecurityGroupConfig_ingressCidrAdd,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSRedshiftSecurityGroupExists("aws_redshift_security_group.bar", &v),
+					resource.TestCheckResourceAttr(
+						"aws_redshift_security_group.bar", "ingress.#", "3"),
+				),
+			},
+
+			resource.TestStep{
+				Config: testAccAWSRedshiftSecurityGroupConfig_ingressCidrReduce,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSRedshiftSecurityGroupExists("aws_redshift_security_group.bar", &v),
+					resource.TestCheckResourceAttr(
+						"aws_redshift_security_group.bar", "ingress.#", "2"),
+				),
+			},
+		},
+	})
+}
+
 func TestAccAWSRedshiftSecurityGroup_ingressSecurityGroup(t *testing.T) {
 	var v redshift.ClusterSecurityGroup
 
@@ -63,6 +101,44 @@ func TestAccAWSRedshiftSecurityGroup_ingressSecurityGroup(t *testing.T) {
 	})
 }
 
+func TestAccAWSRedshiftSecurityGroup_updateIngressSecurityGroup(t *testing.T) {
+	var v redshift.ClusterSecurityGroup
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckAWSRedshiftSecurityGroupDestroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: testAccAWSRedshiftSecurityGroupConfig_ingressSgId,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSRedshiftSecurityGroupExists("aws_redshift_security_group.bar", &v),
+					resource.TestCheckResourceAttr(
+						"aws_redshift_security_group.bar", "ingress.#", "1"),
+				),
+			},
+
+			resource.TestStep{
+				Config: testAccAWSRedshiftSecurityGroupConfig_ingressSgIdAdd,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSRedshiftSecurityGroupExists("aws_redshift_security_group.bar", &v),
+					resource.TestCheckResourceAttr(
+						"aws_redshift_security_group.bar", "ingress.#", "3"),
+				),
+			},
+
+			resource.TestStep{
+				Config: testAccAWSRedshiftSecurityGroupConfig_ingressSgIdReduce,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSRedshiftSecurityGroupExists("aws_redshift_security_group.bar", &v),
+					resource.TestCheckResourceAttr(
+						"aws_redshift_security_group.bar", "ingress.#", "2"),
+				),
+			},
+		},
+	})
+}
+
 func testAccCheckAWSRedshiftSecurityGroupExists(n string, v *redshift.ClusterSecurityGroup) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		rs, ok := s.RootModule().Resources[n]
@@ -176,6 +252,46 @@ resource "aws_redshift_security_group" "bar" {
     }
 }`
 
+const testAccAWSRedshiftSecurityGroupConfig_ingressCidrAdd = `
+provider "aws" {
+    region = "us-east-1"
+}
+
+resource "aws_redshift_security_group" "bar" {
+    name = "redshift-sg-terraform"
+    description = "this is a description"
+
+    ingress {
+        cidr = "10.0.0.1/24"
+    }
+
+    ingress {
+        cidr = "10.0.10.1/24"
+    }
+
+    ingress {
+        cidr = "10.0.20.1/24"
+    }
+}`
+
+const testAccAWSRedshiftSecurityGroupConfig_ingressCidrReduce = `
+provider "aws" {
+    region = "us-east-1"
+}
+
+resource "aws_redshift_security_group" "bar" {
+    name = "redshift-sg-terraform"
+    description = "this is a description"
+
+    ingress {
+        cidr = "10.0.0.1/24"
+    }
+
+    ingress {
+        cidr = "10.0.10.1/24"
+    }
+}`
+
 const testAccAWSRedshiftSecurityGroupConfig_ingressSgId = `
 provider "aws" {
     region = "us-east-1"
@@ -202,3 +318,108 @@ resource "aws_redshift_security_group" "bar" {
         security_group_owner_id = "${aws_security_group.redshift.owner_id}"
     }
 }`
+
+const testAccAWSRedshiftSecurityGroupConfig_ingressSgIdAdd = `
+provider "aws" {
+    region = "us-east-1"
+}
+
+resource "aws_security_group" "redshift" {
+	name = "terraform_redshift_acceptance_test"
+	description = "Used in the redshift acceptance tests"
+
+	ingress {
+		protocol = "tcp"
+		from_port = 22
+		to_port = 22
+		cidr_blocks = ["10.0.0.0/16"]
+	}
+}
+
+resource "aws_security_group" "redshift2" {
+	name = "terraform_redshift_acceptance_test_2"
+	description = "Used in the redshift acceptance tests #2"
+
+	ingress {
+		protocol = "tcp"
+		from_port = 22
+		to_port = 22
+		cidr_blocks = ["10.1.0.0/16"]
+	}
+}
+
+resource "aws_security_group" "redshift3" {
+	name = "terraform_redshift_acceptance_test_3"
+	description = "Used in the redshift acceptance tests #3"
+
+	ingress {
+		protocol = "tcp"
+		from_port = 22
+		to_port = 22
+		cidr_blocks = ["10.2.0.0/16"]
+	}
+}
+
+resource "aws_redshift_security_group" "bar" {
+    name = "redshift-sg-terraform"
+    description = "this is a description"
+
+    ingress {
+        security_group_name = "${aws_security_group.redshift.name}"
+        security_group_owner_id = "${aws_security_group.redshift.owner_id}"
+    }
+
+    ingress {
+        security_group_name = "${aws_security_group.redshift2.name}"
+        security_group_owner_id = "${aws_security_group.redshift.owner_id}"
+    }
+
+    ingress {
+        security_group_name = "${aws_security_group.redshift3.name}"
+        security_group_owner_id = "${aws_security_group.redshift.owner_id}"
+    }
+}`
+
+const testAccAWSRedshiftSecurityGroupConfig_ingressSgIdReduce = `
+provider "aws" {
+    region = "us-east-1"
+}
+
+resource "aws_security_group" "redshift" {
+	name = "terraform_redshift_acceptance_test"
+	description = "Used in the redshift acceptance tests"
+
+	ingress {
+		protocol = "tcp"
+		from_port = 22
+		to_port = 22
+		cidr_blocks = ["10.0.0.0/16"]
+	}
+}
+
+resource "aws_security_group" "redshift2" {
+	name = "terraform_redshift_acceptance_test_2"
+	description = "Used in the redshift acceptance tests #2"
+
+	ingress {
+		protocol = "tcp"
+		from_port = 22
+		to_port = 22
+		cidr_blocks = ["10.1.0.0/16"]
+	}
+}
+
+resource "aws_redshift_security_group" "bar" {
+    name = "redshift-sg-terraform"
+    description = "this is a description"
+
+    ingress {
+        security_group_name = "${aws_security_group.redshift.name}"
+        security_group_owner_id = "${aws_security_group.redshift.owner_id}"
+    }
+
+    ingress {
+        security_group_name = "${aws_security_group.redshift2.name}"
+        security_group_owner_id = "${aws_security_group.redshift.owner_id}"
+    }
+}`