Skip to content

Conversation

@Ma27
Copy link
Member

@Ma27 Ma27 commented Nov 2, 2024

This will be EOL at the end of November, so there's little reason to
keep it in 24.11[1]. As discussed, we'd like to keep it for as long as
possible to make sure there's a state in nixpkgs that has the latest
minor of postgresql_12 available with the most recent CVEs fixed for
people who cannot upgrade[2].

This aspect has been made explicit in the manual now for the next .11
release.

Additionally regrouped the postgresql things in the release notes to
make sure these are all shown consecutively. Otherwise it's a little
hard to keep track of all the changes made to postgresql in 24.11.


cc @SuperSandro2000 who brought the stateVersion issue up
cc @NixOS/nixos-release-managers

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@Ma27 Ma27 requested a review from wolfgangwalther November 2, 2024 16:02
@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Nov 2, 2024
@nix-owners nix-owners bot requested a review from thoughtpolice November 2, 2024 16:03
@emilazy
Copy link
Member

emilazy commented Nov 2, 2024

However, I messed up and missed the freeze deadline, so we can't do breaking stuff like this

I’m pretty sure we can and should remove this, per https://nixos.github.io/release-wiki/Feature-Freeze-Announcement.html#pre-release-cleanup and the discussion at https://matrix.to/#/!aGqRytqbCECitOFhbt:nixos.org/$aprMJ4kXr8ehmjCgXgCfFCXPraCZXYn07Pe1ksnUydI?via=nixos.org&via=matrix.org&via=simonetti.nl. Our stability promise also has exceptions for security issues in general.

@wolfgangwalther
Copy link
Contributor

I’m pretty sure we can and should remove this

You beat me to it, I agree.

@Ma27
Copy link
Member Author

Ma27 commented Nov 2, 2024

Also fine by me, will adjust later this weekend.

@emilazy
Copy link
Member

emilazy commented Nov 2, 2024

FWIW I feel like our freeze policy is very vibes‐based and not as written down as it could be. I think the spirit is “do things that will decrease the risk of the release rather than increase it”, which welcomes dropping insecure packages we can’t support and rejects making a backwards‐incompatible update to a package used throughout the tree, but in practice we don’t seem to have much unambiguous text about the exact expectations and where we do it doesn’t always align with accepted norms. (Just a general remark.)

@SuperSandro2000
Copy link
Member

Our stability promise also has exceptions for security issues in general.

What difference does it make if we already mark it broken on stable? We are not going to do any big updates that would break it anyway and we can remove it on unstable immediately after the branch off.

If we remove it, people that missed it before, need to upgrade their postgres on the old stable but at least they notice it before starting the upgrade.

@wolfgangwalther
Copy link
Contributor

Removing it now leads to one problem though: The last postgresql_12 in nixpkgs-unstable will be the second to last minor version. This means whoever is stuck on v12 right now and just "goes forward as much as possible" or whoever pins v12 from an older commit (I saw that one, too) will not get the latest minor. (Side note: I know for a fact that a CVE will be fixed in the last minor, because I reported it)

I wonder whether we could do the following:

  • Remove postgresql_12 right after branch-off in 24.11 only.
  • Wait for the next minor release (which is up soon) and bump the version to the latest minor in master.
  • Remove v12 in master right after.

Or something else?

@wolfgangwalther
Copy link
Contributor

What difference does it make if we already mark it broken on stable?

Did you mean broken or insecure?

It's not broken. And it's not insecure either - there will be another minor release (the last one) coming up soon.

@wolfgangwalther
Copy link
Contributor

Also note https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/services/databases/postgresql.md#versioning-and-end-of-life-module-services-postgres-versioning, where we have this:

Thus:

  • In September/October the new major version will be released and added to nixos-unstable.
  • In November the last minor version for the oldest major will be released.
  • Both the current stable .05 release and nixos-unstable should be updated to the latest minor.
  • In November, before branch-off for the .11 release, the EOL-ed major will be removed from nixos-unstable.

