From 6bc9bbbf50d2d7ef9b14265d0db155c9889d8fdd Mon Sep 17 00:00:00 2001 From: sam Date: Sun, 2 Jun 2024 12:30:18 -0700 Subject: [PATCH] - beautify comments --- go_client/tests/secret_test.go | 9 --- src/engines/kv.rs | 57 +++------------ src/engines/kv/db_structs.rs | 29 +------- src/engines/kv/http_structs.rs | 14 +--- src/engines/kv/logic.rs | 28 +------- src/engines/kv/tests.rs | 126 +++++++++++++++++---------------- src/main.rs | 5 +- 7 files changed, 78 insertions(+), 190 deletions(-) diff --git a/go_client/tests/secret_test.go b/go_client/tests/secret_test.go index 85dce30..a349147 100644 --- a/go_client/tests/secret_test.go +++ b/go_client/tests/secret_test.go @@ -41,7 +41,6 @@ func TestMain(m *testing.M) { } // https://developer.hashicorp.com/vault/api-docs/secret/kv/kv-v2#create-update-secret -// @Philip der Path steht in der KvV2Write Methode func TestWriteSecret(t *testing.T) { // Path foo _, err := client.Secrets.KvV2Write(ctx, "foo", schema.KvV2WriteRequest{ @@ -98,16 +97,8 @@ func TestWriteSecret2(t *testing.T) { log.Println("kv2: Tried to write Secret at foo at mountpath: ", mountpath2) } -// func TestDeleteSecret(t *testing.T) { -// _, err := client.Secrets.KvV2Delete(ctx, "foo") // currently disregarding modifier options -// if err != nil { -// log.Fatal("kv2: Failed to delete secret:\n\t", err) -// } -// } - func TestReadSecret(t *testing.T) { _, err := client.Secrets.KvV2Read(ctx, "bar") - if err != nil { log.Fatal("kv2: Failed to read secret:\n\t", err) } diff --git a/src/engines/kv.rs b/src/engines/kv.rs index 1d1148e..f367279 100644 --- a/src/engines/kv.rs +++ b/src/engines/kv.rs @@ -1,7 +1,4 @@ -// TODO: Remove -#![allow(dead_code)] - -// pub mod logic; // TODO: Remove or correct errors +// pub mod logic; pub mod db_structs; pub mod http_structs; @@ -64,7 +61,7 @@ async fn get_data( Ok(v) => { let version: i64 = v.get("version_number"); let secret_content: HashMap = HashMap::from([ - // TODO: use sqlx to parse the row to a struct, do not do it manually + // Consider using sqlx to parse the row to a struct, do not do it manually ("secret_data".to_string(), v.get("secret_data")), ("created_time".to_string(), v.get("created_time")), ("deletion_time".to_string(), v.get("deletion_time")), @@ -80,7 +77,7 @@ async fn get_data( .unwrap_or_default() .to_utc(), // TODO custom_metadata: None, - deletion_time: None, // TODO + deletion_time: None, destroyed: false, version: version, }); @@ -93,7 +90,7 @@ async fn get_data( error!("{:?}", e); let error_struct: ErrorStruct = ErrorStruct { err: e.to_string() }; error!("{:?}", error_struct.err); - Ok(error_struct.into_response()) // TODO: API doesn't specify return value in case of error. Error struct correct? Else send empty secret back? + Ok(error_struct.into_response()) // API doesn't specify return value in case of error. Error struct correct? Else send empty secret back? // let error_secret = KvSecretRes{data: None, options: None}; // Ok(Json()) } @@ -108,6 +105,7 @@ async fn post_data( extract::Json(payload): extract::Json, ) -> Result { // Insert Metadata first -> Else: Error because of foreign key constraint + // In a later implementation, a Metadata with default values from the config will be created log::debug!( "Secret: {}, Content: {:?}, Version: {:?}, path: {}", @@ -154,15 +152,6 @@ async fn post_data( Ok(v) => { info!("{:?}", v); - // match sqlx::query("SELECT custom_data FROM metadata where path = $1").bind(&path).fetch_one(&pool).await{ - // Ok(v)=> { - // // let meta = v.get("custom_data"); // TODO: get metadata from query - - // } - // Err(e) => { - // panic!("Couldn't find metadata"); - // } - // } let return_struct = KvSecretResData { created_time: created_time, custom_metadata: None, @@ -179,33 +168,6 @@ async fn post_data( } } -/* mock for return -async fn post_data( - Path(kv_path): Path, - Extension(mount_path): Extension, - Json(body): Json, -) -> Json { - trace!( - "Secret: {}, Content: {:#?}, path: {}", - kv_path, - body.data, - // body.version.unwrap_or(0), - mount_path, - ); - - let res = KvSecretRes { - data: KvSecretResData { - created_time: chrono::Utc::now(), - custom_metadata: None, - deletion_time: None, - destroyed: false, - version: 1, - }, - }; - - Json(res) -} */ - /// TODO: soft delete the secret version at path. can be undone with undelete_secret // https://developer.hashicorp.com/vault/api-docs/secret/kv/kv-v2#delete-latest-version-of-secret // https://developer.hashicorp.com/vault/api-docs/secret/kv/kv-v2#delete-secret-versions @@ -255,9 +217,6 @@ async fn destroy_path( } } } - - // check if all are gone - Ok(StatusCode::NO_CONTENT) } @@ -319,11 +278,11 @@ async fn get_meta( }; if metadata_res.data.current_version < curr_num { - // should be the max of the available version numbers + // the max of the available version numbers metadata_res.data.current_version = curr_num; } if metadata_res.data.oldest_version > curr_num { - // should be the min of the available version numbers + // the min of the available version numbers metadata_res.data.oldest_version = curr_num; } @@ -353,7 +312,7 @@ async fn get_meta( Ok((StatusCode::OK, Json(metadata_res)).into_response()) } -// currently only writes the metadata - No case if already exists +// currently only writes the metadata - Not case if already exists async fn post_meta( State(pool): State, Path(kv_path): Path, diff --git a/src/engines/kv/db_structs.rs b/src/engines/kv/db_structs.rs index df32b72..3d20895 100644 --- a/src/engines/kv/db_structs.rs +++ b/src/engines/kv/db_structs.rs @@ -2,39 +2,20 @@ use chrono::{DateTime, Utc}; use serde::Serialize; use sqlx::FromRow; -// #[derive(Debug)] -// #[deprecated(note = "Add Req or Res respecively if AND move to http file if intended; remove deprecation once used")] -// pub struct SecretMeta { -// pub cas_required: bool, -// pub created_time: DateTime, -// pub current_version: i64, -// /// In Hashicorp: -// /// If not set, the backend's configured delete_version_after is used. -// /// Cannot be greater than the backend's delete_version_after -// // TODO: implement duration type -// pub delete_version_after: String, -// // TODO https://developer.hashicorp.com/vault/docs/concepts/duration-format -// pub max_versions: i64, -// pub oldest_version: i64, -// pub updated_time: DateTime, -// /// User-provided key-value pairs that are used to describe arbitrary and version-agnostic information about a secret. -// pub custom_metadata: Option>, -// pub versions: Vec, -// } - #[derive(FromRow)] #[derive(Debug)] pub struct DbSecretMeta { pub secret_path: String, pub cas_required: bool, pub created_time: DateTime, - // Consider: implement duration type + // Consider implementation of duration type for further development: // https://developer.hashicorp.com/vault/docs/concepts/duration-format /// In Hashicorp: /// If not set, the backend's configured delete_version_after is used. /// Cannot be greater than the backend's delete_version_after pub delete_version_after: Option, + ///In Hashicorp: /// The number of versions to keep per key. /// If not set, the backend’s configured max version is used. @@ -45,12 +26,6 @@ pub struct DbSecretMeta { /// User-provided key-value pairs that are used to describe arbitrary and version-agnostic information about a secret. pub custom_data: Option, - - // TODO: AS HASHMAP - // pub custom_data: Option>, - - // pub current_version: i64, - // pub oldest_version: i64, } #[derive(Serialize,Debug, FromRow)] diff --git a/src/engines/kv/http_structs.rs b/src/engines/kv/http_structs.rs index d288afe..3721029 100644 --- a/src/engines/kv/http_structs.rs +++ b/src/engines/kv/http_structs.rs @@ -21,7 +21,6 @@ pub struct KvSecretReq { pub options: Option>, // Version does not exist for create/update operations // pub version: Option, - // TODO add all fields } #[derive(Serialize, Debug)] @@ -113,20 +112,9 @@ pub struct KvMetaResData { pub updated_time: DateTime, pub custom_metadata: Option>, pub versions: HashMap, - // the key to a version is the version number + // here, the key to a version is the version number } -// Example -// { -// "max_versions": 5, -// "cas_required": false, -// "delete_version_after": "3h25m19s", -// "custom_metadata": { -// "foo": "abc", -// "bar": "123", -// "baz": "5c07d823-3810-48f6-a147-4c06b5219e84" -// } -// } #[derive(Serialize, Debug, Deserialize)] /// HTTP Request to post metadatas pub struct KvMetaReq { diff --git a/src/engines/kv/logic.rs b/src/engines/kv/logic.rs index 6683a07..c9a8e9f 100644 --- a/src/engines/kv/logic.rs +++ b/src/engines/kv/logic.rs @@ -2,33 +2,7 @@ use serde_json::Value; use super::{db_structs::SecretMeta, http_structs::*}; -// TODO create default function - -#[deprecated(note = "Use Axum functionality with structs instead, also, this should be inlined if it is actually needed")] -/// serialize secret to JSON String -pub fn serialize_secret_json(secret: &KvSecretReq) -> Result { - todo!() - // serde_json::to_string(&secret) -} - -#[deprecated(note = "Use Axum functionality with structs instead, also, this should be inlined if it is actually needed")] -/// deserialize JSON String to secret -pub fn deserialize_secret_struct(raw: &String) -> Result { - serde_json::from_str(raw) -} - -#[deprecated(note = "Use Axum functionality with structs instead, also, this should be inlined if it is actually needed")] -/// serialize metadata to JSON String -pub fn serialize_metadata_json(secret: &SecretMeta) -> Result { - serde_json::to_string(&secret) -} - -#[deprecated(note = "Use Axum functionality with structs instead, also, this should be inlined if it is actually needed")] -/// deserialize JSON String to metadata -pub fn deserialize_metadata_struct(raw: &String) -> Result { - serde_json::from_str(raw) -} - +// Consider leaving this here - JSON merge patch is also used in the official implementation #[deprecated(note = "Propably not needed (remove deprecation if actually needed)")] /// Consider: /// Instead of patching JSON, we should apply the modified fields directly to the database diff --git a/src/engines/kv/tests.rs b/src/engines/kv/tests.rs index 4801d5e..38b9af4 100644 --- a/src/engines/kv/tests.rs +++ b/src/engines/kv/tests.rs @@ -1,66 +1,68 @@ -use std::collections::HashMap; +// This file is deprecated. Currently, all tests are in written in go. -use chrono::Utc; -use tests::{ - logic::patch_metadata, - structs::{SecretMeta, VersionMeta}, -}; +// use std::collections::HashMap; -use super::*; +// use chrono::Utc; +// use tests::{ +// logic::patch_metadata, +// structs::{SecretMeta, VersionMeta}, +// }; -#[test] -#[cfg(target_feature = "_disabled")] -fn print_serialized_test() { - let temp_secret = TempSecret { - content: String::from("Hallo"), - version: 12, - }; - let serialized = serialize_secret_json(&temp_secret); - println!("string serialized: {:?}", serialized); - let deserialized = deserialize_secret_struct(&serialized.unwrap()); - println!( - "Struct field from deserialized: {}", - deserialized.unwrap().content - ) -} -#[test] -#[cfg(target_feature = "_disabled")] -fn test_patching() { - // TODO add more assertions - let mut base = create_mock_meta(); - println!("OLD metadata: {:?}", base); - let overwrite: SecretMeta = SecretMeta { - max_versions: 10, - versions: vec![VersionMeta { - created_time: Utc::now(), - deletion_time: Some(Utc::now()), - destroyed: true, - }], - cas_required: true, - delete_version_after: "10m".to_string(), - current_version: 4, - oldest_version: 2, - updated_time: Utc::now(), - created_time: Utc::now(), - custom_metadata: Some(HashMap::new()), - }; - let mut patched: Option = None; // Laurenz here - match patch_metadata(&mut base, &overwrite) { - Ok(meta) => { - println!("NEW metadata: {:?}", meta); - println!("patched successfully"); - patched = Some(meta); - } - Err(e) => { - log::error!("error patching metadata: {}", e); - panic!("Patching failed"); - } - } +// use super::*; - if let Some(patched_meta) = patched { - assert!(patched_meta.current_version == 4); - assert!(patched_meta.versions[0].destroyed == true); - } else { - panic!("patched was not initialized"); - } -} +// #[test] +// #[cfg(target_feature = "_disabled")] +// fn print_serialized_test() { +// let temp_secret = TempSecret { +// content: String::from("Hallo"), +// version: 12, +// }; +// let serialized = serialize_secret_json(&temp_secret); +// println!("string serialized: {:?}", serialized); +// let deserialized = deserialize_secret_struct(&serialized.unwrap()); +// println!( +// "Struct field from deserialized: {}", +// deserialized.unwrap().content +// ) +// } +// #[test] +// #[cfg(target_feature = "_disabled")] +// fn test_patching() { +// // TODO add more assertions +// let mut base = create_mock_meta(); +// println!("OLD metadata: {:?}", base); +// let overwrite: SecretMeta = SecretMeta { +// max_versions: 10, +// versions: vec![VersionMeta { +// created_time: Utc::now(), +// deletion_time: Some(Utc::now()), +// destroyed: true, +// }], +// cas_required: true, +// delete_version_after: "10m".to_string(), +// current_version: 4, +// oldest_version: 2, +// updated_time: Utc::now(), +// created_time: Utc::now(), +// custom_metadata: Some(HashMap::new()), +// }; +// let mut patched: Option = None; +// match patch_metadata(&mut base, &overwrite) { +// Ok(meta) => { +// println!("NEW metadata: {:?}", meta); +// println!("patched successfully"); +// patched = Some(meta); +// } +// Err(e) => { +// log::error!("error patching metadata: {}", e); +// panic!("Patching failed"); +// } +// } + +// if let Some(patched_meta) = patched { +// assert!(patched_meta.current_version == 4); +// assert!(patched_meta.versions[0].destroyed == true); +// } else { +// panic!("patched was not initialized"); +// } +// } diff --git a/src/main.rs b/src/main.rs index d10d37f..07316d5 100644 --- a/src/main.rs +++ b/src/main.rs @@ -24,10 +24,9 @@ mod sys; async fn main() { // To be configured via environment variables // choose from (highest to lowest): error, warn, info, debug, trace, off - env::set_var("RUST_LOG", "trace"); // TODO: Remove to respect user configuration - // env::set_var("DATABASE_URL", "sqlite:test.db"); // TODO: move to .env + env::set_var("RUST_LOG", "trace"); + // env::set_var("DATABASE_URL", "sqlite:test.db"); // Format for the env var config. Consider moving to an .env file env_logger::init(); - // Listen on all IPv4 and IPv6 interfaces on port 8200 by default let listen_addr = env::var("LISTEN_ADDR").unwrap_or("[::]:8200".to_string()); // Do not change let listen_addr = SocketAddr::from_str(&listen_addr).expect("Failed to parse LISTEN_ADDR");