From 85b1756c27e33840d27b53c401e6a7baa027fbcc Mon Sep 17 00:00:00 2001 From: Clint Shryock Date: Fri, 1 May 2015 14:56:16 -0500 Subject: [PATCH] revise tests and check for vpc_id --- .../aws/resource_aws_security_group.go | 11 ++- .../aws/resource_aws_security_group_test.go | 92 +++++++++++++++++++ 2 files changed, 99 insertions(+), 4 deletions(-) diff --git a/builtin/providers/aws/resource_aws_security_group.go b/builtin/providers/aws/resource_aws_security_group.go index 034b41ab3..aa4d85b3d 100644 --- a/builtin/providers/aws/resource_aws_security_group.go +++ b/builtin/providers/aws/resource_aws_security_group.go @@ -146,9 +146,9 @@ func resourceAwsSecurityGroup() *schema.Resource { func resourceAwsSecurityGroupCreate(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).ec2conn - securityGroupOpts := &ec2.CreateSecurityGroupInput{} + securityGroupOpts := &ec2.CreateSecurityGroupInput{} - if v := d.Get("vpc_id"); v != nil { + if v, ok := d.GetOk("vpc_id"); ok { if len(d.Get("egress").(*schema.Set).List()) == 0 { return fmt.Errorf("Error creating Security Group: Security groups inside a VPC require an egress rule. See http://terraform.io/docs/providers/aws/r/security_group.html for more information.") } @@ -189,7 +189,9 @@ func resourceAwsSecurityGroupCreate(d *schema.ResourceData, meta interface{}) er Refresh: SGStateRefreshFunc(conn, d.Id()), Timeout: 1 * time.Minute, } - if _, err := stateConf.WaitForState(); err != nil { + + resp, err := stateConf.WaitForState() + if err != nil { return fmt.Errorf( "Error waiting for Security Group (%s) to become available: %s", d.Id(), err) @@ -197,7 +199,8 @@ func resourceAwsSecurityGroupCreate(d *schema.ResourceData, meta interface{}) er // AWS defaults all Security Groups to have an ALLOW ALL egress rule. Here we // revoke that rule, so users don't unknowningly have/use it. - if v := d.Get("vpc_id"); v != nil { + group := resp.(*ec2.SecurityGroup) + if group.VPCID != nil && *group.VPCID != "" { log.Printf("[DEBUG] Revoking default egress rule for Security Group for %s", d.Id()) req := &ec2.RevokeSecurityGroupEgressInput{ diff --git a/builtin/providers/aws/resource_aws_security_group_test.go b/builtin/providers/aws/resource_aws_security_group_test.go index 7ad1d97b2..165f94029 100644 --- a/builtin/providers/aws/resource_aws_security_group_test.go +++ b/builtin/providers/aws/resource_aws_security_group_test.go @@ -216,6 +216,7 @@ func TestAccAWSSecurityGroup_generatedName(t *testing.T) { func TestAccAWSSecurityGroup_DefaultEgress(t *testing.T) { + // VPC resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, @@ -229,6 +230,22 @@ func TestAccAWSSecurityGroup_DefaultEgress(t *testing.T) { }, }, }) + + // Classic + var group ec2.SecurityGroup + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSSecurityGroupDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccAWSSecurityGroupConfigClassic, + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSSecurityGroupExists("aws_security_group.web", &group), + ), + }, + }, + }) } func testAccCheckAWSSecurityGroupDestroy(s *terraform.State) error { @@ -455,6 +472,13 @@ resource "aws_security_group" "web" { cidr_blocks = ["10.0.0.0/8"] } + egress { + protocol = "tcp" + from_port = 80 + to_port = 8000 + cidr_blocks = ["10.0.0.0/8"] + } + tags { Name = "tf-acc-test" } @@ -479,6 +503,13 @@ resource "aws_security_group" "web" { to_port = 8000 cidr_blocks = ["0.0.0.0/0", "10.0.0.0/8"] } + + egress { + protocol = "tcp" + from_port = 80 + to_port = 8000 + cidr_blocks = ["10.0.0.0/8"] + } } ` @@ -493,6 +524,13 @@ resource "aws_security_group" "web" { to_port = 8000 self = true } + + egress { + protocol = "tcp" + from_port = 80 + to_port = 8000 + cidr_blocks = ["10.0.0.0/8"] + } } ` @@ -533,6 +571,13 @@ resource "aws_security_group" "worker" { to_port = 8000 cidr_blocks = ["10.0.0.0/8"] } + + egress { + protocol = "tcp" + from_port = 80 + to_port = 8000 + cidr_blocks = ["10.0.0.0/8"] + } } resource "aws_security_group" "web" { @@ -559,6 +604,13 @@ resource "aws_security_group" "web" { to_port = 8000 security_groups = ["${aws_security_group.worker.id}"] } + + egress { + protocol = "tcp" + from_port = 80 + to_port = 8000 + cidr_blocks = ["10.0.0.0/8"] + } } ` @@ -574,6 +626,13 @@ resource "aws_security_group" "foo" { cidr_blocks = ["10.0.0.0/8"] } + egress { + protocol = "tcp" + from_port = 80 + to_port = 8000 + cidr_blocks = ["10.0.0.0/8"] + } + tags { foo = "bar" } @@ -592,6 +651,13 @@ resource "aws_security_group" "foo" { cidr_blocks = ["10.0.0.0/8"] } + egress { + protocol = "tcp" + from_port = 80 + to_port = 8000 + cidr_blocks = ["10.0.0.0/8"] + } + tags { bar = "baz" } @@ -607,6 +673,13 @@ resource "aws_security_group" "web" { cidr_blocks = ["10.0.0.0/8"] } + egress { + protocol = "tcp" + from_port = 80 + to_port = 8000 + cidr_blocks = ["10.0.0.0/8"] + } + tags { Name = "tf-acc-test" } @@ -614,9 +687,17 @@ resource "aws_security_group" "web" { ` const testAccAWSSecurityGroupConfigDefaultEgress = ` +resource "aws_vpc" "tf_sg_egress_test" { + cidr_block = "10.0.0.0/16" + tags { + Name = "tf_sg_egress_test" + } +} + resource "aws_security_group" "worker" { name = "terraform_acceptance_test_example_1" description = "Used in the terraform acceptance tests" + vpc_id = "${aws_vpc.tf_sg_egress_test.id}" egress { protocol = "tcp" @@ -626,3 +707,14 @@ resource "aws_security_group" "worker" { } } ` + +const testAccAWSSecurityGroupConfigClassic = ` +provider "aws" { + region = "us-east-1" +} + +resource "aws_security_group" "web" { + name = "terraform_acceptance_test_example_1" + description = "Used in the terraform acceptance tests" +} +`