keycloak_client: remove code that turns attributes dict into list

[fgruenbauer]

SUMMARY

The attribute config item contains a dict with additional settings. The module turns the dict into a list before storing it for the diff/end_state, which turns it into a list of just the keys:

+    "attributes": [
+        "backchannel.logout.revoke.offline.tokens",
+        "backchannel.logout.session.required",
+        "frontchannel.logout.url"
+    ],
+    "baseUrl": "https://www.example.com/",

The actual data sent to the API is not changed, so only --diff/end_state is affected.

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

keycloak_client

ADDITIONAL INFORMATION

Create a client with the module with any of the attributes settings from above set and --diff.

[ansibullbot]

cc @eikef @mattock @ndclt @thomasbach-dev
click here for bot help

[felixfontein]

Thanks for your contribution!

[felixfontein]

Is it possible that this used to be a list, and was changed to a dict at some point?

[fgruenbauer]

It kinda looks like it. But i checked Keycloak 5, 9, 13, 18, 25 and its always a dict. Same for the clientscopes.

[felixfontein]

This was added in #3610 by @pmdumuid. @pmdumuid do you remember why you chose this? The tests added in the PR use Keycloak 12.0.4, maybe that version uses it for whatever reason (even if 9 and 13 do not)?

[pmdumuid]

Hey Felix, I added it because when I ran ansible in check mode, I found that sometimes (possibly after a keycloak restart) the order of attribute didn't match between desired and what keycloak returned. Thus a false difference is reported because the order of attributes doesn't matter I chose to add the sorting there before determining if there is a difference. I think I have might have more changes like this in my personal fork because my use case was to deploy every few weeks, and only proceed if the check passed when running the previous ansible code before the new ansible code. Note : answering in my phone without looking at the full diff.

…

On Wed, 6 Nov 2024, 5:43 am Felix Fontein, ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In plugins/modules/keycloak_client.py <#9077 (comment)> : > @@ -805,9 +805,6 @@ def normalise_cr(clientrep, remove_ids=False): # Avoid the dict passed in to be modified clientrep = clientrep.copy() - if 'attributes' in clientrep: - clientrep['attributes'] = list(sorted(clientrep['attributes'])) This was added in #3610 <#3610> by @pmdumuid <https://github.com/pmdumuid>. @pmdumuid <https://github.com/pmdumuid> do you remember why you chose this? The tests added in the PR use Keycloak 12.0.4, maybe that version uses it for whatever reason (even if 9 and 13 do not)? — Reply to this email directly, view it on GitHub <#9077 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AASCFVVU4IKSQO7NGO4MJSLZ7EKFBAVCNFSM6AAAAABQZ7VC7SVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDIMJWGQ4DOMZXGA> . You are receiving this because you were mentioned.Message ID: ***@***.*** com>

[pmdumuid]

Sorry I have just reread what I responded wirh after sending it. Please replace "I chose to" with ". I avoided this false difference in check mode by choosing to "

…

On Wed, 6 Nov 2024, 9:09 am Pierre Dumuid, ***@***.***> wrote: Hey Felix, I added it because when I ran ansible in check mode, I found that sometimes (possibly after a keycloak restart) the order of attribute didn't match between desired and what keycloak returned. Thus a false difference is reported because the order of attributes doesn't matter I chose to add the sorting there before determining if there is a difference. I think I have might have more changes like this in my personal fork because my use case was to deploy every few weeks, and only proceed if the check passed when running the previous ansible code before the new ansible code. Note : answering in my phone without looking at the full diff. On Wed, 6 Nov 2024, 5:43 am Felix Fontein, ***@***.***> wrote: > ***@***.**** commented on this pull request. > ------------------------------ > > In plugins/modules/keycloak_client.py > <#9077 (comment)> > : > > > @@ -805,9 +805,6 @@ def normalise_cr(clientrep, remove_ids=False): > # Avoid the dict passed in to be modified > clientrep = clientrep.copy() > > - if 'attributes' in clientrep: > - clientrep['attributes'] = list(sorted(clientrep['attributes'])) > > This was added in #3610 > <#3610> by > @pmdumuid <https://github.com/pmdumuid>. @pmdumuid > <https://github.com/pmdumuid> do you remember why you chose this? The > tests added in the PR use Keycloak 12.0.4, maybe that version uses it for > whatever reason (even if 9 and 13 do not)? > > — > Reply to this email directly, view it on GitHub > <#9077 (comment)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AASCFVVU4IKSQO7NGO4MJSLZ7EKFBAVCNFSM6AAAAABQZ7VC7SVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDIMJWGQ4DOMZXGA> > . > You are receiving this because you were mentioned.Message ID: > ***@***.*** > .com> >

[fgruenbauer]

I checked 12.0.4 but its also a dict.

@pmdumuid
Do you have an example client config or just attributes dict that showed incorrect changes in check mode before your changes?

[patchback]

Backport to stable-9: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-9/62cb6087b59064798a51fb90958300827343fdce/pr-9077

Backported as #9115

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

[patchback]

Backport to stable-10: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-10/62cb6087b59064798a51fb90958300827343fdce/pr-9077

Backported as #9116

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

[felixfontein]

I decided to merge this since currently it seems that the PR is correct, and so that it can go into the 10.0.1 release. Thanks everyone!