mirror of
https://github.com/rustfs/rustfs.git
synced 2026-08-22 12:26:37 +00:00
refactor: prefer app context object store in admin zip (#3434)
This commit is contained in:
@@ -5,16 +5,17 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
|
|||||||
## Current Context
|
## Current Context
|
||||||
|
|
||||||
- Issue: [`rustfs/backlog#660`](https://github.com/rustfs/backlog/issues/660)
|
- Issue: [`rustfs/backlog#660`](https://github.com/rustfs/backlog/issues/660)
|
||||||
- Branch: `overtrue/arch-storage-ecfs-object-store-context`
|
- Branch: `overtrue/arch-admin-zip-object-store-context`
|
||||||
- Baseline: `upstream/main` at `046d5386ba9ffe6a0df39ffd78322c62e9f358d2`
|
- Baseline: `upstream/main` at `5e68cf8a296f0453fd050d137830032329685d12`
|
||||||
- PR type for this branch: `consumer-migration`
|
- PR type for this branch: `consumer-migration`
|
||||||
- Runtime behavior changes: no external behavior change expected; S3 ECFS
|
- Runtime behavior changes: no external behavior change expected; admin object
|
||||||
object-store lookups prefer the AppContext-owned object store and keep the
|
ZIP download object-store lookups prefer the AppContext-owned object store
|
||||||
existing global fallback.
|
and keep the existing global fallback.
|
||||||
- Rust code changes: migrate `rustfs/src/storage/ecfs.rs` object-store lookups
|
- Rust code changes: migrate rustfs/src/admin/handlers/object_zip_download.rs
|
||||||
to the shared resolver without touching lower-layer storage infra modules.
|
object-store lookups to the shared resolver without touching infra-layer
|
||||||
|
server/storage modules or standalone crates.
|
||||||
- CI/script changes: none.
|
- CI/script changes: none.
|
||||||
- Docs changes: record `CTX-006` consumer migration scope and verification.
|
- Docs changes: record `CTX-007` consumer migration scope and verification.
|
||||||
|
|
||||||
## Phase 0 Tasks
|
## Phase 0 Tasks
|
||||||
|
|
||||||
@@ -182,6 +183,15 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
|
|||||||
semantics, and existing storage error paths.
|
semantics, and existing storage error paths.
|
||||||
- Verification: formatting, compile check, migration guards, diff hygiene,
|
- Verification: formatting, compile check, migration guards, diff hygiene,
|
||||||
Rust risk scan, and full `make pre-commit`.
|
Rust risk scan, and full `make pre-commit`.
|
||||||
|
- [x] `CTX-007` Migrate admin ZIP object-store consumers.
|
||||||
|
- Do: migrate admin object ZIP download object-store lookups to the shared
|
||||||
|
object-store resolver.
|
||||||
|
- Acceptance: admin ZIP object-store lookups use AppContext when present and
|
||||||
|
preserve the existing global object-layer fallback when absent.
|
||||||
|
- Must preserve: admin download authorization/preflight behavior, ZIP listing
|
||||||
|
and streaming behavior, and existing storage error paths.
|
||||||
|
- Verification: formatting, compile check, migration guards, diff hygiene,
|
||||||
|
Rust risk scan, and full `make pre-commit`.
|
||||||
|
|
||||||
## Phase 1 Security Governance Tasks
|
## Phase 1 Security Governance Tasks
|
||||||
|
|
||||||
@@ -488,8 +498,9 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
|
|||||||
|
|
||||||
## Next PRs
|
## Next PRs
|
||||||
|
|
||||||
1. `consumer-migration`: migrate the next low-risk non-admin AppContext
|
1. `consumer-migration`: design the crate-local object-store resolver path for
|
||||||
consumer group that does not introduce a new reverse layer dependency.
|
crates/protocols, crates/obs, crates/scanner, and crates/notify without
|
||||||
|
introducing reverse dependencies on rustfs.
|
||||||
2. `pure-move`: start `R-009` boot wrapper with the IAM degraded readiness
|
2. `pure-move`: start `R-009` boot wrapper with the IAM degraded readiness
|
||||||
contract covered.
|
contract covered.
|
||||||
|
|
||||||
@@ -497,13 +508,13 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
|
|||||||
|
|
||||||
| Expert | Status | Notes |
|
| Expert | Status | Notes |
|
||||||
|---|---|---|
|
|---|---|---|
|
||||||
| Quality/architecture | pass | Single `consumer-migration` slice; `ecfs.rs` is an interface-layer module already using app usecases, and the layer guard reports no new reverse dependencies. |
|
| Quality/architecture | pass | Single `consumer-migration` slice; admin ZIP code may depend on app context, and the layer guard reports no new reverse dependencies. |
|
||||||
| Migration preservation | pass | AppContext object-store resolution is preferred when present, existing global object-layer fallback and S3 storage error paths are preserved, and ECFS method bodies keep their behavior. |
|
| Migration preservation | pass | AppContext object-store resolution is preferred when present, while existing global object-layer fallback and ZIP error paths are preserved. |
|
||||||
| Testing/verification | pass | Formatting, compile check, diff hygiene, migration/layer guards, added-line Rust risk scan, and full `make pre-commit` passed. |
|
| Testing/verification | pass | Formatting, compile check, diff hygiene, migration/layer guards, added-line Rust risk scan, and full `make pre-commit` passed. |
|
||||||
|
|
||||||
## Verification Notes
|
## Verification Notes
|
||||||
|
|
||||||
Passed on `046d5386ba9ffe6a0df39ffd78322c62e9f358d2`:
|
Passed on `5e68cf8a296f0453fd050d137830032329685d12`:
|
||||||
- `cargo fmt --all`.
|
- `cargo fmt --all`.
|
||||||
- `cargo fmt --all --check`.
|
- `cargo fmt --all --check`.
|
||||||
- `cargo check -p rustfs --lib`.
|
- `cargo check -p rustfs --lib`.
|
||||||
@@ -511,18 +522,23 @@ Passed on `046d5386ba9ffe6a0df39ffd78322c62e9f358d2`:
|
|||||||
- `./scripts/check_architecture_migration_rules.sh`.
|
- `./scripts/check_architecture_migration_rules.sh`.
|
||||||
- `./scripts/check_layer_dependencies.sh`.
|
- `./scripts/check_layer_dependencies.sh`.
|
||||||
- Rust risk scan for changed Rust files; full-file matches were existing
|
- Rust risk scan for changed Rust files; full-file matches were existing
|
||||||
code/tests, and the added-line scan returned no `unwrap`/`expect`, numeric
|
tests and S3 result signatures, and the added-line scan returned no
|
||||||
cast, string error, boxed error, print macro, or relaxed-ordering match.
|
`unwrap`/`expect`, numeric cast, string error, boxed error, print macro, or
|
||||||
- `make pre-commit`; all checks passed, including nextest 5905 passed and
|
relaxed-ordering match.
|
||||||
|
- `make pre-commit`; all checks passed, including nextest 5939 passed and
|
||||||
111 skipped, plus doctests.
|
111 skipped, plus doctests.
|
||||||
|
|
||||||
Notes:
|
Notes:
|
||||||
- This slice migrates one coherent ECFS object-store consumer group.
|
- This slice migrates the admin object ZIP download object-store consumer
|
||||||
|
group.
|
||||||
|
- Server/storage infra consumers still need a crate-local resolver path before
|
||||||
|
migration.
|
||||||
- Global object-layer fallback remains available until the planned cleanup
|
- Global object-layer fallback remains available until the planned cleanup
|
||||||
phase.
|
phase.
|
||||||
|
|
||||||
## Handoff Notes
|
## Handoff Notes
|
||||||
|
|
||||||
- CTX-006 is complete.
|
- CTX-007 is complete.
|
||||||
- Keep the next consumer or boot wrapper PR scoped to one PR type and preserve
|
- Keep infra-layer server/storage consumers on the global accessor until a
|
||||||
global fallback until the planned cleanup phase.
|
crate-local resolver path is designed; preserve global fallback until the
|
||||||
|
planned cleanup phase.
|
||||||
|
|||||||
@@ -13,6 +13,7 @@
|
|||||||
// limitations under the License.
|
// limitations under the License.
|
||||||
|
|
||||||
use crate::admin::router::{ADMIN_OBJECT_ZIP_DOWNLOADS_PATH, AdminOperation, Operation, S3Router};
|
use crate::admin::router::{ADMIN_OBJECT_ZIP_DOWNLOADS_PATH, AdminOperation, Operation, S3Router};
|
||||||
|
use crate::app::context::resolve_object_store_handle;
|
||||||
use crate::auth::{check_key_valid, get_session_token};
|
use crate::auth::{check_key_valid, get_session_token};
|
||||||
use crate::error::ApiError;
|
use crate::error::ApiError;
|
||||||
use crate::license::license_check;
|
use crate::license::license_check;
|
||||||
@@ -32,7 +33,6 @@ use rustfs_config::MAX_ADMIN_REQUEST_BODY_SIZE;
|
|||||||
use rustfs_credentials::get_global_action_cred;
|
use rustfs_credentials::get_global_action_cred;
|
||||||
use rustfs_ecstore::{
|
use rustfs_ecstore::{
|
||||||
global::get_global_region,
|
global::get_global_region,
|
||||||
new_object_layer_fn,
|
|
||||||
store_api::{BucketOperations, ListOperations, ObjectIO, ObjectOperations, ObjectOptions},
|
store_api::{BucketOperations, ListOperations, ObjectIO, ObjectOperations, ObjectOptions},
|
||||||
};
|
};
|
||||||
use rustfs_policy::policy::action::{Action, S3Action};
|
use rustfs_policy::policy::action::{Action, S3Action};
|
||||||
@@ -640,7 +640,7 @@ fn validate_zip_entry_name(entry_name: &str) -> S3Result<()> {
|
|||||||
}
|
}
|
||||||
|
|
||||||
async fn preflight_zip_items(request: &CreateObjectZipDownloadRequest, items: &[ZipDownloadItem]) -> S3Result<()> {
|
async fn preflight_zip_items(request: &CreateObjectZipDownloadRequest, items: &[ZipDownloadItem]) -> S3Result<()> {
|
||||||
let store = new_object_layer_fn().ok_or_else(|| s3_error!(InternalError, "object store not initialized"))?;
|
let store = resolve_object_store_handle().ok_or_else(|| s3_error!(InternalError, "object store not initialized"))?;
|
||||||
for item in items {
|
for item in items {
|
||||||
store
|
store
|
||||||
.get_object_info(&request.bucket, &item.key, &ObjectOptions::default())
|
.get_object_info(&request.bucket, &item.key, &ObjectOptions::default())
|
||||||
@@ -655,7 +655,7 @@ fn storage_error_to_s3(err: rustfs_ecstore::error::Error) -> s3s::S3Error {
|
|||||||
}
|
}
|
||||||
|
|
||||||
async fn validate_zip_download_request(record: &ObjectZipDownloadToken) -> S3Result<()> {
|
async fn validate_zip_download_request(record: &ObjectZipDownloadToken) -> S3Result<()> {
|
||||||
let store = new_object_layer_fn().ok_or_else(|| s3_error!(InternalError, "object store not initialized"))?;
|
let store = resolve_object_store_handle().ok_or_else(|| s3_error!(InternalError, "object store not initialized"))?;
|
||||||
store
|
store
|
||||||
.get_bucket_info(&record.request.bucket, &BucketOptions::default())
|
.get_bucket_info(&record.request.bucket, &BucketOptions::default())
|
||||||
.await
|
.await
|
||||||
@@ -780,7 +780,7 @@ async fn prepare_zip_download_archive(record: &ObjectZipDownloadToken) -> S3Resu
|
|||||||
|
|
||||||
async fn generate_zip_archive(record: &ObjectZipDownloadToken, file: &mut File) -> io::Result<()> {
|
async fn generate_zip_archive(record: &ObjectZipDownloadToken, file: &mut File) -> io::Result<()> {
|
||||||
let mut zip = ZipFileWriter::with_tokio(file).force_zip64();
|
let mut zip = ZipFileWriter::with_tokio(file).force_zip64();
|
||||||
let store = new_object_layer_fn().ok_or_else(|| io::Error::other("object store not initialized"))?;
|
let store = resolve_object_store_handle().ok_or_else(|| io::Error::other("object store not initialized"))?;
|
||||||
|
|
||||||
let explicit_items = collect_explicit_zip_items(&record.request)
|
let explicit_items = collect_explicit_zip_items(&record.request)
|
||||||
.map_err(|err| io::Error::other(format!("failed to prepare explicit ZIP items: {err}")))?;
|
.map_err(|err| io::Error::other(format!("failed to prepare explicit ZIP items: {err}")))?;
|
||||||
@@ -867,7 +867,7 @@ async fn write_zip_item<W>(
|
|||||||
where
|
where
|
||||||
W: tokio::io::AsyncWrite + Unpin,
|
W: tokio::io::AsyncWrite + Unpin,
|
||||||
{
|
{
|
||||||
let store = new_object_layer_fn().ok_or_else(|| io::Error::other("object store not initialized"))?;
|
let store = resolve_object_store_handle().ok_or_else(|| io::Error::other("object store not initialized"))?;
|
||||||
let mut reader = store
|
let mut reader = store
|
||||||
.get_object_reader(bucket, &item.key, None, HeaderMap::new(), &ObjectOptions::default())
|
.get_object_reader(bucket, &item.key, None, HeaderMap::new(), &ObjectOptions::default())
|
||||||
.await
|
.await
|
||||||
|
|||||||
Reference in New Issue
Block a user