Merge pull request #22805 from hashicorp/f-aws_batch_compute_environm…

…ent-no-compute_resources-when-UNMANAGED r/aws_batch_compute_environment: Enforce no `compute_resources` block for `UNMANAGED` compute environments
hashicorp · Feb 2, 2022 · a2b935c · a2b935c
2 parents 18aa232 + 1ccf664
commit a2b935c
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 34 deletions.
diff --git a/.changelog/22805.txt b/.changelog/22805.txt
@@ -0,0 +1,3 @@
+```release-note:breaking-change
+resource/aws_batch_compute_environment: No `compute_resources` configuration block can be specified when `type` is `UNMANAGED`
+```
diff --git a/internal/service/batch/compute_environment.go b/internal/service/batch/compute_environment.go
@@ -248,12 +248,8 @@ func resourceComputeEnvironmentCreate(d *schema.ResourceData, meta interface{})
 		Type:                   aws.String(computeEnvironmentType),
 	}
 
-	// TODO Check in CustomizeDiff that UNMANAGED compute environment has no compute_resources.
-	// TODO This would be a breaking change.
-	if computeEnvironmentType := strings.ToUpper(computeEnvironmentType); computeEnvironmentType == batch.CETypeManaged {
-		if v, ok := d.GetOk("compute_resources"); ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil {
-			input.ComputeResources = expandBatchComputeResource(v.([]interface{})[0].(map[string]interface{}))
-		}
+	if v, ok := d.GetOk("compute_resources"); ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil {
+		input.ComputeResources = expandBatchComputeResource(v.([]interface{})[0].(map[string]interface{}))
 	}
 
 	if v, ok := d.GetOk("state"); ok {
@@ -309,15 +305,12 @@ func resourceComputeEnvironmentRead(d *schema.ResourceData, meta interface{}) er
 	d.Set("status_reason", computeEnvironment.StatusReason)
 	d.Set("type", computeEnvironmentType)
 
-	// TODO See above on how to remove check on type.
-	if computeEnvironmentType == batch.CETypeManaged {
-		if computeEnvironment.ComputeResources != nil {
-			if err := d.Set("compute_resources", []interface{}{flattenBatchComputeResource(computeEnvironment.ComputeResources)}); err != nil {
-				return fmt.Errorf("error setting compute_resources: %w", err)
-			}
-		} else {
-			d.Set("compute_resources", nil)
+	if computeEnvironment.ComputeResources != nil {
+		if err := d.Set("compute_resources", []interface{}{flattenBatchComputeResource(computeEnvironment.ComputeResources)}); err != nil {
+			return fmt.Errorf("error setting compute_resources: %w", err)
 		}
+	} else {
+		d.Set("compute_resources", nil)
 	}
 
 	tags := KeyValueTags(computeEnvironment.Tags).IgnoreAWS().IgnoreConfig(ignoreTagsConfig)
@@ -350,7 +343,6 @@ func resourceComputeEnvironmentUpdate(d *schema.ResourceData, meta interface{})
 			input.State = aws.String(d.Get("state").(string))
 		}
 
-		// TODO See above on how to remove check on type.
 		if computeEnvironmentType := strings.ToUpper(d.Get("type").(string)); computeEnvironmentType == batch.CETypeManaged {
 			// "At least one compute-resources attribute must be specified"
 			computeResourceUpdate := &batch.ComputeResourceUpdate{
@@ -435,6 +427,13 @@ func resourceComputeEnvironmentDelete(d *schema.ResourceData, meta interface{})
 }
 
 func resourceComputeEnvironmentCustomizeDiff(_ context.Context, diff *schema.ResourceDiff, meta interface{}) error {
+	if computeEnvironmentType := strings.ToUpper(diff.Get("type").(string)); computeEnvironmentType == batch.CETypeUnmanaged {
+		// UNMANAGED compute environments can have no compute_resources configured.
+		if v, ok := diff.GetOk("compute_resources"); ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil {
+			return fmt.Errorf("no `compute_resources` can be specified when `type` is %q", computeEnvironmentType)
+		}
+	}
+
 	if diff.Id() != "" {
 		// Update.
 

diff --git a/internal/service/batch/compute_environment_test.go b/internal/service/batch/compute_environment_test.go
@@ -1324,10 +1324,7 @@ func TestAccBatchComputeEnvironment_tags(t *testing.T) {
 }
 
 func TestAccBatchComputeEnvironment_createUnmanagedWithComputeResources(t *testing.T) {
-	var ce batch.ComputeEnvironmentDetail
 	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
-	resourceName := "aws_batch_compute_environment.test"
-	serviceRoleResourceName := "aws_iam_role.batch_service"
 
 	resource.ParallelTest(t, resource.TestCase{
 		PreCheck:     func() { acctest.PreCheck(t); testAccPreCheck(t) },
@@ -1336,23 +1333,9 @@ func TestAccBatchComputeEnvironment_createUnmanagedWithComputeResources(t *testi
 		CheckDestroy: testAccCheckBatchComputeEnvironmentDestroy,
 		Steps: []resource.TestStep{
 			{
-				Config: testAccComputeEnvironmentUnmanagedWithComputeResourcesConfig(rName),
-				Check: resource.ComposeTestCheckFunc(
-					testAccCheckComputeEnvironmentExists(resourceName, &ce),
-					acctest.CheckResourceAttrRegionalARN(resourceName, "arn", "batch", fmt.Sprintf("compute-environment/%s", rName)),
-					resource.TestCheckResourceAttr(resourceName, "compute_environment_name", rName),
-					resource.TestCheckResourceAttr(resourceName, "compute_environment_name_prefix", ""),
-					resource.TestCheckResourceAttr(resourceName, "compute_resources.#", "1"),
-					resource.TestCheckResourceAttrSet(resourceName, "ecs_cluster_arn"),
-					resource.TestCheckResourceAttrPair(resourceName, "service_role", serviceRoleResourceName, "arn"),
-					resource.TestCheckResourceAttr(resourceName, "state", "ENABLED"),
-					resource.TestCheckResourceAttrSet(resourceName, "status"),
-					resource.TestCheckResourceAttrSet(resourceName, "status_reason"),
-					resource.TestCheckResourceAttr(resourceName, "tags.%", "0"),
-					resource.TestCheckResourceAttr(resourceName, "type", "UNMANAGED"),
-				),
+				Config:      testAccComputeEnvironmentUnmanagedWithComputeResourcesConfig(rName),
+				ExpectError: regexp.MustCompile("no `compute_resources` can be specified when `type` is \"UNMANAGED\""),
 			},
-			// Can't import in this scenario.
 		},
 	})
 }