Skip to content

Commit 2fba0ff

Browse files
authored
fix(openmetrics): keep the Prometheus secret across xo-server restarts (#10290)
* fix(openmetrics): keep the Prometheus secret across xo-server restarts The `secret` property of `configurationSchema` used a random `default`. That expression is re-evaluated every time the module is loaded, and xo-server never persists the values ajv fills in from schema defaults, so each restart handed the metrics endpoint a brand new bearer token and Prometheus started getting 401s. Drop the default and generate the secret in `load()` instead, saving it through `xo.configurePlugin()` so it survives a restart. Introduced by #9323 See https://xcp-ng.org/forum/topic/12415 * chore(changelog): add openmetrics secret entry * fix(xo-server-openmetrics): generate a 256-bit Prometheus secret Buffer.from(getRandomValues(new Uint32Array(8))) coerces each 32-bit element to a single byte, so the generated secret held 64 bits of entropy instead of 256. Use randomBytes(32) instead. Introduced by fe29bd0
1 parent 62cc3e3 commit 2fba0ff

5 files changed

Lines changed: 92 additions & 8 deletions

File tree

‎@vates/types/src/xo-app.mts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,8 @@ export type XoApp = {
204204
backupGuard(poolId: XoPool['id']): Promise<void>
205205
/* Throw if no authorization */
206206
checkFeatureAuthorization(featureCode: FeatureCode): Promise<void>
207+
/* validate, apply and persist the configuration of a plugin */
208+
configurePlugin(id: string, configuration: unknown, mergeWithExisting?: boolean): Promise<void>
207209
/* connect a server (XCP-ng/XenServer) */
208210
connectXenServer(id: XoServer['id']): Promise<void>
209211
// TODO: replace all XoAclBasePrivilege with a more strict type. (discriminate union)

‎CHANGELOG.unreleased.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919

2020
> Users must be able to say: "I had this issue, happy to know it's fixed"
2121
22+
- [OpenMetrics] The Prometheus secret no longer changes on every xo-server restart: it is now generated once and saved in the plugin configuration (PR [#10290](https://github.com/vatesfr/xen-orchestra/pull/10290))
23+
2224
### Packages to release
2325

2426
> When modifying a package, add it here with its release type.
@@ -37,6 +39,7 @@
3739

3840
- @xen-orchestra/web minor
3941
- xo-server minor
42+
- xo-server-openmetrics patch
4043
- xo-web minor
4144

4245
<!--packages-end-->

‎docs/docs/xo5/advanced.md‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -403,14 +403,14 @@ The OpenMetrics plugin exposes a `/metrics` endpoint that Prometheus can scrape
403403
2. Find and enable the **OpenMetrics** plugin.
404404
3. Configure the following options:
405405

406-
| Option | Default | Description |
407-
| --------------------- | ---------- | --------------------------------------------------------- |
408-
| **Prometheus secret** | (required) | Bearer token for authentication - you must set this value |
406+
| Option | Default | Description |
407+
| --------------------- | ----------- | -------------------------------------------------------------------------- |
408+
| **Prometheus secret** | (generated) | Bearer token for authentication - generated on first load if left empty |
409409

410410
4. Save and load the plugin.
411411

412-
:::warning
413-
You must set a **Prometheus secret** before loading the plugin. Without a valid secret, the metrics endpoint authentication is ineffective, potentially exposing sensitive infrastructure data (host resources, VM configurations, network details) to unauthorized access. Use a strong, random string (e.g., generated with `openssl rand -hex 32`).
412+
:::tip
413+
If you leave the **Prometheus secret** empty, one is generated the first time the plugin is loaded and saved in the plugin configuration, where you can read it back to configure Prometheus. You can also set your own strong, random string (e.g., generated with `openssl rand -hex 32`). Either way the secret is kept across xo-server restarts.
414414
:::
415415

416416
### Prometheus Configuration

‎packages/xo-server-openmetrics/src/index.mts‎

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import { createLogger } from '@xen-orchestra/log'
2525
import { fork, type ChildProcess } from 'node:child_process'
2626
import { dirname, join } from 'node:path'
2727
import { fileURLToPath } from 'node:url'
28-
import { getRandomValues } from 'node:crypto'
28+
import { randomBytes } from 'node:crypto'
2929
import { performance } from 'node:perf_hooks'
3030
import v8 from 'node:v8'
3131

@@ -415,6 +415,9 @@ const __dirname = dirname(__filename)
415415

416416
const logger = createLogger('xo:xo-server-openmetrics')
417417

418+
/** Id under which xo-server registers this plugin (directory name minus the `xo-server-` prefix) */
419+
const PLUGIN_ID = 'openmetrics'
420+
418421
/** Default port for the OpenMetrics HTTP server */
419422
const DEFAULT_PORT = 9004
420423

@@ -481,12 +484,36 @@ export const configurationSchema = {
481484
type: 'string',
482485
title: 'Prometheus secret',
483486
description: 'Add this secret to http_config > authorization > credentials, and set type to Bearer',
484-
default: Buffer.from(getRandomValues(new Uint32Array(8))).toString('hex'),
485487
},
486488
},
487489
additionalProperties: false,
488490
}
489491

492+
/**
493+
* Return the Prometheus bearer token, generating and persisting one on first use.
494+
*
495+
* The secret must survive an xo-server restart. It used to be a random
496+
* `default` in `configurationSchema`: that expression is re-evaluated every
497+
* time the module is loaded, and xo-server never saves the values it fills in
498+
* from schema defaults, so each restart silently invalidated the token
499+
* Prometheus was configured with.
500+
*
501+
* Exported for testability.
502+
*/
503+
export async function ensureSecret(
504+
configuration: PluginConfiguration | undefined,
505+
persist: (configuration: PluginConfiguration) => Promise<void>
506+
): Promise<string> {
507+
const secret = configuration?.secret
508+
if (secret !== undefined && secret !== '') {
509+
return secret
510+
}
511+
512+
const generated = randomBytes(32).toString('hex')
513+
await persist({ secret: generated })
514+
return generated
515+
}
516+
490517
// ============================================================================
491518
// XOSTOR helpers (module scope)
492519
// ============================================================================
@@ -779,11 +806,15 @@ class OpenMetricsPlugin {
779806
return
780807
}
781808

809+
const secret = await ensureSecret(this.#configuration, configuration =>
810+
this.#xo.configurePlugin(PLUGIN_ID, configuration, true)
811+
)
812+
782813
// Port and bindAddress are fixed for security (server is behind xo-server proxy)
783814
const serverConfig: ServerConfiguration = {
784815
port: DEFAULT_PORT,
785816
bindAddress: DEFAULT_BIND_ADDRESS,
786-
secret: this.#configuration?.secret ?? '',
817+
secret,
787818
}
788819

789820
logger.info('Starting OpenMetrics server', {
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
/**
2+
* Tests for the plugin configuration (Prometheus secret handling)
3+
*/
4+
5+
import assert from 'node:assert/strict'
6+
import { describe, it } from 'node:test'
7+
8+
import { configurationSchema, ensureSecret } from './index.mjs'
9+
10+
describe('configurationSchema', () => {
11+
it('does not generate the secret as a schema default', () => {
12+
// a `default` here is re-evaluated on every module load and never persisted
13+
// by xo-server, which used to change the Prometheus secret at each restart
14+
assert.equal((configurationSchema.properties.secret as Record<string, unknown>).default, undefined)
15+
})
16+
})
17+
18+
describe('ensureSecret', () => {
19+
it('generates and persists a secret when none is configured', async () => {
20+
const persisted: unknown[] = []
21+
const secret = await ensureSecret(undefined, async configuration => {
22+
persisted.push(configuration)
23+
})
24+
25+
assert.match(secret, /^[0-9a-f]{64}$/)
26+
assert.deepEqual(persisted, [{ secret }])
27+
})
28+
29+
it('generates and persists a secret when the configured one is empty', async () => {
30+
const persisted: unknown[] = []
31+
const secret = await ensureSecret({ secret: '' }, async configuration => {
32+
persisted.push(configuration)
33+
})
34+
35+
assert.match(secret, /^[0-9a-f]{64}$/)
36+
assert.deepEqual(persisted, [{ secret }])
37+
})
38+
39+
it('keeps the configured secret and persists nothing', async () => {
40+
const persisted: unknown[] = []
41+
const secret = await ensureSecret({ secret: 'cafecafecafecafe' }, async configuration => {
42+
persisted.push(configuration)
43+
})
44+
45+
assert.equal(secret, 'cafecafecafecafe')
46+
assert.deepEqual(persisted, [])
47+
})
48+
})

0 commit comments

Comments
 (0)