Skip to content

nixos/miniflux: add options for all secret files#429983

Closed
jfly wants to merge 1 commit intoNixOS:masterfrom
jfly:miniflux-add-client-secret-files
Closed

nixos/miniflux: add options for all secret files#429983
jfly wants to merge 1 commit intoNixOS:masterfrom
jfly:miniflux-add-client-secret-files

Conversation

@jfly
Copy link
Contributor

@jfly jfly commented Jul 31, 2025

I added these new options:

  • services.miniflux.adminUsernameFile
  • services.miniflux.adminPasswordFile
  • services.miniflux.oauth2ClientSecret
  • services.miniflux.pgpassFile

I deprecated services.miniflux.adminCredentialsFile in favor of the
above options.

The new options all use systemd's LoadCredential functionality, so
people don't have to futz around with permissions. I learned the hard
way it's particularly annoying to create a PGPASSFILE that works: now
that's all abstracted away for people.

Things done

  • Built on platform:
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Ran nixpkgs-review on this PR. See nixpkgs-review usage.
  • Tested basic functionality of all binary files, usually in ./result/bin/.
  • Nixpkgs Release Notes
    • Package update: when the change is major or breaking.
  • NixOS Release Notes
    • Module addition: when adding a new NixOS module.
    • Module update: when the change is significant.
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

Add a 👍 reaction to pull requests you find important.

@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 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 Aug 1, 2025
@jfly
Copy link
Contributor Author

jfly commented Aug 1, 2025

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review
Commit: f8ce0d562fbc0805a6155cd1dc9305e16f662037


x86_64-linux

⏩ 1 package blacklisted:
  • nixos-install-tools

@nixpkgs-ci nixpkgs-ci bot added the 9.needs: reviewer This PR currently has no reviewers requested and needs attention. label Aug 1, 2025
@acid-bong
Copy link
Contributor

Since you're editing a module, the title prefix should be nixos/MODULENAME

I added these new options:

- `services.miniflux.adminUsernameFile`
- `services.miniflux.adminPasswordFile`
- `services.miniflux.oauth2ClientSecret`
- `services.miniflux.pgpassFile`

I deprecated `services.miniflux.adminCredentialsFile` in favor of the
above options.

The new options all use systemd's `LoadCredential` functionality, so
people don't have to futz around with permissions. I learned the hard
way it's particularly annoying to create a `PGPASSFILE` that works: now
that's all abstracted away for people.
@jfly jfly force-pushed the miniflux-add-client-secret-files branch from f8ce0d5 to 0a2fd56 Compare August 1, 2025 05:38
@jfly jfly changed the title miniflux: add options for all secret files nixos/miniflux: add options for all secret files Aug 1, 2025
@jfly
Copy link
Contributor Author

jfly commented Aug 20, 2025

(pinging the miniflux maintainers. please let me know if this is bad etiquette)

@nixpkgs-ci nixpkgs-ci bot removed the 9.needs: reviewer This PR currently has no reviewers requested and needs attention. label Aug 20, 2025
@adamcstephens
Copy link
Contributor

adamcstephens commented Aug 21, 2025

This would actually break my current use of the module. I'm not sure why we need all this complexity when EnvironmentFile will do everything we need? Maybe just renaming adminCredentialsFile to more clearly name that it's passed to EnvironmentFile.

pgpass requiring the hostname:port really negates its value over specifying the database URL in an env, in my opinion. You can't change one without the other.

EnvironmentFile doesn't require you to mess with permissions, since it's read by systemd.

Here's what I currently have:

age.secrets."miniflux-secrets.env".file = ./secrets.env.age;

services.miniflux = {
  enable = true;
  adminCredentialsFile = config.age.secrets."miniflux-secrets.env".path;
};

Where the env file has

ADMIN_PASSWORD=
DATABASE_URL=
OAUTH2_CLIENT_SECRET=

Copy link
Contributor

@adamcstephens adamcstephens left a comment

Choose a reason for hiding this comment

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

Sorry, this is a no from me as-is. I don't see the value in the credentials file/script abstraction for most of the environment variables. We can rename the option to make it clear it's an EnvironmentFile and if really necessary expose a pgpass file option.

@jfly
Copy link
Contributor Author

jfly commented Sep 12, 2025

Sorry, this is a no from me as-is.

Ah, bummer. I was under the mistaken impression that the goal was for nixos modules to expose secrets as standalone options. I had sent in PRs in the past that were accepted that make this sort of change: #388853.

I've since talked to some people, and now understand that this is just a style thing that's up to the module maintainer.

I do still see value in exposing secrets as individual knobs, but this definitely isn't the venue to debate that. Perhaps vars (or something like it) will encourage us all to get on the same page someday.

@jfly jfly closed this Sep 12, 2025
@emilylange
Copy link
Member

Please also note that the approach you used here is unnecessarily indirect and also incomplete.

Indirect, because ADMIN_USERNAME, ADMIN_PASSWORD and OAUTH2_CLIENT_SECRET all have _FILE variants that should be used instead of export with $(< ) in systemd.services.miniflux.script.

Incomplete, because you forgot a bunch of additional secrets. In my opinion this should have been a free-form secrets option, similar to services.miniflux.config, with RFC42 in mind.

I, too, don't understand why you decided to go with PGPASS over DATABASE_URL or DATABASE_URL_FILE.

Either way, for those that may find this in the future or whatever, you can use e.g.

{
  services.miniflux.config.OAUTH2_CLIENT_SECRET_FILE = "/run/credentials/miniflux.service/OAUTH2_CLIENT_SECRET";
  systemd.services.miniflux.serviceConfig.LoadCredential = [
    "OAUTH2_CLIENT_SECRET:/run/secrets/miniflux/oauth2-client-secret"
  ];
}

or even "%d/OAUTH2_CLIENT_SECRET" instead of "/run/credentials/miniflux.service/OAUTH2_CLIENT_SECRET", but that's an implementation detail.

@jfly
Copy link
Contributor Author

jfly commented Sep 12, 2025

Thanks, @emilylange, good to know.

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: 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.

4 participants