edge / account

Account portal for managing Edge services
Other
3 stars 1 forks source link

Auto-create DNS records when CDN domain(s) are managed by Edge DNS #156

Closed adamkdean closed 1 year ago

adamkdean commented 2 years ago

When any of the domains assigned to a CDN instance are managed by Edge DNS, we should offer to create these records automatically. This would be an option during the 'success' screen, where if there is a domain/DNS match, we show an additional section that offers to set these up and has a button etc.

willgarrett64 commented 1 year ago

For my own reference, this is the success screen:

image

FIX: "Please create the following DNS records to enabled CDN to work:"

willgarrett64 commented 1 year ago

1) call api.dns.getZone(api-url, session-key, domain-name) to check if domain zone exists and it belongs to session account 2) call api.dns.getRecords(api-url, session-key, domain-name, { limit: high-limit }) to check if records exists (first, you can check if zone types exists for the records to create, if yes, run the full getRecords, if no, assume records don't exist). 3) call api.dns.createRecord(api-url, session-key, domain-name, record-options) if user clicks button to auto-create records.

willgarrett64 commented 1 year ago

@adamkdean As an example, let's say a user deploys a CDN with 5 domains, 3 on Edge, 2 not. Out of the 3 on Edge, 1 of them already has the correct record created.

When we get to the success screen, do we want to go the simple route and list all 5 records to be created, or do we want to filter out the one that we know has already been created? Also, where we have a button to auto-create the records, do we need to make it clear which records will be created (as in, the ones with DNS on Edge, but not the others)?

If yes to all the above, I think it'll be better to add an endpoint that will do all these checks/create all these records in one go in the API. Otherwise we need to do multiple API calls (2x for each domain) just to determine which records need creating.

If we want to keep it simple and just show all domains in one table, regardless of whether they are managed by Edge DNS/have existing records, we could just have the button to auto-create all, and it'll return an error for each one that isn't possible. We then just report which ones were created.

willgarrett64 commented 1 year ago

Oh and in addition to the above, I think creating the endpoints to check/create all DNS records for the integration might be best because we could then use the same for when a user manages the integration and adds new domains.

adamkdean commented 1 year ago

In terms of the appearance, I think having an additional column in the list of DNS settings with a button would work well. If the domain isn't in Edge DNS, the button would be disabled, with the text "Not managed by Edge DNS". If the DNS zone is in Edge, then the button would say "Create record", when you click it, it disables and says "Creating... [spinner]" and then either returns to "Create record" if there's an error (while showing an error message below) or "Created" and disabled if successful. The buttons should be thinner, to keep the table appearance compact.

In terms of API calls, the initial call to get the DNS status should be one request, yes, but then an individual request for each of the zones to be created.

If I've missed anything, let me know.

willgarrett64 commented 1 year ago

Ok, so indv button for each rather than a single button to create all, that makes sense. One thing just to note, to check whether the record already exists could currently require 2 API calls per record. 1) getZone(domain) - check if zone exists, and if yes, can also check if there are any CNAME records (this lists number of records per type, but not the full record itself) 2) getRecords(domain) - check all records for a given zone, then iterate through list to see if the CNAME record for that hostname already exists.

adamkdean commented 1 year ago

For my own reference, this is the success screen:

image

FIX: "Please create the following DNS records to enabled CDN to work:"

Also need to fix the label for apex domains to say ALIAS not CNAME, as discussed on discord

adamkdean commented 1 year ago

Ok, so indv button for each rather than a single button to create all, that makes sense. One thing just to note, to check whether the record already exists could currently require 2 API calls per record.

  1. getZone(domain) - check if zone exists, and if yes, can also check if there are any CNAME records (this lists number of records per type, but not the full record itself)
  2. getRecords(domain) - check all records for a given zone, then iterate through list to see if the CNAME record for that hostname already exists.

That's fine. I don't see an issue here. Two calls per individual action is fine -- not the same as multiple calls per second per person viewing a screen for example. In this case, it's a small number and not a problem.