Skip to content

Conversation

@tianon
Copy link
Contributor

@tianon tianon commented Sep 17, 2014

This is the PR I mentioned I was working up on docker-library/official-images#197.

Overall, this is a really simple Dockerfile, and I totally dig it. The hack with just making symlinks for the access log and error log is perfect!

I've tried to split this out into logical chunks with my commits so it's easier to review each change individually if you prefer. I'm also happy to amend or make separate PRs if that's preferable. The one that's the most important is d5b14ea, but I'm copying each commit description here to hopefully make them easier to read over.

  • e683580 Update "apt-key" usage to verify the fingerprint

    This adds a little extra verification to make the build fail outright if the build server has been the subject of a MITM attack.

  • 27ced27 Use same symlink logic for stderr as stdout

    This simplifies the default command by taking advantage of the default configuration just like the precedent set for access.log.

  • 305e286 Simplify default command via PATH and default conf

    This makes it simpler for users to supply custom arguments at runtime, since they essentially have to duplicate this default command line to do so.

  • d5b14ea Add explicit version pinning, for cache-busting

    Because Docker uses the exact textual contents of the line for the purposes of the build cache, just doing apt-get install -y nginx would never actually update unless the base image itself updates.

    This ENV XYZ_VERSION a.b.c pattern is one we've had a lot of success with in other docker-library repos (see especially https://github.com/docker-library/postgres for a good example of how this allows us to magnify maintenance ability to span quite a few versions of PostgreSQL concurrently), and most importantly it creates an explicit, context-sensitive cache-bust right where it's needed.

Also, if you'd be interested in maintaining "stable" versions too, or even just in having this version number easy to update (see update.sh and generate-stackbrew-library.sh in the postgres repo for examples of what I mean), I'd be happy to send a PR setting that kind of thing up so you could have both "1.6" and "1.7" easily, and keep them both updated easily as well. We're happy to be as involved in that as you'd like us to be.

This adds a little extra verification to make the build fail outright if the build server has been the subject of a MITM attack.
This simplifies the default command by taking advantage of the default configuration just like the precedent set for access.log.
This makes it simpler for users to supply custom arguments at runtime, since they essentially have to duplicate this default command line to do so.
Because Docker uses the exact textual contents of the line for the purposes of the build cache, just doing `apt-get install -y nginx` would never actually update unless the base image itself updates.

This `ENV XYZ_VERSION a.b.c` pattern is one we've had a lot of success with in other docker-library repos (see especially https://github.com/docker-library/postgres for a good example of how this allows us to magnify maintenance ability to span quite a few versions of PostgreSQL concurrently), and most importantly it creates an explicit, context-sensitive cache-bust right where it's needed.
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