feat: use a custom diff + new `id` attr on record attrs to allow using them with `for_each` #27

G-Rath · 2021-11-16T03:31:05Z

Because of the way Terraform works, you currently cannot use sending_records or receiving_records in a for_each as they're an arbitrary list set by the API response.

Attempting to do so will cause an error:

╷
│ Error: Invalid for_each argument
│
│   on main.tf line 60, in resource "cloudflare_record" "mailgun_records":
│   60:   for_each = {
│   61:     for record in mailgun_domain.default.sending_records : record.name => {
│   62:       type = record.record_type
│   63:       name = record.name
│   64:       value = record.value
│   65:     }
│   66:   }
│     ├────────────────
│     │ mailgun_domain.default.sending_records is a list of object, known only after apply
│
│ The "for_each" value depends on resource attributes that cannot be determined until apply, so Terraform cannot
│ predict how many instances will be created. To work around this, use the -target argument to first apply only
│ the resources that the for_each depends on.
╵

This is annoying because it means we have to manually setup DNS records in e.g. CloudFlare despite having a Terraform provider that lets us create DNS records 😭

There are two requirements that must be meet for Terraform to be able to plan the above:

The set must have a fixed number items (technically you can get away with more items being added by the read, but that will result in having to do two applies back-to-back to ensure your state is up to date)
one of those properties must be static, in order to be able to provide a consistent plannable ID for the Set

That is what this PR attempts to do, working off the fact that we know the number of records that we'll be provided by the API (3, for sending_records), and what those record names actually are... almost.

The edge case we have to handle is that one of the three records (the _domainkey record) we expect has a random prefix to it, which means we cannot use name itself as the field to compute the stable id for the Set, since that will result in at best an unstable diff.

We work around by adding a new id attribute to the sending_records map, which is effectively a normalised version of each records name - which is to say for two of the three records it is their name, but for the _domainkey record we drop the random prefix.

This allows us to do this:

data "cloudflare_zone" "root" {
  name = local.app_root_domain
}

resource "mailgun_domain" "default" {
  name        = local.app_full_domain
  wildcard    = false
  spam_action = "disabled"
}

resource "cloudflare_record" "mailgun_records" {
  for_each = {
    for record in mailgun_domain.default.sending_records : record.id => {
      type  = record.record_type
      name  = record.name
      value = record.value
    }
  }

  type    = each.value.type
  name    = each.value.name
  value   = each.value.value
  zone_id = data.cloudflare_zone.root.zone_id
  proxied = false

  depends_on = [mailgun_domain.default]
}

mailgun_domain.default: Creating...
mailgun_domain.default: Creation complete after 4s [id=example.com]
cloudflare_record.mailgun_records["email.example.com"]: Creating...
cloudflare_record.mailgun_records["_domainkey.example.com"]: Creating...
cloudflare_record.mailgun_records["example.com"]: Creating...
cloudflare_record.mailgun_records["email.example.com"]: Creation complete after 1s [id=ec8884a85b1b2a7390568ab4be549a3c]
cloudflare_record.mailgun_records["_domainkey.example.com"]: Creation complete after 1s [id=9012e7c43698cc96d22eb4e70d0712cf]
cloudflare_record.mailgun_records["example.com"]: Creation complete after 2s [id=87b932427914022d79707becfb37a5cd]

