ci: use pr for maven and npm release and set permissions #1361
Conversation
📝 WalkthroughWalkthroughAdds Changes
*Note: the npm-release.yml file appears in both the Release workflows cohort (for Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/maven-release.yml (1)
23-30: Addpull-requests: writepermission to support PR creation.The
use-pr: 'true'parameter at line 37 suggests the action will create pull requests for version bumps. However, this workflow lacks an explicitpermissionsblock withpull-requests: write, which is present in.github/workflows/npm-release.yml(line 32).Without this permission, the action may fail to create PRs when branch protection rules are enforced.
🔒 Add required permissions
jobs: release-maven: runs-on: ubuntu-latest + permissions: + contents: write + pull-requests: write outputs: MVN_ARTIFACT_ID: ${{ steps.maven-release-step.outputs.MVN_ARTIFACT_ID }} ARTIFACT_NAME: ${{ steps.maven-release-step.outputs.artifact-name }}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/maven-release.yml.github/workflows/npm-release.yml
🔇 Additional comments (2)
.github/workflows/maven-release.yml (1)
37-37: Theuse-prparameter is properly supported and correctly interprets the string value'true'.The action's
action.ymldefines theuse-prinput with a default value of"false"and description "Use PR for version bump instead of direct push". The implementation correctly compares the string value against'true'in two places: setting Maven arguments (-DpushChanges=${{ inputs.use-pr != 'true' }}) and gating the pull request creation step (if: ${{ inputs.use-pr == 'true' }}). No issues found..github/workflows/npm-release.yml (1)
43-43: No action needed. Theaction-npm-releaseaction at the specified commit (12966547963631f8429d42bab860485b413003c2) fully supports theuse-prparameter, which is defined in itsaction.ymlwith a default value of"false". The action correctly interprets the string value'true'using string comparison (== 'true') to determine whether to create a pull request or push changes directly (lines 53 and 58 of the action).
hupling
left a comment
hupling
left a comment
There was a problem hiding this comment.
lgtm. ich habe noch das in der beschreibung ergänzt it-at-m/lhm_actions#19
simonhir
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/deploy-docs.yml (1)
26-28: Add condition to deploy only from main branch.The comment states "Only deploy documentation from the main branch to prevent unauthorized changes," but the workflow runs on both
pushto main andpull_requestevents (lines 11-14). Without a condition, the deploy step will execute on pull requests as well, contradicting the stated intent.🔒 Proposed fix to restrict deployment to main branch
- id: deploy_docs + if: github.event_name == 'push' && github.ref == 'refs/heads/main' # Only deploy documentation from the main branch to prevent unauthorized changes uses: it-at-m/lhm_actions/action-templates/actions/action-deploy-docs@12966547963631f8429d42bab860485b413003c2 # v1.0.22
🤖 Fix all issues with AI agents
In @.github/workflows/maven-node-build.yml:
- Around line 12-14: Permissions are currently set globally to "contents: read"
and "packages: write"; to tighten scope optionally move "packages: write" off
the global permissions and grant it only to the job that pushes images (the job
that runs conditionally on main / github.ref == 'refs/heads/main'), leaving
global permissions as "contents: read" for PR builds; update the workflow by
removing packages: write from the top-level permissions and adding permissions:
packages: write to the image-push job (or create a separate push job) so PRs
only get read access while main push jobs get write.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/actionlint.yml.github/workflows/dependency-review.yml.github/workflows/deploy-docs.yml.github/workflows/dockercompose-healthcheck.yml.github/workflows/maven-node-build.yml.github/workflows/maven-release.yml.github/workflows/npm-release.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build (refarch-backend)
🔇 Additional comments (12)
.github/workflows/actionlint.yml (1)
12-13: LGTM! Appropriate permission scoping for actionlint.The
contents: readpermission is correctly scoped for the actionlint job, which only needs to read workflow files. This follows the principle of least privilege..github/workflows/dependency-review.yml (1)
9-10: LGTM! Appropriate permission scoping for dependency review.The
contents: readpermission is correctly scoped for the dependency-review job, which only needs to read dependency manifests and lockfiles..github/workflows/dockercompose-healthcheck.yml (1)
11-12: LGTM! Appropriate permission scoping for docker compose healthcheck.The
contents: readpermission is correctly scoped for the healthcheck job, which only needs to read the repository to run docker-compose healthchecks..github/workflows/npm-release.yml (4)
57-59: LGTM! Appropriate permission for image push operation.The
packages: writepermission is correctly scoped for the build-image job, which needs to push container images to GitHub Packages (line 61).
75-76: LGTM! Appropriate permission for GitHub release creation.The
contents: writepermission is correctly scoped for the create-github-release job, which needs to create GitHub releases (line 79).
1-82: No action needed. The maven-release.yml file already containsuse-pr: 'true'(line 40) and has appropriate job-level permissions (contents: write, pull-requests: write). Both release workflows are properly aligned with the PR objectives.Likely an incorrect or invalid review comment.
43-43: Parameter is supported. Theuse-pr: 'true'parameter is properly defined in theaction-npm-releaseaction at version v1.0.22. When enabled, it creates a pull request viapeter-evans/create-pull-requestaction instead of pushing directly to the branch. The existingpull-requests: writepermission allows PR creation, so the implementation is correct..github/workflows/deploy-docs.yml (1)
19-22: LGTM! Job-level permissions properly scoped.The permissions are appropriate for a GitHub Pages deployment:
contents: readfor code checkoutpages: writefor deploymentid-token: writefor OIDC authenticationScoping permissions at the job level rather than workflow level is a security best practice.
.github/workflows/maven-release.yml (4)
25-27: LGTM! Permissions align with PR-based release workflow.The permissions are correctly set for the PR-based release workflow:
contents: writeenables committing version changespull-requests: writeallows creating PRs for version bumpsThis aligns with the PR objective of using pull requests to support main branch protection.
49-50: LGTM! Appropriate permissions for container image push.The
packages: writepermission is correctly scoped for the build-image job, enabling it to push container images to the GitHub Container Registry.
66-67: LGTM! Appropriate permissions for GitHub release creation.The
contents: writepermission is correctly scoped for the create-github-release job, allowing it to create releases and upload artifacts.
40-40: Parameteruse-pris supported.The
use-prparameter is correctly defined in theaction-maven-releaseaction at v1.0.22, with a default value offalse. Settinguse-pr: 'true'properly configures Maven release to use the PR-based workflow by settingpushChanges=false, preventing direct commits as intended for the version bump process.
it-at-m/lhm_actions#19
Pull Request
Changes
Checklist
Note: If some checklist items are not relevant for your PR, just remove them.
General
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.