mikebridge opened a new pull request, #45550:
URL: https://github.com/apache/superset/pull/45550
### SUMMARY
**What users saw.** With `ENABLE_EXTENSIONS` turned on through a runtime
flag hook after the server started, the menu showed **Settings → Manage →
Extensions**, but opening it failed with an error that did not name the cause.
Depending on how a deployment presents API errors, it could read as a database
problem. The browser console showed "Error setting up extensions", and
extensions never loaded. Every `/api/v1/extensions/` call returned 404.
**Why.** The flag was read at two different times. The Extensions menu, the
frontend route and the frontend extension loader check it on every request. The
extensions REST APIs were registered only if the flag was already on when the
server started. So a flag turned on later showed the UI with nothing behind it.
The reverse also held: a server that started with the flag on exposed the
extensions APIs to every request, even ones whose runtime decision was off.
**What this changes.**
- Both extensions APIs (`ExtensionsRestApi`, `ExtensionStorageRestApi`) are
always registered. Each request checks `ENABLE_EXTENSIONS` before
authentication or any handler work. When the flag is off, every route returns
404, matching the hidden menu, and the browser never receives a bundle list or
runs extension code. Semantic-layer types contributed by extensions are hidden
as well: from the types list and the configuration schema, and layers of those
types cannot be created or reconfigured. This follows #45061, which did the
same for `SEMANTIC_LAYERS`.
- Extension **loading** is unchanged: extension code still loads only when
the flag is on at startup. If a request turns the flag on but the server did
not load extensions:
- `GET /api/v1/extensions/` returns an empty list with `status:
"not_loaded"`.
- `GET /api/v1/extensions/_info` reports the same status.
- The Extensions page shows "Extensions not loaded — restart the server to
load them" instead of a misleading error. The page takes the rows and the
notice from the same list response. During a rolling restart, when some servers
have loaded extensions and others have not, the notice always matches the list
on screen.
- `GET /api/v1/semantic_layer/types` reports `extensions_status`.
- No extension bundle, chunk or storage route is served from such a server.
- The startup log records whether extensions were loaded, how many loaded
and how many failed to initialize. If extension discovery fails at startup, the
log records the failure with its traceback.
- The extensions APIs now keep CSRF protection like other Superset APIs.
They subclassed FAB's `BaseApi`, which exempts by default, so the storage write
routes (PUT/DELETE) were CSRF-exempt on servers that started with the flag on.
The frontend already sends the token.
- The storage API docstrings mixed `'` and `"` in descriptions, which FAB's
OpenAPI round-trip couldn't parse. That was harmless only while the API was
rarely registered; it is fixed so `superset update-api-docs` and
`/api/v1/_openapi` keep working. The list endpoint's response schema now
appears in the spec: its docstring key was `get_list:`, which FAB ignores.
**Not covered.** Anything an extension backend registers at startup, other
than semantic-layer types, is not gated per request: its own REST APIs, its MCP
tools, and queries against semantic layers that already use an extension type
keep working on a server that loaded them. Gating those would need
per-extension hooks and is left for a follow-up. The startup setting remains
the way to keep that code from loading at all. "Loaded" is tracked per server,
not per extension.
**Permissions.** Permission names and role rules are unchanged. Because the
APIs are always registered, `superset init` now creates their existing
permissions on deployments where the flag was off. Admin, Alpha and Gamma
receive them, as on deployments that already started with the flag on, and they
have no effect while the flag is off. #45061 made the same trade-off. Upgrade
order: deploy, run `superset init`, then enable the flag at runtime.
**Rollout.** Turning the flag off per request hides extensions immediately.
Stopping extension code from running still needs the startup setting turned off
and a restart. Hosts that change which value the flag has at startup should
ship that change after this one, and roll it back first.
@Antonio-RiveroMartnez, could you review this when you have a chance?
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before: Extensions page with the flag turned on after startup → error toast;
`GET /api/v1/extensions/_info` → 404.
After: same setup → page loads with the "Extensions not loaded" notice;
`_info` → 200 `{"permissions": ["can_read"], "status": "not_loaded", "message":
"..."}`.
### TESTING INSTRUCTIONS
1. Start Superset with `FEATURE_FLAGS = {"ENABLE_EXTENSIONS": False}`.
Supply a runtime decision that returns `True` for `ENABLE_EXTENSIONS`, for
example `IS_FEATURE_ENABLED_FUNC = lambda name, default: True if name ==
"ENABLE_EXTENSIONS" else default`. Open Settings → Manage → Extensions as an
admin: the page shows the restart notice, and `/api/v1/extensions/` returns an
empty list with `status: "not_loaded"`.
2. Restart with `ENABLE_EXTENSIONS: True` at startup: the list returns the
configured extensions with `status: "loaded"`, and the log shows "Loaded N
extension(s) at startup; 0 failed to initialize".
3. With the flag on at startup, make the runtime decision return `False`:
the menu is hidden, every `/api/v1/extensions/...` route returns 404, and
extension semantic-layer types disappear from `/api/v1/semantic_layer/types`.
4. Automated:
- `pytest tests/unit_tests/extensions
tests/unit_tests/test_init_extensions_logging.py
tests/unit_tests/security/api_test.py` covers:
- all four startup × request combinations, and runtime flips both ways;
- menu/API agreement;
- extension semantic types in the list, schema, create and update paths;
- authentication, unchanged permission names and method permissions;
- CSRF non-exemption;
- startup load marking.
- `npm run test -- src/extensions/ExtensionsList.test.tsx` covers the
notice, including list and `_info` responses from servers in different load
states, and the error path.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [x] Required feature flags: `ENABLE_EXTENSIONS`
- [x] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
