-
Notifications
You must be signed in to change notification settings - Fork 2.1k
fix(server-core): rebuild the cached driver when its configuration changes #11453
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
MikeNitsenko
wants to merge
16
commits into
master
Choose a base branch
from
mikhail/cub-3599-rebuild-driver-on-config-change
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
cfa2eb1
fix(server-core): rebuild the cached driver when its configuration ch…
MikeNitsenko 23dbff0
fix(server-core): invalidate both pre-aggregation driver keys together
MikeNitsenko f92891d
fix(server-core): make the driver rebuild path concurrency-safe
MikeNitsenko 6456197
fix(server-core): surface driver rebuilds at the default log level
MikeNitsenko 9c46ab1
fix(server-core): never reuse or re-release a driver once ownership i…
MikeNitsenko e63ba89
fix(server-core): rate-limit driver rebuilds, and never release the f…
MikeNitsenko 1f8d972
docs: fix the per-user OAuth recipe's fallback when no expiry is adve…
MikeNitsenko 75e0d04
docs: address PR review on the OAuth recipe and driver rebuild scope
MikeNitsenko b5d9ed9
fix(server-core): replace a driver whose credential stopped rotating
MikeNitsenko 69cc641
docs: state the connection's lifetime in the per-user OAuth recipe
MikeNitsenko 07a25cd
fix(server-core): bound driver replacement to sustained, fixable causes
claude dc9e97b
fix(server-core): bound the ignored-lifetime warning, and the lifetim…
claude e099885
fix(server-core): judge a driver lifetime when stated, not as it ages
claude aea10a5
fix(server-core): keep an accepted lifetime, and count refusal incidents
claude 44fd85b
fix(server-core): bound a refusal incident by its duration, not only …
claude 6ba21b0
docs(server-core): describe both routes to giving a driver up
claude File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
301 changes: 244 additions & 57 deletions
301
docs-mintlify/admin/connect-to-data/oauth-authentication.mdx
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
83 changes: 83 additions & 0 deletions
83
packages/cubejs-server-core/src/core/driver-config-expiry.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| /** | ||
| * @copyright Cube Dev, Inc. | ||
| * @license Apache-2.0 | ||
| * @fileoverview The optional lifetime a `driverFactory` can put on the | ||
| * configuration it returns. | ||
| */ | ||
|
|
||
| import type { DriverConfig } from './types'; | ||
|
|
||
| /** | ||
| * Above this a number is read as epoch milliseconds, below it as epoch seconds. | ||
| * 1e11 milliseconds is 1973 and 1e11 seconds is the year 5138, so nothing | ||
| * anyone can mean today is ambiguous. Both spellings are accepted because the | ||
| * value is usually copied straight off a credential, and the ecosystems | ||
| * disagree: JavaScript counts milliseconds, Python's `time.time()` seconds. | ||
| */ | ||
| const MILLISECONDS_THRESHOLD = 1e11; | ||
|
|
||
| /** | ||
| * A driver lifetime as a POSIX timestamp in milliseconds, or undefined when | ||
| * none was given or the value cannot be read as a moment in time. | ||
| * | ||
| * Unreadable input is dropped rather than rejected: an expiry is an | ||
| * optimisation over comparing configurations, and failing a deployment's | ||
| * queries over a malformed one would be a worse outcome than the behaviour it | ||
| * had before the field existed. | ||
| */ | ||
| export function parseDriverExpiry(value: unknown): number | undefined { | ||
| if (value === undefined || value === null) { | ||
| return undefined; | ||
| } | ||
|
|
||
| if (value instanceof Date) { | ||
| const time = value.getTime(); | ||
|
|
||
| return Number.isNaN(time) ? undefined : time; | ||
| } | ||
|
|
||
| if (typeof value === 'number') { | ||
| if (!Number.isFinite(value) || value <= 0) { | ||
| return undefined; | ||
| } | ||
|
|
||
| return value > MILLISECONDS_THRESHOLD ? value : value * 1000; | ||
| } | ||
|
|
||
| if (typeof value === 'string') { | ||
| const text = value.trim(); | ||
|
|
||
| if (!text) { | ||
| return undefined; | ||
| } | ||
|
|
||
| // A stringified timestamp, which `Date.parse` would read as a year. | ||
| if (/^\d+(\.\d+)?$/.test(text)) { | ||
| return parseDriverExpiry(Number(text)); | ||
| } | ||
|
|
||
| const parsed = Date.parse(text); | ||
|
|
||
| return Number.isNaN(parsed) ? undefined : parsed; | ||
| } | ||
|
|
||
| return undefined; | ||
| } | ||
|
|
||
| /** | ||
| * The same configuration without its lifetime, for the two places that must not | ||
| * see it: the fingerprint that decides whether the connection changed, and the | ||
| * options handed to the driver's own constructor. | ||
| * | ||
| * Returns the input untouched when there is nothing to strip, so the common | ||
| * case allocates nothing. | ||
| */ | ||
| export function withoutDriverExpiry(config: DriverConfig): DriverConfig { | ||
| if (!config || typeof config !== 'object' || !('expiresAt' in config)) { | ||
| return config; | ||
| } | ||
|
|
||
| const { expiresAt: _lifetime, ...rest } = config; | ||
|
|
||
| return <DriverConfig>rest; | ||
| } |
105 changes: 105 additions & 0 deletions
105
packages/cubejs-server-core/src/core/driver-config-fingerprint.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,105 @@ | ||
| /** | ||
| * @copyright Cube Dev, Inc. | ||
| * @license Apache-2.0 | ||
| * @fileoverview Fingerprinting for driver configurations and security contexts. | ||
| */ | ||
|
|
||
| import crypto from 'crypto'; | ||
|
|
||
| /** | ||
| * Deterministic JSON used for fingerprinting. Object keys are emitted in sorted | ||
| * order so two structures that differ only in property order hash the same, and | ||
| * values JSON cannot represent are reduced to stable placeholders rather than | ||
| * silently disappearing. Throws on a circular structure, which callers treat as | ||
| * "not fingerprintable". | ||
| */ | ||
| function stableStringify(value: unknown, seen: Set<unknown>): string { | ||
| if (value === undefined || value === null) { | ||
| return 'null'; | ||
| } | ||
|
|
||
| const type = typeof value; | ||
|
|
||
| if (type === 'string' || type === 'number' || type === 'boolean') { | ||
| return JSON.stringify(value); | ||
| } | ||
|
|
||
| if (type === 'bigint') { | ||
| return JSON.stringify((value as bigint).toString()); | ||
| } | ||
|
|
||
| // A closure's identity cannot be compared meaningfully across calls, so it | ||
| // contributes a constant. Two configs differing only in a function body are | ||
| // therefore treated as equal — deliberately conservative: it can only lead to | ||
| // reusing a connection, never to swapping one out unnecessarily. | ||
| // | ||
| // The practical consequence is that a config carrying its credential as a | ||
| // provider callback rather than a resolved value fingerprints identically | ||
| // however the credential rotates, so such a driver is never rebuilt. A | ||
| // `driverFactory` that needs rotation to be noticed has to return the | ||
| // resolved value. | ||
| if (type === 'function' || type === 'symbol') { | ||
| return JSON.stringify(`[${type}]`); | ||
| } | ||
|
|
||
| if (value instanceof Date) { | ||
| return JSON.stringify(value.toISOString()); | ||
| } | ||
|
|
||
| if (seen.has(value)) { | ||
| throw new Error('Circular structure cannot be fingerprinted'); | ||
| } | ||
|
|
||
| seen.add(value); | ||
|
|
||
| try { | ||
| if (Array.isArray(value)) { | ||
| return `[${value.map((item) => stableStringify(item, seen)).join(',')}]`; | ||
| } | ||
|
|
||
| // Own enumerable keys only, so a class instance holding its values behind | ||
| // prototype accessors fingerprints as `{}` — constant, and therefore another | ||
| // shape whose rotation goes unnoticed. Plain configs are unaffected. | ||
| const entries = Object.keys(value as Record<string, unknown>) | ||
| .sort() | ||
| .reduce<string[]>((acc, key) => { | ||
| const entry = (value as Record<string, unknown>)[key]; | ||
|
|
||
| // Match JSON.stringify: undefined-valued properties are absent, so | ||
| // `{ a: undefined }` and `{}` fingerprint the same. | ||
| if (entry !== undefined) { | ||
| acc.push(`${JSON.stringify(key)}:${stableStringify(entry, seen)}`); | ||
| } | ||
|
|
||
| return acc; | ||
| }, []); | ||
|
|
||
| return `{${entries.join(',')}}`; | ||
| } finally { | ||
| seen.delete(value); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * A short, stable digest of `value`, or `null` when it cannot be fingerprinted. | ||
| * | ||
| * Hashed rather than kept verbatim because the values being compared include | ||
| * database passwords and OAuth access tokens: a raw copy would live for the | ||
| * lifetime of the process and surface in any heap dump. `null` means "cannot | ||
| * tell whether this changed", and every caller must treat that as "assume it | ||
| * did not" so behaviour falls back to the previous resolve-once semantics. | ||
| */ | ||
| export function fingerprint(value: unknown): string | null { | ||
| try { | ||
| return crypto | ||
| .createHash('sha256') | ||
| .update(stableStringify(value, new Set())) | ||
| // 32 hex chars = 128 bits, which is far more than an equality check over | ||
| // the handful of configurations one process resolves needs, and keeps the | ||
| // digest short enough to sit in a log line. | ||
| .digest('hex') | ||
| .slice(0, 32); | ||
| } catch (e) { | ||
| return null; | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.