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.

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

sozu: 0.11.56 -> 0.12.0, nixos/sozu: init #120010

Closed
wants to merge 10 commits into from
Closed

sozu: 0.11.56 -> 0.12.0, nixos/sozu: init #120010

wants to merge 10 commits into from

Conversation

netcrns
Copy link
Contributor

@netcrns netcrns commented Apr 21, 2021

Motivation for this change

We need Sozu at work, and this makes it more convenient to set up.

Things done

Added service configurations for Sozu and updated the package's version.
I'm also adding myself to the list of maintainers, to ensure it stays up-to-date.

  • 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.

@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 21, 2021
@SuperSandro2000 SuperSandro2000 changed the title netcrns/sozu sozu: 0.11.56 -> 0.11.59, nixos/sozu: init Apr 21, 2021
SuperSandro2000
SuperSandro2000 reviewed Apr 21, 2021
View reviewed changes
Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

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

Modules do not have a version. Naming the commit nixos/sozu: init is fine. Also please squash the 2nd module commit.

search.nixos.org shows the defaults and examples by default. They can be removed.

and the module is missing a maintainer.

@ofborg ofborg bot requested a review from Br1ght0ne April 21, 2021 16:13
@ofborg ofborg bot added 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 labels Apr 21, 2021
@aanderse
Copy link
Member

Sorry this PR isn't a quick one to review and I haven't found the time yet. I'll start with one comment: with modern software like this you would think upstream would suggest running as a non root user... but I couldn't find any mention on their github project. Would you mind asking the question to upstream on what it would take to run the software as a non root user, taking advantage of systemd to close the gap?

I'll try to find the time this PR deserves for review soon.

@Br1ght0ne
Copy link
Member

I'll wait with my review until other questions are resolved.

@Geal
Copy link

Geal commented Apr 24, 2021

hi, I'm the lead developer for sozu. I just added some documentation about making privileged ports accessible as a non root user. I thought this was common by now, but when looking for clear docs, there was a lot of confusing information. I hope this helps: https://github.com/sozu-proxy/sozu/blob/master/doc/recipes.md#using-port-80-or-443-as-non-root-user

Let me know if there are some things you need to configure the package (FYI sozu 0.12 will be released very soon)

@aanderse
Copy link
Member

@Geal thank you for a clear and concise answer. Much appreciated!

@netcrns netcrns mentioned this pull request Apr 26, 2021
Merged
10 tasks
@netcrns
Copy link
Contributor Author

netcrns commented Apr 26, 2021

Squashed stuff to bring some order into my mess.
I've also removed myself from the maintainers-list, in favour of doing that here, seeing as that PR will probably be merged before this one.

@netcrns netcrns changed the title sozu: 0.11.56 -> 0.11.59, nixos/sozu: init sozu: 0.11.56 -> 0.12.0, nixos/sozu: init Apr 30, 2021
@netcrns
Copy link
Contributor Author

netcrns commented Apr 30, 2021 •
edited
Loading

I'm not sure how I'll be able to make privileged ports accessible to non-root users, as that would require making the sozu user controlling the server unprivileged, if I'm not mistaken. Having it unprivileged makes it unable to create the /run directory, and I'm unsure as to how to create it if not in systemd.services.sozu.preStart.
Edit: Disregard - got it working using RuntimeDirectory, with the help of NobbZ on Discord!

@netcrns netcrns requested a review from SuperSandro2000 April 30, 2021 19:14
lukegb
lukegb requested changes May 3, 2021
View reviewed changes
@lukegb
Copy link
Contributor

lukegb commented May 3, 2021

Sozu looks super neat! I'm gonna add it to my pile of things to look at, heh.

@netcrns netcrns requested a review from lukegb May 6, 2021 13:27
@netcrns
Copy link
Contributor Author

netcrns commented May 9, 2021

/marvin opt-in
/status needs_reviewer

@marvin-mk2
Copy link

marvin-mk2 bot commented May 9, 2021

Hi! I'm an experimental bot. My goal is to guide this PR through its stages, hopefully ending with a merge. You can read up on the usage here.

@marvin-mk2 marvin-mk2 bot added marvin This PR was reviewed by Marvin, a discontinued bot: https://github.com/timokau/marvin-mk2 needs_reviewer (old Marvin label, do not use) labels May 9, 2021
@marvin-mk2 marvin-mk2 bot requested a review from thiagokokada May 9, 2021 20:07
@marvin-mk2 marvin-mk2 bot added awaiting_reviewer (old Marvin label, do not use) and removed needs_reviewer (old Marvin label, do not use) labels May 9, 2021
@marvin-mk2
Copy link

marvin-mk2 bot commented May 23, 2021

