feat(pedm): SQL support #1292

allan2 · 2025-03-31T23:48:15Z

This PR defines the structure for supporting multiple database backends, as well as accessing data from state/middleware, to the PEDM module.

adds support for libSQL and Postgres as database backends
- libSQL is the default
new config file for specifying the database and defining configuration
introduces the Database trait
- required queries are defined in the Database trait
- per backend implementations are defined in the backend-specific file, i.e., libsql.rs or pg.rs
introduces LogLayer, tower middleware for logging request and response metadata
introduces AppState
- the elevate_temporary handler is updated to illustrate accessing data from state (DB connection) and from middleware (request ID)
- policies moved from global OnceLock to AppState
- plays nice with Aide/OpenAPI generation

This PR defines the structure for supporting multiple database backends, as well as accessing data from state/middleware, to the PEDM module. - adds support for libSQL and Postgres as database backends - libSQL is the default - new config file for specifying the database and defining configuration - introduces the `Database` trait - required queries are defined in the `Database` trait - per backend implementations are defined in the backend-specific file, i.e., _libsql.rs_ or _pg.rs_ - introduces `LogLayer`, tower middleware for logging request and response metadata - introduces `AppState` - the _elevate_temporary_ handler is updated to illustrate accessing data from state (DB connection) and from middleware (request ID) - policies moved from global `OnceLock` to `AppState` - plays nice with Aide/OpenAPI generation

github-actions · 2025-03-31T23:48:34Z

Let maintainers know that an action is required on their side

Add the label release-required Please cut a new release (Devolutions Gateway, Devolutions Agent, Jetsocat, PowerShell module) when you request a maintainer to cut a new release (Devolutions Gateway, Devolutions Agent, Jetsocat, PowerShell module)
Add the label release-blocker Follow-up is required before cutting a new release if a follow-up is required before cutting a new release
Add the label publish-required Please publish libraries (`Devolutions.Gateway.Utils`, OpenAPI clients, etc) when you request a maintainer to publish libraries (Devolutions.Gateway.Utils, OpenAPI clients, etc.)
Add the label publish-blocker Follow-up is required before publishing libraries if a follow-up is required before publishing libraries

It was previously not updated because I had test crates in the workspace. But CI is failing. Test crates removed. This updates Cargo.lock.

CBenoit · 2025-04-01T15:02:47Z

Cargo.lock

+[[package]]
+name = "rustls"
+version = "0.22.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "bf4ef73721ac7bcd79b2b315da7779d8fc09718c6b3d2d1b2d94850eb8c18432"
+dependencies = [
+ "log",
+ "ring 0.17.14",
+ "rustls-pki-types",
+ "rustls-webpki 0.102.8",
+ "subtle",
+ "zeroize",
+]


issue: This is duplicating the rustls dependency. 0.22 is the older one.

Cargo.lock

CBenoit · 2025-04-01T15:08:13Z

Cargo.lock

 [[package]]
 name = "hyper-rustls"
-version = "0.27.3"
+version = "0.25.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "399c78f9338483cb7e630c8474b07268983c6bd5acee012e4211f9f7bb21b070"


issue: hyper-rustls also got duplicated, with an older one.

CBenoit

Some suspicious changes in our dependency graph. I don’t expect everything to be fixable, but ideally we should avoid duplicating dependencies or downgrading dependencies. If there is no immediate solution, we will merge now and fix later.

CBenoit · 2025-04-02T15:27:11Z

