Added support for the BookStack System CLI

ssddanbrown commented 1 year ago

[x] I have read the contributing guideline and understand that I have made the correct modifications

Description:

This updates the image with the requirements need to run the now-included "BookStack System CLI". This is a new CLI that automates certain admin-level operations like backup/restore.

To achieve support, the image needed to be updated with php-zip extension support, and the mariadb-client client. These changes , specifically the addition of mariadb-client, does lead to an increase in image size from approximately 280MB to 315MB. I do think this is worth it, especially as I can see some utility of having mariadb-client available for debug purposes outside just the context of the CLI, but that's not my call to make hence why I'm highlighting this size increase here.

Upon the above added packages, this also adds the newly added default BookStack backup directory to the folders that get linked to the /config volume, so that this backup folder is accessible via the mounted volume.

With these changes you'd able able to run the CLI like so:

# Create a backup
docker compose exec bookstack /app/www/bookstack-system-cli backup

# Restore an existing backup ZIP
docker compose exec bookstack /app/www/bookstack-system-cli restore /config/www/backups/bookstack-backup-2023-05-23-120213.zip

I did not update the readme yet with details of the CLI, since this CLI is in early alpha stages, so I don't want to actively advise it's usage right now, but it might be something to add in the future (Alongside details of calling the standard artisan command line).

Benefits of this PR and context:

This allows usage of the newly added BookStack System CLI. Closes #170

While there are alternative or direct means to backup content in this container-based setup (Via direct volume files and/or via DB dumps), this CLI automates these actions in a standard manner, which could be especially useful for users looking to migrate between different BookStack hosting options.

How Has This Been Tested?

I have built the image on Linux (Fedora 38 / AMD64) and on MacOS (ARM), then performed a backup and restore using the CLI on both.

Source / References:

Relevant issue: https://github.com/linuxserver/docker-bookstack/issues/170
CLI details on the BookStack blog: https://www.bookstackapp.com/blog/bookstack-release-v23-05/#new-system-cli-early-alpha
CLI project: https://github.com/BookStackApp/system-cli

LinuxServer-CI commented 1 year ago

I am a bot, here are the test results for this PR: https://ci-tests.linuxserver.io/lspipepr/bookstack/v23.05.2-pkg-261fc739-dev-e3149f929a62b8caf84f071c6142e0eea0f16b12-pr-173/index.html https://ci-tests.linuxserver.io/lspipepr/bookstack/v23.05.2-pkg-261fc739-dev-e3149f929a62b8caf84f071c6142e0eea0f16b12-pr-173/shellcheck-result.xml	Tag	Passed
amd64-v23.05.2-pkg-261fc739-dev-e3149f929a62b8caf84f071c6142e0eea0f16b12-pr-173	✅
arm32v7-v23.05.2-pkg-261fc739-dev-e3149f929a62b8caf84f071c6142e0eea0f16b12-pr-173	✅
arm64v8-v23.05.2-pkg-261fc739-dev-e3149f929a62b8caf84f071c6142e0eea0f16b12-pr-173	✅

nemchik commented 1 year ago

This will need to be rebased now that https://github.com/linuxserver/docker-bookstack/pull/174 has merged.

Key points:

php 8.2
php zip extension is now included in the base image (not required in the Dockerfiles here)
armhf is deprecated and removed

All that being said, I'm not a fan of adding 200mb+ to the image, and I think it might be better to recommend via the readme to use our universal package install mod https://github.com/linuxserver/docker-mods/tree/universal-package-install if you want this functionality.

thespad commented 1 year ago

Sounds like an ideal mod candidate really.

ssddanbrown commented 1 year ago

All that being said, I'm not a fan of adding 200mb+ to the image

Just to confirm, the image size change I observed was approximately from 280MB to 315MB, so a ~35MB change. Unless I've misunderstood and the 200mb referenced is via another form of measurement.

thespad commented 1 year ago

Total extra size for mariadb-client from scratch is about 85Mb, but will probably be less in practice because some of the dependency packages will already be installed on the image.

nemchik commented 1 year ago

All that being said, I'm not a fan of adding 200mb+ to the image

Just to confirm, the image size change I observed was approximately from 280MB to 315MB, so a ~35MB change. Unless I've misunderstood and the 200mb referenced is via another form of measurement.

I misread your initial post and thought you meant there was roughly a 200mb increase in the image size by including the additional package.

~35mb is definitely a lot less, but still a pretty decent number.

I don't have a strong opinion here, and I don't use bookstack at all, so I'll see if the rest of the team would like to chime in and if there are no other opinions I'll approve once the PR is rebased.

thespad commented 1 year ago

Quick build test, existing image is 287MB on disk, adding mariadb-client takes it to 361MB on disk so ~75MB. I don't think that's a problem for core functionality even if it's not going to be used by everyone.

thespad commented 1 year ago

@ssddanbrown I've rebased against the current master branch, let me know if you're happy and if so we'll get it merged.

LinuxServer-CI commented 1 year ago

I am a bot, here are the test results for this PR: https://ci-tests.linuxserver.io/lspipepr/bookstack/v23.05.2-pkg-e7357f94-dev-122d1204292249228dead1fc5fe7c23abef484c4-pr-173/index.html https://ci-tests.linuxserver.io/lspipepr/bookstack/v23.05.2-pkg-e7357f94-dev-122d1204292249228dead1fc5fe7c23abef484c4-pr-173/shellcheck-result.xml	Tag	Passed
amd64-v23.05.2-pkg-e7357f94-dev-122d1204292249228dead1fc5fe7c23abef484c4-pr-173	✅
arm64v8-v23.05.2-pkg-e7357f94-dev-122d1204292249228dead1fc5fe7c23abef484c4-pr-173	✅

ssddanbrown commented 1 year ago

Thanks @thespad. Just gave this a test build and run on mac (arm64).

PHP Timezone Issue

I noticed this error appearing whenever PHP is ran (Particularly via the CLI):

PHP Warning:  PHP Startup: Invalid date.timezone value '', using 'UTC' instead in Unknown on line 0

It looks like PHP date.timezone handling changed in php8.2. Within the base image, a php-local.ini is generated upon container start-up:

https://github.com/linuxserver/docker-baseimage-alpine-nginx/blob/71cc706de6cd077d4649a3a1a996a95e4601212a/root/etc/s6-overlay/s6-rc.d/init-php/run#L6

If the user is not passing a TZ env option, this generates date.timezone with an empty value. I don't think the TZ option has ever been a default value for the example compose setup in the readme, and I don't think there's been a fallback default value. I checked on a few of my past linuxserver compose setups, All of them have an empty date.timezone. This hasn't been an issue before, but now in PHP8.2 it causes the above warning. I think many setups upgrading to the latest image versions would have this warning appear (Also shows up in container start-up logs as PHP commands are ran).

BookStack System CLI

The CLI exits upon unexpected PHP output, so the above scenario causes the CLI to stop upon certain actions. Otherwise, running with a valid date.timezone (Via setting that option in php-local.ini or originally running with a TZ env option), allows backup and restore via the CLI to work as expected.

Summary

I see the timezone issue to be tangential to this PR. I'm happy to go ahead with this being merged, but users with an invalid/empty timezone may have trouble using this until that's addressed for their setup. Probably could do with a separate follow-up PR to update the example docker-compose with a default valid TZ option, and/or a script to fix existing empty date.timezone (Could maybe wait to assess impact before going down that road).

nemchik commented 1 year ago

The timezone issue will be fixed by https://github.com/linuxserver/docker-baseimage-alpine-nginx/pull/143

linuxserver / docker-bookstack