Re: [pve-devel] [PATCH master ceph 1/8] debian: add patch to fix ceph crash dir permissions in postinst hook

[Max Carrara <m.carrara@proxmox.com>]

On 1/31/24 14:18, Fabian Grünbichler wrote:
> On January 30, 2024 7:40 pm, Max Carrara wrote:
>> Ceph has a postinst hook that sets the ownership of '/var/lib/ceph/*'
>> to ceph:ceph (in our case), but misses out on '/var/lib/ceph/crash/posted'.
>>
>> This patch therefore also updates the permissions of '/var/lib/ceph/*/*'.
>>
>> Signed-off-by: Max Carrara <m.carrara@proxmox.com>
>> ---
>>  ...rmissions-of-subdirectories-of-var-l.patch | 42 +++++++++++++++++++
>>  patches/series                                |  1 +
>>  2 files changed, 43 insertions(+)
>>  create mode 100644 patches/0015-debian-adjust-permissions-of-subdirectories-of-var-l.patch
>>
>> diff --git a/patches/0015-debian-adjust-permissions-of-subdirectories-of-var-l.patch b/patches/0015-debian-adjust-permissions-of-subdirectories-of-var-l.patch
>> new file mode 100644
>> index 000000000..951a2a6ed
>> --- /dev/null
>> +++ b/patches/0015-debian-adjust-permissions-of-subdirectories-of-var-l.patch
>> @@ -0,0 +1,42 @@
>> +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
>> +From: Max Carrara <m.carrara@proxmox.com>
>> +Date: Thu, 11 Jan 2024 14:04:16 +0100
>> +Subject: [PATCH] debian: adjust permissions of subdirectories of /var/lib/ceph
>> +
>> +A rather recent PR made ceph-crash run as "ceph" user instead of
>> +root [0]. However, because /var/lib/ceph/crash/posted belongs to root,
>> +ceph-crash cannot actually post any crash logs now.
>> +
>> +This commit fixes this by also updating the permissions of
>> +/var/lib/ceph/*/* - the subdirectories of the directories in
>> +/var/lib/ceph.
>> +
>> +[0]: https://github.com/ceph/ceph/pull/48713
>> +
>> +Signed-off-by: Max Carrara <m.carrara@proxmox.com>
>> +---
>> + debian/ceph-base.postinst | 8 ++++++++
>> + 1 file changed, 8 insertions(+)
>> +
>> +diff --git a/debian/ceph-base.postinst b/debian/ceph-base.postinst
>> +index 75eeb59c624..7ca0b9b6c43 100644
>> +--- a/debian/ceph-base.postinst
>> ++++ b/debian/ceph-base.postinst
>> +@@ -40,6 +40,14 @@ case "$1" in
>> + 		chown $SERVER_USER:$SERVER_GROUP $DIR
>> + 	    fi
>> + 	done
>> ++
>> ++	# also adjust file and directory permissons for subdirectories
>> ++	for SUBDIR in /var/lib/ceph/*/* ; do
>> ++	    if ! dpkg-statoverride --list $SUBDIR >/dev/null
>> ++	    then
>> ++		chown $SERVER_USER:$SERVER_GROUP $SUBDIR
>> ++	    fi
> 
> this would probably benefit from being merged with the loop above and
> being switched to find?
> 
> find(utils) is Essential, so its existence is a given..

Good point, seems sensible!

> 
> did you forward this patch upstream? if not, please do so :)

Will merge the two loops and use `find` instead and then also see
if I can supply a PR upstream (was initially planning to do that
once this patch is applied on our side).

> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
>