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

keycloak: init 9.0.0 #80897

Closed
wants to merge 2 commits into from
Closed

keycloak: init 9.0.0 #80897

wants to merge 2 commits into from

Conversation

ngerstle
Copy link
Contributor

@ngerstle ngerstle commented Feb 23, 2020

Motivation for this change

Keycloak is a common opensource IdP for both home and business use, and is missing from the nixpkg repos. While this does not build from source (which involves a complex maven build), it does grab a build jar and make it available for use.

Running JBOSS_BASE_DIR=/some/mutable/folder standalone.sh allows running keycloak in standalone mode (assuming the folder has necessary configuration files).

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

Copy link
Member

@nlewo nlewo left a comment

Choose a reason for hiding this comment

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

Otherwise, LGTM

pkgs/top-level/all-packages.nix Outdated Show resolved Hide resolved
@rycee
Copy link
Member

rycee commented Feb 25, 2020

Was about to merge this but noticed that the bin directory contains a bunch of shell scripts and also a bunch of non-executables:

$ ls result/bin/
add-user.bat           federation-sssd-setup.sh       migrate-standalone-ha.cli
add-user-keycloak.bat  jboss-cli.bat                  product.conf
add-user-keycloak.sh   jboss-cli-logging.properties   standalone.bat
add-user.properties    jboss-cli.ps1                  standalone.conf
add-user.ps1           jboss-cli.sh                   standalone.conf.bat
add-user.sh            jboss-cli.xml                  standalone.conf.ps1
appclient.sh           jconsole.bat                   standalone.ps1
client                 jconsole.ps1                   standalone.sh
common.bat             jconsole.sh                    vault.bat
common.ps1             jdr.bat                        vault.ps1
common.sh              jdr.ps1                        vault.sh
domain.bat             jdr.sh                         wildfly-elytron-tool.jar
domain.conf            kcadm.bat                      wsconsume.bat
domain.conf.bat        kcadm.sh                       wsconsume.ps1
domain.conf.ps1        kcreg.bat                      wsconsume.sh
domain.ps1             kcreg.sh                       wsprovide.bat
domain.sh              launcher.jar                   wsprovide.ps1
elytron-tool.bat       migrate-domain-clustered.cli   wsprovide.sh
elytron-tool.ps1       migrate-domain-standalone.cli
elytron-tool.sh        migrate-standalone.cli

I think at the very least, the ps1 and bat files should be removed. Also most of the sh files seem to require java so I guess those also should be wrapped?

@ngerstle
Copy link
Contributor Author

Was about to merge this but noticed that the bin directory contains a bunch of shell scripts and also a bunch of non-executables:

$ ls result/bin/
add-user.bat           federation-sssd-setup.sh       migrate-standalone-ha.cli
add-user-keycloak.bat  jboss-cli.bat                  product.conf
add-user-keycloak.sh   jboss-cli-logging.properties   standalone.bat
add-user.properties    jboss-cli.ps1                  standalone.conf
add-user.ps1           jboss-cli.sh                   standalone.conf.bat
add-user.sh            jboss-cli.xml                  standalone.conf.ps1
appclient.sh           jconsole.bat                   standalone.ps1
client                 jconsole.ps1                   standalone.sh
common.bat             jconsole.sh                    vault.bat
common.ps1             jdr.bat                        vault.ps1
common.sh              jdr.ps1                        vault.sh
domain.bat             jdr.sh                         wildfly-elytron-tool.jar
domain.conf            kcadm.bat                      wsconsume.bat
domain.conf.bat        kcadm.sh                       wsconsume.ps1
domain.conf.ps1        kcreg.bat                      wsconsume.sh
domain.ps1             kcreg.sh                       wsprovide.bat
domain.sh              launcher.jar                   wsprovide.ps1
elytron-tool.bat       migrate-domain-clustered.cli   wsprovide.sh
elytron-tool.ps1       migrate-domain-standalone.cli
elytron-tool.sh        migrate-standalone.cli

I think at the very least, the ps1 and bat files should be removed. Also most of the sh files seem to require java so I guess those also should be wrapped?

Good catch. I've removed *.{bat,ps1}, but need to do some more investigation regarding which *.sh files need a wrapped path- wrapping them all breaks bin/standalone.sh in some way.

I presumed imitating the official keycloak docker image wouldn't lead me astray, but the docker doesn't seem all that optimal (not that it's building from source either).

@rycee
Copy link
Member

rycee commented Feb 26, 2020

Yeah, they certainly didn't package it in a "standard" way. I think what you have now is good enough for the time being since I imagine few people would install this into their profile.

rycee pushed a commit that referenced this pull request Feb 26, 2020
@rycee
Copy link
Member

rycee commented Feb 26, 2020

Rebased to master in 2dd9829.

@rycee rycee closed this Feb 26, 2020
dtzWill pushed a commit to dtzWill/nixpkgs that referenced this pull request Feb 27, 2020
PR NixOS#80897

(cherry picked from commit 2dd9829)
@ngerstle ngerstle deleted the init-keycloak branch July 4, 2021 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants