Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions skills/rhdh-pr-review/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,13 @@ Present the full review draft — summary, inline comments with file:line, and e
For cluster testing: deploy the full PR bundle/manifests, not just the operator binary image. PR changes to CRDs, RBAC, default config, or bundle metadata are baked into the OLM bundle or install.yaml — a binary-only image swap misses them.
</principle>

<principle name="deploy_full_chart">
Deploy the full chart from the PR branch, not just a values override.
PR changes to templates, helpers, schema, or dependencies are baked into the chart itself — a values-only change would miss them.
Use `helm upgrade` pointing at the local chart directory from the PR branch with `--reuse-values` to preserve existing user configuration.
See `references/chart-pr-testing.md` for deployment commands.
</principle>

</essential_principles>

<intake>
Expand Down
96 changes: 96 additions & 0 deletions skills/rhdh-pr-review/references/chart-pr-testing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
# Reference: rhdh-chart PR Testing

How to validate and deploy chart changes from a PR. The CI workflow definition lives in the rhdh-chart repo at `.github/workflows/test.yaml` and `.github/actions/test-charts/action.yml` — read them for the authoritative CI behavior.

<what_to_know>

## Key Facts

- Chart CI does NOT build container images — it runs `ct lint` + `ct install` on a KIND cluster
- Charts are published to `oci://quay.io/rhdh/chart` only on merge (via `chart-releaser-action`)
- To test a PR's chart changes, you must check out the PR branch and deploy from the local checkout
- Pre-commit hooks auto-generate `README.md` (helm-docs), `values.schema.json` (jsonschema-dereference), and update Helm dependencies
- Chart architecture details are in `../../rhdh/references/rhdh-repos.md` under the `rhdh-chart` section — do not re-document here

</what_to_know>

<getting_pr_chart>

## Getting the PR Chart Locally

**Requires a local clone of rhdh-chart.** `gh pr checkout` operates on the current git repo — it will fail if CWD is not inside an rhdh-chart clone.

```bash
REPO="redhat-developer/rhdh-chart"
PR_NUMBER=<number>

# Ensure you're inside a local rhdh-chart clone (clone first if needed)
# gh repo clone $REPO && cd rhdh-chart

# Checkout the PR branch
gh pr checkout $PR_NUMBER --repo $REPO --detach

# Update Helm dependencies (required before template/install)
helm repo add bitnami https://charts.bitnami.com/bitnami
helm dependency update charts/backstage
```

If you already have a local clone:

```bash
PR_BRANCH=$(gh pr view $PR_NUMBER --repo $REPO --json headRefName --jq '.headRefName')
git fetch origin $PR_BRANCH
git checkout FETCH_HEAD
helm dependency update charts/backstage
```

</getting_pr_chart>

<local_validation>

## Local Validation

```bash
# Lint the chart
ct lint --config ct-lint.yaml

# Render templates to verify they produce valid YAML
helm template rhdh charts/backstage/ --debug

# Run pre-commit hooks (schema regen, helm-docs, dependency update)
pre-commit run --all-files
```

**Check Chart.yaml version was bumped** — the PR checklist requires it:

```bash
# Compare with base branch
git diff origin/main -- charts/backstage/Chart.yaml | grep '^[+-]version:'
```

</local_validation>

<deploying_to_cluster>

## Deploying to a Cluster

**OpenShift:**

```bash
CLUSTER_ROUTER_BASE=$(oc get route console -n openshift-console -o jsonpath='{.status.ingress[0].host}' | sed 's/^console-openshift-console\.//')

helm upgrade -i redhat-developer-hub charts/backstage/ \
--set global.clusterRouterBase=$CLUSTER_ROUTER_BASE \
--set route.enabled=true
```

**Vanilla Kubernetes:**

```bash
helm upgrade -i redhat-developer-hub charts/backstage/ \
--set route.enabled=false \
--set upstream.ingress.enabled=true \
--set global.host=rhdh.127.0.0.1.sslip.io
```

