Repository navigation
ci: pin the database image and install locked shared test dependencies - #306
Conversation
There was a problem hiding this comment.
🔵 Needs a closer look
The changes pass current checks but depend on the still-unmerged Cacti PR #8345.
0 open findings
What changed in this PR
Hardens CI by pinning fixtures and using Cacti’s shared locked dependency harness.
Changes:
- Pins Cacti, MariaDB, Composer, and GitHub Actions dependencies.
- Restricts permissions, credentials, concurrency, and runtime.
- Uses the shared Pest harness without adding plugin-local Composer files.
| File | Description |
|---|---|
CHANGELOG.md |
Records the CI hardening. |
.github/workflows/plugin-ci-workflow.yml |
Pins dependencies and adopts shared tooling. |
.github/workflows/codeql.yml |
Disables persisted checkout credentials. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
bmfmancini
left a comment
There was a problem hiding this comment.
Reviewed the full diff. Replaces the ad-hoc sudo composer require/install path (flying without a lock) with the pinned Cacti/cacti shared action b1d8dd6a03a3c756a2b22dfe3a0c744b1f11116c and a locked plugin-harness composer install run as the non-root runner user; CACTI_PEST_BIN is exported by that action and consumed by the Pest step. Both the Cacti fixture (d6d230f712c0, 1.2.x) and helper revision are real immutable SHAs; mariadb service and all actions are digest/SHA-pinned; checkouts now persist-credentials: false; workflow gains contents: read, a pull_request-scoped concurrency group, and a 45m timeout. CodeQL checkout gets the same persisted-credentials fix. Verified the sparse-checkout paths (setup-plugin-dependencies, plugin-harness with composer.lock) exist at the helper revision and that CACTI_REF d6d230f is on origin/1.2.x lineage. All 5 checks pass. Approving.
b131a76 to
0d44840
Compare
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
987f193 to
936ff4e
Compare
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
|
TheWitness
left a comment
There was a problem hiding this comment.
Reviewed: pins the MariaDB image by digest, pins the shared CI tooling to an immutable Cacti commit (verified it exists and contains the setup-plugin-dependencies action), installs locked test dependencies, and tightens workflow permissions/credentials. CI is green. LGTM.



CI currently changes Cacti's Composer manifest and resolves dependencies during each run. Check out the current Cacti branch head, pin the MariaDB image, install committed locks through the shared core action, and run Composer as the runner user. Set read-only permissions, disable persisted checkout credentials, cap integration jobs at 45 minutes, and cancel superseded PR runs. Add Actions updates where missing.
The existing events, matrix, job names, PHPUnit configuration, coverage thresholds, plugin schema/localization checks and polling steps are retained. No plugin-local test manifest is added. Stream retains its own runtime dependency contract.
Validation: full actionlint including shellcheck passes. The shared harness passes Thold's Linux tests and FlowView/Stream unit tests. Syslog's two unit failures reproduce with the existing core Pest toolchain. GitHub integration checks on this PR must complete before merge.
Depends on Cacti/cacti#8345. Merge the shared core tooling before this caller migration. The tooling is sparse-checked out at the published helper commit because Cacti source archives exclude CI files. No secrets are passed to the helper.
Changelog follow-up: place this PR's CI entry under an unreleased develop heading instead of released 2.0. The previously existing release sections are preserved. The changelog-only correction passes whitespace checks, released-section preservation checks and a three-way merge against develop. Hosted checks must finish on the updated head.