kubo icon indicating copy to clipboard operation
kubo copied to clipboard

Preserve hostname specified with --api in http request headers

Open softwareplumber opened this issue 1 year ago • 8 comments
trafficstars

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

softwareplumber avatar Nov 25 '23 16:11 softwareplumber

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

Jorropo avatar Nov 25 '23 17:11 Jorropo

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

softwareplumber avatar Nov 25 '23 18:11 softwareplumber

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.

Jorropo avatar Nov 25 '23 19:11 Jorropo

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

softwareplumber avatar Nov 26 '23 04:11 softwareplumber

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

hacdias avatar Nov 27 '23 09:11 hacdias

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

softwareplumber avatar Nov 27 '23 14:11 softwareplumber

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.

Jorropo avatar Nov 27 '23 18:11 Jorropo

:-) 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: @.***>

softwareplumber avatar Nov 27 '23 19:11 softwareplumber