Bug 915726

Tested on rpm-4.8.0-27.el6.i686.

This should be fixed in rpm >= 4.11, along with myriad of other similar corner-cases in %config file handling. Backporting those fixes is an entirely different matter though, it'd be painful and risky to backport them even to 4.10, never mind 4.8.x of RHEL-6.

I dont remember the details off-hand and its *possible* this particular case might be borderline doable, but determining the feasibility will require much closer inspection.

This request was not resolved in time for the current release.
Red Hat invites you to ask your support representative to
propose this request, if still desired, for consideration in
the next release of Red Hat Enterprise Linux.

From https://bugzilla.redhat.com/show_bug.cgi?id=445202#c10:
> This is no longer a dependency of the System Certificates work. The
> processing of the symlinks required custom processing, which Kai has written
> a script for.

I take this is the same as "migrating ca-certificates to the p11-kit-based implementation". Closing NEXTRELEASE as per comment #3: backporting the related and required fixes to the much older codebase would be not only painful but very risky as well.

The custom solution that Kai did is fine for Fedora. It bends the rules of %config(noreplace), and does not produce .rpmnew files. The custom solution produces .rpmsave files. 

The above solution is not fine for RHEL 6.x, when we include the Shared System Certificates feature in RHEL 6.x. Administrators will expect the right .rpmnew behavior for their modified ca-bundle.crt and ca-bundle.trust.crt files.

Reopening on RHEL 6.5.

I'd suggest you investigate the script possibility for rhel-6 as well. We can certainly investigate whether this particular corner case is backportable within reason but I seriously doubt it.

I'm scratching my head on how to do such a script, but we can try. This is significantly different from the Fedora script, which is a hack that writes out an .rpmsave.

But I'll put down here what I understand so far. 

Goal: The new rpm is replacing certain %config(noreplace) files with symlinks. If those files have been modified then the symlinks should go to .rpmnew.

 * We would run 'rpm' from within the %pre to detect if the files in question 
   have been changed. Is running rpm recursively from within an rpm install's
   %pre section a tested and supported thing to do?

 * Once we know whether the files have changed, we would change the path to 
   which the symlinks coming from the new rpm are installed. How do we do this?
   Any ideas here?

 * Perhaps we could make a backup in %pre, let the symlink overwrite the file
   and then move the symlink in %post, and write the original file back.
   This is really racy and fragile.

But yes, the open question is whether this big nasty hack around the brokenness of rpm in 6.5 is worse than backporting an rpm fix for this case.

Calling rpm from within rpm is not an option at all. Even if it might seem to work on casual testing, it will go crazy sooner or later when you least suspect.
Something like the third option of always backing up (and perhaps shuffling it around if the backup happens to be identical to the created file) is on the lines I was thinking.

I wouldn't be suggesting such dirty kludges around what's obviously a bug in rpm if the area wasn't such a minefield. I know it sounds like a case of "how hard can it possibly be?" but I spent a good part of a month going through it last year and the answer is: very tricky, time-consuming and treacherous. The risk of introducing new regressions is high as the rpm 4.8.x test-suite barely touches this ground and upstream test-cases cannot be used without backporting a huge amount of test-suite related other work first. The related upstream work is way too invasive to backport as such, so I'll have to see whether this hole can be plugged with a bit of duct-tape separately.

(In reply to comment #9)
> Calling rpm from within rpm is not an option at all. Even if it might seem
> to work on casual testing, it will go crazy sooner or later when you least
> suspect.

Okay, so that prevents the whole concept of using a script to work around the rpm brokenness. Unless you know of another way to detect whether %config written by a previous rpm have changed or not?

> Something like the third option of always backing up (and perhaps shuffling
> it around if the backup happens to be identical to the created file) is on
> the lines I was thinking.

It wasn't listed as an option. But rather something we'd need to do in addition to detecting whether the %config(noreplace) file has been changed. So now the question is: How can we detect whether a %config file has changed without running 'rpm'?

But I agree that if we can detect the change of the file, then we can probably hack around this with the ugly script.

> I'll have to see whether this hole can be plugged with a bit of duct-tape
> separately.

Yes, I imagine that's what we'd have to do, if we can't find a way to make the script work.

(In reply to comment #10)
> It wasn't listed as an option. But rather something we'd need to do in
> addition to detecting whether the %config(noreplace) file has been changed.
> So now the question is: How can we detect whether a %config file has changed
> without running 'rpm'?

An extremely ugly way might be to ship a %config(noreplace) file with known contents in the "new" RPM, and in %post check whether the file actually has the known contents (== has replaced an earlier unmodified file).

But when the "new" RPM is updated to an "even newer" RPM, the "even newer" RPM will see something else - the %post scrip thas replaced the file with known contents by a symlink.

So something like
> if ($path is not a symlink to $new_destination) && ($path is a regular file of known contents) {
>     rm -f $path; ln -s $new_destination $path; }


Can something like the above work?

That is really nasty, but it might work.

Yeah, detecting whether a config file has changed without calling rpm requires either shipping the known content separately, or shipping a digest (sha256 or whatever) of it so the script can compare it without having the actual content.

Ugly it is either way, sure.

(In reply to comment #13)
> Yeah, detecting whether a config file has changed without calling rpm
> requires either shipping the known content separately, or shipping a digest
> (sha256 or whatever) of it so the script can compare it without having the
> actual content.

Not *a* digest, but every possible digest for every one of these files ever shipped by RHEL 6.5. Otherwise boom, data loss for the customer. 

> Ugly it is either way, sure.

Not just ugly. Completely broken.

Oh well... devel ack for applying some minimal band-aid to rpm to get a backup created in this case.

After digging into it a bit, the bug was introduced by this patch:

commit e64bf5b93ab689e6031fce4489e4ae38ebaebef1
Author: Panu Matilainen <pmatilai>
Date:   Tue Aug 28 09:04:09 2007 +0300

    Avoid .rpmnew when the file hasn't changed in package (rhbz#194246)
    
    The current behavior of %config(noreplace) creates a .rpmnewfile iff the typ
    of the current file has been changed wrto what was originally installed.
    
    The patch changes this behavior so when old and new (in db and in package) i
    identical -> not changed, the function returns FA_SKIP -> it won't clobber
    anything, it simply skips installation of the file from the package.
    This patch handles also the opposite case when old and new packages contain
    %config symlink and we have regular file on disk.
    
    Patch from Tomas Mraz.

In particular this line change:

--- a/lib/rpmfi.c
+++ b/lib/rpmfi.c
@@ -579,7 +579,7 @@ fileAction rpmfiDecideFate(const rpmfi ofi, rpmfi nfi, int s
     if (newWhat == XDIR)
        return FA_CREATE;
 
-    if (diskWhat != newWhat)
+    if (diskWhat != newWhat && dbWhat != REG && dbWhat != LINK)
        return save;
     else if (newWhat != dbWhat && diskWhat != dbWhat)
        return save;

(In reply to comment #17)
> -    if (diskWhat != newWhat)
> +    if (diskWhat != newWhat && dbWhat != REG && dbWhat != LINK)
>         return save;
>      else if (newWhat != dbWhat && diskWhat != dbWhat)
>         return save;

So the problem here is that the additional conditions to the 'if' statement are intended to cause the execution to fall through to the MD5 code below. But what it does instead is cause execution to fall through to a later 'else if (dbWhat != newWhat)' which returns FA_CREATE.

Created attachment 741903 [details]
Fix regression which causes data loss of config files when type changed

This fixes a regression in e64bf5b93ab689e6031fce4489e4ae38ebaebef1
which caused modified %config(noreplace) files to be overwritten
when the file was replaced by a symlink in the rpm.

This patch adjusts the conditions to match the behavior of the original
patch while accounting for these additional corner cases.

e64bf5b adjusts the conditions in which changing of file types cause
file comparisons to occur. However in doing this it caused certain
cases to fall through directly to FA_CREATE without comparison.

Will attach test case files...

Behavior before the patch:

# rpm -i builds/config-1-1.noarch.rpm 
# rpm -V config
# echo "modified" > /etc/cfgxx/config-file; echo "modified" > /etc/cfgxx/becomes-link 
# rpm -V config
S.5....T.  c /etc/cfgxx/becomes-link
S.5....T.  c /etc/cfgxx/config-file
# ls -l /etc/cfgxx
total 8
-rw-r--r--. 1 root root 9 30. Apr 17:40 becomes-link
-rw-r--r--. 1 root root 9 30. Apr 17:40 config-file
lrwxrwxrwx. 1 root root 9 30. Apr 17:39 config-link -> /dev/null
# rpm -U builds/config-2-1.noarch.rpm 
warning: /etc/cfgxx/config-file created as /etc/cfgxx/config-file.rpmnew
# ls -l /etc/cfgxx
total 12
lrwxrwxrwx. 1 root root 11 30. Apr 17:40 becomes-link -> normal-file
-rw-r--r--. 1 root root  9 30. Apr 17:40 config-file
-rw-r--r--. 1 root root 48 30. Apr 17:37 config-file.rpmnew
lrwxrwxrwx. 1 root root  9 30. Apr 17:40 config-link -> /dev/null
-rw-r--r--. 1 root root 26 30. Apr 17:37 normal-file
# rpm -V config
S.5....T.  c /etc/cfgxx/config-file

Note the data loss of modifications to 'becomes-link' after the upgrade.

Expected behavior:

# /opt/build/bin/rpm -i builds/config-1-1.noarch.rpm 
# /opt/build/bin/rpm -V config
# echo "modified" > /etc/cfgxx/config-file; echo "modified" > /etc/cfgxx/becomes-link 
# /opt/build/bin/rpm -V config
S.5....T.  c /etc/cfgxx/becomes-link
S.5....T.  c /etc/cfgxx/config-file
# ls -l /etc/cfgxx
total 8
-rw-r--r--. 1 root root 9 30. Apr 17:38 becomes-link
-rw-r--r--. 1 root root 9 30. Apr 17:38 config-file
lrwxrwxrwx. 1 root root 9 30. Apr 17:38 config-link -> /dev/null
# /opt/build/bin/rpm -U builds/config-2-1.noarch.rpm 
warning: /etc/cfgxx/becomes-link created as /etc/cfgxx/becomes-link.rpmnew
warning: /etc/cfgxx/config-file created as /etc/cfgxx/config-file.rpmnew
# ls -l /etc/cfgxx
total 16
-rw-r--r--. 1 root root  9 30. Apr 17:38 becomes-link
lrwxrwxrwx. 1 root root 11 30. Apr 17:39 becomes-link.rpmnew -> normal-file
-rw-r--r--. 1 root root  9 30. Apr 17:38 config-file
-rw-r--r--. 1 root root 48 30. Apr 17:37 config-file.rpmnew
lrwxrwxrwx. 1 root root  9 30. Apr 17:39 config-link -> /dev/null
-rw-r--r--. 1 root root 26 30. Apr 17:37 normal-file
# /opt/build/bin/rpm -V config
SM5....T.  c /etc/cfgxx/becomes-link
S.5....T.  c /etc/cfgxx/config-file

Note that the new link 'becomes-link' has been redirected to 'becomes-link.rpmnew' as expected, and the original customer data remains in place.

Created attachment 741907 [details]
Test case rpm: original config files

Created attachment 741908 [details]
Test case rpm: replaced config files

Created attachment 741909 [details]
config-1.spec

Created attachment 741910 [details]
config-2.spec

Created attachment 741931 [details]
Patch for rpm-4.10.x

The relevant patches have been tested on:

tag: rpm-4.8.0-release (ie: RHEL 6.x)
branch: rpm 4.8.x (ie: 4.8.1)
branch: rpm 4.10.x (ie: 4.10.3.1, Fedora)

As Panu noted above, the patch is not necessary on rpm's git master (ie: 4.11.x)

Created attachment 742104 [details]
Patch for rpm-4.10.x

Comment on attachment 741903 [details]
Fix regression which causes data loss of config files when type changed

The patch does not regress the original bugfix that caused this regression and it should work for this bug report. I do not know though whether it does not regress some additional corner case. It's apparent that the rpmfiDecideFate() code is extremely sensitive to that.

It turns out we are unable to depend on this for delivery of the Shared System Certificates feature in RHEL 6.5. This is because rpm would need to be updated in a separate transaction than the rpms that rely on this fix.

We will either need to find a solution to work around this, or due to this bug, wait until 6.6 to deliver the Shared System Certificates feature, having first delivered this fix as part of 6.5.

In both cases, I still think this is a good thing to fix as a regression in RHEL 6.x. Others have reported this as a bug from time to time.

Stef, thanks for digging into this, your patch looks just the kind of minimal bandaid I had in mind but hadn't had a chance to look into it yet. 

I also hadn't realized that this was actually a regression introduced by Tomas' .rpmnew avoidance patch, I assumed it had always been broken... because it actually has always been broken on general level: deciding whether a backup is needed on an existing file cannot be correctly done before comparing the on-disk content to what's in db, so the whole pile of fooWhat vs barWhat comparisons was always incorrect to begin with:
http://rpm.org/gitweb?p=rpm.git;a=commitdiff;h=0936fbc7bb51f4643e05dfe9a98ac30c82d215e2

Panu thanks for looking at the patch.

(In reply to Panu Matilainen from comment #13)
> Yeah, detecting whether a config file has changed without calling rpm
> requires either shipping the known content separately, or shipping a digest
> (sha256 or whatever) of it so the script can compare it without having the
> actual content.

For the record, I tried this. And it doesn't work properly because rpm will always think the file has changed, after the new rpm has been installed. Since what's in the rpm does not match what the %post script later replaced it with. This then confuses future upgrades/downgrades.

(In reply to Stef Walter from comment #33)
> For the record, I tried this. And it doesn't work properly because rpm will
> always think the file has changed, after the new rpm has been installed.
> Since what's in the rpm does not match what the %post script later replaced
> it with. This then confuses future upgrades/downgrades.

By the way, this comment was about Mirek's and Panu's work around suggestions. 

Kai found a different work around which we have implemented in Fedora 19, which is stable and works, but obviously isn't beautiful.

See:

http://pkgs.fedoraproject.org/cgit/ca-certificates.git/tree/ca-certificates.spec

and this:

https://fedoraproject.org/wiki/QA:Testcase_Certificate_Trust:Upgrade_Bundle

Further explanation in bug 544376 comment 45

This request was evaluated by Red Hat Product Management for
inclusion in a Red Hat Enterprise Linux release.  Product
Management has requested further review of this request by
Red Hat Engineering, for potential inclusion in a Red Hat
Enterprise Linux release for currently deployed products.
This request is not yet committed for inclusion in a release.

Because the patch is very risky and this functionality is not needed any more, closing this bug as wontfix.

Comment on attachment 741903 [details]
Fix regression which causes data loss of config files when type changed

Clearing review flag