diff --git a/pkg/tests/go.mod b/pkg/tests/go.mod index 214303bdc..cdbf46146 100644 --- a/pkg/tests/go.mod +++ b/pkg/tests/go.mod @@ -15,7 +15,7 @@ require ( github.com/hashicorp/terraform-plugin-sdk/v2 v2.33.0 github.com/hexops/autogold/v2 v2.2.1 github.com/hexops/valast v1.4.4 - github.com/pulumi/providertest v0.1.2 + github.com/pulumi/providertest v0.1.3 github.com/pulumi/pulumi-terraform-bridge/v3 v3.80.0 github.com/pulumi/pulumi/sdk v1.14.1 github.com/stretchr/testify v1.9.0 diff --git a/pkg/tests/go.sum b/pkg/tests/go.sum index 2528efbb1..636453986 100644 --- a/pkg/tests/go.sum +++ b/pkg/tests/go.sum @@ -1980,8 +1980,8 @@ github.com/pulumi/esc v0.10.0 h1:jzBKzkLVW0mePeanDRfqSQoCJ5yrkux0jIwAkUxpRKE= github.com/pulumi/esc v0.10.0/go.mod h1:2Bfa+FWj/xl8CKqRTWbWgDX0SOD4opdQgvYSURTGK2c= github.com/pulumi/inflector v0.1.1 h1:dvlxlWtXwOJTUUtcYDvwnl6Mpg33prhK+7mzeF+SobA= github.com/pulumi/inflector v0.1.1/go.mod h1:HUFCjcPTz96YtTuUlwG3i3EZG4WlniBvR9bd+iJxCUY= -github.com/pulumi/providertest v0.1.2 h1:9pJS9MeNkMyGwyNeHmvh8QqLgJy39Nk2/ym5u7r13ng= -github.com/pulumi/providertest v0.1.2/go.mod h1:GcsqEGgSngwaNOD+kICJPIUQlnA911fGBU8HDlJvVL0= +github.com/pulumi/providertest v0.1.3 h1:GpNKRy/haNjRHiUA9bi4diU4Op2zf3axYXbga5AepHg= +github.com/pulumi/providertest v0.1.3/go.mod h1:GcsqEGgSngwaNOD+kICJPIUQlnA911fGBU8HDlJvVL0= github.com/pulumi/pulumi-java/pkg v0.16.1 h1:orHnDWFbpOERwaBLry9f+6nqPX7x0MsrIkaa5QDGAns= github.com/pulumi/pulumi-java/pkg v0.16.1/go.mod h1:QH0DihZkWYle9XFc+LJ76m4hUo+fA3RdyaM90pqOaSM= github.com/pulumi/pulumi-yaml v1.10.3 h1:j5cjPiE32ILmjrWnC1cfZ0MWdqCZ8fg9wlaWk7HOtM4= diff --git a/pkg/tests/schema_pulumi_test.go b/pkg/tests/schema_pulumi_test.go index f37eb33e4..fc3111d6e 100644 --- a/pkg/tests/schema_pulumi_test.go +++ b/pkg/tests/schema_pulumi_test.go @@ -17,7 +17,9 @@ import ( "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tests/pulcheck" "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tests/tfcheck" "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge" + "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge/info" shimv2 "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/sdk-v2" + "github.com/pulumi/pulumi/sdk/v3/go/auto" "github.com/pulumi/pulumi/sdk/v3/go/auto/optpreview" "github.com/pulumi/pulumi/sdk/v3/go/auto/optrefresh" "github.com/pulumi/pulumi/sdk/v3/go/common/resource" @@ -4159,3 +4161,108 @@ func TestMakeTerraformResultNilVsEmptyMap(t *testing.T) { assert.True(t, props["test"].DeepEquals(emptyMap)) }) } + +func TestSetElementOrderInState(t *testing.T) { + sch := map[string]*schema.Schema{ + "test": { + Type: schema.TypeSet, + Optional: true, + Elem: &schema.Schema{ + Type: schema.TypeString, + }, + }, + } + + resMap := map[string]*schema.Resource{ + "prov_test": { + Schema: sch, + }, + } + + tfp := &schema.Provider{ResourcesMap: resMap} + prov := pulcheck.BridgedProvider(t, "prov", tfp) + + getStringOutputsFromResult := func(res auto.UpResult) []string { + outputs := res.Outputs["testOut"].Value.([]interface{}) + stringOutputs := make([]string, len(outputs)) + for i, v := range outputs { + stringOutputs[i] = v.(string) + } + return stringOutputs + } + + // Test that the order of elements in the state is preserved when created and updated. + runTest := func(t *testing.T, prov info.Provider, props1, props2 []string) { + props1JSON, err := json.Marshal(props1) + require.NoError(t, err) + program := ` + name: test + runtime: yaml + resources: + mainRes: + type: prov:index:Test + properties: + tests: %s + outputs: + testOut: ${mainRes.tests}` + + program1 := fmt.Sprintf(program, props1JSON) + + props2JSON, err := json.Marshal(props2) + require.NoError(t, err) + program2 := fmt.Sprintf(program, props2JSON) + + pt := pulcheck.PulCheck(t, prov, program1) + res := pt.Up(t) + + require.Equal(t, props1, getStringOutputsFromResult(res)) + + pt.WritePulumiYaml(t, program2) + res = pt.Up(t) + require.Equal(t, props2, getStringOutputsFromResult(res)) + } + + t.Run("ordered unchanged", func(t *testing.T) { + runTest(t, prov, + []string{"val1", "val2", "val3"}, + []string{"val1", "val2", "val3"}, + ) + }) + + t.Run("unordered unchanged", func(t *testing.T) { + runTest(t, prov, + []string{"val2", "val3", "val1"}, + []string{"val2", "val3", "val1"}, + ) + }) + + t.Run("ordered added", func(t *testing.T) { + runTest(t, prov, + []string{"val1", "val2"}, + []string{"val1", "val2", "val3"}, + ) + }) + + t.Run("unordered added", func(t *testing.T) { + runTest(t, prov, + []string{"val2", "val1"}, + []string{"val2", "val1", "val3"}, + ) + }) + + t.Run("ordered removed", func(t *testing.T) { + runTest(t, prov, + []string{"val1", "val2", "val3"}, + []string{"val1", "val2"}, + ) + }) + + t.Run("unordered removed", func(t *testing.T) { + runTest(t, prov, + []string{"val2", "val3", "val1"}, + []string{"val2", "val1"}, + ) + }) +} + +//TODO: Test unknown sets, unknowns elements in sets and nested unknowns in set elements. \ No newline at end of file diff --git a/pkg/tfbridge/detailed_diff.go b/pkg/tfbridge/detailed_diff.go index f7d6a00a0..0351add08 100644 --- a/pkg/tfbridge/detailed_diff.go +++ b/pkg/tfbridge/detailed_diff.go @@ -400,7 +400,9 @@ func makeDetailedDiffV2( if err != nil { return nil, err } - props, err := MakeTerraformResult(ctx, prov, proposedState, tfs, ps, assets, supportsSecrets) + props, err := makeTerraformResultWithOpts( + ctx, prov, proposedState, tfs, ps, assets, supportsSecrets, + makeTerraformResultOpts{originalInputs: newInputs}) if err != nil { return nil, err } @@ -409,7 +411,9 @@ func makeDetailedDiffV2( if err != nil { return nil, err } - priorProps, err := MakeTerraformResult(ctx, prov, prior, tfs, ps, assets, supportsSecrets) + priorProps, err := makeTerraformResultWithOpts( + ctx, prov, prior, tfs, ps, assets, supportsSecrets, + makeTerraformResultOpts{originalInputs: oldInputs}) if err != nil { return nil, err } diff --git a/pkg/tfbridge/provider.go b/pkg/tfbridge/provider.go index 1e96af03c..3f6dd7da4 100644 --- a/pkg/tfbridge/provider.go +++ b/pkg/tfbridge/provider.go @@ -1362,7 +1362,10 @@ func (p *Provider) Create(ctx context.Context, req *pulumirpc.CreateRequest) (*p } // Create the ID and property maps and return them. - props, err = MakeTerraformResult(ctx, p.tf, newstate, res.TF.Schema(), res.Schema.Fields, assets, p.supportsSecrets) + props, err = makeTerraformResultWithOpts( + ctx, p.tf, newstate, res.TF.Schema(), res.Schema.Fields, assets, p.supportsSecrets, + makeTerraformResultOpts{originalInputs: props}, + ) if err != nil { reasons = append(reasons, errors.Wrapf(err, "converting result for %s", urn).Error()) } @@ -1465,7 +1468,10 @@ func (p *Provider) Read(ctx context.Context, req *pulumirpc.ReadRequest) (*pulum // Store the ID and properties in the output. The ID *should* be the same as the input ID, but in the case // that the resource no longer exists, we will simply return the empty string and an empty property map. if newstate != nil { - props, err := MakeTerraformResult(ctx, p.tf, newstate, res.TF.Schema(), res.Schema.Fields, assets, p.supportsSecrets) + props, err := makeTerraformResultWithOpts( + ctx, p.tf, newstate, res.TF.Schema(), res.Schema.Fields, assets, p.supportsSecrets, + makeTerraformResultOpts{}, + ) if err != nil { return nil, err } @@ -1707,7 +1713,10 @@ func (p *Provider) Update(ctx context.Context, req *pulumirpc.UpdateRequest) (*p } } - props, err := MakeTerraformResult(ctx, p.tf, newstate, res.TF.Schema(), res.Schema.Fields, assets, p.supportsSecrets) + props, err := makeTerraformResultWithOpts( + ctx, p.tf, newstate, res.TF.Schema(), res.Schema.Fields, assets, p.supportsSecrets, + makeTerraformResultOpts{originalInputs: news}, + ) if err != nil { reasons = append(reasons, errors.Wrapf(err, "converting result for %s", urn).Error()) } @@ -1860,7 +1869,10 @@ func (p *Provider) Invoke(ctx context.Context, req *pulumirpc.InvokeRequest) (*p } // Add the special "id" attribute if it wasn't listed in the schema - props, err := MakeTerraformResult(ctx, p.tf, invoke, ds.TF.Schema(), ds.Schema.Fields, nil, p.supportsSecrets) + props, err := makeTerraformResultWithOpts( + ctx, p.tf, invoke, ds.TF.Schema(), ds.Schema.Fields, nil, p.supportsSecrets, + makeTerraformResultOpts{originalInputs: args}, + ) if err != nil { return nil, err } diff --git a/pkg/tfbridge/recover_set_order.go b/pkg/tfbridge/recover_set_order.go new file mode 100644 index 000000000..8bd39bb61 --- /dev/null +++ b/pkg/tfbridge/recover_set_order.go @@ -0,0 +1,104 @@ +package tfbridge + +import ( + "github.com/pulumi/pulumi/sdk/v3/go/common/resource" + "github.com/pulumi/pulumi/sdk/v3/go/common/util/contract" + + shim "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim" + "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/walk" +) + +// a variant of PropertyPath.Get which works on PropertyMaps +func getPathFromPropertyMap( + path resource.PropertyPath, propertyMap resource.PropertyMap, +) (resource.PropertyValue, bool) { + if len(path) == 0 { + return resource.NewNullProperty(), false + } + + rootKeyStr, ok := path[0].(string) + contract.Assertf(ok && rootKeyStr != "", "root key must be a non-empty string") + rootKey := resource.PropertyKey(rootKeyStr) + restPath := path[1:] + + if len(restPath) == 0 { + return propertyMap[rootKey], true + } + + if !propertyMap.HasValue(rootKey) { + return resource.NewNullProperty(), false + } + + return restPath.Get(propertyMap[rootKey]) +} + +// a variant of PropertyPath.Set which works on PropertyMaps +func setPathInPropertyMap( + path resource.PropertyPath, propertyMap resource.PropertyMap, value resource.PropertyValue, +) bool { + if len(path) == 0 { + return false + } + + rootKeyStr, ok := path[0].(string) + contract.Assertf(ok && rootKeyStr != "", "root key must be a non-empty string") + rootKey := resource.PropertyKey(rootKeyStr) + restPath := path[1:] + + if len(restPath) == 0 { + propertyMap[rootKey] = value + return true + } + + if !propertyMap.HasValue(rootKey) { + return false + } + + return restPath.Set(propertyMap[rootKey], value) +} + +// Sets in Terraform are unordered but are represented as ordered lists in the Pulumi world, and detailed diff must +// return indices into this list to indicate the elements that have changed. +func recoverSetOrderSingle( + target resource.PropertyMap, origin resource.PropertyMap, setPath resource.PropertyPath, +) { + targetSet, ok := getPathFromPropertyMap(setPath, target) + if !ok || !targetSet.IsArray() || targetSet.IsComputed() { + return + } + originSet, ok := getPathFromPropertyMap(setPath, origin) + if !ok || !originSet.IsArray() { + return + } + + // No need to recover the order if the set is empty or has only one element + if len(targetSet.ArrayValue()) == 0 || len(originSet.ArrayValue()) == 0 || len(targetSet.ArrayValue()) == 1 { + return + } + + ok = setPathInPropertyMap(setPath, target, originSet) + contract.Assertf(ok, "failed to set the origin set but it should exist!") +} + +// TODO: Should we cache this? +func getSetPaths(schema shim.SchemaMap) []SchemaPath { + paths := make([]SchemaPath, 0) + + walk.VisitSchemaMap(schema, func(path SchemaPath, schema shim.Schema) { + if schema.Type() == shim.TypeSet { + paths = append(paths, path) + } + }) + + return paths +} + +func recoverSetOrder( + target resource.PropertyMap, origin resource.PropertyMap, tfs shim.SchemaMap, ps map[string]*SchemaInfo, +) { + setPaths := getSetPaths(tfs) + for _, setPath := range setPaths { + propertyPath := SchemaPathToPropertyPath(setPath, tfs, ps) + recoverSetOrderSingle(target, origin, propertyPath) + } +} \ No newline at end of file diff --git a/pkg/tfbridge/schema.go b/pkg/tfbridge/schema.go index 4018c0b12..daa489fa5 100644 --- a/pkg/tfbridge/schema.go +++ b/pkg/tfbridge/schema.go @@ -1002,12 +1002,12 @@ func makeTerraformUnknown(tfs shim.Schema, unknownCollectionsSupported bool) int } } -// metaKey is the key in a TF bridge result that is used to store a resource's meta-attributes. -const metaKey = "__meta" +type makeTerraformResultOpts struct { + // If specified these will be used to recover the original order of elements in arrays representing TF sets. + originalInputs resource.PropertyMap +} -// MakeTerraformResult expands a Terraform state into an expanded Pulumi resource property map. This respects -// the property maps so that results end up with their correct Pulumi names when shipping back to the engine. -func MakeTerraformResult( +func makeTerraformResultWithOpts( ctx context.Context, p shim.Provider, state shim.InstanceState, @@ -1015,8 +1015,8 @@ func MakeTerraformResult( ps map[string]*SchemaInfo, assets AssetTable, supportsSecrets bool, + opts makeTerraformResultOpts, ) (resource.PropertyMap, error) { - var outs map[string]interface{} if state != nil { obj, err := state.Object(tfs) @@ -1035,9 +1035,31 @@ func MakeTerraformResult( outMap[metaKey] = resource.NewStringProperty(string(metaJSON)) } + if opts.originalInputs != nil { + recoverSetOrder(outMap, opts.originalInputs, tfs, ps) + } + return outMap, nil } +// metaKey is the key in a TF bridge result that is used to store a resource's meta-attributes. +const metaKey = "__meta" + +// MakeTerraformResult expands a Terraform state into an expanded Pulumi resource property map. This respects +// the property maps so that results end up with their correct Pulumi names when shipping back to the engine. +// Deprecated: Use makeTerraformResultWithOpts instead. +func MakeTerraformResult( + ctx context.Context, + p shim.Provider, + state shim.InstanceState, + tfs shim.SchemaMap, + ps map[string]*SchemaInfo, + assets AssetTable, + supportsSecrets bool, +) (resource.PropertyMap, error) { + return makeTerraformResultWithOpts(ctx, p, state, tfs, ps, assets, supportsSecrets, makeTerraformResultOpts{}) +} + // MakeTerraformOutputs takes an expanded Terraform property map and returns a Pulumi equivalent. This respects // the property maps so that results end up with their correct Pulumi names when shipping back to the engine. func MakeTerraformOutputs(