Docker publish workflow#115
Conversation
📝 WalkthroughWalkthroughThis PR migrates the PHPLIST application from listening on port 80 to port 8081 by adding Apache vhost configuration, updating Docker Compose port mapping and environment URLs, mounting Apache configs into the container, and introducing a GitHub Actions workflow for automated Docker image building and publishing to Docker Hub. ChangesPort 8081 Migration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
.github/workflows/docker-publish.yml (1)
19-19: 💤 Low valueConsider using ubuntu-latest for automatic updates.
The workflow specifies
ubuntu-22.04explicitly. While this provides consistency, usingubuntu-latestwould automatically benefit from GitHub's runner updates and security patches without requiring workflow changes.Suggested change
docker: - runs-on: ubuntu-22.04 + runs-on: ubuntu-latest permissions: contents: read🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/docker-publish.yml at line 19, Update the GitHub Actions runner specification by changing the runs-on value from "ubuntu-22.04" to "ubuntu-latest" so the workflow automatically uses GitHub's updated Ubuntu runner; locate the runs-on: ubuntu-22.04 entry in the workflow file and replace it with runs-on: ubuntu-latest, then run or validate the workflow to ensure no runner-specific assumptions break.docker/apache/000-default.conf (1)
1-12: Consider mounting the VirtualHost config directly tosites-enabledor explicitly enabling the site.The configuration is mounted to
/etc/apache2/sites-available/, relying on a pre-existing symlink from the basephp:8.1-apacheimage to activate it. For clarity and to follow Apache best practices, either:
- Mount directly to
/etc/apache2/sites-enabled/000-default.conf, or- Add
a2ensite 000-defaultto the startup command in docker-composeThis removes ambiguity about whether the site is properly enabled.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docker/apache/000-default.conf` around lines 1 - 12, The VirtualHost entry (VirtualHost *:8081, ServerName app in 000-default.conf) is being mounted into sites-available and may not be enabled; either mount this config directly into the sites-enabled location (e.g., as 000-default.conf) so Apache picks it up, or ensure the container startup enables it by adding a2ensite 000-default to the service startup/entrypoint in docker-compose; update the compose mount or startup command accordingly so the VirtualHost is unambiguously enabled.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/docker-publish.yml:
- Around line 3-12: The workflow currently only triggers for the branch named
"docker-publish" under the on: push and on: pull_request blocks; update those
blocks so they include "main" (e.g., add "main" alongside "docker-publish" in
the branches lists for push and pull_request) so pushes and PRs to main will
trigger the workflow while leaving the tags ('v*') behavior unchanged; refer to
the "on: push" and "on: pull_request" sections in the workflow file to make this
change.
- Around line 44-47: The current tag entry
"type=raw,value=test,enable={{is_default_branch}}" won't apply on the
docker-publish branch; update the tags section so the "test" tag is enabled when
building from the intended branch (either change the enable condition from
is_default_branch to a branch-name check for "docker-publish" or other target
branch, or change the workflow trigger to run on the default branch), e.g.,
modify the enable expression for the tag in the tags block (the line referencing
type=raw,value=test,enable={{is_default_branch}}) or adjust the workflow
triggers so they align.
- Line 15: The workflow is publishing to a personal Docker Hub image: the
DOCKERHUB_IMAGE variable currently set to "tatevikg1/phplist4" should be changed
to the official organization image (e.g., "phplist/base-distribution" or your
org's canonical repo) to ensure proper ownership and continuity; update the
DOCKERHUB_IMAGE value in the workflow and also rotate/update any related secrets
(Docker Hub username/password or token referenced by the workflow) to use the
organization account credentials so the publish step continues to authenticate
and push successfully.
In `@docker-compose.yml`:
- Line 4: Replace the personal Docker Hub image reference "image:
tatevikg1/phplist:test" with the organization's Docker Hub repository (e.g.,
"image: <ORG_USERNAME>/phplist:test"); update the image value in the
docker-compose service where "image: tatevikg1/phplist:test" appears so it
points to the organization account and tag used by your CI/workflows.
---
Nitpick comments:
In @.github/workflows/docker-publish.yml:
- Line 19: Update the GitHub Actions runner specification by changing the
runs-on value from "ubuntu-22.04" to "ubuntu-latest" so the workflow
automatically uses GitHub's updated Ubuntu runner; locate the runs-on:
ubuntu-22.04 entry in the workflow file and replace it with runs-on:
ubuntu-latest, then run or validate the workflow to ensure no runner-specific
assumptions break.
In `@docker/apache/000-default.conf`:
- Around line 1-12: The VirtualHost entry (VirtualHost *:8081, ServerName app in
000-default.conf) is being mounted into sites-available and may not be enabled;
either mount this config directly into the sites-enabled location (e.g., as
000-default.conf) so Apache picks it up, or ensure the container startup enables
it by adding a2ensite 000-default to the service startup/entrypoint in
docker-compose; update the compose mount or startup command accordingly so the
VirtualHost is unambiguously enabled.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 078c070d-712c-4878-8faa-f7a5ab81162b
📒 Files selected for processing (5)
.github/workflows/docker-publish.ymldocker-compose.ymldocker/apache/000-default.confdocker/apache/ports.confdocker/apache/servername.conf
| on: | ||
| push: | ||
| branches: | ||
| - docker-publish | ||
| tags: | ||
| - 'v*' | ||
| pull_request: | ||
| branches: | ||
| - docker-publish | ||
| workflow_dispatch: |
There was a problem hiding this comment.
Critical: Workflow branch configuration mismatch.
The workflow triggers on the docker-publish branch (lines 5-6 and 10-11), but this PR is targeting the main branch. Once merged to main, this workflow will not trigger on future pushes to main.
This means:
- Pushes to
mainwon't build/publish Docker images - The workflow will only run on the
docker-publishbranch - Tag pushes will still work, but regular branch pushes won't
Update the trigger branches to include main:
🔧 Proposed fix
on:
push:
branches:
- - docker-publish
+ - main
tags:
- 'v*'
pull_request:
branches:
- - docker-publish
+ - main
workflow_dispatch:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| on: | |
| push: | |
| branches: | |
| - docker-publish | |
| tags: | |
| - 'v*' | |
| pull_request: | |
| branches: | |
| - docker-publish | |
| workflow_dispatch: | |
| on: | |
| push: | |
| branches: | |
| - main | |
| tags: | |
| - 'v*' | |
| pull_request: | |
| branches: | |
| - main | |
| workflow_dispatch: |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/docker-publish.yml around lines 3 - 12, The workflow
currently only triggers for the branch named "docker-publish" under the on: push
and on: pull_request blocks; update those blocks so they include "main" (e.g.,
add "main" alongside "docker-publish" in the branches lists for push and
pull_request) so pushes and PRs to main will trigger the workflow while leaving
the tags ('v*') behavior unchanged; refer to the "on: push" and "on:
pull_request" sections in the workflow file to make this change.
| workflow_dispatch: | ||
|
|
||
| env: | ||
| DOCKERHUB_IMAGE: tatevikg1/phplist4 |
There was a problem hiding this comment.
Major: Use organization Docker Hub account instead of personal account.
The workflow uses tatevikg1/phplist4, which appears to be a personal Docker Hub account. For the official phpList project, this should use an organization account (e.g., phplist/base-distribution or similar) to ensure:
- Proper ownership and access control
- Continuity if the personal account becomes unavailable
- Professional branding for the official project
📝 Suggested change
env:
- DOCKERHUB_IMAGE: tatevikg1/phplist4
+ DOCKERHUB_IMAGE: phplist/base-distributionNote: Update the corresponding secrets to use the organization account credentials.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| DOCKERHUB_IMAGE: tatevikg1/phplist4 | |
| env: | |
| DOCKERHUB_IMAGE: phplist/base-distribution |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/docker-publish.yml at line 15, The workflow is publishing
to a personal Docker Hub image: the DOCKERHUB_IMAGE variable currently set to
"tatevikg1/phplist4" should be changed to the official organization image (e.g.,
"phplist/base-distribution" or your org's canonical repo) to ensure proper
ownership and continuity; update the DOCKERHUB_IMAGE value in the workflow and
also rotate/update any related secrets (Docker Hub username/password or token
referenced by the workflow) to use the organization account credentials so the
publish step continues to authenticate and push successfully.
| tags: | | ||
| type=raw,value=test,enable={{is_default_branch}} | ||
| type=ref,event=tag | ||
| type=sha,prefix=sha- |
There was a problem hiding this comment.
Tag strategy may not work as intended.
The tag configuration uses type=raw,value=test,enable={{is_default_branch}}, which will only tag images as "test" when building on the default branch (typically main). However, the workflow currently triggers on the docker-publish branch, so {{is_default_branch}} will be false and the "test" tag won't be applied.
If you intend to tag builds from a specific branch as "test", update the tag condition or the workflow triggers to align.
🏷️ Alternative tag strategies
Option 1: Match the branch name directly
tags: |
- type=raw,value=test,enable={{is_default_branch}}
+ type=raw,value=test,enable=${{ github.ref == 'refs/heads/main' }}
type=ref,event=tag
type=sha,prefix=sha-Option 2: Use branch name as tag
tags: |
- type=raw,value=test,enable={{is_default_branch}}
+ type=ref,event=branch
type=ref,event=tag
type=sha,prefix=sha-📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| tags: | | |
| type=raw,value=test,enable={{is_default_branch}} | |
| type=ref,event=tag | |
| type=sha,prefix=sha- | |
| tags: | | |
| type=raw,value=test,enable=${{ github.ref == 'refs/heads/main' }} | |
| type=ref,event=tag | |
| type=sha,prefix=sha- |
| tags: | | |
| type=raw,value=test,enable={{is_default_branch}} | |
| type=ref,event=tag | |
| type=sha,prefix=sha- | |
| tags: | | |
| type=ref,event=branch | |
| type=ref,event=tag | |
| type=sha,prefix=sha- |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/docker-publish.yml around lines 44 - 47, The current tag
entry "type=raw,value=test,enable={{is_default_branch}}" won't apply on the
docker-publish branch; update the tags section so the "test" tag is enabled when
building from the intended branch (either change the enable condition from
is_default_branch to a branch-name check for "docker-publish" or other target
branch, or change the workflow trigger to run on the default branch), e.g.,
modify the enable expression for the tag in the tags block (the line referencing
type=raw,value=test,enable={{is_default_branch}}) or adjust the workflow
triggers so they align.
| @@ -4,7 +4,7 @@ services: | |||
| image: tatevikg1/phplist:test | |||
There was a problem hiding this comment.
Use organization Docker Hub account for consistency.
Similar to the workflow configuration, this image reference uses a personal Docker Hub account (tatevikg1/phplist:test). For consistency and proper ownership, this should reference the organization account.
Suggested change
app:
build: .
- image: tatevikg1/phplist:test
+ image: phplist/base-distribution:test
container_name: base-distribution-app📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| image: tatevikg1/phplist:test | |
| app: | |
| build: . | |
| image: phplist/base-distribution:test | |
| container_name: base-distribution-app |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docker-compose.yml` at line 4, Replace the personal Docker Hub image
reference "image: tatevikg1/phplist:test" with the organization's Docker Hub
repository (e.g., "image: <ORG_USERNAME>/phplist:test"); update the image value
in the docker-compose service where "image: tatevikg1/phplist:test" appears so
it points to the organization account and tag used by your CI/workflows.
Summary
Thanks for contributing to phpList!
Summary by CodeRabbit