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

nixos/wordpress: Use https by default for caddy #375375

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

danobi
Copy link

@danobi danobi commented Jan 20, 2025

Previously, http:// scheme was hard coded into the caddy config if webserver = "caddy" was chosen. This is fine for local testing, but is problematic if you want your nixos host to be public facing.

In the public facing case, you generally want to be using TLS. But since the wordpress module generates the caddyfile rule, the user's nixos config cannot easily change it to also allow https.

An alternative would be to reverse proxy an https rule to the generated http rule, but that's somewhat questionable as there's not an internal http endpoint to proxy to. It might be possible but I couldn't figure it out.

So simplify by omitting the scheme. This causes caddy to use https by default and 301 redirect any http requests to the https endpoint. Caddy will just do the right thing if it's being hosted on a local/internal hostname (self sign certificates).

This should be backwards compatible with previous default if users are using reasonable browsers/tools.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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/)
  • 25.05 Release Notes (or backporting 24.11 and 25.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
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@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 Jan 20, 2025
@danobi
Copy link
Author

danobi commented Jan 20, 2025

cc @onny who originally added caddy support for wordpress.

@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Jan 20, 2025
@NixOSInfra NixOSInfra added the 12. first-time contribution This PR is the author's first one; please be gentle! label Jan 20, 2025
@danobi
Copy link
Author

danobi commented Jan 20, 2025

Looks like a lot of existing formatting issues. I can fix it in a followup if y'all want.

@danobi danobi force-pushed the wordpress_caddy_https branch from f7cf5c8 to 1566958 Compare January 21, 2025 01:01
@danobi
Copy link
Author

danobi commented Jan 21, 2025

I think I was hitting #337982. Had to add -k to work around (I think it's a workaround).

@onny
Copy link
Contributor

onny commented Jan 21, 2025

Thank you, this is great :) An entry to the release notes like here #327743 would be nice 👍

Previously, `http://` scheme was hard coded into the caddy config if
`webserver = "caddy"` was chosen. This is fine for local testing, but is
problematic if you want your nixos host to be public facing.

In the public facing case, you generally want to be using TLS. But since
the wordpress module generates the caddyfile rule, the user's nixos
config cannot easily change it to also allow https.

An alternative would be to reverse proxy an https rule to the generated
http rule, but that's somewhat questionable as there's not an internal
http endpoint to proxy to. It might be possible but I couldn't figure
it out.

So simplify by omitting the scheme. This causes caddy to use https by
default and 301 redirect any http requests to the https endpoint. Caddy
will just do the right thing if it's being hosted on a local/internal
hostname (self sign certificates).

This should be backwards compatible with previous default if users are
using reasonable browsers/tools.
@danobi
Copy link
Author

danobi commented Jan 24, 2025

Thank you, this is great :) An entry to the release notes like here #327743 would be nice 👍

Thanks, done!

@danobi danobi force-pushed the wordpress_caddy_https branch from 1566958 to 71dc31f Compare January 24, 2025 00:04
@github-actions github-actions bot added 8.has: documentation This PR adds or changes documentation 8.has: changelog labels Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog 8.has: documentation This PR adds or changes documentation 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 12. first-time contribution This PR is the author's first one; please be gentle!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants