Skip to content
Snippets Groups Projects
Verified Commit 172d04ec authored by Кирилл Безуглый's avatar Кирилл Безуглый
Browse files

fix: `picodata run --http-listen localhost` selects a wrong default port

parent 1ba97502
No related branches found
No related tags found
1 merge request!1171fix: `picodata run --http-listen localhost` selects a wrong default port
Pipeline #47591 failed
use std::str::FromStr;
use tarantool::tlua;
const DEFAULT_HOST: &str = "localhost";
const DEFAULT_PORT: &str = "3301";
pub const DEFAULT_USERNAME: &str = "guest";
pub const DEFAULT_LISTEN_HOST: &str = "localhost";
pub const DEFAULT_HTTP_PORT: &str = "8080";
pub const DEFAULT_IPROTO_PORT: &str = "3301";
////////////////////
// IPROTO ADDRESS //
////////////////////
#[derive(Debug, Clone, PartialEq, Eq, tlua::Push, tlua::PushInto)]
pub struct Address {
pub struct IprotoAddress {
pub user: Option<String>,
pub host: String,
pub port: String,
}
impl Address {
#[inline(always)]
pub fn to_host_port(&self) -> String {
format!("{}:{}", self.host, self.port)
impl Default for IprotoAddress {
fn default() -> Self {
Self {
user: None,
host: DEFAULT_LISTEN_HOST.into(),
port: DEFAULT_IPROTO_PORT.into(),
}
}
}
impl std::fmt::Display for Address {
impl IprotoAddress {
#[inline(always)]
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
if let Some(user) = &self.user {
write!(f, "{user}@{}:{}", self.host, self.port)
} else {
write!(f, "{}:{}", self.host, self.port)
}
pub fn to_host_port(&self) -> String {
format!("{}:{}", self.host, self.port)
}
}
impl FromStr for Address {
type Err = String;
fn parse_address(addr: &str) -> Result<Self, String> {
let format_err = Err("valid format: [user@][host][:port]".to_string());
let port_err = Err(format!(
"IPROTO cannot listen on port {DEFAULT_HTTP_PORT} because it is default port for HTTP"
));
fn from_str(text: &str) -> Result<Self, Self::Err> {
let err = Err("valid format: [user@][host][:port]".to_string());
let (user, host_port) = match text.rsplit_once('@') {
let (user, host_port) = match addr.rsplit_once('@') {
Some((user, host_port)) => {
if user.contains(':') || user.contains('@') {
return err;
return format_err;
}
if user.is_empty() {
return err;
return format_err;
}
(Some(user), host_port)
}
None => (None, text),
None => (None, addr),
};
let (host, port) = match host_port.rsplit_once(':') {
Some((host, port)) => {
if host.contains(':') {
return err;
return format_err;
}
if port.is_empty() {
return err;
return format_err;
}
if port == DEFAULT_HTTP_PORT {
return port_err;
}
let host = if host.is_empty() { None } else { Some(host) };
(host, Some(port))
......@@ -61,16 +69,32 @@ impl FromStr for Address {
};
Ok(Self {
user: user.map(Into::into),
host: host.unwrap_or(DEFAULT_HOST).into(),
// FIXME: this is wrong for a general `Address` struct. There's
// currently a bug, if we run with `--http-listen=localhost`, it
// will attempt to bind the http server to port 3301.
port: port.unwrap_or(DEFAULT_PORT).into(),
host: host.unwrap_or(DEFAULT_LISTEN_HOST).into(),
port: port.unwrap_or(DEFAULT_IPROTO_PORT).into(),
})
}
}
impl serde::Serialize for Address {
impl std::fmt::Display for IprotoAddress {
#[inline(always)]
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
if let Some(user) = &self.user {
write!(f, "{user}@{}:{}", self.host, self.port)
} else {
write!(f, "{}:{}", self.host, self.port)
}
}
}
impl FromStr for IprotoAddress {
type Err = String;
fn from_str(addr: &str) -> Result<Self, Self::Err> {
Self::parse_address(addr)
}
}
impl serde::Serialize for IprotoAddress {
#[inline(always)]
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
......@@ -80,7 +104,7 @@ impl serde::Serialize for Address {
}
}
impl<'de> serde::Deserialize<'de> for Address {
impl<'de> serde::Deserialize<'de> for IprotoAddress {
#[inline(always)]
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
......@@ -91,6 +115,100 @@ impl<'de> serde::Deserialize<'de> for Address {
}
}
//////////////////
// HTTP ADDRESS //
//////////////////
#[derive(Debug, Clone, PartialEq, Eq, tlua::Push, tlua::PushInto)]
pub struct HttpAddress {
pub host: String,
pub port: String,
}
impl Default for HttpAddress {
fn default() -> Self {
Self {
host: DEFAULT_LISTEN_HOST.into(),
port: DEFAULT_HTTP_PORT.into(),
}
}
}
impl HttpAddress {
#[inline(always)]
pub fn to_host_port(&self) -> String {
format!("{}:{}", self.host, self.port)
}
fn parse_address(addr: &str) -> Result<Self, String> {
let format_err = Err("valid format: [host][:port]".to_string());
let port_err = Err(format!(
"HTTP cannot listen on port {DEFAULT_IPROTO_PORT} because it is default port for IPROTO"
));
let (host, port) = match addr.rsplit_once(':') {
Some((host, port)) => {
if host.contains(':') {
return format_err;
}
if port.is_empty() {
return format_err;
}
if port == DEFAULT_IPROTO_PORT {
return port_err;
}
let host = if host.is_empty() { None } else { Some(host) };
(host, Some(port))
}
None => (Some(addr), None),
};
Ok(Self {
host: host.unwrap_or(DEFAULT_LISTEN_HOST).into(),
port: port.unwrap_or(DEFAULT_HTTP_PORT).into(),
})
}
}
impl std::fmt::Display for HttpAddress {
#[inline(always)]
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{}:{}", self.host, self.port)
}
}
impl FromStr for HttpAddress {
type Err = String;
fn from_str(addr: &str) -> Result<Self, Self::Err> {
Self::parse_address(addr)
}
}
impl serde::Serialize for HttpAddress {
#[inline(always)]
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: serde::Serializer,
{
self.to_string().serialize(serializer)
}
}
impl<'de> serde::Deserialize<'de> for HttpAddress {
#[inline(always)]
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: serde::Deserializer<'de>,
{
let s: &str = serde::Deserialize::deserialize(deserializer)?;
Self::from_str(s).map_err(serde::de::Error::custom)
}
}
///////////
// TESTS //
///////////
#[cfg(test)]
mod tests {
use super::*;
......@@ -99,7 +217,7 @@ mod tests {
fn try_parse_address() {
assert_eq!(
"1234".parse(),
Ok(Address {
Ok(IprotoAddress {
user: None,
host: "1234".into(),
port: "3301".into()
......@@ -107,7 +225,7 @@ mod tests {
);
assert_eq!(
":1234".parse(),
Ok(Address {
Ok(IprotoAddress {
user: None,
host: "localhost".into(),
port: "1234".into()
......@@ -115,7 +233,7 @@ mod tests {
);
assert_eq!(
"example".parse(),
Ok(Address {
Ok(IprotoAddress {
user: None,
host: "example".into(),
port: "3301".into()
......@@ -123,7 +241,7 @@ mod tests {
);
assert_eq!(
"localhost:1234".parse(),
Ok(Address {
Ok(IprotoAddress {
user: None,
host: "localhost".into(),
port: "1234".into()
......@@ -131,7 +249,7 @@ mod tests {
);
assert_eq!(
"1.2.3.4:1234".parse(),
Ok(Address {
Ok(IprotoAddress {
user: None,
host: "1.2.3.4".into(),
port: "1234".into()
......@@ -139,7 +257,7 @@ mod tests {
);
assert_eq!(
"example:1234".parse(),
Ok(Address {
Ok(IprotoAddress {
user: None,
host: "example".into(),
port: "1234".into()
......@@ -148,7 +266,7 @@ mod tests {
assert_eq!(
"user@host:port".parse(),
Ok(Address {
Ok(IprotoAddress {
user: Some("user".into()),
host: "host".into(),
port: "port".into()
......@@ -157,7 +275,7 @@ mod tests {
assert_eq!(
"user@:port".parse(),
Ok(Address {
Ok(IprotoAddress {
user: Some("user".into()),
host: "localhost".into(),
port: "port".into()
......@@ -166,19 +284,28 @@ mod tests {
assert_eq!(
"user@host".parse(),
Ok(Address {
Ok(IprotoAddress {
user: Some("user".into()),
host: "host".into(),
port: "3301".into()
})
);
assert!("example::1234".parse::<Address>().is_err());
assert!("user@@example".parse::<Address>().is_err());
assert!("user:pass@host:port".parse::<Address>().is_err());
assert!("a:b@c".parse::<Address>().is_err());
assert!("user:pass@host".parse::<Address>().is_err());
assert!("@host".parse::<Address>().is_err());
assert!("host:".parse::<Address>().is_err());
assert!("example::1234".parse::<IprotoAddress>().is_err());
assert!("user@@example".parse::<IprotoAddress>().is_err());
assert!("user:pass@host:port".parse::<IprotoAddress>().is_err());
assert!("a:b@c".parse::<IprotoAddress>().is_err());
assert!("user:pass@host".parse::<IprotoAddress>().is_err());
assert!("@host".parse::<IprotoAddress>().is_err());
assert!("host:".parse::<IprotoAddress>().is_err());
assert_eq!(
"host:8080".parse::<IprotoAddress>().unwrap_err(),
format!("IPROTO cannot listen on port {DEFAULT_HTTP_PORT} because it is default port for HTTP")
);
assert_eq!(
"host:3301".parse::<HttpAddress>().unwrap_err(),
format!("HTTP cannot listen on port {DEFAULT_IPROTO_PORT} because it is default port for IPROTO")
);
}
}
use crate::address::Address;
use crate::address::{HttpAddress, IprotoAddress};
use crate::config::DEFAULT_USERNAME;
use crate::instance::InstanceId;
use crate::util::Uppercase;
......@@ -83,7 +83,7 @@ pub struct Run {
)]
/// Address the other instances should use to connect to this instance.
/// Defaults to `--listen` value.
pub advertise_address: Option<Address>,
pub advertise_address: Option<IprotoAddress>,
#[clap(
short = 'l',
......@@ -94,11 +94,11 @@ pub struct Run {
/// Instance network address.
///
/// By default "localhost:3301" is used.
pub listen: Option<Address>,
pub listen: Option<IprotoAddress>,
/// Pgproto server address.
#[clap(long, value_name = "[HOST][:PORT]", env = "PICODATA_PG_LISTEN")]
pub pg_listen: Option<Address>,
pub pg_listen: Option<IprotoAddress>,
#[clap(
long = "peer",
......@@ -112,7 +112,7 @@ pub struct Run {
/// and joining an instance to an existing cluster.
///
/// By default "localhost:3301" is used.
pub peers: Vec<Address>,
pub peers: Vec<IprotoAddress>,
#[clap(
long = "failure-domain",
......@@ -161,7 +161,7 @@ pub struct Run {
#[clap(long, value_name = "[HOST][:PORT]", env = "PICODATA_HTTP_LISTEN")]
/// HTTP server address.
pub http_listen: Option<Address>,
pub http_listen: Option<HttpAddress>,
#[clap(short = 'i', long = "interactive", env = "PICODATA_INTERACTIVE_MODE")]
/// Enable interactive console. Deprecated in 24.1.
......@@ -355,7 +355,7 @@ pub struct Expel {
default_value = "localhost:3301"
)]
/// Address of any picodata instance of the given cluster.
pub peer_address: Address,
pub peer_address: IprotoAddress,
#[clap(long, env = "PICODATA_PASSWORD_FILE")]
/// Path to a plain-text file with the `admin` password.
......@@ -454,7 +454,7 @@ pub struct Connect {
#[clap(value_name = "ADDRESS")]
/// Picodata instance address to connect. Format:
/// `[user@][host][:port]`.
pub address: Address,
pub address: IprotoAddress,
#[clap(long, env = "PICODATA_PASSWORD_FILE")]
/// Path to a plain-text file with a password.
......
......@@ -3,7 +3,7 @@ use std::fmt::{Debug, Display};
use tarantool::auth::AuthMethod;
use tarantool::network::{AsClient, Client, Config};
use crate::address::Address;
use crate::address::IprotoAddress;
use crate::config::DEFAULT_USERNAME;
use crate::traft::error::Error;
use crate::util::prompt_password;
......@@ -143,7 +143,7 @@ impl Display for ResultSet {
///
/// On success returns the connection object and the chosen username.
pub fn determine_credentials_and_connect(
address: &Address,
address: &IprotoAddress,
user: Option<&str>,
password_file: Option<&str>,
auth_method: AuthMethod,
......
use crate::address::Address;
use crate::address::{HttpAddress, IprotoAddress};
use crate::cli::args;
use crate::cli::args::CONFIG_PARAMETERS_ENV;
use crate::failure_domain::FailureDomain;
......@@ -25,9 +25,7 @@ use std::path::PathBuf;
use tarantool::log::SayLevel;
use tarantool::tlua;
pub const DEFAULT_USERNAME: &str = "guest";
pub const DEFAULT_LISTEN_HOST: &str = "localhost";
pub const DEFAULT_IPROTO_PORT: &str = "3301";
pub use crate::address::{DEFAULT_IPROTO_PORT, DEFAULT_LISTEN_HOST, DEFAULT_USERNAME};
pub const DEFAULT_CONFIG_FILE_NAME: &str = "config.yaml";
////////////////////////////////////////////////////////////////////////////////
......@@ -874,27 +872,19 @@ pub struct InstanceConfig {
pub failure_domain: Option<FailureDomain>,
#[introspection(
config_default = vec![Address {
user: None,
host: DEFAULT_LISTEN_HOST.into(),
port: DEFAULT_IPROTO_PORT.into(),
}]
config_default = vec![IprotoAddress::default()]
)]
pub peer: Option<Vec<Address>>,
pub peer: Option<Vec<IprotoAddress>>,
#[introspection(
config_default = Address {
user: None,
host: DEFAULT_LISTEN_HOST.into(),
port: DEFAULT_IPROTO_PORT.into(),
}
config_default = IprotoAddress::default()
)]
pub listen: Option<Address>,
pub listen: Option<IprotoAddress>,
#[introspection(config_default = self.listen())]
pub advertise_address: Option<Address>,
pub advertise_address: Option<IprotoAddress>,
pub http_listen: Option<Address>,
pub http_listen: Option<HttpAddress>,
#[introspection(config_default = self.data_dir.as_ref().map(|dir| dir.join("admin.sock")))]
pub admin_socket: Option<PathBuf>,
......@@ -980,21 +970,21 @@ impl InstanceConfig {
}
#[inline]
pub fn peers(&self) -> Vec<Address> {
pub fn peers(&self) -> Vec<IprotoAddress> {
self.peer
.clone()
.expect("is set in PicodataConfig::set_defaults_explicitly")
}
#[inline]
pub fn advertise_address(&self) -> Address {
pub fn advertise_address(&self) -> IprotoAddress {
self.advertise_address
.clone()
.expect("is set in PicodataConfig::set_defaults_explicitly")
}
#[inline]
pub fn listen(&self) -> Address {
pub fn listen(&self) -> IprotoAddress {
self.listen
.clone()
.expect("is set in PicodataConfig::set_defaults_explicitly")
......@@ -1431,8 +1421,7 @@ instance:
let config = PicodataConfig::read_yaml_contents(&yaml.trim_start()).unwrap();
let listen = config.instance.http_listen.unwrap();
assert_eq!(listen.host, "localhost");
assert_eq!(listen.port, "3301"); // FIXME: This is wrong! The default
// http port should be something different
assert_eq!(listen.port, "8080");
}
#[track_caller]
......@@ -1492,11 +1481,7 @@ instance:
assert_eq!(
config.instance.peers(),
vec![Address {
user: None,
host: "localhost".into(),
port: "3301".into(),
}]
vec![IprotoAddress::default()]
);
assert_eq!(config.instance.instance_id(), None);
assert_eq!(config.instance.listen().to_host_port(), "localhost:3301");
......@@ -1555,13 +1540,7 @@ instance:
assert_eq!(
config.instance.peers(),
vec![
Address {
user: None,
host: "localhost".into(),
port: "3301".into(),
}
]
vec![IprotoAddress::default()]
);
// only config
......@@ -1576,12 +1555,12 @@ instance:
assert_eq!(
config.instance.peers(),
vec![
Address {
IprotoAddress {
user: None,
host: "bobbert".into(),
port: "420".into(),
},
Address {
IprotoAddress {
user: None,
host: "tomathan".into(),
port: "69".into(),
......@@ -1596,12 +1575,12 @@ instance:
assert_eq!(
config.instance.peers(),
vec![
Address {
IprotoAddress {
user: None,
host: "oops there's a space over here -> <-".into(),
port: "13".into(),
},
Address {
IprotoAddress {
user: None,
host: " maybe we should at least strip these".into(),
port: "37".into(),
......@@ -1618,27 +1597,27 @@ instance:
assert_eq!(
config.instance.peers(),
vec![
Address {
IprotoAddress {
user: None,
host: "one".into(),
port: "1".into(),
},
Address {
IprotoAddress {
user: None,
host: "two".into(),
port: "2".into(),
},
Address {
IprotoAddress {
user: None,
host: " <- same problem here".into(),
port: "3301".into(),
},
Address {
IprotoAddress {
user: None,
host: "localhost".into(),
port: "3".into(),
},
Address {
IprotoAddress {
user: None,
host: "4".into(),
port: "3301".into(),
......@@ -1654,17 +1633,17 @@ instance:
assert_eq!(
config.instance.peers(),
vec![
Address {
IprotoAddress {
user: None,
host: "host".into(),
port: "123".into(),
},
Address {
IprotoAddress {
user: None,
host: "ghost ".into(),
port: "321".into(),
},
Address {
IprotoAddress {
user: None,
host: "гост".into(),
port: "666".into(),
......@@ -1836,7 +1815,10 @@ instance:
let config = setup_for_tests(Some(yaml), &["run"]).unwrap();
let pg = config.instance.pg;
assert!(pg.enabled());
assert_eq!(pg.listen(), Address::from_str("localhost:5432").unwrap());
assert_eq!(
pg.listen(),
IprotoAddress::from_str("localhost:5432").unwrap()
);
assert!(!pg.ssl());
// test config with -c option
......@@ -1844,14 +1826,20 @@ instance:
setup_for_tests(None, &["run", "-c", "instance.pg.listen=localhost:5432"]).unwrap();
let pg = config.instance.pg;
assert!(pg.enabled());
assert_eq!(pg.listen(), Address::from_str("localhost:5432").unwrap());
assert_eq!(
pg.listen(),
IprotoAddress::from_str("localhost:5432").unwrap()
);
assert!(!pg.ssl());
// test config from run args
let config = setup_for_tests(None, &["run", "--pg-listen", "localhost:5432"]).unwrap();
let pg = config.instance.pg;
assert!(pg.enabled());
assert_eq!(pg.listen(), Address::from_str("localhost:5432").unwrap());
assert_eq!(
pg.listen(),
IprotoAddress::from_str("localhost:5432").unwrap()
);
assert!(!pg.ssl());
// test config from env
......@@ -1859,7 +1847,10 @@ instance:
let config = setup_for_tests(None, &["run"]).unwrap();
let pg = config.instance.pg;
assert!(pg.enabled());
assert_eq!(pg.listen(), Address::from_str("localhost:1234").unwrap());
assert_eq!(
pg.listen(),
IprotoAddress::from_str("localhost:1234").unwrap()
);
assert!(!pg.ssl());
}
}
......@@ -24,7 +24,7 @@ use storage::Clusterwide;
use traft::RaftSpaceAccess;
use crate::access_control::user_by_id;
use crate::address::Address;
use crate::address::HttpAddress;
use crate::instance::Instance;
use crate::instance::State;
use crate::instance::StateVariant::*;
......@@ -200,7 +200,7 @@ fn preload_http() {
preload!("http.mime_types", "http/mime_types.lua");
}
fn start_http_server(Address { host, port, .. }: &Address) {
fn start_http_server(HttpAddress { host, port, .. }: &HttpAddress) {
tlog!(Info, "starting http server at {host}:{port}");
let lua = ::tarantool::lua_state();
lua.exec_with(
......
use self::{client::PgClient, error::PgResult, tls::TlsAcceptor};
use crate::{address::Address, introspection::Introspection, tlog, traft::error::Error};
use crate::{address::IprotoAddress, introspection::Introspection, tlog, traft::error::Error};
use std::path::{Path, PathBuf};
use stream::PgStream;
use tarantool::coio::{CoIOListener, CoIOStream};
......@@ -17,7 +17,7 @@ mod value;
#[derive(PartialEq, Default, Debug, Clone, serde::Deserialize, serde::Serialize, Introspection)]
#[serde(deny_unknown_fields)]
pub struct Config {
pub listen: Option<Address>,
pub listen: Option<IprotoAddress>,
#[introspection(config_default = false)]
pub ssl: Option<bool>,
......@@ -29,7 +29,7 @@ impl Config {
self.listen.is_some()
}
pub fn listen(&self) -> Address {
pub fn listen(&self) -> IprotoAddress {
self.listen
.clone()
.expect("must be checked before the call")
......
......@@ -36,6 +36,7 @@ use serde::{Deserialize, Serialize};
use crate::access_control::UserMetadataKind;
use crate::cas::{self, compare_and_swap, Request};
use crate::config::DEFAULT_USERNAME;
use crate::instance::InstanceId;
use crate::pico_service::pico_service_password;
use crate::plugin::PluginIdentifier;
......@@ -1088,12 +1089,12 @@ pub fn system_user_definitions() -> Vec<(UserDef, Vec<PrivilegeDef>)> {
{
let user_def = UserDef {
id: GUEST_ID,
name: "guest".into(),
name: DEFAULT_USERNAME.into(),
// This means the local schema is already up to date and main loop doesn't need to do anything
schema_version: INITIAL_SCHEMA_VERSION,
auth: Some(AuthDef::new(
AuthMethod::ChapSha1,
AuthData::new(&AuthMethod::ChapSha1, "guest", "").into_string(),
AuthData::new(&AuthMethod::ChapSha1, DEFAULT_USERNAME, "").into_string(),
)),
owner: initiator,
ty: UserMetadataKind::User,
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment