Skip to content

feat: implement the read-only database connection#2356

Open
ctron wants to merge 7 commits into
guacsec:mainfrom
ctron:feature/ro_instance_1
Open

feat: implement the read-only database connection#2356
ctron wants to merge 7 commits into
guacsec:mainfrom
ctron:feature/ro_instance_1

Conversation

@ctron
Copy link
Copy Markdown
Contributor

@ctron ctron commented May 8, 2026

Implement a second DB connection which is only used for read-only operations. Defaulting that connection to the primary (read-write) one.

The idea is to configure the system to use read-only replicas, scaling up more pods. But using the R/W connection when required.

Also see: https://redhat.atlassian.net/browse/TC-3439

Summary by Sourcery

Introduce separate read-write and read-only database connection wrappers and wire the application to use them across modules, enabling optional use of a dedicated read-only database while enforcing read-only access at the transaction level.

New Features:

  • Add ReadWrite and ReadOnly database connection wrapper types exposing appropriate SeaORM traits for mutation and read-only use cases.
  • Introduce separate read-only database configuration (CLI flags and environment variables) that can override or fall back to the primary database settings.
  • Provide module- and endpoint-level wiring to inject read-only or read-write connections explicitly via actix-web data, including for analysis, importer, ingestor, user, and fundamental modules.

Enhancements:

  • Enforce read-only access by opening read-only transactions from ReadOnly connections and mapping database errors into module error types with appropriate HTTP responses.
  • Refactor ingestion, SBOM, advisory, product, SBOM group, and dataset write paths to use explicit read-write transactions with manual begin/commit instead of generic transaction helpers.
  • Adjust OpenAPI generation and tests to construct and use both read-write and read-only database wrappers for better parity with runtime configuration.

Documentation:

  • Add ADR 00018 documenting the design and rationale for dual database connections (read-write primary plus optional read-only replica) and their configuration.

Tests:

  • Update module and integration tests to use the new ReadWrite and ReadOnly wrappers instead of the raw Database where appropriate.

Chores:

  • Propagate new database wrappers through server startup configuration and module configure() signatures without changing the external HTTP API surface.

@sourcery-ai
Copy link
Copy Markdown
Contributor

sourcery-ai Bot commented May 8, 2026

Reviewer's Guide

Introduce explicit read-write and read-only database connection wrappers, add separate read-only DB configuration, and refactor modules/endpoints to consume the appropriate connection type (including enforcing read-only transactions) while keeping existing behavior via sensible fallbacks and updated tests/ADR documentation.

Sequence diagram for a read-only SBOM fetch using ReadOnly connection

sequenceDiagram
    actor Client
    participant Actix as ActixHandler
    participant SbomSvc as SbomService
    participant DBRO as ReadOnly
    participant Tx as DatabaseTransaction
    participant Replica as PostgreSQLReplica

    Client->>Actix: GET /v2/sbom/{id}
    Actix->>DBRO: begin()
    DBRO-->>Actix: Result(DatabaseTransaction)
    Actix->>SbomSvc: fetch_sbom_summary(id, &Tx)
    SbomSvc->>Tx: SELECT ... FROM sbom ...
    Tx->>Replica: execute read-only query
    Replica-->>Tx: rows
    Tx-->>SbomSvc: QueryResult
    SbomSvc-->>Actix: Option<SbomSummary>
    Actix->>Tx: commit()
    Tx-->>DBRO: ok
    Actix-->>Client: 200 OK with SBOM summary
Loading

Sequence diagram for SBOM upload using ReadWrite connection

sequenceDiagram
    actor Client
    participant Actix as ActixHandler
    participant Ingestor as IngestorService
    participant GroupSvc as SbomGroupService
    participant DBRW as ReadWrite
    participant Tx as DatabaseTransaction
    participant Primary as PostgreSQLPrimary

    Client->>Actix: POST /v2/sbom/upload
    Actix->>Actix: decompress bytes
    Actix->>DBRW: begin()
    DBRW-->>Actix: DatabaseTransaction

    Actix->>Ingestor: ingest(bytes, format, labels, cache, &Tx)
    Ingestor->>Tx: INSERT sbom, components, relations
    Tx->>Primary: execute writes
    Primary-->>Tx: ok
    Ingestor-->>Actix: IngestResult{ id }

    alt group not empty
        Actix->>GroupSvc: update_assignments(id, None, group, &Tx)
        GroupSvc->>Tx: INSERT/DELETE group links
        Tx->>Primary: execute writes
        Primary-->>Tx: ok
    end

    Actix->>Actix: rewrite result.id with urn:uuid: prefix
    Actix->>Tx: commit()
    Tx-->>DBRW: ok

    Actix-->>Client: 201 Created with SBOM metadata
Loading

Class diagram for new read-write and read-only database wrappers

classDiagram
    class Database {
        +name() &str
        +ping() Result
        +close() Result
    }

    class ReadWrite {
        -Database inner
        +new(db Database) ReadWrite
        +name() &str
        +ping() Result
        +close() Result
        +into_inner() Database
    }

    class ReadOnly {
        -Database inner
        +new(db Database) ReadOnly
        +name() &str
        +ping() Result
        +close() Result
        +begin() Result~DatabaseTransaction,DbError~
        +begin_with_config(isolation_level Option~IsolationLevel~, access_mode Option~AccessMode~) Result~DatabaseTransaction,DbError~
        -validate_access_mode(mode Option~AccessMode~) Result~Option~AccessMode~,DbError~
        +into_inner() Database
    }

    class DatabaseReadOnly {
        +url Option~String~
        +username Option~String~
        +password Option~Hide~String~~
        +host Option~String~
        +port Option~u16~
        +name Option~String~
        +max_conn Option~u32~
        +min_conn Option~u32~
        +sslmode Option~SslMode~
        +connect_timeout Option~u64~
        +acquire_timeout Option~u64~
        +max_lifetime Option~u64~
        +idle_timeout Option~u64~
        +is_configured() bool
        +to_database_config(fallback Database) Database
    }

    class DbError {
        <<enum>>
        Database
        Unavailable
        ReadOnly
    }

    class ConnectionTrait {
        <<interface>>
        +get_database_backend() DbBackend
        +execute(stmt Statement) Result~ExecResult,DbErr~
        +execute_unprepared(sql &str) Result~ExecResult,DbErr~
        +query_one(stmt Statement) Result~Option~QueryResult~,DbErr~
        +query_all(stmt Statement) Result~Vec~QueryResult~,DbErr~
        +support_returning() bool
    }

    class StreamTrait {
        <<interface>>
        +stream(stmt Statement) Future~Stream,DbErr~
    }

    class TransactionTrait {
        <<interface>>
        +begin() Result~DatabaseTransaction,DbErr~
        +begin_with_config(isolation_level Option~IsolationLevel~, access_mode Option~AccessMode~) Result~DatabaseTransaction,DbErr~
        +transaction(callback F) Result~T,TransactionError~E~~
        +transaction_with_config(callback F, isolation_level Option~IsolationLevel~, access_mode Option~AccessMode~) Result~T,TransactionError~E~~
    }

    class IntoSchemaManagerConnection {
        <<interface>>
        +into_schema_manager_connection() SchemaManagerConnection
    }

    class DatabaseConnection

    ReadWrite o-- Database : wraps
    ReadOnly o-- Database : wraps
    DatabaseReadOnly --> Database : builds

    ReadWrite ..|> ConnectionTrait
    ReadWrite ..|> StreamTrait
    ReadWrite ..|> TransactionTrait
    ReadWrite ..|> IntoSchemaManagerConnection

    ReadOnly ..|> IntoSchemaManagerConnection
    ReadOnly ..> TransactionTrait : uses begin_with_config

    DatabaseConnection ..|> ConnectionTrait
    DatabaseConnection ..|> StreamTrait
    DatabaseConnection ..|> TransactionTrait

    DbError --> DbErr : wraps
    DbError ..> HttpResponse : ResponseError

    ReadWrite --> DatabaseConnection : Deref
    ReadOnly --> DatabaseConnection : used inside begin_with_config
Loading

File-Level Changes

Change Details Files
Add ReadWrite and ReadOnly database wrappers with trait implementations and error handling to enforce read-only transactions and integrate with actix and SeaORM.
  • Introduce ReadWrite newtype around Database with Deref/DerefMut to DatabaseConnection and full ConnectionTrait, StreamTrait, TransactionTrait delegations.
  • Introduce ReadOnly newtype around Database that exposes only high-level helpers plus begin/begin_with_config methods that always open transactions with AccessMode::ReadOnly, including validation to reject explicit ReadWrite access mode.
  • Add DbError enum (Database/Unavailable/ReadOnly) with From and ResponseError implementations to translate database errors and read-only violations into appropriate HTTP responses.
  • Implement IntoSchemaManagerConnection for &ReadWrite to support schema management using the underlying Database connection.
common/src/db/mod.rs
Add separate read-only database configuration struct and CLI/env wiring, with overlay semantics and fallback to the primary database config.
  • Define DatabaseReadOnly struct with optional fields mirroring Database and dedicated TRUSTD_DB_RO_* environment variables and CLI flags.
  • Implement DatabaseReadOnly::is_configured to detect when any read-only-specific option is provided.
  • Implement DatabaseReadOnly::to_database_config to overlay read-only options on top of the primary Database configuration.
  • Extend Run profile configuration to include database_ro and propagate it through InitData and module Config wiring.
common/src/config.rs
server/src/profile/api.rs
Instantiate and pass ReadWrite/ReadOnly into server and module configuration, splitting responsibilities between read-heavy and write-heavy paths.
  • Create db_rw and db_ro instances in InitData::new: db_rw always wraps the primary Database; db_ro either connects to an explicit read-only database config or falls back to the primary connection.
  • Extend server Config to carry db_rw and db_ro alongside the existing db and propagate them into the top-level API configure function.
  • Update top-level API wiring to pass db_rw to importer/ingestor/user and db_rw+db_ro to fundamental, and db_ro to analysis, aligning module responsibilities with read/write patterns.
  • Adjust openapi test harness to construct db_rw/db_ro wrappers and feed them into module configuration for schema generation.
server/src/profile/api.rs
server/src/openapi.rs
Refactor fundamental module endpoint configuration to accept and use ReadWrite and ReadOnly connections appropriately across submodules.
  • Change fundamental::endpoints::configure signature to accept db_rw and db_ro, using db_rw.into_inner() where a raw Database is still required (e.g., Graph construction) and routing submodule configuration based on read/write behavior.
  • Update advisory, sbom, sbom_group, product, organization, license, purl, vulnerability, weakness endpoint configure functions to accept db_rw and/or db_ro and register them as web::Data as needed.
  • Ensure services that internally need DatabaseConnection (e.g., AdvisoryService, SbomService, WeaknessService, ImporterService, UserPreferenceService) are constructed with db_rw.into_inner() or db_ro.clone().into_inner() instead of the wrapper types.
  • Wire both db_rw and db_ro as actix app_data where endpoints need to extract the correct connection type per handler.
modules/fundamental/src/endpoints.rs
modules/fundamental/src/advisory/endpoints/mod.rs
modules/fundamental/src/sbom/endpoints/mod.rs
modules/fundamental/src/sbom_group/endpoints/mod.rs
modules/fundamental/src/organization/endpoints/mod.rs
modules/fundamental/src/product/endpoints/mod.rs
modules/fundamental/src/vulnerability/endpoints/mod.rs
modules/fundamental/src/purl/endpoints/mod.rs
modules/fundamental/src/purl/endpoints/base.rs
modules/fundamental/src/license/endpoints/mod.rs
modules/fundamental/src/weakness/endpoints/mod.rs
modules/user/src/endpoints.rs
modules/importer/src/endpoints.rs
Update individual endpoint handlers to use ReadOnly for read paths and ReadWrite for write paths, switching from begin_read/transaction helpers to explicit begin/commit on the wrapper types.
  • Change handler signatures to extract web::Datadb::ReadOnly for read-only operations (e.g., listing/fetching SBOMs, advisories, vulnerabilities, organizations, products, purls, labels, analysis queries) and replace db.begin_read() with db.begin().
  • Change mutating handlers (e.g., SBOM/advisory uploads and deletes, label set/update, group create/update/delete, dataset ingestion) to extract web::Datadb::ReadWrite, explicitly start transactions with db.begin(), pass &tx into services, and commit with tx.commit().await? instead of using db.transaction(...).
  • Adjust AnalysisService-facing endpoints to open a read-only transaction via db.begin() and pass &DatabaseTransaction into service methods instead of a &DatabaseConnection.
  • Ensure return types and error conversions use the new DbError via From implemented on module Error types where necessary.
modules/fundamental/src/sbom/endpoints/mod.rs
modules/fundamental/src/sbom/endpoints/label.rs
modules/fundamental/src/advisory/endpoints/mod.rs
modules/fundamental/src/advisory/endpoints/label.rs
modules/fundamental/src/sbom_group/endpoints/mod.rs
modules/fundamental/src/product/endpoints/mod.rs
modules/fundamental/src/organization/endpoints/mod.rs
modules/fundamental/src/vulnerability/endpoints/mod.rs
modules/fundamental/src/purl/endpoints/mod.rs
modules/fundamental/src/purl/endpoints/base.rs
modules/fundamental/src/license/endpoints/mod.rs
modules/fundamental/src/weakness/endpoints/mod.rs
modules/analysis/src/endpoints/mod.rs
modules/ingestor/src/endpoints.rs
Propagate DbError into module error types to preserve HTTP error semantics when using ReadOnly/ReadWrite wrappers.
  • Add From for modules::analysis::Error, mapping DbError::Database to Database, DbError::Unavailable to Unavailable, and DbError::ReadOnly to Internal with the error message.
  • Add similar From for modules::fundamental::Error with the same mapping semantics.
  • Ensure endpoints that now return DbError via ReadOnly/ReadWrite begin()/transaction calls can still use ? with module-level Error.
modules/analysis/src/error.rs
modules/fundamental/src/error.rs
Update tests, test helpers, and documentation to account for the dual connection model and new wrappers.
  • Adjust test callers and app builders in fundamental, analysis, ingestor, importer, and user modules to construct db::ReadWrite and/or db::ReadOnly wrappers around the embedded test Database and pass them into configure functions instead of raw Database.
  • Update server tests to create Config instances with db_rw and db_ro in addition to db, matching the new wiring used in production code.
  • Add ADR 00018 documenting the rationale, design, alternatives, and consequences of introducing dual database connections (R/W + R/O), including configuration and interaction with existing read-only mode ADR.
  • Ensure OpenAPI generation tests create db_rw/db_ro for schema generation using in-memory DB and storage backends.
modules/fundamental/src/test/common.rs
modules/analysis/src/test/mod.rs
modules/importer/src/test.rs
modules/ingestor/tests/common.rs
modules/user/src/test.rs
server/src/profile/api.rs
server/src/openapi.rs
docs/adrs/00018-dual-database-connections.md

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@ctron ctron force-pushed the feature/ro_instance_1 branch 2 times, most recently from 6d215af to e8d3263 Compare May 11, 2026 13:32
@ctron ctron changed the title Feature/ro instance 1 feat: implement the read-only database connection May 11, 2026
@ctron ctron requested review from jcrossley3 and mrizzi May 11, 2026 14:19
@ctron ctron marked this pull request as ready for review May 11, 2026 14:19
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @ctron, your pull request is larger than the review limit of 150000 diff characters

@codecov
Copy link
Copy Markdown

codecov Bot commented May 11, 2026

Codecov Report

❌ Patch coverage is 61.80556% with 220 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.71%. Comparing base (480d2bd) to head (0e0d3cd).

Files with missing lines Patch % Lines
common/src/db/mod.rs 36.55% 89 Missing and 3 partials ⚠️
modules/fundamental/src/sbom/endpoints/mod.rs 62.22% 4 Missing and 13 partials ⚠️
modules/analysis/src/service/mod.rs 26.66% 11 Missing ⚠️
modules/analysis/src/endpoints/mod.rs 47.36% 0 Missing and 10 partials ⚠️
modules/user/src/endpoints.rs 28.57% 10 Missing ⚠️
...odules/fundamental/src/sbom_group/endpoints/mod.rs 70.00% 0 Missing and 9 partials ⚠️
modules/analysis/src/error.rs 0.00% 6 Missing ⚠️
modules/fundamental/src/error.rs 0.00% 6 Missing ⚠️
...les/fundamental/src/vulnerability/endpoints/mod.rs 33.33% 4 Missing and 2 partials ⚠️
...odules/fundamental/src/advisory/endpoints/label.rs 61.53% 1 Missing and 4 partials ⚠️
... and 20 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2356      +/-   ##
==========================================
- Coverage   70.90%   70.71%   -0.19%     
==========================================
  Files         442      440       -2     
  Lines       25369    25669     +300     
  Branches    25369    25669     +300     
==========================================
+ Hits        17987    18152     +165     
- Misses       6400     6510     +110     
- Partials      982     1007      +25     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@queria
Copy link
Copy Markdown
Contributor

queria commented May 19, 2026

I think You may want also update the guidance in CONVENTIONS.md:

## Endpoint Patterns
...
- Read operations acquire a read transaction: `let tx = db.begin_read().await?;`

and maybe hint there when ReadOnly vs ReadWrite should be used (although that may be obvious to people or AI enough).

@queria
Copy link
Copy Markdown
Contributor

queria commented May 19, 2026

Also seems to me that with this, the begin_read and so DatabaseExt may be unecessary?

Copy link
Copy Markdown
Contributor

@queria queria left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if the mode without R/W DB configuration, as mentioned in ADR, is possible currently?

Comment thread docs/adrs/00018-dual-database-connections.md
Comment thread server/src/profile/api.rs
false => None,
};

let db = db::Database::new(&run.database).await?;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Database instance is not conditional - at least not conditional like db_ro = database_ro.is_configured() { ... } below.

Comment thread server/src/profile/api.rs
} else {
db::ReadOnly::new(db.clone())
};
let db_rw = db::ReadWrite::new(db.clone());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And it's then always cloned and below passed to InitData.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is used later. But, passing it on to the Analysis service, that should be the readonly connection.

@helio-frota
Copy link
Copy Markdown
Contributor

helio-frota commented May 19, 2026

Also seems to me that with this, the begin_read and so DatabaseExt may be unecessary?

yeah seems unused:

➜  trustify git:(feature/ro_instance_1) rg DatabaseExt
modules/fundamental/src/db.rs
5:pub trait DatabaseExt {
17:impl<T> DatabaseExt for T

when runnin it with pm-mode, I'm not 100% following why I'm logged as 'Marvin' and it shows extra parts that I can see only when using SSO

2026-05-19_11-05 2026-05-19_11-05_1

UPDATE: nvm ^^^ I forgot auth-disabled env var ...

Copy link
Copy Markdown
Contributor

@jcrossley3 jcrossley3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is application-level DB routing required? What existing problem is it solving? And is the configuration complexity trade-off worth it? Whoever will be responsible for that configuration should review this PR.

The `server/src/profile/api.rs` `configure()` function constructs both wrapper types and passes them
to each module accordingly.

## Alternatives considered
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was PGPool-II considered?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, as we would need to build, productize and support it. It would also be required to incorporated into deployments like RDS.

RDS on the other side offers read-only replicas out of the box.

@helio-frota
Copy link
Copy Markdown
Contributor

other than git conflict, overall lgtm

@ctron
Copy link
Copy Markdown
Contributor Author

ctron commented May 21, 2026

Why is application-level DB routing required? What existing problem is it solving?

I updated the ADR to explain this better.

And is the configuration complexity trade-off worth it?

Yes. As all we're talking about is adding a new set of DB connection parameters only if this should be enabled.

ctron and others added 7 commits May 21, 2026 10:08
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
@ctron ctron force-pushed the feature/ro_instance_1 branch from afd8caa to 0e0d3cd Compare May 21, 2026 08:19
@helio-frota
Copy link
Copy Markdown
Contributor

helio-frota commented May 21, 2026

it was cool to see working!

| Primary + replica, full mode | Primary | Replica | `false` | Reads go to replica, writes to primary |

I ran the first time using the env-var pointing to the primary and then changed later to the replica and refreshed the dashboard N times:

primary:

2026-05-21_10-24

replica:

2026-05-21_10-21

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

4 participants