diff --git a/.changelog/26553.txt b/.changelog/26553.txt new file mode 100644 index 00000000000..862b55aae84 --- /dev/null +++ b/.changelog/26553.txt @@ -0,0 +1,15 @@ +```release-note:bug +resource/aws_security_group: Fix complex dependency violations such as using a security group with an EMR cluster +``` + +```release-note:note +resource/aws_security_group: With AWS's retirement of EC2-Classic, `aws_security_group` has been updated to remove support for EC2-Classic +``` + +```release-note:note +resource/aws_default_security_group: With AWS's retirement of EC2-Classic, `aws_default_security_group` has been updated to remove support for EC2-Classic +``` + +```release-note:note +resource/aws_security_group_rule: With AWS's retirement of EC2-Classic, `aws_security_group_rule` has been updated to remove support for EC2-Classic +``` \ No newline at end of file diff --git a/GNUmakefile b/GNUmakefile index f07c6f9d7d0..b8d7f8d9a38 100644 --- a/GNUmakefile +++ b/GNUmakefile @@ -19,6 +19,41 @@ ifneq ($(origin SWEEPERS), undefined) SWEEPARGS = -sweep-run='$(SWEEPERS)' endif +ifeq ($(PKG_NAME), internal/service/ebs) + PKG_NAME = internal/service/ec2 + TEST = ./$(PKG_NAME)/... +endif + +ifeq ($(PKG_NAME), internal/service/ipam) + PKG_NAME = internal/service/ec2 + TEST = ./$(PKG_NAME)/... +endif + +ifeq ($(PKG_NAME), internal/service/transitgateway) + PKG_NAME = internal/service/ec2 + TEST = ./$(PKG_NAME)/... +endif + +ifeq ($(PKG_NAME), internal/service/vpc) + PKG_NAME = internal/service/ec2 + TEST = ./$(PKG_NAME)/... +endif + +ifeq ($(PKG_NAME), internal/service/vpnclient) + PKG_NAME = internal/service/ec2 + TEST = ./$(PKG_NAME)/... +endif + +ifeq ($(PKG_NAME), internal/service/vpnsite) + PKG_NAME = internal/service/ec2 + TEST = ./$(PKG_NAME)/... +endif + +ifeq ($(PKG_NAME), internal/service/wavelength) + PKG_NAME = internal/service/ec2 + TEST = ./$(PKG_NAME)/... +endif + default: build build: fmtcheck diff --git a/internal/service/ec2/errors.go b/internal/service/ec2/errors.go index 8fa544a4eac..5d7f6b67984 100644 --- a/internal/service/ec2/errors.go +++ b/internal/service/ec2/errors.go @@ -64,6 +64,7 @@ const ( errCodeInvalidRouteTableIDNotFound = "InvalidRouteTableID.NotFound" errCodeInvalidRouteTableIdNotFound = "InvalidRouteTableId.NotFound" errCodeInvalidSecurityGroupIDNotFound = "InvalidSecurityGroupID.NotFound" + errCodeInvalidSecurityGroupRuleIDNotFound = "InvalidSecurityGroupRuleId.NotFound" errCodeInvalidServiceName = "InvalidServiceName" errCodeInvalidSnapshotInUse = "InvalidSnapshot.InUse" errCodeInvalidSnapshotNotFound = "InvalidSnapshot.NotFound" diff --git a/internal/service/ec2/vpc_default_security_group.go b/internal/service/ec2/vpc_default_security_group.go index 28a363a64a9..e43ae011d3d 100644 --- a/internal/service/ec2/vpc_default_security_group.go +++ b/internal/service/ec2/vpc_default_security_group.go @@ -117,7 +117,7 @@ func resourceDefaultSecurityGroupCreate(d *schema.ResourceData, meta interface{} } } - if err := forceRevokeSecurityGroupRules(conn, d.Id()); err != nil { + if err := forceRevokeSecurityGroupRules(conn, d.Id(), false); err != nil { return err } diff --git a/internal/service/ec2/vpc_default_security_group_test.go b/internal/service/ec2/vpc_default_security_group_test.go index b977486c713..e0dd2efed65 100644 --- a/internal/service/ec2/vpc_default_security_group_test.go +++ b/internal/service/ec2/vpc_default_security_group_test.go @@ -12,7 +12,7 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/acctest" ) -func TestAccVPCDefaultSecurityGroup_VPC_basic(t *testing.T) { +func TestAccVPCDefaultSecurityGroup_basic(t *testing.T) { var group ec2.SecurityGroup rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_default_security_group.test" @@ -66,7 +66,7 @@ func TestAccVPCDefaultSecurityGroup_VPC_basic(t *testing.T) { }) } -func TestAccVPCDefaultSecurityGroup_VPC_empty(t *testing.T) { +func TestAccVPCDefaultSecurityGroup_empty(t *testing.T) { var group ec2.SecurityGroup rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_default_security_group.test" @@ -97,103 +97,12 @@ func TestAccVPCDefaultSecurityGroup_VPC_empty(t *testing.T) { }) } -func TestAccVPCDefaultSecurityGroup_Classic_serial(t *testing.T) { - testCases := map[string]func(t *testing.T){ - "basic": testAccVPCDefaultSecurityGroup_Classic_basic, - "empty": testAccVPCDefaultSecurityGroup_Classic_empty, - } - - for name, tc := range testCases { - tc := tc - t.Run(name, func(t *testing.T) { - tc(t) - }) - } -} - -func testAccVPCDefaultSecurityGroup_Classic_basic(t *testing.T) { - var group ec2.SecurityGroup - rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) - resourceName := "aws_default_security_group.test" - - resource.Test(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(t); acctest.PreCheckEC2Classic(t) }, - ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID), - ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, - CheckDestroy: acctest.CheckDestroyNoop, - Steps: []resource.TestStep{ - { - Config: testAccVPCDefaultSecurityGroupConfig_classic(rName), - Check: resource.ComposeTestCheckFunc( - testAccCheckSecurityGroupEC2ClassicExists(resourceName, &group), - resource.TestCheckResourceAttr(resourceName, "name", "default"), - resource.TestCheckResourceAttr(resourceName, "description", "default group"), - resource.TestCheckResourceAttr(resourceName, "vpc_id", ""), - resource.TestCheckResourceAttr(resourceName, "ingress.#", "1"), - resource.TestCheckTypeSetElemNestedAttrs(resourceName, "ingress.*", map[string]string{ - "protocol": "tcp", - "from_port": "80", - "to_port": "8000", - "cidr_blocks.#": "1", - "cidr_blocks.0": "10.0.0.0/8", - }), - resource.TestCheckResourceAttr(resourceName, "egress.#", "0"), - testAccCheckDefaultSecurityGroupARNClassic(resourceName, &group), - acctest.CheckResourceAttrAccountID(resourceName, "owner_id"), - resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), - resource.TestCheckResourceAttr(resourceName, "tags.Name", rName), - ), - }, - { - Config: testAccVPCDefaultSecurityGroupConfig_classic(rName), - PlanOnly: true, - }, - { - Config: testAccVPCDefaultSecurityGroupConfig_classic(rName), - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"revoke_rules_on_delete"}, - }, - }, - }) -} - -func testAccVPCDefaultSecurityGroup_Classic_empty(t *testing.T) { - var group ec2.SecurityGroup - resourceName := "aws_default_security_group.test" - - resource.Test(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(t); acctest.PreCheckEC2Classic(t) }, - ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID), - ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, - CheckDestroy: acctest.CheckDestroyNoop, - Steps: []resource.TestStep{ - { - Config: testAccVPCDefaultSecurityGroupConfig_classicEmpty(), - Check: resource.ComposeTestCheckFunc( - testAccCheckSecurityGroupEC2ClassicExists(resourceName, &group), - resource.TestCheckResourceAttr(resourceName, "ingress.#", "0"), - resource.TestCheckResourceAttr(resourceName, "egress.#", "0"), - resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), - ), - }, - }, - }) -} - func testAccCheckDefaultSecurityGroupARN(resourceName string, group *ec2.SecurityGroup) resource.TestCheckFunc { return func(s *terraform.State) error { return acctest.CheckResourceAttrRegionalARN(resourceName, "arn", "ec2", fmt.Sprintf("security-group/%s", aws.StringValue(group.GroupId)))(s) } } -func testAccCheckDefaultSecurityGroupARNClassic(resourceName string, group *ec2.SecurityGroup) resource.TestCheckFunc { - return func(s *terraform.State) error { - return acctest.CheckResourceAttrRegionalARNEC2Classic(resourceName, "arn", "ec2", fmt.Sprintf("security-group/%s", aws.StringValue(group.GroupId)))(s) - } -} - func testAccVPCDefaultSecurityGroupConfig_basic(rName string) string { return fmt.Sprintf(` resource "aws_vpc" "test" { @@ -243,27 +152,3 @@ resource "aws_default_security_group" "test" { } `, rName) } - -func testAccVPCDefaultSecurityGroupConfig_classic(rName string) string { - return acctest.ConfigCompose(acctest.ConfigEC2ClassicRegionProvider(), fmt.Sprintf(` -resource "aws_default_security_group" "test" { - ingress { - protocol = "6" - from_port = 80 - to_port = 8000 - cidr_blocks = ["10.0.0.0/8"] - } - - tags = { - Name = %[1]q - } -} -`, rName)) -} - -func testAccVPCDefaultSecurityGroupConfig_classicEmpty() string { - return acctest.ConfigCompose(acctest.ConfigEC2ClassicRegionProvider(), ` -resource "aws_default_security_group" "test" { - # No attributes set. -}`) -} diff --git a/internal/service/ec2/vpc_security_group.go b/internal/service/ec2/vpc_security_group.go index 17d634a44be..5be0253a131 100644 --- a/internal/service/ec2/vpc_security_group.go +++ b/internal/service/ec2/vpc_security_group.go @@ -23,6 +23,7 @@ import ( tftags "github.com/hashicorp/terraform-provider-aws/internal/tags" "github.com/hashicorp/terraform-provider-aws/internal/tfresource" "github.com/hashicorp/terraform-provider-aws/internal/verify" + "github.com/hashicorp/terraform-provider-aws/names" ) func ResourceSecurityGroup() *schema.Resource { @@ -370,20 +371,16 @@ func resourceSecurityGroupDelete(d *schema.ResourceData, meta interface{}) error // conditionally revoke rules first before attempting to delete the group if v := d.Get("revoke_rules_on_delete").(bool); v { - err := forceRevokeSecurityGroupRules(conn, d.Id()) - - if tfawserr.ErrCodeEquals(err, errCodeInvalidGroupNotFound) { - return nil - } + err := forceRevokeSecurityGroupRules(conn, d.Id(), false) if err != nil { - return err + return create.Error(names.EC2, create.ErrActionDeleting, "Security Group", d.Id(), err) } } log.Printf("[DEBUG] Deleting Security Group: %s", d.Id()) _, err := tfresource.RetryWhenAWSErrCodeEquals( - d.Timeout(schema.TimeoutDelete), + 2*time.Minute, // short initial attempt followed by full length attempt func() (interface{}, error) { return conn.DeleteSecurityGroup(&ec2.DeleteSecurityGroupInput{ GroupId: aws.String(d.Id()), @@ -392,6 +389,26 @@ func resourceSecurityGroupDelete(d *schema.ResourceData, meta interface{}) error errCodeDependencyViolation, errCodeInvalidGroupInUse, ) + if tfawserr.ErrCodeEquals(err, errCodeDependencyViolation) { + if v := d.Get("revoke_rules_on_delete").(bool); v { + err := forceRevokeSecurityGroupRules(conn, d.Id(), true) + + if err != nil { + return create.Error(names.EC2, create.ErrActionDeleting, "Security Group", d.Id(), err) + } + } + + _, err = tfresource.RetryWhenAWSErrCodeEquals( + d.Timeout(schema.TimeoutDelete), + func() (interface{}, error) { + return conn.DeleteSecurityGroup(&ec2.DeleteSecurityGroupInput{ + GroupId: aws.String(d.Id()), + }) + }, + errCodeDependencyViolation, errCodeInvalidGroupInUse, + ) + } + if tfawserr.ErrCodeEquals(err, errCodeInvalidGroupNotFound) { return nil } @@ -411,44 +428,159 @@ func resourceSecurityGroupDelete(d *schema.ResourceData, meta interface{}) error return nil } -// forceRevokeSecurityGroupRules revokes all of the specified Security Group's ingress & egress rules. -func forceRevokeSecurityGroupRules(conn *ec2.EC2, id string) error { - group, err := FindSecurityGroupByID(conn, id) - +// forceRevokeSecurityGroupRules revokes all of the security group's ingress & egress rules +// AND rules in other security groups that depend on this security group. Trying to delete +// this security group with rules that originate in other groups but point here, will cause +// a DepedencyViolation error. searchAll = true means to search every security group +// looking for a rule depending on this security group. Otherwise, it will only look at +// groups that this group knows about. +func forceRevokeSecurityGroupRules(conn *ec2.EC2, id string, searchAll bool) error { + conns.GlobalMutexKV.Lock(id) + defer conns.GlobalMutexKV.Unlock(id) + + rules, err := rulesInSGsTouchingThis(conn, id, searchAll) if err != nil { - return fmt.Errorf("reading Security Group (%s): %w", id, err) + return fmt.Errorf("describing security group rules: %s", err) } - if len(group.IpPermissions) > 0 { - input := &ec2.RevokeSecurityGroupIngressInput{ - IpPermissions: group.IpPermissions, - } + for _, rule := range rules { + var err error + + if rule.IsEgress == nil || !aws.BoolValue(rule.IsEgress) { + input := &ec2.RevokeSecurityGroupIngressInput{ + SecurityGroupRuleIds: []*string{rule.SecurityGroupRuleId}, + } + + if rule.GroupId != nil { + input.GroupId = rule.GroupId + } else { + // If this rule isn't "owned" by this group, this will be wrong. + // However, ec2.SecurityGroupRule doesn't include name so can't + // be used. If it affects anything, this would affect default + // VPCs. + sg, err := FindSecurityGroupByID(conn, id) + if err != nil { + return fmt.Errorf("reading Security Group (%s): %w", id, err) + } - if aws.StringValue(group.VpcId) == "" { - input.GroupName = group.GroupName + input.GroupName = sg.GroupName + } + + _, err = conn.RevokeSecurityGroupIngress(input) } else { - input.GroupId = group.GroupId + input := &ec2.RevokeSecurityGroupEgressInput{ + GroupId: rule.GroupId, + SecurityGroupRuleIds: []*string{rule.SecurityGroupRuleId}, + } + + _, err = conn.RevokeSecurityGroupEgress(input) } - if _, err := conn.RevokeSecurityGroupIngress(input); err != nil { - return fmt.Errorf("revoking Security Group (%s) ingress rules: %w", id, err) + if tfawserr.ErrCodeEquals(err, errCodeInvalidSecurityGroupRuleIDNotFound) { + continue } - } - if len(group.IpPermissionsEgress) > 0 { - input := &ec2.RevokeSecurityGroupEgressInput{ - GroupId: group.GroupId, - IpPermissions: group.IpPermissionsEgress, + if tfawserr.ErrCodeEquals(err, errCodeInvalidGroupNotFound) { + continue } - if _, err := conn.RevokeSecurityGroupEgress(input); err != nil { - return fmt.Errorf("revoking Security Group (%s) egress rules: %w", id, err) + if err != nil { + return fmt.Errorf("revoking Security Group (%s) Rule (%s): %w", id, aws.StringValue(rule.SecurityGroupRuleId), err) } } return nil } +// rulesInSGsTouchingThis finds all rules related to this group even if they live in +// other groups. If searchAll = true, this could take a while as it looks through every +// security group accessible from the account. This should only be used for troublesome +// DependencyViolations. +func rulesInSGsTouchingThis(conn *ec2.EC2, id string, searchAll bool) ([]*ec2.SecurityGroupRule, error) { + var input *ec2.DescribeSecurityGroupRulesInput + + if searchAll { + input = &ec2.DescribeSecurityGroupRulesInput{} + } else { + sgs, err := relatedSGs(conn, id) + if err != nil { + return nil, fmt.Errorf("describing security group rules: %s", err) + } + + input = &ec2.DescribeSecurityGroupRulesInput{ + Filters: []*ec2.Filter{ + { + Name: aws.String("group-id"), + Values: aws.StringSlice(sgs), + }, + }, + } + } + + rules := []*ec2.SecurityGroupRule{} + + err := conn.DescribeSecurityGroupRulesPages(input, + func(page *ec2.DescribeSecurityGroupRulesOutput, lastPage bool) bool { + for _, rule := range page.SecurityGroupRules { + if rule == nil || rule.GroupId == nil { + continue + } + + if aws.StringValue(rule.GroupId) == id { + rules = append(rules, rule) + continue + } + + if rule.ReferencedGroupInfo != nil && rule.ReferencedGroupInfo.GroupId != nil && aws.StringValue(rule.ReferencedGroupInfo.GroupId) == id { + rules = append(rules, rule) + continue + } + } + return lastPage + }) + + if err != nil { + return nil, fmt.Errorf("describing security group rules: %w", err) + } + + return rules, nil +} + +// relatedSGs returns security group IDs of any other security group that is related +// to this one through a rule that this group knows about. This group can still have +// dependent rules beyond those in these groups. However, the majority of the time, +// revoking related rules should allow the group to be deleted. +func relatedSGs(conn *ec2.EC2, id string) ([]string, error) { + relatedSGs := []string{id} + + sg, err := FindSecurityGroupByID(conn, id) + if err != nil { + return nil, fmt.Errorf("reading Security Group (%s): %w", id, err) + } + + if len(sg.IpPermissions) > 0 { + for _, v := range sg.IpPermissions { + for _, v := range v.UserIdGroupPairs { + if v.GroupId != nil && aws.StringValue(v.GroupId) != id { + relatedSGs = append(relatedSGs, aws.StringValue(v.GroupId)) + } + } + } + } + + if len(sg.IpPermissionsEgress) > 0 { + for _, v := range sg.IpPermissionsEgress { + for _, v := range v.UserIdGroupPairs { + if v.GroupId != nil && aws.StringValue(v.GroupId) != id { + relatedSGs = append(relatedSGs, aws.StringValue(v.GroupId)) + } + } + } + } + + return relatedSGs, nil +} + func SecurityGroupRuleHash(v interface{}) int { var buf bytes.Buffer m := v.(map[string]interface{}) @@ -622,13 +754,13 @@ func updateSecurityGroupRules(conn *ec2.EC2, d *schema.ResourceData, ruleType st del, err := ExpandIPPerms(group, SecurityGroupCollapseRules(ruleType, os.Difference(ns).List())) if err != nil { - return err + return fmt.Errorf("updating rules: %w", err) } add, err := ExpandIPPerms(group, SecurityGroupCollapseRules(ruleType, ns.Difference(os).List())) if err != nil { - return err + return fmt.Errorf("updating rules: %w", err) } // TODO: We need to handle partial state better in the in-between diff --git a/internal/service/ec2/vpc_security_group_rule.go b/internal/service/ec2/vpc_security_group_rule.go index f5d7c1f85c2..d46d54e243a 100644 --- a/internal/service/ec2/vpc_security_group_rule.go +++ b/internal/service/ec2/vpc_security_group_rule.go @@ -155,7 +155,7 @@ func resourceSecurityGroupRuleCreate(d *schema.ResourceData, meta interface{}) e return fmt.Errorf("reading Security Group (%s): %w", securityGroupID, err) } - ipPermission := expandIpPermission(d, sg) + ipPermission := expandIPPermission(d, sg) ruleType := d.Get("type").(string) isVPC := aws.StringValue(sg.VpcId) != "" id := SecurityGroupRuleCreateID(securityGroupID, ruleType, ipPermission) @@ -245,7 +245,7 @@ func resourceSecurityGroupRuleRead(d *schema.ResourceData, meta interface{}) err return fmt.Errorf("reading Security Group (%s): %w", securityGroupID, err) } - ipPermission := expandIpPermission(d, sg) + ipPermission := expandIPPermission(d, sg) isVPC := aws.StringValue(sg.VpcId) != "" var rules []*ec2.IpPermission @@ -297,7 +297,7 @@ func resourceSecurityGroupRuleUpdate(d *schema.ResourceData, meta interface{}) e return fmt.Errorf("reading Security Group (%s): %w", securityGroupID, err) } - ipPermission := expandIpPermission(d, sg) + ipPermission := expandIPPermission(d, sg) ruleType := d.Get("type").(string) isVPC := aws.StringValue(sg.VpcId) != "" @@ -345,7 +345,7 @@ func resourceSecurityGroupRuleDelete(d *schema.ResourceData, meta interface{}) e return fmt.Errorf("reading Security Group (%s): %w", securityGroupID, err) } - ipPermission := expandIpPermission(d, sg) + ipPermission := expandIPPermission(d, sg) ruleType := d.Get("type").(string) isVPC := aws.StringValue(sg.VpcId) != "" @@ -688,7 +688,7 @@ func SecurityGroupRuleCreateID(securityGroupID, ruleType string, ip *ec2.IpPermi return fmt.Sprintf("sgrule-%d", create.StringHashcode(buf.String())) } -func expandIpPermission(d *schema.ResourceData, sg *ec2.SecurityGroup) *ec2.IpPermission { // nosemgrep:ci.caps5-in-func-name +func expandIPPermission(d *schema.ResourceData, sg *ec2.SecurityGroup) *ec2.IpPermission { // nosemgrep:ci.caps5-in-func-name apiObject := &ec2.IpPermission{ IpProtocol: aws.String(ProtocolForValue(d.Get("protocol").(string))), } diff --git a/internal/service/ec2/vpc_security_group_rule_test.go b/internal/service/ec2/vpc_security_group_rule_test.go index ba184214ac3..da8a8f22bfd 100644 --- a/internal/service/ec2/vpc_security_group_rule_test.go +++ b/internal/service/ec2/vpc_security_group_rule_test.go @@ -302,46 +302,6 @@ func TestAccVPCSecurityGroupRule_Ingress_ipv6(t *testing.T) { }) } -func TestAccVPCSecurityGroupRule_Ingress_classic(t *testing.T) { - var group ec2.SecurityGroup - rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) - resourceName := "aws_security_group_rule.test" - sgResourceName := "aws_security_group.test" - - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(t); acctest.PreCheckEC2Classic(t) }, - ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID), - ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, - CheckDestroy: testAccCheckSecurityGroupDestroy, - Steps: []resource.TestStep{ - { - Config: testAccVPCSecurityGroupRuleConfig_ingressClassic(rName), - Check: resource.ComposeAggregateTestCheckFunc( - testAccCheckSecurityGroupEC2ClassicExists(sgResourceName, &group), - resource.TestCheckResourceAttr(resourceName, "cidr_blocks.#", "1"), - resource.TestCheckResourceAttr(resourceName, "cidr_blocks.0", "10.0.0.0/8"), - resource.TestCheckNoResourceAttr(resourceName, "description"), - resource.TestCheckResourceAttr(resourceName, "from_port", "80"), - resource.TestCheckResourceAttr(resourceName, "ipv6_cidr_blocks.#", "0"), - resource.TestCheckResourceAttr(resourceName, "protocol", "tcp"), - resource.TestCheckResourceAttr(resourceName, "prefix_list_ids.#", "0"), - resource.TestCheckResourceAttrPair(resourceName, "security_group_id", sgResourceName, "id"), - resource.TestCheckResourceAttr(resourceName, "self", "false"), - resource.TestCheckNoResourceAttr(resourceName, "source_security_group_id"), - resource.TestCheckResourceAttr(resourceName, "to_port", "8000"), - resource.TestCheckResourceAttr(resourceName, "type", "ingress"), - ), - }, - { - ResourceName: resourceName, - ImportState: true, - ImportStateIdFunc: testAccSecurityGroupRuleImportStateIdFunc(resourceName), - ImportStateVerify: true, - }, - }, - }) -} - func TestAccVPCSecurityGroupRule_egress(t *testing.T) { var group ec2.SecurityGroup rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) @@ -1478,6 +1438,59 @@ func TestAccVPCSecurityGroupRule_Ingress_prefixListAndSource(t *testing.T) { }) } +func TestAccVPCSecurityGroupRule_protocolChange(t *testing.T) { + var group ec2.SecurityGroup + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_security_group_rule.test" + resourceName2 := "aws_security_group_rule.test2" + sgName := "aws_security_group.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t); testAccPreCheckManagedPrefixList(t) }, + ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckSecurityGroupDestroy, + Steps: []resource.TestStep{ + { + Config: testAccVPCSecurityGroupRuleConfig_protocolChange(rName, "tcp"), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckSecurityGroupExists(sgName, &group), + resource.TestCheckResourceAttr(resourceName, "cidr_blocks.#", "1"), + resource.TestCheckResourceAttr(resourceName, "protocol", "tcp"), + resource.TestCheckResourceAttr(resourceName, "type", "ingress"), + resource.TestCheckResourceAttr(resourceName2, "cidr_blocks.#", "1"), + resource.TestCheckResourceAttr(resourceName2, "protocol", "tcp"), + resource.TestCheckResourceAttr(resourceName2, "type", "ingress"), + ), + }, + { + Config: testAccVPCSecurityGroupRuleConfig_protocolChange(rName, "udp"), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckSecurityGroupExists(sgName, &group), + resource.TestCheckResourceAttr(resourceName, "cidr_blocks.#", "1"), + resource.TestCheckResourceAttr(resourceName, "protocol", "udp"), + resource.TestCheckResourceAttr(resourceName, "type", "ingress"), + resource.TestCheckResourceAttr(resourceName2, "cidr_blocks.#", "1"), + resource.TestCheckResourceAttr(resourceName2, "protocol", "udp"), + resource.TestCheckResourceAttr(resourceName2, "type", "ingress"), + ), + }, + { + Config: testAccVPCSecurityGroupRuleConfig_protocolChange(rName, "tcp"), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckSecurityGroupExists(sgName, &group), + resource.TestCheckResourceAttr(resourceName, "cidr_blocks.#", "1"), + resource.TestCheckResourceAttr(resourceName, "protocol", "tcp"), + resource.TestCheckResourceAttr(resourceName, "type", "ingress"), + resource.TestCheckResourceAttr(resourceName2, "cidr_blocks.#", "1"), + resource.TestCheckResourceAttr(resourceName2, "protocol", "tcp"), + resource.TestCheckResourceAttr(resourceName2, "type", "ingress"), + ), + }, + }, + }) +} + func testAccSecurityGroupRuleImportStateIdFunc(resourceName string) resource.ImportStateIdFunc { return func(s *terraform.State) (string, error) { rs, ok := s.RootModule().Resources[resourceName] @@ -1677,28 +1690,6 @@ resource "aws_security_group_rule" "test" { `, rName) } -func testAccVPCSecurityGroupRuleConfig_ingressClassic(rName string) string { - return acctest.ConfigCompose(acctest.ConfigEC2ClassicRegionProvider(), fmt.Sprintf(` -resource "aws_security_group" "test" { - name = %[1]q - - tags = { - Name = %[1]q - } -} - -resource "aws_security_group_rule" "test" { - type = "ingress" - protocol = "tcp" - from_port = 80 - to_port = 8000 - cidr_blocks = ["10.0.0.0/8"] - - security_group_id = aws_security_group.test.id -} -`, rName)) -} - func testAccVPCSecurityGroupRuleConfig_egress(rName string) string { return fmt.Sprintf(` resource "aws_security_group" "test" { @@ -2630,3 +2621,42 @@ resource "aws_security_group_rule" "test" { } `, rName) } + +func testAccVPCSecurityGroupRuleConfig_protocolChange(rName, protocol string) string { + return fmt.Sprintf(` +resource "aws_vpc" "test" { + cidr_block = "10.0.0.0/16" + + tags = { + Name = %[1]q + } +} + +resource "aws_security_group" "test" { + vpc_id = aws_vpc.test.id + name = %[1]q + + tags = { + Name = %[1]q + } +} + +resource "aws_security_group_rule" "test" { + type = "ingress" + from_port = 9443 + to_port = 9443 + protocol = %[2]q + cidr_blocks = [aws_vpc.test.cidr_block] + security_group_id = aws_security_group.test.id +} + +resource "aws_security_group_rule" "test2" { + type = "ingress" + from_port = 8989 + to_port = 8989 + protocol = %[2]q + cidr_blocks = [aws_vpc.test.cidr_block] + security_group_id = aws_security_group.test.id +} +`, rName, protocol) +} diff --git a/internal/service/ec2/vpc_security_group_test.go b/internal/service/ec2/vpc_security_group_test.go index ab84416b45c..8b83ca226e1 100644 --- a/internal/service/ec2/vpc_security_group_test.go +++ b/internal/service/ec2/vpc_security_group_test.go @@ -2,8 +2,6 @@ package ec2_test import ( "fmt" - "log" - "os" "reflect" "regexp" "strconv" @@ -18,8 +16,10 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" "github.com/hashicorp/terraform-provider-aws/internal/acctest" "github.com/hashicorp/terraform-provider-aws/internal/conns" + "github.com/hashicorp/terraform-provider-aws/internal/create" tfec2 "github.com/hashicorp/terraform-provider-aws/internal/service/ec2" "github.com/hashicorp/terraform-provider-aws/internal/tfresource" + "github.com/hashicorp/terraform-provider-aws/names" ) func TestProtocolStateFunc(t *testing.T) { @@ -395,12 +395,6 @@ func TestSecurityGroupIPPermGather(t *testing.T) { FromPort: aws.Int64(443), ToPort: aws.Int64(443), UserIdGroupPairs: []*ec2.UserIdGroupPair{ - // Classic - { - UserId: aws.String("12345"), - GroupId: aws.String("sg-33333"), - GroupName: aws.String("ec2_classic"), - }, { UserId: aws.String("amazon-elb"), GroupId: aws.String("sg-d2c979d3"), @@ -444,15 +438,6 @@ func TestSecurityGroupIPPermGather(t *testing.T) { "sg-22222", }), }, - { - "protocol": "tcp", - "from_port": int64(443), - "to_port": int64(443), - "security_groups": schema.NewSet(schema.HashString, []interface{}{ - "ec2_classic", - "amazon-elb/amazon-elb-sg", - }), - }, { "protocol": "-1", "from_port": int64(0), @@ -530,43 +515,6 @@ func TestAccVPCSecurityGroup_basic(t *testing.T) { }) } -func TestAccVPCSecurityGroup_basicEC2Classic(t *testing.T) { - var group ec2.SecurityGroup - resourceName := "aws_security_group.test" - rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) - - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(t); acctest.PreCheckEC2Classic(t) }, - ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID), - ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, - CheckDestroy: testAccCheckSecurityGroupEC2ClassicDestroy, - Steps: []resource.TestStep{ - { - Config: testAccVPCSecurityGroupConfig_ec2Classic(rName), - Check: resource.ComposeAggregateTestCheckFunc( - testAccCheckSecurityGroupEC2ClassicExists(resourceName, &group), - resource.TestCheckResourceAttr(resourceName, "description", "Managed by Terraform"), - resource.TestCheckResourceAttr(resourceName, "egress.#", "0"), - resource.TestCheckResourceAttr(resourceName, "ingress.#", "0"), - resource.TestCheckResourceAttr(resourceName, "name", rName), - resource.TestCheckResourceAttr(resourceName, "name_prefix", ""), - acctest.CheckResourceAttrAccountID(resourceName, "owner_id"), - resource.TestCheckResourceAttr(resourceName, "revoke_rules_on_delete", "false"), - resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), - resource.TestCheckResourceAttr(resourceName, "vpc_id", ""), - ), - }, - { - Config: testAccVPCSecurityGroupConfig_ec2Classic(rName), - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"revoke_rules_on_delete"}, - }, - }, - }) -} - func TestAccVPCSecurityGroup_disappears(t *testing.T) { var group ec2.SecurityGroup resourceName := "aws_security_group.test" @@ -1884,47 +1832,6 @@ func TestAccVPCSecurityGroup_ingressWithCIDRAndSGsVPC(t *testing.T) { }) } -func TestAccVPCSecurityGroup_ingressWithCIDRAndSGsClassic(t *testing.T) { - var group ec2.SecurityGroup - resourceName := "aws_security_group.test1" - rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) - - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(t); acctest.PreCheckEC2Classic(t) }, - ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID), - ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, - CheckDestroy: testAccCheckSecurityGroupEC2ClassicDestroy, - Steps: []resource.TestStep{ - { - Config: testAccVPCSecurityGroupConfig_ingressWithCIDRAndSGsEC2Classic(rName), - Check: resource.ComposeTestCheckFunc( - testAccCheckSecurityGroupEC2ClassicExists(resourceName, &group), - resource.TestCheckResourceAttr(resourceName, "egress.#", "0"), - resource.TestCheckResourceAttr(resourceName, "ingress.#", "2"), - resource.TestCheckTypeSetElemNestedAttrs(resourceName, "ingress.*", map[string]string{ - "cidr_blocks.#": "1", - "cidr_blocks.0": "192.168.0.1/32", - "description": "", - "from_port": "22", - "ipv6_cidr_blocks.#": "0", - "protocol": "tcp", - "security_groups.#": "0", - "self": "false", - "to_port": "22", - }), - ), - }, - { - Config: testAccVPCSecurityGroupConfig_ingressWithCIDRAndSGsEC2Classic(rName), - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"revoke_rules_on_delete"}, - }, - }, - }) -} - func TestAccVPCSecurityGroup_egressWithPrefixList(t *testing.T) { var group ec2.SecurityGroup resourceName := "aws_security_group.test" @@ -2057,8 +1964,35 @@ func TestAccVPCSecurityGroup_failWithDiffMismatch(t *testing.T) { }) } -func TestAccVPCSecurityGroup_ruleLimitExceededAppend(t *testing.T) { - ruleLimit := testAccSecurityGroupRulesPerGroupLimitFromEnv() +var ruleLimit int + +// testAccSecurityGroup_ruleLimit sets the global "ruleLimit" and is only called once +// but does not run in parallel slowing down tests. The mutex limits it to one slow down +// instead of one per test. It cannot run in parallel since it is called by another test +// and double paralleling is a panic. +func testAccSecurityGroup_ruleLimit(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckSecurityGroupDestroy, + Steps: []resource.TestStep{ + // get limit + { + Config: testAccVPCSecurityGroupConfig_getLimit(), + Check: resource.ComposeTestCheckFunc( + testAccCheckSecurityGroupRuleLimit("data.aws_servicequotas_service_quota.test", &ruleLimit), + ), + PreventPostDestroyRefresh: true, // saves a few seconds + }, + }, + }) +} + +func TestAccVPCSecurityGroup_RuleLimit_exceededAppend(t *testing.T) { + if ruleLimit == 0 { + testAccSecurityGroup_ruleLimit(t) + } var group ec2.SecurityGroup rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) @@ -2104,8 +2038,10 @@ func TestAccVPCSecurityGroup_ruleLimitExceededAppend(t *testing.T) { }) } -func TestAccVPCSecurityGroup_ruleLimitCIDRBlockExceededAppend(t *testing.T) { - ruleLimit := testAccSecurityGroupRulesPerGroupLimitFromEnv() +func TestAccVPCSecurityGroup_RuleLimit_cidrBlockExceededAppend(t *testing.T) { + if ruleLimit == 0 { + testAccSecurityGroup_ruleLimit(t) + } var group ec2.SecurityGroup rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) @@ -2165,8 +2101,10 @@ func TestAccVPCSecurityGroup_ruleLimitCIDRBlockExceededAppend(t *testing.T) { }) } -func TestAccVPCSecurityGroup_ruleLimitExceededPrepend(t *testing.T) { - ruleLimit := testAccSecurityGroupRulesPerGroupLimitFromEnv() +func TestAccVPCSecurityGroup_RuleLimit_exceededPrepend(t *testing.T) { + if ruleLimit == 0 { + testAccSecurityGroup_ruleLimit(t) + } var group ec2.SecurityGroup rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) @@ -2210,8 +2148,10 @@ func TestAccVPCSecurityGroup_ruleLimitExceededPrepend(t *testing.T) { }) } -func TestAccVPCSecurityGroup_ruleLimitExceededAllNew(t *testing.T) { - ruleLimit := testAccSecurityGroupRulesPerGroupLimitFromEnv() +func TestAccVPCSecurityGroup_RuleLimit_exceededAllNew(t *testing.T) { + if ruleLimit == 0 { + testAccSecurityGroup_ruleLimit(t) + } var group ec2.SecurityGroup rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) @@ -2287,6 +2227,40 @@ func TestAccVPCSecurityGroup_rulesDropOnError(t *testing.T) { }) } +// TestAccVPCSecurityGroup_emrDependencyViolation is very complex but captures +// a problem seen in EMR and other services. The main gist is that a security +// group can have 0 rules and still have dependencies. Services, like EMR, +// create rules in security groups. If a 0-rule SG is listed as the source of +// a rule in another SG, it could not previously be deleted. +func TestAccVPCSecurityGroup_emrDependencyViolation(t *testing.T) { + var group ec2.SecurityGroup + resourceName := "aws_security_group.allow_access" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckSecurityGroupDestroy, + Steps: []resource.TestStep{ + { + Config: testAccVPCSecurityGroupConfig_emrLinkedRulesDestroy(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckSecurityGroupExists(resourceName, &group), + acctest.MatchResourceAttrRegionalARN(resourceName, "arn", "ec2", regexp.MustCompile(`security-group/.+$`)), + resource.TestCheckResourceAttr(resourceName, "egress.#", "1"), + resource.TestCheckResourceAttr(resourceName, "ingress.#", "1"), + acctest.CheckResourceAttrAccountID(resourceName, "owner_id"), + resource.TestCheckResourceAttr(resourceName, "revoke_rules_on_delete", "true"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttrSet(resourceName, "vpc_id"), + ), + ExpectError: regexp.MustCompile("unexpected state"), + }, + }, + }) +} + // cycleIPPermForGroup returns an IpPermission struct with a configured // UserIdGroupPair for the groupid given. Used in // TestAccAWSSecurityGroup_forceRevokeRules_should_fail to create a cyclic rule @@ -2399,7 +2373,7 @@ func testAccCheckSecurityGroupDestroy(s *terraform.State) error { } if err != nil { - return err + return create.Error(names.EC2, create.ErrActionCheckingDestroyed, "Security Group", rs.Primary.ID, err) } return fmt.Errorf("VPC Security Group (%s) still exists.", rs.Primary.ID) @@ -2408,30 +2382,6 @@ func testAccCheckSecurityGroupDestroy(s *terraform.State) error { return nil } -func testAccCheckSecurityGroupEC2ClassicDestroy(s *terraform.State) error { // nosemgrep:ci.ec2-in-func-name - conn := acctest.ProviderEC2Classic.Meta().(*conns.AWSClient).EC2Conn - - for _, rs := range s.RootModule().Resources { - if rs.Type != "aws_security_group" { - continue - } - - _, err := tfec2.FindSecurityGroupByID(conn, rs.Primary.ID) - - if tfresource.NotFound(err) { - continue - } - - if err != nil { - return err - } - - return fmt.Errorf("EC2 Classic Security Group (%s) still exists.", rs.Primary.ID) - } - - return nil -} - func testAccCheckSecurityGroupExists(n string, v *ec2.SecurityGroup) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] @@ -2448,7 +2398,7 @@ func testAccCheckSecurityGroupExists(n string, v *ec2.SecurityGroup) resource.Te output, err := tfec2.FindSecurityGroupByID(conn, rs.Primary.ID) if err != nil { - return err + return create.Error(names.EC2, create.ErrActionCheckingExistence, "Security Group", rs.Primary.ID, err) } *v = *output @@ -2457,7 +2407,7 @@ func testAccCheckSecurityGroupExists(n string, v *ec2.SecurityGroup) resource.Te } } -func testAccCheckSecurityGroupEC2ClassicExists(n string, v *ec2.SecurityGroup) resource.TestCheckFunc { // nosemgrep:ci.ec2-in-func-name +func testAccCheckSecurityGroupRuleLimit(n string, v *int) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] if !ok { @@ -2465,45 +2415,20 @@ func testAccCheckSecurityGroupEC2ClassicExists(n string, v *ec2.SecurityGroup) r } if rs.Primary.ID == "" { - return fmt.Errorf("No EC2 Classic Security Group ID is set") + return fmt.Errorf("No Service Quotas ID is set") } - conn := acctest.ProviderEC2Classic.Meta().(*conns.AWSClient).EC2Conn - - output, err := tfec2.FindSecurityGroupByID(conn, rs.Primary.ID) - + limit, err := strconv.Atoi(rs.Primary.Attributes["value"]) if err != nil { - return err + return fmt.Errorf("converting value to int: %s", err) } - *v = *output + *v = limit return nil } } -// testAccSecurityGroupRulesPerGroupLimitFromEnv returns security group rules per group limit -// Currently this information is not available from any EC2 or Trusted Advisor API -// Prefers the EC2_SECURITY_GROUP_RULES_PER_GROUP_LIMIT environment variable or defaults to 50 -func testAccSecurityGroupRulesPerGroupLimitFromEnv() int { - const defaultLimit = 50 - const envVar = "EC2_SECURITY_GROUP_RULES_PER_GROUP_LIMIT" - - envLimitStr := os.Getenv(envVar) - if envLimitStr == "" { - return defaultLimit - } - envLimitInt, err := strconv.Atoi(envLimitStr) - if err != nil { - log.Printf("[WARN] Error converting %q environment variable value %q to integer: %s", envVar, envLimitStr, err) - return defaultLimit - } - if envLimitInt <= 50 { - return defaultLimit - } - return envLimitInt -} - func testAccCheckSecurityGroupRuleCount(group *ec2.SecurityGroup, expectedIngressCount, expectedEgressCount int) resource.TestCheckFunc { return func(s *terraform.State) error { id := aws.StringValue(group.GroupId) @@ -2519,7 +2444,7 @@ func testSecurityGroupRuleCount(id string, expectedIngressCount, expectedEgressC return fmt.Errorf("Security Group (%s) not found: %w", id, err) } if err != nil { - return err + return create.Error(names.EC2, create.ErrActionChecking, "Security Group", id, err) } if actual := len(group.IpPermissions); actual != expectedIngressCount { @@ -2550,14 +2475,6 @@ resource "aws_security_group" "test" { `, rName) } -func testAccVPCSecurityGroupConfig_ec2Classic(rName string) string { // nosemgrep:ci.ec2-in-func-name - return acctest.ConfigCompose(acctest.ConfigEC2ClassicRegionProvider(), fmt.Sprintf(` -resource "aws_security_group" "test" { - name = %[1]q -} -`, rName)) -} - func testAccVPCSecurityGroupConfig_nameGenerated(rName string) string { return fmt.Sprintf(` resource "aws_vpc" "test" { @@ -2634,6 +2551,15 @@ resource "aws_security_group" "test" { `, rName, tagKey1, tagValue1, tagKey2, tagValue2) } +func testAccVPCSecurityGroupConfig_getLimit() string { + return ` +data "aws_servicequotas_service_quota" "test" { + quota_name = "Inbound or outbound rules per security group" + service_code = "vpc" +} +` +} + func testAccVPCSecurityGroupConfig_ruleLimit(rName string, egressStartIndex, egressRulesCount int) string { var egressRules strings.Builder for i := egressStartIndex; i < egressRulesCount+egressStartIndex; i++ { @@ -2646,7 +2572,6 @@ func testAccVPCSecurityGroupConfig_ruleLimit(rName string, egressStartIndex, egr } `, i) } - return fmt.Sprintf(` resource "aws_vpc" "test" { cidr_block = "10.1.0.0/16" @@ -2837,6 +2762,10 @@ resource "aws_security_group" "primary" { tags = { Name = %[1]q } + + timeouts { + delete = "2m" + } } resource "aws_security_group" "secondary" { @@ -2846,6 +2775,10 @@ resource "aws_security_group" "secondary" { tags = { Name = %[1]q } + + timeouts { + delete = "2m" + } } `, rName) } @@ -3481,44 +3414,6 @@ resource "aws_security_group" "test1" { `, rName) } -func testAccVPCSecurityGroupConfig_ingressWithCIDRAndSGsEC2Classic(rName string) string { // nosemgrep:ci.ec2-in-func-name - return acctest.ConfigCompose(acctest.ConfigEC2ClassicRegionProvider(), fmt.Sprintf(` -resource "aws_security_group" "test2" { - name = "%[1]s-2" - - tags = { - Name = %[1]q - } -} - -resource "aws_security_group" "test1" { - name = "%[1]s-1" - - ingress { - protocol = "tcp" - from_port = "22" - to_port = "22" - - cidr_blocks = [ - "192.168.0.1/32", - ] - } - - ingress { - protocol = "tcp" - from_port = 80 - to_port = 8000 - cidr_blocks = ["10.0.0.0/8"] - security_groups = [aws_security_group.test2.name] - } - - tags = { - Name = %[1]q - } -} -`, rName)) -} - // fails to apply in one pass with the error "diffs didn't match during apply" // GH-2027 func testAccVPCSecurityGroupConfig_failWithDiffMismatch(rName string) string { @@ -4337,3 +4232,435 @@ resource "aws_security_group" "test" { } `, rName) } + +// testAccVPCSecurityGroupConfig_emrLinkedRulesDestroy is very involved but captures +// a problem seen in EMR and other contexts. +func testAccVPCSecurityGroupConfig_emrLinkedRulesDestroy(rName string) string { + return acctest.ConfigCompose( + acctest.ConfigAvailableAZsNoOptInDefaultExclude(), + fmt.Sprintf(` +# VPC +resource "aws_vpc" "main" { + cidr_block = "10.1.0.0/16" + tags = { + Name = %[1]q + } +} + +# subnets +resource "aws_subnet" "private" { + vpc_id = aws_vpc.main.id + cidr_block = "10.1.0.0/24" + availability_zone = data.aws_availability_zones.available.names[0] + + tags = { + Name = %[1]q + } +} + +resource "aws_security_group" "allow_ssh" { + name = "%[1]s-ssh" + description = "ssh" + vpc_id = aws_vpc.main.id + + tags = { + Name = "%[1]s-ssh" + } +} + +# internet gateway +resource "aws_internet_gateway" "gw" { + vpc_id = aws_vpc.main.id + + tags = { + Name = %[1]q + } +} + +# elastic ip for NAT gateway +resource "aws_eip" "nat" { + vpc = true + tags = { + Name = %[1]q + } +} + +# NAT gateway +resource "aws_nat_gateway" "nat" { + allocation_id = aws_eip.nat.id + subnet_id = aws_subnet.private.id + + tags = { + Name = %[1]q + } + + # To ensure proper ordering, it is recommended to add an explicit dependency + # on the Internet Gateway for the VPC. + depends_on = [aws_internet_gateway.gw] +} + +# route tables +# add internet gateway +resource "aws_route_table" "test" { + vpc_id = aws_vpc.main.id + + route { + cidr_block = "0.0.0.0/0" + gateway_id = aws_internet_gateway.gw.id + } + + tags = { + Name = %[1]q + } +} + +# route table for nat +resource "aws_route_table" "nat" { + vpc_id = aws_vpc.main.id + + route { + cidr_block = "0.0.0.0/0" + nat_gateway_id = aws_nat_gateway.nat.id + } + + tags = { + Name = %[1]q + } +} + +# associate nat route table with subnet +resource "aws_route_table_association" "nat" { + subnet_id = aws_subnet.private.id + route_table_id = aws_route_table.nat.id +} + +resource "aws_security_group" "allow_access" { + name = "%[1]s-allow-access" + description = "Allow inbound traffic" + vpc_id = aws_vpc.main.id + revoke_rules_on_delete = true + + ingress { + from_port = 0 + to_port = 0 + protocol = "-1" + cidr_blocks = [aws_vpc.main.cidr_block] + } + + egress { + from_port = 0 + to_port = 0 + protocol = "-1" + cidr_blocks = ["0.0.0.0/0"] + } + + lifecycle { + ignore_changes = [ + ingress, + egress, + ] + } + + tags = { + name = "%[1]s-allow-access" + } +} + +resource "aws_security_group" "service_access" { + name = "%[1]s-service-access" + description = "Allow inbound traffic" + vpc_id = aws_vpc.main.id + revoke_rules_on_delete = true + + ingress { + from_port = 0 + to_port = 0 + protocol = "-1" + cidr_blocks = [aws_vpc.main.cidr_block] + } + + ingress { + from_port = 8443 + to_port = 8443 + protocol = "tcp" + cidr_blocks = [aws_vpc.main.cidr_block] + security_groups = [aws_security_group.allow_access.id] + } + + ingress { + from_port = 9443 + to_port = 9443 + protocol = "tcp" + cidr_blocks = [aws_vpc.main.cidr_block] + security_groups = [aws_security_group.allow_access.id] + } + + egress { + from_port = 0 + to_port = 0 + protocol = "-1" + cidr_blocks = ["0.0.0.0/0"] + } + + lifecycle { + ignore_changes = [ + ingress, + egress, + ] + } + + tags = { + name = "%[1]s-service-access" + } +} + +# IAM role for EMR Service +resource "aws_iam_role" "iam_emr_service_role" { + name = "%[1]s-service-role" + + assume_role_policy = < **NOTE:** This is an advanced resource with special caveats. Please read this document in its entirety before using this resource. The `aws_default_security_group` resource behaves differently from normal resources. Terraform does not _create_ this resource but instead attempts to "adopt" it into management. -For EC2 Classic accounts, each region comes with a default security group. Additionally, each VPC created in AWS comes with a default security group that can be managed but not destroyed. - -When Terraform first adopts the default security group, it **immediately removes all ingress and egress rules in the Security Group**. It then creates any rules specified in the configuration. This way only the rules specified in the configuration are created. +When Terraform first begins managing the default security group, it **immediately removes all ingress and egress rules in the Security Group**. It then creates any rules specified in the configuration. This way only the rules specified in the configuration are created. This resource treats its inline rules as absolute; only the rules defined inline are created, and any additions/removals external to this resource will result in diff shown. For these reasons, this resource is incompatible with the `aws_security_group_rule` resource. @@ -94,7 +92,7 @@ Both `egress` and `ingress` objects have the same arguments. * `ipv6_cidr_blocks` - (Optional) List of IPv6 CIDR blocks. * `prefix_list_ids` - (Optional) List of prefix list IDs (for allowing access to VPC endpoints) * `protocol` - (Required) Protocol. If you select a protocol of "-1" (semantically equivalent to `all`, which is not a valid value here), you must specify a `from_port` and `to_port` equal to `0`. If not `icmp`, `tcp`, `udp`, or `-1` use the [protocol number](https://www.iana.org/assignments/protocol-numbers/protocol-numbers.xhtml). -* `security_groups` - (Optional) List of security group Group Names if using EC2-Classic, or Group IDs if using a VPC. +* `security_groups` - (Optional) List of security groups. A group name can be used relative to the default VPC. Otherwise, group ID. * `self` - (Optional) Whether the security group itself will be added as a source to this egress rule. * `to_port` - (Required) End range port (or ICMP code if protocol is `icmp`). diff --git a/website/docs/r/security_group.html.markdown b/website/docs/r/security_group.html.markdown index 5bc26ce4c71..fe1cb69e858 100644 --- a/website/docs/r/security_group.html.markdown +++ b/website/docs/r/security_group.html.markdown @@ -143,7 +143,7 @@ The following arguments are optional: * `description` - (Optional) Description of this ingress rule. * `ipv6_cidr_blocks` - (Optional) List of IPv6 CIDR blocks. * `prefix_list_ids` - (Optional) List of Prefix List IDs. -* `security_groups` - (Optional) List of security group Group Names if using EC2-Classic, or Group IDs if using a VPC. +* `security_groups` - (Optional) List of security groups. A group name can be used relative to the default VPC. Otherwise, group ID. * `self` - (Optional) Whether the security group itself will be added as a source to this ingress rule. ### egress @@ -162,7 +162,7 @@ The following arguments are optional: * `ipv6_cidr_blocks` - (Optional) List of IPv6 CIDR blocks. * `prefix_list_ids` - (Optional) List of Prefix List IDs. * `protocol` - (Required) Protocol. If you select a protocol of `-1` (semantically equivalent to `all`, which is not a valid value here), you must specify a `from_port` and `to_port` equal to 0. The supported values are defined in the `IpProtocol` argument in the [IpPermission](https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_IpPermission.html) API reference. This argument is normalized to a lowercase value to match the AWS API requirement when using Terraform 0.12.x and above. Please make sure that the value of the protocol is specified as lowercase when used with older version of Terraform to avoid issues during upgrade. -* `security_groups` - (Optional) List of security group Group Names if using EC2-Classic, or Group IDs if using a VPC. +* `security_groups` - (Optional) List of security groups. A group name can be used relative to the default VPC. Otherwise, group ID. * `self` - (Optional) Whether the security group itself will be added as a source to this egress rule. ## Attributes Reference