Skip to content

Update Dockerfile#4

Open
obi12341 wants to merge 1 commit intomasterfrom
claude-code-pr-review
Open

Update Dockerfile#4
obi12341 wants to merge 1 commit intomasterfrom
claude-code-pr-review

Conversation

@obi12341
Copy link
Owner

No description provided.

Comment on lines +28 to +29
EXPOSE 22
EXPOSE 8080
Copy link

Choose a reason for hiding this comment

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

🔴 EXPOSE 8080 is added but no service in this container listens on port 8080 — only sshd on port 22. This misleads users and orchestration tools (docker-compose -P, Kubernetes) that inspect exposed ports. Remove the EXPOSE 8080 line, and also fix the trailing whitespace introduced on the EXPOSE 22 line.

Extended reasoning...

What the bug is

This PR adds EXPOSE 8080 to the Dockerfile, but there is no service configured to listen on port 8080. The container is an SFTP server that exclusively runs sshd on port 22.

Proof by tracing the code

  1. sshd_config line 1: Port 22 — sshd only listens on port 22.
  2. start.sh line 97: exec /usr/sbin/sshd -D -e — the only process started is sshd. No other daemon, web server, or proxy is launched.
  3. Dockerfile RUN: apk add only installs openssh-server, openssh-server-pam, openssh-sftp-server, shadow, and perl. No web server (nginx, Apache, etc.) or other network service is installed.
  4. Conclusion: Nothing in this image will ever bind to port 8080.

Impact

The EXPOSE directive is metadata that documents which ports the container listens on. Tools and users rely on this:

  • docker run -P will create a host-to-container mapping for port 8080 that goes nowhere.
  • Docker Compose and Kubernetes service discovery inspect exposed ports, leading to incorrect routing configuration.
  • Users reading the Dockerfile will incorrectly assume a service runs on port 8080.

Additional issue: trailing whitespace on EXPOSE 22

The diff also shows the original EXPOSE 22 was changed to EXPOSE 22 (with a trailing space). While this has no functional impact on Docker, it is a whitespace regression that will be flagged by linters and is generally bad practice.

Fix

Remove the EXPOSE 8080 line entirely, and restore EXPOSE 22 without the trailing space:

EXPOSE 22

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant