ci: Make Linux CI runners configurable and split macOS legs into disable-able workflows. - #2445
ci: Make Linux CI runners configurable and split macOS legs into disable-able workflows.#2445jackluo923 wants to merge 2 commits into
Conversation
WalkthroughThe pull request configures GitHub Actions runner selection, separates amd64 and arm64 artifact packaging, and adds macOS workflows for documentation, linting, generated-code checks, and uv validation. ChangesArtifact build architecture flow
Linux and macOS workflow execution
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant DependencyImageJobs
participant PackageImageAmd64
participant PackageImageArm64
participant MultiArchManifest
DependencyImageJobs->>PackageImageAmd64: provide amd64 dependency image
DependencyImageJobs->>PackageImageArm64: provide arm64 dependency image
PackageImageAmd64->>MultiArchManifest: complete amd64 package image
PackageImageArm64->>MultiArchManifest: complete arm64 package image
MultiArchManifest->>MultiArchManifest: publish multi-architecture manifest
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
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/clp-artifact-build.yaml:
- Around line 785-787: Update the conditions controlling package-image-arm64 and
package-image-multiarch-manifest so successful, non-cancelled release-tag refs
(refs/tags/<release>) are permitted alongside the existing main-branch path;
apply this change at both .github/workflows/clp-artifact-build.yaml lines
785-787 and 830-834, preserving the existing exclusions for cancelled or failed
runs.
- Around line 752-780: The package-image-amd64 job currently exposes
GITHUB_TOKEN to the pull-request-controlled clp-build-runtime-image action.
Remove the image_registry_username and image_registry_password inputs from this
PR job, configure its permissions as read-only, and retain registry credentials
only in the trusted main-only publish job while preserving the runtime image
build behavior.
In @.github/workflows/clp-docs-macos.yaml:
- Around line 16-25: Restrict the documentation workflow permissions to
contents: read and update the actions/checkout step to set persist-credentials
to false. Apply these changes to the workflow containing the build-macos job,
preserving its existing LFS and recursive submodule settings.
In @.github/workflows/clp-lint-macos.yaml:
- Around line 22-24: Add top-level permissions restricting contents to read in
.github/workflows/clp-lint-macos.yaml,
.github/workflows/clp-s-generated-code-checks-macos.yaml, and
.github/workflows/clp-uv-checks-macos.yaml; in each workflow’s checkout step,
set persist-credentials to false while preserving recursive submodule checkout.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f20dfe61-dd22-4b0a-8ded-1efed1f0c46a
📒 Files selected for processing (14)
.github/workflows/clp-artifact-build.yaml.github/workflows/clp-core-build-macos.yaml.github/workflows/clp-docs-generated-code-checks.yaml.github/workflows/clp-docs-macos.yaml.github/workflows/clp-docs.yaml.github/workflows/clp-lint-macos.yaml.github/workflows/clp-lint.yaml.github/workflows/clp-package-helm.yaml.github/workflows/clp-pr-title-checks.yaml.github/workflows/clp-rust-checks.yaml.github/workflows/clp-s-generated-code-checks-macos.yaml.github/workflows/clp-s-generated-code-checks.yaml.github/workflows/clp-uv-checks-macos.yaml.github/workflows/clp-uv-checks.yaml
27dad11 to
dbe5341
Compare
…-able workflows Replace hard-coded self-hosted runner selectors with 4 repo vars GH_RUNNER_LINUX_[HEAVY|LIGHT]_[X64|ARM64]. Every Linux runs-on is `fromJSON(vars.X || '["<github-hosted>"]')`, so unset vars (upstream before config, or any fork) fall back to GitHub-hosted unchanged; no more repository_owner special-casing. The vars are named GH_RUNNER_LINUX_* (the LINUX segment scopes them to Linux legs; macOS is never overridden) and the HEAVY values include a "docker" label so artifact-build routes to the DinD-capable runners. The 4 macOS+Linux matrix workflows (clp-uv-checks, clp-lint, clp-docs, clp-s-generated-code-checks) are split into Linux-only originals plus new *-macos.yaml siblings, each gated by `if: vars.GH_DISABLE_MACOS != 'true'` so a fork without macOS capacity can skip every macOS job via one repo var (job-level if can't see the matrix context, hence the split). clp-core-build-macos gets the same gate. A skipped required check counts as success for branch protection; the var is opt-out (unset = macOS runs, so upstream and forks are unaffected). package-image is split into package-image-amd64 + package-image-arm64 (arm64 main-only via a plain if:); each depends only on its own arch's deps image (the old single matrix needed both), and package-image-multiarch-manifest still gates the release tag on both arches. Categorization by core-benefit: HEAVY_X64 = artifact-build x86 + clp-docs-generated-code-checks (release Rust codegen); HEAVY_ARM64 = package-image-arm64; LIGHT_X64 = lint/docs/s-gen/uv-checks/helm/pr-title/rust-checks; LIGHT_ARM64 = arm64 deps-images (network-bound). rust-checks stays on LIGHT_X64 because tests:rust-all starts LocalStack on 127.0.0.1, broken on self-hosted runners. Validated: yamllint clean; actionlint only the pre-existing SC2046 chown warnings on the runner-images#6775 workaround step.
…flows.
Applies the repository's existing hardening idiom to the four macOS workflows added by this branch: a workflow-level `permissions: {}` deny-all, a job-level `contents: "read"` grant for checkout, and `persist-credentials: false` so the token is not left in `.git/config` for subsequent steps.
dbe5341 to
7a3792d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/clp-artifact-build.yaml:
- Around line 750-753: Add job-level permissions to
ubuntu-jammy-integration-tests-core at
.github/workflows/clp-artifact-build.yaml:750-753, granting only contents:
"read". Also update package-image-amd64 at
.github/workflows/clp-artifact-build.yaml:799-810 with contents: "read" and
packages: "write" permissions for GHCR publishing.
🪄 Autofix
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d095d9fc-e472-4110-89de-59f1caac01ed
📒 Files selected for processing (1)
.github/workflows/clp-artifact-build.yaml
| ubuntu-jammy-integration-tests-core: | ||
| needs: "ubuntu-jammy-binaries" | ||
| runs-on: *runner | ||
| runs-on: >- | ||
| ${{ fromJSON(vars.GH_RUNNER_LINUX_HEAVY_X64 || '["ubuntu-24.04"]') }} |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Set explicit least-privilege permissions for the new jobs.
These jobs inherit the repository-default GITHUB_TOKEN scope. Set contents: "read" for ubuntu-jammy-integration-tests-core. Set contents: "read" and packages: "write" for package-image-amd64, because its trusted main path publishes to GHCR.
.github/workflows/clp-artifact-build.yaml#L750-L753: addpermissions: { contents: "read" }..github/workflows/clp-artifact-build.yaml#L799-L810: addpermissionswithcontents: "read"andpackages: "write".
Proposed fix
ubuntu-jammy-integration-tests-core:
+ permissions:
+ contents: "read"
needs: "ubuntu-jammy-binaries"
package-image-amd64:
+ permissions:
+ contents: "read"
+ packages: "write"
name: "package-image-amd64"Based on learnings, GitHub Actions workflows should grant only the permissions required by each job.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ubuntu-jammy-integration-tests-core: | |
| needs: "ubuntu-jammy-binaries" | |
| runs-on: *runner | |
| runs-on: >- | |
| ${{ fromJSON(vars.GH_RUNNER_LINUX_HEAVY_X64 || '["ubuntu-24.04"]') }} | |
| ubuntu-jammy-integration-tests-core: | |
| permissions: | |
| contents: "read" | |
| needs: "ubuntu-jammy-binaries" | |
| runs-on: >- | |
| ${{ fromJSON(vars.GH_RUNNER_LINUX_HEAVY_X64 || '["ubuntu-24.04"]') }} | |
| package-image-amd64: | |
| permissions: | |
| contents: "read" | |
| packages: "write" | |
| name: "package-image-amd64" | |
| if: >- | |
| !cancelled() && !failure() && ( | |
| needs.filter-relevant-changes.outputs.ubuntu_jammy_image_changed == 'false' | |
| || needs.ubuntu-jammy-x86_64-deps-image.result == 'success' | |
| ) | |
| needs: | |
| - "filter-relevant-changes" | |
| - "ubuntu-jammy-x86_64-deps-image" | |
| runs-on: >- | |
| ${{ fromJSON(vars.GH_RUNNER_LINUX_HEAVY_X64 || '["ubuntu-24.04"]') }} |
🧰 Tools
🪛 zizmor (1.29.0)
[warning] 750-797: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
[info] 750-750: workflow or action definition without a name (anonymous-definition): this job
(anonymous-definition)
📍 Affects 1 file
.github/workflows/clp-artifact-build.yaml#L750-L753(this comment).github/workflows/clp-artifact-build.yaml#L799-L810
🤖 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/clp-artifact-build.yaml around lines 750 - 753, Add
job-level permissions to ubuntu-jammy-integration-tests-core at
.github/workflows/clp-artifact-build.yaml:750-753, granting only contents:
"read". Also update package-image-amd64 at
.github/workflows/clp-artifact-build.yaml:799-810 with contents: "read" and
packages: "write" permissions for GHCR publishing.
Sources: Learnings, Linters/SAST tools
Description
Replaces the hard-coded
github.repository_owner == 'y-scope'runner selector with four repository variablesGH_RUNNER_LINUX_[HEAVY|LIGHT]_[X64|ARM64], so any repo with self-hosted capacity can opt in without forking the workflows. Unset variables fall back to the previous GitHub-hosted runners, so behavior is unchanged for forks and for upstream before configuration.*-macos.yamlsiblings, each gated byif: vars.GH_DISABLE_MACOS != 'true'(opt-out: unset = macOS runs).clp-core-build-macosgets the same gate.package-imageis split intopackage-image-amd64+package-image-arm64(arm64 main-only);package-image-multiarch-manifeststill gates the release tag on both arches.Status: WIP — opened as draft while the upstream run validates self-hosted routing.
Checklist
Validation performed
SC2046warnings on thechownrunner-images workaround step).clp-artifact-buildandclp-core-build-macosstill running — to be confirmed once complete.Summary by CodeRabbit
New Features
Improvements