chore: store: db: reuse connection pool and change configuration

The pool object is meant to be reused and since we're initializing the store as part of a long running process, we should save the pool object and pass it around/use it. Have a proper db url directly in configuration files instead of using environment variables. Signed-off-by: Antonio Murdaca <[email protected]>
fdo-rs · Aug 19, 2024 · 59066c2 · 59066c2
1 parent f524b28
commit 59066c2
Show file tree

Hide file tree

Showing 15 changed files with 1,278 additions and 1,246 deletions.
diff --git a/HOWTO.md b/HOWTO.md
@@ -343,20 +343,22 @@ Where:
       ```
     - `Sqlite`: will use a Sqlite database to store the ownership vouchers.
       When using this option you must set `Manufacturer` as the DB type as
-      shown below:
+      shown below as well as a connection url (including username/password/port if needed):
       ```
       ownership_voucher_store_driver:
         Sqlite:
-          Manufacturer
+          server: Manufacturer
+          url: sqlite:///path/to/db/sqlite
       ```
       Please refer to the [Database management section](#database-management) on how to initialize databases.
     - `Postgres`: will use a Postgres database to store the ownership vouchers.
       When using this option you must set `Manufacturer` as the DB type as
-      shown below:
+      shown below as well as a connection url (including username/password/port if needed):
       ```
       ownership_voucher_store_driver:
         Postgres:
-          Manufacturer
+          server: Manufacturer
+          url: postgresql://username:password@host:5432/database_name?option1=value1&option2=value2
       ```
       Please refer to the [Database management section](#database-management) on how to initialize databases.
 - `public_key_store_driver:` [OPTIONAL] path to a directory that will hold the
@@ -473,20 +475,22 @@ Where:
       ```
     - `Sqlite`: will use a Sqlite database to store the ownership vouchers.
       When using this option you must set `Owner` as the DB type as
