aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAlex Auvolat <alex@adnab.me>2022-01-04 18:59:17 +0100
committerAlex Auvolat <alex@adnab.me>2022-01-04 18:59:17 +0100
commit677ab60cc117677bf53dc4887a6ff1c094e17cd0 (patch)
treedeed8c5e3a0e22fa0d1d987d4ca1313fd5c54fb7
parentdf35feba18787cac06b2a87e3426752fb78da2d5 (diff)
downloadgarage-new-buckets.tar.gz
garage-new-buckets.zip
Small changes in key model and refactoringnew-buckets
-rw-r--r--src/api/s3_bucket.rs8
-rw-r--r--src/api/signature.rs3
-rw-r--r--src/garage/admin.rs21
-rw-r--r--src/garage/cli/util.rs31
-rw-r--r--src/model/bucket_table.rs29
-rw-r--r--src/model/key_table.rs116
-rw-r--r--src/model/permission.rs14
7 files changed, 119 insertions, 103 deletions
diff --git a/src/api/s3_bucket.rs b/src/api/s3_bucket.rs
index 50aeb173..27208ffa 100644
--- a/src/api/s3_bucket.rs
+++ b/src/api/s3_bucket.rs
@@ -38,8 +38,8 @@ pub fn handle_get_bucket_versioning() -> Result<Response<Body>, Error> {
}
pub async fn handle_list_buckets(garage: &Garage, api_key: &Key) -> Result<Response<Body>, Error> {
- let key_state = api_key.state.as_option().ok_or_internal_error(
- "Key should not be in deleted state at this point (internal error)",
+ let key_p = api_key.params().ok_or_internal_error(
+ "Key should not be in deleted state at this point (in handle_list_buckets)",
)?;
// Collect buckets user has access to
@@ -74,7 +74,7 @@ pub async fn handle_list_buckets(garage: &Garage, api_key: &Key) -> Result<Respo
}
}
- for (alias, _, id_opt) in key_state.local_aliases.items() {
+ for (alias, _, id_opt) in key_p.local_aliases.items() {
if let Some(id) = id_opt {
aliases.insert(alias.clone(), *id);
}
@@ -83,7 +83,7 @@ pub async fn handle_list_buckets(garage: &Garage, api_key: &Key) -> Result<Respo
// Generate response
let list_buckets = s3_xml::ListAllMyBucketsResult {
owner: s3_xml::Owner {
- display_name: s3_xml::Value(api_key.name.get().to_string()),
+ display_name: s3_xml::Value(key_p.name.get().to_string()),
id: s3_xml::Value(api_key.key_id.to_string()),
},
buckets: s3_xml::BucketList {
diff --git a/src/api/signature.rs b/src/api/signature.rs
index b5da7b62..c580cb3a 100644
--- a/src/api/signature.rs
+++ b/src/api/signature.rs
@@ -66,6 +66,7 @@ pub async fn check_signature(
.await?
.filter(|k| !k.state.is_deleted())
.ok_or_else(|| Error::Forbidden(format!("No such key: {}", authorization.key_id)))?;
+ let key_p = key.params().unwrap();
let canonical_request = canonical_request(
request.method(),
@@ -79,7 +80,7 @@ pub async fn check_signature(
let mut hmac = signing_hmac(
&date,
- &key.secret_key,
+ &key_p.secret_key,
&garage.config.s3_api.s3_region,
"s3",
)
diff --git a/src/garage/admin.rs b/src/garage/admin.rs
index b1eb6915..a682075f 100644
--- a/src/garage/admin.rs
+++ b/src/garage/admin.rs
@@ -107,7 +107,13 @@ impl AdminRpcHandler {
.items()
.iter()
{
- if let Some(key) = self.garage.key_table.get(&EmptyKey, k).await? {
+ if let Some(key) = self
+ .garage
+ .key_table
+ .get(&EmptyKey, k)
+ .await?
+ .filter(|k| !k.is_deleted())
+ {
relevant_keys.insert(k.clone(), key);
}
}
@@ -222,7 +228,7 @@ impl AdminRpcHandler {
// 1. delete authorization from keys that had access
for (key_id, _) in bucket.authorized_keys() {
helper
- .set_bucket_key_permissions(bucket.id, key_id, BucketKeyPerm::no_permissions())
+ .set_bucket_key_permissions(bucket.id, key_id, BucketKeyPerm::NO_PERMISSIONS)
.await?;
}
@@ -439,7 +445,7 @@ impl AdminRpcHandler {
)
.await?
.iter()
- .map(|k| (k.key_id.to_string(), k.name.get().clone()))
+ .map(|k| (k.key_id.to_string(), k.params().unwrap().name.get().clone()))
.collect::<Vec<_>>();
Ok(AdminRpc::KeyList(key_ids))
}
@@ -454,7 +460,7 @@ impl AdminRpcHandler {
}
async fn handle_create_key(&self, query: &KeyNewOpt) -> Result<AdminRpc, Error> {
- let key = Key::new(query.name.clone());
+ let key = Key::new(&query.name);
self.garage.key_table.insert(&key).await?;
self.key_info_result(key).await
}
@@ -465,7 +471,10 @@ impl AdminRpcHandler {
.bucket_helper()
.get_existing_matching_key(&query.key_pattern)
.await?;
- key.name.update(query.new_name.clone());
+ key.params_mut()
+ .unwrap()
+ .name
+ .update(query.new_name.clone());
self.garage.key_table.insert(&key).await?;
self.key_info_result(key).await
}
@@ -500,7 +509,7 @@ impl AdminRpcHandler {
// 2. Remove permissions on all authorized buckets
for (ab_id, _auth) in state.authorized_buckets.items().iter() {
helper
- .set_bucket_key_permissions(*ab_id, &key.key_id, BucketKeyPerm::no_permissions())
+ .set_bucket_key_permissions(*ab_id, &key.key_id, BucketKeyPerm::NO_PERMISSIONS)
.await?;
}
diff --git a/src/garage/cli/util.rs b/src/garage/cli/util.rs
index 7a7d0e9b..365831c4 100644
--- a/src/garage/cli/util.rs
+++ b/src/garage/cli/util.rs
@@ -65,11 +65,11 @@ pub fn print_key_info(key: &Key, relevant_buckets: &HashMap<Uuid, Bucket>) {
"".to_string()
};
- println!("Key name: {}", key.name.get());
- println!("Key ID: {}", key.key_id);
- println!("Secret key: {}", key.secret_key);
match &key.state {
Deletable::Present(p) => {
+ println!("Key name: {}", p.name.get());
+ println!("Key ID: {}", key.key_id);
+ println!("Secret key: {}", p.secret_key);
println!("Can create buckets: {}", p.allow_create_bucket.get());
println!("\nKey-specific bucket aliases:");
let mut table = vec![];
@@ -112,12 +112,19 @@ pub fn print_key_info(key: &Key, relevant_buckets: &HashMap<Uuid, Bucket>) {
format_table(table);
}
Deletable::Deleted => {
- println!("\nKey is deleted.");
+ println!("Key {} is deleted.", key.key_id);
}
}
}
pub fn print_bucket_info(bucket: &Bucket, relevant_keys: &HashMap<String, Key>) {
+ let key_name = |k| {
+ relevant_keys
+ .get(k)
+ .map(|k| k.params().unwrap().name.get().as_str())
+ .unwrap_or("<deleted>")
+ };
+
println!("Bucket: {}", hex::encode(bucket.id));
match &bucket.state {
Deletable::Deleted => println!("Bucket is deleted."),
@@ -135,11 +142,7 @@ pub fn print_bucket_info(bucket: &Bucket, relevant_keys: &HashMap<String, Key>)
let mut table = vec![];
for ((key_id, alias), _, active) in p.local_aliases.items().iter() {
if *active {
- let key_name = relevant_keys
- .get(key_id)
- .map(|k| k.name.get().as_str())
- .unwrap_or("");
- table.push(format!("\t{} ({})\t{}", key_id, key_name, alias));
+ table.push(format!("\t{} ({})\t{}", key_id, key_name(key_id), alias));
}
}
format_table(table);
@@ -150,13 +153,13 @@ pub fn print_bucket_info(bucket: &Bucket, relevant_keys: &HashMap<String, Key>)
let rflag = if perm.allow_read { "R" } else { " " };
let wflag = if perm.allow_write { "W" } else { " " };
let oflag = if perm.allow_owner { "O" } else { " " };
- let key_name = relevant_keys
- .get(k)
- .map(|k| k.name.get().as_str())
- .unwrap_or("");
table.push(format!(
"\t{}{}{}\t{}\t{}",
- rflag, wflag, oflag, k, key_name
+ rflag,
+ wflag,
+ oflag,
+ k,
+ key_name(k)
));
}
format_table(table);
diff --git a/src/model/bucket_table.rs b/src/model/bucket_table.rs
index d687e774..db7cec18 100644
--- a/src/model/bucket_table.rs
+++ b/src/model/bucket_table.rs
@@ -97,27 +97,32 @@ impl Bucket {
self.state.is_deleted()
}
+ /// Returns an option representing the parameters (None if in deleted state)
+ pub fn params(&self) -> Option<&BucketParams> {
+ self.state.as_option()
+ }
+
+ /// Mutable version of `.params()`
+ pub fn params_mut(&mut self) -> Option<&mut BucketParams> {
+ self.state.as_option_mut()
+ }
+
/// Return the list of authorized keys, when each was updated, and the permission associated to
/// the key
pub fn authorized_keys(&self) -> &[(String, BucketKeyPerm)] {
- match &self.state {
- crdt::Deletable::Deleted => &[],
- crdt::Deletable::Present(state) => state.authorized_keys.items(),
- }
+ self.params()
+ .map(|s| s.authorized_keys.items())
+ .unwrap_or(&[])
}
pub fn aliases(&self) -> &[(String, u64, bool)] {
- match &self.state {
- crdt::Deletable::Deleted => &[],
- crdt::Deletable::Present(state) => state.aliases.items(),
- }
+ self.params().map(|s| s.aliases.items()).unwrap_or(&[])
}
pub fn local_aliases(&self) -> &[((String, String), u64, bool)] {
- match &self.state {
- crdt::Deletable::Deleted => &[],
- crdt::Deletable::Present(state) => state.local_aliases.items(),
- }
+ self.params()
+ .map(|s| s.local_aliases.items())
+ .unwrap_or(&[])
}
}
diff --git a/src/model/key_table.rs b/src/model/key_table.rs
index c25f2da4..d5e30f3f 100644
--- a/src/model/key_table.rs
+++ b/src/model/key_table.rs
@@ -14,29 +14,37 @@ pub struct Key {
/// The id of the key (immutable), used as partition key
pub key_id: String,
- /// The secret_key associated
+ /// Internal state of the key
+ pub state: crdt::Deletable<KeyParams>,
+}
+
+/// Configuration for a key
+#[derive(PartialEq, Clone, Debug, Serialize, Deserialize)]
+pub struct KeyParams {
+ /// The secret_key associated (immutable)
pub secret_key: String,
/// Name for the key
pub name: crdt::Lww<String>,
+ /// Flag to allow users having this key to create buckets
+ pub allow_create_bucket: crdt::Lww<bool>,
+
/// If the key is present: it gives some permissions,
/// a map of bucket IDs (uuids) to permissions.
/// Otherwise no permissions are granted to key
- pub state: crdt::Deletable<KeyParams>,
-}
-
-/// Configuration for a key
-#[derive(PartialEq, Clone, Debug, Serialize, Deserialize)]
-pub struct KeyParams {
- pub allow_create_bucket: crdt::Lww<bool>,
pub authorized_buckets: crdt::Map<Uuid, BucketKeyPerm>,
+
+ /// A key can have a local view of buckets names it is
+ /// the only one to see, this is the namespace for these aliases
pub local_aliases: crdt::LwwMap<String, Option<Uuid>>,
}
impl KeyParams {
- pub fn new() -> Self {
+ fn new(secret_key: &str, name: &str) -> Self {
KeyParams {
+ secret_key: secret_key.to_string(),
+ name: crdt::Lww::new(name.to_string()),
allow_create_bucket: crdt::Lww::new(false),
authorized_buckets: crdt::Map::new(),
local_aliases: crdt::LwwMap::new(),
@@ -44,14 +52,9 @@ impl KeyParams {
}
}
-impl Default for KeyParams {
- fn default() -> Self {
- Self::new()
- }
-}
-
impl Crdt for KeyParams {
fn merge(&mut self, o: &Self) {
+ self.name.merge(&o.name);
self.allow_create_bucket.merge(&o.allow_create_bucket);
self.authorized_buckets.merge(&o.authorized_buckets);
self.local_aliases.merge(&o.local_aliases);
@@ -60,14 +63,12 @@ impl Crdt for KeyParams {
impl Key {
/// Initialize a new Key, generating a random identifier and associated secret key
- pub fn new(name: String) -> Self {
+ pub fn new(name: &str) -> Self {
let key_id = format!("GK{}", hex::encode(&rand::random::<[u8; 12]>()[..]));
let secret_key = hex::encode(&rand::random::<[u8; 32]>()[..]);
Self {
key_id,
- secret_key,
- name: crdt::Lww::new(name),
- state: crdt::Deletable::present(KeyParams::new()),
+ state: crdt::Deletable::present(KeyParams::new(&secret_key, name)),
}
}
@@ -75,9 +76,7 @@ impl Key {
pub fn import(key_id: &str, secret_key: &str, name: &str) -> Self {
Self {
key_id: key_id.to_string(),
- secret_key: secret_key.to_string(),
- name: crdt::Lww::new(name.to_string()),
- state: crdt::Deletable::present(KeyParams::new()),
+ state: crdt::Deletable::present(KeyParams::new(secret_key, name)),
}
}
@@ -85,49 +84,47 @@ impl Key {
pub fn delete(key_id: String) -> Self {
Self {
key_id,
- secret_key: "".into(),
- name: crdt::Lww::new("".to_string()),
state: crdt::Deletable::Deleted,
}
}
+ /// Returns true if this represents a deleted bucket
+ pub fn is_deleted(&self) -> bool {
+ self.state.is_deleted()
+ }
+
+ /// Returns an option representing the params (None if in deleted state)
+ pub fn params(&self) -> Option<&KeyParams> {
+ self.state.as_option()
+ }
+
+ /// Mutable version of `.state()`
+ pub fn params_mut(&mut self) -> Option<&mut KeyParams> {
+ self.state.as_option_mut()
+ }
+
+ /// Get permissions for a bucket
+ pub fn bucket_permissions(&self, bucket: &Uuid) -> BucketKeyPerm {
+ self.params()
+ .map(|params| params.authorized_buckets.get(bucket))
+ .flatten()
+ .cloned()
+ .unwrap_or(BucketKeyPerm::NO_PERMISSIONS)
+ }
+
/// Check if `Key` is allowed to read in bucket
pub fn allow_read(&self, bucket: &Uuid) -> bool {
- if let crdt::Deletable::Present(params) = &self.state {
- params
- .authorized_buckets
- .get(bucket)
- .map(|x| x.allow_read)
- .unwrap_or(false)
- } else {
- false
- }
+ self.bucket_permissions(bucket).allow_read
}
/// Check if `Key` is allowed to write in bucket
pub fn allow_write(&self, bucket: &Uuid) -> bool {
- if let crdt::Deletable::Present(params) = &self.state {
- params
- .authorized_buckets
- .get(bucket)
- .map(|x| x.allow_write)
- .unwrap_or(false)
- } else {
- false
- }
+ self.bucket_permissions(bucket).allow_write
}
/// Check if `Key` is owner of bucket
pub fn allow_owner(&self, bucket: &Uuid) -> bool {
- if let crdt::Deletable::Present(params) = &self.state {
- params
- .authorized_buckets
- .get(bucket)
- .map(|x| x.allow_owner)
- .unwrap_or(false)
- } else {
- false
- }
+ self.bucket_permissions(bucket).allow_owner
}
}
@@ -142,7 +139,6 @@ impl Entry<EmptyKey, String> for Key {
impl Crdt for Key {
fn merge(&mut self, other: &Self) {
- self.name.merge(&other.name);
self.state.merge(&other.state);
}
}
@@ -168,15 +164,20 @@ impl TableSchema for KeyTable {
KeyFilter::Deleted(df) => df.apply(entry.state.is_deleted()),
KeyFilter::MatchesAndNotDeleted(pat) => {
let pat = pat.to_lowercase();
- !entry.state.is_deleted()
- && (entry.key_id.to_lowercase().starts_with(&pat)
- || entry.name.get().to_lowercase() == pat)
+ entry
+ .params()
+ .map(|p| {
+ entry.key_id.to_lowercase().starts_with(&pat)
+ || p.name.get().to_lowercase() == pat
+ })
+ .unwrap_or(false)
}
}
}
fn try_migrate(bytes: &[u8]) -> Option<Self::E> {
let old_k = rmp_serde::decode::from_read_ref::<_, old::Key>(bytes).ok()?;
+ let name = crdt::Lww::raw(old_k.name.timestamp(), old_k.name.get().clone());
let state = if old_k.deleted.get() {
crdt::Deletable::Deleted
@@ -185,16 +186,15 @@ impl TableSchema for KeyTable {
// migration is performed in specific migration code in
// garage/migrate.rs
crdt::Deletable::Present(KeyParams {
+ secret_key: old_k.secret_key,
+ name,
allow_create_bucket: crdt::Lww::new(false),
authorized_buckets: crdt::Map::new(),
local_aliases: crdt::LwwMap::new(),
})
};
- let name = crdt::Lww::raw(old_k.name.timestamp(), old_k.name.get().clone());
Some(Key {
key_id: old_k.key_id,
- secret_key: old_k.secret_key,
- name,
state,
})
}
diff --git a/src/model/permission.rs b/src/model/permission.rs
index b8f7dd71..67527ed0 100644
--- a/src/model/permission.rs
+++ b/src/model/permission.rs
@@ -21,14 +21,12 @@ pub struct BucketKeyPerm {
}
impl BucketKeyPerm {
- pub fn no_permissions() -> Self {
- Self {
- timestamp: 0,
- allow_read: false,
- allow_write: false,
- allow_owner: false,
- }
- }
+ pub const NO_PERMISSIONS: Self = Self {
+ timestamp: 0,
+ allow_read: false,
+ allow_write: false,
+ allow_owner: false,
+ };
}
impl Crdt for BucketKeyPerm {