[pve-devel] [PATCH v2 pve-manager 11/11] fix #4759: debian/postinst: configure ceph-crash.service and its key
Max Carrara
m.carrara at proxmox.com
Tue Feb 13 10:25:57 CET 2024
On 2/12/24 14:34, Fabian Grünbichler wrote:
> On February 5, 2024 6:54 pm, Max Carrara wrote:
>> This commit adds the `set_ceph_crash_conf` function, which dynamically
>> adapts the host's Ceph configuration in order to allow the Ceph crash
>> module's daemon to run without elevated privileges.
>>
>> This adaptation is only performed if:
>> * Ceph is installed
>> * Ceph is configured ('/etc/pve/ceph.conf' exists)
>> * Connection to RADOS is successful
>>
>> If the above conditions are met, the function will ensure that:
>> * Ceph possesses a key named 'client.crash'
>> * The key is saved to '/etc/pve/ceph/ceph.client.crash.keyring'
>> * A section for 'client.crash' exists in '/etc/pve/ceph.conf'
>> * The 'client.crash' section has a key named 'keyring' which
>> references '/etc/pve/ceph/ceph.client.crash.keyring'
>>
>> Furthermore, if a key named 'client.crash' already exists within the
>> cluster, it shall be reused and not regenerated. Also, the
>> configuration is not altered if the conditions above are already met.
>>
>> This way the keyring file is available as read-only in
>> '/etc/pve/ceph/' for the `www-data` group (due to how pmxcfs works).
>> Because the `ceph` user has been made part of said `www-data` group
>> [0], it may access the file without requiring any additional
>> privileges.
>>
>> Thus, the configuration for the Ceph crash daemon is safely adapted as
>> expected by PVE tooling and also shared via pmxcfs across one's
>> cluster.
>
> I still don't think this is a good idea, even a simple perl -e '..'
> invocation or two (or a small helper script that doesn't live in $PATH)
> for doing the two steps we want (initialize key if missing, lock+modify
> config if key was missing) would be better (although compared to the
> "hidden" or regular command approach, it has the downside that somebody
> might miss the calls here when refactoring),
>
> among other things the code below
> - doesn't lock /etc/pve/ceph.conf but modifies it
> - implements yet another broken parser for ceph.conf (e.g., it doesn't
> handle the stuff you fix in the perl variant in this series!)
> - duplicates constants from the perl code that risk running out of sync,
> like paths or the key profile
> - still has issues that you fixed in the perl code between v1 and v2
> (restarting services)
>
> I haven't reviewed the bash code in detail for that reason!
>
> another issue - IMHO this should be version-guarded, since any new setup
> would already gain it when setting up a monitor, and we avoid access to
> pmxcfs in the upgrade hot path which can cause problems (cluster
> non-quorate, ..).
I hadn't considered the points you mentioned above - I agree with all of them,
actually. I'll see if I can rewrite all this in Perl (would probably be easier
than BASH anyway).
As discussed off-list, instead of writing an inline Perl script, a separate
helper script (as an executable) would probably be better - a sensible place
for this script would be in '/usr/share/pve-manager/helpers'. That way we
can also point users to this script if something goes wrong during the update.
>
>>
>> [0]: https://git.proxmox.com/?p=ceph.git;a=commitdiff;h=f72c698a55905d93e9a0b7b95674616547deba8a
>>
>> Signed-off-by: Max Carrara <m.carrara at proxmox.com>
>> ---
>> Changes v1 --> v2:
>> * fix 'keyring' key being appended to 'client.crash' section even
>> if it already exists and configured correctly
>>
>> debian/postinst | 113 ++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 113 insertions(+)
>>
>> diff --git a/debian/postinst b/debian/postinst
>> index 6138ef6d..267a62ae 100755
>> --- a/debian/postinst
>> +++ b/debian/postinst
>> @@ -110,6 +110,118 @@ migrate_apt_auth_conf() {
>> fi
>> }
>>
>> +set_ceph_crash_conf() {
>> + PVE_CEPH_CONFFILE='/etc/pve/ceph.conf'
>> + PVE_CEPH_CONFDIR='/etc/pve/ceph'
>> + PVE_CEPH_CRASH_KEY="${PVE_CEPH_CONFDIR}/ceph.client.crash.keyring"
>> + PVE_CEPH_CRASH_KEY_REF="${PVE_CEPH_CONFDIR}/\$cluster.\$name.keyring"
>> +
>> + # ceph isn't installed -> nothing to do
>> + if ! which ceph > /dev/null 2>&1; then
>> + return 0
>> + fi
>> +
>> + # ceph isn't configured -> nothing to do
>> + if test ! -f "${PVE_CEPH_CONFFILE}"; then
>> + return 0
>> + fi
>> +
>> + CEPH_AUTH_RES="$(ceph auth get-or-create client.crash mon 'profile crash' mgr 'profile crash' 2>&1 || true)"
>> +
>> + # ceph is installed and possibly configured, but no connection to RADOS
>> + # -> assume no monitor was created, nothing to do
>> + if echo "${CEPH_AUTH_RES}" | grep -i -q 'RADOS object not found'; then
>> + return 0
>> + fi
>> +
>> + SECTION_RE='^\[\S+\]$'
>> + CRASH_SECTION_RE='^\[client\.crash\]$'
>> +
>> + if echo "${CEPH_AUTH_RES}" | grep -q -E "${CRASH_SECTION_RE}"; then
>> + DO_RESTART_UNIT=0
>> + CRASH_KEY="$(echo "${CEPH_AUTH_RES}" | grep 'key' | sed -E 's/^\s+key\s+=\s+//')"
>> +
>> + if test ! -d "${PVE_CEPH_CONFDIR}"; then
>> + mkdir -p "${PVE_CEPH_CONFDIR}"
>> + fi
>> +
>> + # keyring file doesn't exist or contains wrong key
>> + if test ! -f "${PVE_CEPH_CRASH_KEY}" || ! grep -q "${CRASH_KEY}" "${PVE_CEPH_CRASH_KEY}"; then
>> + echo "Saving key for 'client.crash' as '${PVE_CEPH_CRASH_KEY}'"
>> + echo "${CEPH_AUTH_RES}" > "${PVE_CEPH_CRASH_KEY}"
>> + DO_RESTART_UNIT=1
>> + fi
>> +
>> + # 'client.crash' section is in conf file
>> + if grep -q -E "${CRASH_SECTION_RE}" "${PVE_CEPH_CONFFILE}"; then
>> + IFS=''
>> + NEW_PVE_CEPH_CONFFILE=''
>> + IN_CRASH_SECTION=0
>> + HAS_KEYRING=0
>> + REPLACED_KEYRING=0
>> +
>> + # look for 'keyring' key in 'client.crash' section
>> + # -> replace it if it points to the wrong location
>> + while read -r LINE; do
>> + if test "${IN_CRASH_SECTION}" = "1"; then
>> + if echo "${LINE}" | grep -q -E "${SECTION_RE}"; then
>> + IN_CRASH_SECTION=0
>> + elif echo "${LINE}" | grep -q -E '\s+keyring'; then
>> + HAS_KEYRING=1
>> +
>> + if ! echo "${LINE}" | grep -q "${PVE_CEPH_CRASH_KEY_REF}"; then
>> + echo "Replacing keyring value in section 'client.crash' of '${PVE_CEPH_CONFFILE}'"
>> + LINE="$(printf '\t keyring = %s' "${PVE_CEPH_CRASH_KEY_REF}")"
>> + REPLACED_KEYRING=1
>> + fi
>> + fi
>> + elif echo "${LINE}" | grep -q -E "${CRASH_SECTION_RE}"; then
>> + IN_CRASH_SECTION=1
>> + fi
>> +
>> + NEW_PVE_CEPH_CONFFILE="${NEW_PVE_CEPH_CONFFILE}${LINE}\n"
>> + done < "${PVE_CEPH_CONFFILE}"
>> +
>> + unset IFS
>> +
>> + if test "${HAS_KEYRING}" = "1"; then
>> + # 'keyring' key was replaced -> write to file
>> + if test "${REPLACED_KEYRING}" = "1"; then
>> + echo "${NEW_PVE_CEPH_CONFFILE}" > "${PVE_CEPH_CONFFILE}"
>> + DO_RESTART_UNIT=1
>> + fi
>> +
>> + # client.crash section exists, but contained no 'keyring' key
>> + # -> put 'keyring' key into 'client.crash' section
>> + else
>> + sed -i -E "s#(${CRASH_SECTION_RE})#\1\n\t keyring = ${PVE_CEPH_CRASH_KEY_REF}#" \
>> + "${PVE_CEPH_CONFFILE}"
>> + DO_RESTART_UNIT=1
>> + fi
>> +
>> + # 'client.crash' section doesn't exist -> add it
>> + else
>> + echo "Adding section for key in '${PVE_CEPH_CONFFILE}'"
>> + printf '[client.crash]\n\tkeyring = %s\n\n' "${PVE_CEPH_CRASH_KEY_REF}" \
>> + >> "${PVE_CEPH_CONFFILE}"
>> + DO_RESTART_UNIT=1
>> + fi
>> +
>> + if test "${DO_RESTART_UNIT}" = "1"; then
>> + UNIT='ceph-crash.service'
>> +
>> + if systemctl -q is-enabled "${UNIT}"; then
>> + echo "Restarting ceph-crash.service"
>> + deb-systemd-invoke restart "${UNIT}"
>> + fi
>> + fi
>> +
>> + else
>> + echo "WARNING: Ceph: Unable to retrieve key for 'client.crash' - output:"
>> + printf '%s\n\n' "${CEPH_AUTH_RES}"
>> + fi
>> +}
>> +
>> case "$1" in
>> triggered)
>> # We don't print a status message here, as dpkg already said
>> @@ -189,6 +301,7 @@ case "$1" in
>> fi
>>
>> set_lvm_conf
>> + set_ceph_crash_conf
>>
>> if test ! -e /proxmox_install_mode; then
>> # modeled after code generated by dh_start
>> --
>> 2.39.2
>>
>>
>>
>> _______________________________________________
>> pve-devel mailing list
>> pve-devel at lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>>
>>
>>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel at lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
More information about the pve-devel
mailing list