Fix PF dynamic type diff error (#3098)

In #2945 we
accidentally changed the behaviour for providers where RawStateDeltas
were not enabled. After 2945 we would unconditionally produce the JSON
serialization of the state even if it isn't used.

It is only used for upgrading state and for RawStateDeltas. The
unintended effect here is that we exposed
#3078 to all
users of attributes of DynamicType even if no upgrades or RawStateDeltas
are used.

This PR changes that to not produce the JSON serialization if not
necessary. I've also added some tests for both the behaviour fixed here
and the behaviour which needs
#3078

fixes #3095

Author: VenelinMartinov

pkg/pf/tests/diff_test/diff_test.go

@@ -1,12 +1,17 @@
 package tfbridgetests
 
 import (
+	"context"
+	"math/big"
+	"os"
 	"testing"
 
+	"github.com/hashicorp/terraform-plugin-framework/resource"
 	rschema "github.com/hashicorp/terraform-plugin-framework/resource/schema"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier"
 	"github.com/hexops/autogold/v2"
+	"github.com/pulumi/pulumi/sdk/v3/go/common/util/cmdutil"
 	"github.com/zclconf/go-cty/cty"
 
 	pb "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/pf/internal/providerbuilder"
@@ -176,3 +181,111 @@ func TestPFDetailedDiffStringAttribute(t *testing.T) {
 		})
 	}
 }
+
+func TestPFDetailedDiffDynamicType(t *testing.T) {
+	t.Parallel()
+	if d, ok := os.LookupEnv("PULUMI_RAW_STATE_DELTA_ENABLED"); ok && cmdutil.IsTruthy(d) {
+		// TODO[pulumi/pulumi-terraform-bridge#3078]
+		t.Skip("Does not work with PULUMI_RAW_STATE_DELTA_ENABLED=true")
+	}
+
+	attributeSchema := rschema.Schema{
+		Attributes: map[string]rschema.Attribute{
+			"key": rschema.DynamicAttribute{
+				Optional: true,
+			},
+		},
+	}
+	res := pb.NewResource(pb.NewResourceArgs{
+		ResourceSchema: attributeSchema,
+	})
+
+	t.Run("no change", func(t *testing.T) {
+		crosstests.Diff(t, res,
+			map[string]cty.Value{"key": cty.StringVal("value")},
+			map[string]cty.Value{"key": cty.StringVal("value")},
+		)
+	})
+
+	t.Run("change", func(t *testing.T) {
+		crosstests.Diff(t, res,
+			map[string]cty.Value{"key": cty.StringVal("value")},
+			map[string]cty.Value{"key": cty.StringVal("value1")},
+		)
+	})
+
+	t.Run("int no change", func(t *testing.T) {
+		crosstests.Diff(t, res,
+			map[string]cty.Value{"key": cty.NumberVal(big.NewFloat(1))},
+			map[string]cty.Value{"key": cty.NumberVal(big.NewFloat(1))},
+		)
+	})
+
+	t.Run("type change", func(t *testing.T) {
+		// TODO[pulumi/pulumi-terraform-bridge#3078]
+		t.Skip(`Error converting tftypes.Number<"1"> (value2) at "AttributeName(\"key\")": can't unmarshal tftypes.Number into *string, expected string`)
+		crosstests.Diff(t, res,
+			map[string]cty.Value{"key": cty.StringVal("value")},
+			map[string]cty.Value{"key": cty.NumberVal(big.NewFloat(1))},
+		)
+	})
+}
+
+func TestPFDetailedDiffDynamicTypeWithMigration(t *testing.T) {
+	t.Parallel()
+	// TODO[pulumi/pulumi-terraform-bridge#3078]
+	t.Skip("DynamicPseudoType is not supported")
+
+	attributeSchema := rschema.Schema{
+		Attributes: map[string]rschema.Attribute{
+			"key": rschema.DynamicAttribute{
+				Optional: true,
+			},
+		},
+	}
+	res1 := pb.NewResource(pb.NewResourceArgs{
+		ResourceSchema: attributeSchema,
+	})
+
+	schema2 := rschema.Schema{
+		Attributes: map[string]rschema.Attribute{
+			"key": rschema.DynamicAttribute{
+				Optional: true,
+			},
+		},
+		Version: 1,
+	}
+	res2 := pb.NewResource(pb.NewResourceArgs{
+		ResourceSchema: schema2,
+		UpgradeStateFunc: func(ctx context.Context) map[int64]resource.StateUpgrader {
+			return map[int64]resource.StateUpgrader{
+				0: {
+					PriorSchema: &res1.ResourceSchema,
+					StateUpgrader: func(ctx context.Context, usr1 resource.UpgradeStateRequest, usr2 *resource.UpgradeStateResponse) {
+						usr2.State = *usr1.State
+					},
+				},
+			}
+		},
+	})
+
+	t.Run("no change", func(t *testing.T) {
+		crosstests.Diff(t, res1,
+			map[string]cty.Value{"key": cty.StringVal("value")},
+			map[string]cty.Value{"key": cty.StringVal("value")},
+			crosstests.DiffProviderUpgradedSchema(
+				res2,
+			),
+		)
+	})
+
+	t.Run("change", func(t *testing.T) {
+		crosstests.Diff(t, res1,
+			map[string]cty.Value{"key": cty.StringVal("value")},
+			map[string]cty.Value{"key": cty.StringVal("value1")},
+			crosstests.DiffProviderUpgradedSchema(
+				res2,
+			),
+		)
+	})
+}

pkg/pf/tfbridge/resource_state.go

@@ -251,15 +251,10 @@ func (p *provider) parseAndUpgradeResourceState(
 	}
 
 	// Otherwise fallback to imprecise legacy parsing.
-	tfType := rh.schema.Type(ctx).(tftypes.Object)
 	value, err := convert.EncodePropertyMap(rh.encoder, props)
 	if err != nil {
 		return nil, fmt.Errorf("[pf/tfbridge] Error calling EncodePropertyMap: %w", err)
 	}
-	rawState, err := pfutils.NewRawState(tfType, value)
-	if err != nil {
-		return nil, fmt.Errorf("[pf/tfbridge] Error calling NewRawState: %w", err)
-	}
 
 	// Before EnableRawStateDelta rollout, the behavior used to be to skip the upgrade method in case of an exact
 	// version match. This seems incorrect, but to derisk fixing this problem it is flagged together with
@@ -272,6 +267,12 @@ func (p *provider) parseAndUpgradeResourceState(
 		}, nil
 	}
 
+	tfType := rh.schema.Type(ctx).(tftypes.Object)
+	rawState, err := pfutils.NewRawState(tfType, value)
+	if err != nil {
+		return nil, fmt.Errorf("[pf/tfbridge] Error calling NewRawState: %w", err)
+	}
+
 	return p.upgradeResourceState(ctx, rh, rawState, parsedMeta.PrivateState, stateVersion)
 }