This was shamelessly ripped off the aws_acm_certificate resource, which does almost the same thing using the domain_name (related PR hashicorp/terraform-provider-aws#14199)

~~I'm opening this as a draft for now as I'm cleaning up the code and still have to apply it to the receiving records attribute, but wanted to try and get a review going especially because~~ I don't know how well I'll be able to actually write tests for this 😬

I can say that I've tested this a bunch locally, and I believe if MailGun were to suddenly add a new record that shouldn't cause the provider to fall over as a result of this change - especially since we're always setting an ID, so if anything breaks it would be for_eachs which are already broken right now anyway so this would still be better than what happens currently 😅

G-Rath · 2022-08-11T02:43:28Z

@wgebis any chance of a review?

wgebis · 2022-09-29T12:14:47Z

Hello there! Thanks for your contribution! Could you provide unit test impl in order to validate if modified resources work with for_each loop?

G-Rath · 2022-09-29T18:38:20Z

👋 hiya! I'm not that experienced with writing tests for Terraform providers, and it also seems like the terraform-plugin-sdk doesn't support testing for_each (you'd also need a suitable resource or module, which I don't think this module has already) - I have manually tested this a few times.

I'm happy to explore options, but would need some help :)

wgebis · 2022-10-06T12:14:39Z

@G-Rath I have just pushed to master upgrade of go, terraform sdk and Mailgun API, could you sync your branch with master and run make testacc? Then paste the output here. Thanks.

…g them with `for_each`

G-Rath · 2022-10-07T00:20:03Z

@wgebis here you go:

❯ make testacc
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v  -timeout 120m -count=1
?       github.com/terraform-providers/terraform-provider-mailgun       [no test files]
=== RUN   TestAccMailgunDomainDataSource_Basic
2022/10/07 11:50:41 [DEBUG] resourceMailgunCredential()
2022/10/07 11:50:41 [DEBUG] resourceMailgunWebhook()
--- PASS: TestAccMailgunDomainDataSource_Basic (42.40s)
=== RUN   TestProvider
--- PASS: TestProvider (0.00s)
=== RUN   TestProvider_impl
--- PASS: TestProvider_impl (0.00s)
=== RUN   TestAccMailgunDomainCredential_Basic
--- PASS: TestAccMailgunDomainCredential_Basic (41.09s)
=== RUN   TestAccMailgunDomainCredential_Update
--- PASS: TestAccMailgunDomainCredential_Update (72.21s)
=== RUN   TestAccMailgunDomain_Basic
--- PASS: TestAccMailgunDomain_Basic (39.23s)
=== RUN   TestAccMailgunDomain_Import
--- PASS: TestAccMailgunDomain_Import (53.63s)
=== RUN   TestAccMailgunRoute_Basic
--- PASS: TestAccMailgunRoute_Basic (36.76s)
=== RUN   TestAccMailgunRoute_Import
--- PASS: TestAccMailgunRoute_Import (48.25s)
=== RUN   TestAccMailgunWebhook_Basic
--- PASS: TestAccMailgunWebhook_Basic (49.00s)
PASS
ok      github.com/terraform-providers/terraform-provider-mailgun/mailgun       382.590s

I've also confirmed it worked using Route53

❯ tfa
╷
│ Warning: Provider development overrides are in effect
│
│ The following provider development overrides are set in the CLI configuration:
│  - wgebis/mailgun in /c/Users/G-Rath/workspace/projects-oss/terraform-provider-mailgun
│
│ The behavior may therefore not match any released version of the provider and applying changes may cause the state to become incompatible with published releases.
╵

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  # aws_route53_record.mailgun_records["_domainkey.my-domain-1234567891.com"] will be created
  + resource "aws_route53_record" "mailgun_records" {
      + allow_overwrite = (known after apply)
      + fqdn            = (known after apply)
      + id              = (known after apply)
      + name            = (known after apply)
      + records         = (known after apply)
      + ttl             = 300
      + type            = (known after apply)
      + zone_id         = (known after apply)
    }

  # aws_route53_record.mailgun_records["email.my-domain-1234567891.com"] will be created
  + resource "aws_route53_record" "mailgun_records" {
      + allow_overwrite = (known after apply)
      + fqdn            = (known after apply)
      + id              = (known after apply)
      + name            = (known after apply)
      + records         = (known after apply)
      + ttl             = 300
      + type            = (known after apply)
      + zone_id         = (known after apply)
    }

  # aws_route53_record.mailgun_records["my-domain-1234567891.com"] will be created
  + resource "aws_route53_record" "mailgun_records" {
      + allow_overwrite = (known after apply)
      + fqdn            = (known after apply)
      + id              = (known after apply)
      + name            = (known after apply)
      + records         = (known after apply)
      + ttl             = 300
      + type            = (known after apply)
      + zone_id         = (known after apply)
    }

  # aws_route53_zone.mine will be created
  + resource "aws_route53_zone" "mine" {
      + arn           = (known after apply)
      + comment       = "Managed by Terraform"
      + force_destroy = false
      + id            = (known after apply)
      + name          = "my-domain-1234567891.com"
      + name_servers  = (known after apply)
      + tags_all      = (known after apply)
      + zone_id       = (known after apply)
    }

  # mailgun_domain.default will be created
  + resource "mailgun_domain" "default" {
      + id                = (known after apply)
      + name              = "my-domain-1234567891.com"
      + receiving_records = [
          + {
              + id          = "mxa.mailgun.org"
              + priority    = (known after apply)
              + record_type = (known after apply)
              + valid       = (known after apply)
              + value       = (known after apply)
            },
          + {
              + id          = "mxb.mailgun.org"
              + priority    = (known after apply)
              + record_type = (known after apply)
              + valid       = (known after apply)
              + value       = (known after apply)
            },
        ]
      + region            = "us"
      + sending_records   = [
          + {
              + id          = "_domainkey.my-domain-1234567891.com"
              + name        = (known after apply)
              + record_type = (known after apply)
              + valid       = (known after apply)
              + value       = (known after apply)
            },
          + {
              + id          = "email.my-domain-1234567891.com"
              + name        = (known after apply)
              + record_type = (known after apply)
              + valid       = (known after apply)
              + value       = (known after apply)
            },
          + {
              + id          = "my-domain-1234567891.com"
              + name        = (known after apply)
              + record_type = (known after apply)
              + valid       = (known after apply)
              + value       = (known after apply)
            },
        ]
      + smtp_login        = (known after apply)
      + smtp_password     = (known after apply)
      + spam_action       = "disabled"
      + wildcard          = false
    }

Plan: 5 to add, 0 to change, 0 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

mailgun_domain.default: Creating...
aws_route53_zone.mine: Creating...
mailgun_domain.default: Creation complete after 3s [id=my-domain-1234567891.com]
aws_route53_zone.mine: Still creating... [10s elapsed]
aws_route53_zone.mine: Still creating... [20s elapsed]
aws_route53_zone.mine: Still creating... [30s elapsed]
aws_route53_zone.mine: Still creating... [40s elapsed]
aws_route53_zone.mine: Still creating... [50s elapsed]
aws_route53_zone.mine: Creation complete after 53s [id=Z055174026N06MOMUYHWB]
aws_route53_record.mailgun_records["my-domain-1234567891.com"]: Creating...
aws_route53_record.mailgun_records["_domainkey.my-domain-1234567891.com"]: Creating...
aws_route53_record.mailgun_records["email.my-domain-1234567891.com"]: Creating...
aws_route53_record.mailgun_records["my-domain-1234567891.com"]: Still creating... [10s elapsed]
aws_route53_record.mailgun_records["_domainkey.my-domain-1234567891.com"]: Still creating... [10s elapsed]
aws_route53_record.mailgun_records["email.my-domain-1234567891.com"]: Still creating... [10s elapsed]
aws_route53_record.mailgun_records["email.my-domain-1234567891.com"]: Still creating... [20s elapsed]
aws_route53_record.mailgun_records["my-domain-1234567891.com"]: Still creating... [20s elapsed]
aws_route53_record.mailgun_records["_domainkey.my-domain-1234567891.com"]: Still creating... [20s elapsed]
aws_route53_record.mailgun_records["my-domain-1234567891.com"]: Still creating... [30s elapsed]
aws_route53_record.mailgun_records["_domainkey.my-domain-1234567891.com"]: Still creating... [30s elapsed]
aws_route53_record.mailgun_records["email.my-domain-1234567891.com"]: Still creating... [30s elapsed]
aws_route53_record.mailgun_records["my-domain-1234567891.com"]: Creation complete after 35s [id=Z055174026N06MOMUYHWB_my-domain-1234567891.com_TXT]
aws_route53_record.mailgun_records["email.my-domain-1234567891.com"]: Creation complete after 35s [id=Z055174026N06MOMUYHWB_email.my-domain-1234567891.com_CNAME]
aws_route53_record.mailgun_records["_domainkey.my-domain-1234567891.com"]: Still creating... [40s elapsed]
aws_route53_record.mailgun_records["_domainkey.my-domain-1234567891.com"]: Creation complete after 43s [id=Z055174026N06MOMUYHWB_k1._domainkey.my-domain-1234567891.com_TXT]

Apply complete! Resources: 5 added, 0 changed, 0 destroyed.

terraform-provider-mailgun/infra on  make-sending-records-predicable via 💠 default on ☁️  training (ap-southeast-2) took 1m43s
❯ tfa
╷
│ Warning: Provider development overrides are in effect
│
│ The following provider development overrides are set in the CLI configuration:
│  - wgebis/mailgun in /c/Users/G-Rath/workspace/projects-oss/terraform-provider-mailgun
│
│ The behavior may therefore not match any released version of the provider and applying changes may cause the state to become incompatible with published releases.
╵
mailgun_domain.default: Refreshing state... [id=my-domain-1234567891.com]
aws_route53_zone.mine: Refreshing state... [id=Z055174026N06MOMUYHWB]
aws_route53_record.mailgun_records["email.my-domain-1234567891.com"]: Refreshing state... [id=Z055174026N06MOMUYHWB_email.my-domain-1234567891.com_CNAME]
aws_route53_record.mailgun_records["_domainkey.my-domain-1234567891.com"]: Refreshing state... [id=Z055174026N06MOMUYHWB_k1._domainkey.my-domain-1234567891.com_TXT]
aws_route53_record.mailgun_records["my-domain-1234567891.com"]: Refreshing state... [id=Z055174026N06MOMUYHWB_my-domain-1234567891.com_TXT]

No changes. Your infrastructure matches the configuration.

Terraform has compared your real infrastructure against your configuration and found no differences, so no changes are needed.

Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

terraform-provider-mailgun/infra on  make-sending-records-predicable via 💠 default on ☁️  training (ap-southeast-2) took 8s
❯

wgebis · 2022-10-07T06:18:47Z

I just realized that it isn’t backward compatible 😔. Is any chance to achieve it via TypeList?

G-Rath · 2022-10-07T06:22:45Z

@wgebis not as far as I know (and I did try a few ways before going with this one to get the smallest change possible).

Given that youre on v0.x doing a semi-breaking change should be ok so long as it's documented right?

wgebis · 2022-10-07T06:31:07Z

Right, but i’m wondering if it possible to upgrade your hcl resource from List to Set without touching external resources. Do you have any idea how to do this?
In the current state e.g. when you have resources based on simple loop via count keyword I’m not sure if it’s convertible via terraform state mv at all.

wgebis · 2022-10-07T06:38:47Z

e.g. how to convert resources via terraform state mv from the resources below to the form with TypeSet ? For me it isn't possible without resource recreation... 😞 do you have any idea?

resource "cloudflare_record" "receiving" {
  count    = 2
  zone_id   = var.cf_zone_id
  type     = mailgun_domain.this.receiving_records[count.index]["record_type"]
  name     = "${mailgun_domain.this.name}."
  value    = mailgun_domain.this.receiving_records[count.index]["value"]
  priority = mailgun_domain.this.receiving_records[count.index]["priority"]
}

resource "cloudflare_record" "sending" {
  count  = 3
  zone_id = var.cf_zone_id
  type   = mailgun_domain.this.sending_records[count.index]["record_type"]
  name   = "${mailgun_domain.this.sending_records[count.index]["name"]}."
  value  = mailgun_domain.this.sending_records[count.index]["value"]
}

G-Rath · 2022-10-07T06:42:34Z

I don't know off the top of my head, but you'll always be able to rm + import.

I actually feel like it's not stable to use a count becuase I would have tried that originally, but I might be able to give this a try tomorrow morning for you.

wgebis · 2022-10-07T06:50:00Z

Yes it's true, but it could be super annoying that after provider upgrade you have to refactor a lot of objects. So let's validate other possibilities.

OK, so give me a feedback how count works in your case. I'm using above constructions from years and so far didn't observe any issues related do receiving_records and sending_records.

wgebis · 2022-10-07T06:52:06Z

I'm aware of limitation of count, but the DNS records domains to domain are fixed for years, so maybe is good enough?

wgebis · 2022-10-07T07:05:44Z

BTW: From my perspective I recommend to keep all stuff related to Mailgun domain in the module e.g. below. It's super clean and might be used easily reused.

An example of a module definition with CF as DNS provider:

resource "mailgun_domain" "this" {
  name          = var.domain_name
  spam_action   = "disabled"
  region        = "us"
  wildcard      = "false"
}

resource "mailgun_domain_credential" "this" {
  domain = var.domain_name
  login = "admin"
  password = var.smtp_password
  region = "us"

  lifecycle {
    ignore_changes = [ password ]
  }
}

resource "cloudflare_record" "receiving" {
  count    = 2
  zone_id   = var.cf_zone_id
  type     = mailgun_domain.this.receiving_records[count.index]["record_type"]
  name     = "${mailgun_domain.this.name}."
  value    = mailgun_domain.this.receiving_records[count.index]["value"]
  priority = mailgun_domain.this.receiving_records[count.index]["priority"]
}

resource "cloudflare_record" "sending" {
  count  = 3
  zone_id = var.cf_zone_id
  type   = mailgun_domain.this.sending_records[count.index]["record_type"]
  name   = "${mailgun_domain.this.sending_records[count.index]["name"]}."
  value  = mailgun_domain.this.sending_records[count.index]["value"]
}

resource "cloudflare_record" "dmarc" {
  zone_id = var.cf_zone_id
  type   = "TXT"
  name   = "_dmarc.${var.domain_name}."
  value  = "v=DMARC1; p=none;"
}

usage:

module "mailgun-domain" {
  source = "./../../modules/mailgun/domain"
  cf_zone_id = cloudflare_zone.example_com.id
  domain_name = "example.com"
  smtp_password = "superstrongpassword"
}

G-Rath · 2022-10-07T07:20:06Z

It's not good to rely on count for a set of items (i.e. items whose values are uniquely different from each other), because its harder to manage and not a proper representation of the state - it means you open yourself up (or worse, other developers) to unexpected state changes when coming back months or years later.

I've confirmed you can easily move the resources with terraform mv, which is even easier when using the moved block:

moved {
  from = aws_route53_record.mailgun_records[0]
  to   = aws_route53_record.mailgun_records["_domainkey.my-domain-1234567891.com"]
}
moved {
  from = aws_route53_record.mailgun_records[1]
  to   = aws_route53_record.mailgun_records["email.my-domain-1234567891.com"]
}
moved {
  from = aws_route53_record.mailgun_records[2]
  to   = aws_route53_record.mailgun_records["my-domain-1234567891.com"]
}

You do have to figure out which index in the list holds what unique item (which is why using a set is a better type here):

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
-/+ destroy and then create replacement

Terraform will perform the following actions:

  # aws_route53_record.mailgun_records["_domainkey.my-domain-1234567891.com"] must be replaced
  # (moved from aws_route53_record.mailgun_records[0])
-/+ resource "aws_route53_record" "mailgun_records" {
      + allow_overwrite = (known after apply)
      ~ fqdn            = "my-domain-1234567891.com" -> (known after apply)
      ~ id              = "Z055174026N06MOMUYHWB_my-domain-1234567891.com._TXT" -> (known after apply)
      ~ name            = "my-domain-1234567891.com" -> "k1._domainkey.my-domain-1234567891.com" # forces replacement
      ~ records         = [
          + "k=rsa; p=MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDZyYWhRwL8Og6znnbzEeDAQ5mqXJ1cFzIEczBJqFSQdU5UBAIBoiC4ddsDXACS6/dQS66YHneO8KEdezkuiE1VO7ekzu48QwZHCgrWPB6CeC7hkHUJ8Y0cfrs2Ao7cVnuF8Qq/wyPBr77Gjjwr2nRd5I2s9cssRxrIz7K0Sm50mQIDAQAB",
          - "v=spf1 include:mailgun.org ~all",
        ]
        # (3 unchanged attributes hidden)
    }

  # aws_route53_record.mailgun_records["email.my-domain-1234567891.com"] must be replaced
  # (moved from aws_route53_record.mailgun_records[1])
-/+ resource "aws_route53_record" "mailgun_records" {
      + allow_overwrite = (known after apply)
      ~ fqdn            = "k1._domainkey.my-domain-1234567891.com" -> (known after apply)
      ~ id              = "Z055174026N06MOMUYHWB_k1._domainkey.my-domain-1234567891.com._TXT" -> (known after apply)
      ~ name            = "k1._domainkey.my-domain-1234567891.com" -> "email.my-domain-1234567891.com" # forces replacement
      ~ records         = [
          - "k=rsa; p=MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDZyYWhRwL8Og6znnbzEeDAQ5mqXJ1cFzIEczBJqFSQdU5UBAIBoiC4ddsDXACS6/dQS66YHneO8KEdezkuiE1VO7ekzu48QwZHCgrWPB6CeC7hkHUJ8Y0cfrs2Ao7cVnuF8Qq/wyPBr77Gjjwr2nRd5I2s9cssRxrIz7K0Sm50mQIDAQAB",
          + "mailgun.org",
        ]
      ~ type            = "TXT" -> "CNAME"
        # (2 unchanged attributes hidden)
    }

  # aws_route53_record.mailgun_records["my-domain-1234567891.com"] must be replaced
  # (moved from aws_route53_record.mailgun_records[2])
-/+ resource "aws_route53_record" "mailgun_records" {
      + allow_overwrite = (known after apply)
      ~ fqdn            = "email.my-domain-1234567891.com" -> (known after apply)
      ~ id              = "Z055174026N06MOMUYHWB_email.my-domain-1234567891.com._CNAME" -> (known after apply)
      ~ name            = "email.my-domain-1234567891.com" -> "my-domain-1234567891.com" # forces replacement
      ~ records         = [
          - "mailgun.org",
          + "v=spf1 include:mailgun.org ~all",
        ]
      ~ type            = "CNAME" -> "TXT"
        # (2 unchanged attributes hidden)
    }

Plan: 3 to add, 0 to change, 3 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value:

wgebis · 2022-10-07T07:27:35Z

Thanks for info. I restored you branch to my repo and will make some more tests locally. #35

wgebis · 2022-10-07T07:30:01Z

Just added you as a collaborator, feel free to make changes in this repo directly.

G-Rath · 2022-10-07T07:34:15Z

@wgebis thank you, I'll be happy to help out; we've been looking to expand our usage of Terraform to include more services (beyond AWS) of which Mailgun is one such service.

Do you have any preference on what you'd like to see before re-landing #27/#35 or should I leave that with you for now?

wgebis · 2022-10-07T07:47:43Z

For now leave it for me, I will make some local tests.

If I decide to go step further with this PR, some description about dealing with braking changes will be necessary to put in the resource documentation, so I will let you know.

wgebis · 2022-10-07T15:02:16Z

I have one question in terms of your snipped:

resource "cloudflare_record" "mailgun_records" {
  for_each = {
    for record in mailgun_domain.default.sending_records : record.id => {
      type  = record.record_type
      name  = record.name
      value = record.value
    }
  }

  type    = each.value.type
  name    = each.value.name
  value   = each.value.value
  zone_id = data.cloudflare_zone.root.zone_id
  proxied = false

  depends_on = [mailgun_domain.default]
}

Why just not use e.g. md5(record.value) instead of computed record.id? It means uniq values as well.

Still this workaround is necessary for for_each, but I believe it's required in both cases.

The "for_each" value depends on resource attributes that cannot be determined until apply, so Terraform cannot predict how many instances will be created. To work around this, use the -target argument to first apply only the resources that the for_each depends on.

G-Rath · 2022-10-07T21:33:32Z

It makes it a bit easier to identify the resources at a glance, e.g.

❯ tf state list
aws_route53_record.mailgun_records["_domainkey.my-domain-1234567891.com"]
aws_route53_record.mailgun_records["email.my-domain-1234567891.com"]
aws_route53_record.mailgun_records["my-domain-1234567891.com"]
aws_route53_zone.mine
mailgun_domain.default

vs

❯ tf state list
aws_route53_record.mailgun_records["1db5b20d391e28d3746bab92e4201e6f"]
aws_route53_record.mailgun_records["df39e534c1ac7e601acff59bb8b4b729"]
aws_route53_record.mailgun_records["f8172a9fa781ee5898a8aa9053f299af"]
aws_route53_zone.mine
mailgun_domain.default

and

aws_route53_record.mailgun_records["1db5b20d391e28d3746bab92e4201e6f"]: Destroying... [id=Z055174026N06MOMUYHWB_my-domain-1234567891.com_TXT]
aws_route53_record.mailgun_records["f8172a9fa781ee5898a8aa9053f299af"]: Destroying... [id=Z055174026N06MOMUYHWB_email.my-domain-1234567891.com_CNAME]
aws_route53_record.mailgun_records["df39e534c1ac7e601acff59bb8b4b729"]: Destroying... [id=Z055174026N06MOMUYHWB_k1._domainkey.my-domain-1234567891.com_TXT]
aws_route53_record.mailgun_records["f8172a9fa781ee5898a8aa9053f299af"]: Still destroying... [id=Z055174026N06MOMUYHWB_email.my-domain-1234567891.com_CNAME, 10s elapsed]
aws_route53_record.mailgun_records["df39e534c1ac7e601acff59bb8b4b729"]: Still destroying... [id=Z055174026N06MOMUYHWB_k1._domainkey.my-domain-1234567891.com_TXT, 10s elapsed]
aws_route53_record.mailgun_records["1db5b20d391e28d3746bab92e4201e6f"]: Still destroying... [id=Z055174026N06MOMUYHWB_my-domain-1234567891.com_TXT, 10s elapsed]
aws_route53_record.mailgun_records["f8172a9fa781ee5898a8aa9053f299af"]: Still destroying... [id=Z055174026N06MOMUYHWB_email.my-domain-1234567891.com_CNAME, 20s elapsed]
aws_route53_record.mailgun_records["df39e534c1ac7e601acff59bb8b4b729"]: Still destroying... [id=Z055174026N06MOMUYHWB_k1._domainkey.my-domain-1234567891.com_TXT, 20s elapsed]
aws_route53_record.mailgun_records["1db5b20d391e28d3746bab92e4201e6f"]: Still destroying... [id=Z055174026N06MOMUYHWB_my-domain-1234567891.com_TXT, 20s elapsed]
aws_route53_record.mailgun_records["df39e534c1ac7e601acff59bb8b4b729"]: Still destroying... [id=Z055174026N06MOMUYHWB_k1._domainkey.my-domain-1234567891.com_TXT, 30s elapsed]
aws_route53_record.mailgun_records["f8172a9fa781ee5898a8aa9053f299af"]: Still destroying... [id=Z055174026N06MOMUYHWB_email.my-domain-1234567891.com_CNAME, 30s elapsed]
aws_route53_record.mailgun_records["1db5b20d391e28d3746bab92e4201e6f"]: Still destroying... [id=Z055174026N06MOMUYHWB_my-domain-1234567891.com_TXT, 30s elapsed]
aws_route53_record.mailgun_records["f8172a9fa781ee5898a8aa9053f299af"]: Destruction complete after 38s
aws_route53_record.mailgun_records["df39e534c1ac7e601acff59bb8b4b729"]: Destruction complete after 38s
aws_route53_record.mailgun_records["1db5b20d391e28d3746bab92e4201e6f"]: Still destroying... [id=Z055174026N06MOMUYHWB_my-domain-1234567891.com_TXT, 40s elapsed]
aws_route53_record.mailgun_records["1db5b20d391e28d3746bab92e4201e6f"]: Destruction complete after 43s
aws_route53_record.mailgun_records["_domainkey.my-domain-1234567891.com"]: Creating...
aws_route53_record.mailgun_records["my-domain-1234567891.com"]: Creating...
aws_route53_record.mailgun_records["email.my-domain-1234567891.com"]: Creating...
aws_route53_record.mailgun_records["_domainkey.my-domain-1234567891.com"]: Still creating... [10s elapsed]
aws_route53_record.mailgun_records["email.my-domain-1234567891.com"]: Still creating... [10s elapsed]
aws_route53_record.mailgun_records["my-domain-1234567891.com"]: Still creating... [10s elapsed]
aws_route53_record.mailgun_records["_domainkey.my-domain-1234567891.com"]: Still creating... [20s elapsed]
aws_route53_record.mailgun_records["email.my-domain-1234567891.com"]: Still creating... [20s elapsed]
aws_route53_record.mailgun_records["my-domain-1234567891.com"]: Still creating... [20s elapsed]
aws_route53_record.mailgun_records["_domainkey.my-domain-1234567891.com"]: Still creating... [30s elapsed]
aws_route53_record.mailgun_records["email.my-domain-1234567891.com"]: Still creating... [30s elapsed]
aws_route53_record.mailgun_records["my-domain-1234567891.com"]: Still creating... [30s elapsed]
aws_route53_record.mailgun_records["my-domain-1234567891.com"]: Creation complete after 37s [id=Z055174026N06MOMUYHWB_my-domain-1234567891.com_TXT]
aws_route53_record.mailgun_records["_domainkey.my-domain-1234567891.com"]: Still creating... [40s elapsed]
aws_route53_record.mailgun_records["email.my-domain-1234567891.com"]: Still creating... [40s elapsed]
aws_route53_record.mailgun_records["email.my-domain-1234567891.com"]: Creation complete after 48s [id=Z055174026N06MOMUYHWB_email.my-domain-1234567891.com_CNAME]
aws_route53_record.mailgun_records["_domainkey.my-domain-1234567891.com"]: Creation complete after 49s [id=Z055174026N06MOMUYHWB_k1._domainkey.my-domain-1234567891.com_TXT]

The information is technically still in the state elsewhere (i.e. the ID of the aws_route53_record has the same info) but that's not always shown and depends on the resource/provider (since they decide what they use for their IDs)

G-Rath · 2022-11-21T18:40:36Z

@wgebis have you had a chance to look back over this? it'd be great to get landed and released

wgebis · 2022-11-23T14:32:10Z

@G-Rath I'm not sure it's worth of breaking the provider backward compatibility. You can handle for_each using md5 workaround. In that form it's possible to user both count and for_each (with hash calculation) approaches.

This PR includes a logic where a core is in fact calculating the IDs of resources, don't you think?

G-Rath · 2022-11-23T18:49:39Z

@wgebis using md5 is not stable because Terraform evaluates for_each before it does any applying:

resource "aws_route53_record" "mailgun_records" {
  for_each = {
    for record in mailgun_domain.default.sending_records : md5(record.name) => {
      type  = record.record_type
      name  = record.name
      value = record.value
    }
  }

  type    = each.value.type
  name    = each.value.name
  records = [each.value.value]
  ttl = 300
  zone_id = aws_route53_zone.mine.zone_id

  depends_on = [mailgun_domain.default]
}

Gives

╷
│ Error: Invalid for_each argument
│
│   on main.tf line 98, in resource "aws_route53_record" "mailgun_records":
│   98:   for_each = {
│   99:     for record in mailgun_domain.default.sending_records : md5(record.name) => {
│  100:       type  = record.record_type
│  101:       name  = record.name
│  102:       value = record.value
│  103:     }
│  104:   }
│     ├────────────────
│     │ mailgun_domain.default.sending_records is a list of object, known only after apply
│
│ The "for_each" map includes keys derived from resource attributes that cannot be determined until apply, and so Terraform cannot determine the full set of keys that will identify the instances of this resource.
│
│ When working with unknown values in for_each, it's better to define the map keys statically in your configuration and place apply-time results only in the map values.
│
│ Alternatively, you could use the -target planning option to first apply only the resources that the for_each value depends on, and then apply a second time to fully converge.

While you might be able to use -target, that's a messy solution that doesn't fit well with services like Terraform Cloud when using a CI/CD workflow since -target is a manual flag.

This PR includes a logic where a core is in fact calculating the IDs of resources, don't you think?

I'm not sure what you mean by this, but the fact that the provider is calculating the ID of resources is the key part of this - that provides Terraform with a set of static values that it can use in the static plan phase, and then fill out the rest in the dynamic compute/apply phase.

But look what if we just introduced this as new attributes i.e. sending_records_set and receiving_records_set - that way backwards compatibility will be preserved for the meantime; we could also go the other way with sending_records_list and receiving_records_list.

I do think in the long-run though the current list-based attributes should be deprecated and replaced with the set version because its more accurate and fits with expectations based on other providers (i.e. this all started with us assuming we can use the same structure as we can for resources like aws_acm_certificate and aws_acm_certificate_validation).

wgebis · 2022-11-23T19:46:38Z

Hi!

While you might be able to use -target, that's a messy solution that doesn't fit well with services like Terraform Cloud when using a CI/CD workflow since -target is a manual flag.

Yep, I got the same result when tested code from this PR. It's because total objects for for_each elements are now known before creation mailgun_domain. In count variant, number of item is fixed in both lists (receiving and sending).

But look what if we just introduced this as new attributes i.e. sending_records_set and receiving_records_set - that way backwards compatibility will be preserved for the meantime; we could also go the other way with sending_records_list and receiving_records_list.

Yeah, it definitely make sense 👍. So, sending_records and receiving_records might be marked as deprecated. It will be seamless approach.
But still, I'm afraid that you will get the same: elements "known only after apply", but let's try.

Feel free to make these changes, you should have permissions to push directly to this repo to a new branch. 👍

G-Rath marked this pull request as ready for review November 16, 2021 03:54

G-Rath added 6 commits October 7, 2022 10:26

feat: use a custom diff + new id attr on record attrs to allow usin…

bcd570b

…g them with `for_each`

chore: rename function

4f4283e

feat: handle receiving_records as well

c2d9428

refactor: move helper out of resource file and make private

c317e04

chore: sort import a bit better

2a5d6c8

test: update case to account for non-static name value

14140e6

wgebis approved these changes Oct 7, 2022

View reviewed changes

wgebis merged commit f0a2a0a into wgebis:master Oct 7, 2022

G-Rath deleted the make-sending-records-predicable branch October 7, 2022 05:18

wgebis mentioned this pull request Oct 7, 2022

Revert "feat: use a custom diff + new id attr on record attrs to allow using them with for_each" #34

Merged

G-Rath mentioned this pull request Nov 24, 2022

feat: add receiving_records_set and sending_records_set attributes #37

Merged

G-Rath mentioned this pull request Apr 14, 2023

Documentation examples needed for *_records_set #41

Open

G-Rath mentioned this pull request Mar 15, 2024

I can't create the dns records at the same run I create the domain #50

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: use a custom diff + new `id` attr on record attrs to allow using them with `for_each` #27

feat: use a custom diff + new `id` attr on record attrs to allow using them with `for_each` #27

G-Rath commented Nov 16, 2021 •

edited

Loading

G-Rath commented Aug 11, 2022

wgebis commented Sep 29, 2022

G-Rath commented Sep 29, 2022

wgebis commented Oct 6, 2022 •

edited

Loading

G-Rath commented Oct 7, 2022

wgebis commented Oct 7, 2022

G-Rath commented Oct 7, 2022

wgebis commented Oct 7, 2022

wgebis commented Oct 7, 2022

G-Rath commented Oct 7, 2022

wgebis commented Oct 7, 2022 •

edited

Loading

wgebis commented Oct 7, 2022

wgebis commented Oct 7, 2022

G-Rath commented Oct 7, 2022 •

edited

Loading

wgebis commented Oct 7, 2022

wgebis commented Oct 7, 2022

G-Rath commented Oct 7, 2022

wgebis commented Oct 7, 2022

wgebis commented Oct 7, 2022 •

edited

Loading

G-Rath commented Oct 7, 2022

G-Rath commented Nov 21, 2022

wgebis commented Nov 23, 2022

G-Rath commented Nov 23, 2022

wgebis commented Nov 23, 2022

feat: use a custom diff + new id attr on record attrs to allow using them with for_each #27

feat: use a custom diff + new id attr on record attrs to allow using them with for_each #27

Conversation

G-Rath commented Nov 16, 2021 • edited Loading

G-Rath commented Aug 11, 2022

wgebis commented Sep 29, 2022

G-Rath commented Sep 29, 2022

wgebis commented Oct 6, 2022 • edited Loading

G-Rath commented Oct 7, 2022

wgebis commented Oct 7, 2022

G-Rath commented Oct 7, 2022

wgebis commented Oct 7, 2022

wgebis commented Oct 7, 2022

G-Rath commented Oct 7, 2022

wgebis commented Oct 7, 2022 • edited Loading

wgebis commented Oct 7, 2022

wgebis commented Oct 7, 2022

G-Rath commented Oct 7, 2022 • edited Loading

wgebis commented Oct 7, 2022

wgebis commented Oct 7, 2022

G-Rath commented Oct 7, 2022

wgebis commented Oct 7, 2022

wgebis commented Oct 7, 2022 • edited Loading

G-Rath commented Oct 7, 2022

G-Rath commented Nov 21, 2022

wgebis commented Nov 23, 2022

G-Rath commented Nov 23, 2022

wgebis commented Nov 23, 2022

feat: use a custom diff + new `id` attr on record attrs to allow using them with `for_each` #27

feat: use a custom diff + new `id` attr on record attrs to allow using them with `for_each` #27

G-Rath commented Nov 16, 2021 •

edited

Loading

wgebis commented Oct 6, 2022 •

edited

Loading

wgebis commented Oct 7, 2022 •

edited

Loading

G-Rath commented Oct 7, 2022 •

edited

Loading

wgebis commented Oct 7, 2022 •

edited

Loading