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

Start API on both IPv4 and IPv6 #9637

Open
3 tasks done
Tracked by #10000
Rishi556 opened this issue Feb 9, 2023 · 5 comments · May be fixed by #9788
Open
3 tasks done
Tracked by #10000

Start API on both IPv4 and IPv6 #9637

Rishi556 opened this issue Feb 9, 2023 · 5 comments · May be fixed by #9788
Labels
dif/easy effort/hours Estimated to take one or several hours kind/enhancement A net-new feature or improvement to an existing feature P1 High: Likely tackled by core team if no one steps up status/blocked Unable to be worked further until needs are met

Comments

@Rishi556
Copy link

Rishi556 commented Feb 9, 2023

Checklist

  • My issue is specific & actionable.
  • I am not suggesting a protocol enhancement.
  • I have searched on the issue tracker for my issue.

Description

By default, Kubo only listens on 127.0.0.1:5001 for api connections. With IPv6 usage growing, listening on [::1]:5001 as well should be default behavior.

@Rishi556 Rishi556 added the kind/enhancement A net-new feature or improvement to an existing feature label Feb 9, 2023
@Jorropo
Copy link
Contributor

Jorropo commented Feb 10, 2023

It's localhost, I don't belive this is an issue.

It's still intresting to solve just because we would need to make API accept a list of listeners (it might already idk).

@Rishi556
Copy link
Author

It does cause issues since Node.js uses IPv6 for localhost by default (dns is verbatim from os, not reordered to have IPv4 at the start) from version 18 onward. It would be helpful for Kubo to listen on both stacks to ensure that doesn't cause problems for anyone else like it did for me.

@Jorropo
Copy link
Contributor

Jorropo commented Feb 10, 2023

I'm surprised that nodejs does this, ¯\_(ツ)_/¯ I guess we gotta fix this then.

@Jorropo Jorropo added P1 High: Likely tackled by core team if no one steps up help wanted Seeking public contribution on this issue effort/hours Estimated to take one or several hours dif/easy labels Feb 10, 2023
@Rishi556
Copy link
Author

Rishi556 commented Apr 2, 2023

This looks like it's pretty simple to fix, by changing the Addresses section of the config to have an array of addresses with both v4 and v6. I'll see if I can find where in the code this is done at.

  "Addresses": {
    "API": ["/ip4/127.0.0.1/tcp/5001", "/ip6/::1/tcp/5001"],
    "Gateway": ["/ip4/127.0.0.1/tcp/8080","/ip6/::1/tcp/8080"]
  }

@Rishi556 Rishi556 linked a pull request Apr 2, 2023 that will close this issue
@lidel
Copy link
Member

lidel commented May 23, 2023

Exposing more than one listener is blocked by two bugs:

We need to solve them first, before merging #9788

@BigLep BigLep mentioned this issue Aug 3, 2023
@Jorropo Jorropo added status/blocked Unable to be worked further until needs are met and removed help wanted Seeking public contribution on this issue labels Aug 17, 2023
gammazero added a commit that referenced this issue Aug 21, 2024
When there are multiple listeners configured for Addresses.API, serving metrics results in an errors: "<metric> was collected before with the same name and label values".

This PR fixes this by maintaining a global map of metrics handlers, and only creating and reginstering them once. The same metrics handlers are provided to the mux for every listener.

Fixes #9891
Fixes #9397

Unblocks #9637
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dif/easy effort/hours Estimated to take one or several hours kind/enhancement A net-new feature or improvement to an existing feature P1 High: Likely tackled by core team if no one steps up status/blocked Unable to be worked further until needs are met
Projects
No open projects
Status: 🔎 In Review
Development

Successfully merging a pull request may close this issue.

3 participants