Merge branch 'main' into codex/fix-5166-metacache-timeout

This commit is contained in:
Zhengchao An
2026-08-01 10:44:38 +08:00
committed by GitHub
28 changed files with 6745 additions and 938 deletions
+64 -19
View File
@@ -105,33 +105,78 @@ CI) fails the build if anything is committed under `docs/superpowers/`, even via
## Verification Before PR
Convert changes into independently verifiable outcomes. Prefer focused tests for behavior changes and run the relevant checks before declaring completion.
Non-exempt changes must also pass Adversarial Validation (next section) before the checks below count as completion.
Convert changes into independently verifiable outcomes. This section controls
agent-run local validation; preparing a commit or PR does not by itself require
the broadest gate. Inspect only the final task-owned diff, classify it by
behavioral impact rather than line count or path alone, and run the smallest
set of checks that provides meaningful coverage. Do not let unrelated
worktree changes or a generic contributor checklist expand the scope.
Non-exempt changes must also pass Adversarial Validation (next section) before
the checks below count as completion.
For code changes, run and pass the following before opening a PR:
### Validation floor
```bash
make pre-pr
```
- Every change that is not documentation-only must finish with
`cargo fmt --all --check` passing. An umbrella gate that runs this exact
check satisfies the requirement; do not run it twice. Use `cargo fmt --all`
only when formatting needs to be fixed. Run the configured formatter or
validator for other changed languages when one exists.
- Documentation-only or instruction-only means all task-owned changes are
prose or documentation assets and cannot affect runtime, builds, CI,
dependencies, generated code, or tests. Run `git diff --check` and any
relevant documentation guard, but skip Cargo formatting, compilation,
Clippy, tests, `make pre-commit`, and `make pre-pr`.
- Behavior changes require relevant existing or new tests. Prefer the most
focused test or affected package. A passing targeted test can also provide
sufficient compilation coverage when it builds every changed target and
feature involved; do not add a redundant `cargo check` in that case.
- `cargo check` supplements compilation coverage; it never substitutes for a
behavioral test. If a relevant test cannot reasonably be added or run, use
the narrowest compilation check and report the reason and remaining risk.
Before committing code changes, prefer focused verification for the touched
surface and use the faster local gate when a broad smoke check is needed:
### Validation tiers
```bash
make pre-commit
```
1. **Documentation/instruction-only:** Apply the exemption above. Run a guard
such as `make doc-paths-check` only when it is relevant to the edited text.
2. **Non-behavioral source change:** For comments, formatting, or another
demonstrably non-executable change, run the formatting floor. Compilation,
Clippy, and tests may be skipped only when the edit cannot affect
compilation or runtime behavior; run targeted doctests if executable
documentation examples changed.
3. **Localized or bounded behavior change:** Run the formatting floor and the
narrowest relevant tests. Add package-scoped `cargo check` or Clippy only
for changed targets, features, APIs, error handling, async behavior, or
control flow not already covered. When several crates are affected but the
dependency set is identifiable, validate those packages and known
dependents instead of the whole workspace. Use `make pre-commit` only when
a repository-wide fast gate adds useful confidence beyond those checks.
4. **Broad or high-risk change:** Run `make pre-pr` only when targeted coverage
cannot bound the impact, including:
- dependency, feature, build-script, procedural-macro, code-generation,
toolchain, or CI changes that alter compilation or the test matrix;
- cross-crate public APIs, shared foundational code, or broad refactors with
an unbounded dependent set;
- locking, storage durability or formats, erasure coding, replication,
RPC/protocol compatibility, IAM/KMS/auth, cryptography, or other
security-sensitive behavior;
- a targeted check that reveals wider impact, an explicit user request, or
a release policy that requires the full gate.
For migration batches, do not run the full `make pre-pr` gate before every
intermediate commit. Use focused tests and `make pre-commit` during
development, then reserve `make pre-pr` for the final PR-ready branch.
Documentation-only and non-behavioral classifications take precedence over
path-based triggers. A small diff can still be high-risk, while a CI comment,
manifest comment, or release-note edit does not require full validation.
Before pushing code changes, make sure formatting is clean:
`make pre-pr` includes `make pre-commit` coverage. Never run both for the same
unchanged diff, and do not repeat equivalent checks during PR preparation or
because a local hook already ran them. Rerun only checks whose scope is affected
by later edits. Full workspace checks do not replace a relevant integration or
E2E test for changed behavior; run that focused test when required and
available, or report why it was not run and the remaining risk.
- Run `cargo fmt --all`.
- Run `cargo fmt --all --check` and ensure no files are modified unexpectedly.
If `make` is unavailable, run the equivalent checks defined under
`.config/make/`. At handoff, list the checks actually run, checks intentionally
skipped, and the reason for the selected tier.
If `make` is unavailable, run the equivalent checks defined under `.config/make/`.
Documentation-only or instruction-only changes are exempt from the verification commands above (including the `.config/make/` equivalents), though any locally installed git pre-commit hooks may still run on commit unless explicitly skipped.
After build-based verification completes, clean generated build artifacts before wrapping up to avoid unnecessary disk usage.
Do not open a PR with code changes when the required checks fail.
Make a failing check pass by fixing the cause, never by weakening the gate:
@@ -56,6 +56,21 @@ mod tests {
);
}
async fn assert_current_list_hides_delete_marker(client: &Client, bucket: &str, key: &str) {
let listed = client
.list_objects_v2()
.bucket(bucket)
.prefix(key)
.send()
.await
.expect("list current objects after delete marker");
assert!(
listed.contents().iter().all(|object| object.key() != Some(key)),
"ListObjectsV2 must hide an object whose latest version is a delete marker"
);
}
#[tokio::test]
#[serial]
async fn test_versioning_only_delete_marker_has_minio_compatible_visibility_for_migration_proof() {
@@ -94,6 +109,7 @@ mod tests {
assert_eq!(markers[0].version_id(), Some(delete_marker_version_id));
assert_eq!(markers[0].is_latest(), Some(true));
assert_current_get_is_delete_marker_not_found(&client, bucket, key).await;
assert_current_list_hides_delete_marker(&client, bucket, key).await;
}
#[tokio::test]
@@ -118,6 +134,17 @@ mod tests {
.await
.expect("put historical version");
let data_version_id = put.version_id().expect("put should return data version id");
let listed_before_delete = client
.list_objects_v2()
.bucket(bucket)
.prefix(key)
.send()
.await
.expect("list current object before creating delete marker");
assert!(
listed_before_delete.contents().iter().any(|object| object.key() == Some(key)),
"ListObjectsV2 must include the current object before it is deleted"
);
let delete_marker = client
.delete_object()
@@ -145,6 +172,7 @@ mod tests {
assert_eq!(markers[0].version_id(), Some(delete_marker_version_id));
assert_eq!(markers[0].is_latest(), Some(true));
assert_current_get_is_delete_marker_not_found(&client, bucket, key).await;
assert_current_list_hides_delete_marker(&client, bucket, key).await;
let historical = client
.get_object()
@@ -26,6 +26,7 @@
use super::common::*;
use aws_sdk_s3::Client;
use aws_sdk_s3::error::ProvideErrorMetadata;
use aws_sdk_s3::primitives::{ByteStream, DateTimeFormat};
use aws_sdk_s3::types::{
CompletedMultipartUpload, CompletedPart, Delete, MetadataDirective, ObjectIdentifier, ObjectLockLegalHoldStatus,
@@ -2120,6 +2121,127 @@ async fn test_multipart_default_retention_fixed_at_create() {
// Versioning Auto-Enable Tests
// ============================================================================
#[tokio::test]
#[serial]
async fn test_unretained_object_lock_object_delete_and_bucket_cleanup() {
init_logging();
info!("🧪 Test: Unretained Object Lock object delete and bucket cleanup (Issue #5339)");
let mut env = ObjectLockTestEnvironment::new()
.await
.expect("failed to create Object Lock test environment");
env.start_rustfs().await.expect("failed to start RustFS");
let bucket = "test-object-lock-delete-cleanup";
let key = "unretained-object";
env.create_object_lock_bucket(bucket)
.await
.expect("failed to create Object Lock bucket");
let client = env.s3_client();
let put_response = client
.put_object()
.bucket(bucket)
.key(key)
.body(ByteStream::from_static(b"unretained data"))
.send()
.await
.expect("failed to upload unretained object");
let object_version_id = put_response
.version_id()
.expect("Object Lock buckets must create versioned objects")
.to_string();
let delete_response = client
.delete_object()
.bucket(bucket)
.key(key)
.send()
.await
.expect("failed to create delete marker");
assert_eq!(delete_response.delete_marker(), Some(true));
let delete_marker_version_id = delete_response
.version_id()
.expect("Deleting without a version ID must create a delete marker")
.to_string();
let get_error = client
.get_object()
.bucket(bucket)
.key(key)
.send()
.await
.expect_err("GET must not return an object hidden by a delete marker");
assert_eq!(get_error.raw_response().map(|response| response.status().as_u16()), Some(404));
assert_eq!(get_error.as_service_error().and_then(|error| error.code()), Some("NoSuchKey"));
let listed_objects = client
.list_objects_v2()
.bucket(bucket)
.send()
.await
.expect("failed to list current objects");
assert!(
listed_objects.contents().iter().all(|object| object.key() != Some(key)),
"ListObjectsV2 must hide objects whose latest version is a delete marker"
);
let listed_versions = client
.list_object_versions()
.bucket(bucket)
.send()
.await
.expect("failed to list object versions");
assert!(
listed_versions
.versions()
.iter()
.any(|version| version.key() == Some(key) && version.version_id() == Some(object_version_id.as_str())),
"The data version must remain until it is explicitly deleted"
);
assert!(
listed_versions
.delete_markers()
.iter()
.any(|marker| marker.key() == Some(key) && marker.version_id() == Some(delete_marker_version_id.as_str())),
"ListObjectVersions must expose the delete marker"
);
client
.delete_object()
.bucket(bucket)
.key(key)
.version_id(object_version_id)
.send()
.await
.expect("failed to delete the data version");
client
.delete_object()
.bucket(bucket)
.key(key)
.version_id(delete_marker_version_id)
.send()
.await
.expect("failed to delete the delete marker");
let remaining_versions = client
.list_object_versions()
.bucket(bucket)
.send()
.await
.expect("failed to list versions after cleanup");
assert!(remaining_versions.versions().is_empty());
assert!(remaining_versions.delete_markers().is_empty());
client
.delete_bucket()
.bucket(bucket)
.send()
.await
.expect("Deleting every version must remove xl.meta so the bucket can be deleted normally");
}
#[tokio::test]
#[serial]
async fn test_versioning_auto_enabled_with_object_lock() {
+7 -6
View File
@@ -267,12 +267,13 @@ pub mod config {
pub mod com {
pub use crate::config::com::{
COMMA_SEPARATED_LISTS, CONFIG_PREFIX, ENV_CONFIG_RECOVER_ON_CORRUPTION, STORAGE_CLASS_SUB_SYS,
ServerConfigCorruptError, ServerConfigSnapshot, delete_config, is_server_config_corrupt_error, lookup_configs,
read_config, read_config_no_lock, read_config_with_metadata, read_config_without_migrate,
read_config_without_migrate_no_lock, read_existing_server_config_no_lock, read_server_config_snapshot, save_config,
save_config_no_lock, save_config_with_opts, save_server_config, save_server_config_no_lock,
save_server_config_snapshot, server_config_path, try_migrate_server_config, with_config_object_read_lock,
with_config_object_write_lock, with_server_config_read_lock, with_server_config_write_lock,
ServerConfigCorruptError, ServerConfigSaveResult, ServerConfigSnapshot, delete_config,
is_server_config_corrupt_error, lookup_configs, read_config, read_config_no_lock, read_config_with_metadata,
read_config_without_migrate, read_config_without_migrate_no_lock, read_existing_server_config_no_lock,
read_server_config_snapshot, save_config, save_config_no_lock, save_config_with_opts, save_server_config,
save_server_config_no_lock, save_server_config_snapshot, save_server_config_snapshot_with_generation,
server_config_path, try_migrate_server_config, with_config_object_read_lock, with_config_object_write_lock,
with_server_config_read_lock, with_server_config_write_lock,
};
}
+642 -53
View File
@@ -53,12 +53,14 @@ use std::sync::LazyLock;
use std::sync::{Arc, RwLock};
use tokio::sync::{OwnedRwLockWriteGuard, RwLock as AsyncRwLock};
use tracing::{debug, error, info, instrument, warn};
use uuid::Uuid;
pub const CONFIG_PREFIX: &str = "config";
const SERVER_CONFIG_OBJECT: &str = "config/config.json";
const CONFIG_TRANSACTION_LOCK_SUFFIX: &str = ".transaction.lock";
// Server-config lock order: SERVER_CONFIG_LOCK -> distributed namespace lock
// for SERVER_CONFIG_OBJECT. Readers and writers must never reverse this order.
// Server-config lock order: SERVER_CONFIG_LOCK -> transaction lock ->
// SERVER_CONFIG_OBJECT. Readers and writers must never reverse this order.
static SERVER_CONFIG_LOCK: LazyLock<Arc<AsyncRwLock<()>>> = LazyLock::new(|| Arc::new(AsyncRwLock::new(())));
fn config_task_join_error(operation: &'static str, error: tokio::task::JoinError) -> Error {
@@ -76,8 +78,11 @@ where
T: Send + 'static,
{
tokio::spawn(async move {
// Lock order: SERVER_CONFIG_LOCK -> namespace write lock.
// Lock order: SERVER_CONFIG_LOCK -> transaction lock -> object lock.
let _local_guard = SERVER_CONFIG_LOCK.write().await;
let transaction_lock = server_config_transaction_lock_path();
let transaction_lock = store.new_ns_lock(RUSTFS_META_BUCKET, &transaction_lock).await?;
let _transaction_guard = transaction_lock.get_write_lock(get_lock_acquire_timeout()).await?;
let namespace_lock = store.new_ns_lock(RUSTFS_META_BUCKET, SERVER_CONFIG_OBJECT).await?;
let _write_guard = namespace_lock.get_write_lock(get_lock_acquire_timeout()).await?;
Ok(operation().await)
@@ -96,8 +101,11 @@ where
T: Send + 'static,
{
tokio::spawn(async move {
// Lock order: SERVER_CONFIG_LOCK -> namespace read lock.
// Lock order: SERVER_CONFIG_LOCK -> transaction lock -> object lock.
let _local_guard = SERVER_CONFIG_LOCK.read().await;
let transaction_lock = server_config_transaction_lock_path();
let transaction_lock = store.new_ns_lock(RUSTFS_META_BUCKET, &transaction_lock).await?;
let _transaction_guard = transaction_lock.get_read_lock(get_lock_acquire_timeout()).await?;
let namespace_lock = store.new_ns_lock(RUSTFS_META_BUCKET, SERVER_CONFIG_OBJECT).await?;
let _read_guard = namespace_lock.get_read_lock(get_lock_acquire_timeout()).await?;
Ok(operation().await)
@@ -567,6 +575,21 @@ where
}
pub async fn save_config_with_opts<S>(api: Arc<S>, file: &str, data: Vec<u8>, opts: &ObjectOptions) -> Result<()>
where
S: ObjectIO<
Error = Error,
RangeSpec = HTTPRangeSpec,
HeaderMap = HeaderMap,
ObjectOptions = ObjectOptions,
ObjectInfo = ObjectInfo,
GetObjectReader = GetObjectReader,
PutObjectReader = PutObjReader,
>,
{
save_config_with_opts_and_metadata(api, file, data, opts).await.map(|_| ())
}
async fn save_config_with_opts_and_metadata<S>(api: Arc<S>, file: &str, data: Vec<u8>, opts: &ObjectOptions) -> Result<ObjectInfo>
where
S: ObjectIO<
Error = Error,
@@ -579,11 +602,13 @@ where
>,
{
let mut put_data = PutObjReader::from_vec(data);
if let Err(err) = api.put_object(RUSTFS_META_BUCKET, file, &mut put_data, opts).await {
error!("save_config_with_opts: err: {:?}, file: {}", err, file);
return Err(err);
match api.put_object(RUSTFS_META_BUCKET, file, &mut put_data, opts).await {
Ok(object_info) => Ok(object_info),
Err(err) => {
error!("save_config_with_opts: err: {:?}, file: {}", err, file);
Err(err)
}
}
Ok(())
}
fn new_server_config() -> Config {
@@ -594,8 +619,12 @@ async fn new_and_save_server_config<S>(api: Arc<S>) -> Result<Config>
where
S: EcstoreObjectIO + StorageAdminApi + NamespaceLocking<Error = Error, NamespaceLock = rustfs_lock::NamespaceLockWrapper>,
{
let snapshot = read_server_config_snapshot(api.clone()).await?;
if snapshot.object_exists() {
return Ok(snapshot.config.clone());
}
let cfg = new_server_config();
save_server_config(api, &cfg).await?;
save_server_config_snapshot(api, &cfg, &snapshot).await?;
Ok(cfg)
}
@@ -617,6 +646,10 @@ pub fn server_config_path() -> String {
SERVER_CONFIG_OBJECT.to_string()
}
fn server_config_transaction_lock_path() -> String {
format!("{}{CONFIG_TRANSACTION_LOCK_SUFFIX}", server_config_path())
}
fn storage_class_kvs_mut(cfg: &mut Config) -> &mut KVS {
let sub_cfg = cfg.0.entry(STORAGE_CLASS_SUB_SYS.to_string()).or_insert_with(|| {
let mut section = HashMap::new();
@@ -819,6 +852,9 @@ fn apply_external_scalar_config_map(
let Some(config_value) = root.get(descriptor.subsystem_key) else {
return Ok(false);
};
if descriptor.subsystem_key == HEAL_SUB_SYS && config_value.is_null() {
return Ok(false);
}
let overrides = decode_scalar_config_value(config_value, descriptor)?;
if overrides.is_empty() {
@@ -1463,21 +1499,142 @@ fn build_audit_object(cfg: &Config) -> Map<String, Value> {
build_target_object(cfg, &audit_target_descriptors())
}
fn sync_rendered_target_instance(existing: Value, rendered: Option<&Value>, valid_keys: &[&str]) -> Option<Value> {
match existing {
Value::Object(mut instance) => {
for key in valid_keys {
instance.remove(*key);
}
if let Some(Value::Object(rendered)) = rendered {
instance.extend(rendered.clone());
}
(!instance.is_empty()).then_some(Value::Object(instance))
}
Value::Array(entries) => {
let mut pending = rendered
.and_then(Value::as_object)
.map(|rendered| {
rendered
.iter()
.filter_map(|(key, value)| parse_target_scalar_value(key, value).map(|value| (key.clone(), value)))
.collect::<HashMap<_, _>>()
})
.unwrap_or_default();
let mut updated = Vec::with_capacity(entries.len().saturating_add(pending.len()));
for entry in entries {
let Some(entry_obj) = entry.as_object() else {
updated.push(entry);
continue;
};
let Some(key) = entry_obj.get("key").and_then(Value::as_str) else {
updated.push(entry);
continue;
};
if !valid_keys.contains(&key) {
updated.push(entry);
continue;
}
let Some(value) = pending.remove(key) else {
continue;
};
let mut entry_obj = entry_obj.clone();
entry_obj.insert("value".to_string(), Value::String(value));
updated.push(Value::Object(entry_obj));
}
updated.extend(rendered_scalar_config_kvs_entries(&pending));
(!updated.is_empty()).then_some(Value::Array(updated))
}
value if rendered.is_none() => Some(value),
_ => rendered.cloned(),
}
}
fn sync_rendered_target_object(
target_obj: &mut Map<String, Value>,
rendered_target: &Map<String, Value>,
descriptors: &[TargetConfigDescriptor],
) {
for descriptor in descriptors {
match rendered_target.get(descriptor.external_key) {
Some(Value::Object(v)) => {
target_obj.insert(descriptor.external_key.to_string(), Value::Object(v.clone()));
target_obj.remove(descriptor.subsystem_key);
let existing = target_obj.remove(descriptor.external_key);
let alias = target_obj.remove(descriptor.subsystem_key);
let mut section = existing
.or(alias)
.and_then(|value| value.as_object().cloned())
.unwrap_or_default();
let rendered = rendered_target.get(descriptor.external_key).and_then(Value::as_object);
if is_target_instance_shorthand(&section, descriptor.valid_keys) {
let has_named_instances = rendered.is_some_and(|instances| instances.keys().any(|name| name != "default"));
if !has_named_instances {
if let Some(section) = sync_rendered_target_instance(
Value::Object(section),
rendered.and_then(|instances| instances.get("default")),
descriptor.valid_keys,
) {
target_obj.insert(descriptor.external_key.to_string(), section);
}
continue;
}
_ => {
target_obj.remove(descriptor.external_key);
target_obj.remove(descriptor.subsystem_key);
let mut nested = Map::new();
if let Some(default) = sync_rendered_target_instance(
Value::Object(section),
rendered.and_then(|instances| instances.get("default")),
descriptor.valid_keys,
) {
nested.insert("default".to_string(), default);
}
if let Some(rendered) = rendered {
for (instance_name, instance) in rendered {
if instance_name != "default" {
nested.insert(instance_name.clone(), instance.clone());
}
}
}
if !nested.is_empty() {
target_obj.insert(descriptor.external_key.to_string(), Value::Object(nested));
}
continue;
}
if let Some(default_alias) = section.remove(DEFAULT_DELIMITER) {
if let Some(default) = section.get_mut("default") {
if let Some(alias) = sync_rendered_target_instance(default_alias, None, descriptor.valid_keys) {
match (default, alias) {
(Value::Object(default), Value::Object(alias)) => {
for (key, value) in alias {
default.entry(key).or_insert(value);
}
}
(Value::Array(default), Value::Array(alias)) => default.extend(alias),
_ => {}
}
}
} else {
section.insert("default".to_string(), default_alias);
}
}
let mut merged = Map::new();
for (instance_name, instance) in section {
if let Some(instance) = sync_rendered_target_instance(
instance,
rendered.and_then(|instances| instances.get(&instance_name)),
descriptor.valid_keys,
) {
merged.insert(instance_name, instance);
}
}
if let Some(rendered) = rendered {
for (instance_name, instance) in rendered {
if !merged.contains_key(instance_name) {
merged.insert(instance_name.clone(), instance.clone());
}
}
}
if !merged.is_empty() {
target_obj.insert(descriptor.external_key.to_string(), Value::Object(merged));
}
}
}
@@ -1496,6 +1653,14 @@ fn encode_server_config_blob(cfg: &Config, seed: Option<&[u8]>) -> Result<Vec<u8
Some(Value::Object(v)) => v,
_ => Map::new(),
};
for key in [
storageclass::CLASS_STANDARD,
storageclass::CLASS_RRS,
storageclass::OPTIMIZE,
storageclass::INLINE_BLOCK,
] {
sc_obj.remove(key);
}
for (k, v) in build_storageclass_object(cfg) {
sc_obj.insert(k, v);
}
@@ -1503,7 +1668,10 @@ fn encode_server_config_blob(cfg: &Config, seed: Option<&[u8]>) -> Result<Vec<u8
root.remove("storage_class");
for descriptor in [scanner_config_descriptor(), heal_config_descriptor()] {
let existing = root.remove(descriptor.subsystem_key);
let mut existing = root.remove(descriptor.subsystem_key);
if descriptor.subsystem_key == HEAL_SUB_SYS && existing.as_ref().is_some_and(Value::is_null) {
existing = None;
}
let rendered = build_scalar_config_object(cfg, descriptor);
if let Some(config_value) = sync_rendered_scalar_config_value(existing, &rendered, descriptor)? {
root.insert(descriptor.subsystem_key.to_string(), config_value);
@@ -1560,6 +1728,7 @@ fn is_standard_object_server_config(data: &[u8]) -> bool {
matches!(root.get("version"), Some(Value::String(v)) if !v.trim().is_empty())
&& matches!(root.get("storageclass"), Some(Value::Object(_)))
&& !root.contains_key("storage_class")
&& !matches!(root.get(HEAL_SUB_SYS), Some(Value::Null))
}
fn configs_semantically_equal(lhs: &Config, rhs: &Config) -> bool {
@@ -1593,7 +1762,7 @@ where
FileInfo = FileInfo,
ObjectToDelete = ObjectToDelete,
DeletedObject = DeletedObject,
>,
> + NamespaceLocking<Error = Error, NamespaceLock = rustfs_lock::NamespaceLockWrapper>,
{
if let Some(decrypt) = &decrypt_fn {
register_server_config_decrypt_fn(decrypt.clone());
@@ -1601,14 +1770,7 @@ where
let config_file = server_config_path();
match api
.get_object_info(
RUSTFS_META_BUCKET,
&config_file,
&ObjectOptions {
no_lock: true,
..Default::default()
},
)
.get_object_info(RUSTFS_META_BUCKET, &config_file, &ObjectOptions::default())
.await
{
Ok(_) => {
@@ -1624,7 +1786,6 @@ where
let opts = ObjectOptions {
max_parity: true,
no_lock: true,
..Default::default()
};
@@ -1677,7 +1838,33 @@ where
}
};
match save_config(api, &config_file, normalized).await {
let snapshot = match read_server_config_snapshot(api.clone()).await {
Ok(snapshot) => snapshot,
Err(err) => {
warn!("recheck target server config failed, skip migration: {:?}", err);
return;
}
};
if snapshot.object_exists() {
debug!("server config was created while legacy migration was preparing, skip migration");
return;
}
match save_config_with_opts(
api,
&config_file,
normalized,
&ObjectOptions {
max_parity: true,
http_preconditions: Some(HTTPPreconditions {
if_none_match: Some("*".to_string()),
..Default::default()
}),
..Default::default()
},
)
.await
{
Ok(()) => {
info!("Migrated compatible server config from legacy metadata bucket");
}
@@ -1769,8 +1956,13 @@ where
{
let config_file = server_config_path();
// Try to read the configuration file
match read_config_no_lock(api.clone(), &config_file).await {
// Try to read the configuration file.
let data = if namespace_lock_held {
read_config_no_lock(api.clone(), &config_file).await
} else {
read_config(api.clone(), &config_file).await
};
match data {
Ok(data) => read_server_config(api, &data, namespace_lock_held).await,
Err(Error::ConfigNotFound) => handle_missing_config(api, "Read the main configuration", namespace_lock_held).await,
Err(err) => handle_config_read_error(err, &config_file),
@@ -1787,7 +1979,12 @@ where
warn!("Received empty configuration data, try to reread from '{}'", config_file);
// Try to read the configuration again
match read_config_no_lock(api.clone(), &config_file).await {
let data = if namespace_lock_held {
read_config_no_lock(api.clone(), &config_file).await
} else {
read_config(api.clone(), &config_file).await
};
match data {
Ok(cfg_data) => {
let cfg = decode_persisted_server_config(&cfg_data)?;
return Ok(cfg.merge());
@@ -2036,11 +2233,16 @@ pub struct ServerConfigSnapshot {
raw: Option<Vec<u8>>,
seed: Option<Vec<u8>>,
etag: Option<String>,
generation: Option<Uuid>,
_local_guard: OwnedRwLockWriteGuard<()>,
_guard: rustfs_lock::NamespaceLockGuard,
}
impl ServerConfigSnapshot {
pub fn object_exists(&self) -> bool {
self.raw.is_some()
}
pub fn ensure_lock_held(&self) -> Result<()> {
if self._guard.is_lock_lost() {
return Err(Error::other("server config transaction lock was lost"));
@@ -2051,12 +2253,34 @@ impl ServerConfigSnapshot {
pub fn is_lock_lost(&self) -> bool {
self._guard.is_lock_lost()
}
pub fn generation(&self) -> Option<Uuid> {
self.generation
}
}
/// Read a server config transaction snapshot while holding the same local and
/// distributed write locks used by every other server-config writer. Internal
/// reads and the later conditional write use no-lock object I/O; the guards
/// remain live until the snapshot is dropped.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct ServerConfigSaveResult {
persisted: bool,
generation: Option<Uuid>,
}
impl ServerConfigSaveResult {
pub fn persisted(&self) -> bool {
self.persisted
}
pub fn generation(&self) -> Option<Uuid> {
self.generation
}
}
/// Read a server config transaction snapshot while holding a dedicated
/// transaction lock. The config object's normal namespace lock remains
/// available to fence reads and the conditional write at commit time.
/// The transaction guard remains live until the snapshot is dropped,
/// serializing persistence and history ordering across admin nodes. Runtime
/// state is reloaded from the durable object after this guard is released.
pub async fn read_server_config_snapshot<S>(api: Arc<S>) -> Result<ServerConfigSnapshot>
where
S: ObjectIO<
@@ -2071,12 +2295,10 @@ where
{
let config_file = server_config_path();
let local_guard = SERVER_CONFIG_LOCK.clone().write_owned().await;
let lock = api.new_ns_lock(RUSTFS_META_BUCKET, &config_file).await?;
let transaction_lock = server_config_transaction_lock_path();
let lock = api.new_ns_lock(RUSTFS_META_BUCKET, &transaction_lock).await?;
let guard = lock.get_write_lock(get_lock_acquire_timeout()).await?;
let read_options = ObjectOptions {
no_lock: true,
..Default::default()
};
let read_options = ObjectOptions::default();
match read_config_with_metadata_inner(api, &config_file, &read_options, true).await {
Ok((raw, object_info)) => {
let (config, seed) = decode_persisted_server_config_with_seed(&raw)?;
@@ -2085,6 +2307,7 @@ where
raw: Some(raw),
seed: Some(seed),
etag: object_info.etag,
generation: object_info.data_dir.filter(|generation| !generation.is_nil()),
_local_guard: local_guard,
_guard: guard,
})
@@ -2094,6 +2317,7 @@ where
raw: None,
seed: None,
etag: None,
generation: None,
_local_guard: local_guard,
_guard: guard,
}),
@@ -2108,6 +2332,27 @@ where
/// lock, so a concurrent update or transaction lease loss cannot commit an
/// unfenced overwrite.
pub async fn save_server_config_snapshot<S>(api: Arc<S>, cfg: &Config, snapshot: &ServerConfigSnapshot) -> Result<bool>
where
S: ObjectIO<
Error = Error,
RangeSpec = HTTPRangeSpec,
HeaderMap = HeaderMap,
ObjectOptions = ObjectOptions,
ObjectInfo = ObjectInfo,
GetObjectReader = GetObjectReader,
PutObjectReader = PutObjReader,
> + NamespaceLocking<Error = Error, NamespaceLock = rustfs_lock::NamespaceLockWrapper>,
{
save_server_config_snapshot_with_generation(api, cfg, snapshot)
.await
.map(|result| result.persisted())
}
pub async fn save_server_config_snapshot_with_generation<S>(
api: Arc<S>,
cfg: &Config,
snapshot: &ServerConfigSnapshot,
) -> Result<ServerConfigSaveResult>
where
S: ObjectIO<
Error = Error,
@@ -2126,13 +2371,19 @@ where
&& configs_semantically_equal(&snapshot.config, cfg)
{
debug!("server config unchanged and already in standard object shape, skip write");
return Ok(false);
return Ok(ServerConfigSaveResult {
persisted: false,
generation: snapshot.generation(),
});
}
let data = encode_server_config_blob(cfg, snapshot.seed.as_deref())?;
if snapshot.raw.as_deref().is_some_and(|current| current == data.as_slice()) {
debug!("server config bytes unchanged after encode, skip write");
return Ok(false);
return Ok(ServerConfigSaveResult {
persisted: false,
generation: snapshot.generation(),
});
}
let http_preconditions = if snapshot.raw.is_some() {
@@ -2152,19 +2403,22 @@ where
}
};
save_config_with_opts(
snapshot.ensure_lock_held()?;
let object_info = save_config_with_opts_and_metadata(
api,
&config_file,
data,
&ObjectOptions {
max_parity: true,
no_lock: true,
http_preconditions: Some(http_preconditions),
..Default::default()
},
)
.await?;
Ok(true)
Ok(ServerConfigSaveResult {
persisted: true,
generation: object_info.data_dir.filter(|generation| !generation.is_nil()),
})
}
/// Saves the server config while an upper layer holds the namespace write
@@ -2301,8 +2555,9 @@ mod tests {
use super::{
SERVER_CONFIG_LOCK, ServerConfigSnapshot, apply_dynamic_config_for_sub_sys_with, config_task_join_error,
configs_semantically_equal, decode_server_config_blob, encode_server_config_blob, is_standard_object_server_config,
lookup_configs, read_config, read_config_preserve_empty, read_config_with_metadata, read_config_without_migrate,
read_server_config_snapshot, save_server_config, save_server_config_snapshot, server_config_path, storage_class_kvs_mut,
lookup_configs, new_and_save_server_config, read_config, read_config_preserve_empty, read_config_with_metadata,
read_config_without_migrate, read_server_config_snapshot, save_server_config, save_server_config_snapshot,
save_server_config_snapshot_with_generation, server_config_transaction_lock_path, storage_class_kvs_mut,
};
use crate::config::{audit, heal, notify, oidc, scanner};
use crate::disk::endpoint::Endpoint;
@@ -2311,7 +2566,9 @@ mod tests {
use crate::object_api::{GetObjectReader, ObjectInfo, ObjectOptions, PutObjReader};
use crate::runtime::sources as runtime_sources;
use crate::set_disk::SetDisks;
use crate::storage_api_contracts::{admin::StorageAdminApi, namespace::NamespaceLocking as _, range::HTTPRangeSpec};
use crate::storage_api_contracts::{
admin::StorageAdminApi, namespace::NamespaceLocking as _, object::HTTPPreconditions, range::HTTPRangeSpec,
};
use http::HeaderMap;
use rustfs_config::audit::{AUDIT_AMQP_SUB_SYS, AUDIT_KAFKA_SUB_SYS, AUDIT_MQTT_SUB_SYS, AUDIT_WEBHOOK_SUB_SYS};
use rustfs_config::notify::{
@@ -3104,6 +3361,85 @@ mod tests {
);
}
#[test]
fn root_heal_null_decodes_as_no_override_and_is_canonicalized_on_save() {
let seed = br#"{
"version":"33",
"storageclass":{"standard":"","rrs":""},
"heal":null,
"future_root":{"mode":"keep"},
"openid":{"default":{
"config_url":"https://issuer.example/.well-known/openid-configuration",
"client_id":"console",
"client_secret":"oidc-secret",
"future_provider_control":"keep"
}},
"notify":{"webhook":{"primary":{
"enable":true,
"endpoint":"https://notify.example/hook",
"auth_token":"notify-secret",
"future_notify_control":"keep"
}}},
"logger":{"webhook":{"primary":{
"enable":true,
"endpoint":"https://audit.example/hook",
"auth_token":"audit-secret",
"future_audit_control":"keep"
}}}
}"#;
let cfg = decode_server_config_blob(seed).expect("root heal null should mean no persisted override");
assert!(cfg.get_value(HEAL_SUB_SYS, DEFAULT_DELIMITER).is_none());
assert!(!is_standard_object_server_config(seed));
let encoded = encode_server_config_blob(&cfg, Some(seed)).expect("legacy seed should canonicalize on an authorized save");
let value: Value = serde_json::from_slice(&encoded).expect("canonical config should be valid JSON");
assert!(value.get(HEAL_SUB_SYS).is_none());
assert_eq!(value["future_root"]["mode"].as_str(), Some("keep"));
assert_eq!(value["openid"]["default"]["client_secret"].as_str(), Some("oidc-secret"));
assert_eq!(value["openid"]["default"]["future_provider_control"].as_str(), Some("keep"));
assert_eq!(value["notify"]["webhook"]["primary"]["auth_token"].as_str(), Some("notify-secret"));
assert_eq!(value["notify"]["webhook"]["primary"]["future_notify_control"].as_str(), Some("keep"));
assert_eq!(value["logger"]["webhook"]["primary"]["auth_token"].as_str(), Some("audit-secret"));
assert_eq!(value["logger"]["webhook"]["primary"]["future_audit_control"].as_str(), Some("keep"));
assert!(is_standard_object_server_config(&encoded));
}
#[test]
fn invalid_scalar_and_nested_null_config_shapes_remain_rejected() {
let invalid_sections = [
r#""scanner":null"#,
r#""heal":"""#,
r#""heal":false"#,
r#""heal":0"#,
r#""heal":{"default":null}"#,
r#""heal":{"_":null}"#,
r#""heal":{"bitrot_cycle":null}"#,
r#""heal":[{"key":"bitrot_cycle","value":null}]"#,
];
for section in invalid_sections {
let input = format!(r#"{{"version":"33","storageclass":{{"standard":"","rrs":""}},{section}}}"#);
let err = decode_server_config_blob(input.as_bytes()).expect_err("invalid scalar shape must remain rejected");
assert!(
err.to_string().contains("expected"),
"invalid section {section} returned an unrelated error: {err}"
);
}
}
#[test]
fn valid_heal_object_and_kvs_array_shapes_remain_accepted() {
let empty_object = br#"{"version":"33","storageclass":{"standard":"","rrs":""},"heal":{}}"#;
let cfg = decode_server_config_blob(empty_object).expect("empty heal object should decode as no override");
assert!(cfg.get_value(HEAL_SUB_SYS, DEFAULT_DELIMITER).is_none());
let kvs_array =
br#"{"version":"33","storageclass":{"standard":"","rrs":""},"heal":[{"key":"bitrot_cycle","value":"off"}]}"#;
let cfg = decode_server_config_blob(kvs_array).expect("heal KVS array should decode");
assert!(cfg.get_value(HEAL_SUB_SYS, DEFAULT_DELIMITER).is_some());
}
#[test]
fn scanner_update_preserves_unknown_root_and_oidc_provider_fields() {
let seed = br#"{
@@ -3131,6 +3467,171 @@ mod tests {
assert_eq!(value["openid"]["default"]["client_id"].as_str(), Some("console"));
}
#[test]
fn storageclass_reset_removes_stale_inline_block_from_seed() {
let seed = br#"{
"version":"33",
"storageclass":{
"standard":"EC:2",
"rrs":"EC:1",
"optimize":"availability",
"inline_block":"64KiB",
"future_storage_control":"keep"
}
}"#;
let encoded = encode_server_config_blob(&Config::new(), Some(seed)).expect("storageclass reset should encode");
let value: Value = serde_json::from_slice(&encoded).expect("encoded config should be valid json");
let storageclass = value["storageclass"].as_object().expect("storageclass object");
assert!(storageclass.get(crate::config::storageclass::INLINE_BLOCK).is_none());
assert_eq!(storageclass["future_storage_control"].as_str(), Some("keep"));
}
#[test]
fn target_update_preserves_unknown_fields_without_restoring_removed_instances() {
let seed = br#"{
"version":"33",
"storageclass":{"standard":"","rrs":""},
"notify":{"webhook":{
"primary":{
"enable":true,
"endpoint":"https://notify.example/old",
"auth_token":"notify-secret",
"future_control":"keep"
},
"removed":{"enable":true,"endpoint":"https://notify.example/removed"},
"retained":{"enable":true,"endpoint":"https://notify.example/retained","future_control":"keep"},
"enable":{"enable":true,"endpoint":"https://notify.example/named-enable","future_control":"keep"}
}}
}"#;
let mut cfg = decode_server_config_blob(seed).expect("target seed should decode");
let webhook = cfg
.0
.get_mut(NOTIFY_WEBHOOK_SUB_SYS)
.expect("notify webhook subsystem should exist");
webhook
.get_mut("primary")
.expect("primary target should exist")
.insert(rustfs_config::WEBHOOK_ENDPOINT.to_string(), "https://notify.example/new".to_string());
webhook.remove("removed");
webhook.remove("retained");
let encoded = encode_server_config_blob(&cfg, Some(seed)).expect("target update should encode");
let value: Value = serde_json::from_slice(&encoded).expect("encoded config should be valid json");
let webhook = value["notify"]["webhook"].as_object().expect("webhook section");
assert_eq!(webhook["primary"]["endpoint"].as_str(), Some("https://notify.example/new"));
assert_eq!(webhook["primary"]["auth_token"].as_str(), Some("notify-secret"));
assert_eq!(webhook["primary"]["future_control"].as_str(), Some("keep"));
assert!(webhook.get("removed").is_none());
assert_eq!(webhook["retained"]["future_control"].as_str(), Some("keep"));
assert!(webhook["retained"].get("enable").is_none());
assert!(webhook["retained"].get("endpoint").is_none());
assert_eq!(webhook["enable"]["endpoint"].as_str(), Some("https://notify.example/named-enable"));
assert_eq!(webhook["enable"]["future_control"].as_str(), Some("keep"));
}
#[test]
fn shorthand_target_update_preserves_shape_and_unknown_nested_fields() {
let seed = br#"{
"version":"33",
"storageclass":{"standard":"","rrs":""},
"notify":{"webhook":{
"enable":true,
"endpoint":"https://notify.example/old",
"future_control":{"endpoint":"leave-untouched","mode":"keep"}
}}
}"#;
let mut cfg = decode_server_config_blob(seed).expect("shorthand target should decode");
cfg.0
.get_mut(NOTIFY_WEBHOOK_SUB_SYS)
.and_then(|targets| targets.get_mut(DEFAULT_DELIMITER))
.expect("default webhook target should exist")
.insert(rustfs_config::WEBHOOK_ENDPOINT.to_string(), "https://notify.example/new".to_string());
let encoded = encode_server_config_blob(&cfg, Some(seed)).expect("shorthand target update should encode");
let value: Value = serde_json::from_slice(&encoded).expect("encoded config should be valid json");
let webhook = value["notify"]["webhook"].as_object().expect("webhook shorthand object");
assert_eq!(webhook["endpoint"].as_str(), Some("https://notify.example/new"));
assert!(webhook.get("default").is_none());
assert_eq!(webhook["future_control"]["endpoint"].as_str(), Some("leave-untouched"));
assert_eq!(webhook["future_control"]["mode"].as_str(), Some("keep"));
let decoded = decode_server_config_blob(&encoded).expect("updated shorthand target should remain decodable");
assert_eq!(
decoded
.get_value(NOTIFY_WEBHOOK_SUB_SYS, DEFAULT_DELIMITER)
.expect("updated default webhook target")
.get(rustfs_config::WEBHOOK_ENDPOINT),
"https://notify.example/new"
);
}
#[test]
fn target_kvs_update_preserves_unknown_entries_and_attributes() {
let seed = br#"{
"version":"33",
"storageclass":{"standard":"","rrs":""},
"notify":{"webhook":{"primary":[
{"key":"enable","value":"on","hidden_if_empty":false},
{"key":"endpoint","value":"https://notify.example/old","future_attribute":"keep-endpoint"},
{"key":"future_control","value":"keep","future_attribute":"keep-control"}
]}}
}"#;
let mut cfg = decode_server_config_blob(seed).expect("target KVS seed should decode");
cfg.0
.get_mut(NOTIFY_WEBHOOK_SUB_SYS)
.and_then(|targets| targets.get_mut("primary"))
.expect("primary webhook target should exist")
.insert(rustfs_config::WEBHOOK_ENDPOINT.to_string(), "https://notify.example/new".to_string());
let encoded = encode_server_config_blob(&cfg, Some(seed)).expect("target KVS update should encode");
let value: Value = serde_json::from_slice(&encoded).expect("encoded config should be valid json");
let entries = value["notify"]["webhook"]["primary"]
.as_array()
.expect("target KVS shape should be preserved");
let endpoint = entries
.iter()
.find(|entry| entry["key"].as_str() == Some(rustfs_config::WEBHOOK_ENDPOINT))
.expect("endpoint entry should remain");
let future = entries
.iter()
.find(|entry| entry["key"].as_str() == Some("future_control"))
.expect("unknown target entry should remain");
assert_eq!(endpoint["value"].as_str(), Some("https://notify.example/new"));
assert_eq!(endpoint["future_attribute"].as_str(), Some("keep-endpoint"));
assert_eq!(future["value"].as_str(), Some("keep"));
assert_eq!(future["future_attribute"].as_str(), Some("keep-control"));
}
#[test]
fn target_default_alias_is_canonicalized_without_losing_unknown_fields() {
let seed = br#"{
"version":"33",
"storageclass":{"standard":"","rrs":""},
"notify":{"webhook":{
"_":{"enable":false,"endpoint":"https://notify.example/alias","future_alias":"keep"},
"default":{"enable":true,"endpoint":"https://notify.example/default","future_default":"keep"}
}}
}"#;
let cfg = decode_server_config_blob(seed).expect("dual default aliases should decode");
let expected_endpoint = cfg
.get_value(NOTIFY_WEBHOOK_SUB_SYS, DEFAULT_DELIMITER)
.expect("default webhook target should exist")
.get(rustfs_config::WEBHOOK_ENDPOINT);
let encoded = encode_server_config_blob(&cfg, Some(seed)).expect("default alias should canonicalize");
let value: Value = serde_json::from_slice(&encoded).expect("encoded config should be valid json");
let webhook = value["notify"]["webhook"].as_object().expect("webhook section");
assert!(webhook.get(DEFAULT_DELIMITER).is_none());
assert_eq!(webhook["default"]["endpoint"].as_str(), Some(expected_endpoint.as_str()));
assert_eq!(webhook["default"]["future_alias"].as_str(), Some("keep"));
assert_eq!(webhook["default"]["future_default"].as_str(), Some("keep"));
}
#[test]
fn test_scanner_config_changes_are_semantically_significant() {
let baseline = Config::new();
@@ -4124,6 +4625,7 @@ mod tests {
/// What reads of the config object currently return.
enum RecoveryReadState {
Missing,
Blob(Vec<u8>),
QuorumError,
}
@@ -4136,6 +4638,7 @@ mod tests {
heal_calls: AtomicUsize,
write_calls: AtomicUsize,
last_put_no_lock: AtomicBool,
last_put_preconditions: Mutex<Option<HTTPPreconditions>>,
revision: AtomicUsize,
drive_counts: Vec<usize>,
lock_manager: Arc<rustfs_lock::GlobalLockManager>,
@@ -4150,6 +4653,7 @@ mod tests {
heal_calls: AtomicUsize::new(0),
write_calls: AtomicUsize::new(0),
last_put_no_lock: AtomicBool::new(false),
last_put_preconditions: Mutex::new(None),
revision: AtomicUsize::new(1),
drive_counts: vec![2],
lock_manager: Arc::new(rustfs_lock::GlobalLockManager::new()),
@@ -4206,6 +4710,7 @@ mod tests {
_opts: &ObjectOptions,
) -> Result<GetObjectReader> {
let data = match &*self.state.lock().expect("state lock poisoned") {
RecoveryReadState::Missing => return Err(Error::ConfigNotFound),
RecoveryReadState::Blob(data) => data.clone(),
RecoveryReadState::QuorumError => return Err(Error::ErasureReadQuorum),
};
@@ -4213,6 +4718,9 @@ mod tests {
size: data.len() as i64,
actual_size: data.len() as i64,
etag: Some(format!("config-{}", self.revision.load(Ordering::SeqCst))),
data_dir: Some(uuid::Uuid::from_u128(
u128::try_from(self.revision.load(Ordering::SeqCst)).expect("test revision should fit in u128"),
)),
..Default::default()
};
Ok(GetObjectReader {
@@ -4231,15 +4739,19 @@ mod tests {
opts: &ObjectOptions,
) -> Result<ObjectInfo> {
let current_etag = format!("config-{}", self.revision.load(Ordering::SeqCst));
let object_exists = matches!(&*self.state.lock().expect("state lock poisoned"), RecoveryReadState::Blob(_));
if let Some(preconditions) = &opts.http_preconditions
&& (preconditions.if_match_value().is_some_and(|etag| etag != current_etag)
|| preconditions.if_none_match_value() == Some("*"))
&& (preconditions
.if_match_value()
.is_some_and(|etag| !object_exists || etag != current_etag)
|| (object_exists && preconditions.if_none_match_value() == Some("*")))
{
return Err(Error::PreconditionFailed);
}
let mut body = Vec::new();
data.stream.read_to_end(&mut body).await?;
self.last_put_no_lock.store(opts.no_lock, Ordering::SeqCst);
*self.last_put_preconditions.lock().expect("preconditions lock poisoned") = opts.http_preconditions.clone();
self.write_calls.fetch_add(1, Ordering::SeqCst);
*self.state.lock().expect("state lock poisoned") = RecoveryReadState::Blob(body.clone());
let revision = self.revision.fetch_add(1, Ordering::SeqCst) + 1;
@@ -4247,6 +4759,7 @@ mod tests {
size: i64::try_from(body.len()).expect("test config should fit in i64"),
actual_size: i64::try_from(body.len()).expect("test config should fit in i64"),
etag: Some(format!("config-{revision}")),
data_dir: Some(uuid::Uuid::from_u128(u128::try_from(revision).expect("test revision should fit in u128"))),
..Default::default()
})
}
@@ -4284,10 +4797,18 @@ mod tests {
.expect("scanner-only config change should be persisted");
assert_eq!(store.write_calls.load(Ordering::SeqCst), 1);
assert!(store.last_put_no_lock.load(Ordering::SeqCst));
assert!(!store.last_put_no_lock.load(Ordering::SeqCst));
let preconditions = store
.last_put_preconditions
.lock()
.expect("preconditions lock poisoned")
.clone()
.expect("existing config update must be conditional");
assert_eq!(preconditions.if_match_value(), Some("config-1"));
assert_eq!(preconditions.if_none_match_value(), None);
assert_eq!(
store.lock_resources.lock().expect("lock resources mutex poisoned").as_slice(),
&[server_config_path()]
&[server_config_transaction_lock_path()]
);
let decoded = read_config_without_migrate(store)
.await
@@ -4301,6 +4822,73 @@ mod tests {
);
}
#[tokio::test]
async fn server_config_snapshot_save_returns_committed_generation() {
let baseline = encode_server_config_blob(&Config::new(), None).expect("baseline config should encode");
let store = Arc::new(RecoveryMockStore::new(RecoveryReadState::Blob(baseline), None));
let snapshot = read_server_config_snapshot(store.clone())
.await
.expect("server config snapshot");
let result = save_server_config_snapshot_with_generation(store, &config_with_scanner_cycle("61"), &snapshot)
.await
.expect("conditional config save");
assert!(result.persisted());
assert_eq!(result.generation(), Some(uuid::Uuid::from_u128(2)));
}
#[tokio::test]
async fn missing_server_config_is_created_with_if_none_match() {
let store = Arc::new(RecoveryMockStore::new(RecoveryReadState::Missing, None));
let cfg = config_with_scanner_cycle("61");
save_server_config(store.clone(), &cfg)
.await
.expect("missing config should be created conditionally");
assert_eq!(store.write_calls.load(Ordering::SeqCst), 1);
assert!(!store.last_put_no_lock.load(Ordering::SeqCst));
let preconditions = store
.last_put_preconditions
.lock()
.expect("preconditions lock poisoned")
.clone()
.expect("missing config create must be conditional");
assert_eq!(preconditions.if_match_value(), None);
assert_eq!(preconditions.if_none_match_value(), Some("*"));
let persisted = read_config_without_migrate(store)
.await
.expect("created config should reload");
assert_eq!(
persisted
.get_value(SCANNER_SUB_SYS, DEFAULT_DELIMITER)
.expect("persisted scanner config")
.get(SCANNER_CYCLE),
"61"
);
}
#[tokio::test]
async fn missing_config_initialization_recheck_preserves_concurrent_config() {
let existing = config_with_scanner_cycle("73");
let baseline = encode_server_config_blob(&existing, None).expect("existing config should encode");
let store = Arc::new(RecoveryMockStore::new(RecoveryReadState::Blob(baseline), None));
let observed = new_and_save_server_config(store.clone())
.await
.expect("initialization recheck should return the config created by another writer");
assert_eq!(store.write_calls.load(Ordering::SeqCst), 0);
assert_eq!(
observed
.get_value(SCANNER_SUB_SYS, DEFAULT_DELIMITER)
.expect("concurrent scanner config")
.get(SCANNER_CYCLE),
"73"
);
}
#[tokio::test]
async fn stale_server_config_snapshot_cannot_overwrite_newer_update() {
let baseline = encode_server_config_blob(&Config::new(), None).expect("baseline config should encode");
@@ -4357,7 +4945,7 @@ mod tests {
let lock = rustfs_lock::NamespaceLock::new("server-config-lease-loss".to_string(), client.clone());
let guard = lock
.lock_guard(
rustfs_lock::ObjectKey::new(crate::disk::RUSTFS_META_BUCKET, server_config_path()),
rustfs_lock::ObjectKey::new(crate::disk::RUSTFS_META_BUCKET, server_config_transaction_lock_path()),
"server-config-lease-loss",
std::time::Duration::from_secs(1),
std::time::Duration::from_millis(120),
@@ -4372,6 +4960,7 @@ mod tests {
raw: Some(baseline.clone()),
seed: None,
etag: Some("config-0".to_string()),
generation: Some(uuid::Uuid::from_u128(1)),
_local_guard: local_guard,
_guard: guard,
};
+224 -1
View File
@@ -4714,6 +4714,7 @@ mod tests {
use crate::layout::endpoints::SetupType;
use crate::object_api::BLOCK_SIZE_V2;
use crate::object_api::ObjectInfo;
use crate::set_disk::core::io_primitives::rename_fanout_barrier;
use crate::storage_api_contracts::{
heal::HealOperations as _, lifecycle::TransitionedObject, list::ListOperations as _, multipart::CompletePart,
namespace::NamespaceLocking as _, object::ObjectIO as _, object::ObjectOperations as _,
@@ -9565,6 +9566,15 @@ mod tests {
make_local_bucket_test_set_disks_with_drive_count(2).await
}
fn assert_exclusive_object_lock_held(set_disks: &SetDisks, bucket: &str, object: &str) {
let lock = set_disks
.local_lock_manager_for_test()
.get_lock_info(&ObjectKey::new(bucket, object))
.expect("object lock should be visible while rename is paused");
assert!(matches!(lock.mode, rustfs_lock::LockMode::Exclusive));
assert_eq!(lock.owner.as_ref(), set_disks.locker_owner.as_str());
}
async fn make_local_bucket_test_set_disks_with_drive_count(drive_count: usize) -> Arc<SetDisks> {
let format = FormatV3::new(1, drive_count);
let mut endpoints = Vec::new();
@@ -9599,7 +9609,9 @@ mod tests {
disks.push(Some(disk));
}
let set_disks = SetDisks::new(
let instance_ctx = Arc::new(InstanceContext::new());
instance_ctx.update_erasure_type(SetupType::Erasure).await;
let set_disks = SetDisks::new_with_instance_ctx(
"test-owner".to_string(),
Arc::new(RwLock::new(disks)),
drive_count,
@@ -9609,6 +9621,7 @@ mod tests {
endpoints,
format,
Vec::new(),
instance_ctx,
)
.await;
set_disks.set_test_storage_class_config(
@@ -10262,6 +10275,216 @@ mod tests {
));
}
#[tokio::test]
async fn conditional_replace_holds_object_lock_through_rename() {
let set_disks = make_local_bucket_test_set_disks().await;
let bucket = "bucket-conditional-replace-fence";
let object = "config/conditional-replace.json";
set_disks
.make_bucket(bucket, &MakeBucketOptions::default())
.await
.expect("bucket should be created");
let mut initial_reader = PutObjReader::from_vec(b"initial config".to_vec());
let initial = set_disks
.put_object(
bucket,
object,
&mut initial_reader,
&ObjectOptions {
no_lock: true,
..Default::default()
},
)
.await
.expect("initial config should be written");
let initial_etag = initial.etag.expect("initial config should have an ETag");
let barrier = rename_fanout_barrier::arm(object, 0, rename_fanout_barrier::PHASE_RENAME);
let writer_store = set_disks.clone();
let expected_etag = initial_etag.clone();
let writer = tokio::spawn(async move {
let mut reader = PutObjReader::from_vec(b"replacement config".to_vec());
writer_store
.put_object(
bucket,
object,
&mut reader,
&ObjectOptions {
preserve_etag: Some("replacement-etag".to_string()),
http_preconditions: Some(HTTPPreconditions {
if_match: Some(expected_etag),
..Default::default()
}),
..Default::default()
},
)
.await
});
tokio::time::timeout(std::time::Duration::from_secs(30), barrier.wait_until_paused())
.await
.expect("conditional replace should reach the rename barrier");
assert_exclusive_object_lock_held(&set_disks, bucket, object);
barrier.release();
writer
.await
.expect("conditional writer task should finish")
.expect("matching conditional replace should commit");
assert!(
set_disks
.local_lock_manager_for_test()
.get_lock_info(&ObjectKey::new(bucket, object))
.is_none(),
"conditional replace should release the object lock after commit"
);
let contender = set_disks
.new_ns_lock(bucket, object)
.await
.expect("contender namespace lock should be created");
let contender_guard = contender
.get_write_lock(std::time::Duration::from_secs(30))
.await
.expect("contender should acquire after conditional replace commits");
drop(contender_guard);
let mut stale_reader = PutObjReader::from_vec(b"stale config".to_vec());
let err = set_disks
.put_object(
bucket,
object,
&mut stale_reader,
&ObjectOptions {
http_preconditions: Some(HTTPPreconditions {
if_match: Some(initial_etag),
..Default::default()
}),
..Default::default()
},
)
.await
.expect_err("the old ETag must fail after the fenced replacement commits");
assert_eq!(err, StorageError::PreconditionFailed);
}
#[tokio::test]
async fn repeated_body_write_keeps_etag_but_changes_data_dir_generation() {
let set_disks = make_local_bucket_test_set_disks().await;
let bucket = "bucket-write-generation";
let object = "config/write-generation.json";
let body = b"identical config body".to_vec();
set_disks
.make_bucket(bucket, &MakeBucketOptions::default())
.await
.expect("bucket should be created");
let mut first_reader = PutObjReader::from_vec(body.clone());
let first = set_disks
.put_object(
bucket,
object,
&mut first_reader,
&ObjectOptions {
no_lock: true,
..Default::default()
},
)
.await
.expect("first config body should be written");
let mut second_reader = PutObjReader::from_vec(body);
let second = set_disks
.put_object(
bucket,
object,
&mut second_reader,
&ObjectOptions {
no_lock: true,
..Default::default()
},
)
.await
.expect("identical config body should be rewritten");
assert_eq!(first.etag, second.etag, "content ETag should expose the ABA collision");
assert_ne!(first.data_dir, second.data_dir, "each committed body write needs a unique generation");
assert!(first.data_dir.is_some() && second.data_dir.is_some());
}
#[tokio::test]
async fn conditional_create_holds_object_lock_through_rename() {
let set_disks = make_local_bucket_test_set_disks().await;
let bucket = "bucket-conditional-create-fence";
let object = "config/conditional-create.json";
set_disks
.make_bucket(bucket, &MakeBucketOptions::default())
.await
.expect("bucket should be created");
let barrier = rename_fanout_barrier::arm(object, 0, rename_fanout_barrier::PHASE_RENAME);
let writer_store = set_disks.clone();
let writer = tokio::spawn(async move {
let mut reader = PutObjReader::from_vec(b"created config".to_vec());
writer_store
.put_object(
bucket,
object,
&mut reader,
&ObjectOptions {
http_preconditions: Some(HTTPPreconditions {
if_none_match: Some("*".to_string()),
..Default::default()
}),
..Default::default()
},
)
.await
});
tokio::time::timeout(std::time::Duration::from_secs(30), barrier.wait_until_paused())
.await
.expect("conditional create should reach the rename barrier");
assert_exclusive_object_lock_held(&set_disks, bucket, object);
barrier.release();
writer
.await
.expect("conditional writer task should finish")
.expect("first conditional create should commit");
assert!(
set_disks
.local_lock_manager_for_test()
.get_lock_info(&ObjectKey::new(bucket, object))
.is_none(),
"conditional create should release the object lock after commit"
);
let contender = set_disks
.new_ns_lock(bucket, object)
.await
.expect("contender namespace lock should be created");
let contender_guard = contender
.get_write_lock(std::time::Duration::from_secs(30))
.await
.expect("contender should acquire after conditional create commits");
drop(contender_guard);
let mut duplicate_reader = PutObjReader::from_vec(b"duplicate config".to_vec());
let err = set_disks
.put_object(
bucket,
object,
&mut duplicate_reader,
&ObjectOptions {
http_preconditions: Some(HTTPPreconditions {
if_none_match: Some("*".to_string()),
..Default::default()
}),
..Default::default()
},
)
.await
.expect_err("a second create-only write must not replace the committed config");
assert_eq!(err, StorageError::PreconditionFailed);
}
#[tokio::test]
async fn set_level_if_none_match_fails_closed_without_read_quorum() {
let set_disks = make_local_bucket_test_set_disks_with_drive_count(4).await;
+127 -12
View File
@@ -46,7 +46,10 @@ use zeroize::Zeroizing;
/// `key_dir` is accepted, so identifiers already in use by existing deployments keep
/// resolving. Only separators, traversal and the degenerate cases are refused, which is
/// what stops `key_dir.join(...)` from escaping.
fn validate_key_id(key_id: &str) -> Result<()> {
///
/// pub(crate) because the backup restore path applies the same containment
/// rule to key identifiers recovered from bundle artifacts.
pub(crate) fn validate_key_id(key_id: &str) -> Result<()> {
if key_id.is_empty() {
return Err(KmsError::invalid_key("key identifier must not be empty"));
}
@@ -68,7 +71,15 @@ fn validate_key_id(key_id: &str) -> Result<()> {
}
}
const LOCAL_KMS_MASTER_KEY_SALT_FILE: &str = ".master-key.salt";
// The salt and restore-marker file names are pub(crate) so the backup/restore
// modules (`crate::backup`) address the exact on-disk names instead of copies
// that could drift.
pub(crate) const LOCAL_KMS_MASTER_KEY_SALT_FILE: &str = ".master-key.salt";
/// Commit marker of an in-progress Local restore cutover (see
/// `crate::backup::local_restore`). Its presence means the key directory is
/// mid-cutover: startup must fail closed until the restore is rolled forward
/// or explicitly aborted.
pub(crate) const LOCAL_RESTORE_COMMIT_MARKER_FILE: &str = ".restore-commit.json";
// The KDF parameters are pub(crate) so the backup manifest contract
// (`crate::backup`) records the exact compiled-in derivation instead of a
// copy that could drift.
@@ -87,7 +98,11 @@ pub(crate) const LOCAL_KMS_ARGON2_P_COST: u32 = 1;
/// `foo.tmp-<uuid>` is stored as `foo.tmp-<uuid>.key` — so the `.key` guard
/// plus the exact hyphenated-UUID check makes it impossible to match an
/// authoritative file.
fn is_orphan_commit_temp_name(file_name: &str) -> bool {
///
/// pub(crate) because the backup restore path applies the same classification
/// when it re-enters an interrupted run: a leftover commit temp is never
/// authoritative state, so it does not make a target non-empty.
pub(crate) fn is_orphan_commit_temp_name(file_name: &str) -> bool {
if file_name.ends_with(".key") {
return false;
}
@@ -110,12 +125,15 @@ fn is_orphan_commit_temp_name(file_name: &str) -> bool {
///
/// This intentionally mirrors ecstore's fsync helpers without depending on the
/// ecstore crate: the KMS backend stays decoupled from storage internals.
mod durable_file {
///
/// pub(crate) because the backup restore path (`crate::backup::local_restore`)
/// commits staged files and its cutover marker through the same protocol.
pub(crate) mod durable_file {
use std::io::{self, Write};
use std::path::{Path, PathBuf};
/// How the fully written temp file becomes visible under its final name.
pub(super) enum Publish {
pub(crate) enum Publish {
/// Atomically replace whatever is at the destination via `rename`.
Replace,
/// Publish via `hard_link`, failing with [`CommitError::AlreadyExists`]
@@ -124,7 +142,7 @@ mod durable_file {
}
#[derive(Debug)]
pub(super) enum CommitError {
pub(crate) enum CommitError {
AlreadyExists,
Io(io::Error),
/// Test-only simulated crash: the protocol stops after the given step
@@ -158,14 +176,14 @@ mod durable_file {
/// to prove that every interrupted prefix recovers to either the complete
/// old state or the complete new state.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub(super) enum CommitStep {
pub(crate) enum CommitStep {
TempWritten,
FileSynced,
Published,
DirSynced,
}
pub(super) async fn commit(
pub(crate) async fn commit(
temp_path: PathBuf,
final_path: PathBuf,
content: Vec<u8>,
@@ -179,7 +197,7 @@ mod durable_file {
/// Remove a published file durably: without the parent directory fsync a
/// deleted key could resurface after power loss.
pub(super) async fn remove_durably(path: PathBuf) -> io::Result<()> {
pub(crate) async fn remove_durably(path: PathBuf) -> io::Result<()> {
tokio::task::spawn_blocking(move || {
std::fs::remove_file(&path)?;
let parent = path
@@ -191,6 +209,41 @@ mod durable_file {
.map_err(io::Error::other)?
}
/// Publish an already-durable file under a second name via `hard_link`,
/// then fsync the destination's parent directory.
///
/// This is the restore cutover primitive: the source (a staged file that
/// went through [`commit`]) is already durable, so linking plus a parent
/// fsync is a complete publish. `AlreadyExists` is idempotent success only
/// when the destination content is byte-identical to the source — that is
/// exactly the re-entry case of a cutover interrupted after this link —
/// and a hard failure otherwise, so the primitive can never clobber or
/// silently accept foreign state.
pub(crate) async fn link_durably(source: PathBuf, dest: PathBuf) -> io::Result<()> {
tokio::task::spawn_blocking(move || {
match std::fs::hard_link(&source, &dest) {
Ok(()) => {}
Err(error) if error.kind() == io::ErrorKind::AlreadyExists => {
let existing = std::fs::read(&dest)?;
let staged = std::fs::read(&source)?;
if existing != staged {
return Err(io::Error::new(
io::ErrorKind::AlreadyExists,
format!("destination {} already exists with different content", dest.display()),
));
}
}
Err(error) => return Err(error),
}
let parent = dest
.parent()
.ok_or_else(|| io::Error::other("destination has no parent directory"))?;
fsync_dir(parent)
})
.await
.map_err(io::Error::other)?
}
fn commit_blocking(
temp_path: &Path,
final_path: &Path,
@@ -355,7 +408,7 @@ mod durable_file {
/// Test-only failpoints simulating a crash after a given commit step.
/// Armed per directory so parallel tests never affect each other.
#[cfg(test)]
pub(super) mod failpoint {
pub(crate) mod failpoint {
use super::CommitStep;
use std::path::{Path, PathBuf};
use std::sync::Mutex;
@@ -460,6 +513,11 @@ impl LocalKmsClient {
debug!(path = ?config.key_dir, "KMS key directory created");
}
// The restore-marker guard must run before anything else touches the
// directory (in particular before salt load/creation): a directory
// mid-cutover holds an arbitrary mix of old and new state.
Self::ensure_no_restore_marker(&config).await?;
// Initialize master cipher if master key is provided
let (master_cipher, legacy_master_cipher) = if let Some(ref master_key) = config.master_key {
let salt = Self::load_or_create_master_key_salt(&config).await?;
@@ -491,6 +549,7 @@ impl LocalKmsClient {
if !fs::try_exists(&config.key_dir).await? {
return Err(KmsError::configuration_error("Local KMS key directory does not exist"));
}
Self::ensure_no_restore_marker(&config).await?;
let (master_cipher, legacy_master_cipher) = if let Some(ref master_key) = config.master_key {
let legacy_key = Self::derive_legacy_master_key(master_key)?;
@@ -567,8 +626,36 @@ impl LocalKmsClient {
Self::master_key_salt_path(&self.config)
}
/// Operator-configured master key string, exposed for the backup export
/// module so it can record a one-way verifier in the bundle manifest.
/// Never log or persist this value.
pub(crate) fn configured_master_key(&self) -> Option<&str> {
self.config.master_key.as_deref()
}
/// Fail closed while a restore cutover marker is present: the directory
/// then holds an arbitrary mix of pre-restore and restored state, and the
/// only valid next steps are re-running the restore with the same bundle
/// (roll forward) or explicitly aborting it. This mirrors the missing-salt
/// guard: startup must never paper over a half-applied restore.
async fn ensure_no_restore_marker(config: &LocalConfig) -> Result<()> {
let marker = config.key_dir.join(LOCAL_RESTORE_COMMIT_MARKER_FILE);
if fs::try_exists(&marker).await? {
return Err(KmsError::configuration_error(format!(
"Local KMS key directory has an unfinished restore (marker {} present); \
re-run the restore with the same bundle to roll it forward or abort it explicitly",
marker.display()
)));
}
Ok(())
}
/// Derive a 256-bit key from the master key string using a persistent Argon2id salt.
fn derive_master_key(master_key: &str, salt: &[u8]) -> Result<Key<Aes256Gcm>> {
///
/// pub(crate) because the backup restore path derives the same key from
/// the operator-supplied master key and the bundled salt for its verifier
/// check and staged decryption probe.
pub(crate) fn derive_master_key(master_key: &str, salt: &[u8]) -> Result<Key<Aes256Gcm>> {
let params = Params::new(
LOCAL_KMS_ARGON2_M_COST_KIB,
LOCAL_KMS_ARGON2_T_COST,
@@ -585,7 +672,7 @@ impl LocalKmsClient {
Ok(key)
}
fn derive_legacy_master_key(master_key: &str) -> Result<Key<Aes256Gcm>> {
pub(crate) fn derive_legacy_master_key(master_key: &str) -> Result<Key<Aes256Gcm>> {
let mut hasher = Sha256::new();
hasher.update(master_key.as_bytes());
hasher.update(b"rustfs-kms-local");
@@ -2453,6 +2540,34 @@ mod tests {
assert!(!is_orphan_commit_temp_name("mykey.tmp-"));
}
#[tokio::test]
async fn link_durably_publishes_no_clobber_and_is_content_idempotent() {
let temp_dir = TempDir::new().expect("temp dir");
let source = temp_dir.path().join("staged");
let dest = temp_dir.path().join("published");
fs::write(&source, b"staged-content").await.expect("write source");
durable_file::link_durably(source.clone(), dest.clone())
.await
.expect("first link must succeed");
assert_eq!(fs::read(&dest).await.expect("read dest"), b"staged-content");
// Re-entry with identical content is idempotent success — exactly the
// resumed-cutover case.
durable_file::link_durably(source.clone(), dest.clone())
.await
.expect("re-linking identical content must be idempotent");
// Existing content that differs is a hard failure, never a clobber.
let foreign = temp_dir.path().join("foreign");
fs::write(&foreign, b"different-content").await.expect("write foreign");
let error = durable_file::link_durably(foreign, dest.clone())
.await
.expect_err("differing content must not be clobbered");
assert_eq!(error.kind(), std::io::ErrorKind::AlreadyExists);
assert_eq!(fs::read(&dest).await.expect("dest unchanged"), b"staged-content");
}
#[tokio::test]
async fn durable_commit_fsyncs_every_write_path() {
use durable_file::fsync_recorder;
+30 -12
View File
@@ -61,7 +61,7 @@ impl ScriptedResponse {
pub(crate) struct ScriptedVault {
/// Base address (`http://127.0.0.1:port`) to point a Vault client at.
pub(crate) address: String,
requests: Arc<Mutex<Vec<String>>>,
requests: Arc<Mutex<Vec<(String, String)>>>,
}
impl ScriptedVault {
@@ -81,13 +81,10 @@ impl ScriptedVault {
let Ok((mut stream, _)) = listener.accept().await else {
return;
};
let Some(request_line) = read_request(&mut stream).await else {
let Some(request) = read_request(&mut stream).await else {
continue;
};
recorded
.lock()
.expect("scripted vault request log poisoned")
.push(request_line);
recorded.lock().expect("scripted vault request log poisoned").push(request);
let response = responses
.next()
.unwrap_or_else(|| ScriptedResponse::error(599, "scripted vault: script exhausted"));
@@ -107,14 +104,32 @@ impl ScriptedVault {
/// The `METHOD /path` lines of every request served so far, in order.
pub(crate) fn requests(&self) -> Vec<String> {
self.requests.lock().expect("scripted vault request log poisoned").clone()
self.requests
.lock()
.expect("scripted vault request log poisoned")
.iter()
.map(|(line, _)| line.clone())
.collect()
}
/// The request bodies, in the same order as [`Self::requests`]; empty for
/// bodyless requests. Lets tests assert what a write actually persisted
/// (record contents, check-and-set options), not just that a write happened.
pub(crate) fn request_bodies(&self) -> Vec<String> {
self.requests
.lock()
.expect("scripted vault request log poisoned")
.iter()
.map(|(_, body)| body.clone())
.collect()
}
}
/// Read one HTTP/1.1 request (head plus content-length body) and return its
/// `METHOD /path` line. Draining the body before responding keeps the client
/// from seeing a connection reset while it is still writing.
async fn read_request(stream: &mut TcpStream) -> Option<String> {
/// `METHOD /path` line together with the body. Draining the body before
/// responding keeps the client from seeing a connection reset while it is
/// still writing.
async fn read_request(stream: &mut TcpStream) -> Option<(String, String)> {
let mut buffer = Vec::new();
let mut chunk = [0u8; 4096];
let head_end = loop {
@@ -146,14 +161,17 @@ async fn read_request(stream: &mut TcpStream) -> Option<String> {
})
.next()
.unwrap_or(0);
let mut remaining = content_length.saturating_sub(buffer.len() - head_end);
let mut body = buffer[head_end..].to_vec();
let mut remaining = content_length.saturating_sub(body.len());
while remaining > 0 {
let read = stream.read(&mut chunk).await.ok()?;
if read == 0 {
break;
}
body.extend_from_slice(&chunk[..read]);
remaining = remaining.saturating_sub(read);
}
body.truncate(content_length);
Some(format!("{method} {path}"))
Some((format!("{method} {path}"), String::from_utf8_lossy(&body).into_owned()))
}
File diff suppressed because it is too large Load Diff
File diff suppressed because it is too large Load Diff
+111 -15
View File
@@ -14,9 +14,9 @@
//! Local backend backup export: sealed, KEK-protected bundle production.
//!
//! This is the producer side only; restore lives in a follow-up change. The
//! admin API is not wired here either — callers construct the request and
//! supply the backup KEK explicitly.
//! This is the producer side; the consumer side lives in
//! [`crate::backup::local_restore`]. The admin API is not wired here —
//! callers construct the request and supply the backup KEK explicitly.
//!
//! # Bundle layout
//!
@@ -63,6 +63,7 @@ use aes_gcm::{
use jiff::Zoned;
use rand::RngExt;
use serde::Deserialize;
use sha2::{Digest, Sha256};
use std::path::{Path, PathBuf};
use tokio::fs;
use tokio::io::AsyncWriteExt;
@@ -76,6 +77,12 @@ const SALT_ARTIFACT_PATH: &str = "artifacts/master-key.salt.enc";
const AEAD_NONCE_LEN: usize = 12;
/// Domain-separation context for the artifact AAD binding.
const BUNDLE_AAD_CONTEXT: &str = "rustfs-kms-local-backup:v1";
/// Domain-separation context for the master-key verifier.
const MASTER_KEY_VERIFIER_CONTEXT: &str = "rustfs-kms-local-master-key-verifier:v1";
/// Verifier scheme prefix for the Argon2id (salted) derivation.
pub(crate) const MASTER_KEY_VERIFIER_ARGON2ID_PREFIX: &str = "argon2id-v1:";
/// Verifier scheme prefix for the legacy pre-beta.9 SHA-256 derivation.
pub(crate) const MASTER_KEY_VERIFIER_LEGACY_PREFIX: &str = "legacy-sha256-v1:";
/// Caller-supplied backup KEK: a trust root deliberately separate from the
/// business KMS hierarchy (it must not be a key that is itself part of the
@@ -208,7 +215,15 @@ pub async fn export_local_backup(
));
}
let manifest = build_and_write_bundle(kek, request, &snapshot).await?;
// The verifier lets a restore detect a wrong operator-supplied master key
// before touching any target state; dev-mode directories have no master
// key to verify.
let master_key_verifier = match client.configured_master_key() {
Some(master_key) => Some(compute_master_key_verifier(master_key, snapshot.salt.as_deref(), &request.backup_id)?),
None => None,
};
let manifest = build_and_write_bundle(kek, request, &snapshot, master_key_verifier).await?;
Ok(manifest)
}
@@ -262,6 +277,21 @@ pub async fn decrypt_bundle_artifact(
Err(error) => return Err(error.into()),
};
decrypt_artifact_payload(&manifest.backup_id, manifest.snapshot_generation, descriptor, kek, &payload)
}
/// Verify and decrypt one artifact payload already read into memory.
///
/// Fail-closed order: declared length, encrypted digest, nonce framing, then
/// AEAD authentication. Shared by [`decrypt_bundle_artifact`] and the export
/// pre-seal probe so producer and consumer can never drift on the framing.
fn decrypt_artifact_payload(
backup_id: &str,
snapshot_generation: u64,
descriptor: &ArtifactDescriptor,
kek: &BackupKek,
payload: &[u8],
) -> Result<Zeroizing<Vec<u8>>> {
if (payload.len() as u64) < descriptor.len {
return Err(BackupError::truncated(format!(
"artifact '{}' is {} bytes, manifest declares {}",
@@ -280,7 +310,7 @@ pub async fn decrypt_bundle_artifact(
))
.into());
}
if ContentDigest::sha256_of(&payload) != descriptor.encrypted_digest {
if ContentDigest::sha256_of(payload) != descriptor.encrypted_digest {
return Err(BackupError::corrupted(format!("artifact '{}' does not match its manifest digest", descriptor.path)).into());
}
if payload.len() < AEAD_NONCE_LEN {
@@ -290,7 +320,7 @@ pub async fn decrypt_bundle_artifact(
let (nonce_bytes, ciphertext) = payload.split_at(AEAD_NONCE_LEN);
let mut nonce = [0u8; AEAD_NONCE_LEN];
nonce.copy_from_slice(nonce_bytes);
let aad = artifact_aad(&manifest.backup_id, manifest.snapshot_generation, &descriptor.path);
let aad = artifact_aad(backup_id, snapshot_generation, &descriptor.path);
let plaintext = kek
.cipher()
.decrypt(
@@ -360,6 +390,7 @@ async fn build_and_write_bundle(
kek: &BackupKek,
request: &LocalBackupExportRequest,
snapshot: &CollectedSnapshot,
master_key_verifier: Option<String>,
) -> Result<BackupManifest> {
let mut artifacts = Vec::with_capacity(snapshot.records.len() + 1);
for record in &snapshot.records {
@@ -392,7 +423,7 @@ async fn build_and_write_bundle(
snapshot_generation: request.snapshot_generation,
backup_kek: kek.descriptor(),
artifacts,
local_kdf: Some(local_kdf_descriptor(snapshot)),
local_kdf: Some(local_kdf_descriptor(snapshot, master_key_verifier)),
key_versions: None,
capability_discovery: None,
completeness: CompletenessState::InProgress,
@@ -448,13 +479,25 @@ async fn encrypt_and_write_artifact(
)));
}
Ok(ArtifactDescriptor {
let descriptor = ArtifactDescriptor {
kind,
path: artifact_path.to_string(),
len: payload.len() as u64,
aead_algorithm: AeadAlgorithm::Aes256Gcm,
encrypted_digest: digest,
})
};
// Pre-seal decryption probe: digest equality only proves the ciphertext
// landed intact; this proves the stored artifact actually opens under the
// backup KEK and AAD binding before the manifest may reference it.
let reopened = decrypt_artifact_payload(&request.backup_id, request.snapshot_generation, &descriptor, kek, &written)?;
if reopened.as_slice() != plaintext {
return Err(KmsError::internal_error(format!(
"bundle artifact '{artifact_path}' failed the pre-seal decryption probe"
)));
}
Ok(descriptor)
}
/// AAD binding an artifact to its bundle identity and path. A JSON tuple
@@ -464,6 +507,29 @@ fn artifact_aad(backup_id: &str, snapshot_generation: u64, artifact_path: &str)
.expect("AAD tuple of strings and integers always serializes")
}
/// Compute the opaque one-way master-key verifier recorded in the manifest:
/// `<scheme-prefix>` + `hex(SHA-256(json(context, backup_id) || derived_key))`.
///
/// The derivation follows the directory's KDF state: Argon2id over the
/// persistent salt when one exists, the legacy SHA-256 derivation otherwise.
/// The verifier is not an offline-guessing oracle: computing a candidate
/// requires the salt, which exists only inside the KEK-sealed bundle, and a
/// party holding the KEK already has the strictly stronger oracle of the
/// artifact AEAD tags — while every guess still pays the full Argon2id cost.
/// Binding `backup_id` prevents cross-bundle fingerprint correlation.
pub(crate) fn compute_master_key_verifier(master_key: &str, salt: Option<&[u8]>, backup_id: &str) -> Result<String> {
let framing = serde_json::to_vec(&(MASTER_KEY_VERIFIER_CONTEXT, backup_id))
.expect("verifier framing tuple of strings always serializes");
let (prefix, derived) = match salt {
Some(salt) => (MASTER_KEY_VERIFIER_ARGON2ID_PREFIX, LocalKmsClient::derive_master_key(master_key, salt)?),
None => (MASTER_KEY_VERIFIER_LEGACY_PREFIX, LocalKmsClient::derive_legacy_master_key(master_key)?),
};
let mut hasher = Sha256::new();
hasher.update(&framing);
hasher.update(derived.as_slice());
Ok(format!("{prefix}{}", hex::encode(hasher.finalize())))
}
/// The bundle-level protection label is the weakest state observed across
/// records: any plaintext-dev-only record marks the whole bundle, then any
/// legacy-unspecified marker (unknown until read), and only a uniformly
@@ -484,7 +550,7 @@ fn weakest_observed_protection(records: &[CollectedRecord]) -> AtRestProtection
}
}
fn local_kdf_descriptor(snapshot: &CollectedSnapshot) -> LocalKdfDescriptor {
fn local_kdf_descriptor(snapshot: &CollectedSnapshot, master_key_verifier: Option<String>) -> LocalKdfDescriptor {
let mut modes = Vec::new();
for (marker, mode) in [
(StoredKeyProtection::EncryptedMasterKey, AtRestProtection::EncryptedMasterKey),
@@ -508,9 +574,7 @@ fn local_kdf_descriptor(snapshot: &CollectedSnapshot) -> LocalKdfDescriptor {
LocalKdfDescriptor {
derivation,
protection_modes: modes,
// The verifier shape is left to the restore change; the schema keeps
// it optional so bundles without one stay valid.
master_key_verifier: None,
master_key_verifier,
}
}
@@ -543,8 +607,9 @@ async fn write_new_file(path: &Path, bytes: &[u8]) -> Result<()> {
/// Fsync a directory so freshly created bundle entries survive power loss.
/// No-op on non-Unix platforms where directories cannot be opened for
/// syncing (mirrors the local backend's durable commit helper).
async fn fsync_dir(path: &Path) -> Result<()> {
/// syncing (mirrors the local backend's durable commit helper). Shared with
/// the restore module for the same durability points on the target side.
pub(crate) async fn fsync_dir(path: &Path) -> Result<()> {
#[cfg(unix)]
{
let path = path.to_path_buf();
@@ -725,6 +790,37 @@ mod tests {
assert_eq!(decrypted.as_slice(), source_record.as_slice());
}
#[tokio::test]
async fn export_records_a_master_key_verifier_matching_recomputation() {
let (client, _key_dir) = encrypted_client().await;
client.create_key("verified", "AES_256", None).await.expect("create key");
let bundle = TempDir::new().expect("bundle dir");
let manifest = export_local_backup(&client, &test_kek(), &export_request(bundle.path().join("bundle")))
.await
.expect("export should succeed");
let verifier = manifest
.local_kdf
.as_ref()
.expect("local kdf descriptor")
.master_key_verifier
.as_deref()
.expect("encrypted bundles must record a verifier");
assert!(verifier.starts_with(MASTER_KEY_VERIFIER_ARGON2ID_PREFIX), "got {verifier:?}");
// The verifier is a pure function of (master key, salt, backup id):
// the restore side recomputes it from the operator-supplied key and
// the bundled salt.
let salt = fs::read(client.master_key_salt_file()).await.expect("salt");
let recomputed = compute_master_key_verifier("test-master-key", Some(&salt), "backup-0001").expect("recompute");
assert_eq!(recomputed, verifier);
let wrong_key = compute_master_key_verifier("wrong-master-key", Some(&salt), "backup-0001").expect("recompute");
assert_ne!(wrong_key, verifier, "a different master key must change the verifier");
let other_bundle = compute_master_key_verifier("test-master-key", Some(&salt), "backup-0002").expect("recompute");
assert_ne!(other_bundle, verifier, "verifiers must be bundle-bound");
}
#[tokio::test]
async fn export_fence_blocks_writers_until_released() {
let (client, _key_dir) = encrypted_client().await;
File diff suppressed because it is too large Load Diff
+9 -3
View File
@@ -16,9 +16,10 @@
//!
//! The contract side defines the versioned backup manifest, the per-backend
//! responsibility matrix, typed failure modes, and the restore dry-run
//! report. [`local_export`] implements the producer side for the Local
//! backend as a crate-internal API; restore orchestration and the admin API
//! build on these pieces in follow-up changes.
//! report. [`local_export`] implements the producer side and
//! [`local_restore`] the consumer side for the Local backend as
//! crate-internal APIs; the admin API builds on these pieces in follow-up
//! changes.
//!
//! # Bundle model
//!
@@ -51,6 +52,7 @@ mod capability;
mod dry_run;
mod error;
pub mod local_export;
pub mod local_restore;
mod manifest;
pub use capability::{AtRestProtection, BackupBackendKind, BackupResponsibility};
@@ -62,6 +64,10 @@ pub use local_export::{
BackupKek, LOCAL_BUNDLE_MANIFEST_FILE, LocalBackupExportRequest, decrypt_bundle_artifact, export_local_backup,
read_local_bundle_manifest,
};
pub use local_restore::{
LocalRestoreReport, LocalRestoreRequest, RestoreConflictPolicy, abort_local_restore, dry_run_local_restore,
restore_local_backup,
};
pub use manifest::{
AeadAlgorithm, ArtifactDescriptor, ArtifactKind, BackupKekDescriptor, BackupManifest, CompletenessState, ContentDigest,
DigestAlgorithm, LocalKdfDescriptor, LocalKeyDerivation, ReservedSlot,
+105 -96
View File
@@ -19,7 +19,7 @@ use crate::{
resolve_notify_object_store_handle,
rule_engine::NotifyRuleEngine,
runtime_facade::NotifyRuntimeFacade,
with_notify_server_config_read_lock, with_notify_server_config_write_lock,
with_notify_server_config_read_lock,
};
use rustfs_config::notify::{
NOTIFY_AMQP_SUB_SYS, NOTIFY_KAFKA_SUB_SYS, NOTIFY_MQTT_SUB_SYS, NOTIFY_MYSQL_SUB_SYS, NOTIFY_NATS_SUB_SYS,
@@ -41,12 +41,32 @@ enum NotifyConfigStoreError {
StorageNotAvailable,
Read(String),
Save(String),
Converge { persisted: bool, error: String },
}
fn notification_convergence_error(persisted: bool, error: impl std::fmt::Display) -> NotificationError {
let durable_state = if persisted { "persisted" } else { "unchanged" };
NotificationError::Configuration(format!(
"configuration was {durable_state} but notification runtime convergence failed: {error}"
))
}
async fn supervise_config_update<T>(
mutation: impl std::future::Future<Output = Result<T, NotifyConfigStoreError>> + Send + 'static,
) -> Result<T, NotifyConfigStoreError>
where
T: Send + 'static,
{
tokio::spawn(mutation).await.map_err(|error| {
let outcome = if error.is_cancelled() { "cancelled" } else { "panicked" };
NotifyConfigStoreError::Lock(format!("notify config update task {outcome}"))
})?
}
async fn update_server_config<F>(
modifier: F,
mut modifier: F,
lifecycle: NotifyLifecycleCoordinator,
) -> Result<Option<crate::lifecycle::NotificationLifecycleTransition>, NotifyConfigStoreError>
) -> Result<Option<(crate::lifecycle::NotificationLifecycleTransition, bool)>, NotifyConfigStoreError>
where
F: FnMut(&mut Config) -> bool + Send + 'static,
{
@@ -54,51 +74,31 @@ where
return Err(NotifyConfigStoreError::StorageNotAvailable);
};
let store_for_read = store.clone();
let store_for_save = store.clone();
with_notify_server_config_write_lock(store, move || {
read_modify_write(
modifier,
move || async move {
crate::read_notify_server_config_without_migrate_no_lock(store_for_read)
.await
.map_err(NotifyConfigStoreError::Read)
},
move |config| async move {
crate::save_notify_server_config_no_lock(store_for_save, &config)
.await
.map_err(NotifyConfigStoreError::Save)
},
move |config| lifecycle.update_config(config),
)
supervise_config_update(async move {
let snapshot = crate::read_notify_server_config_snapshot(store.clone())
.await
.map_err(NotifyConfigStoreError::Read)?;
let mut config = snapshot.config.clone();
if !modifier(&mut config) {
return Ok(None);
}
let persisted = crate::save_notify_server_config_snapshot(store.clone(), &config, &snapshot)
.await
.map_err(NotifyConfigStoreError::Save)?;
drop(snapshot);
let read_store = store.clone();
with_notify_server_config_read_lock(store, move || async move {
let latest = crate::read_existing_notify_server_config_no_lock(read_store)
.await
.map_err(|error| NotifyConfigStoreError::Converge { persisted, error })?;
Ok::<_, NotifyConfigStoreError>(Some((lifecycle.update_config(latest), persisted)))
})
.await
.map_err(|error| NotifyConfigStoreError::Converge { persisted, error })?
})
.await
.map_err(NotifyConfigStoreError::Lock)?
}
async fn read_modify_write<F, R, RFut, S, SFut, P, T>(
mut modifier: F,
read: R,
save: S,
publish: P,
) -> Result<Option<T>, NotifyConfigStoreError>
where
F: FnMut(&mut Config) -> bool,
R: FnOnce() -> RFut,
RFut: std::future::Future<Output = Result<Config, NotifyConfigStoreError>>,
S: FnOnce(Config) -> SFut,
SFut: std::future::Future<Output = Result<(), NotifyConfigStoreError>>,
P: FnOnce(Config) -> T,
{
let mut new_config = read().await?;
if !modifier(&mut new_config) {
return Ok(None);
}
save(new_config.clone()).await?;
Ok(Some(publish(new_config)))
}
pub(crate) fn notify_configuration_hint() -> String {
@@ -345,16 +345,18 @@ impl NotifyConfigManager {
if self.lifecycle.state() == NotificationRuntimeState::Terminated {
return Err(NotificationError::Initialization("Notification runtime has terminated".to_string()));
}
let Some(transition) = update_server_config(modifier, self.lifecycle.clone())
.await
.map_err(|err| match err {
NotifyConfigStoreError::Lock(err) => NotificationError::StorageNotAvailable(err),
NotifyConfigStoreError::StorageNotAvailable => NotificationError::StorageNotAvailable(
"Failed to save target configuration: server storage not initialized".to_string(),
),
NotifyConfigStoreError::Read(err) => NotificationError::ReadConfig(err),
NotifyConfigStoreError::Save(err) => NotificationError::SaveConfig(err),
})?
let Some((transition, persisted)) =
update_server_config(modifier, self.lifecycle.clone())
.await
.map_err(|err| match err {
NotifyConfigStoreError::Lock(err) => NotificationError::StorageNotAvailable(err),
NotifyConfigStoreError::StorageNotAvailable => NotificationError::StorageNotAvailable(
"Failed to save target configuration: server storage not initialized".to_string(),
),
NotifyConfigStoreError::Read(err) => NotificationError::ReadConfig(err),
NotifyConfigStoreError::Save(err) => NotificationError::SaveConfig(err),
NotifyConfigStoreError::Converge { persisted, error } => notification_convergence_error(persisted, error),
})?
else {
debug!(
event = EVENT_NOTIFY_CONFIG_UPDATE,
@@ -367,23 +369,53 @@ impl NotifyConfigManager {
return Ok(());
};
let result = if persisted { "updated" } else { "unchanged" };
info!(
event = EVENT_NOTIFY_CONFIG_UPDATE,
component = LOG_COMPONENT_NOTIFY,
subsystem = LOG_SUBSYSTEM_CONFIG,
action = "reload_if_changed",
result = "updated",
result,
"notify config update"
);
transition.wait().await
transition
.wait()
.await
.map_err(|err| notification_convergence_error(persisted, err))
}
}
#[cfg(test)]
mod tests {
use super::{NotifyConfigManager, NotifyConfigStoreError, read_modify_write, runtime_target_id_for_subsystem};
use super::{
NotifyConfigManager, NotifyConfigStoreError, notification_convergence_error, runtime_target_id_for_subsystem,
supervise_config_update,
};
use crate::rules::RulesMap;
use crate::{NotificationError, NotificationRuntimeState};
#[tokio::test]
async fn supervised_config_update_survives_waiter_cancellation() {
let (started_tx, started_rx) = tokio::sync::oneshot::channel();
let (release_tx, release_rx) = tokio::sync::oneshot::channel();
let (completed_tx, completed_rx) = tokio::sync::oneshot::channel();
let waiter = tokio::spawn(async move {
supervise_config_update(async move {
let _ = started_tx.send(());
let _ = release_rx.await;
let _ = completed_tx.send(());
Ok::<_, NotifyConfigStoreError>(())
})
.await
});
started_rx.await.expect("config update should start");
waiter.abort();
release_tx.send(()).expect("config update should be released");
let completed = tokio::time::timeout(std::time::Duration::from_secs(30), completed_rx).await;
assert!(matches!(completed, Ok(Ok(()))), "detached config update should complete");
}
use crate::{
integration::NotificationMetrics, notifier::EventNotifier, registry::TargetRegistry, rule_engine::NotifyRuleEngine,
runtime_facade::NotifyRuntimeFacade,
@@ -392,14 +424,11 @@ mod tests {
NOTIFY_AMQP_SUB_SYS, NOTIFY_KAFKA_SUB_SYS, NOTIFY_MQTT_SUB_SYS, NOTIFY_NATS_SUB_SYS, NOTIFY_POSTGRES_SUB_SYS,
NOTIFY_PULSAR_SUB_SYS, NOTIFY_REDIS_SUB_SYS, NOTIFY_WEBHOOK_SUB_SYS,
};
use rustfs_config::server_config::{Config, KVS};
use rustfs_config::server_config::Config;
use rustfs_s3_types::EventName;
use rustfs_targets::ReplayWorkerManager;
use rustfs_targets::arn::TargetID;
use std::sync::{
Arc,
atomic::{AtomicBool, Ordering},
};
use std::sync::Arc;
use tokio::sync::{RwLock, Semaphore};
fn build_manager() -> NotifyConfigManager {
@@ -463,38 +492,6 @@ mod tests {
assert!(matches!(manager.lifecycle().state(), NotificationRuntimeState::TargetsEnabled { .. }));
}
#[tokio::test]
async fn read_modify_write_publishes_only_after_save() {
let saved = Arc::new(AtomicBool::new(false));
let saved_by_writer = saved.clone();
let observed_by_publisher = saved.clone();
read_modify_write(
|config| {
config
.0
.entry(NOTIFY_WEBHOOK_SUB_SYS.to_string())
.or_default()
.insert("primary".to_string(), KVS::default());
true
},
|| async { Ok::<_, NotifyConfigStoreError>(Config::default()) },
move |_config| async move {
saved_by_writer.store(true, Ordering::Release);
Ok::<_, NotifyConfigStoreError>(())
},
move |_config| {
assert!(
observed_by_publisher.load(Ordering::Acquire),
"publication must observe the completed save"
);
},
)
.await
.expect("read-modify-write should succeed")
.expect("changed config should publish");
}
#[test]
fn runtime_target_id_for_subsystem_maps_notify_webhook_to_runtime_type() {
let target_id = runtime_target_id_for_subsystem(NOTIFY_WEBHOOK_SUB_SYS, "Primary");
@@ -550,4 +547,16 @@ mod tests {
assert_eq!(target_id.id, "audittrail");
assert_eq!(target_id.name, "postgres");
}
#[test]
fn convergence_error_reports_durable_write_state() {
for (persisted, expected) in [(true, "configuration was persisted"), (false, "configuration was unchanged")] {
let error = notification_convergence_error(persisted, "injected failure");
let NotificationError::Configuration(message) = error else {
panic!("convergence error should be a configuration error");
};
assert!(message.contains(expected), "unexpected convergence error: {message}");
assert!(message.contains("injected failure"));
}
}
}
+2 -3
View File
@@ -57,7 +57,6 @@ pub use services::NotifyServices;
pub use status_view::NotifyStatusView;
pub use storage_api::NotifyStore;
pub(crate) use storage_api::crate_boundary::{
read_existing_notify_server_config_no_lock, read_notify_server_config_without_migrate_no_lock,
resolve_notify_object_store_handle, save_notify_server_config_no_lock, with_notify_server_config_read_lock,
with_notify_server_config_write_lock,
read_existing_notify_server_config_no_lock, read_notify_server_config_snapshot, resolve_notify_object_store_handle,
save_notify_server_config_snapshot, with_notify_server_config_read_lock,
};
+12 -24
View File
@@ -15,11 +15,10 @@
use std::sync::Arc;
use rustfs_ecstore::api::config::com::{
read_config_without_migrate_no_lock as read_notify_config_without_migrate_from_backend_no_lock,
read_existing_server_config_no_lock as read_existing_notify_config_from_backend_no_lock,
save_server_config_no_lock as save_notify_server_config_to_backend_no_lock,
read_server_config_snapshot as read_notify_server_config_snapshot_from_backend,
save_server_config_snapshot as save_notify_server_config_snapshot_to_backend,
with_server_config_read_lock as with_notify_server_config_read_lock_from_backend,
with_server_config_write_lock as with_notify_server_config_write_lock_from_backend,
};
use rustfs_ecstore::api::runtime::object_store_handle as resolve_notify_object_store_handle_from_backend;
pub use rustfs_ecstore::api::storage::ECStore as NotifyStore;
@@ -28,10 +27,10 @@ pub(crate) fn resolve_notify_object_store_handle() -> Option<Arc<NotifyStore>> {
resolve_notify_object_store_handle_from_backend()
}
pub(crate) async fn read_notify_server_config_without_migrate_no_lock(
store: Arc<NotifyStore>,
) -> Result<rustfs_config::server_config::Config, String> {
read_notify_config_without_migrate_from_backend_no_lock(store)
pub(crate) type NotifyServerConfigSnapshot = rustfs_ecstore::api::config::com::ServerConfigSnapshot;
pub(crate) async fn read_notify_server_config_snapshot(store: Arc<NotifyStore>) -> Result<NotifyServerConfigSnapshot, String> {
read_notify_server_config_snapshot_from_backend(store)
.await
.map_err(|err| err.to_string())
}
@@ -44,22 +43,12 @@ pub(crate) async fn read_existing_notify_server_config_no_lock(
.map_err(|err| err.to_string())
}
pub(crate) async fn save_notify_server_config_no_lock(
pub(crate) async fn save_notify_server_config_snapshot(
store: Arc<NotifyStore>,
config: &rustfs_config::server_config::Config,
) -> Result<(), String> {
save_notify_server_config_to_backend_no_lock(store, config)
.await
.map_err(|err| err.to_string())
}
pub(crate) async fn with_notify_server_config_write_lock<F, Fut, T>(store: Arc<NotifyStore>, operation: F) -> Result<T, String>
where
F: FnOnce() -> Fut + Send + 'static,
Fut: std::future::Future<Output = T> + Send + 'static,
T: Send + 'static,
{
with_notify_server_config_write_lock_from_backend(store, operation)
snapshot: &NotifyServerConfigSnapshot,
) -> Result<bool, String> {
save_notify_server_config_snapshot_to_backend(store, config, snapshot)
.await
.map_err(|err| err.to_string())
}
@@ -77,8 +66,7 @@ where
pub(crate) mod crate_boundary {
pub(crate) use super::{
read_existing_notify_server_config_no_lock, read_notify_server_config_without_migrate_no_lock,
resolve_notify_object_store_handle, save_notify_server_config_no_lock, with_notify_server_config_read_lock,
with_notify_server_config_write_lock,
read_existing_notify_server_config_no_lock, read_notify_server_config_snapshot, resolve_notify_object_store_handle,
save_notify_server_config_snapshot, with_notify_server_config_read_lock,
};
}
+6
View File
@@ -72,4 +72,10 @@ pub enum Error {
#[error("invalid resource, type: '{0}', pattern: '{1}'")]
InvalidResource(String, String),
#[error("KMS resources require a statement whose actions are all KMS actions")]
KmsResourceWithNonKmsAction,
#[error("bucket policies do not support KMS actions or resources")]
KmsUnsupportedInBucketPolicy,
}
+3
View File
@@ -728,6 +728,8 @@ pub enum KmsAction {
ListKeysAction,
#[strum(serialize = "kms:DescribeKey")]
DescribeKeyAction,
#[strum(serialize = "kms:Decrypt")]
DecryptAction,
}
#[cfg(test)]
@@ -764,6 +766,7 @@ mod tests {
("kms:RotateKey", KmsAction::RotateKeyAction),
("kms:ListKeys", KmsAction::ListKeysAction),
("kms:DescribeKey", KmsAction::DescribeKeyAction),
("kms:Decrypt", KmsAction::DecryptAction),
] {
let action = Action::try_from(raw).expect("Should parse KMS action");
assert_eq!(action, Action::KmsAction(expected));
+385
View File
@@ -1760,6 +1760,391 @@ mod test {
);
}
fn kms_args<'a>(
action: Action,
key_id: &'a str,
conditions: &'a HashMap<String, Vec<String>>,
claims: &'a HashMap<String, Value>,
) -> Args<'a> {
Args {
account: "testuser",
groups: &None,
action,
bucket: "",
conditions,
is_owner: false,
object: key_id,
claims,
deny_only: false,
}
}
#[tokio::test]
async fn test_kms_statement_with_key_resource_scopes_by_key() -> Result<()> {
use crate::policy::action::{Action, KmsAction};
let policy = Policy::parse_config(
br#"{
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"Action": ["kms:DisableKey"],
"Resource": ["arn:aws:kms:::key/key-a"]
}
]
}"#,
)?;
let conditions = HashMap::new();
let claims = HashMap::new();
let action = Action::KmsAction(KmsAction::DisableKeyAction);
assert!(
policy.is_allowed(&kms_args(action, "key-a", &conditions, &claims)).await,
"the granted key must be allowed"
);
assert!(
!policy.is_allowed(&kms_args(action, "key-b", &conditions, &claims)).await,
"a key outside the granted resource must be denied"
);
assert!(
!policy
.is_allowed(&kms_args(Action::KmsAction(KmsAction::EnableKeyAction), "key-a", &conditions, &claims))
.await,
"an action outside the grant must stay denied even for the granted key"
);
assert!(
policy.is_allowed(&kms_args(action, "", &conditions, &claims)).await,
"call sites that do not pass a key resource keep the legacy match-every-key behaviour"
);
Ok(())
}
#[tokio::test]
async fn test_kms_statement_with_wildcard_key_resource() -> Result<()> {
use crate::policy::action::{Action, KmsAction};
let policy = Policy::parse_config(
br#"{
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"Action": ["kms:GenerateDataKey"],
"Resource": ["arn:aws:kms:::key/app-*"]
}
]
}"#,
)?;
let conditions = HashMap::new();
let claims = HashMap::new();
let action = Action::KmsAction(KmsAction::GenerateDataKeyAction);
assert!(
policy
.is_allowed(&kms_args(action, "app-primary", &conditions, &claims))
.await
);
assert!(
!policy
.is_allowed(&kms_args(action, "backup-primary", &conditions, &claims))
.await
);
Ok(())
}
#[tokio::test]
async fn test_kms_statement_without_resource_matches_every_key() -> Result<()> {
use crate::policy::action::{Action, KmsAction};
let policy = Policy::parse_config(
br#"{
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"Action": ["kms:*"]
}
]
}"#,
)?;
let conditions = HashMap::new();
let claims = HashMap::new();
for key_id in ["", "key-a", "any-other-key"] {
assert!(
policy
.is_allowed(&kms_args(Action::KmsAction(KmsAction::DisableKeyAction), key_id, &conditions, &claims))
.await,
"resource-less KMS statement must keep matching every key (key_id: {key_id:?})"
);
}
Ok(())
}
#[tokio::test]
async fn test_kms_deny_with_wildcard_resource_overrides_allow() -> Result<()> {
use crate::policy::action::{Action, KmsAction};
let policy = Policy::parse_config(
br#"{
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"Action": ["kms:*"],
"Resource": ["arn:aws:kms:::key/key-a"]
},
{
"Effect": "Deny",
"Action": ["kms:DisableKey"],
"Resource": ["arn:aws:kms:::key/*"]
}
]
}"#,
)?;
let conditions = HashMap::new();
let claims = HashMap::new();
assert!(
!policy
.is_allowed(&kms_args(Action::KmsAction(KmsAction::DisableKeyAction), "key-a", &conditions, &claims))
.await,
"a wildcard Deny must override the narrower Allow"
);
assert!(
policy
.is_allowed(&kms_args(Action::KmsAction(KmsAction::RotateKeyAction), "key-a", &conditions, &claims))
.await,
"actions outside the Deny keep the Allow"
);
Ok(())
}
#[tokio::test]
async fn test_kms_statement_with_not_resource_excludes_keys() -> Result<()> {
use crate::policy::action::{Action, KmsAction};
let policy = Policy::parse_config(
br#"{
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"Action": ["kms:DescribeKey"],
"NotResource": ["arn:aws:kms:::key/prod-*"]
}
]
}"#,
)?;
let conditions = HashMap::new();
let claims = HashMap::new();
let action = Action::KmsAction(KmsAction::DescribeKeyAction);
assert!(policy.is_allowed(&kms_args(action, "dev-key", &conditions, &claims)).await);
assert!(!policy.is_allowed(&kms_args(action, "prod-key", &conditions, &claims)).await);
Ok(())
}
#[tokio::test]
async fn test_kms_statement_with_s3_resource_is_treated_as_unscoped() -> Result<()> {
use crate::policy::action::{Action, KmsAction};
// Statements combining KMS actions with S3 resources predate KMS resource
// support and were always evaluated as if unscoped. Pin that they still
// match every key (a warning is logged during evaluation).
let policy = Policy::parse_config(
br#"{
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"Action": ["kms:DisableKey"],
"Resource": ["arn:aws:s3:::somebucket/*"]
}
]
}"#,
)?;
let conditions = HashMap::new();
let claims = HashMap::new();
let action = Action::KmsAction(KmsAction::DisableKeyAction);
for key_id in ["", "key-a"] {
assert!(
policy.is_allowed(&kms_args(action, key_id, &conditions, &claims)).await,
"malformed KMS statement with S3 resources must keep matching every key (key_id: {key_id:?})"
);
}
Ok(())
}
#[tokio::test]
async fn test_kms_alias_resource_parses_but_matches_no_key() -> Result<()> {
use crate::policy::action::{Action, KmsAction};
let policy = Policy::parse_config(
br#"{
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"Action": ["kms:DescribeKey"],
"Resource": ["arn:aws:kms:::alias/app-alias"]
}
]
}"#,
)?;
let conditions = HashMap::new();
let claims = HashMap::new();
let action = Action::KmsAction(KmsAction::DescribeKeyAction);
assert!(
!policy.is_allowed(&kms_args(action, "app-alias", &conditions, &claims)).await,
"alias patterns are parse-only until alias resolution lands and must not match key requests"
);
assert!(
policy.is_allowed(&kms_args(action, "", &conditions, &claims)).await,
"call sites without a key resource keep the legacy match-every-key behaviour"
);
Ok(())
}
#[test]
fn test_kms_resource_policy_round_trips() {
let policy = Policy::parse_config(
br#"{
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"Action": ["kms:GenerateDataKey", "kms:Decrypt"],
"Resource": ["arn:aws:kms:::key/app-*", "arn:aws:kms:::alias/app-alias"]
}
]
}"#,
)
.expect("KMS resource policy should parse");
let json = serde_json::to_string(&policy).expect("policy should serialize");
let round_trip = Policy::parse_config(json.as_bytes()).expect("serialized KMS resource policy should re-parse");
assert_eq!(round_trip.statements[0].resources, policy.statements[0].resources);
assert_eq!(round_trip.statements[0].actions, policy.statements[0].actions);
let value: serde_json::Value = serde_json::from_str(&json).expect("JSON valid");
let resources: Vec<_> = value["Statement"][0]["Resource"]
.as_array()
.expect("Resource should serialize as an array")
.iter()
.map(|resource| resource.as_str().expect("Resource entries should be strings"))
.collect();
assert_eq!(resources, vec!["arn:aws:kms:::key/app-*", "arn:aws:kms:::alias/app-alias"]);
}
#[test]
fn test_kms_resource_with_non_kms_action_is_invalid() {
for action in ["s3:GetObject", "admin:ServerInfo", "sts:AssumeRole"] {
let data = format!(
r#"{{
"Version": "2012-10-17",
"Statement": [
{{
"Effect": "Allow",
"Action": ["{action}"],
"Resource": ["arn:aws:kms:::key/key-a"]
}}
]
}}"#
);
let result = Policy::parse_config(data.as_bytes());
assert!(
matches!(result.as_ref().unwrap_err(), Error::PolicyError(IamError::KmsResourceWithNonKmsAction)),
"{action} with a KMS resource should fail with KmsResourceWithNonKmsAction, got: {result:?}"
);
}
}
#[test]
fn test_bucket_policy_with_kms_statement_is_invalid() {
for statement in [
r#"{"Effect":"Allow","Principal":{"AWS":"*"},"Action":["kms:GenerateDataKey"],"Resource":["arn:aws:s3:::bucket/*"]}"#,
r#"{"Effect":"Allow","Principal":{"AWS":"*"},"Action":["s3:GetObject"],"Resource":["arn:aws:kms:::key/key-a"]}"#,
r#"{"Effect":"Allow","Principal":{"AWS":"*"},"NotAction":["kms:*"],"Resource":["arn:aws:s3:::bucket/*"]}"#,
] {
let data = format!(r#"{{"Version":"2012-10-17","Statement":[{statement}]}}"#);
let policy: BucketPolicy =
serde_json::from_str(&data).expect("bucket policy with KMS content should still deserialize");
let result = policy.is_valid();
assert!(
matches!(result.as_ref().unwrap_err(), Error::PolicyError(IamError::KmsUnsupportedInBucketPolicy)),
"bucket policy statement {statement} should fail with KmsUnsupportedInBucketPolicy, got: {result:?}"
);
}
}
#[tokio::test]
async fn test_stored_bucket_policy_with_kms_statement_loads_and_is_ignored() -> Result<()> {
// Policies stored before KMS statements were rejected at validation must keep
// deserializing, and their KMS statements must not affect bucket traffic.
let bucket_policy: BucketPolicy = serde_json::from_str(
r#"{
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"Principal": {"AWS": "*"},
"Action": ["kms:GenerateDataKey"],
"Resource": ["arn:aws:s3:::bucket/*"]
},
{
"Effect": "Allow",
"Principal": {"AWS": "*"},
"Action": ["s3:GetObject"],
"Resource": ["arn:aws:s3:::bucket/*"]
}
]
}"#,
)?;
let conditions = HashMap::new();
let args = BucketPolicyArgs {
account: "testuser",
groups: &None,
action: Action::S3Action(crate::policy::action::S3Action::GetObjectAction),
bucket: "bucket",
conditions: &conditions,
is_owner: false,
object: "a.txt",
};
assert!(
bucket_policy.is_allowed(&args).await,
"the S3 statement must keep working alongside an ignored KMS statement"
);
let put_args = BucketPolicyArgs {
account: "testuser",
groups: &None,
action: Action::S3Action(crate::policy::action::S3Action::PutObjectAction),
bucket: "bucket",
conditions: &conditions,
is_owner: false,
object: "a.txt",
};
assert!(
!bucket_policy.is_allowed(&put_args).await,
"the ignored KMS statement must not grant anything"
);
Ok(())
}
#[test]
fn test_mixed_action_families_are_invalid_even_with_resource() {
let data = r#"
+93 -10
View File
@@ -40,7 +40,7 @@ impl Serialize for ResourceSet {
for resource in &self.0 {
let resource_str = match resource {
Resource::S3(value) => format!("{}{}", Resource::S3_PREFIX, value),
Resource::Kms(value) => value.clone(),
Resource::Kms(value) => format!("{}{}", Resource::KMS_PREFIX, value),
};
seq.serialize_element(&resource_str)?;
}
@@ -171,6 +171,20 @@ pub enum Resource {
impl Resource {
pub const S3_PREFIX: &'static str = "arn:aws:s3:::";
/// KMS ARNs use the same empty-account form as [`Self::S3_PREFIX`]; the suffix is
/// `key/<key_id>` (wildcards allowed in the id), `alias/<name>`, or a bare `*`.
pub const KMS_PREFIX: &'static str = "arn:aws:kms:::";
/// Resource-type segment for key ids; request-side KMS resource strings are
/// `key/<key_id>` so they line up with these patterns.
pub const KMS_KEY_SEGMENT: &'static str = "key/";
/// Resource-type segment reserved for key aliases. Alias patterns parse and
/// validate, but requests are always evaluated against `key/<key_id>` strings,
/// so an alias pattern matches nothing until alias resolution lands.
pub const KMS_ALIAS_SEGMENT: &'static str = "alias/";
pub fn is_kms(&self) -> bool {
matches!(self, Resource::Kms(_))
}
pub async fn is_match(&self, resource: &str, conditions: &HashMap<String, Vec<String>>) -> bool {
self.is_match_with_resolver(resource, conditions, None).await
@@ -228,10 +242,13 @@ impl Resource {
impl TryFrom<&str> for Resource {
type Error = Error;
fn try_from(value: &str) -> std::result::Result<Self, Self::Error> {
let Some(value) = value.strip_prefix(Self::S3_PREFIX) else {
let resource = if let Some(suffix) = value.strip_prefix(Self::S3_PREFIX) {
Resource::S3(suffix.into())
} else if let Some(suffix) = value.strip_prefix(Self::KMS_PREFIX) {
Resource::Kms(suffix.into())
} else {
return Err(IamError::InvalidResource("unknown".into(), value.into()).into());
};
let resource = Resource::S3(value.into());
resource.is_valid()?;
Ok(resource)
@@ -248,13 +265,17 @@ impl Validator for Resource {
}
}
Self::Kms(pattern) => {
if pattern.is_empty()
// A bare `*` matches every key resource; anything else must carry a
// resource-type segment. Key ids never contain separators (the KMS
// backends reject them), while alias names may nest ("alias/aws/s3").
let well_formed = pattern == "*"
|| pattern
.char_indices()
.find(|&(_, c)| c == '/' || c == '\\' || c == '.')
.map(|(i, _)| i)
.is_some()
{
.strip_prefix(Self::KMS_KEY_SEGMENT)
.is_some_and(|id| !id.is_empty() && !id.contains('/') && !id.contains('\\'))
|| pattern
.strip_prefix(Self::KMS_ALIAS_SEGMENT)
.is_some_and(|name| !name.is_empty() && !name.contains('\\'));
if !well_formed {
return Err(IamError::InvalidResource("kms".into(), pattern.into()).into());
}
}
@@ -270,7 +291,7 @@ impl Serialize for Resource {
{
match self {
Resource::S3(s) => serializer.serialize_str(&format!("{}{}", Self::S3_PREFIX, s)),
Resource::Kms(s) => serializer.serialize_str(s),
Resource::Kms(s) => serializer.serialize_str(&format!("{}{}", Self::KMS_PREFIX, s)),
}
}
}
@@ -308,9 +329,71 @@ mod tests {
#[test_case("arn:aws:s3:::mybucket","mybucket/myobject" => false; "15")]
#[test_case("arn:aws:s3:::attacker-bucket/*","attacker-bucket/../victim-bucket/evil.txt" => false; "16")]
#[test_case("arn:aws:s3:::attacker-bucket/*","attacker-bucket/safe/../../victim-bucket/evil.txt" => false; "17")]
#[test_case("arn:aws:kms:::key/mykey","key/mykey" => true; "kms exact key")]
#[test_case("arn:aws:kms:::key/mykey","key/otherkey" => false; "kms wrong key")]
#[test_case("arn:aws:kms:::key/*","key/mykey" => true; "kms key wildcard")]
#[test_case("arn:aws:kms:::key/app-*","key/app-primary" => true; "kms key prefix wildcard")]
#[test_case("arn:aws:kms:::key/app-*","key/backup-primary" => false; "kms key prefix mismatch")]
#[test_case("arn:aws:kms:::key/mykey?","key/mykey1" => true; "kms key question mark")]
#[test_case("arn:aws:kms:::*","key/mykey" => true; "kms bare star")]
#[test_case("arn:aws:kms:::key/mykey","key/mykey/../otherkey" => false; "kms traversal cleaned")]
#[test_case("arn:aws:kms:::alias/myalias","key/myalias" => false; "kms alias never matches key")]
#[test_case("arn:aws:kms:::alias/*","key/mykey" => false; "kms alias wildcard never matches key")]
#[test_case("arn:aws:kms:::key/mykey","mykey" => false; "kms bare id lacks key segment")]
fn test_resource_is_match(resource: &str, object: &str) -> bool {
let resource: Resource = resource.try_into().unwrap();
pollster::block_on(resource.is_match(object, &HashMap::new()))
}
#[test_case("arn:aws:kms:::key/mykey" => true; "key id parses")]
#[test_case("arn:aws:kms:::key/*" => true; "key wildcard parses")]
#[test_case("arn:aws:kms:::key/app-key.v2" => true; "key id with dot parses")]
#[test_case("arn:aws:kms:::alias/myalias" => true; "alias parses")]
#[test_case("arn:aws:kms:::alias/aws/s3" => true; "nested alias parses")]
#[test_case("arn:aws:kms:::*" => true; "bare star parses")]
#[test_case("arn:aws:kms:::" => false; "empty suffix rejected")]
#[test_case("arn:aws:kms:::key/" => false; "empty key id rejected")]
#[test_case("arn:aws:kms:::alias/" => false; "empty alias name rejected")]
#[test_case("arn:aws:kms:::mykey" => false; "missing resource type segment rejected")]
#[test_case("arn:aws:kms:::key/a/b" => false; "separator in key id rejected")]
#[test_case("arn:aws:kms:::key/a\\b" => false; "backslash in key id rejected")]
#[test_case("arn:aws:kms:us-east-1:123456789012:key/mykey" => false; "region and account form rejected")]
fn test_kms_resource_parse(resource: &str) -> bool {
Resource::try_from(resource).is_ok()
}
#[test]
fn test_kms_resource_serialization_round_trip() {
for raw in [
"arn:aws:kms:::key/mykey",
"arn:aws:kms:::key/*",
"arn:aws:kms:::alias/myalias",
"arn:aws:kms:::*",
] {
let resource = Resource::try_from(raw).expect("KMS resource should parse");
assert!(resource.is_kms());
let json = serde_json::to_string(&resource).expect("KMS resource should serialize");
assert_eq!(json, format!("\"{raw}\""), "serialization must write back the full ARN");
let round_trip: Resource = serde_json::from_str(&json).expect("serialized KMS resource should deserialize");
assert_eq!(round_trip, resource);
}
}
#[test]
fn test_kms_resource_set_serialization_round_trip() {
use crate::policy::resource::ResourceSet;
let set: ResourceSet =
serde_json::from_str(r#"["arn:aws:kms:::key/app-*","arn:aws:kms:::alias/myalias"]"#).expect("set should parse");
assert_eq!(set.len(), 2);
let json = serde_json::to_string(&set).expect("set should serialize");
assert_eq!(json, r#"["arn:aws:kms:::key/app-*","arn:aws:kms:::alias/myalias"]"#);
let round_trip: ResourceSet = serde_json::from_str(&json).expect("serialized set should deserialize");
assert_eq!(round_trip, set);
}
}
+111 -5
View File
@@ -16,6 +16,7 @@ use super::{
ActionSet, Args, BucketPolicyArgs, Effect, Error as IamError, Functions, ID, Principal, ResourceSet, Validator,
action::{Action, S3Action},
function::key_name::{KeyName, S3KeyName},
resource::Resource,
variables::{VariableContext, VariableResolver},
};
use crate::error::{Error, Result};
@@ -173,13 +174,79 @@ impl Statement {
Some(ActionFamily::Mixed)
}
/// Resource scope check for KMS statements, which match `arn:aws:kms:::key/<key_id>`
/// patterns instead of the bucket/object path grammar used by S3 statements.
///
/// Call-site contract (wired up by the admin/SSE authorization paths): the requested
/// key identifier (pre-alias-resolution) travels in `args.object` with `args.bucket`
/// left empty. An empty `args.object` means the caller did not scope the request to a
/// key, which preserves the legacy match-every-key behaviour.
async fn kms_key_scope_matches(&self, args: &Args<'_>, resolver: &VariableResolver) -> bool {
let kms_resources: Vec<&Resource> = self.resources.iter().filter(|resource| resource.is_kms()).collect();
let kms_not_resources: Vec<&Resource> = self.not_resources.iter().filter(|resource| resource.is_kms()).collect();
if kms_resources.len() != self.resources.len() || kms_not_resources.len() != self.not_resources.len() {
// Statements combining KMS actions with S3 resources predate KMS resource
// support and were always evaluated as if unscoped; keep that behaviour
// but surface it, since the S3 patterns never constrain key access.
tracing::warn!(
sid = %self.sid.0,
"KMS statement carries non-KMS resources; they are ignored and the statement matches every key"
);
}
if kms_resources.is_empty() && kms_not_resources.is_empty() {
// No KMS resources: the statement scopes by action only (legacy form).
return true;
}
if args.object.is_empty() {
// Call sites that do not pass a key resource keep the pre-resource-scoping
// behaviour where any key matches.
return true;
}
let requested = format!("{}{}", Resource::KMS_KEY_SEGMENT, args.object);
if !kms_resources.is_empty() {
let mut matched = false;
for resource in kms_resources {
if resource
.is_match_with_resolver(&requested, args.conditions, Some(resolver))
.await
{
matched = true;
break;
}
}
if !matched {
return false;
}
}
for resource in kms_not_resources {
if resource
.is_match_with_resolver(&requested, args.conditions, Some(resolver))
.await
{
return false;
}
}
true
}
/// Returns true when this statement would reach `conditions.evaluate_with_resolver` in
/// [`Statement::is_allowed`] (including the KMS shortcut path). Does not evaluate conditions.
/// [`Statement::is_allowed`] (including the KMS resource path). Does not evaluate conditions.
pub(crate) async fn request_reaches_condition_eval(&self, args: &Args<'_>, resolver: &VariableResolver) -> bool {
if (!self.actions.is_match(&args.action) && !self.actions.is_empty()) || self.not_actions.is_match(&args.action) {
return false;
}
if self.is_kms() {
return self.kms_key_scope_matches(args, resolver).await;
}
let resource = build_resource(
&args.action,
args.bucket,
@@ -187,10 +254,6 @@ impl Statement {
self.conditions.references_key_name(&KeyName::S3(S3KeyName::S3Prefix)),
);
if self.is_kms() && (resource == "/" || self.resources.is_empty()) {
return true;
}
if self.resources.is_empty() && self.not_resources.is_empty() && !self.is_admin() && !self.is_sts() {
return false;
}
@@ -273,6 +336,19 @@ impl Validator for Statement {
return Err(IamError::BothResourceAndNotResource.into());
}
// KMS resources only make sense on pure-KMS statements. The reverse
// combination (KMS actions with S3 resources) predates KMS resources,
// may already be stored, and stays loadable; evaluation treats it as
// unscoped and warns.
let has_kms_resource = self
.resources
.iter()
.chain(self.not_resources.iter())
.any(|resource| resource.is_kms());
if has_kms_resource && !matches!(action_family, Some(ActionFamily::Kms)) {
return Err(IamError::KmsResourceWithNonKmsAction.into());
}
self.actions.is_valid()?;
self.not_actions.is_valid()?;
self.resources.is_valid()?;
@@ -323,6 +399,18 @@ pub struct BPStatement {
impl BPStatement {
/// Returns true when this statement would reach `conditions.evaluate` in [`BPStatement::is_allowed`].
pub(crate) async fn request_reaches_condition_eval(&self, args: &BucketPolicyArgs<'_>) -> bool {
if !self.actions.is_empty() && self.actions.iter().all(|action| matches!(action, Action::KmsAction(_))) {
// Bucket policies cannot grant or deny KMS access; such statements are
// rejected at validation but may exist in policies stored before that
// check. Skip them so they never influence bucket traffic. Statements
// mixing KMS with S3 actions keep evaluating their S3 actions as before.
tracing::warn!(
sid = %self.sid.0,
"ignoring bucket policy statement with KMS actions during evaluation"
);
return false;
}
if !self.principal.is_match(args.account) {
return false;
}
@@ -379,6 +467,24 @@ impl Validator for BPStatement {
return Err(IamError::BothActionAndNotAction.into());
}
// Bucket policies govern S3 access; KMS grants belong in identity policies.
// Rejected here (PutBucketPolicy) only: deserialization stays permissive so
// stored policies from before this check keep loading, and evaluation skips
// pure-KMS statements with a warning.
let has_kms_action = self
.actions
.iter()
.chain(self.not_actions.iter())
.any(|action| matches!(action, Action::KmsAction(_)));
let has_kms_resource = self
.resources
.iter()
.chain(self.not_resources.iter())
.any(|resource| resource.is_kms());
if has_kms_action || has_kms_resource {
return Err(IamError::KmsUnsupportedInBucketPolicy.into());
}
if self.resources.is_empty() && self.not_resources.is_empty() {
return Err(IamError::NonResource.into());
}
+153
View File
@@ -26,6 +26,12 @@
//! policy is also allowed by the widened policy;
//! (c) an empty policy denies every non-owner request (default deny).
//!
//! Properties (d)-(g) extend the same invariants to KMS key resources
//! (`arn:aws:kms:::key/<key_id>`, backlog#1582): Deny-first over key scopes,
//! wildcard/resource-less supersets implying concrete key grants, exact key
//! scoping without cross-key leaks, and the legacy resource-less
//! match-every-key compatibility pin.
//!
//! Pure evaluation: no IO, no global state, parallel-safe. Statements are built
//! from JSON exactly like production policies arriving via PutPolicy. Generated
//! bucket/key/action pools avoid wildcard metacharacters so resource patterns
@@ -47,10 +53,23 @@ const OBJECT_ACTIONS: &[&str] = &[
"s3:PutObjectTagging",
];
/// Key-scoped KMS actions safe to pair with an `arn:aws:kms:::key/<id>` resource.
const KMS_KEY_ACTIONS: &[&str] = &[
"kms:GenerateDataKey",
"kms:Decrypt",
"kms:DisableKey",
"kms:RotateKey",
"kms:DescribeKey",
];
fn statement_json(effect: &str, action: &str, resource: &str) -> String {
format!(r#"{{"Effect":"{effect}","Action":["{action}"],"Resource":["{resource}"]}}"#)
}
fn resourceless_statement_json(effect: &str, action: &str) -> String {
format!(r#"{{"Effect":"{effect}","Action":["{action}"]}}"#)
}
fn policy_from_statements(statements: &[String]) -> Policy {
let json = format!(r#"{{"Version":"2012-10-17","Statement":[{}]}}"#, statements.join(","));
serde_json::from_str(&json).expect("generated policy JSON should parse")
@@ -88,6 +107,22 @@ fn action_strategy() -> impl Strategy<Value = &'static str> {
proptest::sample::select(OBJECT_ACTIONS)
}
/// Strategy: a KMS key id without wildcard metacharacters or separators.
fn key_id_strategy() -> impl Strategy<Value = String> {
"[a-z][a-z0-9-]{2,11}"
}
/// Strategy: one action name from the key-scoped KMS action pool.
fn kms_action_strategy() -> impl Strategy<Value = &'static str> {
proptest::sample::select(KMS_KEY_ACTIONS)
}
/// KMS evaluation contract: the requested key id travels in `args.object` with an
/// empty bucket (see `Statement::kms_key_scope_matches`).
fn is_allowed_for_key(policy: &Policy, action: &str, key_id: &str) -> bool {
is_allowed(policy, action, "", key_id)
}
proptest! {
/// (a) Deny anywhere wins: a Deny statement matching the request denies it,
/// no matter how many broad Allow statements surround it or at which index
@@ -205,4 +240,122 @@ proptest! {
"Policy::default() must deny {action} on {bucket}/{key}"
);
}
/// (d) KMS Deny anywhere wins: a Deny scoped to the exact key (or `key/*`)
/// denies the request no matter how many broad KMS Allow statements
/// (resource-less or `key/*`-scoped) surround it.
#[test]
fn kms_explicit_deny_anywhere_denies(
key_id in key_id_strategy(),
action in kms_action_strategy(),
allow_count in 0usize..4,
deny_pos_seed in 0usize..16,
broad in proptest::bool::ANY,
wildcard_deny in proptest::bool::ANY,
) {
let mut statements: Vec<String> = (0..allow_count)
.map(|_| {
if broad {
resourceless_statement_json("Allow", "kms:*")
} else {
statement_json("Allow", "kms:*", "arn:aws:kms:::key/*")
}
})
.collect();
let deny_resource = if wildcard_deny {
"arn:aws:kms:::key/*".to_string()
} else {
format!("arn:aws:kms:::key/{key_id}")
};
let deny = statement_json("Deny", action, &deny_resource);
let deny_pos = deny_pos_seed % (statements.len() + 1);
statements.insert(deny_pos, deny);
let policy = policy_from_statements(&statements);
if allow_count > 0 {
let mut allows_only = statements.clone();
allows_only.remove(deny_pos);
let allow_policy = policy_from_statements(&allows_only);
prop_assert!(
is_allowed_for_key(&allow_policy, action, &key_id),
"sanity: the KMS Allow statements alone should permit {action} on key {key_id}"
);
}
prop_assert!(
!is_allowed_for_key(&policy, action, &key_id),
"explicit KMS Deny at index {deny_pos} of {} statements must deny {action} on key {key_id}",
statements.len()
);
}
/// (e) KMS wildcard superset implies the concrete key grant: whatever an
/// exact `key/<id>` Allow permits is also permitted by `key/*`, by a bare
/// `arn:aws:kms:::*`, and by the legacy resource-less statement form.
#[test]
fn kms_wildcard_superset_implies_concrete_match(
key_id in key_id_strategy(),
action in kms_action_strategy(),
) {
let narrow = policy_from_statements(&[statement_json(
"Allow",
action,
&format!("arn:aws:kms:::key/{key_id}"),
)]);
let widened = policy_from_statements(&[statement_json("Allow", "kms:*", "arn:aws:kms:::key/*")]);
let star = policy_from_statements(&[statement_json("Allow", "kms:*", "arn:aws:kms:::*")]);
let resourceless = policy_from_statements(&[resourceless_statement_json("Allow", "kms:*")]);
prop_assert!(is_allowed_for_key(&narrow, action, &key_id), "narrow KMS policy must allow its own grant");
prop_assert!(is_allowed_for_key(&widened, action, &key_id), "kms:* on key/* must imply the concrete grant");
prop_assert!(is_allowed_for_key(&star, action, &key_id), "kms:* on arn:aws:kms:::* must imply the concrete grant");
prop_assert!(
is_allowed_for_key(&resourceless, action, &key_id),
"the legacy resource-less KMS statement must imply the concrete grant"
);
}
/// (f) Key scoping is exact: an Allow on `key/<a>` never leaks to a
/// different key id, while the compatibility contract keeps unscoped
/// requests (no key id passed) matching.
#[test]
fn kms_key_scope_does_not_leak_across_keys(
key_a in key_id_strategy(),
key_b in key_id_strategy(),
action in kms_action_strategy(),
) {
prop_assume!(key_a != key_b);
let policy = policy_from_statements(&[statement_json(
"Allow",
action,
&format!("arn:aws:kms:::key/{key_a}"),
)]);
prop_assert!(is_allowed_for_key(&policy, action, &key_a), "the granted key must be allowed");
prop_assert!(
!is_allowed_for_key(&policy, action, &key_b),
"an Allow scoped to key {key_a} must not leak to key {key_b}"
);
prop_assert!(
is_allowed_for_key(&policy, action, ""),
"call sites that pass no key resource keep the legacy match-every-key behaviour"
);
}
/// (g) Legacy compatibility pin: resource-less KMS statements match every
/// generated key id, exactly as before KMS resources existed.
#[test]
fn kms_resourceless_statement_matches_every_key(
key_id in key_id_strategy(),
action in kms_action_strategy(),
) {
let policy = policy_from_statements(&[resourceless_statement_json("Allow", action)]);
prop_assert!(
is_allowed_for_key(&policy, action, &key_id),
"resource-less KMS statement must keep matching {action} on key {key_id}"
);
}
}
@@ -15,13 +15,15 @@
use crate::admin::handlers::supervise_admin_mutation;
use crate::admin::handlers::target_descriptor::AdminTargetSpec;
use crate::admin::runtime_sources::{AppContext, current_app_context, current_object_store_handle_for_context};
use crate::admin::service::config::with_runtime_config_reload_lock;
use crate::admin::storage_api::config::{
read_admin_config_without_migrate, read_admin_server_config_snapshot, save_admin_server_config_snapshot,
read_admin_config_without_migrate, read_admin_server_config_snapshot, read_existing_admin_server_config_no_lock,
save_admin_server_config_snapshot, with_admin_server_config_read_lock,
};
use rustfs_audit::{audit_system, start_audit_system as start_global_audit_system, system::AuditSystemState};
use rustfs_config::DEFAULT_DELIMITER;
use rustfs_config::server_config::Config;
use s3s::{S3Result, s3_error};
use s3s::{S3Error, S3Result, s3_error};
use tracing::warn;
pub(crate) async fn load_server_config_from_store_for_context(context: Option<&AppContext>) -> S3Result<Config> {
@@ -48,6 +50,14 @@ fn has_any_audit_targets(specs: &[AdminTargetSpec], config: &Config) -> bool {
})
}
fn audit_config_convergence_error(persisted: bool, error: impl std::fmt::Display) -> S3Error {
if persisted {
s3_error!(InternalError, "audit config persisted but runtime convergence failed: {}", error)
} else {
s3_error!(InternalError, "audit config unchanged but runtime convergence failed: {}", error)
}
}
pub(crate) async fn apply_audit_runtime_config(specs: &[AdminTargetSpec], config: Config) -> S3Result<()> {
let has_targets = has_any_audit_targets(specs, &config);
@@ -107,15 +117,23 @@ where
return Ok(());
}
save_admin_server_config_snapshot(store, &config, &snapshot)
let persisted = save_admin_server_config_snapshot(store.clone(), &config, &snapshot)
.await
.map(|_| ())
.map_err(|e| s3_error!(InternalError, "failed to save audit config: {}", e))?;
.map_err(|e| s3_error!(InternalError, "failed to save audit config: {}", e))?
.persisted();
drop(snapshot);
// Keep persistence and runtime publication in one detached, serialized
// mutation. Otherwise a cancelled caller or two concurrent updates can
// leave the persisted config and active audit generation disagreeing.
apply_audit_runtime_config(&specs, config).await
let read_store = store.clone();
with_runtime_config_reload_lock(async move {
let latest = with_admin_server_config_read_lock(store, move || read_existing_admin_server_config_no_lock(read_store))
.await
.map_err(|e| s3_error!(InternalError, "failed to lock server config for audit reload: {}", e))?
.map_err(|e| s3_error!(InternalError, "failed to read latest server config for audit reload: {}", e))?;
apply_audit_runtime_config(&specs, latest).await
})
.await
.map_err(|e| audit_config_convergence_error(persisted, e))
})
.await
}
@@ -164,3 +182,154 @@ pub(crate) async fn remove_audit_target_config(specs: &[AdminTargetSpec], subsys
})
.await
}
#[cfg(test)]
mod tests {
use super::*;
use crate::admin::handlers::target_descriptor::admin_target_spec_from_builtin;
use crate::admin::runtime_sources::{IamInterface, KmsInterface};
use crate::admin::storage_api::config::save_admin_server_config;
use rustfs_config::audit::AUDIT_WEBHOOK_SUB_SYS;
use rustfs_config::server_config::KVS;
use rustfs_config::{ENABLE_KEY, EnableState, SCANNER_CYCLE, SCANNER_SUB_SYS, WEBHOOK_ENDPOINT, WEBHOOK_QUEUE_DIR};
use rustfs_iam::{store::object::ObjectStore, sys::IamSys};
use rustfs_kms::KmsServiceManager;
use rustfs_targets::catalog::builtin::builtin_audit_target_admin_descriptors;
use std::sync::Arc;
use std::time::Duration;
use tempfile::TempDir;
struct TestIam;
impl IamInterface for TestIam {
fn handle(&self) -> Arc<IamSys<ObjectStore>> {
unreachable!("audit config tests do not use IAM")
}
fn is_ready(&self) -> bool {
false
}
}
struct TestKms;
impl KmsInterface for TestKms {
fn handle(&self) -> Arc<KmsServiceManager> {
Arc::new(KmsServiceManager::new())
}
}
fn audit_specs() -> Vec<AdminTargetSpec> {
builtin_audit_target_admin_descriptors()
.into_iter()
.map(|descriptor| admin_target_spec_from_builtin(&descriptor))
.collect()
}
async fn wait_for_persisted_target(store: Arc<crate::admin::storage_api::runtime::ECStore>, subsystem: &str, target: &str) {
tokio::time::timeout(Duration::from_secs(30), async {
let mut poll = tokio::time::interval(Duration::from_millis(10));
loop {
poll.tick().await;
let config = read_admin_config_without_migrate(store.clone())
.await
.expect("read persisted server config");
if config.0.get(subsystem).is_some_and(|targets| targets.contains_key(target)) {
return;
}
}
})
.await
.expect("config mutation should become durable");
}
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
#[serial_test::serial]
async fn audit_reload_reads_latest_durable_config_after_releasing_write_snapshot() {
let temp_dir = TempDir::new().expect("audit config temp dir");
let env = rustfs_test_utils::TestECStoreEnv::builder()
.base_dir(temp_dir.path())
.disk_count(1)
.init_bucket_metadata(false)
.build()
.await;
save_admin_server_config(env.ecstore.clone(), &Config::new())
.await
.expect("persist baseline server config");
let context = Arc::new(AppContext::new(env.ecstore.clone(), Arc::new(TestIam), Arc::new(TestKms)));
let (locked_tx, locked_rx) = tokio::sync::oneshot::channel();
let (release_tx, release_rx) = tokio::sync::oneshot::channel();
let blocker = tokio::spawn(async move {
with_runtime_config_reload_lock(async move {
locked_tx.send(()).expect("signal runtime reload lock acquisition");
release_rx.await.expect("release runtime reload lock");
Ok(())
})
.await
.expect("runtime reload lock blocker");
});
locked_rx.await.expect("runtime reload lock should be held");
let older_context = context.clone();
let older = tokio::spawn(async move {
let specs = audit_specs();
update_audit_config_and_reload_for_context(Some(older_context.as_ref()), &specs, |config| {
config
.0
.entry(SCANNER_SUB_SYS.to_string())
.or_default()
.entry(DEFAULT_DELIMITER.to_string())
.or_insert_with(KVS::new)
.insert(SCANNER_CYCLE.to_string(), "15s".to_string());
true
})
.await
});
wait_for_persisted_target(env.ecstore.clone(), SCANNER_SUB_SYS, DEFAULT_DELIMITER).await;
assert!(!older.is_finished(), "audit runtime publication must wait for the shared reload lock");
let snapshot = read_admin_server_config_snapshot(env.ecstore.clone())
.await
.expect("read newer server config snapshot");
let mut latest = snapshot.config.clone();
let mut latest_target = KVS::new();
latest_target.insert(ENABLE_KEY.to_string(), EnableState::On.to_string());
latest_target.insert(WEBHOOK_ENDPOINT.to_string(), "https://audit.invalid/hook".to_string());
latest_target.insert(
WEBHOOK_QUEUE_DIR.to_string(),
temp_dir.path().join("audit-queue").to_string_lossy().into_owned(),
);
latest
.0
.entry(AUDIT_WEBHOOK_SUB_SYS.to_string())
.or_default()
.insert("latest".to_string(), latest_target);
save_admin_server_config_snapshot(env.ecstore.clone(), &latest, &snapshot)
.await
.expect("persist newer audit config");
drop(snapshot);
release_tx.send(()).expect("release runtime reload blocker");
blocker.await.expect("runtime reload blocker task");
tokio::time::timeout(Duration::from_secs(30), older)
.await
.expect("older audit update should complete")
.expect("older audit update task should not panic")
.expect("older audit update should converge from the latest durable config");
let system = audit_system().expect("latest durable audit target should start the audit system");
let targets = system.list_targets().await;
assert!(targets.iter().any(|target| target.contains("latest")), "active targets: {targets:?}");
system.close().await.expect("audit system should stop after the test");
}
#[test]
fn audit_convergence_error_reports_durable_write_state() {
for (persisted, expected) in [(true, "audit config persisted"), (false, "audit config unchanged")] {
let error = audit_config_convergence_error(persisted, "injected failure");
assert!(error.to_string().contains(expected), "unexpected convergence error: {error}");
assert!(error.to_string().contains("injected failure"));
}
}
}
+139 -204
View File
@@ -17,18 +17,17 @@ use crate::admin::handlers::supervise_admin_mutation;
use crate::admin::router::{AdminOperation, Operation, S3Router};
use crate::admin::runtime_sources::{
current_action_credentials, current_app_context, current_object_store_handle_for_context, current_server_config_for_context,
publish_server_config,
};
use crate::admin::service::config::{
CONFIG_WORKER_RELOAD_FAILURE_STATE, EVENT_CONFIG_WORKER_RELOAD_FAILED, FULL_CONFIG_WORKER_SUBSYSTEMS, LOG_COMPONENT_ADMIN,
LOG_SUBSYSTEM_CONFIG, PreparedRuntimeConfig, is_dynamic_config_subsystem, preflight_dynamic_config_reload,
prepare_server_config, reload_dynamic_config_runtime_state, reload_runtime_config_snapshot, signal_config_snapshot_reload,
signal_config_snapshot_reload_checked, signal_dynamic_config_reload_checked,
FULL_CONFIG_WORKER_SUBSYSTEMS, is_dynamic_config_subsystem, preflight_dynamic_config_reload, prepare_server_config,
publish_latest_runtime_config_snapshot, reload_dynamic_config_runtime_state, reload_runtime_config_snapshot,
signal_config_snapshot_reload, signal_config_snapshot_reload_checked, signal_dynamic_config_reload_checked,
};
use crate::admin::storage_api::config::storageclass::{INLINE_BLOCK_ENV, OPTIMIZE_ENV, RRS_ENV, STANDARD_ENV};
use crate::admin::storage_api::config::{
AdminServerConfigSnapshot, RUSTFS_META_BUCKET, STORAGE_CLASS_SUB_SYS, delete_admin_config, read_admin_config,
read_admin_config_without_migrate, read_admin_server_config_snapshot, save_admin_config, save_admin_server_config_snapshot,
AdminServerConfigSaveResult, AdminServerConfigSnapshot, RUSTFS_META_BUCKET, STORAGE_CLASS_SUB_SYS, delete_admin_config,
read_admin_config, read_admin_config_without_migrate, read_admin_server_config_snapshot, save_admin_config,
save_admin_server_config_snapshot,
};
use crate::admin::storage_api::contract::list::ListOperations as _;
use crate::admin::utils::{encode_compatible_admin_payload, is_compat_admin_request, read_compatible_admin_body};
@@ -766,7 +765,10 @@ async fn load_server_config_snapshot_from_store() -> S3Result<AdminServerConfigS
.map_err(Into::into)
}
async fn save_server_config_to_store(config: &ServerConfig, snapshot: &AdminServerConfigSnapshot) -> S3Result<bool> {
async fn save_server_config_to_store(
config: &ServerConfig,
snapshot: &AdminServerConfigSnapshot,
) -> S3Result<AdminServerConfigSaveResult> {
let store = object_store()?;
save_admin_server_config_snapshot(store, config, snapshot)
.await
@@ -987,8 +989,8 @@ fn decode_config_history_snapshot(data: &[u8]) -> S3Result<ServerConfig> {
Ok(config)
}
fn validate_restore_rollback_generation(current: &ServerConfig, restored: &ServerConfig) -> S3Result<()> {
if current == restored {
fn validate_restore_rollback_generation(current: Option<Uuid>, committed: Option<Uuid>) -> S3Result<()> {
if current.is_some() && current == committed {
Ok(())
} else {
Err(s3_error!(
@@ -1004,12 +1006,12 @@ async fn save_server_config_history_snapshot(config: &ServerConfig) -> S3Result<
save_server_config_history(&sealed).await
}
async fn cleanup_failed_config_history_snapshot(restore_id: &str) {
async fn cleanup_config_history_snapshot(restore_id: &str) {
if let Err(err) = delete_server_config_history(restore_id).await {
warn!(
restore_id,
error = %err,
"Failed to remove config history snapshot for an uncommitted mutation"
"Failed to remove config history snapshot"
);
}
}
@@ -1843,23 +1845,6 @@ async fn reject_lost_config_transaction<T>(snapshot: AdminServerConfigSnapshot,
))
}
fn publish_prepared_config_snapshots(config: ServerConfig, prepared: PreparedRuntimeConfig) -> S3Result<()> {
prepared.publish_storage_class()?;
publish_server_config(config);
Ok(())
}
/// Re-apply local mutable worker families after a full-config replacement.
/// Peers receive one full-snapshot signal after this returns; signaling each
/// family here as well would recreate audit/scanner targets twice per peer.
fn publish_notify_config_intent(
config: &ServerConfig,
sub_system: Option<&str>,
) -> Option<rustfs_notify::NotificationLifecycleTransition> {
(sub_system.is_none() || sub_system.is_some_and(|sub_system| NOTIFY_SUB_SYSTEMS.contains(&sub_system)))
.then(|| rustfs_notify::ensure_live_events().publish_config(config.clone()))
}
fn config_preflight_subsystems(sub_system: Option<&str>) -> Vec<&str> {
if let Some(sub_system) = sub_system {
return is_dynamic_config_subsystem(sub_system)
@@ -1884,78 +1869,26 @@ async fn preflight_config_intent(sub_system: Option<&str>) -> S3Result<()> {
Ok(())
}
async fn wait_notify_config_intent(transition: Option<rustfs_notify::NotificationLifecycleTransition>) -> S3Result<bool> {
let Some(transition) = transition else {
return Ok(false);
};
transition.wait().await.map_err(|err| {
warn!(error = %err, "Failed to apply local notification config");
s3_error!(InternalError, "failed to apply notification config")
})?;
Ok(true)
}
async fn reload_non_notify_dynamic_subsystems() -> Vec<String> {
let mut failures = Vec::new();
for sub_system in FULL_CONFIG_WORKER_SUBSYSTEMS {
if NOTIFY_SUB_SYSTEMS.contains(&sub_system) {
continue;
}
if reload_dynamic_config_runtime_state(sub_system).await.is_err() {
failures.push(format!("local {sub_system}"));
warn!(
event = EVENT_CONFIG_WORKER_RELOAD_FAILED,
component = LOG_COMPONENT_ADMIN,
subsystem = LOG_SUBSYSTEM_CONFIG,
config_subsystem = sub_system,
state = CONFIG_WORKER_RELOAD_FAILURE_STATE,
reason = "apply_failed",
"Published server config but failed to reload a local worker subsystem"
);
}
}
failures
}
fn finish_config_reconciliation(errors: Vec<String>) -> S3Result<()> {
fn finish_config_reconciliation(errors: Vec<String>, persisted: bool) -> S3Result<()> {
if errors.is_empty() {
Ok(())
} else {
} else if persisted {
Err(s3_error!(
InternalError,
"server config persisted but runtime convergence failed: {}",
errors.join("; ")
))
} else {
Err(s3_error!(
InternalError,
"server config was unchanged but runtime convergence failed: {}",
errors.join("; ")
))
}
}
async fn reconcile_targeted_config(
sub_system: Option<String>,
storage_class_applied: bool,
notify_transition: Option<rustfs_notify::NotificationLifecycleTransition>,
) -> S3Result<bool> {
let mut errors = Vec::new();
let notify_applied = notify_transition.is_some();
if let Err(err) = wait_notify_config_intent(notify_transition).await {
warn!(error = %err, "Local notification config failed to converge");
errors.push("local notify".to_string());
}
let config_applied = if notify_applied {
if let Some(sub_system) = sub_system.as_deref()
&& let Err(err) = signal_dynamic_config_reload_checked(sub_system).await
{
warn!(config_subsystem = sub_system, error = %err, "Peer config reload failed");
errors.push(format!("peer {sub_system}"));
}
true
} else if storage_class_applied {
if let Err(err) = signal_dynamic_config_reload_checked(STORAGE_CLASS_SUB_SYS).await {
warn!(error = %err, "Peer storage-class reload failed");
errors.push(format!("peer {STORAGE_CLASS_SUB_SYS}"));
}
true
} else if let Some(sub_system) = sub_system.as_deref()
async fn reconcile_targeted_config(sub_system: Option<String>, persisted: bool, mut errors: Vec<String>) -> S3Result<bool> {
let config_applied = if let Some(sub_system) = sub_system.as_deref()
&& is_dynamic_config_subsystem(sub_system)
{
let config_applied = match reload_dynamic_config_runtime_state(sub_system).await {
@@ -1979,21 +1912,19 @@ async fn reconcile_targeted_config(
false
};
finish_config_reconciliation(errors)?;
finish_config_reconciliation(errors, persisted)?;
Ok(config_applied)
}
async fn reconcile_full_config(notify_transition: Option<rustfs_notify::NotificationLifecycleTransition>) -> S3Result<()> {
let mut errors = Vec::new();
if let Err(err) = wait_notify_config_intent(notify_transition).await {
warn!(error = %err, "Local notification config failed to converge");
async fn reconcile_full_config(persisted: bool, mut errors: Vec<String>) -> S3Result<()> {
if let Err(err) = reload_dynamic_config_runtime_state(NOTIFY_WEBHOOK_SUB_SYS).await {
warn!(error = %err, "Local notification config failed to converge from durable config");
errors.push("local notify".to_string());
}
if let Err(err) = signal_dynamic_config_reload_checked(STORAGE_CLASS_SUB_SYS).await {
warn!(error = %err, "Peer storage-class reload failed");
errors.push(format!("peer {STORAGE_CLASS_SUB_SYS}"));
}
errors.extend(reload_non_notify_dynamic_subsystems().await);
if let Err(err) = signal_dynamic_config_reload_checked(NOTIFY_WEBHOOK_SUB_SYS).await {
warn!(error = %err, "Peer notification config reload failed");
errors.push("peer notify".to_string());
@@ -2002,91 +1933,82 @@ async fn reconcile_full_config(notify_transition: Option<rustfs_notify::Notifica
warn!(error = %err, "Peer config snapshot reload failed");
errors.push("peer config snapshot".to_string());
}
finish_config_reconciliation(errors)
finish_config_reconciliation(errors, persisted)
}
struct PersistedConfigTransaction {
previous_config: ServerConfig,
history_restore_id: Option<String>,
storage_class_applied: bool,
notify_transition: Option<rustfs_notify::NotificationLifecycleTransition>,
persisted: bool,
committed_generation: Option<Uuid>,
}
async fn persist_server_config_transaction(
config: ServerConfig,
prepared: PreparedRuntimeConfig,
snapshot: AdminServerConfigSnapshot,
sub_system: Option<&str>,
) -> S3Result<PersistedConfigTransaction> {
snapshot.ensure_lock_held().map_err(ApiError::from).map_err(S3Error::from)?;
let previous_config = snapshot.config.clone();
let history_restore_id = save_server_config_history_snapshot(&previous_config).await?;
if snapshot.is_lock_lost() {
cleanup_failed_config_history_snapshot(&history_restore_id).await;
cleanup_config_history_snapshot(&history_restore_id).await;
return reject_lost_config_transaction(snapshot, "before persistence").await;
}
let persisted = match save_server_config_to_store(&config, &snapshot).await {
Ok(persisted) => persisted,
let save_result = match save_server_config_to_store(&config, &snapshot).await {
Ok(result) => result,
Err(err) => {
cleanup_failed_config_history_snapshot(&history_restore_id).await;
cleanup_config_history_snapshot(&history_restore_id).await;
return Err(err);
}
};
let persisted = save_result.persisted();
let committed_generation = save_result.generation();
let history_restore_id = if persisted {
Some(history_restore_id)
} else {
cleanup_failed_config_history_snapshot(&history_restore_id).await;
cleanup_config_history_snapshot(&history_restore_id).await;
None
};
if snapshot.is_lock_lost() {
return reject_lost_config_transaction(snapshot, "after persistence").await;
}
let storage_class_applied = sub_system.is_none_or(|value| value == STORAGE_CLASS_SUB_SYS);
if storage_class_applied {
if let Err(err) = publish_prepared_config_snapshots(config.clone(), prepared) {
return Err(match history_restore_id.as_deref() {
Some(restore_id) => s3_error!(
InternalError,
"config persisted but runtime publish failed; recovery snapshot restoreId={}: {}",
restore_id,
err
),
None => err,
});
}
} else {
publish_server_config(config.clone());
}
let notify_transition = publish_notify_config_intent(&config, sub_system);
if snapshot.is_lock_lost() {
return reject_lost_config_transaction(snapshot, "while publishing runtime snapshots").await;
}
drop(snapshot);
Ok(PersistedConfigTransaction {
previous_config,
history_restore_id,
storage_class_applied,
notify_transition,
persisted,
committed_generation,
})
}
async fn reconcile_committed_config(sub_system: Option<String>, persisted: bool) -> S3Result<bool> {
let mut errors = Vec::new();
let publication_result = match sub_system.as_deref() {
Some(sub_system) => publish_latest_runtime_config_snapshot(sub_system).await,
None => reload_runtime_config_snapshot().await,
};
if let Err(err) = publication_result {
warn!(error = %err, "Failed to publish the latest durable server config");
errors.push("local config snapshot".to_string());
}
if sub_system.is_none() {
reconcile_full_config(persisted, errors).await?;
return Ok(false);
}
reconcile_targeted_config(sub_system, persisted, errors).await
}
async fn commit_server_config_transaction(
config: ServerConfig,
prepared: PreparedRuntimeConfig,
snapshot: AdminServerConfigSnapshot,
sub_system: Option<String>,
) -> S3Result<bool> {
let transaction = persist_server_config_transaction(config, prepared, snapshot, sub_system.as_deref()).await?;
if sub_system.is_none() {
reconcile_full_config(transaction.notify_transition).await?;
return Ok(false);
let transaction = persist_server_config_transaction(config, snapshot).await?;
let result = reconcile_committed_config(sub_system, transaction.persisted).await;
match (result, transaction.history_restore_id.as_deref()) {
(Err(err), Some(restore_id)) => Err(s3_error!(InternalError, "{}; recovery snapshot restoreId={}", err, restore_id)),
(result, _) => result,
}
reconcile_targeted_config(sub_system, transaction.storage_class_applied, transaction.notify_transition).await
}
pub struct GetConfigKVHandler {}
@@ -2127,8 +2049,8 @@ impl Operation for SetConfigKVHandler {
let snapshot = load_server_config_snapshot_from_store().await?;
let mut config = snapshot.config.clone();
apply_set_directives(&mut config, &directives)?;
let prepared = prepare_server_config(&config, sub_system.as_deref()).await?;
commit_server_config_transaction(config, prepared, snapshot, sub_system).await
prepare_server_config(&config, sub_system.as_deref()).await?;
commit_server_config_transaction(config, snapshot, sub_system).await
})
.await?;
@@ -2155,8 +2077,8 @@ impl Operation for DelConfigKVHandler {
let snapshot = load_server_config_snapshot_from_store().await?;
let mut config = snapshot.config.clone();
apply_delete_directives(&mut config, &directives);
let prepared = prepare_server_config(&config, sub_system.as_deref()).await?;
commit_server_config_transaction(config, prepared, snapshot, sub_system).await
prepare_server_config(&config, sub_system.as_deref()).await?;
commit_server_config_transaction(config, snapshot, sub_system).await
})
.await?;
@@ -2239,15 +2161,19 @@ impl Operation for RestoreConfigHistoryKVHandler {
supervise_admin_mutation("config mutation", async move {
preflight_config_intent(None).await?;
let snapshot = load_server_config_snapshot_from_store().await?;
let prepared = prepare_server_config(&config, None).await?;
let restored_config = config.clone();
let transaction = persist_server_config_transaction(config, prepared, snapshot, None).await?;
let Err(restore_error) = reconcile_full_config(transaction.notify_transition).await else {
prepare_server_config(&config, None).await?;
let transaction = persist_server_config_transaction(config, snapshot).await?;
let Err(restore_error) = reconcile_committed_config(None, transaction.persisted).await else {
return Ok(());
};
if !transaction.persisted {
return Err(restore_error);
}
let previous_config = transaction.previous_config;
let recovery_restore_id = transaction.history_restore_id;
let committed_generation = transaction.committed_generation;
let recovery_reference = recovery_restore_id.as_deref().unwrap_or("not-created");
let rollback_snapshot = match load_server_config_snapshot_from_store().await {
Ok(snapshot) => snapshot,
@@ -2261,7 +2187,9 @@ impl Operation for RestoreConfigHistoryKVHandler {
));
}
};
if let Err(rollback_error) = validate_restore_rollback_generation(&rollback_snapshot.config, &restored_config) {
if let Err(rollback_error) =
validate_restore_rollback_generation(rollback_snapshot.generation(), committed_generation)
{
return Err(s3_error!(
InternalError,
"config restore failed: {}; automatic rollback failed: {}; recovery snapshot restoreId={}",
@@ -2271,12 +2199,20 @@ impl Operation for RestoreConfigHistoryKVHandler {
));
}
let rollback_prepared = prepare_server_config(&previous_config, None).await?;
rollback_snapshot
if let Err(rollback_error) = prepare_server_config(&previous_config, None).await {
return Err(s3_error!(
InternalError,
"config restore failed: {}; automatic rollback failed: {}; recovery snapshot restoreId={}",
restore_error,
rollback_error,
recovery_reference
));
}
if let Err(rollback_error) = rollback_snapshot
.ensure_lock_held()
.map_err(ApiError::from)
.map_err(S3Error::from)?;
if let Err(rollback_error) = save_server_config_to_store(&previous_config, &rollback_snapshot).await {
.map_err(S3Error::from)
{
return Err(s3_error!(
InternalError,
"config restore failed: {}; automatic rollback failed: {}; recovery snapshot restoreId={}",
@@ -2285,10 +2221,9 @@ impl Operation for RestoreConfigHistoryKVHandler {
recovery_reference
));
}
if rollback_snapshot.is_lock_lost() {
if let Err(rollback_error) =
reject_lost_config_transaction::<()>(rollback_snapshot, "after restore rollback persistence").await
{
let rollback_persisted = match save_server_config_to_store(&previous_config, &rollback_snapshot).await {
Ok(result) => result.persisted(),
Err(rollback_error) => {
return Err(s3_error!(
InternalError,
"config restore failed: {}; automatic rollback failed: {}; recovery snapshot restoreId={}",
@@ -2297,46 +2232,20 @@ impl Operation for RestoreConfigHistoryKVHandler {
recovery_reference
));
}
return Err(s3_error!(InternalError, "restore rollback lock-loss handling returned unexpectedly"));
}
if let Err(rollback_error) = publish_prepared_config_snapshots(previous_config.clone(), rollback_prepared) {
return Err(s3_error!(
InternalError,
"config restore failed: {}; automatic rollback publish failed: {}; recovery snapshot restoreId={}",
restore_error,
rollback_error,
recovery_reference
));
}
let rollback_transition = publish_notify_config_intent(&previous_config, None);
if rollback_snapshot.is_lock_lost() {
if let Err(rollback_error) =
reject_lost_config_transaction::<()>(rollback_snapshot, "while publishing restore rollback").await
{
return Err(s3_error!(
InternalError,
"config restore failed: {}; automatic rollback failed: {}; recovery snapshot restoreId={}",
restore_error,
rollback_error,
recovery_reference
));
}
return Err(s3_error!(InternalError, "restore rollback lock-loss handling returned unexpectedly"));
}
};
drop(rollback_snapshot);
if let Err(rollback_error) = reconcile_full_config(rollback_transition).await {
if let Err(rollback_error) = reconcile_committed_config(None, rollback_persisted).await {
return Err(s3_error!(
InternalError,
"config restore failed: {}; persisted rollback did not converge: {}; recovery snapshot restoreId={}",
"config restore failed: {}; automatic rollback convergence failed: {}; recovery snapshot restoreId={}",
restore_error,
rollback_error,
recovery_reference
));
}
if let Some(recovery_restore_id) = recovery_restore_id.as_deref() {
cleanup_failed_config_history_snapshot(recovery_restore_id).await;
if rollback_persisted && let Some(recovery_restore_id) = recovery_restore_id.as_deref() {
cleanup_config_history_snapshot(recovery_restore_id).await;
}
Err(s3_error!(InternalError, "config restore failed and was rolled back: {}", restore_error))
})
@@ -2377,8 +2286,8 @@ impl Operation for SetConfigHandler {
let snapshot = load_server_config_snapshot_from_store().await?;
let mut config = ServerConfig::new();
apply_set_directives(&mut config, &directives)?;
let prepared = prepare_server_config(&config, None).await?;
commit_server_config_transaction(config, prepared, snapshot, None).await?;
prepare_server_config(&config, None).await?;
commit_server_config_transaction(config, snapshot, None).await?;
Ok(())
})
.await?;
@@ -2408,6 +2317,23 @@ mod tests {
);
}
#[test]
fn reconciliation_error_reports_whether_config_was_persisted() {
let persisted = finish_config_reconciliation(vec!["local config snapshot".to_string()], true)
.expect_err("committed config convergence failure must be reported");
let unchanged = finish_config_reconciliation(vec!["local config snapshot".to_string()], false)
.expect_err("unchanged config convergence failure must be reported");
assert_eq!(
persisted.message(),
Some("server config persisted but runtime convergence failed: local config snapshot")
);
assert_eq!(
unchanged.message(),
Some("server config was unchanged but runtime convergence failed: local config snapshot")
);
}
#[test]
fn tokenize_config_line_handles_quotes_and_escapes() {
let tokens = tokenize_config_line(r#"identity_openid client_id="console app" client_secret="s3cr\"et" enable=on"#)
@@ -3191,23 +3117,32 @@ notify_webhook:secondary endpoint="https://secondary.example" auth_token="second
}
#[test]
fn restore_rollback_generation_rejects_concurrent_config_change() {
crate::admin::storage_api::config::init_admin_config_defaults();
let restored = ServerConfig::new();
let mut concurrent = restored.clone();
apply_set_directives(
&mut concurrent,
&parse_config_directives(r#"identity_openid client_id="concurrent-client""#, false).expect("parse concurrent"),
)
.expect("apply concurrent");
fn restore_rollback_generation_accepts_committed_write_identity() {
let committed = Uuid::from_u128(1);
validate_restore_rollback_generation(Some(committed), Some(committed)).expect("matching committed generation");
}
validate_restore_rollback_generation(&restored, &restored).expect("unchanged restore generation");
let error = validate_restore_rollback_generation(&concurrent, &restored)
.expect_err("concurrent change must fence automatic rollback");
#[test]
fn restore_rollback_generation_rejects_aba_with_matching_config_content() {
let error = validate_restore_rollback_generation(Some(Uuid::from_u128(2)), Some(Uuid::from_u128(1)))
.expect_err("a later generation must fence automatic rollback even when config content matches");
assert_eq!(error.code(), &S3ErrorCode::InvalidRequest);
assert!(error.to_string().contains("concurrent configuration change"));
}
#[test]
fn restore_rollback_generation_rejects_missing_write_identity() {
for (current, committed) in [
(None, Some(Uuid::from_u128(1))),
(Some(Uuid::from_u128(1)), None),
(None, None),
] {
let error = validate_restore_rollback_generation(current, committed)
.expect_err("missing generation metadata must fence automatic rollback");
assert_eq!(error.code(), &S3ErrorCode::InvalidRequest);
}
}
#[test]
fn legacy_directive_history_is_rejected_without_being_applied_as_a_snapshot() {
let error = decode_config_history_snapshot(b"identity_openid client_id=\"legacy\"")
+369 -77
View File
@@ -16,7 +16,9 @@ use crate::admin::runtime_sources::{
AppContext, current_app_context, current_notification_system_for_context, current_object_store_handle_for_context,
publish_server_config, publish_storage_class_config,
};
use crate::admin::storage_api::config::{STORAGE_CLASS_SUB_SYS, read_admin_config_without_migrate, storageclass};
use crate::admin::storage_api::config::{
STORAGE_CLASS_SUB_SYS, read_existing_admin_server_config_no_lock, storageclass, with_admin_server_config_read_lock,
};
use crate::admin::storage_api::contract::admin::StorageAdminApi;
use crate::admin::storage_api::runtime::ECStore;
use crate::server::{
@@ -43,6 +45,25 @@ use url::Url;
static RUNTIME_CONFIG_RELOAD_MUTEX: AsyncMutex<()> = AsyncMutex::const_new(());
// Runtime publication lock order: reload mutex -> server-config local lock ->
// transaction lock -> config object lock.
pub(crate) async fn with_runtime_config_reload_lock<Fut, T>(operation: Fut) -> S3Result<T>
where
Fut: Future<Output = S3Result<T>> + Send + 'static,
T: Send + 'static,
{
let reload_guard = RUNTIME_CONFIG_RELOAD_MUTEX.lock().await;
tokio::spawn(async move {
let _reload_guard = reload_guard;
operation.await
})
.await
.map_err(|err| {
let outcome = if err.is_cancelled() { "cancelled" } else { "panicked" };
internal_error(format!("runtime config reload task {outcome}"))
})?
}
pub fn is_dynamic_config_subsystem(sub_system: &str) -> bool {
NOTIFY_SUB_SYSTEMS.contains(&sub_system)
|| matches!(
@@ -121,11 +142,6 @@ impl PreparedRuntimeConfig {
fn publish_storage_class_for_context(self, context: Option<&AppContext>) -> S3Result<()> {
self.publish_storage_class_for_context_with(context, publish_storage_class_config)
}
pub(crate) fn publish_storage_class(self) -> S3Result<()> {
let context = current_app_context();
self.publish_storage_class_for_context(context.as_deref())
}
}
fn publish_server_config_for_context(context: Option<&AppContext>, config: ServerConfig) {
@@ -396,7 +412,18 @@ pub async fn apply_dynamic_config_for_subsystem(config: &ServerConfig, sub_syste
}
pub async fn reload_dynamic_config_runtime_state_for_context(context: Option<&AppContext>, sub_system: &str) -> S3Result<()> {
let _reload_guard = RUNTIME_CONFIG_RELOAD_MUTEX.lock().await;
let context = context.cloned();
let sub_system = sub_system.to_owned();
with_runtime_config_reload_lock(async move {
reload_dynamic_config_runtime_state_under_reload_lock_for_context(context.as_ref(), &sub_system).await
})
.await
}
async fn reload_dynamic_config_runtime_state_under_reload_lock_for_context(
context: Option<&AppContext>,
sub_system: &str,
) -> S3Result<()> {
if sub_system == MODULE_SWITCHES_SIGNAL_SUBSYSTEM {
let store = resolve_runtime_config_store_for_context(context)?;
let notify_result = reconcile_event_notifier_from_store(store).await;
@@ -414,18 +441,54 @@ pub async fn reload_dynamic_config_runtime_state_for_context(context: Option<&Ap
}
let store = resolve_runtime_config_store_for_context(context)?;
let config = read_admin_config_without_migrate(store).await.map_err(|err| {
warn!("peer reload_dynamic_config: failed to load server config for {sub_system}: {err}");
internal_error(format!("failed to load server config: {err}"))
})?;
let read_store = store.clone();
let publication_context = context.cloned();
let publication_sub_system = sub_system.to_owned();
let notify_config = with_admin_server_config_read_lock(store, move || async move {
let config = read_existing_admin_server_config_no_lock(read_store).await.map_err(|err| {
warn!("peer reload_dynamic_config: failed to load server config for {publication_sub_system}: {err}");
internal_error(format!("failed to load server config: {err}"))
})?;
let prepared = prepare_server_config_for_context(publication_context.as_ref(), &config, Some(&publication_sub_system))
.await
.map_err(|err| {
if publication_sub_system == STORAGE_CLASS_SUB_SYS {
internal_error(format!("failed to apply storage class config: {err}"))
} else {
err
}
})?;
if matches!(sub_system, SCANNER_SUB_SYS | HEAL_SUB_SYS) {
validate_server_config_for_context(context, &config, Some(sub_system)).await?;
// Scanner cycles refresh from the process-wide server config before
// each pass. Publish the same validated snapshot first so that refresh
// cannot overwrite this peer's dynamic scanner update with stale data.
publish_server_config_for_context(context, config.clone());
}
if publication_sub_system == STORAGE_CLASS_SUB_SYS {
prepared.publish_storage_class_for_context(publication_context.as_ref())?;
return Ok::<Option<ServerConfig>, S3Error>(None);
}
if NOTIFY_SUB_SYSTEMS.contains(&publication_sub_system.as_str()) {
return Ok(Some(config));
}
if matches!(publication_sub_system.as_str(), SCANNER_SUB_SYS | HEAL_SUB_SYS) {
publish_server_config_for_context(publication_context.as_ref(), config.clone());
}
apply_dynamic_config_for_subsystem_for_context(publication_context.as_ref(), &config, &publication_sub_system)
.await
.inspect_err(|_| {
warn!(
config_subsystem = publication_sub_system,
reason = "apply_failed",
"Peer dynamic config apply failed"
);
})?;
Ok(None)
})
.await
.map_err(|err| {
warn!("peer reload_dynamic_config: failed to acquire server config publication fence for {sub_system}: {err}");
internal_error(format!("failed to lock server config: {err}"))
})??;
let Some(config) = notify_config else {
return Ok(());
};
apply_dynamic_config_for_subsystem_for_context(context, &config, sub_system)
.await
.inspect_err(|_| {
@@ -439,23 +502,16 @@ pub async fn reload_dynamic_config_runtime_state(sub_system: &str) -> S3Result<(
reload_dynamic_config_runtime_state_for_context(context.as_deref(), sub_system).await
}
async fn reload_runtime_config_snapshot_with<ReadFuture, Prepare, PrepareFuture, Publish, ApplyWorkers, ApplyWorkersFuture>(
read: ReadFuture,
prepare: Prepare,
publish: Publish,
async fn reload_runtime_config_snapshot_with<PublishFuture, ApplyWorkers, ApplyWorkersFuture>(
publish_snapshot: PublishFuture,
apply_workers: ApplyWorkers,
) -> S3Result<()>
where
ReadFuture: Future<Output = S3Result<ServerConfig>>,
Prepare: FnOnce(ServerConfig) -> PrepareFuture,
PrepareFuture: Future<Output = S3Result<(ServerConfig, PreparedRuntimeConfig)>>,
Publish: FnOnce(&ServerConfig, PreparedRuntimeConfig) -> S3Result<()>,
PublishFuture: Future<Output = S3Result<ServerConfig>>,
ApplyWorkers: FnOnce(ServerConfig) -> ApplyWorkersFuture,
ApplyWorkersFuture: Future<Output = S3Result<()>>,
{
let config = read.await?;
let (config, prepared) = prepare(config).await?;
publish(&config, prepared)?;
let config = publish_snapshot.await?;
// Worker reloads mutate live state and have no rollback contract, so the
// validated snapshots stay published. The RPC still reports convergence
@@ -474,55 +530,105 @@ where
Ok(())
}
pub async fn reload_runtime_config_snapshot_for_context(context: Option<&AppContext>) -> S3Result<()> {
let _reload_guard = RUNTIME_CONFIG_RELOAD_MUTEX.lock().await;
async fn publish_latest_runtime_config_snapshot_under_reload_lock_for_context<ApplyWorkers, ApplyWorkersFuture>(
context: Option<&AppContext>,
sub_system: Option<&str>,
apply_workers: ApplyWorkers,
) -> S3Result<()>
where
ApplyWorkers: FnOnce(ServerConfig) -> ApplyWorkersFuture + Send + 'static,
ApplyWorkersFuture: Future<Output = S3Result<()>> + Send + 'static,
{
let store = resolve_runtime_config_store_for_context(context)?;
let read_store = store.clone();
let publication_context = context.cloned();
let publication_sub_system = sub_system.map(str::to_owned);
reload_runtime_config_snapshot_with(
async move {
read_admin_config_without_migrate(store).await.map_err(|err| {
warn!("peer reload_runtime_config_snapshot: failed to load server config: {err}");
internal_error(format!("failed to load server config: {err}"))
})
},
|config| async move {
let prepared = prepare_server_config_for_context(context, &config, None).await.map_err(|_| {
warn!("peer reload_runtime_config_snapshot: failed to prepare server config");
internal_error("failed to prepare server config")
})?;
Ok((config, prepared))
},
|config, prepared| {
prepared.publish_storage_class_for_context(context)?;
publish_server_config_for_context(context, config.clone());
Ok(())
},
|config| async move {
let mut failures = Vec::new();
for sub_system in FULL_CONFIG_WORKER_SUBSYSTEMS {
if apply_dynamic_config_for_subsystem_for_context(context, &config, sub_system)
.await
.is_err()
with_admin_server_config_read_lock(store, move || async move {
let config = read_existing_admin_server_config_no_lock(read_store).await.map_err(|err| {
warn!("runtime config publication failed to load server config: {err}");
internal_error(format!("failed to load server config: {err}"))
})?;
let prepared =
prepare_server_config_for_context(publication_context.as_ref(), &config, publication_sub_system.as_deref())
.await
.map_err(|err| match publication_sub_system.as_deref() {
None => {
warn!("peer reload_runtime_config_snapshot: failed to prepare server config");
internal_error("failed to prepare server config")
}
Some(STORAGE_CLASS_SUB_SYS) => internal_error(format!("failed to apply storage class config: {err}")),
Some(_) => err,
})?;
reload_runtime_config_snapshot_with(
async move {
if publication_sub_system
.as_deref()
.is_none_or(|sub_system| sub_system == STORAGE_CLASS_SUB_SYS)
{
failures.push(sub_system);
warn!(
event = EVENT_CONFIG_WORKER_RELOAD_FAILED,
component = LOG_COMPONENT_ADMIN,
subsystem = LOG_SUBSYSTEM_CONFIG,
config_subsystem = sub_system,
state = CONFIG_WORKER_RELOAD_FAILURE_STATE,
reason = "apply_failed",
"Peer runtime config snapshot was published but a subsystem worker reload failed"
);
prepared.publish_storage_class_for_context(publication_context.as_ref())?;
}
publish_server_config_for_context(publication_context.as_ref(), config.clone());
Ok(config)
},
apply_workers,
)
.await
})
.await
.map_err(|err| {
warn!("runtime config publication failed to acquire server config publication fence: {err}");
internal_error(format!("failed to lock server config: {err}"))
})?
}
pub(crate) async fn publish_latest_runtime_config_snapshot(sub_system: &str) -> S3Result<()> {
let context = current_app_context();
let sub_system = sub_system.to_owned();
with_runtime_config_reload_lock(async move {
publish_latest_runtime_config_snapshot_under_reload_lock_for_context(context.as_deref(), Some(&sub_system), |_| async {
Ok(())
})
.await
})
.await
}
pub async fn reload_runtime_config_snapshot_for_context(context: Option<&AppContext>) -> S3Result<()> {
let context = context.cloned();
with_runtime_config_reload_lock(async move {
reload_runtime_config_snapshot_under_reload_lock_for_context(context.as_ref()).await
})
.await
}
async fn reload_runtime_config_snapshot_under_reload_lock_for_context(context: Option<&AppContext>) -> S3Result<()> {
let worker_context = context.cloned();
publish_latest_runtime_config_snapshot_under_reload_lock_for_context(context, None, move |config| async move {
let mut failures = Vec::new();
for sub_system in FULL_CONFIG_WORKER_SUBSYSTEMS {
if apply_dynamic_config_for_subsystem_for_context(worker_context.as_ref(), &config, sub_system)
.await
.is_err()
{
failures.push(sub_system);
warn!(
event = EVENT_CONFIG_WORKER_RELOAD_FAILED,
component = LOG_COMPONENT_ADMIN,
subsystem = LOG_SUBSYSTEM_CONFIG,
config_subsystem = sub_system,
state = CONFIG_WORKER_RELOAD_FAILURE_STATE,
reason = "apply_failed",
"Peer runtime config snapshot was published but a subsystem worker reload failed"
);
}
if failures.is_empty() {
Ok(())
} else {
Err(internal_error(format!("runtime worker reload failed: {}", failures.join("; "))))
}
},
)
}
if failures.is_empty() {
Ok(())
} else {
Err(internal_error(format!("runtime worker reload failed: {}", failures.join("; "))))
}
})
.await
}
@@ -699,6 +805,77 @@ mod tests {
}
}
#[tokio::test]
async fn runtime_reload_waiter_cancellation_does_not_leave_queued_work() {
let _blocker = RUNTIME_CONFIG_RELOAD_MUTEX.lock().await;
let (operation_started_tx, mut operation_started_rx) = tokio::sync::oneshot::channel();
let mut reload = Box::pin(with_runtime_config_reload_lock(async move {
let _ = operation_started_tx.send(());
Ok(())
}));
poll_fn(|cx| match reload.as_mut().poll(cx) {
Poll::Pending => Poll::Ready(()),
Poll::Ready(_) => panic!("reload must wait while the mutex is held"),
})
.await;
drop(reload);
assert!(
matches!(operation_started_rx.try_recv(), Err(tokio::sync::oneshot::error::TryRecvError::Closed)),
"cancelling a queued reload must drop its work instead of detaching it"
);
}
#[tokio::test]
async fn runtime_reload_lock_survives_waiter_cancellation() {
let (started_tx, started_rx) = tokio::sync::oneshot::channel();
let (release_tx, release_rx) = tokio::sync::oneshot::channel();
let (completed_tx, completed_rx) = tokio::sync::oneshot::channel();
let waiter = tokio::spawn(async move {
with_runtime_config_reload_lock(async move {
started_tx.send(()).expect("signal runtime reload start");
release_rx.await.expect("release supervised runtime reload");
completed_tx.send(()).expect("signal runtime reload completion");
Ok(())
})
.await
});
started_rx.await.expect("supervised runtime reload started");
waiter.abort();
assert!(waiter.await.expect_err("reload waiter should be cancelled").is_cancelled());
let (contender_polled_tx, contender_polled_rx) = tokio::sync::oneshot::channel();
let (contender_entered_tx, mut contender_entered_rx) = tokio::sync::oneshot::channel();
let contender = tokio::spawn(async move {
let mut lock = Box::pin(RUNTIME_CONFIG_RELOAD_MUTEX.lock());
let mut contender_polled_tx = Some(contender_polled_tx);
let _guard = poll_fn(|cx| {
if let Some(tx) = contender_polled_tx.take() {
let _ = tx.send(());
}
lock.as_mut().poll(cx)
})
.await;
let _ = contender_entered_tx.send(());
});
contender_polled_rx.await.expect("contending reload should be polled");
assert!(
matches!(contender_entered_rx.try_recv(), Err(tokio::sync::oneshot::error::TryRecvError::Empty)),
"a cancelled waiter must not release the reload mutex while its detached reload is still running"
);
release_tx.send(()).expect("release detached runtime reload");
completed_rx.await.expect("detached runtime reload should complete");
contender_entered_rx
.await
.expect("contending reload should enter after completion");
contender.await.expect("contending reload task should not panic");
}
#[tokio::test]
async fn checked_scanner_reload_reports_unreachable_peer() {
let temp_dir = TempDir::new().expect("scanner reload temp dir");
@@ -1221,6 +1398,123 @@ mod tests {
.await;
}
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
#[serial_test::serial(storage_class_env)]
async fn full_reload_keeps_worker_apply_inside_durable_read_fence() {
temp_env::async_with_vars(
[
(storageclass::STANDARD_ENV, None::<&str>),
(storageclass::RRS_ENV, None::<&str>),
(storageclass::OPTIMIZE_ENV, None::<&str>),
(storageclass::INLINE_BLOCK_ENV, None::<&str>),
],
async {
let fixture = runtime_config_reload_fixture().await;
let older = scanner_server_config("41");
save_admin_server_config(fixture.context.object_store(), &older)
.await
.expect("persist older scanner config");
let (worker_entered_tx, worker_entered_rx) = tokio::sync::oneshot::channel();
let (release_worker_tx, release_worker_rx) = tokio::sync::oneshot::channel();
let reload_context = fixture.context.clone();
let reload = tokio::spawn(async move {
publish_latest_runtime_config_snapshot_under_reload_lock_for_context(
Some(&reload_context),
None,
move |config| async move {
worker_entered_tx.send(config).expect("signal runtime config worker entry");
release_worker_rx.await.expect("release runtime config worker");
Ok(())
},
)
.await
});
let worker_config = tokio::time::timeout(REAL_STORE_TEST_TIMEOUT, worker_entered_rx)
.await
.expect("worker apply should enter")
.expect("worker apply entry signal should be delivered");
assert_eq!(
worker_config
.get_value(SCANNER_SUB_SYS, DEFAULT_DELIMITER)
.expect("older scanner config should be loaded")
.get(SCANNER_CYCLE),
"41"
);
let latest = scanner_server_config("71");
let writer_store = fixture.context.object_store();
let (writer_polled_tx, writer_polled_rx) = tokio::sync::oneshot::channel();
let (writer_entered_tx, mut writer_entered_rx) = tokio::sync::oneshot::channel();
let writer = tokio::spawn(async move {
let transaction_store = writer_store.clone();
let transaction = with_admin_server_config_write_lock(writer_store, move || async move {
writer_entered_tx.send(()).expect("signal writer entry");
save_admin_server_config_no_lock(transaction_store, &latest).await
});
tokio::pin!(transaction);
let mut writer_polled_tx = Some(writer_polled_tx);
poll_fn(|cx| match transaction.as_mut().poll(cx) {
Poll::Pending => {
if let Some(tx) = writer_polled_tx.take() {
let _ = tx.send(());
}
Poll::Ready(())
}
Poll::Ready(_) => panic!("writer entered before the worker apply released its read fence"),
})
.await;
transaction
.await
.expect("writer should acquire the server-config locks")
.expect("writer should persist the latest config");
});
tokio::time::timeout(REAL_STORE_TEST_TIMEOUT, writer_polled_rx)
.await
.expect("writer should be polled")
.expect("writer poll signal should be delivered");
assert!(
matches!(writer_entered_rx.try_recv(), Err(tokio::sync::oneshot::error::TryRecvError::Empty)),
"a server-config writer must wait until the old worker apply completes"
);
release_worker_tx.send(()).expect("release worker apply");
tokio::time::timeout(REAL_STORE_TEST_TIMEOUT, async {
reload
.await
.expect("full reload task should not panic")
.expect("full reload should finish");
writer.await.expect("writer task should not panic");
})
.await
.expect("reload and writer should finish");
reload_runtime_config_snapshot_for_context(Some(&fixture.context))
.await
.expect("a later full reload should publish the latest durable config");
let snapshot = fixture
.server_snapshot
.lock()
.expect("server config result lock")
.clone()
.expect("latest server config should be published");
assert_eq!(
snapshot
.get_value(SCANNER_SUB_SYS, DEFAULT_DELIMITER)
.expect("latest scanner config should be present")
.get(SCANNER_CYCLE),
"71"
);
assert_eq!(rustfs_scanner::scanner_runtime_config_status().cycle_interval_seconds.value, 71);
rustfs_scanner::apply_scanner_runtime_config(&ServerConfig::new()).expect("restore scanner runtime defaults");
},
)
.await;
}
#[tokio::test]
#[serial_test::serial(storage_class_env)]
async fn peer_full_reload_rejects_later_pool_without_publishing() {
@@ -1251,11 +1545,9 @@ mod tests {
let worker_events = events.clone();
let err = reload_runtime_config_snapshot_with(
async { Ok(ServerConfig::new()) },
|config| async { Ok((config, PreparedRuntimeConfig::default())) },
move |_config, _prepared| {
async move {
publish_events.lock().expect("reload event lock").push("publish");
Ok(())
Ok(ServerConfig::new())
},
move |_config| async move {
let mut events = worker_events.lock().expect("reload event lock");
+20 -5
View File
@@ -644,12 +644,17 @@ pub(crate) async fn read_admin_config_without_migrate(api: Arc<ECStore>) -> Resu
ecstore_config::com::read_config_without_migrate(api).await
}
pub(crate) async fn read_existing_admin_server_config_no_lock(api: Arc<ECStore>) -> Result<rustfs_config::server_config::Config> {
ecstore_config::com::read_existing_server_config_no_lock(api).await
}
#[cfg(test)]
pub(crate) async fn read_admin_config_without_migrate_no_lock(api: Arc<ECStore>) -> Result<rustfs_config::server_config::Config> {
ecstore_config::com::read_config_without_migrate_no_lock(api).await
}
pub(crate) type AdminServerConfigSnapshot = ecstore_config::com::ServerConfigSnapshot;
pub(crate) type AdminServerConfigSaveResult = ecstore_config::com::ServerConfigSaveResult;
pub(crate) async fn save_admin_config(api: Arc<ECStore>, file: &str, data: Vec<u8>) -> Result<()> {
ecstore_config::com::save_config(api, file, data).await
@@ -690,8 +695,17 @@ pub(crate) async fn save_admin_server_config_snapshot(
api: Arc<ECStore>,
cfg: &rustfs_config::server_config::Config,
snapshot: &AdminServerConfigSnapshot,
) -> Result<bool> {
ecstore_config::com::save_server_config_snapshot(api, cfg, snapshot).await
) -> Result<AdminServerConfigSaveResult> {
ecstore_config::com::save_server_config_snapshot_with_generation(api, cfg, snapshot).await
}
pub(crate) async fn with_admin_server_config_read_lock<F, Fut, T>(api: Arc<ECStore>, operation: F) -> Result<T>
where
F: FnOnce() -> Fut + Send + 'static,
Fut: std::future::Future<Output = T> + Send + 'static,
T: Send + 'static,
{
ecstore_config::com::with_server_config_read_lock(api, operation).await
}
pub(crate) fn init_admin_config_defaults() {
@@ -793,9 +807,10 @@ pub(crate) mod cluster {
pub(crate) mod config {
pub(crate) use super::storageclass;
pub(crate) use super::{
AdminServerConfigSnapshot, RUSTFS_META_BUCKET, STORAGE_CLASS_SUB_SYS, delete_admin_config, init_admin_config_defaults,
read_admin_config, read_admin_config_without_migrate, read_admin_server_config_snapshot, save_admin_config,
save_admin_server_config_snapshot,
AdminServerConfigSaveResult, AdminServerConfigSnapshot, RUSTFS_META_BUCKET, STORAGE_CLASS_SUB_SYS, delete_admin_config,
init_admin_config_defaults, read_admin_config, read_admin_config_without_migrate, read_admin_server_config_snapshot,
read_existing_admin_server_config_no_lock, save_admin_config, save_admin_server_config_snapshot,
with_admin_server_config_read_lock,
};
#[cfg(test)]
pub(crate) use super::{
+36 -1
View File
@@ -12,9 +12,32 @@
// See the License for the specific language governing permissions and
// limitations under the License.
#[cfg(all(feature = "hotpath", feature = "hotpath-alloc"))]
use std::alloc::{GlobalAlloc, Layout};
#[cfg(all(feature = "hotpath", feature = "hotpath-alloc"))]
#[derive(Default)]
struct DefaultMiMalloc;
#[cfg(all(feature = "hotpath", feature = "hotpath-alloc"))]
// SAFETY: allocation and deallocation are forwarded unchanged to MiMalloc, so
// MiMalloc's GlobalAlloc guarantees apply to every returned pointer and layout.
#[allow(unsafe_code)]
unsafe impl GlobalAlloc for DefaultMiMalloc {
unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
// SAFETY: the caller upholds GlobalAlloc's contract for layout.
unsafe { mimalloc::MiMalloc.alloc(layout) }
}
unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) {
// SAFETY: ptr and layout came from this allocator and are forwarded unchanged.
unsafe { mimalloc::MiMalloc.dealloc(ptr, layout) }
}
}
#[cfg(all(feature = "hotpath", feature = "hotpath-alloc"))]
#[global_allocator]
static GLOBAL: hotpath::CountingAllocator = hotpath::CountingAllocator::new();
static GLOBAL: hotpath::CountingAllocator<DefaultMiMalloc> = hotpath::CountingAllocator::new();
#[cfg(not(all(feature = "hotpath", feature = "hotpath-alloc")))]
#[global_allocator]
@@ -25,3 +48,15 @@ fn main() {
rustfs::startup_entrypoint::run_process();
}
#[cfg(all(test, feature = "hotpath", feature = "hotpath-alloc"))]
mod tests {
#[test]
#[allow(unsafe_code)]
fn hotpath_allocator_uses_mimalloc() {
let allocation = Box::new([0_u8; 64]);
// SAFETY: the live Box pointer is valid to inspect for heap ownership.
assert!(unsafe { libmimalloc_sys::mi_is_in_heap_region(allocation.as_ptr().cast()) });
}
}
+13 -1
View File
@@ -40,7 +40,19 @@ export RUST_BACKTRACE=full
"$BIN" --address "127.0.0.1:${RUSTFS_TEST_PORT}" "$VOLUME" > "${RUSTFS_TEST_LOG}" 2>&1 &
RUSTFS_PID=$!
sleep 10
for _ in {1..60}; do
if curl --noproxy '*' --silent --fail "http://127.0.0.1:${RUSTFS_TEST_PORT}/health/ready" >/dev/null; then
break
fi
if ! kill -0 "${RUSTFS_PID}" 2>/dev/null; then
wait "${RUSTFS_PID}"
fi
sleep 1
done
curl --noproxy '*' --silent --show-error --fail "http://127.0.0.1:${RUSTFS_TEST_PORT}/health/ready" >/dev/null
export AWS_ACCESS_KEY_ID="${RUSTFS_ACCESS_KEY:-rustfsadmin}"
export AWS_SECRET_ACCESS_KEY="${RUSTFS_SECRET_KEY:-rustfsadmin}"