Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should not upgrade when pinned to non-allowed origin? #319

Open
matthijskooijman opened this issue Apr 29, 2022 · 12 comments · May be fixed by #334
Open

Should not upgrade when pinned to non-allowed origin? #319

matthijskooijman opened this issue Apr 29, 2022 · 12 comments · May be fixed by #334

Comments

@matthijskooijman
Copy link

I ran into an issue where u-a upgraded my Firefox in a way I had not expected. The situation is a bit specific, so I'm not sure what the correct behavior would be, but what happens now is at least surprising. Here's the situation.

  1. I'm running Ubuntu 22.04 with unattended-upgrades 2.8ubuntu1 and default settings.

  2. I added this PPA: https://launchpad.net/~mozillateam/+archive/ubuntu/ppa/+packages

  3. I installed firefox from that PPA

  4. I set up apt pinning to always prefer firefox from that PPA:

    Package: firefox*
    Pin: release o=LP-PPA-mozillateam
    Pin-Priority: 1000
    
  5. The PPA published a new version, making my locally installed version no longer match the pin (you can reproduce this by installing an older version from the PPA, e.g. https://launchpad.net/~mozillateam/+archive/ubuntu/ppa/+build/23545952`).

  6. Unattended-upgrades decides to upgrade the firefox package to the original Ubuntu version, replacing the PPA version.

I thought I had sufficiently configured the system to prefer the PPA version using the pin, but u-a seems to do something different. Given the PPA is not in Allowed-Origins, I would not expect the package to be upgraded to the new PPA version, but instead would expect u-a to just leave the package alone in this case. It seems that in practice u-a completely ignores packages outside of its Allowed-Origins (selecting the highest version, or highest priority probably, from the set of packages from allowed origins plus the already installed versions, I suppose?).

At step 5 above, here's what I get:

matthijs@dottie:~$ apt-cache policy firefox
firefox:
  Installed: 99.0.1+build1-0ubuntu0.22.04.1~mt1
  Candidate: 100.0+build1-0ubuntu0.22.04.1~mt1
  Version table:
     1:1snap1-0ubuntu2 500
        500 http://nl.archive.ubuntu.com/ubuntu jammy/main amd64 Packages
     100.0+build1-0ubuntu0.22.04.1~mt1 1000
        500 https://ppa.launchpadcontent.net/mozillateam/ppa/ubuntu jammy/main amd64 Packages
 *** 99.0.1+build1-0ubuntu0.22.04.1~mt1 100
        100 /var/lib/dpkg/status
     99.0-1 50
         50 http://ftp.nl.debian.org/debian sid/main amd64 Packages

From this, I would expect u-a to see that the highest prio version is from the PPA, so outside of its Allowed-Origins, so ignore it.

Instead, u-a chooses to upgrade (see u-a debug output at the end of this post).

matthijs@dottie:~$ sudo unattended-upgrade --debug --dry-run &> u-a.txt

u-a.txt

From the output, here's a bit relevant to the decision to update. It seems that u-a also sees that something is weird (sanity check failed for: set() : no package is selected to be upgraded or installed), but maybe I'm misinterpreting that entry.

Checking: firefox ([<Origin component:'main' archive:'jammy' origin:'LP-PPA-mozillateam' label:'Firefox ESR and Thunderbird stable builds' site:'ppa.launchpadcontent.net' isTrusted:True>])
sanity check failed for: {'firefox=100.0+build1-0ubuntu0.22.04.1~mt1'} : pkg firefox is not in an allowed origin
falling back to adjusting firefox's dependencies
[...unrelated output about libusb snipped by Matthijs...]
sanity check failed for: set() : no package is selected to be upgraded or installed
pkgs that look like they should be upgraded: firefox

I can of course fix this by changing u-a config (add the PPA to Allowed-Origins or blacklist the package), but I'd expect this to be not needed (also, I would prefer not spreading around this configuration between u-a config and pinning preferences).

As a workaround, I now added a pin for the non-PPA firefox versions with prio 50 (so it would be less than the 100 for the already installed version), which seems to make u-a not upgrade the package anymore.

Package: firefox*
Pin: release o=LP-PPA-mozillateam
Pin-Priority: 1000

Package: firefox*
Pin: release o=Ubuntu
Pin-Priority: 50
@julian-klode
Copy link
Contributor

julian-klode commented Apr 29, 2022

This is hard. You see, imagine your PPA were Ubuntu -updates, it should not be installing from there, but from release pocket or security. So the not-allowed origin must be ignored, hence the jammy one becomes the highest available version and is installed.

So unattended-upgrades and third-party replacement repositories don't mix well. Another reason why I want to change apt so that PPAs by default can only add packages, not replace official ones, to prevent weird effects in all sorts of places. By well tracking the "o=" value and not allowing that to change between upgrades.

This would then also fix this issue presumably, as it would stick firefox into the PPA origin if you install it from there, and it would not allow switching origins, although u-u might need to learn about this (not sure).

In any case, expressing your preference as "I don't want Ubuntu's version" works better than expressing it as "I want the PPA one", as you noticed.

Presumably if the candidate has priority > 1000, it might be a good idea to not allow upgrades from other origins (< 1000), as it will always go back to the 1000 origin on next dist-upgrade.

@julian-klode
Copy link
Contributor

You can try and see if

diff --git a/unattended-upgrade b/unattended-upgrade
index 802d4d5..fc32175 100755
--- a/unattended-upgrade
+++ b/unattended-upgrade
@@ -191,9 +191,11 @@ class UnattendedUpgradesCache(apt.Cache):
             logging.debug(
                 "Package %s has a higher version available, checking if it is "
                 "from an allowed origin and is not pinned down.", pkg.name)
+            fixed_version = any(v.policy_priority >= 1000 for v in pkg.versions)
             for v in pkg.versions:
                 if pkg.installed < v \
                    and pkg.installed.policy_priority <= v.policy_priority \
+                   and (not fixed_version or v.policy_priority >= 1000) \
                    and is_in_allowed_origin(v, self.allowed_origins):
                     return v
         return None

makes you happy. I think it's not yet optimal, it should not pick the highest version that's allowed but the highest (priority, version) pair, but that's an easy change.

@julian-klode
Copy link
Contributor

I think this is the right approach though:

diff --git a/unattended-upgrade b/unattended-upgrade
index 802d4d5..ea230aa 100755
--- a/unattended-upgrade
+++ b/unattended-upgrade
@@ -191,11 +191,20 @@ class UnattendedUpgradesCache(apt.Cache):
             logging.debug(
                 "Package %s has a higher version available, checking if it is "
                 "from an allowed origin and is not pinned down.", pkg.name)
-            for v in pkg.versions:
-                if pkg.installed < v \
-                   and pkg.installed.policy_priority <= v.policy_priority \
-                   and is_in_allowed_origin(v, self.allowed_origins):
-                    return v
+            fixed_version = any(v.policy_priority >= 1000 for v in pkg.versions)
+            _priority, best_version = max(
+                (
+                    (v.policy_priority, v) for v in pkg.versions
+                    if (pkg.installed < v
+                        and pkg.installed.policy_priority <= v.policy_priority
+                        and (not fixed_version or v.policy_priority >= 1000)
+                        and is_in_allowed_origin(v, self.allowed_origins))
+                ),
+                default=(None, None)
+            )
+
+            return best_version
+
         return None
 
     def find_kept_packages(self, dry_run):

One could avoid the two iterations and triple policy_priority reads, but oh well.

@julian-klode
Copy link
Contributor

Oh I think this is a totally unrelated function actually, it's just for printing kept back packages.

@matthijskooijman
Copy link
Author

This is hard. You see, imagine your PPA were Ubuntu -updates, it should not be installing from there, but from release pocket or security. So the not-allowed origin must be ignored, hence the jammy one becomes the highest available version and is installed.

Hm, good point.

So unattended-upgrades and third-party replacement repositories don't mix well. Another reason why I want to change apt so that PPAs by default can only add packages, not replace official ones, to prevent weird effects in all sorts of places. By well tracking the "o=" value and not allowing that to change between upgrades.

This would then also fix this issue presumably, as it would stick firefox into the PPA origin if you install it from there, and it would not allow switching origins, although u-u might need to learn about this (not sure).

Yeah, I think part of the issue is that apt does not remember where the local package version came from. As soon as a new version is available in the archives, the locally-installed version can no longer be tied to any origin and can also no longer be meaningfully pinned. Remembering the origin for a package, as you suggest, might indeed be an elegant way to fix this (and will probably lead to horrible breakage when origin names are changed, of course...)

Presumably if the candidate has priority > 1000, it might be a good idea to not allow upgrades from other origins (< 1000), as it will always go back to the 1000 origin on next dist-upgrade.

Yeah, that might also work. Relevant here is that the firefox PPA version numbers are smaller than the main Ubuntu versions (if they were not, u-a would just refuse to downgrade and this problem did not exist). So having special rules for >= 1000 (which does allow downgrades, and as you say, the next upgrade will indeed downgrade again).

You can try and see if makes you happy. I think it's not yet optimal, it should not pick the highest version that's allowed but the highest (priority, version) pair, but that's an easy change.

Right, that seems to make sense. I'm not sure what you mean by the not yet optimal, isn't the pkg.versions list already sorted by (prio, version)? If not, isn't that already a problem of the existing implementation?

Oh I think this is a totally unrelated function actually, it's just for printing kept back packages.

Yeah, I noticed that when I tried to test your first patch. For completeness: it doesn't work indeed ;-p

@julian-klode
Copy link
Contributor

julian-klode commented Apr 29, 2022

OK so what happens is that unattended-upgrades sets the NEVER_PIN on disallowed origins which is not overridable by preferences. This is by design, as that was designed primarily for ESM repositories to be in your sources.list to inform you about updates you could get, but not actually install from them if they are enrolled.

Allowing you to override the pin for >= 1000 would mean that if you had an o=UbuntuESM pin to 1000, it would suddenly try to install an ESM update that's not actually ready for you. Bad.

So this would require APT work to have two meanings of never pin:

  • Never install from this repository
  • Never install from this repository, unless forced by >= 1000

It's hard. I think it's won't fix.

@matthijskooijman
Copy link
Author

I'm not sure I follow the ESM usecase (I have no idea how ESM works, though)? If ESM sources are in your sources.list, but not in AllowedOrigins, then u-a would never install them, even with the change you proposed, right? And whether apt would try to install from these sources depends on their prio and version numbers? How is this different from any other source?

It's hard. I think it's won't fix.

I can imagine. I won't be too sad ;-)

@markkrj
Copy link

markkrj commented Aug 29, 2022

Hi, I was also bitten by this with a software we use. So, I added #334 as a way to blacklist repositories from being updated by u-u. I don't know if such approach is acceptable, so I decided to get feedback from maintainers before further working on tests and docs. What do you guys think? @mvo5 @julian-klode @rbalint

@rbalint
Copy link
Collaborator

rbalint commented Aug 30, 2022

@matthijskooijman @markkrj regarding the original problem everything seems to be working as designed and IMO the design is still good. The trick is not pinning up the PPA version of firefox, but pinning the archive's version down, as described in https://balintreczey.hu/blog/firefox-on-ubuntu-22-04-from-deb-not-from-snap/ .

@matthijskooijman
Copy link
Author

@rbalint, Thanks for pointing that out. I guess that's what I effectively ended up doing (see the workaround at the end of the original issue above), though I also increased prio for the PPA version (which I guess is not strictly required).

I wonder if the documentation could somehow be improved to point out this caveat, though I'm not sure where this should be then (I guess I did not look at any documentation for u-u specifically, I just set up my original pins based on experience with and documentation of apt - so maybe this caveat could be documented in the apt_preferences manpage, but that is out of scope for this issue I guess).

@markkrj
Copy link

markkrj commented Aug 30, 2022

@rbalint That way we end up having to use u-u for managing third party repository, or, with @matthijskooijman workaround, for every package we install from a third party repository, we need to pin it down in official repos. My solution allows pinning down the entire third party repository while still using u-u for official repos, so we can manage updates from third party repositories manually, without introducing drawbacks (at least I didn't find one).

@phd
Copy link

phd commented Jul 23, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants