feat: autoconf support #123
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Set delegated routing endpoint using environ var - Fix autoconf disabled error message
Member
|
Triage note:
|
20 tasks
ensures autoconf client uses mainnet fallback config when server is unreachable or cache is empty
replaces dht.GetDefaultBootstrapPeerAddrInfos with autoconf.FallbackBootstrapPeers to align with kubo's approach
changes '--endpoing' to '--endpoint'
- set all three endpoint types (provider, peer, IPNS) to default to 'auto' - expand and validate endpoint URLs per flag to support custom configurations - accept both base URLs (https://example.com) and full URLs with routing paths - strip routing paths after validation to get base URLs for HTTP client - validate path matches flag type, error on mismatches with helpful messages - update stdout to show routing paths explicitly (/routing/v1/providers) - add spec references to Routing V1 and IPFS Mainnet in docs - simplify documentation following godoc style This ensures endpoints only receive requests they support (e.g., cid.contact only gets provider requests, not peers or IPNS).
when the same base URL appears in multiple endpoint configs (provider/peer/ipns), create only ONE HTTP client instead of three separate clients with duplicate connection pools this was not an issue with cid.contact as it only supported /providers, but is necessary to future-proof if autoconf provides more routing systems in the future implementation: - client_delegated_routing.go: new file for delegated routing client logic - collectEndpoints: deduplicates URLs and aggregates capabilities - createDelegatedHTTPRouters: creates one client per unique base URL and registers metrics once for all delegated HTTP clients - combineRouters: simplified to combine routers without client creation - clientRouter: moved from server_routers.go to co-locate with client creation same client instance is reused across routing types when base URL matches, significantly reducing memory and connection overhead
- add file-level godoc to autoconf.go - fix parameter shadowing in createAutoConfClient - add validation for empty strings in collectEndpoints - add tests for collectEndpoints deduplication logic - add trailing slash normalization tests
- inline autoconfDisabledError (single use function) - add newDelegatedRoutingClient helper for consistent HTTP client creation - use helper in both server and CLI code to ensure consistent options - rename client_delegated_routing.go to server_delegated_routing.go - add file-level documentation explaining server-side delegated routing
lidel
approved these changes
Nov 19, 2025
Member
There was a problem hiding this comment.
Thank you @gammazero!
The autoconf implementation was really solid and basically feature-complete for how things (cid.contact) work today. I just tweaked a few things to match Kubo's configuration patterns better and be more future-proof if we have more routing systems in the future (so they are picked up without us having to do any code changes again):
- Wired the endpoint configuration into all three flags (added to
--peer-endpoints,--ipns-endpoints). - Moved things around to avoid creating duplicate HTTP clients when same URL appears in multiple places (e.g. providers, peers and ipns). Now we create one client per base URL and reuse across routing types (helps with HTTP/2+ mux).
- Both issues above were not a problem today since
cid.contactonly supports/routing/v1/providers, but future-proofs us for multi-capability endpoints. - Added some tests and docs
If this looks good, feel free to merge, I plan to make release later this week if no concerns.
gammazero
commented
Nov 19, 2025
lidel
added a commit
that referenced
this pull request
Nov 19, 2025
…sabled fixes two issues with the GetClosestPeers endpoint: 1. endpoint returned HTTP 200 with empty results instead of actual DHT peers 2. when DHT disabled, returned HTTP 200 with empty results instead of error root cause: during rebase of PR #124 (commit 0dad3f7) when integrating with autoconf refactor (PR #123 / ec76365), the dhtRouters initialization was accidentally omitted from server.go. the autoconf PR renamed getCombinedRouting to combineRouters and changed its signature, but the rebase failed to preserve the dhtRouters creation line that existed in commit 42bd221. changes: - server.go:201-208: add explicit dhtRouters creation with caching and sanitization, similar to original 42bd221 implementation - server.go:232: wire dhtRouters to composableRouter.dht field - server.go:338: update combineRouters signature to accept host.Host parameter needed for GetClosestPeers peerstore lookups - server_routers.go:73-77: return routing.ErrNotSupported instead of empty iterator when DHT is nil, resulting in HTTP 500 instead of misleading HTTP 200 - server_test.go:16,20,24: update combineRouters test calls with new signature - server_dht_test.go:355-379: add test verifying HTTP 500 when DHT disabled
hsanjuan
pushed a commit
that referenced
this pull request
Nov 20, 2025
…sabled (#127) fixes two issues with the GetClosestPeers endpoint: 1. endpoint returned HTTP 200 with empty results instead of actual DHT peers 2. when DHT disabled, returned HTTP 200 with empty results instead of error root cause: during rebase of PR #124 (commit 0dad3f7) when integrating with autoconf refactor (PR #123 / ec76365), the dhtRouters initialization was accidentally omitted from server.go. the autoconf PR renamed getCombinedRouting to combineRouters and changed its signature, but the rebase failed to preserve the dhtRouters creation line that existed in commit 42bd221. changes: - server.go:201-208: add explicit dhtRouters creation with caching and sanitization, similar to original 42bd221 implementation - server.go:232: wire dhtRouters to composableRouter.dht field - server.go:338: update combineRouters signature to accept host.Host parameter needed for GetClosestPeers peerstore lookups - server_routers.go:73-77: return routing.ErrNotSupported instead of empty iterator when DHT is nil, resulting in HTTP 500 instead of misleading HTTP 200 - server_test.go:16,20,24: update combineRouters test calls with new signature - server_dht_test.go:355-379: add test verifying HTTP 500 when DHT disabled
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.