ci: speed up SCA dependency checks with shared module cache - #422
Conversation
loveRhythm1990
left a comment
There was a problem hiding this comment.
The job-wide GOPROXY change is directionally sound, but the cache additions currently have no writable producer in the MatrixOne repository context. Please address the blocking cache lifecycle issue before merging. I also left a scope comment for non-Shanghai SCA runner overrides.
aptend
left a comment
There was a problem hiding this comment.
Deep re-review completed on exact head f41f24e, including the prior inline discussions and the paired MatrixOne producer PR #27128.
The two earlier concerns are resolved: the cluster-local GOPROXY is now installed only for the default or Shanghai-labelled runner, and the pull_request_target consumer uses actions/cache/restore only, with no post-job save path. The producer and consumer use the same MatrixOne-owned GOMODCACHE path and the same OS/architecture/Go-version/dependency-hash key plus compatible restore prefix. Dependency-changing PRs can safely restore a trusted main-prefix cache and download only the delta without being able to overwrite shared cache state.
I also checked the setup ordering, fallback semantics, cache ownership/scope, key compatibility, workflow diff, and validation result. No blocking correctness, security, or lifecycle issue remains. The performance benefit depends on landing the trusted producer in matrixorigin/matrixone#27128, but that is an ordering dependency rather than a defect in this restore-only consumer.
|
Deep re-review found and fixed one additional cross-runner cache portability issue in a157b07. actions/cache includes the raw path in its hidden cache version and stores archive entries relative to the workspace. Using an absolute go env GOMODCACHE meant producer and consumer could have the same visible key but still miss (or restore to the wrong layout) when their runner HOME/workspace paths differed. The cache now:
Local validation: producer/consumer path+key+prefix equality check passed, cache cleanup/relocation lifecycle simulation passed, diff check passed, and actionlint reports only the two unchanged baseline findings in ci.yaml. |
## What type of PR is this? - [ ] API-change - [ ] BUG - [x] Improvement - [ ] Documentation - [ ] Feature - [x] Test and CI - [ ] Code Refactoring ## Which issue(s) this PR fixes: issue #27076 ## What this PR does / why we need it: Adds a trusted MatrixOne-side producer for the SCA Go module cache consumed by matrixorigin/CI#422. - runs on trusted main pushes that change go.mod/go.sum, a daily schedule, and workflow_dispatch - restores an existing compatible main cache before downloading deltas - downloads all modules with the Shanghai proxy when running on a Shanghai SCA runner - saves the exact cache key in the MatrixOne default-branch scope - serializes warm-up runs and skips download/save on an exact hit - uses the same relative staging path as the consumer so the hidden cache version and tar layout remain portable across runner nodes The reusable PR workflow remains restore-only because MatrixOne CI is triggered by pull_request_target. Different PRs can therefore share this trusted main cache without being able to poison or overwrite it. The consumer moves the restored cache out of the source tree before license-eye scans source files. Related: matrixorigin/CI#422 Validation: - YAML syntax parsed successfully - actionlint passed - producer/consumer path, key, and restore prefix match mechanically - cache cleanup and relocation lifecycle simulated successfully - git diff --check passed
Summary
Cache lifecycle
The cache belongs to the calling matrixorigin/matrixone repository, not this reusable-workflow repository.
Producer and consumer use the same relative staging path and key algorithm. The relative path keeps both the actions/cache hidden version and archive entries portable across runner layouts. The consumer moves the restored module cache into RUNNER_TEMP before static checks so license-eye does not enumerate cached dependency sources.
Why
A recent MatrixOne SCA run spent about 85 minutes in make static-check, including 64m16s in license-eye dep check. That check repeats Go module downloads. The previous GOPROXY was scoped only to Prepare ENV and no cross-PR MatrixOne main cache existed.
With an existing module cache, license-eye dep check completes in about 0.6 seconds. No SCA validation is skipped.
Validation
Related producer: matrixorigin/matrixone#27128