diff --git a/src/api/api_server.rs b/src/api/api_server.rs index 42987e78..f5ebed37 100644 --- a/src/api/api_server.rs +++ b/src/api/api_server.rs @@ -7,7 +7,6 @@ use hyper::server::conn::AddrStream; use hyper::service::{make_service_fn, service_fn}; use hyper::{Body, Request, Response, Server}; -use garage_util::crdt; use garage_util::data::*; use garage_util::error::Error as GarageError; @@ -306,8 +305,7 @@ async fn resolve_bucket( .as_option() .ok_or_else(|| Error::Forbidden("Operation is not allowed for this key.".to_string()))?; - if let Some(crdt::Deletable::Present(bucket_id)) = api_key_params.local_aliases.get(bucket_name) - { + if let Some(Some(bucket_id)) = api_key_params.local_aliases.get(bucket_name) { Ok(*bucket_id) } else { Ok(garage diff --git a/src/api/s3_bucket.rs b/src/api/s3_bucket.rs index 785b89dd..24ec6b98 100644 --- a/src/api/s3_bucket.rs +++ b/src/api/s3_bucket.rs @@ -65,8 +65,8 @@ pub async fn handle_list_buckets(garage: &Garage, api_key: &Key) -> Result Result) { println!("\nKey-specific bucket aliases:"); let mut table = vec![]; for (alias_name, _, alias) in p.local_aliases.items().iter() { - if let Some(bucket_id) = alias.as_option() { + if let Some(bucket_id) = alias { table.push(format!( "\t{}\t{}\t{}", alias_name, @@ -55,7 +55,7 @@ pub fn print_key_info(key: &Key, relevant_buckets: &HashMap) { .local_aliases .items() .iter() - .filter(|(_, _, a)| a.as_option() == Some(bucket_id)) + .filter(|(_, _, a)| *a == Some(*bucket_id)) .map(|(a, _, _)| a.clone()) .collect::>() .join(", "); diff --git a/src/model/bucket_alias_table.rs b/src/model/bucket_alias_table.rs index 45807178..fce03d04 100644 --- a/src/model/bucket_alias_table.rs +++ b/src/model/bucket_alias_table.rs @@ -10,32 +10,23 @@ use garage_table::*; #[derive(PartialEq, Clone, Debug, Serialize, Deserialize)] pub struct BucketAlias { name: String, - pub state: crdt::Lww>, -} - -#[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Debug, Serialize, Deserialize)] -pub struct AliasParams { - pub bucket_id: Uuid, -} - -impl AutoCrdt for AliasParams { - const WARN_IF_DIFFERENT: bool = true; + pub state: crdt::Lww>, } impl BucketAlias { - pub fn new(name: String, ts: u64, bucket_id: Uuid) -> Option { + pub fn new(name: String, ts: u64, bucket_id: Option) -> Option { if !is_valid_bucket_name(&name) { None } else { Some(BucketAlias { name, - state: crdt::Lww::raw(ts, crdt::Deletable::present(AliasParams { bucket_id })), + state: crdt::Lww::raw(ts, bucket_id), }) } } pub fn is_deleted(&self) -> bool { - self.state.get().is_deleted() + self.state.get().is_none() } pub fn name(&self) -> &str { &self.name diff --git a/src/model/bucket_table.rs b/src/model/bucket_table.rs index fef29b62..52c2316c 100644 --- a/src/model/bucket_table.rs +++ b/src/model/bucket_table.rs @@ -63,6 +63,7 @@ impl BucketParams { impl Crdt for BucketParams { fn merge(&mut self, o: &Self) { + self.creation_date = std::cmp::min(self.creation_date, o.creation_date); self.authorized_keys.merge(&o.authorized_keys); self.website_config.merge(&o.website_config); self.aliases.merge(&o.aliases); diff --git a/src/model/helper/bucket.rs b/src/model/helper/bucket.rs index 8bc54b3f..52cedb12 100644 --- a/src/model/helper/bucket.rs +++ b/src/model/helper/bucket.rs @@ -46,7 +46,7 @@ impl<'a> BucketHelper<'a> { .bucket_alias_table .get(&EmptyKey, bucket_name) .await? - .map(|x| x.state.get().as_option().map(|x| x.bucket_id)) + .map(|x| *x.state.get()) .flatten()) } } @@ -154,11 +154,11 @@ impl<'a> BucketHelper<'a> { let alias = self.0.bucket_alias_table.get(&EmptyKey, alias_name).await?; if let Some(existing_alias) = alias.as_ref() { - if let Some(p) = existing_alias.state.get().as_option() { - if p.bucket_id != bucket_id { + if let Some(p_bucket) = existing_alias.state.get() { + if *p_bucket != bucket_id { return Err(Error::BadRequest(format!( "Alias {} already exists and points to different bucket: {:?}", - alias_name, p.bucket_id + alias_name, p_bucket ))); } } @@ -176,10 +176,10 @@ impl<'a> BucketHelper<'a> { // writes are now done and all writes use timestamp alias_ts let alias = match alias { - None => BucketAlias::new(alias_name.clone(), alias_ts, bucket_id) + None => BucketAlias::new(alias_name.clone(), alias_ts, Some(bucket_id)) .ok_or_bad_request(format!("{}: {}", alias_name, INVALID_BUCKET_NAME_MESSAGE))?, Some(mut a) => { - a.state = Lww::raw(alias_ts, Deletable::present(AliasParams { bucket_id })); + a.state = Lww::raw(alias_ts, Some(bucket_id)); a } }; @@ -209,13 +209,7 @@ impl<'a> BucketHelper<'a> { .bucket_alias_table .get(&EmptyKey, alias_name) .await? - .filter(|a| { - a.state - .get() - .as_option() - .map(|x| x.bucket_id == bucket_id) - .unwrap_or(false) - }) + .filter(|a| a.state.get().map(|x| x == bucket_id).unwrap_or(false)) .ok_or_message(format!( "Internal error: alias not found or does not point to bucket {:?}", bucket_id @@ -244,7 +238,7 @@ impl<'a> BucketHelper<'a> { // ---- timestamp-ensured causality barrier ---- // writes are now done and all writes use timestamp alias_ts - alias.state = Lww::raw(alias_ts, Deletable::delete()); + alias.state = Lww::raw(alias_ts, None); self.0.bucket_alias_table.insert(&alias).await?; bucket_state.aliases = LwwMap::raw_item(alias_name.clone(), alias_ts, false); @@ -253,6 +247,51 @@ impl<'a> BucketHelper<'a> { Ok(()) } + /// Ensures a bucket does not have a certain global alias. + /// Contrarily to unset_global_bucket_alias, this does not + /// fail on any condition other than: + /// - bucket cannot be found (its fine if it is in deleted state) + /// - alias cannot be found (its fine if it points to nothing or + /// to another bucket) + pub async fn purge_global_bucket_alias( + &self, + bucket_id: Uuid, + alias_name: &String, + ) -> Result<(), Error> { + let mut bucket = self.get_internal_bucket(bucket_id).await?; + + let mut alias = self + .0 + .bucket_alias_table + .get(&EmptyKey, alias_name) + .await? + .ok_or_message(format!("Alias {} not found", alias_name))?; + + // Checks ok, remove alias + let alias_ts = match bucket.state.as_option() { + Some(bucket_state) => increment_logical_clock_2( + alias.state.timestamp(), + bucket_state.aliases.get_timestamp(alias_name), + ), + None => increment_logical_clock(alias.state.timestamp()), + }; + + // ---- timestamp-ensured causality barrier ---- + // writes are now done and all writes use timestamp alias_ts + + if alias.state.get() == &Some(bucket_id) { + alias.state = Lww::raw(alias_ts, None); + self.0.bucket_alias_table.insert(&alias).await?; + } + + if let Some(mut bucket_state) = bucket.state.as_option_mut() { + bucket_state.aliases = LwwMap::raw_item(alias_name.clone(), alias_ts, false); + self.0.bucket_table.insert(&bucket).await?; + } + + Ok(()) + } + /// Sets a new alias for a bucket in the local namespace of a key. /// This function fails if: /// - alias name is not valid according to S3 spec @@ -277,7 +316,7 @@ impl<'a> BucketHelper<'a> { let mut key_param = key.state.as_option_mut().unwrap(); - if let Some(Deletable::Present(existing_alias)) = key_param.local_aliases.get(alias_name) { + if let Some(Some(existing_alias)) = key_param.local_aliases.get(alias_name) { if *existing_alias != bucket_id { return Err(Error::BadRequest(format!("Alias {} already exists in namespace of key {} and points to different bucket: {:?}", alias_name, key.key_id, existing_alias))); } @@ -301,8 +340,7 @@ impl<'a> BucketHelper<'a> { // ---- timestamp-ensured causality barrier ---- // writes are now done and all writes use timestamp alias_ts - key_param.local_aliases = - LwwMap::raw_item(alias_name.clone(), alias_ts, Deletable::present(bucket_id)); + key_param.local_aliases = LwwMap::raw_item(alias_name.clone(), alias_ts, Some(bucket_id)); self.0.key_table.insert(&key).await?; bucket_p.local_aliases = LwwMap::raw_item(bucket_p_local_alias_key, alias_ts, true); @@ -334,8 +372,8 @@ impl<'a> BucketHelper<'a> { .unwrap() .local_aliases .get(alias_name) - .map(|x| x.as_option()) - .flatten() != Some(&bucket_id) + .cloned() + .flatten() != Some(bucket_id) { return Err(GarageError::Message(format!( "Bucket {:?} does not have alias {} in namespace of key {}", @@ -372,8 +410,7 @@ impl<'a> BucketHelper<'a> { // ---- timestamp-ensured causality barrier ---- // writes are now done and all writes use timestamp alias_ts - key_param.local_aliases = - LwwMap::raw_item(alias_name.clone(), alias_ts, Deletable::delete()); + key_param.local_aliases = LwwMap::raw_item(alias_name.clone(), alias_ts, None); self.0.key_table.insert(&key).await?; bucket_p.local_aliases = LwwMap::raw_item(bucket_p_local_alias_key, alias_ts, false); diff --git a/src/model/key_table.rs b/src/model/key_table.rs index 7afa0337..c25f2da4 100644 --- a/src/model/key_table.rs +++ b/src/model/key_table.rs @@ -31,7 +31,7 @@ pub struct Key { pub struct KeyParams { pub allow_create_bucket: crdt::Lww, pub authorized_buckets: crdt::Map, - pub local_aliases: crdt::LwwMap>, + pub local_aliases: crdt::LwwMap>, } impl KeyParams { diff --git a/src/util/crdt/crdt.rs b/src/util/crdt/crdt.rs index 00bb2e3b..06876897 100644 --- a/src/util/crdt/crdt.rs +++ b/src/util/crdt/crdt.rs @@ -1,5 +1,3 @@ -use crate::data::*; - /// Definition of a CRDT - all CRDT Rust types implement this. /// /// A CRDT is defined as a merge operator that respects a certain set of axioms. @@ -87,7 +85,3 @@ impl AutoCrdt for String { impl AutoCrdt for bool { const WARN_IF_DIFFERENT: bool = true; } - -impl AutoCrdt for FixedBytes32 { - const WARN_IF_DIFFERENT: bool = true; -} diff --git a/src/util/data.rs b/src/util/data.rs index 6b8ee527..f0744307 100644 --- a/src/util/data.rs +++ b/src/util/data.rs @@ -5,7 +5,7 @@ use serde::{Deserialize, Deserializer, Serialize, Serializer}; use std::fmt; /// An array of 32 bytes -#[derive(Default, PartialOrd, Ord, Clone, Hash, PartialEq, Copy)] +#[derive(Default, Clone, Hash, PartialEq, Eq, PartialOrd, Ord, Copy)] pub struct FixedBytes32([u8; 32]); impl From<[u8; 32]> for FixedBytes32 { @@ -20,8 +20,6 @@ impl std::convert::AsRef<[u8]> for FixedBytes32 { } } -impl Eq for FixedBytes32 {} - impl fmt::Debug for FixedBytes32 { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "{}…", hex::encode(&self.0[..8])) diff --git a/src/web/web_server.rs b/src/web/web_server.rs index cc6eed57..f13f289e 100644 --- a/src/web/web_server.rs +++ b/src/web/web_server.rs @@ -86,9 +86,8 @@ async fn serve_file(garage: Arc, req: Request) -> Result