smebberson / docker-alpine

Docker containers running Alpine Linux and s6 for process management. Solid, reliable containers.
MIT License
596 stars 187 forks source link

Base images should rarely expose ports #68

Closed rbellamy closed 7 years ago

rbellamy commented 7 years ago

Given that it is unlikely that the alpine-nginx image would be deployed as-is, it should not expose ports. Instead that should be left to the image that uses alpine-nginx as its base.

This is especially true because, unlike CMD or ENTRYPOINT a derived image cannot override the EXPOSE command of a base image. This means that should a derived image want nginx to not expose ports 80 or 443, they cannot use this as a base image.

Also, nginx logs should always be redirected to stdout and stderr, and therefore the commands to make this happen should remain the responsibility of alpine-nginx base image.

smebberson commented 7 years ago

@rbellamy, yeah I agree on the ports issue. Let me think about the logs issue a little more...

rbellamy commented 7 years ago

@smebberson I'd be interested in hearing a use-case where you would NOT want the nginx logs forwarded to stdout or stderr in a running Docker image.

smebberson commented 7 years ago

@rbellamy, I run multiple instances of alpine-nginx and alpine-nodejs and therefore anything going to stdout and stderr is cumbersome to get to. I push all logs centrally to a remote log aggregator and I can view everything that happened from there.

Much nicer than trying to get stdout for a particular container, when you're running many instances.

A quick Google search gives you plenty of mixed opinions on this matter. I would say the majority log to stdout and stderr while pushing data out to aggregators.

I mean, I guess all of the other services here do log to stdout and stderr as a default. s6-overlay has some logging utilities too https://github.com/just-containers/s6-overlay#logging

rbellamy commented 7 years ago

From my admittedly limited experience and reading, the "best practice" for log aggregation seems to be to offer that as a service within kubernetes, not within the docker image itself (e.g. https://github.com/deis/fluentd).

While I (obviously) understand the need and use of the s6-overlay within the docker image, I think it's important to guard against a leaky abstraction, and not fall into the trap of expecting more from the docker image than is absolutely necessary. My 2¢.

rbellamy commented 7 years ago

I'm considering the best way to allow a downstream image to modify the nginx config - and debating with myself whether overwriting either /etc/nginx/nginx.conf or /etc/nginx/conf.d/default.conf is best, or some kind of structural solution with included confs (like the include /etc/nginx/conf.d/*.conf; stanza in nginx.conf).

rbellamy commented 7 years ago

An example of the use this could be put to is removing the main logging stanza from the nginx.conf and putting a standard one in the conf.d directory. This would make a "sane" choice for the upstream image (pending the result of our current discussion) but would also allow a downstream image to alter nginx logging behavior.

I see a similar need in configuring the server section.

rbellamy commented 7 years ago

One last thing and I'll stop spamming you - in order to get the http_realip_module module we'll need the newest version - our current image doesn't have it. That module is pretty much required for nginx to properly resolve real vs proxied IP addresses - something pretty important for containers that are running behind proxies (probably almost all?).

smebberson commented 7 years ago

I'm closing this, because I think the PR is a little premature and more discussion needs to take place. I've created https://github.com/smebberson/docker-alpine/issues/81 and https://github.com/smebberson/docker-alpine/issues/82 to continue the discussion there.