feat: single global secret - #123
Conversation
vegarsti
left a comment
vegarsti
left a comment
There was a problem hiding this comment.
I think this makes a lot of sense, and the implementation is sound! Thank you and sorry for not reviewing this before
| return []; | ||
| } | ||
|
|
||
| const byIdentifier = new Map(entries.map((entry) => [entry.identifier, entry])); |
There was a problem hiding this comment.
The clanker suggested this should be changed since custom-placeholder secrets also get a marker alias. collectMarkerSecretReferenceRanges() builds the marker identifier map from all secret entries, including entries with explicit custom placeholders
Not sure it's correct 🤔
const byIdentifier = new Map(
entries
.filter(
(entry) => entry.placeholder === `${context.marker}.${entry.identifier}`,
)
.map((entry) => [entry.identifier, entry]),
);
| name: string, | ||
| secret: SecretDefinition, | ||
| mode: SecretPlaceholderMode, | ||
| marker: string | undefined, |
There was a problem hiding this comment.
What happens if marker is undefined here? I think we can get <undefined>.${identifier} on line 369 then? 🤔
There was a problem hiding this comment.
In practice nothing, because createHttpHooks() only calls resolveSecretPlaceholder() with mode === "shared" when it has already created a marker. But hardened the type to throw anyways, thanks!
|
I think this is fine, but I don't think we should call the other mode |
3 participants
This PR adds the proposal in #122.
It adds a flag
'shared' | 'unique', with defaultshared. This makes HTTP-hook secret placeholders now use marker mode: guest env vars get<random-marker>.<normalized_secret_name>instead of per-secret random placeholders.The host replacement path was updated to recognize these structured references in headers, basic auth, and optional query params, resolve them back to the configured secret by identifier, and then apply the existing allowlist/update/delete behavior.
Unique
GONDOLIN_SECRET_<random>placeholders remain available viasecretPlaceholderMode: "unique", and docs/tests were updated accordingly.