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

Run service as keycloak_service_user #106

Merged
merged 7 commits into from
Aug 30, 2023

Conversation

schmaxit
Copy link
Contributor

@schmaxit schmaxit commented Aug 8, 2023

The actual implementation always runs the service as root user.
This PR uses the existing variables keycloak_service_user and keycloak_service_group to set the user and group of the startup script and so of the actual java process.
I have also changed the default keycloak_service_pidfile value allowing to set the correct permissions on the folder containing the pidfile

@schmaxit schmaxit marked this pull request as ready for review August 8, 2023 16:43
@schmaxit schmaxit force-pushed the main branch 2 times, most recently from 22531fb to 5bb036d Compare August 9, 2023 07:29
Signed-off-by: Massimo Schiavon <[email protected]>
@guidograzioli guidograzioli self-requested a review August 9, 2023 08:19
@guidograzioli guidograzioli added enhancement New feature or request major_changes Major changes mean the user can CHOOSE to make a change when they update but do not have to breaking_changes MUST include changes that break existing playbooks and removed enhancement New feature or request major_changes Major changes mean the user can CHOOSE to make a change when they update but do not have to labels Aug 9, 2023
@guidograzioli
Copy link
Member

Hello thanks for the PR; this is a much needed change, but it will create many side-effect on existing deployments (systemd not finding the pidfile for the running instance, files owned by root around not readable by the process, and so on). So for backwards compatibility, and to lower the PR level to major_change from breaking_change, can you please:

  1. add a keycloak_service_runas (bool, default false) flag and protect your feature behind it; by default it retains the current behaviour of running as root for backwards compatibility
  2. add a conditional task when keycloak_service_runas is true, which recursively re-owns to keycloak_service_user everything under keycloak.home

The above for the keycloak role only, while for keycloak_quarkus it is good as it is now

roles/keycloak/tasks/install.yml Outdated Show resolved Hide resolved
add keycloak_service_runas feature flag
fix previous installs permissions
@guidograzioli guidograzioli added major_changes Major changes mean the user can CHOOSE to make a change when they update but do not have to and removed breaking_changes MUST include changes that break existing playbooks labels Aug 30, 2023
@guidograzioli
Copy link
Member

LGTM; thank you very much for this contribution

@guidograzioli guidograzioli merged commit d12f62b into ansible-middleware:main Aug 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
major_changes Major changes mean the user can CHOOSE to make a change when they update but do not have to
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants