tfbridge: match scalar set elements by value when extracting inputs

pkg/tests/regress_set_order_test.go

@@ -0,0 +1,95 @@
+package tests
+
+import (
+	"context"
+	"encoding/json"
+	"testing"
+
+	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
+	"github.com/pulumi/providertest/pulumitest"
+	"github.com/stretchr/testify/require"
+
+	"github.com/pulumi/pulumi-terraform-bridge/v3/pkg/internal/tests/pulcheck"
+)
+
+// Refresh must not reorder the recorded inputs of a scalar TypeSet. A set is unordered, so a
+// provider returns its elements in an implementation-defined order that generally differs from the
+// order the values appear in the program. Rewriting the inputs into that order leaves state
+// permanently different from what the program produces. The difference is invisible to Diff(), but
+// it defeats the engine's checkpoint write elision and forces a full state write for the resource
+// on every subsequent update.
+//
+// The program writes ["zeta", "alpha"] and the provider returns ["alpha", "zeta"].
+func TestRegressScalarSetOrderRefreshPreservesStateInputs(t *testing.T) {
+	t.Parallel()
+
+	resMap := map[string]*schema.Resource{
+		"prov_test": {
+			Schema: map[string]*schema.Schema{
+				"administrators": {
+					Type:     schema.TypeSet,
+					Optional: true,
+					Computed: true,
+					Elem:     &schema.Schema{Type: schema.TypeString},
+					Set:      schema.HashString,
+				},
+			},
+			ReadContext: func(_ context.Context, rd *schema.ResourceData, _ interface{}) diag.Diagnostics {
+				require.NoError(t, rd.Set("administrators", []interface{}{"zeta", "alpha"}))
+				return nil
+			},
+			CreateContext: func(_ context.Context, rd *schema.ResourceData, _ interface{}) diag.Diagnostics {
+				rd.SetId("id0")
+				return nil
+			},
+		},
+	}
+
+	bridgedProvider := pulcheck.BridgedProvider(t, "prov", &schema.Provider{ResourcesMap: resMap})
+	pt := pulcheck.PulCheck(t, bridgedProvider, `
+name: test
+runtime: yaml
+resources:
+  mainRes:
+    type: prov:index:Test
+    properties:
+      administrators: ["zeta", "alpha"]
+`)
+	pt.Up(t)
+
+	before := setResInputs(t, pt)
+	require.Equal(t, []interface{}{"zeta", "alpha"}, before["administrators"],
+		"precondition: state should record the order the program wrote")
+
+	pt.Refresh(t)
+
+	// Assert on the set alone rather than the whole input bag: the empty __defaults marker is
+	// dropped by refresh through an unrelated bug, tracked separately as #3567.
+	require.Equal(t, before["administrators"], setResInputs(t, pt)["administrators"],
+		"refresh must not reorder the inputs recorded in state")
+}
+
+func setResInputs(t *testing.T, pt *pulumitest.PulumiTest) map[string]interface{} {
+	t.Helper()
+
+	data, err := pt.ExportStack(t).Deployment.MarshalJSON()
+	require.NoError(t, err)
+
+	var deployment struct {
+		Resources []struct {
+			Type   string                 `json:"type"`
+			Inputs map[string]interface{} `json:"inputs"`
+		} `json:"resources"`
+	}
+	require.NoError(t, json.Unmarshal(data, &deployment))
+
+	for _, r := range deployment.Resources {
+		if r.Type == "prov:index/test:Test" {
+			return r.Inputs
+		}
+	}
+
+	require.Fail(t, "did not find the test resource in the exported stack")
+	return nil
+}

pkg/tfbridge/schema.go

@@ -1650,26 +1650,82 @@ func convertTfStringToFloat(stringValue string) (interface{}, error) {
 	return floatVal, nil
 }
 
-// matchSetElements returns a mapping from old array indices to new array indices for TypeSet
-// arrays, using property-overlap scoring to find the best content-based match. Returns nil to
-// signal that the caller should fall back to positional matching (e.g. for non-TypeSet schemas,
-// scalar set elements, or nil schemas).
-func matchSetElements(
-	oldArray, newArray []resource.PropertyValue, tfs shim.Schema,
-) map[int]int {
-	if tfs == nil || tfs.Type() != shim.TypeSet {
+func allObjectValues(values []resource.PropertyValue) bool {
+	for _, v := range values {
+		if !v.IsObject() {
+			return false
+		}
+	}
+	return true
+}
+
+// scalarSetKey returns a comparable key for a scalar set element. Values that are not plain
+// scalars (computed, secret, output, nested collections) report false so that the caller falls
+// back to positional matching rather than guessing.
+func scalarSetKey(v resource.PropertyValue) (string, bool) {
+	switch {
+	case v.IsString():
+		return "s:" + v.StringValue(), true
+	case v.IsNumber():
+		return "n:" + strconv.FormatFloat(v.NumberValue(), 'g', -1, 64), true
+	case v.IsBool():
+		return "b:" + strconv.FormatBool(v.BoolValue()), true
+	}
+	return "", false
+}
+
+// matchScalarSetElements matches scalar set elements by exact value. A set is unordered, so a
+// provider returns its elements in an implementation-defined order that generally differs from the
+// order the values were written in the program. Refreshing a scalar set would otherwise rewrite the
+// recorded inputs into the provider's order and leave them permanently different from what the
+// program produces.
+//
+// A mapping is only produced when both sides hold the same multiset of values. When set membership
+// genuinely changed, nil is returned so the caller keeps its positional behavior and the change
+// stays visible as drift.
+func matchScalarSetElements(oldArray, newArray []resource.PropertyValue) map[int]int {
+	if len(oldArray) != len(newArray) {
 		return nil
 	}
-	// Only match object elements — scalar sets have no keys to score on.
-	for _, v := range oldArray {
-		if !v.IsObject() {
+
+	available := make(map[string][]int, len(newArray))
+	for ni, newElem := range newArray {
+		key, ok := scalarSetKey(newElem)
+		if !ok {
 			return nil
 		}
+		available[key] = append(available[key], ni)
 	}
-	for _, v := range newArray {
-		if !v.IsObject() {
+
+	indexMap := make(map[int]int, len(oldArray))
+	for oi, oldElem := range oldArray {
+		key, ok := scalarSetKey(oldElem)
+		if !ok {
+			return nil
+		}
+		matches := available[key]
+		if len(matches) == 0 {
 			return nil
 		}
+		indexMap[oi] = matches[0]
+		available[key] = matches[1:]
+	}
+	return indexMap
+}
+
+// matchSetElements returns a mapping from old array indices to new array indices for TypeSet
+// arrays. Object elements are matched by property-overlap scoring; scalar elements are matched by
+// exact value. Returns nil to signal that the caller should fall back to positional matching (e.g.
+// for non-TypeSet schemas or nil schemas).
+func matchSetElements(
+	oldArray, newArray []resource.PropertyValue, tfs shim.Schema,
+) map[int]int {
+	if tfs == nil || tfs.Type() != shim.TypeSet {
+		return nil
+	}
+	// Scalar sets have no keys to score on, but their elements can be matched exactly by value.
+	if !allObjectValues(oldArray) || !allObjectValues(newArray) {
+		return matchScalarSetElements(oldArray, newArray)
 	}
 
 	type candidate struct {

pkg/tfbridge/schema_test.go

@@ -4671,6 +4671,85 @@ func TestRefreshExtractInputsTypeSetReorder(t *testing.T) {
 	})
 }
 
+// TestRefreshExtractInputsScalarTypeSetReorder verifies that extractInputs preserves the recorded
+// order of a scalar TypeSet when the provider returns the same values in Terraform's set hash
+// order. Rewriting the inputs into hash order leaves them permanently different from what the
+// program produces, which forces a checkpoint write on every update even though nothing changed.
+func TestRefreshExtractInputsScalarTypeSetReorder(t *testing.T) {
+	t.Parallel()
+
+	scalarSetSchema := func(typ shim.ValueType) shim.SchemaMap {
+		return schemaMap(map[string]*schema.Schema{
+			"administrators": {
+				Type:     typ,
+				Optional: true,
+				Elem:     (&schema.Schema{Type: shim.TypeString}).Shim(),
+			},
+		})
+	}
+
+	strs := func(values ...string) resource.PropertyValue {
+		elems := make([]resource.PropertyValue, 0, len(values))
+		for _, v := range values {
+			elems = append(elems, resource.NewStringProperty(v))
+		}
+		return resource.NewArrayProperty(elems)
+	}
+
+	actualStrings := func(t *testing.T, pm resource.PropertyMap) []string {
+		t.Helper()
+		out := []string{}
+		for _, v := range pm["administrators"].ArrayValue() {
+			out = append(out, v.StringValue())
+		}
+		return out
+	}
+
+	t.Run("set_order_preserved", func(t *testing.T) {
+		t.Parallel()
+
+		oldInputs := resource.PropertyMap{"administrators": strs("bob", "alice")}
+		outs := resource.PropertyMap{"administrators": strs("alice", "bob")}
+
+		actual, err := ExtractInputsFromOutputs(oldInputs, outs, scalarSetSchema(shim.TypeSet), nil, true)
+		require.NoError(t, err)
+		assert.Equal(t, []string{"bob", "alice"}, actualStrings(t, actual))
+	})
+
+	t.Run("typelist_still_positional", func(t *testing.T) {
+		t.Parallel()
+
+		oldInputs := resource.PropertyMap{"administrators": strs("bob", "alice")}
+		outs := resource.PropertyMap{"administrators": strs("alice", "bob")}
+
+		actual, err := ExtractInputsFromOutputs(oldInputs, outs, scalarSetSchema(shim.TypeList), nil, true)
+		require.NoError(t, err)
+		assert.Equal(t, []string{"alice", "bob"}, actualStrings(t, actual))
+	})
+
+	t.Run("changed_membership_still_visible", func(t *testing.T) {
+		t.Parallel()
+
+		oldInputs := resource.PropertyMap{"administrators": strs("bob", "alice")}
+		outs := resource.PropertyMap{"administrators": strs("alice", "carol")}
+
+		actual, err := ExtractInputsFromOutputs(oldInputs, outs, scalarSetSchema(shim.TypeSet), nil, true)
+		require.NoError(t, err)
+		assert.Equal(t, []string{"alice", "carol"}, actualStrings(t, actual))
+	})
+
+	t.Run("duplicate_values_preserved", func(t *testing.T) {
+		t.Parallel()
+
+		oldInputs := resource.PropertyMap{"administrators": strs("bob", "alice", "bob")}
+		outs := resource.PropertyMap{"administrators": strs("alice", "bob", "bob")}
+
+		actual, err := ExtractInputsFromOutputs(oldInputs, outs, scalarSetSchema(shim.TypeSet), nil, true)
+		require.NoError(t, err)
+		assert.Equal(t, []string{"bob", "alice", "bob"}, actualStrings(t, actual))
+	})
+}
+
 // TestCheckMakeTerraformInputsTypeSetReorder verifies that makeTerraformInputs correctly matches
 // TypeSet elements by content rather than position when old state has a different order than new inputs.
 // See https://github.com/pulumi/pulumi-terraform-bridge/issues/3392.