Skip to content

Commit

Permalink
ditch default value
Browse files Browse the repository at this point in the history
  • Loading branch information
rukai committed Nov 2, 2022
1 parent 07d9574 commit 23ec995
Show file tree
Hide file tree
Showing 14 changed files with 31 additions and 63 deletions.
8 changes: 6 additions & 2 deletions docs/src/transforms.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ While `system.peers`/`system.peers_v2` will be rewritten to list the configured
# certificate_path: "tls/localhost.crt"
# # Path to the private key file, typically named with a .key extension.
# private_key_path: "tls/localhost.key"
# # Enable/disable verifying the hostname of the certificate provided by the destination. Enabled by default
# # Enable/disable verifying the hostname of the certificate provided by the destination.
# #verify_hostname: true

# Timeout in seconds after which to give up waiting for a response from the destination.
Expand Down Expand Up @@ -158,7 +158,7 @@ No cluster discovery or routing occurs with this transform.
# certificate_path: "tls/localhost.crt"
# # Path to the private key file, typically named with a .key extension.
# private_key_path: "tls/localhost.key"
# # Enable/disable verifying the hostname of the certificate provided by the destination. Enabled by default
# # Enable/disable verifying the hostname of the certificate provided by the destination.
# #verify_hostname: true

# Timeout in seconds after which to give up waiting for a response from the destination.
Expand Down Expand Up @@ -431,6 +431,8 @@ This transform is a full featured Redis driver that will connect to a Redis clus
# certificate_path: "tls/redis.crt"
# # Path to the private key file, typically named with a .key extension.
# private_key_path: "tls/redis.key"
# # Enable/disable verifying the hostname of the certificate provided by the destination.
# #verify_hostname: true
```

Unlike other Redis cluster drivers, this transform does support pipelining. It does however turn each command from the pipeline into a group of requests split between the master Redis node that owns them, buffering results as within different Redis nodes as needed. This is done sequentially and there is room to make this transform split requests between master nodes in a more concurrent manner.
Expand Down Expand Up @@ -468,6 +470,8 @@ This transform will take a query, serialise it into a RESP2 compatible format an
# certificate_path: "tls/redis.crt"
# # Path to the private key file, typically named with a .key extension.
# private_key_path: "tls/redis.key"
# # Enable/disable verifying the hostname of the certificate provided by the destination.
# #verify_hostname: true
```

Note: this will just pass the query to the remote node. No cluster discovery or routing occurs with this transform.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,6 @@ chain_config:
certificate_authority_path: "example-configs/docker-images/cassandra-tls-4.0.6/certs/localhost_CA.crt"
certificate_path: "example-configs/docker-images/cassandra-tls-4.0.6/certs/localhost.crt"
private_key_path: "example-configs/docker-images/cassandra-tls-4.0.6/certs/localhost.key"
verify_hostname: true
source_to_chain_mapping:
cassandra_prod: main_chain
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,6 @@ chain_config:
certificate_authority_path: "example-configs/docker-images/cassandra-tls-4.0.6/certs/localhost_CA.crt"
certificate_path: "example-configs/docker-images/cassandra-tls-4.0.6/certs/localhost.crt"
private_key_path: "example-configs/docker-images/cassandra-tls-4.0.6/certs/localhost.key"
verify_hostname: true
source_to_chain_mapping:
cassandra_prod: main_chain
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,6 @@ chain_config:
certificate_authority_path: "example-configs/redis-tls/certs/ca.crt"
certificate_path: "example-configs/redis-tls/certs/redis.crt"
private_key_path: "example-configs/redis-tls/certs/redis.key"
verify_hostname: true
source_to_chain_mapping:
redis_prod: redis_chain
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,6 @@ chain_config:
first_contact_points: ["127.0.0.1:2220", "127.0.0.1:2221", "127.0.0.1:2222", "127.0.0.1:2223", "127.0.0.1:2224", "127.0.0.1:2225"]
tls:
certificate_authority_path: "example-configs/redis-tls/certs/ca.crt"
verify_hostname: true
source_to_chain_mapping:
redis_prod: redis_chain
1 change: 1 addition & 0 deletions shotover-proxy/example-configs/redis-tls/topology.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ chain_config:
certificate_authority_path: "example-configs/redis-tls/certs/ca.crt"
certificate_path: "example-configs/redis-tls/certs/redis.crt"
private_key_path: "example-configs/redis-tls/certs/redis.key"
verify_hostname: false
source_to_chain_mapping:
redis_prod: redis_chain_tls
redis_prod_tls: redis_chain_tls
30 changes: 4 additions & 26 deletions shotover-proxy/src/tls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,8 @@ pub struct TlsConnectorConfig {
pub certificate_path: Option<String>,
/// Path to the private key in PEM format
pub private_key_path: Option<String>,
/// whether to enable verifying the hostname of the destination's certificate.
/// Some(true) - enable verify_hostname
/// Some(false) - disable verify_hostname
/// None - protocol specific default value
pub verify_hostname: Option<bool>,
/// enable/disable verifying the hostname of the destination's certificate.
pub verify_hostname: bool,
}

