Skip to content

Commit

Permalink
service/batch: Support resource tagging and prevent differences with …
Browse files Browse the repository at this point in the history 
…new secrets support in container properties (#15470)

* service/batch: Support resource tagging and prevent differences with new secrets support in container properties

Reference: #12997
Reference: #15469
Reference: https://aws.amazon.com/about-aws/whats-new/2020/10/aws-batch-now-supports-custom-logging-configurations-swap-space-and-shared-memory/

Changes:

```
* data-source/aws_batch_compute_environment: Add `tags` attribute
* data-source/aws_batch_job_queue: Add `tags` attribute
* resource/aws_batch_compute_environment: Add `tags` argument
* resource/aws_batch_job_definition: Add `tags` argument
* resource/aws_batch_job_queue: Add `tags` argument

BUG FIXES

* resource/aws_batch_job_definition: Prevent unexpected plan difference for `container_properties` argument value with new secrets support
```

With the recent Batch service updates, the new secrets support in container properties would show as a confusing plan difference:

```
              ~ container_properties = jsonencode(
                  ~ {
                        command              = [
                            "echo",
                            "test",
                        ]
                      - environment          = [] -> null
                        image                = "busybox"
                        memory               = 128
                      - mountPoints          = [] -> null
                      - resourceRequirements = [] -> null
                      - secrets              = [] -> null
                      - ulimits              = [] -> null
                        vcpus                = 1
                      - volumes              = [] -> null
                    }
                )
```

This augments the Batch equivalency to ensure the canonicalized API response of an empty secrets array matches no secrets configuration.

While verifying the updates, this fixes the following two AWS GovCloud (US) test failures due to hardcoded partition handling:

```
=== CONT  TestAccDataSourceAwsBatchJobQueue_basic
    data_source_aws_batch_job_queue_test.go:16: Step 1/1 error: terraform failed: exit status 1

        stderr:

        Error: Provider produced inconsistent result after apply

        When applying changes to aws_iam_role_policy_attachment.ecs_instance_role,
        provider "registry.terraform.io/-/aws" produced an unexpected new value for
        was present, but now absent.

        This is a bug in the provider, which should be reported in the provider's own
        issue tracker.

        Error: Provider produced inconsistent result after apply

        When applying changes to
        aws_iam_role_policy_attachment.aws_batch_service_role, provider
        "registry.terraform.io/-/aws" produced an unexpected new value for was
        present, but now absent.

        This is a bug in the provider, which should be reported in the provider's own
        issue tracker.

=== CONT  TestAccDataSourceAwsBatchComputeEnvironment_basic
    data_source_aws_batch_compute_environment_test.go:16: Step 1/1 error: terraform failed: exit status 1

        stderr:

        Error: Provider produced inconsistent result after apply

        When applying changes to aws_iam_role_policy_attachment.ecs_instance_role,
        provider "registry.terraform.io/-/aws" produced an unexpected new value for
        was present, but now absent.

        This is a bug in the provider, which should be reported in the provider's own
        issue tracker.

        Error: Provider produced inconsistent result after apply

        When applying changes to
        aws_iam_role_policy_attachment.aws_batch_service_role, provider
        "registry.terraform.io/-/aws" produced an unexpected new value for was
        present, but now absent.

        This is a bug in the provider, which should be reported in the provider's own
        issue tracker.
```

Output from acceptance testing in AWS Commercial:

```
--- PASS: TestAccAWSBatchComputeEnvironment_ComputeResources_DesiredVcpus_Computed (224.88s)
--- PASS: TestAccAWSBatchComputeEnvironment_ComputeResources_MaxVcpus (105.82s)
--- PASS: TestAccAWSBatchComputeEnvironment_ComputeResources_MinVcpus (213.34s)
--- PASS: TestAccAWSBatchComputeEnvironment_createEc2 (51.12s)
--- PASS: TestAccAWSBatchComputeEnvironment_createEc2WithoutComputeResources (32.16s)
--- PASS: TestAccAWSBatchComputeEnvironment_createEc2WithTags (46.93s)
--- PASS: TestAccAWSBatchComputeEnvironment_createSpot (54.38s)
--- PASS: TestAccAWSBatchComputeEnvironment_createSpotWithAllocationStrategy (54.66s)
--- PASS: TestAccAWSBatchComputeEnvironment_createSpotWithoutBidPercentage (22.83s)
--- PASS: TestAccAWSBatchComputeEnvironment_createUnmanaged (46.86s)
--- PASS: TestAccAWSBatchComputeEnvironment_createUnmanagedWithComputeResources (54.73s)
--- PASS: TestAccAWSBatchComputeEnvironment_createWithNamePrefix (54.19s)
--- PASS: TestAccAWSBatchComputeEnvironment_disappears (49.79s)
--- PASS: TestAccAWSBatchComputeEnvironment_launchTemplate (62.07s)
--- PASS: TestAccAWSBatchComputeEnvironment_Tags (107.51s)
--- PASS: TestAccAWSBatchComputeEnvironment_updateComputeEnvironmentName (97.15s)
--- PASS: TestAccAWSBatchComputeEnvironment_updateInstanceType (96.17s)
--- PASS: TestAccAWSBatchComputeEnvironment_UpdateLaunchTemplate (110.14s)
--- PASS: TestAccAWSBatchComputeEnvironment_updateState (92.78s)

--- PASS: TestAccAWSBatchJobDefinition_basic (31.33s)
--- PASS: TestAccAWSBatchJobDefinition_ContainerProperties_Advanced (30.82s)
--- PASS: TestAccAWSBatchJobDefinition_Tags (51.78s)
--- PASS: TestAccAWSBatchJobDefinition_updateForcesNewResource (36.03s)

--- PASS: TestAccAWSBatchJobQueue_basic (142.93s)
--- PASS: TestAccAWSBatchJobQueue_ComputeEnvironments_ExternalOrderUpdate (146.15s)
--- PASS: TestAccAWSBatchJobQueue_disappears (159.82s)
--- PASS: TestAccAWSBatchJobQueue_Priority (182.59s)
--- PASS: TestAccAWSBatchJobQueue_State (146.39s)
--- PASS: TestAccAWSBatchJobQueue_Tags (210.84s)

--- PASS: TestAccDataSourceAwsBatchComputeEnvironment_basic (58.52s)

--- PASS: TestAccDataSourceAwsBatchJobQueue_basic (145.37s)
```

Output from acceptance testing in AWS GovCloud (US):

```
--- PASS: TestAccAWSBatchComputeEnvironment_ComputeResources_DesiredVcpus_Computed (246.11s)
--- PASS: TestAccAWSBatchComputeEnvironment_ComputeResources_MaxVcpus (143.57s)
--- PASS: TestAccAWSBatchComputeEnvironment_ComputeResources_MinVcpus (271.07s)
--- PASS: TestAccAWSBatchComputeEnvironment_createEc2 (52.91s)
--- PASS: TestAccAWSBatchComputeEnvironment_createEc2WithoutComputeResources (24.92s)
--- PASS: TestAccAWSBatchComputeEnvironment_createEc2WithTags (58.91s)
--- PASS: TestAccAWSBatchComputeEnvironment_createSpot (62.24s)
--- PASS: TestAccAWSBatchComputeEnvironment_createSpotWithAllocationStrategy (57.05s)
--- PASS: TestAccAWSBatchComputeEnvironment_createSpotWithoutBidPercentage (22.58s)
--- PASS: TestAccAWSBatchComputeEnvironment_createUnmanaged (51.34s)
--- PASS: TestAccAWSBatchComputeEnvironment_createUnmanagedWithComputeResources (64.74s)
--- PASS: TestAccAWSBatchComputeEnvironment_createWithNamePrefix (51.71s)
--- PASS: TestAccAWSBatchComputeEnvironment_disappears (60.41s)
--- PASS: TestAccAWSBatchComputeEnvironment_launchTemplate (77.36s)
--- PASS: TestAccAWSBatchComputeEnvironment_Tags (121.87s)
--- PASS: TestAccAWSBatchComputeEnvironment_updateComputeEnvironmentName (108.52s)
--- PASS: TestAccAWSBatchComputeEnvironment_updateInstanceType (125.32s)
--- PASS: TestAccAWSBatchComputeEnvironment_UpdateLaunchTemplate (110.03s)
--- PASS: TestAccAWSBatchComputeEnvironment_updateState (91.09s)

--- PASS: TestAccAWSBatchJobDefinition_basic (24.22s)
--- PASS: TestAccAWSBatchJobDefinition_ContainerProperties_Advanced (24.59s)
--- PASS: TestAccAWSBatchJobDefinition_Tags (67.28s)
--- PASS: TestAccAWSBatchJobDefinition_updateForcesNewResource (36.09s)

--- PASS: TestAccAWSBatchJobQueue_basic (150.57s)
--- PASS: TestAccAWSBatchJobQueue_ComputeEnvironments_ExternalOrderUpdate (160.87s)
--- PASS: TestAccAWSBatchJobQueue_disappears (139.57s)
--- PASS: TestAccAWSBatchJobQueue_Priority (155.30s)
--- PASS: TestAccAWSBatchJobQueue_State (182.97s)
--- PASS: TestAccAWSBatchJobQueue_Tags (159.11s)

--- PASS: TestAccDataSourceAwsBatchComputeEnvironment_basic (54.23s)

--- PASS: TestAccDataSourceAwsBatchJobQueue_basic (139.32s)
```

* Update aws/data_source_aws_batch_compute_environment.go

Co-authored-by: Graham Davison <[email protected]>

Co-authored-by: Graham Davison <[email protected]>
  • Loading branch information
@bflad @gdavison
bflad and gdavison authored Oct 7, 2020
1 parent 2cb328a commit 3249fb4
Show file tree
Hide file tree
Showing 24 changed files with 501 additions and 135 deletions.
9 changes: 9 additions & 0 deletions aws/data_source_aws_batch_compute_environment.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/batch"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags"
)

