Skip to content

NEW FEATURE: Support DNS LOC record type#2174

Merged
tlimoncelli merged 1 commit into
DNSControl:masterfrom
systemcrash:LOC_record_type
Mar 16, 2023
Merged

NEW FEATURE: Support DNS LOC record type#2174
tlimoncelli merged 1 commit into
DNSControl:masterfrom
systemcrash:LOC_record_type

Conversation

@systemcrash
Copy link
Copy Markdown
Contributor

@systemcrash systemcrash commented Mar 15, 2023

Add LOC record type.

Step 1: Update RecordConfig in models/record.go ✅

Step 2: Add a capability for the record ✅
+providerCapabilityChecks ✅
-TestCapabilitiesAreFiltered ❌

Step 3: Add a helper function ✅

LOC() - requires all 12 parameters
LOC_BUILDER_DD() - accepts x,y coordinates
LOC_BUILDER_DMM_STR() - accepts DMM 25.24°S 153.15°E
LOC_BUILDER_DMS_STR() - accepts DMS 33°51′31″S 151°12′51″E
LOC_BUILDER_STR() - tries LOC_BUILDER_DM*STR()

Step 4: Search for #rtype_variations ✅

Step 5: Add a parse_tests test case ✅

Step 6: Add an integrationTest test case ✅

    --- PASS: TestDNSProviders/example.com/47:LOC:Single_LOC_record (6.59s)
    --- PASS: TestDNSProviders/example.com/47:LOC:Update_single_LOC_record (6.69s)
    --- PASS: TestDNSProviders/example.com/47:LOC:Multiple_LOC_records (18.33s)
    --- PASS: TestDNSProviders/example.com/Post_cleanup:Empty#03 (15.70s)

Step 7: Support more providers ✅ (bind, loopia, plus several others)

Step 8: Write documentation ✅

See the docs for the *_STR() helper functions: they each only process a
specific text string format. There are too many formats to consider.
Internally it will import text from existing DNS LOC records - which
are (hopefully) 'known good' - from provider APIs who return them in a
text format.

11/45 available providers support LOC records, or clearly document that
they do. Just shy of 25%.

@tlimoncelli
Copy link
Copy Markdown
Contributor

That's one impressive first draft! Looking good!

@systemcrash
Copy link
Copy Markdown
Contributor Author

Feels ready. So, if all of the tests in pkg/js and integrationTest work out, then it's ready?

@systemcrash systemcrash marked this pull request as ready for review March 16, 2023 03:22
@tlimoncelli
Copy link
Copy Markdown
Contributor

One minor issue:

models/t_loc.go:29:1: exported method RecordConfig.SetLOCParams should have comment or be unexported

Comment thread documentation/functions/domain/LOC.md Outdated
@systemcrash
Copy link
Copy Markdown
Contributor Author

RecordConfig.SetLOCParams

Hmm - that breaks the integration tests. Did you get that from a linter?

@tlimoncelli
Copy link
Copy Markdown
Contributor

RecordConfig.SetLOCParams

Hmm - that breaks the integration tests. Did you get that from a linter?

RecordConfig.SetLOCParams

Hmm - that breaks the integration tests. Did you get that from a linter?

What broke? You just need to add a comment that begins with the word SetLOCParams.

@systemcrash
Copy link
Copy Markdown
Contributor Author

Ah, sorry, misread your initial comment. Fixed.

Step 1: Update RecordConfig in models/record.go ✅

Step 2: Add a capability for the record ✅
+providerCapabilityChecks ✅
-TestCapabilitiesAreFiltered ❌

Step 3: Add a helper function ✅

LOC() - requires all 12 parameters
LOC_BUILDER_DD() - accepts x,y coordinates
LOC_BUILDER_DMM_STR() - accepts DMM 25.24°S 153.15°E
LOC_BUILDER_DMS_STR() - accepts DMS 33°51′31″S 151°12′51″E
LOC_BUILDER_STR() - tries LOC_BUILDER_DM*STR()

Step 4: Search for #rtype_variations ✅

Step 5: Add a parse_tests test case ✅

Step 6: Add an integrationTest test case ✅

        --- PASS: TestDNSProviders/example.com/47:LOC:Single_LOC_record (6.59s)
        --- PASS: TestDNSProviders/example.com/47:LOC:Update_single_LOC_record (6.69s)
        --- PASS: TestDNSProviders/example.com/47:LOC:Multiple_LOC_records (18.33s)
        --- PASS: TestDNSProviders/example.com/Post_cleanup:Empty#03 (15.70s)

Step 7: Support more providers ✅ (bind, loopia, plus several others)

Step 8: Write documentation ✅

See the docs for the *_STR() helper functions: they each only process a
specific text string format. There are too many formats to consider.
Internally it will import text from existing DNS LOC records - which
are (hopefully) 'known good' - from provider APIs who return them in a
text format.

11/45 available providers support LOC records, or clearly document that
they do. Just shy of 25%.
@tlimoncelli tlimoncelli changed the title Add LOC record type. NEW FEATURE: Support DNS LOC record type Mar 16, 2023
@tlimoncelli tlimoncelli merged commit 3b6591f into DNSControl:master Mar 16, 2023
@systemcrash systemcrash deleted the LOC_record_type branch March 24, 2023 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants