Updated the image for PHP 8.3 with a few added extensions, plus a GH actions build #1
Conversation
WalkthroughThe changes introduce a new GitHub Actions workflow file named Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GitHub Actions
participant QEMU
participant Buildah
participant Quay.io
User->>GitHub Actions: Push code
GitHub Actions->>GitHub Actions: Checkout code
GitHub Actions->>QEMU: Install qemu-user-static
GitHub Actions->>Buildah: Build container image (linux/amd64, linux/arm64)
Buildah-->>GitHub Actions: Image built
GitHub Actions->>Quay.io: Push image (if on master)
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (1)
.github/workflows/multiarch-build.yaml (1)
3-3: Optimize workflow triggersThe current configuration runs on all push events, which may unnecessarily consume CI minutes. Consider limiting the workflow to specific branches or paths.
-on: [push] +on: + push: + branches: + - 'master' + - 'release/**' + paths: + - 'Containerfile' + - '.github/workflows/multiarch-build.yaml'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/multiarch-build.yaml(1 hunks)
🧰 Additional context used
🪛 actionlint
.github/workflows/multiarch-build.yaml
10-10: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
29-29: property "base" is not defined in object type {action: string; action_path: string; action_ref: string; action_repository: string; action_status: string; actor: string; actor_id: string; api_url: string; base_ref: string; env: string; event: object; event_name: string; event_path: string; graphql_url: string; head_ref: string; job: string; job_workflow_sha: string; path: string; ref: string; ref_name: string; ref_protected: string; ref_type: string; repository: string; repository_id: string; repository_owner: string; repository_owner_id: string; repositoryurl: string; retention_days: number; run_attempt: string; run_id: string; run_number: string; secret_source: string; server_url: string; sha: string; token: string; triggering_actor: string; workflow: string; workflow_ref: string; workflow_sha: string; workspace: string}
(expression)
🔇 Additional comments (1)
.github/workflows/multiarch-build.yaml (1)
12-15: LGTM!
The QEMU setup is correctly implemented for multi-architecture build support.
cardil
force-pushed
the
feature/gh-action-build
branch
from
a588534 to
b0dc09d
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (6)
.github/workflows/multiarch-build.yaml (3)
1-3: Consider refining the workflow triggersThe current trigger configuration runs on all push events, which might be unnecessary. Consider limiting the workflow to:
- Specific branches (e.g., main, develop)
- Relevant file paths (e.g., Containerfile, deployment configs)
name: Multiarch build -on: [push] +on: + push: + branches: [ master, develop ] + paths: + - 'Containerfile' + - '.github/workflows/multiarch-build.yaml' + - 'docker/**'
26-35: Add error handling and push verificationWhile the push configuration is correct, consider adding:
- Error handling for failed pushes
- Post-push verification step
- name: Push To quay.io id: push-to-quay uses: redhat-actions/push-to-registry@v2 if: github.event.name == 'push' && github.ref == 'refs/heads/master' with: image: ${{ steps.build-image.outputs.image }} tags: ${{ steps.build-image.outputs.tags }} registry: quay.io/wavesoftware username: ${{ secrets.QUAY_USER }} password: ${{ secrets.QUAY_PASSWORD }} + + - name: Verify Push + if: success() && github.event.name == 'push' && github.ref == 'refs/heads/master' + run: | + # Wait for image to be available + sleep 30 + # Verify image exists in registry + curl --fail -H "Authorization: Bearer ${{ secrets.QUAY_PASSWORD }}" \ + "https://quay.io/api/v1/repository/wavesoftware/${{ steps.build-image.outputs.image }}/tag/${{ steps.build-image.outputs.tags }}"
6-8: Add timeout configuration to prevent hanging buildsConsider adding a timeout to the job to prevent excessive resource usage from hanging builds.
multiarch-build: name: Build images runs-on: ubuntu-22.04 + timeout-minutes: 60Containerfile (3)
57-58: Track the imagick version updateThe TODO comment indicates a pending issue that needs to be resolved.
Would you like me to create a GitHub issue to track the imagick version update once issue Imagick/imagick#640 is resolved?
59-62: Consider pinning extension versionsExtensions installed without version pins could lead to non-deterministic builds:
- apcu
- memcached
- redis
- mcrypt
Consider pinning versions like:
- apcu \ - memcached \ - redis \ - mcrypt + apcu-5.1.22 \ + memcached-3.2.0 \ + redis-5.3.7 \ + mcrypt-1.0.6
64-73: Review memory limit for containerized environmentThe memory limit of 512M might be high for a containerized environment. Consider making this configurable via environment variables to allow for different deployment scenarios.
Consider this pattern:
- echo 'memory_limit=512M' > /usr/local/etc/php/conf.d/memory-limit.ini + echo "memory_limit=${PHP_MEMORY_LIMIT:-128M}" > /usr/local/etc/php/conf.d/memory-limit.ini
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
.editorconfig(1 hunks).github/workflows/multiarch-build.yaml(1 hunks)Containerfile(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .editorconfig
🔇 Additional comments (3)
.github/workflows/multiarch-build.yaml (2)
5-8: Add explicit permissions configuration
For better security, explicitly define the required permissions for the workflow.
17-24: Image name 'php' should be more specific to avoid conflicts
Containerfile (1)
52-54: 🛠️ Refactor suggestion
Add checksum verification for downloaded installer
While using a specific version is good, adding checksum verification would improve security.
Apply this pattern:
+ADD --checksum=sha256:expected_checksum \
--chmod=0755 \
https://github.com/mlocati/docker-php-extension-installer/releases/download/2.6.3/install-php-extensions \
/usr/local/bin/
cardil
force-pushed
the
feature/gh-action-build
branch
from
b0dc09d to
09c7748
Compare
cardil
force-pushed
the
feature/gh-action-build
branch
from
09c7748 to
5979851
Compare
cardil
changed the title
| gmp \ | ||
| iconv \ | ||
| # TODO: Use latest released version, after https://github.com/Imagick/imagick/issues/640 is fixed | ||
| imagick/imagick@28f27044e435a2b203e32675e942eb8de620ee58 \ |
There was a problem hiding this comment.
Unpin this when Imagick/imagick#640 is fixed
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
.github/workflows/multiarch-build.yaml (1)
3-10: Consider optimizing the workflow triggersThe workflow currently builds on every branch push, which might consume unnecessary resources. Consider limiting it to specific branches or paths that affect the container build.
on: push: branches: - - '**' + - 'master' + - 'feature/**' + paths: + - 'Containerfile' + - '.github/workflows/multiarch-build.yaml' schedule: # Every month's 2nd on 5:30 - cron: '30 5 2 * *'🧰 Tools
🪛 yamllint
[error] 3-3: trailing spaces
(trailing-spaces)
[warning] 9-9: wrong indentation: expected 4 but found 2
(indentation)
Containerfile (1)
36-46: Consider environment-specific memory settingsThe memory settings look reasonable, but consider:
- Making the memory limit configurable via environment variables for different environments
- Documenting the rationale for the opcache settings
Example improvement:
- echo 'memory_limit=512M' > /usr/local/etc/php/conf.d/memory-limit.ini + echo "memory_limit=${PHP_MEMORY_LIMIT:-512M}" > /usr/local/etc/php/conf.d/memory-limit.ini
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
.editorconfig(1 hunks).github/workflows/multiarch-build.yaml(1 hunks)Containerfile(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .editorconfig
🧰 Additional context used
📓 Learnings (1)
.github/workflows/multiarch-build.yaml (1)
Learnt from: cardil
PR: wavesoftware/container-php#1
File: .github/workflows/multiarch-build.yaml:23-30
Timestamp: 2024-11-12T11:11:57.864Z
Learning: In the `.github/workflows/multiarch-build.yaml` file, the image name should remain as 'php' even if it's a custom image.
🪛 yamllint
.github/workflows/multiarch-build.yaml
[error] 3-3: trailing spaces
(trailing-spaces)
[warning] 9-9: wrong indentation: expected 4 but found 2
(indentation)
🔇 Additional comments (5)
.github/workflows/multiarch-build.yaml (3)
11-14: LGTM! Job configuration is appropriate
The Ubuntu 22.04 runner is a good choice for container builds, providing a stable and well-supported environment.
23-30: Verify multi-arch build configuration
The build configuration looks good, with proper platform support for amd64 and arm64. Let's verify the Containerfile compatibility with multi-arch builds.
#!/bin/bash
# Check if Containerfile has any architecture-specific commands
echo "Checking Containerfile for architecture-specific commands..."
rg -i "arch|amd64|x86_64|arm64|aarch64" Containerfile
# Check for potential multi-arch issues in base images
echo "Checking base image references..."
rg "^FROM" Containerfile32-41: LGTM! Secure registry push configuration
The push configuration is well-structured with:
- Proper condition to push only from master branch
- Secure handling of registry credentials using secrets
- Correct reference to build step outputs
Containerfile (2)
4-6: LGTM! Good use of the PHP extension installer
Using a dedicated extension installer with a pinned version improves reliability and simplifies the installation process.
8-34: Verify PHP 8.x compatibility for all extensions
The extensive list of extensions looks good, but please verify that all extensions are fully compatible with PHP 8.x, especially:
- mcrypt (which was removed from PHP core)
- imagick (currently pinned to a specific commit)
✅ Verification successful
[/run_scripts]
mcrypt is not used in the codebase and can be removed
No usages or dependencies on mcrypt were found in the codebase. It is safe to remove mcrypt from the extensions list.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check extension compatibility with PHP 8.x
# Note: This is a basic check that could help identify potential issues
# Search for any reported compatibility issues
echo "Searching for PHP 8.x compatibility issues in extension documentation and issues..."
rg -i "php\s+8|compatibility" . -g "README*" -g "CHANGELOG*"
# Look for version constraints in composer files that might indicate compatibility
echo "Checking for version constraints in composer files..."
find . -name "composer.json" -exec grep -l "php" {} \;
Length of output: 412
Script:
#!/bin/bash
# Description: Identify usages and dependencies of mcrypt in the codebase
echo "Searching for usages of mcrypt in the codebase..."
rg -i "mcrypt" --type-add 'php:*.php' --type php .
echo "Checking Composer dependencies for mcrypt requirement..."
rg -i "mcrypt" composer.json
Length of output: 365
Summary by CodeRabbit
New Features
linux/amd64andlinux/arm64platforms..editorconfigfile to enforce coding style guidelines.Improvements
Containerfile, streamlining and optimizing the build process.