func dataSourceAwsBatchComputeEnvironment() *schema.Resource {
Expand Down Expand Up @@ -34,6 +35,8 @@ func dataSourceAwsBatchComputeEnvironment() *schema.Resource {
Computed: true,
},

"tags": tagsSchemaComputed(),

"type": {
Type: schema.TypeString,
Computed: true,
Expand All @@ -59,6 +62,7 @@ func dataSourceAwsBatchComputeEnvironment() *schema.Resource {

func dataSourceAwsBatchComputeEnvironmentRead(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).batchconn
ignoreTagsConfig := meta.(*AWSClient).IgnoreTagsConfig

params := &batch.DescribeComputeEnvironmentsInput{
ComputeEnvironments: []*string{aws.String(d.Get("compute_environment_name").(string))},
Expand Down Expand Up @@ -88,5 +92,10 @@ func dataSourceAwsBatchComputeEnvironmentRead(d *schema.ResourceData, meta inter
d.Set("status", computeEnvironment.Status)
d.Set("status_reason", computeEnvironment.StatusReason)
d.Set("state", computeEnvironment.State)

if err := d.Set("tags", keyvaluetags.BatchKeyValueTags(computeEnvironment.Tags).IgnoreAws().IgnoreConfig(ignoreTagsConfig).Map()); err != nil {
return fmt.Errorf("error setting tags: %w", err)
}

return nil
}
51 changes: 13 additions & 38 deletions aws/data_source_aws_batch_compute_environment_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (

"github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
)

func TestAccDataSourceAwsBatchComputeEnvironment_basic(t *testing.T) {
Expand All @@ -21,47 +20,23 @@ func TestAccDataSourceAwsBatchComputeEnvironment_basic(t *testing.T) {
{
Config: testAccDataSourceAwsBatchComputeEnvironmentConfig(rName),
Check: resource.ComposeTestCheckFunc(
testAccDataSourceAwsBatchComputeEnvironmentCheck(datasourceName, resourceName),
resource.TestCheckResourceAttrPair(datasourceName, "arn", resourceName, "arn"),
resource.TestCheckResourceAttrPair(datasourceName, "compute_environment_name", resourceName, "compute_environment_name"),
resource.TestCheckResourceAttrPair(datasourceName, "ecs_cluster_arn", resourceName, "ecs_cluster_arn"),
resource.TestCheckResourceAttrPair(datasourceName, "service_role", resourceName, "service_role"),
resource.TestCheckResourceAttrPair(datasourceName, "state", resourceName, "state"),
resource.TestCheckResourceAttrPair(datasourceName, "tags.%", resourceName, "tags.%"),
resource.TestCheckResourceAttrPair(datasourceName, "type", resourceName, "type"),
),
},
},
})
}

func testAccDataSourceAwsBatchComputeEnvironmentCheck(datasourceName, resourceName string) resource.TestCheckFunc {
return func(s *terraform.State) error {
ds, ok := s.RootModule().Resources[datasourceName]
if !ok {
return fmt.Errorf("root module has no data source called %s", datasourceName)
}

batchCeRs, ok := s.RootModule().Resources[resourceName]
if !ok {
return fmt.Errorf("root module has no resource called %s", resourceName)
}

attrNames := []string{
"arn",
"compute_environment_name",
}

for _, attrName := range attrNames {
if ds.Primary.Attributes[attrName] != batchCeRs.Primary.Attributes[attrName] {
return fmt.Errorf(
"%s is %s; want %s",
attrName,
ds.Primary.Attributes[attrName],
batchCeRs.Primary.Attributes[attrName],
)
}
}

return nil
}
}

func testAccDataSourceAwsBatchComputeEnvironmentConfig(rName string) string {
return fmt.Sprintf(`
data "aws_partition" "current" {}
resource "aws_iam_role" "ecs_instance_role" {
name = "ecs_%[1]s"
Expand All @@ -73,7 +48,7 @@ resource "aws_iam_role" "ecs_instance_role" {
"Action": "sts:AssumeRole",
"Effect": "Allow",
"Principal": {
"Service": "ec2.amazonaws.com"
"Service": "ec2.${data.aws_partition.current.dns_suffix}"
}
}
]
Expand All @@ -83,7 +58,7 @@ EOF
resource "aws_iam_role_policy_attachment" "ecs_instance_role" {
role = aws_iam_role.ecs_instance_role.name
policy_arn = "arn:aws:iam::aws:policy/service-role/AmazonEC2ContainerServiceforEC2Role"
policy_arn = "arn:${data.aws_partition.current.partition}:iam::aws:policy/service-role/AmazonEC2ContainerServiceforEC2Role"
}
resource "aws_iam_instance_profile" "ecs_instance_role" {
Expand All @@ -102,7 +77,7 @@ resource "aws_iam_role" "aws_batch_service_role" {
"Action": "sts:AssumeRole",
"Effect": "Allow",
"Principal": {
"Service": "batch.amazonaws.com"
"Service": "batch.${data.aws_partition.current.dns_suffix}"
}
}
]
Expand All @@ -112,7 +87,7 @@ EOF
resource "aws_iam_role_policy_attachment" "aws_batch_service_role" {
role = aws_iam_role.aws_batch_service_role.name
policy_arn = "arn:aws:iam::aws:policy/service-role/AWSBatchServiceRole"
policy_arn = "arn:${data.aws_partition.current.partition}:iam::aws:policy/service-role/AWSBatchServiceRole"
}
resource "aws_security_group" "sample" {
Expand Down
8 changes: 8 additions & 0 deletions aws/data_source_aws_batch_job_queue.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/batch"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags"
)

func dataSourceAwsBatchJobQueue() *schema.Resource {
Expand Down Expand Up @@ -39,6 +40,8 @@ func dataSourceAwsBatchJobQueue() *schema.Resource {
Computed: true,
},

"tags": tagsSchemaComputed(),

"priority": {
Type: schema.TypeInt,
Computed: true,
Expand Down Expand Up @@ -66,6 +69,7 @@ func dataSourceAwsBatchJobQueue() *schema.Resource {

func dataSourceAwsBatchJobQueueRead(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).batchconn
ignoreTagsConfig := meta.(*AWSClient).IgnoreTagsConfig

params := &batch.DescribeJobQueuesInput{
JobQueues: []*string{aws.String(d.Get("name").(string))},
Expand Down Expand Up @@ -105,5 +109,9 @@ func dataSourceAwsBatchJobQueueRead(d *schema.ResourceData, meta interface{}) er
return fmt.Errorf("error setting compute_environment_order: %s", err)
}

if err := d.Set("tags", keyvaluetags.BatchKeyValueTags(jobQueue.Tags).IgnoreAws().IgnoreConfig(ignoreTagsConfig).Map()); err != nil {
return fmt.Errorf("error setting tags: %s", err)
}

return nil
}
52 changes: 12 additions & 40 deletions aws/data_source_aws_batch_job_queue_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (

"github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
)

func TestAccDataSourceAwsBatchJobQueue_basic(t *testing.T) {
Expand All @@ -21,49 +20,22 @@ func TestAccDataSourceAwsBatchJobQueue_basic(t *testing.T) {
{
Config: testAccDataSourceAwsBatchJobQueueConfig(rName),
Check: resource.ComposeTestCheckFunc(
testAccDataSourceAwsBatchJobQueueCheck(datasourceName, resourceName),
resource.TestCheckResourceAttrPair(datasourceName, "arn", resourceName, "arn"),
resource.TestCheckResourceAttrPair(datasourceName, "compute_environment_order.#", resourceName, "compute_environments.#"),
resource.TestCheckResourceAttrPair(datasourceName, "name", resourceName, "name"),
resource.TestCheckResourceAttrPair(datasourceName, "priority", resourceName, "priority"),
resource.TestCheckResourceAttrPair(datasourceName, "state", resourceName, "state"),
resource.TestCheckResourceAttrPair(datasourceName, "tags.%", resourceName, "tags.%"),
),
},
},
})
}

func testAccDataSourceAwsBatchJobQueueCheck(datasourceName, resourceName string) resource.TestCheckFunc {
return func(s *terraform.State) error {
ds, ok := s.RootModule().Resources[datasourceName]
if !ok {
return fmt.Errorf("root module has no data source called %s", datasourceName)
}

jobQueueRs, ok := s.RootModule().Resources[resourceName]
if !ok {
return fmt.Errorf("root module has no resource called %s", resourceName)
}

attrNames := []string{
"arn",
"name",
"state",
"priority",
}

for _, attrName := range attrNames {
if ds.Primary.Attributes[attrName] != jobQueueRs.Primary.Attributes[attrName] {
return fmt.Errorf(
"%s is %s; want %s",
attrName,
ds.Primary.Attributes[attrName],
jobQueueRs.Primary.Attributes[attrName],
)
}
}

return nil
}
}

func testAccDataSourceAwsBatchJobQueueConfig(rName string) string {
return fmt.Sprintf(`
data "aws_partition" "current" {}
resource "aws_iam_role" "ecs_instance_role" {
name = "ecs_%[1]s"
Expand All @@ -75,7 +47,7 @@ resource "aws_iam_role" "ecs_instance_role" {
"Action": "sts:AssumeRole",
"Effect": "Allow",
"Principal": {
"Service": "ec2.amazonaws.com"
"Service": "ec2.${data.aws_partition.current.dns_suffix}"
}
}
]
Expand All @@ -85,7 +57,7 @@ EOF
resource "aws_iam_role_policy_attachment" "ecs_instance_role" {
role = aws_iam_role.ecs_instance_role.name
policy_arn = "arn:aws:iam::aws:policy/service-role/AmazonEC2ContainerServiceforEC2Role"
policy_arn = "arn:${data.aws_partition.current.partition}:iam::aws:policy/service-role/AmazonEC2ContainerServiceforEC2Role"
}
resource "aws_iam_instance_profile" "ecs_instance_role" {
Expand All @@ -104,7 +76,7 @@ resource "aws_iam_role" "aws_batch_service_role" {
"Action": "sts:AssumeRole",
"Effect": "Allow",
"Principal": {
"Service": "batch.amazonaws.com"
"Service": "batch.${data.aws_partition.current.dns_suffix}"
}
}
]
Expand All @@ -114,7 +86,7 @@ EOF
resource "aws_iam_role_policy_attachment" "aws_batch_service_role" {
role = aws_iam_role.aws_batch_service_role.name
policy_arn = "arn:aws:iam::aws:policy/service-role/AWSBatchServiceRole"
policy_arn = "arn:${data.aws_partition.current.partition}:iam::aws:policy/service-role/AWSBatchServiceRole"
}
resource "aws_security_group" "sample" {
Expand Down
1 change: 1 addition & 0 deletions aws/internal/keyvaluetags/generators/gettag/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const filename = `get_tag_gen.go`

var serviceNames = []string{
"autoscaling",
"batch",
"dynamodb",
"ec2",
"ecs",
Expand Down
1 change: 1 addition & 0 deletions aws/internal/keyvaluetags/generators/listtags/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ var serviceNames = []string{
"athena",
"autoscaling",
"backup",
"batch",
"cloud9",
"cloudfront",
"cloudhsmv2",
Expand Down
1 change: 1 addition & 0 deletions aws/internal/keyvaluetags/generators/updatetags/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ var serviceNames = []string{
"athena",
"autoscaling",
"backup",
"batch",
"cloud9",
"cloudfront",
"cloudhsmv2",
Expand Down
16 changes: 16 additions & 0 deletions aws/internal/keyvaluetags/get_tag_gen.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 18 additions & 0 deletions aws/internal/keyvaluetags/list_tags_gen.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions aws/internal/keyvaluetags/service_generation_customizations.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"github.com/aws/aws-sdk-go/service/athena"
"github.com/aws/aws-sdk-go/service/autoscaling"
"github.com/aws/aws-sdk-go/service/backup"
"github.com/aws/aws-sdk-go/service/batch"
"github.com/aws/aws-sdk-go/service/cloud9"
"github.com/aws/aws-sdk-go/service/cloudfront"
"github.com/aws/aws-sdk-go/service/cloudhsmv2"
Expand Down Expand Up @@ -147,6 +148,8 @@ func ServiceClientType(serviceName string) string {
funcType = reflect.TypeOf(autoscaling.New)
case "backup":
funcType = reflect.TypeOf(backup.New)
case "batch":
funcType = reflect.TypeOf(batch.New)
case "cloud9":
funcType = reflect.TypeOf(cloud9.New)
case "cloudfront":
Expand Down
Loading

0 comments on commit 3249fb4

Please sign in to comment.