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

fix: regression in 'ipfs dns' #10212

Merged
merged 1 commit into from
Nov 9, 2023
Merged

fix: regression in 'ipfs dns' #10212

merged 1 commit into from
Nov 9, 2023

Conversation

lidel
Copy link
Member

@lidel lidel commented Nov 8, 2023

Seems we've introduced regression to the deprecated dns command, it now requires /ipns/ path, which breaks every software which used it for DNSLink lookups.

It was already deprecated, so no need for patch release, but let's fix it and improve helptext that is displayed at https://docs.ipfs.tech/reference/kubo/rpc/#api-v0-dns when the next release ships.

before

$ ipfs dns dist.ipfs.tech
Error: invalid path "dist.ipfs.tech": path does not have enough components

expected behavior (this fix)

$ ipfs dns dist.ipfs.tech
/ipfs/QmXZX3Jiw2p1DKyShCKCbfNBCEL7eYW57B1asPr8FKPPmm

@lidel lidel added the skip/changelog This change does NOT require a changelog entry label Nov 8, 2023
@lidel lidel requested a review from hacdias November 8, 2023 20:34
@lidel lidel requested a review from a team as a code owner November 8, 2023 20:34
Copy link
Contributor

@Jorropo Jorropo left a comment

Choose a reason for hiding this comment

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

Is there zero tests which can catch this ?
I'm having a hard time we let something like that slide.

lidel added a commit to ipfs/ipfs-companion that referenced this pull request Nov 8, 2023
ipfs.dns is deprecated, and broken in Kubo 0.24:
ipfs/kubo#10212
lidel added a commit to ipfs/ipfs-companion that referenced this pull request Nov 8, 2023
ipfs.dns is deprecated, and broken in Kubo 0.24:
ipfs/kubo#10212
this command used to work with domain without `/ipns/` prefix.

we've switched it to the same backend as `resolve` command,
which requires the prefix, so we add it if it is missing
@hacdias
Copy link
Member

hacdias commented Nov 9, 2023

Is there zero tests which can catch this ?

It seems so. This command has been deprecated for over 1 year and a half. The fact that no one complained or we didn't see things failing also shows that it's likely not used. Wouldn't be surprised if we end up removing it in some months.

This command is also very hard to test! It uses DNS resolver directly and you can't override resolution via environment variables.

@hacdias hacdias merged commit 670ce70 into master Nov 9, 2023
23 checks passed
@hacdias hacdias deleted the fix/dns-command branch November 9, 2023 09:23
lidel added a commit to ipfs/ipfs-companion that referenced this pull request Jan 24, 2024
ipfs.dns is deprecated, and broken in Kubo 0.24:
ipfs/kubo#10212
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip/changelog This change does NOT require a changelog entry
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants