From 9424bfae22d6a808371959c87aa1106701a34ad5 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 1 Mar 2022 22:34:42 +0200 Subject: [PATCH] Use a separate newtype for ZId that (de)serialize as hex strings --- control_plane/src/local_env.rs | 7 +- pageserver/src/branches.rs | 3 +- pageserver/src/http/routes.rs | 10 +- zenith/src/main.rs | 4 +- zenith_utils/src/auth.rs | 44 ++------ zenith_utils/src/zid.rs | 198 ++++++++++++++++++++++++++------- 6 files changed, 179 insertions(+), 87 deletions(-) diff --git a/control_plane/src/local_env.rs b/control_plane/src/local_env.rs index 55d0b00496..238c78821e 100644 --- a/control_plane/src/local_env.rs +++ b/control_plane/src/local_env.rs @@ -12,7 +12,7 @@ use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; use zenith_utils::auth::{encode_from_key_file, Claims, Scope}; use zenith_utils::postgres_backend::AuthType; -use zenith_utils::zid::{opt_display_serde, ZNodeId, ZTenantId}; +use zenith_utils::zid::{HexZTenantId, ZNodeId, ZTenantId}; use crate::safekeeper::SafekeeperNode; @@ -47,9 +47,8 @@ pub struct LocalEnv { // Default tenant ID to use with the 'zenith' command line utility, when // --tenantid is not explicitly specified. - #[serde(with = "opt_display_serde")] #[serde(default)] - pub default_tenantid: Option, + pub default_tenantid: Option, // used to issue tokens during e.g pg start #[serde(default)] @@ -185,7 +184,7 @@ impl LocalEnv { // If no initial tenant ID was given, generate it. if env.default_tenantid.is_none() { - env.default_tenantid = Some(ZTenantId::generate()); + env.default_tenantid = Some(HexZTenantId::from(ZTenantId::generate())); } env.base_data_dir = base_path(); diff --git a/pageserver/src/branches.rs b/pageserver/src/branches.rs index 8a411060de..43f27af5ea 100644 --- a/pageserver/src/branches.rs +++ b/pageserver/src/branches.rs @@ -16,10 +16,9 @@ use std::{ }; use tracing::*; -use zenith_utils::crashsafe_dir; -use zenith_utils::logging; use zenith_utils::lsn::Lsn; use zenith_utils::zid::{ZTenantId, ZTimelineId}; +use zenith_utils::{crashsafe_dir, logging}; use crate::walredo::WalRedoManager; use crate::CheckpointConfig; diff --git a/pageserver/src/http/routes.rs b/pageserver/src/http/routes.rs index 4fc41d6e82..26d473efaf 100644 --- a/pageserver/src/http/routes.rs +++ b/pageserver/src/http/routes.rs @@ -19,7 +19,8 @@ use zenith_utils::http::{ }; use zenith_utils::http::{RequestExt, RouterBuilder}; use zenith_utils::lsn::Lsn; -use zenith_utils::zid::{opt_display_serde, ZTimelineId}; +use zenith_utils::zid::HexZTimelineId; +use zenith_utils::zid::ZTimelineId; use super::models::BranchCreateRequest; use super::models::StatusResponse; @@ -198,8 +199,7 @@ enum TimelineInfo { timeline_id: ZTimelineId, #[serde(with = "hex")] tenant_id: ZTenantId, - #[serde(with = "opt_display_serde")] - ancestor_timeline_id: Option, + ancestor_timeline_id: Option, last_record_lsn: Lsn, prev_record_lsn: Lsn, disk_consistent_lsn: Lsn, @@ -232,7 +232,9 @@ async fn timeline_detail_handler(request: Request) -> Result TimelineInfo::Local { timeline_id, tenant_id, - ancestor_timeline_id: timeline.get_ancestor_timeline_id(), + ancestor_timeline_id: timeline + .get_ancestor_timeline_id() + .map(HexZTimelineId::from), disk_consistent_lsn: timeline.get_disk_consistent_lsn(), last_record_lsn: timeline.get_last_record_lsn(), prev_record_lsn: timeline.get_prev_record_lsn(), diff --git a/zenith/src/main.rs b/zenith/src/main.rs index 5500d924ea..bc42af5943 100644 --- a/zenith/src/main.rs +++ b/zenith/src/main.rs @@ -392,7 +392,7 @@ fn get_tenantid(sub_match: &ArgMatches, env: &local_env::LocalEnv) -> Result Result<()> { let pageserver = PageServerNode::from_env(&env); if let Err(e) = pageserver.init( // default_tenantid was generated by the `env.init()` call above - Some(&env.default_tenantid.unwrap().to_string()), + Some(&ZTenantId::from(env.default_tenantid.unwrap()).to_string()), &pageserver_config_overrides(init_match), ) { eprintln!("pageserver init failed: {}", e); diff --git a/zenith_utils/src/auth.rs b/zenith_utils/src/auth.rs index 274dd13bee..cbc4fcee61 100644 --- a/zenith_utils/src/auth.rs +++ b/zenith_utils/src/auth.rs @@ -5,9 +5,7 @@ // The second one is that we wanted to use ed25519 keys, but they are also not supported until next version. So we go with RSA keys for now. // Relevant issue: https://github.com/Keats/jsonwebtoken/issues/162 -use hex::{self, FromHex}; -use serde::de::Error; -use serde::{self, Deserializer, Serializer}; +use serde; use std::fs; use std::path::Path; @@ -17,7 +15,7 @@ use jsonwebtoken::{ }; use serde::{Deserialize, Serialize}; -use crate::zid::ZTenantId; +use crate::zid::{HexZTenantId, ZTenantId}; const JWT_ALGORITHM: Algorithm = Algorithm::RS256; @@ -28,44 +26,18 @@ pub enum Scope { PageServerApi, } -pub fn to_hex_option(value: &Option, serializer: S) -> Result -where - S: Serializer, -{ - match value { - Some(tid) => hex::serialize(tid, serializer), - None => Option::serialize(value, serializer), - } -} - -fn from_hex_option<'de, D>(deserializer: D) -> Result, D::Error> -where - D: Deserializer<'de>, -{ - let opt: Option = Option::deserialize(deserializer)?; - match opt { - Some(tid) => Ok(Some(ZTenantId::from_hex(tid).map_err(Error::custom)?)), - None => Ok(None), - } -} - #[derive(Debug, Serialize, Deserialize, Clone)] pub struct Claims { - // this custom serialize/deserialize_with is needed because Option is not transparent to serde - // so clearest option is serde(with = "hex") but it is not working, for details see https://github.com/serde-rs/serde/issues/1301 - #[serde( - default, - skip_serializing_if = "Option::is_none", - serialize_with = "to_hex_option", - deserialize_with = "from_hex_option" - )] - pub tenant_id: Option, + pub tenant_id: Option, pub scope: Scope, } impl Claims { pub fn new(tenant_id: Option, scope: Scope) -> Self { - Self { tenant_id, scope } + Self { + tenant_id: tenant_id.map(HexZTenantId::from), + scope, + } } } @@ -75,7 +47,7 @@ pub fn check_permission(claims: &Claims, tenantid: Option) -> Result< bail!("Attempt to access management api with tenant scope. Permission denied") } (Scope::Tenant, Some(tenantid)) => { - if claims.tenant_id.unwrap() != tenantid { + if ZTenantId::from(claims.tenant_id.unwrap()) != tenantid { bail!("Tenant id mismatch. Permission denied") } Ok(()) diff --git a/zenith_utils/src/zid.rs b/zenith_utils/src/zid.rs index 7dfffd96d7..813eb3f8f4 100644 --- a/zenith_utils/src/zid.rs +++ b/zenith_utils/src/zid.rs @@ -2,13 +2,100 @@ use std::{fmt, str::FromStr}; use hex::FromHex; use rand::Rng; -use serde::{Deserialize, Serialize}; +use serde::{ + de::{self, Visitor}, + Deserialize, Serialize, +}; -// Zenith ID is a 128-bit random ID. -// Used to represent various identifiers. Provides handy utility methods and impls. +macro_rules! mutual_from { + ($id1:ident, $id2:ident) => { + impl From<$id1> for $id2 { + fn from(id1: $id1) -> Self { + Self(id1.0.into()) + } + } + + impl From<$id2> for $id1 { + fn from(id2: $id2) -> Self { + Self(id2.0.into()) + } + } + }; +} + +/// Zenith ID is a 128-bit random ID. +/// Used to represent various identifiers. Provides handy utility methods and impls. +/// +/// NOTE: It (de)serializes as an array of hex bytes, so the string representation would look +/// like `[173,80,132,115,129,226,72,254,170,201,135,108,199,26,228,24]`. +/// Use [`HexZId`] to serialize it as hex string instead: `ad50847381e248feaac9876cc71ae418`. #[derive(Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize, PartialOrd, Ord)] struct ZId([u8; 16]); +/// [`ZId`] version that serializes and deserializes as a hex string. +/// Useful for various json serializations, where hex byte array from original id is not convenient. +/// +/// Plain `ZId` could be (de)serialized into hex string with `#[serde(with = "hex")]` attribute. +/// This however won't work on nested types like `Option` or `Vec`, see https://github.com/serde-rs/serde/issues/723 for the details. +/// Every separate type currently needs a new (de)serializing method for every type separately. +/// +/// To provide a generic way to serialize the ZId as a hex string where `#[serde(with = "hex")]` is not enough, this wrapper is created. +/// The default wrapper serialization is left unchanged due to +/// * byte array (de)serialization being faster and simpler +/// * byte deserialization being used in Safekeeper already, with those bytes coming from compute (see `ProposerGreeting` in safekeeper) +/// * current `HexZId`'s deserialization impl breaks on compute byte array deserialization, having it by default is dangerous +#[derive(Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)] +struct HexZId([u8; 16]); + +impl Serialize for HexZId { + fn serialize(&self, ser: S) -> Result + where + S: serde::Serializer, + { + hex::encode(self.0).serialize(ser) + } +} + +impl<'de> Deserialize<'de> for HexZId { + fn deserialize(de: D) -> Result + where + D: serde::Deserializer<'de>, + { + de.deserialize_bytes(HexVisitor) + } +} + +struct HexVisitor; + +impl<'de> Visitor<'de> for HexVisitor { + type Value = HexZId; + + fn expecting(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!( + f, + "A hexadecimal representation of a 128-bit random Zenith ID" + ) + } + + fn visit_bytes(self, hex_bytes: &[u8]) -> Result + where + E: de::Error, + { + ZId::from_hex(hex_bytes) + .map(HexZId::from) + .map_err(de::Error::custom) + } + + fn visit_str(self, hex_bytes_str: &str) -> Result + where + E: de::Error, + { + Self::visit_bytes(self, hex_bytes_str.as_bytes()) + } +} + +mutual_from!(ZId, HexZId); + impl ZId { pub fn get_from_buf(buf: &mut dyn bytes::Buf) -> ZId { let mut arr = [0u8; 16]; @@ -155,46 +242,80 @@ macro_rules! zid_newtype { /// is separate from PostgreSQL timelines, and doesn't have those /// limitations. A zenith timeline is identified by a 128-bit ID, which /// is usually printed out as a hex string. +/// +/// NOTE: It (de)serializes as an array of hex bytes, so the string representation would look +/// like `[173,80,132,115,129,226,72,254,170,201,135,108,199,26,228,24]`. +/// Use [`HexZTimelineId`] to serialize it as hex string instead: `ad50847381e248feaac9876cc71ae418`. #[derive(Clone, Copy, PartialEq, Eq, Hash, Ord, PartialOrd, Serialize, Deserialize)] pub struct ZTimelineId(ZId); -zid_newtype!(ZTimelineId); +/// A [`ZTimelineId`] version that gets (de)serialized as a hex string. +/// Use in complex types, where `#[serde(with = "hex")]` does not work. +/// See [`HexZId`] for more details. +#[derive(Clone, Copy, PartialEq, Eq, Hash, Ord, PartialOrd, Serialize, Deserialize)] +pub struct HexZTimelineId(HexZId); -// Zenith Tenant Id represents identifiar of a particular tenant. -// Is used for distinguishing requests and data belonging to different users. +impl std::fmt::Debug for HexZTimelineId { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + ZTimelineId::from(*self).fmt(f) + } +} + +impl std::fmt::Display for HexZTimelineId { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + ZTimelineId::from(*self).fmt(f) + } +} + +impl FromStr for HexZTimelineId { + type Err = ::Err; + + fn from_str(s: &str) -> Result { + Ok(HexZTimelineId::from(ZTimelineId::from_str(s)?)) + } +} + +zid_newtype!(ZTimelineId); +mutual_from!(ZTimelineId, HexZTimelineId); + +/// Zenith Tenant Id represents identifiar of a particular tenant. +/// Is used for distinguishing requests and data belonging to different users. +/// +/// NOTE: It (de)serializes as an array of hex bytes, so the string representation would look +/// like `[173,80,132,115,129,226,72,254,170,201,135,108,199,26,228,24]`. +/// Use [`HexZTenantId`] to serialize it as hex string instead: `ad50847381e248feaac9876cc71ae418`. #[derive(Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize, PartialOrd, Ord)] pub struct ZTenantId(ZId); -zid_newtype!(ZTenantId); +/// A [`ZTenantId`] version that gets (de)serialized as a hex string. +/// Use in complex types, where `#[serde(with = "hex")]` does not work. +/// See [`HexZId`] for more details. +#[derive(Clone, Copy, PartialEq, Eq, Hash, Ord, PartialOrd, Serialize, Deserialize)] +pub struct HexZTenantId(HexZId); -/// Serde routines for Option (de)serialization, using `T:Display` representations for inner values. -/// Useful for Option and Option to get their hex representations into serialized string and deserialize them back. -pub mod opt_display_serde { - use serde::{de, Deserialize, Deserializer, Serialize, Serializer}; - use std::{fmt::Display, str::FromStr}; - - pub fn serialize(id: &Option, ser: S) -> Result - where - S: Serializer, - Id: Display, - { - id.as_ref().map(ToString::to_string).serialize(ser) - } - - pub fn deserialize<'de, D, Id>(des: D) -> Result, D::Error> - where - D: Deserializer<'de>, - Id: FromStr, - ::Err: Display, - { - Ok(if let Some(s) = Option::::deserialize(des)? { - Some(Id::from_str(&s).map_err(de::Error::custom)?) - } else { - None - }) +impl std::fmt::Debug for HexZTenantId { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + ZTenantId::from(*self).fmt(f) } } +impl std::fmt::Display for HexZTenantId { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + ZTenantId::from(*self).fmt(f) + } +} + +impl FromStr for HexZTenantId { + type Err = ::Err; + + fn from_str(s: &str) -> Result { + Ok(HexZTenantId::from(ZTenantId::from_str(s)?)) + } +} + +zid_newtype!(ZTenantId); +mutual_from!(ZTenantId, HexZTenantId); + // A pair uniquely identifying Zenith instance. #[derive(Debug, Clone, Copy, PartialOrd, Ord, PartialEq, Eq, Hash)] pub struct ZTenantTimelineId { @@ -243,16 +364,15 @@ mod tests { #[derive(Debug, PartialEq, Eq, Serialize, Deserialize)] struct TestStruct + Display> { - #[serde(with = "opt_display_serde")] field: Option, } #[test] fn test_hex_serializations_tenant_id() { let original_struct = TestStruct { - field: Some(ZTenantId::from_array(hex!( + field: Some(HexZTenantId::from(ZTenantId::from_array(hex!( "11223344556677881122334455667788" - ))), + )))), }; let serialized_string = serde_json::to_string(&original_struct).unwrap(); @@ -261,7 +381,7 @@ mod tests { r#"{"field":"11223344556677881122334455667788"}"# ); - let deserialized_struct: TestStruct = + let deserialized_struct: TestStruct = serde_json::from_str(&serialized_string).unwrap(); assert_eq!(original_struct, deserialized_struct); } @@ -269,9 +389,9 @@ mod tests { #[test] fn test_hex_serializations_timeline_id() { let original_struct = TestStruct { - field: Some(ZTimelineId::from_array(hex!( + field: Some(HexZTimelineId::from(ZTimelineId::from_array(hex!( "AA223344556677881122334455667788" - ))), + )))), }; let serialized_string = serde_json::to_string(&original_struct).unwrap(); @@ -280,7 +400,7 @@ mod tests { r#"{"field":"aa223344556677881122334455667788"}"# ); - let deserialized_struct: TestStruct = + let deserialized_struct: TestStruct = serde_json::from_str(&serialized_string).unwrap(); assert_eq!(original_struct, deserialized_struct); }