Add keycloak plugin for identity service deployment#377
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughThis PR adds a Keycloak addon plugin with default image settings, a pre-validation play that requires cert-manager's ChangesKeycloak Plugin Deployment
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
| value: keycloak | ||
| - name: KC_DB_PASSWORD | ||
| value: "" | ||
| image: quay.io/keycloak/keycloak:latest |
There was a problem hiding this comment.
this has to be exposed at the plugin.yaml level
There was a problem hiding this comment.
I've added them in plugin.yaml:
defaults:
keycloak_image: quay.io/keycloak/keycloak:latest
keycloak_db_image: quay.io/sclorg/postgresql-15-c9s:latest
keycloak_db_wait_image: postgres
keycloak_cli_image: quay.io/openshift/origin-cli:latest
or do you mean under registries? I'm trying to make it work in connected mode for now, but it's good to raise these issues at this moment
| name: keycloak-database-password | ||
| - name: POSTGRESQL_DATABASE | ||
| value: keycloak | ||
| image: quay.io/sclorg/postgresql-15-c9s:latest |
|
not sure what we think about a keycloak plugin, cc @kbsingh |
![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
🧹 Nitpick comments (1)
plugins/keycloak/tasks/deploy.yaml (1)
19-20: ⚡ Quick winUse configurable retry/delay values for readiness polling as well.
Readiness waits are hard-coded, while apply uses
k8s_retries/k8s_delay. Reusing configurable values keeps behavior consistent across cluster sizes and avoids brittle timeouts.Suggested patch
- name: Wait for Keycloak database kubernetes.core.k8s_info: @@ register: __r_keycloak_db - retries: 30 - delay: 10 + retries: "{{ k8s_retries }}" + delay: "{{ k8s_delay }}" @@ - name: Wait for Keycloak service kubernetes.core.k8s_info: @@ register: __r_keycloak_svc - retries: 60 - delay: 10 + retries: "{{ k8s_retries }}" + delay: "{{ k8s_delay }}"Also applies to: 32-33
🤖 Prompt for 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. In `@plugins/keycloak/tasks/deploy.yaml` around lines 19 - 20, Replace the hard-coded readiness wait values with the existing configurable variables used by apply: change the readiness polling steps that currently set "retries: 30" and "delay: 10" to reference the shared variables (e.g., use k8s_retries and k8s_delay) so polling uses the same configurable values; update the other occurrence(s) (the second readiness block at the later occurrence) as well so all readiness polls in this file use k8s_retries/k8s_delay consistently.
🤖 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 `@plugins/keycloak/tasks/pre-validate.yaml`:
- Around line 2-19: The kubernetes.core.k8s_info call can error out (CRD/API
missing) and abort the play before your explicit failure message; change the
"Check default-ca ClusterIssuer exists" task (the kubernetes.core.k8s_info task
that registers __r_ca_issuer) to not abort the play by adding failed_when: false
(or ignore_errors: true), then update the "Fail if default-ca ClusterIssuer is
not ready" ansible.builtin.fail task to trigger when __r_ca_issuer.failed |
default(false) is true OR when __r_ca_issuer.resources is empty or has no Ready
condition (use the existing selectattr checks), so the controlled failure
message is always shown even if the k8s_info call hit a missing API/CRD error.
---
Nitpick comments:
In `@plugins/keycloak/tasks/deploy.yaml`:
- Around line 19-20: Replace the hard-coded readiness wait values with the
existing configurable variables used by apply: change the readiness polling
steps that currently set "retries: 30" and "delay: 10" to reference the shared
variables (e.g., use k8s_retries and k8s_delay) so polling uses the same
configurable values; update the other occurrence(s) (the second readiness block
at the later occurrence) as well so all readiness polls in this file use
k8s_retries/k8s_delay consistently.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f9509899-f42f-4b41-8b96-abdfd60daf69
📒 Files selected for processing (4)
plugins/keycloak/files/keycloak.yaml.j2plugins/keycloak/plugin.yamlplugins/keycloak/tasks/deploy.yamlplugins/keycloak/tasks/pre-validate.yaml
✅ Files skipped from review due to trivial changes (1)
- plugins/keycloak/plugin.yaml
Deploys a self-managed PostgreSQL database and Keycloak service based on the osac-installer prerequisites. Pre-validates that the default-ca ClusterIssuer from the trust-manager plugin is ready before deploying.
2 participants
Deploys a self-managed PostgreSQL database and Keycloak service based on the osac-installer prerequisites.
Pre-validates that the default-ca ClusterIssuer from the trust-manager plugin is ready before deploying.
Summary by CodeRabbit