docker: Add proper entrypoint

[oliv3r]

As per docker guidelines 0 a container should always really have a consistent entrypoint, without having to override it or do special tricks.

The behavior should be identical as before, but will no longer trigger errors because docker-gen doesn't understand certain parameters (/bin/sh for example being common). Further more, allows a proper entrypoint for a CI to work easily with the container as well. Allowing for scenario's such as apk add git && docker-gen renew in a docker-gen image for example.

E.g. docker run docker-gen --help works, as does docker run docker-gen /bin/sh or docker run docker-gen ls.

[buchdag]

Hi. Thanks for the PR.

In addition to the change requests I made below, I'd prefer the entrypoint script to be located at /app/docker-entrypoint.sh, both in the repo and inside the image, for consistency with https://github.com/nginx-proxy/nginx-proxy

[buchdag]

What's the rationale behind the [ -d "${1}" ] check (this would end up passing a folder as argument to docker-gen, and AFAIK this does not work) ? 🤔

While I understand the command -v "${1}" > '/dev/null' 2>&1 check, I don't think that's the way to go because it would mean commands with typo would be passed as argument to docker-gen instead of failing with a foo: not found as they should. A simple [ -f "${1}" ] work in this case: executables in PATH do not check as files.

Finally the exec ${entrypoint:+${bin:?}} "${@}" is hard to decipher and needlessly complex, the way it's done in the official docker example is clearer.

Suggestion taking all of the above into account:

#!/bin/sh

set -eu

# if command starts with an "-" or is a file, pass it as argument(s) to docker-gen, else run it.
if [ "${#}" -le 0 ] || [ "${1#-}" != "${1}" ] || [ -f "${1}" ]; then
    set -- docker-gen "${@}"
fi

exec "${@}"

The exit 0 at the end as no real use since we're using set -e.

[oliv3r]

It looks like github ate my comment, as I'm very sure I replied in detail. I'll try to find it again.

[buchdag]

Aw crap. 😞

[oliv3r]

Yep, gone; was hoping I could get it in the edit field on my laptop, no such luck ... so let me try again.

What's the rationale behind the [ -d "${1}" ] check (this would end up passing a folder as argument to docker-gen, and AFAIK this does not work) ? thinking

I have to dig deep now, as I use this template for everything (to keep them all the same, nice and consistent). Afaik, if you have a container, that accepts a directory as input, lets say the ls container, if the directory exists, it'll allow you to pass the directory as argument to the binary. But you got me, I'm trying remember really hard what the usecase was (there was one :p)

While I understand the command -v "${1}" > '/dev/null' 2>&1 check, I don't think that's the way to go because it would mean commands with typo would be passed as argument to docker-gen instead of failing with a foo: not found as they should. A simple [ -f "${1}" ] work in this case: executables in PATH do not check as files.

But was it a command you where trying to execute? or was it something you wanted to pass a long. If you assume the script to be dedicated only to docker-gen yeah, sure. But in a more generic case. Also, if I supply cp instead of /usr/bin/cp it should still work.

Finally the exec ${entrypoint:+${bin:?}} "${@}" is hard to decipher and needlessly complex, the way it's done in the official docker example is clearer.

Eye of the beholder, sure enough. I used to have ${bin:+${bin}} before, is that easier on the eyes? I took this new approach, as I wanted to keep the bin definition at the top of the file, for easy recognition.

The docker approach is equally 'confusing'. What's with the set --, that is also less clear (doesnt' that eat the existing set -eu also? Idk, never had to use it yet). The reason it is a variable though; is to ensure a single exit point of the script.

Suggestion taking all of the above into account:

#!/bin/sh

set -eu

# if command starts with an "-" or is a file, pass it as argument(s) to docker-gen, else run it.
if [ "${#}" -le 0 ] || [ "${1#-}" != "${1}" ] || [ -f "${1}" ]; then
    set -- docker-gen "${@}"
fi

exec "${@}"

The exit 0 at the end as no real use since we're using set -e.
ah, but what if some hacker dude does echo 'rm -rf /' >> script.sh? It's a shell scripting best-practise (afaik its in the google bash styleguide even). I like best-practises :) 'always have a defined exit point at the end of your script', in case something goes haywire.

[rhansen]

entrypoint should be initialized to the empty string in case it is already exported to this script's environment.

[rhansen]

Not needed; exec cannot return unless it can't execute the command, in which case exec's return value should be returned.

Suggested change

	exit 0

[rhansen]

Isn't this implied by the command check?

[rhansen]

If you pass -- to command, it will prevent $1 from being interpreted as an option. That would make it possible to eliminate this check. (I don't think it's a problem accidentally running a command named something like -x; such commands shouldn't exist, and if they do then running it is no worse than accidentally running any other command that was intended to be a command-line option to docker-gen.)

[rhansen]

command will return 1 if the command name is the empty string, so this check isn't necessary.

[rhansen]

I agree with @buchdag that this should use set -- docker-gen "$@". This is the idiomatic way to "unshift", which makes it more readable.

[rhansen]

Putting it all together, I would do something like this:

Suggested change

	bin='docker-gen'
	# run command if it is not starting with a "-" and is an executable in PATH
	if [ "${#}" -le 0 ] || \
	[ "${1#-}" != "${1}" ] || \
	[ -d "${1}" ] || \
	! command -v "${1}" > '/dev/null' 2>&1; then
	entrypoint='true'
	fi
	exec ${entrypoint:+${bin:?}} "${@}"
	exit 0
	command -v -- "$1" >/dev/null 2>&1 || set -- docker-gen "$@"
	exec "$@"

[buchdag]

Superseded by #573