feat:add tls support fot memcached #5471
Conversation
| @@ -82,6 +87,18 @@ impl MemcachedBuilder { | |||
| self.config.default_ttl = Some(ttl); | |||
| self | |||
| } | |||
|
|
|||
| /// Set the tls connect on. | |||
| pub fn tls(mut self, tls: bool) -> Self { | |||
There was a problem hiding this comment.
Let's use enable_tls to align with other config in opendal.
| } | ||
|
|
||
| /// Set the tls connect on. | ||
| pub fn cafile(mut self, cafile: PathBuf) -> Self { |
There was a problem hiding this comment.
I believe we should provide two options here:
- We should load users' CA by default (via
rustls_native_certs). - We should provide
keyandcertotpion for users to use their own cert.
| /// default is false | ||
| pub tls: Option<bool>, | ||
| /// Path to the CA certificate for TLS verification. | ||
| pub cafile: Option<PathBuf>, |
There was a problem hiding this comment.
Please don't use PathBuf in config, use String instead.
| @@ -40,4 +41,8 @@ pub struct MemcachedConfig { | |||
| pub password: Option<String>, | |||
| /// The default ttl for put operations. | |||
| pub default_ttl: Option<Duration>, | |||
| /// default is false | |||
| pub tls: Option<bool>, | |||
There was a problem hiding this comment.
Use bool is good enough that we make it default to false.
| @@ -150,6 +175,9 @@ impl Builder for MemcachedBuilder { | |||
| endpoint, | |||
| username: self.config.username.clone(), | |||
| password: self.config.password.clone(), | |||
| tls: self.config.tls.clone(), | |||
| cafile: self.config.cafile.clone(), | |||
| host, | |||
There was a problem hiding this comment.
Because ServerName requires it, I don't want to return an additional error.
| } | ||
| } | ||
|
|
||
| pub async fn auth(&mut self, username: &str, password: &str) -> Result<()> { |
There was a problem hiding this comment.
It seems like we're repeating a significant amount of code. Perhaps we could work directly with AsyncWrite? I believe both TlsStream<TcpStream> and TcpStream have implemented it.
| conn.auth(username, password).await?; | ||
| } | ||
| let config = rustls::ClientConfig::builder() | ||
| .with_root_certificates(root_cert_store) |
There was a problem hiding this comment.
I suppose we don't need to manually install root certificates.
Which issue does this PR close?
Closes #5419.
Rationale for this change
see #5419
What changes are included in this PR?
Modified the
opendal::services::Memcachedto support TLS connections.Are there any user-facing changes?
Users can enable TLS using
.tls()and provide the CA file using.cafile().example: