ci: add GitHub Actions workflow mirroring Prow CI jobs#275
Open
sebrandon1 wants to merge 1 commit into
Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds a new GitHub Actions workflow (.github/workflows/pre-main.yml) that runs on pushes and PRs to main and executes golangci-lint, go module tidy checks, vendoring verification, unit tests, security scans (gosec, govulncheck), a linux/amd64 build, and markdown linting. ChangesPre-Main CI workflow
Sequence Diagram(s)sequenceDiagram
participant GitHub as GitHub (PR / Push)
participant Actions as GitHub Actions Runner
participant Repo as Repository Workspace
participant Tools as Linters & Scanners
GitHub->>Actions: trigger workflow
Actions->>Repo: checkout code
Actions->>Tools: setup Go environment
Actions->>Tools: run golangci-lint
Tools-->>Actions: lint results
Actions->>Tools: run go mod tidy (modtidy)
Tools-->>Actions: modtidy diff result
Actions->>Tools: run go mod vendor (verify-deps)
Tools-->>Actions: vendor diff result
Actions->>Tools: run tests (make test-all)
Tools-->>Actions: test results
Actions->>Tools: run gosec & govulncheck
Tools-->>Actions: security results
Actions->>Tools: run GOOS build
Tools-->>Actions: build artifact/status
Actions->>Tools: run markdownlint
Tools-->>Actions: markdown lint results
Actions->>GitHub: post status checks
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes 🚥 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 NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.github/workflows/pre-main.yml (2)
3-10: ⚡ Quick winConsider adding
concurrencyto cancel superseded runs (optional).Since this workflow runs on every PR update and every push to
main, adding a top-levelconcurrencyblock (cancel-in-progress: true) can reduce wasted compute when multiple commits land close together.🟡 Optional concurrency block
on: pull_request: branches: - main push: branches: - main + +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/pre-main.yml around lines 3 - 10, Add a top-level GitHub Actions concurrency block to the workflow to cancel superseded runs: under the top-level (same level as "on:") add a "concurrency" stanza with a descriptive group key (e.g. using "${{ github.workflow }}-${{ github.ref }}" or similar) and set "cancel-in-progress: true" so that when multiple commits/PR updates trigger the workflow only the latest run proceeds; update the workflow metadata near the existing "on:" block to include this "concurrency" key.
88-109: ⚖️ Poor tradeoffPin security scanner tool versions; verify CLI flags & whether
vendor/is scanned.The
securityjob installs both scanners with@latest:
gosec ...@latest(Line 102)govulncheck ...@latest(Line 107)This can break CI unpredictably when either tool changes behavior/defaults. Also, please verify two things:
- The gosec flag
-exclude-generatedand-exclude-dir=vendorare valid forgosec/v2as installed.- Whether
govulncheck ./...effectively excludesvendor/in module/package discovery (to avoid slow/duplicate scanning).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/pre-main.yml around lines 88 - 109, Pin the scanner installs in the security job by replacing the two `@latest` installs (the `go install github.com/securego/gosec/v2/cmd/gosec@latest` and `go install golang.org/x/vuln/cmd/govulncheck@latest` commands) with explicit, tested version tags (e.g. `@vX.Y.Z`), then validate the gosec CLI flags `-exclude-generated` and `-exclude-dir=vendor` used in the `gosec -exclude-generated -exclude-dir=vendor ./...` line against the pinned gosec v2 release and correct them if the flag names differ; finally confirm whether `govulncheck ./...` will include `vendor/` for the pinned govulncheck release and if it does, change the invocation to either pass an explicit package list that excludes vendor (via `go list` filtering) or use the govulncheck flag/option supported by that pinned release to exclude vendor, so CI behavior is stable and vendor is not scanned twice.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/pre-main.yml:
- Around line 110-124: The images job's checkout step uses a shallow fetch which
can omit tags/history needed by make build's git describe --tag versioning;
update the actions/checkout@v4 step in the images job to fetch full history/tags
(e.g., set fetch-depth: 0 or otherwise enable fetching tags) so git describe
--tag (used by make build/BUILD_VERSION) can find tags and produce stable build
metadata.
- Around line 53-72: The verify-deps job currently regenerates the vendor dir
and then runs git diff --exit-code which misses untracked files; modify the
check step in the verify-deps job (the step after "Regenerate vendor directory"
that currently runs git diff --exit-code) to also detect untracked files — e.g.,
run git status --porcelain and fail if any output exists (or use git ls-files
--others --exclude-standard and fail on output) so untracked vendor changes
cause the job to fail.
---
Nitpick comments:
In @.github/workflows/pre-main.yml:
- Around line 3-10: Add a top-level GitHub Actions concurrency block to the
workflow to cancel superseded runs: under the top-level (same level as "on:")
add a "concurrency" stanza with a descriptive group key (e.g. using "${{
github.workflow }}-${{ github.ref }}" or similar) and set "cancel-in-progress:
true" so that when multiple commits/PR updates trigger the workflow only the
latest run proceeds; update the workflow metadata near the existing "on:" block
to include this "concurrency" key.
- Around line 88-109: Pin the scanner installs in the security job by replacing
the two `@latest` installs (the `go install
github.com/securego/gosec/v2/cmd/gosec@latest` and `go install
golang.org/x/vuln/cmd/govulncheck@latest` commands) with explicit, tested
version tags (e.g. `@vX.Y.Z`), then validate the gosec CLI flags
`-exclude-generated` and `-exclude-dir=vendor` used in the `gosec
-exclude-generated -exclude-dir=vendor ./...` line against the pinned gosec v2
release and correct them if the flag names differ; finally confirm whether
`govulncheck ./...` will include `vendor/` for the pinned govulncheck release
and if it does, change the invocation to either pass an explicit package list
that excludes vendor (via `go list` filtering) or use the govulncheck
flag/option supported by that pinned release to exclude vendor, so CI behavior
is stable and vendor is not scanned twice.
🪄 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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e848a127-7523-4d4f-a419-3850f9fc5531
📒 Files selected for processing (1)
.github/workflows/pre-main.yml
Comment on lines
+53
to
+72
| verify-deps: | ||
| name: Verify vendor dependencies | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Set up Go | ||
| uses: actions/setup-go@v5 | ||
| with: | ||
| go-version-file: go.mod | ||
|
|
||
| - name: Regenerate vendor directory | ||
| run: | | ||
| go mod tidy | ||
| go mod vendor | ||
|
|
||
| - name: Check for changes | ||
| run: git diff --exit-code | ||
|
|
There was a problem hiding this comment.
Make verify-deps fail on untracked changes (not just diffs).
verify-deps regenerates vendor and then runs git diff --exit-code (Line 71). git diff won’t fail on newly-created untracked files. If regeneration introduces any untracked outputs (or if vendor/ isn’t fully tracked for some reason), the job can incorrectly pass.
✅ Suggested change (use `git status --porcelain` gate)
- name: Check for changes
- run: git diff --exit-code
+ run: |
+ git diff --exit-code
+ status="$(git status --porcelain)"
+ if [ -n "$status" ]; then
+ echo "$status"
+ exit 1
+ fi📝 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.
Suggested change
| verify-deps: | |
| name: Verify vendor dependencies | |
| runs-on: ubuntu-latest | |
| steps: | |
| - name: Checkout code | |
| uses: actions/checkout@v4 | |
| - name: Set up Go | |
| uses: actions/setup-go@v5 | |
| with: | |
| go-version-file: go.mod | |
| - name: Regenerate vendor directory | |
| run: | | |
| go mod tidy | |
| go mod vendor | |
| - name: Check for changes | |
| run: git diff --exit-code | |
| verify-deps: | |
| name: Verify vendor dependencies | |
| runs-on: ubuntu-latest | |
| steps: | |
| - name: Checkout code | |
| uses: actions/checkout@v4 | |
| - name: Set up Go | |
| uses: actions/setup-go@v5 | |
| with: | |
| go-version-file: go.mod | |
| - name: Regenerate vendor directory | |
| run: | | |
| go mod tidy | |
| go mod vendor | |
| - name: Check for changes | |
| run: | | |
| git diff --exit-code | |
| status="$(git status --porcelain)" | |
| if [ -n "$status" ]; then | |
| echo "$status" | |
| exit 1 | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/pre-main.yml around lines 53 - 72, The verify-deps job
currently regenerates the vendor dir and then runs git diff --exit-code which
misses untracked files; modify the check step in the verify-deps job (the step
after "Regenerate vendor directory" that currently runs git diff --exit-code) to
also detect untracked files — e.g., run git status --porcelain and fail if any
output exists (or use git ls-files --others --exclude-standard and fail on
output) so untracked vendor changes cause the job to fail.
Comment on lines
+110
to
+124
| images: | ||
| name: Build verification | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Set up Go | ||
| uses: actions/setup-go@v5 | ||
| with: | ||
| go-version-file: go.mod | ||
|
|
||
| - name: Build for linux/amd64 | ||
| run: GOOS=linux GOARCH=amd64 make build | ||
|
|
There was a problem hiding this comment.
Ensure images checkout fetches git tags/history for make build versioning.
Your images job checks out with default depth, but make build (per the project Makefile) appears to use git describe --tag to derive BUILD_VERSION when OS_GIT_VERSION isn’t set. With a shallow checkout, tags may be missing, which can make build metadata unstable or fail the version extraction.
✅ Suggested change (fetch full history/tags in `images`)
- name: Checkout code
uses: actions/checkout@v4
+ with:
+ fetch-depth: 0📝 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.
Suggested change
| images: | |
| name: Build verification | |
| runs-on: ubuntu-latest | |
| steps: | |
| - name: Checkout code | |
| uses: actions/checkout@v4 | |
| - name: Set up Go | |
| uses: actions/setup-go@v5 | |
| with: | |
| go-version-file: go.mod | |
| - name: Build for linux/amd64 | |
| run: GOOS=linux GOARCH=amd64 make build | |
| images: | |
| name: Build verification | |
| runs-on: ubuntu-latest | |
| steps: | |
| - name: Checkout code | |
| uses: actions/checkout@v4 | |
| with: | |
| fetch-depth: 0 | |
| - name: Set up Go | |
| uses: actions/setup-go@v5 | |
| with: | |
| go-version-file: go.mod | |
| - name: Build for linux/amd64 | |
| run: GOOS=linux GOARCH=amd64 make build |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/pre-main.yml around lines 110 - 124, The images job's
checkout step uses a shallow fetch which can omit tags/history needed by make
build's git describe --tag versioning; update the actions/checkout@v4 step in
the images job to fetch full history/tags (e.g., set fetch-depth: 0 or otherwise
enable fetching tags) so git describe --tag (used by make build/BUILD_VERSION)
can find tags and produce stable build metadata.
Add a pre-main workflow with 7 independent jobs that mirror the existing Prow CI checks: golint, modtidy, verify-deps, unit tests, security scanning, build verification, and markdownlint. This provides redundant CI coverage via GitHub Actions while the OpenShift CI cluster experiences capacity issues.
sebrandon1
force-pushed
the
ci/github-actions-workflow
branch
from
6d4a721 to
b2c2049
Compare
|
@sebrandon1: 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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a GitHub Actions workflow (
.github/workflows/pre-main.yml) that mirrors all 7 existing Prow CI jobs. This provides redundant CI coverage while the OpenShift CI cluster experiences capacity/scheduling issues (e.g., pod scheduling timeouts onci/prow/verify-deps).Supersedes #246 — that PR was closed because it bundled too many changes together (golangci-lint config migration to v2 format, source code fixes, golden file updates). This PR takes a minimal approach: only the workflow file is added, no other files are modified.
Jobs
golintci/prow/golintmodtidyci/prow/modtidygo mod tidyproduces no changesverify-depsci/prow/verify-depsunitci/prow/unitmake test-all(pkg + addon tools)securityci/prow/securityimagesci/prow/imagesmarkdownlintci/prow/markdownlintKey decisions vs #246
.golangci.ymlchanges — the existing v1 config is used as-isTest plan
Summary by CodeRabbit