From fdbe8aa6a899fea7d35a6e11541e3ec4b12cd6a8 Mon Sep 17 00:00:00 2001 From: Marko Malenic Date: Fri, 23 Aug 2024 13:10:09 +1000 Subject: [PATCH 1/9] refactor(filemanager): mirror content-type and content-encoding from client when generating presigned urls --- .../filemanager-api-lambda/src/main.rs | 9 ++- .../filemanager-api-server/src/main.rs | 20 ++--- .../filemanager-inventory-lambda/src/main.rs | 8 +- .../filemanager-migrate-lambda/src/main.rs | 11 ++- .../stacks/filemanager/filemanager/build.rs | 3 +- .../filemanager/src/clients/aws/s3.rs | 10 ++- .../filemanager/src/database/mod.rs | 8 +- .../stacks/filemanager/filemanager/src/env.rs | 5 +- .../filemanager/filemanager/src/error.rs | 2 +- .../filemanager/src/events/aws/inventory.rs | 3 +- .../filemanager/src/handlers/aws.rs | 8 +- .../filemanager/src/queries/get.rs | 3 +- .../filemanager/src/queries/list.rs | 19 ++--- .../filemanager/src/queries/update.rs | 9 +-- .../filemanager/src/routes/error.rs | 8 +- .../filemanager/src/routes/filter/mod.rs | 9 ++- .../filemanager/src/routes/filter/wildcard.rs | 11 ++- .../filemanager/filemanager/src/routes/get.rs | 35 +++++---- .../filemanager/src/routes/header.rs | 34 ++++++++ .../filemanager/src/routes/ingest.rs | 16 ++-- .../filemanager/src/routes/list.rs | 77 ++++++++++--------- .../filemanager/filemanager/src/routes/mod.rs | 33 ++++---- .../filemanager/src/routes/pagination.rs | 13 ++-- .../filemanager/src/routes/presign.rs | 72 ++++++++++++++--- .../filemanager/src/routes/update.rs | 8 +- 25 files changed, 285 insertions(+), 149 deletions(-) create mode 100644 lib/workload/stateless/stacks/filemanager/filemanager/src/routes/header.rs diff --git a/lib/workload/stateless/stacks/filemanager/filemanager-api-lambda/src/main.rs b/lib/workload/stateless/stacks/filemanager/filemanager-api-lambda/src/main.rs index ac7a588a9..de35ee843 100644 --- a/lib/workload/stateless/stacks/filemanager/filemanager-api-lambda/src/main.rs +++ b/lib/workload/stateless/stacks/filemanager/filemanager-api-lambda/src/main.rs @@ -1,18 +1,19 @@ +use std::sync::Arc; + use axum::extract::{Request, State}; use axum::middleware::{from_fn_with_state, Next}; use axum::response::{IntoResponse, Response}; -use filemanager::env::Config; +use lambda_http::run; use lambda_http::Error; -use std::sync::Arc; +use tracing::debug; use filemanager::clients::aws::s3; use filemanager::database::Client; +use filemanager::env::Config; use filemanager::handlers::aws::{create_database_pool, update_credentials}; use filemanager::handlers::init_tracing; use filemanager::routes::error::{ErrorResponse, ErrorStatusCode}; use filemanager::routes::{router, AppState}; -use lambda_http::run; -use tracing::debug; #[tokio::main] async fn main() -> Result<(), Error> { diff --git a/lib/workload/stateless/stacks/filemanager/filemanager-api-server/src/main.rs b/lib/workload/stateless/stacks/filemanager/filemanager-api-server/src/main.rs index 62044696c..6412b9e88 100644 --- a/lib/workload/stateless/stacks/filemanager/filemanager-api-server/src/main.rs +++ b/lib/workload/stateless/stacks/filemanager/filemanager-api-server/src/main.rs @@ -1,5 +1,16 @@ +use std::io; +use std::path::PathBuf; +use std::sync::Arc; + use axum::serve; use clap::{Parser, Subcommand}; +use http::Uri; +use sea_orm::ConnectionTrait; +use tokio::fs::File; +use tokio::io::AsyncReadExt; +use tokio::net::TcpListener; +use tracing::{debug, info}; + use filemanager::clients::aws::s3; use filemanager::database::aws::migration::Migration; use filemanager::database::{Client, Migrate}; @@ -10,15 +21,6 @@ use filemanager::handlers::init_tracing_with_format; use filemanager::handlers::Format::Pretty; use filemanager::queries::EntriesBuilder; use filemanager::routes::{router, AppState}; -use http::Uri; -use sea_orm::ConnectionTrait; -use std::io; -use std::path::PathBuf; -use std::sync::Arc; -use tokio::fs::File; -use tokio::io::AsyncReadExt; -use tokio::net::TcpListener; -use tracing::{debug, info}; /// Run the filemanager API server locally to explore the API. #[derive(Parser, Debug)] diff --git a/lib/workload/stateless/stacks/filemanager/filemanager-inventory-lambda/src/main.rs b/lib/workload/stateless/stacks/filemanager/filemanager-inventory-lambda/src/main.rs index 719d6eff6..e11f6b1d4 100644 --- a/lib/workload/stateless/stacks/filemanager/filemanager-inventory-lambda/src/main.rs +++ b/lib/workload/stateless/stacks/filemanager/filemanager-inventory-lambda/src/main.rs @@ -1,7 +1,7 @@ -use filemanager::clients::aws::s3::Client; use lambda_runtime::{run, service_fn, Error, LambdaEvent}; use serde::Deserialize; +use filemanager::clients::aws::s3::Client; use filemanager::database::Client as DbClient; use filemanager::env::Config; use filemanager::events::aws::inventory::Manifest; @@ -59,11 +59,13 @@ async fn main() -> Result<(), Error> { #[cfg(test)] mod tests { - use super::*; use aws_sdk_s3::types::InventoryFormat; - use filemanager::events::aws::inventory::File; use serde_json::{from_str, json}; + use filemanager::events::aws::inventory::File; + + use super::*; + #[test] fn deserialize_bucket_key() { let bucket_key = json!({ diff --git a/lib/workload/stateless/stacks/filemanager/filemanager-migrate-lambda/src/main.rs b/lib/workload/stateless/stacks/filemanager/filemanager-migrate-lambda/src/main.rs index cdf757396..dbdb81a40 100644 --- a/lib/workload/stateless/stacks/filemanager/filemanager-migrate-lambda/src/main.rs +++ b/lib/workload/stateless/stacks/filemanager/filemanager-migrate-lambda/src/main.rs @@ -2,8 +2,6 @@ use lambda_runtime::{run, service_fn, Error, LambdaEvent}; use serde::de::IgnoredAny; use serde::Deserialize; -use crate::CloudFormationRequest::Delete; -use crate::Event::Provider; use filemanager::database::aws::migration::Migration; use filemanager::database::Client as DbClient; use filemanager::database::Migrate; @@ -11,6 +9,9 @@ use filemanager::env::Config; use filemanager::handlers::aws::{create_database_pool, update_credentials}; use filemanager::handlers::init_tracing; +use crate::CloudFormationRequest::Delete; +use crate::Event::Provider; + /// The lambda event for this function. This is normally a CloudFormationCustomResourceRequest. /// If anything else is present, the migrate lambda will still attempt to perform a migration. #[derive(Debug, Deserialize)] @@ -61,10 +62,12 @@ async fn main() -> Result<(), Error> { #[cfg(test)] mod tests { - use super::*; + use serde_json::{from_value, json}; + use crate::CloudFormationRequest::Delete; use crate::Event::Ignored; - use serde_json::{from_value, json}; + + use super::*; #[test] fn event_deserialize_provider_delete() { diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/build.rs b/lib/workload/stateless/stacks/filemanager/filemanager/build.rs index b81beca24..aaa7b7091 100644 --- a/lib/workload/stateless/stacks/filemanager/filemanager/build.rs +++ b/lib/workload/stateless/stacks/filemanager/filemanager/build.rs @@ -1,7 +1,8 @@ +use miette::Result; + use filemanager_build::gen_entities::generate_entities; use filemanager_build::gen_openapi::generate_openapi; use filemanager_build::rebuild_if_changed; -use miette::Result; #[tokio::main] async fn main() -> Result<()> { diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/src/clients/aws/s3.rs b/lib/workload/stateless/stacks/filemanager/filemanager/src/clients/aws/s3.rs index 5671066f4..30a26b833 100644 --- a/lib/workload/stateless/stacks/filemanager/filemanager/src/clients/aws/s3.rs +++ b/lib/workload/stateless/stacks/filemanager/filemanager/src/clients/aws/s3.rs @@ -1,7 +1,8 @@ //! A mockable wrapper around the S3 client. //! -use crate::clients::aws::config::Config; +use std::result; + use aws_sdk_s3 as s3; use aws_sdk_s3::error::SdkError; use aws_sdk_s3::operation::get_object::{GetObjectError, GetObjectOutput}; @@ -11,7 +12,8 @@ use aws_sdk_s3::presigning::{PresignedRequest, PresigningConfig}; use aws_sdk_s3::types::ChecksumMode::Enabled; use chrono::Duration; use mockall::automock; -use std::result; + +use crate::clients::aws::config::Config; pub type Result = result::Result>; @@ -74,11 +76,15 @@ impl Client { key: &str, bucket: &str, response_content_disposition: &str, + response_content_type: Option, + response_content_encoding: Option, expires_in: Duration, ) -> Result { self.inner .get_object() .response_content_disposition(response_content_disposition) + .set_response_content_type(response_content_type) + .set_response_content_encoding(response_content_encoding) .key(key) .bucket(bucket) .presigned( diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/src/database/mod.rs b/lib/workload/stateless/stacks/filemanager/filemanager/src/database/mod.rs index e74d2cf6b..2403cde65 100644 --- a/lib/workload/stateless/stacks/filemanager/filemanager/src/database/mod.rs +++ b/lib/workload/stateless/stacks/filemanager/filemanager/src/database/mod.rs @@ -1,16 +1,16 @@ //! This module handles connecting to the filemanager database for actions such as ingesting events. //! -use crate::database::aws::credentials::IamGenerator; -use crate::database::aws::ingester::Ingester; -use crate::database::aws::ingester_paired::IngesterPaired; -use crate::env::Config; use async_trait::async_trait; use sea_orm::{DatabaseConnection, SqlxPostgresConnector}; use sqlx::postgres::PgConnectOptions; use sqlx::PgPool; use tracing::debug; +use crate::database::aws::credentials::IamGenerator; +use crate::database::aws::ingester::Ingester; +use crate::database::aws::ingester_paired::IngesterPaired; +use crate::env::Config; use crate::error::Result; use crate::events::EventSourceType; diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/src/env.rs b/lib/workload/stateless/stacks/filemanager/filemanager/src/env.rs index 2ed04726e..839698619 100644 --- a/lib/workload/stateless/stacks/filemanager/filemanager/src/env.rs +++ b/lib/workload/stateless/stacks/filemanager/filemanager/src/env.rs @@ -1,8 +1,6 @@ //! Handles loading environment variables as config options for filemanager. //! -use crate::error::Error::ConfigError; -use crate::error::Result; use chrono::Duration; use envy::from_env; use serde::Deserialize; @@ -10,6 +8,9 @@ use serde_with::serde_as; use serde_with::DurationSeconds; use url::Url; +use crate::error::Error::ConfigError; +use crate::error::Result; + /// Configuration environment variables for filemanager. #[serde_as] #[derive(Debug, Clone, Deserialize, Default, Eq, PartialEq)] diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/src/error.rs b/lib/workload/stateless/stacks/filemanager/filemanager/src/error.rs index 6445e6c26..c4832c247 100644 --- a/lib/workload/stateless/stacks/filemanager/filemanager/src/error.rs +++ b/lib/workload/stateless/stacks/filemanager/filemanager/src/error.rs @@ -1,9 +1,9 @@ //! Errors used by the filemanager crate. //! -use sea_orm::{sqlx_error_to_query_err, DbErr}; use std::{io, result}; +use sea_orm::{sqlx_error_to_query_err, DbErr}; use sqlx::migrate::MigrateError; use thiserror::Error; use url::ParseError; diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/src/events/aws/inventory.rs b/lib/workload/stateless/stacks/filemanager/filemanager/src/events/aws/inventory.rs index 46162226c..009bec9bc 100644 --- a/lib/workload/stateless/stacks/filemanager/filemanager/src/events/aws/inventory.rs +++ b/lib/workload/stateless/stacks/filemanager/filemanager/src/events/aws/inventory.rs @@ -585,10 +585,11 @@ pub(crate) mod tests { use serde_json::json; use serde_json::Value; - use super::*; use crate::events::aws::inventory::Manifest; use crate::events::aws::tests::EXPECTED_E_TAG; + use super::*; + const CSV_MANIFEST_SCHEMA: &str = "Bucket, Key, VersionId, IsLatest, IsDeleteMarker, Size, \ LastModifiedDate, ETag, StorageClass, IsMultipartUploaded, ReplicationStatus, \ EncryptionStatus, ObjectLockRetainUntilDate, ObjectLockMode, \ diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/src/handlers/aws.rs b/lib/workload/stateless/stacks/filemanager/filemanager/src/handlers/aws.rs index 2bfbd36b8..d32ff1129 100644 --- a/lib/workload/stateless/stacks/filemanager/filemanager/src/handlers/aws.rs +++ b/lib/workload/stateless/stacks/filemanager/filemanager/src/handlers/aws.rs @@ -204,12 +204,13 @@ pub async fn update_credentials( #[cfg(test)] pub(crate) mod tests { + use std::future::Future; + use aws_lambda_events::sqs::SqsMessage; use chrono::DateTime; use sqlx::postgres::PgRow; - use std::future::Future; + use sqlx::PgPool; - use super::*; use crate::database::aws::ingester::tests::{ assert_row, expected_message, fetch_results, remove_version_ids, replace_sequencers, test_events, test_ingester, @@ -232,7 +233,8 @@ pub(crate) mod tests { }; use crate::events::aws::FlatS3EventMessage; use crate::events::EventSourceType::S3; - use sqlx::PgPool; + + use super::*; #[sqlx::test(migrator = "MIGRATOR")] async fn test_receive_and_ingest(pool: PgPool) { diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/src/queries/get.rs b/lib/workload/stateless/stacks/filemanager/filemanager/src/queries/get.rs index 817b304ca..2c9c4461a 100644 --- a/lib/workload/stateless/stacks/filemanager/filemanager/src/queries/get.rs +++ b/lib/workload/stateless/stacks/filemanager/filemanager/src/queries/get.rs @@ -36,11 +36,12 @@ where mod tests { use sqlx::PgPool; - use super::*; use crate::database::aws::migration::tests::MIGRATOR; use crate::database::Client; use crate::queries::EntriesBuilder; + use super::*; + #[sqlx::test(migrator = "MIGRATOR")] async fn test_list_s3(pool: PgPool) { let client = Client::from_pool(pool); diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/src/queries/list.rs b/lib/workload/stateless/stacks/filemanager/filemanager/src/queries/list.rs index 4c527589f..f16bb711c 100644 --- a/lib/workload/stateless/stacks/filemanager/filemanager/src/queries/list.rs +++ b/lib/workload/stateless/stacks/filemanager/filemanager/src/queries/list.rs @@ -1,14 +1,6 @@ //! Query builder involving list operations on the database. //! -use crate::database::entities::s3_object; -use crate::error::Error::OverflowError; -use crate::error::{Error, Result}; -use crate::routes::filter::wildcard::{Wildcard, WildcardEither}; -use crate::routes::filter::S3ObjectsFilter; -use crate::routes::list::ListCount; -use crate::routes::pagination::{ListResponse, Pagination}; - use sea_orm::prelude::Expr; use sea_orm::sea_query::extension::postgres::PgExpr; use sea_orm::sea_query::{ @@ -22,6 +14,14 @@ use sea_orm::{ use tracing::trace; use url::Url; +use crate::database::entities::s3_object; +use crate::error::Error::OverflowError; +use crate::error::{Error, Result}; +use crate::routes::filter::wildcard::{Wildcard, WildcardEither}; +use crate::routes::filter::S3ObjectsFilter; +use crate::routes::list::ListCount; +use crate::routes::pagination::{ListResponse, Pagination}; + /// A query builder for list operations. #[derive(Debug, Clone)] pub struct ListQueryBuilder<'a, C, E> @@ -464,7 +464,6 @@ pub(crate) mod tests { use serde_json::json; use sqlx::PgPool; - use super::*; use crate::database::aws::migration::tests::MIGRATOR; use crate::database::entities::sea_orm_active_enums::{EventType, StorageClass}; use crate::database::Client; @@ -473,6 +472,8 @@ pub(crate) mod tests { use crate::routes::filter::wildcard::Wildcard; use crate::routes::pagination::Links; + use super::*; + #[sqlx::test(migrator = "MIGRATOR")] async fn test_current_s3(pool: PgPool) { let client = Client::from_pool(pool); diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/src/queries/update.rs b/lib/workload/stateless/stacks/filemanager/filemanager/src/queries/update.rs index 56a9f1e2e..4e02c5d29 100644 --- a/lib/workload/stateless/stacks/filemanager/filemanager/src/queries/update.rs +++ b/lib/workload/stateless/stacks/filemanager/filemanager/src/queries/update.rs @@ -286,11 +286,8 @@ where #[cfg(test)] pub(crate) mod tests { - use std::ops::{Index, Range}; - use crate::queries::{Entries, EntriesBuilder}; - use crate::routes::filter::wildcard::Wildcard; use sea_orm::{ActiveModelTrait, IntoActiveModel}; use sea_orm::{DatabaseConnection, Set}; use serde_json::json; @@ -298,11 +295,13 @@ pub(crate) mod tests { use sqlx::PgPool; use crate::database::aws::migration::tests::MIGRATOR; - - use super::*; use crate::database::Client; + use crate::queries::{Entries, EntriesBuilder}; + use crate::routes::filter::wildcard::Wildcard; use crate::routes::filter::wildcard::WildcardEither; + use super::*; + #[sqlx::test(migrator = "MIGRATOR")] async fn update_attributes_unsupported(pool: PgPool) { let client = Client::from_pool(pool); diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/error.rs b/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/error.rs index 18a01bbcd..3c49c4555 100644 --- a/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/error.rs +++ b/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/error.rs @@ -1,7 +1,9 @@ //! Error related parsing code specific to HTTP routes and responses. //! -use crate::error::Error; +use std::fmt; +use std::fmt::{Debug, Display, Formatter}; + use aws_lambda_events::http::StatusCode; use axum::extract; use axum::extract::rejection::{JsonRejection, PathRejection, QueryRejection}; @@ -10,11 +12,11 @@ use axum_extra::extract::WithRejection; use sea_orm::DbErr; use serde::{Deserialize, Serialize}; use serde_qs::axum::QsQueryRejection; -use std::fmt; -use std::fmt::{Debug, Display, Formatter}; use thiserror::Error; use utoipa::{IntoResponses, ToSchema}; +use crate::error::Error; + /// Type alias for a Query with a custom rejection. pub type Query = WithRejection, ErrorStatusCode>; diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/filter/mod.rs b/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/filter/mod.rs index 0d6a261e4..42b3bcd62 100644 --- a/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/filter/mod.rs +++ b/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/filter/mod.rs @@ -1,14 +1,15 @@ //! Routing logic for query filtering. //! -pub mod wildcard; - -use crate::database::entities::sea_orm_active_enums::{EventType, StorageClass}; -use crate::routes::filter::wildcard::{Wildcard, WildcardEither}; use sea_orm::prelude::{DateTimeWithTimeZone, Json}; use serde::{Deserialize, Serialize}; use utoipa::IntoParams; +use crate::database::entities::sea_orm_active_enums::{EventType, StorageClass}; +use crate::routes::filter::wildcard::{Wildcard, WildcardEither}; + +pub mod wildcard; + /// The available fields to filter `s3_object` queries by. Each query parameter represents /// an `and` clause in the SQL statement. Nested query string style syntax is supported on /// JSON attributes. Wildcards are supported on some of the fields. diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/filter/wildcard.rs b/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/filter/wildcard.rs index 3ddd85437..36f5663ed 100644 --- a/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/filter/wildcard.rs +++ b/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/filter/wildcard.rs @@ -1,11 +1,12 @@ //! Wildcard filtering logic. //! -use crate::error::Error::ParseError; -use crate::error::Result; use serde::{Deserialize, Serialize}; use utoipa::ToSchema; +use crate::error::Error::ParseError; +use crate::error::Result; + /// An enum which deserializes into a concrete type or a wildcard. This is used for better /// type support when non-string filter parameters such as `StorageClass` or `EventType`. #[derive(Serialize, Deserialize, Debug, Eq, PartialEq, Clone)] @@ -180,11 +181,13 @@ impl Wildcard { #[cfg(test)] mod tests { - use super::*; - use crate::database::entities::sea_orm_active_enums::{EventType, StorageClass}; use sea_orm::prelude::DateTimeWithTimeZone; use serde_json::json; + use crate::database::entities::sea_orm_active_enums::{EventType, StorageClass}; + + use super::*; + #[test] fn deserialize_wildcard_either() { let wildcard: WildcardEither = serde_json::from_value(json!("Created")).unwrap(); diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/get.rs b/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/get.rs index 4b8614d6e..c9fdc9cb8 100644 --- a/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/get.rs +++ b/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/get.rs @@ -1,6 +1,15 @@ //! Route logic for get API calls. //! +use axum::extract::{Request, State}; +use axum::http::header::{CONTENT_ENCODING, CONTENT_TYPE}; +use axum::routing::get; +use axum::{extract, Json, Router}; +use axum_extra::extract::WithRejection; +use sea_orm::{ConnectionTrait, TransactionTrait}; +use url::Url; +use uuid::Uuid; + use crate::database::entities::s3_object; use crate::database::entities::s3_object::Model as S3; use crate::database::entities::sea_orm_active_enums::EventType; @@ -11,15 +20,9 @@ use crate::queries::list::ListQueryBuilder; use crate::routes::error::{ErrorStatusCode, Path, Query}; use crate::routes::filter::wildcard::Wildcard; use crate::routes::filter::S3ObjectsFilter; +use crate::routes::header::HeaderParser; use crate::routes::presign::{PresignedParams, PresignedUrlBuilder}; use crate::routes::AppState; -use axum::extract::State; -use axum::routing::get; -use axum::{extract, Json, Router}; -use axum_extra::extract::WithRejection; -use sea_orm::{ConnectionTrait, TransactionTrait}; -use url::Url; -use uuid::Uuid; async fn get_s3_from_connection( connection: &C, @@ -71,6 +74,7 @@ pub async fn presign_s3_by_id( state: State, id: Path, WithRejection(extract::Query(presigned), _): Query, + request: Request, ) -> Result>> { let txn = state.database_client().connection_ref().begin().await?; @@ -98,6 +102,9 @@ pub async fn presign_s3_by_id( txn.commit().await?; + let content_type = HeaderParser::new(request.headers()).parse_header(CONTENT_TYPE)?; + let content_encoding = HeaderParser::new(request.headers()).parse_header(CONTENT_ENCODING)?; + // If the last object ordered by sequencer is the requested one, then this is a // current object. if let Some(current) = current.last() { @@ -107,6 +114,8 @@ pub async fn presign_s3_by_id( &state, response, presigned.response_content_disposition(), + content_type, + content_encoding, ) .await?, )); @@ -128,18 +137,20 @@ mod tests { use aws_smithy_mocks_experimental::{mock_client, RuleMode}; use axum::body::Body; use axum::http::{Method, StatusCode}; + use serde_json::Value; use sqlx::PgPool; - use super::*; use crate::clients::aws::s3; use crate::database::aws::migration::tests::MIGRATOR; use crate::env::Config; use crate::queries::EntriesBuilder; use crate::routes::list::tests::mock_get_object; use crate::routes::list::tests::{response_from, response_from_get}; + use crate::routes::presign::tests::assert_presigned_params; use crate::routes::AppState; use crate::uuid::UuidGenerator; - use serde_json::Value; + + use super::*; #[sqlx::test(migrator = "MIGRATOR")] async fn get_s3_api(pool: PgPool) { @@ -193,8 +204,7 @@ mod tests { .unwrap(); let query = result.query().unwrap(); - assert!(query.contains("X-Amz-Expires=300")); - assert!(query.contains("response-content-disposition=inline")); + assert_presigned_params(query, "inline"); assert_eq!(result.path(), "/0/0"); } @@ -226,8 +236,7 @@ mod tests { .unwrap(); let query = result.query().unwrap(); - assert!(query.contains("X-Amz-Expires=300")); - assert!(query.contains("response-content-disposition=attachment%3B%20filename%3D%220%22")); + assert_presigned_params(query, "attachment%3B%20filename%3D%220%22"); assert_eq!(result.path(), "/0/0"); } diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/header.rs b/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/header.rs new file mode 100644 index 000000000..bf3f70e40 --- /dev/null +++ b/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/header.rs @@ -0,0 +1,34 @@ +//! Functions related to parsing header information. +//! + +use axum::http::header::AsHeaderName; +use axum::http::HeaderMap; + +use crate::error::Error::ParseError; +use crate::error::{Error, Result}; + +pub struct HeaderParser<'a> { + headers: &'a HeaderMap, +} + +impl<'a> HeaderParser<'a> { + /// Create a header parser. + pub fn new(headers: &'a HeaderMap) -> Self { + Self { headers } + } + + /// Parse a header into a string. + pub fn parse_header(&self, header: K) -> Result> { + self.headers + .get(header) + .map(|content_type| { + Ok::<_, Error>( + content_type + .to_str() + .map_err(|err| ParseError(err.to_string()))? + .to_string(), + ) + }) + .transpose() + } +} diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/ingest.rs b/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/ingest.rs index b1a9ba81f..8bfe6a32e 100644 --- a/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/ingest.rs +++ b/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/ingest.rs @@ -1,6 +1,13 @@ //! Route logic for ingesting entries into the database. //! +use axum::extract::State; +use axum::routing::post; +use axum::{Json, Router}; +use mockall_double::double; +use serde::{Deserialize, Serialize}; +use utoipa::ToSchema; + #[double] use crate::clients::aws::s3::Client as S3Client; #[double] @@ -9,12 +16,6 @@ use crate::error::Result; use crate::handlers::aws::receive_and_ingest; use crate::routes::error::ErrorStatusCode; use crate::routes::AppState; -use axum::extract::State; -use axum::routing::post; -use axum::{Json, Router}; -use mockall_double::double; -use serde::{Deserialize, Serialize}; -use utoipa::ToSchema; /// The return value for ingest endpoints indicating how many records were processed. #[derive(Debug, Deserialize, Serialize, ToSchema)] @@ -55,10 +56,11 @@ pub fn ingest_router() -> Router { #[cfg(test)] mod tests { + use std::sync::Arc; + use axum::body::Body; use axum::http::{Method, Request}; use sqlx::PgPool; - use std::sync::Arc; use tower::ServiceExt; use crate::database::aws::migration::tests::MIGRATOR; diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/list.rs b/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/list.rs index f36994ef0..21c01a2ff 100644 --- a/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/list.rs +++ b/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/list.rs @@ -1,26 +1,29 @@ //! Route logic for list API calls. //! +use std::marker::PhantomData; + +use axum::extract::{Request, State}; +use axum::http::header::{CONTENT_ENCODING, CONTENT_TYPE, HOST}; +use axum::routing::get; +use axum::{extract, Json, Router}; +use axum_extra::extract::WithRejection; +use sea_orm::TransactionTrait; +use serde::{Deserialize, Serialize}; +use url::Url; +use utoipa::{IntoParams, ToSchema}; + use crate::database::entities::s3_object; use crate::database::entities::s3_object::Model as S3; -use crate::error::Error::{MissingHostHeader, ParseError}; +use crate::error::Error::MissingHostHeader; use crate::error::Result; use crate::queries::list::ListQueryBuilder; use crate::routes::error::{ErrorStatusCode, QsQuery, Query}; use crate::routes::filter::S3ObjectsFilter; +use crate::routes::header::HeaderParser; use crate::routes::pagination::{ListResponse, Pagination}; use crate::routes::presign::{PresignedParams, PresignedUrlBuilder}; use crate::routes::AppState; -use axum::extract::{Request, State}; -use axum::http::header::HOST; -use axum::routing::get; -use axum::{extract, Json, Router}; -use axum_extra::extract::WithRejection; -use sea_orm::TransactionTrait; -use serde::{Deserialize, Serialize}; -use std::marker::PhantomData; -use url::Url; -use utoipa::{IntoParams, ToSchema}; /// The return value for count operations showing the number of records in the database. #[derive(Debug, Deserialize, Serialize, ToSchema, Eq, PartialEq)] @@ -133,13 +136,9 @@ pub async fn list_s3( let url = if let Some(url) = state.config().api_links_url() { url } else { - let mut host = request - .headers() - .get(HOST) - .ok_or_else(|| MissingHostHeader)? - .to_str() - .map_err(|err| ParseError(err.to_string()))? - .to_string(); + let mut host = HeaderParser::new(request.headers()) + .parse_header(HOST)? + .ok_or_else(|| MissingHostHeader)?; // A `HOST` is not a valid URL yet. if !host.starts_with("https://") && !host.starts_with("http://") { @@ -223,6 +222,9 @@ pub async fn presign_s3( filter_all: QsQuery, request: Request, ) -> Result>> { + let content_type = HeaderParser::new(request.headers()).parse_header(CONTENT_TYPE)?; + let content_encoding = HeaderParser::new(request.headers()).parse_header(CONTENT_ENCODING)?; + let Json(ListResponse { links, pagination, @@ -243,6 +245,8 @@ pub async fn presign_s3( &state, result, presigned.response_content_disposition(), + content_type.clone(), + content_encoding.clone(), ) .await? { @@ -264,17 +268,6 @@ pub fn list_router() -> Router { #[cfg(test)] pub(crate) mod tests { - use super::*; - use crate::clients::aws::s3; - use crate::database::aws::migration::tests::MIGRATOR; - use crate::database::entities::sea_orm_active_enums::EventType; - use crate::env::Config; - use crate::queries::list::tests::filter_event_type; - use crate::queries::update::tests::{assert_contains, entries_many}; - use crate::queries::update::tests::{change_key, change_many}; - use crate::queries::EntriesBuilder; - use crate::routes::api_router; - use crate::routes::pagination::Links; use aws_sdk_s3::operation::get_object::GetObjectOutput; use aws_sdk_s3::primitives::ByteStream; use aws_smithy_mocks_experimental::{mock, mock_client, Rule, RuleMode}; @@ -289,6 +282,20 @@ pub(crate) mod tests { use tower::ServiceExt; use uuid::Uuid; + use crate::clients::aws::s3; + use crate::database::aws::migration::tests::MIGRATOR; + use crate::database::entities::sea_orm_active_enums::EventType; + use crate::env::Config; + use crate::queries::list::tests::filter_event_type; + use crate::queries::update::tests::{assert_contains, entries_many}; + use crate::queries::update::tests::{change_key, change_many}; + use crate::queries::EntriesBuilder; + use crate::routes::api_router; + use crate::routes::pagination::Links; + use crate::routes::presign::tests::assert_presigned_params; + + use super::*; + #[sqlx::test(migrator = "MIGRATOR")] async fn list_s3_api(pool: PgPool) { let state = AppState::from_pool(pool).await; @@ -419,12 +426,12 @@ pub(crate) mod tests { let query = result.results()[0].query().unwrap(); assert!(query.contains("X-Amz-Expires=300")); - assert!(query.contains("response-content-disposition=inline")); + assert_presigned_params(query, "inline"); + assert_eq!(result.results()[0].path(), "/0/0"); let query = result.results()[1].query().unwrap(); - assert!(query.contains("X-Amz-Expires=300")); - assert!(query.contains("response-content-disposition=inline")); + assert_presigned_params(query, "inline"); assert_eq!(result.results()[1].path(), "/2/2"); } @@ -457,12 +464,11 @@ pub(crate) mod tests { let query = result.results()[0].query().unwrap(); assert!(query.contains("X-Amz-Expires=300")); - assert!(query.contains("response-content-disposition=attachment%3B%20filename%3D%220%22")); + assert_presigned_params(query, "attachment%3B%20filename%3D%220%22"); assert_eq!(result.results()[0].path(), "/0/0"); let query = result.results()[1].query().unwrap(); - assert!(query.contains("X-Amz-Expires=300")); - assert!(query.contains("response-content-disposition=attachment%3B%20filename%3D%222%22")); + assert_presigned_params(query, "attachment%3B%20filename%3D%222%22"); assert_eq!(result.results()[1].path(), "/2/2"); } @@ -781,6 +787,7 @@ pub(crate) mod tests { .uri(uri) .header(HOST, "example.com") .header(CONTENT_TYPE, "application/json") + .header(CONTENT_ENCODING, "gzip") .body(body) .unwrap(), ) diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/mod.rs b/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/mod.rs index 4dbd9c6f5..b404f2c0e 100644 --- a/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/mod.rs +++ b/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/mod.rs @@ -1,6 +1,17 @@ //! This module handles API routing. //! +use std::sync::Arc; + +use axum::http::header::AUTHORIZATION; +use axum::http::{HeaderValue, Method}; +use axum::Router; +use chrono::Duration; +use sqlx::PgPool; +use tower_http::cors::CorsLayer; +use tower_http::trace::TraceLayer; +use tracing::trace; + use crate::clients::aws::s3; use crate::database; use crate::env::Config; @@ -12,19 +23,11 @@ use crate::routes::ingest::ingest_router; use crate::routes::list::*; use crate::routes::openapi::swagger_ui; use crate::routes::update::update_router; -use axum::http::header::AUTHORIZATION; -use axum::http::{HeaderValue, Method}; -use axum::Router; -use chrono::Duration; -use sqlx::PgPool; -use std::sync::Arc; -use tower_http::cors::CorsLayer; -use tower_http::trace::TraceLayer; -use tracing::trace; pub mod error; pub mod filter; pub mod get; +pub mod header; pub mod ingest; pub mod list; pub mod openapi; @@ -168,10 +171,8 @@ pub fn api_router(state: AppState) -> Result { #[cfg(test)] mod tests { - use crate::database::aws::migration::tests::MIGRATOR; - use crate::env::Config; - use crate::error::Error; - use crate::routes::{router, AppState}; + use std::sync::Arc; + use aws_lambda_events::http::header::ACCESS_CONTROL_ALLOW_HEADERS; use aws_lambda_events::http::Request; use axum::body::Body; @@ -182,9 +183,13 @@ mod tests { use axum::http::{Method, StatusCode}; use axum::response::IntoResponse; use sqlx::PgPool; - use std::sync::Arc; use tower::ServiceExt; + use crate::database::aws::migration::tests::MIGRATOR; + use crate::env::Config; + use crate::error::Error; + use crate::routes::{router, AppState}; + #[tokio::test] async fn internal_error_into_response() { let response = Error::MigrateError("error".to_string()).into_response(); diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/pagination.rs b/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/pagination.rs index bec46b540..e0a0aebe8 100644 --- a/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/pagination.rs +++ b/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/pagination.rs @@ -1,15 +1,17 @@ //! Pagination structs and logic for API routes. //! -use crate::database::entities::s3_object::Model as S3; -use crate::error::Error::OverflowError; -use crate::error::{Error, Result}; -use serde::{Deserialize, Deserializer, Serialize}; use std::num::NonZeroU64; use std::result; + +use serde::{Deserialize, Deserializer, Serialize}; use url::Url; use utoipa::{IntoParams, ToSchema}; +use crate::database::entities::s3_object::Model as S3; +use crate::error::Error::OverflowError; +use crate::error::{Error, Result}; + /// The response type for list operations. #[derive(Debug, Clone, Deserialize, Serialize, ToSchema, Eq, PartialEq)] #[serde(rename_all = "camelCase")] @@ -221,7 +223,6 @@ mod tests { use axum::http::Method; use sqlx::PgPool; - use super::*; use crate::database::aws::migration::tests::MIGRATOR; use crate::database::entities::s3_object::Model as S3Object; use crate::queries::EntriesBuilder; @@ -229,6 +230,8 @@ mod tests { use crate::routes::list::tests::{response_from, response_from_get}; use crate::routes::AppState; + use super::*; + #[sqlx::test(migrator = "MIGRATOR")] async fn list_s3_api_paginate(pool: PgPool) { let state = AppState::from_pool(pool).await; diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/presign.rs b/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/presign.rs index e27a568e6..942e0eccb 100644 --- a/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/presign.rs +++ b/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/presign.rs @@ -1,16 +1,17 @@ //! Logic for the presigned url route. //! +use chrono::Duration; +use serde::{Deserialize, Serialize}; +use url::Url; +use utoipa::{IntoParams, ToSchema}; + use crate::clients::aws::s3; use crate::database::entities::s3_object; use crate::env::Config; use crate::error::Error::PresignedUrlError; use crate::error::Result; use crate::routes::AppState; -use chrono::Duration; -use serde::{Deserialize, Serialize}; -use url::Url; -use utoipa::{IntoParams, ToSchema}; /// Parameters for presigned URL routes. #[derive(Serialize, Deserialize, Debug, Default, IntoParams)] @@ -86,6 +87,8 @@ impl<'a> PresignedUrlBuilder<'a> { key: &str, bucket: &str, response_content_disposition: ContentDisposition, + response_content_type: Option, + response_content_encoding: Option, ) -> Result> { let limit = if let Some(size) = self.object_size { u64::try_from(size).unwrap_or_default() @@ -109,6 +112,8 @@ impl<'a> PresignedUrlBuilder<'a> { key, bucket, content_disposition, + response_content_type, + response_content_encoding, self.config .api_presign_expiry() .unwrap_or(DEFAULT_PRESIGN_EXPIRY), @@ -127,11 +132,19 @@ impl<'a> PresignedUrlBuilder<'a> { state: &'a AppState, model: s3_object::Model, response_content_disposition: ContentDisposition, + response_content_type: Option, + response_content_encoding: Option, ) -> Result> { let builder = Self::new(state.s3_client(), state.config()).set_object_size(model.size); if let Some(presigned) = builder - .presign_url(&model.key, &model.bucket, response_content_disposition) + .presign_url( + &model.key, + &model.bucket, + response_content_disposition, + response_content_type, + response_content_encoding, + ) .await? { Ok(Some(presigned)) @@ -145,11 +158,12 @@ impl<'a> PresignedUrlBuilder<'a> { pub(crate) mod tests { use aws_smithy_mocks_experimental::{mock_client, RuleMode}; - use super::*; use crate::clients::aws::s3; use crate::env::Config; use crate::routes::list::tests::mock_get_object; + use super::*; + #[tokio::test] async fn presign() { let client = s3::Client::new(mock_client!( @@ -161,7 +175,7 @@ pub(crate) mod tests { let builder = PresignedUrlBuilder::new(&client, &config).set_object_size(None); let url = builder - .presign_url("0", "1", ContentDisposition::Inline) + .presign_url("0", "1", ContentDisposition::Inline, None, None) .await .unwrap() .unwrap(); @@ -173,7 +187,7 @@ pub(crate) mod tests { let builder = PresignedUrlBuilder::new(&client, &config).set_object_size(Some(2)); let url = builder - .presign_url("0", "1", ContentDisposition::Inline) + .presign_url("0", "1", ContentDisposition::Inline, None, None) .await .unwrap() .unwrap(); @@ -184,6 +198,33 @@ pub(crate) mod tests { assert_eq!(url.path(), "/1/0"); } + #[tokio::test] + async fn presign_mirror_headers() { + let client = s3::Client::new(mock_client!( + aws_sdk_s3, + RuleMode::MatchAny, + &[&mock_get_object("0", "1", b""),] + )); + let config = Default::default(); + + let builder = PresignedUrlBuilder::new(&client, &config).set_object_size(None); + let url = builder + .presign_url( + "0", + "1", + ContentDisposition::Inline, + Some("application/json".to_string()), + Some("gzip".to_string()), + ) + .await + .unwrap() + .unwrap(); + + let query = url.query().unwrap(); + assert_presigned_params(query, "inline"); + assert_eq!(url.path(), "/1/0"); + } + #[tokio::test] async fn presign_attachment() { let client = s3::Client::new(mock_client!( @@ -195,7 +236,7 @@ pub(crate) mod tests { let builder = PresignedUrlBuilder::new(&client, &config).set_object_size(None); let url = builder - .presign_url("0", "1", ContentDisposition::Attachment) + .presign_url("0", "1", ContentDisposition::Attachment, None, None) .await .unwrap() .unwrap(); @@ -220,7 +261,7 @@ pub(crate) mod tests { let builder = PresignedUrlBuilder::new(&client, &config).set_object_size(Some(2)); let url = builder - .presign_url("0", "1", ContentDisposition::Inline) + .presign_url("0", "1", ContentDisposition::Inline, None, None) .await .unwrap(); @@ -241,7 +282,7 @@ pub(crate) mod tests { let builder = PresignedUrlBuilder::new(&client, &config).set_object_size(Some(2)); let url = builder - .presign_url("0", "1", ContentDisposition::Inline) + .presign_url("0", "1", ContentDisposition::Inline, None, None) .await .unwrap() .unwrap(); @@ -251,4 +292,13 @@ pub(crate) mod tests { assert!(query.contains("response-content-disposition=inline")); assert_eq!(url.path(), "/1/0"); } + + pub(crate) fn assert_presigned_params(query: &str, content_disposition: &str) { + assert!(query.contains("X-Amz-Expires=300")); + assert!(query.contains(&format!( + "response-content-disposition={content_disposition}" + ))); + assert!(query.contains("response-content-type=application%2Fjson")); + assert!(query.contains("response-content-encoding=gzip")); + } } diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/update.rs b/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/update.rs index 3fb4eded2..0bc78df37 100644 --- a/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/update.rs +++ b/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/update.rs @@ -154,15 +154,14 @@ pub fn update_router() -> Router { #[cfg(test)] mod tests { - use crate::queries::update::tests::{assert_contains, entries_many}; use axum::body::Body; use axum::http::{Method, StatusCode}; use serde_json::json; + use serde_json::Value; use sqlx::PgPool; use crate::database::aws::migration::tests::MIGRATOR; - - use super::*; + use crate::queries::update::tests::{assert_contains, entries_many}; use crate::queries::update::tests::{ assert_correct_records, assert_model_contains, assert_wildcard_update, change_attribute_entries, change_attributes, change_many, @@ -170,7 +169,8 @@ mod tests { use crate::queries::EntriesBuilder; use crate::routes::list::tests::response_from; use crate::uuid::UuidGenerator; - use serde_json::Value; + + use super::*; #[sqlx::test(migrator = "MIGRATOR")] async fn update_attribute_api_unsupported(pool: PgPool) { From eb59b7a43f21e837ed2ed75d7697a0dbb3669045 Mon Sep 17 00:00:00 2001 From: Marko Malenic Date: Fri, 23 Aug 2024 16:14:46 +1000 Subject: [PATCH 2/9] feat(filemanager): make CORS allow methods and allow headers configurable --- compose.yml | 1 + .../stateless/stacks/filemanager/.env.example | 1 + .../stateless/stacks/filemanager/compose.yml | 1 + .../stacks/filemanager/docs/API_GUIDE.md | 14 ++-- .../stacks/filemanager/filemanager/src/env.rs | 64 ++++++++++++++++++- .../filemanager/src/queries/list.rs | 1 - .../filemanager/filemanager/src/routes/mod.rs | 41 ++++++++---- 7 files changed, 101 insertions(+), 22 deletions(-) diff --git a/compose.yml b/compose.yml index 0d041dff1..c5d5bfebc 100644 --- a/compose.yml +++ b/compose.yml @@ -71,6 +71,7 @@ services: - DATABASE_URL=postgresql://orcabus:orcabus@db:5432/filemanager - RUST_LOG=debug - FILEMANAGER_API_CORS_ALLOW_ORIGINS=${FILEMANAGER_API_CORS_ALLOW_ORIGINS:-http://localhost:8400} + - FILEMANAGER_API_CORS_ALLOW_HEADERS=${FILEMANAGER_API_CORS_ALLOW_HEADERS:-accept,authorization,content-type,user-agent,x-csrftoken,x-requested-with,x-amz-security-token,x-amz-date,content-disposition} - AWS_ACCESS_KEY_ID=${AWS_ACCESS_KEY_ID:-access_key_id} - AWS_SECRET_ACCESS_KEY=${AWS_SECRET_ACCESS_KEY:-secret_access_key} - AWS_DEFAULT_REGION=${AWS_DEFAULT_REGION:-ap-southeast-2} diff --git a/lib/workload/stateless/stacks/filemanager/.env.example b/lib/workload/stateless/stacks/filemanager/.env.example index a14ddbb25..88a97b5da 100644 --- a/lib/workload/stateless/stacks/filemanager/.env.example +++ b/lib/workload/stateless/stacks/filemanager/.env.example @@ -6,3 +6,4 @@ DATABASE_URL=postgresql://filemanager:filemanager@${FILEMANAGER_DATABASE_HOST}:$ FILEMANAGER_LINKS_URL=localhost:8000 FILEMANAGER_API_CORS_ALLOW_ORIGINS=http://localhost:8000 +FILEMANAGER_API_CORS_ALLOW_HEADERS=accept,authorization,content-type,user-agent,x-csrftoken,x-requested-with,x-amz-security-token,x-amz-date,content-disposition diff --git a/lib/workload/stateless/stacks/filemanager/compose.yml b/lib/workload/stateless/stacks/filemanager/compose.yml index 9cce1d27c..a90a799d6 100644 --- a/lib/workload/stateless/stacks/filemanager/compose.yml +++ b/lib/workload/stateless/stacks/filemanager/compose.yml @@ -20,6 +20,7 @@ services: - DATABASE_URL=postgresql://filemanager:filemanager@postgres:4321/filemanager - RUST_LOG=debug - FILEMANAGER_API_CORS_ALLOW_ORIGINS=${FILEMANAGER_API_CORS_ALLOW_ORIGINS:-http://localhost:8000} + - FILEMANAGER_API_CORS_ALLOW_HEADERS=${FILEMANAGER_API_CORS_ALLOW_HEADERS:-accept,authorization,content-type,user-agent,x-csrftoken,x-requested-with,x-amz-security-token,x-amz-date,content-disposition} - AWS_ACCESS_KEY_ID=${AWS_ACCESS_KEY_ID:-access_key_id} - AWS_SECRET_ACCESS_KEY=${AWS_SECRET_ACCESS_KEY:-secret_access_key} - AWS_DEFAULT_REGION=${AWS_DEFAULT_REGION:-ap-southeast-2} diff --git a/lib/workload/stateless/stacks/filemanager/docs/API_GUIDE.md b/lib/workload/stateless/stacks/filemanager/docs/API_GUIDE.md index f1c38c35a..2969d0169 100644 --- a/lib/workload/stateless/stacks/filemanager/docs/API_GUIDE.md +++ b/lib/workload/stateless/stacks/filemanager/docs/API_GUIDE.md @@ -14,12 +14,14 @@ This serves Swagger OpenAPI docs at `http://localhost:8000/swagger-ui` when usin The API has some environment variables that can be used to configure behaviour (for the presigned url route): -| Option | Description | Type | Default | -|--------------------------------------|--------------------------------------------------------------------------------------------------------------------------------|---------------------|-----------------------------| -| `FILEMANAGER_API_LINKS_URL` | Override the URL which is used to generate pagination links. By default the `HOST` header is used to created pagination links. | URL | Not set | -| `FILEMANAGER_API_PRESIGN_LIMIT` | The maximum file size in bytes which presigned URLs will be generated for. | Integer | `"20971520"` | -| `FILEMANAGER_API_PRESIGN_EXPIRY` | The expiry time for presigned urls. | Duration in seconds | `"300"` | -| `FILEMANAGER_API_CORS_ALLOW_ORIGINS` | The origins to allow for CORS. | List of origins | Not set, no origins allowed | +| Option | Description | Type | Default | +|--------------------------------------|--------------------------------------------------------------------------------------------------------------------------------|---------------------|---------------------------------| +| `FILEMANAGER_API_LINKS_URL` | Override the URL which is used to generate pagination links. By default the `HOST` header is used to created pagination links. | URL | Not set | +| `FILEMANAGER_API_PRESIGN_LIMIT` | The maximum file size in bytes which presigned URLs will be generated for. | Integer | `"20971520"` | +| `FILEMANAGER_API_PRESIGN_EXPIRY` | The expiry time for presigned urls. | Duration in seconds | `"300"` | +| `FILEMANAGER_API_CORS_ALLOW_ORIGINS` | The origins to allow for CORS. | List of origins | Not set, no origins allowed | +| `FILEMANAGER_API_CORS_ALLOW_METHODS` | The methods to allow for CORS. | List of origins | `"GET,HEAD,OPTIONS,POST,PATCH"` | +| `FILEMANAGER_API_CORS_ALLOW_HEADERS` | The headers to allow for CORS. | List of origins | `"authorization"` | The deployed instance of the filemanager API can be reached using the desired stage at `https://file..umccr.org` using the orcabus API token. To retrieve the token, run: diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/src/env.rs b/lib/workload/stateless/stacks/filemanager/filemanager/src/env.rs index 839698619..9e1591943 100644 --- a/lib/workload/stateless/stacks/filemanager/filemanager/src/env.rs +++ b/lib/workload/stateless/stacks/filemanager/filemanager/src/env.rs @@ -1,6 +1,8 @@ //! Handles loading environment variables as config options for filemanager. //! +use axum::http::header::AUTHORIZATION; +use axum::http::Method; use chrono::Duration; use envy::from_env; use serde::Deserialize; @@ -13,7 +15,7 @@ use crate::error::Result; /// Configuration environment variables for filemanager. #[serde_as] -#[derive(Debug, Clone, Deserialize, Default, Eq, PartialEq)] +#[derive(Debug, Clone, Deserialize, Eq, PartialEq)] pub struct Config { pub(crate) database_url: Option, pub(crate) pgpassword: Option, @@ -33,6 +35,50 @@ pub struct Config { pub(crate) api_presign_expiry: Option, #[serde(rename = "filemanager_api_cors_allow_origins")] pub(crate) api_cors_allow_origins: Option>, + #[serde( + default = "default_allow_methods", + rename = "filemanager_api_cors_allow_methods" + )] + pub(crate) api_cors_allow_methods: Vec, + #[serde( + default = "default_allow_headers", + rename = "filemanager_api_cors_allow_headers" + )] + pub(crate) api_cors_allow_headers: Vec, +} + +impl Default for Config { + fn default() -> Self { + Self { + database_url: None, + pgpassword: None, + pghost: None, + pgport: None, + pguser: None, + sqs_url: None, + paired_ingest_mode: false, + api_links_url: None, + api_presign_limit: None, + api_presign_expiry: None, + api_cors_allow_origins: None, + api_cors_allow_methods: default_allow_methods(), + api_cors_allow_headers: default_allow_headers(), + } + } +} + +fn default_allow_methods() -> Vec { + vec![ + Method::GET.to_string(), + Method::HEAD.to_string(), + Method::OPTIONS.to_string(), + Method::POST.to_string(), + Method::PATCH.to_string(), + ] +} + +fn default_allow_headers() -> Vec { + vec![AUTHORIZATION.to_string()] } impl Config { @@ -107,6 +153,16 @@ impl Config { self.api_cors_allow_origins.as_deref() } + /// Get the allowed origins + pub fn api_cors_allow_methods(&self) -> &[String] { + self.api_cors_allow_methods.as_slice() + } + + /// Get the allowed origins + pub fn api_cors_allow_headers(&self) -> &[String] { + self.api_cors_allow_headers.as_slice() + } + /// Get the value from an optional, or else try and get a different value, unwrapping into a Result. pub fn value_or_else(value: Option, or_else: Option) -> Result { value @@ -143,6 +199,8 @@ mod tests { "FILEMANAGER_API_CORS_ALLOW_ORIGINS", "localhost:8000,127.0.0.1", ), + ("FILEMANAGER_API_CORS_ALLOW_METHODS", "GET,POST"), + ("FILEMANAGER_API_CORS_ALLOW_HEADERS", "Authorization,Accept"), ] .into_iter() .map(|(key, value)| (key.to_string(), value.to_string())); @@ -165,7 +223,9 @@ mod tests { api_cors_allow_origins: Some(vec![ "localhost:8000".to_string(), "127.0.0.1".to_string() - ]) + ]), + api_cors_allow_methods: vec!["GET".to_string(), "POST".to_string()], + api_cors_allow_headers: vec!["Authorization".to_string(), "Accept".to_string()] } ) } diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/src/queries/list.rs b/lib/workload/stateless/stacks/filemanager/filemanager/src/queries/list.rs index f16bb711c..1d8274b14 100644 --- a/lib/workload/stateless/stacks/filemanager/filemanager/src/queries/list.rs +++ b/lib/workload/stateless/stacks/filemanager/filemanager/src/queries/list.rs @@ -449,7 +449,6 @@ where "{message}: {}", self.select.as_query().to_string(PostgresQueryBuilder) ); - println!("{}", self.select.as_query().to_string(PostgresQueryBuilder)); } } diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/mod.rs b/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/mod.rs index b404f2c0e..df326cbf3 100644 --- a/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/mod.rs +++ b/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/mod.rs @@ -3,8 +3,9 @@ use std::sync::Arc; -use axum::http::header::AUTHORIZATION; -use axum::http::{HeaderValue, Method}; +use axum::http::header::InvalidHeaderName; +use axum::http::method::InvalidMethod; +use axum::http::HeaderValue; use axum::Router; use chrono::Duration; use sqlx::PgPool; @@ -124,23 +125,37 @@ pub fn router(state: AppState) -> Result { } /// Configure the cors layer -pub fn cors_layer(allow_origins: Option<&[String]>) -> Result { +pub fn cors_layer(config: &Config) -> Result { let mut layer = CorsLayer::new() - .allow_headers([AUTHORIZATION]) - .allow_methods([ - Method::GET, - Method::HEAD, - Method::OPTIONS, - Method::POST, - Method::PATCH, - ]) + .allow_headers( + config + .api_cors_allow_headers() + .iter() + .map(|method| { + method + .parse() + .map_err(|err: InvalidHeaderName| ApiConfigurationError(err.to_string())) + }) + .collect::>>()?, + ) + .allow_methods( + config + .api_cors_allow_methods() + .iter() + .map(|method| { + method + .parse() + .map_err(|err: InvalidMethod| ApiConfigurationError(err.to_string())) + }) + .collect::>>()?, + ) .max_age( Duration::days(10) .to_std() .map_err(|err| ApiConfigurationError(err.to_string()))?, ); - if let Some(origins) = allow_origins { + if let Some(origins) = config.api_cors_allow_origins() { let origins = origins .iter() .map(|origin| { @@ -164,7 +179,7 @@ pub fn api_router(state: AppState) -> Result { .merge(ingest_router()) .merge(list_router()) .merge(update_router()) - .layer(cors_layer(state.config().api_cors_allow_origins())?) + .layer(cors_layer(state.config())?) .layer(TraceLayer::new_for_http()) .with_state(state)) } From c78d6a87c69b2223c6e5215b2c74fb93c8ddcc0a Mon Sep 17 00:00:00 2001 From: Marko Malenic Date: Fri, 23 Aug 2024 16:55:39 +1000 Subject: [PATCH 3/9] feat(filemanager): add convenience attributes route --- .../stacks/filemanager/docs/API_GUIDE.md | 8 ++ .../filemanager/src/routes/filter/mod.rs | 46 ++++++++++- .../filemanager/src/routes/list.rs | 76 ++++++++++++++++++- .../filemanager/src/routes/openapi.rs | 1 + 4 files changed, 127 insertions(+), 4 deletions(-) diff --git a/lib/workload/stateless/stacks/filemanager/docs/API_GUIDE.md b/lib/workload/stateless/stacks/filemanager/docs/API_GUIDE.md index 2969d0169..37b266335 100644 --- a/lib/workload/stateless/stacks/filemanager/docs/API_GUIDE.md +++ b/lib/workload/stateless/stacks/filemanager/docs/API_GUIDE.md @@ -104,6 +104,14 @@ curl --get -H "Authorization: Bearer $TOKEN" --data-urlencode "attributes[portal > Attributes on filemanager records start empty. They need to be added to the record to query on them later. > See [updating records](#updating-records) +As a convience, the filemanager has an attributes route that can be used to query by top-level attribute properties. +For example, the following is equivalent to the above query: + +```sh +curl --get -H "Authorization: Bearer $TOKEN" --data-urlencode "portalRunId=202405212aecb782" \ +"https://file.dev.umccr.org/api/v1/s3/attributes" | jq +``` + ### Wilcard matching The API supports using wildcards to match multiple characters in a value for most field. Use `*` to match multiple characters diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/filter/mod.rs b/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/filter/mod.rs index 42b3bcd62..18d536647 100644 --- a/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/filter/mod.rs +++ b/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/filter/mod.rs @@ -3,6 +3,7 @@ use sea_orm::prelude::{DateTimeWithTimeZone, Json}; use serde::{Deserialize, Serialize}; +use serde_json::Map; use utoipa::IntoParams; use crate::database::entities::sea_orm_active_enums::{EventType, StorageClass}; @@ -10,10 +11,26 @@ use crate::routes::filter::wildcard::{Wildcard, WildcardEither}; pub mod wildcard; +/// Capture any parameters and assume that they are top-level attributes fields. +#[derive(Serialize, Deserialize, Debug, Default, Eq, PartialEq, IntoParams)] +#[serde(default, transparent, rename_all = "camelCase")] +#[into_params(names("params"), parameter_in = Query)] +pub struct AttributesOnlyFilter(Map); + +impl From for S3ObjectsFilter { + /// Convert to `S3ObjectsFilter`, merging into `attributes`. + fn from(value: AttributesOnlyFilter) -> Self { + Self { + attributes: Some(Json::Object(value.0)), + ..Default::default() + } + } +} + /// The available fields to filter `s3_object` queries by. Each query parameter represents /// an `and` clause in the SQL statement. Nested query string style syntax is supported on /// JSON attributes. Wildcards are supported on some of the fields. -#[derive(Serialize, Deserialize, Debug, Default, IntoParams, Clone)] +#[derive(Serialize, Deserialize, Debug, Default, IntoParams, Clone, PartialEq, Eq)] #[serde(default, rename_all = "camelCase")] #[into_params(parameter_in = Query)] pub struct S3ObjectsFilter { @@ -58,3 +75,30 @@ pub struct S3ObjectsFilter { /// rather than `{ "attribute_id" = 1 }`. Supports wildcards. pub(crate) attributes: Option, } + +#[cfg(test)] +mod tests { + use serde_json::json; + + use super::*; + + #[test] + fn deserialize_attribute_only_filter() { + let qs = "key=key&bucket=bucket&attributeId=attributeId&nestedId[attributeId]=wildcard*"; + let params: AttributesOnlyFilter = serde_qs::from_str(qs).unwrap(); + assert_eq!( + S3ObjectsFilter::from(params), + S3ObjectsFilter { + attributes: Some(json!({ + "key": "key", + "bucket": "bucket", + "attributeId": "attributeId", + "nestedId": { + "attributeId": "wildcard*" + } + })), + ..Default::default() + } + ); + } +} diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/list.rs b/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/list.rs index 21c01a2ff..ce4adab25 100644 --- a/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/list.rs +++ b/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/list.rs @@ -1,8 +1,6 @@ //! Route logic for list API calls. //! -use std::marker::PhantomData; - use axum::extract::{Request, State}; use axum::http::header::{CONTENT_ENCODING, CONTENT_TYPE, HOST}; use axum::routing::get; @@ -10,6 +8,7 @@ use axum::{extract, Json, Router}; use axum_extra::extract::WithRejection; use sea_orm::TransactionTrait; use serde::{Deserialize, Serialize}; +use std::marker::PhantomData; use url::Url; use utoipa::{IntoParams, ToSchema}; @@ -19,7 +18,7 @@ use crate::error::Error::MissingHostHeader; use crate::error::Result; use crate::queries::list::ListQueryBuilder; use crate::routes::error::{ErrorStatusCode, QsQuery, Query}; -use crate::routes::filter::S3ObjectsFilter; +use crate::routes::filter::{AttributesOnlyFilter, S3ObjectsFilter}; use crate::routes::header::HeaderParser; use crate::routes::pagination::{ListResponse, Pagination}; use crate::routes::presign::{PresignedParams, PresignedUrlBuilder}; @@ -258,12 +257,50 @@ pub async fn presign_s3( Ok(Json(response)) } +/// List all S3 objects according to a set of attribute filter parameters. +/// This route is a convenience for querying using top-level attributes and accepts arbitrary +/// parameters. For example, instead of using `/api/v1/s3?attributes[attributeId]=...`, this route +/// can express the same query as `/api/v1/s3/attributes?attributeId=...`. Similar to the +/// `attributes` filter parameter, nested JSON queries are supported using the bracket notation. +/// Note that regular filtering parameters, like `key` or `bucket` are not supported on this route. +#[utoipa::path( + get, + path = "/s3/attributes", + responses( + (status = OK, description = "The collection of s3_objects", body = ListResponseS3), + ErrorStatusCode, + ), + params(Pagination, WildcardParams, ListS3Params, AttributesOnlyFilter), + context_path = "/api/v1", + tag = "list", +)] +pub async fn attributes_s3( + state: State, + pagination: Query, + wildcard: Query, + list: Query, + WithRejection(serde_qs::axum::QsQuery(attributes_only), _): QsQuery, + request: Request, +) -> Result>> { + let filter = S3ObjectsFilter::from(attributes_only); + list_s3( + state, + pagination, + wildcard, + list, + WithRejection(serde_qs::axum::QsQuery(filter), PhantomData), + request, + ) + .await +} + /// The router for list objects. pub fn list_router() -> Router { Router::new() .route("/s3", get(list_s3)) .route("/s3/count", get(count_s3)) .route("/s3/presign", get(presign_s3)) + .route("/s3/attributes", get(attributes_s3)) } #[cfg(test)] @@ -628,6 +665,39 @@ pub(crate) mod tests { assert_eq!(result.pagination().count, 1); } + #[sqlx::test(migrator = "MIGRATOR")] + async fn attributes_s3(pool: PgPool) { + let state = AppState::from_pool(pool).await; + let entries = EntriesBuilder::default() + .with_shuffle(true) + .build(state.database_client()) + .await + .s3_objects; + + let result: ListResponse = + response_from_get(state.clone(), "/s3/attributes?attributeId=1").await; + assert_eq!(result.results(), vec![entries[1].clone()]); + assert_eq!(result.pagination().count, 1); + + let result: ListResponse = + response_from_get(state.clone(), "/s3/attributes?nestedId[attributeId]=4").await; + assert_eq!(result.results(), vec![entries[4].clone()]); + assert_eq!(result.pagination().count, 1); + + let result: ListResponse = + response_from_get(state.clone(), "/s3/attributes?nonExistentId=1").await; + assert!(result.results().is_empty()); + assert_eq!(result.pagination().count, 0); + + let result: ListResponse = response_from_get( + state.clone(), + "/s3/attributes?attributeId=1&nonExistentId=2", + ) + .await; + assert!(result.results().is_empty()); + assert_eq!(result.pagination().count, 0); + } + #[sqlx::test(migrator = "MIGRATOR")] async fn list_s3_filter_attributes_wildcard(pool: PgPool) { let state = AppState::from_pool(pool).await; diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/openapi.rs b/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/openapi.rs index 897f12ba0..5747cdce9 100644 --- a/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/openapi.rs +++ b/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/openapi.rs @@ -35,6 +35,7 @@ pub struct Json(pub Value); paths( list_s3, presign_s3, + attributes_s3, get_s3_by_id, presign_s3_by_id, count_s3, From bad848a1a45711c0a1f52eba8a9d36e665c70b88 Mon Sep 17 00:00:00 2001 From: Victor San Kho Lin Date: Sat, 24 Aug 2024 18:19:44 +1000 Subject: [PATCH 4/9] Fixed WorkflowRun State creation business logic * WorkflowRun State creation is WRSC timestamp dependant. It needs to check "time window" condition before saving and emitting (relaying) WRSC event. --- .../workflow_manager_proc/services/create_workflow_run.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/workload/stateless/stacks/workflow-manager/workflow_manager_proc/services/create_workflow_run.py b/lib/workload/stateless/stacks/workflow-manager/workflow_manager_proc/services/create_workflow_run.py index 267da069a..ca8ebe72d 100644 --- a/lib/workload/stateless/stacks/workflow-manager/workflow_manager_proc/services/create_workflow_run.py +++ b/lib/workload/stateless/stacks/workflow-manager/workflow_manager_proc/services/create_workflow_run.py @@ -69,7 +69,7 @@ def handler(event, context): wfr.save() # create the related state & payload entries for the WRSC - create_workflow_run_state(wrsc=wrsc, wfr=wfr) + # create_workflow_run_state(wrsc=wrsc, wfr=wfr) # FIXME State creation is "time window" WRSC timestamp dependant # if the workflow run is linked to library record(s), create the association(s) input_libraries: list[LibraryRecord] = wrsc.linkedLibraries From 17aae7f6e69fcb5d982e96178a12926d9819e5f0 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sun, 25 Aug 2024 23:48:09 +0000 Subject: [PATCH 5/9] build(deps): bump micromatch from 4.0.7 to 4.0.8 Bumps [micromatch](https://github.com/micromatch/micromatch) from 4.0.7 to 4.0.8. - [Release notes](https://github.com/micromatch/micromatch/releases) - [Changelog](https://github.com/micromatch/micromatch/blob/4.0.8/CHANGELOG.md) - [Commits](https://github.com/micromatch/micromatch/compare/4.0.7...4.0.8) --- updated-dependencies: - dependency-name: micromatch dependency-type: indirect ... Signed-off-by: dependabot[bot] --- yarn.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/yarn.lock b/yarn.lock index 7566f84af..491b46630 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3752,12 +3752,12 @@ __metadata: linkType: hard "micromatch@npm:^4.0.4": - version: 4.0.7 - resolution: "micromatch@npm:4.0.7" + version: 4.0.8 + resolution: "micromatch@npm:4.0.8" dependencies: braces: "npm:^3.0.3" picomatch: "npm:^2.3.1" - checksum: 10/a11ed1cb67dcbbe9a5fc02c4062cf8bb0157d73bf86956003af8dcfdf9b287f9e15ec0f6d6925ff6b8b5b496202335e497b01de4d95ef6cf06411bc5e5c474a0 + checksum: 10/6bf2a01672e7965eb9941d1f02044fad2bd12486b5553dc1116ff24c09a8723157601dc992e74c911d896175918448762df3b3fd0a6b61037dd1a9766ddfbf58 languageName: node linkType: hard From a81e391c33b605ab053d850c83a3b7b83a4a76be Mon Sep 17 00:00:00 2001 From: Marko Malenic Date: Mon, 26 Aug 2024 11:33:51 +1000 Subject: [PATCH 6/9] fix(filemanager): increase migrate function timeout --- .../filemanager/deploy/constructs/functions/api.ts | 2 +- .../deploy/constructs/functions/function.ts | 14 +++++++++----- .../deploy/constructs/functions/ingest.ts | 2 +- .../deploy/constructs/functions/inventory.ts | 2 +- .../deploy/constructs/functions/migrate.ts | 9 +++++++-- 5 files changed, 19 insertions(+), 10 deletions(-) diff --git a/lib/workload/stateless/stacks/filemanager/deploy/constructs/functions/api.ts b/lib/workload/stateless/stacks/filemanager/deploy/constructs/functions/api.ts index 5182a8117..2466dd508 100644 --- a/lib/workload/stateless/stacks/filemanager/deploy/constructs/functions/api.ts +++ b/lib/workload/stateless/stacks/filemanager/deploy/constructs/functions/api.ts @@ -6,7 +6,7 @@ import { BucketProps } from './ingest'; /** * Props for the API function. */ -export type ApiFunctionProps = fn.FunctionPropsNoPackage & DatabaseProps & BucketProps; +export type ApiFunctionProps = fn.FunctionPropsConfigurable & DatabaseProps & BucketProps; /** * A construct for the Lambda API function. diff --git a/lib/workload/stateless/stacks/filemanager/deploy/constructs/functions/function.ts b/lib/workload/stateless/stacks/filemanager/deploy/constructs/functions/function.ts index df7f26d2e..bc0ef1bef 100644 --- a/lib/workload/stateless/stacks/filemanager/deploy/constructs/functions/function.ts +++ b/lib/workload/stateless/stacks/filemanager/deploy/constructs/functions/function.ts @@ -31,9 +31,9 @@ export type DatabaseProps = { }; /** - * Props for a Rust function without the package. + * Props for a Rust function which can be configured from the top-level orcabus context. */ -export type FunctionPropsNoPackage = { +export type FunctionPropsConfigurable = { /** * Additional build environment variables when building the Lambda function. */ @@ -57,9 +57,9 @@ export type FunctionPropsNoPackage = { }; /** - * Props for the Rust function. + * Props for the Rust function which can be configured from the top-level orcabus context. */ -export type FunctionProps = FunctionPropsNoPackage & +export type FunctionProps = FunctionPropsConfigurable & DatabaseProps & { /** * The package to build for this function. @@ -69,6 +69,10 @@ export type FunctionProps = FunctionPropsNoPackage & * Name of the Lambda function resource. */ readonly functionName?: string; + /** + * The timeout for the Lambda function, defaults to 28 seconds. + */ + readonly timeout?: Duration; }; /** @@ -121,7 +125,7 @@ export class Function extends Construct { }, }, memorySize: 128, - timeout: Duration.seconds(28), + timeout: props.timeout ?? Duration.seconds(28), environment: { // No password here, using RDS IAM to generate credentials. PGHOST: props.host, diff --git a/lib/workload/stateless/stacks/filemanager/deploy/constructs/functions/ingest.ts b/lib/workload/stateless/stacks/filemanager/deploy/constructs/functions/ingest.ts index 0c3628591..9ff5eece4 100644 --- a/lib/workload/stateless/stacks/filemanager/deploy/constructs/functions/ingest.ts +++ b/lib/workload/stateless/stacks/filemanager/deploy/constructs/functions/ingest.ts @@ -28,7 +28,7 @@ export type EventSourceProps = { /** * Props for the ingest function. */ -export type IngestFunctionProps = fn.FunctionPropsNoPackage & DatabaseProps & EventSourceProps; +export type IngestFunctionProps = fn.FunctionPropsConfigurable & DatabaseProps & EventSourceProps; /** * A construct for the Lambda ingest function. diff --git a/lib/workload/stateless/stacks/filemanager/deploy/constructs/functions/inventory.ts b/lib/workload/stateless/stacks/filemanager/deploy/constructs/functions/inventory.ts index a0eed6a43..8555800da 100644 --- a/lib/workload/stateless/stacks/filemanager/deploy/constructs/functions/inventory.ts +++ b/lib/workload/stateless/stacks/filemanager/deploy/constructs/functions/inventory.ts @@ -19,7 +19,7 @@ export type InventoryFunctionConfig = { /** * Props for the inventory function. */ -export type InventoryFunctionProps = fn.FunctionPropsNoPackage & +export type InventoryFunctionProps = fn.FunctionPropsConfigurable & DatabaseProps & InventoryFunctionConfig; diff --git a/lib/workload/stateless/stacks/filemanager/deploy/constructs/functions/migrate.ts b/lib/workload/stateless/stacks/filemanager/deploy/constructs/functions/migrate.ts index 97abb6e0f..3556b0c20 100644 --- a/lib/workload/stateless/stacks/filemanager/deploy/constructs/functions/migrate.ts +++ b/lib/workload/stateless/stacks/filemanager/deploy/constructs/functions/migrate.ts @@ -1,17 +1,22 @@ import { Construct } from 'constructs'; import * as fn from './function'; import { DatabaseProps } from './function'; +import { Duration } from 'aws-cdk-lib'; /** * Props for the migrate function. */ -export type MigrateFunctionProps = fn.FunctionPropsNoPackage & DatabaseProps; +export type MigrateFunctionProps = fn.FunctionPropsConfigurable & DatabaseProps; /** * A construct for the Lambda migrate function. */ export class MigrateFunction extends fn.Function { constructor(scope: Construct, id: string, props: MigrateFunctionProps) { - super(scope, id, { package: 'filemanager-migrate-lambda', ...props }); + super(scope, id, { + package: 'filemanager-migrate-lambda', + timeout: Duration.minutes(2), + ...props, + }); } } From 903ff7bab271b3c9766e923f962c7d1fe60f432f Mon Sep 17 00:00:00 2001 From: Marko Malenic Date: Mon, 26 Aug 2024 15:22:16 +1000 Subject: [PATCH 7/9] fix(filemanager): check stack status to ensure that migration doesn't get stuck in UPDATE_ROLLBACK_FAILED --- .../stateless/stacks/filemanager/Cargo.lock | 25 +++ .../deploy/constructs/functions/migrate.ts | 11 +- .../filemanager-migrate-lambda/Cargo.toml | 3 + .../filemanager-migrate-lambda/src/main.rs | 163 +++++++----------- 4 files changed, 104 insertions(+), 98 deletions(-) diff --git a/lib/workload/stateless/stacks/filemanager/Cargo.lock b/lib/workload/stateless/stacks/filemanager/Cargo.lock index 7e307a632..456c3063e 100644 --- a/lib/workload/stateless/stacks/filemanager/Cargo.lock +++ b/lib/workload/stateless/stacks/filemanager/Cargo.lock @@ -548,6 +548,30 @@ dependencies = [ "uuid", ] +[[package]] +name = "aws-sdk-cloudformation" +version = "1.30.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8ace4f1ef88afc41ef46a3343ce9ac49f78e0d03dbf749ed27e9f032004f1581" +dependencies = [ + "aws-credential-types", + "aws-runtime", + "aws-smithy-async", + "aws-smithy-http", + "aws-smithy-json", + "aws-smithy-query", + "aws-smithy-runtime", + "aws-smithy-runtime-api", + "aws-smithy-types", + "aws-smithy-xml", + "aws-types", + "fastrand", + "http 0.2.12", + "once_cell", + "regex-lite", + "tracing", +] + [[package]] name = "aws-sdk-s3" version = "1.29.0" @@ -2260,6 +2284,7 @@ dependencies = [ name = "filemanager-migrate-lambda" version = "0.1.0" dependencies = [ + "aws-sdk-cloudformation", "aws_lambda_events", "filemanager", "lambda_runtime", diff --git a/lib/workload/stateless/stacks/filemanager/deploy/constructs/functions/migrate.ts b/lib/workload/stateless/stacks/filemanager/deploy/constructs/functions/migrate.ts index 3556b0c20..ed3e42605 100644 --- a/lib/workload/stateless/stacks/filemanager/deploy/constructs/functions/migrate.ts +++ b/lib/workload/stateless/stacks/filemanager/deploy/constructs/functions/migrate.ts @@ -1,7 +1,8 @@ import { Construct } from 'constructs'; import * as fn from './function'; import { DatabaseProps } from './function'; -import { Duration } from 'aws-cdk-lib'; +import { Duration, Stack } from 'aws-cdk-lib'; +import { PolicyStatement } from 'aws-cdk-lib/aws-iam'; /** * Props for the migrate function. @@ -18,5 +19,13 @@ export class MigrateFunction extends fn.Function { timeout: Duration.minutes(2), ...props, }); + + // Need to be able to determine if the stack is in rollback state. + this.addToPolicy( + new PolicyStatement({ + actions: ['cloudformation:DescribeStacks'], + resources: [Stack.of(this).stackId], + }) + ); } } diff --git a/lib/workload/stateless/stacks/filemanager/filemanager-migrate-lambda/Cargo.toml b/lib/workload/stateless/stacks/filemanager/filemanager-migrate-lambda/Cargo.toml index db96868ec..cb96d3188 100644 --- a/lib/workload/stateless/stacks/filemanager/filemanager-migrate-lambda/Cargo.toml +++ b/lib/workload/stateless/stacks/filemanager/filemanager-migrate-lambda/Cargo.toml @@ -8,10 +8,13 @@ rust-version.workspace = true [dependencies] serde = { version = "1", features = ["derive"] } +serde_json = "1" + tokio = { version = "1", features = ["macros"] } tracing = { version = "0.1" } aws_lambda_events = "0.15" +aws-sdk-cloudformation = "1" lambda_runtime = "0.13" filemanager = { path = "../filemanager", features = ["migrate"] } diff --git a/lib/workload/stateless/stacks/filemanager/filemanager-migrate-lambda/src/main.rs b/lib/workload/stateless/stacks/filemanager/filemanager-migrate-lambda/src/main.rs index cdf757396..c0db09df6 100644 --- a/lib/workload/stateless/stacks/filemanager/filemanager-migrate-lambda/src/main.rs +++ b/lib/workload/stateless/stacks/filemanager/filemanager-migrate-lambda/src/main.rs @@ -1,33 +1,15 @@ -use lambda_runtime::{run, service_fn, Error, LambdaEvent}; -use serde::de::IgnoredAny; -use serde::Deserialize; - -use crate::CloudFormationRequest::Delete; -use crate::Event::Provider; +use aws_lambda_events::cloudformation::provider::CloudFormationCustomResourceRequest; +use aws_sdk_cloudformation::types::StackStatus; +use aws_sdk_cloudformation::Client; +use filemanager::clients::aws::config; use filemanager::database::aws::migration::Migration; use filemanager::database::Client as DbClient; use filemanager::database::Migrate; use filemanager::env::Config; use filemanager::handlers::aws::{create_database_pool, update_credentials}; use filemanager::handlers::init_tracing; - -/// The lambda event for this function. This is normally a CloudFormationCustomResourceRequest. -/// If anything else is present, the migrate lambda will still attempt to perform a migration. -#[derive(Debug, Deserialize)] -#[serde(untagged)] -pub enum Event { - Provider(CloudFormationRequest), - Ignored(IgnoredAny), -} - -/// Deserialize only the Delete type because this is the only event with different behaviour. -/// Todo, replace with `provider::CloudFormationCustomResourceRequest` when it gets released: -/// https://github.com/awslabs/aws-lambda-rust-runtime/pull/846 -#[derive(Debug, Deserialize)] -#[serde(tag = "RequestType")] -pub enum CloudFormationRequest { - Delete, -} +use lambda_runtime::{run, service_fn, Error, LambdaEvent}; +use tracing::trace; #[tokio::main] async fn main() -> Result<(), Error> { @@ -35,84 +17,71 @@ async fn main() -> Result<(), Error> { let config = &Config::load()?; let options = &create_database_pool(config).await?; - run(service_fn(|event: LambdaEvent| async move { - update_credentials(options, config).await?; + let cfn_client = &Client::new(&config::Config::with_defaults().await.load()); - // Migrate depending on the type of lifecycle event using the CDK provider framework: - // https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.custom_resources-readme.html#provider-framework - // - // Note, we don't care what's contained within the event, as the action will always be - // to try and migrate unless this is a Delete event. - match event.payload { - // If it's a Delete there's no need to do anything. - Provider(Delete) => Ok(()), - _ => { - // If there's nothing to migrate, then this will just return Ok. - Ok::<_, Error>( - Migration::new(DbClient::new(options.clone())) - .migrate() - .await?, - ) - } - } - })) - .await -} + run(service_fn( + |event: LambdaEvent| async move { + update_credentials(options, config).await?; -#[cfg(test)] -mod tests { - use super::*; - use crate::CloudFormationRequest::Delete; - use crate::Event::Ignored; - use serde_json::{from_value, json}; + // Migrate depending on the type of lifecycle event using the CDK provider framework: + // https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.custom_resources-readme.html#provider-framework + match event.payload { + // Migrate normally if this resource is being created. + CloudFormationCustomResourceRequest::Create(create) => { + trace!(create = ?create, "during create"); - #[test] - fn event_deserialize_provider_delete() { - // From https://github.com/awslabs/aws-lambda-rust-runtime/blob/a68de584154958c524692cb43dc208d520d05a13/lambda-events/src/fixtures/example-cloudformation-custom-resource-provider-delete-request.json - let event = json!({ - "RequestType": "Delete", - "RequestId": "ef70561d-d4ba-42a4-801b-33ad88dafc37", - "StackId": "arn:aws:cloudformation:us-east-1:123456789012:stack/stack-name/16580499-7622-4a9c-b32f-4eba35da93da", - "ResourceType": "Custom::MyCustomResourceType", - "LogicalResourceId": "CustomResource", - "PhysicalResourceId": "custom-resource-f4bd5382-3de3-4caf-b7ad-1be06b899647", - "ResourceProperties": { - "Key1" : "string", - "Key2" : ["list"], - "Key3" : { "Key4": "map" } - } - }); + Ok::<_, Error>( + Migration::new(DbClient::new(options.clone())) + .migrate() + .await?, + ) + } + // If this is an update event, then we need to check if a rollback is in progress. + CloudFormationCustomResourceRequest::Update(update) => { + trace!(update = ?update, "during update"); - // A Provider lifecycle event should deserialize into the Provider enum. - assert!(matches!(from_value(event).unwrap(), Provider(Delete))); - } + // Find the state of the top-level stack which is being updated. This will + // contain a status indicating if this is the first update, or a rollback update. + let stack_state = cfn_client + .describe_stacks() + .stack_name(update.common.stack_id.as_str()) + .send() + .await? + .stacks + .and_then(|stacks| { + stacks.into_iter().find(|stack| { + stack.stack_id() == Some(update.common.stack_id.as_str()) + }) + }) + .and_then(|stack| stack.stack_status); - #[test] - fn event_deserialize_ignored_create() { - // From https://github.com/awslabs/aws-lambda-rust-runtime/blob/a68de584154958c524692cb43dc208d520d05a13/lambda-events/src/fixtures/example-cloudformation-custom-resource-provider-create-request.json - let event = json!({ - "RequestType": "Create", - "RequestId": "82304eb2-bdda-469f-a33b-a3f1406d0a52", - "StackId": "arn:aws:cloudformation:us-east-1:123456789012:stack/stack-name/16580499-7622-4a9c-b32f-4eba35da93da", - "ResourceType": "Custom::MyCustomResourceType", - "LogicalResourceId": "CustomResource", - "ResourceProperties": { - "Key1": "string", - "Key2": ["list"], - "Key3": { "Key4": "map" } - } - }); + // Only migrate when this is a normal update. + if let Some(ref status) = stack_state { + trace!(stack_state = ?stack_state); - // Any non-deleted cloud formation event data should be ignored. - assert!(matches!(from_value(event).unwrap(), Ignored(IgnoredAny))); - } + if let StackStatus::UpdateInProgress = status { + return Ok::<_, Error>( + Migration::new(DbClient::new(options.clone())) + .migrate() + .await?, + ); + } + } - #[test] - fn event_deserialize_ignored_empty() { - // Any other data should deserialize into the Ignored enum. - assert!(matches!( - from_value(json!({})).unwrap(), - Ignored(IgnoredAny) - )); - } + // If this was a rollback update, then no migration should be performed, + // because the previous update indicated a failed migration, and the migration + // would have already been rolled back. If a migration occurred here it would + // just fail again, resulting in an `UPDATE_ROLLBACK_FAILED`. + Ok(()) + } + // If this is a delete event, there is nothing to do. + CloudFormationCustomResourceRequest::Delete(delete) => { + trace!(delete = ?delete, "during delete"); + + Ok(()) + } + } + }, + )) + .await } From f9a028b7567ed79005713c5c604bbd468507a081 Mon Sep 17 00:00:00 2001 From: Marko Malenic Date: Mon, 26 Aug 2024 15:31:24 +1000 Subject: [PATCH 8/9] refactor: change default filemanager CORS allow origins --- compose.yml | 2 +- lib/workload/stateless/stacks/filemanager/compose.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compose.yml b/compose.yml index c5d5bfebc..a35762c1a 100644 --- a/compose.yml +++ b/compose.yml @@ -70,7 +70,7 @@ services: # Container database address for running server inside a docker container. - DATABASE_URL=postgresql://orcabus:orcabus@db:5432/filemanager - RUST_LOG=debug - - FILEMANAGER_API_CORS_ALLOW_ORIGINS=${FILEMANAGER_API_CORS_ALLOW_ORIGINS:-http://localhost:8400} + - FILEMANAGER_API_CORS_ALLOW_ORIGINS=${FILEMANAGER_API_CORS_ALLOW_ORIGINS:-http://localhost:3000} - FILEMANAGER_API_CORS_ALLOW_HEADERS=${FILEMANAGER_API_CORS_ALLOW_HEADERS:-accept,authorization,content-type,user-agent,x-csrftoken,x-requested-with,x-amz-security-token,x-amz-date,content-disposition} - AWS_ACCESS_KEY_ID=${AWS_ACCESS_KEY_ID:-access_key_id} - AWS_SECRET_ACCESS_KEY=${AWS_SECRET_ACCESS_KEY:-secret_access_key} diff --git a/lib/workload/stateless/stacks/filemanager/compose.yml b/lib/workload/stateless/stacks/filemanager/compose.yml index a90a799d6..234e01807 100644 --- a/lib/workload/stateless/stacks/filemanager/compose.yml +++ b/lib/workload/stateless/stacks/filemanager/compose.yml @@ -19,7 +19,7 @@ services: # Container database address for running server inside a docker container. - DATABASE_URL=postgresql://filemanager:filemanager@postgres:4321/filemanager - RUST_LOG=debug - - FILEMANAGER_API_CORS_ALLOW_ORIGINS=${FILEMANAGER_API_CORS_ALLOW_ORIGINS:-http://localhost:8000} + - FILEMANAGER_API_CORS_ALLOW_ORIGINS=${FILEMANAGER_API_CORS_ALLOW_ORIGINS:-http://localhost:3000} - FILEMANAGER_API_CORS_ALLOW_HEADERS=${FILEMANAGER_API_CORS_ALLOW_HEADERS:-accept,authorization,content-type,user-agent,x-csrftoken,x-requested-with,x-amz-security-token,x-amz-date,content-disposition} - AWS_ACCESS_KEY_ID=${AWS_ACCESS_KEY_ID:-access_key_id} - AWS_SECRET_ACCESS_KEY=${AWS_SECRET_ACCESS_KEY:-secret_access_key} From e268d93e8766cca6ed7037730501f01e115ef51a Mon Sep 17 00:00:00 2001 From: William Putra Intan <61998484+williamputraintan@users.noreply.github.com> Date: Tue, 27 Aug 2024 12:31:42 +1000 Subject: [PATCH 9/9] Fix Django OpenAPI Schema (#520) --- .../stateless/stacks/metadata-manager/app/settings/local.py | 2 +- .../stateless/stacks/metadata-manager/app/viewsets/lab.py | 5 ++++- .../sequence_run_manager/settings/local.py | 5 +++++ .../workflow-manager/workflow_manager/settings/local.py | 5 +++++ .../workflow-manager/workflow_manager/viewsets/library.py | 1 + 5 files changed, 16 insertions(+), 2 deletions(-) diff --git a/lib/workload/stateless/stacks/metadata-manager/app/settings/local.py b/lib/workload/stateless/stacks/metadata-manager/app/settings/local.py index e9d141b6d..70b97d8de 100644 --- a/lib/workload/stateless/stacks/metadata-manager/app/settings/local.py +++ b/lib/workload/stateless/stacks/metadata-manager/app/settings/local.py @@ -56,7 +56,7 @@ "description": "Terms of service", "url": "https://umccr.org/", }, - 'CAMELIZE_NAMES': False, + 'CAMELIZE_NAMES': True, 'POSTPROCESSING_HOOKS': [ 'drf_spectacular.contrib.djangorestframework_camel_case.camelize_serializer_fields', 'drf_spectacular.hooks.postprocess_schema_enums' diff --git a/lib/workload/stateless/stacks/metadata-manager/app/viewsets/lab.py b/lib/workload/stateless/stacks/metadata-manager/app/viewsets/lab.py index 089b9f630..e0670dfed 100644 --- a/lib/workload/stateless/stacks/metadata-manager/app/viewsets/lab.py +++ b/lib/workload/stateless/stacks/metadata-manager/app/viewsets/lab.py @@ -107,7 +107,10 @@ def list(self, request, *args, **kwargs): def get_queryset(self): return Library.objects.get_by_keyword(**self.request.query_params) - @extend_schema(responses={200: LibraryFullSerializer(many=True)}) + @extend_schema( + parameters=[LibrarySerializer], + responses={200: LibraryFullSerializer(many=True)} + ) @action(detail=False, methods=['get'], url_path='full') def get_full_model_set(self, request): qs = Library.objects.select_related("specimen__subject").all().order_by("-library_id") diff --git a/lib/workload/stateless/stacks/sequence-run-manager/sequence_run_manager/settings/local.py b/lib/workload/stateless/stacks/sequence-run-manager/sequence_run_manager/settings/local.py index 878e04992..08c870530 100644 --- a/lib/workload/stateless/stacks/sequence-run-manager/sequence_run_manager/settings/local.py +++ b/lib/workload/stateless/stacks/sequence-run-manager/sequence_run_manager/settings/local.py @@ -58,4 +58,9 @@ "description": "Terms of service", "url": "https://umccr.org/", }, + 'CAMELIZE_NAMES': True, + 'POSTPROCESSING_HOOKS': [ + 'drf_spectacular.contrib.djangorestframework_camel_case.camelize_serializer_fields', + 'drf_spectacular.hooks.postprocess_schema_enums' + ], } diff --git a/lib/workload/stateless/stacks/workflow-manager/workflow_manager/settings/local.py b/lib/workload/stateless/stacks/workflow-manager/workflow_manager/settings/local.py index d7feb2fff..c942eb120 100644 --- a/lib/workload/stateless/stacks/workflow-manager/workflow_manager/settings/local.py +++ b/lib/workload/stateless/stacks/workflow-manager/workflow_manager/settings/local.py @@ -57,4 +57,9 @@ "description": "Terms of service", "url": "https://umccr.org/", }, + 'CAMELIZE_NAMES': True, + 'POSTPROCESSING_HOOKS': [ + 'drf_spectacular.contrib.djangorestframework_camel_case.camelize_serializer_fields', + 'drf_spectacular.hooks.postprocess_schema_enums' + ], } diff --git a/lib/workload/stateless/stacks/workflow-manager/workflow_manager/viewsets/library.py b/lib/workload/stateless/stacks/workflow-manager/workflow_manager/viewsets/library.py index 36d1349e8..04ef9a5a3 100644 --- a/lib/workload/stateless/stacks/workflow-manager/workflow_manager/viewsets/library.py +++ b/lib/workload/stateless/stacks/workflow-manager/workflow_manager/viewsets/library.py @@ -7,6 +7,7 @@ class LibraryViewSet(ReadOnlyModelViewSet): + lookup_value_regex = "[^/]+" serializer_class = LibraryModelSerializer pagination_class = StandardResultsSetPagination filter_backends = [filters.OrderingFilter, filters.SearchFilter]