feat: openshift compatible container support #92
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
About
This PR adds support for running the alpine container as a user with arbitrary UID.
Changes
Default user no longer root
The default user is now set to puppet, but can be set to a user with any UID, for example UID 1337:
--user 1337. ~~Or back to root:--user root. ~~ EDIT: running as root will require some additional changes due to therunuserinstruction inforeground. Not sure right now what needs to be changed, but if this is important I can check it out more closely once I get more time.The default group is still root.
Group = root on relevant directories
The following directories have changed their group membership to root:
and their group access is mirroring the user permissions. The
setgidbit has been set on these directories and their subdirectories to ensure permission inheritance.The list of required directories may not be complete and could potentially be appended, but for basic operation they should suffice.
Any volumes should also be given root group and the needed permissions in order for the server to have access to them.
This pattern is the recommended approach to making OpenShift compatible containers:
https://developers.redhat.com/blog/2020/10/26/adapting-docker-and-kubernetes-containers-to-run-on-red-hat-openshift-container-platform
https://docs.redhat.com/en/documentation/openshift_container_platform/3.11/html/creating_images/creating-images-guidelines
30-set-permissions.shremovalAnother change was the removal of 30-set-permissions.sh. This script is not compatible with rootless containers, as it tries to recursively change owner and group to puppet on most of the directories that we just set the root group.
30-ensure-config.shadditionThis runtime script ensures that the default directories that are used for interpolation are the same for the current user and root. it also sets
manage_internal_file_permissionsto false, which is crucial for keeping the group membership as rootUSER="" in /etc/default/puppetserver
Before running the server the via the
foreground, the EUID is compared to the UID of the user that is defined in/etc/default/puppetserver. If it is not the same, a user switch is done before execution.We want this check to pass and in order to do so we could directly edit the foreground script, or we can set the USER variable to an empty string, as the check uses
id -u ${USER} to get the UID, and whenid -u` is called with no arguments it will give the UID of the current user.This is not ideal, but probably stable enough. Maybe we could implement some upstream changes in the future to make this work without resorting to something like that.
Disabling link creation of CA dir in
puppetserver setupWhen calling
puppetserver ca setupa symlink is created from/etc/puppetlabs/puppetserver/cato$ssldir/ca. This is done for compatibility with puppet 6 in which thecadirstill was in$ssldir.During link creation
chownis called, which is incompatible with can't be done without root or theCAP_CHOWNcapability.Therefore I opted to simply comment out the code line that creates starts the link creation via
sedand a hardcoded path to thesetup.rbscript.. This is definitively brittle, maybe you have a better suggestion here?Perhaps we could also monkey patch this or wrap the
setupscript otherwise.Ultimately, I believe this should be fixed upstream. Maybe the support for the "old" cadir location could be dropped anyways, at least in the next major version. There was a bunch of issues in my initial approaches due to this, for example
puppetserver ca setupdefaults to $HOME/.puppetlabs/etc/puppetserver/ca for the cadir when non-root (which is congruent with all the other non-root paths) butpuppetonly defaults to that path if it exists, otherwise it will take the system path.No more hardcoded Paths
The commit cca7655 changes a lot of hardcoded paths to be resolved via
puppet config printinstead. It may not strictly be necessary, as the paths do not change, but it makes the image more robust for future divergences.