From 8caaf31ddfded78970e2f957b3c5d6dc35864395 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 27 Jan 2022 16:45:11 -0500 Subject: [PATCH 1/2] r/aws_batch_compute_environment: Enforce no compute_resources block for UNMANAGED compute environments. --- internal/service/batch/compute_environment.go | 29 +++++++++---------- .../service/batch/compute_environment_test.go | 21 ++------------ 2 files changed, 16 insertions(+), 34 deletions(-) diff --git a/internal/service/batch/compute_environment.go b/internal/service/batch/compute_environment.go index b8a149e30af..7960bb2a592 100644 --- 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 index 637f85958c9..de4f18d5fc4 100644 --- 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. }, }) } From 1ccf664ba0874e569283fcd77ac22af672a3601f Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 27 Jan 2022 16:48:53 -0500 Subject: [PATCH 2/2] Add CHANGELOG entry. --- .changelog/22805.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/22805.txt diff --git a/.changelog/22805.txt b/.changelog/22805.txt new file mode 100644 index 00000000000..712a76bdae3 --- /dev/null +++ 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` +``` \ No newline at end of file