nixos/miniflux: vendor & use upstream systemd service

[NotAShelf]

Description of changes

Continuation of #271605. Rebased around master and removed lib from two
instances of lib.mkIf as mkIf is already in scope.

Supersedes #271605.

Vendor upstream systemd service in miniflux package and add it to systemd.packages

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:
  • NixOS test(s)
    (look inside
    nixos/tests)
  • and/or
    package tests
  • or, for functions and "core" functionality, tests in
    lib/tests or
    pkgs/test
  • made sure NixOS tests are
    linked
    to the relevant packages
• 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.

[aanderse]

wow upstream has such a nice systemd service! why are we not directly using it via systemd.packages? let's fix that right here in this very PR!

[NotAShelf]

Seems simple enough, but that would require me to change the derivation for miniflux to also ship the systemd service unit. Should I repurpose this PR?

Also, I am not entirely convinced if systemd.packages is actually a better approach. What if the user wants to override the unit, how is that handled under systemd.packages in comparison to the service config set that can be overriden?

[aanderse]

Seems simple enough, but that would require me to change the derivation for miniflux to also ship the systemd service unit. Should I repurpose this PR?

if you're willing to that would be great!

Also, I am not entirely convinced if systemd.packages is actually a better approach. What if the user wants to override the unit, how is that handled under systemd.packages in comparison to the service config set that can be overriden?

you're entirely about to override everything in the unit without significant difference - and no one knows upstream better than upstream! they really ship a good unit so to me it is a clear win to utilize it

[NotAShelf]

you're entirely about to override everything in the unit without significant difference - and no one knows upstream better than upstream! 

What if the end-user wants to change service settings, though? From the top of my head I can think of a case where the user wants to de-harden the service, or try to change its wants or after (e.g. for Postgres). Does systemd.packages allow overriding for such cases?

[aanderse]

yes

[NotAShelf]

That's good to hear, I'll proceed with the change tomorrow morning.

Until then, would you mind telling me how the unit settings are overriden?

[aanderse]

for sure

as a quick example to override ExecStart in nix:

serviceConfig = {
  ExecStart = [
    ""
    "${cfg.package}/bin/... etc...";
  ];
};

this follows systemd rules to erase the impact of a previously set value... arch wiki writes a bit about this but upstream has more specific details

[NotAShelf]

@aanderse I've repurposed to PR to 1. vendor the upstream service and 2. use it in systemd.packages.

This is my first time using systemd.packages for a service however, and I am not sure how to resolve the failing test with nix build .#nixosTests.miniflux --builders "". Looking at other services, most of them seem to also vendor a <service>.socket, which Miniflux does not. What am I missing here?

[NotAShelf]

I've pushed some change to hopefully bring the service on par with the desired hardening status. Though I still can't seem to figure out why the NixOS test is failing for me, logs indicate that it's something to do with KVM modules in my kernel but I'd appreciate someone testing to see if it's failing for them as well.

[mweinelt]

default # [    3.888742] systemd[1]: miniflux.service: Service has more than one ExecStart= setting, which is only allowed for Type=oneshot services. Refusing.
default # [    3.931554] systemd[1]: miniflux.service: Cannot add dependency job, ignoring: Unit miniflux.service has a bad unit file setting.

[NotAShelf]

Test seems to be building now.

[NotAShelf]

I have forgotten about this...

Rebased to resolve conflicts with (#334257). @mweinelt if you could re-check the security report and give me your thoughts, I think we can get this merged soon.

[eclairevoyant]

overall gist of the PR looks fine, systemd-analyze security miniflux now shows 1.2 both before and after applying this PR.

@NotAShelf can you rebase on master and drop the merge commit?

[eclairevoyant]

@ofborg build nixosTests.miniflux

test failures

[eclairevoyant]

Though it works when I add these commits to my config and rebuild, tests appear to be failing:

default # [  968.400197] systemd[1]: miniflux.service: Scheduled restart job, restart counter is at 183.
default # [  968.405866] systemd[1]: Starting Miniflux database setup...
default # [  968.471223] n9qh3r1l0sqamvrqpxz3syvqgd7yrl06-miniflux-pre-start[5711]: NOTICE:  extension "hstore" already exists, skipping
default # [  968.476451] n9qh3r1l0sqamvrqpxz3syvqgd7yrl06-miniflux-pre-start[5711]: CREATE EXTENSION
default # [  968.482399] systemd[1]: miniflux-dbsetup.service: Deactivated successfully.
default # [  968.493215] systemd[1]: Finished Miniflux database setup.
default # [  968.497975] systemd[1]: miniflux.service: Failed to load environment files: No such file or directory
default # [  968.504917] systemd[1]: miniflux.service: Failed to spawn 'start' task: No such file or directory
default # [  968.506880] systemd[1]: miniflux.service: Failed with result 'resources'.
default # [  968.511316] systemd[1]: Failed to start Miniflux service.

[NotAShelf]

Okay this should be good to go now, thank you @adamcstephens for the pointer :)

[NotAShelf]

Are there any blockers left for me to fix? If not, I think we could get this in.

[emilylange]

I get that this is well intended and all, but I simply do not understand the benefit here.

All of a sudden, you have no longer access to the systemd service's configuration at eval time and need to merge the unit file shipped with the package and the overrides made in .nix by hand.

Or, alternatively, deploy it blindly and then check whatever systemctl cat and systemctl show make of it.

Not to mention in case cfg.package is a custom derivation, it suddenly also needs to provide the same unit file as upstream.

In my mind, this is a huge decline of usability and increase of overhead.

And finally, I don't believe every single maintainer and drive-by merger checks the whole upstream diff of a version bump for changes in the unit file and then also understands the implications of it - meaning there is genuine risk of regressions by some unassuming upstream unit file change.

Take for example literally this PR.
It first worsened the sandboxing and then had an EnvironmentFile= regression.
The latter is something we likely only noticed by luck, as the VM tests don't cover a lot of variety.

[NotAShelf]

You might have noticed, by scrolling up, that including the upstream systemd service was something I have added on request - not by my own discretion. Initially this PR was intended solely to close a stale PR (which hardens the systemd service, and nothing else). The tone of your message is entirely unwarranted.

Then came the first request to use the upstream service, which didn't hear any negative feedback. In fact, the response was "I don't particularly care how we reach hardening parity with what we have right now". Great.

Frankly I'm getting a bit tired of getting pushed back and forth. "Do this", "don't to that", "wait that's a wrong word change that" (and I'm not even given the courtesy of proper spelling)... Enough is enough.

Edit: The regression in the service was caused by an oversight in me resolving merge conflicts. Not a slight on anyone's behalf, but those things happen when something takes 4 months to make the least amount of progress.