Skip to content

Commit 651abfb

Browse files
jdavredbeardclaude
andcommitted
Address PR review feedback: guard against error values from getDefaultValue
- Add error type check to prevent getDefaultValue returning a raw error from corrupting state via resource.NewPropertyValue - Rephrase test comments to describe invariants instead of referencing issue numbers Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 60ce245 commit 651abfb

2 files changed

Lines changed: 14 additions & 11 deletions

File tree

pkg/tfbridge/schema.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2000,7 +2000,7 @@ func extractSchemaInputsObject(
20002000
if allowDrop && !etfs.Required() && isDefaultOrZeroValue(etfs, eps, ev) {
20012001
// If the field has a schema default, keep it rather than dropping.
20022002
// Dropping causes spurious diffs on next preview when PlanResourceChange
2003-
// re-applies the same default. See pulumi/pulumi-terraform-bridge#2436.
2003+
// re-applies the same default.
20042004
if getDefaultValue(etfs, eps) == nil {
20052005
pulumilog.V(9).Infof("skipping '%v' (not required + zero value, no schema default)", k)
20062006
continue
@@ -2013,8 +2013,12 @@ func extractSchemaInputsObject(
20132013
// PlanResourceChange will apply the default, causing a null → default diff.
20142014
if ev.IsNull() && allowDrop && !etfs.Required() {
20152015
if dv := getDefaultValue(etfs, eps); dv != nil {
2016-
v[k] = resource.NewPropertyValue(dv)
2017-
continue
2016+
// getDefaultValue may return an error as interface{} on DefaultFunc
2017+
// failure; skip injection in that case to avoid corrupting state.
2018+
if _, isErr := dv.(error); !isErr {
2019+
v[k] = resource.NewPropertyValue(dv)
2020+
continue
2021+
}
20182022
}
20192023
}
20202024

pkg/tfbridge/schema_test.go

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2717,9 +2717,8 @@ func TestExtractDefaultSecretInputs(t *testing.T) {
27172717
reservedkeys.Defaults: []interface{}{},
27182718
"inputA": "input_a_read",
27192719
"inputC": "input_c_read",
2720-
// With the fix for #2436, inputD is retained because its value
2721-
// matches the schema default. Previously it was dropped, causing
2722-
// a spurious diff on next preview.
2720+
// inputD is retained: its value matches the schema default, and dropping it would
2721+
// cause a spurious diff when PlanResourceChange re-applies the default on next preview.
27232722
"inputD": "input_d_default",
27242723
})
27252724
assert.Equal(t, expected, ins)
@@ -2792,8 +2791,8 @@ func TestExtractDefaultIntegerInputs(t *testing.T) {
27922791
assert.NoError(t, err)
27932792
expected := resource.NewPropertyMapFromMap(map[string]interface{}{
27942793
reservedkeys.Defaults: []interface{}{},
2795-
// With the fix for #2436, inputC and inputD are retained because
2796-
// their values match the schema defaults (-1). Previously dropped.
2794+
// inputC and inputD are retained: their values match the schema defaults (-1),
2795+
// and dropping them would cause spurious diffs on next preview.
27972796
"inputC": -1,
27982797
"inputD": -1,
27992798
})
@@ -3657,9 +3656,9 @@ func TestExtractInputsFromOutputsSdkv2(t *testing.T) {
36573656
}),
36583657
},
36593658
{
3660-
// With the fix for #2436, fields whose value matches the schema default
3661-
// are retained rather than dropped. Dropping them caused spurious diffs
3662-
// when PlanResourceChange re-applied the same default on next preview.
3659+
// Fields whose value matches the schema default are retained rather than
3660+
// dropped. Dropping them would cause spurious diffs when PlanResourceChange
3661+
// re-applies the same default on next preview.
36633662
name: "string attribute matching default is retained",
36643663
props: resource.NewPropertyMapFromMap(map[string]interface{}{"foo": "baz"}),
36653664
schemaMap: map[string]*schemav2.Schema{

0 commit comments

Comments
 (0)