diff --git a/gel-dsn/Cargo.toml b/gel-dsn/Cargo.toml index 822429b2..cfec18e5 100644 --- a/gel-dsn/Cargo.toml +++ b/gel-dsn/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "gel-dsn" license = "MIT/Apache-2.0" -version = "0.1.0" +version = "0.1.1" authors = ["MagicStack Inc. "] edition = "2021" description = "Data-source name (DSN) parser for Gel and PostgreSQL databases." diff --git a/gel-dsn/src/gel/config.rs b/gel-dsn/src/gel/config.rs index ba079d7a..d15dbd70 100644 --- a/gel-dsn/src/gel/config.rs +++ b/gel-dsn/src/gel/config.rs @@ -658,11 +658,17 @@ impl TryInto for ConnectionOptions { } if self.tls_ca.is_some() && self.tls_ca_file.is_some() { - return Err(ParseError::ExclusiveOptions); + return Err(ParseError::ExclusiveOptions( + "tls_ca".to_string(), + "tls_ca_file".to_string(), + )); } if self.branch.is_some() && self.database.is_some() { - return Err(ParseError::ExclusiveOptions); + return Err(ParseError::ExclusiveOptions( + "branch".to_string(), + "database".to_string(), + )); } let mut credentials = Param::from_file(self.credentials_file.clone()); diff --git a/gel-dsn/src/gel/env.rs b/gel-dsn/src/gel/env.rs index 57629e50..3cf87600 100644 --- a/gel-dsn/src/gel/env.rs +++ b/gel-dsn/src/gel/env.rs @@ -1,5 +1,6 @@ use super::{ - error::*, BuildContext, ClientSecurity, CloudCerts, FromParamStr, InstanceName, TlsSecurity, + error::*, BuildContext, ClientSecurity, CloudCerts, FromParamStr, InstanceName, ParamSource, + TlsSecurity, }; use crate::host::HostType; use crate::EnvVar; @@ -186,7 +187,7 @@ pub fn get_envs( Err(std::env::VarError::NotPresent) => continue, Err(err @ std::env::VarError::NotUnicode(_)) => { return Err(ParseError::EnvNotFound( - EnvironmentSource::Explicit, + ParamSource::Explicit, err.to_string(), )); } diff --git a/gel-dsn/src/gel/error.rs b/gel-dsn/src/gel/error.rs index 84a77650..b067b8df 100644 --- a/gel-dsn/src/gel/error.rs +++ b/gel-dsn/src/gel/error.rs @@ -1,6 +1,8 @@ use crate::host::HostParseError; use std::{convert::Infallible, num::ParseIntError}; +use super::ParamSource; + #[derive(Debug, Clone, PartialEq, Eq, derive_more::Display, PartialOrd, Ord)] pub enum CompoundSource { Dsn, @@ -46,22 +48,14 @@ pub enum InvalidDsnError { BranchAndDatabase, } -#[derive(Debug, Clone, PartialEq, Eq, derive_more::Display, PartialOrd, Ord)] -pub enum EnvironmentSource { - #[display("Explicit")] - Explicit, - #[display("Param::Env")] - Param, -} - #[derive(Debug, derive_more::Error, derive_more::Display, PartialEq, Eq, PartialOrd, Ord)] pub enum ParseError { #[display("Credentials file not found")] CredentialsFileNotFound, - #[display("Environment variable not found: {_1} ({_0})")] - EnvNotFound(EnvironmentSource, #[error(not(source))] String), - #[display("Exclusive options")] - ExclusiveOptions, + #[display("Environment variable was not set: {_1} (from {_0})")] + EnvNotFound(ParamSource, #[error(not(source))] String), + #[display("{_0} and {_1} are mutually exclusive and cannot be used together")] + ExclusiveOptions(String, String), #[display("File not found")] FileNotFound, #[display("Invalid credentials file: {_0}")] @@ -94,7 +88,7 @@ pub enum ParseError { MultipleCompoundOpts(#[error(not(source))] Vec), #[display("No options or .toml file")] NoOptionsOrToml, - #[display("Project not initialised")] + #[display("Project not initialized")] ProjectNotInitialised, #[display("Secret key not found")] SecretKeyNotFound, @@ -107,7 +101,7 @@ impl ParseError { match self { Self::EnvNotFound(..) => "env_not_found", Self::CredentialsFileNotFound => "credentials_file_not_found", - Self::ExclusiveOptions => "exclusive_options", + Self::ExclusiveOptions(..) => "exclusive_options", Self::FileNotFound => "file_not_found", Self::InvalidCredentialsFile(_) => "invalid_credentials_file", Self::InvalidDatabase => "invalid_database", @@ -136,7 +130,6 @@ impl ParseError { match self { Self::EnvNotFound(..) | Self::CredentialsFileNotFound - | Self::ExclusiveOptions | Self::FileNotFound | Self::InvalidCredentialsFile(_) | Self::InvalidDatabase @@ -150,15 +143,24 @@ impl ParseError { | Self::InvalidUser | Self::InvalidCertificate | Self::InvalidDuration - | Self::MultipleCompoundEnv(_) - | Self::MultipleCompoundOpts(_) - | Self::NoOptionsOrToml - | Self::ProjectNotInitialised | Self::UnixSocketUnsupported => { + // The argument is invalid + gel_errors::InvalidArgumentError::with_source(self) + } + Self::MultipleCompoundEnv(_) + | Self::MultipleCompoundOpts(_) + | Self::ExclusiveOptions(..) => { + // The argument is valid, but the use is invalid + gel_errors::InterfaceError::with_source(self) + } + Self::NoOptionsOrToml | Self::ProjectNotInitialised => { + // Credentials are missing gel_errors::ClientNoCredentialsError::with_source(self) } - - Self::SecretKeyNotFound => gel_errors::NoCloudConfigFound::with_source(self), + Self::SecretKeyNotFound => { + // Required cloud configuration is missing + gel_errors::NoCloudConfigFound::with_source(self) + } } } } diff --git a/gel-dsn/src/gel/param.rs b/gel-dsn/src/gel/param.rs index b525e46c..f9ab761a 100644 --- a/gel-dsn/src/gel/param.rs +++ b/gel-dsn/src/gel/param.rs @@ -152,15 +152,23 @@ pub enum Param { /// Unparsed value. Unparsed(String), /// Value from given environment variable. - Env(String), + Env(ParamSource, String), /// Value from given file. File(PathBuf), /// Value from environment variable pointing to a file. - EnvFile(String), + EnvFile(ParamSource, String), /// Parsed value. Parsed(T), } +#[derive(Debug, Copy, Clone, derive_more::Display, PartialEq, Eq, PartialOrd, Ord)] +pub enum ParamSource { + #[display("DSN")] + Dsn, + #[display("explicit")] + Explicit, +} + #[allow(private_bounds)] impl Param where @@ -199,9 +207,9 @@ where match self { Self::None => Ok(Param::None), Self::Unparsed(value) => Ok(Param::Unparsed(value)), - Self::Env(value) => Ok(Param::Env(value)), + Self::Env(source, value) => Ok(Param::Env(source, value)), Self::File(value) => Ok(Param::File(value)), - Self::EnvFile(value) => Ok(Param::EnvFile(value)), + Self::EnvFile(source, value) => Ok(Param::EnvFile(source, value)), Self::Parsed(value) => Err(Self::Parsed(value)), } } @@ -212,15 +220,13 @@ where return Ok(None); } Self::Unparsed(value) => value.clone(), - Self::Env(key) => { - context_trace!(context, "Reading env: {key}"); + Self::Env(source, key) => { + context_trace!(context, "Reading env: {key} (from {source})"); context .env() .read(key) .map(|s| s.to_string()) - .map_err(|_| { - ParseError::EnvNotFound(EnvironmentSource::Param, key.to_string()) - })? + .map_err(|_| ParseError::EnvNotFound(*source, key.to_string()))? } Self::File(path) => { context_trace!(context, "Reading file: {path:?}"); @@ -232,14 +238,12 @@ where context_trace!(context, "File content: {res:?}"); res? } - Self::EnvFile(key) => { - context_trace!(context, "Reading env for file: {key}"); + Self::EnvFile(source, key) => { + context_trace!(context, "Reading env for file: {key} (from {source})"); let env = context .env() .read(key) - .map_err(|_| { - ParseError::EnvNotFound(EnvironmentSource::Param, key.to_string()) - })? + .map_err(|_| ParseError::EnvNotFound(*source, key.to_string()))? .to_string(); context_trace!(context, "Reading file: {env}"); let res = context diff --git a/gel-dsn/src/gel/params.rs b/gel-dsn/src/gel/params.rs index a40f61cf..2f70a2dd 100644 --- a/gel-dsn/src/gel/params.rs +++ b/gel-dsn/src/gel/params.rs @@ -16,8 +16,8 @@ use super::{ error::*, project::{find_project_file, ProjectDir}, BuildContext, BuildContextImpl, ClientSecurity, CloudCerts, Config, DatabaseBranch, - FromParamStr, InstanceName, Logging, Param, TcpKeepalive, TlsSecurity, DEFAULT_CONNECT_TIMEOUT, - DEFAULT_PORT, DEFAULT_WAIT, + FromParamStr, InstanceName, Logging, Param, ParamSource, TcpKeepalive, TlsSecurity, + DEFAULT_CONNECT_TIMEOUT, DEFAULT_PORT, DEFAULT_WAIT, }; use crate::{ env::SystemEnvVars, @@ -147,12 +147,10 @@ macro_rules! define_params { impl Builder { $( - paste::paste!( - $(#[doc = $doc])* - pub fn $name(mut self, value: impl Into<$type>) -> Self { - self.params.$name = Param::Parsed(value.into()); - self - } + // Note that paste! forces re-interpretation of type token and allows us + // to match the __maybe__* macros below. + paste::paste!{ + define_params!{__maybe_into__ $(#[doc = $doc])* $name: $type} $(#[doc = $doc])* #[doc = "\n\nThis value will be loaded from `path` in the filesystem and parsed as [`"] @@ -168,15 +166,31 @@ macro_rules! define_params { #[doc = stringify!($type)] #[doc = "`]."] pub fn [<$name _env>](mut self, value: impl AsRef) -> Self { - self.params.$name = Param::Env(value.as_ref().to_string()); + self.params.$name = Param::Env(ParamSource::Explicit, value.as_ref().to_string()); self } - ); - define_params!(__maybe_string__ $(#[doc = $doc])* $name: $type); + define_params!{__maybe_string__ $(#[doc = $doc])* $name: $type} + } )* } }; + // NOTE: Special case u16 since number literals don't cooperate well with + // type inference + Into. + (__maybe_into__ $(#[doc = $doc:expr])* $name:ident: u16) => { + $(#[doc = $doc])* + pub fn $name(mut self, value: u16) -> Self { + self.params.$name = Param::Parsed(value); + self + } + }; + (__maybe_into__ $(#[doc = $doc:expr])* $name:ident: $type:ty) => { + $(#[doc = $doc])* + pub fn $name(mut self, value: impl Into<$type>) -> Self { + self.params.$name = Param::Parsed(value.into()); + self + } + }; (__maybe_string__ $(#[doc = $doc:expr])* $name:ident: String) => { }; (__maybe_string__ $(#[doc = $doc:expr])* $name:ident: $type:ty) => { @@ -703,7 +717,9 @@ impl Params { sources.push(CompoundSource::Dsn); } if self.instance.is_some() { - sources.push(CompoundSource::Instance); + if self.unix_path.is_none() { + sources.push(CompoundSource::Instance); + } } if self.unix_path.is_some() { sources.push(CompoundSource::UnixSocket); @@ -772,38 +788,42 @@ impl Params { let instance = instance?; if let Some(instance) = &instance { - match &instance { - InstanceName::Local(local) => { - let instance = parse_instance(local, context)?; - context_trace!(context, "Instance: {:?}", instance); - explicit.merge(instance); - } - InstanceName::Cloud { .. } => { - let profile = explicit - .cloud_profile - .get(context)? - .unwrap_or("default".to_string()); - let cloud = parse_cloud(&profile, context)?; - context_trace!(context, "Cloud: {:?}", cloud); - explicit.merge(cloud); - - if let Some(secret_key) = explicit.secret_key.get(context)? { - match instance.cloud_address(&secret_key) { - Ok(Some(address)) => { - explicit.host = Param::Unparsed(address); - } - Ok(None) => { - unreachable!(); - } - Err(e) => { - // Special case: we ignore the secret key error until the final phase - if phase == BuildPhase::Project { - return Err(e); + // Special case: if the unix path is set we ignore the instance + // credentials. + if explicit.unix_path.is_none() { + match &instance { + InstanceName::Local(local) => { + let instance = parse_instance(local, context)?; + context_trace!(context, "Instance: {:?}", instance); + explicit.merge(instance); + } + InstanceName::Cloud { .. } => { + let profile = explicit + .cloud_profile + .get(context)? + .unwrap_or("default".to_string()); + let cloud = parse_cloud(&profile, context)?; + context_trace!(context, "Cloud: {:?}", cloud); + explicit.merge(cloud); + + if let Some(secret_key) = explicit.secret_key.get(context)? { + match instance.cloud_address(&secret_key) { + Ok(Some(address)) => { + explicit.host = Param::Unparsed(address); + } + Ok(None) => { + unreachable!(); + } + Err(e) => { + // Special case: we ignore the secret key error until the final phase + if phase == BuildPhase::Project { + return Err(e); + } } } + } else { + return Err(ParseError::SecretKeyNotFound); } - } else { - return Err(ParseError::SecretKeyNotFound); } } } @@ -1031,9 +1051,12 @@ fn parse_dsn(dsn: &str, context: &mut impl BuildContext) -> Result::Env(value.to_string())) + ( + key, + Param::::Env(ParamSource::Dsn, value.to_string()), + ) } else if let Some(key) = key.strip_suffix("_file") { (key, Param::File(PathBuf::from(value.to_string()))) } else { @@ -1125,14 +1148,20 @@ fn parse_env(context: &mut impl BuildContext) -> Result { }; if explicit.branch.is_some() && explicit.database.is_some() { - return Err(ParseError::ExclusiveOptions); + return Err(ParseError::ExclusiveOptions( + "branch".to_string(), + "database".to_string(), + )); } let ca_file = Param::from_file(context.read_env(Env::tls_ca_file)?); if explicit.tls_ca.is_none() { explicit.tls_ca = ca_file; } else if ca_file.is_some() { - return Err(ParseError::ExclusiveOptions); + return Err(ParseError::ExclusiveOptions( + "tls_ca".to_string(), + "tls_ca_file".to_string(), + )); } Ok(explicit) @@ -1483,4 +1512,39 @@ mod tests { // This intentionally won't work // let params = Builder::default().with_env().project_dir(Path::new(".")); } + + #[test] + fn test_with_unix_socket() { + let params = Builder::default() + .unix_path(Path::new("/")) + .without_system() + .build() + .expect("Just a unix path is OK"); + assert_eq!(params.host.to_string(), "/"); + eprintln!("{:?}", params); + + let params = Builder::default() + .unix_path(Path::new("/")) + .port(1234) + .without_system() + .build() + .expect("Unix path and port is OK"); + assert_eq!(params.host.to_string(), "/.s.EDGEDB.admin.1234"); + eprintln!("{:?}", params); + + // This is allowed, but instance credentials are ignored in this case + // and just transferred to the output Config. + let params = Builder::default() + .instance(InstanceName::Local("instancedoesnotexist".to_string())) + .unix_path(Path::new("/")) + .without_system() + .build() + .expect("Unix path and instance is OK"); + assert_eq!(params.host.to_string(), "/"); + assert_eq!( + params.instance_name, + Some(InstanceName::Local("instancedoesnotexist".to_string())) + ); + eprintln!("{:?}", params); + } } diff --git a/gel-dsn/src/host.rs b/gel-dsn/src/host.rs index 308043be..337b43bc 100644 --- a/gel-dsn/src/host.rs +++ b/gel-dsn/src/host.rs @@ -94,7 +94,7 @@ impl std::fmt::Display for Host { HostTypeInner::IP(ip, None) => write!(f, "[{}]:{}", ip, port), HostTypeInner::Path(path) => { if let Some(target_name) = self.2.target_name(port) { - write!(f, "{}/{}", path.display(), target_name) + write!(f, "{}", path.join(target_name).display()) } else { write!(f, "{}", path.display()) }