resource/aws_batch_job_definition: Properly set container_properties and name into Terraform state and fix basic test

aws/internal/service/batch/equivalency/container_properties.go

@@ -0,0 +1,76 @@
+package equivalency
+
+import (
+	"bytes"
+	"encoding/json"
+	"log"
+	"sort"
+
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/private/protocol/json/jsonutil"
+	"github.com/aws/aws-sdk-go/service/batch"
+)
+
+type containerProperties batch.ContainerProperties
+
+func (cp *containerProperties) Reduce() error {
+	// Deal with Environment objects which may be re-ordered in the API
+	sort.Slice(cp.Environment, func(i, j int) bool {
+		return aws.StringValue(cp.Environment[i].Name) < aws.StringValue(cp.Environment[j].Name)
+	})
+
+	if len(cp.ResourceRequirements) == 0 {
+		cp.ResourceRequirements = nil
+	}
+
+	return nil
+}
+
+// EquivalentBatchContainerPropertiesJSON determines equality between two Batch ContainerProperties JSON strings
+func EquivalentBatchContainerPropertiesJSON(str1, str2 string) (bool, error) {
+	if str1 == "" {
+		str1 = "{}"
+	}
+
+	if str2 == "" {
+		str2 = "{}"
+	}
+
+	var cp1, cp2 containerProperties
+
+	if err := json.Unmarshal([]byte(str1), &cp1); err != nil {
+		return false, err
+	}
+
+	if err := cp1.Reduce(); err != nil {
+		return false, err
+	}
+
+	canonicalJson1, err := jsonutil.BuildJSON(cp1)
+
+	if err != nil {
+		return false, err
+	}
+
+	if err := json.Unmarshal([]byte(str2), &cp2); err != nil {
+		return false, err
+	}
+
+	if err := cp2.Reduce(); err != nil {
+		return false, err
+	}
+
+	canonicalJson2, err := jsonutil.BuildJSON(cp2)
+
+	if err != nil {
+		return false, err
+	}
+
+	equal := bytes.Equal(canonicalJson1, canonicalJson2)
+
+	if !equal {
+		log.Printf("[DEBUG] Canonical Batch Container Properties JSON are not equal.\nFirst: %s\nSecond: %s\n", canonicalJson1, canonicalJson2)
+	}
+
+	return equal, nil
+}

aws/internal/service/batch/equivalency/container_properties_test.go

@@ -0,0 +1,203 @@
+package equivalency
+
+import (
+	"testing"
+)
+
+func TestEquivalentBatchContainerPropertiesJSON(t *testing.T) {
+	testCases := []struct {
+		Name              string
+		ApiJson           string
+		ConfigurationJson string
+		ExpectEquivalent  bool
+		ExpectError       bool
+	}{
+		{
+			Name:              "empty",
+			ApiJson:           ``,
+			ConfigurationJson: ``,
+			ExpectEquivalent:  true,
+		},
+		{
+			Name: "empty ResourceRequirements",
+			ApiJson: `
+{
+	"command": ["ls", "-la"],
+	"environment": [
+		{
+			"name": "VARNAME",
+			"value": "VARVAL"
+		}
+	],
+	"image": "busybox",
+	"memory":512,
+	"mountPoints": [
+		{
+			"containerPath": "/tmp",
+			"readOnly": false,
+			"sourceVolume": "tmp"
+		}
+	],
+	"resourceRequirements": [],
+	"ulimits": [
+		{
+			"hardLimit": 1024,
+			"name": "nofile",
+			"softLimit": 1024
+		}
+	],
+	"vcpus": 1,
+	"volumes": [
+		{
+			"host": {
+				"sourcePath": "/tmp"
+			},
+			"name": "tmp"
+		}
+	]
+}
+`,
+			ConfigurationJson: `
+{
+	"command": ["ls", "-la"],
+	"environment": [
+		{
+			"name": "VARNAME",
+			"value": "VARVAL"
+		}
+	],
+	"image": "busybox",
+	"memory":512,
+	"mountPoints": [
+		{
+			"containerPath": "/tmp",
+			"readOnly": false,
+			"sourceVolume": "tmp"
+		}
+	],
+	"ulimits": [
+		{
+			"hardLimit": 1024,
+			"name": "nofile",
+			"softLimit": 1024
+		}
+	],
+	"vcpus": 1,
+	"volumes": [
+		{
+			"host": {
+				"sourcePath": "/tmp"
+			},
+			"name": "tmp"
+		}
+	]
+}
+`,
+			ExpectEquivalent: true,
+		},
+		{
+			Name: "reordered Environment",
+			ApiJson: `
+{
+	"command": ["ls", "-la"],
+	"environment": [
+		{
+			"name": "VARNAME1",
+			"value": "VARVAL1"
+		},
+		{
+			"name": "VARNAME2",
+			"value": "VARVAL2"
+		}
+	],
+	"image": "busybox",
+	"memory":512,
+	"mountPoints": [
+		{
+			"containerPath": "/tmp",
+			"readOnly": false,
+			"sourceVolume": "tmp"
+		}
+	],
+	"resourceRequirements": [],
+	"ulimits": [
+		{
+			"hardLimit": 1024,
+			"name": "nofile",
+			"softLimit": 1024
+		}
+	],
+	"vcpus": 1,
+	"volumes": [
+		{
+			"host": {
+				"sourcePath": "/tmp"
+			},
+			"name": "tmp"
+		}
+	]
+}
+`,
+			ConfigurationJson: `
+{
+	"command": ["ls", "-la"],
+	"environment": [
+		{
+			"name": "VARNAME2",
+			"value": "VARVAL2"
+		},
+		{
+			"name": "VARNAME1",
+			"value": "VARVAL1"
+		}
+	],
+	"image": "busybox",
+	"memory":512,
+	"mountPoints": [
+		{
+			"containerPath": "/tmp",
+			"readOnly": false,
+			"sourceVolume": "tmp"
+		}
+	],
+	"resourceRequirements": [],
+	"ulimits": [
+		{
+			"hardLimit": 1024,
+			"name": "nofile",
+			"softLimit": 1024
+		}
+	],
+	"vcpus": 1,
+	"volumes": [
+		{
+			"host": {
+				"sourcePath": "/tmp"
+			},
+			"name": "tmp"
+		}
+	]
+}
+`,
+			ExpectEquivalent: true,
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.Name, func(t *testing.T) {
+			got, err := EquivalentBatchContainerPropertiesJSON(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")
+			}
+
+			if got != testCase.ExpectEquivalent {
+				t.Errorf("got %t, expected %t", got, testCase.ExpectEquivalent)
+			}
+		})
+	}
+}

