Separate diff decision from presentation (#2379)

Adds a `DiffEqualDecisionOverride` to the shim layer which allows
provider implementations to specify a diff decision instead of relying
on the shim layer to do that.

Also adds a `DiffEqualDecisionOverride` to the PRC sdkv2 implementation
which opentofu does.

Also adds a feature flag `EnableAccurateBridgePreview` which guards this
feature.

will fix once rolled out:
#2293
will fix once rolled out:
#1501
will undo once rolled out:
#1502 as the
underlying issue was fixed during the PRC work.
Stacked on #2380

Author: VenelinMartinov

pkg/tfbridge/diff_test.go

@@ -2077,7 +2077,7 @@ func TestListNestedAddMaxItemsOne(t *testing.T) {
 }
 
 type diffTestCase struct {
-	resourceSchema      map[string]*schema.Schema
+	resourceSchema      map[string]*v2Schema.Schema
 	resourceFields      map[string]*SchemaInfo
 	state               resource.PropertyMap
 	inputs              resource.PropertyMap
@@ -2088,11 +2088,11 @@ type diffTestCase struct {
 
 func diffTest2(t *testing.T, tc diffTestCase) {
 	ctx := context.Background()
-	res := &schema.Resource{
+	res := &v2Schema.Resource{
 		Schema: tc.resourceSchema,
 	}
-	provider := shimv1.NewProvider(&schema.Provider{
-		ResourcesMap: map[string]*schema.Resource{
+	provider := shimv2.NewProvider(&v2Schema.Provider{
+		ResourcesMap: map[string]*v2Schema.Resource{
 			"p_resource": res,
 		},
 	})
@@ -2130,21 +2130,21 @@ func diffTest2(t *testing.T, tc diffTestCase) {
 }
 
 func TestChangingMaxItems1FilterProperty(t *testing.T) {
-	schema := map[string]*schema.Schema{
+	schema := map[string]*v2Schema.Schema{
 		"rule": {
-			Type:     schema.TypeList,
+			Type:     v2Schema.TypeList,
 			Required: true,
 			MaxItems: 1000,
-			Elem: &schema.Resource{
-				Schema: map[string]*schema.Schema{
+			Elem: &v2Schema.Resource{
+				Schema: map[string]*v2Schema.Schema{
 					"filter": {
-						Type:     schema.TypeList,
+						Type:     v2Schema.TypeList,
 						Optional: true,
 						MaxItems: 1,
-						Elem: &schema.Resource{
-							Schema: map[string]*schema.Schema{
+						Elem: &v2Schema.Resource{
+							Schema: map[string]*v2Schema.Schema{
 								"prefix": {
-									Type:     schema.TypeString,
+									Type:     v2Schema.TypeString,
 									Optional: true,
 								},
 							},

pkg/tfbridge/info/info.go

@@ -159,6 +159,9 @@ type Provider struct {
 	// EnableZeroDefaultSchemaVersion makes the provider default
 	// to version 0 when no version is specified in the state of a resource.
 	EnableZeroDefaultSchemaVersion bool
+	// EnableAccurateBridgePreview makes the bridge use an experimental feature
+	// to generate more accurate diffs and previews for resources
+	EnableAccurateBridgePreview bool
 }
 
 // HclExampler represents a supplemental HCL example for a given resource or function.

pkg/tfbridge/provider.go

@@ -56,7 +56,8 @@ import (
 )
 
 type providerOptions struct {
-	defaultZeroSchemaVersion bool
+	defaultZeroSchemaVersion    bool
+	enableAccurateBridgePreview bool
 }
 
 type providerOption func(providerOptions) (providerOptions, error)
@@ -68,6 +69,13 @@ func WithDefaultZeroSchemaVersion() providerOption { //nolint:revive
 	}
 }
 
+func withAccurateBridgePreview() providerOption {
+	return func(opts providerOptions) (providerOptions, error) {
+		opts.enableAccurateBridgePreview = true
+		return opts, nil
+	}
+}
+
 func getProviderOptions(opts []providerOption) (providerOptions, error) {
 	res := providerOptions{}
 	for _, o := range opts {
@@ -268,6 +276,11 @@ func newProvider(ctx context.Context, host *provider.HostClient,
 	if info.EnableZeroDefaultSchemaVersion {
 		opts = append(opts, WithDefaultZeroSchemaVersion())
 	}
+
+	if info.EnableAccurateBridgePreview || cmdutil.IsTruthy(os.Getenv("PULUMI_TF_BRIDGE_ACCURATE_BRIDGE_PREVIEW")) {
+		opts = append(opts, withAccurateBridgePreview())
+	}
+
 	p := &Provider{
 		host:          host,
 		module:        module,
@@ -1148,15 +1161,28 @@ func (p *Provider) Diff(ctx context.Context, req *pulumirpc.DiffRequest) (*pulum
 	dd := makeDetailedDiffExtra(ctx, schema, fields, olds, news, diff)
 	detailedDiff, changes := dd.diffs, dd.changes
 
-	// There are some providers/situations which `makeDetailedDiff` distorts the expected changes, leading
-	// to changes being dropped by Pulumi.
-	// Until we fix `makeDetailedDiff`, it is safer to refer to the Terraform Diff attribute length for setting
-	// the DiffResponse.
-	// We will still use `detailedDiff` for diff display purposes.
+	if opts.enableAccurateBridgePreview {
+		if decision := diff.DiffEqualDecisionOverride(); decision != shim.DiffNoOverride {
+			if decision == shim.DiffOverrideNoUpdate {
+				changes = pulumirpc.DiffResponse_DIFF_NONE
+			} else {
+				changes = pulumirpc.DiffResponse_DIFF_SOME
+			}
+		}
+	} else {
+		// There are some providers/situations which `makeDetailedDiff` distorts the expected changes, leading
+		// to changes being dropped by Pulumi.
+		// Until we fix `makeDetailedDiff`, it is safer to refer to the Terraform Diff attribute length for setting
+		// the DiffResponse.
+		// We will still use `detailedDiff` for diff display purposes.
+
+		// See also https://github.com/pulumi/pulumi-terraform-bridge/issues/1501.
+		if !diff.HasNoChanges() {
+			changes = pulumirpc.DiffResponse_DIFF_SOME
+		}
+	}
 
-	// See also https://github.com/pulumi/pulumi-terraform-bridge/issues/1501.
-	if !diff.HasNoChanges() {
-		changes = pulumirpc.DiffResponse_DIFF_SOME
+	if 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 {
 			detailedDiff[path] = diff

pkg/tfshim/sdk-v1/instance_diff.go

@@ -43,6 +43,10 @@ type v1InstanceDiff struct {
 	tf *terraform.InstanceDiff
 }
 
+func (d v1InstanceDiff) DiffEqualDecisionOverride() shim.DiffOverride {
+	return shim.DiffNoOverride
+}
+
 func (d v1InstanceDiff) applyTimeoutOptions(opts shim.TimeoutOptions) {
 	if opts.ResourceTimeout != nil {
 		err := d.encodeTimeouts(opts.ResourceTimeout)

pkg/tfshim/sdk-v2/instance_diff.go

@@ -33,6 +33,10 @@ type v2InstanceDiff struct {
 	tf *terraform.InstanceDiff
 }
 
+func (d v2InstanceDiff) DiffEqualDecisionOverride() shim.DiffOverride {
+	return shim.DiffNoOverride
+}
+
 func (d v2InstanceDiff) applyTimeoutOptions(opts shim.TimeoutOptions) {
 	// This method is no longer used with PlanResourceChange; we handle timeouts more directly.
 	if opts.ResourceTimeout != nil {

pkg/tfshim/sdk-v2/provider2.go

@@ -112,9 +112,10 @@ func (s *v2InstanceState2) Meta() map[string]interface{} {
 type v2InstanceDiff2 struct {
 	v2InstanceDiff
 
-	config         cty.Value
-	plannedState   cty.Value
-	plannedPrivate map[string]interface{}
+	config                    cty.Value
+	plannedState              cty.Value
+	plannedPrivate            map[string]interface{}
+	diffEqualDecisionOverride shim.DiffOverride
 }
 
 func (d *v2InstanceDiff2) String() string {
@@ -146,6 +147,10 @@ func (d *v2InstanceDiff2) ProposedState(
 	}, nil
 }
 
+func (d *v2InstanceDiff2) DiffEqualDecisionOverride() shim.DiffOverride {
+	return d.diffEqualDecisionOverride
+}
+
 // Provides PlanResourceChange handling for select resources.
 type planResourceChangeImpl struct {
 	tf           *schema.Provider
@@ -269,13 +274,31 @@ func (p *planResourceChangeImpl) Diff(
 		TfToken:        t,
 		PlanState:      plan.PlannedState,
 	})
+
+	//nolint:lll
+	// Taken from https://github.com/opentofu/opentofu/blob/864aa9d1d629090cfc4ddf9fdd344d34dee9793e/internal/tofu/node_resource_abstract_instance.go#L1024
+	// We need to unmark the values to make sure Equals works.
+	// Equals will return unknown if either value is unknown.
+	// START
+	unmarkedPrior, _ := st.UnmarkDeep()
+	unmarkedPlan, _ := plannedState.UnmarkDeep()
+	eqV := unmarkedPrior.Equals(unmarkedPlan)
+	eq := eqV.IsKnown() && eqV.True()
+	// END
+
+	diffOverride := shim.DiffOverrideUpdate
+	if eq {
+		diffOverride = shim.DiffOverrideNoUpdate
+	}
+
 	return &v2InstanceDiff2{
 		v2InstanceDiff: v2InstanceDiff{
 			tf: plan.PlannedDiff,
 		},
-		config:         cfg,
-		plannedState:   plannedState,
-		plannedPrivate: plan.PlannedPrivate,
+		config:                    cfg,
+		plannedState:              plannedState,
+		diffEqualDecisionOverride: diffOverride,
+		plannedPrivate:            plan.PlannedPrivate,
 	}, err
 }
 
@@ -507,7 +530,8 @@ func (s *grpcServer) PlanResourceChange(
 	PlannedState   cty.Value
 	PlannedPrivate map[string]interface{}
 	PlannedDiff    *terraform.InstanceDiff
-}, error) {
+}, error,
+) {
 	configVal, err := msgpack.Marshal(config, ty)
 	if err != nil {
 		return nil, err

pkg/tfshim/shim.go

@@ -44,12 +44,25 @@ type ResourceAttrDiff struct {
 	Type        DiffAttrType
 }
 
+type DiffOverride string
+
+const (
+	DiffNoOverride       DiffOverride = "no-override"
+	DiffOverrideNoUpdate DiffOverride = "no-update"
+	DiffOverrideUpdate   DiffOverride = "update"
+)
+
 type InstanceDiff interface {
 	Attribute(key string) *ResourceAttrDiff
 	HasNoChanges() bool
 	ProposedState(res Resource, priorState InstanceState) (InstanceState, error)
 	Destroy() bool
 	RequiresNew() bool
+
+	// DiffEqualDecisionOverride can return a non-null value to override the default decision of if the diff is equal.
+	//
+	// DiffEqualDecisionOverride is only respected when EnableAccurateBridgePreview is set.
+	DiffEqualDecisionOverride() DiffOverride
 }
 
 type ValueType int

pkg/tfshim/tfplugin5/instance_diff.go

@@ -28,6 +28,10 @@ type instanceDiff struct {
 	attributes  map[string]shim.ResourceAttrDiff
 }
 
+func (d instanceDiff) DiffEqualDecisionOverride() shim.DiffOverride {
+	return shim.DiffNoOverride
+}
+
 func (d instanceDiff) applyTimeoutOptions(opts shim.TimeoutOptions) {
 	if opts.ResourceTimeout != nil {
 		err := d.encodeTimeouts(opts.ResourceTimeout)
@@ -39,8 +43,8 @@ func (d instanceDiff) applyTimeoutOptions(opts shim.TimeoutOptions) {
 }
 
 func newInstanceDiff(config, prior, planned cty.Value, meta map[string]interface{},
-	requiresReplace []*proto.AttributePath) *instanceDiff {
-
+	requiresReplace []*proto.AttributePath,
+) *instanceDiff {
 	attributes, requiresNew := computeDiff(prior, planned, requiresReplace)
 	return &instanceDiff{
 		config:      config,
@@ -216,8 +220,8 @@ func rangeValue(val cty.Value, each func(k, v cty.Value)) {
 }
 
 func computeDiff(prior, planned cty.Value,
-	requiresReplace []*proto.AttributePath) (map[string]shim.ResourceAttrDiff, bool) {
-
+	requiresReplace []*proto.AttributePath,
+) (map[string]shim.ResourceAttrDiff, bool) {
 	requiresNew := stringSet{}
 	for _, path := range requiresReplace {
 		requiresNew.add(pathString(path))