ci/quality: Add QNX CodeQL analysis with platform-specific delta reporting - #834
ci/quality: Add QNX CodeQL analysis with platform-specific delta reporting#834castler wants to merge 1 commit into
Conversation
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
777de20 to
42c0bd4
Compare
| - name: Upload MISRA compliance reports (QNX) | ||
| if: inputs.fetch-only != 'true' && always() && inputs.run-qnx == 'true' | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: codeql-misra-reports-qnx | ||
| path: /tmp/codeql-results/qnx/analysis_reports/ | ||
| retention-days: 30 |
There was a problem hiding this comment.
We upload this but never use it
| "name": row.get("name") or row.get("Name") or row.get("rule_id") or row.get("Rule") or "", | ||
| "message": row.get("message") or row.get("Message") or row.get("description") or row.get("Description") or "", | ||
| "path": row.get("path") or row.get("Path") or row.get("file") or row.get("File") or "", | ||
| "line": row.get("start:line") or row.get("Line") or row.get("line_number") or "", |
There was a problem hiding this comment.
Why all these conditions, should the format not be specific and we should fail if the format is not as expected?
| srcs = ["merge_codeql_findings.py"], | ||
| main = "merge_codeql_findings.py", | ||
| target_compatible_with = ["@platforms//os:linux"], | ||
| visibility = ["//visibility:public"], |
There was a problem hiding this comment.
This should not be public
| py_binary( | ||
| name = "merge_codeql_findings", | ||
| srcs = ["merge_codeql_findings.py"], | ||
| main = "merge_codeql_findings.py", |
There was a problem hiding this comment.
main is not required.
| rule_id = result.get("ruleId", "") | ||
| message = "" | ||
| msg = result.get("message") | ||
| if isinstance(msg, dict): | ||
| message = msg.get("text", "") | ||
| uri = start_line = start_col = "" | ||
| locations = result.get("locations") or [] | ||
| if locations: | ||
| phys = (locations[0] or {}).get("physicalLocation") or {} | ||
| uri = ((phys.get("artifactLocation") or {}).get("uri") or "") | ||
| region = phys.get("region") or {} | ||
| start_line = str(region.get("startLine", "")) | ||
| start_col = str(region.get("startColumn", "")) |
There was a problem hiding this comment.
Should we not fail if the schema is not as expected, instead of giving a lot of default values?
42c0bd4 to
6767a81
Compare
| run-qnx: | ||
| description: "When true, additionally analyzes the --config=qnx build and | ||
| publishes the deduplicated union plus a QNX-only delta. Requires the | ||
| SCORE_QNX_* secrets. Defaults to false so cache-warming callers keep the | ||
| Linux-only behavior." |
There was a problem hiding this comment.
This workflow is not part of checks or gate. And codeql must run uncached to provide correct results.
We may now decide to still do repository-caching. But then we should do this for both configs, since the caches could differ based on config.
If we decide to forego repository-caching as well, since this is a nightly job that can take 2mins longer to build up the repositories, then we should not talk about cache warming here at all.
| # Choose the canonical published outputs: the deduplicated union when the | ||
| # QNX analysis ran, otherwise the plain Linux results. Downstream steps and | ||
| # nightly_quality.yml always consume codeql-nightly.{csv,sarif}, keeping | ||
| # cache-warming (Linux-only) callers unchanged. |
There was a problem hiding this comment.
| # Choose the canonical published outputs: the deduplicated union when the | |
| # QNX analysis ran, otherwise the plain Linux results. Downstream steps and | |
| # nightly_quality.yml always consume codeql-nightly.{csv,sarif}, keeping | |
| # cache-warming (Linux-only) callers unchanged. | |
| # Choose the canonical published outputs: the deduplicated union when the | |
| # QNX analysis ran, otherwise the plain Linux results. |
| - name: Upload MISRA compliance reports | ||
| - name: Upload MISRA compliance reports (Linux) | ||
| if: inputs.fetch-only != 'true' && always() | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: codeql-misra-reports | ||
| path: /tmp/codeql-results/analysis_reports/ | ||
| path: /tmp/codeql-results/linux/analysis_reports/ | ||
| retention-days: 30 | ||
| if-no-files-found: warn |
There was a problem hiding this comment.
Why not upload the QNX ones as well?
| sanity-check: `union == linux + qnx_unique` and `delta == qnx \ linux`. CI uploads the union under | ||
| Code Scanning category `codeql-nightly` and the delta under `codeql-nightly-qnx-delta`; the union | ||
| CSV feeds the dashboard KPI, and the QNX-only delta CSV is passed alongside it | ||
| (`--codeql-qnx-delta-csv`) so the dashboard flags which findings are QNX-specific. |
There was a problem hiding this comment.
In S-CORE Linux is also a target platform. So should we do the same as for QNX and provide the only-Linux-findings?
| @@ -0,0 +1,252 @@ | |||
| # ******************************************************************************* | |||
There was a problem hiding this comment.
I'm honestly leaning more and more to make SARIF the sole output of CodeQL.
SARIF is a very simple format. There are a lot of tools out there that can do the conversion from SARIF to CSV and I trust that there is at least one that does this reliably.
Removing the CSV path would allow us to do diffing/merging/filtering based on established tools like https://github.com/microsoft/sarif-tools
Doing this by hand is at least equally error-prone as using an existing tool. And it raises the maintenance cost on our side.
Note: did not review this file further
| return keys | ||
|
|
||
|
|
||
| def load_codeql_csv(path: pathlib.Path, qnx_delta_keys: set | None = None) -> dict | None: |
There was a problem hiding this comment.
Use https://pypi.org/project/simple-sarif/ to reduce the manual parsing efforts here.
…rting Extends the CodeQL nightly workflow with an optional QNX build pass controlled by the new `run-qnx` input flag. When enabled, the workflow: - Runs `codeql_lint` for both Linux and `--config=qnx` targets. - Merges the two result sets via the new `merge_codeql_findings` script, producing a deduplicated Linux∪QNX union and a QNX-only delta. - Publishes union findings under the `codeql-nightly` Code Scanning category and QNX-specific findings under `codeql-nightly-qnx-delta`. - Uploads separate CSV artifacts while keeping the shared MISRA reports used by the nightly dashboard. The quality dashboard (`generate_dashboard.py`) now expects the fixed CodeQL CSV schema, validates it strictly, and marks QNX-only findings using the dedup keys emitted by `merge_codeql_findings.py`. That same script now also validates its CSV/SARIF inputs strictly instead of silently accepting unexpected shapes. The `generate_quality_links.bzl` Bazel rule gains release-asset URL generation for the dashboard, Clang-Tidy, and CodeQL reports, in addition to the new `|codeql_qnx_delta_report_link|` RST substitution. QNX environment setup delegates to the shared `.github/actions/00_infrastructure/setup_qnx_environment` composite action, replacing the previous inline license setup. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
6767a81 to
b72ce6e
Compare
2 participants
Extends the CodeQL nightly workflow with an optional QNX build pass controlled by the new
run-qnxinput flag. When enabled, the workflow:codeql_lintfor both Linux and--config=qnxtargets.merge_codeql_findingsscript, producing a deduplicated Linux∪QNX union and a QNX-only delta.codeql-nightlyCode Scanning category and QNX-specific findings undercodeql-nightly-qnx-delta.The quality dashboard (
generate_dashboard.py) gains awareness of the QNX-only delta: it tags each finding withqnx_onlyand displays per-severity QNX-only counts in both the rendered HTML and the GitHub step summary. Theload_codeql_csv/load_qnx_delta_keyshelpers mirror the deduplication key used bymerge_codeql_findings.pyso that QNX-delta rows are matched correctly.