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

lldap: init at 0.4.3; nixos/lldap: init; nixosTests.lldap: init #227916

Merged
merged 3 commits into from
Apr 29, 2023

Conversation

emilylange
Copy link
Member

@emilylange emilylange commented Apr 24, 2023

Description of changes

https://github.com/lldap/lldap

Package request: #142275
Supersedes previous attempt: #197362

Unfortunately, we have to grab the wasm frontend from the upstream release tarball, as the wasm32-unknown-unknown rustc target isn't available in nixpkgs yet.
Tracking issue: #89426

I made sure the frontend works and looks as expected, and successfully ran the following very simple ldap query:

❯ ldapsearch -H ldap://localhost:3890 -D uid=admin,ou=people,dc=example,dc=com -b "ou=people,dc=example,dc=com" -W

(default admin password is password)

Not sure if I will actually end up replacing openldap setups with this.
I just saw this project, checked if it's packaged, noticed it isn't, saw the package request and past attempts and then wanted to try packaging it myself :)

If anyone wants to be added to meta.maintainers, let me know!

PS: There is also https://github.com/kanidm/kanidm (in nixpkgs) which provides a ldap read-only interface, if that's all one needs.

resolves #142275, closes #197362

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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/)
  • 23.05 Release Notes (or backporting 22.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.

@LongerHV
Copy link
Contributor

LongerHV commented Apr 24, 2023

Very nice. Extractig frontend from amd64 package seems a little hacky,
but I didn't find a better way as well...
(maybe we should request including a pre-built frontend as a separate artifact in the upstream releases?)

I have implemented a basic nixos module for lldap for my own use, so maybe we can add it together with the package?

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin and removed 10.rebuild-darwin: 1 10.rebuild-darwin: 1-10 labels Apr 24, 2023
@emilylange
Copy link
Member Author

For what it's worth,

postBuild = ''
# We don't compile the wasm-part form source, as there isn't a rustc for
# wasm32-unknown-unknown in nixpkgs yet.
mkdir $out
cp -r kanidmd_web_ui/pkg $out/ui
'';
doesn't build its frontend from source because of #89426 either 🤷‍♀️

Regarding your nixos module:
I can give you write access to https://github.com/IndeedNotJames/nixpkgs, so you can push directly to this branch, if you wish.

There are a few things I would do different at first glance (e.g. mkPackageOptionMD) but nothing deal breaking :)
You can also reach me at https://matrix.to/#/@me:indeednotjames.com if you want to chat about it

I guess we could also build a simple VM test 🤔

@emilylange emilylange force-pushed the lldap branch 2 times, most recently from d43135f to 69870e9 Compare April 24, 2023 12:01
@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 Apr 25, 2023
@emilylange emilylange changed the title lldap: init at 0.4.3 lldap: init at 0.4.3; nixos/lldap: init; nixosTests.lldap: init Apr 25, 2023
@emilylange
Copy link
Member Author

@LongerHV I added you in Co-authored-by in the nixos/lldap: init commit, because I copied most of it from your nixos module you shared. Thank you!

@ofborg test lldap

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/2135

@emilylange
Copy link
Member Author

I added services.lldap to the 23.05 release notes.
Something I forget when committing the module initially :)

pkgs/servers/ldap/lldap/default.nix Show resolved Hide resolved
Copy link
Member

@aanderse aanderse left a comment

Choose a reason for hiding this comment

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

This looks great. Thanks to everyone for doing this.

nixos/modules/services/databases/lldap.nix Outdated Show resolved Hide resolved
nixos/modules/services/databases/lldap.nix Outdated Show resolved Hide resolved
@emilylange
Copy link
Member Author

@ofborg test lldap

@kira-bruneau
Copy link
Contributor

Result of nixpkgs-review pr 227916 run on x86_64-linux 1

2 packages blacklisted:
  • nixos-install-tools
  • tests.nixos-functions.nixos-test
1 package built:
  • lldap

@kira-bruneau kira-bruneau merged commit f81a619 into NixOS:master Apr 29, 2023
@emilylange emilylange deleted the lldap branch April 29, 2023 19:38
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 8.has: documentation This PR adds or changes documentation 8.has: module (update) This PR changes an existing module in `nixos/` 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Package request: lldap
8 participants