Skip to content

feat: only do a full chown of the /data directory if requested or if the /data directory is not owned by plone #201

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
erral wants to merge 6 commits into
base: 6.1.x
Choose a base branch
from
Open

feat: only do a full chown of the /data directory if requested or if the /data directory is not owned by plone #201

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7a22939
feat: only do a full chown of the /data directory if requested or if …
erral Mar 19, 2026
a5190cb
fix
erral Mar 19, 2026
f5bdf86
remove echo
erral Mar 19, 2026
e8e032e
output a message when the docker image is running the find&chown
erral May 7, 2026
825dbde
add tests
erral May 7, 2026
2bf5196
Merge branch '6.1.x' into fix-chown
erral May 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion skeleton/docker-entrypoint.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,11 @@ USER="$(id -u)"
# Create directories to be used by Plone
mkdir -p /data/filestorage /data/blobstorage /data/cache /data/log $CLIENT_HOME
if [ "$USER" = '0' ]; then
find /data -not -user plone -exec chown plone:plone {} \+
# Check ownership, OR check if we are explicitly forcing a fix
if [ "$(stat -c '%U' /data)" != "plone" ] || [ "$FORCE_CHOWN" = "1" ]; then
echo "Running chown to fix ownership of the data directory"
find /data -not -user plone -exec chown plone:plone {} \+
Copy link
Copy Markdown
Member

@davisagli davisagli Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe log something before this starts, so it's more obvious when it happens?

Copy link
Copy Markdown

@yurj yurj Mar 20, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also find + chown is a lot slower, just use chown -R and it will be fast and you don't need any check :-) or, at least, move the check on the file, so do find and use stat to see if the file owner should be changed and change it only if needed. This avoid to write no fs when unnecessary. Something like "find /datadirectory ! ( -user plone -group plone ) -exec chown nomeutente:nomegruppo {} +"
Find will check if a file have an incorrect user or group and write only if it happens. So it should be very fast. For example (in a blobstorage I've created a file named zero with owner root and group plone):

$ ls -l 0
-rw-r--r-- 1 root plone 0 20 mar 10.08 0
$ time find . ! \( -user plone -group plone \) -exec echo {} +
./0

real	0m1,535s
user	0m0,654s
sys	0m0,880s
 time find . | wc -l
133625

real	0m1,348s
user	0m0,559s
sys	0m0,854s

# time find . ! \( -user plone -group plone \) -exec chown plone:plone {} +

real	0m1,452s
user	0m0,637s
sys	0m0,813s
root@portaletest:/usr/local/sw/plone/plone_prod2025/var/blobstorage# ls -l 0
-rw-r--r-- 1 plone plone 0 20 mar 10.08 0

in a couple of seconds you can check things, and the time depends on the number of files to fix. Said that, it is quite uncommon to have mixed users/group in the blobstorage, so I don't know if this optimisation can really help. Also, the command above don't check for file permissions/suid directories.

So the best command is:

$ time chown -R plone:plone .

real	0m1,606s
user	0m0,495s
sys	0m1,109s

but this is because -R already checks if the owner and group are already ok. So you've the check already in place in chown -R. In this case it is all ok and in a 1,6 secs you can do the check. This is a local filesystem, things can be different in other cases but the command just read until it really needs to write. Chown has a -c flag that outputs only changed files, it can be handy when testing.

Note: beware of syntax and escaping in the find command above, can be tricky because of ! and (

Copy link
Copy Markdown
Member Author

@erral erral May 7, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your stats @yurj.

I have 2 sites that I have already moved to plone-backend + plone-zeo and where I have running a custom plone-backend image without the find-chown dance. When I run the said chown command I get the following information:

Site 1: 590667 blobs

real	2m38.144s
user	0m0.010s
sys	0m0.005s

Site 2: many more blobs than Site 1. It is still running after more than 30 minutes and hasn't finished yet 😓

In this other site, I have a buildout based project, and I have run the same command:

Site 3: 95238 blobs

real	0m22.840s
user	0m0.572s
sys	0m5.470s

So I will update the PR to add the information message as suggested.

fi
sudo="gosu plone"
else
sudo=""
Expand Down
1 change: 1 addition & 0 deletions test/config.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ plone-listenport
plone-zeoclient
plone-relstorage
plone-shared-blob-dir
plone-basics-chown
'

imageTests+=(
Expand Down
5 changes: 5 additions & 0 deletions test/tests/plone-basics-chown/expected-std-out.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
--- Case 1: Automatic fix ---
Running chown to fix ownership of the data directory
--- Case 2: Forced fix ---
Running chown to fix ownership of the data directory
--- Case 3: No fix ---
29 changes: 29 additions & 0 deletions test/tests/plone-basics-chown/run.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
#!/bin/bash
set -eo pipefail

image="$1"

# Case 1: /data owned by root. Should fix it automatically.
echo "--- Case 1: Automatic fix ---"
cname1="plone-chown-auto-$RANDOM"
docker run --name "$cname1" -v /data busybox sh -c "chown root:root /data && touch /data/auto"
docker run --rm --volumes-from "$cname1" "$image" true
docker run --rm --volumes-from "$cname1" busybox stat -c "%u:%g" /data/auto | grep -q "500:500"
docker rm -v "$cname1" > /dev/null

# Case 2: /data owned by plone, but FORCE_CHOWN=1. Should fix it manually.
echo "--- Case 2: Forced fix ---"
cname2="plone-chown-force-$RANDOM"
docker run --name "$cname2" -v /data busybox sh -c "chown 500:500 /data && touch /data/forced && chown root:root /data/forced"
docker run --rm --volumes-from "$cname2" -e FORCE_CHOWN="1" "$image" true
docker run --rm --volumes-from "$cname2" busybox stat -c "%u:%g" /data/forced | grep -q "500:500"
docker rm -v "$cname2" > /dev/null

# Case 3: /data owned by plone, no FORCE_CHOWN. Should NOT fix (no message).
echo "--- Case 3: No fix ---"
cname3="plone-chown-none-$RANDOM"
docker run --name "$cname3" -v /data busybox sh -c "chown 500:500 /data && touch /data/no-fix && chown root:root /data/no-fix"
docker run --rm --volumes-from "$cname3" "$image" true
# Should still be root:root (0:0)
docker run --rm --volumes-from "$cname3" busybox stat -c "%u:%g" /data/no-fix | grep -q "0:0"
docker rm -v "$cname3" > /dev/null
Loading