crates/devolutions-pedm/src/api/mod.rs

    loop {
        server.connect().await?;
        let client = server;

        server = create_pipe(pipe_name)?;

-        let tower_service = make_service.call(&client).await?;
+        let tower_service = make_service.call(&client).await.unwrap(); // return type is Infallible


style: A better way of handling Infaillible is:

Suggested change

let tower_service = make_service.call(&client).await.unwrap(); // return type is Infallible

let Ok(tower_service) = make_service.call(&client).await;

Implemented in e99d392.

CBenoit · 2025-04-02T15:28:25Z

crates/devolutions-pedm/src/api/mod.rs

-                error!(%error, "Failed to serve connection");
+                error!(%e, "Failed to serve connection");


style: https://github.com/Devolutions/IronRDP/blob/master/STYLE.md#logging

Implemented in baf5f47.

CBenoit · 2025-04-02T16:02:27Z

crates/devolutions-pedm/src/api/mod.rs


    let mut make_service = app.into_make_service_with_connect_info::<RawNamedPipeConnectInfo>();

    let mut server = create_pipe(pipe_name)?;
+
+    // log the server startup


style: https://github.com/Devolutions/IronRDP/blob/817abb9805ed80cac822694bf5097980e1d7007e/STYLE.md#inline-code-comments-are-proper-sentences

Implemented in cd67021.

CBenoit · 2025-04-02T19:01:10Z

crates/devolutions-pedm/src/db/pg.rs

+
+use super::{Database, DbError};
+
+pub(crate) struct PgPool(Pool<PostgresConnectionManager<NoTls>>);


question: Is it really fine to use the "NoTls" variant?

I can add TLS support in the future.

CBenoit

Approving as discussed on Slack

This removes postgres as a default feature. The crate now builds with libsql, postgres, or both features enabled.

allan2 added 5 commits March 31, 2025 23:33

fmt

87a1147

styling

653e50f

Update Cargo.lock

cf4b725

It was previously not updated because I had test crates in the workspace. But CI is failing. Test crates removed. This updates Cargo.lock.

Keep anyhow::Error in custom errors

d052e77

Finish anyhow error changes

9d1553d

CBenoit reviewed Apr 1, 2025

View reviewed changes

Cargo.lock Outdated Show resolved Hide resolved

CBenoit reviewed Apr 1, 2025

View reviewed changes

Cargo.lock Outdated Show resolved Hide resolved

CBenoit reviewed Apr 1, 2025

View reviewed changes

allan2 added 3 commits April 1, 2025 16:56

Merge 'origin/master' into pedm-libsql

b854c4c

Use libsql 0.9

5285895

Fix Linux build

7015a51

CBenoit reviewed Apr 2, 2025

View reviewed changes

allan2 added 3 commits April 7, 2025 14:20

Better Infallible handling

e99d392

Spell out error

baf5f47

Use full sentence for comment

cd67021

CBenoit approved these changes Apr 8, 2025

View reviewed changes

allan2 added 6 commits April 8, 2025 09:50

Replace Cargo.lock with version from origin/master

57ee05a

Merge origin/master

d93aebc

Conditional compilation of DB features

979cf17

This removes postgres as a default feature. The crate now builds with libsql, postgres, or both features enabled.

Switch config from TOML to JSON

cce7934

Update Cargo.lock

b8be8a1

Unformat Cargo.toml

2b9b629

CBenoit enabled auto-merge (squash) April 8, 2025 14:29

CBenoit merged commit abc57f9 into master Apr 8, 2025
38 checks passed

CBenoit deleted the pedm-libsql branch April 8, 2025 14:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(pedm): SQL support #1292

feat(pedm): SQL support #1292

allan2 commented Mar 31, 2025

github-actions bot commented Mar 31, 2025

CBenoit Apr 1, 2025 •

edited

Loading

CBenoit Apr 1, 2025

CBenoit left a comment

CBenoit Apr 2, 2025

allan2 Apr 7, 2025

CBenoit Apr 2, 2025 •

edited

Loading

allan2 Apr 7, 2025

CBenoit Apr 2, 2025 •

edited

Loading

allan2 Apr 7, 2025

CBenoit Apr 2, 2025

allan2 Apr 7, 2025

CBenoit left a comment

	let tower_service = make_service.call(&client).await.unwrap(); // return type is Infallible
	let Ok(tower_service) = make_service.call(&client).await;

		error!(%error, "Failed to serve connection");
		error!(%e, "Failed to serve connection");


		use super::{Database, DbError};

		pub(crate) struct PgPool(Pool<PostgresConnectionManager<NoTls>>);

feat(pedm): SQL support #1292

feat(pedm): SQL support #1292

Conversation

allan2 commented Mar 31, 2025

github-actions bot commented Mar 31, 2025

Let maintainers know that an action is required on their side

CBenoit Apr 1, 2025 • edited Loading

Choose a reason for hiding this comment

CBenoit Apr 1, 2025

Choose a reason for hiding this comment

CBenoit left a comment

Choose a reason for hiding this comment

CBenoit Apr 2, 2025

Choose a reason for hiding this comment

allan2 Apr 7, 2025

Choose a reason for hiding this comment

CBenoit Apr 2, 2025 • edited Loading

Choose a reason for hiding this comment

allan2 Apr 7, 2025

Choose a reason for hiding this comment

CBenoit Apr 2, 2025 • edited Loading

Choose a reason for hiding this comment

allan2 Apr 7, 2025

Choose a reason for hiding this comment

CBenoit Apr 2, 2025

Choose a reason for hiding this comment

allan2 Apr 7, 2025

Choose a reason for hiding this comment

CBenoit left a comment

Choose a reason for hiding this comment

CBenoit Apr 1, 2025 •

edited

Loading

CBenoit Apr 2, 2025 •

edited

Loading

CBenoit Apr 2, 2025 •

edited

Loading