diff --git a/objectstore-server/src/auth/context.rs b/objectstore-server/src/auth/context.rs index 2569fcfe..dc6fb9e3 100644 --- a/objectstore-server/src/auth/context.rs +++ b/objectstore-server/src/auth/context.rs @@ -1,7 +1,7 @@ use std::collections::{BTreeMap, HashSet}; use jsonwebtoken::{Algorithm, DecodingKey, Header, TokenData, Validation, decode, decode_header}; -use objectstore_service::id::ObjectId; +use objectstore_service::id::ObjectContext; use secrecy::ExposeSecret; use serde::{Deserialize, Serialize}; use thiserror::Error; @@ -33,9 +33,13 @@ pub enum Permission { #[non_exhaustive] pub struct AuthContext { /// The objectstore usecase that this request may act on. + /// + /// See also: [`ObjectContext::usecase`]. pub usecase: String, - /// The scope elements that this request may act on. See also: [`ObjectId::scopes`]. + /// The scope elements that this request may act on. + /// + /// See also: [`ObjectContext::scopes`]. pub scopes: BTreeMap, /// The permissions that this request has been granted. @@ -188,8 +192,12 @@ impl AuthContext { }) } - fn fail_if_enforced(&self, perm: &Permission, id: &ObjectId) -> Result<(), AuthError> { - tracing::debug!(?self, ?perm, ?id, "Authorization failed"); + fn fail_if_enforced( + &self, + perm: &Permission, + context: &ObjectContext, + ) -> Result<(), AuthError> { + tracing::debug!(?self, ?perm, ?context, "Authorization failed"); if self.enforce { return Err(AuthError::NotPermitted); } @@ -201,19 +209,23 @@ impl AuthContext { /// /// The passed-in `perm` is checked against this `AuthContext`'s `permissions`. If it is not /// present, then the operation is not authorized. - pub fn assert_authorized(&self, perm: Permission, id: &ObjectId) -> Result<(), AuthError> { - if !self.permissions.contains(&perm) || self.usecase != id.usecase { - self.fail_if_enforced(&perm, id)?; + pub fn assert_authorized( + &self, + perm: Permission, + context: &ObjectContext, + ) -> Result<(), AuthError> { + if !self.permissions.contains(&perm) || self.usecase != context.usecase { + self.fail_if_enforced(&perm, context)?; } - for scope in &id.scopes { + for scope in &context.scopes { let authorized = match self.scopes.get(scope.name()) { Some(StringOrWildcard::String(s)) => s == scope.value(), Some(StringOrWildcard::Wildcard) => true, None => false, }; if !authorized { - self.fail_if_enforced(&perm, id)?; + self.fail_if_enforced(&perm, context)?; } } @@ -408,14 +420,13 @@ MC4CAQAwBQYDK2VwBCIEIKwVoE4TmTfWoqH3HgLVsEcHs9PHNe+ar/Hp6e4To8pK Ok(()) } - fn sample_object_path(org: &str, project: &str) -> ObjectId { - ObjectId { + fn sample_object_context(org: &str, project: &str) -> ObjectContext { + ObjectContext { usecase: "attachments".into(), scopes: Scopes::from_iter([ Scope::create("org", org).unwrap(), Scope::create("project", project).unwrap(), ]), - key: "abcde".into(), } } @@ -425,7 +436,7 @@ MC4CAQAwBQYDK2VwBCIEIKwVoE4TmTfWoqH3HgLVsEcHs9PHNe+ar/Hp6e4To8pK #[test] fn test_assert_authorized_exact_scope_allowed() -> Result<(), AuthError> { let auth_context = sample_auth_context("123", "456", max_permission()); - let object = sample_object_path("123", "456"); + let object = sample_object_context("123", "456"); auth_context.assert_authorized(Permission::ObjectRead, &object)?; @@ -438,7 +449,7 @@ MC4CAQAwBQYDK2VwBCIEIKwVoE4TmTfWoqH3HgLVsEcHs9PHNe+ar/Hp6e4To8pK #[test] fn test_assert_authorized_wildcard_project_allowed() -> Result<(), AuthError> { let auth_context = sample_auth_context("123", "*", max_permission()); - let object = sample_object_path("123", "456"); + let object = sample_object_context("123", "456"); auth_context.assert_authorized(Permission::ObjectRead, &object)?; @@ -451,10 +462,9 @@ MC4CAQAwBQYDK2VwBCIEIKwVoE4TmTfWoqH3HgLVsEcHs9PHNe+ar/Hp6e4To8pK #[test] fn test_assert_authorized_org_only_path_allowed() -> Result<(), AuthError> { let auth_context = sample_auth_context("123", "456", max_permission()); - let object = ObjectId { + let object = ObjectContext { usecase: "attachments".into(), scopes: Scopes::from_iter([Scope::create("org", "123").unwrap()]), - key: "abcde".into(), }; auth_context.assert_authorized(Permission::ObjectRead, &object)?; @@ -471,13 +481,13 @@ MC4CAQAwBQYDK2VwBCIEIKwVoE4TmTfWoqH3HgLVsEcHs9PHNe+ar/Hp6e4To8pK #[test] fn test_assert_authorized_scope_mismatch_fails() -> Result<(), AuthError> { let auth_context = sample_auth_context("123", "456", max_permission()); - let object = sample_object_path("123", "999"); + let object = sample_object_context("123", "999"); let result = auth_context.assert_authorized(Permission::ObjectRead, &object); assert_eq!(result, Err(AuthError::NotPermitted)); let auth_context = sample_auth_context("123", "456", max_permission()); - let object = sample_object_path("999", "456"); + let object = sample_object_context("999", "456"); let result = auth_context.assert_authorized(Permission::ObjectRead, &object); assert_eq!(result, Err(AuthError::NotPermitted)); @@ -489,7 +499,7 @@ MC4CAQAwBQYDK2VwBCIEIKwVoE4TmTfWoqH3HgLVsEcHs9PHNe+ar/Hp6e4To8pK fn test_assert_authorized_wrong_usecase_fails() -> Result<(), AuthError> { let mut auth_context = sample_auth_context("123", "456", max_permission()); auth_context.usecase = "debug-files".into(); - let object = sample_object_path("123", "456"); + let object = sample_object_context("123", "456"); let result = auth_context.assert_authorized(Permission::ObjectRead, &object); assert_eq!(result, Err(AuthError::NotPermitted)); @@ -501,7 +511,7 @@ MC4CAQAwBQYDK2VwBCIEIKwVoE4TmTfWoqH3HgLVsEcHs9PHNe+ar/Hp6e4To8pK fn test_assert_authorized_auth_context_missing_permission_fails() -> Result<(), AuthError> { let auth_context = sample_auth_context("123", "456", HashSet::from([Permission::ObjectRead])); - let object = sample_object_path("123", "456"); + let object = sample_object_context("123", "456"); let result = auth_context.assert_authorized(Permission::ObjectWrite, &object); assert_eq!(result, Err(AuthError::NotPermitted)); @@ -515,7 +525,7 @@ MC4CAQAwBQYDK2VwBCIEIKwVoE4TmTfWoqH3HgLVsEcHs9PHNe+ar/Hp6e4To8pK let mut auth_context = sample_auth_context("123", "456", HashSet::from([Permission::ObjectRead])); // Object's scope is not covered by the auth context - let object = sample_object_path("999", "999"); + let object = sample_object_context("999", "999"); // Auth fails for two reasons, but because enforcement is off, it should not return an error auth_context.enforce = false; diff --git a/objectstore-server/src/auth/service.rs b/objectstore-server/src/auth/service.rs index 608c0a24..9845b9f7 100644 --- a/objectstore-server/src/auth/service.rs +++ b/objectstore-server/src/auth/service.rs @@ -1,6 +1,6 @@ use axum::extract::FromRequestParts; use axum::http::{StatusCode, header, request::Parts}; -use objectstore_service::id::ObjectId; +use objectstore_service::id::{ObjectContext, ObjectId}; use objectstore_service::{PayloadStream, StorageService}; use objectstore_types::Metadata; @@ -39,27 +39,30 @@ pub struct AuthAwareService { } impl AuthAwareService { - fn assert_authorized(&self, perm: Permission, id: &ObjectId) -> anyhow::Result<()> { + fn assert_authorized(&self, perm: Permission, context: &ObjectContext) -> anyhow::Result<()> { if self.enforce { - let context = self + let auth = self .context .as_ref() .ok_or(AuthError::VerificationFailure)?; - context.assert_authorized(perm, id)?; + auth.assert_authorized(perm, context)?; } Ok(()) } - /// Auth-aware wrapper around [`StorageService::put_object`]. - pub async fn put_object( + /// Auth-aware wrapper around [`StorageService::insert_object`]. + pub async fn insert_object( &self, - id: ObjectId, + context: ObjectContext, + key: Option, metadata: &Metadata, stream: PayloadStream, ) -> anyhow::Result { - self.assert_authorized(Permission::ObjectWrite, &id)?; - self.service.put_object(id, metadata, stream).await + self.assert_authorized(Permission::ObjectWrite, &context)?; + self.service + .insert_object(context, key, metadata, stream) + .await } /// Auth-aware wrapper around [`StorageService::get_object`]. @@ -67,13 +70,13 @@ impl AuthAwareService { &self, id: &ObjectId, ) -> anyhow::Result> { - self.assert_authorized(Permission::ObjectRead, id)?; + self.assert_authorized(Permission::ObjectRead, id.context())?; self.service.get_object(id).await } /// Auth-aware wrapper around [`StorageService::delete_object`]. pub async fn delete_object(&self, id: &ObjectId) -> anyhow::Result<()> { - self.assert_authorized(Permission::ObjectDelete, id)?; + self.assert_authorized(Permission::ObjectDelete, id.context())?; self.service.delete_object(id).await } } diff --git a/objectstore-server/src/endpoints/helpers.rs b/objectstore-server/src/endpoints/helpers.rs index 9a58cb02..67482548 100644 --- a/objectstore-server/src/endpoints/helpers.rs +++ b/objectstore-server/src/endpoints/helpers.rs @@ -1,9 +1,17 @@ -use objectstore_service::id::ObjectId; +use objectstore_service::id::{ObjectContext, ObjectId}; -pub fn populate_sentry_scope(path: &ObjectId) { +pub fn populate_sentry_context(context: &ObjectContext) { sentry::configure_scope(|s| { - s.set_tag("usecase", &path.usecase); - s.set_extra("scope", path.scopes.as_storage_path().to_string().into()); - s.set_extra("key", path.key.clone().into()); + s.set_tag("usecase", &context.usecase); + for scope in &context.scopes { + s.set_tag(&format!("scope.{}", scope.name()), scope.value()); + } + }); +} + +pub fn populate_sentry_object_id(id: &ObjectId) { + populate_sentry_context(id.context()); + sentry::configure_scope(|s| { + s.set_extra("key", id.key().into()); }); } diff --git a/objectstore-server/src/endpoints/objects.rs b/objectstore-server/src/endpoints/objects.rs index 55f859af..a533b5fd 100644 --- a/objectstore-server/src/endpoints/objects.rs +++ b/objectstore-server/src/endpoints/objects.rs @@ -10,7 +10,7 @@ use axum::response::{IntoResponse, Response}; use axum::routing; use axum::{Json, Router}; use futures_util::{StreamExt, TryStreamExt}; -use objectstore_service::id::{ObjectId, Scope, Scopes}; +use objectstore_service::id::{ObjectContext, ObjectId, Scope, Scopes}; use objectstore_types::Metadata; use serde::{Deserialize, Serialize, de}; @@ -48,17 +48,19 @@ async fn objects_post( headers: HeaderMap, body: Body, ) -> ApiResult { - let id = params.create_object_id(); - helpers::populate_sentry_scope(&id); + let context = params.into_context(); + helpers::populate_sentry_context(&context); let mut metadata = Metadata::from_headers(&headers, "").context("extracting metadata from headers")?; metadata.time_created = Some(SystemTime::now()); let stream = body.into_data_stream().map_err(io::Error::other).boxed(); - let response_path = service.put_object(id, &metadata, stream).await?; + let response_id = service + .insert_object(context, None, &metadata, stream) + .await?; let response = Json(InsertObjectResponse { - key: response_path.key.to_string(), + key: response_id.key().to_string(), }); Ok((StatusCode::CREATED, response).into_response()) @@ -69,7 +71,7 @@ async fn object_get( Path(params): Path, ) -> ApiResult { let id = params.into_object_id(); - helpers::populate_sentry_scope(&id); + helpers::populate_sentry_object_id(&id); let Some((metadata, stream)) = service.get_object(&id).await? else { return Ok(StatusCode::NOT_FOUND.into_response()); @@ -86,7 +88,7 @@ async fn object_head( Path(params): Path, ) -> ApiResult { let id = params.into_object_id(); - helpers::populate_sentry_scope(&id); + helpers::populate_sentry_object_id(&id); let Some((metadata, _stream)) = service.get_object(&id).await? else { return Ok(StatusCode::NOT_FOUND.into_response()); @@ -106,16 +108,20 @@ async fn object_put( body: Body, ) -> ApiResult { let id = params.into_object_id(); - helpers::populate_sentry_scope(&id); + helpers::populate_sentry_object_id(&id); let mut metadata = Metadata::from_headers(&headers, "").context("extracting metadata from headers")?; metadata.time_created = Some(SystemTime::now()); + let ObjectId { context, key } = id; let stream = body.into_data_stream().map_err(io::Error::other).boxed(); - let response_path = service.put_object(id, &metadata, stream).await?; + let response_id = service + .insert_object(context, Some(key), &metadata, stream) + .await?; + let response = Json(InsertObjectResponse { - key: response_path.key.to_string(), + key: response_id.key.to_string(), }); Ok((StatusCode::OK, response).into_response()) @@ -126,14 +132,14 @@ async fn object_delete( Path(params): Path, ) -> ApiResult { let id = params.into_object_id(); - helpers::populate_sentry_scope(&id); + helpers::populate_sentry_object_id(&id); service.delete_object(&id).await?; Ok(StatusCode::NO_CONTENT) } -/// Path parameters used for collection-level endpoints. +/// Path parameters used for collection-level endpoints without a key. /// /// This is meant to be used with the axum `Path` extractor. #[derive(Clone, Debug, Deserialize)] @@ -144,9 +150,12 @@ struct CollectionParams { } impl CollectionParams { - /// Converts the params into a new [`ObjectId`] with a random unique `key`. - pub fn create_object_id(self) -> ObjectId { - ObjectId::random(self.usecase, self.scopes) + /// Converts the params into an [`ObjectContext`]. + pub fn into_context(self) -> ObjectContext { + ObjectContext { + usecase: self.usecase, + scopes: self.scopes, + } } } @@ -164,11 +173,7 @@ struct ObjectParams { impl ObjectParams { /// Converts the params into an [`ObjectId`]. pub fn into_object_id(self) -> ObjectId { - ObjectId { - usecase: self.usecase, - scopes: self.scopes, - key: self.key, - } + ObjectId::from_parts(self.usecase, self.scopes, self.key) } } diff --git a/objectstore-service/src/backend/bigtable.rs b/objectstore-service/src/backend/bigtable.rs index a789c4b8..7f63cbef 100644 --- a/objectstore-service/src/backend/bigtable.rs +++ b/objectstore-service/src/backend/bigtable.rs @@ -326,9 +326,7 @@ fn micros_to_time(micros: i64) -> Option { mod tests { use std::collections::BTreeMap; - use uuid::Uuid; - - use crate::id::{Scope, Scopes}; + use crate::id::{ObjectContext, Scope, Scopes}; use super::*; @@ -360,19 +358,18 @@ mod tests { Ok(payload) } - fn make_key() -> ObjectId { - ObjectId { + fn make_id() -> ObjectId { + ObjectId::random(ObjectContext { usecase: "testing".into(), scopes: Scopes::from_iter([Scope::create("testing", "value").unwrap()]), - key: Uuid::new_v4().to_string(), - } + }) } #[tokio::test] async fn test_roundtrip() -> Result<()> { let backend = create_test_backend().await?; - let path = make_key(); + let id = make_id(); let metadata = Metadata { content_type: "text/plain".into(), time_created: Some(SystemTime::now()), @@ -381,10 +378,10 @@ mod tests { }; backend - .put_object(&path, &metadata, make_stream(b"hello, world")) + .put_object(&id, &metadata, make_stream(b"hello, world")) .await?; - let (meta, stream) = backend.get_object(&path).await?.unwrap(); + let (meta, stream) = backend.get_object(&id).await?.unwrap(); let payload = read_to_vec(stream).await?; let str_payload = str::from_utf8(&payload).unwrap(); @@ -399,8 +396,8 @@ mod tests { async fn test_get_nonexistent() -> Result<()> { let backend = create_test_backend().await?; - let path = make_key(); - let result = backend.get_object(&path).await?; + let id = make_id(); + let result = backend.get_object(&id).await?; assert!(result.is_none()); Ok(()) @@ -410,8 +407,8 @@ mod tests { async fn test_delete_nonexistent() -> Result<()> { let backend = create_test_backend().await?; - let path = make_key(); - backend.delete_object(&path).await?; + let id = make_id(); + backend.delete_object(&id).await?; Ok(()) } @@ -420,14 +417,14 @@ mod tests { async fn test_overwrite() -> Result<()> { let backend = create_test_backend().await?; - let path = make_key(); + let id = make_id(); let metadata = Metadata { custom: BTreeMap::from_iter([("invalid".into(), "invalid".into())]), ..Default::default() }; backend - .put_object(&path, &metadata, make_stream(b"hello")) + .put_object(&id, &metadata, make_stream(b"hello")) .await?; let metadata = Metadata { @@ -436,10 +433,10 @@ mod tests { }; backend - .put_object(&path, &metadata, make_stream(b"world")) + .put_object(&id, &metadata, make_stream(b"world")) .await?; - let (meta, stream) = backend.get_object(&path).await?.unwrap(); + let (meta, stream) = backend.get_object(&id).await?.unwrap(); let payload = read_to_vec(stream).await?; let str_payload = str::from_utf8(&payload).unwrap(); @@ -453,16 +450,16 @@ mod tests { async fn test_read_after_delete() -> Result<()> { let backend = create_test_backend().await?; - let path = make_key(); + let id = make_id(); let metadata = Metadata::default(); backend - .put_object(&path, &metadata, make_stream(b"hello, world")) + .put_object(&id, &metadata, make_stream(b"hello, world")) .await?; - backend.delete_object(&path).await?; + backend.delete_object(&id).await?; - let result = backend.get_object(&path).await?; + let result = backend.get_object(&id).await?; assert!(result.is_none()); Ok(()) @@ -475,17 +472,17 @@ mod tests { let backend = create_test_backend().await?; - let path = make_key(); + let id = make_id(); let metadata = Metadata { expiration_policy: ExpirationPolicy::TimeToLive(Duration::from_secs(0)), ..Default::default() }; backend - .put_object(&path, &metadata, make_stream(b"hello, world")) + .put_object(&id, &metadata, make_stream(b"hello, world")) .await?; - let result = backend.get_object(&path).await?; + let result = backend.get_object(&id).await?; assert!(result.is_none()); Ok(()) @@ -498,17 +495,17 @@ mod tests { let backend = create_test_backend().await?; - let path = make_key(); + let id = make_id(); let metadata = Metadata { expiration_policy: ExpirationPolicy::TimeToIdle(Duration::from_secs(0)), ..Default::default() }; backend - .put_object(&path, &metadata, make_stream(b"hello, world")) + .put_object(&id, &metadata, make_stream(b"hello, world")) .await?; - let result = backend.get_object(&path).await?; + let result = backend.get_object(&id).await?; assert!(result.is_none()); Ok(()) diff --git a/objectstore-service/src/backend/gcs.rs b/objectstore-service/src/backend/gcs.rs index 2e22b371..9a0bbfce 100644 --- a/objectstore-service/src/backend/gcs.rs +++ b/objectstore-service/src/backend/gcs.rs @@ -435,9 +435,7 @@ impl Backend for GcsBackend { mod tests { use std::collections::BTreeMap; - use uuid::Uuid; - - use crate::id::{Scope, Scopes}; + use crate::id::{ObjectContext, Scope, Scopes}; use super::*; @@ -462,19 +460,18 @@ mod tests { Ok(payload) } - fn make_key() -> ObjectId { - ObjectId { + fn make_id() -> ObjectId { + ObjectId::random(ObjectContext { usecase: "testing".into(), scopes: Scopes::from_iter([Scope::create("testing", "value").unwrap()]), - key: Uuid::new_v4().to_string(), - } + }) } #[tokio::test] async fn test_roundtrip() -> Result<()> { let backend = create_test_backend().await?; - let path = make_key(); + let id = make_id(); let metadata = Metadata { is_redirect_tombstone: None, content_type: "text/plain".into(), @@ -487,10 +484,10 @@ mod tests { }; backend - .put_object(&path, &metadata, make_stream(b"hello, world")) + .put_object(&id, &metadata, make_stream(b"hello, world")) .await?; - let (meta, stream) = backend.get_object(&path).await?.unwrap(); + let (meta, stream) = backend.get_object(&id).await?.unwrap(); let payload = read_to_vec(stream).await?; let str_payload = str::from_utf8(&payload).unwrap(); @@ -506,8 +503,8 @@ mod tests { async fn test_get_nonexistent() -> Result<()> { let backend = create_test_backend().await?; - let path = make_key(); - let result = backend.get_object(&path).await?; + let id = make_id(); + let result = backend.get_object(&id).await?; assert!(result.is_none()); Ok(()) @@ -517,8 +514,8 @@ mod tests { async fn test_delete_nonexistent() -> Result<()> { let backend = create_test_backend().await?; - let path = make_key(); - backend.delete_object(&path).await?; + let id = make_id(); + backend.delete_object(&id).await?; Ok(()) } @@ -527,14 +524,14 @@ mod tests { async fn test_overwrite() -> Result<()> { let backend = create_test_backend().await?; - let path = make_key(); + let id = make_id(); let metadata = Metadata { custom: BTreeMap::from_iter([("invalid".into(), "invalid".into())]), ..Default::default() }; backend - .put_object(&path, &metadata, make_stream(b"hello")) + .put_object(&id, &metadata, make_stream(b"hello")) .await?; let metadata = Metadata { @@ -543,10 +540,10 @@ mod tests { }; backend - .put_object(&path, &metadata, make_stream(b"world")) + .put_object(&id, &metadata, make_stream(b"world")) .await?; - let (meta, stream) = backend.get_object(&path).await?.unwrap(); + let (meta, stream) = backend.get_object(&id).await?.unwrap(); let payload = read_to_vec(stream).await?; let str_payload = str::from_utf8(&payload).unwrap(); @@ -560,16 +557,16 @@ mod tests { async fn test_read_after_delete() -> Result<()> { let backend = create_test_backend().await?; - let path = make_key(); + let id = make_id(); let metadata = Metadata::default(); backend - .put_object(&path, &metadata, make_stream(b"hello, world")) + .put_object(&id, &metadata, make_stream(b"hello, world")) .await?; - backend.delete_object(&path).await?; + backend.delete_object(&id).await?; - let result = backend.get_object(&path).await?; + let result = backend.get_object(&id).await?; assert!(result.is_none()); Ok(()) @@ -582,17 +579,17 @@ mod tests { let backend = create_test_backend().await?; - let path = make_key(); + let id = make_id(); let metadata = Metadata { expiration_policy: ExpirationPolicy::TimeToLive(Duration::from_secs(0)), ..Default::default() }; backend - .put_object(&path, &metadata, make_stream(b"hello, world")) + .put_object(&id, &metadata, make_stream(b"hello, world")) .await?; - let result = backend.get_object(&path).await?; + let result = backend.get_object(&id).await?; assert!(result.is_none()); Ok(()) @@ -605,17 +602,17 @@ mod tests { let backend = create_test_backend().await?; - let path = make_key(); + let id = make_id(); let metadata = Metadata { expiration_policy: ExpirationPolicy::TimeToIdle(Duration::from_secs(0)), ..Default::default() }; backend - .put_object(&path, &metadata, make_stream(b"hello, world")) + .put_object(&id, &metadata, make_stream(b"hello, world")) .await?; - let result = backend.get_object(&path).await?; + let result = backend.get_object(&id).await?; assert!(result.is_none()); Ok(()) diff --git a/objectstore-service/src/backend/local_fs.rs b/objectstore-service/src/backend/local_fs.rs index cfefa8f3..81daedfa 100644 --- a/objectstore-service/src/backend/local_fs.rs +++ b/objectstore-service/src/backend/local_fs.rs @@ -108,7 +108,7 @@ mod tests { use futures_util::TryStreamExt; use objectstore_types::{Compression, ExpirationPolicy}; - use crate::id::{Scope, Scopes}; + use crate::id::{ObjectContext, Scope, Scopes}; use super::*; @@ -121,11 +121,11 @@ mod tests { let tempdir = tempfile::tempdir().unwrap(); let backend = LocalFsBackend::new(tempdir.path()); - let key = ObjectId { + let id = ObjectId::random(ObjectContext { usecase: "testing".into(), scopes: Scopes::from_iter([Scope::create("testing", "value").unwrap()]), - key: "testing".into(), - }; + }); + let metadata = Metadata { is_redirect_tombstone: None, content_type: "text/plain".into(), @@ -137,11 +137,11 @@ mod tests { size: None, }; backend - .put_object(&key, &metadata, make_stream(b"oh hai!")) + .put_object(&id, &metadata, make_stream(b"oh hai!")) .await .unwrap(); - let (read_metadata, stream) = backend.get_object(&key).await.unwrap().unwrap(); + let (read_metadata, stream) = backend.get_object(&id).await.unwrap().unwrap(); let file_contents: BytesMut = stream.try_collect().await.unwrap(); assert_eq!(read_metadata, metadata); diff --git a/objectstore-service/src/id.rs b/objectstore-service/src/id.rs index 98a9793c..ff0f170e 100644 --- a/objectstore-service/src/id.rs +++ b/objectstore-service/src/id.rs @@ -134,11 +134,11 @@ impl FromIterator for Scopes { } } -/// The fully qualified identifier of an object. +/// Defines where an object belongs within the object store. /// -/// This consists of a usecase, the scopes, and the key. +/// This is part of the full object identifier, see [`ObjectId`]. #[derive(Debug, Clone, PartialEq, Eq, Hash)] -pub struct ObjectId { +pub struct ObjectContext { /// The usecase, or "product" this object belongs to. /// /// This can be defined on-the-fly by the client, but special server logic @@ -167,48 +167,101 @@ pub struct ObjectId { /// you must use [`Scope::create`] to create them: /// /// ``` - /// use objectstore_service::id::{ObjectId, Scope, Scopes}; + /// use objectstore_service::id::{ObjectContext, Scope, Scopes}; /// - /// let object_id = ObjectId { + /// let object_id = ObjectContext { /// usecase: "my_usecase".to_string(), /// scopes: Scopes::from_iter([ /// Scope::create("organization", "17").unwrap(), /// Scope::create("project", "42").unwrap(), /// ]), - /// key: "my_object_key".to_string(), /// }; /// ``` pub scopes: Scopes, +} + +/// The fully qualified identifier of an object. +/// +/// This consists of a usecase and the scopes, which make up the object's context and define where +/// the object belongs within objectstore, as well as the unique key within the context. +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct ObjectId { + /// The usecase and scopes this object belongs to. + pub context: ObjectContext, /// This key uniquely identifies the object within its usecase and scopes. /// - /// Note that keys can be reused across different scopes. Only in combination with the usecase - /// and scopes a key makes a unique identifier. + /// Note that keys can be reused across different contexts. Only in combination with the context + /// a key makes a unique identifier. /// /// Keys can be assigned by the service. For this, use [`ObjectId::random`]. pub key: String, } impl ObjectId { + /// Creates a new `ObjectId` with the given `context` and `key`. + pub fn new(context: ObjectContext, key: String) -> Self { + Self::optional(context, Some(key)) + } + + /// Creates a new `ObjectId` from all of its parts. + pub fn from_parts(usecase: String, scopes: Scopes, key: String) -> Self { + Self::new(ObjectContext { usecase, scopes }, key) + } + /// Creates a unique `ObjectId` with a random key. /// /// This can be used when creating an object with a server-generated key. - pub fn random(usecase: String, scopes: Scopes) -> Self { - Self::optional(usecase, scopes, None) + pub fn random(context: ObjectContext) -> Self { + Self::optional(context, None) } /// Creates a new `ObjectId`, generating a key if none is provided. /// /// This creates a unique key like [`ObjectId::random`] if no `key` is provided, or otherwise /// uses the provided `key`. - pub fn optional(usecase: String, scopes: Scopes, key: Option) -> Self { + pub fn optional(context: ObjectContext, key: Option) -> Self { Self { - usecase, - scopes, + context, key: key.unwrap_or_else(|| uuid::Uuid::new_v4().to_string()), } } + /// Returns the key of the object. + /// + /// See [`key`](field@ObjectId::key) for more information. + pub fn key(&self) -> &str { + &self.key + } + + /// Returns the context of the object. + /// + /// See [`context`](field@ObjectId::context) for more information. + pub fn context(&self) -> &ObjectContext { + &self.context + } + + /// Returns the usecase of the object. + /// + /// See [`ObjectContext::usecase`] for more information. + pub fn usecase(&self) -> &str { + &self.context.usecase + } + + /// Returns the scopes of the object. + /// + /// See [`ObjectContext::scopes`] for more information. + pub fn scopes(&self) -> &Scopes { + &self.context.scopes + } + + /// Returns an iterator over all scopes of the object. + /// + /// See [`ObjectContext::scopes`] for more information. + pub fn iter_scopes(&self) -> impl Iterator { + self.context.scopes.iter() + } + /// Returns a view that formats this ID as a storage path. /// /// This will format a hierarchical path in the format @@ -241,9 +294,9 @@ impl fmt::Display for AsStoragePath<'_, Scopes> { impl fmt::Display for AsStoragePath<'_, ObjectId> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}/", self.inner.usecase)?; - if !self.inner.scopes.is_empty() { - write!(f, "{}/", self.inner.scopes.as_storage_path())?; + write!(f, "{}/", self.inner.context.usecase)?; + if !self.inner.context.scopes.is_empty() { + write!(f, "{}/", self.inner.context.scopes.as_storage_path())?; } write!(f, "objects/{}", self.inner.key) } @@ -256,11 +309,13 @@ mod tests { #[test] fn test_storage_path() { let object_id = ObjectId { - usecase: "testing".to_string(), - scopes: Scopes::from_iter([ - Scope::create("org", "12345").unwrap(), - Scope::create("project", "1337").unwrap(), - ]), + context: ObjectContext { + usecase: "testing".to_string(), + scopes: Scopes::from_iter([ + Scope::create("org", "12345").unwrap(), + Scope::create("project", "1337").unwrap(), + ]), + }, key: "foo/bar".to_string(), }; @@ -271,8 +326,10 @@ mod tests { #[test] fn test_storage_path_empty_scopes() { let object_id = ObjectId { - usecase: "testing".to_string(), - scopes: Scopes::empty(), + context: ObjectContext { + usecase: "testing".to_string(), + scopes: Scopes::empty(), + }, key: "foo/bar".to_string(), }; diff --git a/objectstore-service/src/lib.rs b/objectstore-service/src/lib.rs index 4e0fbaca..e68be9df 100644 --- a/objectstore-service/src/lib.rs +++ b/objectstore-service/src/lib.rs @@ -19,7 +19,7 @@ use futures_util::{StreamExt, TryStreamExt, stream::BoxStream}; use objectstore_types::Metadata; use crate::backend::common::BoxedBackend; -use crate::id::ObjectId; +use crate::id::{ObjectContext, ObjectId}; /// The threshold up until which we will go to the "high volume" backend. const BACKEND_SIZE_THRESHOLD: usize = 1024 * 1024; // 1 MiB @@ -102,10 +102,14 @@ impl StorageService { Ok(Self(Arc::new(inner))) } - /// Stores or overwrites an object at the given key. - pub async fn put_object( + /// Creates or overwrites an object. + /// + /// The object is identified by the components of an [`ObjectId`]. The `context` is required, + /// while the `key` can be assigned automatically if set to `None`. + pub async fn insert_object( &self, - id: ObjectId, + context: ObjectContext, + key: Option, metadata: &Metadata, mut stream: PayloadStream, ) -> anyhow::Result { @@ -122,12 +126,17 @@ impl StorageService { } } + let has_key = key.is_some(); + let id = ObjectId::optional(context, key); + // There might currently be a tombstone at the given path from a previously stored object. - let previously_stored_object = self.0.high_volume_backend.get_object(&id).await?; - if is_tombstoned(&previously_stored_object) { - // Write the object to the other backend and keep the tombstone in place - backend = BackendChoice::LongTerm; - } + if has_key { + let previously_stored_object = self.0.high_volume_backend.get_object(&id).await?; + if is_tombstoned(&previously_stored_object) { + // Write the object to the other backend and keep the tombstone in place + backend = BackendChoice::LongTerm; + } + }; let (backend_choice, backend_ty, stored_size) = match backend { BackendChoice::HighVolume => { @@ -193,18 +202,17 @@ impl StorageService { merni::distribution!( "put.latency"@s: start.elapsed(), - "usecase" => id.usecase, + "usecase" => id.usecase(), "backend_choice" => backend_choice, "backend_type" => backend_ty ); merni::distribution!( "put.size"@b: stored_size, - "usecase" => id.usecase, + "usecase" => id.usecase(), "backend_choice" => backend_choice, "backend_type" => backend_ty ); - // TODO(ja): Return a struct here Ok(id) } @@ -227,7 +235,7 @@ impl StorageService { merni::distribution!( "get.latency.pre-response"@s: start.elapsed(), - "usecase" => id.usecase, + "usecase" => id.usecase(), "backend_choice" => backend_choice, "backend_type" => backend_type ); @@ -236,7 +244,7 @@ impl StorageService { if let Some(size) = metadata.size { merni::distribution!( "get.size"@b: size, - "usecase" => id.usecase, + "usecase" => id.usecase(), "backend_choice" => backend_choice, "backend_type" => backend_type ); @@ -261,7 +269,7 @@ impl StorageService { merni::distribution!( "delete.latency"@s: start.elapsed(), - "usecase" => id.usecase + "usecase" => id.usecase() ); Ok(()) @@ -324,11 +332,10 @@ mod tests { tokio_stream::once(Ok(contents.to_vec().into())).boxed() } - fn make_path() -> ObjectId { - ObjectId { + fn make_context() -> ObjectContext { + ObjectContext { usecase: "testing".into(), scopes: Scopes::from_iter([Scope::create("testing", "value").unwrap()]), - key: "testing".into(), } } @@ -341,7 +348,12 @@ mod tests { let service = StorageService::new(config.clone(), config).await.unwrap(); let key = service - .put_object(make_path(), &Default::default(), make_stream(b"oh hai!")) + .insert_object( + make_context(), + Some("testing".into()), + &Default::default(), + make_stream(b"oh hai!"), + ) .await .unwrap(); @@ -360,7 +372,12 @@ mod tests { let service = StorageService::new(config.clone(), config).await.unwrap(); let key = service - .put_object(make_path(), &Default::default(), make_stream(b"oh hai!")) + .insert_object( + make_context(), + Some("testing".into()), + &Default::default(), + make_stream(b"oh hai!"), + ) .await .unwrap();