fix(cli): fix FS object store handling of absolute paths (#7018)

* fix(cli): fix FS object store handling of absolute paths

Signed-off-by: WenyXu <wenymedia@gmail.com>

* test: add unit tests

Signed-off-by: WenyXu <wenymedia@gmail.com>

* Update src/cli/src/utils.rs

Co-authored-by: LFC <990479+MichaelScofield@users.noreply.github.com>

---------

Signed-off-by: WenyXu <wenymedia@gmail.com>
Co-authored-by: LFC <990479+MichaelScofield@users.noreply.github.com>
This commit is contained in:
Weny Xu
2025-09-25 11:38:33 +08:00
committed by GitHub
parent 6d0dd2540e
commit 07b9de620e
8 changed files with 298 additions and 108 deletions

View File

@@ -15,5 +15,5 @@
mod object_store;
mod store;
pub use object_store::ObjectStoreConfig;
pub use object_store::{ObjectStoreConfig, new_fs_object_store};
pub use store::StoreConfig;

View File

@@ -14,8 +14,7 @@
use common_base::secrets::SecretString;
use common_error::ext::BoxedError;
use object_store::config::FileConfig;
use object_store::services::{Azblob, Gcs, Oss, S3};
use object_store::services::{Azblob, Fs, Gcs, Oss, S3};
use object_store::util::{with_instrument_layers, with_retry_layers};
use object_store::{AzblobConnection, GcsConnection, ObjectStore, OssConnection, S3Connection};
use paste::paste;
@@ -164,18 +163,19 @@ pub struct ObjectStoreConfig {
}
/// Creates a new file system object store.
pub fn new_fs_object_store_in_current_dir() -> std::result::Result<ObjectStore, BoxedError> {
let data_home = ".";
let object_store =
object_store::factory::new_fs_object_store(data_home, &FileConfig::default())
.map_err(BoxedError::new)?;
pub fn new_fs_object_store(root: &str) -> std::result::Result<ObjectStore, BoxedError> {
let builder = Fs::default().root(root);
let object_store = ObjectStore::new(builder)
.context(error::InitBackendSnafu)
.map_err(BoxedError::new)?
.finish();
Ok(with_instrument_layers(object_store, false))
}
impl ObjectStoreConfig {
/// Builds the object store from the config.
pub fn build(&self) -> Result<ObjectStore, BoxedError> {
pub fn build(&self) -> Result<Option<ObjectStore>, BoxedError> {
let object_store = if self.enable_s3 {
let s3 = S3Connection::from(self.s3.clone());
common_telemetry::info!("Building object store with s3: {:?}", s3);
@@ -219,12 +219,6 @@ impl ObjectStoreConfig {
let object_store = object_store
.map(|object_store| with_instrument_layers(with_retry_layers(object_store), false));
match object_store {
Some(object_store) => Ok(object_store),
None => Ok(with_instrument_layers(
new_fs_object_store_in_current_dir()?,
false,
)),
}
Ok(object_store)
}
}

View File

@@ -23,7 +23,7 @@ use meta_srv::metasrv::BackendImpl;
use meta_srv::utils::etcd::create_etcd_client_with_tls;
use servers::tls::{TlsMode, TlsOption};
use crate::error::{EmptyStoreAddrsSnafu, UnsupportedMemoryBackendSnafu};
use crate::error::EmptyStoreAddrsSnafu;
#[derive(Debug, Default, Parser)]
pub struct StoreConfig {
@@ -154,9 +154,20 @@ impl StoreConfig {
.await
.map_err(BoxedError::new)?)
}
BackendImpl::MemoryStore => UnsupportedMemoryBackendSnafu
.fail()
.map_err(BoxedError::new),
#[cfg(not(test))]
BackendImpl::MemoryStore => {
use crate::error::UnsupportedMemoryBackendSnafu;
UnsupportedMemoryBackendSnafu
.fail()
.map_err(BoxedError::new)
}
#[cfg(test)]
BackendImpl::MemoryStore => {
use common_meta::kv_backend::memory::MemoryKvBackend;
Ok(Arc::new(MemoryKvBackend::default()) as _)
}
};
if self.store_key_prefix.is_empty() {
kvbackend

View File

@@ -313,6 +313,14 @@ pub enum Error {
location: Location,
source: common_meta::error::Error,
},
#[snafu(display("Failed to get current directory"))]
GetCurrentDir {
#[snafu(implicit)]
location: Location,
#[snafu(source)]
error: std::io::Error,
},
}
pub type Result<T> = std::result::Result<T, Error>;
@@ -362,7 +370,9 @@ impl ErrorExt for Error {
Error::BuildRuntime { source, .. } => source.status_code(),
Error::CacheRequired { .. } | Error::BuildCacheRegistry { .. } => StatusCode::Internal,
Error::CacheRequired { .. }
| Error::BuildCacheRegistry { .. }
| Error::GetCurrentDir { .. } => StatusCode::Internal,
Error::MetaClientInit { source, .. } => source.status_code(),
Error::TableNotFound { .. } => StatusCode::TableNotFound,
Error::SchemaNotFound { .. } => StatusCode::DatabaseNotFound,

View File

@@ -19,6 +19,7 @@ mod data;
mod database;
pub mod error;
mod metadata;
pub mod utils;
use async_trait::async_trait;
use clap::Parser;

View File

@@ -12,18 +12,15 @@
// See the License for the specific language governing permissions and
// limitations under the License.
use std::path::Path;
use async_trait::async_trait;
use clap::{Parser, Subcommand};
use common_error::ext::BoxedError;
use common_meta::snapshot::MetadataSnapshotManager;
use object_store::ObjectStore;
use snafu::OptionExt;
use object_store::{ObjectStore, Scheme};
use crate::Tool;
use crate::common::{ObjectStoreConfig, StoreConfig};
use crate::error::UnexpectedSnafu;
use crate::common::{ObjectStoreConfig, StoreConfig, new_fs_object_store};
use crate::utils::resolve_relative_path_with_current_dir;
/// Subcommand for metadata snapshot operations, including saving snapshots, restoring from snapshots, and viewing snapshot information.
#[derive(Subcommand)]
@@ -39,9 +36,9 @@ pub enum SnapshotCommand {
impl SnapshotCommand {
pub async fn build(&self) -> Result<Box<dyn Tool>, BoxedError> {
match self {
SnapshotCommand::Save(cmd) => cmd.build().await,
SnapshotCommand::Restore(cmd) => cmd.build().await,
SnapshotCommand::Info(cmd) => cmd.build().await,
SnapshotCommand::Save(cmd) => Ok(Box::new(cmd.build().await?)),
SnapshotCommand::Restore(cmd) => Ok(Box::new(cmd.build().await?)),
SnapshotCommand::Info(cmd) => Ok(Box::new(cmd.build().await?)),
}
}
}
@@ -58,38 +55,44 @@ pub struct SaveCommand {
/// The object store configuration.
#[clap(flatten)]
object_store: ObjectStoreConfig,
/// The name of the target snapshot file. we will add the file extension automatically.
#[clap(long, default_value = "metadata_snapshot")]
file_name: String,
/// The directory to store the snapshot file.
#[clap(long, default_value = "", alias = "output_dir")]
/// The path of the target snapshot file.
#[clap(
long,
default_value = "metadata_snapshot.metadata.fb",
alias = "file_name"
)]
file_path: String,
/// Specifies the root directory used for I/O operations.
#[clap(long, default_value = "/", alias = "output_dir")]
dir: String,
}
impl SaveCommand {
pub async fn build(&self) -> Result<Box<dyn Tool>, BoxedError> {
async fn build(&self) -> Result<MetaSnapshotTool, BoxedError> {
let kvbackend = self.store.build().await?;
let object_store = self.object_store.build().map_err(BoxedError::new)?;
let (object_store, file_path) = build_object_store_and_resolve_file_path(
self.object_store.clone(),
&self.dir,
&self.file_path,
)?;
let tool = MetaSnapshotTool {
inner: MetadataSnapshotManager::new(kvbackend, object_store),
path: self.dir.clone(),
file_name: self.file_name.clone(),
file_path,
};
Ok(Box::new(tool))
Ok(tool)
}
}
struct MetaSnapshotTool {
inner: MetadataSnapshotManager,
path: String,
file_name: String,
file_path: String,
}
#[async_trait]
impl Tool for MetaSnapshotTool {
async fn do_work(&self) -> std::result::Result<(), BoxedError> {
self.inner
.dump(&self.path, &self.file_name)
.dump(&self.file_path)
.await
.map_err(BoxedError::new)?;
Ok(())
@@ -109,38 +112,35 @@ pub struct RestoreCommand {
/// The object store config.
#[clap(flatten)]
object_store: ObjectStoreConfig,
/// The name of the target snapshot file.
#[clap(long, default_value = "metadata_snapshot.metadata.fb")]
file_name: String,
/// The directory to store the snapshot file.
#[clap(long, default_value = ".", alias = "input_dir")]
/// The path of the target snapshot file.
#[clap(
long,
default_value = "metadata_snapshot.metadata.fb",
alias = "file_name"
)]
file_path: String,
/// Specifies the root directory used for I/O operations.
#[clap(long, default_value = "/", alias = "input_dir")]
dir: String,
#[clap(long, default_value = "false")]
force: bool,
}
impl RestoreCommand {
pub async fn build(&self) -> Result<Box<dyn Tool>, BoxedError> {
async fn build(&self) -> Result<MetaRestoreTool, BoxedError> {
let kvbackend = self.store.build().await?;
let input_dir = &self.dir;
let file_path = Path::new(input_dir).join(&self.file_name);
let file_path = file_path
.to_str()
.with_context(|| UnexpectedSnafu {
msg: format!(
"Invalid file path, input dir: {}, file name: {}",
input_dir, &self.file_name
),
})
.map_err(BoxedError::new)?;
let object_store = self.object_store.build().map_err(BoxedError::new)?;
let (object_store, file_path) = build_object_store_and_resolve_file_path(
self.object_store.clone(),
&self.dir,
&self.file_path,
)
.map_err(BoxedError::new)?;
let tool = MetaRestoreTool::new(
MetadataSnapshotManager::new(kvbackend, object_store),
file_path.to_string(),
file_path,
self.force,
);
Ok(Box::new(tool))
Ok(tool)
}
}
@@ -204,11 +204,15 @@ pub struct InfoCommand {
/// The object store config.
#[clap(flatten)]
object_store: ObjectStoreConfig,
/// The name of the target snapshot file. we will add the file extension automatically.
#[clap(long, default_value = "metadata_snapshot")]
file_name: String,
/// The directory to store the snapshot file.
#[clap(long, default_value = ".", alias = "input_dir")]
/// The path of the target snapshot file.
#[clap(
long,
default_value = "metadata_snapshot.metadata.fb",
alias = "file_name"
)]
file_path: String,
/// Specifies the root directory used for I/O operations.
#[clap(long, default_value = "/", alias = "input_dir")]
dir: String,
/// The query string to filter the metadata.
#[clap(long, default_value = "*")]
@@ -244,24 +248,90 @@ impl Tool for MetaInfoTool {
}
impl InfoCommand {
pub async fn build(&self) -> Result<Box<dyn Tool>, BoxedError> {
let object_store = self.object_store.build().map_err(BoxedError::new)?;
let file_path = Path::new(&self.dir).join(&self.file_name);
let file_path = file_path
.to_str()
.with_context(|| UnexpectedSnafu {
msg: format!(
"Invalid file path, input dir: {}, file name: {}",
&self.dir, &self.file_name
),
})
.map_err(BoxedError::new)?;
async fn build(&self) -> Result<MetaInfoTool, BoxedError> {
let (object_store, file_path) = build_object_store_and_resolve_file_path(
self.object_store.clone(),
&self.dir,
&self.file_path,
)?;
let tool = MetaInfoTool {
inner: object_store,
file_path: file_path.to_string(),
file_path,
inspect_key: self.inspect_key.clone(),
limit: self.limit,
};
Ok(Box::new(tool))
Ok(tool)
}
}
/// Builds the object store and resolves the file path.
fn build_object_store_and_resolve_file_path(
object_store: ObjectStoreConfig,
fs_root: &str,
file_path: &str,
) -> Result<(ObjectStore, String), BoxedError> {
let object_store = object_store.build().map_err(BoxedError::new)?;
let object_store = match object_store {
Some(object_store) => object_store,
None => new_fs_object_store(fs_root)?,
};
let file_path = if matches!(object_store.info().scheme(), Scheme::Fs) {
resolve_relative_path_with_current_dir(file_path).map_err(BoxedError::new)?
} else {
file_path.to_string()
};
Ok((object_store, file_path))
}
#[cfg(test)]
mod tests {
use std::env;
use clap::Parser;
use crate::metadata::snapshot::RestoreCommand;
#[tokio::test]
async fn test_cmd_resolve_file_path() {
common_telemetry::init_default_ut_logging();
let cmd = RestoreCommand::parse_from([
"",
"--file_name",
"metadata_snapshot.metadata.fb",
"--backend",
"memory-store",
"--store-addrs",
"memory://",
]);
let tool = cmd.build().await.unwrap();
let current_dir = env::current_dir().unwrap();
let file_path = current_dir.join("metadata_snapshot.metadata.fb");
assert_eq!(tool.file_path, file_path.to_string_lossy().to_string());
let cmd = RestoreCommand::parse_from([
"",
"--file_name",
"metadata_snapshot.metadata.fb",
"--backend",
"memory-store",
"--store-addrs",
"memory://",
]);
let tool = cmd.build().await.unwrap();
assert_eq!(tool.file_path, file_path.to_string_lossy().to_string());
let cmd = RestoreCommand::parse_from([
"",
"--file_name",
"metadata_snapshot.metadata.fb",
"--backend",
"memory-store",
"--store-addrs",
"memory://",
]);
let tool = cmd.build().await.unwrap();
assert_eq!(tool.file_path, file_path.to_string_lossy().to_string());
}
}

93
src/cli/src/utils.rs Normal file
View File

@@ -0,0 +1,93 @@
// Copyright 2023 Greptime Team
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
use std::env;
use std::path::Path;
use snafu::ResultExt;
use crate::error::{GetCurrentDirSnafu, Result};
/// Resolves the relative path to an absolute path.
pub fn resolve_relative_path(current_dir: impl AsRef<Path>, path_str: &str) -> String {
let path = Path::new(path_str);
if path.is_relative() {
let path = current_dir.as_ref().join(path);
common_telemetry::debug!("Resolved relative path: {}", path.to_string_lossy());
path.to_string_lossy().to_string()
} else {
path_str.to_string()
}
}
/// Resolves the relative path to an absolute path.
pub fn resolve_relative_path_with_current_dir(path_str: &str) -> Result<String> {
let current_dir = env::current_dir().context(GetCurrentDirSnafu)?;
Ok(resolve_relative_path(current_dir, path_str))
}
#[cfg(test)]
mod tests {
use std::env;
use std::path::PathBuf;
use super::*;
#[test]
fn test_resolve_relative_path_absolute() {
let abs_path = if cfg!(windows) {
"C:\\foo\\bar"
} else {
"/foo/bar"
};
let current_dir = PathBuf::from("/tmp");
let result = resolve_relative_path(&current_dir, abs_path);
assert_eq!(result, abs_path);
}
#[test]
fn test_resolve_relative_path_relative() {
let current_dir = PathBuf::from("/tmp");
let rel_path = "foo/bar";
let expected = "/tmp/foo/bar";
let result = resolve_relative_path(&current_dir, rel_path);
// On Windows, the separator is '\', so normalize for comparison
if cfg!(windows) {
assert!(result.ends_with("foo\\bar"));
assert!(result.contains("\\tmp\\") || result.contains("C:\\tmp\\"));
} else {
assert_eq!(result, expected);
}
}
#[test]
fn test_resolve_relative_path_with_current_dir_absolute() {
let abs_path = if cfg!(windows) {
"C:\\foo\\bar"
} else {
"/foo/bar"
};
let result = resolve_relative_path_with_current_dir(abs_path).unwrap();
assert_eq!(result, abs_path);
}
#[test]
fn test_resolve_relative_path_with_current_dir_relative() {
let rel_path = "foo/bar";
let current_dir = env::current_dir().unwrap();
let expected = current_dir.join(rel_path).to_string_lossy().to_string();
let result = resolve_relative_path_with_current_dir(rel_path).unwrap();
assert_eq!(result, expected);
}
}

View File

@@ -16,7 +16,7 @@ pub mod file;
use std::borrow::Cow;
use std::fmt::{Display, Formatter};
use std::path::{Path, PathBuf};
use std::path::Path;
use std::time::Instant;
use common_telemetry::info;
@@ -232,23 +232,34 @@ impl MetadataSnapshotManager {
}
/// Dumps the metadata to the backup file.
pub async fn dump(&self, path: &str, filename_str: &str) -> Result<(String, u64)> {
pub async fn dump(&self, file_path_str: &str) -> Result<(String, u64)> {
let format = FileFormat::FlexBuffers;
let filename = FileName::new(
filename_str.to_string(),
FileExtension {
format,
data_type: DataType::Metadata,
},
);
let file_path_buf = [path, filename.to_string().as_str()]
.iter()
.collect::<PathBuf>();
let file_path = file_path_buf
.to_str()
.with_context(|| InvalidFileNameSnafu {
reason: format!("Invalid file path: {}, filename: {}", path, filename_str),
let path = Path::new(file_path_str)
.parent()
.context(InvalidFilePathSnafu {
file_path: file_path_str,
})?;
let raw_file_name = Path::new(file_path_str)
.file_name()
.and_then(|s| s.to_str())
.context(InvalidFilePathSnafu {
file_path: file_path_str,
})?;
let parsed_file_name = FileName::try_from(raw_file_name).unwrap_or_else(|_| {
FileName::new(
raw_file_name.to_string(),
FileExtension {
format,
data_type: DataType::Metadata,
},
)
});
let file_path = path.join(parsed_file_name.to_string());
let file_path = file_path.to_str().context(InvalidFilePathSnafu {
file_path: file_path_str,
})?;
// Ensure the file does not exist
ensure!(
!self
@@ -295,7 +306,7 @@ impl MetadataSnapshotManager {
now.elapsed()
);
Ok((filename.to_string(), num_keyvalues as u64))
Ok((parsed_file_name.to_string(), num_keyvalues as u64))
}
fn format_output(key: Cow<'_, str>, value: Cow<'_, str>) -> String {
@@ -411,19 +422,19 @@ mod tests {
}
let dump_path = temp_path.join("snapshot");
manager
.dump(
&dump_path.as_path().display().to_string(),
"metadata_snapshot",
)
.dump(&format!(
"{}/metadata_snapshot",
&dump_path.as_path().display().to_string()
))
.await
.unwrap();
// Clean up the kv backend
kv_backend.clear();
let err = manager
.dump(
&dump_path.as_path().display().to_string(),
"metadata_snapshot",
)
.dump(&format!(
"{}/metadata_snapshot.metadata.fb",
&dump_path.as_path().display().to_string()
))
.await
.unwrap_err();
assert_matches!(err, Error::Unexpected { .. });