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

support http delegated content routing protocol #45

Merged
merged 6 commits into from
Dec 3, 2022

Conversation

willscott
Copy link
Member

per ipfs/specs#337

Note, there may be some follow-up extensions to this:

  • the spec specifies that a client may wish to filter on both transports received and on the contents of the multiaddr (e.g. only websocket providers)
  • the spec also has some discussion of pagination / limiting size of returned results.

This PR should be enough to provide a compatible implementation to allow interoperability testing.

server.go Outdated Show resolved Hide resolved
delegated_translator.go Outdated Show resolved Hide resolved
delegated_translator.go Outdated Show resolved Hide resolved
Protocol: proto.String(),
Peer: p.Provider.ID,
Addrs: p.Provider.Addrs,
Metadata: plb,
Copy link
Contributor

@guseggert guseggert Nov 30, 2022

Choose a reason for hiding this comment

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

the Metadata field will be dropped by the client since it is not part of the spec for the Bitswap provider record, do you need this to be accessible by the client?

Copy link
Member Author

Choose a reason for hiding this comment

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

for graphsync records, yes - for bitswap, the plb will be nil and the omitempty annotation on line 104 should result in the field not being present

@BigLep
Copy link

BigLep commented Dec 2, 2022

@guseggert : do you need to look again to give an approval?

@willscott
Copy link
Member Author

we can merge / deploy to dev to create a test endpoint

@willscott willscott requested a review from masih December 2, 2022 23:33

h := w.Header()
h.Add("Access-Control-Allow-Origin", "*")
h.Add("Access-Control-Allow-Methods", "GET, PUT, OPTIONS")
Copy link
Member

Choose a reason for hiding this comment

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

Side note: CORS will be handled by K8S ingress controller here as a cross cutting HTTP routing feature. See docs here.

Ok to leave this in code for folks whom would be running indexstar straight up 👍

@masih masih force-pushed the feat/http-delegated-routing branch from 46b7fd5 to 22f75df Compare December 3, 2022 11:34
@masih masih merged commit f76da43 into main Dec 3, 2022
@masih masih deleted the feat/http-delegated-routing branch December 3, 2022 11:43
masih added a commit to ipni/storetheindex that referenced this pull request Dec 3, 2022
Deploy support for the new HTTP delegated routing in `dev` environment
for testing purposes.

See: ipni/indexstar#45
masih added a commit to ipni/storetheindex that referenced this pull request Dec 3, 2022
Deploy support for the new HTTP delegated routing in `dev` environment
for testing purposes.

See: ipni/indexstar#45
masih added a commit that referenced this pull request Dec 3, 2022
The HTTP delegated routing uses `GET /routing/v1/providers/{CID}` to
lookup CIDs. Translate the CID lookup URI. For the translation
introduced #45 to work correctly, the URI needs to be changed to
`/cid/{CID}`.

Since delegated translator instantiates its own mux, on server handler
mapping the URI prefix needs to be stripped.
masih added a commit that referenced this pull request Dec 3, 2022
The HTTP delegated routing uses `GET /routing/v1/providers/{CID}` to
lookup CIDs. Translate the CID lookup URI. For the translation
introduced #45 to work correctly, the URI needs to be changed to
`/cid/{CID}`.

Since delegated translator instantiates its own mux, on server handler
mapping the URI prefix needs to be stripped.
@masih
Copy link
Member

masih commented Dec 3, 2022

That's the changes deployed on dev environment at cid.conctact:

curl -s https://dev.cid.contact/routing/v1/providers/bafykbzacedzugqdzfjjbd4pgsvwrpkfdpk7rzfrsgwqj2kehxv2ffjy2tlny6 | jq
{
  "Providers": [
    {
      "Protocol": "transport-bitswap",
      "ID": "12D3KooWE8yt84RVwW3sFcd6WMjbUdWrZer2YtT4dmtj3dHdahSZ",
      "Addrs": [
        "/ip4/85.11.148.122/tcp/24001"
      ],
      "Metadata": "gBI="
    },
    {
      "Protocol": "transport-graphsync-filecoinv1",
      "ID": "12D3KooWE8yt84RVwW3sFcd6WMjbUdWrZer2YtT4dmtj3dHdahSZ",
      "Addrs": [
        "/ip4/85.11.148.122/tcp/24001"
      ],
      "Metadata": "kBKjaFBpZWNlQ0lE2CpYKAABgeIDkiAgGFQCpj6i9ZqZIog4XfYgWl6FP9616K18c44shEj+IQdsVmVyaWZpZWREZWFs9G1GYXN0UmV0cmlldmFs9Q=="
    }
  ]
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants