mirror of
https://github.com/GNS3/gns3-server.git
synced 2026-08-28 21:10:14 +03:00
docs: document server settings API and add API test writing skill
Move the server settings roadmap to implemented/ rewritten per the documentation standard (architecture and PUT flow diagrams, endpoint table, design notes). Add a skill covering the pytest conftest fixture model and the shared-client order-dependency trap hit while writing the settings tests.
This commit is contained in:
parent
e0962a59ed
commit
05fa4ec376
58
.claude/skills/gns3-api-test-writing/SKILL.md
Normal file
58
.claude/skills/gns3-api-test-writing/SKILL.md
Normal file
@ -0,0 +1,58 @@
|
||||
---
|
||||
name: gns3-api-test-writing
|
||||
description: Use this skill when writing pytest tests for gns3-server API routes — the conftest fixture model, auth token variants, config isolation, and the shared-client order-dependency trap.
|
||||
version: 1.0.0
|
||||
---
|
||||
|
||||
# Writing pytest API Route Tests
|
||||
|
||||
## Test Environment
|
||||
|
||||
- Run tests with the repo venv: `venv/bin/python -m pytest tests/api/routes/controller/test_xxx.py` (there is no system python/pytest).
|
||||
- The app runs in-process (httpx `ASGIWebSocketTransport`); the database is in-memory sqlite; superadmin `admin` is seeded when the users table is created.
|
||||
- `pytestmark = pytest.mark.asyncio`. Tests run on function-scoped event loops while class-scoped fixtures bind to the class loop — this works, but don't move class fixtures to function scope casually.
|
||||
|
||||
## The Fixture Model (tests/conftest.py)
|
||||
|
||||
Class-scoped: `app`, `db_session`, `base_client` (**one** shared httpx `AsyncClient`), `test_user` (idempotent `user1` in the "Users" group).
|
||||
|
||||
`client` (admin), `authorized_client` (user1), `unauthorized_client`, `compute_client` are class-scoped wrappers that each **rewrite the shared `base_client.headers` at instantiation time**. They are instantiated lazily — by the first test that requests them.
|
||||
|
||||
### The order-dependency trap
|
||||
|
||||
- `unauthorized_client` is a passthrough: it sets no header and only behaves as "unauthorized" if nothing set a token on `base_client` before it.
|
||||
- Mixing auth variants in one test class makes the default `Authorization` header depend on fixture instantiation order → tests pass alone but fail in a class run (or the reverse).
|
||||
- Symptom signature: a 401/403 assertion receives 200. That is fixture pollution, **not** an RBAC/auth bug in the product.
|
||||
|
||||
### Rule: per-request headers for auth variants
|
||||
|
||||
Never rely on the client's default header for 401/403/specific-user tests. Send the token explicitly (request-level headers override client defaults — the `test_users.py` idiom):
|
||||
|
||||
```python
|
||||
from gns3server.services import auth_service
|
||||
from gns3server.services.authentication import DEFAULT_JWT_SECRET_KEY
|
||||
|
||||
token = auth_service.create_access_token(test_user.username, secret_key=DEFAULT_JWT_SECRET_KEY)
|
||||
response = await client.get(url, headers={"Authorization": f"Bearer {token}"}) # specific user
|
||||
response = await client.get(url, headers={"Authorization": "Bearer invalid_token"}) # 401
|
||||
```
|
||||
|
||||
Note the import: `auth_service` lives in `gns3server.services`, NOT `gns3server.services.authentication` (which only exports `DEFAULT_JWT_SECRET_KEY` and the class).
|
||||
|
||||
Always pass `secret_key=DEFAULT_JWT_SECRET_KEY`: the autouse `run_around_tests` resets `Config` per test and forces the default secret, so a token minted with the config-of-the-moment dies in the next test.
|
||||
|
||||
## Config-Isolated Tests
|
||||
|
||||
- Request the function-scoped `config` fixture whenever the test reads/writes configuration or the endpoint under test reloads it. It points `Config` at `tmpdir/server.conf` (accessible as `config._main_config_file`).
|
||||
- Any endpoint that triggers a config reload re-reads `<secrets_dir>/gns3_jwt_secret_key`, which invalidates class-scoped bearer tokens. Fix: write `DEFAULT_JWT_SECRET_KEY` to that file first — see the `stable_jwt_secret` fixture in `tests/api/routes/controller/test_settings.py`.
|
||||
|
||||
## Misc Gotchas
|
||||
|
||||
- Build URLs with `app.url_path_for("route_function_name")`. Router introspection is unreliable (lazy `_IncludedRouter` wrapper) — check `GET /openapi.json` instead.
|
||||
- pydantic v2: an empty `SecretStr('')` serializes as `""`, **not** the mask — assert `in ("", SECRET_MASK)` for secrets that may be unset in tests.
|
||||
- New privileges are seeded only at table creation (`gns3server/db/models/privileges.py`); the fresh in-memory test DB always has them, but existing deployments need manual grants.
|
||||
- Error mapping: `ControllerBadRequestError` → 400, `ControllerError`/`HTTPException(409)` → 409, request schema violations → 422.
|
||||
|
||||
## Failure-Diagnosis Heuristic
|
||||
|
||||
**Passes in isolation, fails in a class run → suspect shared fixture state first** (`base_client.headers`, the `Config` singleton, class-scoped DB rows) — never the product code. Reproduce with `-k "test_a or test_b"` pairs to find the polluting test. Do not add debug prints to product code to chase test-order issues; make the test order-independent with explicit per-request headers instead.
|
||||
103
docs/gns3-copilot/implemented/server-settings-api.md
Normal file
103
docs/gns3-copilot/implemented/server-settings-api.md
Normal file
@ -0,0 +1,103 @@
|
||||
<!--
|
||||
SPDX-License-Identifier: CC-BY-SA-4.0
|
||||
See LICENSE file for licensing information.
|
||||
-->
|
||||
|
||||
# Server Settings API
|
||||
|
||||
## Overview
|
||||
|
||||
REST API for reading and updating `gns3_server.conf` at runtime, enabling a server settings page in the Web UI. Updates are persisted with a read-modify-write strategy (unknown options in the file are preserved), validated before anything touches disk, and hot-reloaded into the running server; the response tells the caller which changes require a restart.
|
||||
|
||||
## Architecture
|
||||
|
||||
```mermaid
|
||||
flowchart LR
|
||||
U["Web UI / Client"] -->|"GET /v3/settings"| R["routes/controller/settings.py"]
|
||||
U -->|"PUT /v3/settings"| R
|
||||
R -->|"Server.Audit / Server.Modify"| RBAC["privilege check"]
|
||||
R --> RS["SettingsResponse / SettingsUpdate schemas<br/>(extra=forbid, secret masking)"]
|
||||
RS --> UC["Config.update_config()<br/>read-modify-write"]
|
||||
UC --> FILE["gns3_server.conf<br/>(atomic replace, mode 0600)"]
|
||||
UC --> RN["reload_and_notify()"]
|
||||
RN --> CB["file-watch callbacks<br/>(runtime hot reload)"]
|
||||
R --> NOTIF["notification stream:<br/>settings.updated"]
|
||||
```
|
||||
|
||||
- **Exposure** — all sections except the deprecated `VirtualBox`/`VMware`: `Server`, `Controller`, `VPCS`, `Dynamips`, `IOU`, `Qemu`, `WebWireshark`. `Controller.jwt_secret_key` is excluded entirely: it is loaded from `<secrets_dir>/gns3_jwt_secret_key` and writing it to the configuration file is a no-op.
|
||||
- **Write strategy** — `Config.update_config()` re-reads the configuration files with `configparser`, applies only the submitted options, and atomically rewrites the main configuration file. Comments and formatting are lost (accepted trade-off); options unknown to the schema are preserved.
|
||||
- **Validate before write** — the merged view of all configuration files is validated as `ServerConfig` *before* any disk write. A validation error must never reach disk: the `FileWatcher` reload callback would raise and permanently stop polling that file.
|
||||
|
||||
## Business Process (PUT)
|
||||
|
||||
```mermaid
|
||||
sequenceDiagram
|
||||
participant C as Client
|
||||
participant R as PUT /v3/settings
|
||||
participant U as Config.update_config()
|
||||
participant D as gns3_server.conf
|
||||
C->>R: PUT {"Section": {"option": value | null}}
|
||||
R->>R: schema validation (unknown key/section → 422)
|
||||
R->>U: changes (masked/empty secrets skipped)
|
||||
U->>U: locate owning file per option (later file wins)
|
||||
alt option owned by a non-main configuration file
|
||||
U-->>R: ConfigConflictError → 409 (nothing written)
|
||||
else
|
||||
U->>U: set / remove option (null removes, value falls back to default)
|
||||
U->>U: validate merged ServerConfig (failure → 400, file untouched)
|
||||
U->>D: atomic write (.tmp + os.replace, mode 0600)
|
||||
U->>U: reload_and_notify() → runtime hot reload
|
||||
R-->>C: 200 — new values + restart_required
|
||||
R->>C: notification settings.updated (option names only, no values)
|
||||
end
|
||||
```
|
||||
|
||||
## API Endpoints
|
||||
|
||||
| Method | Path | Description | Privilege |
|
||||
|--------|------|-------------|-----------|
|
||||
| GET | `/v3/settings` | Return all current server settings | `Server.Audit` |
|
||||
| PUT | `/v3/settings` | Update and persist server settings | `Server.Modify` |
|
||||
|
||||
Request example:
|
||||
|
||||
```json
|
||||
{
|
||||
"Server": {
|
||||
"report_errors": true,
|
||||
"allowed_interfaces": ["eth0", "lo"],
|
||||
"compute_password": "**********"
|
||||
},
|
||||
"Qemu": {
|
||||
"enable_monitor": false
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Response example (abbreviated):
|
||||
|
||||
```json
|
||||
{
|
||||
"Server": { "report_errors": true, "allowed_interfaces": ["eth0", "lo"], "...": "..." },
|
||||
"Qemu": { "enable_monitor": false },
|
||||
"restart_required": ["Server.port"]
|
||||
}
|
||||
```
|
||||
|
||||
## Notes
|
||||
|
||||
- **Secrets** — `SecretStr` fields are masked in responses. An empty secret (e.g. `compute_password` before the server generates one) serializes as `""` rather than the mask. On PUT, the mask or an empty string means "leave unchanged"; only an explicit new value is written, in clear text like a hand-edited file.
|
||||
- **`restart_required`** — options that only take effect after a server restart (bind host/port, protocol, TLS and certificates, port ranges, image/symbol/config paths, GNS3 VM credentials, skills paths, …). Everything else hot-reloads via `Config.instance().settings`.
|
||||
- **GET reflects runtime values** — in-memory settings may differ from disk (e.g. the generated `compute_password`, the resolved `secrets_dir`); the mask/empty skip rule guarantees PUT never writes echoed values back.
|
||||
- **Privileges** — `Server.Audit`/`Server.Modify` are seeded into the `Administrator` role at table creation only; existing databases need a manual grant. Superadmins bypass RBAC.
|
||||
- **Hardening** — the file watcher callback is exception-guarded (`utils/file_watcher.py`): a callback failure is logged instead of silently killing the polling loop.
|
||||
|
||||
### Related Files
|
||||
|
||||
| File | Role |
|
||||
|------|------|
|
||||
| `gns3server/config.py` | `Config.update_config()` (read-modify-write, validate-before-write), `reload_and_notify()` |
|
||||
| `gns3server/utils/file_watcher.py` | callback exception hardening |
|
||||
| `gns3server/schemas/controller/settings.py` | response/update models, `SECRET_MASK` |
|
||||
| `gns3server/api/routes/controller/settings.py` | GET/PUT endpoints, `restart_required`, notification |
|
||||
| `gns3server/db/models/privileges.py` | `Server.Audit` / `Server.Modify` privilege seeds |
|
||||
@ -1,67 +0,0 @@
|
||||
<!--
|
||||
SPDX-License-Identifier: CC-BY-SA-4.0
|
||||
See LICENSE file for licensing information.
|
||||
-->
|
||||
|
||||
> This document is a roadmap/planning document. The described features have not been implemented yet.
|
||||
|
||||
|
||||
# Server Settings REST API — Roadmap
|
||||
|
||||
## Problem
|
||||
|
||||
Currently, `gns3_server.conf` can only be modified by directly editing the file on disk. There is no REST API endpoint to read or write server configuration, which prevents the Web UI from offering a settings page for server parameters.
|
||||
|
||||
## Proposed API
|
||||
|
||||
```
|
||||
GET /v3/settings → Return all current server settings
|
||||
PUT /v3/settings → Update and persist server settings
|
||||
```
|
||||
|
||||
### Implementation Plan
|
||||
|
||||
**1. Add `save_config()` to `Config` class** (`gns3server/config.py`)
|
||||
|
||||
The `Config` class currently only reads configuration (via `read_config()` / `reload()`). A `save_config()` method is needed to serialize the in-memory `ServerConfig` pydantic model back to INI format and write it to disk.
|
||||
|
||||
Serialization details:
|
||||
- `bool` → `"True"` / `"False"` (configparser convention)
|
||||
- `SecretStr` → `get_secret_value()`
|
||||
- `Enum` → `.value`
|
||||
- `List[str]` → semi-colon for `additional_images_paths`, comma for `allowed_interfaces`
|
||||
- `None` → skip
|
||||
|
||||
**2. Add a settings getter/setter** to `Config` to allow programmatic updates to the in-memory settings.
|
||||
|
||||
**3. New route file** (`gns3server/api/routes/controller/settings.py`):
|
||||
|
||||
- `GET /v3/settings` — returns the full `ServerConfig` as JSON (pydantic automatically masks `SecretStr` fields as `"********"`)
|
||||
- `PUT /v3/settings` — accepts `ServerConfig`, merges existing secrets when placeholder values (`"********"`) are submitted, calls `save_config()`, and triggers runtime config update callbacks
|
||||
|
||||
Both endpoints require `get_current_active_user` for authentication.
|
||||
|
||||
**4. Register the new router** in `gns3server/api/routes/controller/__init__.py` under the `/settings` prefix.
|
||||
|
||||
### Security
|
||||
|
||||
- All settings endpoints require admin authentication (`get_current_active_user`)
|
||||
- `SecretStr` fields (`compute_password`, `default_admin_password`, `jwt_secret_key`) are masked in responses
|
||||
- On write, unchanged secrets are preserved via placeholder detection
|
||||
|
||||
### Related Files
|
||||
|
||||
| File | Role |
|
||||
|------|------|
|
||||
| `gns3server/config.py` | Config singleton with `read_config()` / `reload()` |
|
||||
| `gns3server/schemas/config.py` | `ServerConfig` pydantic model with all 9 sub-models |
|
||||
| `gns3server/api/routes/controller/__init__.py` | Controller router mounting |
|
||||
| `gns3server/controller/__init__.py` | `Controller._update_config()` for runtime credential sync |
|
||||
|
||||
## Status
|
||||
|
||||
- [ ] gns3server/config.py: add `save_config()` method
|
||||
- [ ] gns3server/config.py: add settings getter/setter
|
||||
- [ ] gns3server/api/routes/controller/settings.py: new route file with GET and PUT endpoints
|
||||
- [ ] gns3server/api/routes/controller/__init__.py: register settings router under `/settings`
|
||||
- [ ] gns3server/api/routes/controller/controller.py: add notification emission on config change
|
||||
Loading…
x
Reference in New Issue
Block a user