[WIP] feat: add IsValid check for toolsets#490
[WIP] feat: add IsValid check for toolsets#490Cali0707 wants to merge 1 commit intocontainers:mainfrom
Conversation
Signed-off-by: Calum Murray <cmurray@redhat.com>
| ) | ||
| } | ||
|
|
||
| func (t *Toolset) IsValid(k *internalk8s.Kubernetes) bool { |
There was a problem hiding this comment.
@aljesusg when you have time can you take a look at this method and let me know if this makes sense?
It's supposed to validate that there is a working kiali installation in the target cluster
There was a problem hiding this comment.
Yes… although I would do the following: since the AuthInfoEndpoint endpoint is already created, I would create a method to call it, as it doesn’t require any parameters and is ‘available’ whether the user is authenticated or not.
There was a problem hiding this comment.
Furthermore, Kiali supports multicluster deployments, allowing a single Kiali endpoint to communicate with instances in other clusters. However, I am still evaluating whether this specific architecture will meet our requirements.
In some tools, we have a clusterName attribute that defaults to 'local' if it isn't specified by the LLM
|
Will add tests to this next, then it should be good to merge (if people like the approach here) |
|
I believe that this entire functionality requires more thought. This is similar to what's already done in: kubernetes-mcp-server/pkg/api/toolsets.go Line 46 in 0e88935 and related methods/functons. Note This was part of the first spike of the MCP server hence it doesn't make sense in the current state of the cluster. In the early stages of the Kubernetes MCP server this was just enough:
In the current state (multi-cluster + auth support), when a user/client performs a list request (tools, prompts, and so on), the MCP server SHOULD account for everything that applies to that user in its specific scenario:
I'm not convinced about the idea of completely enabling or disabling an entire toolset (as this PR proposes). One of the challenges to do this properly is that go-sdk (or any of the other SDKs) require the tools to be loaded at start-up and don't provide callbacks to provide the tool or resource list dynamically or per session/user (AFAIK, maybe this has changed). In this PR we're getting the derived Kubernetes when reloading the tools, but this will likely fail if there's no global auth for that cluster. I'm really not sure how the problem can be properly tackled. Maybe checking the internals of the SDK and seeing if the list/tools request can be intercepted or something along those lines. |
|
@Cali0707 wanna still do this? |
This PR allows us to dynamically enable/disable toolsets, as well as the targets within tools, based on whether or not prerequisites for the toolset are installed in a given target cluster.
That way, users can enable e.g.
kialitoolset, and if it is only installed in 3/4 clusters, we will only hint targets for those 3 clusters to the MCP client. Similarly, if none of the clusters havekialiinstalled, we will simply disable the toolset and not present it to any clients.