Skip to content

Conversation

@filippomc
Copy link
Collaborator

@filippomc filippomc commented Dec 9, 2025

Closes CH-213

Implemented solution

...

How to test this PR

...

Sanity checks:

  • The pull request is explicitly linked to the relevant issue(s)
  • The issue is well described: clearly states the problem and the general proposed solution(s)
  • In this PR it is explicitly stated how to test the current change
  • The labels in the issue set the scope and the type of issue (bug, feature, etc.)
  • The relevant components are indicated in the issue (if any)
  • All the automated test checks are passing
  • All the linked issues are included in one Sprint
  • All the linked issues are in the Review state
  • All the linked issues are assigned

Breaking changes (select one):

  • The present changes do not change the preexisting api in any way
  • This PR and the issue are tagged as a breaking-change and the migration procedure is well described above

Possible deployment updates issues (select one):

  • There is no reason why deployments based on CloudHarness may break after the current update
  • This PR and the issue are tagged as alert:deployment

Test coverage (select one):

  • Tests for the relevant cases are included in this pr
  • The changes included in this pr are out of the current test coverage scope

Documentation (select one):

  • The documentation has been updated to match the current changes
  • The changes included in this PR are out of the current documentation scope

Nice to have (if relevant):

  • Screenshots of the changes
  • Explanatory video/animated gif

@filippomc filippomc requested review from Copilot and zsinnema December 9, 2025 17:35
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the API user initialization logic by moving authentication and user creation logic from the Keycloak entrypoint script into the dedicated create_api_user.sh script, improving separation of concerns and maintainability.

Key Changes:

  • Moved API user authentication and creation logic from kc-entrypoint.sh to create_api_user.sh
  • Simplified the entrypoint script to only handle startup script execution
  • Consolidated all API user management in a single location

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
applications/accounts/scripts/kc-entrypoint.sh Removed API user authentication logic and conditional execution, simplified to only run startup scripts
applications/accounts/scripts/create_api_user.sh Added authentication logic and wrapped user creation in conditional block to handle existing user scenarios
Comments suppressed due to low confidence (1)

applications/accounts/scripts/kc-entrypoint.sh:1

  • Missing closing 'fi' for the inner if statement on line 9. The script has mismatched conditional blocks.
#! /bin/bash

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


# Run startup scripts to create admin_api user
for script in /opt/keycloak/startup-scripts/*.sh;
# Run startup scripts to create admin_api user
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

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

[nitpick] Corrected spelling in comment: 'create' should be 'creates' or rephrase to 'creation of admin_api user'.

Suggested change
# Run startup scripts to create admin_api user
# Run startup scripts to create the admin_api user

Copilot uses AI. Check for mistakes.
API_USERNAME="admin_api"
API_PASSWORD=$(cat /opt/cloudharness/resources/auth/api_user_password 2>/dev/null || echo "")

echo "create_api_user: waiting for Keycloak to start..."
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

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

[nitpick] Corrected spelling: 'create_api_user' should use consistent formatting as 'Creating API user' elsewhere in the script.

Suggested change
echo "create_api_user: waiting for Keycloak to start..."
echo "Creating API user: waiting for Keycloak to start..."

Copilot uses AI. Check for mistakes.
echo "admin_api user does not exist or authentication failed. Authenticating as bootstrap admin to create the user..."

# Authenticate as bootstrap admin to create admin_api user
if ! /opt/keycloak/bin/kcadm.sh config credentials \
Copy link
Contributor

Choose a reason for hiding this comment

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

critical: this still doesn't work when the bootstap user password is changed. Let me think of an automated way to set the api admin user's password

Copy link
Contributor

Choose a reason for hiding this comment

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

@filippomc I think this will do the job:

kc.sh bootstrap-admin service --client-id=tmp_client --client-secret:env=KC_BOOTSTRAP_ADMIN_USERNAME
kcadm.sh config credentials --server http://localhost:8080 --realm master --client tmp_client --secret ${KC_BOOTSTRAP_ADMIN_USERNAME}

kcadm.sh create users -s "username=${API_USERNAME}" -s enabled=True
kcadm.sh set-password --username "${API_USERNAME}" --new-password "${API_PASSWORD}"

client_id=$(kcadm.sh get clients -r master -q "clientId=tmp_client"|grep \"id\"|cut -d ":" -f 2|tr -d '", ')
kcadm.sh delete clients/${client_id}

explanation:

  1. create a temporary service account
  2. create the api user and set the password
  3. remove the temporary service account

Copy link
Contributor

Choose a reason for hiding this comment

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

@filippomc wdyt of my last commit?

@filippomc filippomc merged commit 9ecb4a9 into develop Dec 10, 2025
6 of 7 checks passed
@filippomc filippomc deleted the keycloak-init-fix branch December 10, 2025 16:00
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.

4 participants