Skip to content

ci: enforce cross-platform quality gates and connector coverage - #618

Open
somethingwithproof wants to merge 13 commits into
Cacti:developfrom
somethingwithproof:chore/597-build-test-infrastructure
Open

ci: enforce cross-platform quality gates and connector coverage#618
somethingwithproof wants to merge 13 commits into
Cacti:developfrom
somethingwithproof:chore/597-build-test-infrastructure

Conversation

@somethingwithproof

@somethingwithproof somethingwithproof commented Sep 6, 2026

Copy link
Copy Markdown
Member

Summary

Consolidates the connector/build work from #613 and the broader test-infrastructure work extracted from #597 into one review:

  • discover MariaDB Connector/C first while retaining MySQL and Percona-compatible support
  • test MariaDB and MySQL Connector/C with strict GCC and Clang builds on Ubuntu 24.04
  • add a real macOS 15 build and unit-test lane with portable Autotools platform detection
  • run unit tests under ASan/UBSan and ThreadSanitizer
  • enforce the measured 7% global coverage baseline and 70% changed-line coverage
  • add libFuzzer coverage for output parsing alongside the existing hostname parser
  • exercise the SNMPv3 integration suite against MariaDB and MySQL 8.4
  • add cppcheck, shellcheck, DCO verification, CODEOWNERS, distcheck, and Linux-capability checks while retaining the repository-managed default CodeQL setup
  • expose a stable CI / required aggregate check for branch protection
  • pin runners and third-party actions and keep workflow permissions read-only by default

Correctness fixes found by the new gates

  • reject truncated capability strings and poller-output SQL tuples instead of publishing partial data
  • make bootstrap fail closed and select libtoolize/glibtoolize portably
  • use Autoconf's canonical host for Darwin detection instead of an empty native-build alias
  • remove stale initialization/name-shadow findings and make read-only interfaces const-correct
  • make the ICMP parser test portable across Linux and BSD header names

Verification

  • strict GCC and Clang builds: 4/4 unit targets passed
  • ASan/UBSan: 4/4 passed
  • ThreadSanitizer: 4/4 passed
  • macOS Clang/Homebrew build: 4/4 passed
  • MariaDB integration: passed
  • MySQL 8.4 integration: passed
  • both libFuzzer targets: passed sustained local runs
  • coverage: 7.2% global and 84% on changed lines
  • cppcheck, shellcheck, actionlint, DCO verification, and git diff --check: passed locally

Post-merge repository settings

After these workflows exist on develop, an administrator should require CI / required, Fuzz, Integration, Rocky Linux, and the repository-managed CodeQL/code-scanning result in the active ruleset; enable code-owner approval; and require the branch to be current (or use a merge queue). Enabling those settings before this PR merges would create required checks that the base branch cannot yet emit.

Consolidates and supersedes #613. Closes #612.

Copilot AI lite review requested due to automatic review settings September 6, 2026 00:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The fuzz workflow is path-filtered and now depends on ./.github/actions/build-spine, but changes to that action won’t trigger the fuzz workflow on PRs unless the path filter is updated.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR centralizes the CI build/setup logic into a reusable composite action and updates CI jobs to run a broader set of checks (notably running make check under ASan/UBSan) while removing unnecessary MySQL server installation.

Changes:

  • Added a composite action (.github/actions/build-spine) to install dependencies and run ./bootstrap && ./configure consistently across workflows.
  • Updated CI workflows to use the composite action, removing duplicated apt/bootstrap/configure steps (and dropping mysql-server where unused).
  • Strengthened sanitizer coverage by running the unit test suite (make check) under ASan/UBSan.
File summaries
File Description
.github/workflows/integration.yml Pins checkout and hardens step shells; keeps integration script execution intact.
.github/workflows/fuzz.yml Switches build setup to the shared composite action and updates artifact upload pin.
.github/workflows/ci.yml Replaces duplicated setup with the composite action; runs make check under sanitizers; removes mysql-server installs.
.github/actions/build-spine/action.yml New composite action encapsulating apt deps + bootstrap/configure with configurable args/flags.
Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/workflows/fuzz.yml
@somethingwithproof

Copy link
Copy Markdown
Member Author

@copilot review

@somethingwithproof

Copy link
Copy Markdown
Member Author

@copilot review

Discover mariadb_config before mysql_config while retaining MySQL and Percona-compatible fallback linking. Validate both headers and client symbols instead of relying on filename probes.

Exercise MariaDB Connector/C and MySQL Connector/C across the GCC and Clang build matrix and update the build documentation.

Closes Cacti#612

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
… from

The fallback discovery scanned seven prefixes for mysql.h but only added -L
paths when MYSQL_DIR was set, so a client installed under any other prefix
compiled and then failed to link.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Every build job repeated the same apt-get install and bootstrap/configure
pair, so a new dependency meant editing five places. The setup now lives in
.github/actions/build-spine.

mysql-server is dropped from the package set: configure only looks for
mysql.h and libmysqlclient, both from libmysqlclient-dev, and no job starts
or connects to a server.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
The sanitizer build only ran --version and --help, which exercises almost
none of the poller. make check is what drives the code, so it is what the
sanitizers should watch.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
@somethingwithproof
somethingwithproof force-pushed the chore/597-build-test-infrastructure branch from 8660558 to e84a752 Compare September 6, 2026 07:04
@somethingwithproof somethingwithproof changed the title ci: centralize build setup and exercise tests under sanitizers build(ci): modernize connector discovery and shared test setup Sep 6, 2026
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
@somethingwithproof somethingwithproof changed the title build(ci): modernize connector discovery and shared test setup ci: enforce cross-platform quality gates and connector coverage Sep 6, 2026
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Prefer MariaDB Connector/C while retaining MySQL compatibility

2 participants