feat: deployed Keycloak and configure agentcube#367
Conversation
- Declarative Realm Import: Created a ConfigMap with the realm JSON defining clients (agentcube-sdk, agentcube-router, workloadmanager, agentcube-admin) and role hierarchy (sandbox:invoke -> sandbox:manage -> admin). - Deployment & Service: Added Keycloak Deployment and Service manifests with management-port health probes and a 300s startupProbe window. - Helm Integration: Added keycloak configuration values to values.yaml, gated behind 'keycloak.enabled: false' to ensure zero impact by default. Signed-off-by: Mahil Patel <mahilpatel0808@gmail.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![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 introduces a Keycloak deployment, service, and realm configuration to the Helm chart for external user authentication. The reviewer identified several critical security and configuration issues: wildcard redirect URIs and web origins on the confidential SDK client should be configurable; the admin password should be sourced from a Kubernetes Secret instead of plaintext; production mode lacks support for external databases and proxy/HTTPS settings; SSL requirements should be dynamically enabled; and global image pull secrets need to be propagated to the Keycloak pod template.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds an optional Keycloak deployment to the Helm base chart to support external user authentication, including default values, a Deployment/Service, and a realm-import ConfigMap.
Changes:
- Introduces
keycloakconfiguration block in chart values. - Adds Keycloak Deployment + Service template gated by
keycloak.enabled. - Adds a realm JSON import ConfigMap template for bootstrapping clients/roles.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 10 comments.
| File | Description |
|---|---|
| manifests/charts/base/values.yaml | Adds configurable values for enabling and running a Keycloak instance |
| manifests/charts/base/templates/keycloak/keycloak.yaml | Adds Deployment/Service templates for Keycloak |
| manifests/charts/base/templates/keycloak/keycloak-realm.yaml | Adds ConfigMap for importing an initial realm configuration |
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #367 +/- ##
==========================================
+ Coverage 47.57% 55.54% +7.97%
==========================================
Files 30 34 +4
Lines 2819 3190 +371
==========================================
+ Hits 1341 1772 +431
+ Misses 1338 1239 -99
- Partials 140 179 +39
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Parameterized SDK redirect URIs with secure defaults, added support for secretKeyRef for admin/database credentials, and made sslRequired enforcement dynamic based on devMode. - Added configuration blocks for external databases (PostgreSQL/MySQL) and reverse proxy settings (headers, hostname). - Propagated global imagePullSecrets, decoupled Service targetPort by using the named 'http' port, and dynamically derived the realm import filename from Helm values. Signed-off-by: Mahil Patel <mahilpatel0808@gmail.com>
|
|
||
| "clients": [ | ||
| { | ||
| "clientId": "agentcube-sdk", |
There was a problem hiding this comment.
The confidential client secrets should be explicit. If Keycloak generates them during realm import, SDK/router/admin clients have no stable secret to configure against after Helm install.
| name: {{ .Values.keycloak.existingSecret }} | ||
| key: {{ .Values.keycloak.existingSecretKey | default "admin-password" }} | ||
| {{- else }} | ||
| value: {{ .Values.keycloak.adminPassword | quote }} |
There was a problem hiding this comment.
This still renders the bootstrap admin password into the Deployment when no Secret is set. We should require a Secret when Keycloak is enabled instead of keeping the plaintext fallback.
| name: {{ .Values.keycloak.database.existingSecret }} | ||
| key: {{ .Values.keycloak.database.existingSecretKey | default "db-password" }} | ||
| {{- else }} | ||
| value: {{ .Values.keycloak.database.password | quote }} |
There was a problem hiding this comment.
Same for the database password. The production path should require database.existingSecret instead of rendering the DB credential as a literal env value.
| {{- if .Values.keycloak.devMode }} | ||
| args: ["start-dev", "--import-realm"] | ||
| {{- else }} | ||
| args: ["start", "--import-realm"] |
There was a problem hiding this comment.
devMode=false should fail template rendering when required production settings are missing. Otherwise Helm succeeds but Keycloak starts with an incomplete production config.
hzxuzhonghu
left a comment
hzxuzhonghu
left a comment
There was a problem hiding this comment.
before adding a bunch of yamls, can you first add a docs exmplain about the proposal and the workflow
What type of PR is this?
/kind feature
What this PR does / why we need it:
Description
This PR deploys Keycloak as the external identity provider for AgentCube and configures the
agentcuberealm with all necessary clients and roles. This establishes the OIDC/OAuth2 foundation that the Router JWT verification middleware will consume.Core Changes
Keycloak Kubernetes Deployment: Added Deployment, Service, and ConfigMap manifests for Keycloak in
agentcube-system, following the same Helm chart conventions as the existing SPIRE infrastructure. All resources are gated behindkeycloak.enabled(default:false) to ensure zero impact on existing deployments. Includes astartupProbewith a 300s window to tolerate Keycloak's JVM startup and realm import phase.Declarative Realm Configuration: The
agentcuberealm is defined as a JSON import file mounted into the Keycloak pod via ConfigMap. The realm includes:agentcube-sdk(confidential, client_credentials + authorization_code),agentcube-router(confidential),workloadmanager(confidential, all auth flows disabled),agentcube-admin(confidential)sandbox:invoke,sandbox:manage(inherits sandbox:invoke),admin(inherits sandbox:manage)sandbox:invokeautomatically**Helm Values Integration: ** Added
keycloak:section tovalues.yamlwith configurable image, admin credentials, service ports, and resource limits. Supports both dev mode (embedded H2 database) and production mode toggle.Verification
Local deployment successfully verified on a kind cluster:
Successful Realm Import Logs
Configured Clients & Role Hierarchy API Checks
Decoded SDK JWT Payload with sandbox:invoke Role
Which issue(s) this PR fixes:
Fixes Part of #243
Special notes for your reviewer:
bearer-only: Theworkloadmanagerclient is configured as a standard confidential client with all authentication flows disabled (the modern Quarkus replacement for the deprecatedbearer-onlyprofile).OVERWRITE_EXISTINGduring the--import-realmbootstrap phase, meaning updates to the ConfigMap JSON will correctly overwrite the realm config on pod restart.Does this PR introduce a user-facing change?:
yes