diff --git a/pkg/tfbridge/diff_test.go b/pkg/tfbridge/diff_test.go index 2d1571108..b1bfabe00 100644 --- a/pkg/tfbridge/diff_test.go +++ b/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, }, }, diff --git a/pkg/tfbridge/info/info.go b/pkg/tfbridge/info/info.go index aeb8689ab..cd37bd0f2 100644 --- a/pkg/tfbridge/info/info.go +++ b/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. diff --git a/pkg/tfbridge/provider.go b/pkg/tfbridge/provider.go index 011e93efd..4ef8f5f06 100644 --- a/pkg/tfbridge/provider.go +++ b/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 diff --git a/pkg/tfshim/sdk-v1/instance_diff.go b/pkg/tfshim/sdk-v1/instance_diff.go index 90ab1a0d2..6c6d39c19 100644 --- a/pkg/tfshim/sdk-v1/instance_diff.go +++ b/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) diff --git a/pkg/tfshim/sdk-v2/instance_diff.go b/pkg/tfshim/sdk-v2/instance_diff.go index ec2c4db2d..a2ffe8350 100644 --- a/pkg/tfshim/sdk-v2/instance_diff.go +++ b/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 { diff --git a/pkg/tfshim/sdk-v2/provider2.go b/pkg/tfshim/sdk-v2/provider2.go index 77f8e1520..0e245f4a4 100644 --- a/pkg/tfshim/sdk-v2/provider2.go +++ b/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 diff --git a/pkg/tfshim/shim.go b/pkg/tfshim/shim.go index 8c6f0f650..b2a2632d1 100644 --- a/pkg/tfshim/shim.go +++ b/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 diff --git a/pkg/tfshim/tfplugin5/instance_diff.go b/pkg/tfshim/tfplugin5/instance_diff.go index 050b85172..c8e0d397c 100644 --- a/pkg/tfshim/tfplugin5/instance_diff.go +++ b/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))