Skip to content

CI: Run the self-test on pull requests - #51

Open
ModeSevenIndustrialSolutions wants to merge 3 commits into
lfreleng-actions:mainfrom
modeseven-lfreleng-actions:ci/pr-triggered-self-test
Open

CI: Run the self-test on pull requests#51
ModeSevenIndustrialSolutions wants to merge 3 commits into
lfreleng-actions:mainfrom
modeseven-lfreleng-actions:ci/pr-triggered-self-test

Conversation

@ModeSevenIndustrialSolutions

@ModeSevenIndustrialSolutionsModeSevenIndustrialSolutions commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Closes#49.

Three commits:

  1. Chore: Bump maven-build-action to v0.4.2
  2. CI: Run the self-test on pull requests
  3. Docs: Correct the action pin release table

Problem

testing.yaml was workflow_dispatch: only. The reusable Maven and Gradle lanes had no PR-time validation at all — pull requests ran the meta-checks and never exercised the workflows themselves. Its last recorded run was 2026-08-25, and it failed.

This is very likely why #47 went unnoticed: the Maven lane produces an SBOM containing a single unversioned dependency, Grype dutifully reports No vulnerabilities found, and nothing ever checked.

1. Bump maven-build-action v0.4.0 → v0.4.2

v0.4.0 places the shared JaCoCo execution data in <base>/target — the reactor root's own build directory. Maven orders the root last wherever the root artifact depends on one of its modules, so the root's clean runs after every subproject has contributed coverage and deletes the merged file. The report step then fails a build whose compile and tests both passed.

Fixed upstream by cdc2803 (2026-08-29), which moves the file to <base>/.jacoco — a directory no module's clean owns — plus 1803ab5, which assembles the shared data after the build rather than during it. v0.4.0 was the last release without those.

This lands first so the self-test commit needs no workaround.

2. Enable PR-time self-testing

workflow_dispatch:pull_request:

Adopts the trigger python-workflows uses, along with its documented rationale for excluding the manual one:

A manual run happens on the default branch, which would hand this job a cache token with write access to the default-branch scope while it builds and tests third-party consumer repositories. Untrusted code from those repositories could then poison caches that later runs restore (CWE-349). Pull request runs write only to their own cache scope.

This repo had not merely omitted pull_request — it used exactly the trigger python-workflows rejected on cache-poisoning grounds, and omitted the one it adopted.

Maven lane repointed at the fixture

- repository: 'apache/maven'- ref: 'maven-3.9.9'- harden_runner_egress: 'audit'+ repository: 'lfreleng-actions/test-maven-project'
  • Fast enough for every PR — a three-module reactor instead of building Apache Maven itself.
  • block egress instead of audit. The fixture depends only on JUnit, so its footprint is the allow-listed toolchain set. The audit override existed solely because the placeholder's footprint was unknown; that justification is gone. Verified by three green runs, not assumed.
  • No ref pin, matching how python-workflows references org-owned consumers.

Safe to build unattended: the fixture ships FailingGreeterTest, but its parent POM sets surefire.excludes = **/Failing*Test.java and only the failing-tests profile clears it.

Gradle lane unchanged

Keeps onap/portal-ng-preferences at its pinned commit with audit egress, pending a fixture (#50).

Docs

README.md and docs/BRIEF.md both described the workflow as workflow_dispatch-only and skipped on PRs, and BRIEF.md listed creating both fixtures as a prerequisite.

3. Correct the action pin table

Copilot flagged the maven-build-action row as stale. Cross-checking the rest against the actual uses: pins found four of five rows wrong and grype-scan-action missing from both that table and the building-block table above it:

ActionTable saidActual
build-metadata-actionv0.7.0v0.8.0
maven-build-actionv0.3.0v0.4.2
gradle-build-actionv0.5.0v0.6.0
junit-test-report-actionv0.0.1v0.0.1 ✅
sbom-actionv0.0.1v0.0.2
grype-scan-action(absent)v0.0.2

The maven-build-action row was already stale before this branch (v0.3.0 documented against a v0.4.0 pin); the bump only made it visible. Kept as a separate commit since most of it is pre-existing drift.

How the pin problem surfaced

Turning the self-test on failed Maven Build/Test / Build on the first run:

14:48:59 jacoco:report @ test-maven-core -- Loading execution data file <shared> EXISTS
14:49:01 jacoco:report @ test-maven-app -- Loading execution data file <shared> STILL EXISTS
14:49:01 clean:clean @ test-maven-project DELETES <base>/target/
14:49:01 BUILD SUCCESS
14:49:02 Error: tests ran, yet no JaCoCo execution data gone

I initially filed this as a new bug (lfreleng-actions/maven-build-action#139) — but it had been fixed three days earlier and verified against opendaylight/yangtools, whose root sorts 120th of 120. The action was never broken on main; this repo was pinned to the last release without the fix. #139 is closed as a duplicate.

Coverage now demonstrably aggregates across modules rather than silently no-op'ing:

Combined JaCoCo execution data from 2 subproject file(s) into .../.jacoco/jacoco-aggregate.exec

⚠️ That is the cross-module attribution #28 closed with outstanding"test-maven-project is multi-module (parent/core/app); assert its badge/summary coverage reflects cross-module test attribution after the bump." It had never been executed, because there was nowhere to execute it.

⚠️ What green means here

This PR makes the Java lanes testable. It does not make them correct.

The Grype Audit SBOM check passes trivially against the near-empty SBOM measured in #47. A green run means "the Maven lane now runs on PRs", not "the Maven lane is healthy". #47 makes the result meaningful; this PR is its prerequisite.

Rebase note

Rebased onto main after #52 merged. Verified zero issues: read reintroduced — this branch predated #52 and its testing.yaml still carried those lines, so a careless merge risked partially reverting your change.

Validation

  • yamllint, actionlint, markdown-table-fixer — clean
  • Full prek run over changed files — all hooks passed
  • Executed end-to-end on this PR itself, coverage enabled, 22/22 green
  • zizmor --persona=auditor reports 2 pre-existing self-repository findings on the uses: ./... lines — unchanged (2 before, 2 after) and deliberately not fixed, since actionlint does not yet understand the $/... syntax Zizmor wants

Related

@ModeSevenIndustrialSolutions
ModeSevenIndustrialSolutions requested review from a team and a balanced review from CopilotSeptember 1, 2026 14:47
@github-actionsgithub-actionsBot added the CI CI and tests updates label Sep 1, 2026

This comment was marked as resolved.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

@ModeSevenIndustrialSolutions
ModeSevenIndustrialSolutions marked this pull request as draft September 1, 2026 22:00
v0.4.0 places the shared JaCoCo execution data in <base>/target,
which is the reactor root's own build directory. Maven orders the
root last wherever the root artifact depends on one of its modules,
so the root's clean runs after every subproject has contributed
coverage and deletes the merged file. The report step then fails a
build whose compile and tests both passed.
v0.4.1 moves that file to <base>/.jacoco, which no module's clean
owns, and v0.4.2 assembles the shared data after the build rather
than during it.
Co-authored-by: Claude <noreply@anthropic.com>
Signed-off-by: Matthew Watkins <mwatkins@linuxfoundation.org>
The reusable Maven and Gradle lanes had no PR-time validation: the
self-test was workflow_dispatch only, so pull requests ran the meta
checks and never exercised the workflows themselves. Its last run
was 2026-08-25 and it failed. A near-empty SBOM passing a Grype
scan is exactly the kind of defect this hid.
Switch to pull_request and drop workflow_dispatch, matching
python-workflows. That repo documents why the manual trigger is
unsafe here: a dispatch runs on the default branch and hands the
job a cache token with default-branch write scope while it builds
third-party code, which can poison caches later runs restore
(CWE-349). Pull request runs write only to their own scope.
Repoint the Maven lane at test-maven-project, as the file's own
TODO asked. The fixture is a three-module reactor depending only on
JUnit, so its egress footprint is the allow-listed toolchain set and
the lane runs under block rather than audit. Its Failing*Test
classes stay excluded unless the failing-tests profile is active.
The Gradle lane keeps its pinned upstream target and audit egress
because no test-gradle-project fixture exists yet, tracked in lfreleng-actions#50.
Co-authored-by: Claude <noreply@anthropic.com>
Signed-off-by: Matthew Watkins <mwatkins@linuxfoundation.org>
@ModeSevenIndustrialSolutions
ModeSevenIndustrialSolutions marked this pull request as ready for review September 2, 2026 08:30
CopilotAI review requested due to automatic review settings September 2, 2026 08:30

This comment was marked as outdated.

CopilotAI review requested due to automatic review settings September 2, 2026 14:10

This comment was marked as outdated.

BRIEF.md listed a release number per building-block action. Dependabot
maintains those pins weekly, so the list was stale within days of being
written: it landed once in the initial commit and was never updated,
through bumps to build-metadata-action, sbom-action and others.
The claim the table supported is durable; the numbers were not. Keep
the claim, that no action is consumed from an unreleased ref, and
point at the '# vX.Y.Z' comment beside each 'uses:' as the record.
A version list here only guarantees that every future bump PR
contradicts the documentation.
Add grype-scan-action to the building-block table. It composes the
verify lane and appears in the job graph, but was absent. That entry
names a role rather than a version, so it does not decay.
Co-authored-by: Claude <noreply@anthropic.com>
Signed-off-by: Matthew Watkins <mwatkins@linuxfoundation.org>

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

The changes are consistent, correctly documented, and validated by successful end-to-end workflow runs.

Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CICI and tests updates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reusable lanes get no PR-time end-to-end testing

2 participants

@ModeSevenIndustrialSolutions