mirror of
https://github.com/ipfs/kubo.git
synced 2026-02-21 10:27:46 +08:00
fix(routing): use LegacyProvider for HTTP-only custom routing (#11112)
* fix(routing): use LegacyProvider for HTTP-only custom routing when `Routing.Type=custom` with only HTTP routers and no DHT, fall back to LegacyProvider instead of SweepingProvider. SweepingProvider requires a DHT client which is unavailable in HTTP-only configurations, causing it to return NoopProvider and breaking provider record announcements to HTTP routers. fixes #11089 * test(routing): verify provide stat works with HTTP-only routing * docs(config): clarify SweepEnabled fallback for HTTP-only routing --------- Co-authored-by: Andrew Gillis <11790789+gammazero@users.noreply.github.com>
This commit is contained in:
parent
aa3c88dcdd
commit
23ba660ef0
@ -692,6 +692,48 @@ See docs: https://github.com/ipfs/kubo/blob/master/docs/config.md#providedhtmaxw
|
||||
|
||||
// ONLINE/OFFLINE
|
||||
|
||||
// hasDHTRouting checks if the routing configuration includes a DHT component.
|
||||
// Returns false for HTTP-only custom routing configurations (e.g., Routing.Type="custom"
|
||||
// with only HTTP routers). This is used to determine whether SweepingProviderOpt
|
||||
// can be used, since it requires a DHT client.
|
||||
func hasDHTRouting(cfg *config.Config) bool {
|
||||
routingType := cfg.Routing.Type.WithDefault(config.DefaultRoutingType)
|
||||
switch routingType {
|
||||
case "auto", "autoclient", "dht", "dhtclient", "dhtserver":
|
||||
return true
|
||||
case "custom":
|
||||
// Check if any router in custom config is DHT-based
|
||||
for _, router := range cfg.Routing.Routers {
|
||||
if routerIncludesDHT(router, cfg) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
default: // "none", "delegated"
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
// routerIncludesDHT recursively checks if a router configuration includes DHT.
|
||||
// Handles parallel and sequential composite routers by checking their children.
|
||||
func routerIncludesDHT(rp config.RouterParser, cfg *config.Config) bool {
|
||||
switch rp.Type {
|
||||
case config.RouterTypeDHT:
|
||||
return true
|
||||
case config.RouterTypeParallel, config.RouterTypeSequential:
|
||||
if children, ok := rp.Parameters.(*config.ComposableRouterParams); ok {
|
||||
for _, child := range children.Routers {
|
||||
if childRouter, exists := cfg.Routing.Routers[child.RouterName]; exists {
|
||||
if routerIncludesDHT(childRouter, cfg) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// OnlineProviders groups units managing provide routing records online
|
||||
func OnlineProviders(provide bool, cfg *config.Config) fx.Option {
|
||||
if !provide {
|
||||
@ -708,7 +750,15 @@ func OnlineProviders(provide bool, cfg *config.Config) fx.Option {
|
||||
opts := []fx.Option{
|
||||
fx.Provide(setReproviderKeyProvider(providerStrategy)),
|
||||
}
|
||||
if cfg.Provide.DHT.SweepEnabled.WithDefault(config.DefaultProvideDHTSweepEnabled) {
|
||||
|
||||
sweepEnabled := cfg.Provide.DHT.SweepEnabled.WithDefault(config.DefaultProvideDHTSweepEnabled)
|
||||
dhtAvailable := hasDHTRouting(cfg)
|
||||
|
||||
// Use SweepingProvider only when both sweep is enabled AND DHT is available.
|
||||
// For HTTP-only routing (e.g., Routing.Type="custom" with only HTTP routers),
|
||||
// fall back to LegacyProvider which works with ProvideManyRouter.
|
||||
// See https://github.com/ipfs/kubo/issues/11089
|
||||
if sweepEnabled && dhtAvailable {
|
||||
opts = append(opts, SweepingProviderOpt(cfg))
|
||||
} else {
|
||||
reprovideInterval := cfg.Provide.DHT.Interval.WithDefault(config.DefaultProvideDHTInterval)
|
||||
|
||||
@ -2195,6 +2195,9 @@ You can compare the effectiveness of sweep mode vs legacy mode by monitoring the
|
||||
> [!NOTE]
|
||||
> This is the default provider system as of Kubo v0.39. To use the legacy provider instead, set `Provide.DHT.SweepEnabled=false`.
|
||||
|
||||
> [!NOTE]
|
||||
> When DHT routing is unavailable (e.g., `Routing.Type=custom` with only HTTP routers), the provider automatically falls back to the legacy provider regardless of this setting.
|
||||
|
||||
Default: `true`
|
||||
|
||||
Type: `flag`
|
||||
|
||||
@ -7,6 +7,7 @@ import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
@ -764,3 +765,81 @@ func TestProvider(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestHTTPOnlyProviderWithSweepEnabled tests that provider records are correctly
|
||||
// sent to HTTP routers when Routing.Type="custom" with only HTTP routers configured,
|
||||
// even when Provide.DHT.SweepEnabled=true (the default since v0.39).
|
||||
//
|
||||
// This is a regression test for https://github.com/ipfs/kubo/issues/11089
|
||||
func TestHTTPOnlyProviderWithSweepEnabled(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Track provide requests received by the mock HTTP router
|
||||
var provideRequests atomic.Int32
|
||||
mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if (r.Method == http.MethodPut || r.Method == http.MethodPost) &&
|
||||
strings.HasPrefix(r.URL.Path, "/routing/v1/providers") {
|
||||
provideRequests.Add(1)
|
||||
w.WriteHeader(http.StatusOK)
|
||||
} else if strings.HasPrefix(r.URL.Path, "/routing/v1/providers") && r.Method == http.MethodGet {
|
||||
// Return empty providers for findprovs
|
||||
w.Header().Set("Content-Type", "application/x-ndjson")
|
||||
w.WriteHeader(http.StatusOK)
|
||||
} else {
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
}
|
||||
}))
|
||||
defer mockServer.Close()
|
||||
|
||||
h := harness.NewT(t)
|
||||
node := h.NewNode().Init()
|
||||
|
||||
// Explicitly set SweepEnabled=true (the default since v0.39, but be explicit for test clarity)
|
||||
node.SetIPFSConfig("Provide.DHT.SweepEnabled", true)
|
||||
node.SetIPFSConfig("Provide.Enabled", true)
|
||||
|
||||
// Configure HTTP-only custom routing (no DHT) with explicit Routing.Type=custom
|
||||
routingConf := map[string]any{
|
||||
"Type": "custom", // Explicitly set Routing.Type=custom
|
||||
"Methods": map[string]any{
|
||||
"provide": map[string]any{"RouterName": "HTTPRouter"},
|
||||
"get-ipns": map[string]any{"RouterName": "HTTPRouter"},
|
||||
"put-ipns": map[string]any{"RouterName": "HTTPRouter"},
|
||||
"find-peers": map[string]any{"RouterName": "HTTPRouter"},
|
||||
"find-providers": map[string]any{"RouterName": "HTTPRouter"},
|
||||
},
|
||||
"Routers": map[string]any{
|
||||
"HTTPRouter": map[string]any{
|
||||
"Type": "http",
|
||||
"Parameters": map[string]any{
|
||||
"Endpoint": mockServer.URL,
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
node.SetIPFSConfig("Routing", routingConf)
|
||||
node.StartDaemon()
|
||||
defer node.StopDaemon()
|
||||
|
||||
// Add content and manually provide it
|
||||
cid := node.IPFSAddStr(time.Now().String())
|
||||
|
||||
// Manual provide should succeed even without libp2p peers
|
||||
res := node.RunIPFS("routing", "provide", cid)
|
||||
// Check that the command succeeded (exit code 0) and no provide-related errors
|
||||
assert.Equal(t, 0, res.ExitCode(), "routing provide should succeed with HTTP-only routing and SweepEnabled=true")
|
||||
assert.NotContains(t, res.Stderr.String(), "cannot provide", "should not have provide errors")
|
||||
|
||||
// Verify HTTP router received at least one provide request
|
||||
assert.Greater(t, provideRequests.Load(), int32(0),
|
||||
"HTTP router should have received provide requests")
|
||||
|
||||
// Verify 'provide stat' works with HTTP-only routing (regression test for stats)
|
||||
statRes := node.RunIPFS("provide", "stat")
|
||||
assert.Equal(t, 0, statRes.ExitCode(), "provide stat should succeed with HTTP-only routing")
|
||||
assert.NotContains(t, statRes.Stderr.String(), "stats not available",
|
||||
"should not report stats unavailable")
|
||||
// LegacyProvider outputs "TotalReprovides:" in its stats
|
||||
assert.Contains(t, statRes.Stdout.String(), "TotalReprovides:",
|
||||
"should show legacy provider stats")
|
||||
}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user