Skip to content

[misc] Add CI check for proto version string changes#5854

Merged
lixmal merged 2 commits intomainfrom
proto-version-ci
Apr 14, 2026
Merged

[misc] Add CI check for proto version string changes#5854
lixmal merged 2 commits intomainfrom
proto-version-ci

Conversation

@lixmal
Copy link
Copy Markdown
Collaborator

@lixmal lixmal commented Apr 11, 2026

Describe your changes

Add a PR-only CI check that fails when a *.pb.go file changes the protoc or protoc-gen-go version comment. Catches accidental version drift from regenerating with the wrong tool versions. Non-required check, so you can merge anyway when intentionally upgrading.

Issue ticket number and link

Stack

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

Documentation

Select exactly one:

  • I added/updated documentation for this change
  • Documentation is not needed for this change (explain why)

Docs PR URL (required if "docs added" is checked)

Paste the PR link from https://github.com/netbirdio/docs here:

https://github.com/netbirdio/docs/pull/__

Summary by CodeRabbit

  • Chores
    • Added an automated validation workflow for pull requests that modify generated protocol buffer files.
    • The workflow scans PR diffs for protoc/protoc-gen-go version comment lines, reports any per-file matches, and blocks the PR when such changes are detected to surface and detail affected files before merge.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 11, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4e31a3b2-82ec-477f-8a70-195d068a6d41

📥 Commits

Reviewing files that changed from the base of the PR and between f83bed1 and 236788f.

📒 Files selected for processing (1)
  • .github/workflows/proto-version-check.yml
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/proto-version-check.yml

📝 Walkthrough

Walkthrough

Adds a new GitHub Actions workflow that runs on pull requests touching **/*.pb.go, lists PR files via the GitHub API, inspects each .pb.go patch for added/removed protoc or protoc-gen-go version comment lines, and fails the run when any such lines are detected.

Changes

Cohort / File(s) Summary
Proto Version Check Workflow
.github/workflows/proto-version-check.yml
New workflow triggered on pull_request for **/*.pb.go. Lists PR files (paginated), filters .pb.go entries, ensures patches exist, scans added/removed patch lines for // protoc... v<digits> or // protoc-gen-go... v<digits> patterns, and calls core.setFailed with per-file details if matches are found.

Sequence Diagram(s)

sequenceDiagram
    participant PR as Pull Request
    participant Actions as GitHub Actions Runner
    participant API as GitHub API
    participant Script as actions/github-script
    PR->>Actions: pull_request event for `**/*.pb.go`
    Actions->>API: list pull request files (paginated, up to 100)
    API-->>Actions: file entries (with `patch` fields)
    Actions->>Script: filter `.pb.go`, validate patches exist
    Script->>Script: scan patch lines for protoc/protoc-gen-go version comments
    alt matches found
        Script-->>Actions: matched file list + lines
        Actions->>Actions: core.setFailed(message with per-file details)
    else no matches
        Script-->>Actions: "no proto version string changes detected"
    end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 I hopped through patches, sniffing each line,
Watching for protoc versions out of line,
A twitch of my nose, a flagged little clue —
I stop the build till the notes are true.
Patch-guard rabbit, vigilant and spry.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change—adding a CI check for proto version string changes in protobuf-generated files.
Description check ✅ Passed The description covers the changes, purpose, and documentation status. All required template sections are present and appropriately filled, with the feature enhancement and 'documentation not needed' options selected.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch proto-version-ci

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@lixmal lixmal force-pushed the proto-version-ci branch 2 times, most recently from 9ca1d07 to f714c37 Compare April 11, 2026 16:57
@lixmal lixmal force-pushed the proto-version-ci branch from f714c37 to f83bed1 Compare April 11, 2026 16:57
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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/proto-version-check.yml:
- Around line 23-30: The current filter silently ignores .pb.go files that lack
a patch (pbFiles = files.filter(... && f.patch)), letting changed protobufs
bypass validation; instead, first collect all protobuf candidates (e.g.,
pbCandidates = files.filter(f => f.filename.endsWith('.pb.go'))), detect any
with missing patch data (missing = pbCandidates.filter(f => !f.patch)), and
fail-fast by throwing or returning an error/violation that lists those filenames
before proceeding to use versionPattern and the loop (for (const file of ...))
so the check cannot be skipped due to absent patch content.
- Around line 16-21: The current call to github.rest.pulls.listFiles (assigned
to files) only fetches up to per_page:100 and later filters on f.patch, which
misses files in multi-page PRs and silently ignores files where GitHub omitted
patch data; update the logic to paginate through listFiles results (loop over
pages via page parameter until no more files) and accumulate all file entries,
and change the post-fetch check that inspects f.patch for .pb.go files so that
if a .pb.go file has no f.patch you either mark the file as needing verification
or fail the check (e.g., log a warning and return failure) instead of skipping
it — adjust references around github.rest.pulls.listFiles, the files variable,
and the f.patch filter accordingly.
🪄 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: CHILL

Plan: Pro

Run ID: a5f7fb8b-b9a5-4be6-80cd-ddbf1d51f448

📥 Commits

Reviewing files that changed from the base of the PR and between 5259e5d and acfd71e.

📒 Files selected for processing (1)
  • .github/workflows/proto-version-check.yml

Comment thread .github/workflows/proto-version-check.yml Outdated
Comment thread .github/workflows/proto-version-check.yml Outdated
@sonarqubecloud
Copy link
Copy Markdown

@lixmal lixmal merged commit d7ad908 into main Apr 14, 2026
42 checks passed
@lixmal lixmal deleted the proto-version-ci branch April 14, 2026 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants