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

feat: trustless-only mode (RAINBOW_TRUSTLESS_GATEWAY_DOMAINS) #81

Merged
merged 5 commits into from
Feb 14, 2024

Conversation

hacdias
Copy link
Member

@hacdias hacdias commented Feb 9, 2024

Closes #71.

I made it such that a trustless gateway can also be a subdomain gateway, if you add the domain to both options.

@hacdias hacdias requested a review from lidel February 9, 2024 11:15
@hacdias hacdias self-assigned this Feb 9, 2024
Copy link
Member

@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.

Thanks, works as expected.

I've expanded docs a bit, making it very clear to which response types we limit on RAINBOW_TRUSTLESS_GATEWAY_DOMAINS.

This is because many people will learn about them by having to deploy IPFS with rainbow, making it a good educational oportunity (cc @2color for another set of eyes at docs/ here)

RAINBOW_TRUSTLESS_GATEWAY_DOMAINS flag needs basic end-to-end regression test

This flag will protect people from liability around web hosting of deserialized responses (phishing, hostlinking etc), we can't risk it stopping working and people get hit by regressions/slips like the one we had during early days of trustless-gateway.link.

Before we merge this, we must have end-to-end test that confirms RAINBOW_TRUSTLESS_GATEWAY_DOMAINS works.

I think mvp test is to run with RAINBOW_TRUSTLESS_GATEWAY_DOMAINS=trustless-gateway.link and then confirm

  • curl -H "Host: trustless-gateway.link" http://127.0.0.1:8090/ipfs/bafybeigdyrzt5sfp7udm7hu76uh7y26nf3efuylqabf3oclgtqy55fbzdi -s -i returns HTTP 406
  • curl -H "Host: trustless-gateway.link" "http://127.0.0.1:8090/ipfs/bafybeigdyrzt5sfp7udm7hu76uh7y26nf3efuylqabf3oclgtqy55fbzdi?format=raw" -s -i and curl -H "Host: trustless-gateway.link" -H "Accept: application/vnd.ipld.raw" http://127.0.0.1:8090/ipfs/bafybeigdyrzt5sfp7udm7hu76uh7y26nf3efuylqabf3oclgtqy55fbzdi -s -i both return HTTP 200 with Content-Type: application/vnd.ipld.raw

This has to be end-to-end test specific to rainbow, because gateway-conformance tests won't catch regression when RAINBOW_TRUSTLESS_GATEWAY_DOMAINS stops disabling deserialized responses.

handlers.go Outdated Show resolved Hide resolved
@lidel lidel requested a review from 2color February 11, 2024 22:06
@hacdias hacdias force-pushed the trustless-gateway branch 3 times, most recently from c4e4abb to b2503d7 Compare February 13, 2024 11:31
@hacdias hacdias requested review from lidel February 13, 2024 12:19
@hacdias
Copy link
Member Author

hacdias commented Feb 13, 2024

@lidel I've added the trustless test.

hacdias and others added 4 commits February 13, 2024 17:28
Co-authored-by: Daniel Norman <1992255+2color@users.noreply.github.com>
Co-authored-by: Marcin Rataj <lidel@lidel.org>
this ensures configured domains are logged suring startup
Copy link
Member

@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, thanks! This closes #71

Next step is to fix trustless-gateway.link by setting RAINBOW_TRUSTLESS_GATEWAY_DOMAINS in our infra and removing nginx filtering.

Comment on lines +342 to +347
fmt.Printf("IPFS Gateway listening at %s\n\n", gatewayListen)

printIfListConfigured(" RAINBOW_GATEWAY_DOMAINS = ", cfg.GatewayDomains)
printIfListConfigured(" RAINBOW_SUBDOMAIN_GATEWAY_DOMAINS = ", cfg.SubdomainGatewayDomains)
printIfListConfigured(" RAINBOW_TRUSTLESS_GATEWAY_DOMAINS = ", cfg.TrustlessGatewayDomains)
printIfListConfigured(" Legacy RPC at /api/v0 will redirect to KUBO_RPC_URL = ", cfg.KuboRPCURLs)
Copy link
Member

Choose a reason for hiding this comment

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

ℹ️ I've added this to make it easy to eyeball in logs/terminal if/when specific domains got set up correctly.

Comment on lines +158 to +159
ts, gnd := mustTestServer(t, Config{
TrustlessGatewayDomains: []string{"trustless.com"},
Copy link
Member

Choose a reason for hiding this comment

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

💭 should be good enough to merge, but not a real end-to-end, because we can break the way config gets set via RAINBOW_TRUSTLESS_GATEWAY_DOMAINS or CLI parameter.

Filled #89 to figure out better end-to-end test setup for config (not just this feature).

@lidel lidel changed the title feat: trustless gateway option feat: trustless-only mode (RAINBOW_TRUSTLESS_GATEWAY_DOMAINS) Feb 14, 2024
@lidel lidel merged commit 2cecd47 into main Feb 14, 2024
10 checks passed
@lidel lidel deleted the trustless-gateway branch February 14, 2024 23:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Add RAINBOW_TRUSTLESS_GATEWAY_DOMAINS=trustless-gateway.link
3 participants