ResiLien
/
terraform
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #11861 from hashicorp/b-fix-security-group-rule-regression 

provider/aws: Fix SecurityGroupRule regression
This commit is contained in:
Jake Champlin 2017-02-10 13:53:47 -05:00 committed by GitHub
parent 8271a739f2 4586b3727d
commit aa1c4cafa4
2 changed files with 223 additions and 208 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
builtin/providers/aws/resource_aws_security_group_rule.go
View File

@ -608,9 +608,10 @@ func validateAwsSecurityGroupRule(d *schema.ResourceData) error {
_, blocksOk := d.GetOk("cidr_blocks") _, blocksOk := d.GetOk("cidr_blocks")
_, sourceOk := d.GetOk("source_security_group_id") _, sourceOk := d.GetOk("source_security_group_id")
_, selfOk := d.GetOk("self") _, selfOk := d.GetOk("self")
if !blocksOk && !sourceOk && !selfOk { _, prefixOk := d.GetOk("prefix_list_ids")
if !blocksOk && !sourceOk && !selfOk && !prefixOk {
return fmt.Errorf( return fmt.Errorf(
"One of ['cidr_blocks', 'self', 'source_security_group_id'] must be set to create an AWS Security Group Rule") "One of ['cidr_blocks', 'self', 'source_security_group_id', 'prefix_list_ids'] must be set to create an AWS Security Group Rule")
} }
return nil return nil
} }

160
builtin/providers/aws/resource_aws_security_group_rule_test.go
View File

@ -4,6 +4,7 @@ import (
"bytes" "bytes"
"fmt" "fmt"
"log" "log"
"regexp"
"testing" "testing"
"github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws"
@ -12,7 +13,6 @@ import (
"github.com/hashicorp/terraform/helper/acctest" "github.com/hashicorp/terraform/helper/acctest"
"github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/helper/resource"
"github.com/hashicorp/terraform/terraform" "github.com/hashicorp/terraform/terraform"
"regexp"
) )
func TestIpPermissionIDHash(t *testing.T) { func TestIpPermissionIDHash(t *testing.T) {
@ -110,6 +110,7 @@ func TestIpPermissionIDHash(t *testing.T) {
func TestAccAWSSecurityGroupRule_Ingress_VPC(t *testing.T) { func TestAccAWSSecurityGroupRule_Ingress_VPC(t *testing.T) {
var group ec2.SecurityGroup var group ec2.SecurityGroup
rInt := acctest.RandInt()
testRuleCount := func(*terraform.State) error { testRuleCount := func(*terraform.State) error {
if len(group.IpPermissions) != 1 { if len(group.IpPermissions) != 1 {
@ -132,7 +133,7 @@ func TestAccAWSSecurityGroupRule_Ingress_VPC(t *testing.T) {
CheckDestroy: testAccCheckAWSSecurityGroupRuleDestroy, CheckDestroy: testAccCheckAWSSecurityGroupRuleDestroy,
Steps: []resource.TestStep{ Steps: []resource.TestStep{
{ {
Config: testAccAWSSecurityGroupRuleIngressConfig, Config: testAccAWSSecurityGroupRuleIngressConfig(rInt),
Check: resource.ComposeTestCheckFunc( Check: resource.ComposeTestCheckFunc(
testAccCheckAWSSecurityGroupRuleExists("aws_security_group.web", &group), testAccCheckAWSSecurityGroupRuleExists("aws_security_group.web", &group),
testAccCheckAWSSecurityGroupRuleAttributes("aws_security_group_rule.ingress_1", &group, nil, "ingress"), testAccCheckAWSSecurityGroupRuleAttributes("aws_security_group_rule.ingress_1", &group, nil, "ingress"),
@ -184,6 +185,7 @@ func TestAccAWSSecurityGroupRule_Ingress_Protocol(t *testing.T) {
func TestAccAWSSecurityGroupRule_Ingress_Classic(t *testing.T) { func TestAccAWSSecurityGroupRule_Ingress_Classic(t *testing.T) {
var group ec2.SecurityGroup var group ec2.SecurityGroup
rInt := acctest.RandInt()
testRuleCount := func(*terraform.State) error { testRuleCount := func(*terraform.State) error {
if len(group.IpPermissions) != 1 { if len(group.IpPermissions) != 1 {
@ -206,7 +208,7 @@ func TestAccAWSSecurityGroupRule_Ingress_Classic(t *testing.T) {
CheckDestroy: testAccCheckAWSSecurityGroupRuleDestroy, CheckDestroy: testAccCheckAWSSecurityGroupRuleDestroy,
Steps: []resource.TestStep{ Steps: []resource.TestStep{
{ {
Config: testAccAWSSecurityGroupRuleIngressClassicConfig, Config: testAccAWSSecurityGroupRuleIngressClassicConfig(rInt),
Check: resource.ComposeTestCheckFunc( Check: resource.ComposeTestCheckFunc(
testAccCheckAWSSecurityGroupRuleExists("aws_security_group.web", &group), testAccCheckAWSSecurityGroupRuleExists("aws_security_group.web", &group),
testAccCheckAWSSecurityGroupRuleAttributes("aws_security_group_rule.ingress_1", &group, nil, "ingress"), testAccCheckAWSSecurityGroupRuleAttributes("aws_security_group_rule.ingress_1", &group, nil, "ingress"),
@ -261,6 +263,7 @@ func TestAccAWSSecurityGroupRule_MultiIngress(t *testing.T) {
func TestAccAWSSecurityGroupRule_Egress(t *testing.T) { func TestAccAWSSecurityGroupRule_Egress(t *testing.T) {
var group ec2.SecurityGroup var group ec2.SecurityGroup
rInt := acctest.RandInt()
resource.Test(t, resource.TestCase{ resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) }, PreCheck: func() { testAccPreCheck(t) },
@ -268,7 +271,7 @@ func TestAccAWSSecurityGroupRule_Egress(t *testing.T) {
CheckDestroy: testAccCheckAWSSecurityGroupRuleDestroy, CheckDestroy: testAccCheckAWSSecurityGroupRuleDestroy,
Steps: []resource.TestStep{ Steps: []resource.TestStep{
{ {
Config: testAccAWSSecurityGroupRuleEgressConfig, Config: testAccAWSSecurityGroupRuleEgressConfig(rInt),
Check: resource.ComposeTestCheckFunc( Check: resource.ComposeTestCheckFunc(
testAccCheckAWSSecurityGroupRuleExists("aws_security_group.web", &group), testAccCheckAWSSecurityGroupRuleExists("aws_security_group.web", &group),
testAccCheckAWSSecurityGroupRuleAttributes("aws_security_group_rule.egress_1", &group, nil, "egress"), testAccCheckAWSSecurityGroupRuleAttributes("aws_security_group_rule.egress_1", &group, nil, "egress"),
@ -297,13 +300,14 @@ func TestAccAWSSecurityGroupRule_SelfReference(t *testing.T) {
} }
func TestAccAWSSecurityGroupRule_ExpectInvalidTypeError(t *testing.T) { func TestAccAWSSecurityGroupRule_ExpectInvalidTypeError(t *testing.T) {
rInt := acctest.RandInt()
resource.Test(t, resource.TestCase{ resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) }, PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders, Providers: testAccProviders,
CheckDestroy: testAccCheckAWSSecurityGroupRuleDestroy, CheckDestroy: testAccCheckAWSSecurityGroupRuleDestroy,
Steps: []resource.TestStep{ Steps: []resource.TestStep{
{ {
Config: testAccAWSSecurityGroupRuleExpectInvalidType, Config: testAccAWSSecurityGroupRuleExpectInvalidType(rInt),
ExpectError: regexp.MustCompile(`\\"type\\" contains an invalid Security Group Rule type \\"foobar\\"`), ExpectError: regexp.MustCompile(`\\"type\\" contains an invalid Security Group Rule type \\"foobar\\"`),
}, },
}, },
@ -313,6 +317,7 @@ func TestAccAWSSecurityGroupRule_ExpectInvalidTypeError(t *testing.T) {
// testing partial match implementation // testing partial match implementation
func TestAccAWSSecurityGroupRule_PartialMatching_basic(t *testing.T) { func TestAccAWSSecurityGroupRule_PartialMatching_basic(t *testing.T) {
var group ec2.SecurityGroup var group ec2.SecurityGroup
rInt := acctest.RandInt()
p := ec2.IpPermission{ p := ec2.IpPermission{
FromPort: aws.Int64(80), FromPort: aws.Int64(80),
@ -340,7 +345,7 @@ func TestAccAWSSecurityGroupRule_PartialMatching_basic(t *testing.T) {
CheckDestroy: testAccCheckAWSSecurityGroupRuleDestroy, CheckDestroy: testAccCheckAWSSecurityGroupRuleDestroy,
Steps: []resource.TestStep{ Steps: []resource.TestStep{
{ {
Config: testAccAWSSecurityGroupRulePartialMatching, Config: testAccAWSSecurityGroupRulePartialMatching(rInt),
Check: resource.ComposeTestCheckFunc( Check: resource.ComposeTestCheckFunc(
testAccCheckAWSSecurityGroupRuleExists("aws_security_group.web", &group), testAccCheckAWSSecurityGroupRuleExists("aws_security_group.web", &group),
testAccCheckAWSSecurityGroupRuleAttributes("aws_security_group_rule.ingress", &group, &p, "ingress"), testAccCheckAWSSecurityGroupRuleAttributes("aws_security_group_rule.ingress", &group, &p, "ingress"),
@ -356,6 +361,7 @@ func TestAccAWSSecurityGroupRule_PartialMatching_Source(t *testing.T) {
var group ec2.SecurityGroup var group ec2.SecurityGroup
var nat ec2.SecurityGroup var nat ec2.SecurityGroup
var p ec2.IpPermission var p ec2.IpPermission
rInt := acctest.RandInt()
// This function creates the expected IPPermission with the group id from an // This function creates the expected IPPermission with the group id from an
// external security group, needed because Security Group IDs are generated on // external security group, needed because Security Group IDs are generated on
@ -383,7 +389,7 @@ func TestAccAWSSecurityGroupRule_PartialMatching_Source(t *testing.T) {
CheckDestroy: testAccCheckAWSSecurityGroupRuleDestroy, CheckDestroy: testAccCheckAWSSecurityGroupRuleDestroy,
Steps: []resource.TestStep{ Steps: []resource.TestStep{
{ {
Config: testAccAWSSecurityGroupRulePartialMatching_Source, Config: testAccAWSSecurityGroupRulePartialMatching_Source(rInt),
Check: resource.ComposeTestCheckFunc( Check: resource.ComposeTestCheckFunc(
testAccCheckAWSSecurityGroupRuleExists("aws_security_group.web", &group), testAccCheckAWSSecurityGroupRuleExists("aws_security_group.web", &group),
testAccCheckAWSSecurityGroupRuleExists("aws_security_group.nat", &nat), testAccCheckAWSSecurityGroupRuleExists("aws_security_group.nat", &nat),
@ -433,6 +439,7 @@ func TestAccAWSSecurityGroupRule_Race(t *testing.T) {
func TestAccAWSSecurityGroupRule_SelfSource(t *testing.T) { func TestAccAWSSecurityGroupRule_SelfSource(t *testing.T) {
var group ec2.SecurityGroup var group ec2.SecurityGroup
rInt := acctest.RandInt()
resource.Test(t, resource.TestCase{ resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) }, PreCheck: func() { testAccPreCheck(t) },
@ -440,7 +447,7 @@ func TestAccAWSSecurityGroupRule_SelfSource(t *testing.T) {
CheckDestroy: testAccCheckAWSSecurityGroupRuleDestroy, CheckDestroy: testAccCheckAWSSecurityGroupRuleDestroy,
Steps: []resource.TestStep{ Steps: []resource.TestStep{
{ {
Config: testAccAWSSecurityGroupRuleSelfInSource, Config: testAccAWSSecurityGroupRuleSelfInSource(rInt),
Check: resource.ComposeTestCheckFunc( Check: resource.ComposeTestCheckFunc(
testAccCheckAWSSecurityGroupRuleExists("aws_security_group.web", &group), testAccCheckAWSSecurityGroupRuleExists("aws_security_group.web", &group),
), ),
@ -667,17 +674,18 @@ func testAccCheckAWSSecurityGroupRuleAttributes(n string, group *ec2.SecurityGro
} }
} }
const testAccAWSSecurityGroupRuleIngressConfig = ` func testAccAWSSecurityGroupRuleIngressConfig(rInt int) string {
resource "aws_security_group" "web" { return fmt.Sprintf(`
name = "terraform_acceptance_test_example" resource "aws_security_group" "web" {
name = "terraform_test_%d"
description = "Used in the terraform acceptance tests" description = "Used in the terraform acceptance tests"
tags { tags {
Name = "tf-acc-test" Name = "tf-acc-test"
} }
} }
resource "aws_security_group_rule" "ingress_1" { resource "aws_security_group_rule" "ingress_1" {
type = "ingress" type = "ingress"
protocol = "tcp" protocol = "tcp"
from_port = 80 from_port = 80
@ -685,8 +693,8 @@ resource "aws_security_group_rule" "ingress_1" {
cidr_blocks = ["10.0.0.0/8"] cidr_blocks = ["10.0.0.0/8"]
security_group_id = "${aws_security_group.web.id}" security_group_id = "${aws_security_group.web.id}"
}`, rInt)
} }
`
const testAccAWSSecurityGroupRuleIngress_protocolConfig = ` const testAccAWSSecurityGroupRuleIngress_protocolConfig = `
resource "aws_vpc" "tftest" { resource "aws_vpc" "tftest" {
@ -737,21 +745,22 @@ resource "aws_security_group_rule" "issue_5310" {
} }
` `
const testAccAWSSecurityGroupRuleIngressClassicConfig = ` func testAccAWSSecurityGroupRuleIngressClassicConfig(rInt int) string {
provider "aws" { return fmt.Sprintf(`
provider "aws" {
region = "us-east-1" region = "us-east-1"
} }
resource "aws_security_group" "web" { resource "aws_security_group" "web" {
name = "terraform_acceptance_test_example" name = "terraform_test_%d"
description = "Used in the terraform acceptance tests" description = "Used in the terraform acceptance tests"
tags { tags {
Name = "tf-acc-test" Name = "tf-acc-test"
} }
} }
resource "aws_security_group_rule" "ingress_1" { resource "aws_security_group_rule" "ingress_1" {
type = "ingress" type = "ingress"
protocol = "tcp" protocol = "tcp"
from_port = 80 from_port = 80
@ -759,20 +768,21 @@ resource "aws_security_group_rule" "ingress_1" {
cidr_blocks = ["10.0.0.0/8"] cidr_blocks = ["10.0.0.0/8"]
security_group_id = "${aws_security_group.web.id}" security_group_id = "${aws_security_group.web.id}"
}`, rInt)
} }
`
const testAccAWSSecurityGroupRuleEgressConfig = ` func testAccAWSSecurityGroupRuleEgressConfig(rInt int) string {
resource "aws_security_group" "web" { return fmt.Sprintf(`
name = "terraform_acceptance_test_example" resource "aws_security_group" "web" {
name = "terraform_test_%d"
description = "Used in the terraform acceptance tests" description = "Used in the terraform acceptance tests"
tags { tags {
Name = "tf-acc-test" Name = "tf-acc-test"
} }
} }
resource "aws_security_group_rule" "egress_1" { resource "aws_security_group_rule" "egress_1" {
type = "egress" type = "egress"
protocol = "tcp" protocol = "tcp"
from_port = 80 from_port = 80
@ -780,8 +790,8 @@ resource "aws_security_group_rule" "egress_1" {
cidr_blocks = ["10.0.0.0/8"] cidr_blocks = ["10.0.0.0/8"]
security_group_id = "${aws_security_group.web.id}" security_group_id = "${aws_security_group.web.id}"
}`, rInt)
} }
`
const testAccAWSSecurityGroupRuleConfigMultiIngress = ` const testAccAWSSecurityGroupRuleConfigMultiIngress = `
resource "aws_security_group" "web" { resource "aws_security_group" "web" {
@ -847,31 +857,32 @@ resource "aws_security_group_rule" "self" {
} }
` `
const testAccAWSSecurityGroupRulePartialMatching = ` func testAccAWSSecurityGroupRulePartialMatching(rInt int) string {
resource "aws_vpc" "default" { return fmt.Sprintf(`
resource "aws_vpc" "default" {
cidr_block = "10.0.0.0/16" cidr_block = "10.0.0.0/16"
tags { tags {
Name = "tf-sg-rule-bug" Name = "tf-sg-rule-bug"
} }
} }
resource "aws_security_group" "web" { resource "aws_security_group" "web" {
name = "tf-other" name = "tf-other-%d"
vpc_id = "${aws_vpc.default.id}" vpc_id = "${aws_vpc.default.id}"
tags { tags {
Name = "tf-other-sg" Name = "tf-other-sg"
} }
} }
resource "aws_security_group" "nat" { resource "aws_security_group" "nat" {
name = "tf-nat" name = "tf-nat-%d"
vpc_id = "${aws_vpc.default.id}" vpc_id = "${aws_vpc.default.id}"
tags { tags {
Name = "tf-nat-sg" Name = "tf-nat-sg"
} }
} }
resource "aws_security_group_rule" "ingress" { resource "aws_security_group_rule" "ingress" {
type = "ingress" type = "ingress"
from_port = 80 from_port = 80
to_port = 80 to_port = 80
@ -879,9 +890,9 @@ resource "aws_security_group_rule" "ingress" {
cidr_blocks = ["10.0.2.0/24", "10.0.3.0/24", "10.0.4.0/24"] cidr_blocks = ["10.0.2.0/24", "10.0.3.0/24", "10.0.4.0/24"]
security_group_id = "${aws_security_group.web.id}" security_group_id = "${aws_security_group.web.id}"
} }
resource "aws_security_group_rule" "other" { resource "aws_security_group_rule" "other" {
type = "ingress" type = "ingress"
from_port = 80 from_port = 80
to_port = 80 to_port = 80
@ -889,10 +900,10 @@ resource "aws_security_group_rule" "other" {
cidr_blocks = ["10.0.5.0/24"] cidr_blocks = ["10.0.5.0/24"]
security_group_id = "${aws_security_group.web.id}" security_group_id = "${aws_security_group.web.id}"
} }
// same a above, but different group, to guard against bad hashing // same a above, but different group, to guard against bad hashing
resource "aws_security_group_rule" "nat_ingress" { resource "aws_security_group_rule" "nat_ingress" {
type = "ingress" type = "ingress"
from_port = 80 from_port = 80
to_port = 80 to_port = 80
@ -900,34 +911,35 @@ resource "aws_security_group_rule" "nat_ingress" {
cidr_blocks = ["10.0.2.0/24", "10.0.3.0/24", "10.0.4.0/24"] cidr_blocks = ["10.0.2.0/24", "10.0.3.0/24", "10.0.4.0/24"]
security_group_id = "${aws_security_group.nat.id}" security_group_id = "${aws_security_group.nat.id}"
}`, rInt, rInt)
} }
`
const testAccAWSSecurityGroupRulePartialMatching_Source = ` func testAccAWSSecurityGroupRulePartialMatching_Source(rInt int) string {
resource "aws_vpc" "default" { return fmt.Sprintf(`
resource "aws_vpc" "default" {
cidr_block = "10.0.0.0/16" cidr_block = "10.0.0.0/16"
tags { tags {
Name = "tf-sg-rule-bug" Name = "tf-sg-rule-bug"
} }
} }
resource "aws_security_group" "web" { resource "aws_security_group" "web" {
name = "tf-other" name = "tf-other-%d"
vpc_id = "${aws_vpc.default.id}" vpc_id = "${aws_vpc.default.id}"
tags { tags {
Name = "tf-other-sg" Name = "tf-other-sg"
} }
} }
resource "aws_security_group" "nat" { resource "aws_security_group" "nat" {
name = "tf-nat" name = "tf-nat-%d"
vpc_id = "${aws_vpc.default.id}" vpc_id = "${aws_vpc.default.id}"
tags { tags {
Name = "tf-nat-sg" Name = "tf-nat-sg"
} }
} }
resource "aws_security_group_rule" "source_ingress" { resource "aws_security_group_rule" "source_ingress" {
type = "ingress" type = "ingress"
from_port = 80 from_port = 80
to_port = 80 to_port = 80
@ -935,9 +947,9 @@ resource "aws_security_group_rule" "source_ingress" {
source_security_group_id = "${aws_security_group.nat.id}" source_security_group_id = "${aws_security_group.nat.id}"
security_group_id = "${aws_security_group.web.id}" security_group_id = "${aws_security_group.web.id}"
} }
resource "aws_security_group_rule" "other_ingress" { resource "aws_security_group_rule" "other_ingress" {
type = "ingress" type = "ingress"
from_port = 80 from_port = 80
to_port = 80 to_port = 80
@ -945,8 +957,8 @@ resource "aws_security_group_rule" "other_ingress" {
cidr_blocks = ["10.0.2.0/24", "10.0.3.0/24", "10.0.4.0/24"] cidr_blocks = ["10.0.2.0/24", "10.0.3.0/24", "10.0.4.0/24"]
security_group_id = "${aws_security_group.web.id}" security_group_id = "${aws_security_group.web.id}"
}`, rInt, rInt)
} }
`
var testAccAWSSecurityGroupRuleRace = func() string { var testAccAWSSecurityGroupRuleRace = func() string {
var b bytes.Buffer var b bytes.Buffer
@ -1035,52 +1047,54 @@ resource "aws_security_group_rule" "egress_1" {
} }
` `
const testAccAWSSecurityGroupRuleSelfInSource = ` func testAccAWSSecurityGroupRuleSelfInSource(rInt int) string {
resource "aws_vpc" "foo" { return fmt.Sprintf(`
resource "aws_vpc" "foo" {
cidr_block = "10.1.0.0/16" cidr_block = "10.1.0.0/16"
tags { tags {
Name = "tf_sg_rule_self_group" Name = "tf_sg_rule_self_group"
} }
} }
resource "aws_security_group" "web" { resource "aws_security_group" "web" {
name = "allow_all" name = "allow_all-%d"
description = "Allow all inbound traffic" description = "Allow all inbound traffic"
vpc_id = "${aws_vpc.foo.id}" vpc_id = "${aws_vpc.foo.id}"
} }
resource "aws_security_group_rule" "allow_self" { resource "aws_security_group_rule" "allow_self" {
type = "ingress" type = "ingress"
from_port = 0 from_port = 0
to_port = 0 to_port = 0
protocol = "-1" protocol = "-1"
security_group_id = "${aws_security_group.web.id}" security_group_id = "${aws_security_group.web.id}"
source_security_group_id = "${aws_security_group.web.id}" source_security_group_id = "${aws_security_group.web.id}"
}`, rInt)
} }
`
const testAccAWSSecurityGroupRuleExpectInvalidType = ` func testAccAWSSecurityGroupRuleExpectInvalidType(rInt int) string {
resource "aws_vpc" "foo" { return fmt.Sprintf(`
resource "aws_vpc" "foo" {
cidr_block = "10.1.0.0/16" cidr_block = "10.1.0.0/16"
tags { tags {
Name = "tf_sg_rule_self_group" Name = "tf_sg_rule_self_group"
} }
} }
resource "aws_security_group" "web" { resource "aws_security_group" "web" {
name = "allow_all" name = "allow_all-%d"
description = "Allow all inbound traffic" description = "Allow all inbound traffic"
vpc_id = "${aws_vpc.foo.id}" vpc_id = "${aws_vpc.foo.id}"
} }
resource "aws_security_group_rule" "allow_self" { resource "aws_security_group_rule" "allow_self" {
type = "foobar" type = "foobar"
from_port = 0 from_port = 0
to_port = 0 to_port = 0
protocol = "-1" protocol = "-1"
security_group_id = "${aws_security_group.web.id}" security_group_id = "${aws_security_group.web.id}"
source_security_group_id = "${aws_security_group.web.id}" source_security_group_id = "${aws_security_group.web.id}"
}`, rInt)
} }
`