[Freeipa-devel] [PATCH 477] spec file: Add Requires(pre) on selinux-policy

[Alexander Bokovoy]

On Tue, 25 Aug 2015, Martin Kosek wrote:
>On 08/25/2015 05:37 PM, Alexander Bokovoy wrote:
>>On Tue, 25 Aug 2015, Martin Kosek wrote:
>>>On 08/25/2015 04:37 PM, Jan Cholasta wrote:
>>>>On 25.8.2015 14:50, Alexander Bokovoy wrote:
>>>>>On Tue, 25 Aug 2015, Jan Cholasta wrote:
>>>>>>On 25.8.2015 14:23, Alexander Bokovoy wrote:
>>>>>>>On Tue, 25 Aug 2015, Jan Cholasta wrote:
>>>>>>>>Hi,
>>>>>>>>
>>>>>>>>the attached patch fixes
>>>>>>>><https://fedorahosted.org/freeipa/ticket/5256>.
>>>>>>>>
>>>>>>>>Honza
>>>>>>>>
>>>>>>>>--
>>>>>>>>Jan Cholasta
>>>>>>>
>>>>>>>>From 216be8de30747f80f490d4e91a7cca4af3e767d6 Mon Sep 17 00:00:00 2001
>>>>>>>>From: Jan Cholasta <jcholast at redhat.com>
>>>>>>>>Date: Tue, 25 Aug 2015 14:14:25 +0200
>>>>>>>>Subject: [PATCH] spec file: Add Requires(pre) on selinux-policy
>>>>>>>>
>>>>>>>>This prevents ipa-server-upgrade failures on SELinux AVCs because of
>>>>>>>>old
>>>>>>>>selinux-policy version.
>>>>>>>>
>>>>>>>>https://fedorahosted.org/freeipa/ticket/5256
>>>>>>>>---
>>>>>>>>freeipa.spec.in | 1 +
>>>>>>>>1 file changed, 1 insertion(+)
>>>>>>>>
>>>>>>>>diff --git a/freeipa.spec.in b/freeipa.spec.in
>>>>>>>>index cba91fe..fd73cda 100644
>>>>>>>>--- a/freeipa.spec.in
>>>>>>>>+++ b/freeipa.spec.in
>>>>>>>>@@ -139,6 +139,7 @@ Requires: systemd-units >= 38
>>>>>>>>Requires(pre): shadow-utils
>>>>>>>>Requires(pre): systemd-units
>>>>>>>>Requires(post): systemd-units
>>>>>>>>+Requires(pre): selinux-policy >= %{selinux_policy_version}
>>>>>>>>Requires: selinux-policy >= %{selinux_policy_version}
>>>>>>>>Requires(post): selinux-policy-base
>>>>>>>>Requires: slapi-nis >= 0.54.2-1
>>>>>>>If we have it in Requires(pre), we don't need it in Requires, as
>>>>>>>Requires(pre) is a superset of guarantees that Requires gives you.
>>>>>>
>>>>>>Martin (CCed) told me Requires(pre) does not imply Requires.
>>>>>See http://rpm.org/api/4.4.2.2/tsort.html (available since 2007):
>>>>>----------------
>>>>>Since the only way out of a dependency loop is to snip the loop
>>>>>somewhere, rpm uses hints from Requires: dependencies to distinguish
>>>>>co-requisite (these are not needed to install, only to use, a package)
>>>>>from pre-requisite (these are guaranteed to be installed before the
>>>>>package that includes the dependency) relations.
>>>>>----------------
>>>>>
>>>>>>>
>>>>>>>Requires(pre) ensures that selinux-policy of specific version is
>>>>>>>installed before pre scripts of freeipa-server would run, be it in the
>>>>>>>same transaction or in a previous one.
>>>>>>>
>>>>>>
>>>>>>Hmm, ipa-server-upgrade is run in posttrans. Should the Requires(pre)
>>>>>>be changed to Required(posttrans)?
>>>>>I don't think there is posttrans target. Perhaps, we can just make sure
>>>>>Requires(post) is enough.
>>>>
>>>>OK, let's try that. Updated patch attached.
>>>>
>>>
>>>Will this really make a difference? I thought the problem is caused by
>>>selinux-policy being installed after freeipa-server package upgrade. We already
>>>have Requires on selinux-policy, so I am not sure what is actually changed by
>>>this patch.
>>The change is that with Requires(pre) or Requires(post) we are
>>guaranteed that selinux-policy is installed and available before our pre
>>or post scriptlets are run. With Requires only we are not guaranteed to
>>be installed after selinux-policy, only that it would be available as
>>part of the same transaction we are installed in.
>>
>>We don't really need to have Requires(pre) because we don't rely on
>>selinux-policy being available in pre scriptlet. Forcing Requires(pre)
>>doesn't help anyone else (rpm/yum/dnf need to solve dependency loops and
>>we are only complicating with Requires(pre) if we don't actually need
>>it). Thus, choosing Require(post) is more correct from distribution
>>point of view.
>
>Sure, but given that FreeIPA upgrade is run in the posttrans phase:
>
>%posttrans server
># This must be run in posttrans so that updates from previous
># execution that may no longer be shipped are not applied.
>/usr/sbin/ipa-server-upgrade --quiet >/dev/null || :
>
>I am now not sure how Requires(pre) or Requires(post) help here, in 
>all cases, the right selinux-policy should be there before all the 
>posttrans scripts are being run.
I've looked at the rpm source code and here is the list of all supported
requires/dependencies types:
https://github.com/rpm-software-management/rpm/blob/140744377b019e0de81d76d0931c32228d2ed57e/build/rpmfc.c#L1056

Requires(posttrans) is there so we could use this one too but it was
added only in 4.12-alpha which means it is missing in RHEL/CentOS 7, for
example, as they are only up to 4.11. 

>Maybe the new selinux-policy is required for certmonger itself or some 
>other event during upgrade?
No, I don't think so. However, we cannot set Requires(posttrans), thus
we should be using closest target before it, i.e. Requires(post).
-- 
/ Alexander Bokovoy