-
Notifications
You must be signed in to change notification settings - Fork 7
LCORE passes the k8s token as a standard Authorization header #14
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
Conversation
|
@iNecas if there was another use case you had in mind for "kubernetes-authorization" as the header name, let me know, but this change makes it work with LCORE integration using an LCORE config like: We could always make the header name configurable, but if we don't need it i'd rather just stick with the standard |
|
We just mimicked the approach taken in other mcp servers https://github.com/openshift/cluster-health-analyzer/blob/4ba006224a882dff0fae6b89a8cbbbf1e6c05f51/pkg/mcp/server.go#L15 (uses just kuberentes-authoriazation), and containers/kubernetes-mcp-server@9ffb818#diff-c97ec8241004eb2d4bc81231071540f96e7ebb84c15ed128b44285912bd2138aR29 (uses authorization with fallback to kuberetes-authorization, but it used to be just kuberetnes-authorization frist), ligthspeed service changed it here some time ago. https://github.com/openshift/lightspeed-service/pull/2634/files. Is this some backward incompatible change between old lightspeed-service and LCORE one? I'm fine witt the change. I only wonder if we need to consider backward compatibility. |
|
After asking and looking around it seems like we just would need to change the header. I guess we would need keep the original ones if we wanted to working with some older OLS versions, but that's not the case. Let's get it in. |
i'm not sure what lightspeed used to support vs now, but my understanding is now they expect an cc @blubinsky who would have more knowledge about what header names LCORE expects to use when invoking mcp servers w/ k8s auth tokens. |
eeac3bf to
a244ca4
Compare
|
Approved, the PR settings requires verified signature. Mind updating @bparees ? |
a244ca4 to
4136c2b
Compare
done |
|
@iNecas it seems like i had a misunderstanding about how the LCORE mcp auth header is configured[1], so in theory i think LCORE could be configured to pass a header named "kubernetes-authorization" along w/ the k8s token. That said, I still think "Authorization" is a more standard header name, so i'd vote for either using that, or making the header name configurable(which seems like overkill). |
|
Agreed |
When using LCORE and configuring an mcp server to use "kubernetes" auth, LCORE passes the token using a standard header named "Authorization", so this PR aligns the obs-mcp server w/ that header name