ldraney/godaddy-tofu
1
0
Fork 0
Code Issues 2 Pull requests 1 Projects Releases Packages Wiki Activity Actions

Add Keycloak public ingress documentation #37

Open
ldraney wants to merge 3 commits from docs/keycloak-public-ingress into main
pull from: docs/keycloak-public-ingress
merge into: ldraney:main
Conversation 2 Commits 3 Files changed 4 +272 -2
ldraney commented 2026-06-18 01:05:46 +00:00
Owner
Copy link

Summary

  • Documents auth.palinks.app architecture for exposing Keycloak through the Hetzner edge proxy to public users
  • Adds Mermaid diagrams showing the public auth flow, dual-access design, and per-repo change list
  • Updates deployment.md to include auth domain in the infrastructure diagram and target domains table

Changes

  • docs/keycloak-public-ingress.md: new doc covering problem (KC_HOSTNAME blocks public auth), solution (remove KC_HOSTNAME, rely on X-Forwarded-Host), architecture diagrams, dual-access table, per-repo change list, and rollback procedure
  • docs/deployment.md: added auth.palinks.app DNS record and Keycloak pod to end-to-end flow diagram; updated target domains table with auth domain and cross-reference
  • README.md: added Keycloak Public Ingress entry to documentation table

Test Plan

  • Mermaid diagrams render correctly in Forgejo
  • Cross-check per-repo change list against actual file paths in pal-e-platform and pal-e-services
  • No regressions in existing docs

Review Checklist

  • Passed automated review-fix loop
  • No secrets committed
  • No unnecessary file changes
  • Commit messages are descriptive
  • Feature flag needed? No — docs only

Related Notes

  • Closes #36
  • godaddy-tofu — this repo's deployment and architecture docs
## Summary - Documents `auth.palinks.app` architecture for exposing Keycloak through the Hetzner edge proxy to public users - Adds Mermaid diagrams showing the public auth flow, dual-access design, and per-repo change list - Updates `deployment.md` to include auth domain in the infrastructure diagram and target domains table ## Changes - `docs/keycloak-public-ingress.md`: new doc covering problem (KC_HOSTNAME blocks public auth), solution (remove KC_HOSTNAME, rely on X-Forwarded-Host), architecture diagrams, dual-access table, per-repo change list, and rollback procedure - `docs/deployment.md`: added `auth.palinks.app` DNS record and Keycloak pod to end-to-end flow diagram; updated target domains table with auth domain and cross-reference - `README.md`: added Keycloak Public Ingress entry to documentation table ## Test Plan - [ ] Mermaid diagrams render correctly in Forgejo - [ ] Cross-check per-repo change list against actual file paths in pal-e-platform and pal-e-services - [ ] No regressions in existing docs ## Review Checklist - [ ] Passed automated review-fix loop - [ ] No secrets committed - [ ] No unnecessary file changes - [ ] Commit messages are descriptive - [ ] Feature flag needed? No — docs only ## Related Notes - Closes #36 - `godaddy-tofu` — this repo's deployment and architecture docs
Add Keycloak public ingress documentation
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
Details
ci/woodpecker/pr/woodpecker Pipeline was successful
Details
29aca0316e 
Documents the auth.palinks.app architecture for exposing Keycloak
through the edge proxy so public-facing apps can authenticate users
outside the Tailscale network. Includes Mermaid diagrams for the auth
flow, dual-access design, and per-repo change list.

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

PR #37 Review

DOMAIN REVIEW

Tech stack: Markdown documentation (Mermaid diagrams, HCL code examples, YAML config snippets). This is a docs-only PR -- no executable code changes. Domain review focuses on documentation quality, technical accuracy, and consistency with the existing codebase.

Documentation quality:

The new docs/keycloak-public-ingress.md is well-structured. It follows a clear problem-solution-architecture pattern, includes Mermaid sequence diagrams for both the broken state and the working state, and provides concrete per-repo change instructions with inline code samples.

