fix(admin): keep exporting past unreadable configs and allow explicit target repair (#7247)

* test(ecstore): pin MinIO array-shaped targets blob as unreadable

* fix(admin): mark unreadable configs instead of aborting export

* feat(admin): opt-in replacement of unreadable bucket targets

crates/ecstore/src/api/mod.rs

@@ -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,
         };
     }
 

crates/ecstore/src/bucket/bucket_target_sys.rs

@@ -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");
+        }
+    }
 }

crates/ecstore/src/bucket/metadata.rs

@@ -1611,6 +1611,34 @@ mod test {
         assert!(bm.bucket_target_config.is_none());
     }
 
+    /// rustfs/backlog#2309: the MinIO-origin `.metadata.bin` this repository
+    /// already carries as a compatibility fixture stores
+    /// `BucketTargetsConfigJSON` as a bare JSON array, which `BucketTargets`
+    /// (a `{"targets":[…]}` struct with no array fallback) cannot decode. The
+    /// bytes below are the exact payload the fixture in
+    /// `metadata_test.rs::TEST_BUCKET_METADATA_HEX` decodes to, so if RustFS
+    /// ever grows the array-shaped compatibility parse, this test is where the
+    /// upgrade break is pinned and where the decision has to be recorded.
+    #[test]
+    fn minio_array_shaped_bucket_targets_are_unreadable() {
+        let minio_array = br#"[{"endpoint":"http://target.example.com","targetBucket":"tb","region":"us-east-1"}]"#.to_vec();
+        let mut bm = BucketMetadata::new("minio-array-targets");
+        bm.bucket_targets_config_json = minio_array.clone();
+
+        bm.parse_all_configs()
+            .expect("a MinIO-shaped targets blob must not fail the whole metadata load");
+
+        assert!(
+            bm.bucket_targets_unreadable(),
+            "an array-shaped MinIO targets blob is unreadable, not an empty target set"
+        );
+        assert!(bm.bucket_target_config.is_none());
+        assert_eq!(
+            bm.bucket_targets_config_json, minio_array,
+            "the raw MinIO bytes must survive so the configuration stays recoverable"
+        );
+    }
+
     /// The invariant every branch of `parse_all_configs` shares: a stored but
     /// undecodable payload keeps its raw bytes and leaves the typed field
     /// `None`, so no branch fabricates a value. What a reader may then do with