From 682a19a7e395d0cacbb9817b93f7cc30e49439a1 Mon Sep 17 00:00:00 2001 From: guineveresaenger Date: Fri, 19 Apr 2024 16:14:19 -0700 Subject: [PATCH] Remove experimental flag for SkipDetailedDiff --- 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, 36 insertions(+), 21 deletions(-) diff --git a/pkg/tfbridge/diff_test.go b/pkg/tfbridge/diff_test.go index d4456b439..f3e17e72e 100644 --- a/pkg/tfbridge/diff_test.go +++ b/pkg/tfbridge/diff_test.go @@ -2071,14 +2071,13 @@ 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 - XSkipDetailedDiffForChanges bool + 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 } func diffTest2(t *testing.T, tc diffTestCase) { @@ -2100,7 +2099,6 @@ 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", @@ -2151,8 +2149,7 @@ func TestChangingMaxItems1FilterProperty(t *testing.T) { }, } diffTest2(t, diffTestCase{ - XSkipDetailedDiffForChanges: true, - resourceSchema: schema, + 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 77085e76d..70a0e2b9d 100644 --- a/pkg/tfbridge/info/info.go +++ b/pkg/tfbridge/info/info.go @@ -151,11 +151,6 @@ 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 b36b271b8..46d329933 100644 --- a/pkg/tfbridge/provider.go +++ b/pkg/tfbridge/provider.go @@ -926,7 +926,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 p.info.XSkipDetailedDiffForChanges && len(diff.Attributes()) > 0 { + if 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 89b85cd48..0f90d39ed 100644 --- a/pkg/tfbridge/provider_test.go +++ b/pkg/tfbridge/provider_test.go @@ -2156,9 +2156,6 @@ func TestSkipDetailedDiff(t *testing.T) { Schema: &ResourceInfo{Tok: "Replace"}, }, }, - info: ProviderInfo{ - XSkipDetailedDiffForChanges: skipDetailedDiffForChanges, - }, } } t.Run("Diff", func(t *testing.T) { @@ -2432,7 +2429,7 @@ func TestMaxItemOneWrongStateDiff(t *testing.T) { "urn": "urn:pulumi:dev::teststack::NestedStrRes::exres", "id": "0", "olds": { - "nested_str": [] + "nested_str": [""] }, "news": { "nested_str": "" @@ -2444,6 +2441,32 @@ func TestMaxItemOneWrongStateDiff(t *testing.T) { } }`) }) + t.Run("DiffNilListAndVal", 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, + "detailedDiff": { + "nested_str": { + "kind": "UPDATE" + } + }, + "diffs": ["nested_str"] + } + }`) + }) t.Run("DiffListAndValNonEmpty", func(t *testing.T) { testutils.Replay(t, provider, ` {