feat(workflows): add optional per-job runner_type overrides - #636
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
Summary by CodeRabbit
WalkthroughThe pull request adds per-job runner overrides across reusable workflows and their callers. Each override uses ChangesWorkflow runner overrides
Deployment matrix changes
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
…-hub app to deployment matrix
Adds `severino-testing-final` to `apps.registry` and to `clusters.anacleto.apps` and `clusters.benedita.apps` (the repo's own lint requires an app in a cluster list to also be in the registry). Which environments actually receive updates is NOT decided here — this file has no environment dimension. It is decided by whether the app's values.yaml exists in the gitops repo, which the same automation created only for the environments the requester selected. Environments without one are logged as a WARNING by gitops-update.yml and skipped. Provisioned-By: Severino app-provisioning automation Provisioning-Request: 41ab349b-4565-43f0-9793-041a799f17ad Provisioning-App: severino-testing-final Provisioning-Requester: admin Provisioning-Team: devops-team
Removes `severino-testing-final` from `apps.registry`, from every cluster list it appeared under, and from any `app_helmfile_env` override — the app was torn down, so leaving it registered would keep gitops-update attempting it on every release and logging "values file not found" indefinitely. Provisioned-By: Severino app-provisioning automation Provisioning-Request: 41ab349b-4565-43f0-9793-041a799f17ad Provisioning-App: severino-testing-final Provisioning-Requester: admin Provisioning-Team: devops-team
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
6fd1c48 to
5513648
Compare
🔍 PR Validation Summary✅ PR Mergeable — no blocking failures
|
🔍 Lint Analysis
|
🛡️ CodeQL Analysis ResultsLanguages analyzed: Found 1 issue(s): 1 Medium
🔍 View full scan logs | 🛡️ Security tab |
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 4
🤖 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/go-release.yml:
- Line 445: Update the extra_build invocation to forward the build_runner_type
input alongside runner_type, matching the primary build call so matrix jobs use
the configured runner override.
In @.github/workflows/release.yml:
- Around line 31-34: Update the documented js-release.yml umbrella workflow to
declare the publish_runner_type input and forward its value into the underlying
release workflow’s corresponding input, then update docs/js-release.md to
document the new option and mapping. Preserve the existing fallback behavior
when the input is empty.
In `@docs/go-pr-analysis-workflow.md`:
- Around line 99-101: Update the documentation entries for test_runner_type,
coverage_runner_type, and build_runner_type to describe their fallback chain: an
empty value uses vars.GENERAL_RUNNERS, then runner_type. Keep the input table
synchronized with the workflow contract and match the fallback wording used for
lint_runner_type.
In `@docs/go-release-workflow.md`:
- Line 30: Update the release_runner_type row in the inputs table to document
its empty-value fallback chain: vars.GENERAL_RUNNERS, then runner_type, matching
the workflow input description and keeping the table synchronized with the
workflow behavior.
🪄 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: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1565453d-4f71-4ec5-bfd2-d020f17a2a4d
📒 Files selected for processing (15)
.github/workflows/build.yml.github/workflows/go-pr-analysis.yml.github/workflows/go-pr-validation.yml.github/workflows/go-release.yml.github/workflows/pr-security-scan.yml.github/workflows/pr-validation.yml.github/workflows/release.ymlconfig/deployment-matrix.ymldocs/build-workflow.mddocs/go-pr-analysis-workflow.mddocs/go-pr-validation.mddocs/go-release-workflow.mddocs/pr-security-scan-workflow.mddocs/pr-validation.mddocs/release-workflow.md
…s-release forwarding Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
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/build.yml:
- Line 284: Restrict the build workflow’s callable execution so pull-request
callers cannot run it with fork-controlled inputs. Update the job/workflow
guards around the build entry point and runner selection using the visible
build_runner_type, runner_type, and checkout_ref inputs, allowing only trusted
release callers and approved runners while preserving legitimate release builds.
In `@docs/go-pr-validation.md`:
- Around line 26-31: Update the inputs tables to document the fallback chain
vars.GENERAL_RUNNERS, then runner_type for all six inputs in
docs/go-pr-validation.md lines 26-31: lint_runner_type, test_runner_type,
coverage_runner_type, build_runner_type, security_scan_runner_type, and
pr_checks_summary_runner_type. Apply the same documentation update to
release_runner_type in docs/go-release-workflow.md line 30, keeping both tables
synchronized with the workflow behavior.
🪄 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: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 741790c9-ae56-4b4f-baf0-ad9e5e1535b0
📒 Files selected for processing (15)
.github/workflows/build.yml.github/workflows/go-pr-analysis.yml.github/workflows/go-pr-validation.yml.github/workflows/go-release.yml.github/workflows/pr-security-scan.yml.github/workflows/pr-validation.yml.github/workflows/release.ymlconfig/deployment-matrix.ymldocs/build-workflow.mddocs/go-pr-analysis-workflow.mddocs/go-pr-validation.mddocs/go-release-workflow.mddocs/pr-security-scan-workflow.mddocs/pr-validation.mddocs/release-workflow.md
| needs: prepare | ||
| if: needs.prepare.outputs.has_builds == 'true' | ||
| runs-on: ${{ vars.GENERAL_RUNNERS || inputs.runner_type }} | ||
| runs-on: ${{ inputs.build_runner_type || vars.GENERAL_RUNNERS || inputs.runner_type }} |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 12 \
'pull_request|uses:.*build\.yml|checkout_ref|build_runner_type|id-token: write|packages: write' \
.github/workflowsRepository: LerianStudio/github-actions-shared-workflows
Length of output: 50401
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- build.yml structure ---'
ast-grep outline .github/workflows/build.yml
printf '%s\n' '--- build.yml inputs, triggers, checkout, runner, and job conditions ---'
rg -n -C 8 \
'workflow_call|workflow_dispatch|checkout_ref|build_runner_type|runs-on:|actions/checkout|if:' \
.github/workflows/build.yml
printf '%s\n' '--- all build.yml callers ---'
rg -n -C 10 \
'uses:\s+(\./)?\.github/workflows/build\.yml|uses:.*build\.yml' \
.github/workflows
printf '%s\n' '--- caller trigger declarations ---'
python3 - <<'PY'
from pathlib import Path
import re
for path in Path(".github/workflows").glob("*.yml"):
text = path.read_text()
if re.search(r'uses:\s+.*build\.yml', text):
print(f"\n### {path}")
for i, line in enumerate(text.splitlines(), 1):
if re.match(r'\s*(on:|workflow_call:|workflow_dispatch:|pull_request(?:_target)?:|push:|workflow_run:)', line):
print(f"{i}:{line}")
PYRepository: LerianStudio/github-actions-shared-workflows
Length of output: 19991
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- build.yml trigger and input definitions ---'
sed -n '1,205p' .github/workflows/build.yml
printf '%s\n' '--- go-release trigger and build caller conditions ---'
sed -n '1,80p' .github/workflows/go-release.yml
sed -n '400,470p' .github/workflows/go-release.yml
sed -n '610,655p' .github/workflows/go-release.yml
printf '%s\n' '--- every repository reference to build.yml ---'
rg -n -C 4 \
'build\.yml|checkout_ref|build_runner_type|runner_type' \
--glob '!*.log' \
--glob '!node_modules/**' \
.
printf '%s\n' '--- pull-request triggers in workflows that call reusable workflows ---'
python3 - <<'PY'
from pathlib import Path
for path in Path(".github/workflows").glob("*.yml"):
lines = path.read_text().splitlines()
has_build_call = any("build.yml" in line for line in lines)
if not has_build_call:
continue
print(f"\n### {path}")
for i, line in enumerate(lines, 1):
if any(token in line for token in (
"pull_request:", "pull_request_target:", "workflow_call:",
"workflow_dispatch:", "push:", "workflow_run:"
)):
print(f"{i}:{line}")
PYRepository: LerianStudio/github-actions-shared-workflows
Length of output: 50403
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- build workflow documentation and examples ---'
sed -n '1,125p' docs/build-workflow.md
sed -n '200,240p' docs/build-workflow.md
printf '%s\n' '--- repository guidance for pull_request runners and reusable callers ---'
rg -n -C 5 \
'pull_request|self-hosted|workflow_call|external|build_runner_type|checkout_ref' \
AGENTS.md docs .github/workflows \
--glob '*.md' --glob '*.yml'
printf '%s\n' '--- static caller/event summary ---'
python3 - <<'PY'
from pathlib import Path
import re
for path in sorted(Path(".github/workflows").glob("*.yml")):
text = path.read_text()
if "build.yml" not in text:
continue
events = re.findall(
r"^\s{0,4}(pull_request_target|pull_request|workflow_call|workflow_dispatch|workflow_run|push):",
text,
flags=re.MULTILINE,
)
calls = [
line.strip()
for line in text.splitlines()
if "build.yml" in line and "uses:" in line
]
print(path)
print("events:", events or ["none"])
print("calls:", calls)
PYRepository: LerianStudio/github-actions-shared-workflows
Length of output: 50403
Block pull-request execution of this build job.
build.yml is callable by external repositories, has no event guard, and accepts unrestricted checkout_ref and build_runner_type inputs. A pull_request caller can therefore run fork-controlled Docker build code on a caller-selected runner, including a self-hosted runner. Reject pull-request events or restrict this workflow to trusted release callers and approved runners.
🤖 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/build.yml at line 284, Restrict the build workflow’s
callable execution so pull-request callers cannot run it with fork-controlled
inputs. Update the job/workflow guards around the build entry point and runner
selection using the visible build_runner_type, runner_type, and checkout_ref
inputs, allowing only trusted release callers and approved runners while
preserving legitimate release builds.
| | `lint_runner_type` | Optional runner override for the Go analysis Lint jobs only | string | `''` | | ||
| | `test_runner_type` | Optional runner override for the Go analysis Tests jobs only | string | `''` | | ||
| | `coverage_runner_type` | Optional runner override for the Go analysis Coverage jobs only | string | `''` | | ||
| | `build_runner_type` | Optional runner override for the Go analysis Build jobs only | string | `''` | | ||
| | `security_scan_runner_type` | Optional runner override for the security_scan jobs only | string | `''` | | ||
| | `pr_checks_summary_runner_type` | Optional runner override for the PR Checks Summary job only | string | `''` | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keep runner input tables synchronized with the fallback contract.
The new runner inputs use the same empty-value resolution: vars.GENERAL_RUNNERS, then runner_type. The documentation omits this contract in both tables.
docs/go-pr-validation.md#L26-L31: add the fallback chain tolint_runner_type,test_runner_type,coverage_runner_type,build_runner_type,security_scan_runner_type, andpr_checks_summary_runner_type.docs/go-release-workflow.md#L30-L30: add the fallback chain torelease_runner_type.
As per path instructions, keep the inputs table synchronized with the corresponding workflow changes in this PR.
📍 Affects 2 files
docs/go-pr-validation.md#L26-L31(this comment)docs/go-release-workflow.md#L30-L30
🤖 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/go-pr-validation.md` around lines 26 - 31, Update the inputs tables to
document the fallback chain vars.GENERAL_RUNNERS, then runner_type for all six
inputs in docs/go-pr-validation.md lines 26-31: lint_runner_type,
test_runner_type, coverage_runner_type, build_runner_type,
security_scan_runner_type, and pr_checks_summary_runner_type. Apply the same
documentation update to release_runner_type in docs/go-release-workflow.md line
30, keeping both tables synchronized with the workflow behavior.
Source: Path instructions
Description
Today every job in the Go umbrella pipelines runs on the single
runner_typeinput, so a caller cannot right-size individual jobs: Blacksmith telemetry onLerianStudio/matchershows the Go Lint/Tests jobs saturating all 4 vCPUs (95th percentile CPU at 99-100%) while trivial gate/summary jobs idle at ~20% average CPU on the same SKU. This PR adds optional per-job runner overrides so callers can tune hot and idle jobs independently, without changing anything for callers that do not set them.Affected workflows:
go-pr-validation.yml,go-pr-analysis.yml,pr-security-scan.yml,pr-validation.yml,go-release.yml,build.yml,release.yml(plus their docs pages).New inputs (all
type: string,default: ''= no override):go-pr-analysis.yml:lint_runner_type,test_runner_type,coverage_runner_type,build_runner_typepr-security-scan.yml:security_scan_runner_typepr-validation.yml:pr_checks_summary_runner_typego-pr-validation.yml(umbrella): exposes and forwards all of the above, plusgate_runner_typefor its own utility jobs (Detect non-doc changes and the Go Analysis / Security / Lib Version result gates)build.yml:build_runner_type(Build jobs only; prepare/notify stay onrunner_type)release.yml:publish_runner_type(Release publish jobs only)go-release.yml(umbrella):build_runner_typeandrelease_runner_type, forwarded tobuild.yml/release.yml(extra_buildintentionally stays onrunner_type)The resolution pattern on every touched
runs-on:line is:An explicit per-job override deliberately wins over
vars.GENERAL_RUNNERS, since it is an ad-hoc, caller-declared decision for that specific job; leaving it empty preserves today's resolution exactly. This mirrors the precedent of the existing job-scoped runner inputs ingo-release.yml(apidog_runner_type,e2e_runner_type,ungoliant_runner_type).First consumer:
LerianStudio/matcher, which will pin Go Lint/Tests, the root security scan, and the release image build toblacksmith-8vcpu-ubuntu-2404, and Coverage, PR Checks Summary, the Lib Version gate, and the release publish job toblacksmith-2vcpu-ubuntu-2404, per its 30-day right-sizing report.Type of Change
feat: New workflow or new input/output/step in an existing workflowfix: Bug fix in a workflow (incorrect behavior, broken step, wrong condition)perf: Performance improvement (e.g. caching, parallelism, reduced steps)refactor: Internal restructuring with no behavior changedocs: Documentation only (README, docs/, inline comments)ci: Changes to self-CI (workflows under.github/workflows/that run on this repo)chore: Dependency bumps, config updates, maintenancetest: Adding or updating testsBREAKING CHANGE: Callers must update their configuration after this PRBreaking Changes
None. All new inputs default to
'', which reproduces the currentvars.GENERAL_RUNNERS || inputs.runner_typeresolution unchanged. No existing input names, defaults, outputs, or secrets were modified.Testing
@this-branchor the beta tagCaller repo / workflow run: to be validated from
LerianStudio/matcherby pointingpr-validation.ymlat@feat/per-job-runner-overrideswith the new inputs set, before tagging.Related Issues
N/A (follow-up to the matcher runner right-sizing report)