feat(chart): enable device auth grant

[mohammad-alisafaee]

Fixes: #2490

[ableuler]

How do you want to handle the transition period? I guess one option would be for the server side to support both, old and new CLI login for a certain period.

[mohammad-alisafaee]

Yes, that would work. This PR does not remove anything so, it's safe to merge it. We should keep the custom auth flow in the gateway for a while (i.e. not merging SwissDataScienceCenter/renku-gateway#558 for now) to support old CLI clients.

[ableuler]

Sounds good. But for this to work we'd have to still allow the standard flow on the renku-cli for the time being, no?

[mohammad-alisafaee]

That's right. I've enabled it. I'll create a new story to disable it once we decided to drop support for the old login flow.

[ableuler]

Ok, that looks good. Can you check what happens when you make these changes to an already existing deployment? I think it won't automatically update the client settings. We've stumbled over this before. So if you could check the init-realm script for this, that would be nice.

[mohammad-alisafaee]

I've checked the init-realm.py script and it doesn't update clients automatically. It only warns about mismatches between existing and new clients. I see that this scripts hasn't been updated in ages so I'm not sure what's the policy regarding it: Should we update it or should we apply changes to the deployments manually? If former, then I can add a section to the script to copy new client attributes to an existing one.

[ableuler]

Thanks for checking. I think the post-install job should indeed delete and recreate clients which do not match the expected configuration.

[rokroskar]

Is this waiting for a change to the post-install job?

[mohammad-alisafaee]

I've updated the init-realm.py script to update recreate existing clients if there is a change in the configuration. This can be controlled with a --apply-changes flag.
Note that there is a caveat: The comparison is done at the top-level keys, meaning that if there is a difference deeper in the clients' dictionaries, the update might fail. For example, clients have a list of protocol mappers where each mapper is a dict itself; if a mapper changes then at the top level the script only sees that the lists are different and adds the new element to the existing list, so, there will be two mappers with the same name in the list which causes the update to fail.

[rokroskar]

@mohammad-sdsc thanks for looking into this! Can't we just completely remove the old configuration and apply the new one in the case that there is a config change? That's effectively what we do by hand now and it works fine...

[mohammad-alisafaee]

Can't we just completely remove the old configuration and apply the new one in the case that there is a config change? That's effectively what we do by hand now and it works fine...

I didn't realize this. Thanks for mentioning it! It's more robust and simpler to implement.

[rokroskar]

@ableuler please chime in if I misrepresented how this is currently handled :)

[ableuler]

Can't we just completely remove the old configuration

What do you mean by existing configuration? Do you mean all clients in the Renku realm?

[rokroskar]

I meant the client that has the configuration conflict. Though in practice we actually removed all the clients and let the post-install script recreate them iirc, but I wouldn't do that in general.

[ableuler]

I never delete all clients by hand and even more so I think that we should not do this programmatically. So the solution that @mohammad-sdsc has already implemented (recreate a specific client whose configuration has changed) is the way to go.

[mohammad-alisafaee]

I never delete all clients by hand and even more so I think that we should not do this programmatically.

It doesn't delete all clients; it only deletes/recreates clients that have changed. The difference with the previous approach is in the configuration that is used to recreate the client: Before it was using the existing configuration but now it's using the new configuration. Basically, this should be fine if all the required configuration is already in the value files/templates. If there are manual changes to the clients (i.e. changes via Keycloak UI) then those are gone.

[ableuler]

Yeah, the recreate client instead of update aspect makes perfect sense to me. I just wasn't sure if Rok was talking about the same thing. Anyway, I think we all agree.

[mohammad-alisafaee]

Follow up story: #2504

[ableuler]

Thanks @mohammad-sdsc, let's get this merged!