-
Notifications
You must be signed in to change notification settings - Fork 23
Add RHBK kustomise resources and update deploy script #1425
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Reviewer's GuideThis PR extends the Keycloak deployment by refactoring the install script to support RHBK or SSO modes and introduces kustomize-based manifests for the RHBK operator, PostgreSQL backend, Keycloak CR, and realm import. Sequence diagram for RHBK Keycloak installation flowsequenceDiagram
participant Script
participant OpenShift
participant RHBK Operator
participant PostgreSQL DB
participant Keycloak
Script->>OpenShift: Apply RHBK Operator manifests
OpenShift->>RHBK Operator: Start operator pod
Script->>OpenShift: Check RHBK Operator pod status
Script->>OpenShift: Apply PostgreSQL DB manifests
OpenShift->>PostgreSQL DB: Start DB pod
Script->>OpenShift: Check PostgreSQL DB pod status
Script->>OpenShift: Apply Keycloak CR manifests
OpenShift->>Keycloak: Start Keycloak pod
Script->>OpenShift: Check Keycloak pod status
Script->>OpenShift: Apply Keycloak Realm Import
OpenShift->>Keycloak: Import realm and users
Entity relationship diagram for Keycloak and PostgreSQL resourceserDiagram
POSTGRESQL_DB_SECRET {
string database
string username
string password
}
POSTGRESQL_DB {
string POSTGRESQL_USER
string POSTGRESQL_PASSWORD
string POSTGRESQL_DATABASE
}
KEYCLOAK {
string db_host
string db_passwordSecret
string db_usernameSecret
string db_vendor
string hostname_hostname
int instances
}
KEYCLOAK_REALM_IMPORT {
string keycloakCRName
string realm_id
string realm_realm
string realm_displayName
bool realm_enabled
string realm_clients[]
string realm_users[]
}
POSTGRESQL_DB_SECRET ||--o| POSTGRESQL_DB : provides_credentials
POSTGRESQL_DB ||--o| KEYCLOAK : used_by
KEYCLOAK ||--o| KEYCLOAK_REALM_IMPORT : imports_realm
Class diagram for new Keycloak and PostgreSQL resource definitionsclassDiagram
class Keycloak {
+db: string
+hostname: string
+ingress: string
+proxy: string
+http: string
+instances: int
}
class KeycloakRealmImport {
+keycloakCRName: string
+realm: string
}
class PostgreSQLDB {
+env: string
+image: string
+livenessProbe: string
+readinessProbe: string
+securityContext: string
+volumeMounts: string
}
class PostgreSQLDBSecret {
+database: string
+username: string
+password: string
}
Keycloak --> PostgreSQLDB : uses
KeycloakRealmImport --> Keycloak : imports into
PostgreSQLDB <-- PostgreSQLDBSecret : gets credentials from
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes and found some issues that need to be addressed.
- Add a default case to handle invalid arguments (e.g. print usage and exit) to avoid silent no-ops when the choice flag is mis-typed.
- Extract the repeated oc apply + check_pod_status logic into a helper function to reduce duplication and improve maintainability.
- Consider pinning the postgresql container image to a specific version instead of using 'latest' to ensure consistent deployments.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Add a default case to handle invalid arguments (e.g. print usage and exit) to avoid silent no-ops when the choice flag is mis-typed.
- Extract the repeated oc apply + check_pod_status logic into a helper function to reduce duplication and improve maintainability.
- Consider pinning the postgresql container image to a specific version instead of using 'latest' to ensure consistent deployments.
## Individual Comments
### Comment 1
<location> `ci/openshift/tas-keycloak-install.sh:79-88` </location>
<code_context>
+ fi
+}
+
+choice="${1:-sso}"
+case "$choice" in
+ rhbk)
+ install_rhbk_sso_keycloak
+ ;;
+ sso)
+ install_sso_keycloak
+ ;;
+ -h|--help|help)
+ usage
+ ;;
+esac
</code_context>
<issue_to_address>
**suggestion:** Script does not handle invalid arguments gracefully.
Add a default case to the 'case' statement to display usage information and exit with a non-zero status when an unsupported argument is given.
</issue_to_address>
### Comment 2
<location> `ci/rhbk/resources/base/postgresql_db_secret.yaml:8` </location>
<code_context>
+stringData:
+ database: keycloak
+ username: keycloak
+ password: keycloak
</code_context>
<issue_to_address>
**🚨 issue (security):** Use of static credentials for database secret poses a security risk.
Hardcoding credentials increases the risk of exposure. Use environment variables, secret management tools, or dynamic generation for production deployments.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||||||
8da0877 to
b813082
Compare
|
@osmman When you get a chance can I get you to look at this again? |
Summary by Sourcery
Add Kustomize configurations for Red Hat backed Keycloak deployment and update the OpenShift deploy script to optionally install RHBK or the existing SSO setup.
New Features:
Enhancements: