ci: Restrict release workflow to master branch builds and fix broken tests#325
Open
dividedmind wants to merge 6 commits intomasterfrom
Open
ci: Restrict release workflow to master branch builds and fix broken tests#325dividedmind wants to merge 6 commits intomasterfrom
dividedmind wants to merge 6 commits intomasterfrom
Conversation
Previously, the release workflow triggered on every "Build and test" run regardless of branch. Although a job-level condition prevented actual releases, the workflow still ran and prompted admin approval due to its access to signing and publishing secrets. Added a branches filter to the workflow_run trigger so the workflow is never queued for feature branch builds, and removed the now-redundant head_branch check from the job condition.
There was a problem hiding this comment.
Pull request overview
This PR updates the release GitHub Actions workflow so it only triggers from successful “Build and test” runs on the master branch, preventing the release workflow from being queued (and requesting elevated approvals/secrets) for feature-branch builds.
Changes:
- Added a
branches: [master]filter to theworkflow_runtrigger. - Simplified the release job condition by removing the redundant
head_branch == 'master'check.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
When triggered by workflow_run, actions/checkout defaults to the HEAD of the default branch at the time the workflow starts, not the commit that triggered the parent workflow. This creates a race window where a commit that hasn't been tested could be released if new commits land on master between CI completing and the release workflow starting. Pin the checkout to github.event.workflow_run.head_sha when triggered by workflow_run so the release always runs against the tested commit. The ref is left empty for workflow_dispatch, preserving the default behavior of checking out the dispatched ref.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...st/fixture/shared/src/main/java/org/springframework/samples/petclinic/web/ShowAvailable.java
Show resolved
Hide resolved
...st/fixture/shared/src/main/java/org/springframework/samples/petclinic/web/ShowAvailable.java
Outdated
Show resolved
Hide resolved
dividedmind
changed the title
dividedmind
force-pushed
the
ci/dont-dispatch-release-on-feature-branches
branch
from
5f9410d to
6b4e2be
Compare
dividedmind
changed the title
Updated GitHub Actions to their latest major versions across all workflows to keep the CI environment up-to-date and secure: - actions/checkout to v6 - actions/setup-java to v5 - actions/upload-artifact to v6 - actions/download-artifact to v8 - gradle/actions/setup-gradle to v5 Configuration parameters remain compatible with the new versions.
The upstream spring-framework-petclinic project changed Jackson coordinates from com.fasterxml.jackson to tools.jackson.core, breaking our custom ShowAvailable controller in petclinic-fw tests. Replaced ObjectMapper with manual JSON formatting and removed the now-unused Result inner class, making the test fixture resilient to upstream dependency changes.
Upstream now requires Java 21.
The upstream spring-petclinic project renamed the `testOwnerDetails` test method to `ownerDetails` and changed the log level of some statements from INFO to DEBUG. - Updated `petclinic-tests.bats` to support both test method names by using wildcards/regex. - Expanded the log label assertion to match both `info` and `debug` functions. - Wrapped the jq log label extraction in `first()` to prevent multiple matches from causing the `assert_json_eq` to fail with multiple lines.
dividedmind
force-pushed
the
ci/dont-dispatch-release-on-feature-branches
branch
from
6b4e2be to
6dd4744
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously, the release workflow triggered on every "Build and test" run regardless of branch. Although a job-level condition prevented actual releases, the workflow still ran and prompted admin approval due to its access to signing and publishing secrets.
Added a branches filter to the workflow_run trigger so the workflow is never queued for feature branch builds, and removed the now-redundant head_branch check from the job condition.
In addition, I noticed that release always ran against current master, instead of the revision that actually passed tests; this is fixed now.
Also, I discovered a few issues with our integration tests that failed due to upstream project changes. These have now been fixed.