mirror of
https://github.com/rustfs/rustfs.git
synced 2026-09-06 12:09:12 +00:00
feat(admin): opt-in replacement of unreadable bucket targets
This commit is contained in:
@@ -32,7 +32,7 @@ pub mod bucket {
|
||||
pub mod bucket_target_sys {
|
||||
pub use crate::bucket::bucket_target_sys::{
|
||||
AdvancedPutOptions, BucketTargetError, BucketTargetSys, PutObjectOptions, RemoveObjectOptions, S3ClientError,
|
||||
SsecPassthroughCapability, TargetClient, append_version_id_query,
|
||||
SsecPassthroughCapability, TargetClient, UnreadableTargetsPolicy, append_version_id_query,
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -369,6 +369,26 @@ struct SsecPassthroughRecord {
|
||||
recorded_at: Instant,
|
||||
}
|
||||
|
||||
/// What a target write does when the bucket's persisted target set exists but
|
||||
/// cannot be decoded.
|
||||
///
|
||||
/// `docs/architecture/remote-credential-sealing-adr.md` forbids rewriting a
|
||||
/// configuration that could not be fully read, because re-serializing a
|
||||
/// partial in-memory view is the one mechanism by which a configured target
|
||||
/// really disappears. That rule guards against an *unintentional* overwrite,
|
||||
/// so an operator who names the hazard keeps a repair path
|
||||
/// (rustfs/backlog#2309); everything that does not name it stays refused.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]
|
||||
pub enum UnreadableTargetsPolicy {
|
||||
/// Refuse the write with [`BucketTargetError::BucketRemoteTargetsUnreadable`].
|
||||
#[default]
|
||||
FailClosed,
|
||||
/// Discard the unreadable set; the target being written becomes the whole
|
||||
/// configuration. Reachable only from an admin request that asked for it
|
||||
/// explicitly, and audited by the caller.
|
||||
Replace,
|
||||
}
|
||||
|
||||
#[derive(Debug, Default)]
|
||||
pub struct BucketTargetSys {
|
||||
pub arn_remotes_map: Arc<RwLock<HashMap<String, ArnTarget>>>,
|
||||
@@ -791,20 +811,45 @@ impl BucketTargetSys {
|
||||
bucket: &str,
|
||||
target: &BucketTarget,
|
||||
update: bool,
|
||||
unreadable_policy: UnreadableTargetsPolicy,
|
||||
) -> Result<BucketTargets, BucketTargetError> {
|
||||
self.validate_target(bucket, target).await?;
|
||||
|
||||
let mut bucket_targets = match self.list_bucket_targets(bucket).await {
|
||||
Ok(targets) => targets,
|
||||
Err(BucketTargetError::BucketRemoteTargetNotFound { .. }) => BucketTargets::default(),
|
||||
Err(err) => return Err(err),
|
||||
};
|
||||
let mut bucket_targets = self.targets_base_for_write(bucket, unreadable_policy).await?;
|
||||
|
||||
Self::upsert_target_entry(&mut bucket_targets.targets, target, update)?;
|
||||
|
||||
Ok(bucket_targets)
|
||||
}
|
||||
|
||||
/// The persisted target set a write merges into.
|
||||
///
|
||||
/// An absent configuration starts from the empty set. An unreadable one is
|
||||
/// refused, because re-serializing a partial view of a set this node could
|
||||
/// not decode is how a configured target disappears for good — unless the
|
||||
/// caller carries the operator's explicit
|
||||
/// [`UnreadableTargetsPolicy::Replace`] opt-in, which discards it
|
||||
/// deliberately (rustfs/backlog#2309).
|
||||
async fn targets_base_for_write(
|
||||
&self,
|
||||
bucket: &str,
|
||||
unreadable_policy: UnreadableTargetsPolicy,
|
||||
) -> Result<BucketTargets, BucketTargetError> {
|
||||
match self.list_bucket_targets(bucket).await {
|
||||
Ok(targets) => Ok(targets),
|
||||
Err(BucketTargetError::BucketRemoteTargetNotFound { .. }) => Ok(BucketTargets::default()),
|
||||
// The opt-in discards only a set this node genuinely cannot read.
|
||||
// A readable set still merges through the arm above, so the policy
|
||||
// can never drop a target that was visible here.
|
||||
Err(BucketTargetError::BucketRemoteTargetsUnreadable { .. })
|
||||
if unreadable_policy == UnreadableTargetsPolicy::Replace =>
|
||||
{
|
||||
Ok(BucketTargets::default())
|
||||
}
|
||||
Err(err) => Err(err),
|
||||
}
|
||||
}
|
||||
|
||||
pub async fn validate_target(&self, bucket: &str, target: &BucketTarget) -> Result<(), BucketTargetError> {
|
||||
if !target.target_type.is_valid() {
|
||||
return Err(BucketTargetError::BucketRemoteArnTypeInvalid {
|
||||
@@ -4313,4 +4358,75 @@ mod tests {
|
||||
let window = LastMinuteLatency::new();
|
||||
assert_eq!(window.get_total().avg, Duration::from_secs(0));
|
||||
}
|
||||
|
||||
fn repair_target(bucket: &str, id: &str) -> BucketTarget {
|
||||
BucketTarget {
|
||||
source_bucket: bucket.to_string(),
|
||||
endpoint: "remote.example.com".to_string(),
|
||||
target_bucket: "remote".to_string(),
|
||||
arn: format!("arn:rustfs:replication:us-east-1:{bucket}:{id}"),
|
||||
target_type: BucketTargetType::ReplicationService,
|
||||
region: "us-east-1".to_string(),
|
||||
..Default::default()
|
||||
}
|
||||
}
|
||||
|
||||
/// rustfs/backlog#2309: after rustfs/rustfs#7172 an undecodable
|
||||
/// `bucket-targets.json` left the bucket with no API repair path at all.
|
||||
/// The refusal is the default and stays the default; the operator's
|
||||
/// explicit opt-in is the only thing that discards the set, and it starts
|
||||
/// the replacement from empty rather than from a partial view of bytes
|
||||
/// this node never decoded.
|
||||
#[tokio::test]
|
||||
async fn an_unreadable_target_set_is_replaced_only_with_the_explicit_opt_in() {
|
||||
let sys = BucketTargetSys::default();
|
||||
let bucket = "targets-repair-opt-in";
|
||||
sys.mark_targets_unreadable(bucket).await;
|
||||
|
||||
assert!(
|
||||
matches!(
|
||||
sys.targets_base_for_write(bucket, UnreadableTargetsPolicy::FailClosed).await,
|
||||
Err(BucketTargetError::BucketRemoteTargetsUnreadable { .. })
|
||||
),
|
||||
"without the opt-in an unreadable target set must still refuse the write"
|
||||
);
|
||||
assert_eq!(
|
||||
UnreadableTargetsPolicy::default(),
|
||||
UnreadableTargetsPolicy::FailClosed,
|
||||
"a caller that says nothing must get the refusal"
|
||||
);
|
||||
|
||||
let base = sys
|
||||
.targets_base_for_write(bucket, UnreadableTargetsPolicy::Replace)
|
||||
.await
|
||||
.expect("the explicit opt-in must let an operator replace an unreadable set");
|
||||
assert!(
|
||||
base.is_empty(),
|
||||
"the replacement must start from an empty set, never from a partial decode"
|
||||
);
|
||||
}
|
||||
|
||||
/// The opt-in is not a wipe switch. On a set this node can read, both
|
||||
/// policies take the same merge path, so a stray `replace-unreadable=true`
|
||||
/// cannot drop a visible target — which is what makes the flag safe to
|
||||
/// repeat in an operator's repair script.
|
||||
#[tokio::test]
|
||||
async fn the_opt_in_never_discards_a_readable_target_set() {
|
||||
let sys = BucketTargetSys::default();
|
||||
let bucket = "targets-repair-readable";
|
||||
let existing = repair_target(bucket, "keep");
|
||||
sys.targets_map
|
||||
.write()
|
||||
.await
|
||||
.insert(bucket.to_string(), vec![existing.clone()]);
|
||||
|
||||
for policy in [UnreadableTargetsPolicy::FailClosed, UnreadableTargetsPolicy::Replace] {
|
||||
let base = sys
|
||||
.targets_base_for_write(bucket, policy)
|
||||
.await
|
||||
.expect("a readable target set must be readable under either policy");
|
||||
assert_eq!(base.targets.len(), 1, "{policy:?} must keep the persisted target");
|
||||
assert_eq!(base.targets[0].arn, existing.arn, "{policy:?} must not rewrite the persisted target");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -29,7 +29,7 @@ use crate::admin::storage_api::bucket::replication::{REMOTE_TARGET_READ_ONLY_HIS
|
||||
use crate::admin::storage_api::bucket::target::{
|
||||
BucketTarget, BucketTargetType, Credentials as TargetCredentials, LatencyStat, duration_from_secs_or_nanos,
|
||||
};
|
||||
use crate::admin::storage_api::bucket::target_sys::{BucketTargetError, BucketTargetSys};
|
||||
use crate::admin::storage_api::bucket::target_sys::{BucketTargetError, BucketTargetSys, UnreadableTargetsPolicy};
|
||||
use crate::admin::storage_api::contract::bucket::{BucketOperations, BucketOptions};
|
||||
use crate::admin::storage_api::contract::list::ListOperations as _;
|
||||
use crate::admin::storage_api::error::StorageError;
|
||||
@@ -57,6 +57,20 @@ use url::Host;
|
||||
|
||||
const SUPPORTED_REMOTE_TARGET_API: &str = "s3v4";
|
||||
|
||||
const LOG_COMPONENT_ADMIN: &str = "admin";
|
||||
const LOG_SUBSYSTEM_REPLICATION: &str = "replication";
|
||||
const EVENT_ADMIN_REMOTE_TARGET_STATE: &str = "admin_remote_target_state";
|
||||
|
||||
/// `set-remote-target?replace-unreadable=true`: the operator's explicit
|
||||
/// acknowledgement that this bucket's persisted target set cannot be decoded
|
||||
/// and is to be discarded (rustfs/backlog#2309).
|
||||
///
|
||||
/// Without it the refusal from rustfs/rustfs#7172 stands, which is what keeps
|
||||
/// an unreadable set from being silently rewritten from a partial view. The
|
||||
/// flag is deliberately absent from `PutBucketReplication`: targets are
|
||||
/// repaired first, then the rule is set.
|
||||
const REPLACE_UNREADABLE_TARGETS_PARAM: &str = "replace-unreadable";
|
||||
|
||||
/// Field groups a `set-remote-target?update=true` request may modify, mirroring
|
||||
/// MinIO's `TargetUpdateType` / `GetTargetUpdateOps` query contract: the update
|
||||
/// overlays only the requested groups onto the stored target, so a client can
|
||||
@@ -541,6 +555,7 @@ impl Operation for SetRemoteTargetHandler {
|
||||
};
|
||||
|
||||
let update = queries.get("update").is_some_and(|v| v == "true");
|
||||
let replace_unreadable = queries.get(REPLACE_UNREADABLE_TARGETS_PARAM).is_some_and(|v| v == "true");
|
||||
|
||||
warn!("set remote target, bucket: {}, update: {}", bucket, update);
|
||||
|
||||
@@ -708,10 +723,37 @@ impl Operation for SetRemoteTargetHandler {
|
||||
|
||||
let arn = remote_target.arn.clone();
|
||||
|
||||
let unreadable_policy = if replace_unreadable {
|
||||
UnreadableTargetsPolicy::Replace
|
||||
} else {
|
||||
UnreadableTargetsPolicy::FailClosed
|
||||
};
|
||||
let discarding_unreadable_targets = replace_unreadable
|
||||
&& matches!(
|
||||
bucket_target_sys.list_bucket_targets(bucket).await,
|
||||
Err(BucketTargetError::BucketRemoteTargetsUnreadable { .. })
|
||||
);
|
||||
|
||||
let targets = bucket_target_sys
|
||||
.set_target(bucket, &remote_target, update)
|
||||
.set_target(bucket, &remote_target, update, unreadable_policy)
|
||||
.await
|
||||
.map_err(map_bucket_target_error)?;
|
||||
|
||||
// Audited only where the discard actually happened: the flag alone is
|
||||
// not an event, on a readable set it changes nothing, and a refused
|
||||
// write must not leave a record claiming the set was replaced.
|
||||
if discarding_unreadable_targets {
|
||||
warn!(
|
||||
event = EVENT_ADMIN_REMOTE_TARGET_STATE,
|
||||
component = LOG_COMPONENT_ADMIN,
|
||||
subsystem = LOG_SUBSYSTEM_REPLICATION,
|
||||
action = "set_remote_target",
|
||||
result = "unreadable_targets_replaced",
|
||||
bucket = %bucket,
|
||||
arn = %remote_target.arn,
|
||||
"admin remote target state"
|
||||
);
|
||||
}
|
||||
let json_targets = serde_json::to_vec(&targets).map_err(|e| {
|
||||
error!("Serialization error: {}", e);
|
||||
S3Error::with_message(S3ErrorCode::InternalError, "Failed to serialize targets".to_string())
|
||||
|
||||
@@ -212,6 +212,7 @@ pub(crate) mod bucket_target_sys {
|
||||
pub(crate) type S3ClientError = super::ecstore_bucket::bucket_target_sys::S3ClientError;
|
||||
pub(crate) type SsecPassthroughCapability = super::ecstore_bucket::bucket_target_sys::SsecPassthroughCapability;
|
||||
pub(crate) type TargetClient = super::ecstore_bucket::bucket_target_sys::TargetClient;
|
||||
pub(crate) type UnreadableTargetsPolicy = super::ecstore_bucket::bucket_target_sys::UnreadableTargetsPolicy;
|
||||
}
|
||||
|
||||
pub(crate) mod lifecycle {
|
||||
|
||||
Reference in New Issue
Block a user