-
Notifications
You must be signed in to change notification settings - Fork 17
Add /prepare skill for installation config generation #95
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
base: main
Are you sure you want to change the base?
Changes from all commits
ad52f3a
5d8f162
390e912
652baf0
ef9d5a4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,193 @@ | ||
| --- | ||
| description: Prepare a dev-scripts cluster config with a resolved OpenShift release image | ||
| --- | ||
|
|
||
| You are preparing an OpenShift cluster configuration for deployment via dev-scripts. This skill resolves a release image, generates a config file, and validates the environment — but never deploys. | ||
|
|
||
| ## Arguments | ||
|
|
||
| `/prepare <topology> <method> <medium> <version...> [key=value...]` | ||
|
|
||
| **Positional (required):** | ||
| - **topology**: `tna` / `arbiter`, `tnf` / `fencing`, or `sno` | ||
| - **method**: `ipi` or `agent` | ||
| - **medium**: `aws` or `external` | ||
| - **version**: free text mapped to a resolver spec (see below) | ||
|
|
||
| **Optional key=value overrides:** | ||
| - `ip-stack=v4|v6|v4v6` (default: v4) | ||
| - `arch=x86_64|aarch64` (override auto-detection) | ||
| - `force=true` (overwrite existing config) | ||
| - `ds-repo=URL` (dev-scripts fork URL) | ||
| - `ds-branch=BRANCH` (dev-scripts fork branch) | ||
|
|
||
| ## Version Mapping | ||
|
|
||
| | User says | Resolver spec | | ||
| |-----------|---------------| | ||
| | `4.21 nightly` | `4.21-nightly` | | ||
| | `latest 4.22 EC/RC` | `4.22-prerelease` | | ||
| | `4.22 EC` | `4.22-ec` | | ||
| | `4.20` or `4.20 GA` | `4.20` | | ||
| | `4.20.5` | `4.20.5` | | ||
| | Contains `/` or `@sha256:` | `--pullspec` (explicit) | | ||
|
|
||
| ## Non-Interactive Contract | ||
|
|
||
| When all positional arguments are present, **never prompt open-endedly**. If prerequisites are missing, emit one consolidated failure block listing each issue with its fix, then stop: | ||
|
|
||
| ``` | ||
| Missing prerequisites: | ||
| - pull-secret.json: run /setup dev-scripts | ||
| - CI Token: run /setup, CI Token section | ||
| - inventory.ini: run /setup external (for external medium) | ||
| ``` | ||
|
|
||
| Interactive prompting is only for humans invoking with missing arguments. | ||
|
|
||
| ## Scope | ||
|
|
||
| **Dev-scripts only** (v1). No kcli, assisted, or baremetal-adopt. If the user asks for kcli or assisted, tell them it's out of scope and point to the relevant make targets. | ||
|
|
||
| ## CI Token Sourcing | ||
|
|
||
| 1. Check existing `config/config_*.sh` files for an active `export CI_TOKEN="..."` line (any topology) | ||
| 2. If found and not a placeholder (`<PASTE`), reuse it silently | ||
| 3. If not found: ask the human / fail the agent path with a pointer to `/setup` | ||
| 4. **Never echo the token into chat output** | ||
|
|
||
| ## Architecture Determination | ||
|
|
||
| 1. `arch=` key wins if provided | ||
| 2. medium=aws: read `EC2_INSTANCE_TYPE` from `config/instance.env` — Graviton families (`c7g`, `m7g`, `r7g`, `c6g`, `m6g`, `r6g`, etc.) → `aarch64`; otherwise `x86_64` | ||
| 3. medium=external: default `x86_64` with a note that `arch=aarch64` can override | ||
|
|
||
| ## Constraint Matrix | ||
|
|
||
| | Constraint | Effect | | ||
| |------------|--------| | ||
| | aarch64 + ip-stack != v4 | **Blocked** — IPv6/dual-stack unsupported on ARM | | ||
| | aarch64 + -multi image | **Blocked** — explicit aarch64 payload required | | ||
| | agent + ip-stack != v4 | **Warning** — scenario name must exist in dev-scripts e2e list | | ||
| | CI_TOKEN missing/placeholder | **Blocked** — always required | | ||
|
|
||
| These are enforced in the helper scripts. If the user hits one, explain why and what to change. | ||
|
|
||
| ## Execution Steps | ||
|
|
||
| Run each step as a separate Bash call. Branch on exit codes. | ||
|
|
||
| ### Step 1: Bootstrap worktree config (worktrees only) | ||
|
|
||
| ```bash | ||
| helpers/sync-worktree-config.sh | ||
| ``` | ||
|
|
||
| The script auto-detects whether it is running in a worktree. If so, it copies essential config files (`pull-secret.json`, `instance.env`, `config_*.sh`) from the main checkout into the worktree's `config/` folder — picking the newer of `config/<file>` and the canonical sync destination when both exist. If not a worktree, it exits silently. | ||
|
|
||
| After this step, `config/pull-secret.json` must exist. If it is still missing, fail with the standard prerequisites block. | ||
|
|
||
| ### Step 2: Resolve release image | ||
|
|
||
| ```bash | ||
| helpers/resolve-release-image.sh \ | ||
| --version <spec> \ | ||
| --arch <arch> \ | ||
| $([ "<method>" = "agent" ] && echo "--digest") \ | ||
| --pull-secret config/pull-secret.json \ | ||
| --validate-access \ | ||
| $([ -n "<ci_token>" ] && echo "--ci-token <ci_token>") | ||
| ``` | ||
|
|
||
| Capture stdout (single-line pullspec). On failure: | ||
| - Exit 2: invalid version spec → show valid formats | ||
| - Exit 3: no matching release → suggest checking version number | ||
| - Exit 5: access denied → point to `/setup` for credentials | ||
|
|
||
| ### Step 3: Ensure instance.env exists (medium=aws only) | ||
|
|
||
| Skip this step when medium is not `aws`. | ||
|
|
||
| Check if `config/instance.env` exists: | ||
| - **If present**: proceed silently (do not re-copy the template over user edits) | ||
| - **If missing**: copy the template and warn about defaults | ||
|
|
||
| ```bash | ||
| cp config/instance.env.template config/instance.env | ||
| ``` | ||
|
|
||
| After copying, emit a warning (not a failure — the defaults are usable): | ||
| ``` | ||
| NOTE: Created config/instance.env from template with defaults: | ||
| - EC2_INSTANCE_TYPE=c5n.metal (x86_64 Intel) | ||
| - REGION=us-west-2 | ||
| - AWS_PROFILE=microshift-dev | ||
| Review and edit config/instance.env if these defaults are not suitable. | ||
| ``` | ||
|
|
||
| Do NOT prompt for edits when all positional args are present (non-interactive contract). Just warn and continue. | ||
|
|
||
| ### Step 4: Generate config | ||
|
|
||
| ```bash | ||
| helpers/prepare-config.sh \ | ||
| --topology <topology> \ | ||
| --method <method> \ | ||
| --release-image <pullspec from step 2> \ | ||
| --ci-token <ci_token> \ | ||
| --ip-stack <stack> \ | ||
| --arch <arch> \ | ||
| $([ "<force>" = "true" ] && echo "--force") \ | ||
| $([ -n "<ds_repo>" ] && echo "--ds-repo <ds_repo>") \ | ||
| $([ -n "<ds_branch>" ] && echo "--ds-branch <ds_branch>") | ||
|
Comment on lines
+140
to
+142
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift Shell-quote and validate free-form repository inputs before execution.
As per path instructions, 🤖 Prompt for AI AgentsSource: Path instructions
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Won't fix — skill template placeholders are substituted by Claude, not by shell expansion. The helper script's argument parser handles quoting via positional shift, and sed delimiter
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. #!/bin/bash
set -e
sed -n '1,160p' .claude/commands/prepare.md
echo "-----"
fd -e sh . helpers
echo "-----"
cat -n helpers/prepare-config.sh | sed -n '1,80p'
</shell>
<!-- This is an auto-generated reply by CodeRabbit --> |
||
| ``` | ||
|
|
||
| On failure: | ||
| - Exit 3: constraint violation → explain the specific constraint | ||
| - Exit 4: config exists → ask if they want `force=true` (human) or tell agent callers to pass `force=true` | ||
| - Exit 5: self-check failed → likely template drift, report verbatim | ||
|
|
||
| ### Step 5: Run doctor | ||
|
|
||
| Before running Make, normalize the topology: if the user said `tnf`, use `fencing`; if `tna`, use `arbiter`. The `<topology>` below must be `arbiter`, `fencing`, or `sno`. | ||
|
|
||
| **medium=aws** (default — no AWS= override needed): | ||
| ```bash | ||
| cd deploy && make sync-config && make doctor <topology>-<method> | ||
| ``` | ||
|
|
||
| **medium=external:** | ||
| ```bash | ||
| cd deploy && make doctor AWS=0 <topology>-<method> | ||
| ``` | ||
|
|
||
| The `sync-config` call propagates `config/instance.env` to the canonical location (`deploy/aws-hypervisor/instance.env`) that doctor reads. `AWS=0` opts out of the instance.env requirement for external hosts. | ||
|
|
||
| Relay FAIL lines verbatim. Doctor is read-only — it won't break anything. | ||
|
|
||
| ### Step 6: Print deployment commands (never run them) | ||
|
|
||
| **medium=aws:** | ||
| ``` | ||
| cd deploy && make deploy <topology>-<method> | ||
| ``` | ||
| Note: if an instance is already running (`make status`), `make <topology>-<method>` reuses it. | ||
|
|
||
| **medium=external:** | ||
| ``` | ||
| ansible-playbook deploy/openshift-clusters/setup.yml \ | ||
| -e "topology=<topology>" -e "interactive_mode=false" \ | ||
| $([ "<method>" = "agent" ] && echo '-e "method=agent"') \ | ||
| -i deploy/openshift-clusters/inventory.ini | ||
| ``` | ||
| Note for first-time external hosts: run `ansible-playbook deploy/openshift-clusters/init-host.yml -i deploy/openshift-clusters/inventory.ini` first. | ||
|
|
||
| ## After Completion | ||
|
|
||
| Report what was done: | ||
| - Resolved image (tag or digest) | ||
| - Config file path written | ||
| - Doctor result (pass/fail) | ||
| - Deployment command to run | ||
|
|
||
| Do **not** offer to run the deployment. The user decides when to deploy. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| # /prepare — Installation Preparation | ||
|
|
||
| Resolves an OpenShift release image, generates a dev-scripts config file, validates the environment, and prints the deployment command — without deploying. | ||
|
|
||
| ## Usage | ||
|
|
||
| ``` | ||
| /prepare <topology> <method> <medium> <version> [options] | ||
| ``` | ||
|
|
||
| ### Examples | ||
|
|
||
| ``` | ||
| # TNF fencing cluster, IPI on AWS, latest 4.21 GA | ||
| /prepare tnf ipi aws 4.21 | ||
|
|
||
| # Arbiter cluster, agent method on external host, latest nightly | ||
| /prepare tna agent external 4.22 nightly | ||
|
|
||
| # SNO with specific version, forced overwrite | ||
| /prepare sno ipi aws 4.20.5 force=true | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| # Fencing on aarch64 Graviton instance | ||
| /prepare tnf ipi aws 4.21 arch=aarch64 | ||
|
|
||
| # Using a dev-scripts fork for testing | ||
| /prepare tnf ipi aws 4.21 nightly ds-repo=https://github.com/user/dev-scripts ds-branch=fix/my-change | ||
| ``` | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| ### Parameters | ||
|
|
||
| | Parameter | Values | Default | | ||
| |-----------|--------|---------| | ||
| | topology | `tna`/`arbiter`, `tnf`/`fencing`, `sno` | required | | ||
| | method | `ipi`, `agent` | required | | ||
| | medium | `aws`, `external` | required | | ||
| | version | `4.21`, `4.21 nightly`, `4.22 EC`, etc. | required | | ||
| | ip-stack | `v4`, `v6`, `v4v6` | `v4` | | ||
| | arch | `x86_64`, `aarch64` | auto-detected | | ||
| | force | `true` | `false` | | ||
| | ds-repo | fork URL | none | | ||
| | ds-branch | fork branch | none | | ||
|
|
||
| ## What It Does | ||
|
|
||
| 1. **Resolves** the version spec to a concrete release image (nightly from CI registry, or GA/EC/RC from quay.io) | ||
| 2. **Pins by digest** when method=agent (agent installs require digest-pinned images) | ||
| 3. **Generates** `config/config_<topology>.sh` from the example template with correct vars | ||
| 4. **Validates** CI token, pull secret auth, and architecture constraints | ||
| 5. **Runs** `make doctor <topology>-<method>` as a final gate | ||
| 6. **Prints** the deployment command — but never runs it | ||
|
|
||
| ## What It Does NOT Do | ||
|
|
||
| - Deploy a cluster (that's `make deploy` or `ansible-playbook`) | ||
| - Set up credentials or pull secrets (that's `/setup`) | ||
| - Manage kcli, assisted, or baremetal-adopt configs (dev-scripts only in v1) | ||
|
|
||
| ## Helper Scripts | ||
|
|
||
| For scripted or CI use, call the helpers directly: | ||
|
|
||
| ```bash | ||
| # Resolve a release image | ||
| helpers/resolve-release-image.sh --version 4.21-nightly --arch x86_64 | ||
|
|
||
| # Generate config | ||
| helpers/prepare-config.sh --topology fencing --method ipi \ | ||
| --release-image <pullspec> --ci-token <token> | ||
| ``` | ||
|
|
||
| Run each script with `--help` for full usage. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,9 @@ | ||
| # Valid cluster types for 'make deploy <cluster-type>' | ||
| VALID_CLUSTER_TYPES := fencing-ipi fencing-agent fencing-assisted arbiter-ipi arbiter-agent arbiter-kcli fencing-kcli sno-ipi sno-agent | ||
|
|
||
| # AWS hypervisor checks are on by default for doctor; opt out with AWS=0 | ||
| AWS ?= 1 | ||
|
|
||
|
Comment on lines
+4
to
+6
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Make AWS prerequisite enforcement explicit and fail closed. The current contract can silently run the AWS preparation flow without AWS checks when
As per coding guidelines, configuration should be validated early and fail fast. 📍 Affects 2 files
🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| # Handle 'make deploy <cluster-type>' pattern | ||
| # When 'deploy' is first, validate any following arguments are valid cluster types | ||
| ifeq (deploy,$(firstword $(MAKECMDGOALS))) | ||
|
|
@@ -91,7 +94,7 @@ sync-config: | |
| fi' | ||
|
|
||
| doctor: | ||
| @./openshift-clusters/scripts/doctor.sh $(DOCTOR_ARGS) | ||
| @./openshift-clusters/scripts/doctor.sh $(if $(filter 1,$(AWS)),--aws) $(DOCTOR_ARGS) | ||
|
|
||
| ifeq (doctor,$(firstword $(MAKECMDGOALS))) | ||
| # Cluster types after 'doctor' are arguments to doctor.sh, not deployments | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Do not interpolate the CI token into the generated command.
The contract says never echo the token into chat, but these templates instruct the agent to substitute
<ci_token>directly into Bash. Keep the credential in a protected environment/secret channel and avoid rendering its value in the command transcript; ideally support environment-variable or stdin-based token input in the helpers.Based on learnings, secrets must use environment variables and never be included in code or commits.
As per path instructions,
.claudecommand changes require security-focused review of credential handling.Also applies to: 99-105
🤖 Prompt for AI Agents
Sources: Path instructions, Learnings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't fix — this is a Claude skill prompt, not executable shell. The
<ci_token>placeholders are substituted at runtime. The--ci-tokenCLI flag is the repo's established helper pattern, and the token is intentionally written to config files (the Ansible role greps for it).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fonta-rh, thanks for the clarification — that makes sense. Since.claude/commands/prepare.mdis a prompt template consumed by the agent (not a shell script), and--ci-tokenis the established helper pattern with the token intentionally persisted toconfig/config_*.shfor the Ansible role to consume, the original concern doesn't apply here. Withdrawing this one.✏️ Learnings added