Mark the resources as computed so they don't force new resource when default is used (#5075)

[blckct]

Fixes #5075

emr_managed_master_security_group,emr_managed_slave_security_group and service_access_security_group get default security groups if none are given, that in turn causes terraform to want to create the resource again to change the group to empty. Marking them as computed fixes that problem while still allowing tf to mark the resource for recreation if some

• Mark emr_managed_master_security_group,emr_managed_slave_security_group and service_access_security_groupas computed

Output from acceptance testing:
N/A I'm not sure how to create a test to see if plan hasn't changed

[bflad]

I'm not sure how to create a test to see if plan hasn't changed

The acceptance testing framework performs the following steps (unless configured otherwise 😉 ):

• Apply
• Plan (without refresh) -- more for legacy purposes
• Plan (with refresh) -- will automatically report back a test failure if the configuration is still generating a difference. 👍

Your best bet is to setup a test like:

func TestAccAWSEMRCluster_XXXXXXX(t *testing.T) {
	var cluster emr.Cluster
	rInt := acctest.RandInt()
	resourceName := "aws_emr_cluster.tf-test-cluster"

	resource.Test(t, resource.TestCase{
		PreCheck:     func() { testAccPreCheck(t) },
		Providers:    testAccProviders,
		CheckDestroy: testAccCheckAWSEmrDestroy,
		Steps: []resource.TestStep{
			{
				Config: testAccAWSEmrClusterConfig_XXXXXXX(rInt),
				Check: resource.ComposeTestCheckFunc(
					testAccCheckAWSEmrClusterExists(resourceName, &cluster),
				),
			},
		},
	})
}

Where the provided Config will fail against master (show plan differences for the mentioned attributes after apply) and succeed with this change (no plan differences after apply). See aws/resource_aws_emr_cluster_test.go for the other testing as examples.

[blckct]

I'm in a bit of a pickle, since EMR creates default security groups in VPC now my tests fall with

`testing.go:588: Error destroying resource! WARNING: Dangling resources
may exist. The full state and error is shown below.

	Error: Error applying: 1 error occurred:
		* aws_vpc.vpc (destroy): 1 error occurred:
		* aws_vpc.vpc: DependencyViolation: The vpc 'vpc-XXXXXXXXXXXX' has dependencies and cannot be deleted.

`

Since the group name are know would it be okay to query the sg by name and delete all that match?

[blckct]

Apart from that the test seems to work

Master:

make && make testacc TESTARGS='-run=TestAccAWSEMRCluster_defaultRoles'
==> Checking that code complies with gofmt requirements...
go install
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./... -v -run=TestAccAWSEMRCluster_defaultRoles -timeout 120m
?   	github.com/terraform-providers/terraform-provider-aws	[no test files]



=== RUN   TestAccAWSEMRCluster_defaultRoles
--- FAIL: TestAccAWSEMRCluster_defaultRoles (610.60s)
	testing.go:527: Step 0 error: After applying this step, the plan was not empty:
(...)
	testing.go:588: Error destroying resource! WARNING: Dangling resources
		may exist. The full state and error is shown below.
(...)

My branch:

 make && make testacc TESTARGS='-run=TestAccAWSEMRCluster_defaultRoles'
==> Checking that code complies with gofmt requirements...
go install
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./... -v -run=TestAccAWSEMRCluster_defaultRoles -timeout 120m
?   	github.com/terraform-providers/terraform-provider-aws	[no test files]



=== RUN   TestAccAWSEMRCluster_defaultRoles
--- FAIL: TestAccAWSEMRCluster_defaultRoles (627.41s)
	testing.go:588: Error destroying resource! WARNING: Dangling resources
(...)

[blckct]

@bflad So could I have a hint how to implement a sweeper that will delete the security groups and then the VPC?

[bflad]

Hi again @blckct 👋 Sorry this got lost in the shuffle of other work. This type of acceptance testing implementation is very uncommon, but it is doable with using the Destroy, ExpectError, and PreCheck functionality of the acceptance testing framework. I don't believe there were any existing examples of this so it took some work to try and get it right. Here's what I came up with:

