From ba6524913c1a4b5b254bcc20465ed59b83b92b34 Mon Sep 17 00:00:00 2001 From: Florian Stadler Date: Fri, 10 May 2024 21:40:58 +0200 Subject: [PATCH] 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 | 10 ++++ examples/regress-3905/Pulumi.yaml | 10 ++++ examples/regress-3905/__main__.py | 46 +++++++++++++++++++ examples/regress-3905/requirements.txt | 2 + ...strategy-of-Batch-JobDefinitions-cau.patch | 32 +++++++++++++ 5 files changed, 100 insertions(+) create mode 100644 examples/regress-3905/Pulumi.yaml create mode 100644 examples/regress-3905/__main__.py create mode 100644 examples/regress-3905/requirements.txt create mode 100644 patches/0057-Fix-empty-retry_strategy-of-Batch-JobDefinitions-cau.patch diff --git a/examples/examples_py_test.go b/examples/examples_py_test.go index a2974a349b9..cad8b7af53b 100644 --- a/examples/examples_py_test.go +++ b/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{ diff --git a/examples/regress-3905/Pulumi.yaml b/examples/regress-3905/Pulumi.yaml new file mode 100644 index 00000000000..96608d3bd2e --- /dev/null +++ b/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: "" diff --git a/examples/regress-3905/__main__.py b/examples/regress-3905/__main__.py new file mode 100644 index 00000000000..3e2ce2f2c84 --- /dev/null +++ b/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, + }], + })) diff --git a/examples/regress-3905/requirements.txt b/examples/regress-3905/requirements.txt new file mode 100644 index 00000000000..5f626e8c2af --- /dev/null +++ b/examples/regress-3905/requirements.txt @@ -0,0 +1,2 @@ +pulumi>=3.0.0,<4.0.0 +pulumi-aws>=6.0.0,<7.0.0 diff --git a/patches/0057-Fix-empty-retry_strategy-of-Batch-JobDefinitions-cau.patch b/patches/0057-Fix-empty-retry_strategy-of-Batch-JobDefinitions-cau.patch new file mode 100644 index 00000000000..4463c1a9d2a --- /dev/null +++ b/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 +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) + }