Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate /api/v0/dns #8607

Closed
lidel opened this issue Dec 14, 2021 · 2 comments · Fixed by #8893
Closed

Deprecate /api/v0/dns #8607

lidel opened this issue Dec 14, 2021 · 2 comments · Fixed by #8893
Labels
exp/intermediate Prior experience is likely helpful good first issue Good issue for new contributors help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws) P3 Low: Not priority right now topic/dns topic/docs-ipfs Topic docs-ipfs
Milestone

Comments

@lidel
Copy link
Member

lidel commented Dec 14, 2021

Problem

  • /api/v0/dns does not work with DNSLinks that have /ipns/.. content paths (/api/v0/dns/ fails with "not a valid domain name" error on IPNS DNSLinks #6454), and /api/v0/resolve works fine will all content paths.
    • IPFS companion switched its DNSLink detection to /api/v0/resolve a while ago. It is passing DNSLink names as content paths: /ipns/example.com and they work fine no matter if IPNS is used or not.
  • Having two ways of resolving DNSLink is confusing enough, having one that fails when content path is /ipns/ is simply harmful, and even if we fix that, adds unnecessary complexity to the API surface.
    • In reality, users just want to resolve a content path. They dont care if DNS or IPNS is involved. What matters is resolving content path. Either one level, or all the way, until immutable /ipfs/{cid} is hit.

Proposed solution

Deprecate /api/v0/dns:

  • remove it from cli --help and hide from API docs
  • to ensure nothng breaks, keep /api/v0/dns endpoint, but remove its custom DNS lookup code, and switch it to use /api/v0/resolve internally
@lidel lidel added kind/bug A bug in existing code (including security flaws) topic/docs-ipfs Topic docs-ipfs need/triage Needs initial labeling and prioritization topic/dns labels Dec 14, 2021
@aschmahmann aschmahmann added exp/intermediate Prior experience is likely helpful P3 Low: Not priority right now and removed need/triage Needs initial labeling and prioritization labels Feb 18, 2022
@aschmahmann
Copy link
Contributor

@lidel do we have any utility for DNSAddr resolution from a go-ipfs command here? Otherwise, this makes sense since we already have ipfs name resolve and ipfs resolve

@BigLep BigLep added this to the TBD milestone Mar 10, 2022
@lidel
Copy link
Member Author

lidel commented Apr 15, 2022

There is no utility for resolving /dnsaddr, and DNSLink works better with ipfs resolve -r anyway.

I think it is safe for this to be be deprecated like we did with tar commands in https://github.com/ipfs/go-ipfs/pull/8849/files

@lidel lidel modified the milestones: TBD, go-ipfs 0.13, go-ipfs 0.14 Apr 15, 2022
@lidel lidel added help wanted Seeking public contribution on this issue good first issue Good issue for new contributors labels Apr 15, 2022
lidel added a commit that referenced this issue Apr 15, 2022
lidel added a commit that referenced this issue Apr 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exp/intermediate Prior experience is likely helpful good first issue Good issue for new contributors help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws) P3 Low: Not priority right now topic/dns topic/docs-ipfs Topic docs-ipfs
Projects
No open projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants