Restructure local object store layout

This commit is contained in:
Bryan Helmkamp 2026-04-14 13:08:07 -04:00
parent 2047e490d8
commit 74dfb9f652
9 changed files with 226 additions and 59 deletions

View file

@ -16,12 +16,15 @@ use dialoguer::{MultiSelect, Select};
use fabro_api::types::{CreateSecretRequest, SecretType as ApiSecretType};
use fabro_auth::{AuthCredential, AuthMethod, codex_oauth_config, credential_id_for};
use fabro_config::user::SETTINGS_CONFIG_FILENAME;
use fabro_config::{Storage, envfile, legacy_env};
use fabro_config::{ResolveError, Storage, envfile, legacy_env};
use fabro_model::Provider;
use fabro_types::settings::CliSettings;
use fabro_server::serve;
use fabro_store::ArtifactStore;
use fabro_types::settings::{CliSettings, SettingsLayer};
use fabro_types::settings::cli::{CliLayer, OutputFormat};
use fabro_util::printer::Printer;
use fabro_util::terminal::Styles;
use fabro_util::version::FABRO_VERSION;
use fabro_util::{dev_token, session_secret};
use futures::future::BoxFuture;
use rand::Rng;
@ -1190,6 +1193,29 @@ async fn persist_install_outputs(
.await
}
fn render_server_resolve_errors(errors: Vec<ResolveError>) -> anyhow::Error {
anyhow::anyhow!(
"failed to resolve server settings:\n{}",
errors
.into_iter()
.map(|error| error.to_string())
.collect::<Vec<_>>()
.join("\n")
)
}
async fn write_artifact_store_metadata(
settings: &SettingsLayer,
fabro_version: &str,
) -> Result<()> {
let resolved =
fabro_config::resolve_server_from_file(settings).map_err(render_server_resolve_errors)?;
let (object_store, prefix) = serve::build_artifact_object_store(&resolved)?;
let artifact_store = ArtifactStore::new(object_store, prefix);
artifact_store.write_metadata(fabro_version).await?;
Ok(())
}
async fn persist_install_outputs_with_settings(
storage_dir: &Path,
server_env_secrets: &[(String, String)],
@ -1505,6 +1531,18 @@ async fn run_install_inner(
server_was_running,
)
.await?;
let install_settings = user_config::apply_storage_dir_override(
fabro_config::parse_settings_layer(&settings_toml)
.context("failed to parse generated settings.toml")?,
args.storage_dir.as_deref(),
);
if let Err(err) = write_artifact_store_metadata(&install_settings, FABRO_VERSION).await {
fabro_util::printerr!(
printer,
" {} failed to write artifact store metadata: {err}",
s.yellow.apply_to("Warning:")
);
}
fabro_util::printerr!(
printer,
" {} Saved {} runtime secrets to {}",
@ -2118,6 +2156,38 @@ client_id = "client-id"
);
}
#[tokio::test]
async fn write_artifact_store_metadata_creates_marker_in_resolved_store() {
let dir = tempfile::tempdir().unwrap();
let settings = fabro_config::parse_settings_layer(&format!(
r#"
_version = 1
[server.storage]
root = "{}"
"#,
dir.path().display()
))
.unwrap();
write_artifact_store_metadata(&settings, "test-version")
.await
.unwrap();
let value: serde_json::Value = serde_json::from_str(
&std::fs::read_to_string(
dir.path()
.join("objects")
.join("artifacts")
.join("store-metadata.json"),
)
.unwrap(),
)
.unwrap();
assert_eq!(value["fabro_version"], "test-version");
assert!(value["created_at"].as_str().is_some());
}
#[test]
fn non_interactive_source_rejects_missing_scripted_inputs() {
let args = install_args(true, InstallNonInteractiveArgs::default());

View file

@ -46,7 +46,7 @@ max_concurrent_runs = 5
[server.artifacts]
provider = "local"
prefix = "artifacts"
prefix = ""
[server.slatedb]
provider = "local"

View file

@ -166,7 +166,7 @@ fn resolve_artifacts(
provider,
layer.and_then(|artifacts| artifacts.local.as_ref()),
layer.and_then(|artifacts| artifacts.s3.as_ref()),
storage_root,
&object_store_default_root(storage_root, "artifacts"),
"server.artifacts",
errors,
),
@ -190,7 +190,7 @@ fn resolve_slatedb(
provider,
layer.and_then(|slatedb| slatedb.local.as_ref()),
layer.and_then(|slatedb| slatedb.s3.as_ref()),
storage_root,
&object_store_default_root(storage_root, "slatedb"),
"server.slatedb",
errors,
),
@ -236,6 +236,12 @@ fn resolve_object_store(
}
}
fn object_store_default_root(storage_root: &InterpString, domain: &str) -> InterpString {
let root = storage_root.as_source();
let root = root.trim_end_matches('/');
InterpString::parse(&format!("{root}/objects/{domain}"))
}
fn resolve_integrations(layer: Option<&ServerIntegrationsLayer>) -> ServerIntegrationsSettings {
ServerIntegrationsSettings {
github: layer

View file

@ -58,13 +58,18 @@ impl Storage {
}
#[must_use]
pub fn store_dir(&self) -> PathBuf {
self.root.join("store")
pub fn objects_dir(&self) -> PathBuf {
self.root.join("objects")
}
#[must_use]
pub fn artifact_store_dir(&self) -> PathBuf {
self.root.join("artifacts")
pub fn slatedb_dir(&self) -> PathBuf {
self.objects_dir().join("slatedb")
}
#[must_use]
pub fn artifacts_dir(&self) -> PathBuf {
self.objects_dir().join("artifacts")
}
}
@ -164,12 +169,16 @@ mod tests {
std::path::Path::new("/tmp/fabro-data/vaults/default/secrets.json")
);
assert_eq!(
storage.store_dir(),
std::path::Path::new("/tmp/fabro-data/store")
storage.objects_dir(),
std::path::Path::new("/tmp/fabro-data/objects")
);
assert_eq!(
storage.artifact_store_dir(),
std::path::Path::new("/tmp/fabro-data/artifacts")
storage.slatedb_dir(),
std::path::Path::new("/tmp/fabro-data/objects/slatedb")
);
assert_eq!(
storage.artifacts_dir(),
std::path::Path::new("/tmp/fabro-data/objects/artifacts")
);
assert_eq!(
storage.server_state().record_path(),

View file

@ -36,12 +36,30 @@ fn resolves_server_defaults_from_empty_settings() {
ObjectStoreSettings::Local { root } => {
assert_eq!(
root.as_source(),
Home::from_env().storage_dir().to_string_lossy()
Home::from_env()
.storage_dir()
.join("objects")
.join("artifacts")
.to_string_lossy()
);
}
ObjectStoreSettings::S3 { .. } => panic!("expected local artifact store by default"),
}
assert_eq!(settings.artifacts.prefix.as_source(), "artifacts");
assert_eq!(settings.artifacts.prefix.as_source(), "");
match settings.slatedb.store {
ObjectStoreSettings::Local { root } => {
assert_eq!(
root.as_source(),
Home::from_env()
.storage_dir()
.join("objects")
.join("slatedb")
.to_string_lossy()
);
}
ObjectStoreSettings::S3 { .. } => panic!("expected local slatedb store by default"),
}
}
#[test]

View file

@ -143,7 +143,7 @@ fn use_in_memory_store() -> bool {
)
}
fn build_object_store_with_preference(
fn build_local_object_store_with_preference(
store_path: &Path,
use_in_memory: bool,
) -> anyhow::Result<Arc<dyn ObjectStore>> {
@ -155,8 +155,33 @@ fn build_object_store_with_preference(
Ok(Arc::new(LocalFileSystem::new_with_prefix(store_path)?))
}
fn build_object_store(store_path: &Path) -> anyhow::Result<Arc<dyn ObjectStore>> {
build_object_store_with_preference(store_path, use_in_memory_store())
fn build_object_store_from_settings(
settings: &ObjectStoreSettings,
) -> anyhow::Result<Arc<dyn ObjectStore>> {
if use_in_memory_store() {
return Ok(Arc::new(InMemory::new()));
}
match settings {
ObjectStoreSettings::Local { root } => {
build_local_object_store_with_preference(&resolve_interp_path(root)?, false)
}
ObjectStoreSettings::S3 {
bucket,
region,
endpoint,
path_style,
} => {
let mut builder = AmazonS3Builder::from_env()
.with_bucket_name(resolve_interp(bucket)?)
.with_region(resolve_interp(region)?)
.with_virtual_hosted_style_request(!*path_style);
if let Some(endpoint) = endpoint.as_ref() {
builder = builder.with_endpoint(resolve_interp(endpoint)?);
}
Ok(Arc::new(builder.build()?))
}
}
}
fn resolve_server_settings(file: &SettingsLayer) -> anyhow::Result<ResolvedServerSettings> {
@ -228,39 +253,20 @@ fn resolve_interp_path(value: &InterpString) -> anyhow::Result<PathBuf> {
Ok(PathBuf::from(resolve_interp(value)?))
}
fn build_artifact_object_store(
pub fn build_artifact_object_store(
settings: &ResolvedServerSettings,
) -> anyhow::Result<(Arc<dyn ObjectStore>, String)> {
let prefix = resolve_interp(&settings.artifacts.prefix)?;
let object_store = build_object_store_from_settings(&settings.artifacts.store)?;
Ok((object_store, prefix))
}
if use_in_memory_store() {
return Ok((Arc::new(InMemory::new()), prefix));
}
match &settings.artifacts.store {
ObjectStoreSettings::Local { root } => {
let root = resolve_interp_path(root)?;
std::fs::create_dir_all(&root)?;
let object_store = Arc::new(LocalFileSystem::new_with_prefix(&root)?);
Ok((object_store, prefix))
}
ObjectStoreSettings::S3 {
bucket,
region,
endpoint,
path_style,
} => {
let mut builder = AmazonS3Builder::from_env()
.with_bucket_name(resolve_interp(bucket)?)
.with_region(resolve_interp(region)?)
.with_virtual_hosted_style_request(!*path_style);
if let Some(endpoint) = endpoint.as_ref() {
builder = builder.with_endpoint(resolve_interp(endpoint)?);
}
let object_store = Arc::new(builder.build()?);
Ok((object_store, prefix))
}
}
fn build_slatedb_store(
settings: &ResolvedServerSettings,
) -> anyhow::Result<(Arc<dyn ObjectStore>, String, Duration)> {
let prefix = resolve_interp(&settings.slatedb.prefix)?;
let object_store = build_object_store_from_settings(&settings.slatedb.store)?;
Ok((object_store, prefix, settings.slatedb.flush_interval))
}
/// Start the HTTP API server.
@ -310,12 +316,12 @@ where
};
let web_enabled = router_web_enabled(&resolved_server_settings);
let store_path = storage.store_dir();
let object_store = build_object_store(&store_path)?;
let (object_store, slatedb_prefix, flush_interval) =
build_slatedb_store(&resolved_server_settings)?;
let store = Arc::new(fabro_store::Database::new(
Arc::clone(&object_store),
"",
Duration::from_millis(1),
object_store,
slatedb_prefix,
flush_interval,
));
let (artifact_object_store, artifact_prefix) =
build_artifact_object_store(&resolved_server_settings)?;
@ -653,6 +659,7 @@ fn server_bind_title(bind: &Bind) -> String {
#[cfg(test)]
mod tests {
use std::path::PathBuf;
use std::time::Duration;
use fabro_config::parse_settings_layer;
use fabro_types::settings::SettingsLayer;
@ -660,8 +667,9 @@ mod tests {
use super::{
ServeArgs, ServerTitlePhase, apply_runtime_settings, bind_tcp_host_with_fallback,
build_object_store_with_preference, resolve_bind_request_from_settings,
resolve_server_settings, router_web_enabled, server_bind_title, server_title,
build_local_object_store_with_preference, build_slatedb_store,
resolve_bind_request_from_settings, resolve_server_settings, router_web_enabled,
server_bind_title, server_title,
};
use crate::bind::{Bind, BindRequest};
@ -851,7 +859,7 @@ strategy = "token"
let temp = tempfile::tempdir().unwrap();
let store_path = temp.path().join("store");
let disk_store = build_object_store_with_preference(&store_path, false)
let disk_store = build_local_object_store_with_preference(&store_path, false)
.expect("disk-backed store should build");
assert!(
store_path.exists(),
@ -860,7 +868,7 @@ strategy = "token"
drop(disk_store);
let mem_path = temp.path().join("memory-store");
let mem_store = build_object_store_with_preference(&mem_path, true)
let mem_store = build_local_object_store_with_preference(&mem_path, true)
.expect("memory-backed store should build");
assert!(
!mem_path.exists(),
@ -869,6 +877,29 @@ strategy = "token"
drop(mem_store);
}
#[test]
fn build_slatedb_store_uses_configured_local_root() {
let temp = tempfile::tempdir().unwrap();
let root = temp.path().join("custom-slatedb");
let settings = parse_settings(&format!(
r#"
_version = 1
[server.slatedb.local]
root = "{}"
"#,
root.display()
));
let resolved = resolve_server_settings(&settings).expect("settings should resolve");
let (_object_store, prefix, flush_interval) =
build_slatedb_store(&resolved).expect("slatedb store should build");
assert!(root.exists(), "configured SlateDB root should be created");
assert_eq!(prefix, "");
assert_eq!(flush_interval, Duration::from_millis(1));
}
#[tokio::test]
async fn tcp_host_request_uses_preferred_port_when_available() {
let preferred = std::net::TcpListener::bind("127.0.0.1:0").unwrap();

View file

@ -1,6 +1,7 @@
use std::sync::Arc;
use bytes::Bytes;
use chrono::Utc;
use fabro_types::RunId;
use futures::StreamExt;
use object_store::ObjectStore;
@ -146,6 +147,19 @@ impl ArtifactStore {
Ok(())
}
pub async fn write_metadata(&self, fabro_version: &str) -> Result<()> {
let path = parse_object_path(&self.prefixed_raw("store-metadata.json"))?;
let body = serde_json::to_vec(&serde_json::json!({
"created_at": Utc::now().to_rfc3339(),
"fabro_version": fabro_version,
}))
.map_err(|err| Error::Other(format!("artifact metadata serialization failed: {err}")))?;
self.object_store
.put(&path, Bytes::from(body).into())
.await?;
Ok(())
}
fn run_prefix(&self, run_id: &RunId) -> Result<ObjectPath> {
parse_object_path(&self.prefixed_raw(&run_id.to_string()))
}
@ -286,6 +300,25 @@ mod tests {
ArtifactStore::new(object_store, "artifacts")
}
#[tokio::test]
async fn write_metadata_persists_store_marker() {
let object_store: Arc<dyn ObjectStore> = Arc::new(InMemory::new());
let store = ArtifactStore::new(object_store.clone(), "artifacts");
store.write_metadata("test-version").await.unwrap();
let bytes = object_store
.get(&ObjectPath::from("artifacts/store-metadata.json"))
.await
.unwrap()
.bytes()
.await
.unwrap();
let value: serde_json::Value = serde_json::from_slice(&bytes).unwrap();
assert_eq!(value["fabro_version"], "test-version");
assert!(value["created_at"].as_str().is_some());
}
#[tokio::test]
async fn round_trips_unicode_nodes_and_nested_filenames() {
let store = test_store();

View file

@ -48,7 +48,7 @@ impl Database {
}
fn shared_db_prefix(&self) -> String {
format!("{}slatedb", self.base_prefix)
self.base_prefix.clone()
}
async fn open_db(&self) -> Result<slatedb::Db> {
@ -399,7 +399,7 @@ mod tests {
let remaining = store.list_runs(&ListRunsQuery::default()).await.unwrap();
assert_eq!(remaining.len(), 1);
assert_eq!(remaining[0].run_id, test_run_id("run-2"));
assert!(!list_paths(object_store, "runs/slatedb").await.is_empty());
assert!(!list_paths(object_store, "runs/").await.is_empty());
}
#[tokio::test]

View file

@ -388,7 +388,7 @@ pub enum ObjectStoreProvider {
#[serde(deny_unknown_fields)]
pub struct ObjectStoreLocalLayer {
/// Overrides the default root, which otherwise falls back to
/// `server.storage.root`.
/// `{server.storage.root}/objects/{domain}`.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub root: Option<InterpString>,
}