Support for running with a read-only root filesystem

[wkbrd]

Immutable container file systems are detailed as a recommendation in the CISA Kubernetes Hardening Guide.
https://www.cisa.gov/news-events/alerts/2022/03/15/updated-kubernetes-hardening-guide

Additional details on the security benefits of this change:

• https://kubesec.io/basics/containers-securitycontext-readonlyrootfilesystem-true/
• https://kyverno.io/policies/best-practices/require-ro-rootfs/require-ro-rootfs/

Support for running with a read-only root filesystem for improved security. The implementation involves an init container that merges configuration items from defaults and any overrides.

[sklarsa]

@wkbrd I fixed the template syntax error, in a separate PR. If you merge/rebase master, it should be fixed.

It looks like there are 2 separate issues in this PR:

1. Read-Only root filesystem: I need to check with the core team to make sure that the db doesn't write anywhere else inside the container before enabling this feature. Also, we should set the default to false to avoid breaking existing deployments.

2. Config overrides: Can you clarify what this change accomplishes? override_conf and merged_conf are nonstandard directories and the db won't know to look for them on startup.

[wkbrd]

Hi @sklarsa ,
The second item on your list (Config overrides) is a workaround/mitigation for what might be a bug with questdb.

In my testing/environment, if one is only overriding a single setting file (e.g., serverConfig), then I am observing that the container will not start properly.

Here is an example:

helm install -n qdb qdb ./questdb/ \
            --set "persistence.size=5Gi" \
            --set resources.limits.ephemeral-storage=1Gi \
            --set resources.requests.ephemeral-storage=1Gi \
            --set questdb.serverConfig.enabled=true \
            --set questdb.serverConfig.options."http\.enabled"=false

With the following container log before the container crashes:

No arguments found in the configuration, start with default arguments
Running as questdb user
io.questdb.Bootstrap$BootstrapException: Could not extract log configuration file
	at [email protected]/io.questdb.Bootstrap.<init>(Bootstrap.java:129)
	at [email protected]/io.questdb.Bootstrap.<init>(Bootstrap.java:97)
	at [email protected]/io.questdb.ServerMain.<init>(ServerMain.java:91)
	at [email protected]/io.questdb.ServerMain.main(ServerMain.java:185)

The usage of this helm install command currently fails on the mainline questdb chart (excluding this PR). This fails with an initial installation (i.e., no PersistentVolumeClaim prior to the helm install).

Providing multiple config items (logConfig) can mitigate this. This PR includes a mitigation by using an initContainer that copies in the override files and by doing so, the main container launch properly will copy in the non-overridden files. However, if the install arguments above were to work properly (and also is confirmed to work with questdb.configStorageType=secret), then this PR would likely get simpler and not need the initContainer for the config prep and associated override_conf/merged_conf.

Should someone attempt to address the problem above first before we proceed with read-only root filesystem changes?

[sklarsa]

Should someone attempt to address the problem above first before we proceed with read-only root filesystem changes?

Yes, I will take a look at reproducing and fixing the problem this week. Thanks for pointing it out!

[sklarsa]

This appears to a be a permissions issue inside the container. We're running as user questdb, but trying to create log.conf inside /var/lib/questdb/conf, which is owned by root.

Running a shell inside the container...

questdb@questdb-0:~$ whoami
questdb
questdb@questdb-0:~$ ls -lah
total 12K
drwxrwxrwx 3 root root 4.0K Oct 28 14:20 .
drwxr-xr-x 1 root root 4.0K Oct  3 13:59 ..
drwxr-xr-x 2 root root 4.0K Oct 28 14:20 conf
questdb@questdb-0:~$ ls -lah conf
total 12K
drwxr-xr-x 2 root root    4.0K Oct 28 14:20 .
drwxrwxrwx 3 root root    4.0K Oct 28 14:20 ..
-rw-r--r-- 1 root questdb   44 Oct 28 14:21 server.conf
questdb@questdb-0:~$ touch conf/log.conf
touch: cannot touch 'conf/log.conf': Permission denied

We usually avoid this issue in our docker-entrypoint.sh by chowning all child database directories and then stepping down into a non-root user using gosu before starting the database. But because the container is not running as root, the chown never happens and we get a permission denied error when trying to create the default log.conf.

By explicitly creating the file (or dir) beforehand with a volume mount, the issue is avoided.

I need to think about our options here. I'm leaning towards reverting the security context defaults we set earlier in #223. In that case, I would leave the key available for you to customize the securityContext and podSecurityContext as you see fit.

Because our docker container is used in a wide variety of orchestration tools, I'm hesitant to start making further changes to the entrypoint at this time.

What do you think?

[wkbrd]

Hi @sklarsa , the config-mount changes (part 2 in your original comment) in this PR accommodates this problem by making the conf directory writeable. The overrides are copied into the temporary volume during container startup. Then when the main container runs, it is able to add the default files (non-overridden) as you are mentioning.

It is important for security reasons that the container to be designed to be an immutable container and that writeable areas are properly handled/designated in them.

Excluding the securityContext.readOnlyRootFilesystem setting in values.yaml (line 17), are you OK with the changes in this PR to allow the optional overrides to work?

[sklarsa]

@wkbrd I understand that this PR technically solves the problem, but in my opinion, it adds unnecessary complexity to the helm chart as a whole.

I prefer to structure the helm chart to allow users to layer in additional security configurations to fulfill their organization's specific needs. This way, we can ensure that the default helm chart will work across as wide of a user base as possible.

As we've seen, the default security context changes have already caused one issue fairly early on in the database execution path. And there may be more things that we aren't able to catch without extensive testing all possible combinations of helm chart values, command line flags, and database versions.

Because of this, I am going to remove the securityContext and podSecurityContext default values set in #223 in a new PR. Based on your requirements, you are free to update initContainers, securityContext, and podSecurityContext to fit your needs.

I really appreciate the work that you've put into this over the past few weeks. I understand the importance of security-by-default, but I need to weigh this against the risk of difficult-to-debug outages as a result of helm chart updates.