-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat: Sequencing workflows so that faster workflows run before slower ones #4670
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
Changes from all commits
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 |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| name: Base Tests | ||
|
|
||
| on: | ||
| push: | ||
| workflow_call: | ||
|
|
||
| jobs: | ||
| test: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| name: Build Without Go Proxy | ||
|
|
||
| on: | ||
| push: | ||
| workflow_call: | ||
|
|
||
| jobs: | ||
| build-no-proxy: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| name: Build | ||
|
|
||
| on: | ||
| push: | ||
| workflow_call: | ||
|
|
||
|
Comment on lines
3
to
5
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. 🛠️ Refactor suggestion Reusable conversion approved; expose inputs for matrix control and set permissions. Allow the orchestrator to pass a subset of OS/arch to shorten slow builds when not needed. on:
- workflow_call:
+ workflow_call:
+ inputs:
+ matrix_include:
+ description: 'YAML/JSON list for matrix.include (os/arch pairs)'
+ required: false
+ type: string
+ secrets: {}Then, adapt the matrix to read from input when provided (outside the selected range): strategy:
matrix:
include: ${{ inputs.matrix_include && fromJSON(inputs.matrix_include) || matrix.include }}Also add: permissions:
contents: read🤖 Prompt for AI Agents |
||
| jobs: | ||
| build: | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,62 @@ | ||||||||||||||||||||||||||||||||||
| name: CI | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| on: | ||||||||||||||||||||||||||||||||||
| push: | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| jobs: | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+3
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. 🛠️ Refactor suggestion Add pull_request + workflow_dispatch triggers, default least-privilege permissions, and cancel-in-progress concurrency This centralizes CI while ensuring PRs run, manual runs are possible, and redundant in-flight runs are canceled on new pushes to the same ref. Also sets minimal default GITHUB_TOKEN permissions. Apply: name: CI
-on:
- push:
+on:
+ push:
+ pull_request:
+ workflow_dispatch:
+
+permissions:
+ contents: read
+
+concurrency:
+ group: ci-${{ github.workflow }}-${{ github.ref }}
+ cancel-in-progress: true🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
| lint: | ||||||||||||||||||||||||||||||||||
| uses: ./.github/workflows/lint.yml | ||||||||||||||||||||||||||||||||||
| secrets: inherit | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| precommit: | ||||||||||||||||||||||||||||||||||
| uses: ./.github/workflows/precommit.yml | ||||||||||||||||||||||||||||||||||
| secrets: inherit | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| codespell: | ||||||||||||||||||||||||||||||||||
| uses: ./.github/workflows/codespell.yml | ||||||||||||||||||||||||||||||||||
| secrets: inherit | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| markdownlint: | ||||||||||||||||||||||||||||||||||
| uses: ./.github/workflows/markdownlint.yml | ||||||||||||||||||||||||||||||||||
| secrets: inherit | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| license_check: | ||||||||||||||||||||||||||||||||||
| uses: ./.github/workflows/license-check.yml | ||||||||||||||||||||||||||||||||||
| secrets: inherit | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
Comment on lines
+7
to
+26
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. 💡 Verification agent 🧩 Analysis chainVerify all referenced reusable workflows are switched to workflow_call and inputs/secrets match Given the conversion, ensure each callee has on: workflow_call and that any required inputs match (none provided here). This avoids silent dispatch failures. Also applies to: 27-44, 45-60 🏁 Script executed: #!/bin/bash
set -euo pipefail
files=(
".github/workflows/lint.yml"
".github/workflows/precommit.yml"
".github/workflows/codespell.yml"
".github/workflows/markdownlint.yml"
".github/workflows/license-check.yml"
".github/workflows/base-test.yml"
".github/workflows/build.yml"
".github/workflows/build-no-proxy.yml"
".github/workflows/integration-test.yml"
".github/workflows/oidc-integration-test.yml"
)
echo "Checking reusable workflow configuration..."
for f in "${files[@]}"; do
if [[ ! -f "$f" ]]; then
echo "MISSING: $f"
continue
fi
echo "---- $f"
rg -n "^\s*on:\s*workflow_call" "$f" || echo "WARNING: $f is not configured with on: workflow_call"
echo "Declared inputs (if any):"
rg -n "^\s*inputs:" -n "$f" -A 50 | sed '/^\s*outputs:/,$d' || true
echo
done
echo "If any workflow declares required inputs, add a with: { ... } block to the corresponding job in ci.yml."Length of output: 6429 Add All of the workflows invoked from on:
workflow_call:
# define inputs here if the workflow requires any• .github/workflows/lint.yml Since none of these declare required inputs, no |
||||||||||||||||||||||||||||||||||
| base_tests: | ||||||||||||||||||||||||||||||||||
| needs: [lint, precommit, codespell, markdownlint, license_check] | ||||||||||||||||||||||||||||||||||
| uses: ./.github/workflows/base-test.yml | ||||||||||||||||||||||||||||||||||
| permissions: | ||||||||||||||||||||||||||||||||||
| contents: read | ||||||||||||||||||||||||||||||||||
| checks: write | ||||||||||||||||||||||||||||||||||
| secrets: inherit | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| build: | ||||||||||||||||||||||||||||||||||
| needs: [lint, precommit, codespell, markdownlint, license_check] | ||||||||||||||||||||||||||||||||||
| uses: ./.github/workflows/build.yml | ||||||||||||||||||||||||||||||||||
| secrets: inherit | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| build_no_proxy: | ||||||||||||||||||||||||||||||||||
| needs: [lint, precommit, codespell, markdownlint, license_check] | ||||||||||||||||||||||||||||||||||
| uses: ./.github/workflows/build-no-proxy.yml | ||||||||||||||||||||||||||||||||||
| secrets: inherit | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| integration_tests: | ||||||||||||||||||||||||||||||||||
| needs: [base_tests, build, build_no_proxy] | ||||||||||||||||||||||||||||||||||
| uses: ./.github/workflows/integration-test.yml | ||||||||||||||||||||||||||||||||||
| permissions: | ||||||||||||||||||||||||||||||||||
| contents: read | ||||||||||||||||||||||||||||||||||
| checks: write | ||||||||||||||||||||||||||||||||||
| secrets: inherit | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
Comment on lines
+45
to
+52
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. 💡 Verification agent ❓ Verification inconclusiveGuard integration tests on forked PRs and set a timeout Secrets are not available to workflows on pull_request from forks. Guard to skip instead of fail, and add a sensible timeout to avoid hung runs. integration_tests:
needs: [base_tests, build, build_no_proxy]
+ if: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.fork == false }}
uses: ./.github/workflows/integration-test.yml
permissions:
contents: read
checks: write
+ timeout-minutes: 90
secrets: inheritIf your integration tests are expected to run on forked PRs (without secrets), reply and I can propose a paths-filter + conditional to only run a non-secret subset. Otherwise, verify the skip behavior matches expectations. Guard integration tests on forked PRs and add a timeout Integration tests rely on inherited secrets, which are not available for workflows triggered by • File: integration_tests:
needs: [base_tests, build, build_no_proxy]
+ if: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.fork == false }}
uses: ./.github/workflows/integration-test.yml
permissions:
contents: read
checks: write
+ timeout-minutes: 90
secrets: inheritIf you still need to run a subset of integration tests on forked PRs (without secrets), let me know and I can suggest a conditional paths filter. Otherwise, please apply this guard to avoid broken or hung CI runs. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
| oidc_integration_tests: | ||||||||||||||||||||||||||||||||||
| needs: [base_tests, build, build_no_proxy] | ||||||||||||||||||||||||||||||||||
| uses: ./.github/workflows/oidc-integration-test.yml | ||||||||||||||||||||||||||||||||||
| permissions: | ||||||||||||||||||||||||||||||||||
| id-token: write | ||||||||||||||||||||||||||||||||||
| contents: read | ||||||||||||||||||||||||||||||||||
| checks: write | ||||||||||||||||||||||||||||||||||
| secrets: inherit | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+53
to
+60
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. 💡 Verification agent 🧩 Analysis chainGuard OIDC integration tests on forked PRs and set a timeout Same motivation as the non-OIDC integration suite. Keeps forks green without exposing secrets and avoids hangs. oidc_integration_tests:
needs: [base_tests, build, build_no_proxy]
+ if: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.fork == false }}
uses: ./.github/workflows/oidc-integration-test.yml
permissions:
id-token: write
contents: read
checks: write
+ timeout-minutes: 90
secrets: inherit🏁 Script executed: #!/bin/bash
set -e
echo "Searching in ci.yml for non-OIDC integration_tests job"
rg -n "integration_tests:" .github/workflows/ci.yml || true
echo
echo "Searching for timeout-minutes usage across all workflow files"
rg -n "timeout-minutes" .github/workflows || trueLength of output: 422 🏁 Script executed: #!/bin/bash
set -e
echo "Showing lines around the non-OIDC integration_tests job for context"
rg -n -C5 "integration_tests:" .github/workflows/ci.ymlLength of output: 812 Guard OIDC integration tests on forked PRs and set a timeout Same motivation as the non-OIDC integration suite: keeps forks green without exposing secrets and prevents hung jobs. • File: Suggested diff: oidc_integration_tests:
needs: [base_tests, build, build_no_proxy]
+ if: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.fork == false }}
uses: ./.github/workflows/oidc-integration-test.yml
permissions:
id-token: write
contents: read
checks: write
+ timeout-minutes: 90
secrets: inheritThis will skip OIDC tests on forked PRs and fail any run exceeding 90 minutes. 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| name: Codespell | ||
|
|
||
| on: | ||
| push: | ||
| workflow_call: | ||
|
|
||
| jobs: | ||
| codespell: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| name: Integration Tests | ||
|
|
||
| on: | ||
| push: | ||
| workflow_call: | ||
|
|
||
|
Comment on lines
3
to
5
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. 🛠️ Refactor suggestion Reusable conversion looks good; explicitly declare required secrets and inputs. This workflow consumes many secrets via the setup and test steps. Make the requirements explicit to prevent accidental silent failures at call time and to document contract. on:
- workflow_call:
+ workflow_call:
+ inputs:
+ include_matrix:
+ description: 'Subset of integrations to run (JSON/YAML list of matrix entries)'
+ required: false
+ type: string
+ secrets:
+ GHA_DEPLOY_KEY:
+ required: false
+ AWS_ACCESS_KEY_ID:
+ required: false
+ AWS_SECRET_ACCESS_KEY:
+ required: false
+ AWS_TEST_OIDC_ROLE_ARN:
+ required: false
+ GCLOUD_SERVICE_KEY:
+ required: false
+ GOOGLE_CLOUD_PROJECT:
+ required: false
+ GOOGLE_COMPUTE_ZONE:
+ required: false
+ GOOGLE_IDENTITY_EMAIL:
+ required: false
+ GOOGLE_PROJECT_ID:
+ required: false
+ GCLOUD_SERVICE_KEY_IMPERSONATOR:
+ required: false
+ AWS_TEST_S3_ASSUME_ROLE:
+ required: falseAdd least-privilege permissions (outside selected range). If any test uses OIDC, add permissions:
contents: read
# id-token: write # enable only if OIDC tests are run in this workflow🤖 Prompt for AI Agents |
||
| jobs: | ||
| test: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| name: License Check | ||
|
|
||
| on: | ||
| push: | ||
| workflow_call: | ||
|
|
||
| jobs: | ||
| license-check: | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,7 +1,7 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| name: Lint | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| on: | ||||||||||||||||||||||||||||||||||||||||||||||||||
| push: | ||||||||||||||||||||||||||||||||||||||||||||||||||
| workflow_call: | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
3
to
5
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. 💡 Verification agent 🧩 Analysis chainGood conversion to a reusable workflow; consider formalizing inputs/secrets and least-privilege permissions. Switching to Apply this diff to expand on:
- workflow_call:
+ workflow_call:
+ inputs:
+ go_version:
+ description: 'Go toolchain version (mise will resolve)'
+ required: false
+ type: string
+ golangci_lint_cache_path:
+ description: 'Path to golangci-lint cache on runner'
+ required: false
+ type: string
+ secrets: {}Then set least-privilege permissions at the workflow level (outside the selected range; add near the top-level keys): permissions:
contents: readIf you want, I can propagate the same pattern to all reusable workflows in this PR and wire inputs from the new orchestrator. 🏁 Script executed: #!/bin/bash
# Inspect the presence and context of workflow_call, inputs, secrets, and permissions in lint.yml
echo "---- snippet around workflow_call ----"
rg -n "workflow_call" -A5 -B5 .github/workflows/lint.yml || true
echo -e "\n---- top of lint.yml (first 20 lines) ----"
sed -n '1,20p' .github/workflows/lint.ymlLength of output: 764 Formalize reusable workflow inputs/secrets and enforce least-privilege permissions This workflow currently exposes • File: .github/workflows/lint.yml Suggested changes: name: Lint
+
+permissions:
+ contents: read
on:
workflow_call:
+ inputs:
+ go_version:
+ description: 'Go toolchain version (mise will resolve)'
+ required: false
+ type: string
+ golangci_lint_cache_path:
+ description: 'Path to golangci-lint cache on runner'
+ required: false
+ type: string
+ secrets: {}
jobs:
lint:
runs-on: ubuntu-latest📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||
| jobs: | ||||||||||||||||||||||||||||||||||||||||||||||||||
| lint: | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| name: Markdown Lint | ||
|
|
||
| on: | ||
| push: | ||
| workflow_call: | ||
|
|
||
|
Comment on lines
3
to
5
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. 🛠️ Refactor suggestion Reusable workflow conversion is fine; add inputs and restrict permissions. Let callers customize globs and keep permissions minimal. on:
- workflow_call:
+ workflow_call:
+ inputs:
+ globs:
+ description: 'Markdown file globs to lint (newline separated)'
+ required: false
+ type: string
+ secrets: {}Add (outside selected range): permissions:
contents: read🤖 Prompt for AI Agents |
||
| jobs: | ||
| markdownlint: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,7 +5,7 @@ | |
| name: OIDC Integration Tests | ||
|
|
||
| on: | ||
| push: | ||
| workflow_call: | ||
|
|
||
| permissions: | ||
| id-token: write | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| name: Pre-commit | ||
|
|
||
| on: | ||
| push: | ||
| workflow_call: | ||
|
|
||
| jobs: | ||
| precommit: | ||
|
|
||
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.
🛠️ Refactor suggestion
Reusable workflow conversion looks good; declare inputs and permissions.
Define inputs (e.g., OS matrix override, test timeout) and least-privilege permissions. This formalizes the contract and improves security.
Then add (outside selected range):
🤖 Prompt for AI Agents