diff --git a/pkg/tfbridge/provider.go b/pkg/tfbridge/provider.go index 5144271e3..3922b1b6c 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 p.info.XSkipDetailedDiffForChanges && len(diff.Attributes()) > 0 { + if p.info.XSkipDetailedDiffForChanges && !diff.HasNoChanges() { 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/tests/provider_test.go b/pkg/tfbridge/tests/provider_test.go index c7296ed85..02c1cf0a1 100644 --- a/pkg/tfbridge/tests/provider_test.go +++ b/pkg/tfbridge/tests/provider_test.go @@ -3,6 +3,7 @@ package tfbridgetests import ( "context" "encoding/json" + "fmt" "io" "testing" @@ -68,6 +69,136 @@ func TestWithNewTestProvider(t *testing.T) { `) } +func TestReproMinimalDiffCycle(t *testing.T) { + customResponseSchema := func() *schema.Schema { + return &schema.Schema{ + Type: schema.TypeList, + Optional: true, + MaxItems: 1, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "custom_response_body_key": { + Type: schema.TypeString, + Optional: true, + }, + }, + }, + } + } + blockConfigSchema := func() *schema.Schema { + return &schema.Schema{ + Type: schema.TypeList, + Optional: true, + MaxItems: 1, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "custom_response": customResponseSchema(), + }, + }, + } + } + ruleElement := &schema.Resource{ + Schema: map[string]*schema.Schema{ + "action": { + Type: schema.TypeList, + Optional: true, + MaxItems: 1, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "block": blockConfigSchema(), + }, + }, + }, + }, + } + + resource := &schema.Resource{ + Schema: map[string]*schema.Schema{ + "rule": { + Type: schema.TypeSet, + Optional: true, + Elem: ruleElement, + }, + }, + } + + // Here i may receive maps or slices over base types and *schema.Set which is not friendly to diffing. + resource.Schema["rule"].Set = func(i interface{}) int { + actual := schema.HashResource(resource.Schema["rule"].Elem.(*schema.Resource))(i) + fmt.Printf("hashing %#v as %d\n", i, actual) + return actual + } + ctx := context.Background() + p := newTestProvider(ctx, tfbridge.ProviderInfo{ + P: shimv2.NewProvider(&schema.Provider{ + Schema: map[string]*schema.Schema{}, + ResourcesMap: map[string]*schema.Resource{ + "example_resource": resource, + }, + }, shimv2.WithPlanResourceChange(func(tfResourceType string) bool { + return true + })), + Name: "testprov", + ResourcePrefix: "example", + Resources: map[string]*tfbridge.ResourceInfo{ + "example_resource": {Tok: "testprov:index:ExampleResource"}, + }, + }, newTestProviderOptions{}) + + replay.Replay(t, p, ` + { + "method": "/pulumirpc.ResourceProvider/Diff", + "request": { + "id": "newid", + "urn": "urn:pulumi:test::project::testprov:index:ExampleResource::example", + "olds": { + "id": "newid", + "rules": [ + { + "action": { + "block": { + "customResponse": null + } + } + } + ] + }, + "news": { + "__defaults": [], + "rules": [ + { + "__defaults": [], + "action": { + "__defaults": [], + "block": { + "__defaults": [] + } + } + } + ] + }, + "oldInputs": { + "__defaults": [], + "rules": [ + { + "__defaults": [], + "action": { + "__defaults": [], + "block": { + "__defaults": [] + } + } + } + ] + } + }, + "response": { + "changes": "DIFF_NONE", + "hasDetailedDiff": true + } + }`) +} + func nilSink() diag.Sink { nilSink := diag.DefaultSink(io.Discard, io.Discard, diag.FormatOptions{ Color: colors.Never, diff --git a/pkg/tfshim/sdk-v1/instance_diff.go b/pkg/tfshim/sdk-v1/instance_diff.go index d347a1dc7..2badd8c27 100644 --- a/pkg/tfshim/sdk-v1/instance_diff.go +++ b/pkg/tfshim/sdk-v1/instance_diff.go @@ -57,6 +57,10 @@ func (d v1InstanceDiff) Attribute(key string) *shim.ResourceAttrDiff { return resourceAttrDiffToShim(d.tf.Attributes[key]) } +func (d v1InstanceDiff) HasNoChanges() bool { + return len(d.Attributes()) == 0 +} + func (d v1InstanceDiff) Attributes() map[string]shim.ResourceAttrDiff { m := map[string]shim.ResourceAttrDiff{} for k, v := range d.tf.Attributes { diff --git a/pkg/tfshim/sdk-v2/instance_diff.go b/pkg/tfshim/sdk-v2/instance_diff.go index aaac5c00f..c12ad306a 100644 --- a/pkg/tfshim/sdk-v2/instance_diff.go +++ b/pkg/tfshim/sdk-v2/instance_diff.go @@ -46,6 +46,10 @@ func (d v2InstanceDiff) Attribute(key string) *shim.ResourceAttrDiff { return resourceAttrDiffToShim(d.tf.Attributes[key]) } +func (d v2InstanceDiff) HasNoChanges() bool { + return len(d.Attributes()) == 0 +} + func (d v2InstanceDiff) Attributes() map[string]shim.ResourceAttrDiff { m := map[string]shim.ResourceAttrDiff{} for k, v := range d.tf.Attributes { diff --git a/pkg/tfshim/sdk-v2/provider2.go b/pkg/tfshim/sdk-v2/provider2.go index e4428670e..627eeb8bc 100644 --- a/pkg/tfshim/sdk-v2/provider2.go +++ b/pkg/tfshim/sdk-v2/provider2.go @@ -444,6 +444,17 @@ func (s *grpcServer) PlanResourceChange( if err != nil { return nil, err } + + // There are cases where planned state is equal to the original state, but InstanceDiff still displays changes. + // Pulumi considers this to be a no-change diff, and as a workaround here any InstanceDiff changes are deleted + // and ignored (simlar to processIgnoreChanges). + // + // See pulumi/pulumi-aws#3880 + same := plannedState.Equals(priorState) + if same.IsKnown() && same.True() { + resp.InstanceDiff.Attributes = map[string]*terraform.ResourceAttrDiff{} + } + var meta map[string]interface{} if resp.PlannedPrivate != nil { if err := json.Unmarshal(resp.PlannedPrivate, &meta); err != nil { diff --git a/pkg/tfshim/shim.go b/pkg/tfshim/shim.go index 38b7175b5..23b3f4161 100644 --- a/pkg/tfshim/shim.go +++ b/pkg/tfshim/shim.go @@ -43,7 +43,7 @@ type ResourceAttrDiff struct { type InstanceDiff interface { Attribute(key string) *ResourceAttrDiff - Attributes() map[string]ResourceAttrDiff + HasNoChanges() bool ProposedState(res Resource, priorState InstanceState) (InstanceState, error) Destroy() bool RequiresNew() bool diff --git a/pkg/tfshim/tfplugin5/instance_diff.go b/pkg/tfshim/tfplugin5/instance_diff.go index 75f6d206c..c8848f6cb 100644 --- a/pkg/tfshim/tfplugin5/instance_diff.go +++ b/pkg/tfshim/tfplugin5/instance_diff.go @@ -63,6 +63,10 @@ func (d *instanceDiff) Attributes() map[string]shim.ResourceAttrDiff { return d.attributes } +func (d *instanceDiff) HasNoChanges() bool { + return len(d.attributes) == 0 +} + func (d *instanceDiff) ProposedState(res shim.Resource, priorState shim.InstanceState) (shim.InstanceState, error) { plannedObject, err := ctyToGo(d.planned) if err != nil { diff --git a/pkg/tfshim/tfplugin5/provider_test.go b/pkg/tfshim/tfplugin5/provider_test.go index 06adfc438..0865bf506 100644 --- a/pkg/tfshim/tfplugin5/provider_test.go +++ b/pkg/tfshim/tfplugin5/provider_test.go @@ -1180,7 +1180,7 @@ func TestApply(t *testing.T) { diff, err := p.Diff(ctx, "example_resource", state, config, shim.DiffOptions{}) require.NoError(t, err) - if len(diff.Attributes()) == 0 { + if diff.HasNoChanges() { return }