From cdecf0cddd1c89ba20a887185f6a0636e450949b Mon Sep 17 00:00:00 2001 From: Venelin Date: Mon, 6 May 2024 19:40:10 +0100 Subject: [PATCH] Revert "Remove experimental flag for SkipDetailedDiff (#1893)" This reverts commit c57799fef75dc06aa2a3cae1bf25e97bd28bef56, reversing changes made to e71c1a6dafa243907e0b67fa12f13ec8fd8ca76c. --- pkg/tfbridge/diff_test.go | 19 +++++++++++-------- pkg/tfbridge/info/info.go | 5 +++++ pkg/tfbridge/provider.go | 2 +- pkg/tfbridge/provider_test.go | 31 ++++--------------------------- 4 files changed, 21 insertions(+), 36 deletions(-) diff --git a/pkg/tfbridge/diff_test.go b/pkg/tfbridge/diff_test.go index f3e17e72e..d4456b439 100644 --- a/pkg/tfbridge/diff_test.go +++ b/pkg/tfbridge/diff_test.go @@ -2071,13 +2071,14 @@ func TestListNestedAddMaxItemsOne(t *testing.T) { } type diffTestCase struct { - resourceSchema map[string]*schema.Schema - resourceFields map[string]*SchemaInfo - state resource.PropertyMap - inputs resource.PropertyMap - expected map[string]*pulumirpc.PropertyDiff - expectedDiffChanges pulumirpc.DiffResponse_DiffChanges - ignoreChanges []string + resourceSchema map[string]*schema.Schema + resourceFields map[string]*SchemaInfo + state resource.PropertyMap + inputs resource.PropertyMap + expected map[string]*pulumirpc.PropertyDiff + expectedDiffChanges pulumirpc.DiffResponse_DiffChanges + ignoreChanges []string + XSkipDetailedDiffForChanges bool } func diffTest2(t *testing.T, tc diffTestCase) { @@ -2099,6 +2100,7 @@ func diffTest2(t *testing.T, tc diffTestCase) { p := Provider{ tf: provider, info: ProviderInfo{ + XSkipDetailedDiffForChanges: tc.XSkipDetailedDiffForChanges, Resources: map[string]*ResourceInfo{ "p_resource": { Tok: "pkg:index:PResource", @@ -2149,7 +2151,8 @@ func TestChangingMaxItems1FilterProperty(t *testing.T) { }, } diffTest2(t, diffTestCase{ - resourceSchema: schema, + XSkipDetailedDiffForChanges: true, + resourceSchema: schema, state: resource.PropertyMap{ "rules": resource.NewArrayProperty( []resource.PropertyValue{ diff --git a/pkg/tfbridge/info/info.go b/pkg/tfbridge/info/info.go index 70a0e2b9d..77085e76d 100644 --- a/pkg/tfbridge/info/info.go +++ b/pkg/tfbridge/info/info.go @@ -151,6 +151,11 @@ type Provider struct { // See also: pulumi/pulumi-terraform-bridge#1448 SkipValidateProviderConfigForPluginFramework bool + // Disables using detailed diff to determine diff changes and falls back on the length of TF Diff Attributes. + // + // See https://github.com/pulumi/pulumi-terraform-bridge/issues/1501 + XSkipDetailedDiffForChanges bool + // Enables generation of a trimmed, runtime-only metadata file // to help reduce resource plugin start time // diff --git a/pkg/tfbridge/provider.go b/pkg/tfbridge/provider.go index 01a2db418..5144271e3 100644 --- a/pkg/tfbridge/provider.go +++ b/pkg/tfbridge/provider.go @@ -960,7 +960,7 @@ func (p *Provider) Diff(ctx context.Context, req *pulumirpc.DiffRequest) (*pulum // We will still use `detailedDiff` for diff display purposes. // See also https://github.com/pulumi/pulumi-terraform-bridge/issues/1501. - if len(diff.Attributes()) > 0 { + if p.info.XSkipDetailedDiffForChanges && len(diff.Attributes()) > 0 { changes = pulumirpc.DiffResponse_DIFF_SOME // Perhaps collectionDiffs can shed some light and locate the changes to the end-user. for path, diff := range dd.collectionDiffs { diff --git a/pkg/tfbridge/provider_test.go b/pkg/tfbridge/provider_test.go index 0f90d39ed..89b85cd48 100644 --- a/pkg/tfbridge/provider_test.go +++ b/pkg/tfbridge/provider_test.go @@ -2156,6 +2156,9 @@ func TestSkipDetailedDiff(t *testing.T) { Schema: &ResourceInfo{Tok: "Replace"}, }, }, + info: ProviderInfo{ + XSkipDetailedDiffForChanges: skipDetailedDiffForChanges, + }, } } t.Run("Diff", func(t *testing.T) { @@ -2422,26 +2425,6 @@ func TestMaxItemOneWrongStateDiff(t *testing.T) { }, } t.Run("DiffListAndVal", func(t *testing.T) { - testutils.Replay(t, provider, ` - { - "method": "/pulumirpc.ResourceProvider/Diff", - "request": { - "urn": "urn:pulumi:dev::teststack::NestedStrRes::exres", - "id": "0", - "olds": { - "nested_str": [""] - }, - "news": { - "nested_str": "" - } - }, - "response": { - "changes": "DIFF_SOME", - "hasDetailedDiff": true - } - }`) - }) - t.Run("DiffNilListAndVal", func(t *testing.T) { testutils.Replay(t, provider, ` { "method": "/pulumirpc.ResourceProvider/Diff", @@ -2457,13 +2440,7 @@ func TestMaxItemOneWrongStateDiff(t *testing.T) { }, "response": { "changes": "DIFF_SOME", - "hasDetailedDiff": true, - "detailedDiff": { - "nested_str": { - "kind": "UPDATE" - } - }, - "diffs": ["nested_str"] + "hasDetailedDiff": true } }`) })