feat(authup): mount an operator console theme - #5
Merged
Conversation
|
Note Reviews pausedUse the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe chart adds server theme configuration from inline files or an existing ConfigMap. It validates theme sources and paths, creates or mounts theme resources, exports theme environment settings, and supports optional raw head fragments. ChangesServer theme support
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant HelmValues
participant ThemeValidation
participant ThemeConfigMap
participant ServerDeployment
participant AuthupServer
HelmValues->>ThemeValidation: theme source configuration
ThemeValidation->>ThemeConfigMap: validate and render inline files
ThemeValidation->>ServerDeployment: theme mount and environment settings
ThemeConfigMap->>ServerDeployment: ConfigMap volume source
ServerDeployment->>AuthupServer: mounted theme files and theme environment
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@charts/authup/templates/_server-env.tpl`:
- Around line 230-239: Validate each $path in the server.theme.files loop before
generating ConfigMap entries, rejecting empty paths or any path whose encoded
themeConfigMapKey contains characters outside Kubernetes’ allowed [A-Za-z0-9._-]
set. Preserve the existing absolute-path, traversal, and "__" checks, and add
render coverage for spaces, colons, and empty paths.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c9093b68-ca07-4bb8-908a-57e6db47a85e
📒 Files selected for processing (7)
charts/authup/README.mdcharts/authup/ci/theme-values.yamlcharts/authup/templates/_server-env.tplcharts/authup/templates/server/configmap-theme.yamlcharts/authup/templates/server/deployment.yamlcharts/authup/values.schema.jsoncharts/authup/values.yaml
There was a problem hiding this comment.
Pull request overview
Adds first-class chart support for mounting an operator-managed Authup console theme via values, so console rebranding becomes a values-only change (inline files or an existing ConfigMap) rather than bespoke manifests and manual volume wiring.
Changes:
- Introduces
server.themevalues (inline file map and/or existing ConfigMap projection) and corresponding render-time validation/fail-loud behavior. - Wires theme mounting into the server Deployment (volumes/volumeMounts + checksum for inline theme ConfigMap) and sets theme-related env vars.
- Updates generated docs/schema and adds a CI render scenario (
ci/theme-values.yaml) to exercise the feature.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| charts/authup/values.yaml | Adds new server.theme values surface (files/existing ConfigMap/items/fragments flag). |
| charts/authup/values.schema.json | Regenerates schema to include server.theme fields. |
| charts/authup/templates/server/deployment.yaml | Adds theme checksum + conditionally appends theme volumes/volumeMounts. |
| charts/authup/templates/server/configmap-theme.yaml | New inline theme ConfigMap (with key-flattening for subdirectories). |
| charts/authup/templates/_server-env.tpl | Adds theme validation + env wiring + theme volume helpers. |
| charts/authup/README.md | Regenerates parameter table to document server.theme.*. |
| charts/authup/ci/theme-values.yaml | Adds CI values case covering inline theme files + fragments. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Author
|
@coderabbitai pause |
✅ Action performedReviews paused. |
tada5hi
added a commit
that referenced
this pull request
Aug 4, 2026
Two review findings on #5. The path -> ConfigMap key flattening only replaced "/", so a key carrying a space, a colon, an "@" or a leading dot produced a data key outside Kubernetes' ^[A-Za-z0-9._-]+$ and failed at apply time with a validation error rather than at render time. An empty key did the same. The path is now checked against the character set the server's own asset handler accepts, so the chart rejects at render time exactly what the server would 404 at request time. THEME_DIRECTORY_PATH / THEME_FRAGMENTS_ENABLED moved out of authup.server.configEnv into a deployment-only block, for the same reason the theme volume is not in authup.server.volumes: the migration Job inlines configEnv, so it was being told about a theme directory it deliberately does not mount. Nothing reads it there today (the migration command builds only the config and logger modules, never the http one), so this is consistency rather than a live failure, but the env should not describe a pod that does not exist. THEME_* stays in configEnv's reserved-key list, so a `server.config` entry still cannot emit a duplicate key into the same ConfigMap.
Adds `server.theme`, so rebranding the served consoles is a values change
rather than a hand-written ConfigMap plus two volume stanzas:
server:
theme:
enabled: true
files:
theme.json: |
{"version": 1, "tokens": {"--authup-periwinkle": "#c0392b"}}
assets/theme.css: |
.a-auth-shell-card { border-radius: 2px; }
The volume is mounted at /etc/authup/theme and THEME_DIRECTORY_PATH plus
THEME_FRAGMENTS_ENABLED are derived from it, mirroring the existing
`server.provisioning` UX (files map, existingConfigMap, tpl-rendered
content).
A ConfigMap key cannot contain "/", but the theme layout has
subdirectories, so keys are flattened to "__" and projected back by the
volume's items list. The operator writes "assets/theme.css" and never
sees the encoding. `existingConfigMapItems` exposes the same projection
for an existing ConfigMap, which is also the route for binary assets
(binaryData) that a values map cannot express.
The theme volume is deliberately NOT part of the shared
authup.server.volumes helper. The migration Job is a pre-upgrade hook and
hooks precede regular resources, so on the upgrade that first enables
theming it would reference a ConfigMap that does not exist yet and hang.
A migration run has no use for the theme either way. (The same shape
exists for `server.provisioning`, which this change deliberately leaves
alone.)
Render-time fails, in the chart's fail-loud style: enabled with no
content, files and existingConfigMap together, existingConfigMapItems
without existingConfigMap, and file keys that are absolute, traverse out
of the theme root, or contain the reserved "__". Theming's dominant
failure mode is a page that looks exactly like an un-themed page, so none
of these may render silently inert.
Whole-volume projection rather than subPath: a subPath mount is frozen
until the pod restarts, which would destroy authup's live theme reload.
Requires an authup image that supports THEME_DIRECTORY_PATH. Older images
ignore the variable, so enabling this against one is inert rather than
broken. With the theme off the rendered manifests are unchanged.
Two review findings on #5. The path -> ConfigMap key flattening only replaced "/", so a key carrying a space, a colon, an "@" or a leading dot produced a data key outside Kubernetes' ^[A-Za-z0-9._-]+$ and failed at apply time with a validation error rather than at render time. An empty key did the same. The path is now checked against the character set the server's own asset handler accepts, so the chart rejects at render time exactly what the server would 404 at request time. THEME_DIRECTORY_PATH / THEME_FRAGMENTS_ENABLED moved out of authup.server.configEnv into a deployment-only block, for the same reason the theme volume is not in authup.server.volumes: the migration Job inlines configEnv, so it was being told about a theme directory it deliberately does not mount. Nothing reads it there today (the migration command builds only the config and logger modules, never the http one), so this is consistency rather than a live failure, but the env should not describe a pod that does not exist. THEME_* stays in configEnv's reserved-key list, so a `server.config` entry still cannot emit a duplicate key into the same ConfigMap.
tada5hi
force-pushed
the
feat/theme-configmap
branch
from
August 4, 2026 07:39
f260f00 to
548b725
Compare
THEME_* moved out of authup.server.configEnv so the migration Job would not inherit env for a volume it does not mount. That also dropped it out of checksum/env, which hashes configEnv alone: flipping server.theme.fragmentsEnabled rewrote the server env ConfigMap without rolling the pods, and envFrom is snapshotted at container start, so running pods kept the stale value indefinitely. The annotation now hashes both halves of that ConfigMap. With the theme disabled the hash is unchanged, so non-theme releases do not roll on upgrade.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rebranding the served consoles becomes a values change instead of a hand-written ConfigMap plus two volume stanzas.
The volume mounts at
/etc/authup/theme, andTHEME_DIRECTORY_PATH/THEME_FRAGMENTS_ENABLEDare derived from it. Values UX mirrors the existingserver.provisioning:filesmap,existingConfigMap, tpl-rendered content.Pairs with authup/authup#3385, which adds the server side.
Subdirectories in a flat ConfigMap
A ConfigMap key cannot contain
/, but the theme layout hasassets/andfragments/. Keys are flattened to__and projected back by the volume'sitemslist, so the operator writesassets/theme.cssand never sees the encoding:existingConfigMapItemsexposes the same projection for an existing ConfigMap, which is also the route for binary assets (abinaryDatalogo) that a values map cannot express.The migration Job
The theme volume is deliberately not part of the shared
authup.server.volumeshelper. The migration Job is apre-upgradehook, and hooks precede regular resources, so on the upgrade that first enables theming it would reference a ConfigMap that does not exist yet and hang. A migration run has no use for the theme either way.The same shape exists today for
server.provisioning. I left it alone rather than widening this PR, but it is worth a follow-up.Render-time fails
Theming's dominant failure mode is a page that looks exactly like an un-themed page, so nothing may render silently inert. In the chart's existing fail-loud style:
enabledwith neitherfilesnorexistingConfigMapfilesandexistingConfigMaptogether (the latter would win, the former vanish)existingConfigMapItemswithoutexistingConfigMap.., or contain the reserved__Notes
Whole-volume projection rather than
subPath, because asubPathmount is frozen until the pod restarts and would destroy authup's live theme reload.Requires an authup image supporting
THEME_DIRECTORY_PATH; older images ignore the variable, so enabling it against one is inert rather than broken.Verification
make testgreen: helm lint, all 6 ci values files render, values-coverage 284 pathsci/theme-values.yamlscenario, so the kind matrix rolls the mount out liveSummary by CodeRabbit
New Features
Validation
Documentation