ldraney/pal-e-platform
1
0
Fork 0
Code Issues 104 Pull requests 7 Projects Releases Packages Wiki Activity Actions

feat: add Caddy reverse proxy Salt state for palinks.app (#425) #439

Merged
ldraney merged 1 commit from 425-caddy-palinks-app into main 2026-06-16 00:19:40 +00:00
Conversation 2 Commits 1 Files changed 2 +54
ldraney commented 2026-06-15 23:36:47 +00:00
Owner
Copy link

Closes #425

Summary

Adds salt/states/caddy/ with an init.sls and Caddyfile.j2 template to manage the Caddy reverse proxy on the Hetzner edge-proxy node. Routes palinks.app to its Tailscale backend with TLS SNI and redirects www.palinks.app to the apex domain.

Changes

  • salt/states/caddy/init.sls — Salt state managing Caddyfile deployment via file.managed and Caddy service lifecycle via service.running with watch on config changes
  • salt/states/caddy/Caddyfile.j2 — Jinja2 template with palinks.app reverse_proxy block (TLS SNI to Tailscale backend) and www-to-apex permanent redirect

Discovered Scope

None

Terraform Changes

N/A

Test Plan

  • Apply the state: salt 'edge-proxy' state.apply caddy
  • Verify Caddyfile rendered: salt 'edge-proxy' cmd.run 'cat /etc/caddy/Caddyfile'
  • Verify Caddy running: salt 'edge-proxy' cmd.run 'systemctl status caddy'
  • Verify TLS and routing: curl -I https://palinks.app (expect 200)
  • Verify www redirect: curl -I https://www.palinks.app (expect 301 to apex)

Review Checklist

  • Passed automated review-fix loop
  • No secrets committed
  • No unnecessary file changes
  • Commit messages are descriptive
  • Feature flag needed? No — infrastructure Salt state, no user-visible workflow

Related Notes

  • ldraney/pal-e-platform #425 — Configure Caddy reverse proxy for palinks.app on edge-proxy
  • dns-custom-domain-waves — Three-wave chain: Terraform/DNS, Caddy, app config
Closes #425 ## Summary Adds `salt/states/caddy/` with an init.sls and Caddyfile.j2 template to manage the Caddy reverse proxy on the Hetzner edge-proxy node. Routes `palinks.app` to its Tailscale backend with TLS SNI and redirects `www.palinks.app` to the apex domain. ## Changes - `salt/states/caddy/init.sls` — Salt state managing Caddyfile deployment via file.managed and Caddy service lifecycle via service.running with watch on config changes - `salt/states/caddy/Caddyfile.j2` — Jinja2 template with palinks.app reverse_proxy block (TLS SNI to Tailscale backend) and www-to-apex permanent redirect ## Discovered Scope None ## Terraform Changes N/A ## Test Plan - [ ] Apply the state: `salt 'edge-proxy' state.apply caddy` - [ ] Verify Caddyfile rendered: `salt 'edge-proxy' cmd.run 'cat /etc/caddy/Caddyfile'` - [ ] Verify Caddy running: `salt 'edge-proxy' cmd.run 'systemctl status caddy'` - [ ] Verify TLS and routing: `curl -I https://palinks.app` (expect 200) - [ ] Verify www redirect: `curl -I https://www.palinks.app` (expect 301 to apex) ## Review Checklist - [ ] Passed automated review-fix loop - [ ] No secrets committed - [ ] No unnecessary file changes - [ ] Commit messages are descriptive - [ ] Feature flag needed? No — infrastructure Salt state, no user-visible workflow ## Related Notes - `ldraney/pal-e-platform #425` — Configure Caddy reverse proxy for palinks.app on edge-proxy - `dns-custom-domain-waves` — Three-wave chain: Terraform/DNS, Caddy, app config
feat: add Caddy reverse proxy Salt state for palinks.app
All checks were successful
ci/woodpecker/push/terraform Pipeline was successful
Details
ci/woodpecker/pr/terraform Pipeline was successful
Details
ci/woodpecker/pull_request_closed/terraform Pipeline was successful
Details
6c83061ed6 
Establishes salt/states/caddy/ with init.sls and Caddyfile.j2 to manage
the Caddy reverse proxy on the Hetzner edge-proxy node. The Caddyfile
routes palinks.app to its Tailscale backend with TLS SNI and redirects
www.palinks.app to the apex domain.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ldraney commented 2026-06-15 23:38:18 +00:00
Author
Owner
Copy link

PR #439 Review

DOMAIN REVIEW

Stack: Salt / Caddy (Infrastructure)

Two new files under salt/states/caddy/:

init.sls -- Salt state

  • file.managed with correct ownership (root:root), mode (0644), source, and template: jinja -- follows standard Salt patterns.
  • service.running with enable: True and watch on the file state -- correct pattern. Caddy will reload when the Caddyfile changes. (Note: Caddy's reload command is graceful, and service.running with watch triggers a full restart via systemd. This is fine for a config-only change, but for future reference, Caddy supports caddy reload for zero-downtime config changes. Not blocking.)

Caddyfile.j2 -- Caddy template

  • TLS SNI (tls_server_name palinks.tail5b443a.ts.net) is correctly configured so Caddy validates the upstream Tailscale certificate. Good practice.
  • reverse_proxy to port 443 with explicit transport http { tls_server_name } is the correct Caddy pattern for HTTPS upstream with custom SNI.
  • www-to-apex redirect uses redir ... permanent (HTTP 301) -- correct.
  • The Tailscale MagicDNS hostname is hardcoded rather than pillar-driven. Acceptable for a single-domain template, but worth pillarizing when #434 (landscaping-assistant.app) is added to avoid duplicating the pattern.

Template without Jinja expressions: The file is named .j2 and rendered with template: jinja, but contains zero Jinja expressions ({{ }}, {% %}). This is forward-looking design (ready for pillar data), not a bug, but it means the template engine adds overhead with no current benefit. Not blocking.

BLOCKERS

None.

NITS

  1. No Jinja expressions in .j2 template -- Consider either (a) adding a comment explaining the template is intentionally static pending pillar-driven multi-domain support, or (b) pillarizing the domain/backend mapping now so the pattern is established before #434. Low priority.

  2. service.running vs caddy reload -- Salt's watch triggers a full service restart via systemd. Caddy supports caddy reload --config /etc/caddy/Caddyfile for zero-downtime config application. A cmd.wait state calling caddy reload instead of a full restart would be more graceful, but this is only relevant under load. Not blocking for the current single-domain setup.

  3. Hardcoded Tailscale hostname -- palinks.tail5b443a.ts.net appears twice in the Caddyfile (once in reverse_proxy, once in tls_server_name). When adding more domains, extract this to pillar data to keep the template DRY.

SOP COMPLIANCE

  • PR body has Summary, Changes, Test Plan, Related
  • No secrets committed (Tailscale MagicDNS hostname is internal but not a secret)
  • No unnecessary file changes (2 files, both on-scope for #425)
  • Commit message is descriptive (feat: add Caddy reverse proxy Salt state for palinks.app (#425))
  • Discovered Scope section present and explicitly marked "None"
  • Test plan is thorough (5 manual verification steps covering state apply, rendered config, service status, TLS routing, and www redirect)

PROCESS OBSERVATIONS

  • This PR is Wave 2 of the three-wave custom domain chain (Terraform/DNS -> Caddy -> app config). Wave 1 (#435, #436) is merged. Clean dependency chain.
  • Issue #434 (landscaping-assistant.app Caddy config) will follow the same pattern. The nit about pillarizing is most relevant there -- establishing the pattern now saves refactoring later.
  • Change failure risk is low: this is additive infrastructure (new files, no modifications to existing state), and Caddy's ACME TLS is well-tested. The test plan covers the critical verification path.

VERDICT: APPROVED

## PR #439 Review ### DOMAIN REVIEW **Stack: Salt / Caddy (Infrastructure)** Two new files under `salt/states/caddy/`: **`init.sls` -- Salt state** - `file.managed` with correct ownership (`root:root`), mode (`0644`), source, and `template: jinja` -- follows standard Salt patterns. - `service.running` with `enable: True` and `watch` on the file state -- correct pattern. Caddy will reload when the Caddyfile changes. (Note: Caddy's `reload` command is graceful, and `service.running` with `watch` triggers a full restart via systemd. This is fine for a config-only change, but for future reference, Caddy supports `caddy reload` for zero-downtime config changes. Not blocking.) **`Caddyfile.j2` -- Caddy template** - TLS SNI (`tls_server_name palinks.tail5b443a.ts.net`) is correctly configured so Caddy validates the upstream Tailscale certificate. Good practice. - `reverse_proxy` to port 443 with explicit `transport http { tls_server_name }` is the correct Caddy pattern for HTTPS upstream with custom SNI. - www-to-apex redirect uses `redir ... permanent` (HTTP 301) -- correct. - The Tailscale MagicDNS hostname is hardcoded rather than pillar-driven. Acceptable for a single-domain template, but worth pillarizing when #434 (landscaping-assistant.app) is added to avoid duplicating the pattern. **Template without Jinja expressions:** The file is named `.j2` and rendered with `template: jinja`, but contains zero Jinja expressions (`{{ }}`, `{% %}`). This is forward-looking design (ready for pillar data), not a bug, but it means the template engine adds overhead with no current benefit. Not blocking. ### BLOCKERS None. ### NITS 1. **No Jinja expressions in `.j2` template** -- Consider either (a) adding a comment explaining the template is intentionally static pending pillar-driven multi-domain support, or (b) pillarizing the domain/backend mapping now so the pattern is established before #434. Low priority. 2. **`service.running` vs `caddy reload`** -- Salt's `watch` triggers a full service restart via systemd. Caddy supports `caddy reload --config /etc/caddy/Caddyfile` for zero-downtime config application. A `cmd.wait` state calling `caddy reload` instead of a full restart would be more graceful, but this is only relevant under load. Not blocking for the current single-domain setup. 3. **Hardcoded Tailscale hostname** -- `palinks.tail5b443a.ts.net` appears twice in the Caddyfile (once in `reverse_proxy`, once in `tls_server_name`). When adding more domains, extract this to pillar data to keep the template DRY. ### SOP COMPLIANCE - [x] PR body has Summary, Changes, Test Plan, Related - [x] No secrets committed (Tailscale MagicDNS hostname is internal but not a secret) - [x] No unnecessary file changes (2 files, both on-scope for #425) - [x] Commit message is descriptive (`feat: add Caddy reverse proxy Salt state for palinks.app (#425)`) - [x] Discovered Scope section present and explicitly marked "None" - [x] Test plan is thorough (5 manual verification steps covering state apply, rendered config, service status, TLS routing, and www redirect) ### PROCESS OBSERVATIONS - This PR is Wave 2 of the three-wave custom domain chain (Terraform/DNS -> **Caddy** -> app config). Wave 1 (#435, #436) is merged. Clean dependency chain. - Issue #434 (landscaping-assistant.app Caddy config) will follow the same pattern. The nit about pillarizing is most relevant there -- establishing the pattern now saves refactoring later. - Change failure risk is low: this is additive infrastructure (new files, no modifications to existing state), and Caddy's ACME TLS is well-tested. The test plan covers the critical verification path. ### VERDICT: APPROVED
ldraney commented 2026-06-15 23:55:44 +00:00
Author
Owner
Copy link

PR #439 Review

Title: feat: add Caddy reverse proxy Salt state for palinks.app (#425)
Parent Issue: #425

DOMAIN REVIEW

Tech stack: Salt (SaltStack states + Jinja2 templates), Caddy reverse proxy configuration.

Salt compliance (salt/states/caddy/init.sls):

  • State IDs follow Salt conventions (caddy-caddyfile, caddy-service) -- good.
  • file.managed correctly sets ownership (root:root), mode (0644), and uses template: jinja for Jinja2 rendering.
  • service.running with enable: True and watch on the file state is the correct pattern -- Caddy will reload on config changes. Confirmed: Caddy supports graceful reload on systemctl reload, and Salt's service.running with watch triggers a restart (not reload). This is acceptable for a single-site config but see nit below.
  • Header comments clearly document purpose, origin, and anti-edit warning. Good operational hygiene.
  • No pillar references used -- the template is static Jinja2 with no pillar variables. This is acceptable for a first domain but will need refactoring when landscaping-assistant.app (#434) and westsidekingsandqueens.com are added. Not blocking for this PR.

Caddy configuration (salt/states/caddy/Caddyfile.j2):

  • Upstream palinks.tail5b443a.ts.net:443 matches the Tailscale tailnet domain defined in terraform/variables.tf (tail5b443a.ts.net). The palinks service is exposed via the Tailscale subnet router advertising 10.43.0.0/16, making this hostname reachable from the edge-proxy node (which is on the tailnet via cloud-init).
  • tls_server_name palinks.tail5b443a.ts.net in the transport block ensures Caddy validates the upstream TLS certificate against the correct SNI -- essential since the public domain (palinks.app) differs from the upstream hostname. Correct security practice.
  • www.palinks.app redirect with permanent (301) and {uri} path preservation is correct.
  • No tls directive needed on the site blocks -- Caddy's automatic HTTPS via ACME (Let's Encrypt) handles TLS for both palinks.app and www.palinks.app. Cloud-init installs Caddy and the Hetzner firewall opens ports 80/443 (confirmed in terraform/modules/hetzner-edge/main.tf), so ACME HTTP-01 challenge will succeed.
  • The transport http block does not set tls explicitly, which means Caddy will use HTTPS to the upstream (port 443) but with default TLS settings. The tls_server_name alone is sufficient -- Caddy infers TLS from the port. Correct.

Architecture alignment:

  • Traffic path matches docs/hetzner-edge.md: DNS (GoDaddy A record at 178.156.129.142) -> Hetzner edge Caddy -> Tailscale mesh -> k3s palinks pod. DNS records confirmed in terraform/dns.tf (godaddy_dns_record.palinks_a).
  • This is wave 2 of the three-wave custom domain chain (wave 1: DNS/Terraform, wave 2: Caddy, wave 3: app config). Wave 1 landed in PR #436/#438. Sequencing is correct.

BLOCKERS

None.

  • No secrets committed -- the Tailscale hostname tail5b443a.ts.net is not a secret (it is a public-facing MagicDNS domain suffix, also already documented in terraform/variables.tf and multiple docs files).
  • No unvalidated user input -- these are static infrastructure configs.
  • Scope is tight: two files, both directly serving the issue objective.

NITS

  1. service.running watch triggers restart, not reload. Salt's default behavior when a watched state changes is to restart the service. Caddy supports graceful reload (systemctl reload caddy), which avoids dropping in-flight connections. Consider adding - reload: True to the service.running state for zero-downtime config updates:

    caddy-service:
  service.running:
    - name: caddy
    - enable: True
    - reload: True
    - watch:
      - file: caddy-caddyfile

    Not blocking because this is the first domain and downtime during reload is negligible, but will matter when multiple domains are served.

  2. top.sls not updated. The new caddy state is not added to salt/states/top.sls. Currently, top.sls targets archbox and lucass-macbook-air-1 -- neither is the edge-proxy. The edge-proxy node is not yet a Salt minion target in top.sls. This means the state cannot be applied via salt '*' state.highstate -- it requires manual targeting (salt 'edge-proxy' state.apply caddy). This is noted in the test plan, so it appears intentional. If the edge-proxy minion is expected to be added to top.sls later, this is fine. Just flagging for awareness.

  3. Hardcoded Tailscale hostname in template. palinks.tail5b443a.ts.net is hardcoded in the Jinja2 template. When additional domains are added (#434), this template will need refactoring to use pillar data (e.g., pillar['caddy']['sites'] with per-domain upstream config). Not blocking for a single-domain first pass, but the comment "To add a new domain: add a site block below" invites copy-paste rather than pillar-driven rendering. Consider a TODO comment noting the pillar refactor plan.

  4. transport http naming. The Caddy transport block is named http even though it speaks HTTPS to the upstream. This is correct Caddy syntax (the http transport module handles both HTTP and HTTPS), but a brief inline comment clarifying this would help future readers unfamiliar with Caddy internals.

SOP COMPLIANCE

  • PR body has: Summary, Changes, Discovered Scope, Test Plan, Review Checklist, Related Notes
  • Closes #425 present -- issue linkage correct
  • No secrets committed
  • No unnecessary file changes (2 files, both directly implementing the feature)
  • Commit message is descriptive: "feat: add Caddy reverse proxy Salt state for palinks.app"
  • Test plan is thorough: covers state apply, config render, service status, TLS verification, and redirect behavior
  • Terraform Changes section marked N/A -- correct, this is Salt-only

PROCESS OBSERVATIONS

  • Deployment frequency: This is wave 2 of the custom domain chain. Clean handoff from wave 1 (DNS, PR #436). Good decomposition -- each wave is independently deployable and testable.
  • Change failure risk: Low. Salt state is declarative, Caddy config is simple, and the test plan covers the key validation points. The watch directive ensures config changes propagate to the service.
  • Follow-up tracking: Issue #434 (landscaping-assistant.app Caddy config) is the natural next step. The template is designed for multi-domain use but will need the pillar refactor noted in nit #3 before adding more domains.
  • Documentation: docs/hetzner-edge.md already covers the architecture. No docs gap.

VERDICT: APPROVED

## PR #439 Review **Title:** feat: add Caddy reverse proxy Salt state for palinks.app (#425) **Parent Issue:** #425 --- ### DOMAIN REVIEW **Tech stack:** Salt (SaltStack states + Jinja2 templates), Caddy reverse proxy configuration. **Salt compliance (`salt/states/caddy/init.sls`):** - State IDs follow Salt conventions (`caddy-caddyfile`, `caddy-service`) -- good. - `file.managed` correctly sets ownership (root:root), mode (0644), and uses `template: jinja` for Jinja2 rendering. - `service.running` with `enable: True` and `watch` on the file state is the correct pattern -- Caddy will reload on config changes. Confirmed: Caddy supports graceful reload on `systemctl reload`, and Salt's `service.running` with `watch` triggers a restart (not reload). This is acceptable for a single-site config but see nit below. - Header comments clearly document purpose, origin, and anti-edit warning. Good operational hygiene. - No pillar references used -- the template is static Jinja2 with no pillar variables. This is acceptable for a first domain but will need refactoring when `landscaping-assistant.app` (#434) and `westsidekingsandqueens.com` are added. Not blocking for this PR. **Caddy configuration (`salt/states/caddy/Caddyfile.j2`):** - Upstream `palinks.tail5b443a.ts.net:443` matches the Tailscale tailnet domain defined in `terraform/variables.tf` (`tail5b443a.ts.net`). The palinks service is exposed via the Tailscale subnet router advertising `10.43.0.0/16`, making this hostname reachable from the edge-proxy node (which is on the tailnet via cloud-init). - `tls_server_name palinks.tail5b443a.ts.net` in the transport block ensures Caddy validates the upstream TLS certificate against the correct SNI -- essential since the public domain (`palinks.app`) differs from the upstream hostname. Correct security practice. - `www.palinks.app` redirect with `permanent` (301) and `{uri}` path preservation is correct. - No `tls` directive needed on the site blocks -- Caddy's automatic HTTPS via ACME (Let's Encrypt) handles TLS for both `palinks.app` and `www.palinks.app`. Cloud-init installs Caddy and the Hetzner firewall opens ports 80/443 (confirmed in `terraform/modules/hetzner-edge/main.tf`), so ACME HTTP-01 challenge will succeed. - The `transport http` block does not set `tls` explicitly, which means Caddy will use HTTPS to the upstream (port 443) but with default TLS settings. The `tls_server_name` alone is sufficient -- Caddy infers TLS from the port. Correct. **Architecture alignment:** - Traffic path matches `docs/hetzner-edge.md`: DNS (GoDaddy A record at 178.156.129.142) -> Hetzner edge Caddy -> Tailscale mesh -> k3s palinks pod. DNS records confirmed in `terraform/dns.tf` (`godaddy_dns_record.palinks_a`). - This is wave 2 of the three-wave custom domain chain (wave 1: DNS/Terraform, wave 2: Caddy, wave 3: app config). Wave 1 landed in PR #436/#438. Sequencing is correct. --- ### BLOCKERS **None.** - No secrets committed -- the Tailscale hostname `tail5b443a.ts.net` is not a secret (it is a public-facing MagicDNS domain suffix, also already documented in `terraform/variables.tf` and multiple docs files). - No unvalidated user input -- these are static infrastructure configs. - Scope is tight: two files, both directly serving the issue objective. --- ### NITS 1. **`service.running` watch triggers restart, not reload.** Salt's default behavior when a watched state changes is to restart the service. Caddy supports graceful reload (`systemctl reload caddy`), which avoids dropping in-flight connections. Consider adding `- reload: True` to the `service.running` state for zero-downtime config updates: ```yaml caddy-service: service.running: - name: caddy - enable: True - reload: True - watch: - file: caddy-caddyfile ``` Not blocking because this is the first domain and downtime during reload is negligible, but will matter when multiple domains are served. 2. **`top.sls` not updated.** The new `caddy` state is not added to `salt/states/top.sls`. Currently, `top.sls` targets `archbox` and `lucass-macbook-air-1` -- neither is the edge-proxy. The edge-proxy node is not yet a Salt minion target in `top.sls`. This means the state cannot be applied via `salt '*' state.highstate` -- it requires manual targeting (`salt 'edge-proxy' state.apply caddy`). This is noted in the test plan, so it appears intentional. If the edge-proxy minion is expected to be added to `top.sls` later, this is fine. Just flagging for awareness. 3. **Hardcoded Tailscale hostname in template.** `palinks.tail5b443a.ts.net` is hardcoded in the Jinja2 template. When additional domains are added (#434), this template will need refactoring to use pillar data (e.g., `pillar['caddy']['sites']` with per-domain upstream config). Not blocking for a single-domain first pass, but the comment "To add a new domain: add a site block below" invites copy-paste rather than pillar-driven rendering. Consider a TODO comment noting the pillar refactor plan. 4. **`transport http` naming.** The Caddy transport block is named `http` even though it speaks HTTPS to the upstream. This is correct Caddy syntax (the `http` transport module handles both HTTP and HTTPS), but a brief inline comment clarifying this would help future readers unfamiliar with Caddy internals. --- ### SOP COMPLIANCE - [x] PR body has: Summary, Changes, Discovered Scope, Test Plan, Review Checklist, Related Notes - [x] `Closes #425` present -- issue linkage correct - [x] No secrets committed - [x] No unnecessary file changes (2 files, both directly implementing the feature) - [x] Commit message is descriptive: "feat: add Caddy reverse proxy Salt state for palinks.app" - [x] Test plan is thorough: covers state apply, config render, service status, TLS verification, and redirect behavior - [x] Terraform Changes section marked N/A -- correct, this is Salt-only --- ### PROCESS OBSERVATIONS - **Deployment frequency:** This is wave 2 of the custom domain chain. Clean handoff from wave 1 (DNS, PR #436). Good decomposition -- each wave is independently deployable and testable. - **Change failure risk:** Low. Salt state is declarative, Caddy config is simple, and the test plan covers the key validation points. The `watch` directive ensures config changes propagate to the service. - **Follow-up tracking:** Issue #434 (landscaping-assistant.app Caddy config) is the natural next step. The template is designed for multi-domain use but will need the pillar refactor noted in nit #3 before adding more domains. - **Documentation:** `docs/hetzner-edge.md` already covers the architecture. No docs gap. --- ### VERDICT: APPROVED
ldraney deleted branch 425-caddy-palinks-app 2026-06-16 00:19:40 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
domain:backend

   
domain:devops

   
domain:frontend

   
status:approved

QA passed, awaiting merge approval

  
status:in-progress

Dev agent is actively working

  
status:needs-fix

QA found issues, back to dev

  
status:qa

PR submitted, awaiting QA review

  
type:bug

Bug fix

  
type:devops

Infrastructure/CI/config work

  
type:feature

New functionality

No labels
domain:backend
domain:devops
domain:frontend
status:approved
status:in-progress
status:needs-fix
status:qa
type:bug
type:devops
type:feature
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
ldraney/pal-e-platform!439
No description provided.