Fix empty retry_strategy of Batch JobDefinition causing panics (#3921)

Computing whether a Batch JobDefinition has a diff panics if the retry_strategy is empty. This is a valid configuration because all properties of retry_strategy are optional.

This patch fixes that by adding the missing nil checks for the retry strategy.

What's notable is that having an empty retry_strategy will cause a perma diff. MaxItems=1 blocks with all optional attributes lead to permanent diffs if the user provides an empty block.

Fixes #3905

examples/examples_py_test.go

@@ -67,6 +67,16 @@ func TestSecretManagerPy(t *testing.T) {
 	integration.ProgramTest(t, &test)
 }
 
+func TestRegress3905(t *testing.T) {
+	test := getPythonBaseOptions(t).
+		With(integration.ProgramTestOptions{
+			Dir: filepath.Join(getCwd(t), "regress-3905"),
+			ExpectRefreshChanges: true, // JobDefinition.retry_strategy is suffering from a perma diff if the dict is empty. This is caused by the upstream provider ignoring empty object types
+		})
+
+	integration.ProgramTest(t, &test)
+}
+
 func getPythonBaseOptions(t *testing.T) integration.ProgramTestOptions {
 	envRegion := getEnvRegion(t)
 	pythonBase := integration.ProgramTestOptions{

examples/regress-3905/Pulumi.yaml

@@ -0,0 +1,10 @@
+name: regress-3905
+runtime:
+  name: python
+  options:
+    virtualenv: venv
+description: A minimal AWS Python Pulumi program to reproduce regression 3905
+config:
+  pulumi:tags:
+    value:
+      pulumi:template: ""

examples/regress-3905/__main__.py

@@ -0,0 +1,46 @@
+import json
+import pulumi
+import pulumi_aws as aws
+
+test = aws.batch.JobDefinition("regress-3905",
+    name="regress-3905",
+    opts=pulumi.ResourceOptions(ignore_changes=["retry_strategy"]), # retry_strategy is suffering from a perma diff if the dict is empty. This is caused by the upstream provider ignoring empty object types
+    type="container",
+    retry_strategy={}, # empty dict here causes regression 3905
+    container_properties=json.dumps({
+        "command": [
+            "ls",
+            "-la",
+        ],
+        "image": "busybox",
+        "resourceRequirements": [
+            {
+                "type": "VCPU",
+                "value": "4",
+            },
+            {
+                "type": "MEMORY",
+                "value": "512",
+            },
+        ],
+        "volumes": [{
+            "host": {
+                "sourcePath": "/tmp",
+            },
+            "name": "tmp",
+        }],
+        "environment": [{
+            "name": "VARNAME",
+            "value": "VARVAL",
+        }],
+        "mountPoints": [{
+            "sourceVolume": "tmp",
+            "containerPath": "/tmp",
+            "readOnly": False,
+        }],
+        "ulimits": [{
+            "hardLimit": 1024,
+            "name": "nofile",
+            "softLimit": 1024,
+        }],
+    }))

examples/regress-3905/requirements.txt

@@ -0,0 +1,2 @@
+pulumi>=3.0.0,<4.0.0
+pulumi-aws>=6.0.0,<7.0.0

patches/0057-Fix-empty-retry_strategy-of-Batch-JobDefinitions-cau.patch

@@ -0,0 +1,32 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Florian Stadler <[email protected]>
+Date: Fri, 10 May 2024 18:14:47 +0200
+Subject: [PATCH] Fix empty retry_strategy of Batch JobDefinitions causing
+ panics
+
+Computing whether a Batch JobDefinition has a diff panics if the
+retry_strategy is empty. This is a valid configuration because all
+properties of retry_strategy are optional.
+
+This patch fixes that by adding the missing nil checks for the
+retry strategy.
+
+diff --git a/internal/service/batch/job_definition.go b/internal/service/batch/job_definition.go
+index d1b5102162..a6a9c190c8 100644
+--- a/internal/service/batch/job_definition.go
++++ b/internal/service/batch/job_definition.go
+@@ -528,12 +528,12 @@ func needsJobDefUpdate(d *schema.ResourceDiff) bool {
+ 		}
+
+ 		var ors, nrs *batch.RetryStrategy
+-		if len(o.([]interface{})) > 0 {
++		if len(o.([]interface{})) > 0 && o.([]interface{})[0] != nil {
+ 			oProps := o.([]interface{})[0].(map[string]interface{})
+ 			ors = expandRetryStrategy(oProps)
+ 		}
+
+-		if len(n.([]interface{})) > 0 {
++		if len(n.([]interface{})) > 0 && n.([]interface{})[0] != nil {
+ 			nProps := n.([]interface{})[0].(map[string]interface{})
+ 			nrs = expandRetryStrategy(nProps)
+ 		}