diff --git a/dynamic/go.sum b/dynamic/go.sum index 522e87ed7..404bd7896 100644 --- a/dynamic/go.sum +++ b/dynamic/go.sum @@ -752,8 +752,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.11.1 h1:ULUL9fpb2Bwgf3jJHx0FamKYm0ld0KxBQr/uSAslRLk= diff --git a/go.mod b/go.mod index fef0a9828..baf2d86e9 100644 --- a/go.mod +++ b/go.mod @@ -41,7 +41,7 @@ require ( github.com/mitchellh/reflectwalk v1.0.2 github.com/pkg/errors v0.9.1 github.com/pulumi/inflector v0.1.1 - github.com/pulumi/providertest v0.1.2 + github.com/pulumi/providertest v0.1.3 github.com/pulumi/pulumi-java/pkg v0.16.1 github.com/pulumi/pulumi-yaml v1.11.1 github.com/pulumi/schema-tools v0.1.2 diff --git a/go.sum b/go.sum index e75d1d7be..c16eb0719 100644 --- a/go.sum +++ b/go.sum @@ -1932,8 +1932,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.11.1 h1:ULUL9fpb2Bwgf3jJHx0FamKYm0ld0KxBQr/uSAslRLk= diff --git a/pf/go.sum b/pf/go.sum index d02b9d021..b8a7b904c 100644 --- a/pf/go.sum +++ b/pf/go.sum @@ -712,8 +712,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.11.1 h1:ULUL9fpb2Bwgf3jJHx0FamKYm0ld0KxBQr/uSAslRLk= diff --git a/pkg/tests/schema_pulumi_test.go b/pkg/tests/schema_pulumi_test.go index f37eb33e4..1666a5fa3 100644 --- a/pkg/tests/schema_pulumi_test.go +++ b/pkg/tests/schema_pulumi_test.go @@ -17,6 +17,7 @@ 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/optpreview" "github.com/pulumi/pulumi/sdk/v3/go/auto/optrefresh" @@ -4159,3 +4160,429 @@ func TestMakeTerraformResultNilVsEmptyMap(t *testing.T) { assert.True(t, props["test"].DeepEquals(emptyMap)) }) } + +func TestUnknownCollectionForceNewDetailedDiff(t *testing.T) { + // TODO: Remove this once accurate bridge previews are rolled out + t.Setenv("PULUMI_TF_BRIDGE_ACCURATE_BRIDGE_PREVIEW", "true") + + collectionForceNewResource := func(typ schema.ValueType) *schema.Resource { + return &schema.Resource{ + Schema: map[string]*schema.Schema{ + "test": { + Type: typ, + Optional: true, + ForceNew: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "prop": { + Type: schema.TypeString, + Optional: true, + }, + }, + }, + }, + }, + } + } + + propertyForceNewResource := func(typ schema.ValueType) *schema.Resource { + return &schema.Resource{ + Schema: map[string]*schema.Schema{ + "test": { + Type: typ, + Optional: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "prop": { + Type: schema.TypeString, + Optional: true, + ForceNew: true, + }, + }, + }, + }, + }, + } + } + + auxResource := func(typ schema.ValueType) *schema.Resource { + return &schema.Resource{ + Schema: map[string]*schema.Schema{ + "aux": { + Type: typ, + Computed: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "prop": { + Type: schema.TypeString, + Computed: true, + }, + }, + }, + }, + }, + CreateContext: func(_ context.Context, d *schema.ResourceData, _ interface{}) diag.Diagnostics { + d.SetId("aux") + err := d.Set("aux", []map[string]interface{}{{"prop": "aux"}}) + require.NoError(t, err) + return nil + }, + } + } + + initialProgram := ` + name: test + runtime: yaml + resources: + mainRes: + type: prov:index:Test + properties: + tests: [{prop: 'value'}] +` + + program := ` + name: test + runtime: yaml + resources: + auxRes: + type: prov:index:Aux + mainRes: + type: prov:index:Test + properties: + tests: %s +` + + runTest := func(t *testing.T, program2 string, bridgedProvider info.Provider, expectedOutput autogold.Value) { + pt := pulcheck.PulCheck(t, bridgedProvider, initialProgram) + pt.Up(t) + pt.WritePulumiYaml(t, program2) + + res := pt.Preview(t, optpreview.Diff()) + + expectedOutput.Equal(t, res.StdOut) + } + + t.Run("list force new", func(t *testing.T) { + resMap := map[string]*schema.Resource{ + "prov_test": collectionForceNewResource(schema.TypeList), + "prov_aux": auxResource(schema.TypeList), + } + + tfp := &schema.Provider{ResourcesMap: resMap} + bridgedProvider := pulcheck.BridgedProvider(t, "prov", tfp) + runTest := func(t *testing.T, program2 string, expectedOutput autogold.Value) { + runTest(t, program2, bridgedProvider, expectedOutput) + } + + t.Run("unknown plain property", func(t *testing.T) { + program2 := fmt.Sprintf(program, "[{prop: \"${auxRes.auxes[0].prop}\"}]") + runTest(t, program2, autogold.Expect(`Previewing update (test): + pulumi:pulumi:Stack: (same) + [urn=urn:pulumi:test::test::pulumi:pulumi:Stack::test-test] + + prov:index/aux:Aux: (create) + [urn=urn:pulumi:test::test::prov:index/aux:Aux::auxRes] + ~ prov:index/test:Test: (update) + [id=newid] + [urn=urn:pulumi:test::test::prov:index/test:Test::mainRes] + ~ tests: [ + ~ [0]: { + ~ prop: "value" => output + } + ] +Resources: + + 1 to create + ~ 1 to update + 2 changes. 1 unchanged +`)) + }) + + t.Run("unknown object", func(t *testing.T) { + program2 := fmt.Sprintf(program, "[\"${auxRes.auxes[0]}\"]") + runTest(t, program2, autogold.Expect(`Previewing update (test): + pulumi:pulumi:Stack: (same) + [urn=urn:pulumi:test::test::pulumi:pulumi:Stack::test-test] + + prov:index/aux:Aux: (create) + [urn=urn:pulumi:test::test::prov:index/aux:Aux::auxRes] + +-prov:index/test:Test: (replace) + [id=newid] + [urn=urn:pulumi:test::test::prov:index/test:Test::mainRes] + ~ tests: [ + - [0]: { + - prop: "value" + } + + [0]: output + ] +Resources: + + 1 to create + +-1 to replace + 2 changes. 1 unchanged +`)) + }) + + t.Run("unknown collection", func(t *testing.T) { + program2 := fmt.Sprintf(program, "\"${auxRes.auxes}\"") + runTest(t, program2, autogold.Expect(`Previewing update (test): + pulumi:pulumi:Stack: (same) + [urn=urn:pulumi:test::test::pulumi:pulumi:Stack::test-test] + + prov:index/aux:Aux: (create) + [urn=urn:pulumi:test::test::prov:index/aux:Aux::auxRes] + +-prov:index/test:Test: (replace) + [id=newid] + [urn=urn:pulumi:test::test::prov:index/test:Test::mainRes] + - tests: [ + - [0]: { + - prop: "value" + } + ] + + tests: output +Resources: + + 1 to create + +-1 to replace + 2 changes. 1 unchanged +`)) + }) + }) + + t.Run("list property force new", func(t *testing.T) { + resMap := map[string]*schema.Resource{ + "prov_test": propertyForceNewResource(schema.TypeList), + "prov_aux": auxResource(schema.TypeList), + } + + tfp := &schema.Provider{ResourcesMap: resMap} + bridgedProvider := pulcheck.BridgedProvider(t, "prov", tfp) + runTest := func(t *testing.T, program2 string, expectedOutput autogold.Value) { + runTest(t, program2, bridgedProvider, expectedOutput) + } + + t.Run("unknown plain property", func(t *testing.T) { + program2 := fmt.Sprintf(program, "[{prop: \"${auxRes.auxes[0].prop}\"}]") + runTest(t, program2, autogold.Expect(`Previewing update (test): + pulumi:pulumi:Stack: (same) + [urn=urn:pulumi:test::test::pulumi:pulumi:Stack::test-test] + + prov:index/aux:Aux: (create) + [urn=urn:pulumi:test::test::prov:index/aux:Aux::auxRes] + +-prov:index/test:Test: (replace) + [id=newid] + [urn=urn:pulumi:test::test::prov:index/test:Test::mainRes] + ~ tests: [ + ~ [0]: { + ~ prop: "value" => output + } + ] +Resources: + + 1 to create + +-1 to replace + 2 changes. 1 unchanged +`)) + }) + + t.Run("unknown object", func(t *testing.T) { + program2 := fmt.Sprintf(program, "[\"${auxRes.auxes[0]}\"]") + runTest(t, program2, autogold.Expect(`Previewing update (test): + pulumi:pulumi:Stack: (same) + [urn=urn:pulumi:test::test::pulumi:pulumi:Stack::test-test] + + prov:index/aux:Aux: (create) + [urn=urn:pulumi:test::test::prov:index/aux:Aux::auxRes] + +-prov:index/test:Test: (replace) + [id=newid] + [urn=urn:pulumi:test::test::prov:index/test:Test::mainRes] + ~ tests: [ + - [0]: { + - prop: "value" + } + + [0]: output + ] +Resources: + + 1 to create + +-1 to replace + 2 changes. 1 unchanged +`)) + }) + + t.Run("unknown collection", func(t *testing.T) { + program2 := fmt.Sprintf(program, "\"${auxRes.auxes}\"") + runTest(t, program2, autogold.Expect(`Previewing update (test): + pulumi:pulumi:Stack: (same) + [urn=urn:pulumi:test::test::pulumi:pulumi:Stack::test-test] + + prov:index/aux:Aux: (create) + [urn=urn:pulumi:test::test::prov:index/aux:Aux::auxRes] + +-prov:index/test:Test: (replace) + [id=newid] + [urn=urn:pulumi:test::test::prov:index/test:Test::mainRes] + - tests: [ + - [0]: { + - prop: "value" + } + ] + + tests: output +Resources: + + 1 to create + +-1 to replace + 2 changes. 1 unchanged +`)) + }) + }) + + t.Run("set force new", func(t *testing.T) { + resMap := map[string]*schema.Resource{ + "prov_test": collectionForceNewResource(schema.TypeSet), + "prov_aux": auxResource(schema.TypeSet), + } + + tfp := &schema.Provider{ResourcesMap: resMap} + bridgedProvider := pulcheck.BridgedProvider(t, "prov", tfp) + runTest := func(t *testing.T, program2 string, expectedOutput autogold.Value) { + runTest(t, program2, bridgedProvider, expectedOutput) + } + + t.Run("unknown plain property", func(t *testing.T) { + program2 := fmt.Sprintf(program, "[{prop: \"${auxRes.auxes[0].prop}\"}]") + runTest(t, program2, autogold.Expect(`Previewing update (test): + pulumi:pulumi:Stack: (same) + [urn=urn:pulumi:test::test::pulumi:pulumi:Stack::test-test] + + prov:index/aux:Aux: (create) + [urn=urn:pulumi:test::test::prov:index/aux:Aux::auxRes] + ~ prov:index/test:Test: (update) + [id=newid] + [urn=urn:pulumi:test::test::prov:index/test:Test::mainRes] + ~ tests: [ + ~ [0]: { + ~ prop: "value" => output + } + ] +Resources: + + 1 to create + ~ 1 to update + 2 changes. 1 unchanged +`)) + }) + + t.Run("unknown object", func(t *testing.T) { + program2 := fmt.Sprintf(program, "[\"${auxRes.auxes[0]}\"]") + runTest(t, program2, autogold.Expect(`Previewing update (test): + pulumi:pulumi:Stack: (same) + [urn=urn:pulumi:test::test::pulumi:pulumi:Stack::test-test] + + prov:index/aux:Aux: (create) + [urn=urn:pulumi:test::test::prov:index/aux:Aux::auxRes] + +-prov:index/test:Test: (replace) + [id=newid] + [urn=urn:pulumi:test::test::prov:index/test:Test::mainRes] + ~ tests: [ + - [0]: { + - prop: "value" + } + + [0]: output + ] +Resources: + + 1 to create + +-1 to replace + 2 changes. 1 unchanged +`)) + }) + + t.Run("unknown collection", func(t *testing.T) { + program2 := fmt.Sprintf(program, "\"${auxRes.auxes}\"") + runTest(t, program2, autogold.Expect(`Previewing update (test): + pulumi:pulumi:Stack: (same) + [urn=urn:pulumi:test::test::pulumi:pulumi:Stack::test-test] + + prov:index/aux:Aux: (create) + [urn=urn:pulumi:test::test::prov:index/aux:Aux::auxRes] + +-prov:index/test:Test: (replace) + [id=newid] + [urn=urn:pulumi:test::test::prov:index/test:Test::mainRes] + - tests: [ + - [0]: { + - prop: "value" + } + ] + + tests: output +Resources: + + 1 to create + +-1 to replace + 2 changes. 1 unchanged +`)) + }) + }) + + t.Run("set property force new", func(t *testing.T) { + resMap := map[string]*schema.Resource{ + "prov_test": propertyForceNewResource(schema.TypeSet), + "prov_aux": auxResource(schema.TypeSet), + } + + tfp := &schema.Provider{ResourcesMap: resMap} + bridgedProvider := pulcheck.BridgedProvider(t, "prov", tfp) + runTest := func(t *testing.T, program2 string, expectedOutput autogold.Value) { + runTest(t, program2, bridgedProvider, expectedOutput) + } + + t.Run("unknown plain property", func(t *testing.T) { + program2 := fmt.Sprintf(program, "[{prop: \"${auxRes.auxes[0].prop}\"}]") + runTest(t, program2, autogold.Expect(`Previewing update (test): + pulumi:pulumi:Stack: (same) + [urn=urn:pulumi:test::test::pulumi:pulumi:Stack::test-test] + + prov:index/aux:Aux: (create) + [urn=urn:pulumi:test::test::prov:index/aux:Aux::auxRes] + +-prov:index/test:Test: (replace) + [id=newid] + [urn=urn:pulumi:test::test::prov:index/test:Test::mainRes] + ~ tests: [ + ~ [0]: { + ~ prop: "value" => output + } + ] +Resources: + + 1 to create + +-1 to replace + 2 changes. 1 unchanged +`)) + }) + + t.Run("unknown object", func(t *testing.T) { + program2 := fmt.Sprintf(program, "[\"${auxRes.auxes[0]}\"]") + runTest(t, program2, autogold.Expect(`Previewing update (test): + pulumi:pulumi:Stack: (same) + [urn=urn:pulumi:test::test::pulumi:pulumi:Stack::test-test] + + prov:index/aux:Aux: (create) + [urn=urn:pulumi:test::test::prov:index/aux:Aux::auxRes] + +-prov:index/test:Test: (replace) + [id=newid] + [urn=urn:pulumi:test::test::prov:index/test:Test::mainRes] + ~ tests: [ + - [0]: { + - prop: "value" + } + + [0]: output + ] +Resources: + + 1 to create + +-1 to replace + 2 changes. 1 unchanged +`)) + }) + + t.Run("unknown collection", func(t *testing.T) { + program2 := fmt.Sprintf(program, "\"${auxRes.auxes}\"") + runTest(t, program2, autogold.Expect(`Previewing update (test): + pulumi:pulumi:Stack: (same) + [urn=urn:pulumi:test::test::pulumi:pulumi:Stack::test-test] + + prov:index/aux:Aux: (create) + [urn=urn:pulumi:test::test::prov:index/aux:Aux::auxRes] + +-prov:index/test:Test: (replace) + [id=newid] + [urn=urn:pulumi:test::test::prov:index/test:Test::mainRes] + - tests: [ + - [0]: { + - prop: "value" + } + ] + + tests: output +Resources: + + 1 to create + +-1 to replace + 2 changes. 1 unchanged +`)) + }) + }) +} diff --git a/pkg/tfbridge/detailed_diff.go b/pkg/tfbridge/detailed_diff.go index f7d6a00a0..5e87e39f8 100644 --- a/pkg/tfbridge/detailed_diff.go +++ b/pkg/tfbridge/detailed_diff.go @@ -12,6 +12,7 @@ import ( "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge/info" shim "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim" "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/walk" + "github.com/pulumi/pulumi-terraform-bridge/v3/unstable/propertyvalue" ) func isPresent(val resource.PropertyValue) bool { @@ -41,6 +42,87 @@ func sortedMergedKeys[K cmp.Ordered, V any, M ~map[K]V](a, b M) []K { return keysSlice } +func isTypeShapeMismatched(val resource.PropertyValue, propType shim.ValueType) bool { + contract.Assertf(!val.IsSecret() || val.IsOutput(), "secrets and outputs are not handled") + if val.IsComputed() { + return false + } + if !isPresent(val) { + return false + } + switch propType { + case shim.TypeList: + return !val.IsArray() + case shim.TypeSet: + return !val.IsArray() + case shim.TypeMap: + return !val.IsObject() + default: + return false + } +} + +func lookupSchemas( + path propertyPath, tfs shim.SchemaMap, ps map[string]*info.Schema, +) (shim.Schema, *info.Schema, error) { + schemaPath := PropertyPathToSchemaPath(resource.PropertyPath(path), tfs, ps) + return LookupSchemas(schemaPath, tfs, ps) +} + +func propertyPathTriggersReplacement( + path propertyPath, rootTFSchema shim.SchemaMap, rootPulumiSchema map[string]*info.Schema, +) bool { + // A change on a property might trigger a replacement if: + // - The property itself is marked as ForceNew + // - The direct parent property is a collection (list, set, map) and is marked as ForceNew + // See pkg/cross-tests/diff_cross_test.go + // TestAttributeCollectionForceNew, TestBlockCollectionForceNew, TestBlockCollectionElementForceNew + // for a full case study of replacements in TF + tfs, ps, err := lookupSchemas(path, rootTFSchema, rootPulumiSchema) + if err != nil { + return false + } + if isForceNew(tfs, ps) { + return true + } + + if len(path) == 1 { + return false + } + + parent := path[:len(path)-1] + tfs, ps, err = lookupSchemas(parent, rootTFSchema, rootPulumiSchema) + if err != nil { + return false + } + // Note this is mimicking the TF behaviour, so the effective type is not considered here. + if tfs.Type() != shim.TypeList && tfs.Type() != shim.TypeSet && tfs.Type() != shim.TypeMap { + return false + } + return isForceNew(tfs, ps) +} + +func propertyValueTriggersReplacement( + path propertyPath, value resource.PropertyValue, tfs shim.SchemaMap, ps map[string]*info.Schema, +) bool { + replacement := false + visitor := func(subpath resource.PropertyPath, val resource.PropertyValue) (resource.PropertyValue, error) { + if propertyPathTriggersReplacement(propertyPath(subpath), tfs, ps) { + replacement = true + } + return val, nil + } + + _, err := propertyvalue.TransformPropertyValue( + resource.PropertyPath(path), + visitor, + value, + ) + contract.AssertNoErrorf(err, "TransformPropertyValue should not return an error") + + return replacement +} + func promoteToReplace(diff *pulumirpc.PropertyDiff) *pulumirpc.PropertyDiff { if diff == nil { return nil @@ -143,17 +225,6 @@ func (k propertyPath) IsReservedKey() bool { return leaf == "__meta" || leaf == "__defaults" } -func mapHasReplacements(m map[detailedDiffKey]*pulumirpc.PropertyDiff) bool { - for _, diff := range m { - if diff.GetKind() == pulumirpc.PropertyDiff_ADD_REPLACE || - diff.GetKind() == pulumirpc.PropertyDiff_DELETE_REPLACE || - diff.GetKind() == pulumirpc.PropertyDiff_UPDATE_REPLACE { - return true - } - } - return false -} - type detailedDiffer struct { tfs shim.SchemaMap ps map[string]*SchemaInfo @@ -182,71 +253,13 @@ func (differ detailedDiffer) getEffectiveType(path walk.SchemaPath) shim.ValueTy return tfs.Type() } -func (differ detailedDiffer) lookupSchemas(path propertyPath) (shim.Schema, *info.Schema, error) { - schemaPath := PropertyPathToSchemaPath(resource.PropertyPath(path), differ.tfs, differ.ps) - return LookupSchemas(schemaPath, differ.tfs, differ.ps) -} - -func (differ detailedDiffer) isForceNew(pair propertyPath) bool { - // A change on a property might trigger a replacement if: - // - The property itself is marked as ForceNew - // - The direct parent property is a collection (list, set, map) and is marked as ForceNew - // See pkg/cross-tests/diff_cross_test.go - // TestAttributeCollectionForceNew, TestBlockCollectionForceNew, TestBlockCollectionElementForceNew - // for a full case study of replacements in TF - tfs, ps, err := differ.lookupSchemas(pair) - if err != nil { - return false - } - if isForceNew(tfs, ps) { - return true - } - - if len(pair) == 1 { - return false - } - - parent := pair[:len(pair)-1] - tfs, ps, err = differ.lookupSchemas(parent) - if err != nil { - return false - } - // Note this is mimicking the TF behaviour, so the effective type is not considered here. - if tfs.Type() != shim.TypeList && tfs.Type() != shim.TypeSet && tfs.Type() != shim.TypeMap { - return false - } - return isForceNew(tfs, ps) -} - -// We do not short-circuit detailed diffs when comparing non-nil properties against nil ones. The reason for that is -// that a replace might be triggered by a ForceNew inside a nested property of a non-ForceNew property. We instead -// always walk the full tree even when comparing against a nil property. We then later do a simplification step for -// the detailed diff in simplifyDiff in order to reduce the diff to what the user expects to see. -// See [pulumi/pulumi-terraform-bridge#2405] for more details. -func (differ detailedDiffer) simplifyDiff( - diff map[detailedDiffKey]*pulumirpc.PropertyDiff, path propertyPath, old, new resource.PropertyValue, -) (map[detailedDiffKey]*pulumirpc.PropertyDiff, bool) { - baseDiff := makeBaseDiff(old, new) - if baseDiff == undecidedDiff { - return nil, false - } - propDiff := baseDiff.ToPropertyDiff() - if propDiff == nil { - return nil, true - } - if differ.isForceNew(path) || mapHasReplacements(diff) { - propDiff = promoteToReplace(propDiff) - } - return map[detailedDiffKey]*pulumirpc.PropertyDiff{path.Key(): propDiff}, true -} - // makePlainPropDiff is used for plain properties and ones with an unknown schema. // It does not access the TF schema, so it does not know about the type of the property. func (differ detailedDiffer) makePlainPropDiff( path propertyPath, old, new resource.PropertyValue, ) map[detailedDiffKey]*pulumirpc.PropertyDiff { baseDiff := makeBaseDiff(old, new) - isForceNew := differ.isForceNew(path) + isReplacement := propertyPathTriggersReplacement(path, differ.tfs, differ.ps) var propDiff *pulumirpc.PropertyDiff if baseDiff != undecidedDiff { propDiff = baseDiff.ToPropertyDiff() @@ -254,7 +267,7 @@ func (differ detailedDiffer) makePlainPropDiff( propDiff = &pulumirpc.PropertyDiff{Kind: pulumirpc.PropertyDiff_UPDATE} } - if isForceNew { + if isReplacement { propDiff = promoteToReplace(propDiff) } @@ -264,6 +277,32 @@ func (differ detailedDiffer) makePlainPropDiff( return nil } +// makeShortCircuitDiff is used for properties that are nil or computed in either the old or new state. +// It makes sure to check recursively if the property will trigger a replacement. +func (differ detailedDiffer) makeShortCircuitDiff( + path propertyPath, old, new resource.PropertyValue, +) map[detailedDiffKey]*pulumirpc.PropertyDiff { + contract.Assertf(old.IsNull() || new.IsNull() || new.IsComputed(), + "short-circuit diff should only be used for nil properties") + if old.IsNull() && new.IsNull() { + return nil + } + + baseDiff := makeBaseDiff(old, new) + contract.Assertf(baseDiff != undecidedDiff, "short-circuit diff could not determine diff kind") + + propDiff := baseDiff.ToPropertyDiff() + if new.IsComputed() && propertyPathTriggersReplacement(path, differ.tfs, differ.ps) { + propDiff = promoteToReplace(propDiff) + } else if !new.IsNull() && !new.IsComputed() && propertyValueTriggersReplacement(path, new, differ.tfs, differ.ps) { + propDiff = promoteToReplace(propDiff) + } else if !old.IsNull() && propertyValueTriggersReplacement(path, old, differ.tfs, differ.ps) { + propDiff = promoteToReplace(propDiff) + } + + return map[detailedDiffKey]*pulumirpc.PropertyDiff{path.Key(): propDiff} +} + func (differ detailedDiffer) makePropDiff( path propertyPath, old, new resource.PropertyValue, ) map[detailedDiffKey]*pulumirpc.PropertyDiff { @@ -272,6 +311,20 @@ func (differ detailedDiffer) makePropDiff( } propType := differ.getEffectiveType(differ.propertyPathToSchemaPath(path)) + if isTypeShapeMismatched(old, propType) || isTypeShapeMismatched(new, propType) { + return differ.makePlainPropDiff(path, old, new) + } + + if !isPresent(old) { + old = resource.NewNullProperty() + } + if !new.IsComputed() && !isPresent(new) { + new = resource.NewNullProperty() + } + if old.IsNull() || new.IsNull() || new.IsComputed() { + return differ.makeShortCircuitDiff(path, old, new) + } + switch propType { case shim.TypeList: return differ.makeListDiff(path, old, new) @@ -290,14 +343,8 @@ func (differ detailedDiffer) makeListDiff( path propertyPath, old, new resource.PropertyValue, ) map[detailedDiffKey]*pulumirpc.PropertyDiff { diff := make(map[detailedDiffKey]*pulumirpc.PropertyDiff) - oldList := []resource.PropertyValue{} - newList := []resource.PropertyValue{} - if isPresent(old) && old.IsArray() { - oldList = old.ArrayValue() - } - if isPresent(new) && new.IsArray() { - newList = new.ArrayValue() - } + oldList := old.ArrayValue() + newList := new.ArrayValue() // naive diffing of lists // TODO[pulumi/pulumi-terraform-bridge#2295]: implement a more sophisticated diffing algorithm @@ -318,27 +365,15 @@ func (differ detailedDiffer) makeListDiff( } } - simplerDiff, isSimplified := differ.simplifyDiff(diff, path, old, new) - if isSimplified { - return simplerDiff - } - return diff } func (differ detailedDiffer) makeMapDiff( path propertyPath, old, new resource.PropertyValue, ) map[detailedDiffKey]*pulumirpc.PropertyDiff { + oldMap := old.ObjectValue() + newMap := new.ObjectValue() diff := make(map[detailedDiffKey]*pulumirpc.PropertyDiff) - oldMap := resource.PropertyMap{} - newMap := resource.PropertyMap{} - if isPresent(old) && old.IsObject() { - oldMap = old.ObjectValue() - } - if isPresent(new) && new.IsObject() { - newMap = new.ObjectValue() - } - for _, k := range sortedMergedKeys(oldMap, newMap) { subindex := path.Subpath(string(k)) oldVal := oldMap[k] @@ -351,11 +386,6 @@ func (differ detailedDiffer) makeMapDiff( } } - simplerDiff, isSimplified := differ.simplifyDiff(diff, path, old, new) - if isSimplified { - return simplerDiff - } - return diff } diff --git a/pkg/tfbridge/detailed_diff_test.go b/pkg/tfbridge/detailed_diff_test.go index e75216d97..2dbda14ef 100644 --- a/pkg/tfbridge/detailed_diff_test.go +++ b/pkg/tfbridge/detailed_diff_test.go @@ -42,11 +42,9 @@ func TestSchemaLookupMaxItemsOnePlain(t *testing.T) { }, } - differ := detailedDiffer{ - tfs: shimv2.NewSchemaMap(sdkv2Schema), - } + tfs := shimv2.NewSchemaMap(sdkv2Schema) - sch, _, err := differ.lookupSchemas(newPropertyPath("string_prop")) + sch, _, err := lookupSchemas(newPropertyPath("string_prop"), tfs, nil) require.NoError(t, err) require.NotNil(t, sch) require.Equal(t, sch.Type(), shim.TypeList) @@ -71,16 +69,14 @@ func TestSchemaLookupMaxItemsOne(t *testing.T) { }, } - differ := detailedDiffer{ - tfs: shimv2.NewSchemaMap(res.Schema), - } + tfs := shimv2.NewSchemaMap(res.Schema) - sch, _, err := differ.lookupSchemas(newPropertyPath("foo")) + sch, _, err := lookupSchemas(newPropertyPath("foo"), tfs, nil) require.NoError(t, err) require.NotNil(t, sch) require.Equal(t, sch.Type(), shim.TypeList) - sch, _, err = differ.lookupSchemas(newPropertyPath("foo").Subpath("bar")) + sch, _, err = lookupSchemas(newPropertyPath("foo").Subpath("bar"), tfs, nil) require.NoError(t, err) require.NotNil(t, sch) require.Equal(t, sch.Type(), shim.TypeString) @@ -100,16 +96,14 @@ func TestSchemaLookupMap(t *testing.T) { }, } - differ := detailedDiffer{ - tfs: shimv2.NewSchemaMap(res.Schema), - } + tfs := shimv2.NewSchemaMap(res.Schema) - sch, _, err := differ.lookupSchemas(newPropertyPath("foo")) + sch, _, err := lookupSchemas(newPropertyPath("foo"), tfs, nil) require.NoError(t, err) require.NotNil(t, sch) require.Equal(t, sch.Type(), shim.TypeMap) - sch, _, err = differ.lookupSchemas(propertyPath{"foo", "bar"}) + sch, _, err = lookupSchemas(newPropertyPath("foo").Subpath("bar"), tfs, nil) require.NoError(t, err) require.NotNil(t, sch) require.Equal(t, sch.Type(), shim.TypeString) @@ -1753,3 +1747,116 @@ func TestDetailedDiffPulumiSchemaOverride(t *testing.T) { }) }) } + +func TestDetailedDiffMismatchedSchemas(t *testing.T) { + stringSchema := map[string]*schema.Schema{ + "foo": { + Type: schema.TypeString, + Optional: true, + }, + } + + listSchema := map[string]*schema.Schema{ + "foo": { + Type: schema.TypeList, + Optional: true, + Elem: &schema.Schema{ + Type: schema.TypeString, + }, + }, + } + + setSchema := map[string]*schema.Schema{ + "foo": { + Type: schema.TypeSet, + Optional: true, + Elem: &schema.Schema{ + Type: schema.TypeString, + }, + }, + } + + mapSchema := map[string]*schema.Schema{ + "foo": { + Type: schema.TypeMap, + Optional: true, + Elem: &schema.Schema{ + Type: schema.TypeString, + }, + }, + } + + stringValue := resource.NewPropertyMapFromMap( + map[string]interface{}{ + "foo": "string-value", + }, + ) + + listValue := resource.NewPropertyMapFromMap( + map[string]interface{}{ + "foo": []interface{}{"list-value"}, + }, + ) + + mapValue := resource.NewPropertyMapFromMap( + map[string]interface{}{ + "foo": map[string]interface{}{"bar": "map-value"}, + }, + ) + + t.Run("list schema with string value", func(t *testing.T) { + tfs := shimv2.NewSchemaMap(listSchema) + runDetailedDiffTest(t, stringValue, listValue, tfs, nil, map[string]*pulumirpc.PropertyDiff{ + "foo": {Kind: pulumirpc.PropertyDiff_UPDATE}, + }) + }) + + t.Run("list schema with map value", func(t *testing.T) { + tfs := shimv2.NewSchemaMap(listSchema) + runDetailedDiffTest(t, mapValue, listValue, tfs, nil, map[string]*pulumirpc.PropertyDiff{ + "foo": {Kind: pulumirpc.PropertyDiff_UPDATE}, + }) + }) + + t.Run("set schema with string value", func(t *testing.T) { + tfs := shimv2.NewSchemaMap(setSchema) + runDetailedDiffTest(t, stringValue, listValue, tfs, nil, map[string]*pulumirpc.PropertyDiff{ + "foo": {Kind: pulumirpc.PropertyDiff_UPDATE}, + }) + }) + + t.Run("set schema with map value", func(t *testing.T) { + tfs := shimv2.NewSchemaMap(setSchema) + runDetailedDiffTest(t, mapValue, listValue, tfs, nil, map[string]*pulumirpc.PropertyDiff{ + "foo": {Kind: pulumirpc.PropertyDiff_UPDATE}, + }) + }) + + t.Run("string schema with list value", func(t *testing.T) { + tfs := shimv2.NewSchemaMap(stringSchema) + runDetailedDiffTest(t, listValue, stringValue, tfs, nil, map[string]*pulumirpc.PropertyDiff{ + "foo": {Kind: pulumirpc.PropertyDiff_UPDATE}, + }) + }) + + t.Run("string schema with map value", func(t *testing.T) { + tfs := shimv2.NewSchemaMap(stringSchema) + runDetailedDiffTest(t, mapValue, stringValue, tfs, nil, map[string]*pulumirpc.PropertyDiff{ + "foo": {Kind: pulumirpc.PropertyDiff_UPDATE}, + }) + }) + + t.Run("map schema with string value", func(t *testing.T) { + tfs := shimv2.NewSchemaMap(mapSchema) + runDetailedDiffTest(t, stringValue, mapValue, tfs, nil, map[string]*pulumirpc.PropertyDiff{ + "foo": {Kind: pulumirpc.PropertyDiff_UPDATE}, + }) + }) + + t.Run("map schema with list value", func(t *testing.T) { + tfs := shimv2.NewSchemaMap(mapSchema) + runDetailedDiffTest(t, listValue, mapValue, tfs, nil, map[string]*pulumirpc.PropertyDiff{ + "foo": {Kind: pulumirpc.PropertyDiff_UPDATE}, + }) + }) +} diff --git a/x/muxer/go.mod b/x/muxer/go.mod index 04324e372..eecb23ec5 100644 --- a/x/muxer/go.mod +++ b/x/muxer/go.mod @@ -7,7 +7,7 @@ toolchain go1.23.1 replace github.com/pulumi/pulumi-terraform-bridge/v3 => ../../ require ( - github.com/pulumi/providertest v0.1.2 + github.com/pulumi/providertest v0.1.3 github.com/pulumi/pulumi-terraform-bridge/v3 v3.0.0-00010101000000-000000000000 github.com/stretchr/testify v1.9.0 google.golang.org/protobuf v1.34.2 diff --git a/x/muxer/go.sum b/x/muxer/go.sum index 9c575a1e3..599fd9fff 100644 --- a/x/muxer/go.sum +++ b/x/muxer/go.sum @@ -154,8 +154,8 @@ github.com/pulumi/appdash v0.0.0-20231130102222-75f619a67231 h1:vkHw5I/plNdTr435 github.com/pulumi/appdash v0.0.0-20231130102222-75f619a67231/go.mod h1:murToZ2N9hNJzewjHBgfFdXhZKjY3z5cYC1VXk+lbFE= 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/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/pkg/v3 v3.137.0 h1:/KPFQQaB5W0/GhVxSTGnEzv3ZW5uieGN5Q2q+Lsr+Zw= github.com/pulumi/pulumi/pkg/v3 v3.137.0/go.mod h1:ZQXJUTysDwq/mtilutRBKguH6DI+3b2WgNcOrs0whJ0= github.com/pulumi/pulumi/sdk/v3 v3.137.0 h1:bxhYpOY7Z4xt+VmezEpHuhjpOekkaMqOjzxFg/1OhCw=