Skip to content

Optimize dockerfile#10947

Merged
ItzNotABug merged 2 commits into1.8.xfrom
optimize-dockerfile
Dec 13, 2025
Merged

Optimize dockerfile#10947
ItzNotABug merged 2 commits into1.8.xfrom
optimize-dockerfile

Conversation

@ItzNotABug
Copy link
Copy Markdown
Contributor

@ItzNotABug ItzNotABug commented Dec 13, 2025

What does this PR do?

Remove unneeded files from final prod image.

Test Plan

Manual.

Related PRs and Issues

N/A.

Checklist

  • Have you read the Contributing Guidelines on issues?
  • If the PR includes a change to an API's metadata (desc, label, params, etc.), does it also include updated API specs and example docs?

@ItzNotABug ItzNotABug self-assigned this Dec 13, 2025
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Dec 13, 2025 •
edited
Loading

📝 Walkthrough

Walkthrough

The changes convert the Docker build strategy to explicit multi-stage targets and adjust CI workflows to select those targets. The Dockerfile was split into base, production, and development stages (moving dev-only files and debug tooling into the development stage and consolidating cleanup into production). CI workflows (.github/workflows/publish.yml and release.yml) now pass target: production to the build step; release.yml also upgrades docker/setup-qemu-action and docker/setup-buildx-action from v2 to v3. docker-compose.yml sets the app build target to development.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Review Dockerfile stage boundaries: confirm files, COPYs, ports, and CMDs are correct for base/production/development and that production cleanup doesn't remove required runtime artifacts.
  • Verify workflow changes: target: production usage and docker action upgrades (v2 → v3) for behavioral differences or changed inputs.
  • Check docker-compose build.target: ensure development-only artifacts and debug tooling are available as expected during local builds.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Optimize dockerfile' accurately summarizes the main change: refactoring the Dockerfile to remove unneeded files from the production image through multi-stage build optimization.
Description check ✅ Passed The description is directly related to the changeset, clearly stating the goal of removing unneeded files from the final production image, which aligns with the Dockerfile and workflow changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch optimize-dockerfile
📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 45d5f14 and f3f3ac8.

📒 Files selected for processing (2)
  • Dockerfile (2 hunks)
  • docker-compose.yml (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-27T13:59:13.723Z 
Learnt from: ChiragAgg5k
Repo: appwrite/appwrite PR: 10394
File: src/Appwrite/Platform/Modules/Databases/Http/TablesDB/Tables/Get.php:39-39
Timestamp: 2025-08-27T13:59:13.723Z
Learning: The documentation files under docs/references/tablesdb/ (create-table.md, delete-table.md, get-table.md, update-table.md, list-tables.md) exist in the Appwrite repository and should not be flagged as missing.

Applied to files:

  • Dockerfile
⏰ 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). (19)
  • GitHub Check: E2E Service Test (Webhooks)
  • GitHub Check: E2E Service Test (Locale)
  • GitHub Check: E2E Service Test (Messaging)
  • GitHub Check: E2E Service Test (Realtime)
  • GitHub Check: E2E Service Test (VCS)
  • GitHub Check: E2E Service Test (Databases/Legacy)
  • GitHub Check: E2E Service Test (FunctionsSchedule)
  • GitHub Check: E2E Service Test (Teams)
  • GitHub Check: E2E Service Test (Sites)
  • GitHub Check: E2E Service Test (Storage)
  • GitHub Check: E2E Service Test (Avatars)
  • GitHub Check: E2E Service Test (Health)
  • GitHub Check: E2E Service Test (Console)
  • GitHub Check: E2E Service Test (Functions)
  • GitHub Check: Unit Test
  • GitHub Check: E2E General Test
  • GitHub Check: E2E Service Test (Site Screenshots)
  • GitHub Check: E2E Service Test (Dev Keys)
  • GitHub Check: scan
🔇 Additional comments (4)
Dockerfile (3)

15-91: LGTM: Base stage properly establishes shared foundation.

The rename from "final" to "base" improves clarity for a multi-stage build. The base stage correctly consolidates common setup (dependencies, source code, storage directories, executable permissions) that both production and development need. No concerns here.

93-103: LGTM: Production stage cleanup is safe and well-structured.

The production stage appropriately removes development and debug artifacts:

  • Specs removal confirmed safe (verified in prior review: SDK generation is CLI-only, tests use development image)
  • xdebug.so removal correct for production
  • Python bytecode and static library cleanup reduces image bloat without affecting runtime

The conditional execution (|| true) gracefully handles cases where files don't exist. Stage structure with EXPOSE and CMD is clean.

105-118: LGTM: Development stage correctly includes dev-only resources.

The development stage properly copies docs and dev directories from source (not included in base), and conditionally enables xdebug when DEBUG=true. This maintains a complete environment for local development and testing.

One minor note: the xdebug path on line 96 (no-debug-non-zts-20240924) is PHP-version-specific. This is safe as long as the base image version (appwrite/base:0.10.6) remains pinned; if the base image updates PHP versions, this path may need adjustment.

