fix(ci): make forge workflows run outside this repo and on GitLab - #56
Conversation
Both templates ran go run ./cmd/git-byline, which works only in this repo. In any other project the reconstruct job failed on this step. Now the workflow downloads the release pinned on the GIT_BYLINE_VERSION line, checks the archive against checksums.txt and the binary version, same like action.yml and install.sh, and runs that binary. Release Please bumps the pin in both templates, and validate fails when the pin and the manifest drift. A workflow that differs from the template only in the pinned tag still counts as managed, so an upgrade does not make install-hooks warn or uninstall skip the file. The GitLab job runs in buildpack-deps:trixie-scm now, it does not need Go anymore. GIT_BYLINE_VERSION and GIT_BYLINE_RELEASES_URL can be set as CI/CD variables, for example for an internal mirror. The GitHub workflow triggers on the default branch instead of hardcoded main. Fixes #50
The job put GITLAB_TOKEN into http.extraHeader as PRIVATE-TOKEN. This header is for REST calls, Git over HTTPS takes HTTP basic auth. With credential.helper empty Git had no other way to log in, so the first authenticated fetch or the notes push got 401. Now the header is basic auth with user oauth2 and the token as password, same idea like #43 for GitHub. I checked in the runner image that Git sends it on the first request and that it decodes to oauth2:<token>. A run against live GitLab is still to verify in the pilot. A missing GITLAB_TOKEN also fails with a message now, not a bare test. Fixes #51
After a squash merge the MR commits, and their notes, stay only on
refs/merge-requests/<iid>/head. The job took the source from commit
parents, so a squash with fast-forward had an empty source range and a
squash with merge commit pointed at the squash commit, which has no
notes. In both cases the lines stayed untracked.
Now the job reads the <project path>!<iid> reference from the merged
commit message, fetches the MR head and uses it as the source, but only
when its diff has the same stable patch ID as the diff the target
commit brings. Without the reference it keeps the parents like before.
When the diff differs it keeps them too and prints a warning.
The default merge commit template has the reference. The default
squash template is only the title, so with the fast-forward merge
method the project has to add %{reference} there, README says it now.
I checked seven merge setups in the runner image with sh and bash.
Live GitLab is still to verify in the pilot.
Fixes #52
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Warning Review limit reachedNext included review available in 20 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository: comarch/git-byline/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (7)
Summary by CodeRabbit
WalkthroughGitHub and GitLab forge workflows now install checksum-verified, version-pinned release binaries. The GitLab workflow also checks referenced merge-request heads against merged diffs. Workflow installation and validation now account for release pins. ChangesForge workflow updates
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Change: Bug fix · Severity of issue fixed: Medium Merge Risk: 🔵 Low · up to Failed GitLab jobs can leave downloaded files in their build directory. Add failure-path cleanup; the remaining risk is bounded and does not otherwise block merging. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 48.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 25 functions across 6 files. (9 skipped: 9 unsupported.) ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: comarch/git-byline/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Advanced
Run ID: cb5b7b60-c3ab-4967-aadf-0b7fa828ee10
📒 Files selected for processing (15)
.github/workflows/git-byline.yml.gitlab/ci/git-byline.ymlREADME.mddocs/ARCHITECTURE.mddocs/SECURITY_MODEL.mddocs/SUPPLY_CHAIN.mdinternal/ci/ci.gointernal/ci/ci_test.gointernal/ci/forge_detect_test.gointernal/ci/templates/github.ymlinternal/ci/templates/gitlab.ymlrelease-please-config.jsontools/validate/checks.gotools/validate/checks_test.gotools/validate/scan.go
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.
checkCITemplateVersions had a provider lookup that can not fail for the two fixed templates, so its error return was never hit. Read the two template files directly. Also test the checkScans branch when the pin does not match the release manifest.
The job runs under set -eu, so a failed checksum, version check, tar, or ci run exits before the rm and the archive stays in CI_BUILDS_DIR. An EXIT trap removes it on every path.
|
1 participant
Summary
I ran the forge job end to end, the way a consumer project uses it, and it failed in three places, two of them GitLab only. Each fix is its own commit, so it's easier to review one by one.
#50. Both templates ran
go run ./cmd/git-byline, which works only inside this repo. In any other project the job failed on this step. Now the workflow downloads the release pinned on theGIT_BYLINE_VERSIONline, checks the archive againstchecksums.txtand the binary version, same likeaction.ymlandinstall.sh, and runs that binary. Release Please bumps the pin in both templates, and validate fails when the pin and the manifest drift. The GitLab job runs inbuildpack-deps:trixie-scmnow, it does not need Go.#51. The GitLab job put
GITLAB_TOKENintohttp.extraHeaderasPRIVATE-TOKEN. This header is for REST calls, Git over HTTPS takes basic auth, so the first authenticated fetch or the notes push got 401. Now it's basic auth with useroauth2and the token as password, same idea like #43 for GitHub.#52. After a squash merge the MR commits, and their notes, stay only on
refs/merge-requests/<iid>/head. The job took the source from commit parents, so the lines stayed untracked. Now it reads the<project path>!<iid>reference from the merged commit message, fetches the MR head and uses it as source, but only when its diff has the same stable patch ID as the diff the target commit brings. Otherwise it keeps the parents like before and prints a warning. The default GitLab squash template is only the title, so with the fast-forward merge method the project has to add%{reference}there. README says it now.We squash merge, so the block below keeps three changelog entries instead of one:
BEGIN_COMMIT_OVERRIDE
fix(ci): install pinned release binary in forge workflows (#50)
fix(ci): use HTTP basic auth in GitLab forge job (#51)
fix(ci): reconstruct GitLab squash merges from merge request head (#52)
END_COMMIT_OVERRIDE
Scope
install-hooksdoes not warn anduninstallstill removes it.GIT_BYLINE_VERSIONandGIT_BYLINE_RELEASES_URLcan be set as CI/CD variables, for example for an internal mirror. The GitHub workflow triggers on the default branch instead of hardcodedmain.%{reference}in the squash template), docs/SECURITY_MODEL.md, docs/SUPPLY_CHAIN.md, docs/ARCHITECTURE.md..github/workflows/git-byline.ymland.gitlab/ci/git-byline.ymlare byte for byte the embedded templates. release-please-config.json bumps the pin in both templates.checksums.txtand its version before it runs. The MR head is used only when the patch ID matches, so a wrong or crafted reference in a commit message cannot bring attribution from another MR.Related issue
Fixes #50
Fixes #51
Fixes #52
Validation
Not run against live GitLab yet, I will verify that in the pilot.
Checklist