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

postgresql{12,13}Packages.pg_safeupdate: 1.5 -> 1.4 #299136

Merged
merged 3 commits into from
Apr 13, 2024

Conversation

wolfgangwalther
Copy link
Contributor

pg_safeupdate was updated to 1.5 in #269755 (@marsam). v1.5 is not compatible with PostgreSQL 12 and 13 anymore, so those were marked as broken.

However, this blocks anyone using PostgreSQL 12 or 13 with pg_safeupdate from updating nixpkgs.

Instead, the old version should have been kept for PG 12 and 13. If we don't do this, then it's kind of pointless to keep older versions of PostgreSQL in nixpkgs at all.

A general guideline would be:

  • It's fine to introduce a new extension only supporting some of the PG versions.
  • It's not fine to remove an extension via update + marking broken.

One case where this is blocking an update is CI/devtools for PostgREST:

https://github.com/PostgREST/postgrest/blob/a5bb20bbf8e96cdc024bedfacf3b2df186305efe/default.nix#L59-L63

Here we test with all PG versions down to v12 - but can't update now, because pg_safeupdate won't build for 12 and 13 anymore.

Note: The same happened with timescaledb in #257439. Updating to v2.12 removed support for PostgreSQL 12 (this was marked broken in a later commit). From what I can tell, no other packages are currently affected. Let's discuss the general approach how to deal with this first, before I fix that, too.