Reminder: Please review!

This Pull Request is awaiting review. If you are the assigned reviewer, please have a look. Try to find another reviewer if necessary. If you can't, please say so. If the status is not accurate, please change it. If nothing happens, this PR will be should be put back in the needs_reviewer queue in one day.

Note: This feature is currently broken. The bot will not actually change the status. If you see this message multiple times, please request a status change manually.

@SuperSandro2000 SuperSandro2000 added the 2.status: merge conflict This PR has merge conflicts with the target branch label May 23, 2021
netcrns added 9 commits May 23, 2021 19:14
@netcrns
nixos/sozu: init
61359a9 
nixos/sozu: fix typos to address failed checks

NixOS documentation couldn't build due to typos.
This fixes that.
@netcrns
sozu: 0.11.56 -> 0.12.0
eb67e1d
@netcrns
nixos/sozu: make privileged ports accessible to non-root users
0d2ae15
@netcrns
nixos/sozu: address comments in PR
7882acc 
nixos/sozu: remove `extraConfig` in favour of `optional`

nixos/sozu: rename `settings` to `global`

nixos/sozu: fix defaults

nixos/sozu: add examples to freeform submodule

nixos/sozu: fix environment package to be value of `package` option

nixos/sozu: assign group to user `sozu`

nixos/sozu: stylistic changes
@netcrns
nixos/sozu: add description to optional
c8fe7dd
@netcrns
nixos/sozu: provide a static path to the config file
dc54731
@netcrns
nixos/sozu: take strings and paths, instead of just strings
6ffdfd5
@netcrns
nixos/sozu: address comments in pr
0472c74 
nixos/sozu: make `PIDFile` take `cfg.settings.pid_file_path`

nixos/sozu: move options previously in `global` into top-level

nixos/sozu: rename `optional` to `settings`

nixos/sozu: move potentially unwanted options into `settings`

nixos/sozu: add options to `settings`
  * listeners
  * applications
  * metrics
@netcrns
nixos/sozu: add descriptions to options that are missing them
6e5fe14
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label May 23, 2021
@netcrns
sozu: use correct cargo hash
1144d39 
Not sure why this changed in the first place, but there was a conflict
with `master`.
@marvin-mk2
Copy link

marvin-mk2 bot commented May 26, 2021

Reminder: Please review!

This Pull Request is awaiting review. If you are the assigned reviewer, please have a look. Try to find another reviewer if necessary. If you can't, please say so. If the status is not accurate, please change it. If nothing happens, this PR will be should be put back in the needs_reviewer queue in one day.

Note: This feature is currently broken. The bot will not actually change the status. If you see this message multiple times, please request a status change manually.

@netcrns
Copy link
Contributor Author

netcrns commented May 26, 2021

shush marvin
/status awaiting_changes

@marvin-mk2 marvin-mk2 bot added awaiting_changes (old Marvin label, do not use) and removed awaiting_reviewer (old Marvin label, do not use) labels May 26, 2021
@Geal Geal mentioned this pull request Jul 29, 2021
Open
@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
@timokau timokau removed marvin This PR was reviewed by Marvin, a discontinued bot: https://github.com/timokau/marvin-mk2 awaiting_changes (old Marvin label, do not use) labels Apr 16, 2022
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Apr 16, 2022
@timokau
Copy link
Member

timokau commented Apr 16, 2022

Hi!

The marvin-mk2 bot is now discontinued. I have removed the relevant tags from this PR. If you still need someone to look at it, one option would be to ask in this discourse thread.

I am posting this notice to all open PRs with the marvin tag. Please understand that following all of these discussions would take too much time, so I will unsubscribe from this PR unless I have already been involved in it before this message.

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Nov 2, 2022
@Janik-Haag Janik-Haag added the 12. first-time contribution This PR is the author's first one; please be gentle! label Jun 12, 2023
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 12, 2023
@wegank
Copy link
Member

wegank commented Oct 14, 2023

sozu is at 0.15.6 on master, please rebase and reopen a new PR with updated options.

@wegank wegank closed this Oct 14, 2023
@ghost ghost mentioned this pull request Jan 22, 2025
Closed
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aanderse aanderse aanderse left review comments

@Br1ght0ne Br1ght0ne Awaiting requested review from Br1ght0ne

@SuperSandro2000 SuperSandro2000 Awaiting requested review from SuperSandro2000

@lukegb lukegb Awaiting requested review from lukegb

@thiagokokada thiagokokada Awaiting requested review from thiagokokada

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: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes 12. first-time contribution This PR is the author's first one; please be gentle!
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@netcrns @aanderse @Br1ght0ne @Geal @lukegb @timokau @wegank @SuperSandro2000 @Janik-Haag