Skip to content

Commit 0e0dcfa

Browse files
paullizerCopilot
paullizer
and
Copilot
committed
Harden the broker API and add server-side pagination
Server-side counterpart to the portal modernization. Fixes bugs found while reviewing api/app.py against the stored procedures, and adds the two endpoints the new UI needed. Bugs fixed: - The "No Limit" option never worked. The portal sends limit as the string "null"; the procedures declare @limit INT, so SQL Server failed converting it and all three history endpoints returned 500. Limits are now coerced, and the portal no longer sends the sentinel. - Database connections leaked on every exception path: 27 get_db_connection() calls had only 7 finally blocks. All handlers now use a db_connection() context manager that always closes. - 18 handlers returned the raw str(e) to the caller, exposing driver errors, server names and schema detail. Failures now return {"error": ...} and the detail goes to logger.exception. - 14 print() calls became logger calls, including inside get_db_connection() and Key Vault retrieval, so database and secret failures actually reach Application Insights instead of being swallowed. - /api/scaling/rules returned 404 when no rules existed, which made the portal flash an error rather than render its empty state. /api/scaling/log and /api/scaling/rules/history returned a dict when empty and a list otherwise. All three now return a JSON array with 200. - TriggerScalingLogic ran without committing. pymssql does not autocommit, so the power-state updates and the activity-log insert were rolled back while the Azure power operations still went ahead, leaving Azure and the broker out of step and the scaling activity log permanently empty. - is_member_of_group_cached was defined but never called; token_required used the uncached path, so every authenticated request from the AVD and Linux hosts hit Microsoft Graph. Now wired up, and a Graph failure raises rather than returning False so a throttled call is never cached as a denial. - The scaling procedures relied on implicit MM/DD/YYYY date conversion, which depends on the session DATEFORMAT. They now convert explicitly with style 101, matching GetVmHistory, via TRY_CONVERT. Added: - GET /api/vms/summary, so the dashboard no longer fetches every VM row to compute eight counters. - Opt-in page/per_page pagination on the three history endpoints, backed by new paged procedures that return TotalCount via COUNT(*) OVER (). With neither parameter the response stays a bare array, because the scheduled task and older portal builds consume these as plain lists. - api/tests/ (44 tests) with pymssql and the Azure SDKs mocked, plus CI wiring. Verified by mutation testing: each fix was reverted in turn and the suite failed every time. - api/README.md covering the endpoint surface, auth model, consumer map, error envelope and pagination contract. Front end: - The dashboard uses the summary endpoint, and the history pages use server-side pagination, so whole result sets are no longer cached in the Flask session. That caching grew without bound and let two browser tabs clobber each other. - Both have fallbacks for an API deployed behind the portal. Removed: the unused pyodbc dependency and the /api/vms/available endpoint, which had no callers in the repo. External callers of that endpoint, if any, would need checking before deploying. Not addressed: the 54 Dependabot alerts on the default branch. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 62f84fc commit 0e0dcfa

22 files changed

Lines changed: 2144 additions & 819 deletions

‎.github/workflows/front-end-tests.yml‎

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,19 @@
1-
name: Front end tests
1+
name: App tests
22

33
on:
44
pull_request:
55
paths:
66
- 'front_end/**'
7+
- 'api/**'
78
- '.github/workflows/front-end-tests.yml'
89
push:
910
paths:
1011
- 'front_end/**'
12+
- 'api/**'
1113
- '.github/workflows/front-end-tests.yml'
1214

1315
jobs:
14-
test:
16+
front-end-test:
1517
runs-on: ubuntu-latest
1618
permissions:
1719
contents: read
@@ -40,3 +42,42 @@ jobs:
4042
API_URL: https://api.example.invalid
4143
MICROSOFT_PROVIDER_AUTHENTICATION_SECRET: secret
4244
run: pytest
45+
46+
api-test:
47+
runs-on: ubuntu-latest
48+
permissions:
49+
contents: read
50+
51+
steps:
52+
- uses: actions/checkout@v4
53+
54+
- name: Set up Python version
55+
uses: actions/setup-python@v5
56+
with:
57+
python-version: '3.11'
58+
59+
- name: Install dependencies
60+
working-directory: api
61+
run: |
62+
python -m pip install --upgrade pip
63+
pip install -r requirements.txt -r requirements-dev.txt
64+
65+
- name: Run pytest
66+
working-directory: api
67+
env:
68+
TENANT_ID: tenant-id
69+
CLIENT_ID: client-id
70+
VM_SUBSCRIPTION_ID: subscription-id
71+
VM_RESOURCE_GROUP: resource-group
72+
AVD_HOST_GROUP_ID: avd-group-id
73+
LINUX_HOST_GROUP_ID: linux-group-id
74+
DOMAIN_NAME: example.invalid
75+
VAULT_URL: https://vault.example.invalid
76+
KEY_NAME: ssh-key
77+
DB_SERVER: db.example.invalid
78+
DB_DATABASE: LinuxBrokerTest
79+
DB_USERNAME: api-user
80+
DB_PASSWORD_NAME: db-password
81+
MICROSOFT_PROVIDER_AUTHENTICATION_SECRET: provider-secret
82+
NFS_SHARE: /mnt/test
83+
run: pytest

‎api/README.md‎

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
# Broker API
2+
3+
This folder contains the Flask **Broker API** for the Linux Broker for AVD Access solution. It is the control-plane service used by the Service Management Portal, the scheduled scaling task, the AVD host broker, and the Linux host agents. For the full solution architecture and deployment model, see the repository [README](../README.md).
4+
5+
## Purpose
6+
7+
The API brokers Linux host checkouts, records VM state in Azure SQL, manages scaling rules, triggers scaling actions, and delivers the fleet-wide Linux host settings profile. It does not own the database schema; schema and stored procedure changes belong under the [`sql_queries`](../sql_queries/README.md) folder.
8+
9+
## Endpoint Reference
10+
11+
`token_required(...)` grants access when the bearer token has any listed delegated scope or app role. When a group is listed, membership in that configured group also grants access.
12+
13+
| Method | Path | Required scopes, roles, or groups | Description |
14+
| --- | --- | --- | --- |
15+
| GET | `/health` | none | Checks database connectivity and returns API health and version. |
16+
| GET | `/api/version` | none | Returns the API version string. |
17+
| GET | `/api/vms` | `access_as_user`, `FullAccess`, `ScheduledTask` | Lists all broker VM records. |
18+
| GET | `/api/vms/summary` | `access_as_user`, `FullAccess`, `ScheduledTask` | Returns dashboard counters: `TotalVMs`, `Available`, `CheckedOut`, `Maintenance`, `Released`, `PoweredOn`, `PoweredOff`, `Unreachable`, and `Ready`. |
19+
| POST | `/api/vms/checkout` | `AvdHost`, `access_as_user`, `FullAccess`, or `AVD_HOST_GROUP_ID` membership | Checks out a ready Linux host and creates or updates the remote user. |
20+
| POST | `/api/vms/<vmid>/update-attributes` | `ScheduledTask`, `access_as_user`, `FullAccess` | Updates VM power, network, or broker status fields. |
21+
| POST | `/api/vms/<vmid>/delete` | `access_as_user`, `FullAccess` | Deletes a VM record. |
22+
| POST | `/api/vms/add` | `access_as_user`, `FullAccess` | Adds a VM record. |
23+
| GET | `/api/vms/<vmid>` | `access_as_user`, `FullAccess` | Gets one VM record. |
24+
| POST | `/api/vms/<vmid>/return` | `access_as_user`, `FullAccess` | Returns a checked-out VM and removes the remote user when possible. |
25+
| POST | `/api/vms/<hostname>/release` | `LinuxHost`, `access_as_user`, `FullAccess`, or `LINUX_HOST_GROUP_ID` membership | Marks a host-side session released, with optional `username` and `leaseId` validation. |
26+
| POST | `/api/vms/released` | `ScheduledTask`, `access_as_user`, `FullAccess` | Returns expired released VMs to the available pool and removes remote users. |
27+
| POST | `/api/vms/history` | `access_as_user`, `FullAccess` | Returns VM history, optionally paged with `page` and `per_page`. |
28+
| POST | `/api/scaling/log` | `access_as_user`, `FullAccess` | Returns scaling activity history, optionally paged with `page` and `per_page`. |
29+
| POST | `/api/scaling/trigger` | `ScheduledTask`, `access_as_user`, `FullAccess` | Runs scaling logic and starts or stops Azure VMs as directed by SQL. |
30+
| GET | `/api/scaling/rules` | `access_as_user`, `FullAccess` | Lists scaling rules; an empty rule set is `[]` with `200`. |
31+
| GET | `/api/scaling/rules/<int:ruleid>` | `access_as_user`, `FullAccess` | Gets one scaling rule. |
32+
| POST | `/api/scaling/rules/create` | `access_as_user`, `FullAccess` | Creates a scaling rule. |
33+
| POST | `/api/scaling/rules/<int:ruleid>/update` | `access_as_user`, `FullAccess` | Updates a scaling rule. |
34+
| POST | `/api/scaling/rules/<int:ruleid>/delete` | `access_as_user`, `FullAccess` | Deletes a scaling rule. |
35+
| POST | `/api/scaling/rules/history` | `access_as_user`, `FullAccess` | Returns scaling rule history, optionally paged with `page` and `per_page`. |
36+
| GET | `/api/hosts/settings` | `LinuxHost`, `access_as_user`, `FullAccess`, `ScheduledTask`, or `LINUX_HOST_GROUP_ID` membership | Returns the fleet-wide Linux host settings profile. |
37+
| POST | `/api/hosts/settings/update` | `access_as_user`, `FullAccess` | Updates the fleet-wide Linux host settings profile. |
38+
| POST | `/api/hosts/settings/apply` | `access_as_user`, `FullAccess`, `ScheduledTask` | Pushes the current settings profile to reachable hosts over SSH. |
39+
| POST | `/api/hosts/<hostname>/settings/ack` | `LinuxHost`, `access_as_user`, `FullAccess`, or `LINUX_HOST_GROUP_ID` membership | Records the settings version applied by one host. |
40+
41+
`/api/vms/available` is not present in `app.py`; do not add new callers for it.
42+
43+
## Consumers
44+
45+
These callers constrain response shapes and endpoint compatibility.
46+
47+
| Consumer | Endpoints |
48+
| --- | --- |
49+
| `front_end` portal | VM, scaling, and host-settings endpoints. The dashboard prefers `/api/vms/summary`; history pages request `page` and `per_page`. |
50+
| `task\function_app.py` | `/api/vms`, `/api/vms/released`, `/api/vms/<vmid>/update-attributes`, `/api/scaling/trigger` |
51+
| Linux host release agent (`linux_host\...\release-session.sh`) | `/api/vms/<hostname>/release` |
52+
| AVD host (`avd_host\...\Connect-LinuxBroker.ps1`) | `/api/vms/checkout` |
53+
| Linux host settings agent | `/api/hosts/settings`, `/api/hosts/<hostname>/settings/ack` |
54+
55+
## Authentication and Authorization
56+
57+
Clients send Entra ID bearer tokens in the HTTP `Authorization` header. `token_required()` validates the token signature against the tenant JWKS, accepts audiences `CLIENT_ID` and `api://<CLIENT_ID>`, and accepts issuers:
58+
59+
- `{AUTHORITY_HOST}/{TENANT_ID}/v2.0`
60+
- `{AUTHORITY_HOST}/{TENANT_ID}/`
61+
- `{STS_ISSUER_HOST}/{TENANT_ID}/`
62+
63+
Authorization then checks delegated scopes in `scp`, app roles in `roles`, and optional group membership through Microsoft Graph `checkMemberGroups` using the token `oid`.
64+
65+
Cloud endpoints are resolved in [`config.py`](config.py). `AZURE_CLOUD_NAME=AzurePublic` uses `login.microsoftonline.com`, `graph.microsoft.com`, and `sts.windows.net`. `AzureUSGovernment` uses `login.microsoftonline.us` and `graph.microsoft.us`. Any custom or sovereign cloud without a built-in profile must set `AZURE_AUTHORITY_HOST`, `GRAPH_ENDPOINT`, and `STS_ISSUER_HOST` explicitly.
66+
67+
## Error Responses and Logging
68+
69+
Handler failures use a JSON error envelope:
70+
71+
```json
72+
{"error": "Unable to retrieve virtual machines."}
73+
```
74+
75+
Exception detail must not be returned in the response body. Log details with the `linuxbroker.api` logger; when `APPLICATIONINSIGHTS_CONNECTION_STRING` is set, that logger is configured for Azure Monitor. Authentication middleware and `/health` have their own fixed response shapes, but application handler errors should use the envelope.
76+
77+
## Pagination Contract
78+
79+
`/api/vms/history`, `/api/scaling/log`, and `/api/scaling/rules/history` support opt-in pagination. Supplying either `page` or `per_page` in the query string returns an envelope:
80+
81+
```http
82+
POST /api/vms/history?page=2&per_page=25
83+
Content-Type: application/json
84+
85+
{"startdate":"08/01/2026","enddate":"08/19/2026"}
86+
```
87+
88+
```json
89+
{
90+
"items": [
91+
{"VMID": 42, "Hostname": "linux-01"}
92+
],
93+
"page": 2,
94+
"per_page": 25,
95+
"total": 91,
96+
"total_pages": 4
97+
}
98+
```
99+
100+
When neither `page` nor `per_page` is present, the response remains a bare JSON array. Do not remove that default: `task\function_app.py` and older portal builds consume these endpoints as plain lists. The unpaged path also deliberately tolerates `"null"` for `limit`; older portal builds sent that sentinel for **No Limit**, and rolling deployments must not turn it into a SQL `INT` conversion failure.
101+
102+
Empty collection responses are arrays with `200`, including `/api/scaling/rules`, `/api/scaling/log`, and `/api/scaling/rules/history`.
103+
104+
## VM Summary
105+
106+
`GET /api/vms/summary` returns fixed-size dashboard counters instead of requiring the portal to fetch every VM. `Ready` uses the same condition as checkout host selection: `VmStatus='Available'`, `PowerState='On'`, and `NetworkStatus='Reachable'`.
107+
108+
## Configuration
109+
110+
The API reads environment variables directly; it does not load `.env` files by itself. [`env.example`](env.example) shows the deployment settings.
111+
112+
| Variable | Required | Purpose |
113+
| --- | --- | --- |
114+
| `SCM_DO_BUILD_DURING_DEPLOYMENT` | deployment | Enables App Service build during deployment. |
115+
| `APPLICATIONINSIGHTS_CONNECTION_STRING` | optional | Enables Azure Monitor/OpenTelemetry export for `linuxbroker.api`. |
116+
| `ApplicationInsightsAgent_EXTENSION_VERSION` | optional | App Service Application Insights extension version. |
117+
| `APPLICATIONINSIGHTSAGENT_EXTENSION_ENABLED` | optional | Enables the App Service Application Insights extension. |
118+
| `WEBSITE_HTTPLOGGING_RETENTION_DAYS` | optional | App Service HTTP log retention. |
119+
| `VM_SUBSCRIPTION_ID` | required for scaling | Azure subscription used by `/api/scaling/trigger`. |
120+
| `VM_RESOURCE_GROUP` | required for scaling | Resource group containing Linux host VMs. |
121+
| `AVD_HOST_GROUP_ID` | required for AVD host group auth | Entra group whose members may call checkout. |
122+
| `LINUX_HOST_GROUP_ID` | required for Linux host group auth | Entra group whose members may call release and host-settings ack/read endpoints. |
123+
| `LINUX_HOST_ADMIN_LOGIN_NAME` | optional | SSH admin user prefix for remote host commands; defaults to `avdadmin`. |
124+
| `DB_SERVER` | required | Azure SQL Server name or FQDN for `pymssql`. |
125+
| `DB_DATABASE` | required | Azure SQL database name. |
126+
| `DB_USERNAME` | required | SQL login name. |
127+
| `DB_PASSWORD_NAME` | required | Key Vault secret name containing the SQL password. |
128+
| `CLIENT_ID` | required | Broker API app registration client ID and accepted token audience. |
129+
| `TENANT_ID` | required | Entra tenant used for token validation and Graph calls. |
130+
| `AZURE_CLOUD_NAME` | optional | Cloud profile name; defaults to `AzurePublic`. |
131+
| `AZURE_AUTHORITY_HOST` | required for `AzureCustom` | Login authority host override. |
132+
| `GRAPH_ENDPOINT` | required for `AzureCustom` | Microsoft Graph endpoint override. |
133+
| `STS_ISSUER_HOST` | required for `AzureCustom` | STS issuer host override. |
134+
| `GRAPH_API_ENDPOINT` | optional | Legacy Graph scope setting in `config.py`; current token acquisition uses `GRAPH_ENDPOINT`. |
135+
| `MICROSOFT_PROVIDER_AUTHENTICATION_SECRET` | required | Client secret used by the API to call Graph for group checks. |
136+
| `DOMAIN_NAME` | required for SSH actions | DNS suffix used to build `<admin>@<hostname>.<domain>`. |
137+
| `VAULT_URL` | required | Key Vault URL for SQL password and SSH key retrieval. |
138+
| `KEY_NAME` | required for SSH actions | Key Vault secret name containing the PEM SSH private key. |
139+
| `NFS_SHARE` | required for checkout provisioning | NFS share argument passed to `create-user.sh`; used by code but not currently listed in `env.example`. |
140+
141+
## Database Access
142+
143+
Handlers call stored procedures rather than embedding schema logic in Python. `db_connection()` wraps `get_db_connection()` as a context manager so every acquired connection is closed on success or exception.
144+
145+
Keep schema and procedure changes in numbered files under [`sql_queries`](../sql_queries/README.md). The deployment bootstrap applies those scripts in filename order and rewrites procedures to `CREATE OR ALTER PROCEDURE` for reruns.
146+
147+
## Local Development and Tests
148+
149+
Install runtime dependencies from this folder:
150+
151+
```powershell
152+
cd .\api
153+
py -m venv .venv
154+
.\.venv\Scripts\Activate.ps1
155+
pip install -r requirements.txt
156+
python .\app.py
157+
```
158+
159+
Set the required environment variables first. For local test runs, install the dev requirements and run pytest from the `api` folder:
160+
161+
```powershell
162+
pip install -r requirements.txt -r requirements-dev.txt
163+
pytest
164+
```
165+
166+
`api\tests\` contains pytest regression coverage for the hardened API paths, including connection cleanup, error envelopes, empty collections, VM summary, and paged history responses.

0 commit comments

Comments
 (0)