Load restapi_routes from config into runtime on startup#5467
Load restapi_routes from config into runtime on startup#5467renecannao merged 8 commits intov3.0from
restapi_routes from config into runtime on startup#5467Conversation
Co-authored-by: renecannao <3645227+renecannao@users.noreply.github.com> Agent-Logs-Url: https://github.com/sysown/proxysql/sessions/40d309f0-bd3d-4616-9cd6-fa7d8f1cb355
Co-authored-by: renecannao <3645227+renecannao@users.noreply.github.com> Agent-Logs-Url: https://github.com/sysown/proxysql/sessions/40d309f0-bd3d-4616-9cd6-fa7d8f1cb355
Co-authored-by: renecannao <3645227+renecannao@users.noreply.github.com> Agent-Logs-Url: https://github.com/sysown/proxysql/sessions/40d309f0-bd3d-4616-9cd6-fa7d8f1cb355
restapi_routes from config into runtime on startup
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThese changes implement automatic loading of REST API routes from the ProxySQL config file to runtime on startup. The feature supports optional Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1. Validate timeout_ms against the CHECK constraint (100..100000000) and skip routes with invalid or missing values with an error message. Previously, omitting timeout_ms defaulted to 0, which silently failed the SQLite CHECK constraint and dropped the route. 2. Update Write_Restapi_to_configfile to emit 'restapi_routes:' instead of 'restapi:' so save-then-reload cycles use the preferred section name consistently.
a62cfd6 to
d1bb48e
Compare
…restapi-routes-config
…restapi-routes-config # Conflicts: # test/tap/groups/groups.json
|
There was a problem hiding this comment.
Pull request overview
Aligns REST API route loading with the config formats used in deployments so that restapi_routes defined in proxysql.cnf are inserted into restapi_routes on startup and therefore show up in runtime_restapi_routes without requiring manual admin SQL.
Changes:
- Update RESTAPI config read/write to prefer
restapi_routes(while still accepting legacyrestapi) and allow omittingid. - Harden config import by validating
timeout_ms, escaping string fields before SQLite insertion, and adding allocation checks/bounded formatting. - Add TAP coverage for
LOAD RESTAPI FROM CONFIGvalidation and for startup population ofruntime_restapi_routes.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
lib/ProxySQL_Config.cpp |
Reads restapi_routes/restapi from config and writes routes back out as restapi_routes; adds escaping and validation during import. |
test/tap/tests/test_load_restapi_from_config_startup-t.cpp |
New startup-focused TAP test that launches a secondary ProxySQL from a generated config and asserts runtime_restapi_routes is populated. |
test/tap/tests/test_load_from_config_validation-t.cpp |
Extends existing config-load validation coverage to include LOAD RESTAPI FROM CONFIG. |
test/tap/groups/groups.json |
Registers the new startup TAP test in existing TAP groups. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| //fprintf(stderr, "Found %d users\n",count); | ||
| int i; | ||
| int rows=0; | ||
| admindb->execute("PRAGMA foreign_keys = OFF"); |
There was a problem hiding this comment.
Allowing config routes without id means LOAD RESTAPI FROM CONFIG (and startup with --reload + persistent datadir) will INSERT new rows every time, because restapi_routes has no UNIQUE constraint besides the autoincrement id. This can accumulate duplicate (method, uri) entries and break REST API routing (the runtime lookup expects exactly 1 matching row). Consider making config import idempotent when id is omitted (e.g., clear restapi_routes before importing, or upsert by (method, uri) via a deterministic id / UNIQUE index).
| admindb->execute("PRAGMA foreign_keys = OFF"); | |
| admindb->execute("PRAGMA foreign_keys = OFF"); | |
| // Make import idempotent: clear existing routes so repeated LOAD RESTAPI FROM CONFIG | |
| // (or reload on startup) does not accumulate duplicate entries. | |
| admindb->execute("DELETE FROM restapi_routes"); |


restapi_routesdefined inproxysql.cnfwere parsed inconsistently and did not support the config shape used in deployment manifests, which leftruntime_restapi_routesempty after startup. This change aligns REST API route loading with the expected config format and startup behavior used by other config-backed sections.Config parsing
restapi_routesas a config section, while preserving support for the legacyrestapisection.id, matching the common SQL workaround and letting SQLite assign it.Runtime startup behavior
restapi_routesin the shape expected by the existing runtime load path.Safety hardening in the touched path
Coverage
LOAD RESTAPI FROM CONFIG.runtime_restapi_routesis populated.Example config now supported directly:
With this format, the route is loaded into
restapi_routesfrom config and becomes available inruntime_restapi_routeson startup.Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
cdn.mysql.com/usr/bin/curl curl -C - -O -s REDACTED(dns block)/usr/bin/wget wget -nc -q REDACTED(dns block)If you need me to access, download, or install something from one of these locations, you can either:
Original prompt
💬 Send tasks to Copilot coding agent from Slack and Teams to turn conversations into code. Copilot posts an update in your thread when it's finished.
Summary by CodeRabbit
New Features
restapi_routessection with fallback torestapifor backward compatibility.Improvements
Tests