refactor: Revise Dockerfile - #2305

Summary

PR motivated by discussion in https://github.com/drwetter/testssl.sh/issues/2299#issuecomment-1408253127

Beyond updating the base image version, I've submitted some improvements:

• I've reviewed the current Dockerfile and git history related to it (detailed in later sections).
• If you disagree with any particular changes they can be easily reverted due to scoped commits 👍
• The RUN is much simpler now. Using the HereDoc feature could be dropped if you prefer. docker-mailserver has been using using since Oct 2022 without any problems with our user base (most just use the built image we distribute - unless they're contributing, where our Makefile handles the DOCKER_BUILDKIT=1 ENV).
• Each change has been staged into separate commits with commit messages for added context. For your convenience these have been listed below in the next section 👍
• Image weight is now ~30MB or ~34MB with openssl1.1-compat.
  • Base image: 7MB
  • Packages (with openssl1.1-compat): 12.6MB
  • COPY: 14MB

Commit Overview

• chore: Update Dockerfile base image from Alpine 3.16 to 3.17
• chore: Replace RUN chaining with && \ by using HereDoc feature
  HereDoc (with EOF marker) feature avoids needing && \ requires BuildKit during build (either DOCKER_BUILDKIT=1 docker build or by using buildx).
• chore: Use a single COPY by better leveraging .dockerignore patterns
• chore: Collapse package install into single command
  • --no-cache implies --update (aka apk update) while not storing the cache to disk, so no need to manually cleanup the cache with rm -rf afterwards.
  • --upgrade updates existing dependencies of packages.
• chore: Simplify testssl user creation
  • A group will implicitly be created with the same value as the user. addgroup testssl and -G testssl are not needed.
  • Gecos data (-g "testssl user") doesn't appear relevant to the project to be required? Default (Linux User,,,) should be fine.
  • With the HereDoc usage, an inline comment provides additional context.
• chore: Remove redundant mkdir
  If local folder ownership is for example 644 it will fail to handle the COPY regardless (while 744 would work). Creating the directory with higher permissions in the container does not appear to help.
• chore: Remove git (7MB) add openssl1.1-compat (4MB)
  • git isn't needed in the image.
  • Alpine 3.17 does a major version upgrade of OpenSSL to 3.x, while keeping the 1.1.x version may be beneficial? (can perform better)

Image history

Just for the benefit of not having to go through git history :)

• Docker image was originally Debian base, but switched to Alpine in 2018 for a smaller file size (performance can take a hit however due to musl instead of glibc).

Packages

• drill added for the DNS lookups with libidn included to support IDN (Internationalized Domain Names) lookups with drill.
• procps added for ps (commit from Debian base also dropped other DNS clients in favour of drill).
• socat is also related to supporting "STARTTLS injection" (full PR).
• openssl (originally for 1.1.1g) was also added to support "STARTTLS injection".
• openssl1.1-compat added in this PR itself, was found to perform better than the newer 3.x series (introduced by Alpine 3.17) which may matter for reducing CI / test feedback times. It can be used via --openssl /usr/bin/openssl1.1.
• xxd for better performance and debugging support.
• coreutils (approx 9MiB in weight) to replace BusyBox packages, notably for better supporting date -d.
• curl added for --phone-out checks.

Further consideration

The PR presently only covers the 3.1dev branch Dockerfile. If you're happy with the proposed changes the Dockerfile.git can be updated and then an equivalent PR can be opened for 3.0 branch.

• mkdir was dropped as despite original reasoning with 750 permission and ownership, the COPY --chown ensures the testssl user is fine?
• You may want to still run apk upgrade before apk add. Personally I'm not sure what the value of that is, since generally the base image should reflect the latest updates at the time of build.
  • Those updates are likewise only as good as when your image is built, so for example the 3.0 image was last built over 2 months ago. Chances are it's not helping much under that context?
  • Hence the decision to just upgrade only packages that are also dependencies of the new packages from apk add with --upgrade.
  • For reference some official images discouraged / opt-ed out of upgrading packages due to the risk of breaking software built and linked to earlier versions (link also mentions --no-cache handles apk update + cleanup of cache). At the time they reference official Docker docs backing that up as a best practice (that stance later changed for several reputable sources, but they do not encourage upgrading packages either).

Removing the need for Dockerfile.git and simplifying CI maintenance

However, we can easily remove the Dockerfile.git for simpler maintenance by using a multi-stage build. That can default to local COPY, or when building specifying an alternative target / ARG to get the files via git.

I have little information to go by the PR that introduced the current approach beyond the PR (and this commit prior to a separate PR being raised that covers both branches / builds). I'm a bit confused with the approach. You would ideally centralize your Github Actions into a single branch to keep maintenance simple.

• The Dockerfile.git appears to be only intended for serving this CI purpose? If so, you could just have the checkout action checkout the branch and COPY the files, making Dockerfile.git redundant. You could then have a single branch that maintains a single Dockerfile only for builds, not needing to keep it in sync across multiple branches if that appeals to you?

• ARG ARCHIVE_URL doesn't appear to be relevant or used for anything? I found nothing in the PR, or several search engine queries regarding it and the URL itself appears to be a dud?

• git package with an actual repo cloned was apparently only for benefit of git log to provide a commit hash to display in the testssl.sh banner output?. But that adds not only the weight of git package, but also some extra bin/ files you usually exclude from the build. You can just extract this information during the CI build and supply it as a build ARG? I understand if you want to keep it simple as-is though.

• The git package could also be avoided via an ADD instruction to pull in the same git content if you're not going to remove any of it via RUN?

• Another option is to build the image with git URL as the build context:

  DOCKER_BUILDKIT=1 docker build -t testssl:pr https://github.com/drwetter/testssl.sh.git#3.1dev

  NOTE: If the .dockerignore is important, the one in target remote git repo won't used. For the CI it wouldn't matter as you'd be checking out the repo and have a local .dockerignore file that does apply to the remote build context. Otherwise the extra bin/ files are included like with Dockerfile.git already.