Skip to content

Docker file for dhcp and http services to be run from monax.#321

Open
thesrinath wants to merge 3 commits into
openconfig:mainfrom
thesrinath:inttest
Open

Docker file for dhcp and http services to be run from monax.#321
thesrinath wants to merge 3 commits into
openconfig:mainfrom
thesrinath:inttest

Conversation

@thesrinath
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces Dockerfiles for the DHCP and HTTP services using multi-stage builds and distroless base images. The reviewer recommends improving the security of the HTTP service by running it as a non-root user and binding to a non-privileged port instead of running as root to bind to port 80.

Comment thread http/Dockerfile
Comment on lines +11 to +13
# HTTP server might need to run as root to bind to port 80.
USER root
ENTRYPOINT ["/bootz-http"]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Running the HTTP server as root is generally discouraged for security reasons. It is recommended to run as a non-privileged user (like the nonroot user provided by distroless) and bind to a non-privileged port (e.g., 8080). The port mapping to 80 can be handled by the container runtime or orchestrator.

# HTTP server should run as non-root for better security.
USER nonroot
ENTRYPOINT ["/bootz-http"]
CMD ["-address", ":8080"]

@Chounoki
Copy link
Copy Markdown
Contributor

Since these files are exclusively for monax, maybe we can create a root directory like "//monax" or "//test", and just like the directory structure shown in the monax example, we can put them under "//monax/deploy" or "//test/deploy".

What is your opinion?

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.

2 participants