ipfs / kubo

An IPFS implementation in Go
https://docs.ipfs.tech/how-to/command-line-quick-start/
Other
15.83k stars 2.96k forks source link

Preserve hostname specified with --api in http request headers #10233

Open softwareplumber opened 5 months ago

softwareplumber commented 5 months ago

System resolves hostname to ip address which is no use for a reverse proxy (or kubernetes ingress) which may want to route the request to a service using the hostname.

Update still checks api addr is resolvable, but doesn't resolve it, thus enabling the cli to use a remote api behind a reverse proxy.

Closes https://github.com/ipfs/kubo/issues/10232

Jorropo commented 5 months ago

Thx, could you please add a test ? (avoid regressions)

softwareplumber commented 5 months ago

Is there a testing HOWTO someplace? Make test fails for me on a clean checkout of master, even after I installed fuse and zsh. I confess one reason I raised the PR in the first place was that I hoped to see a clean test run in CI.

------ Original Message ------ From "Jorropo" @.> To "ipfs/kubo" @.> Cc "softwareplumber" @.>; "Author" @.> Date 11/25/2023 12:01:08 PM Subject Re: [ipfs/kubo] Preserve hostname specified with --api in http request headers (PR #10233)

Could you please add a test ?

— Reply to this email directly, view it on GitHub https://github.com/ipfs/kubo/pull/10233#issuecomment-1826377410, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGAPFY2GKL4OT6Y3MWZQY4DYGIP5JAVCNFSM6AAAAAA72GT5JCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMRWGM3TONBRGA. You are receiving this because you authored the thread.Message ID: @.***>

Jorropo commented 5 months ago

Screenshot_2023-11-25-20-06-09-015_com.github.android.jpg

I'm busy right now, you can try to view the generated html report.

softwareplumber commented 5 months ago

Yeah, struggling to make sense of that. Looking at the summary at the top of the html report, it would appear (for example) that package kubo/test/cli has no errors and one skipped test out of 791. If I navigate down through the package link in the report to the 'TestGateway' test in this package, it shows a bunch of errors. Are these errors expected? If I run 'go test' in test/cli it appears to run the relevant tests without a problem.

I try to duplicate the failing github workflow locally (setting some environment and then running make test/unit/gotest.junit.xml) the resulting test/unit/gotest.json doesn't seem to have any failed actions.

So I'm not sure where to go from here.

------ Original Message ------ From "Jorropo" @.> To "ipfs/kubo" @.> Cc "softwareplumber" @.>; "Author" @.> Date 11/25/2023 2:07:14 PM Subject Re: [ipfs/kubo] Preserve hostname specified with --api in http request headers (PR #10233)

Screenshot_2023-11-25-20-06-09-015_com.github.android.jpg (view on web) https://github.com/ipfs/kubo/assets/24391983/7a8d7341-b623-437c-b0a0-89a8680c8b9c

I'm busy right now, you can try to view the generated html report.

— Reply to this email directly, view it on GitHub https://github.com/ipfs/kubo/pull/10233#issuecomment-1826400126, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGAPFY2562HRMQXH4WCJKVDYGI6WFAVCNFSM6AAAAAA72GT5JCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMRWGQYDAMJSGY. You are receiving this because you authored the thread.Message ID: @.***>

hacdias commented 5 months ago

@softwareplumber sorry for the problem here. We have some flaky tests that haven't been solved - a panic somewhere causing a lot of tests to fail. I restarted the tests and everything passes.

Regarding make test: without knowing what error you got, it is hard to help debugging what went wrong. Remember that the sharness tests specifically are designed to run on Linux, so they may not work on other platforms. We are moving away from this, but it takes time.

softwareplumber commented 5 months ago

@hacdias many thanks for the help, FWIW I'm running locally under WSL2/Alma9 and I'm almost sure it is the fuse stuff that is causing problems. I will investigate further. @Jorropo if I add a test that shows connectivity between client and server with --api=/dns4/localhost/tcp/ will that be sufficient?

Jorropo commented 5 months ago

You want to add a test that would fail before but work now. As far as I can tell --api=/dns4/localhost/tcp/ works before already because it's able to translate it to /ip4/127.0.0.1/tcp/.

Ideally the test I want to see is something using the new harness test suite: https://github.com/ipfs/kubo/tree/master/test/cli

You would use net/http to setup an http server listening, then you try --api=/dns4/localhost/tcp/ and you check that in your server you indeed get Host: localhost.

I get this is a lot more involved but I can't promise you this will continue to work if we don't have a test, idk what other maintainers thinks here.

softwareplumber commented 5 months ago

:-) That's exactly what I was afraid you'd say. I was trying to dodge actually doing because of the effort involved in setting up a dummy server to inspect the headers, in a test framework I'm not particularly familiar with. It wouldn't happen quickly. Unless there is a similar test somewhere already that I can clone and modify?

------ Original Message ------ From "Jorropo" @.> To "ipfs/kubo" @.> Cc "softwareplumber" @.>; "Mention" @.> Date 11/27/2023 1:13:34 PM Subject Re: [ipfs/kubo] Preserve hostname specified with --api in http request headers (PR #10233)

You want to add a test that would fail before but work now. As far as I can tell --api=/dns4/localhost/tcp/ works before already because it's able to translate it to /ip4/localhost/tcp/.

Ideally the test I want to see is something using the new harness test suite: https://github.com/ipfs/kubo/tree/master/test/cli

You use net/http to setup an http server listening, then you try --api=/dns4/localhost/tcp/ and you check that in your server you indeed get Host: localhost.

I get this is a lot more involved but I can't promise you this will continue to work if we don't have a test, idk what other maintainers thinks here.

— Reply to this email directly, view it on GitHub https://github.com/ipfs/kubo/pull/10233#issuecomment-1828370076, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGAPFY6D7SEW4RMTNMOIOOLYGTJ45AVCNFSM6AAAAAA72GT5JCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMRYGM3TAMBXGY. You are receiving this because you were mentioned.Message ID: @.***>