feat: allow overriding pull secret / SSH key location, prefer Ed25519 - #130
Open
butler54 wants to merge 1 commit into
Open
feat: allow overriding pull secret / SSH key location, prefer Ed25519#130butler54 wants to merge 1 commit into
butler54 wants to merge 1 commit into
Conversation
Fixes validatedpatterns#37 Fixes validatedpatterns#23 The RHDP wrapper scripts hardcoded the OpenShift pull secret to ~/pull-secret.json and the SSH key to ~/.ssh/id_rsa (RSA specifically), with no way to point elsewhere and no support for other key types. rhdp-cluster-define.py now resolves both paths itself: - Pull secret: --pull-secret / PULL_SECRET env var override, else ~/pull-secret.json (unchanged default). - SSH public key: --ssh-public-key / SSH_PUBLIC_KEY env var override, else auto-detect ~/.ssh/id_ed25519.pub, then id_ecdsa.pub, then id_rsa.pub (first match wins) -- Ed25519 preferred as current best practice, while remaining backwards compatible with existing RSA-only setups. Both env vars propagate to the python subprocess automatically, so no new flags are needed on wrapper.sh / wrapper-cluster-only.sh / wrapper-multicluster.sh. Matching --pull-secret/--ssh-public-key CLI flags are also available for direct use of rhdp-cluster-define.py, using typer's native envvar support (see --help output). Resolution now happens before cleanup() runs, so a missing pull secret or SSH key can't trigger a destructive directory wipe only to fail afterwards. Removed the three wrapper scripts' now-outdated hardcoded pre-flight checks (which required ~/pull-secret.json and ~/.ssh/id_rsa specifically, blocking legitimate overrides and Ed25519-only setups). rhdp-cluster-define.py's validation, with clear actionable error messages, is now the single source of truth. Verified with scenario tests covering: no override + no keys (error), no override + RSA-only (backwards compat), no override + Ed25519 and RSA both present (prefers Ed25519), override to a custom path, missing pull secret, and validation-before-cleanup ordering. Also verified end-to-end via the CLI (--help, and full runs exercising each path). isort/black/flake8/mypy all pass against the pinned CI tool versions.
This was referenced Aug 30, 2026
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
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.
Summary
Fixes #37 and #23.
#37 — Allow Pull Secret and SSH Keys to be Specified by the end user
rhdp-cluster-define.pynow resolves both paths itself, rather than the wrapper scripts hardcoding~/pull-secret.jsonand~/.ssh/id_rsa.pub:--pull-secret/PULL_SECRETenv var override, else~/pull-secret.json(unchanged default).--ssh-public-key/SSH_PUBLIC_KEYenv var override, else auto-detect (see Update 'RHDP' wrapper script to support a wider variety of ssh keys #23 below).Per the maintainer's own suggestion on #37 ("optional environmental var override... easiest way to be backwards compatible"), both env vars propagate to the
rhdp-cluster-define.pysubprocess automatically — no new flags needed on the wrapper scripts. Matching--pull-secret/--ssh-public-keyCLI flags are also available for direct use ofrhdp-cluster-define.py(via typer's nativeenvvar=support — see--help).Resolution now happens before
cleanup()runs, so a missing pull secret or SSH key can't trigger a destructive directory wipe only to fail afterwards.#23 — Update RHDP wrapper script to support a wider variety of SSH keys
SSH public key is now auto-detected when no override is given, preferring
~/.ssh/id_ed25519.pub, thenid_ecdsa.pub, thenid_rsa.pub(first match wins) — Ed25519 as current best practice, while remaining backwards compatible with existing RSA-only setups. Previously hardcoded to RSA only, with aTBC: Update to support other key typescomment in the error message (now resolved).Other changes
wrapper.sh,wrapper-cluster-only.sh,wrapper-multicluster.sh) now-outdated hardcoded pre-flight checks, which required~/pull-secret.jsonand~/.ssh/id_rsaspecifically and would otherwise block legitimate overrides or Ed25519-only setups.rhdp-cluster-define.py's validation (with clear, actionable error messages) is now the single source of truth.rhdp/README.mdwith a new "Overriding pull secret / SSH key location" section.Cross-repo / follow-up scope
While investigating, I found two related things intentionally left out of scope for this PR, each tracked separately:
scripts/gen-secrets.sh's separate podvm SSH-debug key (~/.coco-pattern/id_rsa, used for optional peer-pod SSH debugging, unrelated to the RHDP cluster-install SSH key this PR addresses) is also RSA-only. Cross-repo review (coco-pattern → sandboxed-containers-chart → sandboxed-policies-chart) confirmed it's no longer a hard peer-pods requirement (made optional insandboxed-containers-chartPR feat: cleanup some defaults #7) but is still real, wired functionality. Modernizing it to support Ed25519 too would span 3 repos and needs verification of the upstream consumer's expectations — tracked in chore: modernize podvm SSH-debug key to support Ed25519 alongside RSA #129.validatedpatterns/docs) doesn't mention therhdp/wrapper scripts at all today, despite them being the recommended Azure deployment path (per Update documentation #59's comment thread). Tracked in docs: document RHDP wrapper deployment path and configurable secrets locations #131.Test plan
resolve_pull_secret()/resolve_ssh_public_key(): no override + no keys (error), no override + RSA-only (backwards compat), no override + Ed25519 and RSA both present (prefers Ed25519), no override + ECDSA and RSA present (prefers ECDSA), override to a valid custom path, override to a missing path (error), pull secret missing/present/overridden.cleanup()(no directories touched on failure),PULL_SECRETenv var override works, Ed25519 auto-detection resolves and is correctly embedded ininstall-config.yaml.--helpoutput confirms[env var: PULL_SECRET]/[env var: SSH_PUBLIC_KEY]are wired correctly.--recreateguard from the previous PR still works correctly.bash -nsyntax check on all three wrapper scripts.