Skip to content
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

FIX - Keycloak server execution failing at startup #3894

Merged
merged 12 commits into from
Nov 9, 2023

Conversation

Agnul97
Copy link
Contributor

@Agnul97 Agnul97 commented Oct 31, 2023

The startup of the keycloak server inside the kapua/kapua-keycloak image was failing due to missing of some parameters in the startup script. Basically, the server was started in production mode but this mode requires additional configurations of some aspects, as stated here https://www.keycloak.org/server/configuration-production, that was missing.

With this PR, I inserted the missing configuration parameters. In this way, with the kapua/kapua-keycloak image (so, with the --sso option on the deploy) we propose a demo image to test the sso feature. Finally, I updated the documentation, particularly in the part were it is explained how to run a stand-alone keycloak image for the case of a (real) production mode deployment.

Specifically, I worked on these aspects

  1. Inserted missing cli parameters to fix the server startup issue and provide an HTTP deployment
  2. Fixed the way SSL certificates were imported with the volumes setting in the compose files: this was bugged for a typo in the path and certs were not imported at all
  3. Used certs at point 1. to provide the capability of an HTTP + SSL deployment, with a dedicated env. variable to enable/disable SSL if needed (KEYCLOAK_SSL_ENABLE, like done in the console with KAPUA_SSL_ENABLE) and automatically set via the --ssl option with the deployment script
  4. Updated the developer guide in the SSO section: this was not updated after the bump to the image version 21.1 (from the jboss 7 version) and I've fixed the gap, also I touched other aspects that were wrong
  5. Updated the README.md file inside the assembly folder: I noticed that there was a lot of redundant content with respect to the developer guide, so I inserted some references to that guide in order to solve the redundancy

Copy link

codecov bot commented Oct 31, 2023

Codecov Report

Merging #3894 (79dfd2b) into develop (e1a0e7d) will decrease coverage by 0.02%.
Report is 10 commits behind head on develop.
The diff coverage is 10.94%.

❗ Current head 79dfd2b differs from pull request most recent head 2369c4a. Consider uploading reports for the commit 2369c4a to get more accurate results

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #3894      +/-   ##
=============================================
- Coverage      20.54%   20.52%   -0.02%     
  Complexity         6        6              
=============================================
  Files           1940     1944       +4     
  Lines          41547    41649     +102     
  Branches        3940     3946       +6     
=============================================
+ Hits            8534     8549      +15     
- Misses         32616    32703      +87     
  Partials         397      397              
Files Coverage Δ
.../eclipse/kapua/app/api/core/model/CountResult.java 100.00% <ø> (ø)
...eclipse/kapua/app/api/web/JaxbContextResolver.java 77.77% <ø> (ø)
...ice/authentication/DeviceAuthenticationModule.java 100.00% <100.00%> (ø)
...evice/registry/KapuaDeviceRegistrySettingKeys.java 100.00% <100.00%> (ø)
...e/device/registry/connection/DeviceConnection.java 0.00% <ø> (ø)
...stry/connection/option/DeviceConnectionOption.java 0.00% <ø> (ø)
.../service/device/registry/DeviceRegistryModule.java 80.00% <100.00%> (-0.65%) ⬇️
.../eclipse/kapua/locator/guice/GuiceLocatorImpl.java 69.53% <0.00%> (-0.55%) ⬇️
.../api/resources/v1/resources/DeviceConnections.java 0.00% <0.00%> (ø)
...ion/UserPassDeviceConnectionCredentialAdapter.java 50.00% <50.00%> (ø)
... and 11 more

... and 1 file with indirect coverage changes

Copy link
Contributor

@Coduz Coduz left a comment

Choose a reason for hiding this comment

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

First, I want to suggest to switch to KEYCLOAK_SSL_ENABLE instead of KEYCLOAK_DISABLE_SSL which enables SSL when is false and disable SSL true. We can talk about this, if there is any doubt on this point

Second, it would be possible to have the Keycloak SSL enabled based on the --ssl option available in the Docker Compose deployment? "One SSL option to rule them all" it is easier to use.

Please also remove KEYCLOAK_* env defined in deployment/docker/compose/docker-compose.yml for the kapua-console component.

KC=/opt/keycloak/bin/kcadm.sh

echo "Kapua Keycloak Configuration:"
echo " Kapua Console URL: $KAPUA_CONSOLE_URL"
echo " Keycloak Realm: $REALM_NAME"
echo " Keycloak Admin User: $KEYCLOAK_USER"
echo " Keycloak TLS disabled: $KEYCLOAK_DISABLE_SSL"
Copy link
Contributor

Choose a reason for hiding this comment

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

Please don't use this reverse logic ENV to enable component.

It is far easier to understand KEYCLOAK_SSL_ENABLE when is defined and used.
The default can be false to indicate that a feature is disabled by default.

@Agnul97 Agnul97 closed this Nov 9, 2023
@Agnul97 Agnul97 reopened this Nov 9, 2023
@Coduz Coduz added the Bug This is a bug or an unexpected behaviour. Fix it! label Nov 9, 2023
@Coduz Coduz merged commit f0106ac into eclipse:develop Nov 9, 2023
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug This is a bug or an unexpected behaviour. Fix it!
Projects
Development

Successfully merging this pull request may close these issues.

3 participants