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

keycloak_group: fix subgroup creation in Keycloak ≥23 #8979

Merged

Conversation

vgaudard
Copy link
Contributor

@vgaudard vgaudard commented Oct 3, 2024

SUMMARY

In Keycloak versions 23 and later, GroupRepresentation.subGroups has been replaced by GroupRepresentation.subGroupCount and another endpoint to get subgroups.
This caused community.general.keycloak_group to fail when creating a group with a parent.
We now execute this second request if needed.

Fixes #8788

See https://www.keycloak.org/docs/latest/upgrading/#grouprepresentation-changes

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

keycloak_group

ADDITIONAL INFORMATION

Related issue : #8788

Tested on Keycloak versions (Docker images) :

  • 17.0.0
  • 21.1.2
  • 22.0.5
  • 23.0.4
  • 23.0.7
  • 24.0.5
  • 25.0.6

@ansibullbot
Copy link
Collaborator

@ansibullbot ansibullbot added WIP Work in progress bug This issue/PR relates to a bug identity module_utils module_utils new_contributor Help guide this first time contributor plugins plugin (any type) labels Oct 3, 2024
@vgaudard
Copy link
Contributor Author

vgaudard commented Oct 3, 2024

I wasn't able to run tests locally (either before or after my changes). Because they're marked unsupported, I assumed this is expected.

Console output of test execution
$ ansible-test integration keycloak_group --docker -v
Configured locale: C.UTF-8
Falling back to tests in "tests/integration/targets/" because "roles/test/" was not found.
WARNING: Excluding target tests marked "unsupported" which require --allow-unsupported or prefixing with "unsupported/": keycloak_group
WARNING: All targets skipped.

$ ansible-test integration keycloak_group --docker -v --allow-unsupported
Configured locale: C.UTF-8
Falling back to tests in "tests/integration/targets/" because "roles/test/" was not found.
Run command: docker -v
Detected "docker" container runtime version: Docker version 26.1.4, build 5650f9b
Run command: docker info --format '{{ json . }}'
Run command: docker version --format '{{ json . }}'
Container runtime: docker client=26.1.4 server=26.1.4 cgroup=v1 DD+WSL2
Run command: docker image inspect quay.io/ansible/ansible-test-utility-container:2.0.0
Run command: docker run --volume /sys/fs/cgroup:/probe:ro --name ansible-test-probe-yPUyPpLc --rm quay.io/ansible/ansible-test-utility-container:2.0.0 sh -c 'audit-status && cat /proc ...
Container host audit status: EPERM (-1)
Container host max open files: 1048576
Container loginuid: 4294967295 (not set)
Assuming Docker is available on localhost.
Run command with data: docker run --tmpfs /tmp:exec --tmpfs /run:exec --tmpfs /run/lock --volume /var/run/docker.sock:/var/run/docker.sock --cgroupns host --tmpfs /sys/fs/cgroup --vol ...
ERROR: Host DockerConfig(python=NativePythonConfig(version='3.12', path='/usr/bin/python3.12'), name='default', image='quay.io/ansible/default-test-container:8.12.0', memory=None, privileged=False, seccomp='default', cgroup=CGroupVersion.V1_V2, audit=AuditMode.REQUIRED) job failed:
Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/ansible_test/_internal/provisioning.py", line 200, in dispatch_jobs
    thread.wait_for_result()
  File "/usr/lib/python3/dist-packages/ansible_test/_internal/thread.py", line 44, in wait_for_result
    raise exception[1].with_traceback(exception[2])
  File "/usr/lib/python3/dist-packages/ansible_test/_internal/thread.py", line 34, in run
    self._result.put((self.action(), None))
                      ^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/ansible_test/_internal/provisioning.py", line 134, in provision
    profile.provision()
  File "/usr/lib/python3/dist-packages/ansible_test/_internal/host_profiles.py", line 456, in provision
    container = run_support_container(
                ^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/ansible_test/_internal/containers.py", line 135, in run_support_container
    current_container_id = get_docker_container_id()
                           ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/ansible_test/_internal/thread.py", line 59, in wrapper
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/ansible_test/_internal/util.py", line 151, in cache_func
    value = storage[None] = func()
                            ^^^^^^
  File "/usr/lib/python3/dist-packages/ansible_test/_internal/docker_util.py", line 599, in get_docker_container_id
    mounts = MountEntry.loads(mountinfo_path.read_text())
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/ansible_test/_internal/cgroup.py", line 107, in loads
    return tuple(cls.parse(line) for line in value.splitlines())
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/ansible_test/_internal/cgroup.py", line 107, in <genexpr>
    return tuple(cls.parse(line) for line in value.splitlines())
                 ^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/ansible_test/_internal/cgroup.py", line 86, in parse
    assert separator == '-'
           ^^^^^^^^^^^^^^^^
AssertionError:
FATAL: Host job(s) failed. See previous error(s) for details.

@vgaudard vgaudard changed the title [WIP] keycloak_group: fix subgroup creation in Keycloak ≥23 keycloak_group: fix subgroup creation in Keycloak ≥23 Oct 3, 2024
@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-8 Automatically create a backport for the stable-8 branch backport-9 Automatically create a backport for the stable-9 branch labels Oct 3, 2024
@vgaudard
Copy link
Contributor Author

vgaudard commented Oct 3, 2024

@UKFr-DIZ This is a fix for issue #8788 .

@vvanouytsel This PR might fix issue #8366 , could you confirm that the issue happened with Keycloak version 23?
Details : I suspect this was caused by Keycloak returning a subGroupCount > 0 and subGroups == [] after creating the subgroup. This caused after_group to be set to None, and so the module failed when formatting the result message.

@ansibullbot ansibullbot removed the WIP Work in progress label Oct 3, 2024
Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! I've added a first comment below.

Regarding the tests: unsupported means they don't run in CI, you have to run them manually. (See also tests/integration/targets/keycloak_group/readme.adoc in case you haven't seen it yet.) The error you are seeing seems to be a bug in ansible-core, or a problem with Docker on your machine, or a combination of your Docker and a bug in ansible-core, and are unrelated to the tests for this module.

