Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Treat tags as an unordered set #46

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

saj
Copy link
Contributor

@saj saj commented Nov 13, 2019

Breaking change. The tags attribute will change from string-type to
set-of-string type. Users will need to make the following syntactic
change to their Terraform configuration files:

// before:
resource "pingdom_check" "example" {
  // ...
  tags = "foo,bar"
}

// after:
resource "pingdom_check" "example" {
  // ...
  tags = ["foo", "bar"]  // order no longer relevant
}

This commit includes a schema migration for existing user state.


The Pingdom API returns tag elements in an unpredictable order.
Ordering for this type is not documented in the Pingdom API spec.
Current plugin behaviour often leads to spurious diffs against state.

https://www.pingdom.com/api/2.1/#MethodGet+Check+List

Notionally, tags should be considered to be sets in the Pingdom data
model. It does not make much practical sense for us to enforce strict
ordering on them. Any implied ordering is likely an inadvertent
side-effect of JSON's lack of an unordered set type.

This change will hash tag names using the FNV-1a alternate algorithm.
(Tag names are the only labels that users see in the Pingdom UI and API.
Pingdom do not expose tag IDs in their API.)

http://www.isthe.com/chongo/tech/comp/fnv/index.html#FNV-1a

As far as I know, this algorithm is a good fit for set element hash
computation: it is said to be fast, said to provide reasonably good
dispersion, and is a part of the Go standard library.

@saj
Copy link
Contributor Author

saj commented Nov 13, 2019

The CI build for Go 1.11 failed because:

$ go get

go get: warning: modules disabled by GO111MODULE=auto in GOPATH/src;

	ignoring go.mod;

	see 'go help modules'

# github.com/hashicorp/terraform/helper/plugin

../../hashicorp/terraform/helper/plugin/grpc_provisioner.go:173:6: b.Cap undefined (type strings.Builder has no field or method Cap)

This is not unexpected. The Builder.Cap method was introduced in Go 1.12.

Do you still require Go 1.11 support?

Breaking change.  The `tags` attribute will change from string-type to
set-of-string type.  Users will need to make the following syntactic
change to their Terraform configuration files:

    // before:
    resource "pingdom_check" "example" {
      // ...
      tags = "foo,bar"
    }

    // after:
    resource "pingdom_check" "example" {
      // ...
      tags = ["foo", "bar"]  // order no longer relevant
    }

This commit includes a schema migration for existing user state.

---

The Pingdom API returns tag elements in an unpredictable order.
Ordering for this type is not documented in the Pingdom API spec.
Current plugin behaviour often leads to spurious diffs against state.

https://www.pingdom.com/api/2.1/#MethodGet+Check+List

Notionally, tags should be considered to be sets in the Pingdom data
model.  It does not make much practical sense for us to enforce strict
ordering on them.  Any implied ordering is likely an inadvertent
side-effect of JSON's lack of an unordered set type.

This change will hash tag names using the FNV-1a alternate algorithm.
(Tag names are the only labels that users see in the Pingdom UI and API.
Pingdom do not expose tag IDs in their API.)

http://www.isthe.com/chongo/tech/comp/fnv/index.html#FNV-1a

As far as I know, this algorithm is a good fit for set element hash
computation:  it is said to be fast, said to provide reasonably good
dispersion, and is a part of the Go standard library.
@russellcardullo
Copy link
Owner

The CI build for Go 1.11 failed because

Ah yeah sorry about that. It also looks like CI was ignoring the version pinned in go.mod and using a newer commit from Terraform which includes that new function.

I fixed the version pinning issue in #48. I don't think we need Go 1.11 support here anymore, especially as it's not supported in the latest terraform so I've removed that as well in the same PR.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants