From bf9b188bdcd332b5d6c5f92e828a6806efcd1166 Mon Sep 17 00:00:00 2001 From: Chris Butler Date: Sun, 30 Aug 2026 16:27:04 +0000 Subject: [PATCH] feat: allow overriding pull secret / SSH key location, prefer Ed25519 Fixes #37 Fixes #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. --- rhdp/README.md | 35 +++++++++++- rhdp/rhdp-cluster-define.py | 103 +++++++++++++++++++++++++++++++++-- rhdp/wrapper-cluster-only.sh | 16 ++---- rhdp/wrapper-multicluster.sh | 15 ++--- rhdp/wrapper.sh | 16 ++---- 5 files changed, 144 insertions(+), 41 deletions(-) diff --git a/rhdp/README.md b/rhdp/README.md index 50873544..b8f9d711 100644 --- a/rhdp/README.md +++ b/rhdp/README.md @@ -7,8 +7,8 @@ The scripts in this directory help users of that platform automate deployments. - `podman` installed and running (used for reference value collection) - `yq`, `jq` installed -- OpenShift pull secret at `~/pull-secret.json` -- SSH key at `~/.ssh/id_rsa` (RSA) +- OpenShift pull secret (default: `~/pull-secret.json`, override with `PULL_SECRET` — see below) +- An SSH key pair (default: auto-detected, preferring Ed25519 — override with `SSH_PUBLIC_KEY` — see below) - RHDP environment variables loaded (see below) ## Environment variables @@ -54,6 +54,37 @@ The wrapper handles: cluster provisioning, secret generation, PCR reference valu 1. `bash ./rhdp/wrapper-cluster-only.sh eastasia` 2. Provisions the cluster without installing secrets or the pattern +## Overriding pull secret / SSH key location + +By default: + +- The OpenShift pull secret is read from `~/pull-secret.json`. +- The SSH public key embedded in `install-config.yaml` is auto-detected, + preferring `~/.ssh/id_ed25519.pub`, then `~/.ssh/id_ecdsa.pub`, then + `~/.ssh/id_rsa.pub` (first match wins). Ed25519 is current best practice, + but existing RSA-only setups keep working without changes. + +Both can be overridden if your pull secret or SSH key live somewhere else, +via environment variable: + +```shell +export PULL_SECRET=/path/to/pull-secret.json +export SSH_PUBLIC_KEY=/path/to/your/key.pub +bash ./rhdp/wrapper.sh eastasia +``` + +Since these are plain environment variables, they apply to all three +wrapper scripts without any extra flags. If you run `rhdp/rhdp-cluster-define.py` +directly instead of through a wrapper script, the equivalent CLI flags +`--pull-secret` and `--ssh-public-key` are also available and take +precedence over the environment variables. + +If neither an override nor a default/auto-detected file can be found, the +command exits with an error explaining what was checked and how to fix it +(generate a new key with `ssh-keygen -t ed25519`, download a pull secret +from [console.redhat.com](https://console.redhat.com/openshift/downloads), +or set the relevant environment variable). + ## Re-running against an existing install directory All three wrapper scripts (and `rhdp/rhdp-cluster-define.py` directly) refuse diff --git a/rhdp/rhdp-cluster-define.py b/rhdp/rhdp-cluster-define.py index a36b0a21..9971969a 100644 --- a/rhdp/rhdp-cluster-define.py +++ b/rhdp/rhdp-cluster-define.py @@ -5,7 +5,7 @@ import os import pathlib import shutil -from typing import Dict, List +from typing import Dict, List, Optional import typer from jinja2 import Environment, FileSystemLoader, select_autoescape @@ -109,7 +109,8 @@ def cleanup( "Before re-running with --recreate, either:\n" " 1. Destroy the existing cluster's cloud resources yourself:\n" " openshift-install destroy cluster --dir=\n" - " 2. Or confirm the cloud resources are already gone / were never created.\n" + " 2. Or confirm the cloud resources are already gone / were never " + "created.\n" "Re-running with --recreate will DELETE the local install state above\n" "WITHOUT destroying any associated cloud resources, which can orphan them." ) @@ -141,6 +142,66 @@ def validate_dir(): assert pathlib.Path("values-azure.yaml").exists() +# Default SSH public key candidates to auto-detect, in preference order. +# Ed25519 first (current best practice), falling back through ECDSA to RSA +# for backwards compatibility with existing keys. +DEFAULT_SSH_KEY_CANDIDATES = ("id_ed25519", "id_ecdsa", "id_rsa") + + +def resolve_pull_secret(override: Optional[str]) -> pathlib.Path: + """Resolve the OpenShift pull secret path. + + Honors an explicit override (--pull-secret / PULL_SECRET env var), else + falls back to the historical default of ~/pull-secret.json. + """ + path = ( + pathlib.Path(override).expanduser() + if override + else pathlib.Path("~/pull-secret.json").expanduser() + ) + if not path.exists(): + rprint(f"[red]ERROR: OpenShift pull secret not found at {path}[/red]") + rprint( + "Download it from https://console.redhat.com/openshift/downloads " + "and save it there, or point to it with --pull-secret / the " + "PULL_SECRET environment variable." + ) + raise typer.Exit(code=1) + return path + + +def resolve_ssh_public_key(override: Optional[str]) -> pathlib.Path: + """Resolve the SSH public key to embed in install-config.yaml. + + Honors an explicit override (--ssh-public-key / SSH_PUBLIC_KEY env var). + Otherwise auto-detects the user's default key, preferring Ed25519, then + ECDSA, then RSA (first match wins) -- current best practice while + remaining backwards compatible with existing RSA-only setups. + """ + if override: + path = pathlib.Path(override).expanduser() + if not path.exists(): + rprint(f"[red]ERROR: SSH public key not found at {path}[/red]") + raise typer.Exit(code=1) + return path + + ssh_dir = pathlib.Path.home() / ".ssh" + for candidate in DEFAULT_SSH_KEY_CANDIDATES: + candidate_path = ssh_dir / f"{candidate}.pub" + if candidate_path.exists(): + return candidate_path + + checked = ", ".join(str(ssh_dir / f"{c}.pub") for c in DEFAULT_SSH_KEY_CANDIDATES) + rprint("[red]ERROR: No SSH public key found.[/red]") + rprint( + f"Checked (in order): {checked}\n" + "Generate a modern key with: ssh-keygen -t ed25519\n" + "Or point to an existing one with --ssh-public-key / the " + "SSH_PUBLIC_KEY environment variable." + ) + raise typer.Exit(code=1) + + def setup_install( pattern_dir: pathlib.Path, region: str, @@ -224,6 +285,26 @@ def run( ), ), ] = False, + pull_secret: Annotated[ + Optional[str], + typer.Option( + "--pull-secret", + envvar="PULL_SECRET", + help="Path to the OpenShift pull secret (default: ~/pull-secret.json).", + ), + ] = None, + ssh_public_key: Annotated[ + Optional[str], + typer.Option( + "--ssh-public-key", + envvar="SSH_PUBLIC_KEY", + help=( + "Path to an SSH public key to embed in install-config.yaml. " + "Defaults to auto-detecting ~/.ssh/id_ed25519.pub, then " + "id_ecdsa.pub, then id_rsa.pub (first match wins)." + ), + ), + ] = None, ): """ Region flag requires an azure region key which can be (authoritatively) @@ -240,9 +321,23 @@ def run( cluster state. Without it, the command refuses to touch a directory that looks like it belongs to a previous (possibly still-live) cluster. This does NOT run "openshift-install destroy cluster" for you. + + Use --pull-secret (or the PULL_SECRET environment variable) to override + the OpenShift pull secret location (default: ~/pull-secret.json). + + Use --ssh-public-key (or the SSH_PUBLIC_KEY environment variable) to + override the SSH public key embedded in install-config.yaml. Without an + override, the key is auto-detected, preferring ~/.ssh/id_ed25519.pub, + then id_ecdsa.pub, then id_rsa.pub. """ validate_dir() + # Resolve and validate secrets/keys before touching any install + # directory, so a missing pull secret or SSH key can't trigger a + # destructive cleanup() only to fail afterwards. + pull_secret_path = resolve_pull_secret(pull_secret) + ssh_public_key_path = resolve_ssh_public_key(ssh_public_key) + # Choose cluster configurations based on multicluster flag if multicluster: if prefix: @@ -260,8 +355,8 @@ def run( setup_install( pathlib.Path.cwd(), region, - pathlib.Path("~/pull-secret.json"), - pathlib.Path("~/.ssh/id_rsa.pub"), + pull_secret_path, + ssh_public_key_path, cluster_configs, ) write_azure_creds() diff --git a/rhdp/wrapper-cluster-only.sh b/rhdp/wrapper-cluster-only.sh index 877e640e..5b799b74 100755 --- a/rhdp/wrapper-cluster-only.sh +++ b/rhdp/wrapper-cluster-only.sh @@ -108,18 +108,10 @@ echo "requirements installed" echo "---------------------" sleep 5 -if [ ! -f "${HOME}/pull-secret.json" ]; then - echo "A OpenShift pull secret is required at ~/pull-secret.json" - exit 1 -fi - -if [ ! -f "${HOME}/.ssh/id_rsa" ]; then - echo "An rsa ssh key is required at ~/.ssh/id_rsa" - echo "e.g. ssh-keygen -t rsa -b 4096" - echo "TBC: Update to support other key types" - exit 1 -fi - +# The OpenShift pull secret and SSH public key locations are resolved by +# rhdp-cluster-define.py below (with clear error messages if not found). +# Override via the PULL_SECRET / SSH_PUBLIC_KEY environment variables if +# your pull secret or SSH key isn't at the default location. echo "---------------------" echo "defining cluster" diff --git a/rhdp/wrapper-multicluster.sh b/rhdp/wrapper-multicluster.sh index a00bfb38..6b569369 100755 --- a/rhdp/wrapper-multicluster.sh +++ b/rhdp/wrapper-multicluster.sh @@ -140,17 +140,10 @@ echo "requirements installed" echo "---------------------" sleep 5 -if [ ! -f "${HOME}/pull-secret.json" ]; then - echo "A OpenShift pull secret is required at ~/pull-secret.json" - exit 1 -fi - -if [ ! -f "${HOME}/.ssh/id_rsa" ]; then - echo "An rsa ssh key is required at ~/.ssh/id_rsa" - echo "e.g. ssh-keygen -t rsa -b 4096" - echo "TBC: Update to support other key types" - exit 1 -fi +# The OpenShift pull secret and SSH public key locations are resolved by +# rhdp-cluster-define.py below (with clear error messages if not found). +# Override via the PULL_SECRET / SSH_PUBLIC_KEY environment variables if +# your pull secret or SSH key isn't at the default location. echo "---------------------" echo "defining both clusters (hub and spoke)" diff --git a/rhdp/wrapper.sh b/rhdp/wrapper.sh index bdd50130..1bc6ba16 100755 --- a/rhdp/wrapper.sh +++ b/rhdp/wrapper.sh @@ -155,18 +155,10 @@ echo "requirements installed" echo "---------------------" sleep 5 -if [ ! -f "${HOME}/pull-secret.json" ]; then - echo "A OpenShift pull secret is required at ~/pull-secret.json" - exit 1 -fi - -if [ ! -f "${HOME}/.ssh/id_rsa" ]; then - echo "An rsa ssh key is required at ~/.ssh/id_rsa" - echo "e.g. ssh-keygen -t rsa -b 4096" - echo "TBC: Update to support other key types" - exit 1 -fi - +# The OpenShift pull secret and SSH public key locations are resolved by +# rhdp-cluster-define.py below (with clear error messages if not found). +# Override via the PULL_SECRET / SSH_PUBLIC_KEY environment variables if +# your pull secret or SSH key isn't at the default location. echo "---------------------" echo "defining cluster"