Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compare firewall descriptions after converting unicode + HTML entities #758

Conversation

jacobbednarz
Copy link
Member

For a while now, I've noticed that we have an occasional flapping test
in our internal test suite where "example 'string'" is attempting to
trigger an update despite not actually doing anything. Diving further
into this, I found that the Cloudflare API returns unicode + HTML
entities for the descriptions.

{
  "result": {
    "id": "e97a2db142ae4ff09499bb6a458981fd",
    "paused": true,
    "description": "this is a \u0026#39;test\u0026#39;",
    "expression": "(http.host eq \"example.com\")"
  },
  "success": true,
  "errors": [],
  "messages": []
}

However most people (ok, no one) is actually wanting to write the
description that way. Instead, they are using the single characters such
as an apostrophe (').

resource "cloudflare_filter" "%[1]s" {
  description = "this is a 'test'"
  expression = "(http.host eq \"example.com\")"
}

This means that Terraform constantly thinks that it needs to update the
value when it doesn't really.

To combat this, I've added a DiffSuppressFunc to the descriptions
which ensure the string values are compared after being processed by
html.UnescapeString. This accounts for the differences in the strings
and will still ensure we aren't missing updates -- even if someone
decides to actually use entities in their descriptions.

For a while now, I've noticed that we have an occasional flapping test
in our internal test suite where "example 'string'" is attempting to
trigger an update despite not actually doing anything. Diving further
into this, I found that the Cloudflare API returns unicode + HTML
entities for the descriptions.

```
{
  "result": {
    "id": "e97a2db142ae4ff09499bb6a458981fd",
    "paused": true,
    "description": "this is a \u0026#39;test\u0026#39;",
    "expression": "(http.host eq \"example.com\")"
  },
  "success": true,
  "errors": [],
  "messages": []
}
```

However most people (ok, no one) is actually wanting to write the
description that way. Instead, they are using the single characters such
as an apostrophe (`'`).

```
resource "cloudflare_filter" "%[1]s" {
  description = "this is a 'test'"
  expression = "(http.host eq \"example.com\")"
}
```

This means that Terraform constantly thinks that it needs to update the
value when it doesn't really.

To combat this, I've added a `DiffSuppressFunc` to the descriptions
which ensure the string values are compared after being processed by
`html.UnescapeString`. This accounts for the differences in the strings
and will still ensure we aren't missing updates -- even if someone
decides to actually use entities in their descriptions.
@jacobbednarz jacobbednarz merged commit 0cf22a8 into cloudflare:master Aug 14, 2020
@jacobbednarz jacobbednarz deleted the handle-unicode-html-entities-for-firewall-rules branch August 14, 2020 00:20
boekkooi-lengoo pushed a commit to boekkooi-lengoo/terraform-provider-cloudflare that referenced this pull request Feb 28, 2022
…o-dns-firewall

Deprecate virtualdns.go in favor of dns_firewall.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant