Repository navigation
Proposal to support both alias and id in rest endpoints
#42942
Replies: 6 comments 40 replies
|
We could consider in the future to support additional URN types, for example for users we could support: I'd be careful with this though. |
|
perhaps urn:kc:{id,email,username,etc.}:id is more flexible here and doesn't clash with other URNs. |
|
Emails might not be unique depending on the realm settings but we can of course just report an error if somebody tries retrieving by email URN. |
|
I was also thinking about user IDs and the sub claim. I think we should use the urn for the stable identifier (the user id), that's consistent with the other resources: The subject in the token needs to stay at |
|
I think this is a reasonable compromise between idempotent PUT (I have a single endpoint to create or update a resource using a immutable identifier), and changeable aliases (like Client ID). I assume the "kcid" could be user defined, so that I can do Obvious downside is that we cannot have a Client ID starting with |
|
Why would you make the kcid user-supplied? It would cause problems for example for users because we don't want user IDs supplied by users. Isn't it sufficient to have a user-supplied name and alias? |
|
Would you need it as a user-supplied but immutable identifier for k8s? |
|
Btw. I think the fundamental problem we are dealing with in case of the operator is that k8s has a different more loosely-coupled domain model. Entities are associated by means of labels and selectors. So it is fine if things cannot be renamed and the name works as an ID - you can simply delete and recreate a resource and it will be properly associated using labels and selectors on recreation. This is not true in the Keycloak domain model. Deleting a client would delete all client roles and all associations of roles to users. I am not sure we can have this user-supplied immutable identifier for all Keycloak resources in k8s, the Keycloak domain model currently doesn't offer these. What it does have is server-generated immutable identifiers. I don't know if there is a way to use these in k8s. |
Yes, we need an immutable identifier for the declarative config scenarios. Let's say the alias has changed in the declarative config (CR, some YAML in a git repo, ...). The "controller" (the Operator, Terraform, ...) is stateless, we have no guarantee it knows the previous state, basically it may not know the alias has changed, all it sees is the current alias. This could lead to creating a duplicit resource under the new alias instead of renaming the alias on the original resource which might be quite unexpected. A few possible solutions:
|
|
Obviously, an option would be also to make alias changeable only via an action (as suggested by @stianst), not via updating the resource directly. But that doesn't provide a good path of declarative config either if the action is not executed by the controller but by the user. (The controller could reconcile the resource after the user executed the action to update the alias, leading again to creating a duplicit resource.) |
|
Just to summarize what've discussed on today's sync. We will use the approach outlined in this discussion:
|
|
We need to reference via PUT because this is what the Terraform provider will do. There needs to be a stable URL to reference the entity with all possible HTTP verbs. Also the operator should do it that way. Reference by ID will always work, reference by alias will also work except for cases where alias in path and alias in document don't match (i.e. renaming). |
|
Alias in path will work unless something else than the controller (Operator, Terraform, whatever) changed it (which should not happen as we want the managed resources to be read-only for other actors). For example, to change the clientId using the Operator, it might work as follows:
|
|
We already discussed in the last meeting the operator will use the kcid for updating/renaming? |
We'd need to use the alias in case we decided UUIDs can't be used with PUT – as is being discussed below. |
|
PUT with alias should work? You'd need to use the UUID next time around though to make sure it's the same one. |
8BD0
|
A related question to this discussion: should the UUID be user specifiable? I.e. could I do The main motivation for UUIDs is from my perspective the immutability. That means Do you see any issues with user specifiable UUIDs? Do we want to have some validations the provided UUID is actually a UUID? Isn't UUID rather an internal construct that Keycloak should have the control over (versions etc.)? CC @shawkins – related to our private discussion. |
|
AFAIK at least Terraform always performs a state refresh when starting a run, so it checks the actual state of all resources and updates its state. I am not sure how operators handle this, but I would assume they also regularly check if desired state and actual state are still matching before deciding if they need to update anything. |
That is something we want to avoid in the Operator and declarative config in general. The thinking behind that is that it's the server's responsibility to figure out what needs to be done, the controller just sends the desired state to KC and says "make it happen". |
|
That's not how Terraform or Kubernetes work in general. They both acknowledge that the actual state might deviate from the desired state (because of manual changes, pods failing etc.) and take actions to detect that drift and correct for it. There is no way you can guarantee there is no drift, whatever credentials the operator uses, somebody else can use the same credentials to do changes without the operator noticing. |
Yes but I'd argue it's not the controllers responsibility to figure that out. It's the server's, controller only provides the desired state. |
|
I disagree. That's exactly a controller's job. It is basically implementing a control loop matching actual to desired state. |
|
Thinking about it more, I believe allowing to reference resources via UUID as a path parameter is not really something we should do in the end. The main reason is we cannot use UUIDs with PUT. That is because we do not want UUIDs to be user specifiable, hence we can't do something like Additionally, we can't allow changing the alias via PUT as that would break the idempotency:
I can see a couple of options:
In both cases, we'd support referencing by alias as a path param (e.g. For changing the alias, we could either:
To support changing aliases via declarative config resources, we could add a dedicated field just to the declarative representation (called something like newClientId) that would then be cleared when the alias is changed. |
|
Yes, a possible workaround would be it'd fall back to using alias if the resource referenced by UUID is not found. But again, that makes PUT inconsistent – that it can create resources only with alias references but not with UUID references. |
|
I think the problem of not having stable external identifiers or not having idempotency on PUT requests is bigger than the fact that you need to use the alias to create a resource because the identifier is server-generated. Not having stable identifiers means Terraform and no other linking to resources from external would work. Not having idempotency is bad for retries. That you need to use PUT with alias (or POST) to create a resource is something that can be documented. |
|
In order to have idempotency and stable identifiers, we would need additional user-supplied immutable identifiers for clients. We cannot use the client id because it needs to be mutable. Which means we would have to add this to the data model with unique constraint and index for efficient lookup. |
This is what was proposed by @shawkins as well in an offline discussion – basically a 3rd key (in addition to the UUID and alias). I don't like this too much, feels like an overcomplicating stuff a bit and I'm concerned it might feel confusing to users. So to summarize basically we have the following options:
Did I miss anything? I'd be still most inclined toward option 2. I'm not sure I fully understand why Terraform could not use aliases for referencing resources. When a resource is managed by a declarative config source (Terraform provider, Operator, ...) the end goal is to make it read-only by other actors. That means if the alias needs to change, it would need to be done through the owning declarative config source – it's not like the alias will change and Terraform won't know about it. |
|
Option 2 means no stable external references to entities inside Keycloak. Any representation of an entity that was delivered to the outside world would not contain a stable identifier so it might become unusable whenever the alias is changed. Also PUT is not idempotent when the alias changes. Terraform can use the alias but then it may not be changed outside of Terraform. Compensating for outside changes is basically Terraform's main job. Option 1 would be a superset of option 2 without the aforementioned downsides. It would be more complex and more effort to implement, though. |
|
Based on today's offline discussion, opened a related issue #46540. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
In Admin v1 most of the times the UUID is used in the path (
/clients/<uuid>), but sometimes it uses the alias (/identity-provider/instances/{alias}`. The fact that UUIDs are not generated, and not even returned in the response means that after creating a resource you need to parse the location header to get the UUID if you want to make further changes to the resource.To make this easier the idea was to use the public ID/alias for example
/clients/clientIdwhen retrieving things. However, this could change, so the proposal here is to add support for either lookup by public ID or internal ID, but make it explicit with using anurn.For example both paths below refer to the same client:
This would allow choosing either convenience (alias) or non-changeable when using the Admin v2 APIs.
One question is if the returned object should use the
urnsyntax for the id or not.For example should
GET ../clients/urn:kcid:<uuid>return:{ "id": "<uuid>" }or{ "id": "urn:kcid<uuid>" }For completeness for different different resources uses different aliases, which would be the default used in the path unless
urn:kcidis used:/parent/child)myrole)All reactions