Description of changes

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.05 Release Notes (or backporting 23.05 and 23.11 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.

@wolfgangwalther
Copy link
Contributor Author

A general guideline would be:

  • It's fine to introduce a new extension only supporting some of the PG versions.
  • It's not fine to remove an extension via update + marking broken.

[...]
Let's discuss the general approach how to deal with this first, before I fix that, too.

@thoughtpolice @danbst @globin @marsam @ivan @Ma27

@Ma27
Copy link
Member

Ma27 commented Mar 30, 2024

To the core issue: Is 1.4 still maintained? Most importantly, does it receive security fixes if any? I'm asking because if upstream doesn't adhere to the version lifecycle from postgresql we have bad cards already. I know that other (LTS) distros are patching out stuff more aggressively, but I don't think we can do it on a larger scale given how small we are (in comparison to e.g. Debian) vs. how many things we have packaged in here.

If both are well-maintained, then this is fine for me.
My problem however with a general policy is (short of the upstream maintenance topic): what if the rest of the package has changed significantly as well (e.g. the build system is completely rewritten). Then I'd have to maintain two packages effectively. We should take that into account, not sure what to do about it though.

@wolfgangwalther
Copy link
Contributor Author

To the core issue: Is 1.4 still maintained?

In this specific case: No, 1.4 is not maintained, pg_safeupdate only maintains a single version at a time.

Most importantly, does it receive security fixes if any?

I don't think so - but it's not that 1.5 would have any security fixes either.

Literally the only thing that happened in the last years is... the removal of support for PostgreSQL before v14.

I'm asking because if upstream doesn't adhere to the version lifecycle from postgresql we have bad cards already.

True. But I don't think "marking an extension as broken, just because it's easier" is a good way to deal with that bad hand.

If both are well-maintained, then this is fine for me.

That's the case for ext/age.nix. This is maintained for each major version separately. In this case, the problem doesn't come up, because for such an extension nobody would ever "accidentally" mark it as broken for older versions.

My problem however with a general policy is (short of the upstream maintenance topic): what if the rest of the package has changed significantly as well (e.g. the build system is completely rewritten). Then I'd have to maintain two packages effectively. We should take that into account, not sure what to do about it though.

If that case comes up, then we'd have to deal with it, yes. But I guess that would be a very special case - and yes, maybe that means putting the extension in a subfolder and then having versioned nix files or so.

Unrelated to postgresql - what is the general policy on unmaintained packages in nixpkgs? My understanding is that they can be removed at some point, but don't really need to be. Of course if a package has a critical security issue and is not maintained, then it makes sense to remove it. But that would be the exception and not the rule, I guess.

We should apply that same approach here: The general guideline would be to keep those older versions around, to keep all major postgresql versions working. But if an older version should really miss some critical security fixes, then this can still be dropped.

@wolfgangwalther
Copy link
Contributor Author

We should apply that same approach here: The general guideline would be to keep those older versions around, to keep all major postgresql versions working. But if an older version should really miss some critical security fixes, then this can still be dropped.

Although in the case of timescaledb, this is not as easy as with pg_safeupdate. The number of releases, bugfixes and release notes is much bigger: https://github.com/timescale/timescaledb/releases

The latest CVE I can find was solved in 2.9.x, so even 2.11.2 (the last version to support PostgreSQL 12) should be safe. But it's certainly not easy to make that distinction of which older version is fine to keep and which is not.

pg_safeupdate was updated to 1.5 in NixOS#269755. v1.5 is not compatible with
PostgreSQL 12 and 13 anymore, so those were marked as broken.

However, this blocks anyone using PostgreSQL 12 or 13 with pg_safeupdate
from updating nixpkgs.

Instead, the old version should have been kept for PG 12 and 13.
@wolfgangwalther
Copy link
Contributor Author

A general guideline on how to deal with this situation would help me as a user depending on those packages. On the other side of the spectrum to ensure that... would be to add myself as a maintainer for the two extensions we depend on with PostgREST. This doesn't need any guidelines, but I should be notified in advance for such a change. Added commits to do just that. In this case, we also don't need to look at timescaledb now, because I don't need it.

This also has the advantage that the pg_safeupdate now has a maintainer, which it didn't have before ;).

I'm happy to go forward with this minimal approach.

@Ma27 if you happen to find a minute or two, could you look into the actual changes and merge? Those should be pretty straightforward. At least if you agree to keep v1.4 for PG12/13.

@ofborg ofborg bot added the 11.by: package-maintainer This PR was created by the maintainer of the package it changes label Mar 31, 2024
@wolfgangwalther
Copy link
Contributor Author

It would be great to get this merged soon, otherwise 24.05 will ship a broken pg_safeupdate for PG 12 + 13.

@Ma27
Copy link
Member

Ma27 commented Apr 13, 2024

True. But I don't think "marking an extension as broken, just because it's easier" is a good way to deal with that bad hand.

Generally, I disagree: if the maintainer power is insufficient (and well, nobody except me replied here so far), that's a perfectly fine approach.

At least if you agree to keep v1.4 for PG12/13.

I might've missed a few things I wanted to reply on, but: I reviewed the diff from 1.4..1.5 on my own and it's in fact just removal, so I'm fine with merging this patch.

However this is not a precedent for anything: here the case is literally that 1.5 is a 1.4, but without support for older versions, so I'm fine with it.

Nobody stepped up here so far, so I'm making that call here as co-maintainer of the subsystem, but generally this is something the package maintainer needs to decide I think.

Unrelated to postgresql - what is the general policy on unmaintained packages in nixpkgs? My understanding is that they can be removed at some point, but don't really need to be. Of course if a package has a critical security issue and is not maintained, then it makes sense to remove it. But that would be the exception and not the rule, I guess.

Assuming you mean that upstream is unmaintained: usually it will be thrown out as soon as it starts to cause issues / it starts to become an issue from a security perspective. However, a lot of stuff - just like this PR - are case-by-case decisions, so there's no real policy so far.

For versioned stuff such as php/nodejs/python/linux kernel, EOL versions are dropped pretty quickly after upstream maintenance is dropped (however an OS kernel or scripting languages have a relatively high attack surface, so we're especially strict there). OTOH we're generally trying to provide a package set that works as a whole as opposed to a collection of software packaged in various versions.

Generally, I wouldn't count on nixpkgs in this regard if I have unmaintained OSS software in my stack (even if that just means that I write an overlay where I maintain the derivation on my own). Big YMMV though.


A final note: I'm realizing that I'm trying to write down a ruleset here for that kind of problem, but this is not the intention of my comment (not even reasonable in this setting). Above are a few useful rules. The key takeaway is that when you start needing older versions or old software, be prepared to maintain your own overlays. This case is an exception where we can trivially make the live of a few downstream users (you in this case) easier without any downsides.

@Ma27 Ma27 merged commit 2705b90 into NixOS:master Apr 13, 2024
24 checks passed
@wolfgangwalther
Copy link
Contributor Author

Thanks for your explanation and a bit more context/background.

@wolfgangwalther wolfgangwalther deleted the pg-safeupdate branch April 13, 2024 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants