From 36ea6bd3cac1757b32a0c80c86128df906b598a0 Mon Sep 17 00:00:00 2001 From: Christian Schwartz Date: Wed, 18 Mar 2026 17:37:59 +0100 Subject: [PATCH 01/13] feat: Implement User, Role and Permission Models --- config/roles/analysts.yaml | 8 + config/roles/engineers.yaml | 8 + config/roles/schema.json | 59 ++++++ docs/patterns/error-handling.md | 9 +- docs/patterns/ingestion.md | 4 + docs/patterns/models.md | 2 +- docs/patterns/routes.md | 8 +- mock-services/devenv.nix | 15 ++ mock-services/iam/__init__.py | 0 mock-services/iam/app.py | 12 ++ mock-services/iam/data/users.yaml | 18 ++ mock-services/iam/models.py | 14 ++ .../0e9a994a92da_create_auth_tables.py | 137 +++++++++++++ verdict-backend/app/cli.py | 86 ++++++++ verdict-backend/app/config.py | 2 + verdict-backend/app/errors.py | 29 ++- verdict-backend/app/http.py | 22 +++ verdict-backend/app/main.py | 6 + verdict-backend/app/models/__init__.py | 3 +- verdict-backend/app/models/gold_source.py | 11 +- verdict-backend/app/models/user.py | 90 +++++++++ verdict-backend/app/queries.py | 102 ++++++++++ verdict-backend/app/routes/assets.py | 4 +- verdict-backend/app/routes/roles.py | 44 +++++ verdict-backend/app/routes/sync.py | 22 +++ verdict-backend/app/routes/systems.py | 4 +- verdict-backend/app/routes/users.py | 52 +++++ verdict-backend/app/schemas/config.py | 15 ++ verdict-backend/app/schemas/external/iam.py | 14 ++ verdict-backend/app/schemas/ingestion.py | 4 + verdict-backend/app/schemas/role.py | 12 ++ verdict-backend/app/schemas/sync.py | 6 + verdict-backend/app/schemas/user.py | 12 ++ .../app/services/asset_ingestion.py | 78 ++------ .../app/services/cmdb_ingestion.py | 105 +++------- verdict-backend/app/services/config_sync.py | 139 +++++++++++++ verdict-backend/app/services/iam_ingestion.py | 129 ++++++++++++ verdict-backend/devenv.nix | 24 ++- verdict-backend/justfile | 8 + verdict-backend/pyproject.toml | 3 + verdict-backend/tests/cli/__init__.py | 0 verdict-backend/tests/cli/test_create_user.py | 153 ++++++++++++++ verdict-backend/tests/factories/__init__.py | 2 +- verdict-backend/tests/factories/asset.py | 2 +- verdict-backend/tests/factories/system.py | 2 +- verdict-backend/tests/factories/user.py | 52 +++++ .../tests/models/test_gold_source.py | 13 +- verdict-backend/tests/models/test_tags.py | 3 +- .../tests/models/test_timestamp.py | 5 +- verdict-backend/tests/models/test_user.py | 69 +++++++ verdict-backend/tests/routes/test_roles.py | 60 ++++++ verdict-backend/tests/routes/test_sync.py | 60 ++++++ verdict-backend/tests/routes/test_users.py | 56 ++++++ .../tests/services/test_config_sync.py | 186 ++++++++++++++++++ verdict-backend/uv.lock | 73 +++++++ 55 files changed, 1890 insertions(+), 166 deletions(-) create mode 100644 config/roles/analysts.yaml create mode 100644 config/roles/engineers.yaml create mode 100644 config/roles/schema.json create mode 100644 mock-services/iam/__init__.py create mode 100644 mock-services/iam/app.py create mode 100644 mock-services/iam/data/users.yaml create mode 100644 mock-services/iam/models.py create mode 100644 verdict-backend/alembic/versions/0e9a994a92da_create_auth_tables.py create mode 100644 verdict-backend/app/cli.py create mode 100644 verdict-backend/app/http.py create mode 100644 verdict-backend/app/models/user.py create mode 100644 verdict-backend/app/routes/roles.py create mode 100644 verdict-backend/app/routes/sync.py create mode 100644 verdict-backend/app/routes/users.py create mode 100644 verdict-backend/app/schemas/config.py create mode 100644 verdict-backend/app/schemas/external/iam.py create mode 100644 verdict-backend/app/schemas/role.py create mode 100644 verdict-backend/app/schemas/sync.py create mode 100644 verdict-backend/app/schemas/user.py create mode 100644 verdict-backend/app/services/config_sync.py create mode 100644 verdict-backend/app/services/iam_ingestion.py create mode 100644 verdict-backend/tests/cli/__init__.py create mode 100644 verdict-backend/tests/cli/test_create_user.py create mode 100644 verdict-backend/tests/factories/user.py create mode 100644 verdict-backend/tests/models/test_user.py create mode 100644 verdict-backend/tests/routes/test_roles.py create mode 100644 verdict-backend/tests/routes/test_sync.py create mode 100644 verdict-backend/tests/routes/test_users.py create mode 100644 verdict-backend/tests/services/test_config_sync.py diff --git a/config/roles/analysts.yaml b/config/roles/analysts.yaml new file mode 100644 index 0000000..85404a5 --- /dev/null +++ b/config/roles/analysts.yaml @@ -0,0 +1,8 @@ +# yaml-language-server: $schema=./schema.json +name: analysts +gold_source_id: GRP-002 +permissions: + - resource: assets + action: read + - resource: assets + action: write diff --git a/config/roles/engineers.yaml b/config/roles/engineers.yaml new file mode 100644 index 0000000..74237ee --- /dev/null +++ b/config/roles/engineers.yaml @@ -0,0 +1,8 @@ +# yaml-language-server: $schema=./schema.json +name: engineers +gold_source_id: GRP-001 +permissions: + - resource: assets + action: read + - resource: systems + action: read diff --git a/config/roles/schema.json b/config/roles/schema.json new file mode 100644 index 0000000..e810278 --- /dev/null +++ b/config/roles/schema.json @@ -0,0 +1,59 @@ +{ + "$defs": { + "PermissionConfig": { + "properties": { + "resource": { + "title": "Resource", + "type": "string" + }, + "subresource": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "default": null, + "title": "Subresource" + }, + "action": { + "title": "Action", + "type": "string" + } + }, + "required": [ + "resource", + "action" + ], + "title": "PermissionConfig", + "type": "object" + } + }, + "properties": { + "name": { + "pattern": "^[a-z][a-z0-9-]*$", + "title": "Name", + "type": "string" + }, + "gold_source_id": { + "title": "Gold Source Id", + "type": "string" + }, + "permissions": { + "items": { + "$ref": "#/$defs/PermissionConfig" + }, + "title": "Permissions", + "type": "array" + } + }, + "required": [ + "name", + "gold_source_id", + "permissions" + ], + "title": "RoleConfig", + "type": "object" +} diff --git a/docs/patterns/error-handling.md b/docs/patterns/error-handling.md index f344e43..86dac26 100644 --- a/docs/patterns/error-handling.md +++ b/docs/patterns/error-handling.md @@ -10,7 +10,14 @@ A `Result[T, E]` is either `Ok(value)` or `Err(error)`, where the error type is All error types inherit from `AppError`, an abstract base class that enforces a two-level message contract. Each error exposes a `message` property (safe to return to clients) and a `detail` property (for internal logging only, may contain URLs, SQL statements, or driver messages). `__str__` returns `message`, so errors are safe by default anywhere they are converted to strings. Error types store their raw diagnostic data in fields and compute both properties from them. -Error types are domain-specific dataclasses defined in `app/errors.py`. They form unions that describe what can go wrong in each context: `IngestionError` covers fetch failures, schema validation errors, and database errors. Functions declare which error union they can produce, and callers handle each variant explicitly. +Error types are domain-specific dataclasses defined in `app/errors.py`. They form unions that describe what can go wrong in each context: + +- `IngestionError = FetchError | RemoteValidationError | ValidationError | DBError` — covers HTTP failures, remote schema parse errors, local validation errors, and database errors. +- `ConfigSyncError = ConfigError | ValidationError | DBError` — covers YAML file errors (missing directory, unreadable file, empty file), local validation errors, and database errors. + +`RemoteValidationError` is a URL-aware subclass of `ValidationError`. It is produced when an HTTP response body fails Pydantic validation and includes the source URL in its `detail`, making it easier to diagnose which remote endpoint returned an unexpected shape. `ValidationError` is used for local validation failures (e.g., config files) where no URL context is available. + +Functions declare which error union they can produce, and callers handle each variant explicitly. ## Route Helpers diff --git a/docs/patterns/ingestion.md b/docs/patterns/ingestion.md index 1111f06..9a4da26 100644 --- a/docs/patterns/ingestion.md +++ b/docs/patterns/ingestion.md @@ -8,6 +8,8 @@ External data is ingested through a three-phase pipeline that separates fetching **Phase 2 — Fetch Detail.** For each item in the index, the service calls the detail endpoint to retrieve the full representation. This is validated against a separate Pydantic model for the detail response shape. +Both fetch phases go through the `fetch_json` helper (`app/http.py`), which handles HTTP errors uniformly: transport/status failures become `FetchError` (with the URL), and Pydantic parse failures become `RemoteValidationError` (a URL-aware `ValidationError` subclass). Services declare their source error type as `FetchError | RemoteValidationError`. + **Phase 3 — Convert and Upsert.** A pure function maps each external detail to an `XCreate` model instance. The orchestrator then upserts each converted record into the database, keyed by gold source ID (the record's unique identifier in the external system — see [Gold Source Identity](models.md#gold-source-identity)), and returns `XPublic` instances via `model_validate` after flush. The separation of index and detail schemas reflects the reality that list and detail endpoints often return different shapes. Keeping the conversion as a pure function (no IO, no session) makes it independently testable. The orchestrator does not commit — the caller (typically the route handler) owns the session lifecycle, which preserves all-or-nothing semantics: if any phase fails, nothing is persisted. @@ -18,6 +20,8 @@ When multiple sources need to be ingested atomically, a global ingestion endpoin Sources that reference records from other sources (e.g., systems referencing assets) resolve those references by gold source ID during their pipeline. If a referenced record cannot be found, the entire source's ingestion is rejected. +The upsert helpers (`upsert_by_gold_source`, `_upsert_permission`) use a select-then-insert pattern and only catch `OperationalError` on flush. `IntegrityError` from a concurrent insert is deliberately not handled: all ingestion and config-sync callers are single-threaded batch operations, so the race cannot occur. If concurrent callers are ever introduced, add `IntegrityError` handling at the flush site. + ## Link Resolution and Syncing For N:M relationships that cross source boundaries, the ingestion pipeline resolves external IDs to internal IDs using `get_by_gold_source`. After upserting the parent record, a diff-based sync step compares the desired set of linked IDs against the current set in the database, then issues INSERT and DELETE statements against the join table to reconcile the difference. This ensures re-ingestion is idempotent — links are added or removed to match the source of truth without duplicating or orphaning entries. diff --git a/docs/patterns/models.md b/docs/patterns/models.md index 92386f2..5db2f8e 100644 --- a/docs/patterns/models.md +++ b/docs/patterns/models.md @@ -24,7 +24,7 @@ Each domain entity uses a family of SQLModel classes that separate concerns acro ## Gold Source Identity -Models that are synced from external systems inherit from `GoldSourceMixin`, which provides `gold_source_id` and `gold_source_type` fields with a composite unique constraint. This allows any synced record to be looked up by its external identity. Each model adds a composite index on these fields for query performance. The mixin provides a `get_by_gold_source` query method that returns a typed `Result[Option[T], DBError]`, consistent with the error handling pattern. +Models that are synced from external systems inherit from `GoldSourceMixin`, which provides `gold_source_id` (string) and `gold_source_type` (`GoldSourceType` enum, stored as `VARCHAR` to avoid a PostgreSQL enum type) fields with a composite unique constraint. This allows any synced record to be looked up by its external identity. The unique constraint implicitly backs gold-source lookups — no separate index is needed. The mixin provides a `get_by_gold_source` query method that returns a typed `Result[Option[T], DBError]`, consistent with the error handling pattern. ## Query Functions and Type Narrowing diff --git a/docs/patterns/routes.md b/docs/patterns/routes.md index 8f34dd1..c18b714 100644 --- a/docs/patterns/routes.md +++ b/docs/patterns/routes.md @@ -8,10 +8,10 @@ List endpoints support offset/limit pagination. Lookup endpoints return 404 when ## Response Types -`XPublic` models (defined in `app/models/`) serve directly as API response types. There is no separate response schema layer — `XPublic` is used as the `response_model` in route decorators and as the return type of route handlers. +`XPublic` models (defined in `app/models/`) are the default API response type for individual item endpoints. For simple models (no relationships), routes pass `public_class=XPublic` to query functions (`get_by_id`, `get_paginated`, `get_by_gold_source`), which return the narrowed type directly. -For simple models (no relationships), routes pass `public_class=XPublic` to query functions (`get_by_id`, `get_paginated`, `get_by_gold_source`), which return the narrowed type directly. +For models with ORM relationships that need derived fields (e.g., `asset_ids`), routes query the ORM model and convert via a `_to_public` helper that populates the derived fields from the loaded relationship. -For models with relationships that need derived fields (e.g., `asset_ids`), routes query the ORM model and convert via a `_to_public` helper that populates the derived fields from the loaded relationship. +For endpoints that require joined or enriched data beyond the model's own fields (e.g., `GET /users/{id}` returning the user's assigned role names), a schema class under `app/schemas/` extends `XPublic` with the additional fields. These schemas are used as the `response_model` for that specific endpoint; the list endpoint for the same resource continues to use `XPublic` directly to avoid N+1 queries. -List endpoints use lightweight wrapper schemas (e.g., `AssetListResponse`) under `app/schemas/` that pair a list of `XPublic` items with a `total` count. These are the only response schemas — individual item responses use `XPublic` directly. +List endpoints use lightweight wrapper schemas (e.g., `AssetListResponse`, `UserListResponse`) under `app/schemas/` that pair a list of `XPublic` items with a `total` count. diff --git a/mock-services/devenv.nix b/mock-services/devenv.nix index 3b15667..b1083ed 100644 --- a/mock-services/devenv.nix +++ b/mock-services/devenv.nix @@ -47,4 +47,19 @@ in }; }; }; + + processes.iam-mock = { + exec = "cd $MOCK_SERVICES_DIR && uv run uvicorn iam.app:app --host 0.0.0.0 --port 4012"; + process-compose = { + readiness_probe = { + http_get = { + host = "localhost"; + port = 4012; + path = "/users"; + }; + initial_delay_seconds = 2; + period_seconds = 2; + }; + }; + }; } diff --git a/mock-services/iam/__init__.py b/mock-services/iam/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/mock-services/iam/app.py b/mock-services/iam/app.py new file mode 100644 index 0000000..9c873b2 --- /dev/null +++ b/mock-services/iam/app.py @@ -0,0 +1,12 @@ +from pathlib import Path + +from iam.models import IAMUserIndexItem, IAMUserItem +from mock_helpers import create_mock_app + +app, _ = create_mock_app( + title="IAM Mock", + data_path=Path(__file__).parent / "data" / "users.yaml", + full_model=IAMUserItem, + index_model=IAMUserIndexItem, + resource_name="users", +) diff --git a/mock-services/iam/data/users.yaml b/mock-services/iam/data/users.yaml new file mode 100644 index 0000000..9017325 --- /dev/null +++ b/mock-services/iam/data/users.yaml @@ -0,0 +1,18 @@ +- id: "USR-001" + username: "alice" + email: "alice@example.com" + groups: + - "GRP-001" + +- id: "USR-002" + username: "bob" + email: "bob@example.com" + groups: + - "GRP-002" + +- id: "USR-003" + username: "charlie" + email: "charlie@example.com" + groups: + - "GRP-001" + - "GRP-002" diff --git a/mock-services/iam/models.py b/mock-services/iam/models.py new file mode 100644 index 0000000..ba35132 --- /dev/null +++ b/mock-services/iam/models.py @@ -0,0 +1,14 @@ +from pydantic import BaseModel + + +class IAMUserIndexItem(BaseModel): + id: str + username: str + email: str + + +class IAMUserItem(BaseModel): + id: str + username: str + email: str + groups: list[str] diff --git a/verdict-backend/alembic/versions/0e9a994a92da_create_auth_tables.py b/verdict-backend/alembic/versions/0e9a994a92da_create_auth_tables.py new file mode 100644 index 0000000..84f99c2 --- /dev/null +++ b/verdict-backend/alembic/versions/0e9a994a92da_create_auth_tables.py @@ -0,0 +1,137 @@ +"""create auth tables + +Revision ID: 0e9a994a92da +Revises: 3bfa8f122226 +Create Date: 2026-03-15 11:57:33.704856 + +""" + +from collections.abc import Sequence + +import sqlalchemy as sa +import sqlmodel + +from alembic import op + +# revision identifiers, used by Alembic. +revision: str = "0e9a994a92da" +down_revision: str | Sequence[str] | None = "3bfa8f122226" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + + +def upgrade() -> None: + """Upgrade schema.""" + # ### commands auto generated by Alembic - please adjust! ### + op.create_table( + "app_user", + sa.Column("id", sa.Integer(), nullable=False), + sa.Column( + "created_at", + sa.DateTime(timezone=True), + server_default=sa.text("now()"), + nullable=False, + ), + sa.Column( + "updated_at", + sa.DateTime(timezone=True), + server_default=sa.text("now()"), + nullable=False, + ), + sa.Column("gold_source_id", sqlmodel.sql.sqltypes.AutoString(), nullable=False), + sa.Column("gold_source_type", sqlmodel.sql.sqltypes.AutoString(), nullable=False), + sa.Column("username", sqlmodel.sql.sqltypes.AutoString(), nullable=False), + sa.Column("email", sqlmodel.sql.sqltypes.AutoString(), nullable=False), + sa.Column( + "is_active", + sa.Boolean(), + server_default=sa.text("true"), + nullable=False, + ), + sa.PrimaryKeyConstraint("id"), + sa.UniqueConstraint("gold_source_type", "gold_source_id"), + ) + op.create_table( + "permission", + sa.Column("id", sa.Integer(), nullable=False), + sa.Column( + "created_at", + sa.DateTime(timezone=True), + server_default=sa.text("now()"), + nullable=False, + ), + sa.Column( + "updated_at", + sa.DateTime(timezone=True), + server_default=sa.text("now()"), + nullable=False, + ), + sa.Column("resource", sqlmodel.sql.sqltypes.AutoString(), nullable=False), + sa.Column("subresource", sqlmodel.sql.sqltypes.AutoString(), nullable=True), + sa.Column("action", sqlmodel.sql.sqltypes.AutoString(), nullable=False), + sa.PrimaryKeyConstraint("id"), + sa.UniqueConstraint("resource", "subresource", "action"), + ) + op.create_table( + "role", + sa.Column("id", sa.Integer(), nullable=False), + sa.Column( + "created_at", + sa.DateTime(timezone=True), + server_default=sa.text("now()"), + nullable=False, + ), + sa.Column( + "updated_at", + sa.DateTime(timezone=True), + server_default=sa.text("now()"), + nullable=False, + ), + sa.Column("gold_source_id", sqlmodel.sql.sqltypes.AutoString(), nullable=False), + sa.Column("gold_source_type", sqlmodel.sql.sqltypes.AutoString(), nullable=False), + sa.Column("name", sqlmodel.sql.sqltypes.AutoString(), nullable=False), + sa.Column("description", sqlmodel.sql.sqltypes.AutoString(), nullable=False), + sa.PrimaryKeyConstraint("id"), + sa.UniqueConstraint("gold_source_type", "gold_source_id"), + sa.UniqueConstraint("name", name="uq_role_name"), + ) + op.create_table( + "rolepermission", + sa.Column("role_id", sa.Integer(), nullable=False), + sa.Column("permission_id", sa.Integer(), nullable=False), + sa.ForeignKeyConstraint( + ["permission_id"], + ["permission.id"], + ), + sa.ForeignKeyConstraint( + ["role_id"], + ["role.id"], + ), + sa.PrimaryKeyConstraint("role_id", "permission_id"), + ) + op.create_table( + "userrole", + sa.Column("user_id", sa.Integer(), nullable=False), + sa.Column("role_id", sa.Integer(), nullable=False), + sa.ForeignKeyConstraint( + ["role_id"], + ["role.id"], + ), + sa.ForeignKeyConstraint( + ["user_id"], + ["app_user.id"], + ), + sa.PrimaryKeyConstraint("user_id", "role_id"), + ) + # ### end Alembic commands ### + + +def downgrade() -> None: + """Downgrade schema.""" + # ### commands auto generated by Alembic - please adjust! ### + op.drop_table("userrole") + op.drop_table("rolepermission") + op.drop_table("role") + op.drop_table("permission") + op.drop_table("app_user") + # ### end Alembic commands ### diff --git a/verdict-backend/app/cli.py b/verdict-backend/app/cli.py new file mode 100644 index 0000000..9a8d4c7 --- /dev/null +++ b/verdict-backend/app/cli.py @@ -0,0 +1,86 @@ +from typing import Annotated + +import typer +from sqlmodel import Session + +from app.db import engine +from app.errors import DBError +from app.models.gold_source import GoldSourceType +from app.models.user import RolePublic, User, UserPublic +from app.queries import get_role_by_name, sync_user_roles, upsert_by_gold_source +from app.result import Err, Nothing, Ok, Result, Some + +cli = typer.Typer() + + +def _upsert_local_user( + session: Session, + username: str, + email: str, +) -> Result[UserPublic, DBError]: + def on_existing(existing: User) -> None: + existing.username = username + existing.email = email + + def make_new() -> User: + return User( + username=username, + email=email, + is_active=True, + gold_source_type=GoldSourceType.LOCAL_USER, + gold_source_id=username, + ) + + return upsert_by_gold_source( + session, + User, + GoldSourceType.LOCAL_USER, + username, + public_class=UserPublic, + on_existing=on_existing, + make_new=make_new, + ) + + +@cli.command() +def create_user( + username: str = typer.Option(..., help="Username for the local user"), + email: str = typer.Option(..., help="Email address for the local user"), + role: Annotated[ + list[str] | None, typer.Option("--role", help="Role name to assign (repeatable)") + ] = None, +) -> None: + """Create a local user, setting its roles to exactly the provided list. Fully idempotent.""" + with Session(engine) as session: + # Validate all roles up front before modifying anything. + roles: list[RolePublic] = [] + for role_name in role or []: + match get_role_by_name(session, role_name, public_class=RolePublic): + case Err(e): + typer.echo(f"Database error: {e.detail}", err=True) + raise typer.Exit(code=1) + case Ok(Some(r)): + roles.append(r) + case Ok(Nothing()): + typer.echo(f"Role '{role_name}' not found.", err=True) + raise typer.Exit(code=1) + + match _upsert_local_user(session, username, email): + case Err(e): + typer.echo(f"Database error: {e.detail}", err=True) + raise typer.Exit(code=1) + case Ok(user): + typer.echo(f"User '{username}' ready.") + + match sync_user_roles(session, user.id, [r.id for r in roles]): + case Err(e): + typer.echo(f"Database error: {e.detail}", err=True) + raise typer.Exit(code=1) + for rp in roles: + typer.echo(f"Assigned role '{rp.name}'.") + + session.commit() + + +if __name__ == "__main__": + cli() diff --git a/verdict-backend/app/config.py b/verdict-backend/app/config.py index 2e9b513..5ddea36 100644 --- a/verdict-backend/app/config.py +++ b/verdict-backend/app/config.py @@ -8,6 +8,8 @@ class Settings(DatabaseSettings): asset_inventory_url: str = Field(default=...) cmdb_url: str = Field(default=...) + iam_url: str = Field(default=...) + config_basedir: str = Field(default="config") debug: bool = False log_level: str = "info" diff --git a/verdict-backend/app/errors.py b/verdict-backend/app/errors.py index c48f2de..a532789 100644 --- a/verdict-backend/app/errors.py +++ b/verdict-backend/app/errors.py @@ -72,7 +72,6 @@ def detail(self) -> str: return f"fetch error ({self.url}): {self.raw}" -@final @dataclass(frozen=True, slots=True) class ValidationError(AppError): raw: str @@ -86,8 +85,34 @@ def detail(self) -> str: return f"validation error: {self.raw}" +@final +@dataclass(frozen=True, slots=True) +class RemoteValidationError(ValidationError): + url: str + + @property + def detail(self) -> str: + return f"validation error ({self.url}): {self.raw}" + + +@final +@dataclass(frozen=True, slots=True) +class ConfigError(AppError): + path: str + raw: str + + @property + def message(self) -> str: + return "config error" + + @property + def detail(self) -> str: + return f"config error ({self.path}): {self.raw}" + + type WriteError = DuplicateError | DBError -type IngestionError = FetchError | ValidationError | DBError +type IngestionError = FetchError | RemoteValidationError | ValidationError | DBError +type ConfigSyncError = ConfigError | ValidationError | DBError def db_error_from(e: OperationalError) -> DBError: diff --git a/verdict-backend/app/http.py b/verdict-backend/app/http.py new file mode 100644 index 0000000..5880e8c --- /dev/null +++ b/verdict-backend/app/http.py @@ -0,0 +1,22 @@ +import httpx +from pydantic import TypeAdapter +from pydantic import ValidationError as PydanticValidationError + +from app.errors import FetchError, RemoteValidationError +from app.result import Err, Ok, Result + + +def fetch_json[T]( + client: httpx.Client, + url: str, + adapter: TypeAdapter[T], +) -> Result[T, FetchError | RemoteValidationError]: + try: + response = client.get(url) + response.raise_for_status() + except httpx.HTTPError as e: + return Err(FetchError(url=url, raw=str(e))) + try: + return Ok(adapter.validate_json(response.content)) + except PydanticValidationError as e: + return Err(RemoteValidationError(raw=str(e), url=url)) diff --git a/verdict-backend/app/main.py b/verdict-backend/app/main.py index 91d2cec..c69d5b2 100644 --- a/verdict-backend/app/main.py +++ b/verdict-backend/app/main.py @@ -2,12 +2,18 @@ from app.routes.assets import router as assets_router from app.routes.ingestion import router as ingestion_router +from app.routes.roles import router as roles_router +from app.routes.sync import router as sync_router from app.routes.systems import router as systems_router +from app.routes.users import router as users_router app = FastAPI() app.include_router(assets_router) app.include_router(systems_router) app.include_router(ingestion_router) +app.include_router(users_router) +app.include_router(roles_router) +app.include_router(sync_router) @app.get("/") diff --git a/verdict-backend/app/models/__init__.py b/verdict-backend/app/models/__init__.py index fe342dd..7ecd743 100644 --- a/verdict-backend/app/models/__init__.py +++ b/verdict-backend/app/models/__init__.py @@ -1,2 +1,3 @@ import app.models.asset # register for alembic autogenerate -import app.models.system # noqa: F401 # register for alembic autogenerate +import app.models.system # register for alembic autogenerate +import app.models.user # noqa: F401 # register for alembic autogenerate diff --git a/verdict-backend/app/models/gold_source.py b/verdict-backend/app/models/gold_source.py index 18f245b..6f9537a 100644 --- a/verdict-backend/app/models/gold_source.py +++ b/verdict-backend/app/models/gold_source.py @@ -13,6 +13,9 @@ class GoldSourceType(StrEnum): ASSET_INVENTORY = "asset-inventory" CMDB = "cmdb" + IAM_USER = "iam_user" + IAM_GROUP = "iam_group" + LOCAL_USER = "local_user" class GoldSourceMixin(SQLModel): @@ -25,14 +28,14 @@ class GoldSourceMixin(SQLModel): __table_args__ = (sa.UniqueConstraint("gold_source_type", "gold_source_id"),) gold_source_id: str = Field(nullable=False) - gold_source_type: str = Field(nullable=False) + gold_source_type: GoldSourceType = Field(nullable=False, sa_type=sa.String()) # type: ignore[call-overload] # SQLModel Field stub types sa_type as type[Any] but accepts TypeEngine instances too @staticmethod @overload def get_by_gold_source[T: BaseModel]( session: Session, model_class: type[T], - gold_source_type: str, + gold_source_type: GoldSourceType, gold_source_id: str, ) -> Result[Option[T], DBError]: ... @@ -41,7 +44,7 @@ def get_by_gold_source[T: BaseModel]( def get_by_gold_source[T: BaseModel, P: PublicModel]( session: Session, model_class: type[T], - gold_source_type: str, + gold_source_type: GoldSourceType, gold_source_id: str, *, public_class: type[P], @@ -51,7 +54,7 @@ def get_by_gold_source[T: BaseModel, P: PublicModel]( def get_by_gold_source[T: BaseModel, P: PublicModel]( session: Session, model_class: type[T], - gold_source_type: str, + gold_source_type: GoldSourceType, gold_source_id: str, *, public_class: type[P] | None = None, diff --git a/verdict-backend/app/models/user.py b/verdict-backend/app/models/user.py new file mode 100644 index 0000000..db86d0c --- /dev/null +++ b/verdict-backend/app/models/user.py @@ -0,0 +1,90 @@ +import sqlalchemy as sa +from sqlmodel import Field, Relationship, SQLModel + +from app.models.base_model import BaseModel, PublicModel +from app.models.gold_source import GoldSourceMixin + +# --- Link tables (defined first; referenced by link_model= below) --- + + +class UserRole(SQLModel, table=True): + __tablename__ = "userrole" # pyright: ignore[reportAssignmentType] + + user_id: int | None = Field(default=None, foreign_key="app_user.id", primary_key=True) + role_id: int | None = Field(default=None, foreign_key="role.id", primary_key=True) + + +class RolePermission(SQLModel, table=True): + __tablename__ = "rolepermission" # pyright: ignore[reportAssignmentType] + + role_id: int | None = Field(default=None, foreign_key="role.id", primary_key=True) + permission_id: int | None = Field(default=None, foreign_key="permission.id", primary_key=True) + + +# --- Permission --- + + +class PermissionBase(SQLModel): + resource: str = Field(nullable=False) + subresource: str | None = Field(default=None) + action: str = Field(nullable=False) + + +class Permission(PermissionBase, BaseModel, table=True): + __tablename__ = "permission" # pyright: ignore[reportAssignmentType] + __table_args__ = (sa.UniqueConstraint("resource", "subresource", "action"),) + + +class PermissionPublic(PermissionBase, PublicModel): + pass + + +# --- Role --- + + +_ROLE_NAME_PATTERN = r"^[a-z][a-z0-9-]*$" + + +class RoleBase(GoldSourceMixin, SQLModel): + name: str = Field( + nullable=False, regex=_ROLE_NAME_PATTERN + ) # SQLModel uses pydantic v1-style `regex` + description: str = Field(default="", nullable=False) + + +class Role(RoleBase, BaseModel, table=True): + __tablename__ = "role" # pyright: ignore[reportAssignmentType] + __table_args__ = (*GoldSourceMixin.__table_args__, sa.UniqueConstraint("name")) # type: ignore[assignment] # extending base tuple with additional constraint + + permissions: list["Permission"] = Relationship(link_model=RolePermission) + + +class RolePublic(RoleBase, PublicModel): + pass + + +class RoleCreate(RoleBase): + pass + + +# --- User --- + + +class UserBase(GoldSourceMixin, SQLModel): + username: str = Field(nullable=False) + email: str = Field(nullable=False) + is_active: bool = Field(default=True) + + +class User(UserBase, BaseModel, table=True): + __tablename__ = "app_user" # pyright: ignore[reportAssignmentType] + + roles: list["Role"] = Relationship(link_model=UserRole) + + +class UserPublic(UserBase, PublicModel): + pass + + +class UserCreate(UserBase): + pass diff --git a/verdict-backend/app/queries.py b/verdict-backend/app/queries.py index 9f83922..661424b 100644 --- a/verdict-backend/app/queries.py +++ b/verdict-backend/app/queries.py @@ -1,11 +1,15 @@ +from collections.abc import Callable from dataclasses import dataclass from typing import Any, final, overload +import sqlalchemy as sa from sqlalchemy.exc import OperationalError from sqlmodel import Session, func, select from app.errors import DBError, db_error_from from app.models.base_model import BaseModel, PublicModel +from app.models.gold_source import GoldSourceMixin, GoldSourceType +from app.models.user import Role, UserRole from app.result import Err, Nothing, Ok, Option, Result, Some @@ -98,3 +102,101 @@ def get_paginated[T: BaseModel, P: PublicModel]( return Ok(PaginatedResult(items=public_items, total=total)) return Ok(PaginatedResult(items=items, total=total)) + + +@overload +def get_role_by_name(session: Session, name: str) -> Result[Option[Role], DBError]: ... + + +@overload +def get_role_by_name[P: PublicModel]( + session: Session, name: str, *, public_class: type[P] +) -> Result[Option[P], DBError]: ... + + +def get_role_by_name[P: PublicModel]( + session: Session, name: str, *, public_class: type[P] | None = None +) -> Result[Option[Any], DBError]: + try: + role = session.exec(select(Role).where(Role.name == name)).first() + except OperationalError as e: + return Err(db_error_from(e)) + if role is None: + return Ok(Nothing()) + if public_class is not None: + return Ok(Some(public_class.model_validate(role, from_attributes=True))) + return Ok(Some(role)) + + +def upsert_by_gold_source[T: BaseModel, P: PublicModel]( + session: Session, + model_class: type[T], + gold_source_type: GoldSourceType, + gold_source_id: str, + *, + public_class: type[P], + on_existing: Callable[[T], None], + make_new: Callable[[], T], +) -> Result[P, DBError]: + """Select-then-insert upsert by gold source identity. + + Only OperationalError is caught on flush — IntegrityError from a concurrent + insert is not handled. This is intentional: all callers are single-threaded + batch operations (ingestion services, config sync), so the race cannot occur. + If concurrent callers are ever introduced, add IntegrityError handling here. + """ + match GoldSourceMixin.get_by_gold_source( + session, model_class, gold_source_type, gold_source_id + ): + case Err(e): + return Err(e) + case Ok(Some(record)): + on_existing(record) + session.add(record) + case Ok(Nothing()) | _: + record = make_new() + session.add(record) + try: + session.flush() + except OperationalError as e: + return Err(db_error_from(e)) + return Ok(public_class.model_validate(record, from_attributes=True)) + + +def sync_join_table( + session: Session, + owner_col: sa.Column[int], + owner_id: int, + member_col: sa.Column[int], + desired_ids: list[int], +) -> Result[None, DBError]: + """Sync a join table so the member set matches desired_ids exactly + (add missing, remove extra).""" + try: + existing_ids = { + row[0] + for row in session.execute(sa.select(member_col).where(owner_col == owner_id)).all() + } + to_remove = existing_ids - set(desired_ids) + to_add = set(desired_ids) - existing_ids + if to_remove: + session.execute( + sa.delete(owner_col.table).where(owner_col == owner_id, member_col.in_(to_remove)) + ) + for mid in to_add: + session.execute( + sa.insert(owner_col.table).values({owner_col.key: owner_id, member_col.key: mid}) + ) + except OperationalError as e: + return Err(db_error_from(e)) + return Ok(None) + + +def sync_user_roles( + session: Session, + user_id: int, + desired_role_ids: list[int], +) -> Result[None, DBError]: + """Set a user's roles to exactly the provided list (add missing, remove extra).""" + t = UserRole.__table__ # type: ignore[attr-defined] # SQLModel does not expose __table__ in stubs but it exists on table=True models + return sync_join_table(session, t.c.user_id, user_id, t.c.role_id, desired_role_ids) diff --git a/verdict-backend/app/routes/assets.py b/verdict-backend/app/routes/assets.py index 810e15e..3d26ef3 100644 --- a/verdict-backend/app/routes/assets.py +++ b/verdict-backend/app/routes/assets.py @@ -3,7 +3,7 @@ from app.db import get_session from app.models.asset import Asset, AssetPublic -from app.models.gold_source import GoldSourceMixin +from app.models.gold_source import GoldSourceMixin, GoldSourceType from app.queries import get_by_id, get_paginated from app.result import unwrap_optional_or_raise, unwrap_or_raise from app.schemas.asset import AssetListResponse @@ -28,7 +28,7 @@ def list_assets( response_model=AssetPublic, ) def get_asset_by_gold_source( - source_type: str, + source_type: GoldSourceType, source_id: str, session: Session = Depends(get_session), ) -> AssetPublic: diff --git a/verdict-backend/app/routes/roles.py b/verdict-backend/app/routes/roles.py new file mode 100644 index 0000000..44501f1 --- /dev/null +++ b/verdict-backend/app/routes/roles.py @@ -0,0 +1,44 @@ +from fastapi import APIRouter, Depends, Query +from sqlmodel import Session + +from app.db import get_session +from app.models.user import PermissionPublic, Role, RolePublic +from app.queries import get_by_id, get_paginated, get_role_by_name +from app.result import unwrap_optional_or_raise, unwrap_or_raise +from app.schemas.role import RoleDetail, RoleListResponse + +router = APIRouter(prefix="/roles", tags=["roles"]) + + +def _to_role_detail(role: Role) -> RoleDetail: + detail = RoleDetail.model_validate(role, from_attributes=True) + detail.permissions = [ + PermissionPublic.model_validate(p, from_attributes=True) for p in role.permissions + ] + return detail + + +@router.get("/", response_model=RoleListResponse) +def list_roles( + session: Session = Depends(get_session), + offset: int = Query(default=0, ge=0), + limit: int = Query(default=50, ge=1, le=200), +) -> RoleListResponse: + paginated = unwrap_or_raise( + get_paginated(session, Role, offset, limit, public_class=RolePublic) + ) + return RoleListResponse(roles=paginated.items, total=paginated.total) + + +@router.get("/by-name/{name}", response_model=RoleDetail) +def get_role_by_name_route(name: str, session: Session = Depends(get_session)) -> RoleDetail: + result = get_role_by_name(session, name) + role = unwrap_optional_or_raise(result, not_found_detail="Role not found") + return _to_role_detail(role) + + +@router.get("/{role_id}", response_model=RoleDetail) +def get_role(role_id: int, session: Session = Depends(get_session)) -> RoleDetail: + result = get_by_id(session, Role, role_id) + role = unwrap_optional_or_raise(result, not_found_detail="Role not found") + return _to_role_detail(role) diff --git a/verdict-backend/app/routes/sync.py b/verdict-backend/app/routes/sync.py new file mode 100644 index 0000000..aac7915 --- /dev/null +++ b/verdict-backend/app/routes/sync.py @@ -0,0 +1,22 @@ +from pathlib import Path + +from fastapi import APIRouter, Depends +from sqlmodel import Session + +from app.config import settings +from app.db import get_session +from app.result import unwrap_or_raise +from app.schemas.sync import SyncResponse +from app.services.config_sync import sync_config + +router = APIRouter(tags=["sync"]) + + +@router.post("/sync", response_model=SyncResponse) +def trigger_sync( + session: Session = Depends(get_session), +) -> SyncResponse: + result = sync_config(session, Path(settings.config_basedir)) + response = unwrap_or_raise(result, err_status=500) + session.commit() + return response diff --git a/verdict-backend/app/routes/systems.py b/verdict-backend/app/routes/systems.py index ffd95bb..6ac15dc 100644 --- a/verdict-backend/app/routes/systems.py +++ b/verdict-backend/app/routes/systems.py @@ -2,7 +2,7 @@ from sqlmodel import Session from app.db import get_session -from app.models.gold_source import GoldSourceMixin +from app.models.gold_source import GoldSourceMixin, GoldSourceType from app.models.system import System, SystemPublic from app.queries import get_by_id, get_paginated from app.result import unwrap_optional_or_raise, unwrap_or_raise @@ -35,7 +35,7 @@ def list_systems( response_model=SystemPublic, ) def get_system_by_gold_source( - source_type: str, + source_type: GoldSourceType, source_id: str, session: Session = Depends(get_session), ) -> SystemPublic: diff --git a/verdict-backend/app/routes/users.py b/verdict-backend/app/routes/users.py new file mode 100644 index 0000000..0ff7ce8 --- /dev/null +++ b/verdict-backend/app/routes/users.py @@ -0,0 +1,52 @@ +import httpx +from fastapi import APIRouter, Depends, Query +from sqlmodel import Session + +from app.config import settings +from app.db import get_session +from app.deps import get_http_client +from app.models.user import User, UserPublic +from app.queries import get_by_id, get_paginated +from app.result import unwrap_optional_or_raise, unwrap_or_raise +from app.schemas.ingestion import UserIngestionResponse +from app.schemas.user import UserDetail, UserListResponse +from app.services.iam_ingestion import ingest_users + +router = APIRouter(prefix="/users", tags=["users"]) + + +def _to_user_detail(user: User) -> UserDetail: + public = UserPublic.model_validate(user, from_attributes=True) + return UserDetail(**public.model_dump(), roles=[role.name for role in user.roles]) + + +@router.get("/", response_model=UserListResponse) +def list_users( + session: Session = Depends(get_session), + offset: int = Query(default=0, ge=0), + limit: int = Query(default=50, ge=1, le=200), +) -> UserListResponse: + paginated = unwrap_or_raise( + get_paginated(session, User, offset, limit, public_class=UserPublic) + ) + return UserListResponse(users=paginated.items, total=paginated.total) + + +@router.post("/ingest", response_model=UserIngestionResponse) +def trigger_user_ingestion( + session: Session = Depends(get_session), + client: httpx.Client = Depends(get_http_client), +) -> UserIngestionResponse: + users = unwrap_or_raise( + ingest_users(session, client, settings.iam_url), + err_status=502, + ) + session.commit() + return UserIngestionResponse(users_ingested=len(users)) + + +@router.get("/{user_id}", response_model=UserDetail) +def get_user(user_id: int, session: Session = Depends(get_session)) -> UserDetail: + result = get_by_id(session, User, user_id) + user = unwrap_optional_or_raise(result, not_found_detail="User not found") + return _to_user_detail(user) diff --git a/verdict-backend/app/schemas/config.py b/verdict-backend/app/schemas/config.py new file mode 100644 index 0000000..d61a516 --- /dev/null +++ b/verdict-backend/app/schemas/config.py @@ -0,0 +1,15 @@ +from pydantic import BaseModel, Field + +from app.models.user import _ROLE_NAME_PATTERN + + +class PermissionConfig(BaseModel): + resource: str + subresource: str | None = None + action: str + + +class RoleConfig(BaseModel): + name: str = Field(pattern=_ROLE_NAME_PATTERN) + gold_source_id: str + permissions: list[PermissionConfig] diff --git a/verdict-backend/app/schemas/external/iam.py b/verdict-backend/app/schemas/external/iam.py new file mode 100644 index 0000000..1ab3bc7 --- /dev/null +++ b/verdict-backend/app/schemas/external/iam.py @@ -0,0 +1,14 @@ +from pydantic import BaseModel + + +class IAMUserIndexItem(BaseModel): + id: str + username: str + email: str + + +class IAMUserDetail(BaseModel): + id: str + username: str + email: str + groups: list[str] diff --git a/verdict-backend/app/schemas/ingestion.py b/verdict-backend/app/schemas/ingestion.py index 450de33..86a81b3 100644 --- a/verdict-backend/app/schemas/ingestion.py +++ b/verdict-backend/app/schemas/ingestion.py @@ -4,3 +4,7 @@ class FullIngestionResponse(BaseModel): assets_ingested: int systems_ingested: int + + +class UserIngestionResponse(BaseModel): + users_ingested: int diff --git a/verdict-backend/app/schemas/role.py b/verdict-backend/app/schemas/role.py new file mode 100644 index 0000000..e1abe64 --- /dev/null +++ b/verdict-backend/app/schemas/role.py @@ -0,0 +1,12 @@ +from pydantic import BaseModel + +from app.models.user import PermissionPublic, RolePublic + + +class RoleListResponse(BaseModel): + roles: list[RolePublic] + total: int + + +class RoleDetail(RolePublic): + permissions: list[PermissionPublic] diff --git a/verdict-backend/app/schemas/sync.py b/verdict-backend/app/schemas/sync.py new file mode 100644 index 0000000..de145c2 --- /dev/null +++ b/verdict-backend/app/schemas/sync.py @@ -0,0 +1,6 @@ +from pydantic import BaseModel + + +class SyncResponse(BaseModel): + roles_synced: int + permissions_synced: int diff --git a/verdict-backend/app/schemas/user.py b/verdict-backend/app/schemas/user.py new file mode 100644 index 0000000..d857824 --- /dev/null +++ b/verdict-backend/app/schemas/user.py @@ -0,0 +1,12 @@ +from pydantic import BaseModel + +from app.models.user import UserPublic + + +class UserListResponse(BaseModel): + users: list[UserPublic] + total: int + + +class UserDetail(UserPublic): + roles: list[str] diff --git a/verdict-backend/app/services/asset_ingestion.py b/verdict-backend/app/services/asset_ingestion.py index 56d72c4..69664eb 100644 --- a/verdict-backend/app/services/asset_ingestion.py +++ b/verdict-backend/app/services/asset_ingestion.py @@ -1,43 +1,29 @@ -import logging from urllib.parse import quote import httpx from pydantic import TypeAdapter -from pydantic import ValidationError as PydanticValidationError from sqlalchemy.exc import OperationalError from sqlmodel import Session -from app.errors import DBError, FetchError, IngestionError, ValidationError, db_error_from +from app.errors import DBError, FetchError, IngestionError, RemoteValidationError, db_error_from +from app.http import fetch_json from app.models.asset import Asset, AssetCreate, AssetPublic -from app.models.gold_source import GoldSourceMixin, GoldSourceType -from app.result import Err, Ok, Result, Some +from app.models.gold_source import GoldSourceType +from app.queries import upsert_by_gold_source +from app.result import Err, Ok, Result from app.schemas.external.asset_inventory import AssetDetail, AssetIndexItem -logger = logging.getLogger(__name__) - _index_adapter = TypeAdapter(list[AssetIndexItem]) _detail_adapter = TypeAdapter(AssetDetail) -type SourceError = FetchError | ValidationError +type SourceError = FetchError | RemoteValidationError def fetch_index( client: httpx.Client, url: str, ) -> Result[list[AssetIndexItem], SourceError]: - try: - response = client.get(url) - response.raise_for_status() - except httpx.HTTPError as e: - return Err(FetchError(url=url, raw=str(e))) - - try: - items = _index_adapter.validate_json(response.content) - except PydanticValidationError as e: - logger.error("Schema mismatch from asset inventory index: %s", e) - return Err(ValidationError(raw=str(e))) - - return Ok(items) + return fetch_json(client, url, _index_adapter) def fetch_detail( @@ -45,20 +31,7 @@ def fetch_detail( url: str, item_id: str, ) -> Result[AssetDetail, SourceError]: - detail_url = f"{url}/{quote(item_id, safe='')}" - try: - response = client.get(detail_url) - response.raise_for_status() - except httpx.HTTPError as e: - return Err(FetchError(url=detail_url, raw=str(e))) - - try: - detail = _detail_adapter.validate_json(response.content) - except PydanticValidationError as e: - logger.error("Schema mismatch from asset inventory detail %s: %s", item_id, e) - return Err(ValidationError(raw=str(e))) - - return Ok(detail) + return fetch_json(client, f"{url}/{quote(item_id, safe='')}", _detail_adapter) def to_asset(detail: AssetDetail) -> AssetCreate: @@ -103,31 +76,20 @@ def _upsert_asset( session: Session, asset_create: AssetCreate, ) -> Result[AssetPublic, DBError]: - result = GoldSourceMixin.get_by_gold_source( + def on_existing(existing: Asset) -> None: + existing.name = asset_create.name + existing.description = asset_create.description + existing.tags = asset_create.tags + + def make_new() -> Asset: + return Asset.model_validate(asset_create, from_attributes=True) + + return upsert_by_gold_source( session, Asset, asset_create.gold_source_type, asset_create.gold_source_id, + public_class=AssetPublic, + on_existing=on_existing, + make_new=make_new, ) - if isinstance(result, Err): - return Err(result.value) - - match result.value: - case Some(existing): - existing.name = asset_create.name - existing.description = asset_create.description - existing.tags = asset_create.tags - session.add(existing) - try: - session.flush() - except OperationalError as e: - return Err(db_error_from(e)) - return Ok(AssetPublic.model_validate(existing, from_attributes=True)) - case _: - new = Asset.model_validate(asset_create, from_attributes=True) - session.add(new) - try: - session.flush() - except OperationalError as e: - return Err(db_error_from(e)) - return Ok(AssetPublic.model_validate(new, from_attributes=True)) diff --git a/verdict-backend/app/services/cmdb_ingestion.py b/verdict-backend/app/services/cmdb_ingestion.py index 1aa7151..fad6a97 100644 --- a/verdict-backend/app/services/cmdb_ingestion.py +++ b/verdict-backend/app/services/cmdb_ingestion.py @@ -1,45 +1,37 @@ -import logging from urllib.parse import quote import httpx -import sqlalchemy as sa from pydantic import TypeAdapter -from pydantic import ValidationError as PydanticValidationError from sqlalchemy.exc import OperationalError from sqlmodel import Session -from app.errors import DBError, FetchError, IngestionError, ValidationError, db_error_from +from app.errors import ( + DBError, + FetchError, + IngestionError, + RemoteValidationError, + ValidationError, + db_error_from, +) +from app.http import fetch_json from app.models.asset import Asset, AssetPublic from app.models.gold_source import GoldSourceMixin, GoldSourceType from app.models.system import System, SystemCreate, SystemPublic, asset_system +from app.queries import sync_join_table, upsert_by_gold_source from app.result import Err, Nothing, Ok, Result, Some from app.schemas.external.cmdb import SystemDetail, SystemIndexItem -logger = logging.getLogger(__name__) - _index_adapter = TypeAdapter(list[SystemIndexItem]) _detail_adapter = TypeAdapter(SystemDetail) -type SourceError = FetchError | ValidationError +type SourceError = FetchError | RemoteValidationError def fetch_index( client: httpx.Client, url: str, ) -> Result[list[SystemIndexItem], SourceError]: - try: - response = client.get(url) - response.raise_for_status() - except httpx.HTTPError as e: - return Err(FetchError(url=url, raw=str(e))) - - try: - items = _index_adapter.validate_json(response.content) - except PydanticValidationError as e: - logger.error("Schema mismatch from CMDB index: %s", e) - return Err(ValidationError(raw=str(e))) - - return Ok(items) + return fetch_json(client, url, _index_adapter) def fetch_detail( @@ -47,20 +39,7 @@ def fetch_detail( url: str, item_id: str, ) -> Result[SystemDetail, SourceError]: - detail_url = f"{url}/{quote(item_id, safe='')}" - try: - response = client.get(detail_url) - response.raise_for_status() - except httpx.HTTPError as e: - return Err(FetchError(url=detail_url, raw=str(e))) - - try: - detail = _detail_adapter.validate_json(response.content) - except PydanticValidationError as e: - logger.error("Schema mismatch from CMDB detail %s: %s", item_id, e) - return Err(ValidationError(raw=str(e))) - - return Ok(detail) + return fetch_json(client, f"{url}/{quote(item_id, safe='')}", _detail_adapter) def to_system(detail: SystemDetail) -> SystemCreate: @@ -103,33 +82,22 @@ def _upsert_system( session: Session, system_create: SystemCreate, ) -> Result[SystemPublic, DBError]: - result = GoldSourceMixin.get_by_gold_source( + def on_existing(existing: System) -> None: + existing.primary_fqdn = system_create.primary_fqdn + existing.tags = system_create.tags + + def make_new() -> System: + return System.model_validate(system_create, from_attributes=True) + + return upsert_by_gold_source( session, System, system_create.gold_source_type, system_create.gold_source_id, + public_class=SystemPublic, + on_existing=on_existing, + make_new=make_new, ) - if isinstance(result, Err): - return Err(result.value) - - match result.value: - case Some(existing): - existing.primary_fqdn = system_create.primary_fqdn - existing.tags = system_create.tags - session.add(existing) - try: - session.flush() - except OperationalError as e: - return Err(db_error_from(e)) - return Ok(SystemPublic.model_validate(existing, from_attributes=True)) - case _: - new = System.model_validate(system_create, from_attributes=True) - session.add(new) - try: - session.flush() - except OperationalError as e: - return Err(db_error_from(e)) - return Ok(SystemPublic.model_validate(new, from_attributes=True)) def _sync_asset_links( @@ -137,28 +105,9 @@ def _sync_asset_links( system_id: int, desired_asset_ids: list[int], ) -> Result[None, DBError]: - try: - existing_rows = session.execute( - sa.select(asset_system.c.asset_id).where(asset_system.c.system_id == system_id) - ).all() - existing_ids = {row.asset_id for row in existing_rows} - desired_ids = set(desired_asset_ids) - - to_remove = existing_ids - desired_ids - to_add = desired_ids - existing_ids - - if to_remove: - session.execute( - sa.delete(asset_system).where( - asset_system.c.system_id == system_id, - asset_system.c.asset_id.in_(to_remove), - ) - ) - for aid in to_add: - session.execute(sa.insert(asset_system).values(system_id=system_id, asset_id=aid)) - except OperationalError as e: - return Err(db_error_from(e)) - return Ok(None) + return sync_join_table( + session, asset_system.c.system_id, system_id, asset_system.c.asset_id, desired_asset_ids + ) def ingest_systems( diff --git a/verdict-backend/app/services/config_sync.py b/verdict-backend/app/services/config_sync.py new file mode 100644 index 0000000..f4f2a6a --- /dev/null +++ b/verdict-backend/app/services/config_sync.py @@ -0,0 +1,139 @@ +import logging +from pathlib import Path + +import yaml +from pydantic import ValidationError as PydanticValidationError +from sqlalchemy.exc import OperationalError +from sqlmodel import Session, select + +from app.errors import ConfigError, ConfigSyncError, DBError, ValidationError, db_error_from +from app.models.gold_source import GoldSourceType +from app.models.user import Permission, PermissionPublic, Role, RolePermission, RolePublic +from app.queries import sync_join_table, upsert_by_gold_source +from app.result import Err, Ok, Result +from app.schemas.config import PermissionConfig, RoleConfig +from app.schemas.sync import SyncResponse + +logger = logging.getLogger(__name__) + + +def load_role_configs(basedir: Path) -> Result[list[RoleConfig], ConfigSyncError]: + roles_dir = basedir / "roles" + if not roles_dir.is_dir(): + return Err(ConfigError(path=str(roles_dir), raw="roles directory not found")) + configs: list[RoleConfig] = [] + for path in sorted(roles_dir.glob("*.yaml")): + try: + raw = path.read_text() + data = yaml.safe_load(raw) + except Exception as e: + return Err(ConfigError(path=str(path), raw=str(e))) + if data is None: + return Err(ConfigError(path=str(path), raw="empty yaml file")) + try: + configs.append(RoleConfig.model_validate(data)) + except PydanticValidationError as e: + return Err(ValidationError(raw=str(e))) + return Ok(configs) + + +def _upsert_permission( + session: Session, + cfg: PermissionConfig, +) -> Result[PermissionPublic, DBError]: + try: + stmt = select(Permission).where( + Permission.resource == cfg.resource, + Permission.subresource == cfg.subresource, + Permission.action == cfg.action, + ) + existing = session.exec(stmt).first() + except OperationalError as e: + return Err(db_error_from(e)) + + if existing is not None: + return Ok(PermissionPublic.model_validate(existing, from_attributes=True)) + + new = Permission( + resource=cfg.resource, + subresource=cfg.subresource, + action=cfg.action, + ) + session.add(new) + try: + session.flush() + except OperationalError as e: + return Err(db_error_from(e)) + return Ok(PermissionPublic.model_validate(new, from_attributes=True)) + + +def _upsert_role( + session: Session, + cfg: RoleConfig, +) -> Result[RolePublic, DBError]: + def on_existing(existing: Role) -> None: + existing.name = cfg.name + + def make_new() -> Role: + return Role( + name=cfg.name, + gold_source_type=GoldSourceType.IAM_GROUP, + gold_source_id=cfg.gold_source_id, + ) + + return upsert_by_gold_source( + session, + Role, + GoldSourceType.IAM_GROUP, + cfg.gold_source_id, + public_class=RolePublic, + on_existing=on_existing, + make_new=make_new, + ) + + +def _sync_role_permissions( + session: Session, + role_id: int, + permission_ids: list[int], +) -> Result[int, DBError]: + t = RolePermission.__table__ # type: ignore[attr-defined] # SQLModel does not expose __table__ in stubs but it exists on table=True models + result = sync_join_table(session, t.c.role_id, role_id, t.c.permission_id, permission_ids) + if isinstance(result, Err): + return Err(result.value) + return Ok(len(permission_ids)) + + +def sync_config( + session: Session, + basedir: Path, +) -> Result[SyncResponse, ConfigSyncError]: + configs_result = load_role_configs(basedir) + if isinstance(configs_result, Err): + return Err(configs_result.value) + + total_permissions = 0 + for cfg in configs_result.value: + role_result = _upsert_role(session, cfg) + if isinstance(role_result, Err): + return Err(role_result.value) + role = role_result.value + + permission_ids: list[int] = [] + for perm_cfg in cfg.permissions: + perm_result = _upsert_permission(session, perm_cfg) + if isinstance(perm_result, Err): + return Err(perm_result.value) + permission_ids.append(perm_result.value.id) + + sync_result = _sync_role_permissions(session, role.id, permission_ids) + if isinstance(sync_result, Err): + return Err(sync_result.value) + total_permissions += sync_result.value + + return Ok( + SyncResponse( + roles_synced=len(configs_result.value), + permissions_synced=total_permissions, + ) + ) diff --git a/verdict-backend/app/services/iam_ingestion.py b/verdict-backend/app/services/iam_ingestion.py new file mode 100644 index 0000000..317c7a5 --- /dev/null +++ b/verdict-backend/app/services/iam_ingestion.py @@ -0,0 +1,129 @@ +import logging +from urllib.parse import quote + +import httpx +from pydantic import TypeAdapter +from sqlalchemy.exc import OperationalError +from sqlmodel import Session + +from app.errors import DBError, FetchError, IngestionError, RemoteValidationError, db_error_from +from app.http import fetch_json +from app.models.gold_source import GoldSourceMixin, GoldSourceType +from app.models.user import Role, RolePublic, User, UserPublic +from app.queries import sync_user_roles, upsert_by_gold_source +from app.result import Err, Nothing, Ok, Result, Some +from app.schemas.external.iam import IAMUserDetail, IAMUserIndexItem + +logger = logging.getLogger(__name__) + +_index_adapter = TypeAdapter(list[IAMUserIndexItem]) +_detail_adapter = TypeAdapter(IAMUserDetail) + +type SourceError = FetchError | RemoteValidationError + + +def fetch_index( + client: httpx.Client, + url: str, +) -> Result[list[IAMUserIndexItem], SourceError]: + return fetch_json(client, url, _index_adapter) + + +def fetch_detail( + client: httpx.Client, + url: str, + user_id: str, +) -> Result[IAMUserDetail, SourceError]: + return fetch_json(client, f"{url}/{quote(user_id, safe='')}", _detail_adapter) + + +def _upsert_user( + session: Session, + detail: IAMUserDetail, +) -> Result[UserPublic, DBError]: + def on_existing(existing: User) -> None: + existing.username = detail.username + existing.email = detail.email + + def make_new() -> User: + return User( + username=detail.username, + email=detail.email, + is_active=True, + gold_source_type=GoldSourceType.IAM_USER, + gold_source_id=detail.id, + ) + + return upsert_by_gold_source( + session, + User, + GoldSourceType.IAM_USER, + detail.id, + public_class=UserPublic, + on_existing=on_existing, + make_new=make_new, + ) + + +def _resolve_role_ids( + session: Session, + group_ids: list[str], +) -> Result[list[int], DBError]: + """Map IAM group IDs to local Role IDs. Skips groups with no matching role (logs warning).""" + role_ids: list[int] = [] + for gid in group_ids: + result = GoldSourceMixin.get_by_gold_source( + session, + Role, + GoldSourceType.IAM_GROUP, + gid, + public_class=RolePublic, + ) + if isinstance(result, Err): + return Err(result.value) + match result.value: + case Some(role): + role_ids.append(role.id) + case Nothing(): + logger.warning("IAM group %s has no corresponding Role; skipping", gid) + return Ok(role_ids) + + +def ingest_users( + session: Session, + client: httpx.Client, + url: str, +) -> Result[list[UserPublic], IngestionError]: + index_result = fetch_index(client, url) + if isinstance(index_result, Err): + return Err(index_result.value) + + details: list[IAMUserDetail] = [] + for index_item in index_result.value: + detail_result = fetch_detail(client, url, index_item.id) + if isinstance(detail_result, Err): + return Err(detail_result.value) + details.append(detail_result.value) + + users: list[UserPublic] = [] + for detail in details: + upsert_result = _upsert_user(session, detail) + if isinstance(upsert_result, Err): + return Err(upsert_result.value) + user_public = upsert_result.value + + role_ids_result = _resolve_role_ids(session, detail.groups) + if isinstance(role_ids_result, Err): + return Err(role_ids_result.value) + + sync_result = sync_user_roles(session, user_public.id, role_ids_result.value) + if isinstance(sync_result, Err): + return Err(sync_result.value) + + users.append(user_public) + + try: + session.flush() + except OperationalError as e: + return Err(db_error_from(e)) + return Ok(users) diff --git a/verdict-backend/devenv.nix b/verdict-backend/devenv.nix index 101b6ba..23f55e5 100644 --- a/verdict-backend/devenv.nix +++ b/verdict-backend/devenv.nix @@ -106,6 +106,8 @@ in DATABASE_URL=postgresql://${postgres_user}:${postgres_password}@${postgres_host}:${toString postgres_port}/${database_name} ASSET_INVENTORY_URL=http://localhost:4010/assets CMDB_URL=http://localhost:4011/systems + IAM_URL=http://localhost:4012/users + CONFIG_BASEDIR=../config EOF echo "Generated .env.sample" ''; @@ -143,10 +145,11 @@ in # Cleanup background processes on any exit ASSET_MOCK_PID= CMDB_MOCK_PID= + IAM_MOCK_PID= APP_PID= cleanup() { - kill $APP_PID $ASSET_MOCK_PID $CMDB_MOCK_PID 2>/dev/null || true - wait $APP_PID $ASSET_MOCK_PID $CMDB_MOCK_PID 2>/dev/null || true + kill $APP_PID $ASSET_MOCK_PID $CMDB_MOCK_PID $IAM_MOCK_PID 2>/dev/null || true + wait $APP_PID $ASSET_MOCK_PID $CMDB_MOCK_PID $IAM_MOCK_PID 2>/dev/null || true } trap cleanup EXIT INT TERM @@ -159,6 +162,10 @@ in (cd $MOCK_SERVICES_DIR && exec uv run uvicorn cmdb.app:app --host 0.0.0.0 --port 4011) & CMDB_MOCK_PID=$! + echo "Starting mock IAM..." + (cd $MOCK_SERVICES_DIR && exec uv run uvicorn iam.app:app --host 0.0.0.0 --port 4012) & + IAM_MOCK_PID=$! + retries=0 until curl -sf http://localhost:4010/assets > /dev/null 2>&1; do retries=$((retries + 1)) @@ -181,11 +188,24 @@ in done echo "CMDB mock ready" + retries=0 + until curl -sf http://localhost:4012/users > /dev/null 2>&1; do + retries=$((retries + 1)) + if [ $retries -ge 30 ]; then + echo "ERROR: IAM mock failed to start" + exit 1 + fi + sleep 1 + done + echo "IAM mock ready" + # Start verdict app against test DB echo "Starting verdict app..." (DATABASE_NAME=${database_name}_test \ ASSET_INVENTORY_URL=http://localhost:4010/assets \ CMDB_URL=http://localhost:4011/systems \ + IAM_URL=http://localhost:4012/users \ + CONFIG_BASEDIR=../config \ exec uv run uvicorn app.main:app --host 0.0.0.0 --port 8000) & APP_PID=$! diff --git a/verdict-backend/justfile b/verdict-backend/justfile index f97591d..edcb4eb 100644 --- a/verdict-backend/justfile +++ b/verdict-backend/justfile @@ -80,6 +80,14 @@ db-info: echo " DATABASE_USER: $DATABASE_USER" echo " Test DB: $DATABASE_NAME_TEST" +# Regenerate JSON Schema for config/roles/*.yaml from the RoleConfig pydantic model +gen-config-schema: + uv run python -c "from app.schemas.config import RoleConfig; import json; print(json.dumps(RoleConfig.model_json_schema(), indent=2))" > ../config/roles/schema.json + +# Create a local user (--role flags are optional and repeatable) +create-user username email *ROLES: + uv run python -m app.cli --username {{username}} --email {{email}} {{ROLES}} + # Set up the full development environment setup: #!/usr/bin/env bash diff --git a/verdict-backend/pyproject.toml b/verdict-backend/pyproject.toml index e60c436..fc6e8b8 100644 --- a/verdict-backend/pyproject.toml +++ b/verdict-backend/pyproject.toml @@ -19,6 +19,8 @@ dependencies = [ # Configuration "pydantic-settings>=2.0.0", "python-dotenv>=1.0.0", + "typer>=0.24.1", + "pyyaml>=6.0.3", ] [project.optional-dependencies] @@ -147,6 +149,7 @@ dev = [ {include-group = "test"}, "mypy>=1.19.1", "pyright>=1.1.408", + "types-pyyaml>=6.0.12.20250915", ] test = [ "pytest>=9.0.0", diff --git a/verdict-backend/tests/cli/__init__.py b/verdict-backend/tests/cli/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/verdict-backend/tests/cli/test_create_user.py b/verdict-backend/tests/cli/test_create_user.py new file mode 100644 index 0000000..241e53a --- /dev/null +++ b/verdict-backend/tests/cli/test_create_user.py @@ -0,0 +1,153 @@ +import pytest +import sqlalchemy as sa +from sqlmodel import Session, select +from typer.testing import CliRunner + +from app.cli import cli +from app.db import engine +from app.models.gold_source import GoldSourceType +from app.models.user import Role, User, UserRole + +runner = CliRunner() + +_CLI_USERNAMES = ("cli-user-1", "cli-user-2", "cli-user-3", "cli-user-4", "cli-user-5") +_CLI_ROLES = ("cli-analyst", "cli-viewer") + +_userrole_table = UserRole.__table__ # type: ignore[attr-defined] # SQLModel does not expose __table__ in stubs but it exists on table=True models + + +def _cleanup(session: Session) -> None: + """Remove all CLI test users and roles (order matters: links before entities).""" + for username in _CLI_USERNAMES: + u = session.exec( + select(User).where( + User.gold_source_type == GoldSourceType.LOCAL_USER, + User.gold_source_id == username, + ) + ).first() + if u: + session.execute(sa.delete(_userrole_table).where(_userrole_table.c.user_id == u.id)) + session.delete(u) + for role_name in _CLI_ROLES: + r = session.exec(select(Role).where(Role.name == role_name)).first() + if r: + session.execute(sa.delete(_userrole_table).where(_userrole_table.c.role_id == r.id)) + session.delete(r) + session.commit() + + +@pytest.fixture(scope="module", autouse=True) +def _seed_roles(): + """Ensure clean state, insert roles needed by CLI tests, clean up after.""" + # Clean up from any prior failed teardown before seeding. + with Session(engine) as session: + _cleanup(session) + + with Session(engine) as session: + session.add( + Role( + name="cli-analyst", + description="", + gold_source_type=GoldSourceType.IAM_GROUP, + gold_source_id="cli-gs-analyst", + ) + ) + session.add( + Role( + name="cli-viewer", + description="", + gold_source_type=GoldSourceType.IAM_GROUP, + gold_source_id="cli-gs-viewer", + ) + ) + session.commit() + yield + with Session(engine) as session: + _cleanup(session) + + +def test_create_user_succeeds(): + result = runner.invoke( + cli, + ["--username", "cli-user-1", "--email", "cli-user-1@example.com"], + ) + + assert result.exit_code == 0 + assert "User 'cli-user-1' ready." in result.output + + +def test_create_user_is_idempotent(): + runner.invoke( + cli, + ["--username", "cli-user-2", "--email", "cli-user-2@example.com"], + ) + result = runner.invoke( + cli, + ["--username", "cli-user-2", "--email", "cli-user-2@example.com"], + ) + + assert result.exit_code == 0 + assert "User 'cli-user-2' ready." in result.output + + +def test_create_user_with_roles(): + result = runner.invoke( + cli, + [ + "--username", + "cli-user-3", + "--email", + "cli-user-3@example.com", + "--role", + "cli-analyst", + "--role", + "cli-viewer", + ], + ) + + assert result.exit_code == 0 + assert "Assigned role 'cli-analyst'" in result.output + assert "Assigned role 'cli-viewer'" in result.output + + +def test_create_user_role_assignment_is_idempotent(): + runner.invoke( + cli, + [ + "--username", + "cli-user-4", + "--email", + "cli-user-4@example.com", + "--role", + "cli-analyst", + ], + ) + result = runner.invoke( + cli, + [ + "--username", + "cli-user-4", + "--email", + "cli-user-4@example.com", + "--role", + "cli-analyst", + ], + ) + + assert result.exit_code == 0 + + +def test_create_user_unknown_role_fails(): + result = runner.invoke( + cli, + [ + "--username", + "cli-user-5", + "--email", + "cli-user-5@example.com", + "--role", + "nonexistent", + ], + ) + + assert result.exit_code == 1 diff --git a/verdict-backend/tests/factories/__init__.py b/verdict-backend/tests/factories/__init__.py index 7e2ea59..d9ab31d 100644 --- a/verdict-backend/tests/factories/__init__.py +++ b/verdict-backend/tests/factories/__init__.py @@ -18,4 +18,4 @@ def set_session(cls, session: Session) -> None: """Bind all factories (including subclasses) to the given test session.""" cls._meta.sqlalchemy_session = session # type: ignore[attr-defined] # factory-boy stubs don't expose this for subclass in cls.__subclasses__(): - subclass._meta.sqlalchemy_session = session # type: ignore[attr-defined] + subclass._meta.sqlalchemy_session = session # type: ignore[attr-defined] # factory-boy stubs don't expose this diff --git a/verdict-backend/tests/factories/asset.py b/verdict-backend/tests/factories/asset.py index ddb8bcb..d4b86d1 100644 --- a/verdict-backend/tests/factories/asset.py +++ b/verdict-backend/tests/factories/asset.py @@ -7,7 +7,7 @@ class AssetFactory(BaseModelFactory): - class Meta: # type: ignore[override] + class Meta: # type: ignore[override] # factory-boy expects Meta override per subclass model = Asset name = factory.Iterator( diff --git a/verdict-backend/tests/factories/system.py b/verdict-backend/tests/factories/system.py index c18c7d7..912b4b9 100644 --- a/verdict-backend/tests/factories/system.py +++ b/verdict-backend/tests/factories/system.py @@ -7,7 +7,7 @@ class SystemFactory(BaseModelFactory): - class Meta: # type: ignore[override] + class Meta: # type: ignore[override] # factory-boy expects Meta override per subclass model = System primary_fqdn = factory.Sequence(lambda n: f"host-{n:04d}.prod.example.com") diff --git a/verdict-backend/tests/factories/user.py b/verdict-backend/tests/factories/user.py new file mode 100644 index 0000000..f52786f --- /dev/null +++ b/verdict-backend/tests/factories/user.py @@ -0,0 +1,52 @@ +# pyright: reportPrivateImportUsage=none +import factory + +from app.models.gold_source import GoldSourceType +from app.models.user import Permission, Role, RolePermission, User, UserRole +from tests.factories import BaseModelFactory + + +class UserFactory(BaseModelFactory): + class Meta: # type: ignore[override] # factory-boy expects Meta override per subclass + model = User + + username = factory.Sequence(lambda n: f"user-{n:04d}") + email = factory.Sequence(lambda n: f"user-{n:04d}@example.com") + is_active = True + gold_source_id = factory.Sequence(lambda n: f"iam-user-{n:04d}") + gold_source_type = GoldSourceType.IAM_USER + + +class RoleFactory(BaseModelFactory): + class Meta: # type: ignore[override] # factory-boy expects Meta override per subclass + model = Role + + name = factory.Sequence(lambda n: f"role-{n:04d}") + description = factory.Faker("sentence") + gold_source_id = factory.Sequence(lambda n: f"iam-group-{n:04d}") + gold_source_type = GoldSourceType.IAM_GROUP + + +class UserRoleFactory(BaseModelFactory): + class Meta: # type: ignore[override] # factory-boy expects Meta override per subclass + model = UserRole + + user_id = None + role_id = None + + +class RolePermissionFactory(BaseModelFactory): + class Meta: # type: ignore[override] # factory-boy expects Meta override per subclass + model = RolePermission + + role_id = None + permission_id = None + + +class PermissionFactory(BaseModelFactory): + class Meta: # type: ignore[override] # factory-boy expects Meta override per subclass + model = Permission + + resource = factory.Sequence(lambda n: f"resource-{n:04d}") + subresource = None + action = factory.Iterator(["read", "write", "delete", "admin"]) diff --git a/verdict-backend/tests/models/test_gold_source.py b/verdict-backend/tests/models/test_gold_source.py index cea0a65..d60e343 100644 --- a/verdict-backend/tests/models/test_gold_source.py +++ b/verdict-backend/tests/models/test_gold_source.py @@ -2,20 +2,23 @@ from sqlalchemy.exc import IntegrityError from app.models.base import GoldSourceMixin +from app.models.gold_source import GoldSourceType from app.result import Nothing, Ok, Some from tests.models.mixin_test_model import MixinTestModel +_TYPE = GoldSourceType.LOCAL_USER + def test_get_by_gold_source_returns_record(db_session): model = MixinTestModel( name="findable", - gold_source_type="test", + gold_source_type=_TYPE, gold_source_id="abc-123", ) db_session.add(model) db_session.flush() - result = GoldSourceMixin.get_by_gold_source(db_session, MixinTestModel, "test", "abc-123") + result = GoldSourceMixin.get_by_gold_source(db_session, MixinTestModel, _TYPE, "abc-123") assert isinstance(result, Ok) assert isinstance(result.value, Some) @@ -24,7 +27,7 @@ def test_get_by_gold_source_returns_record(db_session): def test_get_by_gold_source_returns_nothing(db_session): - result = GoldSourceMixin.get_by_gold_source(db_session, MixinTestModel, "test", "nonexistent") + result = GoldSourceMixin.get_by_gold_source(db_session, MixinTestModel, _TYPE, "nonexistent") assert isinstance(result, Ok) assert isinstance(result.value, Nothing) @@ -33,7 +36,7 @@ def test_get_by_gold_source_returns_nothing(db_session): def test_duplicate_gold_source_raises_integrity_error(db_session): model1 = MixinTestModel( name="first", - gold_source_type="test", + gold_source_type=_TYPE, gold_source_id="dup-1", ) db_session.add(model1) @@ -41,7 +44,7 @@ def test_duplicate_gold_source_raises_integrity_error(db_session): model2 = MixinTestModel( name="second", - gold_source_type="test", + gold_source_type=_TYPE, gold_source_id="dup-1", ) db_session.add(model2) diff --git a/verdict-backend/tests/models/test_tags.py b/verdict-backend/tests/models/test_tags.py index b3730d7..74c1f78 100644 --- a/verdict-backend/tests/models/test_tags.py +++ b/verdict-backend/tests/models/test_tags.py @@ -1,10 +1,11 @@ +from app.models.gold_source import GoldSourceType from tests.models.mixin_test_model import MixinTestModel def test_tags_round_trip(db_session): model = MixinTestModel( name="tagged", - gold_source_type="test", + gold_source_type=GoldSourceType.LOCAL_USER, gold_source_id="tags-1", tags=["os.linux.ubuntu-22.04", "role.database"], ) diff --git a/verdict-backend/tests/models/test_timestamp.py b/verdict-backend/tests/models/test_timestamp.py index 3f49647..b25dab2 100644 --- a/verdict-backend/tests/models/test_timestamp.py +++ b/verdict-backend/tests/models/test_timestamp.py @@ -1,13 +1,14 @@ import time from datetime import UTC, datetime +from app.models.gold_source import GoldSourceType from tests.models.mixin_test_model import MixinTestModel def test_created_at_auto_populated(db_session): model = MixinTestModel( name="test", - gold_source_type="test", + gold_source_type=GoldSourceType.LOCAL_USER, gold_source_id="created-at-1", ) db_session.add(model) @@ -21,7 +22,7 @@ def test_created_at_auto_populated(db_session): def test_updated_at_changes_on_update(db_session): model = MixinTestModel( name="original", - gold_source_type="test", + gold_source_type=GoldSourceType.LOCAL_USER, gold_source_id="updated-at-1", ) db_session.add(model) diff --git a/verdict-backend/tests/models/test_user.py b/verdict-backend/tests/models/test_user.py new file mode 100644 index 0000000..f8ac722 --- /dev/null +++ b/verdict-backend/tests/models/test_user.py @@ -0,0 +1,69 @@ +import pytest +from sqlalchemy.exc import IntegrityError + +from app.models.user import Permission, RolePermission, UserRole +from tests.factories.user import PermissionFactory, RoleFactory, UserFactory + + +def test_user_persists(db_session): + user = UserFactory.create() + db_session.refresh(user) + + assert user.id is not None + + +def test_role_persists(db_session): + role = RoleFactory.create() + db_session.refresh(role) + + assert role.id is not None + + +def test_permission_persists(db_session): + perm = PermissionFactory.create() + db_session.refresh(perm) + + assert perm.id is not None + + +def test_user_has_role(db_session): + user = UserFactory.create() + role = RoleFactory.create() + db_session.add(UserRole(user_id=user.id, role_id=role.id)) + db_session.flush() + db_session.refresh(user) + + assert role in user.roles + + +def test_role_has_permission(db_session): + role = RoleFactory.create() + perm = PermissionFactory.create() + db_session.add(RolePermission(role_id=role.id, permission_id=perm.id)) + db_session.flush() + db_session.refresh(role) + + assert perm in role.permissions + + +def test_user_permissions_traversal(db_session): + user = UserFactory.create() + role = RoleFactory.create() + perm = PermissionFactory.create() + db_session.add(UserRole(user_id=user.id, role_id=role.id)) + db_session.add(RolePermission(role_id=role.id, permission_id=perm.id)) + db_session.flush() + db_session.refresh(user) + + all_permissions = [p for r in user.roles for p in r.permissions] + + assert perm in all_permissions + + +def test_duplicate_permission_raises_integrity_error(db_session): + db_session.add(Permission(resource="assets", subresource="servers", action="read")) + db_session.flush() + + db_session.add(Permission(resource="assets", subresource="servers", action="read")) + with pytest.raises(IntegrityError): + db_session.flush() diff --git a/verdict-backend/tests/routes/test_roles.py b/verdict-backend/tests/routes/test_roles.py new file mode 100644 index 0000000..8aea7c8 --- /dev/null +++ b/verdict-backend/tests/routes/test_roles.py @@ -0,0 +1,60 @@ +from httpx import AsyncClient + +from tests.factories.user import PermissionFactory, RoleFactory, RolePermissionFactory + + +async def test_list_roles(app_client: AsyncClient, db_session): + RoleFactory.create() + RoleFactory.create() + + response = await app_client.get("/roles/") + + assert response.status_code == 200 + data = response.json() + assert len(data["roles"]) == 2 + assert data["total"] == 2 + + +async def test_get_role_by_id(app_client: AsyncClient, db_session): + role = RoleFactory.create() + + response = await app_client.get(f"/roles/{role.id}") + + assert response.status_code == 200 + data = response.json() + assert data["name"] == role.name + + +async def test_get_role_returns_permissions(app_client: AsyncClient, db_session): + role = RoleFactory.create() + permission = PermissionFactory.create() + RolePermissionFactory.create(role_id=role.id, permission_id=permission.id) + + response = await app_client.get(f"/roles/{role.id}") + + assert response.status_code == 200 + data = response.json() + assert len(data["permissions"]) == 1 + assert data["permissions"][0]["resource"] == permission.resource + + +async def test_get_role_by_name(app_client: AsyncClient, db_session): + role = RoleFactory.create() + + response = await app_client.get(f"/roles/by-name/{role.name}") + + assert response.status_code == 200 + data = response.json() + assert data["name"] == role.name + + +async def test_get_role_by_name_not_found(app_client: AsyncClient, db_session): + response = await app_client.get("/roles/by-name/nonexistent") + + assert response.status_code == 404 + + +async def test_get_role_not_found(app_client: AsyncClient, db_session): + response = await app_client.get("/roles/99999") + + assert response.status_code == 404 diff --git a/verdict-backend/tests/routes/test_sync.py b/verdict-backend/tests/routes/test_sync.py new file mode 100644 index 0000000..7fd9c24 --- /dev/null +++ b/verdict-backend/tests/routes/test_sync.py @@ -0,0 +1,60 @@ +from pathlib import Path +from textwrap import dedent + +import pytest +from httpx import AsyncClient +from sqlmodel import Session + +from app.config import settings +from app.queries import get_role_by_name +from app.result import Ok, Some + + +@pytest.fixture +def config_dir(tmp_path: Path) -> Path: + roles_dir = tmp_path / "roles" + roles_dir.mkdir() + (roles_dir / "engineers.yaml").write_text( + dedent("""\ + name: engineers + gold_source_id: GRP-001 + permissions: + - resource: assets + action: read + """) + ) + return tmp_path + + +async def test_post_sync_returns_200( + app_client: AsyncClient, db_session, config_dir: Path, monkeypatch +): + monkeypatch.setattr(settings, "config_basedir", str(config_dir)) + + response = await app_client.post("/sync") + + assert response.status_code == 200 + data = response.json() + assert data["roles_synced"] == 1 + assert data["permissions_synced"] == 1 + + +async def test_post_sync_persists_role( + app_client: AsyncClient, db_session: Session, config_dir: Path, monkeypatch +): + monkeypatch.setattr(settings, "config_basedir", str(config_dir)) + + await app_client.post("/sync") + + result = get_role_by_name(db_session, "engineers") + assert isinstance(result, Ok) and isinstance(result.value, Some) + + +async def test_post_sync_invalid_config_dir_returns_500( + app_client: AsyncClient, db_session, tmp_path: Path, monkeypatch +): + monkeypatch.setattr(settings, "config_basedir", str(tmp_path / "nonexistent")) + + response = await app_client.post("/sync") + + assert response.status_code == 500 diff --git a/verdict-backend/tests/routes/test_users.py b/verdict-backend/tests/routes/test_users.py new file mode 100644 index 0000000..29a41c6 --- /dev/null +++ b/verdict-backend/tests/routes/test_users.py @@ -0,0 +1,56 @@ +from httpx import AsyncClient + +from tests.factories.user import RoleFactory, UserFactory, UserRoleFactory + + +async def test_list_users(app_client: AsyncClient, db_session): + UserFactory.create() + UserFactory.create() + + response = await app_client.get("/users/") + + assert response.status_code == 200 + data = response.json() + assert len(data["users"]) == 2 + assert data["total"] == 2 + + +async def test_list_users_pagination(app_client: AsyncClient, db_session): + UserFactory.create() + UserFactory.create() + UserFactory.create() + + response = await app_client.get("/users/?offset=1&limit=1") + + assert response.status_code == 200 + data = response.json() + assert len(data["users"]) == 1 + assert data["total"] == 3 + + +async def test_get_user_by_id(app_client: AsyncClient, db_session): + user = UserFactory.create() + + response = await app_client.get(f"/users/{user.id}") + + assert response.status_code == 200 + data = response.json() + assert data["username"] == user.username + + +async def test_get_user_returns_roles(app_client: AsyncClient, db_session): + user = UserFactory.create() + role = RoleFactory.create() + UserRoleFactory.create(user_id=user.id, role_id=role.id) + + response = await app_client.get(f"/users/{user.id}") + + assert response.status_code == 200 + data = response.json() + assert data["roles"] == [role.name] + + +async def test_get_user_not_found(app_client: AsyncClient, db_session): + response = await app_client.get("/users/99999") + + assert response.status_code == 404 diff --git a/verdict-backend/tests/services/test_config_sync.py b/verdict-backend/tests/services/test_config_sync.py new file mode 100644 index 0000000..7413a08 --- /dev/null +++ b/verdict-backend/tests/services/test_config_sync.py @@ -0,0 +1,186 @@ +from pathlib import Path +from textwrap import dedent + +from sqlmodel import Session, select + +from app.errors import ConfigError, ValidationError +from app.models.gold_source import GoldSourceType +from app.models.user import Permission, Role +from app.result import Err, Ok +from app.schemas.config import PermissionConfig, RoleConfig +from app.services.config_sync import ( + _upsert_permission, + _upsert_role, + load_role_configs, + sync_config, +) + +# --- load_role_configs --- + + +def test_load_role_configs_returns_roles(tmp_path): + roles_dir = tmp_path / "roles" + roles_dir.mkdir() + (roles_dir / "engineers.yaml").write_text( + dedent("""\ + name: engineers + gold_source_id: GRP-001 + permissions: + - resource: assets + action: read + """) + ) + + result = load_role_configs(tmp_path) + + assert isinstance(result, Ok) + assert len(result.value) == 1 + assert result.value[0].name == "engineers" + assert result.value[0].gold_source_id == "GRP-001" + assert len(result.value[0].permissions) == 1 + + +def test_load_role_configs_invalid_yaml_returns_config_error(tmp_path): + roles_dir = tmp_path / "roles" + roles_dir.mkdir() + (roles_dir / "bad.yaml").write_text("name: [unclosed bracket\n") + + result = load_role_configs(tmp_path) + + assert isinstance(result, Err) + assert isinstance(result.value, ConfigError) + + +def test_load_role_configs_invalid_schema_returns_validation_error(tmp_path): + roles_dir = tmp_path / "roles" + roles_dir.mkdir() + (roles_dir / "bad.yaml").write_text( + dedent("""\ + name: engineers + # missing gold_source_id and permissions + """) + ) + + result = load_role_configs(tmp_path) + + assert isinstance(result, Err) + assert isinstance(result.value, ValidationError) + + +def test_load_role_configs_empty_dir_returns_empty_list(tmp_path): + (tmp_path / "roles").mkdir() + + result = load_role_configs(tmp_path) + + assert isinstance(result, Ok) + assert result.value == [] + + +# --- _upsert_permission --- + + +def test_upsert_permission_creates_new(db_session: Session): + cfg = PermissionConfig(resource="assets", action="read") + + result = _upsert_permission(db_session, cfg) + + assert isinstance(result, Ok) + assert result.value.resource == "assets" + assert result.value.action == "read" + assert result.value.id is not None + + +def test_upsert_permission_returns_existing(db_session: Session): + cfg = PermissionConfig(resource="assets", action="read") + first = _upsert_permission(db_session, cfg) + assert isinstance(first, Ok) + + second = _upsert_permission(db_session, cfg) + + assert isinstance(second, Ok) + assert second.value.id == first.value.id + + +# --- _upsert_role --- + + +def test_upsert_role_creates_new(db_session: Session): + cfg = RoleConfig(name="engineers", gold_source_id="GRP-001", permissions=[]) + + result = _upsert_role(db_session, cfg) + + assert isinstance(result, Ok) + assert result.value.name == "engineers" + assert result.value.gold_source_id == "GRP-001" + assert result.value.gold_source_type == GoldSourceType.IAM_GROUP + + +def test_upsert_role_updates_existing_name(db_session: Session): + cfg = RoleConfig(name="engineers", gold_source_id="GRP-001", permissions=[]) + _upsert_role(db_session, cfg) + + updated_cfg = RoleConfig(name="senior-engineers", gold_source_id="GRP-001", permissions=[]) + result = _upsert_role(db_session, updated_cfg) + + assert isinstance(result, Ok) + assert result.value.name == "senior-engineers" + roles = db_session.exec(select(Role).where(Role.gold_source_id == "GRP-001")).all() + assert len(roles) == 1 + + +# --- sync_config --- + + +def test_sync_config_upserts_roles_and_permissions(db_session: Session, tmp_path: Path): + roles_dir = tmp_path / "roles" + roles_dir.mkdir() + (roles_dir / "engineers.yaml").write_text( + dedent("""\ + name: engineers + gold_source_id: GRP-001 + permissions: + - resource: assets + action: read + - resource: systems + action: read + """) + ) + + result = sync_config(db_session, tmp_path) + + assert isinstance(result, Ok) + assert result.value.roles_synced == 1 + assert result.value.permissions_synced == 2 + + roles = db_session.exec(select(Role)).all() + assert len(roles) == 1 + assert roles[0].name == "engineers" + + permissions = db_session.exec(select(Permission)).all() + assert len(permissions) == 2 + + +def test_sync_config_idempotent(db_session: Session, tmp_path: Path): + roles_dir = tmp_path / "roles" + roles_dir.mkdir() + (roles_dir / "engineers.yaml").write_text( + dedent("""\ + name: engineers + gold_source_id: GRP-001 + permissions: + - resource: assets + action: read + """) + ) + + sync_config(db_session, tmp_path) + result = sync_config(db_session, tmp_path) + + assert isinstance(result, Ok) + assert result.value.roles_synced == 1 + assert result.value.permissions_synced == 1 + + roles = db_session.exec(select(Role)).all() + assert len(roles) == 1 + permissions = db_session.exec(select(Permission)).all() + assert len(permissions) == 1 diff --git a/verdict-backend/uv.lock b/verdict-backend/uv.lock index 43e3bba..27a90f7 100644 --- a/verdict-backend/uv.lock +++ b/verdict-backend/uv.lock @@ -294,6 +294,18 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/87/fb/99f81ac72ae23375f22b7afdb7642aba97c00a713c217124420147681a2f/mako-1.3.10-py3-none-any.whl", hash = "sha256:baef24a52fc4fc514a0887ac600f9f1cff3d82c61d4d700a1fa84d597b88db59", size = 78509, upload-time = "2025-04-10T12:50:53.297Z" }, ] +[[package]] +name = "markdown-it-py" +version = "4.0.0" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "mdurl" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/5b/f5/4ec618ed16cc4f8fb3b701563655a69816155e79e24a17b651541804721d/markdown_it_py-4.0.0.tar.gz", hash = "sha256:cb0a2b4aa34f932c007117b194e945bd74e0ec24133ceb5bac59009cda1cb9f3", size = 73070, upload-time = "2025-08-11T12:57:52.854Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/94/54/e7d793b573f298e1c9013b8c4dade17d481164aa517d1d7148619c2cedbf/markdown_it_py-4.0.0-py3-none-any.whl", hash = "sha256:87327c59b172c5011896038353a81343b6754500a08cd7a4973bb48c6d578147", size = 87321, upload-time = "2025-08-11T12:57:51.923Z" }, +] + [[package]] name = "markupsafe" version = "3.0.3" @@ -324,6 +336,15 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/70/bc/6f1c2f612465f5fa89b95bead1f44dcb607670fd42891d8fdcd5d039f4f4/markupsafe-3.0.3-cp314-cp314t-win_arm64.whl", hash = "sha256:32001d6a8fc98c8cb5c947787c5d08b0a50663d139f1305bac5885d98d9b40fa", size = 14146, upload-time = "2025-09-27T18:37:28.327Z" }, ] +[[package]] +name = "mdurl" +version = "0.1.2" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/d6/54/cfe61301667036ec958cb99bd3efefba235e65cdeb9c84d24a8293ba1d90/mdurl-0.1.2.tar.gz", hash = "sha256:bb413d29f5eea38f31dd4754dd7377d4465116fb207585f97bf925588687c1ba", size = 8729, upload-time = "2022-08-14T12:40:10.846Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/b3/38/89ba8ad64ae25be8de66a6d463314cf1eb366222074cfda9ee839c56a4b4/mdurl-0.1.2-py3-none-any.whl", hash = "sha256:84008a41e51615a49fc9966191ff91509e3c40b939176e643fd50a5c2196b8f8", size = 9979, upload-time = "2022-08-14T12:40:09.779Z" }, +] + [[package]] name = "mypy" version = "1.19.1" @@ -597,6 +618,28 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/8e/67/afbb0978d5399bc9ea200f1d4489a23c9a1dad4eee6376242b8182389c79/respx-0.22.0-py2.py3-none-any.whl", hash = "sha256:631128d4c9aba15e56903fb5f66fb1eff412ce28dd387ca3a81339e52dbd3ad0", size = 25127, upload-time = "2024-12-19T22:33:57.837Z" }, ] +[[package]] +name = "rich" +version = "14.3.3" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "markdown-it-py" }, + { name = "pygments" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/b3/c6/f3b320c27991c46f43ee9d856302c70dc2d0fb2dba4842ff739d5f46b393/rich-14.3.3.tar.gz", hash = "sha256:b8daa0b9e4eef54dd8cf7c86c03713f53241884e814f4e2f5fb342fe520f639b", size = 230582, upload-time = "2026-02-19T17:23:12.474Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/14/25/b208c5683343959b670dc001595f2f3737e051da617f66c31f7c4fa93abc/rich-14.3.3-py3-none-any.whl", hash = "sha256:793431c1f8619afa7d3b52b2cdec859562b950ea0d4b6b505397612db8d5362d", size = 310458, upload-time = "2026-02-19T17:23:13.732Z" }, +] + +[[package]] +name = "shellingham" +version = "1.5.4" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/58/15/8b3609fd3830ef7b27b655beb4b4e9c62313a4e8da8c676e142cc210d58e/shellingham-1.5.4.tar.gz", hash = "sha256:8dbca0739d487e5bd35ab3ca4b36e11c4078f3a234bfce294b0a0291363404de", size = 10310, upload-time = "2023-10-24T04:13:40.426Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/e0/f9/0595336914c5619e5f28a1fb793285925a8cd4b432c9da0a987836c7f822/shellingham-1.5.4-py2.py3-none-any.whl", hash = "sha256:7ecfff8f2fd72616f7481040475a65b2bf8af90a56c89140852d1120324e8686", size = 9755, upload-time = "2023-10-24T04:13:38.866Z" }, +] + [[package]] name = "sqlalchemy" version = "2.0.46" @@ -658,6 +701,30 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/81/0d/13d1d239a25cbfb19e740db83143e95c772a1fe10202dda4b76792b114dd/starlette-0.52.1-py3-none-any.whl", hash = "sha256:0029d43eb3d273bc4f83a08720b4912ea4b071087a3b48db01b7c839f7954d74", size = 74272, upload-time = "2026-01-18T13:34:09.188Z" }, ] +[[package]] +name = "typer" +version = "0.24.1" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "annotated-doc" }, + { name = "click" }, + { name = "rich" }, + { name = "shellingham" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/f5/24/cb09efec5cc954f7f9b930bf8279447d24618bb6758d4f6adf2574c41780/typer-0.24.1.tar.gz", hash = "sha256:e39b4732d65fbdcde189ae76cf7cd48aeae72919dea1fdfc16593be016256b45", size = 118613, upload-time = "2026-02-21T16:54:40.609Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/4a/91/48db081e7a63bb37284f9fbcefda7c44c277b18b0e13fbc36ea2335b71e6/typer-0.24.1-py3-none-any.whl", hash = "sha256:112c1f0ce578bfb4cab9ffdabc68f031416ebcc216536611ba21f04e9aa84c9e", size = 56085, upload-time = "2026-02-21T16:54:41.616Z" }, +] + +[[package]] +name = "types-pyyaml" +version = "6.0.12.20250915" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/7e/69/3c51b36d04da19b92f9e815be12753125bd8bc247ba0470a982e6979e71c/types_pyyaml-6.0.12.20250915.tar.gz", hash = "sha256:0f8b54a528c303f0e6f7165687dd33fafa81c807fcac23f632b63aa624ced1d3", size = 17522, upload-time = "2025-09-15T03:01:00.728Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/bd/e0/1eed384f02555dde685fff1a1ac805c1c7dcb6dd019c916fe659b1c1f9ec/types_pyyaml-6.0.12.20250915-py3-none-any.whl", hash = "sha256:e7d4d9e064e89a3b3cae120b4990cd370874d2bf12fa5f46c97018dd5d3c9ab6", size = 20338, upload-time = "2025-09-15T03:00:59.218Z" }, +] + [[package]] name = "typing-extensions" version = "4.15.0" @@ -743,8 +810,10 @@ dependencies = [ { name = "psycopg2-binary" }, { name = "pydantic-settings" }, { name = "python-dotenv" }, + { name = "pyyaml" }, { name = "sqlalchemy-json" }, { name = "sqlmodel" }, + { name = "typer" }, { name = "uvicorn", extra = ["standard"] }, ] @@ -764,6 +833,7 @@ dev = [ { name = "pytest-asyncio" }, { name = "pytest-cov" }, { name = "respx" }, + { name = "types-pyyaml" }, ] test = [ { name = "factory-boy" }, @@ -784,8 +854,10 @@ requires-dist = [ { name = "psycopg2-binary", specifier = ">=2.9.9" }, { name = "pydantic-settings", specifier = ">=2.0.0" }, { name = "python-dotenv", specifier = ">=1.0.0" }, + { name = "pyyaml", specifier = ">=6.0.3" }, { name = "sqlalchemy-json", specifier = ">=0.7.0" }, { name = "sqlmodel", specifier = ">=0.0.33" }, + { name = "typer", specifier = ">=0.24.1" }, { name = "uvicorn", extras = ["standard"], specifier = ">=0.32.0" }, ] provides-extras = ["production"] @@ -801,6 +873,7 @@ dev = [ { name = "pytest-asyncio", specifier = ">=0.24.0" }, { name = "pytest-cov", specifier = ">=7.0.0" }, { name = "respx", specifier = ">=0.22.0" }, + { name = "types-pyyaml", specifier = ">=6.0.12.20250915" }, ] test = [ { name = "factory-boy", specifier = ">=3.3.0" }, From f8145cada43a8bdf248c2485dae5a8d112de7ad0 Mon Sep 17 00:00:00 2001 From: Christian Schwartz Date: Wed, 18 Mar 2026 21:13:33 +0100 Subject: [PATCH 02/13] fixed "NULL subresource values bypass this uniqueness constraint." --- .../0e9a994a92da_create_auth_tables.py | 18 +++++++++++++++- verdict-backend/app/models/user.py | 21 ++++++++++++++++++- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/verdict-backend/alembic/versions/0e9a994a92da_create_auth_tables.py b/verdict-backend/alembic/versions/0e9a994a92da_create_auth_tables.py index 84f99c2..e2e4447 100644 --- a/verdict-backend/alembic/versions/0e9a994a92da_create_auth_tables.py +++ b/verdict-backend/alembic/versions/0e9a994a92da_create_auth_tables.py @@ -70,7 +70,23 @@ def upgrade() -> None: sa.Column("subresource", sqlmodel.sql.sqltypes.AutoString(), nullable=True), sa.Column("action", sqlmodel.sql.sqltypes.AutoString(), nullable=False), sa.PrimaryKeyConstraint("id"), - sa.UniqueConstraint("resource", "subresource", "action"), + ) + # A single UniqueConstraint("resource","subresource","action") would not prevent + # duplicates when subresource IS NULL, because PostgreSQL treats NULLs as distinct + # in unique constraints. Two partial indexes give correct uniqueness semantics. + op.create_index( + "uq_permission_resource_action_no_subresource", + "permission", + ["resource", "action"], + unique=True, + postgresql_where=sa.text("subresource IS NULL"), + ) + op.create_index( + "uq_permission_resource_subresource_action", + "permission", + ["resource", "subresource", "action"], + unique=True, + postgresql_where=sa.text("subresource IS NOT NULL"), ) op.create_table( "role", diff --git a/verdict-backend/app/models/user.py b/verdict-backend/app/models/user.py index db86d0c..ab0a303 100644 --- a/verdict-backend/app/models/user.py +++ b/verdict-backend/app/models/user.py @@ -32,7 +32,26 @@ class PermissionBase(SQLModel): class Permission(PermissionBase, BaseModel, table=True): __tablename__ = "permission" # pyright: ignore[reportAssignmentType] - __table_args__ = (sa.UniqueConstraint("resource", "subresource", "action"),) + __table_args__ = ( + # Two partial indexes instead of a single UniqueConstraint: PostgreSQL treats + # NULL as distinct in UNIQUE constraints, so (resource, NULL, action) would + # not be considered a duplicate. Partial indexes give correct semantics. + sa.Index( + "uq_permission_resource_action_no_subresource", + "resource", + "action", + unique=True, + postgresql_where=sa.text("subresource IS NULL"), + ), + sa.Index( + "uq_permission_resource_subresource_action", + "resource", + "subresource", + "action", + unique=True, + postgresql_where=sa.text("subresource IS NOT NULL"), + ), + ) class PermissionPublic(PermissionBase, PublicModel): From d268de2635e4d0aec0d78192eb1b1b6e89a0c976 Mon Sep 17 00:00:00 2001 From: Christian Schwartz Date: Thu, 19 Mar 2026 06:23:09 +0100 Subject: [PATCH 03/13] fixed "Only print success after the transaction succeeds." --- verdict-backend/app/cli.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/verdict-backend/app/cli.py b/verdict-backend/app/cli.py index 9a8d4c7..ab5de59 100644 --- a/verdict-backend/app/cli.py +++ b/verdict-backend/app/cli.py @@ -70,17 +70,19 @@ def create_user( typer.echo(f"Database error: {e.detail}", err=True) raise typer.Exit(code=1) case Ok(user): - typer.echo(f"User '{username}' ready.") + pass match sync_user_roles(session, user.id, [r.id for r in roles]): case Err(e): typer.echo(f"Database error: {e.detail}", err=True) raise typer.Exit(code=1) - for rp in roles: - typer.echo(f"Assigned role '{rp.name}'.") session.commit() + typer.echo(f"User '{username}' ready.") + for rp in roles: + typer.echo(f"Assigned role '{rp.name}'.") + if __name__ == "__main__": cli() From 52e01867ce49397f2f95db8f7a6d8276db8ac1d7 Mon Sep 17 00:00:00 2001 From: Christian Schwartz Date: Thu, 19 Mar 2026 06:23:57 +0100 Subject: [PATCH 04/13] fixed "Give RemoteValidationError its own safe message." --- verdict-backend/app/errors.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/verdict-backend/app/errors.py b/verdict-backend/app/errors.py index a532789..db76fa5 100644 --- a/verdict-backend/app/errors.py +++ b/verdict-backend/app/errors.py @@ -90,6 +90,10 @@ def detail(self) -> str: class RemoteValidationError(ValidationError): url: str + @property + def message(self) -> str: + return "upstream validation error" + @property def detail(self) -> str: return f"validation error ({self.url}): {self.raw}" From 3df8e337cff2ad8ad4b8a53149ab119ef9b35218 Mon Sep 17 00:00:00 2001 From: Christian Schwartz Date: Thu, 19 Mar 2026 06:28:00 +0100 Subject: [PATCH 05/13] Fixed "Change regex parameter to pattern for Pydantic v2 compatibility." --- verdict-backend/app/models/user.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/verdict-backend/app/models/user.py b/verdict-backend/app/models/user.py index ab0a303..406c431 100644 --- a/verdict-backend/app/models/user.py +++ b/verdict-backend/app/models/user.py @@ -65,9 +65,7 @@ class PermissionPublic(PermissionBase, PublicModel): class RoleBase(GoldSourceMixin, SQLModel): - name: str = Field( - nullable=False, regex=_ROLE_NAME_PATTERN - ) # SQLModel uses pydantic v1-style `regex` + name: str = Field(nullable=False, schema_extra={"pattern": _ROLE_NAME_PATTERN}) description: str = Field(default="", nullable=False) From 0a6fa8e2aafdc2a047870520b01044658f5fc935 Mon Sep 17 00:00:00 2001 From: Christian Schwartz Date: Thu, 19 Mar 2026 06:47:47 +0100 Subject: [PATCH 06/13] Fixed "Handle IntegrityError race condition in concurrent upsert calls." --- docs/patterns/ingestion.md | 2 +- verdict-backend/app/queries.py | 31 ++++++++++++++++++++++++++----- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/docs/patterns/ingestion.md b/docs/patterns/ingestion.md index 9a4da26..4787ab2 100644 --- a/docs/patterns/ingestion.md +++ b/docs/patterns/ingestion.md @@ -20,7 +20,7 @@ When multiple sources need to be ingested atomically, a global ingestion endpoin Sources that reference records from other sources (e.g., systems referencing assets) resolve those references by gold source ID during their pipeline. If a referenced record cannot be found, the entire source's ingestion is rejected. -The upsert helpers (`upsert_by_gold_source`, `_upsert_permission`) use a select-then-insert pattern and only catch `OperationalError` on flush. `IntegrityError` from a concurrent insert is deliberately not handled: all ingestion and config-sync callers are single-threaded batch operations, so the race cannot occur. If concurrent callers are ever introduced, add `IntegrityError` handling at the flush site. +The upsert helpers (`upsert_by_gold_source`, `_upsert_permission`) use a select-then-insert pattern. `upsert_by_gold_source` handles `IntegrityError` from concurrent inserts by rolling back and re-querying, so the losing thread returns the canonical record rather than propagating an exception. `_upsert_permission` does not yet handle this case — it is only called from config sync, which is not expected to run concurrently. ## Link Resolution and Syncing diff --git a/verdict-backend/app/queries.py b/verdict-backend/app/queries.py index 661424b..b24782a 100644 --- a/verdict-backend/app/queries.py +++ b/verdict-backend/app/queries.py @@ -3,7 +3,7 @@ from typing import Any, final, overload import sqlalchemy as sa -from sqlalchemy.exc import OperationalError +from sqlalchemy.exc import IntegrityError, OperationalError from sqlmodel import Session, func, select from app.errors import DBError, db_error_from @@ -140,10 +140,9 @@ def upsert_by_gold_source[T: BaseModel, P: PublicModel]( ) -> Result[P, DBError]: """Select-then-insert upsert by gold source identity. - Only OperationalError is caught on flush — IntegrityError from a concurrent - insert is not handled. This is intentional: all callers are single-threaded - batch operations (ingestion services, config sync), so the race cannot occur. - If concurrent callers are ever introduced, add IntegrityError handling here. + On IntegrityError (concurrent insert winning the race), the session is + expired and the existing record is re-queried so the loser returns the + canonical record rather than failing. """ match GoldSourceMixin.get_by_gold_source( session, model_class, gold_source_type, gold_source_id @@ -158,6 +157,28 @@ def upsert_by_gold_source[T: BaseModel, P: PublicModel]( session.add(record) try: session.flush() + except IntegrityError: + session.rollback() + match GoldSourceMixin.get_by_gold_source( + session, model_class, gold_source_type, gold_source_id + ): + case Err(e): + return Err(e) + case Ok(Some(record)): + on_existing(record) + session.add(record) + case Ok(Nothing()) | _: + # Should not happen: integrity error means the row exists. + return Err( + DBError( + statement=None, + raw="concurrent insert race: record not found after rollback", + ) + ) + try: + session.flush() + except OperationalError as e: + return Err(db_error_from(e)) except OperationalError as e: return Err(db_error_from(e)) return Ok(public_class.model_validate(record, from_attributes=True)) From bc219478f9e015c457eb75db6f51091001514b72 Mon Sep 17 00:00:00 2001 From: Christian Schwartz Date: Thu, 19 Mar 2026 06:58:56 +0100 Subject: [PATCH 07/13] Fixed "Distinguish upstream errors (502) from local database failures (500)." --- verdict-backend/app/errors.py | 28 +++++++++++++++++++ verdict-backend/app/result.py | 20 +++++-------- verdict-backend/app/routes/ingestion.py | 10 ++----- verdict-backend/app/routes/sync.py | 2 +- verdict-backend/app/routes/users.py | 5 +--- .../app/services/cmdb_ingestion.py | 9 +++--- 6 files changed, 44 insertions(+), 30 deletions(-) diff --git a/verdict-backend/app/errors.py b/verdict-backend/app/errors.py index db76fa5..4092d9a 100644 --- a/verdict-backend/app/errors.py +++ b/verdict-backend/app/errors.py @@ -21,6 +21,10 @@ def message(self) -> str: ... @abstractmethod def detail(self) -> str: ... + @property + @abstractmethod + def http_status(self) -> int: ... + def __str__(self) -> str: return self.message @@ -39,6 +43,10 @@ def message(self) -> str: def detail(self) -> str: return f"duplicate {self.model}: {self.key}" + @property + def http_status(self) -> int: + return 409 + @final @dataclass(frozen=True, slots=True) @@ -56,6 +64,10 @@ def detail(self) -> str: return f"database error: {self.statement}" return "database error" + @property + def http_status(self) -> int: + return 500 + @final @dataclass(frozen=True, slots=True) @@ -71,6 +83,10 @@ def message(self) -> str: def detail(self) -> str: return f"fetch error ({self.url}): {self.raw}" + @property + def http_status(self) -> int: + return 502 + @dataclass(frozen=True, slots=True) class ValidationError(AppError): @@ -84,6 +100,10 @@ def message(self) -> str: def detail(self) -> str: return f"validation error: {self.raw}" + @property + def http_status(self) -> int: + return 422 + @final @dataclass(frozen=True, slots=True) @@ -98,6 +118,10 @@ def message(self) -> str: def detail(self) -> str: return f"validation error ({self.url}): {self.raw}" + @property + def http_status(self) -> int: + return 502 + @final @dataclass(frozen=True, slots=True) @@ -113,6 +137,10 @@ def message(self) -> str: def detail(self) -> str: return f"config error ({self.path}): {self.raw}" + @property + def http_status(self) -> int: + return 500 + type WriteError = DuplicateError | DBError type IngestionError = FetchError | RemoteValidationError | ValidationError | DBError diff --git a/verdict-backend/app/result.py b/verdict-backend/app/result.py index 23f973e..9f91435 100644 --- a/verdict-backend/app/result.py +++ b/verdict-backend/app/result.py @@ -67,41 +67,35 @@ def __str__(self) -> str: type Option[T] = Some[T] | Nothing -def _raise_for_err(error: object, *, status: int) -> NoReturn: +def _raise_for_err(error: object) -> NoReturn: """Log detail and raise an HTTPException with the safe message.""" if isinstance(error, AppError): logger.error("%s", error.detail) - raise HTTPException(status_code=status, detail=error.message) - raise HTTPException(status_code=status, detail="Internal server error") + raise HTTPException(status_code=error.http_status, detail=error.message) + raise HTTPException(status_code=500, detail="Internal server error") def unwrap_or_raise[T, E]( result: Result[T, E], - *, - err_status: int = 503, ) -> T: - """Unwrap a ``Result[T, E]`` or raise an ``HTTPException``. - - Raises *err_status* for ``Err``. - """ + """Unwrap a ``Result[T, E]`` or raise an ``HTTPException``.""" if isinstance(result, Err): - _raise_for_err(result.value, status=err_status) + _raise_for_err(result.value) return result.value def unwrap_optional_or_raise[T, E]( result: Result[Option[T], E], *, - err_status: int = 503, not_found_status: int = 404, not_found_detail: str = "Not found", ) -> T: """Unwrap a ``Result[Option[T], E]`` or raise an ``HTTPException``. - Raises *err_status* for ``Err`` and *not_found_status* for ``Nothing``. + Raises the error's own ``http_status`` for ``Err`` and *not_found_status* for ``Nothing``. """ if isinstance(result, Err): - _raise_for_err(result.value, status=err_status) + _raise_for_err(result.value) match result.value: case Some(value): return value diff --git a/verdict-backend/app/routes/ingestion.py b/verdict-backend/app/routes/ingestion.py index f519108..d12bbce 100644 --- a/verdict-backend/app/routes/ingestion.py +++ b/verdict-backend/app/routes/ingestion.py @@ -18,14 +18,8 @@ def trigger_full_ingestion( session: Session = Depends(get_session), client: httpx.Client = Depends(get_http_client), ) -> FullIngestionResponse: - assets = unwrap_or_raise( - ingest_assets(session, client, settings.asset_inventory_url), - err_status=502, - ) - systems = unwrap_or_raise( - ingest_systems(session, client, settings.cmdb_url), - err_status=502, - ) + assets = unwrap_or_raise(ingest_assets(session, client, settings.asset_inventory_url)) + systems = unwrap_or_raise(ingest_systems(session, client, settings.cmdb_url)) session.commit() return FullIngestionResponse( assets_ingested=len(assets), diff --git a/verdict-backend/app/routes/sync.py b/verdict-backend/app/routes/sync.py index aac7915..a7371f2 100644 --- a/verdict-backend/app/routes/sync.py +++ b/verdict-backend/app/routes/sync.py @@ -17,6 +17,6 @@ def trigger_sync( session: Session = Depends(get_session), ) -> SyncResponse: result = sync_config(session, Path(settings.config_basedir)) - response = unwrap_or_raise(result, err_status=500) + response = unwrap_or_raise(result) session.commit() return response diff --git a/verdict-backend/app/routes/users.py b/verdict-backend/app/routes/users.py index 0ff7ce8..551efad 100644 --- a/verdict-backend/app/routes/users.py +++ b/verdict-backend/app/routes/users.py @@ -37,10 +37,7 @@ def trigger_user_ingestion( session: Session = Depends(get_session), client: httpx.Client = Depends(get_http_client), ) -> UserIngestionResponse: - users = unwrap_or_raise( - ingest_users(session, client, settings.iam_url), - err_status=502, - ) + users = unwrap_or_raise(ingest_users(session, client, settings.iam_url)) session.commit() return UserIngestionResponse(users_ingested=len(users)) diff --git a/verdict-backend/app/services/cmdb_ingestion.py b/verdict-backend/app/services/cmdb_ingestion.py index fad6a97..92efef2 100644 --- a/verdict-backend/app/services/cmdb_ingestion.py +++ b/verdict-backend/app/services/cmdb_ingestion.py @@ -10,7 +10,6 @@ FetchError, IngestionError, RemoteValidationError, - ValidationError, db_error_from, ) from app.http import fetch_json @@ -54,7 +53,8 @@ def to_system(detail: SystemDetail) -> SystemCreate: def _resolve_asset_ids( session: Session, gold_source_ids: list[str], -) -> Result[list[int], ValidationError | DBError]: + url: str, +) -> Result[list[int], RemoteValidationError | DBError]: asset_ids: list[int] = [] for gs_id in gold_source_ids: result = GoldSourceMixin.get_by_gold_source( @@ -71,7 +71,8 @@ def _resolve_asset_ids( asset_ids.append(asset.id) case Nothing(): return Err( - ValidationError( + RemoteValidationError( + url=url, raw=f"unresolvable asset reference: {gs_id}", ) ) @@ -129,7 +130,7 @@ def ingest_systems( resolved_assets: dict[str, list[int]] = {} for detail in details: if detail.asset_gold_source_ids: - resolve_result = _resolve_asset_ids(session, detail.asset_gold_source_ids) + resolve_result = _resolve_asset_ids(session, detail.asset_gold_source_ids, url) if isinstance(resolve_result, Err): return Err(resolve_result.value) resolved_assets[detail.id] = resolve_result.value From c3d109396c587152a0ed78d6fa4a6163e5d604e5 Mon Sep 17 00:00:00 2001 From: Christian Schwartz Date: Thu, 19 Mar 2026 06:59:52 +0100 Subject: [PATCH 08/13] Fixed "Avoid in-place truncation when regenerating schema." --- verdict-backend/justfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/verdict-backend/justfile b/verdict-backend/justfile index edcb4eb..360f3f5 100644 --- a/verdict-backend/justfile +++ b/verdict-backend/justfile @@ -82,7 +82,7 @@ db-info: # Regenerate JSON Schema for config/roles/*.yaml from the RoleConfig pydantic model gen-config-schema: - uv run python -c "from app.schemas.config import RoleConfig; import json; print(json.dumps(RoleConfig.model_json_schema(), indent=2))" > ../config/roles/schema.json + uv run python -c "from app.schemas.config import RoleConfig; import json; print(json.dumps(RoleConfig.model_json_schema(), indent=2))" > ../config/roles/schema.json.tmp && mv ../config/roles/schema.json.tmp ../config/roles/schema.json # Create a local user (--role flags are optional and repeatable) create-user username email *ROLES: From 4eb5816ca271306ee638c8649c0c0b739d3aad49 Mon Sep 17 00:00:00 2001 From: Christian Schwartz Date: Thu, 19 Mar 2026 07:01:48 +0100 Subject: [PATCH 09/13] Fixed "Assert the first CLI invocation in idempotency tests." --- verdict-backend/tests/cli/test_create_user.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/verdict-backend/tests/cli/test_create_user.py b/verdict-backend/tests/cli/test_create_user.py index 241e53a..a1e6929 100644 --- a/verdict-backend/tests/cli/test_create_user.py +++ b/verdict-backend/tests/cli/test_create_user.py @@ -77,10 +77,11 @@ def test_create_user_succeeds(): def test_create_user_is_idempotent(): - runner.invoke( + first = runner.invoke( cli, ["--username", "cli-user-2", "--email", "cli-user-2@example.com"], ) + assert first.exit_code == 0 result = runner.invoke( cli, ["--username", "cli-user-2", "--email", "cli-user-2@example.com"], @@ -111,7 +112,7 @@ def test_create_user_with_roles(): def test_create_user_role_assignment_is_idempotent(): - runner.invoke( + first = runner.invoke( cli, [ "--username", @@ -122,6 +123,7 @@ def test_create_user_role_assignment_is_idempotent(): "cli-analyst", ], ) + assert first.exit_code == 0 result = runner.invoke( cli, [ From dcd299a0ec784d538be96fd01e06cbc7468b9af2 Mon Sep 17 00:00:00 2001 From: Christian Schwartz Date: Thu, 19 Mar 2026 07:10:11 +0100 Subject: [PATCH 10/13] Fixed: "Type union includes both parent and child classes." --- docs/patterns/error-handling.md | 2 +- verdict-backend/app/errors.py | 3 ++- verdict-backend/tests/services/test_asset_ingestion.py | 8 ++++---- verdict-backend/tests/services/test_cmdb_ingestion.py | 8 ++++---- 4 files changed, 11 insertions(+), 10 deletions(-) diff --git a/docs/patterns/error-handling.md b/docs/patterns/error-handling.md index 86dac26..bf530ba 100644 --- a/docs/patterns/error-handling.md +++ b/docs/patterns/error-handling.md @@ -15,7 +15,7 @@ Error types are domain-specific dataclasses defined in `app/errors.py`. They for - `IngestionError = FetchError | RemoteValidationError | ValidationError | DBError` — covers HTTP failures, remote schema parse errors, local validation errors, and database errors. - `ConfigSyncError = ConfigError | ValidationError | DBError` — covers YAML file errors (missing directory, unreadable file, empty file), local validation errors, and database errors. -`RemoteValidationError` is a URL-aware subclass of `ValidationError`. It is produced when an HTTP response body fails Pydantic validation and includes the source URL in its `detail`, making it easier to diagnose which remote endpoint returned an unexpected shape. `ValidationError` is used for local validation failures (e.g., config files) where no URL context is available. +`RemoteValidationError` is a distinct error type (not a subtype of `ValidationError`) for upstream data quality failures — produced when an HTTP response body fails Pydantic validation or when a remote service references a resource that cannot be resolved locally. It includes the source URL in its `detail` and maps to HTTP 502. `ValidationError` is for local validation failures (e.g., config files) where no URL context is available, and maps to HTTP 422. Functions declare which error union they can produce, and callers handle each variant explicitly. diff --git a/verdict-backend/app/errors.py b/verdict-backend/app/errors.py index 4092d9a..cbb07d2 100644 --- a/verdict-backend/app/errors.py +++ b/verdict-backend/app/errors.py @@ -107,8 +107,9 @@ def http_status(self) -> int: @final @dataclass(frozen=True, slots=True) -class RemoteValidationError(ValidationError): +class RemoteValidationError(AppError): url: str + raw: str @property def message(self) -> str: diff --git a/verdict-backend/tests/services/test_asset_ingestion.py b/verdict-backend/tests/services/test_asset_ingestion.py index 70fedc3..3b21c31 100644 --- a/verdict-backend/tests/services/test_asset_ingestion.py +++ b/verdict-backend/tests/services/test_asset_ingestion.py @@ -2,7 +2,7 @@ import respx from sqlmodel import select -from app.errors import FetchError, ValidationError +from app.errors import FetchError, RemoteValidationError from app.models.asset import Asset, AssetCreate from app.models.gold_source import GoldSourceType from app.result import Err, Ok @@ -62,7 +62,7 @@ def test_fetch_index_invalid_response(): result = fetch_index(client, FAKE_URL) assert isinstance(result, Err) - assert isinstance(result.value, ValidationError) + assert isinstance(result.value, RemoteValidationError) @respx.mock @@ -111,7 +111,7 @@ def test_fetch_detail_invalid_response(): result = fetch_detail(client, FAKE_URL, "SVC-001") assert isinstance(result, Err) - assert isinstance(result.value, ValidationError) + assert isinstance(result.value, RemoteValidationError) # --- to_asset tests --- @@ -162,7 +162,7 @@ def test_ingest_invalid_index_persists_nothing(db_session): result = ingest_assets(db_session, client, FAKE_URL) assert isinstance(result, Err) - assert isinstance(result.value, ValidationError) + assert isinstance(result.value, RemoteValidationError) assets = list(db_session.exec(select(Asset)).all()) assert len(assets) == 0 diff --git a/verdict-backend/tests/services/test_cmdb_ingestion.py b/verdict-backend/tests/services/test_cmdb_ingestion.py index 4d43d75..d45695b 100644 --- a/verdict-backend/tests/services/test_cmdb_ingestion.py +++ b/verdict-backend/tests/services/test_cmdb_ingestion.py @@ -3,7 +3,7 @@ import sqlalchemy as sa from sqlmodel import select -from app.errors import FetchError, ValidationError +from app.errors import FetchError, RemoteValidationError from app.models.gold_source import GoldSourceType from app.models.system import System, SystemCreate, asset_system from app.result import Err, Ok @@ -68,7 +68,7 @@ def test_fetch_index_invalid_response(): result = fetch_index(client, FAKE_URL) assert isinstance(result, Err) - assert isinstance(result.value, ValidationError) + assert isinstance(result.value, RemoteValidationError) @respx.mock @@ -117,7 +117,7 @@ def test_fetch_detail_invalid_response(): result = fetch_detail(client, FAKE_URL, "SYS-001") assert isinstance(result, Err) - assert isinstance(result.value, ValidationError) + assert isinstance(result.value, RemoteValidationError) # --- to_system tests --- @@ -193,7 +193,7 @@ def test_ingest_unresolvable_asset_rejects_all(db_session): result = ingest_systems(db_session, client, FAKE_URL) assert isinstance(result, Err) - assert isinstance(result.value, ValidationError) + assert isinstance(result.value, RemoteValidationError) systems = list(db_session.exec(select(System)).all()) assert len(systems) == 0 From 30670e3edb1e48829b9d0071c1da2f7b4db077cb Mon Sep 17 00:00:00 2001 From: Christian Schwartz Date: Thu, 19 Mar 2026 07:11:47 +0100 Subject: [PATCH 11/13] Fixed "Missing logging for non-AppError cases." --- verdict-backend/app/result.py | 1 + 1 file changed, 1 insertion(+) diff --git a/verdict-backend/app/result.py b/verdict-backend/app/result.py index 9f91435..179dfc9 100644 --- a/verdict-backend/app/result.py +++ b/verdict-backend/app/result.py @@ -72,6 +72,7 @@ def _raise_for_err(error: object) -> NoReturn: if isinstance(error, AppError): logger.error("%s", error.detail) raise HTTPException(status_code=error.http_status, detail=error.message) + logger.error("unexpected error type in unwrap_or_raise: %r", error) raise HTTPException(status_code=500, detail="Internal server error") From 9f735750179c9684955fb6750c270f961bdd080f Mon Sep 17 00:00:00 2001 From: Christian Schwartz Date: Thu, 19 Mar 2026 07:14:25 +0100 Subject: [PATCH 12/13] Fixed "Potential race condition and missing IntegrityError handling." --- verdict-backend/app/services/config_sync.py | 25 ++++++++++++++++----- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/verdict-backend/app/services/config_sync.py b/verdict-backend/app/services/config_sync.py index f4f2a6a..68b9eb5 100644 --- a/verdict-backend/app/services/config_sync.py +++ b/verdict-backend/app/services/config_sync.py @@ -3,7 +3,7 @@ import yaml from pydantic import ValidationError as PydanticValidationError -from sqlalchemy.exc import OperationalError +from sqlalchemy.exc import IntegrityError, OperationalError from sqlmodel import Session, select from app.errors import ConfigError, ConfigSyncError, DBError, ValidationError, db_error_from @@ -41,12 +41,12 @@ def _upsert_permission( session: Session, cfg: PermissionConfig, ) -> Result[PermissionPublic, DBError]: + stmt = select(Permission).where( + Permission.resource == cfg.resource, + Permission.subresource == cfg.subresource, + Permission.action == cfg.action, + ) try: - stmt = select(Permission).where( - Permission.resource == cfg.resource, - Permission.subresource == cfg.subresource, - Permission.action == cfg.action, - ) existing = session.exec(stmt).first() except OperationalError as e: return Err(db_error_from(e)) @@ -62,6 +62,19 @@ def _upsert_permission( session.add(new) try: session.flush() + except IntegrityError: + session.rollback() + try: + existing = session.exec(stmt).first() + except OperationalError as e: + return Err(db_error_from(e)) + if existing is not None: + return Ok(PermissionPublic.model_validate(existing, from_attributes=True)) + return Err( + DBError( + statement=None, raw="concurrent insert race: permission not found after rollback" + ) + ) except OperationalError as e: return Err(db_error_from(e)) return Ok(PermissionPublic.model_validate(new, from_attributes=True)) From 91a3cfd1ce36a47c9ebd6a08d010d49cf4b9924e Mon Sep 17 00:00:00 2001 From: Christian Schwartz Date: Thu, 19 Mar 2026 07:16:07 +0100 Subject: [PATCH 13/13] fixed "Assert the result of the first upsert call." and "Assert the result of the first sync call in idempotency test." --- verdict-backend/tests/services/test_config_sync.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/verdict-backend/tests/services/test_config_sync.py b/verdict-backend/tests/services/test_config_sync.py index 7413a08..9656cd8 100644 --- a/verdict-backend/tests/services/test_config_sync.py +++ b/verdict-backend/tests/services/test_config_sync.py @@ -117,7 +117,8 @@ def test_upsert_role_creates_new(db_session: Session): def test_upsert_role_updates_existing_name(db_session: Session): cfg = RoleConfig(name="engineers", gold_source_id="GRP-001", permissions=[]) - _upsert_role(db_session, cfg) + first = _upsert_role(db_session, cfg) + assert isinstance(first, Ok) updated_cfg = RoleConfig(name="senior-engineers", gold_source_id="GRP-001", permissions=[]) result = _upsert_role(db_session, updated_cfg) @@ -173,7 +174,8 @@ def test_sync_config_idempotent(db_session: Session, tmp_path: Path): """) ) - sync_config(db_session, tmp_path) + first = sync_config(db_session, tmp_path) + assert isinstance(first, Ok) result = sync_config(db_session, tmp_path) assert isinstance(result, Ok)