search

containerd / rust-extensions

Rust crates to extend containerd
https://containerd.io
Apache License 2.0
172 stars 66 forks source link

fix(client): if tonic has tls feature #264

Closed fcfangcc closed 4 months ago

fcfangcc commented 4 months ago

fixed #261

In tonic, if uri startwith "https", will use tls connect.

https://github.com/hyperium/tonic/blob/eeb3268f71ae5d1107c937392389db63d8f721fb/tonic/src/transport/service/connector.rs#L74-L90

Just change uri prefix "http" can skip this logic.Like: https://github.com/hyperium/tonic/blob/eeb3268f71ae5d1107c937392389db63d8f721fb/examples/src/uds/client.rs#L19

jsturtevant commented 4 months ago

LGTM, @fcfangcc could you add a comment explaining what is happening?

fcfangcc commented 4 months ago

@jsturtevant You mean to add a comment explaining in the source code?

jsturtevant commented 4 months ago

Yes, essentially describing a summary of https://github.com/containerd/rust-extensions/pull/264#discussion_r1581426817. We had to dive in and dig around to figure out what was going on. It would be nice to leave a quick explanation for the next person to point out what seems like a strange set up (setting up with http/https but actually ignoring that value all together and using our own logic to connect).