dnscontrol icon indicating copy to clipboard operation
dnscontrol copied to clipboard

Refactor how rtypes are implemented

Open tlimoncelli opened this issue 1 year ago • 5 comments

CC @cafferata

Problem statement:

  • The code for rtypes (A, CNAME, etc. as well as custom types like CF_REDIRECT) is spread over many parts of the code base. For example, if we were to remove CF_REDIRECT we'd have to edit code in providers/cloudflare, documentation/language-reference/domain-modifiers, helpers.js, integrationTest/integration_test.go, pkg/js/parse_tests, and more!
  • The code for rtypes is spilt between helpers.js (written in Javascript) and the Go code. This requires knowledge of two languages, instead of one. The Javascript can be more complex (especially for LOC) and yet we have no testing infrastructure for Javascript.

Proposed solution:

Ideally everything related to a particular rtypes would be isolated to a single directory that includes code, documentation, tests, etc. exclusively for that rtype.

I don't think we can get there in one step, but I think we can improve the current situation.

Proposal:

  1. Move as much code out of helpers.js as possible. Simply gather the user input and send it to the Go code. Go can do the validation, create the RecordConfig, process any metadata, etc.
  2. rtypes should be defined in $git/rtypes/NAME/ or $git/providers/PROVIDERNAME/rtypes/NAME/ where NAME is the rtype name.
  3. rtypes documentation should be in $git/rtypes/NAME/documentation/ or $git/providers/PROVIDERNAME/rtypes/NAME/documentation/. The documentation system should find it there automagicallly.
  4. In places where rtype code must be included outside those directories, create Go interfaces

Implementation:

I'm using https://github.com/StackExchange/dnscontrol/pull/3035 (CF_SINGLE_REDIRECT) as an opportunity to improve some of these issues.

  1. I've implemented a new way to pass information about records from helpers.js to the Go code. To add CF_SINGLE_REDIRECT literally 1 line is added: var CF_SINGLE_REDIRECT = rawrecordBuilder('CF_SINGLE_REDIRECT');
  2. The code for CF_SINGLE_REDIRECT is in $git/providers/cloudflare/rtypes/cfsingleredirect/

No change to where documentation is (that's going to require changes to the go generate code) nor other changes. However this is a start.

tlimoncelli avatar Jul 08 '24 01:07 tlimoncelli

I am in favor of the proposed solution. 👍 I am happy to help with redesigning the documentation generator. 🤓

cafferata avatar Jul 08 '24 20:07 cafferata

I am in favor of the proposed solution. 👍 I am happy to help with redesigning the documentation generator. 🤓

yes, please!

tlimoncelli avatar Jul 08 '24 20:07 tlimoncelli

It turns out the way models.RecordConfig was created makes doing this kind of thing very difficult. That said, I think I have a solution that will make it easier to add new record types, builders, and so on.

tlimoncelli avatar Jan 04 '25 15:01 tlimoncelli

FYI: The branch branch_gorec is slowly working towards making this possible.

tlimoncelli avatar Feb 05 '25 17:02 tlimoncelli

Thanks for the update! Appreciate you keeping me in the loop.

cafferata avatar Feb 05 '25 18:02 cafferata