(Did you try passing --python 3.12 explicitly? Usually if you don't provide a OS docker image like --docker ubuntu2404, you have to specify the Python version. It could be that ansible-test picks a default Python version for the default image, but 🤷)

changelogs/fragments/8979-keycloak_group-fix-subgroups.yml Outdated Show resolved Hide resolved
Copy link
Collaborator

@russoz russoz left a comment

Choose a reason for hiding this comment

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

hi @vgaudard Thanks for the contribution!

Other than my question in the changelog frag, the rest LGTM.

@felixfontein
Copy link
Collaborator

If nobody objects, I'll merge this in a week.

CC @fgruenbauer who lately worked a lot on keycloak_* modules.

@vgaudard
Copy link
Contributor Author

vgaudard commented Oct 4, 2024

Thank you both for your help.

You might want give it a try to andebox by yours truly:

I did, but I can't setup vagrant on my work computer, so I returned to classic ansible-test and fidgeting with dependencies.

I was able to run the tests before and after my changes, and both passed.

I ran the tests again but on Keycloak 25, and... they failed. I forgot to change another method where we want subgroups of a group (get_subgroup_by_chain), which caused sub-sub-groups to fail.
It should be OK now.

@felixfontein felixfontein removed the backport-8 Automatically create a backport for the stable-8 branch label Oct 7, 2024
@russoz
Copy link
Collaborator

russoz commented Oct 11, 2024

hi @vgaudard "it should be OK" is not very reassuring :-) Could you please confirm if you got the test to run successfully with kc 25? TIA

@felixfontein
Copy link
Collaborator

@vgaudard ping

@ansibullbot ansibullbot added the stale_ci CI is older than 7 days, rerun before merging label Oct 17, 2024
@vgaudard
Copy link
Contributor Author

vgaudard commented Oct 19, 2024 via email

@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Oct 19, 2024
@felixfontein felixfontein merged commit 658637d into ansible-collections:main Oct 19, 2024
141 checks passed
Copy link

patchback bot commented Oct 19, 2024

Backport to stable-9: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-9/658637dc700f6e795074a22071fa7a40ef7f11fb/pr-8979

Backported as #9041

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Oct 19, 2024
* keycloak_group: fix subgroup creation in Keycloak ≥23

* Add changelog fragment

* Include issue and pull request in changelog fragment

Co-authored-by: Felix Fontein <[email protected]>

* Use new way to get subgroups when getting a subgroup chain

* Fix indent

---------

Co-authored-by: Felix Fontein <[email protected]>
(cherry picked from commit 658637d)
@felixfontein
Copy link
Collaborator

Thanks for the confirmation!

@vgaudard thanks for your contribution!
@russoz thanks for reviewing!

felixfontein pushed a commit that referenced this pull request Oct 19, 2024
…reation in Keycloak ≥23 (#9041)

keycloak_group: fix subgroup creation in Keycloak ≥23 (#8979)

* keycloak_group: fix subgroup creation in Keycloak ≥23

* Add changelog fragment

* Include issue and pull request in changelog fragment

Co-authored-by: Felix Fontein <[email protected]>

* Use new way to get subgroups when getting a subgroup chain

* Fix indent

---------

Co-authored-by: Felix Fontein <[email protected]>
(cherry picked from commit 658637d)

Co-authored-by: Victor Gaudard <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-9 Automatically create a backport for the stable-9 branch bug This issue/PR relates to a bug identity module_utils module_utils new_contributor Help guide this first time contributor plugins plugin (any type) stale_ci CI is older than 7 days, rerun before merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Keycloak API modules - subgroups issue | Keycloak > 23.0.7 incompatability
4 participants