</deploying_to_cluster>
49 changes: 49 additions & 0 deletions skills/rhdh-pr-review/references/shared-findings-structure.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# Reference: Findings & Recommendations Structure (Shared)

Shared Phase 7 for all PR review workflows. The calling workflow defines repo-specific best-practice bullets and rollback commands before referencing this file.

<findings_structure>

## Findings & Recommendations

Synthesize the verification results and provide a complete review assessment.

### 7.1 Verification summary

Summarize what was tested and the results:

| Category | Test performed | Result | Evidence |
|---|---|---|---|
| *[category]* | *[what was tested]* | Pass/Fail | *[key observation]* |

### 7.2 Best practice assessment

Review the PR's approach against the repo's development best practices. Reference `../../rhdh/references/rhdh-repos.md` for conventions. Use the repo-specific best-practice bullets defined by the calling workflow.

### 7.3 Security review

Evaluate the changes from a security perspective:

- Are secrets handled safely (no plaintext logging, proper Secret resources)?
- Do RBAC changes follow least-privilege principle?
- Are container image references pinned appropriately?
- Are new network exposures (ports, routes, service accounts) intentional and documented?
- Do dependency updates introduce known CVEs?
- Are user-supplied inputs validated before use in resource names or labels?

Add any repo-specific security concerns defined by the calling workflow.

### 7.4 Improvement suggestions

Based on the findings, suggest concrete improvements if any:

- Code or template changes needed (reference specific files and lines from the diff)
- Missing test coverage for the changed code paths
- Documentation gaps
- Configuration or operational concerns

### 7.5 Rollback instructions

Present the rollback commands recorded in Phase 4. Include verification that the rollback succeeded.

</findings_structure>
30 changes: 30 additions & 0 deletions skills/rhdh-pr-review/references/shared-pr-fetch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# Reference: Fetch PR Context (Shared)

Shared Phase 1 for all PR review workflows. The calling workflow must set `REPO` and `PR_NUMBER` before referencing this file.

<fetch_pr>

## Fetch PR Context

```bash
gh pr view $PR_NUMBER --repo $REPO \
--json number,title,state,author,body,files,createdAt,headRefOid,baseRefName
```

Validate:
- PR state is `OPEN` (warn if merged or closed — artifacts may still work but PR is not active)
- PR belongs to the expected `$REPO`

Fetch the diff for later checklist generation:

```bash
gh pr diff $PR_NUMBER --repo $REPO
```

Save the changed file list for Phase 5:

```bash
gh pr view $PR_NUMBER --repo $REPO --json files --jq '.files[].path'
```

</fetch_pr>
36 changes: 36 additions & 0 deletions skills/rhdh-pr-review/references/shared-verification.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# Reference: Active Verification Process (Shared)

Shared Phase 6 for all PR review workflows. This phase verifies the PR's specific changes on the cluster — not generic health checks. The goal is to exercise the exact code paths the PR modified and capture evidence that the behavioral change works as intended.

<verification_process>

## Active Verification

### 6.1 Analyze the diff

Read the diff hunks from Phase 1. For each changed file, understand:

- What the code did **before** the change
- What it does **after**
- What behavioral difference this introduces on a running cluster

Map each change to a concrete cluster-observable effect — something you can trigger and measure. If a change has no cluster-observable effect (e.g., pure refactor with identical behavior, documentation-only update, CI config change), state that explicitly and explain why.

### 6.2 Propose a verification plan

Present the plan to the user. For each test, specify:

- **What to do**: the exact cluster action
- **What to observe**: where to look (logs, resource spec, status, events, HTTP response)
- **Pass criteria**: what output means the change works
- **Fail criteria**: what output means the change is broken

**STOP. Do not run any verification commands. Present the plan and wait for the user to accept it before proceeding to 6.3.**

### 6.3 Execute the plan

Only after the user accepts the plan:

Run each verification step on the cluster. For every step, capture the actual command output as evidence. Do not summarize — show the raw output so the user can see exactly what happened.

</verification_process>
Loading
Loading