Technical accuracy checks:

  1. Edge IP consistency -- The doc uses 178.156.129.142 throughout, which matches the existing deployment.md Mermaid diagram and the architecture.md context. Consistent.

  2. Mermaid diagram updates to deployment.md -- The diff adds DNS_AUTH, KC, and their connections in the correct places within the existing graph TB structure. The additions follow the existing naming conventions (DNS_PA, DNS_LA -> DNS_AUTH; PALINKS, LANDSCAPING -> KC). Clean.

  3. Target domains table update -- Status values changed from First dogfood target / Second target to Live / Live. This is a factual correction that reflects the current state (both domains are deployed and working). The new auth.palinks.app entry is added with status Live. Good.

  4. HCL example in the new doc (resource "godaddy_dns_record" "palinks_auth_a") correctly uses name = "auth" for the subdomain (not name = "@" which would be the apex). The domain, type, data, and ttl fields match the existing patterns in deployment.md examples. Correct.

  5. Cross-repo references -- The doc references pal-e-platform paths (salt/pillar/caddy.sls, modules/keycloak/main.tf) and pal-e-services PR #119. These cannot be verified from this repo alone but are clearly documented for cross-referencing.

  6. Dual-access design table -- Accurately describes how removing KC_HOSTNAME and relying on KC_PROXY_HEADERS=xforwarded lets the Host header drive issuer URL generation. This is correct Keycloak behavior for proxy-aware deployments.

  7. auth.palinks.app is a subdomain, not a separate domain -- The deployment.md target domains table lists it alongside palinks.app and landscaping-assistant.app. This is slightly misleading since auth.palinks.app is a subdomain of palinks.app, not a standalone domain registration. The table implies it is a peer entry. Minor, but worth noting. See nit below.

README.md update -- Adds one row to the documentation table. The description (auth.palinks.app -- shared public auth domain for Keycloak) is accurate and concise. Sorted correctly after Conventions.

BLOCKERS

None.

This is a docs-only PR. No executable code is introduced, so the standard blockers (test coverage, input validation, secrets, auth DRY violations) do not apply. No secrets, credentials, or hardcoded API keys appear in the documentation -- the HCL examples use module.hetzner_edge.server_ipv4 (a Terraform reference) rather than a literal IP for the new record, which is correct practice.

NITS

  1. Target domains table: subdomain vs domain -- auth.palinks.app is a subdomain of palinks.app, not an independently registered domain like the other entries. Consider indenting or annotating it to make this relationship clear (e.g., a footnote, or listing it as a sub-entry under palinks.app). As-is, the table implies all four entries are peer-level domain registrations.

  2. Rollback section uses kubectl directly -- The rollback procedure uses kubectl -n keycloak set env deployment/keycloak KC_HOSTNAME=.... This is fine as emergency documentation, but it bypasses the GitOps workflow (the change would be reverted on next ArgoCD sync). Consider adding a note: "This is an emergency hotfix; for a permanent rollback, re-add KC_HOSTNAME to the Terraform module and apply."

  3. Cross-reference in deployment.md -- The added line See [Keycloak Public Ingress](keycloak-public-ingress.md) uses a relative path. This is correct for Markdown rendering in Forgejo, but it would be more consistent with the README pattern to also add a brief description: See [Keycloak Public Ingress](keycloak-public-ingress.md) for the auth domain architecture and dual-access design. (Wait -- the existing text already says "for the auth domain architecture." This is fine as-is.)

  4. Section "4. pal-e-services" says "Already done" -- If this is a documentation of the planned changes across repos, having a section that says "no changes needed" is slightly confusing. Consider removing it entirely or reframing as a "Verification" step rather than a "Changes Required" item. Alternatively, keep it to confirm that no work is needed -- both approaches are defensible.

SOP COMPLIANCE

  • PR body has: Summary, Changes, Test Plan, Related -- all present and well-structured
  • No secrets committed -- HCL examples use variable references, not literal credentials
  • No unnecessary file changes -- all 3 files are directly relevant (new doc, updated deployment doc, updated README)
  • Commit messages are descriptive (branch name docs/keycloak-public-ingress follows convention)
  • PR body includes Closes #36 linking to parent issue
  • Review Checklist present in PR body

PROCESS OBSERVATIONS

  • Documentation-first approach -- Documenting the auth ingress architecture before implementing the actual infrastructure changes is good practice. The per-repo change list serves as a concrete implementation plan.
  • Cross-repo awareness -- The doc clearly maps which repos need changes and in what order. This reduces deployment risk when the actual implementation happens.
  • Change failure risk -- Low. Docs-only changes cannot break the provider or any infrastructure.

VERDICT: APPROVED

## PR #37 Review ### DOMAIN REVIEW **Tech stack:** Markdown documentation (Mermaid diagrams, HCL code examples, YAML config snippets). This is a docs-only PR -- no executable code changes. Domain review focuses on documentation quality, technical accuracy, and consistency with the existing codebase. **Documentation quality:** The new `docs/keycloak-public-ingress.md` is well-structured. It follows a clear problem-solution-architecture pattern, includes Mermaid sequence diagrams for both the broken state and the working state, and provides concrete per-repo change instructions with inline code samples. **Technical accuracy checks:** 1. **Edge IP consistency** -- The doc uses `178.156.129.142` throughout, which matches the existing `deployment.md` Mermaid diagram and the `architecture.md` context. Consistent. 2. **Mermaid diagram updates to `deployment.md`** -- The diff adds `DNS_AUTH`, `KC`, and their connections in the correct places within the existing `graph TB` structure. The additions follow the existing naming conventions (`DNS_PA`, `DNS_LA` -> `DNS_AUTH`; `PALINKS`, `LANDSCAPING` -> `KC`). Clean. 3. **Target domains table update** -- Status values changed from `First dogfood target` / `Second target` to `Live` / `Live`. This is a factual correction that reflects the current state (both domains are deployed and working). The new `auth.palinks.app` entry is added with status `Live`. Good. 4. **HCL example** in the new doc (`resource "godaddy_dns_record" "palinks_auth_a"`) correctly uses `name = "auth"` for the subdomain (not `name = "@"` which would be the apex). The `domain`, `type`, `data`, and `ttl` fields match the existing patterns in `deployment.md` examples. Correct. 5. **Cross-repo references** -- The doc references `pal-e-platform` paths (`salt/pillar/caddy.sls`, `modules/keycloak/main.tf`) and `pal-e-services` PR #119. These cannot be verified from this repo alone but are clearly documented for cross-referencing. 6. **Dual-access design table** -- Accurately describes how removing `KC_HOSTNAME` and relying on `KC_PROXY_HEADERS=xforwarded` lets the `Host` header drive issuer URL generation. This is correct Keycloak behavior for proxy-aware deployments. 7. **`auth.palinks.app` is a subdomain, not a separate domain** -- The `deployment.md` target domains table lists it alongside `palinks.app` and `landscaping-assistant.app`. This is slightly misleading since `auth.palinks.app` is a subdomain of `palinks.app`, not a standalone domain registration. The table implies it is a peer entry. Minor, but worth noting. See nit below. **README.md update** -- Adds one row to the documentation table. The description (`auth.palinks.app -- shared public auth domain for Keycloak`) is accurate and concise. Sorted correctly after `Conventions`. ### BLOCKERS None. This is a docs-only PR. No executable code is introduced, so the standard blockers (test coverage, input validation, secrets, auth DRY violations) do not apply. No secrets, credentials, or hardcoded API keys appear in the documentation -- the HCL examples use `module.hetzner_edge.server_ipv4` (a Terraform reference) rather than a literal IP for the new record, which is correct practice. ### NITS 1. **Target domains table: subdomain vs domain** -- `auth.palinks.app` is a subdomain of `palinks.app`, not an independently registered domain like the other entries. Consider indenting or annotating it to make this relationship clear (e.g., a footnote, or listing it as a sub-entry under `palinks.app`). As-is, the table implies all four entries are peer-level domain registrations. 2. **Rollback section uses `kubectl` directly** -- The rollback procedure uses `kubectl -n keycloak set env deployment/keycloak KC_HOSTNAME=...`. This is fine as emergency documentation, but it bypasses the GitOps workflow (the change would be reverted on next ArgoCD sync). Consider adding a note: "This is an emergency hotfix; for a permanent rollback, re-add `KC_HOSTNAME` to the Terraform module and apply." 3. **Cross-reference in `deployment.md`** -- The added line `See [Keycloak Public Ingress](keycloak-public-ingress.md)` uses a relative path. This is correct for Markdown rendering in Forgejo, but it would be more consistent with the README pattern to also add a brief description: `See [Keycloak Public Ingress](keycloak-public-ingress.md) for the auth domain architecture and dual-access design.` (Wait -- the existing text already says "for the auth domain architecture." This is fine as-is.) 4. **Section "4. pal-e-services" says "Already done"** -- If this is a documentation of the planned changes across repos, having a section that says "no changes needed" is slightly confusing. Consider removing it entirely or reframing as a "Verification" step rather than a "Changes Required" item. Alternatively, keep it to confirm that no work is needed -- both approaches are defensible. ### SOP COMPLIANCE - [x] PR body has: Summary, Changes, Test Plan, Related -- all present and well-structured - [x] No secrets committed -- HCL examples use variable references, not literal credentials - [x] No unnecessary file changes -- all 3 files are directly relevant (new doc, updated deployment doc, updated README) - [x] Commit messages are descriptive (branch name `docs/keycloak-public-ingress` follows convention) - [x] PR body includes `Closes #36` linking to parent issue - [x] Review Checklist present in PR body ### PROCESS OBSERVATIONS - **Documentation-first approach** -- Documenting the auth ingress architecture before implementing the actual infrastructure changes is good practice. The per-repo change list serves as a concrete implementation plan. - **Cross-repo awareness** -- The doc clearly maps which repos need changes and in what order. This reduces deployment risk when the actual implementation happens. - **Change failure risk** -- Low. Docs-only changes cannot break the provider or any infrastructure. ### VERDICT: APPROVED
Fix review nits: subdomain annotation, rollback clarity, section cleanup
Some checks failed
ci/woodpecker/push/woodpecker Pipeline failed
Details
ci/woodpecker/pr/woodpecker Pipeline failed
Details
4401918b7f 
- Indent auth.palinks.app under palinks.app in target domains table
  to clarify it's a subdomain, not a separate registration