docker-compose.yml (1)

55-61: LGTM: Development target correctly selected for local development.

Specifying target: development ensures docker-compose builds the development stage (which includes docs and dev directories). This aligns with the multi-stage Dockerfile refactor and distinguishes local development builds from production CI builds (which use target: production).

The fact that docker-compose also mounts these directories as volumes (lines 94–98) is redundant but harmless and provides flexibility for rapid code iteration.

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.

❤️ Share

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

@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 13, 2025 •
edited
Loading

Security Scan Results for PR

Docker Image Scan Results

Package Version Vulnerability Severity
libpng 1.6.51-r0 CVE-2025-66293 HIGH
libpng-dev 1.6.51-r0 CVE-2025-66293 HIGH

Source Code Scan Results

🎉 No vulnerabilities found!

@ItzNotABug ItzNotABug marked this pull request as ready for review December 13, 2025 12:24
coderabbitai[bot]
coderabbitai bot reviewed Dec 13, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@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: 1

🧹 Nitpick comments (1)
Dockerfile (1)

101-103: Consider consolidating EXPOSE and CMD into the base stage.

Both production and development stages declare identical EXPOSE 80 and CMD [ "php", "app/http.php" ] directives. Since both stages use the same runtime behavior, consolidating these into the base stage would reduce duplication.

This is an optional refactor—the current approach explicitly documents what each stage exposes and runs, which can aid clarity.

If you choose to consolidate, move lines 101 and 103 (or 114 and 116) into the base stage after line 91:

 RUN mkdir -p /etc/letsencrypt/live/ && chmod -Rf 755 /etc/letsencrypt/live/

+EXPOSE 80
+
+CMD [ "php", "app/http.php" ]
+
 FROM base AS production
 
 RUN rm -rf /usr/src/code/app/config/specs && \

Then remove lines 101–103 from production and lines 114–116 from development.

Also applies to: 114-116

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8e434b7 and 45d5f14.

📒 Files selected for processing (3)
  • .github/workflows/publish.yml (1 hunks)
  • .github/workflows/release.yml (2 hunks)
  • Dockerfile (2 hunks)
⏰ 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). (12)
  • GitHub Check: Benchmark
  • GitHub Check: E2E Service Test (Databases/Legacy)
  • GitHub Check: E2E Service Test (Functions)
  • GitHub Check: E2E Service Test (FunctionsSchedule)
  • GitHub Check: E2E Service Test (Databases/TablesDB)
  • GitHub Check: E2E Service Test (Sites)
  • GitHub Check: E2E Service Test (Migrations)
  • GitHub Check: E2E Service Test (GraphQL)
  • GitHub Check: E2E Service Test (Messaging)
  • GitHub Check: E2E Service Test (Proxy)
  • GitHub Check: E2E Service Test (Realtime)
  • GitHub Check: E2E General Test
🔇 Additional comments (4)
.github/workflows/publish.yml (1)

45-45: Production stage explicitly targeted. Clean integration with multi-stage Dockerfile refactoring.

Dockerfile (2)

97-97: Verify that removing static .a library files is safe.

While static libraries are typically linked at compile time and not needed at runtime, confirm that no runtime dependency loading or plugin systems reference .a files.

Static library files (.a) are generally not loaded at runtime in compiled applications. However, verify this assumption for your use case by checking if any:

  • Runtime plugin systems load static archives
  • Foreign function interfaces (FFI) dynamically reference .a files
  • Custom build processes depend on these artifacts

If you've tested the production image and confirmed functionality, this cleanup is safe.

93-116: Multi-stage refactoring is well structured.

The separation of production and development stages is clean:

  • Production removes unneeded artifacts (specs, xdebug, static libs, Python caches) for a lean image.
  • Development stage includes docs, dev folder, and debugging tooling for enhanced developer experience.
  • Base stage properly contains shared setup without development-specific files.

The cleanup operations are sound and properly handle edge cases with 2>/dev/null || true.

.github/workflows/release.yml (1)

23-23: Docker action upgrades to v3 and production target are current and well integrated.

The upgrades to docker/setup-qemu-action@v3 (latest v3.7.0) and docker/setup-buildx-action@v3 (latest v3.11.1) are aligned with current stable versions. The addition of target: production on line 49 ensures release images use the optimized production stage from the Dockerfile.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 13, 2025

✨ Benchmark results

  • Requests per second: 1,183
  • Requests with 200 status code: 212,915
  • P99 latency: 0.167082847

⚡ Benchmark Comparison

Metric This PR Latest version
RPS 1,183 1,228
200 212,915 221,059
P99 0.167082847 0.167548272

@ItzNotABug ItzNotABug merged commit 7d2a95c into 1.8.x Dec 13, 2025
41 checks passed
@ItzNotABug ItzNotABug deleted the optimize-dockerfile branch December 13, 2025 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@loks0n loks0n loks0n approved these changes

+1 more reviewer

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@ItzNotABug ItzNotABug

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ItzNotABug @loks0n