Overview

Request 886524 superseded

Add new package openSUSE-signkey-cert (bsc#1182641)


Fabian Vogt's avatar

The scriptlets don't take package upgrades/downgrades into account. Those would delete all certificates instead, because %postun of the old package is run last: https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#ordering


Joey Lee's avatar
author source maintainer target maintainer

I have built two version of RPMs for testing upgrades/downgrades by 'rpm -Uvh ' and 'rpm -Uvh --oldpackage'. I didn't see problem.

The %postun will remove xxx-kmp.crt.del file if the corresponding xxx-kmp.crt is still exists after uninstallation. So it only removes remaining *-kmp.crt.del certificate by mokutils.


Fabian Vogt's avatar

Ok. My main confusion with the scripts is that they try to work with multiple files inside the directory, even though there's not actually more than one file in this package and not more than one package planned right now.

AFAICT it's possible to just call mokutil from %preun on the final uninstallation, if the scriptlet knows about the fingerprint, but that would need extra logic to deal with detection of fingerprint changes. I don't really have a better idea for this particular case. The cleanest way would be a package with trigger scripts which can tell whether a certificate is about to be removed, replaced or added, but that's just overengineering.


Fabian Vogt's avatar

Is it necessary to postpone the call to mokutil --delete to %postun or can it be done in %preun instead? Then it wouldn't be needed to do "backups" ($cert.del)


Joey Lee's avatar
author source maintainer target maintainer

Can not. Please see my the above comment.


Fabian Vogt's avatar
if [[ "command -v mokutil >/dev/null" && -d "%{_sysconfdir}/uefi/certs/" ]]; then

That does not actually run command -v, it checks whether the constant string is non-empty. Also, this uses a bashism, so try this instead:

if command -v mokutil >/dev/null && [ -d "%{_sysconfdir}/uefi/certs/" ]; then

Joey Lee's avatar
author source maintainer target maintainer

Thanks for your review! I will fix it in next version.


Fabian Vogt's avatar

Is it necessary to check for the existance of %{_sysconfdir}/uefi/certs/? It's owned by the package, so except for the final uninstallation it'll always be present.


Joey Lee's avatar
author source maintainer target maintainer

I will try to remove it. As I remember that I add the check because I got warning messages in OBS log. I will remove the check to see if there have any warning.

Thanks!


Joey Lee's avatar
author source maintainer target maintainer

I have tested after removed the check for the existance of %{_sysconfdir}/uefi/certs/. Looks no problem. I will remove in next version.



Joey Lee's avatar
author source maintainer target maintainer

I have sent new version request number: 888962


Andreas Jaeger's avatar

Why should this live in Base:System at all? If it's for kernel-modules, why not have it live there?


Joey Lee's avatar
author source maintainer

Thanks for review!

We want to install signkey package by pattern. So I created a independent package to help user to enroll key to MOK. It's only for openSUSE KMP on OBS.


Fabian Vogt's avatar
  • The check in %build shouldn't be needed. If the file is missing, it's going to break anyway
  • Calling rpm in scriptlets might deadlock
  • Scriptlets are also called on upgrades, but then most of them shouldn't do anything unless the file changes
  • Most of the scriptlets can be simplified by moving them inside if command -v mokutil

Joey Lee's avatar
author source maintainer

Thanks for your review!

. The check in %build shouldn't be needed. If the file is missing, it's going to break anyway

OK! I will remove the check.

. Calling rpm in scriptlets might deadlock

I will use "ls" to grab the certificate file name.

. Scriptlets are also called on upgrades, but then most of them shouldn't do anything unless the file changes

This RPM will not be rebuilt unless the openSUSE signkey be changed on OBS. When this RPM be upgraded, it means that the openSUSE signkey also be updated.

. Most of the scriptlets can be simplified by moving them inside if command -v mokutil

OK, I will move them inside ' if command -v mokutil '.


Fabian Vogt's avatar

This RPM will not be rebuilt unless the openSUSE signkey be changed on OBS. When this RPM be upgraded, it means that the openSUSE signkey also be updated.

There will be forced rebuilds without source changes, so the package has to take that into account.


Andreas Jaeger's avatar

Please name the package openSUSE-...

Check: rpmqpack |grep -i opensuse


Joey Lee's avatar
author source maintainer

Thanks! I will change the name.

Request History
Joey Lee's avatar

joeyli created request

Add new package openSUSE-signkey-cert (bsc#1182641)


Joey Lee's avatar

joeyli superseded request

superseded by 888962

openSUSE Build Service is sponsored by