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

caddy: include and utilize systemd service from upstream #147305

Merged
merged 1 commit into from Nov 29, 2021

Conversation

aanderse
Copy link
Member

@aanderse aanderse commented Nov 24, 2021

Motivation for this change

I noticed that upstream has moved from a simple service to a notify service and we never caught that because we don't use the upstream service. Let's use the upstream unit so we don't miss things like that.

Note: Recently I have become interested in caddy and I'm interested in filing some PRs against our caddy module. I don't use caddy yet, but I'm looking to start.

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/)
  • 21.11 Release Notes (or backporting 21.05 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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@happysalada
Copy link
Contributor

happysalada commented Nov 25, 2021

Amazing to have more people interested in caddy ! it's time we give a real chance to the nginx alternative!
Welcome aboard ! :-)

@happysalada
Copy link
Contributor

Just finished switching my server to this PR. All good to merge IMO.
Waiting another couple of days in case anybody else has some feedback.

Comment on lines -189 to -190
AmbientCapabilities = "cap_net_bind_service";
CapabilityBoundingSet = "cap_net_bind_service";
Copy link
Member

Choose a reason for hiding this comment

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

Why are those not needed any more?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, they're in the upstream service file. Never mind me.

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

4 participants