-      shown below:
+      shown below as well as a connection url (including username/password/port if needed):
       ```
       ownership_voucher_store_driver:
         Sqlite:
-          Owner
+          server: Owner
+          url: sqlite:///path/to/db/sqlite
       ```
       Please refer to the [Database management section](#database-management) on how to initialize databases.
     - `Postgres`: will use a Postgres database to store the ownership vouchers.
       When using this option you must set `Owner` as the DB type as
-      shown below:
+      shown below as well as a connection url (including username/password/port if needed):
       ```
       ownership_voucher_store_driver:
         Postgres:
-          Owner
+          server: Owner
+          url: postgresql://username:password@host:5432/database_name?option1=value1&option2=value2
       ```
       Please refer to the [Database management section](#database-management) on how to initialize databases.
 - `session_store_driver`: path to a directory that will hold session
@@ -551,20 +555,22 @@ Where:
       ```
     - `Sqlite`: will use a Sqlite database as the server's storage.
       When using this option you must set `Rendezvous` as the DB type as
-      shown below:
+      shown below as well as a connection url (including username/password/port if needed):
       ```
       storage_driver:
         Sqlite:
-          Rendezvous
+          server: Rendezvous
+          url: sqlite:///path/to/db/sqlite
       ```
       Please refer to the [Database management section](#database-management) on how to initialize databases.
     - `Postgres`: will use a Sqlite database as the server's storage.
       When using this option you must set `Rendezvous` as the DB type as
-      shown below:
+      shown below as well as a connection url (including username/password/port if needed):
       ```
       storage_driver:
         Postgres:
-          Rendezvous
+          server: Rendezvous
+          url: postgresql://username:password@host:5432/database_name?option1=value1&option2=value2
       ```
       Please refer to the [Database management section](#database-management) on how to initialize databases.
 - `session_store_driver`: path to a directory that will hold session
@@ -739,11 +745,6 @@ Please mind how the configuration file must be specifically named (e.g. `-` VS
    file in
    [examples/systemd](https://github.com/fedora-iot/fido-device-onboard-rs/blob/main/examples/systemd/fdo-manufacturing-server.service).
 
-    If you are using a Sqlite or Postgres database for storage, before running
-    the server you must set the `SQLITE_MANUFACTURER_DATABASE_URL` or
-    `POSTGRES_MANUFACTURER_DATABASE_URL` environment variable with the proper
-    connection URL when using Sqlite or Postgres, respectively.
-
 ### Owner Onboarding Server
 
 1. Generate the required keys/certificates for the Owner, see [How to generate
@@ -773,11 +774,6 @@ Please mind how the configuration file must be specifically named (e.g. `-` VS
 4. Execute `fdo-owner-onboarding-server` or run it as a service, see sample
    file in [examples/systemd](https://github.com/fedora-iot/fido-device-onboard-rs/blob/main/examples/systemd/fdo-owner-onboarding-server.service).
 
-   If you are using a Sqlite or Postgres database for storage, before running
-   the server you must set the `SQLITE_OWNER_DATABASE_URL` or
-   `POSTGRES_OWNER_DATABASE_URL` environment variable with the proper
-   connection URL when using Sqlite or Postgres, respectively.
-
 ### Rendezvous Server
 
 1. Configure `rendezvous-server.yml`, see [Configuration
@@ -791,11 +787,6 @@ Please mind how the configuration file must be specifically named (e.g. `-` VS
 2. Execute `fdo-rendezvous-server` or run it as a service, see sample file in
    [examples/systemd](https://github.com/fedora-iot/fido-device-onboard-rs/blob/main/examples/systemd/fdo-rendezvous-server.service).
 
-   If you are using a Sqlite or Postgres database for storage, before running
-   the server you must set the `SQLITE_RENDEZVOUS_DATABASE_URL` or
-   `POSTGRES_RENDEZVOUS_DATABASE_URL` environment variable with the proper
-   connection URL when using Sqlite or Postgres, respectively.
-
 ### Service Info API Server
 
 1. Configure `serviceinfo-api-server.yml`, see [Configuration

diff --git a/data-formats/src/constants/mod.rs b/data-formats/src/constants/mod.rs
@@ -409,7 +409,6 @@ impl TryFrom<u8> for MessageType {
 #[repr(u16)]
 #[non_exhaustive]
 pub enum ErrorCode {
-    InvalidJWT = 1,
     InvalidOwnershipVoucher = 2,
     InvalidOwnerSignBody = 3,
     InvalidIPAddress = 4,

diff --git a/data-formats/src/types.rs b/data-formats/src/types.rs
@@ -1,5 +1,6 @@
 use std::{
     convert::{TryFrom, TryInto},
+    fmt::Display,
     net::IpAddr,
     ops::Deref,
     str::FromStr,
@@ -273,9 +274,9 @@ impl PartialEq for Nonce {
     }
 }
 
-impl ToString for Nonce {
-    fn to_string(&self) -> String {
-        hex::encode(&self.0)
+impl Display for Nonce {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        hex::encode(&self.0).fmt(f)
     }
 }
 
@@ -339,9 +340,9 @@ impl FromStr for Guid {
     }
 }
 
-impl ToString for Guid {
-    fn to_string(&self) -> String {
-        self.as_uuid().to_string()
+impl Display for Guid {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        self.as_uuid().fmt(f)
     }
 }
 
@@ -1276,13 +1277,13 @@ impl FromStr for KexSuite {
     }
 }
 
-impl ToString for KexSuite {
-    fn to_string(&self) -> String {
+impl Display for KexSuite {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {
-            KexSuite::Ecdh256 => "ECDH256".to_string(),
-            KexSuite::Ecdh384 => "ECDH384".to_string(),
-            KexSuite::DhkexId14 => "DHKEXid14".to_string(),
-            KexSuite::DhkexId15 => "DHKEXid15".to_string(),
+            KexSuite::Ecdh256 => write!(f, "ECDH256"),
+            KexSuite::Ecdh384 => write!(f, "ECDH384"),
+            KexSuite::DhkexId14 => write!(f, "DHKEXid14"),
+            KexSuite::DhkexId15 => write!(f, "DHKEXid15"),
         }
     }
 }
@@ -1455,11 +1456,11 @@ impl FromStr for CipherSuite {
     }
 }
 
-impl ToString for CipherSuite {
-    fn to_string(&self) -> String {
+impl Display for CipherSuite {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {
-            CipherSuite::A128Gcm => "A128GCM".to_string(),
-            CipherSuite::A256Gcm => "A256GCM".to_string(),
+            CipherSuite::A128Gcm => write!(f, "A128GCM"),
+            CipherSuite::A256Gcm => write!(f, "A256GCM"),
         }
     }
 }

diff --git a/db/src/lib.rs b/db/src/lib.rs
@@ -20,10 +20,7 @@ where
     T: diesel::r2d2::R2D2Connection + 'static,
 {
     /// Gets a connection pool
-    fn get_conn_pool() -> Pool<ConnectionManager<T>>;
-
-    /// Gets a connection to the db
-    fn get_connection() -> T;
+    fn get_conn_pool(url: String) -> Pool<ConnectionManager<T>>;
 
     /// Inserts an OV
     fn insert_ov(ov: &OV, ttl: Option<i64>, conn: &mut T) -> Result<()>;
@@ -51,10 +48,7 @@ where
     T: diesel::r2d2::R2D2Connection + 'static,
 {
     /// Gets a connection pool
-    fn get_conn_pool() -> Pool<ConnectionManager<T>>;
-
-    /// Gets a connection to the db
-    fn get_connection() -> T;
+    fn get_conn_pool(url: String) -> Pool<ConnectionManager<T>>;
 
     /// Inserts an OV
     fn insert_ov(ov: &OV, to2: Option<bool>, to0: Option<i64>, conn: &mut T) -> Result<()>;
@@ -99,10 +93,7 @@ where
     T: diesel::r2d2::R2D2Connection + 'static,
 {
     /// Gets a connection pool
-    fn get_conn_pool() -> Pool<ConnectionManager<T>>;
-
-    /// Gets a connection to the db
-    fn get_connection() -> T;
+    fn get_conn_pool(url: String) -> Pool<ConnectionManager<T>>;
 
     /// Inserts an OV
     fn insert_ov(ov: &StoredItem, guid: &str, ttl: Option<i64>, conn: &mut T) -> Result<()>;

diff --git a/db/src/postgres.rs b/db/src/postgres.rs
@@ -10,8 +10,6 @@ use diesel::r2d2::ConnectionManager;
 use diesel::r2d2::Pool;
 use diesel::PgConnection;
 
-use std::env;
-
 use anyhow::Result;
 
 use super::models::{ManufacturerOV, NewOwnerOV, NewRendezvousOV, OwnerOV, RendezvousOV};
@@ -22,15 +20,7 @@ use fdo_data_formats::Serializable;
 pub struct PostgresManufacturerDB {}
 
 impl DBStoreManufacturer<PgConnection> for PostgresManufacturerDB {
-    fn get_connection() -> PgConnection {
-        let database_url = env::var("POSTGRES_MANUFACTURER_DATABASE_URL")
-            .expect("POSTGRES_MANUFACTURER_DATABASE_URL must be set");
-        PgConnection::establish(&database_url).expect("Error connecting to database")
-    }
-
-    fn get_conn_pool() -> Pool<ConnectionManager<PgConnection>> {
-        let database_url = env::var("POSTGRES_MANUFACTURER_DATABASE_URL")
-            .expect("POSTGRES_MANUFACTURER_DATABASE_URL must be set");
+    fn get_conn_pool(database_url: String) -> Pool<ConnectionManager<PgConnection>> {
         let manager = ConnectionManager::<PgConnection>::new(database_url);
         Pool::builder()
             .test_on_check_out(true)
@@ -90,15 +80,7 @@ impl DBStoreManufacturer<PgConnection> for PostgresManufacturerDB {
 pub struct PostgresOwnerDB {}
 
 impl DBStoreOwner<PgConnection> for PostgresOwnerDB {
-    fn get_connection() -> PgConnection {
-        let database_url = env::var("POSTGRES_OWNER_DATABASE_URL")
-            .expect("POSTGRES_OWNER_DATABASE_URL must be set");
-        PgConnection::establish(&database_url).expect("Error connecting to database")
-    }
-
-    fn get_conn_pool() -> Pool<ConnectionManager<PgConnection>> {
-        let database_url = env::var("POSTGRES_OWNER_DATABASE_URL")
-            .expect("POSTGRES_OWNER_DATABASE_URL must be set");
+    fn get_conn_pool(database_url: String) -> Pool<ConnectionManager<PgConnection>> {
         let manager = ConnectionManager::<PgConnection>::new(database_url);
         Pool::builder()
             .test_on_check_out(true)
@@ -208,15 +190,7 @@ impl DBStoreOwner<PgConnection> for PostgresOwnerDB {
 pub struct PostgresRendezvousDB {}
 
 impl DBStoreRendezvous<PgConnection> for PostgresRendezvousDB {
-    fn get_connection() -> PgConnection {
-        let database_url = env::var("POSTGRES_RENDEZVOUS_DATABASE_URL")
-            .expect("POSTGRES_RENDEZVOUS_DATABASE_URL must be set");
-        PgConnection::establish(&database_url).expect("Error connecting to database")
-    }
-
-    fn get_conn_pool() -> Pool<ConnectionManager<PgConnection>> {
-        let database_url = env::var("POSTGRES_RENDEZVOUS_DATABASE_URL")
-            .expect("POSTGRES_RENDEZVOUS_DATABASE_URL must be set");
+    fn get_conn_pool(database_url: String) -> Pool<ConnectionManager<PgConnection>> {
         let manager = ConnectionManager::<PgConnection>::new(database_url);
         Pool::builder()
             .test_on_check_out(true)

diff --git a/db/src/sqlite.rs b/db/src/sqlite.rs
@@ -11,8 +11,6 @@ use crate::schema::manufacturer_vouchers;
 use crate::schema::owner_vouchers;
 use crate::schema::rendezvous_vouchers;
 
-use std::env;
-
 use anyhow::Result;
 
 use super::models::{NewOwnerOV, NewRendezvousOV, OwnerOV, RendezvousOV};
@@ -24,15 +22,7 @@ use fdo_data_formats::StoredItem;
 pub struct SqliteManufacturerDB {}
 
 impl DBStoreManufacturer<SqliteConnection> for SqliteManufacturerDB {
-    fn get_connection() -> SqliteConnection {
-        let database_url = env::var("SQLITE_MANUFACTURER_DATABASE_URL")
-            .expect("SQLITE_MANUFACTURER_DATABASE_URL must be set");
-        SqliteConnection::establish(&database_url).expect("Error connecting to database")
-    }
-
-    fn get_conn_pool() -> Pool<ConnectionManager<SqliteConnection>> {
-        let database_url = env::var("SQLITE_MANUFACTURER_DATABASE_URL")
-            .expect("SQLITE_MANUFACTURER_DATABASE_URL must be set");
+    fn get_conn_pool(database_url: String) -> Pool<ConnectionManager<SqliteConnection>> {
         let manager = ConnectionManager::<SqliteConnection>::new(database_url);
         Pool::builder()
             .test_on_check_out(true)
@@ -92,15 +82,7 @@ impl DBStoreManufacturer<SqliteConnection> for SqliteManufacturerDB {
 pub struct SqliteOwnerDB {}
 
 impl DBStoreOwner<SqliteConnection> for SqliteOwnerDB {
-    fn get_connection() -> SqliteConnection {
-        let database_url =
-            env::var("SQLITE_OWNER_DATABASE_URL").expect("SQLITE_OWNER_DATABASE_URL must be set");
-        SqliteConnection::establish(&database_url).expect("Error connecting to database")
-    }
-
-    fn get_conn_pool() -> Pool<ConnectionManager<SqliteConnection>> {
-        let database_url =
-            env::var("SQLITE_OWNER_DATABASE_URL").expect("SQLITE_OWNER_DATABASE_URL must be set");
+    fn get_conn_pool(database_url: String) -> Pool<ConnectionManager<SqliteConnection>> {
         let manager = ConnectionManager::<SqliteConnection>::new(database_url);
         Pool::builder()
             .test_on_check_out(true)
@@ -210,15 +192,7 @@ impl DBStoreOwner<SqliteConnection> for SqliteOwnerDB {
 pub struct SqliteRendezvousDB {}
 
 impl DBStoreRendezvous<SqliteConnection> for SqliteRendezvousDB {
-    fn get_connection() -> SqliteConnection {
-        let database_url = env::var("SQLITE_RENDEZVOUS_DATABASE_URL")
-            .expect("SQLITE_RENDEZVOUS_DATABASE_URL must be set");
-        SqliteConnection::establish(&database_url).expect("Error connecting to database")
-    }
-
-    fn get_conn_pool() -> Pool<ConnectionManager<SqliteConnection>> {
-        let database_url = env::var("SQLITE_RENDEZVOUS_DATABASE_URL")
-            .expect("SQLITE_RENDEZVOUS_DATABASE_URL must be set");
+    fn get_conn_pool(database_url: String) -> Pool<ConnectionManager<SqliteConnection>> {
         let manager = ConnectionManager::<SqliteConnection>::new(database_url);
         Pool::builder()
             .test_on_check_out(true)
@@ -296,7 +270,9 @@ mod tests {
 
         // read test ovs from the integration tests dir
         let mut ov_map = HashMap::new();
-        let pool = SqliteManufacturerDB::get_conn_pool();
+        let database_url = env::var("SQLITE_MANUFACTURER_DATABASE_URL")
+            .expect("SQLITE_MANUFACTURER_DATABASE_URL must be set");
+        let pool = SqliteManufacturerDB::get_conn_pool(database_url);
 
         // last_guid used later to delete an ov with that key
         let mut last_guid = String::new();
@@ -367,7 +343,9 @@ mod tests {
 
         // read test ovs from the integration tests dir
         let mut ov_map = HashMap::new();
-        let pool = SqliteOwnerDB::get_conn_pool();
+        let database_url =
+            env::var("SQLITE_OWNER_DATABASE_URL").expect("SQLITE_OWNER_DATABASE_URL must be set");
+        let pool = SqliteOwnerDB::get_conn_pool(database_url);
 
         // last_guid used later to delete an ov with that key
         let mut last_guid = String::new();
@@ -440,7 +418,9 @@ mod tests {
 
         // read test ovs from the integration tests dir
         let mut ov_map = HashMap::new();
-        let pool = SqliteRendezvousDB::get_conn_pool();
+        let database_url = env::var("SQLITE_RENDEZVOUS_DATABASE_URL")
+            .expect("SQLITE_RENDEZVOUS_DATABASE_URL must be set");
+        let pool = SqliteRendezvousDB::get_conn_pool(database_url);
 
         // last_guid used later to delete an ov with that key
         let mut last_guid = String::new();