Skip to content

topologies: add keycloak topology#183

Merged
pbrezina merged 1 commit intoSSSD:masterfrom
sumit-bose:idp_client
May 16, 2025
Merged

topologies: add keycloak topology#183
pbrezina merged 1 commit intoSSSD:masterfrom
sumit-bose:idp_client

Conversation

@sumit-bose
Copy link
Contributor

No description provided.

@sumit-bose sumit-bose marked this pull request as draft April 30, 2025 19:54
@sumit-bose sumit-bose force-pushed the idp_client branch 2 times, most recently from a857af7 to 6cdd2f6 Compare May 1, 2025 18:53
@sumit-bose
Copy link
Contributor Author

Hi,

as you can see I added the kcadm calls to create an IdP client in Keycloak twice. The main reason is that I haven't found a way to call the method from class KeycloakIdPClient in class KeycloakTopologyController. If there is a way to do it I think it would be best to only have class KeycloakIdPClient. If there is no way, please let me know on which level a class like class KeycloakIdPClient should be added to avoid code duplication.

Thanks.

bye,
Sumit

@sumit-bose sumit-bose marked this pull request as ready for review May 2, 2025 11:57
@andreboscatto andreboscatto requested review from pbrezina and spoore1 May 6, 2025 12:38
@spoore1
Copy link
Contributor

spoore1 commented May 12, 2025

Hi,

as you can see I added the kcadm calls to create an IdP client in Keycloak twice. The main reason is that I haven't found a way to call the method from class KeycloakIdPClient in class KeycloakTopologyController. If there is a way to do it I think it would be best to only have class KeycloakIdPClient. If there is no way, please let me know on which level a class like class KeycloakIdPClient should be added to avoid code duplication.

Thanks.

bye, Sumit

@pbrezina would it be better to move KeycloakIdPClient from the role to a utils class for this? Would that allow us to then reference it from both the Keycloak role and the topology controller without the duplication of code?

@pbrezina
Copy link
Member

Hi,
as you can see I added the kcadm calls to create an IdP client in Keycloak twice. The main reason is that I haven't found a way to call the method from class KeycloakIdPClient in class KeycloakTopologyController. If there is a way to do it I think it would be best to only have class KeycloakIdPClient. If there is no way, please let me know on which level a class like class KeycloakIdPClient should be added to avoid code duplication.
Thanks.
bye, Sumit

@pbrezina would it be better to move KeycloakIdPClient from the role to a utils class for this? Would that allow us to then reference it from both the Keycloak role and the topology controller without the duplication of code?

Yes, converting it to MultihostReentrantUtility would allow you to share the code between roles and hosts. That would be cleanest solution, but maybe an overkill for this use case.

You can implement this functionality on the host object and then call this from the KeycloakIdPClient. That might be easier since you don't need the utility setup and teardown code.

pbrezina
pbrezina previously approved these changes May 15, 2025
@pbrezina
Copy link
Member

@sumit-bose I am approving this but if you want to remove the duplications see the suggestions in my previous comment.

@sumit-bose
Copy link
Contributor Author

Hi,

thank you for the suggestions, but if I see it correctly either way would also require to duplicate some parts of class Keycloak and class KeycloakObject. So I removed the role part for the time being, since it is not needed for the current tests and only keep the topology parts.

bye,
Sumit

Copy link
Contributor

@spoore1 spoore1 left a comment

Choose a reason for hiding this comment

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

LGTM. Tested with tests from PRs #7871 and #7937:

tests/test_idp.py::test_idp__user (keycloak) PASSED
tests/test_idp.py::test_idp__group (keycloak) PASSED
tests/test_idp.py::test_idp__user_groups (keycloak) PASSED
tests/test_idp.py::test_idp__group_members (keycloak) PASSED
tests/test_oidc_child.py::test_oidc_child__get_user (keycloak) PASSED
tests/test_oidc_child.py::test_oidc_child__get_group (keycloak) PASSED
tests/test_oidc_child.py::test_oidc_child__get_user_groups (keycloak) PASSED
tests/test_oidc_child.py::test_oidc_child__get_group_members (keycloak) PASSED

================================================================================== 8 passed, 722 deselected in 452.17s (0:07:32) ==================================================================================

@pbrezina pbrezina merged commit 3db7337 into SSSD:master May 16, 2025
5 checks passed
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.

3 participants