Terraform-provider-digitalocean: "list" attributes order problem

Created on 17 Apr 2020  路  8Comments  路  Source: digitalocean/terraform-provider-digitalocean

Hi,
I've identified some issue in terraform & digitalocean, that makes terraform plan always try to update some resource (seend with load balancers, but the issue probably happens in many other resources), because of the ordering of the elements in the lists

Terraform Version

Terraform v0.12.24

  • provider.digitalocean v1.16.0

Affected Resource(s)

digitalocean_loadbalancer, but probably others

Terraform Configuration Files

variable "do_token" {}

provider "digitalocean" {
    token = var.do_token
}

resource "digitalocean_loadbalancer" "public" {
  name   = "loadbalancer-1"
  region = "ams3"

  forwarding_rule {
          certificate_id = "b178c4bd-709f-4340-b363-062c82da9ff8"
        entry_port     = 443
            target_protocol = "http"
        entry_protocol = "https"
        target_port     = 80
      }

  forwarding_rule {
        entry_port     = 80
            target_protocol = "http"
        entry_protocol = "http"
        target_port     = 80
      }
}


Debug Output

https://gist.github.com/rienafairefr/339ba31e7e30d228263c22872e1b8036

terraform apply always want to reverse the order of the elements, aparently can't change it (either the tf provider orders the element before sending to DO, or DO API calls does not change the order, or DO API calls when querying the resource orders the list)

Expected Behavior

The resource shouldn't be marked alays as "to change", and the ordering of the list should either be inconsequential (order before send to DO, order what we get from DO), or the DO API should respect the order of the list.

Actual Behavior

The resource is always marked as changed

Steps to Reproduce

  1. terraform apply
  2. terraform apply

References

originally seen in pulumi https://github.com/pulumi/pulumi-digitalocean/issues/85

All 8 comments

The forwarding_rule attribue is defined as type schema.TypeList but probably should be schema.TypeSet instead so that the order does not matter. https://github.com/terraform-providers/terraform-provider-digitalocean/blob/bab8ab87051ade5229085ae4a64da81836e7794e/digitalocean/resource_digitalocean_loadbalancer.go#L58

Yep, I concur, probably should be a set. Based on git blame & the PR in terraform here that introduced the digital ocean load_balancer resource, I guess I can mention @stack72 and @catsby

Hello!

Based on git blame & the PR in terraform here

The mentioned PR is over 3 years old so the results my vary 馃槃 Unfortunately I'm not involved in this project anymore however I agree that TypeSet likely makes sense here, unless there's a defined sort ordering to the rules that could be applied. Last I knew changing from TypeList to TypeSet requires a state migration:

The commit history shows @andrewsomething is pretty active here, hopefully they can help!

Thanks @catsby,

I had some exchange with DigitalOcean support team, and the take home message is that the ordering of the array in the POSTed JSON is not always preserved indeed. The JSON is only an intermediate representation. The internal state of the resource stays in the same order, which is the order returned through the GET endpoint, however. Some of the list attributes have been identified as sensitive to ordering (like the user_data in a droplet creation), and this order is preserved from the POSTed JSON.

So, IMO, there should be a migration from List semantics to Set semantics on the terraform side for the attributes where order does not matter (like the forwarding_rules case on which I first stumbled), and keep List where order does matter and the API keeps the ordering consistent. Never modified terraform's internals, and maybe the state migration above is a dealbreaker for people, backwards compatibility, IDK

Here is the full message, in case some details got lost in my summary:

"
The data is returned based on the internal state of the object being queried and the service
endpoint handler that interfaces with the objects. Internally, there is no array order storage, as the
data posted ultimately defines the desired state of an object which is translated to the internal object
state. The JSON is an intermediate representation between the server and the API client, but not a
contract of the ordering of data submitted which is internally stored separate from the JSON
intermediate representation.

Each endpoint has its own service that handles the operations on the objects exposed through the
API, although there is not likely a contract between the order of data passed to the API and the order
of data returned from the API for any given object. What should match however is that the rules set
on the object exist for the LB case, though similarly for other objects such as SSH keys used when
creating a Droplet. That is to say that any order of key fingerprints and IDs passed in may not show
up in the authorized_keys file in that particular order, but each key should be present.

This would have a much higher impact for an object that is sensitive to ordering. If you do find any
examples where ordering impacts the object, please surface that as well to add to the larger user
story here. Some data fields ( like user_data for Droplet creates ) are passed through as is to the
object. For the moment it's unclear if any action will be taken to modify things internally on this to
satisfy the constraint of matching the order. The question of whether the order of data in the
intermediate representation will reflect in the internal states of objects and persists on returned
intermediate representations through the API can certainly be asked however.

A direct impact as a result of sensitive to ordering will add impact to the user story, but at the
moment, the best approach would be for consumers of the API to leverage the API with the JSON
data as an intermediate representation that does not assume order is preserved. Please let us know
if you have any further questions on this.
"

I worked around this issue by ordering the forwarding_rule blocks by entry_port, in ascending order.

Shouldn't the PR also handle state migration?

@tdyas I did not find the need for one in this case. Creating a load balancer with the previous version and then installing a build from the PR worked as expected without it. Manually diffing the state files, the only changes were the order of the rules and the incremented serial. In Terraform >= 0.12, less information is held in the state file itself and more is computed at each run. So it's possible it was needed in the past.

Thanks @andrewsomething !

Was this page helpful?
0 / 5 - 0 ratings