Dockerfile is missing Swift runtime dependencies, causing runtime crashes

[gohanlon]

Dockerfile is missing Swift runtime dependencies, causing runtime crashes

[0xTim]

Omitting them was a deliberate decision. Most server side apps don't use foundation networking or FoundationXML whereas tzdata is used for anything date related.

I don't think we should be installing these by defaults as it just makes the runtime image bigger and the majority of people don't use it. How we help though is a harder problem and up for discussion. Potentially a blog post or doc page that can be indexed so when people Google the error they can find the solution

[0xTim]

Ok PR merged - feel free to open the PR on the blog 🎉

[gohanlon]

Awesome, thanks for the reply, Tim!

Also, this wasn't hard to fix locally, so not a huge issue at all.

This did come up for me today with a fairly bare-bones app. I was putting together a reduced example for a deployment issue effecting my real project (not a Vapor issue). I hit this crash after adding a single dependency to the generated bare Vapor project — vapor-routing which imports FoundationNetworking.

I don't think we should be installing these by defaults as it just makes the runtime image bigger and the majority of people don't use it.

You're probably right that the majority don't need these deps, and I'm not sure how one could measure how many Vapor users depend on libcurl4 or libxml2. Its easy to measure to see that you're right about the size increase being significant:

image	size
vapor-bare-no-libxml2-no-libcurl4	189.04 MB
vapor-bare-with-libxml2-with-libcurl4	233.09 MB

(These are for aarch64, btw.)

How we help though is a harder problem and up for discussion. Potentially a blog post or doc page that can be indexed so when people Google the error they can find the solution

A few thoughts:

1. Split the RUN statement containing "apt-get -q install" into multiple lines to make modification more inviting/welcoming. (Still as a single RUN statement.)
   Something similar to the formatting I use below and in my soon to be closed PR #87. This is an extremely soft nudge, but nonetheless a nudge in the right direction. I think that'd be more readable, and easier to see that only two packages are installed in the run image. It's easier to notice what's not there, too. (That formatting would also be consistent with the companion statement in the build stage.)

2. Add a comment to the generated Dockerfile, e.g.:

# Make sure all system packages are up to date, and install only the most essential packages.
# If your app or its dependencies import FoundationNetworking, also install `libcurl4`.
# If your app or its dependencies import FoundationXML, also install `libxml2`.
RUN export DEBIAN_FRONTEND=noninteractive DEBCONF_NONINTERACTIVE_SEEN=true \
    && apt-get -q update \
    && apt-get -q dist-upgrade -y \
    && apt-get -q install -y \
      ca-certificates \
      tzdata \
    && rm -r /var/lib/apt/lists/*

3. Even this issue page itself could already improve discoverability, once indexed.

For anyone happening upon this issue: on Ubuntu Focal and Swift 5.6.2, the fix is to include libxml2 and/or libcurl4 in your run image. Vapor's default build image is based on the heavier swift:5.6.2-focal image, which has both of those dependencies already. Something like this:

-RUN export DEBIAN_FRONTEND=noninteractive DEBCONF_NONINTERACTIVE_SEEN=true && \
-    apt-get -q update && apt-get -q dist-upgrade -y && apt-get -q install -y ca-certificates tzdata && \
-    rm -r /var/lib/apt/lists/*
+RUN export DEBIAN_FRONTEND=noninteractive DEBCONF_NONINTERACTIVE_SEEN=true \
+    && apt-get -q update \
+    && apt-get -q dist-upgrade -y \
+    && apt-get -q install -y \
+      ca-certificates \
+      libcurl4 \
+      libxml2 \
+      tzdata \
+    && rm -r /var/lib/apt/lists/*

@0xTim Let me know what you think. I'm happy to draft a PR, with your guidance. And, please feel free to close this issue if you think enough has been done. Cheers!

[0xTim]

@gohanlon yeah I like adding it to the Dockerfile as an opt in. I think we should go further and have it inline, something like (if it doesn't break it):

# Make sure all system packages are up to date, and install only the most essential packages.
RUN export DEBIAN_FRONTEND=noninteractive DEBCONF_NONINTERACTIVE_SEEN=true \
    && apt-get -q update \
    && apt-get -q dist-upgrade -y \
    && apt-get -q install -y \
      ca-certificates \
      tzdata \
# If your app or its dependencies import FoundationNetworking, also install `libcurl4`.
      # libcurl4
# If your app or its dependencies import FoundationXML, also install `libxml2`.
      # libxml2
    && rm -r /var/lib/apt/lists/*

If you'd like to do a guest blog post as well (only needs to be a paragraph or two) that would be great too!

[gohanlon]

I think we should go further and have it inline [...]

I was unsure that inlining comments like that would work, but it works. This command:

# Make sure all system packages are up to date, and install only essential packages.
RUN export DEBIAN_FRONTEND=noninteractive DEBCONF_NONINTERACTIVE_SEEN=true \
    && apt-get -q update \
    && apt-get -q dist-upgrade -y \
    && apt-get -q install -y \
      ca-certificates \
# If your app or its dependencies import FoundationNetworking, also install `libcurl4`.
      # libcurl4 \
# If your app or its dependencies import FoundationXML, also install `libxml2`.
      # libxml2 \
      tzdata \
    && rm -r /var/lib/apt/lists/*

works and logs as follows:

...
10 #10 [build  2/12] RUN export DEBIAN_FRONTEND=noninteractive DEBCONF_NONINTERACTIVE_SEEN=true     && apt-get -q update     && apt-get -q dist-upgrade -y     && rm -rf /var/lib/apt/lists/*
...

I'll put together a quick PR for that change.

If you'd like to do a guest blog post as well (only needs to be a paragraph or two) that would be great too!

I'd be happy to! I'll draft a short post, making mention of this change and the specific error messages (to feed the index).

[gohanlon]

Here's the PR I mentioned: #88

[gohanlon]

I also started drafting a blog post, but I think I'll pause on that until PR #88 is resolved.