aws/resource_aws_batch_job_definition.go

@@ -6,10 +6,12 @@ import (
 	"encoding/json"
 
 	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/private/protocol/json/jsonutil"
 	"github.com/aws/aws-sdk-go/service/batch"
 	"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
 	"github.com/hashicorp/terraform-plugin-sdk/helper/structure"
 	"github.com/hashicorp/terraform-plugin-sdk/helper/validation"
+	"github.com/terraform-providers/terraform-provider-aws/aws/internal/service/batch/equivalency"
 )
 
 func resourceAwsBatchJobDefinition() *schema.Resource {
@@ -33,8 +35,12 @@ func resourceAwsBatchJobDefinition() *schema.Resource {
 					json, _ := structure.NormalizeJsonString(v)
 					return json
 				},
-				DiffSuppressFunc: suppressEquivalentJsonDiffs,
-				ValidateFunc:     validateAwsBatchJobContainerProperties,
+				DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool {
+					equal, _ := equivalency.EquivalentBatchContainerPropertiesJSON(old, new)
+
+					return equal
+				},
+				ValidateFunc: validateAwsBatchJobContainerProperties,
 			},
 			"parameters": {
 				Type:     schema.TypeMap,
@@ -142,7 +148,19 @@ func resourceAwsBatchJobDefinitionRead(d *schema.ResourceData, meta interface{})
 		return nil
 	}
 	d.Set("arn", job.JobDefinitionArn)
-	d.Set("container_properties", job.ContainerProperties)
+
+	containerProperties, err := flattenBatchContainerProperties(job.ContainerProperties)
+
+	if err != nil {
+		return fmt.Errorf("error converting Batch Container Properties to JSON: %s", err)
+	}
+
+	if err := d.Set("container_properties", containerProperties); err != nil {
+		return fmt.Errorf("error setting container_properties: %s", err)
+	}
+
+	d.Set("name", job.JobDefinitionName)
+
 	d.Set("parameters", aws.StringValueMap(job.Parameters))
 
 	if err := d.Set("retry_strategy", flattenBatchRetryStrategy(job.RetryStrategy)); err != nil {
@@ -215,6 +233,17 @@ func expandBatchJobContainerProperties(rawProps string) (*batch.ContainerPropert
 	return props, nil
 }
 
+// Convert batch.ContainerProperties object into its JSON representation
+func flattenBatchContainerProperties(containerProperties *batch.ContainerProperties) (string, error) {
+	b, err := jsonutil.BuildJSON(containerProperties)
+
+	if err != nil {
+		return "", err
+	}
+
+	return string(b), nil
+}
+
 func expandJobDefinitionParameters(params map[string]interface{}) map[string]*string {
 	var jobParams = make(map[string]*string)
 	for k, v := range params {

aws/resource_aws_batch_job_definition_test.go

@@ -37,16 +37,17 @@ func TestAccAWSBatchJobDefinition_basic(t *testing.T) {
 			MountPoints: []*batch.MountPoint{
 				{ContainerPath: aws.String("/tmp"), ReadOnly: aws.Bool(false), SourceVolume: aws.String("tmp")},
 			},
+			ResourceRequirements: []*batch.ResourceRequirement{},
 			Ulimits: []*batch.Ulimit{
 				{HardLimit: aws.Int64(int64(1024)), Name: aws.String("nofile"), SoftLimit: aws.Int64(int64(1024))},
 			},
+			Vcpus: aws.Int64(int64(1)),
 			Volumes: []*batch.Volume{
 				{
 					Host: &batch.Host{SourcePath: aws.String("/tmp")},
 					Name: aws.String("tmp"),
 				},
 			},
-			Vcpus: aws.Int64(int64(1)),
 		},
 	}
 	ri := acctest.RandInt()
@@ -60,6 +61,7 @@ func TestAccAWSBatchJobDefinition_basic(t *testing.T) {
 				Config: config,
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckBatchJobDefinitionExists("aws_batch_job_definition.test", &jd),
+					resource.TestCheckResourceAttr("aws_batch_job_definition.test", "name", fmt.Sprintf("tf_acctest_batch_job_definition_%d", ri)),
 					testAccCheckBatchJobDefinitionAttributes(&jd, &compare),
 				),
 			},