Repository navigation
feat(keycloak): expose organizationsEnabled realm toggle - #3031
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new configuration toggle for the Keycloak realm managed by the Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
github-actions
Bot
added
area/uncategorized
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: cozystack/cozystack/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe ChangesKeycloak Organizations Feature Flag
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~5 minutes Change: Feature Merge Risk: ⚪ Minimal · up to The setting remains false by default and allows explicit enablement; rendering tests cover both values. No material merge-blocking risk is evident. Architecture SummaryArchitecture risk: 🔵 Low · up to The change affects 1 system. Changed systems: Architecture concerns Review detailsSystems and components
Before / after behavior
🚥 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 |
dosubot
Bot
added
the
area/platform
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds the organizationsEnabled parameter to the Keycloak configuration chart, allowing the Keycloak Organizations feature to be enabled for the cozy realm (defaulting to false). The review feedback recommends using an explicit boolean comparison instead of the default function in the Helm template, as default can behave unexpectedly with boolean false values.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| spec: | ||
| realmName: cozy | ||
| clusterKeycloakRef: keycloak-cozy | ||
| organizationsEnabled: {{ .Values.organizationsEnabled | default false }} |
There was a problem hiding this comment.
In Helm/Go templates, using the default function with boolean values can be risky because a boolean false is treated as empty/falsy, which triggers the default value. While this works here because the default is also false, a more robust and idiomatic way to handle boolean values is to use an explicit eq comparison against true.
organizationsEnabled: {{ eq .Values.organizationsEnabled true }}
Aleksei Sviridkin (lexfrei)
added
area/keycloak
|
This conflicts with main and needs a rebase. The |
658c083 to
4214cfd
Compare
github-actions
Bot
added
the
area/uncategorized
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
NOT LGTM — the change itself is correct, but this chart already ships a helm-unittest suite over the very template being changed, and the new value arrives untested.
Business context: ClusterKeycloakRealm is rendered from the chart, and the keycloak-operator reconciles it by PUTting that spec back, so enabling Organizations through the admin console is reverted on the next reconcile; the realm field has to be exposed as a chart value to make it durable.
Blockers
B1: new value and new rendered field ship without a unit test
File: packages/system/keycloak-configure/ (missing tests/ case)
Issue: packages/system/keycloak-configure/tests/login_theme_test.yaml already covers templates/configure-kk.yaml — the same template this PR edits — asserting a rendered ClusterKeycloakRealm field from a chart value, and Makefile has test: helm unittest ., so CI runs the suite. Adding organizationsEnabled to values.yaml and a new spec.organizationsEnabled line to the template without a corresponding test leaves the contract unpinned in a chart that has an established test convention.
Impact: nothing pins the default (false, i.e. no behaviour change — the PR's central claim) or the enabled path, so either can regress silently. A future edit to the {{ .Values.organizationsEnabled | default false }} expression would not be caught.
Fix: add a case to tests/ mirroring the existing suite's shape — template: templates/configure-kk.yaml with documentSelector on kind: ClusterKeycloakRealm — asserting that the chart default renders spec.organizationsEnabled: false, and that set: {organizationsEnabled: true} renders spec.organizationsEnabled: true.
Non-blocking follow-up
| default false is correct here — it covers the case where the key is absent entirely, which would otherwise render an empty value. Worth knowing that the guard does not generalise: sprig's default treats false as empty, so if the default is ever flipped to true, | default true would silently override a user's explicit organizationsEnabled: false. At that point the expression would need a hasKey-style guard instead.
What I verified: the vendored CRD at packages/system/keycloak-operator/charts/keycloak-operator/crds/v1.edp.epam.com_clusterkeycloakrealms.yaml does declare organizationsEnabled (with default: false), so the field is not silently pruned by the structural schema and the toggle genuinely reaches the realm. The rendered value is an unquoted YAML boolean, and the default preserves current behaviour.
github-actions
Bot
added
size/M
|
B1 addressed in |
|
This PR has had no activity for 60 days and was marked |
github-actions
Bot
added
the
lifecycle/stale
github-actions
Bot
removed
the
lifecycle/stale
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
mattia-eleuteri NOT LGTM. The code and the new test are fine now, but the first commit carries model attribution trailers that main's required pre-commit check rejects.
The test blocker from July is closed. tests/organizations_enabled_test.yaml pins both the default and the enabled path. On a trial merge with current main the chart suite passes, 6 suites and 13 tests. Both cases are real checks: with the template value hardcoded to false the enabled case fails, and with the line removed both fail. The change still lands in the right place. Main's operator build (edp-keycloak-operator facbc36) maps spec.organizationsEnabled into the realm representation in pkg/realmbuilder/settings_builder.go, and the vendored ClusterKeycloakRealm CRD still declares the field with default: false.
Blocker
B1: model attribution in the trailers of 4214cfd
File: commit 4214cfd71e3e5a30a466c4ad00bf798a51c9b300
The commit ends with these two lines:
Assisted-By: Claude <[email protected]>
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
docs/agents/contributing.md ("AI Agent Attribution") allows one disclosure form only, Assisted-by: LLM, and no model or vendor name in any trailer. Since early September, main also enforces this in CI. .github/workflows/pre-commit.yml runs hack/check-commit-trailers.sh over the PR commits, and on this range it exits 1 with attribution trailer value must be exactly LLM. The only CI run on this head is from July, so it predates that check. After a rebase the required pre-commit check goes red. The repo merges with merge commits, so these trailers would land in main as is. I should have flagged this in July and missed it, sorry for the extra round.
Fix: reword 4214cfd with git rebase --interactive. Replace both lines with a single Assisted-by: LLM next to Signed-off-by, then rebase onto current main and force-push. The rebase also gets fresh CI on the branch.
Non-blocking
Main now takes the realm name from _cluster.oidc-realm-name, with cozy as the fallback. The new values.yaml comment and the PR body both say "the cozy realm". "The platform realm" would stay true for installs that rename it.
The keycloak-configure chart hardcoded the realm with the Organizations feature off, with no value to enable it. Operators who flip organizationsEnabled in the Keycloak admin console see it reverted on the next ClusterKeycloakRealm reconcile, since the EDP operator pushes the chart-rendered spec back to the realm. Expose organizationsEnabled in values.yaml (default false, no behaviour change) and map it to ClusterKeycloakRealm.spec.organizationsEnabled so the feature can be enabled declaratively and survive reconciles. Assisted-by: LLM Signed-off-by: mattia-eleuteri <[email protected]>
Cover both paths of the new value: the chart default renders spec.organizationsEnabled: false (no behaviour change), and setting organizationsEnabled: true renders through to the ClusterKeycloakRealm. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <[email protected]>
068df89 to
874590e
Compare
|
Aleksei Sviridkin (@lexfrei) B1 is fixed. I reworded 4214cfd: both trailers are gone, replaced by a single Non-blocking: the |
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
mattia-eleuteri LGTM. Trailers are fixed, and the only other change is the comment wording.
2 participants
What this PR does
organizationsEnabledvalue to thecozy-keycloak-configurechart (defaultfalse, no behaviour change).ClusterKeycloakRealm.spec.organizationsEnabledso the Keycloak Organizations feature can be turned on declaratively for the platform realm.Why
The chart hardcoded the realm without exposing this field. Enabling Organizations through the Keycloak admin console is not durable: the EDP keycloak-operator reconciles
ClusterKeycloakRealmbyPUTting the chart-rendered spec back to the realm, so the console change is reverted on the next reconcile (operator restart, periodic resync, or any CR change). Verified on Keycloak 26.5.2.Release note
Summary by CodeRabbit
trueto enable them.