Accurate bridge previews trigger unnecessary replacements when adding and removing the same number of elements in a collection

[VenelinMartinov]

What happened?

Under accurate bridge previews we now trigger replacements when we shouldn't for changes where the same number of elements was added and removed from a collection.

Example

For a set property with ForceNew on the set itself if we transition from

[ 
  {nested: val2},
  {nested: val3},
  {nested: val1},
]

to

[ 
  {nested: val2},
  {nested: val4},
  {nested: val1},
]

we get a replacement which is not triggered in TF:

Pulumi:

    +-crossprovider:index/testRes:TestRes: (replace)
        [id=newid]
        [urn=urn:pulumi:test::project::crossprovider:index/testRes:TestRes::example]
      ~ tests: [
          + [1]: {
                  + nested    : "val4"
                }
          - [2]: {
                  - nested: "val3"
                }
        ]

versus TF:

  ~ resource "crossprovider_test_res" "example" {
        id = "newid"

      - test {
          - nested = "val3" -> null
        }
      + test {
          + nested = "val4"
        }

        # (2 unchanged blocks hidden)
    }

We should instead correctly mark only the nested property for update and no trigger a replacement on the resource.

See TestDetailedDiffSet/block_top_level_force_new/same_element_updated_unordered

Output of pulumi about

.

Additional context

No response

Contributing

Vote on this issue by adding a 👍 reaction.
To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

[t0yv0]

Can you shed some more light here the issue as described is very confusing.

1. if a set is ForceNew and changing [val2, val3, val1] to [val2, val4, val1] should this not be an actual replacement plan for the resource? In TF as well? Is this a TF bug or am I missing something?

2. the listings you added for TF and Pulumi do not seem to indicate that a resource replacement is being planned

3. "Accurate bridge previews trigger unnecessary replacements when adding and removing the same number of elements in a collection" makes it sound like changing detailed diff machinery impacts the foundational planning semantics which I thought we tried to remove by design?

[t0yv0]

Can you also add the expected behavior for Pulumi. Thanks.

[VenelinMartinov]

Thank you for the comments. I've updated the description to clarify. The pulumi preview does indicate a replacement with the +- next to the resource.

The point about pulumi detailed diff being purely presentational is very good. I have #2674 for this and I've expanded that to include both directions (Detailed diff marks a non-existent replacement and Detailed diff does not correctly detect a replacement).

[t0yv0]

This highly suspect and I'd like to understand the repro in pure Terraform. This is indicative of a Terraform bug but would be highly surprising. I'm poking around to try finding a few resources in AWS to see if I can reproduce at that level. For sets of strings this seems to actually generate replaces in AWS so this may be still scoped to sets of objects.

[t0yv0]

    shim_test.go:53: ForceNew set on "aws_emr_managed_scaling_policy"."compute_limits"
    shim_test.go:53: ForceNew set on "aws_instance"."ephemeral_block_device"
    shim_test.go:53: ForceNew set on "aws_backup_selection"."selection_tag"
    shim_test.go:53: ForceNew set on "aws_backup_selection"."condition"
    shim_test.go:53: ForceNew set on "aws_cleanrooms_collaboration"."member"
    shim_test.go:53: ForceNew set on "aws_ec2_client_vpn_endpoint"."authentication_options"
    shim_test.go:53: ForceNew set on "aws_imagebuilder_image"."workflow"
    shim_test.go:53: ForceNew set on "aws_dynamodb_table"."local_secondary_index"
    shim_test.go:53: ForceNew set on "aws_imagebuilder_image_recipe"."block_device_mapping"
    shim_test.go:53: ForceNew set on "aws_emr_instance_group"."ebs_config"
    shim_test.go:53: ForceNew set on "aws_emr_instance_fleet"."instance_type_configs"
    shim_test.go:53: ForceNew set on "aws_fsx_file_cache"."data_repository_association"
    shim_test.go:53: ForceNew set on "aws_kms_grant"."constraints"
    shim_test.go:53: ForceNew set on "aws_launch_configuration"."ephemeral_block_device"
    shim_test.go:53: ForceNew set on "aws_autoscaling_group"."initial_lifecycle_hook"
    shim_test.go:53: ForceNew set on "aws_spot_fleet_request"."launch_specification"
    shim_test.go:53: ForceNew set on "aws_spot_fleet_request"."launch_template_config"
    shim_test.go:53: ForceNew set on "aws_lightsail_instance_public_ports"."port_info"
    shim_test.go:53: ForceNew set on "aws_codecommit_trigger"."trigger"
    shim_test.go:53: ForceNew set on "aws_ses_event_destination"."cloudwatch_destination"
    shim_test.go:53: ForceNew set on "aws_ami_copy"."ephemeral_block_device"
    shim_test.go:53: ForceNew set on "aws_ecs_task_definition"."volume"
    shim_test.go:53: ForceNew set on "aws_ecs_task_definition"."inference_accelerator"
    shim_test.go:53: ForceNew set on "aws_ecs_task_definition"."placement_constraints"
    shim_test.go:53: ForceNew set on "aws_ami_from_instance"."ephemeral_block_device"
    shim_test.go:53: ForceNew set on "aws_appstream_image_builder"."access_endpoint"
    shim_test.go:53: ForceNew set on "aws_ecs_task_set"."load_balancer"
    shim_test.go:53: ForceNew set on "aws_ecs_task_set"."capacity_provider_strategy"
    shim_test.go:53: ForceNew set on "aws_lightsail_container_service_deployment_version"."container"
    shim_test.go:53: ForceNew set on "aws_rolesanywhere_trust_anchor"."notification_settings"
    shim_test.go:53: ForceNew set on "aws_elastictranscoder_preset"."video_watermarks"
    shim_test.go:53: ForceNew set on "aws_opsworks_instance"."ebs_block_device"
    shim_test.go:53: ForceNew set on "aws_opsworks_instance"."root_block_device"
    shim_test.go:53: ForceNew set on "aws_opsworks_instance"."ephemeral_block_device"
    shim_test.go:53: ForceNew set on "aws_acm_certificate"."validation_option"
    shim_test.go:53: ForceNew set on "aws_lakeformation_resource_lf_tags"."lf_tag"
    shim_test.go:53: ForceNew set on "aws_mskconnect_connector"."plugin"
    shim_test.go:53: ForceNew set on "aws_globalaccelerator_custom_routing_endpoint_group"."destination_configuration"
    shim_test.go:53: ForceNew set on "aws_globalaccelerator_custom_routing_endpoint_group"."endpoint_configuration"
    shim_test.go:53: ForceNew set on "aws_spot_instance_request"."ebs_block_device"
    shim_test.go:53: ForceNew set on "aws_spot_instance_request"."ephemeral_block_device"
    shim_test.go:53: ForceNew set on "aws_spot_instance_request"."network_interface"
    shim_test.go:53: ForceNew set on "aws_eks_fargate_profile"."selector"
    shim_test.go:53: ForceNew set on "aws_lb_ssl_negotiation_policy"."attribute"
    shim_test.go:53: ForceNew set on "aws_ami"."ephemeral_block_device"

[t0yv0]

Was able to reproduce with this snippet:

data "aws_ami" "ubuntu" {
  most_recent = true

  filter {
    name   = "name"
    values = ["ubuntu/images/hvm-ssd/ubuntu-jammy-22.04-amd64-server-*"]
  }

  filter {
    name   = "virtualization-type"
    values = ["hvm"]
  }

  owners = ["099720109477"] # Canonical
}


resource "aws_instance" "example" {
  ami           = data.aws_ami.ubuntu.id
  instance_type = "t2.micro"

  root_block_device {
    volume_size = 8
  }

  ephemeral_block_device {
    virtual_name = "ephemeral0"
    device_name  = "/dev/sde"
  }

  ephemeral_block_device {
    virtual_name = "ephemeral3" # tweak this
    device_name  = "/dev/sdf"
  }
}

Getting:

  # aws_instance.example will be updated in-place
  ~ resource "aws_instance" "example" {
        id                                   = "i-01645cafa82f6cd58"
        tags                                 = {}
        # (39 unchanged attributes hidden)

      - ephemeral_block_device {
          - device_name  = "/dev/sde" -> null
          - no_device    = false -> null
          - virtual_name = "ephemeral0" -> null
        }
      - ephemeral_block_device {
          - device_name  = "/dev/sdf" -> null
          - no_device    = false -> null
          - virtual_name = "ephemeral1" -> null
        }
      + ephemeral_block_device {
          + device_name  = "/dev/sde"
          + virtual_name = "ephemeral0"
        }
      + ephemeral_block_device {
          + device_name  = "/dev/sdf"
          + virtual_name = "ephemeral3"
        }

        # (8 unchanged blocks hidden)
    }

provider "registry.terraform.io/hashicorp/aws" {
  version = "5.81.0"

Terraform v1.9.8

[VenelinMartinov]

I think it could be argued either way here - the counter-argument is that in this case, the collection was not modified but only the element property was changed.

[VenelinMartinov]

So semantically, setting ForceNew on the collection means that we'd like to replace the resource in case the number of elements in the collection changes.

Modifying a property on an element in the collection does not "change" the collection.

LMK what you think.

[t0yv0]

I've found hashicorp/terraform-plugin-sdk#1314 but hashicorp/terraform#34691 is aged out.

[VenelinMartinov]

1314 seems more related to the schema validation which we had to work around in #2723 (comment)

[t0yv0]

1314 is pertinent here because if TF picked a replace plan then 1314 wouldn't have been an issue it seems.

[t0yv0]

Let's keep this one around to collect cross-links to actual user issues. Seems to be a very odd behavior but indeed present in TF. Very interesting find.

[VenelinMartinov]

This one was meant as an issue for "accurate previews does not match TF", which is fixed in #2747

I can open a new one for "TF (and we) do not replace resources when a nested property is updated on a ForceNew collection"

[VenelinMartinov]

Opened #2753