func TestAccAWSEMRCluster_Ec2Attributes_DefaultManagedSecurityGroups(t *testing.T) {
	var cluster emr.Cluster
	var vpc ec2.Vpc

	rName := acctest.RandomWithPrefix("tf-acc-test")
	resourceName := "aws_emr_cluster.tf-test-cluster"
	vpcResourceName := "aws_vpc.test"

	resource.ParallelTest(t, resource.TestCase{
		PreCheck:     func() { testAccPreCheck(t) },
		Providers:    testAccProviders,
		CheckDestroy: testAccCheckAWSEmrDestroy,
		Steps: []resource.TestStep{
			{
				Config: testAccAWSEmrClusterConfigEc2AttributesDefaultManagedSecurityGroups(rName),
				Check: resource.ComposeTestCheckFunc(
					testAccCheckAWSEmrClusterExists(resourceName, &cluster),
					testAccCheckVpcExists(vpcResourceName, &vpc),
				),
			},
			{
				Config:      testAccAWSEmrClusterConfigEc2AttributesDefaultManagedSecurityGroups(rName),
				Destroy:     true,
				ExpectError: regexp.MustCompile(`DependencyViolation`),
			},
			{
				PreConfig: func() {
					conn := testAccProvider.Meta().(*AWSClient).ec2conn

					err := testAccEmrDeleteManagedSecurityGroups(conn, &vpc)

					if err != nil {
						t.Fatal(err)
					}
				},
				Config:  testAccAWSEmrClusterConfigEc2AttributesDefaultManagedSecurityGroups(rName),
				Destroy: true,
			},
		},
	})
}

func testAccEmrDeleteManagedSecurityGroups(conn *ec2.EC2, vpc *ec2.Vpc) error {
	// Reference: https://docs.aws.amazon.com/emr/latest/ManagementGuide/emr-man-sec-groups.html
	managedSecurityGroups := map[string]*ec2.SecurityGroup{
		"ElasticMapReduce-master": nil,
		"ElasticMapReduce-slave":  nil,
	}

	for groupName := range managedSecurityGroups {
		securityGroup, err := testAccEmrDescribeManagedSecurityGroup(conn, vpc, groupName)

		if err != nil {
			return fmt.Errorf("error describing EMR Managed Security Group (%s): %s", groupName, err)
		}

		managedSecurityGroups[groupName] = securityGroup
	}

	// EMR Managed Security Groups rules reference each other, so rules from all
	// groups must be revoked first.
	for groupName, securityGroup := range managedSecurityGroups {
    if securityGroup == nil {
      continue
    }

		err := testAccEmrRevokeManagedSecurityGroup(conn, securityGroup)

		if err != nil {
			return fmt.Errorf("error revoking EMR Managed Security Group (%s): %s", groupName, err)
		}
	}

	for groupName, securityGroup := range managedSecurityGroups {
    if securityGroup == nil {
      continue
    }

		err := testAccEmrDeleteManagedSecurityGroup(conn, securityGroup)

		if err != nil {
			return fmt.Errorf("error deleting EMR Managed Security Group (%s): %s", groupName, err)
		}
	}

	return nil
}

func testAccEmrDescribeManagedSecurityGroup(conn *ec2.EC2, vpc *ec2.Vpc, securityGroupName string) (*ec2.SecurityGroup, error) {
	input := &ec2.DescribeSecurityGroupsInput{
		Filters: []*ec2.Filter{
			{
				Name:   aws.String("group-name"),
				Values: aws.StringSlice([]string{securityGroupName}),
			},
			{
				Name:   aws.String("vpc-id"),
				Values: []*string{vpc.VpcId},
			},
		},
	}

	output, err := conn.DescribeSecurityGroups(input)

	if err != nil {
		return nil, err
	}

	if output == nil || len(output.SecurityGroups) != 1 {
		return nil, nil
	}

	return output.SecurityGroups[0], nil
}

func testAccEmrRevokeManagedSecurityGroup(conn *ec2.EC2, securityGroup *ec2.SecurityGroup) error {
	input := &ec2.RevokeSecurityGroupIngressInput{
		GroupId:       securityGroup.GroupId,
		IpPermissions: securityGroup.IpPermissions,
	}

	_, err := conn.RevokeSecurityGroupIngress(input)

	return err
}

func testAccEmrDeleteManagedSecurityGroup(conn *ec2.EC2, securityGroup *ec2.SecurityGroup) error {
	input := &ec2.DeleteSecurityGroupInput{
		GroupId: securityGroup.GroupId,
	}

	_, err := conn.DeleteSecurityGroup(input)

	return err
}

func testAccAWSEmrClusterConfigEc2AttributesDefaultManagedSecurityGroups(rName string) string {
	return testAccAWSEmrClusterConfigBaseVpc(false) + fmt.Sprintf(`
resource "aws_emr_cluster" "tf-test-cluster" {
  applications                      = ["Spark"]
  keep_job_flow_alive_when_no_steps = true
  name                              = %[1]q
  release_label                     = "emr-5.28.0"
  service_role                      = "EMR_DefaultRole"

  ec2_attributes {
    instance_profile = "EMR_EC2_DefaultRole"
    subnet_id        = "${aws_subnet.test.id}"
  }

  master_instance_group {
    instance_type = "m4.large"
  }

  depends_on = ["aws_route_table_association.test"]
}
`, rName)
}

I will add this implementation as a followup commit to yours since it is passing:

--- PASS: TestAccAWSEMRCluster_Ec2Attributes_DefaultManagedSecurityGroups (760.51s)

And will merge this in. Thanks for your work!

[ghost]

This has been released in version 2.45.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!