pulumi / pulumi-aws-static-website

Apache License 2.0
11 stars 5 forks source link

Component doesn't work with nested subdomains #17

Open jbrunton opened 1 year ago

jbrunton commented 1 year ago

What happened?

If I set the targetDomain to a nested subdomain (e.g. foo.bar.baz.com), and if my hosted zone is the top level domain (baz.com in this example) then I get an error like this:

error: Error: invocation of aws:route53/getZone:getZone returned an error: invoking aws:route53/getZone:getZone: 1 error occurred:
    * no matching Route53Zone found

Steps to reproduce

Set the targetDomain to a nested subdomain, e.g. foo.bar.baz.com and run pulumi up.

You will see the above error.

From a look at the code, it appears the getDomainAndSubdomain function assumes only the first domain "part" is the subdomain, so in the case of this example it will return:

{
  subdomain: "foo",
  parentDomain: "bar.baz.com."
}

The component then tries to lookup the hosted zone by the parentDomain, but typically this won't be the hosted zone (unless the subdomain has its own hosted zone), so you see the error above.

Expected Behavior

Typically, I would expect that if I specify foo.bar.baz.com for the targetDomain, then the subdomain should be foo.bar and the hosted zone should be baz.com.

I note that AWS does support hosted zones for subdomains. So I think it's impossible to infer the hosted zone in the general case. However, that may not be a case this component needs to support, as I think the use cases are typically only relevant to enterprises which require different IAM roles across different subdomains? (I could be mistaken about this, however.)

Actual Behavior

The error mentioned above.

Output of pulumi about

This isn't relevant to the error but I'll edit the issue with these details if requested.

Additional context

No response

Contributing

Vote on this issue by adding a 👍 reaction. To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

danielrbradley commented 1 year ago

Thanks for writing this up @jbrunton. We'll put this on the backlog for future planning.

I've not followed the implementation through fully, but I think the simplest fix here would be to add an optional zone input property which would be used instead of the automatically calculated "parent".

jbrunton commented 1 year ago

I think the simplest fix here would be to add an optional zone input property which would be used instead of the automatically calculated "parent".

Thanks! I agree with the suggestion here. (It might be necessary to pass the record name as an optional property too.)