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

data { attributes being deleted from MX records? #3965

Closed
3 tasks done
roy-work opened this issue Sep 11, 2024 · 7 comments
Closed
3 tasks done

data { attributes being deleted from MX records? #3965

roy-work opened this issue Sep 11, 2024 · 7 comments
Labels
kind/bug Categorizes issue or PR as related to a bug. triage/needs-information Indicates an issue needs more information in order to work on it.

Comments

@roy-work
Copy link

Confirmation

  • This is a bug with an existing resource and is not a feature request or enhancement. Feature requests should be submitted with Cloudflare Support or your account team.
  • I have searched the issue tracker and my issue isn't already found.
  • I have replicated my issue using the latest version of the provider and it is still present.

Terraform and Cloudflare provider version

Terraform v1.5.7
on darwin_amd64
+ provider registry.terraform.io/cloudflare/cloudflare v4.38.0

(Note that it does reproduce on v4.41.0, but I am not readily able to upgrade to that as it deprecates value, and I need to change literally every resource to account for that.)

Affected resource(s)

  • cloudflare_record

Terraform configuration files

resource "cloudflare_record" "gem_mx" {
  for_each = local.gem_mx
  zone_id = data.cloudflare_zones.commure_com.zones[0].id
  name = "recruiting"
  type = "MX"
  value = each.value
  priority = 10
  # TODO: bump up, once the dust is settled.
  # ttl = 2 * 60 * 60  # 2 hr
  ttl = 5 * 60  # 5 min
}

Link to debug output

https://example.com

Panic output

No response

Expected output

No changes to plan.

Actual output

We get a plan, where nothing has changed. See below.

Steps to reproduce

We changed nothing. This record was already applied, and plans were generating "no changes" until recently. As we've also not upgraded the provider, this implies that CF's API responses have changed in a way that affects the TF provider.

This first showed up as:

  # cloudflare_record.gem_mx["a"] will be updated in-place
  ~ resource "cloudflare_record" "gem_mx" {
        id              = "[snip]"
        name            = "recruiting"
        tags            = []
        # (12 unchanged attributes hidden)

      - data {
          - algorithm      = 0 -> null
          - altitude       = 0 -> null
          - digest_type    = 0 -> null
          - key_tag        = 0 -> null
          - lat_degrees    = 0 -> null
          - lat_minutes    = 0 -> null
          - lat_seconds    = 0 -> null
          - long_degrees   = 0 -> null
          - long_minutes   = 0 -> null
          - long_seconds   = 0 -> null
          - matching_type  = 0 -> null
          - order          = 0 -> null
          - port           = 0 -> null
          - precision_horz = 0 -> null
          - precision_vert = 0 -> null
          - preference     = 0 -> null
          - priority       = 10 -> null
          - protocol       = 0 -> null
          - selector       = 0 -> null
          - size           = 0 -> null
          - target         = "mxa.mailgun.org" -> null
          - type           = 0 -> null
          - usage          = 0 -> null
          - weight         = 0 -> null
        }
    }
    
    # (also one for "b", similar)

Applying this is harmless (the record does not change), but the plan never converges.

I tried changing the resource to be,

resource "cloudflare_record" "gem_mx" {
  for_each = local.gem_mx
  zone_id = data.cloudflare_zones.commure_com.zones[0].id
  name = "recruiting"
  type = "MX"
  data {
    target = each.value
    priority = 10
  }
  # TODO: bump up, once the dust is settled.
  # ttl = 2 * 60 * 60  # 2 hr
  ttl = 5 * 60  # 5 min
}

Figuring that, since I think that's closer to the format the API uses, maybe that'd make the provider happier. That still results in a plan:

  # cloudflare_record.gem_mx["a"] will be updated in-place
  ~ resource "cloudflare_record" "gem_mx" {
        id              = "[snip]"
        name            = "recruiting"
      - priority        = 10 -> null
        tags            = []
        # (11 unchanged attributes hidden)

        # (1 unchanged block hidden)
    }

Again, a no-op but non-converging plan.

This:

resource "cloudflare_record" "gem_mx" {
  for_each = local.gem_mx
  zone_id = data.cloudflare_zones.commure_com.zones[0].id
  name = "recruiting"
  type = "MX"
  data {
    target = each.value
    priority = 10
  }
  priority = 10  # See cloudflare/bugs.md
  # TODO: bump up, once the dust is settled.
  # ttl = 2 * 60 * 60  # 2 hr
  ttl = 5 * 60  # 5 min
}

Works (the plan immediately converges), but is ugly; I shouldn't need to specify priority twice like that.

Additional factoids

No response

References

This smells similar to #3348

@roy-work roy-work added kind/bug Categorizes issue or PR as related to a bug. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Sep 11, 2024
Copy link
Contributor

Community Note

Voting for Prioritization

  • Please vote on this issue by adding a 👍 reaction to the original post to help the community and maintainers prioritize this request.
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.

Volunteering to Work on This Issue

  • If you are interested in working on this issue, please leave a comment.
  • If this would be your first contribution, please review the contribution guide.

Copy link
Contributor

Thank you for reporting this issue! For maintainers to dig into issues it is required that all issues include the entirety of TF_LOG=DEBUG output to be provided. The only parts that should be redacted are your user credentials in the X-Auth-Key, X-Auth-Email and Authorization HTTP headers. Details such as zone or account identifiers are not considered sensitive but can be redacted if you are very cautious. This log file provides additional context from Terraform, the provider and the Cloudflare API that helps in debugging issues. Without it, maintainers are very limited in what they can do and may hamper diagnosis efforts.

This issue has been marked with triage/needs-information and is unlikely to receive maintainer attention until the log file is provided making this a complete bug report.

@github-actions github-actions bot added triage/needs-information Indicates an issue needs more information in order to work on it. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Sep 11, 2024
@jstewart612
Copy link

jstewart612 commented Sep 11, 2024

There is no version to which I could backport to address the concern and the comment #3965 (comment) is correct about having to still have priority outside and then inside the data block (outside as a string and inside as a number) in order for plans to converge successfully without change. So, to my mind, the actionable item here is why make us double tap on a field when it is known it must move under the data block?

Considering that version backporting failed, I am left to assume Cloudflare implemented breaking API change here.

@jacobbednarz
Copy link
Member

this was an unintentional change on the service that lead to https://www.cloudflarestatus.com/incidents/79h1kvybmthk. this wasn't a change in the provider so you should be fine to re-run these and have the state rectified to the correct values again.

@jacobbednarz jacobbednarz closed this as not planned Won't fix, can't repro, duplicate, stale Sep 11, 2024
@ddesboroughv
Copy link

I just performed another plan and am still seeing the removal of the data block:

      - data {
          - algorithm      = 0 -> null
          - altitude       = 0 -> null
          - digest_type    = 0 -> null
          - key_tag        = 0 -> null
          - lat_degrees    = 0 -> null
          - lat_minutes    = 0 -> null
          - lat_seconds    = 0 -> null
          - long_degrees   = 0 -> null
          - long_minutes   = 0 -> null
          - long_seconds   = 0 -> null
          - matching_type  = 0 -> null
            name           = null
          - order          = 0 -> null
          - port           = 0 -> null
          - precision_horz = 0 -> null
          - precision_vert = 0 -> null
          - preference     = 0 -> null
          - priority       = 5 -> null
          - protocol       = 0 -> null
          - selector       = 0 -> null
          - size           = 0 -> null
          - target         = "<redacted>" -> null
          - type           = 0 -> null
          - usage          = 0 -> null
          - weight         = 0 -> null
            # (14 unchanged attributes hidden)
        }

@jacobbednarz Are you saying we should apply this to reconcile things again?

@jacobbednarz
Copy link
Member

yes - a plan does not update the state so that is expected not to do anything. you need to run the apply for the state to be reconciled.

@ddesboroughv
Copy link

To close the loop, as of this morning a plan no longer shows the removal of the data block. I didn't have to do anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. triage/needs-information Indicates an issue needs more information in order to work on it.
Projects
None yet
Development

No branches or pull requests

4 participants