Add dns update workflow

[Genne23v]

Close #270

This PR adds DNS update workflow as below.

1. Update the record by ID in DB with PENDING status
2. Upsert the record in Route53
3. Repeat polling Route53 until it gets the updated status (INSYNC or something else)
4. Update the DB either with active or error

To test this, it needs manual steps as below.

Create a hostedZone using unit test dns.server.test.ts. If you run all test, it will remove hostedZone. Run partial test.
Get hostedZoneId from localhost:5053/moto-api and set AWS_ROUTE53_HOSTED_ZONE_ID
Set ROOT_DOMAIN to starchart.com
Go to http://localhost:8080/dev and add a request button with proper request body properties. DB Record ID must be included.

[humphd]

A general comment, after quickly scanning the code in this PR. For DNS, doing Create, Update, and Delete is all very similar. Instead of creating separate queues/workers for each of these, can we create a single set of queues/workers that take the operation, etc. to run (e.g., create vs. update) and are able to run any of the jobs like this?

I think we should be able to use/leverage the existing queues/workers you already created.

[Genne23v]

I think we should be able to use/leverage the existing queues/workers you already created.

I did some testing to reuse existing worker, somehow it didn't work as expected. Actually only checkDnsStatus would be identical and the rest would be little different. I will spend more time to reduce the repeated worker/queue.

[humphd]

The way you'd do it is to have your worker be more elaborate, and include all the logic of each case. You'd send a arg with the job data that tells it what to do:

For example:

// Names are made up, don't read anything into them...
export async function genericWorker(job: DnsJob) {
  const { type, ...data } = job.data;

  switch(type) {
    case 'create':
      return doCreate(data); 
    case 'update':
      return doUpdate(data); 
    case 'delete':
      return doDelete(data); 
  }
}

[humphd]

Can you rebase this on main so it is easier to test? Also I created #298 to make it easier to deal with dns record creation for a user (i.e., we can just use the initial part of a name vs. the entire thing).

[humphd]

@Genne23v that wasn't a rebase, you added those commits in by mistake.

[Genne23v]

@Genne23v that wasn't a rebase, you added those commits in by mistake.

Sorry, my bad. I'm fixing it now.

[Genne23v]

@humphd @Myrfion I just updated the PR. Sorry that I force pushed. Please note that I haven't refactored the code to separate DB update and Route53 update in this PR. I think the refactoring amount is very big for this one PR. I will quickly push another PR once this gets merged.

[humphd]

Are you going to do the switch to create/update the record in the DB when you add it to the queue vs. when the worker starts? I think you should, so that the front-end can rely on the record existing early.

[Genne23v]

@humphd I did a quick prototype to see how it should work and considered to add it to this PR, but the amount of refactoring is very big. So I changed my mind to do that separation in next release. If it needs to be in this iteration, I will update the PR.

[humphd]

@humphd I did a quick prototype to see how it should work and considered to add it to this PR, but the amount of refactoring is very big. So I changed my mind to do that separation in next release. If it needs to be in this iteration, I will update the PR.

That makes sense to me, but you didn't mention it in the PR. It's good to call out plans like that, and even better to file follow-up issues to clearly communicate intent.

[Genne23v]

Here's the issue number for follow-up. #315

[humphd]

Thanks for filing that issue.

[SerpentBytes]

I left some feedback. In issue #311, we request to replace the logic related to manipulating dates with built-in functions from dayjs, as it does it in fewer lines of code. While this PR is still open, could you use that library instead?