This is an automated email from the ASF dual-hosted git repository. wu-sheng pushed a commit to branch feat/template-modes-env-config in repository https://gitbox.apache.org/repos/asf/skywalking-horizon-ui.git
commit fb614baeeb87ee3fbf8c9fa338b0b96a1a2e3e05 Author: Wu Sheng <[email protected]> AuthorDate: Fri Jun 26 09:59:27 2026 +0800 fix(readonly): close alert-page write gap, yellow read-only banner, stronger env parity Three review findings + a UX fix: - HIGH: the alert page-setup (/api/alarms/config) was still writable in readonly. The readonly backstop only covered /api/admin/templates* and /api/admin/overview-templates*; /api/alarms/config writes the horizon.alert.page-setup template (file-backed) and slipped through. It's now in the config-surface write set — a direct POST returns 409 in readonly — and GET serves the bundled alert page-setup (like every other template) instead of the local store. Validated live: POST 409, GET serves the bundle. - The read-only SyncStatusBanner was muted gray ("white") and easy to miss next to the green per-row sync chips. It's now warning-yellow so it's unmistakable that the whole config surface is bundle-served and uneditable. (The per-row "synced" chip is correct as-is: in readonly the bundle IS the presented remote, so bundled == remote and the diff legitimately reads synced.) - The env parity guard only checked top-level sections + four hard-coded tokens. It now recursively walks every schema-default field and asserts each is covered by a ${HORIZON_…} token in horizon.example.yaml (a scalar token, or a JSON-env token for a whole subtree). It immediately caught a real gap — oap.mqe shipped untokenized — now fixed (HORIZON_OAP_MQE). A future nested field without env coverage will now fail the test. type-check / lint / license / 162 BFF + 116 UI tests / builds green. --- apps/bff/src/config/schema.test.ts | 36 ++++++++++++++++++++++ apps/bff/src/http/config/alarms.ts | 7 ++++- apps/bff/src/rbac/route-policy.test.ts | 6 ++-- apps/bff/src/rbac/route-policy.ts | 16 +++++++--- .../features/admin/_shared/SyncStatusBanner.vue | 11 ++++--- horizon.example.yaml | 3 ++ 6 files changed, 66 insertions(+), 13 deletions(-) diff --git a/apps/bff/src/config/schema.test.ts b/apps/bff/src/config/schema.test.ts index 9d4a7d0..74bc26d 100644 --- a/apps/bff/src/config/schema.test.ts +++ b/apps/bff/src/config/schema.test.ts @@ -73,4 +73,40 @@ describe('horizon.example.yaml — tokenized default + parity', () => { expect(raw).toContain('${HORIZON_TEMPLATES_MODE'); expect(raw).toContain('${HORIZON_OAP_ADMIN_URL'); }); + + // The real contract: EVERY schema-default field is env-overridable. Walk the + // fully-defaulted config and assert each node is covered in the example by a + // `${HORIZON_…}` token — either a scalar token at that path, or a JSON-env + // token standing in for a whole subtree (performance, rbac.roles, …). Catches + // a new nested field shipped without env coverage, which the top-level-section + // check above would miss. + it('every schema-default field is env-tokenized in the example (recursive)', () => { + const defaults = configSchema.parse({}) as Record<string, unknown>; + const example = (YAML.parse(raw) ?? {}) as Record<string, unknown>; + const TOKEN = /\$\{HORIZON_[A-Z0-9_]+(:[\s\S]*)?\}/; + const uncovered: string[] = []; + const walk = (schemaNode: unknown, exampleNode: unknown, path: string): void => { + // A token string covers this node (scalar token, or a JSON-env token + // collapsing a whole subtree to one var). + if (typeof exampleNode === 'string' && TOKEN.test(exampleNode)) return; + if (schemaNode !== null && typeof schemaNode === 'object' && !Array.isArray(schemaNode)) { + if (exampleNode === null || typeof exampleNode !== 'object' || Array.isArray(exampleNode)) { + uncovered.push(`${path} (section absent from example)`); + return; + } + for (const key of Object.keys(schemaNode)) { + if (path === '' && key === 'infra3d') continue; // deprecated passthrough, never tokenized + walk( + (schemaNode as Record<string, unknown>)[key], + (exampleNode as Record<string, unknown>)[key], + path ? `${path}.${key}` : key, + ); + } + return; + } + uncovered.push(`${path} (leaf not tokenized)`); + }; + walk(defaults, example, ''); + expect(uncovered, `fields lacking an env token in horizon.example.yaml:\n ${uncovered.join('\n ')}`).toEqual([]); + }); }); diff --git a/apps/bff/src/http/config/alarms.ts b/apps/bff/src/http/config/alarms.ts index 1b1e5c3..467259d 100644 --- a/apps/bff/src/http/config/alarms.ts +++ b/apps/bff/src/http/config/alarms.ts @@ -30,6 +30,8 @@ import type { ConfigSource } from '../../config/loader.js'; import type { SessionStore } from '../../user/sessions.js'; import type { AuditLogger } from '../../audit/logger.js'; import { requireAuth } from '../../user/middleware.js'; +import { isTemplateReadOnly } from '../../logic/templates/sync.js'; +import { loadBundledAlertPageSetup } from '../../logic/alarms/bundled.js'; import type { ServiceLayerCatalog } from '../../logic/services/service-layer-catalog.js'; import { ALARMS_WINDOW_OPTIONS, @@ -77,7 +79,10 @@ export function registerAlarmsConfigRoutes( '/api/alarms/config', { preHandler: auth }, async (_req: FastifyRequest, reply: FastifyReply) => { - const cfg = await deps.store.load(); + // Readonly mode serves the alert page-setup from the bundle like every + // other template; the local store (a prior live session's edits) is not + // the source here. Writes are denied by the route-policy backstop. + const cfg = isTemplateReadOnly() ? loadBundledAlertPageSetup() : await deps.store.load(); return reply.send(cfg); }, ); diff --git a/apps/bff/src/rbac/route-policy.test.ts b/apps/bff/src/rbac/route-policy.test.ts index 67f3242..750dc3a 100644 --- a/apps/bff/src/rbac/route-policy.test.ts +++ b/apps/bff/src/rbac/route-policy.test.ts @@ -27,12 +27,14 @@ describe('isTemplateWriteRoute — which routes the readonly backstop covers', ( expect(isTemplateWriteRoute('POST', '/api/admin/templates/sync-all')).toBe(true); expect(isTemplateWriteRoute('POST', '/api/admin/overview-templates')).toBe(true); expect(isTemplateWriteRoute('DELETE', '/api/admin/overview-templates/x')).toBe(true); + // The alert page-setup (horizon.alert.page-setup) is config-surface too. + expect(isTemplateWriteRoute('POST', '/api/alarms/config')).toBe(true); }); - it('does NOT match reads, nor non-template writes (runtime-rule / local state stay editable)', () => { + it('does NOT match reads, nor non-config writes (runtime-rule / live-debug stay editable)', () => { expect(isTemplateWriteRoute('GET', '/api/admin/templates/sync-status')).toBe(false); expect(isTemplateWriteRoute('HEAD', '/api/admin/templates/sync-status')).toBe(false); + expect(isTemplateWriteRoute('GET', '/api/alarms/config')).toBe(false); // read stays open expect(isTemplateWriteRoute('POST', '/api/rule')).toBe(false); // runtime rule - expect(isTemplateWriteRoute('POST', '/api/alarms/config')).toBe(false); // local state expect(isTemplateWriteRoute('POST', '/api/debug/session')).toBe(false); // live-debug }); }); diff --git a/apps/bff/src/rbac/route-policy.ts b/apps/bff/src/rbac/route-policy.ts index fb3d617..257a74b 100644 --- a/apps/bff/src/rbac/route-policy.ts +++ b/apps/bff/src/rbac/route-policy.ts @@ -33,13 +33,19 @@ import { logger } from '../logger.js'; export type RoutePolicy = 'public' | 'auth' | string; -/** A config-template write — it pushes to OAP's ui_template store. In - * `templates.mode=readonly` there is no store to write to, so these are denied - * at the edge regardless of the verb grant (the UI hides them too, but a direct - * request must still fail — the BFF is the authority). */ +/** A config-surface write. The template routes push to OAP's ui_template store; + * `/api/alarms/config` writes the alert page-setup (the `horizon.alert.page-setup` + * template, file-backed). In `templates.mode=readonly` the whole config surface + * is served from the local bundle and read-only, so these are denied at the edge + * regardless of the verb grant (the UI hides them too, but a direct request must + * still fail — the BFF is the authority). */ export function isTemplateWriteRoute(method: string, url: string): boolean { if (method === 'GET' || method === 'HEAD') return false; - return url.startsWith('/api/admin/templates') || url.startsWith('/api/admin/overview-templates'); + return ( + url.startsWith('/api/admin/templates') || + url.startsWith('/api/admin/overview-templates') || + url.startsWith('/api/alarms/config') + ); } export async function denyTemplateWriteWhenReadOnly( _req: FastifyRequest, diff --git a/apps/ui/src/features/admin/_shared/SyncStatusBanner.vue b/apps/ui/src/features/admin/_shared/SyncStatusBanner.vue index 69a52c6..8f84279 100644 --- a/apps/ui/src/features/admin/_shared/SyncStatusBanner.vue +++ b/apps/ui/src/features/admin/_shared/SyncStatusBanner.vue @@ -124,13 +124,14 @@ export default { chipLabel }; border-color: var(--sw-muted, #4a525c); background: var(--sw-bg-elev, #161a20); } -/* Deliberate read-only (templates.mode=readonly) — informational, not an - * error, so muted rather than the danger red of `unreachable`. */ +/* Deliberate read-only (templates.mode=readonly) — not an error (no red), but + * warning-yellow so it's unmistakable that the whole surface is uneditable; a + * muted gray strip was too easy to miss next to the green per-row chips. */ .sbb--readonly { - border-color: var(--sw-muted, #4a525c); - background: rgba(255, 255, 255, 0.03); + border-color: var(--sw-warn, #b88500); + background: rgba(184, 133, 0, 0.10); } -.sbb--readonly .sbb__chip { background: var(--sw-muted, #4a525c); color: #fff; } +.sbb--readonly .sbb__chip { background: var(--sw-warn, #b88500); color: #1a1a1a; } .sbb__row { display: flex; gap: 12px; diff --git a/horizon.example.yaml b/horizon.example.yaml index d3c045e..4da1e8f 100644 --- a/horizon.example.yaml +++ b/horizon.example.yaml @@ -58,6 +58,9 @@ oap: # GraphQL port (typical for the demo / k8s deploys). zipkinUrl: "${HORIZON_OAP_ZIPKIN_URL:http://127.0.0.1:9412/zipkin}"; timeoutMs: ${HORIZON_OAP_TIMEOUT_MS:15000} + # Optional MQE host/port override (defaults to the query host). JSON env, e.g. + # HORIZON_OAP_MQE='{"host":"mqe.internal","port":12800}'. + mqe: ${HORIZON_OAP_MQE:null} # Optional basic-auth for outbound OAP calls. JSON env, e.g. # HORIZON_OAP_AUTH='{"username":"skywalking","password":"skywalking"}'. auth: ${HORIZON_OAP_AUTH:null}
