diff --git a/.changelog/37871.txt b/.changelog/37871.txt new file mode 100644 index 00000000000..b57f9d058cb --- /dev/null +++ b/.changelog/37871.txt @@ -0,0 +1,3 @@ +`release-note:enhancement +resource/aws_batch_job_definition: Add `ecs_properties` argument +``` diff --git a/internal/service/batch/container_properties.go b/internal/service/batch/container_properties.go index b201a609948..d3f8ca74e65 100644 --- a/internal/service/batch/container_properties.go +++ b/internal/service/batch/container_properties.go @@ -15,6 +15,10 @@ import ( tfslices "github.com/hashicorp/terraform-provider-aws/internal/slices" ) +const ( + fargatePlatformVersionLatest = "LATEST" +) + type containerProperties awstypes.ContainerProperties func (cp *containerProperties) reduce() { @@ -40,7 +44,7 @@ func (cp *containerProperties) reduce() { // Prevent difference of API response that contains the default Fargate platform configuration. if cp.FargatePlatformConfiguration != nil { - if aws.ToString(cp.FargatePlatformConfiguration.PlatformVersion) == "LATEST" { + if aws.ToString(cp.FargatePlatformConfiguration.PlatformVersion) == fargatePlatformVersionLatest { cp.FargatePlatformConfiguration = nil } } diff --git a/internal/service/batch/container_properties_test.go b/internal/service/batch/container_properties_test.go index 4038a34e081..7a68ba0aa73 100644 --- a/internal/service/batch/container_properties_test.go +++ b/internal/service/batch/container_properties_test.go @@ -6,6 +6,8 @@ package batch_test import ( "testing" + "github.com/google/go-cmp/cmp" + "github.com/hashicorp/terraform-provider-aws/internal/acctest/jsoncmp" tfbatch "github.com/hashicorp/terraform-provider-aws/internal/service/batch" ) @@ -13,18 +15,18 @@ func TestEquivalentContainerPropertiesJSON(t *testing.T) { t.Parallel() testCases := map[string]struct { - ApiJson string - ConfigurationJson string - ExpectEquivalent bool - ExpectError bool + apiJSON string + configurationJSON string + wantEquivalent bool + wantErr bool }{ "empty": { - ApiJson: ``, - ConfigurationJson: ``, - ExpectEquivalent: true, + apiJSON: ``, + configurationJSON: ``, + wantEquivalent: true, }, "empty ResourceRequirements": { - ApiJson: ` + apiJSON: ` { "command": ["ls", "-la"], "environment": [ @@ -61,7 +63,7 @@ func TestEquivalentContainerPropertiesJSON(t *testing.T) { ] } `, - ConfigurationJson: ` + configurationJSON: ` { "command": ["ls", "-la"], "environment": [ @@ -97,10 +99,10 @@ func TestEquivalentContainerPropertiesJSON(t *testing.T) { ] } `, - ExpectEquivalent: true, + wantEquivalent: true, }, "reordered Environment": { - ApiJson: ` + apiJSON: ` { "command": ["ls", "-la"], "environment": [ @@ -141,7 +143,7 @@ func TestEquivalentContainerPropertiesJSON(t *testing.T) { ] } `, - ConfigurationJson: ` + configurationJSON: ` { "command": ["ls", "-la"], "environment": [ @@ -182,11 +184,11 @@ func TestEquivalentContainerPropertiesJSON(t *testing.T) { ] } `, - ExpectEquivalent: true, + wantEquivalent: true, }, "empty environment, mountPoints, ulimits, and volumes": { //lintignore:AWSAT005 - ApiJson: ` + apiJSON: ` { "image": "example:image", "vcpus": 8, @@ -201,7 +203,7 @@ func TestEquivalentContainerPropertiesJSON(t *testing.T) { } `, //lintignore:AWSAT005 - ConfigurationJson: ` + configurationJSON: ` { "command": ["start.py", "Ref::S3bucket", "Ref::S3key"], "image": "example:image", @@ -210,11 +212,11 @@ func TestEquivalentContainerPropertiesJSON(t *testing.T) { "jobRoleArn": "arn:aws:iam::123456789012:role/example" } `, - ExpectEquivalent: true, + wantEquivalent: true, }, "empty command, logConfiguration.secretOptions, mountPoints, resourceRequirements, secrets, ulimits, volumes": { //lintignore:AWSAT003,AWSAT005 - ApiJson: ` + apiJSON: ` { "image": "123.dkr.ecr.us-east-1.amazonaws.com/my-app", "vcpus": 1, @@ -234,7 +236,7 @@ func TestEquivalentContainerPropertiesJSON(t *testing.T) { } `, //lintignore:AWSAT003,AWSAT005 - ConfigurationJson: ` + configurationJSON: ` { "image": "123.dkr.ecr.us-east-1.amazonaws.com/my-app", "memory": 4096, @@ -251,11 +253,11 @@ func TestEquivalentContainerPropertiesJSON(t *testing.T) { } } `, - ExpectEquivalent: true, + wantEquivalent: true, }, "no fargatePlatformConfiguration": { //lintignore:AWSAT003,AWSAT005 - ApiJson: ` + apiJSON: ` { "image": "123.dkr.ecr.us-east-1.amazonaws.com/my-app", "resourceRequirements": [ @@ -274,7 +276,7 @@ func TestEquivalentContainerPropertiesJSON(t *testing.T) { } `, //lintignore:AWSAT003,AWSAT005 - ConfigurationJson: ` + configurationJSON: ` { "image": "123.dkr.ecr.us-east-1.amazonaws.com/my-app", "resourceRequirements": [ @@ -289,11 +291,11 @@ func TestEquivalentContainerPropertiesJSON(t *testing.T) { ] } `, - ExpectEquivalent: true, + wantEquivalent: true, }, "empty linuxParameters.devices, linuxParameters.tmpfs, logConfiguration.options": { //lintignore:AWSAT003,AWSAT005 - ApiJson: ` + apiJSON: ` { "image": "123.dkr.ecr.us-east-1.amazonaws.com/my-app", "vcpus": 1, @@ -312,7 +314,7 @@ func TestEquivalentContainerPropertiesJSON(t *testing.T) { } `, //lintignore:AWSAT003,AWSAT005 - ConfigurationJson: ` + configurationJSON: ` { "image": "123.dkr.ecr.us-east-1.amazonaws.com/my-app", "vcpus": 1, @@ -327,11 +329,11 @@ func TestEquivalentContainerPropertiesJSON(t *testing.T) { } } `, - ExpectEquivalent: true, + wantEquivalent: true, }, "empty linuxParameters.devices.permissions, linuxParameters.tmpfs.mountOptions": { //lintignore:AWSAT003,AWSAT005 - ApiJson: ` + apiJSON: ` { "image": "123.dkr.ecr.us-east-1.amazonaws.com/my-app", "vcpus": 1, @@ -354,7 +356,7 @@ func TestEquivalentContainerPropertiesJSON(t *testing.T) { } `, //lintignore:AWSAT003,AWSAT005 - ConfigurationJson: ` + configurationJSON: ` { "image": "123.dkr.ecr.us-east-1.amazonaws.com/my-app", "vcpus": 1, @@ -374,11 +376,11 @@ func TestEquivalentContainerPropertiesJSON(t *testing.T) { } } `, - ExpectEquivalent: true, + wantEquivalent: true, }, "empty environment variables": { //lintignore:AWSAT005 - ApiJson: ` + apiJSON: ` { "image": "example:image", "vcpus": 8, @@ -397,7 +399,7 @@ func TestEquivalentContainerPropertiesJSON(t *testing.T) { "resourceRequirements": [] }`, //lintignore:AWSAT005 - ConfigurationJson: ` + configurationJSON: ` { "command": ["start.py", "Ref::S3bucket", "Ref::S3key"], "image": "example:image", @@ -415,11 +417,11 @@ func TestEquivalentContainerPropertiesJSON(t *testing.T) { ], "jobRoleArn": "arn:aws:iam::123456789012:role/example" }`, - ExpectEquivalent: true, + wantEquivalent: true, }, "empty environment variable": { //lintignore:AWSAT005 - ApiJson: ` + apiJSON: ` { "image": "example:image", "vcpus": 8, @@ -433,7 +435,7 @@ func TestEquivalentContainerPropertiesJSON(t *testing.T) { "resourceRequirements": [] }`, //lintignore:AWSAT005 - ConfigurationJson: ` + configurationJSON: ` { "command": ["start.py", "Ref::S3bucket", "Ref::S3key"], "image": "example:image", @@ -447,7 +449,7 @@ func TestEquivalentContainerPropertiesJSON(t *testing.T) { ], "jobRoleArn": "arn:aws:iam::123456789012:role/example" }`, - ExpectEquivalent: true, + wantEquivalent: true, }, } @@ -456,18 +458,20 @@ func TestEquivalentContainerPropertiesJSON(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - got, err := tfbatch.EquivalentContainerPropertiesJSON(testCase.ConfigurationJson, testCase.ApiJson) - - if err != nil && !testCase.ExpectError { - t.Errorf("got unexpected error: %s", err) - } - - if err == nil && testCase.ExpectError { - t.Errorf("expected error, but received none") + output, err := tfbatch.EquivalentContainerPropertiesJSON(testCase.configurationJSON, testCase.apiJSON) + if got, want := err != nil, testCase.wantErr; !cmp.Equal(got, want) { + t.Errorf("EquivalentContainerPropertiesJSON err %t, want %t", got, want) } - if got != testCase.ExpectEquivalent { - t.Errorf("got %t, expected %t", got, testCase.ExpectEquivalent) + if err == nil { + if got, want := output, testCase.wantEquivalent; !cmp.Equal(got, want) { + t.Errorf("EquivalentContainerPropertiesJSON equivalent %t, want %t", got, want) + if want { + if diff := jsoncmp.Diff(testCase.configurationJSON, testCase.apiJSON); diff != "" { + t.Errorf("unexpected diff (+wanted, -got): %s", diff) + } + } + } } }) } diff --git a/internal/service/batch/ecs_properties.go b/internal/service/batch/ecs_properties.go new file mode 100644 index 00000000000..b6c317c1934 --- /dev/null +++ b/internal/service/batch/ecs_properties.go @@ -0,0 +1,172 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package batch + +import ( + "sort" + _ "unsafe" // Required for go:linkname + + "github.com/aws/aws-sdk-go-v2/aws" + _ "github.com/aws/aws-sdk-go-v2/service/batch" // Required for go:linkname + awstypes "github.com/aws/aws-sdk-go-v2/service/batch/types" + smithyjson "github.com/aws/smithy-go/encoding/json" + tfjson "github.com/hashicorp/terraform-provider-aws/internal/json" + tfslices "github.com/hashicorp/terraform-provider-aws/internal/slices" +) + +type ecsProperties awstypes.EcsProperties + +func (ep *ecsProperties) reduce() { + ep.orderContainers() + ep.orderEnvironmentVariables() + ep.orderSecrets() + + // Set all empty slices to nil. + // Deal with special fields which have defaults. + for i, taskProps := range ep.TaskProperties { + for j, container := range taskProps.Containers { + if container.Essential == nil { + container.Essential = aws.Bool(true) + } + + if len(container.Command) == 0 { + container.Command = nil + } + if len(container.DependsOn) == 0 { + container.DependsOn = nil + } + if len(container.Environment) == 0 { + container.Environment = nil + } + if container.LogConfiguration != nil && len(container.LogConfiguration.SecretOptions) == 0 { + container.LogConfiguration.SecretOptions = nil + } + if len(container.MountPoints) == 0 { + container.MountPoints = nil + } + if len(container.Secrets) == 0 { + container.Secrets = nil + } + if len(container.Ulimits) == 0 { + container.Ulimits = nil + } + + taskProps.Containers[j] = container + } + + if taskProps.PlatformVersion == nil { + taskProps.PlatformVersion = aws.String(fargatePlatformVersionLatest) + } + + if len(taskProps.Volumes) == 0 { + taskProps.Volumes = nil + } + + ep.TaskProperties[i] = taskProps + } +} + +func (ep *ecsProperties) orderContainers() { + for i, taskProps := range ep.TaskProperties { + sort.Slice(taskProps.Containers, func(i, j int) bool { + return aws.ToString(taskProps.Containers[i].Name) < aws.ToString(taskProps.Containers[j].Name) + }) + + ep.TaskProperties[i].Containers = taskProps.Containers + } +} + +func (ep *ecsProperties) orderEnvironmentVariables() { + for i, taskProps := range ep.TaskProperties { + for j, container := range taskProps.Containers { + // Remove environment variables with empty values. + container.Environment = tfslices.Filter(container.Environment, func(kvp awstypes.KeyValuePair) bool { + return aws.ToString(kvp.Value) != "" + }) + + sort.Slice(container.Environment, func(i, j int) bool { + return aws.ToString(container.Environment[i].Name) < aws.ToString(container.Environment[j].Name) + }) + + ep.TaskProperties[i].Containers[j].Environment = container.Environment + } + } +} + +func (ep *ecsProperties) orderSecrets() { + for i, taskProps := range ep.TaskProperties { + for j, container := range taskProps.Containers { + sort.Slice(container.Secrets, func(i, j int) bool { + return aws.ToString(container.Secrets[i].Name) < aws.ToString(container.Secrets[j].Name) + }) + + ep.TaskProperties[i].Containers[j].Secrets = container.Secrets + } + } +} + +func equivalentECSPropertiesJSON(str1, str2 string) (bool, error) { + if str1 == "" { + str1 = "{}" + } + + if str2 == "" { + str2 = "{}" + } + + var ep1 ecsProperties + err := tfjson.DecodeFromString(str1, &ep1) + if err != nil { + return false, err + } + ep1.reduce() + b1, err := tfjson.EncodeToBytes(ep1) + if err != nil { + return false, err + } + + var ep2 ecsProperties + err = tfjson.DecodeFromString(str2, &ep2) + if err != nil { + return false, err + } + ep2.reduce() + b2, err := tfjson.EncodeToBytes(ep2) + if err != nil { + return false, err + } + + return tfjson.EqualBytes(b1, b2), nil +} + +func expandECSProperties(tfString string) (*awstypes.EcsProperties, error) { + apiObject := &awstypes.EcsProperties{} + + if err := tfjson.DecodeFromString(tfString, apiObject); err != nil { + return nil, err + } + + return apiObject, nil +} + +// Dirty hack to avoid any backwards compatibility issues with the AWS SDK for Go v2 migration. +// Reach down into the SDK and use the same serialization function that the SDK uses. +// +//go:linkname serializeECSPProperties github.com/aws/aws-sdk-go-v2/service/batch.awsRestjson1_serializeDocumentEcsProperties +func serializeECSPProperties(v *awstypes.EcsProperties, value smithyjson.Value) error + +func flattenECSProperties(apiObject *awstypes.EcsProperties) (string, error) { + if apiObject == nil { + return "", nil + } + + jsonEncoder := smithyjson.NewEncoder() + err := serializeECSPProperties(apiObject, jsonEncoder.Value) + + if err != nil { + return "", err + } + + return jsonEncoder.String(), nil +} diff --git a/internal/service/batch/ecs_properties_test.go b/internal/service/batch/ecs_properties_test.go new file mode 100644 index 00000000000..9e8387ba9b4 --- /dev/null +++ b/internal/service/batch/ecs_properties_test.go @@ -0,0 +1,318 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package batch_test + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/hashicorp/terraform-provider-aws/internal/acctest/jsoncmp" + tfbatch "github.com/hashicorp/terraform-provider-aws/internal/service/batch" +) + +func TestEquivalentECSPropertiesJSON(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + apiJSON string + configurationJSON string + wantEquivalent bool + wantErr bool + }{ + "empty": { + apiJSON: ``, + configurationJSON: ``, + wantEquivalent: true, + }, + "reordered containers": { + apiJSON: ` +{ + "taskProperties": [ + { + "containers": [ + { + "name": "container1", + "image": "my_ecr_image1" + }, + { + "name": "container2", + "image": "my_ecr_image2" + } + ] + } + ] + } + `, + configurationJSON: ` +{ + "taskProperties": [ + { + "containers": [ + { + "name": "container2", + "image": "my_ecr_image2" + }, + { + "name": "container1", + "image": "my_ecr_image1" + } + ] + } + ] + } + `, + wantEquivalent: true, + }, + "reordered environment": { + apiJSON: ` +{ + "taskProperties": [ + { + "containers": [ + { + "name": "container1", + "image": "my_ecr_image1", + "environment": [ + { + "name": "VARNAME1", + "value": "VARVAL1" + }, + { + "name": "VARNAME2", + "value": "VARVAL2" + } + ] + }, + { + "name": "container2", + "image": "my_ecr_image2", + "environment": [] + } + ] + } + ] +} + `, + configurationJSON: ` +{ + "taskProperties": [ + { + "containers": [ + { + "name": "container1", + "image": "my_ecr_image1", + "environment": [ + { + "name": "VARNAME2", + "value": "VARVAL2" + }, + { + "name": "VARNAME1", + "value": "VARVAL1" + } + ] + }, + { + "name": "container2", + "image": "my_ecr_image2" + } + ] + } + ] +} + `, + wantEquivalent: true, + }, + "full": { + apiJSON: ` +{ + "taskProperties": [ + { + "containers": [ + { + "command": [ + "sleep", + "60" + ], + "dependsOn": [ + { + "condition": "COMPLETE", + "containerName": "container_b" + } + ], + "environment": [ + { + "name": "test", + "value": "Environment Variable" + } + ], + "essential": true, + "image": "public.ecr.aws/amazonlinux/amazonlinux:1", + "logConfiguration": { + "logDriver": "awslogs", + "options": { + "awslogs-stream-prefix": "ecs", + "awslogs-group": "ewbankkit-test-003", + "awslogs-region": "region-2" + }, + "secretOptions": [] + }, + "mountPoints": [], + "name": "container_a", + "privileged": false, + "readonlyRootFilesystem": false, + "resourceRequirements": [ + { + "type": "VCPU", + "value": "1.0" + }, + { + "type": "MEMORY", + "value": "2048" + } + ], + "secrets": [ + { + "name": "TEST", + "valueFrom": "DUMMY" + } + ], + "ulimits": [] + }, + { + "command": [ + "sleep", + "360" + ], + "dependsOn": [], + "environment": [], + "essential": false, + "image": "public.ecr.aws/amazonlinux/amazonlinux:1", + "mountPoints": [], + "name": "container_b", + "resourceRequirements": [ + { + "type": "VCPU", + "value": "1.0" + }, + { + "type": "MEMORY", + "value": "2048" + } + ], + "secrets": [], + "ulimits": [] + } + ], + "executionRoleArn": "role1", + "platformVersion": "LATEST", + "volumes": [] + } + ] +} + `, + configurationJSON: ` +{ + "taskProperties": [ + { + "containers": [ + { + "command": [ + "sleep", + "60" + ], + "dependsOn": [ + { + "condition": "COMPLETE", + "containerName": "container_b" + } + ], + "environment": [ + { + "name": "test", + "value": "Environment Variable" + } + ], + "essential": true, + "image": "public.ecr.aws/amazonlinux/amazonlinux:1", + "logConfiguration": { + "logDriver": "awslogs", + "options": { + "awslogs-group": "ewbankkit-test-003", + "awslogs-region": "region-2", + "awslogs-stream-prefix": "ecs" + } + }, + "name": "container_a", + "privileged": false, + "readonlyRootFilesystem": false, + "resourceRequirements": [ + { + "type": "VCPU", + "value": "1.0" + }, + { + "type": "MEMORY", + "value": "2048" + } + ], + "secrets": [ + { + "name": "TEST", + "valueFrom": "DUMMY" + } + ] + }, + { + "command": [ + "sleep", + "360" + ], + "essential": false, + "image": "public.ecr.aws/amazonlinux/amazonlinux:1", + "name": "container_b", + "resourceRequirements": [ + { + "type": "VCPU", + "value": "1.0" + }, + { + "type": "MEMORY", + "value": "2048" + } + ] + } + ], + "executionRoleArn": "role1" + } + ] +} + `, + wantEquivalent: true, + }, + } + + for name, testCase := range testCases { + testCase := testCase + t.Run(name, func(t *testing.T) { + t.Parallel() + + output, err := tfbatch.EquivalentECSPropertiesJSON(testCase.configurationJSON, testCase.apiJSON) + if got, want := err != nil, testCase.wantErr; !cmp.Equal(got, want) { + t.Errorf("EquivalentECSPropertiesJSON err %t, want %t", got, want) + } + + if err == nil { + if got, want := output, testCase.wantEquivalent; !cmp.Equal(got, want) { + t.Errorf("EquivalentECSPropertiesJSON equivalent %t, want %t", got, want) + if want { + if diff := jsoncmp.Diff(testCase.configurationJSON, testCase.apiJSON); diff != "" { + t.Errorf("unexpected diff (+wanted, -got): %s", diff) + } + } + } + } + }) + } +} diff --git a/internal/service/batch/exports_test.go b/internal/service/batch/exports_test.go index a2fc1c4654f..38f831debeb 100644 --- a/internal/service/batch/exports_test.go +++ b/internal/service/batch/exports_test.go @@ -11,6 +11,7 @@ var ( ResourceSchedulingPolicy = resourceSchedulingPolicy EquivalentContainerPropertiesJSON = equivalentContainerPropertiesJSON + EquivalentECSPropertiesJSON = equivalentECSPropertiesJSON EquivalentNodePropertiesJSON = equivalentNodePropertiesJSON ExpandEC2ConfigurationsUpdate = expandEC2ConfigurationsUpdate ExpandLaunchTemplateSpecificationUpdate = expandLaunchTemplateSpecificationUpdate diff --git a/internal/service/batch/job_definition.go b/internal/service/batch/job_definition.go index fb4f8d14fcf..a9671f441ad 100644 --- a/internal/service/batch/job_definition.go +++ b/internal/service/batch/job_definition.go @@ -59,7 +59,7 @@ func resourceJobDefinition() *schema.Resource { "container_properties": { Type: schema.TypeString, Optional: true, - ConflictsWith: []string{"eks_properties", "node_properties"}, + ConflictsWith: []string{"ecs_properties", "eks_properties", "node_properties"}, StateFunc: func(v interface{}) string { json, _ := structure.NormalizeJsonString(v) return json @@ -76,11 +76,26 @@ func resourceJobDefinition() *schema.Resource { Default: true, Optional: true, }, + "ecs_properties": { + Type: schema.TypeString, + Optional: true, + ConflictsWith: []string{"container_properties", "eks_properties", "node_properties"}, + StateFunc: func(v interface{}) string { + json, _ := structure.NormalizeJsonString(v) + return json + }, + DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { + equal, _ := equivalentECSPropertiesJSON(old, new) + return equal + }, + DiffSuppressOnRefresh: true, + ValidateFunc: validJobECSProperties, + }, "eks_properties": { Type: schema.TypeList, MaxItems: 1, Optional: true, - ConflictsWith: []string{"container_properties", "node_properties"}, + ConflictsWith: []string{"ecs_properties", "container_properties", "node_properties"}, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "pod_properties": { @@ -322,7 +337,7 @@ func resourceJobDefinition() *schema.Resource { "node_properties": { Type: schema.TypeString, Optional: true, - ConflictsWith: []string{"container_properties", "eks_properties"}, + ConflictsWith: []string{"container_properties", "ecs_properties", "eks_properties"}, StateFunc: func(v interface{}) string { json, _ := structure.NormalizeJsonString(v) return json @@ -431,7 +446,6 @@ func resourceJobDefinition() *schema.Resource { }, }, }, - names.AttrType: { Type: schema.TypeString, Required: true, @@ -478,6 +492,19 @@ func needsJobDefUpdate(d *schema.ResourceDiff) bool { } } + if d.HasChange("ecs_properties") { + o, n := d.GetChange("ecs_properties") + + equivalent, err := equivalentECSPropertiesJSON(o.(string), n.(string)) + if err != nil { + return false + } + + if !equivalent { + return true + } + } + if d.HasChange("node_properties") { o, n := d.GetChange("node_properties") @@ -585,7 +612,8 @@ func resourceJobDefinitionCreate(ctx context.Context, d *schema.ResourceData, me Type: jobDefinitionType, } - if jobDefinitionType == awstypes.JobDefinitionTypeContainer { + switch jobDefinitionType { + case awstypes.JobDefinitionTypeContainer: if v, ok := d.GetOk("node_properties"); ok && v != nil { return sdkdiag.AppendErrorf(diags, "No `node_properties` can be specified when `type` is %q", jobDefinitionType) } @@ -596,10 +624,24 @@ func resourceJobDefinitionCreate(ctx context.Context, d *schema.ResourceData, me return sdkdiag.AppendFromErr(diags, err) } - removeEmptyEnvironmentVariables(&diags, props.Environment, cty.GetAttrPath("container_properties")) + diags = append(diags, removeEmptyEnvironmentVariables(props.Environment, cty.GetAttrPath("container_properties"))...) input.ContainerProperties = props } + if v, ok := d.GetOk("ecs_properties"); ok { + props, err := expandECSProperties(v.(string)) + if err != nil { + return sdkdiag.AppendFromErr(diags, err) + } + + for _, taskProps := range props.TaskProperties { + for _, container := range taskProps.Containers { + diags = append(diags, removeEmptyEnvironmentVariables(container.Environment, cty.GetAttrPath("ecs_properties"))...) + } + } + input.EcsProperties = props + } + if v, ok := d.GetOk("eks_properties"); ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil { eksProps := v.([]interface{})[0].(map[string]interface{}) if podProps, ok := eksProps["pod_properties"].([]interface{}); ok && len(podProps) > 0 { @@ -609,12 +651,14 @@ func resourceJobDefinitionCreate(ctx context.Context, d *schema.ResourceData, me } } } - } - if jobDefinitionType == awstypes.JobDefinitionTypeMultinode { + case awstypes.JobDefinitionTypeMultinode: if v, ok := d.GetOk("container_properties"); ok && v != nil { return sdkdiag.AppendErrorf(diags, "No `container_properties` can be specified when `type` is %q", jobDefinitionType) } + if v, ok := d.GetOk("ecs_properties"); ok && v != nil { + return sdkdiag.AppendErrorf(diags, "No `ecs_properties` can be specified when `type` is %q", jobDefinitionType) + } if v, ok := d.GetOk("eks_properties"); ok && v != nil { return sdkdiag.AppendErrorf(diags, "No `eks_properties` can be specified when `type` is %q", jobDefinitionType) } @@ -626,7 +670,7 @@ func resourceJobDefinitionCreate(ctx context.Context, d *schema.ResourceData, me } for _, node := range props.NodeRangeProperties { - removeEmptyEnvironmentVariables(&diags, node.Container.Environment, cty.GetAttrPath("node_properties")) + diags = append(diags, removeEmptyEnvironmentVariables(node.Container.Environment, cty.GetAttrPath("node_properties"))...) } input.NodeProperties = props } @@ -682,14 +726,16 @@ func resourceJobDefinitionRead(ctx context.Context, d *schema.ResourceData, meta arn, revision := aws.ToString(jobDefinition.JobDefinitionArn), aws.ToInt32(jobDefinition.Revision) d.Set(names.AttrARN, arn) d.Set("arn_prefix", strings.TrimSuffix(arn, fmt.Sprintf(":%d", revision))) - containerProperties, err := flattenContainerProperties(jobDefinition.ContainerProperties) if err != nil { return sdkdiag.AppendFromErr(diags, err) } - if err := d.Set("container_properties", containerProperties); err != nil { - return sdkdiag.AppendErrorf(diags, "setting container_properties: %s", err) + d.Set("container_properties", containerProperties) + ecsProperties, err := flattenECSProperties(jobDefinition.EcsProperties) + if err != nil { + return sdkdiag.AppendFromErr(diags, err) } + d.Set("ecs_properties", ecsProperties) if err := d.Set("eks_properties", flattenEKSProperties(jobDefinition.EksProperties)); err != nil { return sdkdiag.AppendErrorf(diags, "setting eks_properties: %s", err) } @@ -739,42 +785,54 @@ func resourceJobDefinitionUpdate(ctx context.Context, d *schema.ResourceData, me Type: jobDefinitionType, } - if v, ok := d.GetOk("container_properties"); ok { - props, err := expandContainerProperties(v.(string)) - if err != nil { - return sdkdiag.AppendFromErr(diags, err) - } + switch jobDefinitionType { + case awstypes.JobDefinitionTypeContainer: + if v, ok := d.GetOk("container_properties"); ok { + props, err := expandContainerProperties(v.(string)) + if err != nil { + return sdkdiag.AppendFromErr(diags, err) + } - if jobDefinitionType == awstypes.JobDefinitionTypeContainer { - removeEmptyEnvironmentVariables(&diags, props.Environment, cty.GetAttrPath("container_properties")) + diags = append(diags, removeEmptyEnvironmentVariables(props.Environment, cty.GetAttrPath("container_properties"))...) input.ContainerProperties = props } - } - if v, ok := d.GetOk("eks_properties"); ok { - eksProps := v.([]interface{})[0].(map[string]interface{}) - if podProps, ok := eksProps["pod_properties"].([]interface{}); ok && len(podProps) > 0 { - props := expandEKSPodProperties(podProps[0].(map[string]interface{})) - input.EksProperties = &awstypes.EksProperties{ - PodProperties: props, + if v, ok := d.GetOk("ecs_properties"); ok { + props, err := expandECSProperties(v.(string)) + if err != nil { + return sdkdiag.AppendFromErr(diags, err) } - } - } - if v, ok := d.GetOk("node_properties"); ok { - props, err := expandJobNodeProperties(v.(string)) - if err != nil { - return sdkdiag.AppendFromErr(diags, err) + for _, taskProps := range props.TaskProperties { + for _, container := range taskProps.Containers { + diags = append(diags, removeEmptyEnvironmentVariables(container.Environment, cty.GetAttrPath("ecs_properties"))...) + } + } + input.EcsProperties = props } - for _, node := range props.NodeRangeProperties { - removeEmptyEnvironmentVariables(&diags, node.Container.Environment, cty.GetAttrPath("node_properties")) + if v, ok := d.GetOk("eks_properties"); ok { + eksProps := v.([]interface{})[0].(map[string]interface{}) + if podProps, ok := eksProps["pod_properties"].([]interface{}); ok && len(podProps) > 0 { + props := expandEKSPodProperties(podProps[0].(map[string]interface{})) + input.EksProperties = &awstypes.EksProperties{ + PodProperties: props, + } + } } - input.NodeProperties = props - } - if v, ok := d.GetOk(names.AttrPropagateTags); ok { - input.PropagateTags = aws.Bool(v.(bool)) + case awstypes.JobDefinitionTypeMultinode: + if v, ok := d.GetOk("node_properties"); ok { + props, err := expandJobNodeProperties(v.(string)) + if err != nil { + return sdkdiag.AppendFromErr(diags, err) + } + + for _, node := range props.NodeRangeProperties { + diags = append(diags, removeEmptyEnvironmentVariables(node.Container.Environment, cty.GetAttrPath("node_properties"))...) + } + input.NodeProperties = props + } } if v, ok := d.GetOk(names.AttrParameters); ok { @@ -785,14 +843,18 @@ func resourceJobDefinitionUpdate(ctx context.Context, d *schema.ResourceData, me input.PlatformCapabilities = flex.ExpandStringyValueSet[awstypes.PlatformCapability](v.(*schema.Set)) } - if v, ok := d.GetOk("scheduling_priority"); ok { - input.SchedulingPriority = aws.Int32(int32(v.(int))) + if v, ok := d.GetOk(names.AttrPropagateTags); ok { + input.PropagateTags = aws.Bool(v.(bool)) } if v, ok := d.GetOk("retry_strategy"); ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil { input.RetryStrategy = expandRetryStrategy(v.([]interface{})[0].(map[string]interface{})) } + if v, ok := d.GetOk("scheduling_priority"); ok { + input.SchedulingPriority = aws.Int32(int32(v.(int))) + } + if v, ok := d.GetOk(names.AttrTimeout); ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil { input.Timeout = expandJobTimeout(v.([]interface{})[0].(map[string]interface{})) } @@ -917,6 +979,15 @@ func validJobContainerProperties(v interface{}, k string) (ws []string, errors [ return } +func validJobECSProperties(v interface{}, k string) (ws []string, errors []error) { + value := v.(string) + _, err := expandECSProperties(value) + if err != nil { + errors = append(errors, fmt.Errorf("AWS Batch Job ecs_properties is invalid: %s", err)) + } + return +} + func validJobNodeProperties(v interface{}, k string) (ws []string, errors []error) { value := v.(string) _, err := expandJobNodeProperties(value) @@ -1079,16 +1150,20 @@ func flattenJobTimeout(apiObject *awstypes.JobTimeout) map[string]interface{} { return tfMap } -func removeEmptyEnvironmentVariables(diags *diag.Diagnostics, environment []awstypes.KeyValuePair, attributePath cty.Path) { +func removeEmptyEnvironmentVariables(environment []awstypes.KeyValuePair, attributePath cty.Path) diag.Diagnostics { + var diags diag.Diagnostics + for _, env := range environment { if aws.ToString(env.Value) == "" { - *diags = append(*diags, errs.NewAttributeWarningDiagnostic( + diags = append(diags, errs.NewAttributeWarningDiagnostic( attributePath, "Ignoring environment variable", fmt.Sprintf("The environment variable %q has an empty value, which is ignored by the Batch service", aws.ToString(env.Name))), ) } } + + return diags } func expandEKSPodProperties(tfMap map[string]interface{}) *awstypes.EksPodProperties { diff --git a/internal/service/batch/job_definition_test.go b/internal/service/batch/job_definition_test.go index c201b6485cb..0f217b2267a 100644 --- a/internal/service/batch/job_definition_test.go +++ b/internal/service/batch/job_definition_test.go @@ -54,6 +54,8 @@ func TestAccBatchJobDefinition_basic(t *testing.T) { "ulimits": [], "volumes": [] }`), + resource.TestCheckResourceAttr(resourceName, "ecs_properties", ""), + resource.TestCheckResourceAttr(resourceName, "eks_properties.#", acctest.Ct0), resource.TestCheckResourceAttr(resourceName, names.AttrName, rName), resource.TestCheckResourceAttr(resourceName, "parameters.%", acctest.Ct0), resource.TestCheckResourceAttr(resourceName, "platform_capabilities.#", acctest.Ct0), @@ -976,6 +978,42 @@ func TestAccBatchJobDefinition_emptyRetryStrategy(t *testing.T) { }) } +func TestAccBatchJobDefinition_ECSProperties_update(t *testing.T) { + ctx := acctest.Context(t) + var jd awstypes.JobDefinition + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_batch_job_definition.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t); testAccPreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.BatchServiceID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckJobDefinitionDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccJobDefinitionConfig_ECSProperties_basic(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckJobDefinitionExists(ctx, resourceName, &jd), + resource.TestCheckResourceAttrSet(resourceName, "ecs_properties"), + acctest.CheckResourceAttrJMES(resourceName, "ecs_properties", "length(taskProperties)", acctest.Ct1), + acctest.CheckResourceAttrJMES(resourceName, "ecs_properties", "length(taskProperties[0].containers)", acctest.Ct2), + acctest.CheckResourceAttrJMES(resourceName, "ecs_properties", "length(taskProperties[0].containers[0].environment)", acctest.Ct1), + ), + }, + { + Config: testAccJobDefinitionConfig_ECSProperties_updated(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckJobDefinitionExists(ctx, resourceName, &jd), + resource.TestCheckResourceAttrSet(resourceName, "ecs_properties"), + acctest.CheckResourceAttrJMES(resourceName, "ecs_properties", "length(taskProperties)", acctest.Ct1), + acctest.CheckResourceAttrJMES(resourceName, "ecs_properties", "length(taskProperties[0].containers)", acctest.Ct2), + acctest.CheckResourceAttrJMES(resourceName, "ecs_properties", "length(taskProperties[0].containers[0].environment)", acctest.Ct2), + ), + }, + }, + }) +} + func testAccCheckJobDefinitionExists(ctx context.Context, n string, v *awstypes.JobDefinition) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] @@ -1894,3 +1932,215 @@ resource "aws_batch_job_definition" "test" { } `, rName) } + +func testAccJobDefinitionConfig_ECSProperties_basic(rName string) string { + return fmt.Sprintf(` +data "aws_partition" "current" {} + +resource "aws_iam_role" "ecs_task_execution_role" { + name = %[1]q + assume_role_policy = data.aws_iam_policy_document.assume_role_policy.json +} + +data "aws_iam_policy_document" "assume_role_policy" { + statement { + actions = ["sts:AssumeRole"] + + principals { + type = "Service" + identifiers = ["ecs-tasks.amazonaws.com"] + } + } +} + +resource "aws_iam_role_policy_attachment" "ecs_task_execution_role_policy" { + role = aws_iam_role.ecs_task_execution_role.name + policy_arn = "arn:${data.aws_partition.current.partition}:iam::aws:policy/service-role/AmazonECSTaskExecutionRolePolicy" +} + +resource "aws_batch_job_definition" "test" { + name = %[1]q + type = "container" + + platform_capabilities = ["FARGATE"] + + ecs_properties = jsonencode({ + taskProperties = [ + { + executionRoleArn = aws_iam_role.ecs_task_execution_role.arn + containers = [ + { + image = "public.ecr.aws/amazonlinux/amazonlinux:1" + command = ["sleep", "60"] + dependsOn = [ + { + containerName = "container_b" + condition = "COMPLETE" + } + ] + secrets = [ + { + name = "TEST" + valueFrom = "DUMMY" + } + ] + environment = [ + { + name = "test" + value = "Environment Variable" + } + ] + essential = true + logConfiguration = { + logDriver = "awslogs" + options = { + "awslogs-group" = %[1]q + "awslogs-region" = %[2]q + "awslogs-stream-prefix" = "ecs" + } + } + name = "container_a" + privileged = false + readonlyRootFilesystem = false + resourceRequirements = [ + { + value = "1.0" + type = "VCPU" + }, + { + value = "2048" + type = "MEMORY" + } + ] + }, + { + image = "public.ecr.aws/amazonlinux/amazonlinux:1" + command = ["sleep", "360"] + name = "container_b" + essential = false + resourceRequirements = [ + { + value = "1.0" + type = "VCPU" + }, + { + value = "2048" + type = "MEMORY" + } + ] + } + ] + } + ] + }) +} +`, rName, acctest.Region()) +} + +func testAccJobDefinitionConfig_ECSProperties_updated(rName string) string { + return fmt.Sprintf(` +data "aws_partition" "current" {} + +resource "aws_iam_role" "ecs_task_execution_role" { + name = %[1]q + assume_role_policy = data.aws_iam_policy_document.assume_role_policy.json +} + +data "aws_iam_policy_document" "assume_role_policy" { + statement { + actions = ["sts:AssumeRole"] + + principals { + type = "Service" + identifiers = ["ecs-tasks.amazonaws.com"] + } + } +} + +resource "aws_iam_role_policy_attachment" "ecs_task_execution_role_policy" { + role = aws_iam_role.ecs_task_execution_role.name + policy_arn = "arn:${data.aws_partition.current.partition}:iam::aws:policy/service-role/AmazonECSTaskExecutionRolePolicy" +} + +resource "aws_batch_job_definition" "test" { + name = %[1]q + type = "container" + + platform_capabilities = ["FARGATE"] + + ecs_properties = jsonencode({ + taskProperties = [ + { + executionRoleArn = aws_iam_role.ecs_task_execution_role.arn + containers = [ + { + image = "public.ecr.aws/amazonlinux/amazonlinux:1" + command = ["sleep", "60"] + dependsOn = [ + { + containerName = "container_b" + condition = "COMPLETE" + } + ] + secrets = [ + { + name = "TEST" + valueFrom = "DUMMY" + } + ] + environment = [ + { + name = "test 1" + value = "Environment Variable 1" + }, + { + name = "test 2" + value = "Environment Variable 2" + } + ] + essential = true + logConfiguration = { + logDriver = "awslogs" + options = { + "awslogs-group" = %[1]q + "awslogs-region" = %[2]q + "awslogs-stream-prefix" = "ecs" + } + } + name = "container_a" + privileged = false + readonlyRootFilesystem = false + resourceRequirements = [ + { + value = "1.0" + type = "VCPU" + }, + { + value = "2048" + type = "MEMORY" + } + ] + }, + { + image = "public.ecr.aws/amazonlinux/amazonlinux:1" + command = ["sleep", "360"] + name = "container_b" + essential = false + resourceRequirements = [ + { + value = "1.0" + type = "VCPU" + }, + { + value = "2048" + type = "MEMORY" + } + ] + } + ] + } + ] + }) +} +`, rName, acctest.Region()) +} diff --git a/internal/service/batch/node_properties_test.go b/internal/service/batch/node_properties_test.go index e47bfd53fef..7e09f08080b 100644 --- a/internal/service/batch/node_properties_test.go +++ b/internal/service/batch/node_properties_test.go @@ -6,6 +6,8 @@ package batch_test import ( "testing" + "github.com/google/go-cmp/cmp" + "github.com/hashicorp/terraform-provider-aws/internal/acctest/jsoncmp" tfbatch "github.com/hashicorp/terraform-provider-aws/internal/service/batch" ) @@ -13,18 +15,18 @@ func TestEquivalentNodePropertiesJSON(t *testing.T) { t.Parallel() testCases := map[string]struct { - ApiJson string - ConfigurationJson string - ExpectEquivalent bool - ExpectError bool + apiJSON string + configurationJSON string + wantEquivalent bool + wantErr bool }{ "empty": { - ApiJson: ``, - ConfigurationJson: ``, - ExpectEquivalent: true, + apiJSON: ``, + configurationJSON: ``, + wantEquivalent: true, }, "Single Node with empty environment variable": { - ApiJson: ` + apiJSON: ` { "mainNode": 1, "nodeRangeProperties": [ @@ -42,7 +44,7 @@ func TestEquivalentNodePropertiesJSON(t *testing.T) { "numNodes": 2 } `, - ConfigurationJson: ` + configurationJSON: ` { "mainNode": 1, "nodeRangeProperties": [ @@ -65,10 +67,10 @@ func TestEquivalentNodePropertiesJSON(t *testing.T) { "numNodes": 2 } `, - ExpectEquivalent: true, + wantEquivalent: true, }, "Two Nodes with empty command and mountPoints": { - ApiJson: ` + apiJSON: ` { "mainNode": 1, "nodeRangeProperties": [ @@ -99,7 +101,7 @@ func TestEquivalentNodePropertiesJSON(t *testing.T) { "numNodes": 2 } `, - ConfigurationJson: ` + configurationJSON: ` { "mainNode": 1, "nodeRangeProperties": [ @@ -130,7 +132,7 @@ func TestEquivalentNodePropertiesJSON(t *testing.T) { "numNodes": 2 } `, - ExpectEquivalent: true, + wantEquivalent: true, }, } @@ -139,18 +141,20 @@ func TestEquivalentNodePropertiesJSON(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - got, err := tfbatch.EquivalentNodePropertiesJSON(testCase.ConfigurationJson, testCase.ApiJson) - - if err != nil && !testCase.ExpectError { - t.Errorf("got unexpected error: %s", err) - } - - if err == nil && testCase.ExpectError { - t.Errorf("expected error, but received none") + output, err := tfbatch.EquivalentNodePropertiesJSON(testCase.configurationJSON, testCase.apiJSON) + if got, want := err != nil, testCase.wantErr; !cmp.Equal(got, want) { + t.Errorf("EquivalentNodePropertiesJSON err %t, want %t", got, want) } - if got != testCase.ExpectEquivalent { - t.Errorf("got %t, expected %t", got, testCase.ExpectEquivalent) + if err == nil { + if got, want := output, testCase.wantEquivalent; !cmp.Equal(got, want) { + t.Errorf("EquivalentNodePropertiesJSON equivalent %t, want %t", got, want) + if want { + if diff := jsoncmp.Diff(testCase.configurationJSON, testCase.apiJSON); diff != "" { + t.Errorf("unexpected diff (+wanted, -got): %s", diff) + } + } + } } }) } diff --git a/website/docs/r/batch_job_definition.html.markdown b/website/docs/r/batch_job_definition.html.markdown index e32e3def7d3..09936a3bcbd 100644 --- a/website/docs/r/batch_job_definition.html.markdown +++ b/website/docs/r/batch_job_definition.html.markdown @@ -102,7 +102,7 @@ resource "aws_batch_job_definition" "test" { } ``` -### Job Definitionn of type EKS +### Job Definition of type EKS ```terraform resource "aws_batch_job_definition" "test" { @@ -191,6 +191,87 @@ resource "aws_batch_job_definition" "test" { } ``` +### Job definition of type container using `ecs_properties` + +```terraform +resource "aws_batch_job_definition" "test" { + name = "tf_test_batch_job_definition" + type = "container" + + platform_capabilities = ["FARGATE"] + + ecs_properties = jsonencode({ + taskProperties = [ + { + executionRoleArn = aws_iam_role.ecs_task_execution_role.arn + containers = [ + { + image = "public.ecr.aws/amazonlinux/amazonlinux:1" + command = ["sleep", "60"] + dependsOn = [ + { + containerName = "container_b" + condition = "COMPLETE" + } + ] + secrets = [ + { + name = "TEST" + valueFrom = "DUMMY" + } + ] + environment = [ + { + name = "test" + value = "Environment Variable" + } + ] + essential = true + logConfiguration = { + logDriver = "awslogs" + options = { + "awslogs-group" = "tf_test_batch_job" + "awslogs-region" = "us-west-2" + "awslogs-stream-prefix" = "ecs" + } + } + name = "container_a" + privileged = false + readonlyRootFilesystem = false + resourceRequirements = [ + { + value = "1.0" + type = "VCPU" + }, + { + value = "2048" + type = "MEMORY" + } + ] + }, + { + image = "public.ecr.aws/amazonlinux/amazonlinux:1" + command = ["sleep", "360"] + name = "container_b" + essential = false + resourceRequirements = [ + { + value = "1.0" + type = "VCPU" + }, + { + value = "2048" + type = "MEMORY" + } + ] + } + ] + } + ] + }) +} +``` + ## Argument Reference The following arguments are required: @@ -202,6 +283,7 @@ The following arguments are optional: * `container_properties` - (Optional) Valid [container properties](http://docs.aws.amazon.com/batch/latest/APIReference/API_RegisterJobDefinition.html) provided as a single valid JSON document. This parameter is only valid if the `type` parameter is `container`. * `deregister_on_new_revision` - (Optional) When updating a job definition a new revision is created. This parameter determines if the previous version is `deregistered` (`INACTIVE`) or left `ACTIVE`. Defaults to `true`. +* `ecs_properties` - (Optional) Valid [ECS properties](http://docs.aws.amazon.com/batch/latest/APIReference/API_RegisterJobDefinition.html) provided as a single valid JSON document. This parameter is only valid if the `type` parameter is `container`. * `eks_properties` - (Optional) Valid [eks properties](#eks_properties). This parameter is only valid if the `type` parameter is `container`. * `node_properties` - (Optional) Valid [node properties](http://docs.aws.amazon.com/batch/latest/APIReference/API_RegisterJobDefinition.html) provided as a single valid JSON document. This parameter is required if the `type` parameter is `multinode`. * `parameters` - (Optional) Parameter substitution placeholders to set in the job definition.