Comment 23 for bug 1919563

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

b) disable_setting() doesn't check if the sssd config file exists, and instead relies on the || true to never fail:

 disable_setting()
 {
     echo "Disabling sssd.conf setting using invalid value: '$1'"
- sed -i 's/^[^#;]*'"$1"'\b/#&/' /etc/sssd/sssd.conf || true
+ sed -i 's/^[^#;]*'"$1"'\b/#&/' "$SSSD_CONF" || true
 }

That will likely produce an ugly "sed: can't read <file>: No such file or directory" in stderr, though. Since this function is being updated to use $SSSD_CONF, could it perhaps also be updated to check for the conf file?

I checked callers, and maybe it would never be called if there is no sssd.conf file present. The callers are gated on the upgrade logic that starts with:

  if dpkg --compare-versions "$2" lt-nl 2.2.3-3ubuntu0.4; then

and then basically the existence of /etc/pki/nssdb. If this is true, then I guess it's ok to not have to check for the existence of sssd.conf, but if not, or if we have to request another upload of this package due to another reason, it would be good to add that simple check to disable_setting() to avoid a possible error message.