TechToSpeech / terraform-aws-serverless-static-wordpress

Terraform module for deploying Serverless Static Wordpress on AWS
GNU General Public License v3.0
194 stars 69 forks source link

Route53 entries not honouring `site_prefix` variable #51

Open nvnivs opened 2 years ago

nvnivs commented 2 years ago

Perhaps I'm missing something, but can't see how the implementation of the Route53 entries honours the site_prefix variable.

The www record is not taking the site_prefix variable https://github.com/TechToSpeech/terraform-aws-serverless-static-wordpress/blob/master/r53.tf#L3.

Also the apex record will always manage the apex record, which may not be the desired outcome.

On my scenario, i have got an existing website with an already set www and apex records which i don't want to change. I want an additional recordset to test this module that would run in parallel with the existing website.

Woudn't it make more sense for this module to only manage the recordset defined and in site_prefix and give the users the flexibility of managing their apex and www records differently?

That could also help address https://github.com/TechToSpeech/terraform-aws-serverless-static-wordpress/issues/43, by giving users the possibility of setting the non-canonical address to another endpoint which would handle the redirect.

petewilcock commented 2 years ago

This is a very fair call-out. The module is currently built with some hard assumptions - as you note I've assumed wordpress is at the apex of your domain either with or without the www. You may want to have it in a sub-domain, like blog.example.com, and currently this doesn't handle that.

It also occurs to me that this probably wouldn't work if you wanted example.com/blog - you could make WP2Static output to the correct part of the S3 bucket and substitute the correct target URL, but CloudFront would then need to have multiple origins configured (and we don't have that right now either) - one where the apex points at, say, a load balancer, and the /blog directory goes to S3 as expected.

Definitely worth enhancing :+1:

nvnivs commented 2 years ago

I was thinking that something like only setting on record set, which would use the the subdomain of site_prefix or the apex record if not defined.

This would be a breaking change of course, so only worth considering for v2.

I didn't thought about the example.com/blog case, but that sounds like very good use case as well, but i imagine would be a larger piece of work.