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

Redmine server options #85003

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Conversation

pvgoran
Copy link
Contributor

@pvgoran pvgoran commented Apr 11, 2020

Motivation for this change

This PR consists of three commits:

  1. The primary enhancement that is introduced here is the prefix option that allows to specify the URL prefix where Redmine will be served. To allow it to work under, say, https://example.org/redmine instead of https://example.org/ . This commit also includes quality-of-life enhancements: the serverAccessLog option that allows to disable access logging that WEBrick (HTTP server that runs/hosts Redmine) performs, and addition of the SyslogIdentifier setting for the systemd service.
  2. The listen.address option was added to allow specifying the IP address to listen to (which is quite desirable for security purposes); for unification, the port option was renamed to listen.port.
  3. The server option was introduced, which allows to use the Puma HTTP server instead of WEBrick. There is a wide-spread opinion that WEBrick is badly suited for production - although I don't know if it's actually true. Also, in my configuration WEBrick had a specific problem: its access logs show IP address of reverse proxy, rather than IP address of the actual client, whereas Puma shows the client's IP address reported by reverse proxy.

The third commit has a potential problem. It relies on the puma gem, but its presence is not guaranteed. The Gemfile of Redmine's package includes it in the :test group, with the comment "For running system tests". At the minimum, this gem's specification needs to be moved to the root level of the Gemfile with the appropriate comment. It would be even better to somehow ensure its presence in the redmine module; I don't know if it's possible or not.

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.

Also, SyslogIdentifier added for the systemd service.
This option allows to choose which Ruby HTTP server runs Redmine: WEBrick or Puma.
@aanderse
Copy link
Member

@pvgoran looking good 👍 I'll test this in the next week or two.

If you're a redmine user do you have capacity and ability to test this: #84823 as well? Of specialnote to you is the following line in https://github.com/aanderse/nixpkgs/blob/8eefb7ce147bccc9ef4a17175455fe0bc9bcaab1/pkgs/applications/version-management/redmine/default.nix

groups = [ "development" "ldap" "markdown" "minimagick" "openid" "test" ];

@aanderse aanderse self-assigned this Apr 12, 2020
@pvgoran
Copy link
Contributor Author

pvgoran commented Apr 14, 2020

@pvgoran looking good +1 I'll test this in the next week or two.

If you're a redmine user do you have capacity and ability to test this: #84823 as well?

I'll try to test it during this week.

Of specialnote to you is the following line in https://github.com/aanderse/nixpkgs/blob/8eefb7ce147bccc9ef4a17175455fe0bc9bcaab1/pkgs/applications/version-management/redmine/default.nix

groups = [ "development" "ldap" "markdown" "minimagick" "openid" "test" ];

Did you mean to say that this line ensures that the test group's gem are included in nixpkgs' redmine package?

@aanderse
Copy link
Member

@pvgoran I'm not really familiar enough with ruby to give an educated answer on that. Maybe you can comment on that PR as I've already posed a question like that to @manveru and am hoping to hear back.

@stale
Copy link

stale bot commented Jun 7, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 7, 2021
@pvgoran
Copy link
Contributor Author

pvgoran commented Jun 7, 2021

I need to bring this up to date one day...

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 7, 2021
@stale
Copy link

stale bot commented Jan 9, 2022

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 9, 2022
@wegank wegank marked this pull request as draft March 20, 2024 15:40
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