Skip to content
This repository has been archived by the owner on Jun 20, 2024. It is now read-only.

feat!: IPNS routing based on IPIP-351 or IPIP-379 #185

Merged
merged 5 commits into from
Oct 5, 2023
Merged

Conversation

hacdias
Copy link
Collaborator

@hacdias hacdias commented Aug 9, 2023

Towards #151, allows resolving IPNS without KUBO_RPC_URL being set

  • adds IPNS_RECORD_GATEWAY_URL
  • uses either legacy KUBO_RPC_URL or modern IPNS_RECORD_GATEWAY_URL or PROXY_GATEWAY_URL with support for application/vnd.ipfs.ipns-record responses

Not sure what to do regarding testing here. We haven't had testing before and it seems a bit complicated to automate. I tried locally with both Kubo gateway and Routing V1 and it worked 🥳

blockstore_proxy.go Outdated Show resolved Hide resolved
main.go Show resolved Hide resolved
@hacdias hacdias requested a review from lidel August 9, 2023 09:49
@hacdias hacdias self-assigned this Aug 9, 2023
@lidel lidel changed the title feat!: IPNS routing based on ?format=ipns-record or /routing/v1 feat!: IPNS routing based on IPIP-351 or IPIP-379 Aug 9, 2023
Copy link
Collaborator

@lidel lidel left a comment

Choose a reason for hiding this comment

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

Thank you for cleaning this up.
Perhaps we could clean up bit more – see DefaultKuboRPC comment inline.

We haven't had testing before and it seems a bit complicated to automate. I tried locally with both Kubo gateway and Routing V1 and it worked partying_face

I think this is already handled by gateway-conformance, it runs in CI and since #181 we also test IPNS content paths, including raw ipns-record fixtures imported in .github/workflows/gateway-conformance.yml.

So no need for additional testing imo. 👍

docs/environment-variables.md Outdated Show resolved Hide resolved
docs/environment-variables.md Outdated Show resolved Hide resolved
docs/environment-variables.md Outdated Show resolved Hide resolved
docs/environment-variables.md Outdated Show resolved Hide resolved
routing.go Outdated Show resolved Hide resolved
blockstore_proxy.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@lidel lidel left a comment

Choose a reason for hiding this comment

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

@hacdias thanks! two asks inline 🙏

routing.go Show resolved Hide resolved
routing.go Outdated Show resolved Hide resolved
@hacdias
Copy link
Collaborator Author

hacdias commented Sep 18, 2023

Ping @lidel

Copy link
Collaborator

@lidel lidel left a comment

Choose a reason for hiding this comment

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

lgtm, pushed cosmetic fix for better ux and rebased

Note: this should be safe to merge, as it does not impact Rhea deployments which all have KUBO_RPC_URL set:

https://github.com/ipfs/bifrost-gateway/blob/04eb5db2f270271b0675de1a90708d2d173dd780/Dockerfile#L34

@hacdias hacdias merged commit f0bcbcb into main Oct 5, 2023
12 checks passed
@hacdias hacdias deleted the issue-151 branch October 5, 2023 07:07
@BigLep BigLep mentioned this pull request Nov 9, 2023
11 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants