Merge branch 'main' into next-v2

This commit is contained in:
Alex Auvolat
2025-05-23 16:33:07 +02:00
30 changed files with 522 additions and 142 deletions
+1
View File
@@ -1114,6 +1114,7 @@ pub enum RepairType {
BlockRc,
Rebalance,
Scrub(ScrubCommand),
Aliases,
}
#[derive(Debug, Clone, Serialize, Deserialize, ToSchema)]
+3 -3
View File
@@ -159,7 +159,7 @@ impl RequestHandler for CreateBucketRequest {
let helper = garage.locked_helper().await;
if let Some(ga) = &self.global_alias {
if !is_valid_bucket_name(ga) {
if !is_valid_bucket_name(ga, garage.config.allow_punycode) {
return Err(Error::bad_request(format!(
"{}: {}",
ga, INVALID_BUCKET_NAME_MESSAGE
@@ -174,7 +174,7 @@ impl RequestHandler for CreateBucketRequest {
}
if let Some(la) = &self.local_alias {
if !is_valid_bucket_name(&la.alias) {
if !is_valid_bucket_name(&la.alias, garage.config.allow_punycode) {
return Err(Error::bad_request(format!(
"{}: {}",
la.alias, INVALID_BUCKET_NAME_MESSAGE
@@ -255,7 +255,7 @@ impl RequestHandler for DeleteBucketRequest {
for ((key_id, alias), _, active) in state.local_aliases.items().iter() {
if *active {
helper
.unset_local_bucket_alias(bucket.id, key_id, alias)
.purge_local_bucket_alias(bucket.id, key_id, alias)
.await?;
}
}
+4
View File
@@ -89,6 +89,10 @@ impl RequestHandler for LocalLaunchRepairOperationRequest {
garage.block_manager.clone(),
));
}
RepairType::Aliases => {
info!("Repairing bucket aliases (foreground)");
garage.locked_helper().await.repair_aliases().await?;
}
}
Ok(LocalLaunchRepairOperationResponse)
}
-1
View File
@@ -27,7 +27,6 @@ err-derive.workspace = true
hex.workspace = true
hmac.workspace = true
md-5.workspace = true
idna.workspace = true
tracing.workspace = true
nom.workspace = true
pin-project.workspace = true
+1 -2
View File
@@ -8,7 +8,6 @@ use hyper::{
body::{Body, Bytes},
Request, Response,
};
use idna::domain_to_unicode;
use serde::{Deserialize, Serialize};
use garage_model::bucket_table::BucketParams;
@@ -97,7 +96,7 @@ pub fn authority_to_host(authority: &str) -> Result<String, Error> {
authority
))),
};
authority.map(|h| domain_to_unicode(h).0)
authority.map(|h| h.to_ascii_lowercase())
}
/// Extract the bucket name and the key name from an HTTP path and possibly a bucket provided in
+3 -3
View File
@@ -167,7 +167,7 @@ pub async fn handle_create_bucket(
}
// Create the bucket!
if !is_valid_bucket_name(&bucket_name) {
if !is_valid_bucket_name(&bucket_name, garage.config.allow_punycode) {
return Err(Error::bad_request(format!(
"{}: {}",
bucket_name, INVALID_BUCKET_NAME_MESSAGE
@@ -236,11 +236,11 @@ pub async fn handle_delete_bucket(ctx: ReqCtx) -> Result<Response<ResBody>, Erro
// 1. delete bucket alias
if is_local_alias {
helper
.unset_local_bucket_alias(*bucket_id, &api_key.key_id, bucket_name)
.purge_local_bucket_alias(*bucket_id, &api_key.key_id, bucket_name)
.await?;
} else {
helper
.unset_global_bucket_alias(*bucket_id, bucket_name)
.purge_global_bucket_alias(*bucket_id, bucket_name)
.await?;
}
+13 -1
View File
@@ -29,6 +29,7 @@ use crate::error::*;
use crate::get::{full_object_byte_stream, PreconditionHeaders};
use crate::multipart;
use crate::put::{extract_metadata_headers, save_stream, ChecksumMode, SaveStreamResult};
use crate::website::X_AMZ_WEBSITE_REDIRECT_LOCATION;
use crate::xml::{self as s3_xml, xmlns_tag};
pub const X_AMZ_COPY_SOURCE_IF_MATCH: HeaderName =
@@ -109,7 +110,18 @@ pub async fn handle_copy(
Some(v) if v == hyper::header::HeaderValue::from_static("REPLACE") => {
extract_metadata_headers(req.headers())?
}
_ => source_object_meta_inner.into_owned().headers,
_ => {
// The x-amz-website-redirect-location header is not copied, instead
// it is replaced by the value from the request (or removed if no
// value was specified)
let is_redirect =
|(key, _): &(String, String)| key == X_AMZ_WEBSITE_REDIRECT_LOCATION.as_str();
let mut headers: Vec<_> = source_object_meta_inner.headers.clone();
headers.retain(|h| !is_redirect(h));
let new_headers = extract_metadata_headers(req.headers())?;
headers.extend(new_headers.into_iter().filter(is_redirect));
headers
}
},
checksum: source_checksum,
checksum_type: source_checksum_type,
+1 -1
View File
@@ -27,7 +27,7 @@ pub async fn handle_get_lifecycle(ctx: ReqCtx) -> Result<Response<ResBody>, Erro
.body(string_body(xml))?)
} else {
Ok(Response::builder()
.status(StatusCode::NO_CONTENT)
.status(StatusCode::NOT_FOUND)
.body(empty_body())?)
}
}
+1 -1
View File
@@ -109,7 +109,7 @@ impl IDb for LmdbDb {
let mut path = to.clone();
path.push("data.mdb");
self.db
.copy_to_path(path, heed::CompactionOption::Disabled)?;
.copy_to_path(path, heed::CompactionOption::Enabled)?;
Ok(())
}
+1
View File
@@ -91,6 +91,7 @@ impl Cli {
ScrubCmd::Pause => ScrubCommand::Pause,
ScrubCmd::Resume => ScrubCommand::Resume,
}),
RepairWhat::Aliases => RepairType::Aliases,
};
let res = self
+3
View File
@@ -657,6 +657,9 @@ pub enum RepairWhat {
/// Recalculate block reference counters
#[structopt(name = "block-rc", version = garage_version())]
BlockRc,
/// Fix inconsistency in bucket aliases (WARNING: EXPERIMENTAL)
#[structopt(name = "aliases", version = garage_version())]
Aliases,
/// Verify integrity of all blocks on disc
#[structopt(name = "scrub", version = garage_version())]
Scrub {
+2
View File
@@ -65,6 +65,8 @@ rpc_bind_addr = "127.0.0.1:{rpc_port}"
rpc_public_addr = "127.0.0.1:{rpc_port}"
rpc_secret = "{secret}"
allow_punycode = true
[s3_api]
s3_region = "{region}"
api_bind_addr = "127.0.0.1:{s3_port}"
+73
View File
@@ -984,3 +984,76 @@ async fn test_website_invalid_redirect() {
.await
.unwrap_err();
}
#[tokio::test]
async fn test_website_puny() {
const BCKT_NAME: &str = "xn--pda.eu";
let ctx = common::context();
let bucket = ctx.create_bucket(BCKT_NAME);
let data = ByteStream::from_static(BODY);
ctx.client
.put_object()
.bucket(&bucket)
.key("index.html")
.body(data)
.send()
.await
.unwrap();
let client = Client::builder(TokioExecutor::new()).build_http();
let req = |suffix| {
Request::builder()
.method("GET")
.uri(format!("http://127.0.0.1:{}/", ctx.garage.web_port))
.header("Host", format!("{}{}", BCKT_NAME, suffix))
.body(Body::new(Bytes::new()))
.unwrap()
};
ctx.garage
.command()
.args(["bucket", "website", "--allow", BCKT_NAME])
.quiet()
.expect_success_status("Could not allow website on bucket");
let mut resp = client.request(req("")).await.unwrap();
assert_eq!(resp.status(), StatusCode::OK);
assert_eq!(
resp.into_body().collect().await.unwrap().to_bytes(),
BODY.as_ref()
);
resp = client.request(req(".web.garage")).await.unwrap();
assert_eq!(resp.status(), StatusCode::OK);
assert_eq!(
resp.into_body().collect().await.unwrap().to_bytes(),
BODY.as_ref()
);
for bname in [
BCKT_NAME.to_string(),
format!("{BCKT_NAME}.web.garage"),
format!("{BCKT_NAME}.s3.garage"),
] {
let admin_req = || {
Request::builder()
.method("GET")
.uri(format!(
"http://127.0.0.1:{0}/check?domain={1}",
ctx.garage.admin_port, bname
))
.body(Body::new(Bytes::new()))
.unwrap()
};
let admin_resp = client.request(admin_req()).await.unwrap();
assert_eq!(admin_resp.status(), StatusCode::OK);
assert_eq!(
admin_resp.into_body().collect().await.unwrap().to_bytes(),
format!("Domain '{bname}' is managed by Garage").as_bytes()
);
}
}
+8 -10
View File
@@ -22,14 +22,10 @@ mod v08 {
pub use v08::*;
impl BucketAlias {
pub fn new(name: String, ts: u64, bucket_id: Option<Uuid>) -> Option<Self> {
if !is_valid_bucket_name(&name) {
None
} else {
Some(BucketAlias {
name,
state: crdt::Lww::raw(ts, bucket_id),
})
pub fn new(name: String, ts: u64, bucket_id: Option<Uuid>) -> Self {
BucketAlias {
name,
state: crdt::Lww::raw(ts, bucket_id),
}
}
@@ -80,7 +76,7 @@ impl TableSchema for BucketAliasTable {
/// In the case of Garage, bucket names must not be hex-encoded
/// 32 byte string, which is excluded thanks to the
/// maximum length of 63 bytes given in the spec.
pub fn is_valid_bucket_name(n: &str) -> bool {
pub fn is_valid_bucket_name(n: &str, puny: bool) -> bool {
// Bucket names must be between 3 and 63 characters
n.len() >= 3 && n.len() <= 63
// Bucket names must be composed of lowercase letters, numbers,
@@ -92,7 +88,9 @@ pub fn is_valid_bucket_name(n: &str) -> bool {
// Bucket names must not be formatted as an IP address
&& n.parse::<std::net::IpAddr>().is_err()
// Bucket names must not start with "xn--"
&& !n.starts_with("xn--")
&& (!n.starts_with("xn--") || puny)
// We are a bit stricter, to properly restrict punycode in all labels
&& (!n.contains(".xn--") || puny)
// Bucket names must not end with "-s3alias"
&& !n.ends_with("-s3alias")
}
+277 -41
View File
@@ -1,3 +1,7 @@
use std::collections::{HashMap, HashSet};
use garage_db as db;
use garage_util::crdt::*;
use garage_util::data::*;
use garage_util::error::{Error as GarageError, OkOrMessage};
@@ -47,6 +51,10 @@ impl<'a> LockedHelper<'a> {
KeyHelper(self.0)
}
// ================================================
// global bucket aliases
// ================================================
/// Sets a new alias for a bucket in global namespace.
/// This function fails if:
/// - alias name is not valid according to S3 spec
@@ -57,7 +65,7 @@ impl<'a> LockedHelper<'a> {
bucket_id: Uuid,
alias_name: &String,
) -> Result<(), Error> {
if !is_valid_bucket_name(alias_name) {
if !is_valid_bucket_name(alias_name, self.0.config.allow_punycode) {
return Err(Error::InvalidBucketName(alias_name.to_string()));
}
@@ -88,8 +96,7 @@ impl<'a> LockedHelper<'a> {
// writes are now done and all writes use timestamp alias_ts
let alias = match alias {
None => BucketAlias::new(alias_name.clone(), alias_ts, Some(bucket_id))
.ok_or_else(|| Error::InvalidBucketName(alias_name.clone()))?,
None => BucketAlias::new(alias_name.clone(), alias_ts, Some(bucket_id)),
Some(mut a) => {
a.state = Lww::raw(alias_ts, Some(bucket_id));
a
@@ -180,13 +187,14 @@ impl<'a> LockedHelper<'a> {
.ok_or_else(|| Error::NoSuchBucket(alias_name.to_string()))?;
// 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()),
};
let alias_ts = increment_logical_clock_2(
alias.state.timestamp(),
bucket
.state
.as_option()
.map(|p| p.aliases.get_timestamp(alias_name))
.unwrap_or(0),
);
// ---- timestamp-ensured causality barrier ----
// writes are now done and all writes use timestamp alias_ts
@@ -204,6 +212,10 @@ impl<'a> LockedHelper<'a> {
Ok(())
}
// ================================================
// local bucket aliases
// ================================================
/// 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
@@ -216,14 +228,12 @@ impl<'a> LockedHelper<'a> {
key_id: &String,
alias_name: &String,
) -> Result<(), Error> {
let key_helper = KeyHelper(self.0);
if !is_valid_bucket_name(alias_name) {
if !is_valid_bucket_name(alias_name, self.0.config.allow_punycode) {
return Err(Error::InvalidBucketName(alias_name.to_string()));
}
let mut bucket = self.bucket().get_existing_bucket(bucket_id).await?;
let mut key = key_helper.get_existing_key(key_id).await?;
let mut key = self.key().get_existing_key(key_id).await?;
let key_param = key.state.as_option_mut().unwrap();
@@ -272,23 +282,13 @@ impl<'a> LockedHelper<'a> {
key_id: &String,
alias_name: &String,
) -> Result<(), Error> {
let key_helper = KeyHelper(self.0);
let mut bucket = self.bucket().get_existing_bucket(bucket_id).await?;
let mut key = key_helper.get_existing_key(key_id).await?;
let mut key = self.key().get_existing_key(key_id).await?;
let key_p = key.state.as_option().unwrap();
let bucket_p = bucket.state.as_option_mut().unwrap();
if key
.state
.as_option()
.unwrap()
.local_aliases
.get(alias_name)
.cloned()
.flatten()
!= Some(bucket_id)
{
if key_p.local_aliases.get(alias_name).cloned().flatten() != Some(bucket_id) {
return Err(GarageError::Message(format!(
"Bucket {:?} does not have alias {} in namespace of key {}",
bucket_id, alias_name, key_id
@@ -305,17 +305,17 @@ impl<'a> LockedHelper<'a> {
.local_aliases
.items()
.iter()
.any(|((k, n), _, active)| *k == key.key_id && n == alias_name && *active);
.any(|((k, n), _, active)| (*k != key.key_id || n != alias_name) && *active);
if !has_other_global_aliases && !has_other_local_aliases {
return Err(Error::BadRequest(format!("Bucket {} doesn't have other aliases, please delete it instead of just unaliasing.", alias_name)));
}
// Checks ok, remove alias
let key_param = key.state.as_option_mut().unwrap();
let bucket_p_local_alias_key = (key.key_id.clone(), alias_name.clone());
let alias_ts = increment_logical_clock_2(
key_param.local_aliases.get_timestamp(alias_name),
key_p.local_aliases.get_timestamp(alias_name),
bucket_p
.local_aliases
.get_timestamp(&bucket_p_local_alias_key),
@@ -324,7 +324,8 @@ impl<'a> LockedHelper<'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, None);
key.state.as_option_mut().unwrap().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);
@@ -333,21 +334,68 @@ impl<'a> LockedHelper<'a> {
Ok(())
}
/// Ensures a bucket does not have a certain local alias.
/// Contrarily to unset_local_bucket_alias, this does not
/// fail on any condition other than:
/// - bucket cannot be found (its fine if it is in deleted state)
/// - key cannot be found (its fine if alias in key points to nothing
/// or to another bucket)
pub async fn purge_local_bucket_alias(
&self,
bucket_id: Uuid,
key_id: &String,
alias_name: &String,
) -> Result<(), Error> {
let mut bucket = self.bucket().get_internal_bucket(bucket_id).await?;
let mut key = self.key().get_internal_key(key_id).await?;
let bucket_p_local_alias_key = (key.key_id.clone(), alias_name.clone());
let alias_ts = increment_logical_clock_2(
key.state
.as_option()
.map(|p| p.local_aliases.get_timestamp(alias_name))
.unwrap_or(0),
bucket
.state
.as_option()
.map(|p| p.local_aliases.get_timestamp(&bucket_p_local_alias_key))
.unwrap_or(0),
);
// ---- timestamp-ensured causality barrier ----
// writes are now done and all writes use timestamp alias_ts
if let Some(kp) = key.state.as_option_mut() {
kp.local_aliases = LwwMap::raw_item(alias_name.clone(), alias_ts, None);
self.0.key_table.insert(&key).await?;
}
if let Some(bp) = bucket.state.as_option_mut() {
bp.local_aliases = LwwMap::raw_item(bucket_p_local_alias_key, alias_ts, false);
self.0.bucket_table.insert(&bucket).await?;
}
Ok(())
}
// ================================================
// permissions
// ================================================
/// Sets permissions for a key on a bucket.
/// This function fails if:
/// - bucket or key cannot be found at all (its ok if they are in deleted state)
/// - bucket or key is in deleted state and we are trying to set permissions other than "deny
/// all"
/// - bucket or key is in deleted state and we are trying to set
/// permissions other than "deny all"
pub async fn set_bucket_key_permissions(
&self,
bucket_id: Uuid,
key_id: &String,
mut perm: BucketKeyPerm,
) -> Result<(), Error> {
let key_helper = KeyHelper(self.0);
let mut bucket = self.bucket().get_internal_bucket(bucket_id).await?;
let mut key = key_helper.get_internal_key(key_id).await?;
let mut key = self.key().get_internal_key(key_id).await?;
if let Some(bstate) = bucket.state.as_option() {
if let Some(kp) = bstate.authorized_keys.get(key_id) {
@@ -384,21 +432,20 @@ impl<'a> LockedHelper<'a> {
Ok(())
}
// ----
// ================================================
// keys
// ================================================
/// Deletes an API access key
pub async fn delete_key(&self, key: &mut Key) -> Result<(), Error> {
let state = key.state.as_option_mut().unwrap();
// --- done checking, now commit ---
// (the step at unset_local_bucket_alias will fail if a bucket
// does not have another alias, the deletion will be
// interrupted in the middle if that happens)
// 1. Delete local aliases
for (alias, _, to) in state.local_aliases.items().iter() {
if let Some(bucket_id) = to {
self.unset_local_bucket_alias(*bucket_id, &key.key_id, alias)
self.purge_local_bucket_alias(*bucket_id, &key.key_id, alias)
.await?;
}
}
@@ -415,4 +462,193 @@ impl<'a> LockedHelper<'a> {
Ok(())
}
// ================================================
// repair procedure
// ================================================
pub async fn repair_aliases(&self) -> Result<(), GarageError> {
self.0.db.transaction(|tx| {
info!("--- begin repair_aliases transaction ----");
// 1. List all non-deleted buckets, so that we can fix bad aliases
let mut all_buckets: HashSet<Uuid> = HashSet::new();
for item in tx.range::<&[u8], _>(&self.0.bucket_table.data.store, ..)? {
let bucket = self
.0
.bucket_table
.data
.decode_entry(&(item?.1))
.map_err(db::TxError::Abort)?;
if !bucket.is_deleted() {
all_buckets.insert(bucket.id);
}
}
info!("number of buckets: {}", all_buckets.len());
// 2. List all aliases declared in bucket_alias_table and key_table
// Take note of aliases that point to non-existing buckets
let mut global_aliases: HashMap<String, Uuid> = HashMap::new();
{
let mut delete_global = vec![];
for item in tx.range::<&[u8], _>(&self.0.bucket_alias_table.data.store, ..)? {
let mut alias = self
.0
.bucket_alias_table
.data
.decode_entry(&(item?.1))
.map_err(db::TxError::Abort)?;
if let Some(id) = alias.state.get() {
if all_buckets.contains(id) {
// keep aliases
global_aliases.insert(alias.name().to_string(), *id);
} else {
// delete alias
warn!(
"global alias: remove {} -> {:?} (bucket is deleted)",
alias.name(),
id
);
alias.state.update(None);
delete_global.push(alias);
}
}
}
info!("number of global aliases: {}", global_aliases.len());
info!("global alias table: {} entries fixed", delete_global.len());
for ga in delete_global {
debug!("Enqueue update to global alias table: {:?}", ga);
self.0.bucket_alias_table.queue_insert(tx, &ga)?;
}
}
let mut local_aliases: HashMap<(String, String), Uuid> = HashMap::new();
{
let mut delete_local = vec![];
for item in tx.range::<&[u8], _>(&self.0.key_table.data.store, ..)? {
let mut key = self
.0
.key_table
.data
.decode_entry(&(item?.1))
.map_err(db::TxError::Abort)?;
let Some(p) = key.state.as_option_mut() else {
continue;
};
let mut has_changes = false;
for (name, _, to) in p.local_aliases.items().to_vec() {
if let Some(id) = to {
if all_buckets.contains(&id) {
local_aliases.insert((key.key_id.clone(), name), id);
} else {
warn!(
"local alias: remove ({}, {}) -> {:?} (bucket is deleted)",
key.key_id, name, id
);
p.local_aliases.update_in_place(name, None);
has_changes = true;
}
}
}
if has_changes {
delete_local.push(key);
}
}
info!("number of local aliases: {}", local_aliases.len());
info!("key table: {} entries fixed", delete_local.len());
for la in delete_local {
debug!("Enqueue update to key table: {:?}", la);
self.0.key_table.queue_insert(tx, &la)?;
}
}
// 4. Reverse the alias maps to determine the aliases per-bucket
let mut bucket_global: HashMap<Uuid, Vec<String>> = HashMap::new();
let mut bucket_local: HashMap<Uuid, Vec<(String, String)>> = HashMap::new();
for (name, bucket) in global_aliases {
bucket_global.entry(bucket).or_default().push(name);
}
for ((key, name), bucket) in local_aliases {
bucket_local.entry(bucket).or_default().push((key, name));
}
// 5. Fix the bucket table to ensure consistency
let mut bucket_updates = vec![];
for item in tx.range::<&[u8], _>(&self.0.bucket_table.data.store, ..)? {
let bucket = self
.0
.bucket_table
.data
.decode_entry(&(item?.1))
.map_err(db::TxError::Abort)?;
let mut bucket2 = bucket.clone();
let Some(param) = bucket2.state.as_option_mut() else {
continue;
};
// fix global aliases
{
let ga = bucket_global.remove(&bucket.id).unwrap_or_default();
for (name, _, active) in param.aliases.items().to_vec() {
if active && !ga.contains(&name) {
warn!("bucket {:?}: remove global alias {}", bucket.id, name);
param.aliases.update_in_place(name, false);
}
}
for name in ga {
if param.aliases.get(&name).copied() != Some(true) {
warn!("bucket {:?}: add global alias {}", bucket.id, name);
param.aliases.update_in_place(name, true);
}
}
}
// fix local aliases
{
let la = bucket_local.remove(&bucket.id).unwrap_or_default();
for (pair, _, active) in param.local_aliases.items().to_vec() {
if active && !la.contains(&pair) {
warn!("bucket {:?}: remove local alias {:?}", bucket.id, pair);
param.local_aliases.update_in_place(pair, false);
}
}
for pair in la {
if param.local_aliases.get(&pair).copied() != Some(true) {
warn!("bucket {:?}: add local alias {:?}", bucket.id, pair);
param.local_aliases.update_in_place(pair, true);
}
}
}
if bucket2 != bucket {
bucket_updates.push(bucket2);
}
}
info!("bucket table: {} entries fixed", bucket_updates.len());
for b in bucket_updates {
debug!("Enqueue update to bucket table: {:?}", b);
self.0.bucket_table.queue_insert(tx, &b)?;
}
info!("--- end repair_aliases transaction ----");
Ok(())
})?;
info!("repair_aliases is done");
Ok(())
}
}
+4
View File
@@ -135,6 +135,10 @@ pub struct Config {
/// Configuration for the admin API endpoint
#[serde(default = "Default::default")]
pub admin: AdminConfig,
/// Allow punycode in bucket names
#[serde(default)]
pub allow_punycode: bool,
}
/// Value for data_dir: either a single directory or a list of dirs with attributes