-
Notifications
You must be signed in to change notification settings - Fork 7.9k
[admin-api-v2] Every distinct Admin API should be versioned #44527
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
base: main
Are you sure you want to change the base?
Conversation
Is this for the dev experience? We shouldn't provide a endpoint with a schema that may change. Alternatively with additional metadata facilties you could perhaps advertise the default api version.
Everything should be versioned for consistency. Here again metadata facilities could be needed to advertise the versions. This will start to resemble the way kubernetes versions apis - /group/version/[namespace/name]/resource Do we want to consider a group concept, or is the thinking that every root resource is separately versioned - even if there are relationships between them? |
Yes, the intention is mostly for dev experience - having the same convention as for features. It means, that we recommended to our users to use the versioned keys for features, but we accept also the unversioned feature name to be enabled. However, as we start this whole new thing, we can do some changes right away. It means that we can decide to be always on the consistent right way - requiring the version to be always specified. To be fully consistent, as mentioned, we should have the API path for clients as follows: So f.e: I'm ok to have it like this as it's an API and the dev experience does not have to be so "strict" as we have the OpenAPI and swagger anyway. @vmuzikar @keycloak/cloud-native-maintainers WDYT?
Do you have any brief example/use-case how it could look like and how it could be used? Thanks
@shawkins Not sure how it'd look like. Do you have any Keycloak specific example how we could leverage groups? |
I think the belief for features is that most version changes won't have incompatiblity concerns. I don't think the assumption holds here as for example the difference between v1 and v2 is that they are completely incompatible. It would be best not to offer this style of endpoint from the start.
This is part of the issue I meant about relationships between the entities. I'm not sure something like Unless we now know the relationship structure for all api versions moving forward, then we probably don't want this kind of cross-version relationship. There could for example be invalid combinations moving forward that is a realm v3 might only support foo-clients, not clients. Then do we start the versioning of foo-clients at v1 or skip directly to v3? I realize this is a contrived example, but hopefully it conveys the problem.
For example https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#definitions - they have preferred group version defined. Clients could fetch the metadata, then know what default version is.
If there are distinct subgraphs of resources under realms, they could be grouped. |
@shawkins Thanks. +1
Agree.
Let's start simple, with not complicating things for Client API v2 for now, and solves the grouping of other potential APIs later - those would have a different path anyway, so there should not be any clash with that what we introduce now. |
Closes keycloak#44527 Signed-off-by: Martin Bartoš <[email protected]>
|
@keycloak/cloud-native @shawkins Updated (also PR description). |
|
I don't like mixing the schemes (i.e. |
|
CC @stianst |
@vmuzikar Thanks for the comment. I don't think it's a proper resource-based approach, as there should be a hierarchy of these resources (clients are part of the realm). TL;DR I think the Having only the |
I guess I was trying to say that this ^^ kinda contradicts the request for having distinct versions of the individual APIs. Either it's a single API with proper structure, or it's separate APIs with just references to cross-API resources
But what then? Are we ok with |
|
To summarize an offline discussion. We agreed on the following two alternatives:
Option 1 doesn't leave our door open if we ever needed global resources that are not tied to any realm, option 2 does. Another alternative would be to use a reserved word in place of a realm name should a resource be considered global. E.g. I'd vote for option 1 as it doesn't imply the realm is a resource – having @stianst What are your thoughts? Should we even consider the possibility for global resources? |
Realm is a built-in concept of containment. Here we are using exactly the same form of URL as Kubernetes - |
|
@shawkins btw. I added the code formatting for your examples, because the <> content was not visible. After yesterday's discussion, I would opt for option 2 and basically follow the Kubernetes approach. Yesterday, I had concerns about the intuitiveness of the path for users, but as was mentioned, it's an API that will be used by the generated clients. We'll have a nice, polished OpenAPI spec, with nice endpoint visibility. I'd say the namespace|realm should be clear from the first look and the @shawkins @vmuzikar So, I'm for Option 2 as there are more benefits than tradeoffs. |
Closes keycloak#44527 Signed-off-by: Martin Bartoš <[email protected]>
|
I see realm as the tenant, so should be the base of the URL, something like: This allows us to in the future to have things like virtual hosts that points to a realm; it also makes it easier to use different API groups for the same realm, as the base URL doesn't change. Also, adding that in case you want to move a realm to a different cluster, today that can be done by changing the core protocols URLs ( |
Closes keycloak#44527 Signed-off-by: Martin Bartoš <[email protected]>
Closes keycloak#44527 Signed-off-by: Martin Bartoš <[email protected]>
Summary
@vmuzikar @shawkins @stianst WDYT? We can continue with improvements in follow-up tasks. |
|
Just wanted to add more from our v2 meeting. What I'm basically advocated for is along the lines of front-end url == virtural host, which is discussed in #14623 and related issues. To support something like this we need an additional realm option to omit realms/name in paths we construct. Just as today when you set a front-end url at a proxy level you'd still need the mapping:
But instead of accessing keycloak via front-end / realms / name, users would use just front-end / Admin access by default looks like:
However URLs for server scoped resources - realms, server-info, etc. - don't make much sense. At worst every server scoped resource would need added to a rule in the form of - front-end / resource -> kc-host / admin / resource - and we'd have to ensure there could never be a conflict between global resource types and top level paths available from a public realm. Users are likely to need an admin specific mapping anyway to restrict access. An alternative is to only have realms/name omitted for front-end urls, and require an additional admin mapping of:
Now you have:
@vmuzikar can the requirement to have node level specific routing of admin requests using urls like And it was brought up in the meeting we could support both conventions. That is regardless of how they look, just serve requests such as: kc-host / realms / name / admin / server-info I'm not entirely sure, but I think to do this would need some additional realm level configuration option to indicate what admin paths should be formed. cc @keycloak/cloud-native @sschu Also this doesn't have to be based just upon the front-end url - you can have realm specific admin urls as well. |
|
@shawkins Virtual hosts for realm identification is definitely something we should look into in the future but I don't see it as something we need to figure out now, it's not really conflicting with our current work around designing the realm-based multi-tenancy. We will need to have a good story for putting the realm name in the path regardless – not everyone will use virtual hosts. To summarize today's discussion on the WG sync, we need to consider the following:
Overall proposal
|
To clarify I'm referening virutal hosts because that's what @stianst was referencing above. However I'm trying, and probably failing, to relate this functionality to our realm-based front-end URL support and how I think users would actually want their full front-end urls to look. Let me see if I can make this less confusing.
I see a strong overlap between the multi-tennancy concerns that are being brought up here and what URLs will be used externally and how that maps to internal URLs. Let's say I have realm-a on cluster 1 and realm-x on cluster 2. Now I'm tasked with moving realm-a from cluster 1 to cluster 2. Will realm-a and realm-x use the same hostname? Most likely not - which means this transition will involve setting a front-end url on realm-x. Or am I missing something here - are we considering using hostname-strict=false on cluster 2? With the realm level front-end url set, I completely agree it is straight-forward to change how the proxy handles A concern expressed above is that it will be difficult to handle admin access unless we have URLs with relative paths such as Again unless I'm missing something this is not generally possible today because we lack a per-realm ability to set the admin hostname url. So let's assume that feature, or at least a feature that treats the realm front-end url as the admin url, has been added. If you are already handling However using Is that a fair assessment?
I'm not suggesting that they will - realm specific front-end URL differentiation could be based upon path, not just hostname. |
We? Or the user of the API? That all depends on what they are using or not. I could for instance just be doing a CURL and the endpoints I'm using hasn't changed. |
|
We don't need to figure out the full virtual hosts story as part of this; there's a number of complications around that. Some may want a single virtual host that points to both the frontend and the admin endpoints. Some may want to expose it on different virtual hosts, or only use a virtual host for the frontend URLs, but not have a virtual host for the admin endpoints. Quite a lot needs changing to support this, and figuring out, hence why we haven't done that yet. My main point here was that the realm name must be before the API group, not after. Having it after made no sense anyways. My suggestion for new endpoints are:
Examples:
In all example URLs the version can be omitted, in which case the default version is used. I still would suggest using a header for version instead of path though. |
|
EDIT: I see that @stianst is suggesting something similar as described below.
The situation around this is little bit different than using a single prefix. Current Keycloak endpointsWe have Admin and non-Admin endpoints, like AdminRealmsResource and RealmsResource. 1. Realms resource -
|
|
@shawkins I'm not sure I entirely understand the concerns. Could you please elaborate a bit more?
Could you elaborate what you mean here? It's be a single Keycloak instance, we wouldn't be moving realms. Do you mean swapping hostnames between two realms?
Yes, we do not support per realm admin URLs. That's something we'd need to tackle especially with virtual hosts but it's not something we need to find a solution for now, IMHO.
Yes, that's my understanding too. In multi-tenancy scenarios, some customers might be more heavy on the admin API and might have e.g. dedicated nodes.
I think yes.
The API user.
I don't really like having the root resource of the API the client list. What if I want to manage client scopes under the client API? IMHO the root resource should be "nothing", i.e. to get clients, you'd do
This IMHO feels quite confusing. Realms API vs Realm API. Realms accessible only via master realm. It's not really consistent. Do you have any concerns with server scoped resources (that do not belong to any realm)? @mabartos If I understand your proposal correctly, you suggest to have |
At one point I believe a justification for having all urls rooted by
This is something I'd like to better understand. Is this done currently? And what is the basis for the dedication - are for example the cluser nodes somehow hetrogeneous? Or is this seen as an extension of session affinity?
@mabartos this is missing the realms term in the path, it should be - /admin/realms/{realm}/clients-api/v2/ - Clients API group (realm-scoped)
@stianst 's point from #44527 (comment) that he wants the realm name before the api group. That mostly works, but is conceptually confusing for realms themselves and other server scoped admin resources, which @vmuzikar notes in #44527 (comment) It might help to walk through more examples and to clearly separate the thinking about external and internal urls. Let's say we know that my-realm lives on kc-host. If the realm name is not in some simple location at the root of every external URL path, then we can only do proxy based logic if there is a 1-1 mapping between the admin-url and kc-host. Otherwise:
When would the admin-url not be 1-1 with the kc-host? Unless I'm missing something, this only comes into play when there are multiple keycloak clusters behind a single proxy - which relates to the case mentioned before about moving a realm from being hosted on one keycloak cluster to another. The less exotic scenario here is wanting the proxy to do realm specific access restrictions. Finally yet another scenario that was broached involves per realm mappings to specific cluster nodes - I don't have a good grasp on when and why this is being done yet, but that could also influence what seems like an acceptable solution. We can make the per realm manipulation straight-forward if instead we do:
Where do we go from here:
|
Closes keycloak#44527 Signed-off-by: Martin Bartoš <[email protected]>
|
@shawkins Thanks for the clarification. I've just updated the PR a little to include the However, I had to add also the So, the path looks like this: @shawkins @vmuzikar Unfortunately, I can't allocate more time to this before Christmas, so feel free to update this PR (or create a new one) based on your discussion. Thanks! |
|
I'm a little bit lost in this conversation by now, and not really clear on what everyone is suggestion. What's wrong with what I've been suggesting from the start
|
|
The assumption was that an api group subsumes different types of resources, that's why it would be |
|
After some offline discussion with @stianst, there are the following considerations:
So the proposal is:
@stianst Does that accurately capture the discussion we had earlier? |
While this adds more flexibility over a global version, I would add:
Also it would be good to clarify with any non-global versioning scheme what the relationship is to the feature version. In the prototype we have just a single feature version for the entire admin api. Related to this how, or should, admins be able to configure which serving versions are available?
Having api in the path serves a similar purpose - it's a sibling to the v1 /admin/realms
From a client perspective needing to have specialized URL construction handling for a single realm isn't ideal, but it's also not that difficult to manage. For the future it could be good to version server-info as well. Finally just like with other proposals, there's still a potential v1 conflict, this time with admin/api/console - but as of now we haven't proposed to actually have anything served of that endpoint so it's probably ok to ignore this. |
|
Do we need the |
|
@shawkins As for the versioning concerns. I see each API version also a Feature version, i.e. you can selectively enable/disable the API versions. To do that we'll need to add the ability to enable multiple Feature versions simultaniously. But that's for the future, right now we have a single Client API feature. Any subresources will be part of the parent resource API group. Adjacent resources (e.g. users and roles) will use separate API groups with independent versioning, but can use key references to the other resource as we don't expect keys would ever be affected by version bumps. @shawkins @sschu The potential conflict with |
Ok, so we'll have an admin api v1 feature, then a v(1+n) feature for each root resource type, and there is no presumed relationship between the same versions of different resources. If we add a new resource type, it will likely start at v2 to avoid confusion with the global v1 designation.
In general this reinforces the need to minimize additional versions - adding support for version ranges, min versions, etc. will add even more complexity on top of adding a lot of features.
This is true, but can still be confusing. Again with the case of adding a new type - if you need to reference that from an existing type, having to independently manage those versions is less than optimal.
It's probably good to note that v1 already has degenerate cases - a realm named realms, serverinfo, or console. If for example you create a realm called realms, then issue GET /admin/realms/console - the resteasy path matching logic will resolve that to AdminRoot.getRealmsAdmin, not getAdminConsole. Content-Type nor Accept headers come into play as neither operation is looking for them, and those do seem to be one of the last considerations in general. We all agree the v2 proposal adds api to the problematic set of realm names. In addition to the html expected from GET /admin/api/console, subpaths under console expect and produce different media types. While I don't see any conflict yet with v2 resources, it seems like it will be more complicated to address than adding produces / consumes to v1 / v2 methods and could further complicate things down the road. It's not ideal but if worst comes to worst we can just say that api was an unfortunate naming choice for a realm - just as realms, serverinfo, and console currently are. |
Yes, correct.
Yeah, introducing a new API version should be a rare event.
Could you elaborate on that?
I think we can't fully avoid the risk of conflicts– with or without |
With api groups you shouldn't have references in one resource to other resources in the group that don't exist in that version. Let's say we have an api group with version v1 and v2. v2 adds a new resource and that is referenced in a v2 resource that has both a v1 and a v2 version. Everything in my highly related api group is logcally consistent at v2. Fetching the v1 form of the exisitng resource shouldn't even have that new reference present - because it wouldn't be consistent. When you start developing against our fine-grained version management you'll basically be creating a version lock file: realms=v2 Based upon the latest versions supported by the Keycloak you initially target for development against. From there the developer will be responsible for individually updating these versions. And if you for example want to start using new resource v2, that may also require you to update to users=v3 - not an obvious relationship. |
admin/api/v2/realms/master/clients/*-->admin/master/api/clients/v2/clients/*admin/master/api/clients/=admin/master/api/clients/v2admin/master/api/clients/v2/scopes,...