From 03c2c4408f5b253a15c7511e9f1e9f5a1c59dbea Mon Sep 17 00:00:00 2001 From: stevehorsfield Date: Mon, 6 Jun 2016 12:07:19 +0200 Subject: [PATCH] Add support for 'prefix_list_ids' to AWS VPC security group rules Prefix list IDs are used when allowing egress to an AWS VPC Endpoint. See http://docs.aws.amazon.com/AmazonVPC/latest/UserGuide/vpc-endpoints.html#vpc-endpoints-routing --- .../aws/resource_aws_security_group.go | 161 ++++++++++++++---- .../aws/resource_aws_security_group_rule.go | 51 ++++++ .../resource_aws_security_group_rule_test.go | 68 ++++++++ .../aws/resource_aws_security_group_test.go | 113 ++++++++++++ builtin/providers/aws/structure.go | 7 + .../aws/r/security_group.html.markdown | 22 +++ .../aws/r/security_group_rule.html.markdown | 24 +++ 7 files changed, 411 insertions(+), 35 deletions(-) diff --git a/builtin/providers/aws/resource_aws_security_group.go b/builtin/providers/aws/resource_aws_security_group.go index 5296ab16d..a4773e506 100644 --- a/builtin/providers/aws/resource_aws_security_group.go +++ b/builtin/providers/aws/resource_aws_security_group.go @@ -153,6 +153,12 @@ func resourceAwsSecurityGroup() *schema.Resource { Elem: &schema.Schema{Type: schema.TypeString}, }, + "prefix_list_ids": &schema.Schema{ + Type: schema.TypeList, + Optional: true, + Elem: &schema.Schema{Type: schema.TypeString}, + }, + "security_groups": &schema.Schema{ Type: schema.TypeSet, Optional: true, @@ -397,6 +403,18 @@ func resourceAwsSecurityGroupRuleHash(v interface{}) int { buf.WriteString(fmt.Sprintf("%s-", v)) } } + if v, ok := m["prefix_list_ids"]; ok { + vs := v.([]interface{}) + s := make([]string, len(vs)) + for i, raw := range vs { + s[i] = raw.(string) + } + sort.Strings(s) + + for _, v := range s { + buf.WriteString(fmt.Sprintf("%s-", v)) + } + } if v, ok := m["security_groups"]; ok { vs := v.(*schema.Set).List() s := make([]string, len(vs)) @@ -449,6 +467,20 @@ func resourceAwsSecurityGroupIPPermGather(groupId string, permissions []*ec2.IpP m["cidr_blocks"] = list } + if len(perm.PrefixListIds) > 0 { + raw, ok := m["prefix_list_ids"] + if !ok { + raw = make([]string, 0, len(perm.PrefixListIds)) + } + list := raw.([]string) + + for _, pl := range perm.PrefixListIds { + list = append(list, *pl.PrefixListId) + } + + m["prefix_list_ids"] = list + } + groups := flattenSecurityGroups(perm.UserIdGroupPairs, ownerId) for i, g := range groups { if *g.GroupId == groupId { @@ -658,9 +690,10 @@ func matchRules(rType string, local []interface{}, remote []map[string]interface // local rule we're examining rHash := idHash(rType, r["protocol"].(string), r["to_port"].(int64), r["from_port"].(int64), remoteSelfVal) if rHash == localHash { - var numExpectedCidrs, numExpectedSGs, numRemoteCidrs, numRemoteSGs int + var numExpectedCidrs, numExpectedPrefixLists, numExpectedSGs, numRemoteCidrs, numRemotePrefixLists, numRemoteSGs int var matchingCidrs []string var matchingSGs []string + var matchingPrefixLists []string // grab the local/remote cidr and sg groups, capturing the expected and // actual counts @@ -668,6 +701,10 @@ func matchRules(rType string, local []interface{}, remote []map[string]interface if ok { numExpectedCidrs = len(l["cidr_blocks"].([]interface{})) } + lpRaw, ok := l["prefix_list_ids"] + if ok { + numExpectedPrefixLists = len(l["prefix_list_ids"].([]interface{})) + } lsRaw, ok := l["security_groups"] if ok { numExpectedSGs = len(l["security_groups"].(*schema.Set).List()) @@ -677,6 +714,10 @@ func matchRules(rType string, local []interface{}, remote []map[string]interface if ok { numRemoteCidrs = len(r["cidr_blocks"].([]string)) } + rpRaw, ok := r["prefix_list_ids"] + if ok { + numRemotePrefixLists = len(r["prefix_list_ids"].([]string)) + } rsRaw, ok := r["security_groups"] if ok { @@ -688,6 +729,10 @@ func matchRules(rType string, local []interface{}, remote []map[string]interface log.Printf("[DEBUG] Local rule has more CIDR blocks, continuing (%d/%d)", numExpectedCidrs, numRemoteCidrs) continue } + if numExpectedPrefixLists > numRemotePrefixLists { + log.Printf("[DEBUG] Local rule has more prefix lists, continuing (%d/%d)", numExpectedPrefixLists, numRemotePrefixLists) + continue + } if numExpectedSGs > numRemoteSGs { log.Printf("[DEBUG] Local rule has more Security Groups, continuing (%d/%d)", numExpectedSGs, numRemoteSGs) continue @@ -721,6 +766,34 @@ func matchRules(rType string, local []interface{}, remote []map[string]interface } } + // match prefix lists by converting both to sets, and using Set methods + var localPrefixLists []interface{} + if lpRaw != nil { + localPrefixLists = lpRaw.([]interface{}) + } + localPrefixListsSet := schema.NewSet(schema.HashString, localPrefixLists) + + // remote prefix lists are presented as a slice of strings, so we need to + // reformat them into a slice of interfaces to be used in creating the + // remote prefix list set + var remotePrefixLists []string + if rpRaw != nil { + remotePrefixLists = rpRaw.([]string) + } + // convert remote prefix lists to a set, for easy comparison + list = nil + for _, s := range remotePrefixLists { + list = append(list, s) + } + remotePrefixListsSet := schema.NewSet(schema.HashString, list) + + // Build up a list of local prefix lists that are found in the remote set + for _, s := range localPrefixListsSet.List() { + if remotePrefixListsSet.Contains(s) { + matchingPrefixLists = append(matchingPrefixLists, s.(string)) + } + } + // match SGs. Both local and remote are already sets var localSGSet *schema.Set if lsRaw == nil { @@ -748,41 +821,57 @@ func matchRules(rType string, local []interface{}, remote []map[string]interface // match, and then remove those elements from the remote rule, so that // this remote rule can still be considered by other local rules if numExpectedCidrs == len(matchingCidrs) { - if numExpectedSGs == len(matchingSGs) { - // confirm that self references match - var lSelf bool - var rSelf bool - if _, ok := l["self"]; ok { - lSelf = l["self"].(bool) - } - if _, ok := r["self"]; ok { - rSelf = r["self"].(bool) - } - if rSelf == lSelf { - delete(r, "self") - // pop local cidrs from remote - diffCidr := remoteCidrSet.Difference(localCidrSet) - var newCidr []string - for _, cRaw := range diffCidr.List() { - newCidr = append(newCidr, cRaw.(string)) + if numExpectedPrefixLists == len(matchingPrefixLists) { + if numExpectedSGs == len(matchingSGs) { + // confirm that self references match + var lSelf bool + var rSelf bool + if _, ok := l["self"]; ok { + lSelf = l["self"].(bool) } - - // reassigning - if len(newCidr) > 0 { - r["cidr_blocks"] = newCidr - } else { - delete(r, "cidr_blocks") + if _, ok := r["self"]; ok { + rSelf = r["self"].(bool) } + if rSelf == lSelf { + delete(r, "self") + // pop local cidrs from remote + diffCidr := remoteCidrSet.Difference(localCidrSet) + var newCidr []string + for _, cRaw := range diffCidr.List() { + newCidr = append(newCidr, cRaw.(string)) + } - // pop local sgs from remote - diffSGs := remoteSGSet.Difference(localSGSet) - if len(diffSGs.List()) > 0 { - r["security_groups"] = diffSGs - } else { - delete(r, "security_groups") + // reassigning + if len(newCidr) > 0 { + r["cidr_blocks"] = newCidr + } else { + delete(r, "cidr_blocks") + } + + // pop local prefix lists from remote + diffPrefixLists := remotePrefixListsSet.Difference(localPrefixListsSet) + var newPrefixLists []string + for _, pRaw := range diffPrefixLists.List() { + newPrefixLists = append(newPrefixLists, pRaw.(string)) + } + + // reassigning + if len(newPrefixLists) > 0 { + r["prefix_list_ids"] = newPrefixLists + } else { + delete(r, "prefix_list_ids") + } + + // pop local sgs from remote + diffSGs := remoteSGSet.Difference(localSGSet) + if len(diffSGs.List()) > 0 { + r["security_groups"] = diffSGs + } else { + delete(r, "security_groups") + } + + saves = append(saves, l) } - - saves = append(saves, l) } } } @@ -795,11 +884,13 @@ func matchRules(rType string, local []interface{}, remote []map[string]interface // matched locally, and let the graph sort things out. This will happen when // rules are added externally to Terraform for _, r := range remote { - var lenCidr, lenSGs int + var lenCidr, lenPrefixLists, lenSGs int if rCidrs, ok := r["cidr_blocks"]; ok { lenCidr = len(rCidrs.([]string)) } - + if rPrefixLists, ok := r["prefix_list_ids"]; ok { + lenPrefixLists = len(rPrefixLists.([]string)) + } if rawSGs, ok := r["security_groups"]; ok { lenSGs = len(rawSGs.(*schema.Set).List()) } @@ -810,7 +901,7 @@ func matchRules(rType string, local []interface{}, remote []map[string]interface } } - if lenSGs+lenCidr > 0 { + if lenSGs+lenCidr+lenPrefixLists > 0 { log.Printf("[DEBUG] Found a remote Rule that wasn't empty: (%#v)", r) saves = append(saves, r) } diff --git a/builtin/providers/aws/resource_aws_security_group_rule.go b/builtin/providers/aws/resource_aws_security_group_rule.go index 025a951af..e963ca679 100644 --- a/builtin/providers/aws/resource_aws_security_group_rule.go +++ b/builtin/providers/aws/resource_aws_security_group_rule.go @@ -59,6 +59,13 @@ func resourceAwsSecurityGroupRule() *schema.Resource { Elem: &schema.Schema{Type: schema.TypeString}, }, + "prefix_list_ids": &schema.Schema{ + Type: schema.TypeList, + Optional: true, + ForceNew: true, + Elem: &schema.Schema{Type: schema.TypeString}, + }, + "security_group_id": &schema.Schema{ Type: schema.TypeString, Required: true, @@ -363,6 +370,19 @@ func findRuleMatch(p *ec2.IpPermission, rules []*ec2.IpPermission, isVPC bool) * continue } + remaining = len(p.PrefixListIds) + for _, pl := range p.PrefixListIds { + for _, rpl := range r.PrefixListIds { + if *pl.PrefixListId == *rpl.PrefixListId { + remaining-- + } + } + } + + if remaining > 0 { + continue + } + remaining = len(p.UserIdGroupPairs) for _, ip := range p.UserIdGroupPairs { for _, rip := range r.UserIdGroupPairs { @@ -413,6 +433,18 @@ func ipPermissionIDHash(sg_id, ruleType string, ip *ec2.IpPermission) string { } } + if len(ip.PrefixListIds) > 0 { + s := make([]string, len(ip.PrefixListIds)) + for i, pl := range ip.PrefixListIds { + s[i] = *pl.PrefixListId + } + sort.Strings(s) + + for _, v := range s { + buf.WriteString(fmt.Sprintf("%s-", v)) + } + } + if len(ip.UserIdGroupPairs) > 0 { sort.Sort(ByGroupPair(ip.UserIdGroupPairs)) for _, pair := range ip.UserIdGroupPairs { @@ -494,6 +526,18 @@ func expandIPPerm(d *schema.ResourceData, sg *ec2.SecurityGroup) (*ec2.IpPermiss } } + if raw, ok := d.GetOk("prefix_list_ids"); ok { + list := raw.([]interface{}) + perm.PrefixListIds = make([]*ec2.PrefixListId, len(list)) + for i, v := range list { + prefixListID, ok := v.(string) + if !ok { + return nil, fmt.Errorf("empty element found in prefix_list_ids - consider using the compact function") + } + perm.PrefixListIds[i] = &ec2.PrefixListId{PrefixListId: aws.String(prefixListID)} + } + } + return &perm, nil } @@ -514,6 +558,13 @@ func setFromIPPerm(d *schema.ResourceData, sg *ec2.SecurityGroup, rule *ec2.IpPe // 'self' is false by default. Below, we range over the group ids and set true // if the parent sg id is found d.Set("self", false) + + var pl []string + for _, p := range rule.PrefixListIds { + pl = append(pl, *p.PrefixListId) + } + d.Set("prefix_list_ids", pl) + if len(rule.UserIdGroupPairs) > 0 { s := rule.UserIdGroupPairs[0] 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 0e46d02fc..5ed0f680e 100644 --- a/builtin/providers/aws/resource_aws_security_group_rule_test.go +++ b/builtin/providers/aws/resource_aws_security_group_rule_test.go @@ -416,6 +416,25 @@ func TestAccAWSSecurityGroupRule_Race(t *testing.T) { }) } +func TestAccAWSSecurityGroupRule_PrefixListEgress(t *testing.T) { + var group ec2.SecurityGroup + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSSecurityGroupRuleDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccAWSSecurityGroupRulePrefixListEgressConfig, + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSSecurityGroupRuleExists("aws_security_group.egress", &group), + testAccCheckAWSSecurityGroupRuleAttributes("aws_security_group_rule.egress_1", &group, nil, "egress"), + ), + }, + }, + }) +} + func testAccCheckAWSSecurityGroupRuleDestroy(s *terraform.State) error { conn := testAccProvider.Meta().(*AWSClient).ec2conn @@ -879,3 +898,52 @@ var testAccAWSSecurityGroupRuleRace = func() string { } return b.String() }() + +const testAccAWSSecurityGroupRulePrefixListEgressConfig = ` + +resource "aws_vpc" "tf_sg_prefix_list_egress_test" { + cidr_block = "10.0.0.0/16" + tags { + Name = "tf_sg_prefix_list_egress_test" + } +} + +resource "aws_route_table" "default" { + vpc_id = "${aws_vpc.tf_sg_prefix_list_egress_test.id}" +} + +resource "aws_vpc_endpoint" "s3-us-west-2" { + vpc_id = "${aws_vpc.tf_sg_prefix_list_egress_test.id}" + service_name = "com.amazonaws.us-west-2.s3" + route_table_ids = ["${aws_route_table.default.id}"] + policy = <