diff --git a/builtin/providers/aws/resource_aws_security_group_rule.go b/builtin/providers/aws/resource_aws_security_group_rule.go index 4b18b24c0..7a7034f98 100644 --- a/builtin/providers/aws/resource_aws_security_group_rule.go +++ b/builtin/providers/aws/resource_aws_security_group_rule.go @@ -27,53 +27,54 @@ func resourceAwsSecurityGroupRule() *schema.Resource { MigrateState: resourceAwsSecurityGroupRuleMigrateState, Schema: map[string]*schema.Schema{ - "type": &schema.Schema{ - Type: schema.TypeString, - Required: true, - ForceNew: true, - Description: "Type of rule, ingress (inbound) or egress (outbound).", + "type": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + Description: "Type of rule, ingress (inbound) or egress (outbound).", + ValidateFunc: validateSecurityRuleType, }, - "from_port": &schema.Schema{ + "from_port": { Type: schema.TypeInt, Required: true, ForceNew: true, }, - "to_port": &schema.Schema{ + "to_port": { Type: schema.TypeInt, Required: true, ForceNew: true, }, - "protocol": &schema.Schema{ + "protocol": { Type: schema.TypeString, Required: true, ForceNew: true, StateFunc: protocolStateFunc, }, - "cidr_blocks": &schema.Schema{ + "cidr_blocks": { Type: schema.TypeList, Optional: true, ForceNew: true, Elem: &schema.Schema{Type: schema.TypeString}, }, - "prefix_list_ids": &schema.Schema{ + "prefix_list_ids": { Type: schema.TypeList, Optional: true, ForceNew: true, Elem: &schema.Schema{Type: schema.TypeString}, }, - "security_group_id": &schema.Schema{ + "security_group_id": { Type: schema.TypeString, Required: true, ForceNew: true, }, - "source_security_group_id": &schema.Schema{ + "source_security_group_id": { Type: schema.TypeString, Optional: true, ForceNew: true, @@ -81,7 +82,7 @@ func resourceAwsSecurityGroupRule() *schema.Resource { ConflictsWith: []string{"cidr_blocks", "self"}, }, - "self": &schema.Schema{ + "self": { Type: schema.TypeBool, Optional: true, Default: false, diff --git a/builtin/providers/aws/resource_aws_security_group_rule_test.go b/builtin/providers/aws/resource_aws_security_group_rule_test.go index 045a0731e..93d39d357 100644 --- a/builtin/providers/aws/resource_aws_security_group_rule_test.go +++ b/builtin/providers/aws/resource_aws_security_group_rule_test.go @@ -12,6 +12,7 @@ import ( "github.com/hashicorp/terraform/helper/acctest" "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/terraform" + "regexp" ) func TestIpPermissionIDHash(t *testing.T) { @@ -20,7 +21,7 @@ func TestIpPermissionIDHash(t *testing.T) { FromPort: aws.Int64(int64(80)), ToPort: aws.Int64(int64(8000)), IpRanges: []*ec2.IpRange{ - &ec2.IpRange{ + { CidrIp: aws.String("10.0.0.0/8"), }, }, @@ -31,7 +32,7 @@ func TestIpPermissionIDHash(t *testing.T) { FromPort: aws.Int64(int64(80)), ToPort: aws.Int64(int64(8000)), IpRanges: []*ec2.IpRange{ - &ec2.IpRange{ + { CidrIp: aws.String("10.0.0.0/8"), }, }, @@ -40,7 +41,7 @@ func TestIpPermissionIDHash(t *testing.T) { egress_all := &ec2.IpPermission{ IpProtocol: aws.String("-1"), IpRanges: []*ec2.IpRange{ - &ec2.IpRange{ + { CidrIp: aws.String("10.0.0.0/8"), }, }, @@ -130,7 +131,7 @@ func TestAccAWSSecurityGroupRule_Ingress_VPC(t *testing.T) { Providers: testAccProviders, CheckDestroy: testAccCheckAWSSecurityGroupRuleDestroy, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: testAccAWSSecurityGroupRuleIngressConfig, Check: resource.ComposeTestCheckFunc( testAccCheckAWSSecurityGroupRuleExists("aws_security_group.web", &group), @@ -167,7 +168,7 @@ func TestAccAWSSecurityGroupRule_Ingress_Protocol(t *testing.T) { Providers: testAccProviders, CheckDestroy: testAccCheckAWSSecurityGroupRuleDestroy, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: testAccAWSSecurityGroupRuleIngress_protocolConfig, Check: resource.ComposeTestCheckFunc( testAccCheckAWSSecurityGroupRuleExists("aws_security_group.web", &group), @@ -204,7 +205,7 @@ func TestAccAWSSecurityGroupRule_Ingress_Classic(t *testing.T) { Providers: testAccProviders, CheckDestroy: testAccCheckAWSSecurityGroupRuleDestroy, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: testAccAWSSecurityGroupRuleIngressClassicConfig, Check: resource.ComposeTestCheckFunc( testAccCheckAWSSecurityGroupRuleExists("aws_security_group.web", &group), @@ -247,7 +248,7 @@ func TestAccAWSSecurityGroupRule_MultiIngress(t *testing.T) { Providers: testAccProviders, CheckDestroy: testAccCheckAWSSecurityGroupRuleDestroy, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: testAccAWSSecurityGroupRuleConfigMultiIngress, Check: resource.ComposeTestCheckFunc( testAccCheckAWSSecurityGroupRuleExists("aws_security_group.web", &group), @@ -266,7 +267,7 @@ func TestAccAWSSecurityGroupRule_Egress(t *testing.T) { Providers: testAccProviders, CheckDestroy: testAccCheckAWSSecurityGroupRuleDestroy, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: testAccAWSSecurityGroupRuleEgressConfig, Check: resource.ComposeTestCheckFunc( testAccCheckAWSSecurityGroupRuleExists("aws_security_group.web", &group), @@ -285,7 +286,7 @@ func TestAccAWSSecurityGroupRule_SelfReference(t *testing.T) { Providers: testAccProviders, CheckDestroy: testAccCheckAWSSecurityGroupRuleDestroy, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: testAccAWSSecurityGroupRuleConfigSelfReference, Check: resource.ComposeTestCheckFunc( testAccCheckAWSSecurityGroupRuleExists("aws_security_group.web", &group), @@ -295,6 +296,20 @@ func TestAccAWSSecurityGroupRule_SelfReference(t *testing.T) { }) } +func TestAccAWSSecurityGroupRule_ExpectInvalidTypeError(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSSecurityGroupRuleDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSSecurityGroupRuleExpectInvalidType, + ExpectError: regexp.MustCompile(`\\"type\\" contains an invalid Security Group Rule type \\"foobar\\"`), + }, + }, + }) +} + // testing partial match implementation func TestAccAWSSecurityGroupRule_PartialMatching_basic(t *testing.T) { var group ec2.SecurityGroup @@ -304,9 +319,9 @@ func TestAccAWSSecurityGroupRule_PartialMatching_basic(t *testing.T) { ToPort: aws.Int64(80), IpProtocol: aws.String("tcp"), IpRanges: []*ec2.IpRange{ - &ec2.IpRange{CidrIp: aws.String("10.0.2.0/24")}, - &ec2.IpRange{CidrIp: aws.String("10.0.3.0/24")}, - &ec2.IpRange{CidrIp: aws.String("10.0.4.0/24")}, + {CidrIp: aws.String("10.0.2.0/24")}, + {CidrIp: aws.String("10.0.3.0/24")}, + {CidrIp: aws.String("10.0.4.0/24")}, }, } @@ -315,7 +330,7 @@ func TestAccAWSSecurityGroupRule_PartialMatching_basic(t *testing.T) { ToPort: aws.Int64(80), IpProtocol: aws.String("tcp"), IpRanges: []*ec2.IpRange{ - &ec2.IpRange{CidrIp: aws.String("10.0.5.0/24")}, + {CidrIp: aws.String("10.0.5.0/24")}, }, } @@ -324,7 +339,7 @@ func TestAccAWSSecurityGroupRule_PartialMatching_basic(t *testing.T) { Providers: testAccProviders, CheckDestroy: testAccCheckAWSSecurityGroupRuleDestroy, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: testAccAWSSecurityGroupRulePartialMatching, Check: resource.ComposeTestCheckFunc( testAccCheckAWSSecurityGroupRuleExists("aws_security_group.web", &group), @@ -367,7 +382,7 @@ func TestAccAWSSecurityGroupRule_PartialMatching_Source(t *testing.T) { Providers: testAccProviders, CheckDestroy: testAccCheckAWSSecurityGroupRuleDestroy, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: testAccAWSSecurityGroupRulePartialMatching_Source, Check: resource.ComposeTestCheckFunc( testAccCheckAWSSecurityGroupRuleExists("aws_security_group.web", &group), @@ -388,7 +403,7 @@ func TestAccAWSSecurityGroupRule_Issue5310(t *testing.T) { Providers: testAccProviders, CheckDestroy: testAccCheckAWSSecurityGroupRuleDestroy, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: testAccAWSSecurityGroupRuleIssue5310, Check: resource.ComposeTestCheckFunc( testAccCheckAWSSecurityGroupRuleExists("aws_security_group.issue_5310", &group), @@ -406,7 +421,7 @@ func TestAccAWSSecurityGroupRule_Race(t *testing.T) { Providers: testAccProviders, CheckDestroy: testAccCheckAWSSecurityGroupRuleDestroy, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: testAccAWSSecurityGroupRuleRace, Check: resource.ComposeTestCheckFunc( testAccCheckAWSSecurityGroupRuleExists("aws_security_group.race", &group), @@ -424,7 +439,7 @@ func TestAccAWSSecurityGroupRule_SelfSource(t *testing.T) { Providers: testAccProviders, CheckDestroy: testAccCheckAWSSecurityGroupRuleDestroy, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: testAccAWSSecurityGroupRuleSelfInSource, Check: resource.ComposeTestCheckFunc( testAccCheckAWSSecurityGroupRuleExists("aws_security_group.web", &group), @@ -466,7 +481,7 @@ func TestAccAWSSecurityGroupRule_PrefixListEgress(t *testing.T) { p = ec2.IpPermission{ IpProtocol: aws.String("-1"), PrefixListIds: []*ec2.PrefixListId{ - &ec2.PrefixListId{PrefixListId: prefixListsOutput.PrefixLists[0].PrefixListId}, + {PrefixListId: prefixListsOutput.PrefixLists[0].PrefixListId}, }, } @@ -478,7 +493,7 @@ func TestAccAWSSecurityGroupRule_PrefixListEgress(t *testing.T) { Providers: testAccProviders, CheckDestroy: testAccCheckAWSSecurityGroupRuleDestroy, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: testAccAWSSecurityGroupRulePrefixListEgressConfig, Check: resource.ComposeTestCheckFunc( testAccCheckAWSSecurityGroupRuleExists("aws_security_group.egress", &group), @@ -572,7 +587,7 @@ func testAccCheckAWSSecurityGroupRuleAttributes(n string, group *ec2.SecurityGro FromPort: aws.Int64(80), ToPort: aws.Int64(8000), IpProtocol: aws.String("tcp"), - IpRanges: []*ec2.IpRange{&ec2.IpRange{CidrIp: aws.String("10.0.0.0/8")}}, + IpRanges: []*ec2.IpRange{{CidrIp: aws.String("10.0.0.0/8")}}, } } @@ -1044,3 +1059,28 @@ resource "aws_security_group_rule" "allow_self" { source_security_group_id = "${aws_security_group.web.id}" } ` + +const testAccAWSSecurityGroupRuleExpectInvalidType = ` +resource "aws_vpc" "foo" { + cidr_block = "10.1.0.0/16" + + tags { + Name = "tf_sg_rule_self_group" + } +} + +resource "aws_security_group" "web" { + name = "allow_all" + description = "Allow all inbound traffic" + vpc_id = "${aws_vpc.foo.id}" +} + +resource "aws_security_group_rule" "allow_self" { + type = "foobar" + from_port = 0 + to_port = 0 + protocol = "-1" + security_group_id = "${aws_security_group.web.id}" + source_security_group_id = "${aws_security_group.web.id}" +} +` diff --git a/builtin/providers/aws/validators.go b/builtin/providers/aws/validators.go index 940826a51..ed9f2d0b7 100644 --- a/builtin/providers/aws/validators.go +++ b/builtin/providers/aws/validators.go @@ -608,3 +608,19 @@ func validateSNSSubscriptionProtocol(v interface{}, k string) (ws []string, erro } return } + +func validateSecurityRuleType(v interface{}, k string) (ws []string, errors []error) { + value := strings.ToLower(v.(string)) + + validTypes := map[string]bool{ + "ingress": true, + "egress": true, + } + + if _, ok := validTypes[value]; !ok { + errors = append(errors, fmt.Errorf( + "%q contains an invalid Security Group Rule type %q. Valid types are either %q or %q.", + k, value, "ingress", "egress")) + } + return +} diff --git a/builtin/providers/aws/validators_test.go b/builtin/providers/aws/validators_test.go index 7817f80f0..f7de51645 100644 --- a/builtin/providers/aws/validators_test.go +++ b/builtin/providers/aws/validators_test.go @@ -901,3 +901,25 @@ func TestValidateSNSSubscriptionProtocol(t *testing.T) { } } } + +func TestValidateSecurityRuleType(t *testing.T) { + validTypes := []string{ + "ingress", + "egress", + } + for _, v := range validTypes { + if _, errors := validateSecurityRuleType(v, "type"); len(errors) > 0 { + t.Fatalf("%q should be a valid Security Group Rule type: %v", v, errors) + } + } + + invalidTypes := []string{ + "foo", + "ingresss", + } + for _, v := range invalidTypes { + if _, errors := validateSecurityRuleType(v, "type"); len(errors) == 0 { + t.Fatalf("%q should be an invalid Security Group Rule type: %v", v, errors) + } + } +}