dandi / dandi-infrastructure

A repository to collect docs/issues on DANDI project infrastructure
Apache License 2.0
0 stars 4 forks source link

Use a trailing dot for AWS ACM validation record #176

Closed waxlamp closed 5 months ago

waxlamp commented 5 months ago

This corrects a spurious plan element that seems to show up in every plan, in which the domain for the AWS ACM validation record loses a trailing dot.

The Terraform code indeed does not include the trailing dot, but it seems as though at runtime, the deployed resource somehow gains it, leaving the deployed state inconsistent with the code. I don't know what causes that, but this "fixes" the problem by bringing the code into line with what the deployed resource seems to be.

For an example of a TF plan that includes the spurious change, see https://app.terraform.io/app/dandi/workspaces/dandi-prod/runs/run-VFdyCNW18mBqo6ju.

If there's a better way to solve this, let's close this PR and pursue that solution instead.

Unrelated to the fix, but the plan also includes a change to the dandidav buildpack infrastructure. I'm not sure why that's happening, as this PR was intended to be something of a no-op w/r/t the TF plan.

waxlamp commented 5 months ago

I've formally requested a review from @mvandenburgh, but I'm also interested in the opinions of @jjnesbitt and @danlamanna.

mvandenburgh commented 5 months ago

Unrelated to the fix, but the plan also includes a change to the dandidav buildpack infrastructure. I'm not sure why that's happening, as this PR was intended to be something of a no-op w/r/t the TF plan.

177 fixes this.

mvandenburgh commented 5 months ago

@waxlamp now that #177 is merged, if you rebase this PR you should get a clean TF plan.

waxlamp commented 5 months ago

@waxlamp now that #177 is merged, if you rebase this PR you should get a clean TF plan.

Perfect, getting the expected no-op now: https://app.terraform.io/app/dandi/workspaces/dandi-prod/runs/run-dWxXmy46TH7HKfWb

waxlamp commented 5 months ago

AWS treats a record with the trailing dot the same as it does one without a trailing dot - see the "CNAME" section on this page https://docs.aws.amazon.com/Route53/latest/DeveloperGuide/resource-record-sets-values-shared.html. And when you add a CNAME without a trailing dot via the AWS console, AWS automatically appends a dot. I think that's what's happening here, in which case this is the correct way to fix this.

Right, but are you saying this problem only affects records that were created via the console, without the dot, that are then managed by the terraform module? Because we have other CNAME records specified via domain.tf that don't have a trailing dot and don't cause this sort of mismatch problem.

After this is merged and applied (even though that apply would just be a no-op), could we once more remove the dot and settle back in that configuration?

mvandenburgh commented 5 months ago

AWS treats a record with the trailing dot the same as it does one without a trailing dot - see the "CNAME" section on this page docs.aws.amazon.com/Route53/latest/DeveloperGuide/resource-record-sets-values-shared.html. And when you add a CNAME without a trailing dot via the AWS console, AWS automatically appends a dot. I think that's what's happening here, in which case this is the correct way to fix this.

Right, but are you saying this problem only affects records that were created via the console, without the dot, that are then managed by the terraform module? Because we have other CNAME records specified via domain.tf that don't have a trailing dot and don't cause this sort of mismatch problem.

Oh, I didn't notice the other CNAME records that don't have one. In that case I'm not sure, aybe you're right about records created initially in the console, :shrug: