Skip to content

feat: deploy solar-ui as part of the solar chart - #743

Open
rebEllieous wants to merge 3 commits into
mainfrom
feature/solar-ui-in-solar-chart
Open

feat: deploy solar-ui as part of the solar chart#743
rebEllieous wants to merge 3 commits into
mainfrom
feature/solar-ui-in-solar-chart

Conversation

@rebEllieous

@rebEllieous rebEllieous commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

What

Builds and publishes the solar-ui image in CI, and adds a ui component to the solar chart so it can actually be deployed.

Relates to (odd-internal) # 62

Why

The UI is part of SolAr but had no delivery path: the image was never published, and no chart could run it

Testing

  • Built the ui target and ran the image: GET / returns the embedded index.html (200), /assets/*.js the 584 KB bündle
  • helm lint clean. helm template renders the UI objects with GHCR repo, merged pull secrets and env-injected client secret, and fails as intended when ui.enabled=true without an issuer. Default render is identical to before

Notes for reviewers

  • RBAC: new ClusterRole for the UI ServiceAccount — get/list on namespaces (the BFF enumerates with its own identity, then filters per user via SelfSubjectRulesReview), plus impersonate on users/groups only when ui.args.authMode=impersonate
  • Security default: ui.enabled=false, and the template requireds ui.oidc.issuer / ui.oidc.redirectURL. Without an issuer the BFF serves the K8s API as its own ServiceAccount to anyone who reaches the pod

Checklist

  • Tests added/updated
  • No breaking changes
  • Readable commit history (squashed and cleaned up as desired)
  • AI code review considered and comments resolved

@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@rebEllieous, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 58 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: b9c95d8d-7f44-4072-8d1a-8ea96e2c9498

📥 Commits

Reviewing files that changed from the base of the PR and between 8e3f8cb and 2494a3f.

📒 Files selected for processing (10)
  • .github/workflows/docker.yaml
  • Dockerfile
  • charts/solar/README.md
  • charts/solar/templates/_helpers.tpl
  • charts/solar/templates/ui/clusterrole.yaml
  • charts/solar/templates/ui/clusterrolebinding.yaml
  • charts/solar/templates/ui/deployment.yaml
  • charts/solar/templates/ui/service.yaml
  • charts/solar/templates/ui/serviceaccount.yaml
  • charts/solar/values.yaml
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/solar-ui-in-solar-chart

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cbrgm cbrgm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only nitpicks e.g. two tiny things beyond the inline suggestions

  • ui.args.authMode is the only key under args, while extraArgs sits right next to it as a sibling. Reads a bit odd, you'd maybe expect ui.authMode directly, or extraArgs nested under args. Pure naming, not important.
  • ui.imagePullSecrets doc is terser than the siblings (renderer spells out the [{name: my-secret}] shape). Could mirror that one-liner so the value docs stay uniform, up to you 🤷

Comment thread charts/solar/values.yaml Outdated
Comment thread charts/solar/values.yaml Outdated
@cbrgm

cbrgm commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Otherwise LGTM

@rebEllieous

Copy link
Copy Markdown
Contributor Author

Only nitpicks e.g. two tiny things beyond the inline suggestions

* `ui.args.authMode` is the only key under `args`, while `extraArgs` sits right next to it as a sibling. Reads a bit odd, you'd maybe expect `ui.authMode` directly, or `extraArgs` nested under `args`. Pure naming, not important.

* `ui.imagePullSecrets` doc is terser than the siblings (renderer spells out the `[{name: my-secret}]` shape). Could mirror that one-liner so the value docs stay uniform, up to you 🤷

good point, i looked into it: both these point would make the ui section more inconsistent with the rest of the file:
the args, exraArgs split is convention, applied that way in other sections of the chart as well. This also mirrors the python *args, **kwargs pattern (which is why its often used elsewhere) so at least to me not splitting it would read a bit odd to me 😅
as for having it under ui.authMode: the args block, even if it only holds a single item (for now) is what's passed onto the cli call as args for the bff. THe block serves a purpose in that way, i'd prefer to keep it as is.

as for the pullSecrets: renderer is actually the outlier, ui is consistent with apiserver and controller. Renderer looks slightly different because the pod runs in the rt's namespace, so the secret has to exist in multiple namespaces

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants