-
-
Notifications
You must be signed in to change notification settings - Fork 13.9k
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
Add Shibboleth Service Provider #25255
Conversation
Required by the Shibboleth Service Provider Package
Required by the Shibboleth Service Provider
Required by the Shibboleth Service Provider
@jammerful, thanks for your PR! By analyzing the history of the files in this pull request, we identified @abbradar, @mattbillenstein and @benley to be potential reviewers. |
pkgs/top-level/all-packages.nix
Outdated
@@ -10835,6 +10835,10 @@ with pkgs; | |||
|
|||
nginxModules = callPackage ../servers/http/nginx/modules.nix { }; | |||
|
|||
nginxShibboleth = callPackage ../servers/http/nginx/stable.nix { | |||
modules = [ nginxModules.rtmp nginxModules.dav nginxModules.moreheaders nginxModules.shibboleth ]; | |||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see it a little problematic to add new variant for every nginx module combination we have. Either we have an nginx packet, that includes the most modules (nginxFull
) we have or a version, which makes use of nginx dynamic modules architecture, where plugins become a shared library. If somebody wants a custom build, it is always possible to do nginx.override { modules = [ ... ]; }
in configuration.nix
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have machinery that uses nginx dynamic modules in nixpkgs today? I too would like to use this and it seems unfortunate for all of us to have to compile nginx ourselves.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Mic92 another possibility is to just add this with a comment above it saying that if these particular compositions start proliferating, we should look into the loadable modules. I'm obviously partial to not having to compile this myself on every machine I use it on, "just this one time..." 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Mic92 @copumpkin I would like to have dynamic module support as well, but the nginx generic.nix currently doesn't seem to have dynamic module compilation support:
https://github.com/NixOS/nixpkgs/blob/master/pkgs/servers/http/nginx/generic.nix#L48 which is not "--add-dynmaic-module".
I'd typically do an override, but this is for an elastic fleet of servers, and can't have to re-compile nginx on every server.
|
||
meta = { | ||
# apu-1-config was removed from xcode and setting --with-apu1=${aprutil} doesn't | ||
# fix the issue, neithe does --with-apu1=${aprutil}/bin/apu-1-config |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just tried passing in --without-apxs
in the configureFlags
on Darwin and it seemed to fix the build. I assume that's what it's inferring on the Linux build anyway, since the apache crap isn't in scope and it can't find it. The main issue is that the configure script is trying to be helpful and look at /usr/bin
, but in doing so it runs into tooling installed by the Apple SDK and gets confused when it doesn't actually work.
sha256 = "1b5r4nd098lnjwr2g13f04ycqv5fvbrhpwg6fsdk8xy9cigvfzxj"; | ||
}; | ||
|
||
# Needs pkgconfig to find systemd |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You say it wants systemd
but then I don't see systemd
in the buildInputs
. It also seems to build fine without systemd
on Darwin if I force it to ignore the bogus apxs
it finds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Autoconf throws a syntax error without pkgconfig, it can't evaluate whether it should compile with systemd or not.
Worth changing the comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, it probably uses pkgconfig
and then invokes it and realizes that systemd
isn't there and omits systemd
support in that case. Comment is slightly confusing but not the end of the world. I'd change it if you end up changing this commit for other reasons but not bother otherwise.
d3e38eb
to
dfcc8dd
Compare
@Mic92 @copumpkin Removed the nginx commit, will move to a separate PR and add Mic92 to it so we can sort out his concerns seperately from these pkgs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
Motivation for this change
I need to provide SAML based authentication to my web application, and the shibboleth service provider package does exactly that. Adding these packages is the first step. I will also be sending in PR for a shibboleth-sp module.
Things done
(nix.useSandbox on NixOS,
or option
build-use-sandbox
innix.conf
on non-NixOS)
nix-shell -p nox --run "nox-review wip"
./result/bin/
)