Of course.. this only works when the last minor release actually comes in before branch-off 🙈

Looking at the last release dates for postgresql at https://www.postgresql.org/support/versioning/ it would seem that the next set of minor releases should be released within a week or so.

Since the branch-off is scheduled for the 14th of November, we should wait until the 12th/13th to hopefully get the last minor release merged first - then we can remove v12 right after.

@emilazy
Copy link
Member

emilazy commented Nov 2, 2024

To be honest I don’t think we need to concern ourselves too much with people pinning an old Nixpkgs to get EOL software, they’re not going to get fixes for the next CVE anyway…

But we could also just bump it on the release branch after branch‐off. (Edit: I guess that’s what you were proposing avoiding because of people pinning old unstable versions.)

@wolfgangwalther
Copy link
Contributor

But we could also just bump it on the release branch after branch‐off. (Edit: I guess that’s what you were proposing avoiding because of people pinning old unstable versions.)

Well my goal is two-fold:

  • Never have v12 make it into 24.11 at all.
  • Remove v12 from nixpkgs only after bumping it to the latest minor.

I think both are reasonable (at least on their own :D).

@emilazy
Copy link
Member

emilazy commented Nov 2, 2024

I think it’s questionable whether it will line up with Hydra timing such that a build of the last version will even be cached at all, so I’m not sure optimizing for the second goal makes too much sense.

@wolfgangwalther
Copy link
Contributor

I think it’s questionable whether it will line up with Hydra timing such that a build of the last version will even be cached at all, so I’m not sure optimizing for the second goal makes too much sense.

I'm not too concerned about caching, but even that could work out. It all depends on when the next minor release comes out. Looking at the last minor releases they were always released on a Monday. Surely not tomorrow, that's a fact. My best guess right now would be 11th of November. So update on the 11th, remove on the 13th. That should work out?

In any case I don't think it hurts to wait another week with the removal. A removal doesn't cause any rebuilds.

@Ma27
Copy link
Member Author

Ma27 commented Nov 2, 2024

If we remove it, people that missed it before, need to upgrade their postgres on the old stable but at least they notice it before starting the upgrade.

I mean, it's understandable that this happens by accident when relying on the stateVersion, but this is not the case here. Unless you explicitly pinned to postgresql_12, you didn't end up with a machine using it. And in that case I think it's a fair expectation that administrators of such systems also monitor how long they can do this (obviously we'll need a release-notes entry -- if somebody doesn't read those at all, they will run into more trouble anyways I'd argue).

In any case I don't think it hurts to wait another week with the removal. A removal doesn't cause any rebuilds.

Fine by me.
I'll remove postgresql_12 on this branch then and update the release notes accordingly (also our versioning policy). The PR will be marked as draft until postgresql_12 has its update. Does that sound reasonable?

@Ma27 Ma27 marked this pull request as draft November 2, 2024 17:44
@github-actions github-actions bot added 8.has: documentation This PR adds or changes documentation 8.has: changelog This PR adds or changes release notes labels Nov 2, 2024
@Ma27 Ma27 added this to the 24.11 milestone Nov 2, 2024
@Ma27 Ma27 changed the title postgresql_12: mark as insecure postgresql_12: remove Nov 2, 2024
@ofborg ofborg bot added 8.has: clean-up This PR removes packages or removes other cruft 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Nov 3, 2024
@wolfgangwalther
Copy link
Contributor

wolfgangwalther commented Nov 14, 2024

So, the new minors were released today, a couple of days later than expected. This means we have already branched-off for 24.11. Thus - we should backport this PR, once we have merged and backported #355965 (and have waited a bit for the cache to catch up).

@Ma27 Ma27 marked this pull request as ready for review November 14, 2024 21:58
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't currently build this test, because of some broken dependency, but I wonder whether we can change this to just postgresql instead, so we don't need to bump this every year?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be ideal but for future reference in case anyone is reading this later on, it does depend on the ecosystem. With something like postgresql, I guess it wouldn't affect much but with something like LLVM or Flutter, lots of major changes can make it difficult to upgrade so those should be set to a fixed version.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, makes sense. Indeed I think the chances for PostgreSQL are rather low to cause breaks that way. In any case, even if we decide to stay with a pinned version, we should try to bump it as far as possible and not to the now-oldest-one, I guess.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think even for things like LLVM it makes sense to keep tests unpinned: it makes them do their job of spotting regressions when the default is bumped.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't currently build this test, because of some broken dependency

It built when I prepared the patches initially. Right now, we can't test it because of some other dependencies apparently, so I'll leave it that way.
cc pleroma maintainers @picnoir @kloenk @yayayayaka

but I wonder whether we can change this to just postgresql instead, so we don't need to bump this every year?

To me it seemed that the easiest thing I could do was to go one version forward and see if the test builds.
If so, good (the rest is IMHO a problem for the maintainer). If not, the maintainers get a CC and that's it. But yeah, I guess I'll try the default package first next time.

This will be EOL at the end of November, so there's little reason to
keep it in 24.11[1]. As discussed, we'd like to keep it for as long as
possible to make sure there's a state in nixpkgs that has the latest
minor of postgresql_12 available with the most recent CVEs fixed for
people who cannot upgrade[2].

This aspect has been made explicit in the manual now for the next .11
release.

During the discussions it has been brought up that if people just do
`services.postgresql.enable = true;` and let the code decide the
postgresql version based on `system.stateVersion`, there's a chance that
such EOL dates will be missed. To make this harder, a warning will now
be raised when using the stateVersion-condition and the oldest still
available major is selected.

Additionally regrouped the postgresql things in the release notes to
make sure these are all shown consecutively. Otherwise it's a little
hard to keep track of all the changes made to postgresql in 24.11.

[1] https://endoflife.date/postgresql
[2] NixOS#353158 (comment)
@Ma27 Ma27 merged commit 5c01691 into NixOS:master Nov 15, 2024
github-actions bot pushed a commit that referenced this pull request Nov 15, 2024
This will be EOL at the end of November, so there's little reason to
keep it in 24.11[1]. As discussed, we'd like to keep it for as long as
possible to make sure there's a state in nixpkgs that has the latest
minor of postgresql_12 available with the most recent CVEs fixed for
people who cannot upgrade[2].

This aspect has been made explicit in the manual now for the next .11
release.

During the discussions it has been brought up that if people just do
`services.postgresql.enable = true;` and let the code decide the
postgresql version based on `system.stateVersion`, there's a chance that
such EOL dates will be missed. To make this harder, a warning will now
be raised when using the stateVersion-condition and the oldest still
available major is selected.

Additionally regrouped the postgresql things in the release notes to
make sure these are all shown consecutively. Otherwise it's a little
hard to keep track of all the changes made to postgresql in 24.11.

[1] https://endoflife.date/postgresql
[2] #353158 (comment)

(cherry picked from commit 0b3eef7)
@github-actions
Copy link
Contributor

Successfully created backport PR for release-24.11:

@Ma27 Ma27 deleted the postgresql-eol branch November 15, 2024 11:15
@wolfgangwalther
Copy link
Contributor

There has just been an announcement on the PostgreSQL hackers mailing list that an out-of-cycle release is scheduled for next week.

This will also be done for the otherwise-EOL v12 major.

There are two reasons for this:

  • One is, that one of the CVE fixes broke ALTER USER ... SET ROLE .... This in itself is a regression, but not security critical.
  • ABI incompatibilities between minor versions, which means that all extensions have to be rebuilt against the new minor release. This is entirely irrelevant for us, because we rebuild anyway.

Given that we have already removed v12, the last minor that we had in tree for v12 will not be the most up2date. I don't see that as critical, though, because all the known CVE's had been fixed with that last update before removal, so we are still in a good state.

TLDR: That's just for the record, proceed as planned.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog This PR adds or changes release notes 8.has: clean-up This PR removes packages or removes other cruft 8.has: documentation This PR adds or changes documentation 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants