-
Notifications
You must be signed in to change notification settings - Fork 24
fix(ci): make docs-only PRs mergeable (gate required e2e/smoke per-job) #334
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
base: main
Are you sure you want to change the base?
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 | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -6,14 +6,13 @@ on: | |||||||||||||||||||||
| # a branch PRs never target (this said `master`, which does not exist) means | ||||||||||||||||||||||
| # the workflow never runs at all. | ||||||||||||||||||||||
| # | ||||||||||||||||||||||
| # paths-ignore: a kind+cert-manager+Kamaji provisioning run costs ~30-45 | ||||||||||||||||||||||
| # minutes; skip it for PRs that touch nothing the suite exercises | ||||||||||||||||||||||
| # (docs-only changes). Everything else — Go code, manifests, the harness, | ||||||||||||||||||||||
| # the workflows themselves — still gates. | ||||||||||||||||||||||
| # No paths filter on the trigger either: `kamaji-datastore` is a *required* | ||||||||||||||||||||||
| # status check, and a workflow skipped at the trigger never reports its | ||||||||||||||||||||||
| # check, leaving the required context stuck in "Expected" — which blocks the | ||||||||||||||||||||||
| # PR forever (e.g. docs-only PRs). Path filtering instead lives in the | ||||||||||||||||||||||
| # `changes` job below; a job skipped via `if:` still reports its check as | ||||||||||||||||||||||
| # "skipped", which branch protection treats as passing. | ||||||||||||||||||||||
| pull_request: | ||||||||||||||||||||||
| paths-ignore: | ||||||||||||||||||||||
| - '**.md' | ||||||||||||||||||||||
| - 'docs/**' | ||||||||||||||||||||||
| push: | ||||||||||||||||||||||
| tags: [ "v*" ] | ||||||||||||||||||||||
| workflow_dispatch: | ||||||||||||||||||||||
|
|
@@ -23,7 +22,34 @@ concurrency: | |||||||||||||||||||||
| cancel-in-progress: true | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| jobs: | ||||||||||||||||||||||
| changes: | ||||||||||||||||||||||
| # Cheap (~seconds, no checkout — dorny uses the PR API) gate that decides | ||||||||||||||||||||||
| # whether the expensive job below needs to run. PR-only: tag pushes and | ||||||||||||||||||||||
| # manual dispatch always run the suite unconditionally (see the `if` on | ||||||||||||||||||||||
| # kamaji-datastore). | ||||||||||||||||||||||
| if: github.event_name == 'pull_request' | ||||||||||||||||||||||
| runs-on: ubuntu-latest | ||||||||||||||||||||||
| outputs: | ||||||||||||||||||||||
| code: ${{ steps.filter.outputs.code }} | ||||||||||||||||||||||
| steps: | ||||||||||||||||||||||
| - uses: dorny/paths-filter@v3 | ||||||||||||||||||||||
| id: filter | ||||||||||||||||||||||
| with: | ||||||||||||||||||||||
| # `code` is true when the PR touches anything the e2e suite | ||||||||||||||||||||||
| # exercises. Only-negated patterns get an implicit `**`, so this | ||||||||||||||||||||||
| # reads as "all files except Markdown and docs/**" — i.e. docs-only | ||||||||||||||||||||||
| # PRs leave `code` false and the ~30-45 min run below is skipped. | ||||||||||||||||||||||
| filters: | | ||||||||||||||||||||||
| code: | ||||||||||||||||||||||
| - '!**/*.md' | ||||||||||||||||||||||
| - '!docs/**' | ||||||||||||||||||||||
|
Comment on lines
+25
to
+45
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 | 🟠 Major | ⚡ Quick win Restrict permissions for the The 🔐 Proposed permissions block changes:
if: github.event_name == 'pull_request'
runs-on: ubuntu-latest
+ permissions:
+ pull-requests: read
outputs:
code: ${{ steps.filter.outputs.code }}🧰 Tools🪛 zizmor (1.25.2)[warning] 25-45: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block (excessive-permissions) [error] 35-35: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy) (unpinned-uses) 🤖 Prompt for AI AgentsSource: Linters/SAST tools |
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| kamaji-datastore: | ||||||||||||||||||||||
| needs: changes | ||||||||||||||||||||||
| # Always run for tag pushes / manual dispatch; for PRs, run only when | ||||||||||||||||||||||
| # non-docs files changed. When skipped on a docs-only PR the check still | ||||||||||||||||||||||
| # reports (as "skipped" = passing), so the PR is not blocked. | ||||||||||||||||||||||
| if: ${{ github.event_name != 'pull_request' || needs.changes.outputs.code == 'true' }} | ||||||||||||||||||||||
|
Comment on lines
+48
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. Shared root cause: Both
The shared fix is to wrap both jobs' conditions in e2e.yml: kamaji-datastore:
needs: changes
- if: ${{ github.event_name != 'pull_request' || needs.changes.outputs.code == 'true' }}
+ if: ${{ always() && (github.event_name != 'pull_request' || needs.changes.outputs.code == 'true') }}release-smoke.yml: smoke:
needs: changes
- if: ${{ github.event_name != 'pull_request' || needs.changes.outputs.release == 'true' }}
+ if: ${{ always() && (github.event_name != 'pull_request' || needs.changes.outputs.release == 'true') }}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||
| runs-on: ubuntu-latest | ||||||||||||||||||||||
| timeout-minutes: 45 | ||||||||||||||||||||||
| steps: | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -10,25 +10,52 @@ name: Release install smoke | |||||
| # introduces them, not on the first real tag. The image is loaded into kind, | ||||||
| # never pushed — no registry credentials. | ||||||
| on: | ||||||
| # No paths filter on the trigger: `smoke (helm)` and `smoke (manifest)` are | ||||||
| # *required* status checks, and a workflow skipped at the trigger never | ||||||
| # reports them, leaving the required contexts stuck in "Expected" and | ||||||
| # blocking the PR forever (e.g. docs-only PRs). Path filtering lives in the | ||||||
| # `changes` job below instead; the matrix job is skipped via `if:` when | ||||||
| # nothing release-relevant changed, and a skipped check counts as passing. | ||||||
| pull_request: | ||||||
| paths: | ||||||
| - '.github/workflows/release-smoke.yml' | ||||||
| - '.github/workflows/docker-publish.yml' | ||||||
| - '.github/workflows/release-assets.yml' | ||||||
| - '.github/workflows/helm-publish.yml' | ||||||
| - 'hack/release-smoke.sh' | ||||||
| - 'charts/**' | ||||||
| - 'Makefile' | ||||||
| - 'Dockerfile' | ||||||
| - 'api/**' | ||||||
| workflow_dispatch: | ||||||
|
|
||||||
| concurrency: | ||||||
| group: release-smoke-${{ github.ref }} | ||||||
| cancel-in-progress: true | ||||||
|
|
||||||
| jobs: | ||||||
| changes: | ||||||
| # Cheap (~seconds, no checkout) gate. PR-only: manual dispatch always runs | ||||||
| # the smoke matrix unconditionally (see the `if` on smoke). | ||||||
| if: github.event_name == 'pull_request' | ||||||
| runs-on: ubuntu-latest | ||||||
| outputs: | ||||||
| release: ${{ steps.filter.outputs.release }} | ||||||
| steps: | ||||||
| - uses: dorny/paths-filter@v3 | ||||||
|
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. Pin The action reference 🔒 Proposed fix- - uses: dorny/paths-filter@v3
+ - uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36 # v3.0.2
id: filterVerify the SHA corresponds to the intended v3.x release in the dorny/paths-filter repository. 📝 Committable suggestion
Suggested change
🧰 Tools🪛 zizmor (1.25.2)[error] 35-35: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy) (unpinned-uses) 🤖 Prompt for AI AgentsSource: Linters/SAST tools |
||||||
| id: filter | ||||||
| with: | ||||||
| # True when the PR touches the tag-release machinery or anything it | ||||||
| # ships. Matches the paths this workflow used to filter on at the | ||||||
| # trigger; PRs that touch none of these skip the two kind smokes. | ||||||
| filters: | | ||||||
| release: | ||||||
| - '.github/workflows/release-smoke.yml' | ||||||
| - '.github/workflows/docker-publish.yml' | ||||||
| - '.github/workflows/release-assets.yml' | ||||||
| - '.github/workflows/helm-publish.yml' | ||||||
| - 'hack/release-smoke.sh' | ||||||
| - 'charts/**' | ||||||
| - 'Makefile' | ||||||
| - 'Dockerfile' | ||||||
| - 'api/**' | ||||||
|
Comment on lines
+27
to
+51
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 | 🟠 Major | ⚡ Quick win Restrict permissions for the The 🔐 Proposed permissions block changes:
if: github.event_name == 'pull_request'
runs-on: ubuntu-latest
+ permissions:
+ pull-requests: read
outputs:
release: ${{ steps.filter.outputs.release }}🧰 Tools🪛 zizmor (1.25.2)[warning] 27-51: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block (excessive-permissions) [error] 35-35: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy) (unpinned-uses) 🤖 Prompt for AI AgentsSource: Linters/SAST tools |
||||||
|
|
||||||
| smoke: | ||||||
| needs: changes | ||||||
| # Always run on manual dispatch; for PRs, run only when release-relevant | ||||||
| # files changed. When skipped, each matrix leg's required check still | ||||||
| # reports as "skipped" (= passing), so the PR is not blocked. | ||||||
| if: ${{ github.event_name != 'pull_request' || needs.changes.outputs.release == 'true' }} | ||||||
| runs-on: ubuntu-latest | ||||||
| timeout-minutes: 30 | ||||||
| permissions: | ||||||
|
|
||||||
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.
Pin
dorny/paths-filterto a commit SHA.The action reference
dorny/paths-filter@v3is unpinned. Per static analysis policy, action references should be pinned to a commit SHA to prevent supply-chain attacks.🔒 Proposed fix
Verify the SHA corresponds to the intended v3.x release in the dorny/paths-filter repository.
📝 Committable suggestion
🧰 Tools
🪛 zizmor (1.25.2)
[error] 35-35: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Source: Linters/SAST tools