Skip to content

lldap: document settings related to passwords and secrets #425912

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

lldap: document settings related to passwords and secrets #425912

wants to merge 1 commit into from

Conversation

ibizaman
Copy link
Contributor

@ibizaman ibizaman commented Jul 16, 2025 •
edited
Loading

Provides first class support for secrets used by LLDAP: the admin password and the jwt secret. Also provides first class support for the option to replace the secrets on each service start.

This PR precedes PR #425923 which adds first class support for the LLDAP bootstrap script.

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/)
  • Nixpkgs 25.11 Release Notes (or backporting 25.05 Nixpkgs Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
  • NixOS 25.11 Release Notes (or backporting 25.05 NixOS Release notes)
    • (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, pkgs/README.md, maintainers/README.md and other contributing documentation in corresponding paths.

Add a 👍 reaction to pull requests you find important.

@ibizaman ibizaman requested a review from emilylange July 16, 2025 20:57
@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 Jul 16, 2025
@ibizaman ibizaman requested a review from bendlas July 16, 2025 21:19
@ibizaman ibizaman mentioned this pull request Jul 16, 2025
Open
13 tasks
@emilylange emilylange removed their request for review July 17, 2025 01:11
bendlas
bendlas requested changes Jul 21, 2025
View reviewed changes
Copy link
Contributor

@bendlas bendlas left a comment

Choose a reason for hiding this comment

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

For adding logic on top of lldap settings, please just document the relevant settings key in the config.services.lldap.settings freeform type, instead of introducing top-level options.

@ibizaman ibizaman requested a review from bendlas July 21, 2025 21:29
bendlas
bendlas reviewed Jul 22, 2025
View reviewed changes
@ibizaman ibizaman force-pushed the lldap-secrets branch 2 times, most recently from 0433dd2 to 8d75a60 Compare July 23, 2025 06:43
bendlas
bendlas reviewed Jul 23, 2025
View reviewed changes
@ibizaman ibizaman requested a review from bendlas July 23, 2025 08:23
@ibizaman ibizaman changed the title lldap: add options to set important secrets lldap: document settings related to passwords and secrets Jul 23, 2025
@ibizaman ibizaman force-pushed the lldap-secrets branch 2 times, most recently from 081018c to d7b92a6 Compare July 23, 2025 09:03
@ibizaman
Copy link
Contributor Author

Btw in an ulterior version of LLDAP, this commit makes it mandatory to set the LLDAP password. I propose to leave this PR as-is and I'll make the ldap_user_pass_file setting required in the "bump" PR #425918 which I will also update to use version 0.6.2 when it's out.

@bendlas
Copy link
Contributor

bendlas commented Jul 23, 2025

I propose to leave this PR as-is and I'll make the ldap_user_pass_file setting required in the "bump" PR #425918 which I will also update to use version 0.6.2 when it's out.

sounds good to me. there are more warnings that might be good to have, like e.g. when using non - *_file versions of setting secrets, which will expose them in /nix/store, but this is in a good state now, afaict.

@nixpkgs-ci nixpkgs-ci bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Jul 23, 2025
@bendlas bendlas mentioned this pull request Aug 19, 2025
Merged
13 tasks
@ibizaman
Copy link
Contributor Author

Has been merged as part of #425918

@ibizaman ibizaman closed this Aug 24, 2025
@ibizaman ibizaman deleted the lldap-secrets branch August 24, 2025 19:32
@ibizaman ibizaman restored the lldap-secrets branch August 24, 2025 19:34
@ibizaman ibizaman deleted the lldap-secrets branch August 24, 2025 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bendlas bendlas bendlas approved these changes

Assignees
No one assigned
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. 12.approvals: 1 This PR was reviewed and approved by one person.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ibizaman @bendlas