Skip to content

Conversation

hopeseekr
Copy link
Member

@hopeseekr hopeseekr commented Sep 27, 2025

Summary by CodeRabbit

  • New Features
    • Leaner distroless and web images with expanded essential tools; improved startup via direct PHP-FPM and Nginx management.
  • Refactor
    • Switched base to Ubuntu variant across images; adjusted image tagging and build flow.
    • Build matrix narrowed to PHP 8.4.
    • Default starting port changed to 8000 for initial image.
    • Oracle-related builds disabled/commented out.
  • Documentation
    • Updated supported PHP versions in README.

Copy link

coderabbitai bot commented Sep 27, 2025

Walkthrough

The PR switches base images from phpexperts/php to phpexperts/php-ubuntu across Dockerfiles, expands distroless file collection, restructures image build scripts to focus on PHP 8.4 with added distroless/web asset handling, replaces supervisord with a bash-based entrypoint for web, updates initial port selection logic, and refreshes README PHP version details.

Changes

Cohort / File(s) Summary of changes
Documentation
README.md
Updated supported PHP versions/build dates (8.3.25, newer 8.4.11 build date).
Build orchestration
docker/build-images.sh
Narrowed build to PHP 8.4; reworked tagging to phpexperts/php-ubuntu:${VERSION}; added distroless step; introduced per-version web .build-assets; commented out Oracle flows; adjusted latest tags; preserved IonCube skip logic; cleanup of web build assets.
Distroless build flow
docker/build-distroless.sh
On success: retag full image to phpexperts/php-full:${VERSION} before removing phpexperts/php:${VERSION}; keep tagging distroless as main version.
Base image switch (core Dockerfiles)
docker/images/base-debug/Dockerfile, docker/images/base-full/Dockerfile, docker/images/base-ioncube/Dockerfile, docker/images/base-oracle/Dockerfile
Changed FROM to phpexperts/php-ubuntu:$PHP_VERSION. No other steps modified.
Distroless image internals
docker/images/distroless/Dockerfile, docker/images/distroless/grab_files.sh
FROM switched to php-ubuntu; expanded file grabbing to include core utils and shells; grab script now creates /usr/sbin and copies /usr/bin/sh.
Web image and runtime
docker/images/web/Dockerfile, docker/images/web/entrypoint.sh, docker/images/web/grab_files.sh
Introduced intermediate stage (phpexperts/linux:latest); final FROM remains PHP image; moved to apt/packaging adjustments; added distroless prep with grab-files; copied web assets; removed supervisor; new entrypoint manages php-fpm and nginx directly; new grab script to assemble minimal filesystem with deps.
Installer
install.php
Initial port selection now uses findFirstAvailablePort(8000) instead of 80; subsequent port logic unchanged.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Dev as Developer
  participant Build as build-images.sh
  participant Base as Base Dockerfiles
  participant Dist as Distroless Builder
  participant Web as Web Dockerfile

  Dev->>Build: Invoke build for PHP 8.4
  Build->>Base: Build base images FROM phpexperts/php-ubuntu:8.4
  Build->>Dist: Build distroless image
  Dist-->>Build: Tag distroless as main<br/>(retag full to php-full, remove old tag)
  Build->>Web: Prepare .build-assets and build web image
  Web-->>Build: Copy grabbed files and assets into final image
  Build-->>Dev: Push/tag images per new scheme
  note over Build,Dist: Oracle paths skipped/commented
Loading
sequenceDiagram
  autonumber
  participant User as Container Runtime
  participant Entry as web/entrypoint.sh
  participant FPM as php-fpm
  participant Nginx as nginx

  User->>Entry: Start container
  Entry->>FPM: Launch php-fpm (bg)
  Entry->>Nginx: Launch nginx (bg)
  Entry->>Entry: Trap SIGTERM/SIGINT for cleanup
  Entry-->>User: Wait for first child to exit
  alt Termination
    User->>Entry: SIGTERM/SIGINT
    Entry->>FPM: kill
    Entry->>Nginx: kill
    Entry-->>User: Exit with child status
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

A bunny builds with careful hops,
From php to ubuntu stops.
Distroless packs a lighter load,
Web now hums sans supervisor mode.
Ports shift up, the scripts align—
Ship the carrots, all runs fine. 🥕🚀

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly describes the primary focus of the changeset—refactoring to use distroless images—without extraneous information. It is concise and specific enough to convey the main objective to teammates scanning history.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch distroless_refactor

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c481de9 and e05efee.

📒 Files selected for processing (13)
  • README.md (1 hunks)
  • docker/build-distroless.sh (1 hunks)
  • docker/build-images.sh (3 hunks)
  • docker/images/base-debug/Dockerfile (1 hunks)
  • docker/images/base-full/Dockerfile (1 hunks)
  • docker/images/base-ioncube/Dockerfile (1 hunks)
  • docker/images/base-oracle/Dockerfile (1 hunks)
  • docker/images/distroless/Dockerfile (1 hunks)
  • docker/images/distroless/grab_files.sh (2 hunks)
  • docker/images/web/Dockerfile (2 hunks)
  • docker/images/web/entrypoint.sh (2 hunks)
  • docker/images/web/grab_files.sh (1 hunks)
  • install.php (1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.11.0)
docker/images/web/grab_files.sh

[warning] 30-30: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.

(SC2164)


[warning] 31-31: Quote this to prevent word splitting.

(SC2046)


[warning] 48-48: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.

(SC2164)


[warning] 73-73: For loops over find output are fragile. Use find -exec or a while read loop.

(SC2044)


[warning] 74-74: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.

(SC2164)


[warning] 75-75: Quote this to prevent word splitting.

(SC2046)


[warning] 83-83: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.

(SC2164)


[warning] 84-84: Quote this to prevent word splitting.

(SC2046)

🪛 Hadolint (2.14.0)
docker/images/web/Dockerfile

[error] 56-56: Use COPY instead of ADD for files and folders

(DL3020)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant