mirror of
https://github.com/GreptimeTeam/greptimedb.git
synced 2026-09-08 14:39:10 +00:00
fix: sandbox SQL local filesystem access (#8708)
* fix: sandbox SQL local filesystem access Signed-off-by: jeremyhi <fengjiachun@gmail.com> * fix: address local file sandbox review findings Signed-off-by: jeremyhi <fengjiachun@gmail.com> * fix: support Windows local copy paths Signed-off-by: jeremyhi <fengjiachun@gmail.com> * fix: improve sandbox path errors Signed-off-by: jeremyhi <fengjiachun@gmail.com> * refactor: simplify local path error context Signed-off-by: jeremyhi <fengjiachun@gmail.com> * perf: stream secure filesystem listings Signed-off-by: jeremyhi <fengjiachun@gmail.com> * style: derive local file access default Signed-off-by: jeremyhi <fengjiachun@gmail.com> * fix: improve local file access errors Signed-off-by: jeremyhi <fengjiachun@gmail.com> * fix: address local file access review findings Signed-off-by: jeremyhi <fengjiachun@gmail.com> * test: simplify local file access coverage Signed-off-by: jeremyhi <fengjiachun@gmail.com> * fix: harden sandboxed local file backends Signed-off-by: jeremyhi <fengjiachun@gmail.com> * fix: reject directory copy targets before creation Signed-off-by: jeremyhi <fengjiachun@gmail.com> * fix: avoid implicit string clone in file table listing Signed-off-by: jeremyhi <fengjiachun@gmail.com> --------- Signed-off-by: jeremyhi <fengjiachun@gmail.com>
This commit is contained in:
@@ -19,6 +19,7 @@ use std::sync::{Arc, RwLock};
|
||||
use api::region::RegionResponse;
|
||||
use async_trait::async_trait;
|
||||
use common_catalog::consts::FILE_ENGINE;
|
||||
use common_datasource::object_store::LocalFileAccess;
|
||||
use common_error::ext::BoxedError;
|
||||
use common_recordbatch::SendableRecordBatchStream;
|
||||
use common_telemetry::{error, info};
|
||||
@@ -48,9 +49,13 @@ pub struct FileRegionEngine {
|
||||
}
|
||||
|
||||
impl FileRegionEngine {
|
||||
pub fn new(_config: EngineConfig, object_store: ObjectStore) -> Self {
|
||||
pub fn new(
|
||||
_config: EngineConfig,
|
||||
object_store: ObjectStore,
|
||||
local_file_access: LocalFileAccess,
|
||||
) -> Self {
|
||||
Self {
|
||||
inner: Arc::new(EngineInner::new(object_store)),
|
||||
inner: Arc::new(EngineInner::new(object_store, local_file_access)),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -64,7 +69,8 @@ impl FileRegionEngine {
|
||||
.await
|
||||
.context(RegionNotFoundSnafu { region_id })
|
||||
.map_err(BoxedError::new)?
|
||||
.query(request)
|
||||
.query(request, &self.inner.local_file_access)
|
||||
.await
|
||||
.map_err(BoxedError::new)
|
||||
}
|
||||
}
|
||||
@@ -182,6 +188,8 @@ struct EngineInner {
|
||||
region_mutex: Mutex<()>,
|
||||
|
||||
object_store: ObjectStore,
|
||||
|
||||
local_file_access: LocalFileAccess,
|
||||
}
|
||||
|
||||
type EngineInnerRef = Arc<EngineInner>;
|
||||
@@ -205,11 +213,12 @@ fn ensure_region_requirements(
|
||||
}
|
||||
|
||||
impl EngineInner {
|
||||
fn new(object_store: ObjectStore) -> Self {
|
||||
fn new(object_store: ObjectStore, local_file_access: LocalFileAccess) -> Self {
|
||||
Self {
|
||||
regions: RwLock::new(HashMap::new()),
|
||||
region_mutex: Mutex::new(()),
|
||||
object_store,
|
||||
local_file_access,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -19,7 +19,7 @@ use std::pin::Pin;
|
||||
use std::sync::Arc;
|
||||
use std::task::{Context, Poll};
|
||||
|
||||
use common_datasource::object_store::build_backend;
|
||||
use common_datasource::object_store::{LocalFileAccess, build_backend};
|
||||
use common_recordbatch::adapter::RecordBatchMetrics;
|
||||
use common_recordbatch::error::{self as recordbatch_error, Result as RecordBatchResult};
|
||||
use common_recordbatch::{
|
||||
@@ -41,8 +41,14 @@ use crate::error::{BuildBackendSnafu, ProjectSchemaSnafu, ProjectionOutOfBoundsS
|
||||
use crate::region::FileRegion;
|
||||
|
||||
impl FileRegion {
|
||||
pub fn query(&self, request: ScanRequest) -> Result<SendableRecordBatchStream> {
|
||||
let store = build_backend(&self.url, &self.options).context(BuildBackendSnafu)?;
|
||||
pub async fn query(
|
||||
&self,
|
||||
request: ScanRequest,
|
||||
local_file_access: &LocalFileAccess,
|
||||
) -> Result<SendableRecordBatchStream> {
|
||||
let store = build_backend(&self.url, &self.options, local_file_access)
|
||||
.await
|
||||
.context(BuildBackendSnafu)?;
|
||||
|
||||
let projection = request.projection.as_deref();
|
||||
let file_projection = self.projection_pushdown_to_file(projection)?;
|
||||
|
||||
@@ -107,7 +107,11 @@ impl FileRegion {
|
||||
mod tests {
|
||||
use std::assert_matches;
|
||||
|
||||
use common_datasource::object_store::LocalFileAccess;
|
||||
use common_error::ext::{ErrorExt, RetryHint};
|
||||
use common_error::status_code::StatusCode;
|
||||
use store_api::region_request::PathType;
|
||||
use store_api::storage::ScanRequest;
|
||||
|
||||
use super::*;
|
||||
use crate::error::Error;
|
||||
@@ -155,6 +159,41 @@ mod tests {
|
||||
assert_matches!(err, Error::ManifestExists { .. });
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_persisted_local_region_rejected_when_disabled() {
|
||||
let (_dir, object_store) = new_test_object_store("test_disabled_local_region");
|
||||
let request = RegionCreateRequest {
|
||||
engine: "file".to_string(),
|
||||
column_metadatas: new_test_column_metadata(),
|
||||
primary_key: vec![1],
|
||||
options: new_test_options(),
|
||||
table_dir: "disabled_local_region/".to_string(),
|
||||
path_type: PathType::Bare,
|
||||
partition_expr_json: Some("".to_string()),
|
||||
requirements: Default::default(),
|
||||
};
|
||||
let region = FileRegion::create(RegionId::new(1, 0), request, &object_store)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let error = match region
|
||||
.query(ScanRequest::default(), &LocalFileAccess::Disabled)
|
||||
.await
|
||||
{
|
||||
Ok(_) => panic!("local file query must be rejected"),
|
||||
Err(error) => error,
|
||||
};
|
||||
assert_matches!(
|
||||
&error,
|
||||
Error::BuildBackend {
|
||||
source: common_datasource::error::Error::LocalFileAccessDisabled { .. },
|
||||
..
|
||||
}
|
||||
);
|
||||
assert_eq!(error.status_code(), StatusCode::InvalidArguments);
|
||||
assert_eq!(error.retry_hint(), RetryHint::NonRetryable);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_open_region() {
|
||||
let (_dir, object_store) = new_test_object_store("test_open_region");
|
||||
|
||||
Reference in New Issue
Block a user