-
Notifications
You must be signed in to change notification settings - Fork 16
explore: Demonstrate and test retrieval through Kubo #304
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
base: main
Are you sure you want to change the base?
Conversation
alanshaw
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love this ❤️
We should defintely remove the TLS stuff before merging.
| r.HidePort = true | ||
| r.Use(requestLogger(log)) | ||
| r.Use(middleware.Recover()) | ||
| r.GET("/routing/v1/providers/*", func(c echo.Context) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just add this route to the existing echo server? Oh for the TLS hoop jump?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because we need to run one with TLS and one without.
| cobra.CheckErr(viper.BindPFlag("gateway.port", serveCmd.Flags().Lookup("port"))) | ||
|
|
||
| serveCmd.Flags().Int("routing-port", routingPort, "Port for delegated routing server (HTTP, no TLS). Set to 0 to disable.") | ||
| cobra.CheckErr(viper.BindPFlag("gateway.routing-port", serveCmd.Flags().Lookup("routing-port"))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No dash allowed in TOML keys (like JS).
| cobra.CheckErr(viper.BindPFlag("gateway.routing-port", serveCmd.Flags().Lookup("routing-port"))) | |
| cobra.CheckErr(viper.BindPFlag("gateway.routing_port", serveCmd.Flags().Lookup("routing-port"))) |
| cobra.CheckErr(viper.BindPFlag("gateway.log_level", serveCmd.Flags().Lookup("log-level"))) | ||
|
|
||
| serveCmd.Flags().String("tls-cert", "", "Path to TLS certificate file (enables HTTPS)") | ||
| cobra.CheckErr(viper.BindPFlag("gateway.tls-cert", serveCmd.Flags().Lookup("tls-cert"))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| cobra.CheckErr(viper.BindPFlag("gateway.tls-cert", serveCmd.Flags().Lookup("tls-cert"))) | |
| cobra.CheckErr(viper.BindPFlag("gateway.tls_cert", serveCmd.Flags().Lookup("tls-cert"))) |
| cobra.CheckErr(viper.BindPFlag("gateway.tls-cert", serveCmd.Flags().Lookup("tls-cert"))) | ||
|
|
||
| serveCmd.Flags().String("tls-key", "", "Path to TLS key file (enables HTTPS)") | ||
| cobra.CheckErr(viper.BindPFlag("gateway.tls-key", serveCmd.Flags().Lookup("tls-key"))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| cobra.CheckErr(viper.BindPFlag("gateway.tls-key", serveCmd.Flags().Lookup("tls-key"))) | |
| cobra.CheckErr(viper.BindPFlag("gateway.tls_key", serveCmd.Flags().Lookup("tls-key"))) |
| // Port is the port to run the gateway on. | ||
| Port int `mapstructure:"port" flag:"port" toml:"port"` | ||
| // TlsCert is the path to the TLS certificate file. If empty, TLS is disabled. | ||
| TlsCert string `mapstructure:"tls-cert" flag:"tls-cert" toml:"tls-cert"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| TlsCert string `mapstructure:"tls-cert" flag:"tls-cert" toml:"tls-cert"` | |
| TlsCert string `mapstructure:"tls_cert" flag:"tls-cert" toml:"tls_cert"` |
| // TlsCert is the path to the TLS certificate file. If empty, TLS is disabled. | ||
| TlsCert string `mapstructure:"tls-cert" flag:"tls-cert" toml:"tls-cert"` | ||
| // TlsKey is the path to the TLS key file. If empty, TLS is disabled. | ||
| TlsKey string `mapstructure:"tls-key" flag:"tls-key" toml:"tls-key"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| TlsKey string `mapstructure:"tls-key" flag:"tls-key" toml:"tls-key"` | |
| TlsKey string `mapstructure:"tls_key" flag:"tls-key" toml:"tls_key"` |
| TlsKey string `mapstructure:"tls-key" flag:"tls-key" toml:"tls-key"` | ||
| // RoutingPort is the port to use for delegated routing requests. Use 0 to | ||
| // disable delegated routing. | ||
| RoutingPort int `mapstructure:"routing-port" flag:"routing-port" toml:"routing-port"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| RoutingPort int `mapstructure:"routing-port" flag:"routing-port" toml:"routing-port"` | |
| RoutingPort int `mapstructure:"routing_port" flag:"routing-port" toml:"routing_port"` |
Not sure what you mean. How can it work without the TLS stuff? |
In my experience TLS termination is almost always done with a reverse proxy. i.e. run an nginx to terminate the SSL and have it proxy to the service wherever it is running. This is why I queried about the 2 echo servers and I assumed it was for local testing. Personally I would add the route to the existing echo server and caveat that for delegated routing to work the gateway needs to be deployed with an SSL cert. |
| "Schema": "peer", | ||
| "Protocols": ["transport-ipfs-gateway-http"], | ||
| "ID": "k51qzi5uqu5dj26lryc36mobgexftham120h0nu7o4ig6xu56y4h8wdvc4le6t", | ||
| "Addrs": ["/dns4/localhost/tcp/%d/tls/http"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this you'll need to use X-Forwarded-Host as /dns/${X-Forwarded-Host}/tls/http otherwise similar to what you're already doing except don't specify TLS - /dns/localhost/tcp/%d/http
Wait, is that why requests weren't working without TLS - because you had /tls in the multiaddr here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, sort of. It's because the /tls has to be there, or it won't recognize it as an address type it can use. It only looks for /tls/http addresses. If there's a plain /http, it'll ignore it.
Oh, sure. That's what I'd expect for a "real" server. Here, the intention is to use it purely as a local connection between your local I think we want to support both, so since this is a hacky setup already, maybe we should require a wrapping TLS server like nginx anyhow. Nginx isn't hard to run, and we can ship a script that sets everything up for you as long as you have Kubo and nginx installed. |
This doesn't necessarily all need to be merged, although it probably wouldn't hurt to. We do need the changes to the gateway server there to make this possible.
A little breakdown here:
localhostand tell Kubo not to verify it. That's safe, because we know we control both ends of the conversation, on the same box.