provider/aws: `aws_security_group` now creates tags as early as possible (#7849)

in the process

Fixes #7577

7577 discovered that sometimes setting tags at the end of the creation
model doesn't quite work for everyone. We now move that further up the
tree by calling the setTags func a second time.

The setTags func in the Update is not called immediately after creation
as we check for it not being a NewResource

```
% make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSSecurityGroup_'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
TF_ACC=1 go test ./builtin/providers/aws -v
-run=TestAccAWSSecurityGroup_ -timeout 120m
=== RUN   TestAccAWSSecurityGroup_importBasic
--- PASS: TestAccAWSSecurityGroup_importBasic (60.96s)
=== RUN   TestAccAWSSecurityGroup_importSelf
--- PASS: TestAccAWSSecurityGroup_importSelf (72.72s)
=== RUN   TestAccAWSSecurityGroup_basic
--- PASS: TestAccAWSSecurityGroup_basic (62.33s)
=== RUN   TestAccAWSSecurityGroup_namePrefix
--- PASS: TestAccAWSSecurityGroup_namePrefix (22.12s)
=== RUN   TestAccAWSSecurityGroup_self
--- PASS: TestAccAWSSecurityGroup_self (64.26s)
=== RUN   TestAccAWSSecurityGroup_vpc
--- PASS: TestAccAWSSecurityGroup_vpc (58.35s)
=== RUN   TestAccAWSSecurityGroup_vpcNegOneIngress
--- PASS: TestAccAWSSecurityGroup_vpcNegOneIngress (54.95s)
=== RUN   TestAccAWSSecurityGroup_MultiIngress
--- PASS: TestAccAWSSecurityGroup_MultiIngress (64.81s)
=== RUN   TestAccAWSSecurityGroup_Change
--- PASS: TestAccAWSSecurityGroup_Change (96.86s)
=== RUN   TestAccAWSSecurityGroup_generatedName
--- PASS: TestAccAWSSecurityGroup_generatedName (60.75s)
=== RUN   TestAccAWSSecurityGroup_DefaultEgress_VPC
--- PASS: TestAccAWSSecurityGroup_DefaultEgress_VPC (57.05s)
=== RUN   TestAccAWSSecurityGroup_DefaultEgress_Classic
--- PASS: TestAccAWSSecurityGroup_DefaultEgress_Classic (20.94s)
=== RUN   TestAccAWSSecurityGroup_drift
--- PASS: TestAccAWSSecurityGroup_drift (27.39s)
=== RUN   TestAccAWSSecurityGroup_drift_complex
--- PASS: TestAccAWSSecurityGroup_drift_complex (64.62s)
=== RUN   TestAccAWSSecurityGroup_tags
--- PASS: TestAccAWSSecurityGroup_tags (87.49s)
=== RUN   TestAccAWSSecurityGroup_CIDRandGroups
--- PASS: TestAccAWSSecurityGroup_CIDRandGroups (71.62s)
=== RUN   TestAccAWSSecurityGroup_ingressWithCidrAndSGs
--- PASS: TestAccAWSSecurityGroup_ingressWithCidrAndSGs (69.60s)
=== RUN   TestAccAWSSecurityGroup_ingressWithCidrAndSGs_classic
--- PASS: TestAccAWSSecurityGroup_ingressWithCidrAndSGs_classic (25.47s)
=== RUN   TestAccAWSSecurityGroup_egressWithPrefixList
--- PASS: TestAccAWSSecurityGroup_egressWithPrefixList (64.46s)
=== RUN   TestAccAWSSecurityGroup_failWithDiffMismatch
--- PASS: TestAccAWSSecurityGroup_failWithDiffMismatch (60.21s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws
1166.983s
```
This commit is contained in:
Paul Stack 2016-08-15 15:11:52 +01:00 committed by GitHub
parent 3ff1321ef7
commit 73b10c8186
2 changed files with 62 additions and 5 deletions

View File

@ -239,6 +239,10 @@ func resourceAwsSecurityGroupCreate(d *schema.ResourceData, meta interface{}) er
d.Id(), err)
}
if err := setTags(conn, d); err != nil {
return err
}
// AWS defaults all Security Groups to have an ALLOW ALL egress rule. Here we
// revoke that rule, so users don't unknowingly have/use it.
group := resp.(*ec2.SecurityGroup)
@ -340,11 +344,12 @@ func resourceAwsSecurityGroupUpdate(d *schema.ResourceData, meta interface{}) er
}
}
if !d.IsNewResource() {
if err := setTags(conn, d); err != nil {
return err
}
d.SetPartial("tags")
}
return resourceAwsSecurityGroupRead(d, meta)
}

View File

@ -3,6 +3,7 @@ package aws
import (
"fmt"
"reflect"
"regexp"
"strings"
"testing"
@ -287,6 +288,26 @@ func TestAccAWSSecurityGroup_basic(t *testing.T) {
})
}
func TestAccAWSSecurityGroup_tagsCreatedFirst(t *testing.T) {
var group ec2.SecurityGroup
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSSecurityGroupDestroy,
Steps: []resource.TestStep{
resource.TestStep{
Config: testAccAWSSecurityGroupConfigForTagsOrdering,
ExpectError: regexp.MustCompile("InvalidParameterValue"),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSSecurityGroupExists("aws_security_group.foo", &group),
testAccCheckTags(&group.Tags, "Name", "tf-acc-test"),
),
},
},
})
}
func TestAccAWSSecurityGroup_namePrefix(t *testing.T) {
var group ec2.SecurityGroup
@ -791,6 +812,7 @@ func TestAccAWSSecurityGroup_tags(t *testing.T) {
testAccCheckAWSSecurityGroupExists("aws_security_group.foo", &group),
testAccCheckTags(&group.Tags, "foo", ""),
testAccCheckTags(&group.Tags, "bar", "baz"),
testAccCheckTags(&group.Tags, "env", "Production"),
),
},
},
@ -1056,6 +1078,35 @@ func TestAccAWSSecurityGroup_failWithDiffMismatch(t *testing.T) {
})
}
const testAccAWSSecurityGroupConfigForTagsOrdering = `
resource "aws_vpc" "foo" {
cidr_block = "10.1.0.0/16"
}
resource "aws_security_group" "web" {
name = "terraform_acceptance_test_example"
description = "Used in the terraform acceptance tests"
vpc_id = "${aws_vpc.foo.id}"
ingress {
protocol = "6"
from_port = 80
to_port = 80000
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"
}
}`
const testAccAWSSecurityGroupConfig = `
resource "aws_vpc" "foo" {
cidr_block = "10.1.0.0/16"
@ -1305,6 +1356,7 @@ resource "aws_security_group" "foo" {
tags {
bar = "baz"
env = "Production"
}
}
`