fix: use the IdP secret the operator already owns - #31
Merged
Conversation
Naming an existing Secrets Manager secret is the path an enterprise actually
wants — "our secret is already there, here is its name, do not ask us for it" —
and it looked supported (identity.client_secret_name in platform.yaml, or
IDP_CLIENT_SECRET_NAME). It was not:
- prompt_idp probed only <prefix>-idp-client-secret, so a configured name was
never found, the participant was asked to paste the value anyway, and
- upsert_idp_secret then reassigned IDP_CLIENT_SECRET_NAME to its own prefixed
name, storing a second copy and leaving the operator's secret unused.
Both now default to the prefixed name only when none is configured. Rotating a
value into a bring-your-own secret writes to that secret instead of forking it.
Check (l) in check-deploy-config.sh covers both halves: the describe-secret
probe must target the configured name, and the rotation must not mention the
prefixed one. Against the old deploy.sh it fails with
"configured secret name not probed: ... --secret-id check-prefix-idp-client-secret".
Also fixes the demo script, which told readers to pass the secret as
`-c idp_client_secret=...`. The stack rejects that key outright ("no longer
supported"), so the documented command could not work — and it taught the leaky
pattern the secret-name design exists to avoid. Replaced with the two real
steps, plus a note that run-workshop.sh does them for you.
Verified: full check-deploy-config.sh run green (15 checks); shellcheck back to
the 2 pre-existing deploy.sh findings, nothing new.
Robobc
added a commit
that referenced
this pull request
Aug 22, 2026
fix: use the IdP secret the operator already owns (re-land of #31)
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Stacked on #30 — merge that one first and this retargets to
mainautomatically.What
Naming a Secrets Manager secret you already own is the path an enterprise actually wants: "our IdP secret is already in Secrets Manager, here is its name, don't ask us for it." It looked supported —
identity.client_secret_nameinplatform.yaml, orIDP_CLIENT_SECRET_NAME— but it never worked:prompt_idpprobed only<prefix>-idp-client-secret, so a configured name was never found and the participant was asked to paste the value anyway.upsert_idp_secretthen reassigned the name to its own prefixed one, storing a second copy and leaving the operator's secret unused.Both now fall back to the prefixed name only when nothing is configured. Rotating a value into a bring-your-own secret writes to that secret instead of forking it.
Check
Check (l) in
scripts/check-deploy-config.shcovers both halves — thedescribe-secretprobe must target the configured name, and the rotation must not mention the prefixed one. Against the olddeploy.shit fails with:Also: the demo script documented a command that cannot work
workshop-simulation/DEMO_SCRIPT.mdtold readers to deploy with-c idp_client_secret=$IDP_CLIENT_SECRET. The auth stack rejects that context key outright ("no longer supported"), so the documented command fails — and it taught exactly the leaky pattern the secret-name design exists to avoid, since context values land incdk.context.json, process listings, and the synthesized template. Replaced with the two real steps and a note thatrun-workshop.shdoes both for you.Verified
scripts/check-deploy-config.sh: 15 checks green; check (l) fails against the pre-fix script.shellcheck: back to the 2 pre-existingdeploy.shfindings, nothing new.No AWS deploy needed for this one — the behaviour is in the CLI argument the script builds, which is what the check asserts on.