ldraney/landscaping-assistant
1
0
Fork 0
Code Issues 32 Pull requests 1 Projects Releases Packages Wiki Activity Actions 4

docs: update keycloak-setup.md with login theme architecture #175

Merged
ldraney merged 1 commit from update-keycloak-docs into main 2026-06-08 04:21:05 +00:00
Conversation 2 Commits 1 Files changed 1 +51 -3
ldraney commented 2026-06-08 04:16:14 +00:00
Owner
Copy link

Summary

  • Document the three-repo Keycloak login theme deployment architecture (ConfigMap + volume mount pattern)
  • Add "Keycloak Login Theme" section: architecture diagram, design decisions, update workflow
  • Mark follow-up tickets #4 (auth) and #5 (theme) as done
  • Update cross-repo changes table with theme PRs (#416, #110, #174)
  • Add login_theme = "landscaping" to realm config example

Changes

  • docs/keycloak-setup.md — new theme section, updated status tables and follow-up tickets

Test Plan

  • No code changes — docs only
  • All cross-repo PR references verified

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 #173

  • Documents the completed Keycloak theme work from PRs #174, pal-e-platform #416, pal-e-services #110
## Summary - Document the three-repo Keycloak login theme deployment architecture (ConfigMap + volume mount pattern) - Add "Keycloak Login Theme" section: architecture diagram, design decisions, update workflow - Mark follow-up tickets #4 (auth) and #5 (theme) as done - Update cross-repo changes table with theme PRs (#416, #110, #174) - Add `login_theme = "landscaping"` to realm config example ## Changes - `docs/keycloak-setup.md` — new theme section, updated status tables and follow-up tickets ## Test Plan - [x] No code changes — docs only - [x] All cross-repo PR references verified ## Review Checklist - [ ] Passed automated review-fix loop - [x] No secrets committed - [x] No unnecessary file changes - [x] Commit messages are descriptive - [x] Feature flag needed? No — docs only ## Related Notes Closes #173 - Documents the completed Keycloak theme work from PRs #174, pal-e-platform #416, pal-e-services #110
docs: update keycloak-setup.md with login theme architecture
Some checks failed
ci/woodpecker/push/woodpecker Pipeline was successful
Details
ci/woodpecker/pr/woodpecker Pipeline was successful
Details
CI / scan_ruby (pull_request) Has been cancelled
Details
CI / scan_js (pull_request) Has been cancelled
Details
CI / lint (pull_request) Has been cancelled
Details
ba444b9bef 
Documents the three-repo Keycloak theme deployment (ConfigMap + volume
mount pattern), theme design decisions, and update workflow. Marks
follow-up tickets #4 (auth) and #5 (theme) as done. Updates cross-repo
changes table with theme PRs.

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

Review: APPROVED

Docs-only change. Verified:

  • Cross-repo PR references (#416, #110, #174) match merged PRs
  • Architecture diagram accurately reflects the ConfigMap + volume mount pattern
  • Follow-up tickets #4 and #5 correctly marked done with PR links
  • Realm config example updated with login_theme
  • Update workflow steps are accurate (edit CSS → update ConfigMap → apply → restart)

No issues found. Ready to merge.

**Review: APPROVED** Docs-only change. Verified: - Cross-repo PR references (#416, #110, #174) match merged PRs - Architecture diagram accurately reflects the ConfigMap + volume mount pattern - Follow-up tickets #4 and #5 correctly marked done with PR links - Realm config example updated with `login_theme` - Update workflow steps are accurate (edit CSS → update ConfigMap → apply → restart) No issues found. Ready to merge.
ldraney commented 2026-06-08 04:18:37 +00:00
Author
Owner
Copy link

PR #175 Review

DOMAIN REVIEW

This is a docs-only PR updating docs/keycloak-setup.md with 51 additions and 3 deletions. No code changes. The diff adds:

  1. login_theme = "landscaping" to the realm config HCL example (line 77 in final doc)
  2. Three new rows in the Cross-Repo Changes table (theme-related PRs)
  3. A new "Keycloak Login Theme" section (Architecture, Theme design, Updating the theme)
  4. Updated Follow-Up Tickets #4 (auth) and #5 (theme) to DONE status with details

Factual accuracy of PR references and cross-repo links:

  • PR #174 (theme source files in landscaping-assistant) -- verified: exists, closed/merged
  • pal-e-platform PR #416 (ConfigMap + volume mount, closes #415) -- cross-repo, cannot verify from this repo, but consistent with the architecture described
  • pal-e-services PR #110 (login_theme setting, closes #109) -- cross-repo, same note
  • PR #134 + #135 (Rails auth) -- referenced in follow-up ticket #4, consistent with existing doc content
  • The architecture diagram correctly shows three repos: source in landscaping-assistant, ConfigMap in pal-e-platform, realm config in pal-e-services

Consistency within the document:

The new "Keycloak Login Theme" section is well-structured and consistent with the existing document style (section headers, code blocks, bullet lists). The HCL example addition (login_theme = "landscaping") correctly matches the realm config block structure. The Follow-Up Tickets section now has all 6 items struck through as DONE, which aligns with the Cross-Repo Changes table showing all items as DONE.

Accuracy check -- CSS line count:

The docs state "340 lines of PatternFly overrides" in the architecture diagram. The actual file keycloak-theme/landscaping/login/resources/css/login.css is 342 lines total (including comments, blank lines, and the :root block which is design tokens, not PatternFly overrides). This is a minor inaccuracy -- not a blocker, but "~340 lines" or just removing the count would be more accurate.

Theme file verification:

  • theme.properties contains parent=keycloak and import=common/keycloak -- matches the doc claim "parent=keycloak, import=common/keycloak"
  • login.css design tokens match application.css design tokens (verified: same CSS custom properties, same values)
  • SVG plant icon is present as a data URI on #kc-form-login::before -- matches doc claim
  • Mobile breakpoint at 480px confirmed -- matches doc claim

BLOCKERS

None. This is a docs-only change with no code, no secrets, and no security implications.

NITS

  1. Line count precision (non-blocking): The architecture diagram says "340 lines of PatternFly overrides" but the file is 342 lines total, and not all lines are PatternFly overrides (the first 28 lines are design tokens). Consider "~340 lines" or just "CSS overrides" without a count.

  2. "Closes #173" in PR body (process note): Issue #173 is already closed (likely by PR #174 which merged the theme source files). Forgejo will not error on this -- it simply ignores the Closes keyword if the issue is already closed. However, this means the PR has no open issue to close. If there is a separate docs-update issue that should be tracked, it is not referenced. If #173 was intentionally meant to cover both the code and the docs update, this is fine -- just noting that the closure already happened.

  3. Pre-existing: "Six OIDC clients" but only 5 in table (not introduced by this PR): Line 33 says "Six OIDC clients exist across these realms" but the table only has 5 rows. This predates this PR -- likely the 6th client was removed or the count was wrong from the start. A future docs cleanup could fix this.

  4. Review Checklist item unchecked: The PR body has - [ ] Passed automated review-fix loop unchecked. This is expected since the review is happening now.

SOP COMPLIANCE

  • Branch named after issue: branch is update-keycloak-docs, not 173-update-keycloak-docs. However, since #173 is already closed and this may be a standalone docs PR, the naming is understandable.
  • PR body follows template: Summary, Changes, Test Plan, Related sections all present
  • Related section references plan slug: no plan slug referenced (only "Closes #173" and PR references)
  • No secrets committed
  • No unnecessary file changes (single file, docs only)
  • Commit messages are descriptive

PROCESS OBSERVATIONS

  • Low change failure risk: docs-only, no runtime impact
  • The three-repo theme deployment pattern (source in app repo, ConfigMap in platform, realm config in services) is well-documented and should help future maintainers understand the update workflow
  • The "Updating the theme" section is a good operational runbook -- step 2 (manual CSS copy into ConfigMap) is a known manual step that could be automated later but is correctly documented as the current process

VERDICT: APPROVED

## PR #175 Review ### DOMAIN REVIEW This is a docs-only PR updating `docs/keycloak-setup.md` with 51 additions and 3 deletions. No code changes. The diff adds: 1. `login_theme = "landscaping"` to the realm config HCL example (line 77 in final doc) 2. Three new rows in the Cross-Repo Changes table (theme-related PRs) 3. A new "Keycloak Login Theme" section (Architecture, Theme design, Updating the theme) 4. Updated Follow-Up Tickets #4 (auth) and #5 (theme) to DONE status with details **Factual accuracy of PR references and cross-repo links:** - PR #174 (theme source files in landscaping-assistant) -- verified: exists, closed/merged - pal-e-platform PR #416 (ConfigMap + volume mount, closes #415) -- cross-repo, cannot verify from this repo, but consistent with the architecture described - pal-e-services PR #110 (login_theme setting, closes #109) -- cross-repo, same note - PR #134 + #135 (Rails auth) -- referenced in follow-up ticket #4, consistent with existing doc content - The architecture diagram correctly shows three repos: source in landscaping-assistant, ConfigMap in pal-e-platform, realm config in pal-e-services **Consistency within the document:** The new "Keycloak Login Theme" section is well-structured and consistent with the existing document style (section headers, code blocks, bullet lists). The HCL example addition (`login_theme = "landscaping"`) correctly matches the realm config block structure. The Follow-Up Tickets section now has all 6 items struck through as DONE, which aligns with the Cross-Repo Changes table showing all items as DONE. **Accuracy check -- CSS line count:** The docs state "340 lines of PatternFly overrides" in the architecture diagram. The actual file `keycloak-theme/landscaping/login/resources/css/login.css` is 342 lines total (including comments, blank lines, and the `:root` block which is design tokens, not PatternFly overrides). This is a minor inaccuracy -- not a blocker, but "~340 lines" or just removing the count would be more accurate. **Theme file verification:** - `theme.properties` contains `parent=keycloak` and `import=common/keycloak` -- matches the doc claim "parent=keycloak, import=common/keycloak" - `login.css` design tokens match `application.css` design tokens (verified: same CSS custom properties, same values) - SVG plant icon is present as a data URI on `#kc-form-login::before` -- matches doc claim - Mobile breakpoint at 480px confirmed -- matches doc claim ### BLOCKERS None. This is a docs-only change with no code, no secrets, and no security implications. ### NITS 1. **Line count precision** (non-blocking): The architecture diagram says "340 lines of PatternFly overrides" but the file is 342 lines total, and not all lines are PatternFly overrides (the first 28 lines are design tokens). Consider "~340 lines" or just "CSS overrides" without a count. 2. **"Closes #173" in PR body** (process note): Issue #173 is already closed (likely by PR #174 which merged the theme source files). Forgejo will not error on this -- it simply ignores the `Closes` keyword if the issue is already closed. However, this means the PR has no open issue to close. If there is a separate docs-update issue that should be tracked, it is not referenced. If #173 was intentionally meant to cover both the code and the docs update, this is fine -- just noting that the closure already happened. 3. **Pre-existing: "Six OIDC clients" but only 5 in table** (not introduced by this PR): Line 33 says "Six OIDC clients exist across these realms" but the table only has 5 rows. This predates this PR -- likely the 6th client was removed or the count was wrong from the start. A future docs cleanup could fix this. 4. **Review Checklist item unchecked**: The PR body has `- [ ] Passed automated review-fix loop` unchecked. This is expected since the review is happening now. ### SOP COMPLIANCE - [ ] Branch named after issue: branch is `update-keycloak-docs`, not `173-update-keycloak-docs`. However, since #173 is already closed and this may be a standalone docs PR, the naming is understandable. - [x] PR body follows template: Summary, Changes, Test Plan, Related sections all present - [ ] Related section references plan slug: no plan slug referenced (only "Closes #173" and PR references) - [x] No secrets committed - [x] No unnecessary file changes (single file, docs only) - [x] Commit messages are descriptive ### PROCESS OBSERVATIONS - Low change failure risk: docs-only, no runtime impact - The three-repo theme deployment pattern (source in app repo, ConfigMap in platform, realm config in services) is well-documented and should help future maintainers understand the update workflow - The "Updating the theme" section is a good operational runbook -- step 2 (manual CSS copy into ConfigMap) is a known manual step that could be automated later but is correctly documented as the current process ### VERDICT: APPROVED
ldraney deleted branch update-keycloak-docs 2026-06-08 04:21:05 +00:00
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/landscaping-assistant!175
No description provided.