- Add permanent rollback path (Terraform) alongside emergency kubectl
- Remove "already done" pal-e-services section from changes list
- Renumber remaining sections

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

All 4 nits addressed in 4401918:

  1. Subdomain annotation -- auth.palinks.app indented under palinks.app in the target domains table with "(subdomain)" label
  2. Rollback clarity -- Added permanent rollback path via Terraform; noted kubectl override is reverted on next ArgoCD sync
  3. Cross-reference in deployment.md -- was already fine per the review ("This is fine as-is"), no change
  4. "Already done" section removed -- dropped the pal-e-services section entirely, renumbered remaining sections
All 4 nits addressed in 4401918: 1. **Subdomain annotation** -- `auth.palinks.app` indented under `palinks.app` in the target domains table with "(subdomain)" label 2. **Rollback clarity** -- Added permanent rollback path via Terraform; noted kubectl override is reverted on next ArgoCD sync 3. **Cross-reference in deployment.md** -- was already fine per the review ("This is fine as-is"), no change 4. **"Already done" section removed** -- dropped the pal-e-services section entirely, renumbered remaining sections
Add CI pipeline documentation across infra repos
Some checks failed
ci/woodpecker/push/woodpecker Pipeline failed
Details
ci/woodpecker/pr/woodpecker Pipeline was successful
Details
9d09a93a3e 
Documents the Woodpecker CI pipelines for pal-e-platform and
pal-e-services (plan-on-PR, apply-on-merge with module detection,
cross-pillar review) and godaddy-tofu (build + integration tests).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Some checks failed
ci/woodpecker/push/woodpecker Pipeline failed
Details
ci/woodpecker/pr/woodpecker Pipeline was successful
Details
This pull request can be merged automatically.
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin docs/keycloak-public-ingress:docs/keycloak-public-ingress
git switch docs/keycloak-public-ingress

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch main
git merge --no-ff docs/keycloak-public-ingress
git switch docs/keycloak-public-ingress
git rebase main
git switch main
git merge --ff-only docs/keycloak-public-ingress
git switch docs/keycloak-public-ingress
git rebase main
git switch main
git merge --no-ff docs/keycloak-public-ingress
git switch main
git merge --squash docs/keycloak-public-ingress
git switch main
git merge --ff-only docs/keycloak-public-ingress
git switch main
git merge docs/keycloak-public-ingress
git push origin main
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
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/godaddy-tofu!37
No description provided.