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.

Sign up for GitHub

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

feat(http-routers): filter-protocols from IPIP-484 #173

Merged
merged 1 commit into from
Oct 16, 2024
Merged

feat(http-routers): filter-protocols from IPIP-484 #173

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ The following emojis are used to highlight certain changes:
## [Unreleased]

### Added
- Ability to specify the maximum blocksize that bitswap will replace WantHave with WantBlock responses, and to disable replacement when set to zero. [#165](https://github.com/ipfs/rainbow/pull/165)
- Support use and configuration of pebble as [datastore](https://github.com/ipfs/rainbow/blob/main/docs/blockstores.md). Pebble provides a high-performance alternative to badger. Options are available to configure key tuning parameters (`pebble-*` in `rainbow --help`).

- Support implicit default list for `filter-protocols` from [IPIP-484](https://github.com/ipfs/specs/pull/484) and customizing them via `--http-routers-filter-protocols`.

### Changed

Expand All @@ -25,6 +25,13 @@ The following emojis are used to highlight certain changes:

### Security

## [v1.7.0]

### Added

- Ability to specify the maximum blocksize that bitswap will replace WantHave with WantBlock responses, and to disable replacement when set to zero. [#165](https://github.com/ipfs/rainbow/pull/165)
- Support use and configuration of pebble as [datastore](https://github.com/ipfs/rainbow/blob/main/docs/blockstores.md). Pebble provides a high-performance alternative to badger. Options are available to configure key tuning parameters (`pebble-*` in `rainbow --help`).

## [v1.6.0]

### Added
Expand Down
7 changes: 7 additions & 0 deletions main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,12 @@
EnvVars: []string{"RAINBOW_HTTP_ROUTERS"},
Usage: "HTTP servers with /routing/v1 endpoints to use for delegated routing (comma-separated)",
},
&cli.StringSliceFlag{
Name: "http-routers-filter-protocols",
Value: cli.NewStringSlice(httpRoutersFilterProtocols...),
EnvVars: []string{"RAINBOW_HTTP_ROUTERS_FILTER_PROTOCOLS"},
Usage: "IPIP-484 filter-protocols to apply to delegated routing requests (comma-separated)",
},
&cli.StringFlag{
Name: "dht-routing",
Value: "accelerated",
Expand Down Expand Up @@ -502,6 +508,7 @@
MaxFD: cctx.Int("libp2p-max-fd"),
InMemBlockCache: cctx.Int64("inmem-block-cache"),
RoutingV1Endpoints: cctx.StringSlice("http-routers"),
RoutingV1FilterProtocols: cctx.StringSlice("http-routers-filter-protocols"),

Check warning on line 511 in main.go

View check run for this annotation

Codecov / codecov/patch

main.go#L511 

Added line #L511 was not covered by tests
DHTRouting: dhtRouting,
DHTSharedHost: cctx.Bool("dht-shared-host"),
Bitswap: bitswap,
Expand Down
19 changes: 11 additions & 8 deletions setup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ func init() {

const cidContactEndpoint = "https://cid.contact"

var httpRoutersFilterProtocols = []string{"unknown", "transport-bitswap"} // IPIP-484

type DHTRouting string

const (
Expand Down Expand Up @@ -100,14 +102,15 @@ type Config struct {
MaxMemory uint64
MaxFD int

GatewayDomains []string
SubdomainGatewayDomains []string
TrustlessGatewayDomains []string
RoutingV1Endpoints []string
DHTRouting DHTRouting
DHTSharedHost bool
IpnsMaxCacheTTL time.Duration
Bitswap bool
GatewayDomains []string
SubdomainGatewayDomains []string
TrustlessGatewayDomains []string
RoutingV1Endpoints []string
RoutingV1FilterProtocols []string
DHTRouting DHTRouting
DHTSharedHost bool
IpnsMaxCacheTTL time.Duration
Bitswap bool

// BitswapWantHaveReplaceSize tells the bitswap server to replace WantHave
// with WantBlock responses when the block size less then or equal to this
Expand Down
13 changes: 7 additions & 6 deletions setup_routing.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,12 @@
)

for _, endpoint := range cfg.RoutingV1Endpoints {
rv1Opts := []routingv1client.Option{routingv1client.WithHTTPClient(httpClient)}
if endpoint != cidContactEndpoint {
rv1Opts = append(rv1Opts, routingv1client.WithStreamResultsRequired())
}
delegatedRouter, err := delegatedHTTPContentRouter(endpoint, rv1Opts...)
delegatedRouter, err := delegatedHTTPContentRouter(endpoint,
routingv1client.WithHTTPClient(httpClient),
routingv1client.WithProtocolFilter(cfg.RoutingV1FilterProtocols), // IPIP-484
routingv1client.WithStreamResultsRequired(), // https://specs.ipfs.tech/routing/http-routing-v1/#streaming
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of scope for this PR, but...

According to the docs,

server MAY respond with non-streaming application/json response even if the client requested streaming

So, would his option be better named WithStreamResultsRequested or WithStreamResultsAccepted?

Copy link
Member Author

@lidel lidel Oct 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, imo the name is confusing and redundant.

The default accepts both here, and specs says you should always return JSON as fallback anyway.
(afaik this config was onl yadded because cid.contact errored on streaming requests, but it was fixed since then)

routingv1client.WithDisabledLocalFiltering(false), // force local filtering in case remote server does not support IPIP-484
)

Check warning on line 60 in setup_routing.go

View check run for this annotation

Codecov / codecov/patch

setup_routing.go#L55-L60 

Added lines #L55 - L60 were not covered by tests
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -276,7 +277,7 @@
cli, err := routingv1client.New(
endpoint,
append([]routingv1client.Option{
routingv1client.WithUserAgent(buildVersion()),
routingv1client.WithUserAgent("rainbow/" + buildVersion()),

Check warning on line 280 in setup_routing.go

View check run for this annotation

Codecov / codecov/patch

setup_routing.go#L280 

Added line #L280 was not covered by tests
}, rv1Opts...)...,
)
if err != nil {
Expand Down
Loading