terraform-provider-fastly icon indicating copy to clipboard operation
terraform-provider-fastly copied to clipboard

Diffs are hard to read

Open errm opened this issue 4 years ago • 6 comments

Terraform Version

Terraform v0.14.6
+ provider registry.terraform.io/fastly/fastly v0.24.0

Affected Resource(s)

  • fastly_service_v1

Expected Behavior

What should have happened?

When making a change to attributes in a backend block, or a few lines of a custom vcl I would like to see a diff that explained what changed.

e.g.

  # fastly_service_v1.example will be updated in-place
  ~ resource "fastly_service_v1" "example" {
        id             = "redacted"
        name           = "example.example.org"
        # (5 unchanged attributes hidden)

      ~ backend {
          ~ first_byte_timeout    = 21000 -> 1500
          # (n unchanged attributes hidden)
        }

        # (8 unchanged blocks hidden)
    }

Actual Behavior

What actually happened?

changing any attribute in a block shows it being replaced. This makes it much harder to review the changes being made to the service by looking at the diff.

The user experiance e.g. when looking at the diff in the fastly web UI is much nicer.


  # fastly_service_v1.global-api-test will be updated in-place
  ~ resource "fastly_service_v1" "example" {
        id             = "redacted"
        name           = "example.example.org"
        # (5 unchanged attributes hidden)

      + backend {
          + address               = "backend.example.org"
          + auto_loadbalance      = false
          + between_bytes_timeout = 10000
          + connect_timeout       = 1000
          + error_threshold       = 0
          + first_byte_timeout    = 1500
          + max_conn              = 200
          + name                  = "example-backend"
          + port                  = 443
        }
      - backend {
          - address               = "backend.example.org" -> null
          - auto_loadbalance      = false -> null
          - between_bytes_timeout = 10000 -> null
          - connect_timeout       = 1000 -> null
          - error_threshold       = 0 -> null
          - first_byte_timeout    = 21000 -> null
          - max_conn              = 200 -> null
          - name                  = "example-backend" -> null
          - port                  = 443 -> null
        }

        # (8 unchanged blocks hidden)
    }

I am not totally sure if this is technically possible, but I don't recall this kind of experiance e.g. when using the AWS provider.

Presumably if the blocks were keyed of their name it would be possible to diff the old and the new block in a more useful way than is currently output.

errm avatar Feb 11 '21 14:02 errm

I spent some time looking into this.

As far as I can work out this kind of behaviour is not currently possible with the current version of terraform.

Since nested blocks map directly to terraform's Set type there identity is based on a hash of all atributes on the object.


Some possible solutions to this could be:

  1. Using a map, would break backwards compat.

e.g:

resource "fastly_service_v1" "example" {
 
  ...

  backends = {
    example-backend = {
          address               = "backend.example.org"
          auto_loadbalance      = false
          between_bytes_timeout = 10000
          connect_timeout       = 1000
          error_threshold       = 0
          first_byte_timeout    = 1500
          max_conn              = 200
          name                  = "example-backend"
          port                  = 443
    },
    other-backend = {
          address               = "other.example.org"
          auto_loadbalance      = false
          between_bytes_timeout = 10000
          connect_timeout       = 1000
          error_threshold       = 0
          first_byte_timeout    = 1500
          max_conn              = 200
          name                  = "other-backend"
          port                  = 443
    } 
  }
}
  1. Get terraform to provide a way for nested blocks/sets to specify identity.

In the example of backends, really we just want to use a hash of the name for set identity.

I did manage to make some changes to the provider code to achieve this, but when that hash is equal terraform treats the plan as a noop, so there clearly would need to be some changes to terraform core to support this.

  1. parse terraform plans with a script and render a nicer diff...

For now I am using terraform show -json to spit out the plan then using a script to print a nicer diff, it also lets me shell out to diff in order to provide a nice linewise diff of changes to VCL!

errm avatar Feb 26 '21 13:02 errm

Here's a script that can be used to nicely format plan diffs (adapted from https://gist.github.com/Integralist/2b4298d4d287376b8a939c4e9eadd693)

#!/bin/bash
terraform plan -out=tfplan > /dev/null 2>&1
CHANGES=$(terraform show -json tfplan | jq '.resource_changes[].change')
diff -u <(echo "$CHANGES" | jq '.before' | sed 's/\\n/\n/g') <(echo "$CHANGES" | jq '.after' | sed 's/\\n/\n/g')

MasonM avatar Apr 11 '22 18:04 MasonM

I have the same problem. Is there a plan to improve it?

a-suenami avatar Sep 21 '22 03:09 a-suenami

👋🏻 @a-suenami there is a new Hashicorp plugin framework (incompatible with their v2 SDK that the Fastly provider uses) and in the new framework HashiCorp recommend moving away from nested blocks to nested attributes. This would be a breaking interface change for the Fastly provider but it might also be something that helps to improve the diff issue reported here (related: https://github.com/fastly/terraform-provider-fastly/issues/597). Additionally, there is a new 'map' data type with the new framework that might help the diff issue.

Integralist avatar Jan 16 '23 15:01 Integralist

UPDATE: The new HashiCorp framework's map nested attribute type does indeed help with diff clarity.

See https://github.com/Integralist/terraform-provider-fastly-framework/pull/7 for details.

Integralist avatar Jan 26 '23 14:01 Integralist

Hello, is there any update related to that?

We are fine with implementing that as a breaking change. If you provide steps how to migrate or we can start re-create our resources.

pavelpulec avatar Apr 17 '24 14:04 pavelpulec