Conversation
gquintard
force-pushed
the
new_hitch
branch
4 times, most recently
from
20d518f to
51b80cf
Compare
|
The updated docs PR can be found here: docker-library/docs#1675 |
|
Happy Thanksgiving! I just pushed an update to this PR as we have released |
|
FWIW, you didn't do yourself any favors by opening a new PR -- the conversation is now split (having to refer back to that previous thread during my review), and you put yourself back at the bottom of the new-image queue. 😅 ❤️ I think some of my comments from #7611 (comment) are still relevant, but I'll try to recapture the salient bits here. + cp /etc/hitch/testcert.pem /etc/hitch/certs/default
(The docs mention this is a self-signed certificate, but I think more details would probably still be helpful, especially since some systems like Mac OS are picky about the validity period of even self-signed certs.) What user/UID does this end up running as? Does it matter for that to be consistent? (Is there any persistent storage for things like session/cache that might be affected if it were to change underneath a user?)
|
Dagnabit! Sorry, I didn't realize that.
@ThijsFeryn, would you mind updating the docs to match that we now use the packaged default file, and tell us a bit more about it?
Without user configuration, the My instinct here is that we don't need the |
|
@tianon, @ThijsFeryn updated the docs PR to explain the test certificate: https://github.com/docker-library/docs/pull/1675/files#diff-001e280f2a3c2c5174bd4d733268cc2f2b4478f3baa6c5fc09ed6c5b7ee3d20cR43-R53 That should address all your concerns, right? |
|
Hi team, anything you need us to do here? |
|
Generally, I think this looks OK. Thank you for your patience. 🙏 A few minor (non-binding) notes/suggestions:
I'm a little confused at the entrypoint control flow -- if It seems like maybe something like the following was intended? #!/bin/sh
set -eu
# this will check if the first argument is a flag
# but only works if all arguments require a hyphenated flag
# -v; -SL; -f arg; etc will work, but not arg1 arg2
if [ "$#" -eq 0 ] || [ "${1#-}" != "$1" ]; then
if [ -n "${HITCH_CONFIG_FILE:-}" ]; then
# only add --config=... if HITCH_CONFIG_FILE was set and not empty
set -- --config="$HITCH_CONFIG_FILE" "$@"
fi
set -- hitch "$@"
fi
exec "$@"(Also, a rebase/squash of the final proposed commit would indeed be appreciated. 👍) |
|
Hi! So, the logic behind the entrypoint:
For example, replacing The goal was to save the user from setting Thanks for the The UID/GID makes sense, I will change it too, is there a rule for picking these IDs, or will any random values do the trick? |
Ah, so what you're looking for is something even simpler, like this: #!/bin/sh
set -eu
# this will check if the first argument is a flag
# but only works if all arguments require a hyphenated flag
# -v; -SL; -f arg; etc will work, but not arg1 arg2
if [ "$#" -eq 0 ] || [ "${1#-}" != "$1" ]; then
set -- hitch --config="${HITCH_CONFIG_FILE:-/etc/hitch/hitch.conf}" "$@"
fi
exec "$@"
We typically choose something in the high 900s because Another interesting convention we've seen is choosing the "default port" of the service, if it's a reasonable UID/GID that doesn't have a lot of potential for overlap, but given the nature of Given this image hasn't been published yet, you could also just choose 1000 directly, if you think that's reasonable -- the main goal is to avoid Debian UID/GID changes (however unlikely) from changing your Hitch user's UID/GID and breaking users unexpectedly. 😄 |
|
yeah, your code is indeed way more straightforward, I'll go with that. I did think about going with 443 and expected some resitance, 999 has a nice ring to it though. I'll push something this afternoon |
|
I mean, 443 is high enough that it shouldn't have conflicts, and it's kind of cute 😏 -- your call 😄 |
|
fixed, rebased and force-pushed! |
|
If you're using I also just left a review with some more notes/discussion on the docs PR: docker-library/docs#1675 (review) |
|
almost there! The packages are the generic ones, so we don't get to tinker with their creation. I didn't realize the OCSP directory was created by the package (I thought it was done at runtime), so I flipped things around and created the user before the package is installed. Thank you for the docs review, it'll probably wait until @ThijsFeryn is back, but that is great to have your feedback on it |
Diff for 9ea4d0b:diff --git a/_bashbrew-cat b/_bashbrew-cat
index bdfae4a..0b6f9fe 100644
--- a/_bashbrew-cat
+++ b/_bashbrew-cat
@@ -1 +1,5 @@
-Maintainers: New Image! :D (@docker-library-bot)
+Maintainers: Thijs Feryn <thijs@varni.sh> (@thijsferyn), Guillaume Quintard <guillaume@varni.sh> (@gquintard)
+GitRepo: https://github.com/varnish/docker-hitch.git
+
+Tags: 1, 1.7, 1.7.0, 1.7.0-1, latest
+GitCommit: d2feb9f1a1a3426da633383c2bac4a31559248bd
diff --git a/_bashbrew-list b/_bashbrew-list
index e69de29..2b55653 100644
--- a/_bashbrew-list
+++ b/_bashbrew-list
@@ -0,0 +1,5 @@
+hitch:1
+hitch:1.7
+hitch:1.7.0
+hitch:1.7.0-1
+hitch:latest
diff --git a/hitch_latest/Dockerfile b/hitch_latest/Dockerfile
new file mode 100644
index 0000000..d0ccdae
--- /dev/null
+++ b/hitch_latest/Dockerfile
@@ -0,0 +1,37 @@
+FROM debian:buster-slim
+
+ENV HITCH_VERSION 1.7.0-1~buster
+
+RUN set -ex; \
+ fetchDeps=" \
+ dirmngr \
+ gnupg \
+ "; \
+ apt-get update; \
+ apt-get install -y --no-install-recommends ca-certificates $fetchDeps; \
+ key=E35824BB706997D9184818E715A7ECE02FE19401; \
+ export GNUPGHOME="$(mktemp -d)"; \
+ gpg --batch --keyserver ha.pool.sks-keyservers.net --recv-keys $key; \
+ gpg --batch --export export $key > /etc/apt/trusted.gpg.d/hitch.gpg; \
+ gpgconf --kill all; \
+ rm -rf $GNUPGHOME; \
+ echo deb https://packagecloud.io/varnishcache/hitch/debian/ buster main > /etc/apt/sources.list.d/hitch.list; \
+ apt-get update; \
+ adduser --quiet --system --no-create-home --uid 443 --group hitch; \
+ groupmod -g 443 hitch; \
+ apt-get install -y --no-install-recommends hitch=$HITCH_VERSION; \
+ apt-get purge -y --auto-remove -o APT::AutoRemove::RecommendsImportant=false $fetchDeps; \
+ rm -rf /var/lib/apt/lists/*; \
+ mkdir /etc/hitch/certs/ /var/lib/hitch/; \
+ cp /etc/hitch/testcert.pem /etc/hitch/certs/default; \
+ sed -i 's/daemon = on/daemon = off/' /etc/hitch/hitch.conf
+
+WORKDIR /etc/hitch
+
+COPY docker-hitch-entrypoint /usr/local/bin/
+
+ENTRYPOINT ["docker-hitch-entrypoint"]
+
+EXPOSE 443
+
+CMD []
diff --git a/hitch_latest/docker-hitch-entrypoint b/hitch_latest/docker-hitch-entrypoint
new file mode 100755
index 0000000..29eaae7
--- /dev/null
+++ b/hitch_latest/docker-hitch-entrypoint
@@ -0,0 +1,11 @@
+#!/bin/sh
+set -e
+
+# this will check if the first argument is a flag
+# but only works if all arguments require a hyphenated flag
+# -v; -SL; -f arg; etc will work, but not arg1 arg2
+if [ "$#" -eq 0 ] || [ "${1#-}" != "$1" ]; then
+ set -- hitch "--config=${HITCH_CONFIG_FILE:-/etc/hitch/hitch.conf}" "$@"
+fi
+
+exec "$@"
|
4 participants
Hi,
Here's a new version of #7611, hopefully it'll be more aligned with the guidelines. It reuses a lot of the Varnish image experience and tries to fix the problems pointed out by #7611, notably, the command line is much easier to override and we are using a
gpgfingerprint and the "right" keyservers. Oh, and we are not using the new project-built packages instead of the distro ones, so we can keep up with the releases.Checklist for Review
NOTE: This checklist is intended for the use of the Official Images maintainers both to track the status of your PR and to help inform you and others of where we're at. As such, please leave the "checking" of items to the repository maintainers. If there is a point below for which you would like to provide additional information or note completion, please do so by commenting on the PR. Thanks! (and thanks for staying patient with us ❤️)
hitch)foobarneeds Node.js, hasFROM node:...instead of grabbingnodevia other means been considered?) (no, no relevant base image here)if(notFROM scratch, tarballs only exist in a single commit within the associated history?FROM scratch)