#[derive(Clone, Debug)]
Expand All @@ -98,16 +95,8 @@ pub struct TlsConnector {
verify_hostname: bool,
}

pub enum ApplicationProtocol {
Redis,
Cassandra,
}

impl TlsConnector {
pub fn new(
tls_config: TlsConnectorConfig,
protocol: ApplicationProtocol,
) -> Result<TlsConnector> {
pub fn new(tls_config: TlsConnectorConfig) -> Result<TlsConnector> {
check_file_field(
"certificate_authority_path",
&tls_config.certificate_authority_path,
Expand All @@ -132,20 +121,9 @@ impl TlsConnector {
.map_err(openssl_stack_error_to_anyhow)?;
}

let verify_hostname = match (protocol, tls_config.verify_hostname) {
(ApplicationProtocol::Redis, Some(true)) => {
return Err(anyhow!(
"verify_hostname is enabled in TLS config but redis does not support it."
))
}
(ApplicationProtocol::Redis, _) => false,
(_, None) => true,
(_, Some(true)) => true,
(_, Some(false)) => false,
};
Ok(TlsConnector {
connector: Arc::new(builder.build()),
verify_hostname,
verify_hostname: tls_config.verify_hostname,
})
}

Expand Down
8 changes: 2 additions & 6 deletions shotover-proxy/src/transforms/cassandra/sink_cluster/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::error::ChainResponse;
use crate::frame::cassandra::{parse_statement_single, CassandraMetadata, Tracing};
use crate::frame::{CassandraFrame, CassandraOperation, CassandraResult, Frame};
use crate::message::{IntSize, Message, MessageValue, Messages};
use crate::tls::{ApplicationProtocol, TlsConnector, TlsConnectorConfig};
use crate::tls::{TlsConnector, TlsConnectorConfig};
use crate::transforms::cassandra::connection::CassandraConnection;
use crate::transforms::util::Response;
use crate::transforms::{Transform, Transforms, Wrapper};
Expand Down Expand Up @@ -56,11 +56,7 @@ pub struct CassandraSinkClusterConfig {

impl CassandraSinkClusterConfig {
pub async fn get_transform(&self, chain_name: String) -> Result<Transforms> {
let tls = self
.tls
.clone()
.map(|c| TlsConnector::new(c, ApplicationProtocol::Cassandra))
.transpose()?;
let tls = self.tls.clone().map(TlsConnector::new).transpose()?;
let mut shotover_nodes = self.shotover_nodes.clone();
let index = self
.shotover_nodes
Expand Down
8 changes: 2 additions & 6 deletions shotover-proxy/src/transforms/cassandra/sink_single.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use super::connection::CassandraConnection;
use crate::codec::cassandra::CassandraCodec;
use crate::error::ChainResponse;
use crate::message::Messages;
use crate::tls::{ApplicationProtocol, TlsConnector, TlsConnectorConfig};
use crate::tls::{TlsConnector, TlsConnectorConfig};
use crate::transforms::util::Response;
use crate::transforms::{Transform, Transforms, Wrapper};
use anyhow::Result;
Expand All @@ -24,11 +24,7 @@ pub struct CassandraSinkSingleConfig {

impl CassandraSinkSingleConfig {
pub async fn get_transform(&self, chain_name: String) -> Result<Transforms> {
let tls = self
.tls
.clone()
.map(|c| TlsConnector::new(c, ApplicationProtocol::Cassandra))
.transpose()?;
let tls = self.tls.clone().map(TlsConnector::new).transpose()?;
Ok(Transforms::CassandraSinkSingle(CassandraSinkSingle::new(
self.address.clone(),
chain_name,
Expand Down
7 changes: 1 addition & 6 deletions shotover-proxy/src/transforms/redis/sink_single.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use crate::frame::Frame;
use crate::frame::RedisFrame;
use crate::message::{Message, Messages};
use crate::server::CodecReadError;
use crate::tls::ApplicationProtocol;
use crate::tls::{AsyncStream, TlsConnector, TlsConnectorConfig};
use crate::transforms::{Transform, Transforms, Wrapper};
use anyhow::{anyhow, Context, Result};
Expand All @@ -31,11 +30,7 @@ pub struct RedisSinkSingleConfig {

impl RedisSinkSingleConfig {
pub async fn get_transform(&self, chain_name: String) -> Result<Transforms> {
let tls = self
.tls
.clone()
.map(|c| TlsConnector::new(c, ApplicationProtocol::Redis))
.transpose()?;
let tls = self.tls.clone().map(TlsConnector::new).transpose()?;
Ok(Transforms::RedisSinkSingle(RedisSinkSingle::new(
self.address.clone(),
tls,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ use super::Response;
use crate::server::Codec;
use crate::server::CodecReadHalf;
use crate::server::CodecWriteHalf;
use crate::tls::ApplicationProtocol;
use crate::tls::{TlsConnector, TlsConnectorConfig};
use crate::transforms::util::{ConnectionError, Request};
use anyhow::{anyhow, Result};
Expand Down Expand Up @@ -79,9 +78,7 @@ impl<C: Codec + 'static, A: Authenticator<T>, T: Token> ConnectionPool<C, A, T>
) -> Result<Self> {
Ok(Self {
lanes: Arc::new(Mutex::new(HashMap::new())),
tls: tls
.map(|c| TlsConnector::new(c, ApplicationProtocol::Redis))
.transpose()?,
tls: tls.map(TlsConnector::new).transpose()?,
codec,
authenticator,
})
Expand Down
17 changes: 7 additions & 10 deletions shotover-proxy/tests/cassandra_int_tests/cluster/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use cassandra_protocol::frame::Version;
use shotover_proxy::frame::{cassandra::Tracing, CassandraFrame, CassandraOperation, Frame};
use shotover_proxy::message::Message;
use shotover_proxy::tls::{ApplicationProtocol, TlsConnector, TlsConnectorConfig};
use shotover_proxy::tls::{TlsConnector, TlsConnectorConfig};
use shotover_proxy::transforms::cassandra::sink_cluster::{
node::{CassandraNode, ConnectionFactory},
topology::{create_topology_task, TaskConnectionInfo},
Expand All @@ -17,15 +17,12 @@ pub async fn run_topology_task(ca_path: Option<&str>, port: Option<u32>) -> Vec<
let (nodes_tx, mut nodes_rx) = watch::channel(vec![]);
let (task_handshake_tx, task_handshake_rx) = mpsc::channel(1);
let tls = ca_path.map(|ca_path| {
TlsConnector::new(
TlsConnectorConfig {
certificate_authority_path: ca_path.into(),
certificate_path: None,
private_key_path: None,
verify_hostname: None,
},
ApplicationProtocol::Cassandra,
)
TlsConnector::new(TlsConnectorConfig {
certificate_authority_path: ca_path.into(),
certificate_path: None,
private_key_path: None,
verify_hostname: true,
})
.unwrap()
});

Expand Down
4 changes: 2 additions & 2 deletions shotover-proxy/tests/helpers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use anyhow::Result;
use redis::aio::AsyncStream;
use redis::Client;
use shotover_proxy::runner::{ConfigOpts, Runner};
use shotover_proxy::tls::{ApplicationProtocol, TlsConnector, TlsConnectorConfig};
use shotover_proxy::tls::{TlsConnector, TlsConnectorConfig};
use std::pin::Pin;
use std::sync::mpsc;
use std::time::Duration;
Expand Down Expand Up @@ -120,7 +120,7 @@ impl ShotoverManager {
let tcp_stream = tokio::net::TcpStream::connect((address, port))
.await
.unwrap();
let connector = TlsConnector::new(config, ApplicationProtocol::Redis).unwrap();
let connector = TlsConnector::new(config).unwrap();
let tls_stream = connector.connect(tcp_stream).await.unwrap();
ShotoverManager::redis_connection_async_inner(
Box::pin(tls_stream) as Pin<Box<dyn AsyncStream + Send + Sync>>
Expand Down
2 changes: 1 addition & 1 deletion shotover-proxy/tests/redis_int_tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ async fn source_tls_and_single_tls() {
certificate_authority_path: "example-configs/redis-tls/certs/ca.crt".into(),
certificate_path: Some("example-configs/redis-tls/certs/redis.crt".into()),
private_key_path: Some("example-configs/redis-tls/certs/redis.key".into()),
verify_hostname: None,
verify_hostname: true,
};

let mut connection = shotover_manager
Expand Down

0 comments on commit 23ec995

Please sign in to comment.