Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

feat: support _dnslink subdomain specified dnslinks #1843

Merged
merged 5 commits into from
Jan 24, 2019

Conversation

chancehudson
Copy link
Contributor

@chancehudson chancehudson commented Jan 24, 2019

Closes #1842

This includes resolution of _dnslink subdomains outward and inward. If the supplied domain is _dnslink.domain.com and resolution fails then domain.com will be checked (inward checking). If the supplied domain is domain.com and the resolution fails then _dnslink.domain.com will be checked (outward checking).

Inward checking is not specified in the dnslink spec, but seems like it may be useful.

Tests

I added a test case for _dnslink.ipfs.io to ensure that inward resolution works. It probably makes sense to create a dns entry on the ipfs.io domain like _dnslink.dnslink_test.ipfs.io so a test can be added for jsipfs dns dnslink_test.ipfs.io.

The referenced issue contains commands with an example domain config for testing. I checked the go-ipfs test suite for guidance on testing but didn't find anything.

@alanshaw alanshaw changed the title Support _dnslink subdomain specified dnslinks feat: support _dnslink subdomain specified dnslinks Jan 24, 2019
Copy link
Member

@alanshaw alanshaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️ this is awesome - thank you. There's just a few minor things to address but this is great and I think we should get this merged asap!

src/core/runtime/dns-nodejs.js Outdated Show resolved Hide resolved
src/core/runtime/dns-nodejs.js Outdated Show resolved Hide resolved
src/core/runtime/dns-nodejs.js Outdated Show resolved Hide resolved
src/core/runtime/dns-nodejs.js Outdated Show resolved Hide resolved
src/core/runtime/dns-nodejs.js Outdated Show resolved Hide resolved
src/core/runtime/dns-nodejs.js Outdated Show resolved Hide resolved
src/core/runtime/dns-nodejs.js Outdated Show resolved Hide resolved
src/core/runtime/dns-nodejs.js Outdated Show resolved Hide resolved
src/core/runtime/dns-nodejs.js Outdated Show resolved Hide resolved
Copy link
Member

@alanshaw alanshaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also if you could fix up the lint warning that would be rad:

dns-nodejs.js
  30:24  error  Missing space before function parentheses  space-before-function-paren

@alanshaw alanshaw merged commit a17253e into ipfs:master Jan 24, 2019
@chancehudson chancehudson deleted the dnslink-resolution branch January 24, 2019 16:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants