From 1aa2e6420f6cc69388564e976b45a912711f0b68 Mon Sep 17 00:00:00 2001 From: VenelinMartinov Date: Wed, 27 Nov 2024 12:45:24 +0000 Subject: [PATCH] SDKv2 Diff cross tests for replacement of computed properties (#2666) This PR adds Diff cross-tests for the SDKv2 bridge to test the re-computation of computed properties when the resource is marked for replacement. We display an incorrect preview in such cases in the PF and this also affects the SDKv2. related to https://github.com/pulumi/pulumi-terraform-bridge/issues/2660 --- pkg/internal/tests/cross-tests/diff_check.go | 3 +- .../tests/cross-tests/diff_cross_test.go | 113 ++++++++++++++++++ .../TestComputedProperty/no_change.golden | 24 ++++ .../non-computed_added.golden | 45 +++++++ .../non-computed_changed.golden | 47 ++++++++ .../non-computed_removed.golden | 43 +++++++ .../no_change.golden | 24 ++++ .../non-computed_added.golden | 45 +++++++ .../non-computed_changed.golden | 47 ++++++++ .../non-computed_removed.golden | 43 +++++++ 10 files changed, 433 insertions(+), 1 deletion(-) create mode 100644 pkg/internal/tests/cross-tests/testdata/TestComputedProperty/no_change.golden create mode 100644 pkg/internal/tests/cross-tests/testdata/TestComputedProperty/non-computed_added.golden create mode 100644 pkg/internal/tests/cross-tests/testdata/TestComputedProperty/non-computed_changed.golden create mode 100644 pkg/internal/tests/cross-tests/testdata/TestComputedProperty/non-computed_removed.golden create mode 100644 pkg/internal/tests/cross-tests/testdata/TestDetailedDiffReplacementComputedProperty/no_change.golden create mode 100644 pkg/internal/tests/cross-tests/testdata/TestDetailedDiffReplacementComputedProperty/non-computed_added.golden create mode 100644 pkg/internal/tests/cross-tests/testdata/TestDetailedDiffReplacementComputedProperty/non-computed_changed.golden create mode 100644 pkg/internal/tests/cross-tests/testdata/TestDetailedDiffReplacementComputedProperty/non-computed_removed.golden diff --git a/pkg/internal/tests/cross-tests/diff_check.go b/pkg/internal/tests/cross-tests/diff_check.go index ae17b763f..7a7972c01 100644 --- a/pkg/internal/tests/cross-tests/diff_check.go +++ b/pkg/internal/tests/cross-tests/diff_check.go @@ -20,6 +20,7 @@ import ( "github.com/hashicorp/terraform-plugin-go/tftypes" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/pulumi/pulumi/sdk/v3/go/auto/optpreview" "github.com/stretchr/testify/require" crosstestsimpl "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/internal/tests/cross-tests/impl" @@ -79,7 +80,7 @@ func runDiffCheck(t T, tc diffTestCase) crosstestsimpl.DiffResult { err := os.WriteFile(filepath.Join(pt.CurrentStack().Workspace().WorkDir(), "Pulumi.yaml"), yamlProgram, 0o600) require.NoErrorf(t, err, "writing Pulumi.yaml") - previewRes := pt.Preview(t) + previewRes := pt.Preview(t, optpreview.Diff()) diffResponse := crosstestsimpl.GetPulumiDiffResponse(t, pt.GrpcLog(t).Entries) x := pt.Up(t) diff --git a/pkg/internal/tests/cross-tests/diff_cross_test.go b/pkg/internal/tests/cross-tests/diff_cross_test.go index 5fd90b893..1391df0f5 100644 --- a/pkg/internal/tests/cross-tests/diff_cross_test.go +++ b/pkg/internal/tests/cross-tests/diff_cross_test.go @@ -26,7 +26,10 @@ import ( "github.com/hashicorp/terraform-plugin-go/tftypes" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/hexops/autogold/v2" + "github.com/pulumi/pulumi/sdk/v3/go/common/util/contract" "github.com/stretchr/testify/require" + "github.com/zclconf/go-cty/cty" ) func TestUnchangedBasicObject(t *testing.T) { @@ -1621,3 +1624,113 @@ func TestBlockCollectionElementForceNew(t *testing.T) { }) }) } + +func TestDetailedDiffReplacementComputedProperty(t *testing.T) { + t.Parallel() + // TODO[pulumi/pulumi-terraform-bridge#2660] + // We fail to re-compute computed properties when the resource is being replaced. + res := &schema.Resource{ + Schema: map[string]*schema.Schema{ + "computed": { + Type: schema.TypeString, + Computed: true, + }, + "other": { + Type: schema.TypeString, + Optional: true, + ForceNew: true, + }, + }, + CreateContext: func(ctx context.Context, rd *schema.ResourceData, meta interface{}) diag.Diagnostics { + rd.SetId("r1") + + err := rd.Set("computed", "computed_value") + contract.AssertNoErrorf(err, "setting computed") + return nil + }, + } + + type testOutput struct { + initialValue cty.Value + changeValue cty.Value + tfOut string + pulumiOut string + detailedDiff map[string]any + } + + t.Run("no change", func(t *testing.T) { + t.Parallel() + initialValue := cty.ObjectVal(map[string]cty.Value{}) + changeValue := cty.ObjectVal(map[string]cty.Value{}) + diff := runDiffCheck(t, diffTestCase{ + Resource: res, + Config1: initialValue, + Config2: changeValue, + }) + + autogold.ExpectFile(t, testOutput{ + initialValue: initialValue, + changeValue: changeValue, + tfOut: diff.TFOut, + pulumiOut: diff.PulumiOut, + detailedDiff: diff.PulumiDiff.DetailedDiff, + }) + }) + + t.Run("non-computed added", func(t *testing.T) { + t.Parallel() + initialValue := cty.ObjectVal(map[string]cty.Value{}) + changeValue := cty.ObjectVal(map[string]cty.Value{"other": cty.StringVal("other_value")}) + diff := runDiffCheck(t, diffTestCase{ + Resource: res, + Config1: initialValue, + Config2: changeValue, + }) + + autogold.ExpectFile(t, testOutput{ + initialValue: initialValue, + changeValue: changeValue, + tfOut: diff.TFOut, + pulumiOut: diff.PulumiOut, + detailedDiff: diff.PulumiDiff.DetailedDiff, + }) + }) + + t.Run("non-computed removed", func(t *testing.T) { + t.Parallel() + initialValue := cty.ObjectVal(map[string]cty.Value{"other": cty.StringVal("other_value")}) + changeValue := cty.ObjectVal(map[string]cty.Value{}) + diff := runDiffCheck(t, diffTestCase{ + Resource: res, + Config1: initialValue, + Config2: changeValue, + }) + + autogold.ExpectFile(t, testOutput{ + initialValue: initialValue, + changeValue: changeValue, + tfOut: diff.TFOut, + pulumiOut: diff.PulumiOut, + detailedDiff: diff.PulumiDiff.DetailedDiff, + }) + }) + + t.Run("non-computed changed", func(t *testing.T) { + t.Parallel() + initialValue := cty.ObjectVal(map[string]cty.Value{"other": cty.StringVal("other_value")}) + changeValue := cty.ObjectVal(map[string]cty.Value{"other": cty.StringVal("other_value_2")}) + diff := runDiffCheck(t, diffTestCase{ + Resource: res, + Config1: initialValue, + Config2: changeValue, + }) + + autogold.ExpectFile(t, testOutput{ + initialValue: initialValue, + changeValue: changeValue, + tfOut: diff.TFOut, + pulumiOut: diff.PulumiOut, + detailedDiff: diff.PulumiDiff.DetailedDiff, + }) + }) +} diff --git a/pkg/internal/tests/cross-tests/testdata/TestComputedProperty/no_change.golden b/pkg/internal/tests/cross-tests/testdata/TestComputedProperty/no_change.golden new file mode 100644 index 000000000..eb55880fa --- /dev/null +++ b/pkg/internal/tests/cross-tests/testdata/TestComputedProperty/no_change.golden @@ -0,0 +1,24 @@ +crosstests.testOutput{ + initialValue: cty.Value{ + ty: cty.Type{typeImpl: cty.typeObject{ + AttrTypes: map[string]cty.Type{}, + }}, + v: map[string]interface{}{}, + }, + changeValue: cty.Value{ + ty: cty.Type{typeImpl: cty.typeObject{AttrTypes: map[string]cty.Type{}}}, + v: map[string]interface{}{}, + }, + tfOut: ` +No changes. Your infrastructure matches the configuration. + +Terraform has compared your real infrastructure against your configuration +and found no differences, so no changes are needed. +`, + pulumiOut: `Previewing update (test): + pulumi:pulumi:Stack: (same) + [urn=urn:pulumi:test::project::pulumi:pulumi:Stack::project-test] +Resources: + 2 unchanged +`, +} diff --git a/pkg/internal/tests/cross-tests/testdata/TestComputedProperty/non-computed_added.golden b/pkg/internal/tests/cross-tests/testdata/TestComputedProperty/non-computed_added.golden new file mode 100644 index 000000000..fd574f5dc --- /dev/null +++ b/pkg/internal/tests/cross-tests/testdata/TestComputedProperty/non-computed_added.golden @@ -0,0 +1,45 @@ +crosstests.testOutput{ + initialValue: cty.Value{ + ty: cty.Type{typeImpl: cty.typeObject{ + AttrTypes: map[string]cty.Type{}, + }}, + v: map[string]interface{}{}, + }, + changeValue: cty.Value{ + ty: cty.Type{typeImpl: cty.typeObject{AttrTypes: map[string]cty.Type{ + "other": {typeImpl: cty.primitiveType{ + Kind: cty.primitiveTypeKind(83), + }}, + }}}, + v: map[string]interface{}{"other": "other_value"}, + }, + tfOut: ` +Terraform used the selected providers to generate the following execution +plan. Resource actions are indicated with the following symbols: ++/- create replacement and then destroy + +Terraform will perform the following actions: + + # crossprovider_test_res.example must be replaced ++/- resource "crossprovider_test_res" "example" { + ~ computed = "computed_value" -> (known after apply) + ~ id = "r1" -> (known after apply) + + other = "other_value" # forces replacement + } + +Plan: 1 to add, 0 to change, 1 to destroy. + +`, + pulumiOut: `Previewing update (test): + pulumi:pulumi:Stack: (same) + [urn=urn:pulumi:test::project::pulumi:pulumi:Stack::project-test] + +-crossprovider:index/testRes:TestRes: (replace) + [id=r1] + [urn=urn:pulumi:test::project::crossprovider:index/testRes:TestRes::example] + + other: "other_value" +Resources: + +-1 to replace + 1 unchanged +`, + detailedDiff: map[string]interface{}{"other": map[string]interface{}{"kind": "ADD_REPLACE"}}, +} diff --git a/pkg/internal/tests/cross-tests/testdata/TestComputedProperty/non-computed_changed.golden b/pkg/internal/tests/cross-tests/testdata/TestComputedProperty/non-computed_changed.golden new file mode 100644 index 000000000..23f60f5d7 --- /dev/null +++ b/pkg/internal/tests/cross-tests/testdata/TestComputedProperty/non-computed_changed.golden @@ -0,0 +1,47 @@ +crosstests.testOutput{ + initialValue: cty.Value{ + ty: cty.Type{typeImpl: cty.typeObject{ + AttrTypes: map[string]cty.Type{"other": { + typeImpl: cty.primitiveType{Kind: cty.primitiveTypeKind(83)}, + }}, + }}, + v: map[string]interface{}{"other": "other_value"}, + }, + changeValue: cty.Value{ + ty: cty.Type{typeImpl: cty.typeObject{AttrTypes: map[string]cty.Type{ + "other": {typeImpl: cty.primitiveType{ + Kind: cty.primitiveTypeKind(83), + }}, + }}}, + v: map[string]interface{}{"other": "other_value_2"}, + }, + tfOut: ` +Terraform used the selected providers to generate the following execution +plan. Resource actions are indicated with the following symbols: ++/- create replacement and then destroy + +Terraform will perform the following actions: + + # crossprovider_test_res.example must be replaced ++/- resource "crossprovider_test_res" "example" { + ~ computed = "computed_value" -> (known after apply) + ~ id = "r1" -> (known after apply) + ~ other = "other_value" -> "other_value_2" # forces replacement + } + +Plan: 1 to add, 0 to change, 1 to destroy. + +`, + pulumiOut: `Previewing update (test): + pulumi:pulumi:Stack: (same) + [urn=urn:pulumi:test::project::pulumi:pulumi:Stack::project-test] + +-crossprovider:index/testRes:TestRes: (replace) + [id=r1] + [urn=urn:pulumi:test::project::crossprovider:index/testRes:TestRes::example] + ~ other: "other_value" => "other_value_2" +Resources: + +-1 to replace + 1 unchanged +`, + detailedDiff: map[string]interface{}{"other": map[string]interface{}{"kind": "UPDATE_REPLACE"}}, +} diff --git a/pkg/internal/tests/cross-tests/testdata/TestComputedProperty/non-computed_removed.golden b/pkg/internal/tests/cross-tests/testdata/TestComputedProperty/non-computed_removed.golden new file mode 100644 index 000000000..ffdba7ec2 --- /dev/null +++ b/pkg/internal/tests/cross-tests/testdata/TestComputedProperty/non-computed_removed.golden @@ -0,0 +1,43 @@ +crosstests.testOutput{ + initialValue: cty.Value{ + ty: cty.Type{typeImpl: cty.typeObject{ + AttrTypes: map[string]cty.Type{"other": { + typeImpl: cty.primitiveType{Kind: cty.primitiveTypeKind(83)}, + }}, + }}, + v: map[string]interface{}{"other": "other_value"}, + }, + changeValue: cty.Value{ + ty: cty.Type{typeImpl: cty.typeObject{AttrTypes: map[string]cty.Type{}}}, + v: map[string]interface{}{}, + }, + tfOut: ` +Terraform used the selected providers to generate the following execution +plan. Resource actions are indicated with the following symbols: ++/- create replacement and then destroy + +Terraform will perform the following actions: + + # crossprovider_test_res.example must be replaced ++/- resource "crossprovider_test_res" "example" { + ~ computed = "computed_value" -> (known after apply) + ~ id = "r1" -> (known after apply) + - other = "other_value" -> null # forces replacement + } + +Plan: 1 to add, 0 to change, 1 to destroy. + +`, + pulumiOut: `Previewing update (test): + pulumi:pulumi:Stack: (same) + [urn=urn:pulumi:test::project::pulumi:pulumi:Stack::project-test] + +-crossprovider:index/testRes:TestRes: (replace) + [id=r1] + [urn=urn:pulumi:test::project::crossprovider:index/testRes:TestRes::example] + - other: "other_value" +Resources: + +-1 to replace + 1 unchanged +`, + detailedDiff: map[string]interface{}{"other": map[string]interface{}{"kind": "DELETE_REPLACE"}}, +} diff --git a/pkg/internal/tests/cross-tests/testdata/TestDetailedDiffReplacementComputedProperty/no_change.golden b/pkg/internal/tests/cross-tests/testdata/TestDetailedDiffReplacementComputedProperty/no_change.golden new file mode 100644 index 000000000..eb55880fa --- /dev/null +++ b/pkg/internal/tests/cross-tests/testdata/TestDetailedDiffReplacementComputedProperty/no_change.golden @@ -0,0 +1,24 @@ +crosstests.testOutput{ + initialValue: cty.Value{ + ty: cty.Type{typeImpl: cty.typeObject{ + AttrTypes: map[string]cty.Type{}, + }}, + v: map[string]interface{}{}, + }, + changeValue: cty.Value{ + ty: cty.Type{typeImpl: cty.typeObject{AttrTypes: map[string]cty.Type{}}}, + v: map[string]interface{}{}, + }, + tfOut: ` +No changes. Your infrastructure matches the configuration. + +Terraform has compared your real infrastructure against your configuration +and found no differences, so no changes are needed. +`, + pulumiOut: `Previewing update (test): + pulumi:pulumi:Stack: (same) + [urn=urn:pulumi:test::project::pulumi:pulumi:Stack::project-test] +Resources: + 2 unchanged +`, +} diff --git a/pkg/internal/tests/cross-tests/testdata/TestDetailedDiffReplacementComputedProperty/non-computed_added.golden b/pkg/internal/tests/cross-tests/testdata/TestDetailedDiffReplacementComputedProperty/non-computed_added.golden new file mode 100644 index 000000000..fd574f5dc --- /dev/null +++ b/pkg/internal/tests/cross-tests/testdata/TestDetailedDiffReplacementComputedProperty/non-computed_added.golden @@ -0,0 +1,45 @@ +crosstests.testOutput{ + initialValue: cty.Value{ + ty: cty.Type{typeImpl: cty.typeObject{ + AttrTypes: map[string]cty.Type{}, + }}, + v: map[string]interface{}{}, + }, + changeValue: cty.Value{ + ty: cty.Type{typeImpl: cty.typeObject{AttrTypes: map[string]cty.Type{ + "other": {typeImpl: cty.primitiveType{ + Kind: cty.primitiveTypeKind(83), + }}, + }}}, + v: map[string]interface{}{"other": "other_value"}, + }, + tfOut: ` +Terraform used the selected providers to generate the following execution +plan. Resource actions are indicated with the following symbols: ++/- create replacement and then destroy + +Terraform will perform the following actions: + + # crossprovider_test_res.example must be replaced ++/- resource "crossprovider_test_res" "example" { + ~ computed = "computed_value" -> (known after apply) + ~ id = "r1" -> (known after apply) + + other = "other_value" # forces replacement + } + +Plan: 1 to add, 0 to change, 1 to destroy. + +`, + pulumiOut: `Previewing update (test): + pulumi:pulumi:Stack: (same) + [urn=urn:pulumi:test::project::pulumi:pulumi:Stack::project-test] + +-crossprovider:index/testRes:TestRes: (replace) + [id=r1] + [urn=urn:pulumi:test::project::crossprovider:index/testRes:TestRes::example] + + other: "other_value" +Resources: + +-1 to replace + 1 unchanged +`, + detailedDiff: map[string]interface{}{"other": map[string]interface{}{"kind": "ADD_REPLACE"}}, +} diff --git a/pkg/internal/tests/cross-tests/testdata/TestDetailedDiffReplacementComputedProperty/non-computed_changed.golden b/pkg/internal/tests/cross-tests/testdata/TestDetailedDiffReplacementComputedProperty/non-computed_changed.golden new file mode 100644 index 000000000..23f60f5d7 --- /dev/null +++ b/pkg/internal/tests/cross-tests/testdata/TestDetailedDiffReplacementComputedProperty/non-computed_changed.golden @@ -0,0 +1,47 @@ +crosstests.testOutput{ + initialValue: cty.Value{ + ty: cty.Type{typeImpl: cty.typeObject{ + AttrTypes: map[string]cty.Type{"other": { + typeImpl: cty.primitiveType{Kind: cty.primitiveTypeKind(83)}, + }}, + }}, + v: map[string]interface{}{"other": "other_value"}, + }, + changeValue: cty.Value{ + ty: cty.Type{typeImpl: cty.typeObject{AttrTypes: map[string]cty.Type{ + "other": {typeImpl: cty.primitiveType{ + Kind: cty.primitiveTypeKind(83), + }}, + }}}, + v: map[string]interface{}{"other": "other_value_2"}, + }, + tfOut: ` +Terraform used the selected providers to generate the following execution +plan. Resource actions are indicated with the following symbols: ++/- create replacement and then destroy + +Terraform will perform the following actions: + + # crossprovider_test_res.example must be replaced ++/- resource "crossprovider_test_res" "example" { + ~ computed = "computed_value" -> (known after apply) + ~ id = "r1" -> (known after apply) + ~ other = "other_value" -> "other_value_2" # forces replacement + } + +Plan: 1 to add, 0 to change, 1 to destroy. + +`, + pulumiOut: `Previewing update (test): + pulumi:pulumi:Stack: (same) + [urn=urn:pulumi:test::project::pulumi:pulumi:Stack::project-test] + +-crossprovider:index/testRes:TestRes: (replace) + [id=r1] + [urn=urn:pulumi:test::project::crossprovider:index/testRes:TestRes::example] + ~ other: "other_value" => "other_value_2" +Resources: + +-1 to replace + 1 unchanged +`, + detailedDiff: map[string]interface{}{"other": map[string]interface{}{"kind": "UPDATE_REPLACE"}}, +} diff --git a/pkg/internal/tests/cross-tests/testdata/TestDetailedDiffReplacementComputedProperty/non-computed_removed.golden b/pkg/internal/tests/cross-tests/testdata/TestDetailedDiffReplacementComputedProperty/non-computed_removed.golden new file mode 100644 index 000000000..ffdba7ec2 --- /dev/null +++ b/pkg/internal/tests/cross-tests/testdata/TestDetailedDiffReplacementComputedProperty/non-computed_removed.golden @@ -0,0 +1,43 @@ +crosstests.testOutput{ + initialValue: cty.Value{ + ty: cty.Type{typeImpl: cty.typeObject{ + AttrTypes: map[string]cty.Type{"other": { + typeImpl: cty.primitiveType{Kind: cty.primitiveTypeKind(83)}, + }}, + }}, + v: map[string]interface{}{"other": "other_value"}, + }, + changeValue: cty.Value{ + ty: cty.Type{typeImpl: cty.typeObject{AttrTypes: map[string]cty.Type{}}}, + v: map[string]interface{}{}, + }, + tfOut: ` +Terraform used the selected providers to generate the following execution +plan. Resource actions are indicated with the following symbols: ++/- create replacement and then destroy + +Terraform will perform the following actions: + + # crossprovider_test_res.example must be replaced ++/- resource "crossprovider_test_res" "example" { + ~ computed = "computed_value" -> (known after apply) + ~ id = "r1" -> (known after apply) + - other = "other_value" -> null # forces replacement + } + +Plan: 1 to add, 0 to change, 1 to destroy. + +`, + pulumiOut: `Previewing update (test): + pulumi:pulumi:Stack: (same) + [urn=urn:pulumi:test::project::pulumi:pulumi:Stack::project-test] + +-crossprovider:index/testRes:TestRes: (replace) + [id=r1] + [urn=urn:pulumi:test::project::crossprovider:index/testRes:TestRes::example] + - other: "other_value" +Resources: + +-1 to replace + 1 unchanged +`, + detailedDiff: map[string]interface{}{"other": map[string]interface{}{"kind": "DELETE_REPLACE"}}, +}