NO-JIRA: Update GHA docs for reusable workflow pattern#8453
NO-JIRA: Update GHA docs for reusable workflow pattern#8453openshift-merge-bot[bot] merged 2 commits intoopenshift:mainfrom
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
Skipping CI for Draft Pull Request. |
|
@bryan-cox: This pull request explicitly references no jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis pull request documents and implements a caller + reusable GitHub Actions workflow pattern for HyperShift CI, adds a dedicated GitHub Actions guide, updates docs preview docs to a two-workflow build/deploy split, and updates the runner README with caller/reusable examples. It adds nav entries to docs, and enhances Sequence Diagram(s)sequenceDiagram
autonumber
participant Contributor as Contributor/PR
participant GitHub as GitHub (Actions)
participant Caller as Caller Workflow
participant Reusable as Reusable Workflow (`@main`)
participant Runner as ARC Runner (self-hosted)
participant Artifact as Actions Artifact (docs-site)
participant DocsDeploy as Docs Deploy Workflow
participant Cloudflare as Cloudflare Pages
participant GHDeploy as GitHub Deployments API
Contributor->>GitHub: Open PR (changes under docs/)
GitHub->>Caller: Trigger pull_request caller workflow
Caller->>Reusable: uses: reusable-workflow.yaml@main (workflow_call)
Reusable->>Runner: run jobs on arc-runner-set
Runner->>Reusable: build docs, produce site
Reusable->>Artifact: upload docs-site artifact
Artifact->>DocsDeploy: artifact available (workflow_run trigger)
GitHub->>DocsDeploy: workflow_run (on: workflow_run)
DocsDeploy->>Artifact: download docs-site artifact
DocsDeploy->>Cloudflare: deploy site (compute preview URL)
DocsDeploy->>GHDeploy: query deployment by head_sha, set deployment status + environment_url
GHDeploy-->>GitHub: Deployment status updated
GitHub-->>Contributor: "View deployment" link in PR (docs-preview environment)
🚥 Pre-merge checks | ✅ 12✅ Passed checks (12 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bryan-cox The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/content/how-to/ci/github-actions.md (1)
28-29: ⚡ Quick winAvoid absolute security wording for
@mainpinning.Line 28 overstates the guarantee. The
@mainreference pinning ensures the called reusable workflow comes frommain, but does not prevent PRs from modifying the caller workflow itself (which specifies theuses:ref). Rephrase this as risk reduction and note that caller workflow protection is essential.✏️ Suggested wording
-- **Security** — callers pin reusable workflows to `@main`, ensuring PRs cannot modify the workflow code they run under. +- **Security** — callers pin reusable workflows to `@main`, which reduces the risk of PRs changing reusable job logic. Protect caller workflows (for example with branch protection/CODEOWNERS) so untrusted PRs cannot alter the called ref.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/content/how-to/ci/github-actions.md` around lines 28 - 29, Reword the sentence that currently states "**Security** — callers pin reusable workflows to `@main`, ensuring PRs cannot modify the workflow code they run under." to avoid absolute language: say that pinning the reusable workflow reference to `@main` reduces the risk of a PR altering the called reusable workflow, but it does not prevent PRs from changing the caller workflow (the file that sets the `uses:` ref); also add a short note that protecting caller workflows (branch protections/required reviews) is essential to maintain the intended security model.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@docs/content/how-to/ci/github-actions.md`:
- Around line 28-29: Reword the sentence that currently states "**Security** —
callers pin reusable workflows to `@main`, ensuring PRs cannot modify the
workflow code they run under." to avoid absolute language: say that pinning the
reusable workflow reference to `@main` reduces the risk of a PR altering the
called reusable workflow, but it does not prevent PRs from changing the caller
workflow (the file that sets the `uses:` ref); also add a short note that
protecting caller workflows (branch protections/required reviews) is essential
to maintain the intended security model.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 31514dd1-eaf5-4cab-8b62-3973a2abbcd9
⛔ Files ignored due to path filters (1)
docs/content/reference/aggregated-docs.mdis excluded by!docs/content/reference/aggregated-docs.md
📒 Files selected for processing (4)
docs/content/how-to/ci/docs-preview.mddocs/content/how-to/ci/github-actions.mddocs/mkdocs.ymlhack/github-actions-runner/README.md
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8453 +/- ##
==========================================
+ Coverage 37.49% 37.53% +0.04%
==========================================
Files 751 751
Lines 91984 92025 +41
==========================================
+ Hits 34487 34543 +56
+ Misses 54854 54841 -13
+ Partials 2643 2641 -2 see 5 files with indirect coverage changes
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/docs-deploy.yaml:
- Around line 49-52: The step unconditionally uses PR_NUMBER to build ENV_URL
which can be empty/corrupt; add a guard validating PR_NUMBER is a non-empty
numeric string (e.g., regex check like digits only) before constructing ENV_URL
and any deployment metadata, and if validation fails either set ENV_URL to an
empty/null value and skip downstream deploy steps or fail early with a clear
message; reference the variables PR_NUMBER, HEAD_SHA and ENV_URL when
implementing the conditional so the assignment only happens when PR_NUMBER
passes validation.
- Around line 53-59: The deployment environment name is hardcoded as
"docs-preview", causing cross-PR interference; update the JSON sent when
creating the deployment (the command that sets DEPLOY_ID via HEAD_SHA and gh api
"repos/${{ github.repository }}/deployments") to use a per-PR environment like
"docs-preview/pr-${PR_NUMBER}" so each PR gets its own environment, and ensure
the subsequent gh api call that posts the status still references that
deployment ID (DEPLOY_ID) and environment_url as before; confirm proper shell
variable expansion/quoting for PR_NUMBER in the JSON payload.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 5d5a87e2-803f-4544-8569-8203e35dee9f
📒 Files selected for processing (1)
.github/workflows/docs-deploy.yaml
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/docs-deploy-reusable.yaml:
- Around line 50-61: Validate that PR_NUMBER is present and a non-empty numeric
value before using it (return/log error and exit if invalid), then derive a
per-PR deployment environment name (e.g.,
ENV_NAME="docs-preview-pr-${PR_NUMBER}") and use that ENV_NAME wherever ENV_URL
and the Deployments API environment are set (replace ENV_URL and the
"environment":"docs-preview" payload value). Also include "auto_inactive": false
in the deployment creation payload (the JSON assigned to DEPLOY_ID) so separate
PR deployments don’t inactivate each other, and ensure the final statuses API
call uses DEPLOY_ID and environment_url built from the validated
PR_NUMBER/ENV_NAME.
In @.github/workflows/docs-deploy.yaml:
- Around line 12-17: The workflow's deploy job currently points to the test
branch reference
"openshift/hypershift/.github/workflows/docs-deploy-reusable.yaml@update-gha-docs";
before merging update that "uses" ref to point to the production branch by
replacing "@update-gha-docs" with "@main" so the deploy job uses the main
reusable workflow (look for the deploy job and the uses string to change).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 5ed634a8-6b2c-4f6f-b494-9b033d4f6b66
📒 Files selected for processing (2)
.github/workflows/docs-deploy-reusable.yaml.github/workflows/docs-deploy.yaml
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/docs-deploy.yaml (1)
49-54:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReintroduce the previous guard + per-PR deployment environment isolation.
Line 49 uses
PR_NUMBERwithout validation, and Line 53 hardcodes"environment":"docs-preview". This reopens malformed-metadata risk and cross-PR deployment-state interference.Suggested patch
PR_NUMBER="${{ steps.pr.outputs.number }}" HEAD_SHA="${{ github.event.workflow_run.head_sha }}" + if ! [[ "${PR_NUMBER}" =~ ^[0-9]+$ ]]; then + echo "Invalid PR number: '${PR_NUMBER}'" + exit 1 + fi + DEPLOY_ENV="docs-preview/pr-${PR_NUMBER}" ENV_URL="https://pr-${PR_NUMBER}.hypershift.pages.dev" - DEPLOY_ID=$(echo '{"ref":"'"${HEAD_SHA}"'","environment":"docs-preview","auto_merge":false,"required_contexts":[]}' \ + DEPLOY_ID=$(echo '{"ref":"'"${HEAD_SHA}"'","environment":"'"${DEPLOY_ENV}"'","auto_merge":false,"required_contexts":[]}' \ | gh api "repos/${{ github.repository }}/deployments" --input - --jq '.id')In the GitHub Deployments API, when creating a successful deployment status, does default auto_inactive affect previous successful deployments in the same environment?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/docs-deploy.yaml around lines 49 - 54, Validate PR_NUMBER is present and numeric before proceeding (fail fast if empty/invalid), and create a per-PR deployment environment name instead of the fixed "docs-preview" (e.g., ENV_NAME="docs-preview-pr-${PR_NUMBER}") so deployments are isolated per PR; then use that ENV_NAME in the gh api payload when setting DEPLOY_ID and ensure the JSON payload is produced safely (avoid manual string concatenation) so the "environment" field is well-formed and cannot leak between PRs — reference PR_NUMBER, ENV_URL/ENV_NAME, HEAD_SHA and the DEPLOY_ID gh api call in the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/docs-deploy.yaml:
- Around line 54-57: Ensure DEPLOY_ID is validated after it's assigned (the gh
api call that captures the deployment id into DEPLOY_ID) before invoking the
second gh api to post a status; specifically check that DEPLOY_ID is non-empty
and numeric and if it is not, emit a clear error and exit non-zero so the script
does not call /deployments/${DEPLOY_ID}/statuses with null/empty. Reference the
DEPLOY_ID variable and the two gh api commands (the one that sets DEPLOY_ID and
the subsequent gh api "repos/${{ github.repository
}}/deployments/${DEPLOY_ID}/statuses") and add a simple conditional guard that
logs the failure and aborts when DEPLOY_ID is invalid.
---
Duplicate comments:
In @.github/workflows/docs-deploy.yaml:
- Around line 49-54: Validate PR_NUMBER is present and numeric before proceeding
(fail fast if empty/invalid), and create a per-PR deployment environment name
instead of the fixed "docs-preview" (e.g.,
ENV_NAME="docs-preview-pr-${PR_NUMBER}") so deployments are isolated per PR;
then use that ENV_NAME in the gh api payload when setting DEPLOY_ID and ensure
the JSON payload is produced safely (avoid manual string concatenation) so the
"environment" field is well-formed and cannot leak between PRs — reference
PR_NUMBER, ENV_URL/ENV_NAME, HEAD_SHA and the DEPLOY_ID gh api call in the
change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 2e5abd61-0041-4998-87bf-a3034bdf396d
📒 Files selected for processing (1)
.github/workflows/docs-deploy.yaml
There was a problem hiding this comment.
♻️ Duplicate comments (2)
.github/workflows/docs-deploy.yaml (2)
53-59:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse a per-PR deployment environment instead of shared
docs-preview.Using one shared environment can cause deployment lifecycle collisions across open PR previews. Isolate by PR (for example
docs-preview/pr-${PR_NUMBER}) when creating/updating deployment status.Suggested patch
PR_NUMBER="${{ steps.pr.outputs.number }}" HEAD_SHA="${{ github.event.workflow_run.head_sha }}" + DEPLOY_ENV="docs-preview/pr-${PR_NUMBER}" ENV_URL="https://pr-${PR_NUMBER}.hypershift.pages.dev" - DEPLOY_ID=$(echo '{"ref":"'"${HEAD_SHA}"'","environment":"docs-preview","auto_merge":false,"required_contexts":[]}' \ + DEPLOY_ID=$(echo '{"ref":"'"${HEAD_SHA}"'","environment":"'"${DEPLOY_ENV}"'","auto_merge":false,"required_contexts":[]}' \ | gh api "repos/${{ github.repository }}/deployments" --input - --jq '.id') gh api "repos/${{ github.repository }}/deployments/${DEPLOY_ID}/statuses" \ -f state="success" \ + -f environment="${DEPLOY_ENV}" \ -f environment_url="${ENV_URL}" \ -f description="Docs preview for PR #${PR_NUMBER}"In the GitHub REST Deployments API, when creating deployment statuses, does successful status creation auto-inactivate prior deployments in the same environment by default (`auto_inactive=true`)?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/docs-deploy.yaml around lines 53 - 59, The workflow uses a shared deployment environment "docs-preview" which can cause collisions; update the deployment creation and status calls to use a per-PR environment name like docs-preview/pr-${PR_NUMBER} instead of the static "docs-preview", i.e. construct the environment string from PR_NUMBER and pass it to the gh api call that creates the deployment (the invocation that produces DEPLOY_ID via "repos/${{ github.repository }}/deployments") and to the subsequent statuses call that posts the success (the gh api call that uses DEPLOY_ID and sets state, environment_url/ENV_URL, description); also ensure you pass auto_inactive=true when creating/updating statuses so prior deployments in the same environment are inactivated.
49-56:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate
PR_NUMBERandDEPLOY_IDbefore using them in downstream calls.Both values are used as trusted identifiers, but neither is validated. A simple numeric guard avoids malformed deployment/status API calls and makes failures explicit.
Suggested patch
PR_NUMBER="${{ steps.pr.outputs.number }}" HEAD_SHA="${{ github.event.workflow_run.head_sha }}" + if ! [[ "${PR_NUMBER}" =~ ^[0-9]+$ ]]; then + echo "Invalid PR number: '${PR_NUMBER}'" + exit 1 + fi ENV_URL="https://pr-${PR_NUMBER}.hypershift.pages.dev" DEPLOY_ID=$(echo '{"ref":"'"${HEAD_SHA}"'","environment":"docs-preview","auto_merge":false,"required_contexts":[]}' \ | gh api "repos/${{ github.repository }}/deployments" --input - --jq '.id') + if ! [[ "${DEPLOY_ID}" =~ ^[0-9]+$ ]]; then + echo "Failed to create deployment. DEPLOY_ID='${DEPLOY_ID}'" + exit 1 + fi gh api "repos/${{ github.repository }}/deployments/${DEPLOY_ID}/statuses" \🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/docs-deploy.yaml around lines 49 - 56, Validate PR_NUMBER is a non-empty integer and DEPLOY_ID is present and numeric before they are used; after setting PR_NUMBER and after computing DEPLOY_ID check PR_NUMBER matches a numeric regex (e.g. /^[0-9]+$/) and that DEPLOY_ID is non-empty and numeric, and if either check fails echo a clear error including the variable name and exit non-zero so the workflow doesn't call gh api with malformed identifiers; perform these checks before using ENV_URL or calling gh api "repos/.../deployments/${DEPLOY_ID}/statuses".
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In @.github/workflows/docs-deploy.yaml:
- Around line 53-59: The workflow uses a shared deployment environment
"docs-preview" which can cause collisions; update the deployment creation and
status calls to use a per-PR environment name like docs-preview/pr-${PR_NUMBER}
instead of the static "docs-preview", i.e. construct the environment string from
PR_NUMBER and pass it to the gh api call that creates the deployment (the
invocation that produces DEPLOY_ID via "repos/${{ github.repository
}}/deployments") and to the subsequent statuses call that posts the success (the
gh api call that uses DEPLOY_ID and sets state, environment_url/ENV_URL,
description); also ensure you pass auto_inactive=true when creating/updating
statuses so prior deployments in the same environment are inactivated.
- Around line 49-56: Validate PR_NUMBER is a non-empty integer and DEPLOY_ID is
present and numeric before they are used; after setting PR_NUMBER and after
computing DEPLOY_ID check PR_NUMBER matches a numeric regex (e.g. /^[0-9]+$/)
and that DEPLOY_ID is non-empty and numeric, and if either check fails echo a
clear error including the variable name and exit non-zero so the workflow
doesn't call gh api with malformed identifiers; perform these checks before
using ENV_URL or calling gh api "repos/.../deployments/${DEPLOY_ID}/statuses".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 54c442d5-de54-4172-a9fb-5a8d9371fb6d
⛔ Files ignored due to path filters (1)
docs/content/reference/aggregated-docs.mdis excluded by!docs/content/reference/aggregated-docs.md
📒 Files selected for processing (5)
.github/workflows/docs-deploy.yamldocs/content/how-to/ci/docs-preview.mddocs/content/how-to/ci/github-actions.mddocs/mkdocs.ymlhack/github-actions-runner/README.md
✅ Files skipped from review due to trivial changes (2)
- docs/mkdocs.yml
- docs/content/how-to/ci/github-actions.md
- Update hack/github-actions-runner/README.md to document the caller + reusable workflow architecture with updated mermaid diagram and example workflow pair - Add new MkDocs page documenting all GitHub Actions workflows, the reusable pattern, and how to add new workflows - Update docs-preview page to reflect the two-workflow build/deploy split architecture - Soften security claim about @main pinning per review feedback - Add nav entries for GitHub Actions and Sync Community Fork pages Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The docs-deploy workflow runs via workflow_run, which executes in the context of the default branch. This means GitHub deployments are created against the main SHA rather than the PR head SHA, preventing the "View deployment" link from appearing on PRs. Create a GitHub deployment against the workflow_run head SHA and post a success status with the preview URL. Use per-PR deployment environments (docs-preview/pr-N) to prevent cross-PR interference from the auto_inactive default. Validate PR_NUMBER and DEPLOY_ID before use. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/lgtm |
|
Scheduling tests matching the |
|
/override e2e-aks |
|
/verified bypass |
|
@bryan-cox: The DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@bryan-cox: /override requires failed status contexts, check run or a prowjob name to operate on.
Only the following failed contexts/checkruns were expected:
If you are trying to override a checkrun that has a space in it, you must put a double quote on the context. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/override ci/prow/e2e-aks |
|
@bryan-cox: Overrode contexts on behalf of bryan-cox: ci/prow/e2e-aks, ci/prow/e2e-aws, ci/prow/e2e-aws-upgrade-hypershift-operator, ci/prow/e2e-azure-self-managed, ci/prow/e2e-azure-v2-self-managed, ci/prow/e2e-kubevirt-aws-ovn-reduced, ci/prow/e2e-v2-aws DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@bryan-cox: This pull request explicitly references no jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/override ci/prow/e2e-aks |
|
@bryan-cox: Overrode contexts on behalf of bryan-cox: ci/prow/e2e-aks, ci/prow/e2e-aws, ci/prow/e2e-aws-upgrade-hypershift-operator, ci/prow/e2e-azure-self-managed, ci/prow/e2e-azure-v2-self-managed, ci/prow/e2e-kubevirt-aws-ovn-reduced, ci/prow/e2e-v2-aws DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@bryan-cox: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Summary
hack/github-actions-runner/README.mdto reflect the caller + reusable workflow pattern adopted across all GHA workflowsTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
Improvements