Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions pkg/kubernetes/accesscontrol.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package kubernetes

import (
"fmt"

"k8s.io/apimachinery/pkg/runtime/schema"

"github.com/manusa/kubernetes-mcp-server/pkg/config"
)

// isAllowed checks the resource is in denied list or not.
// If it is in denied list, this function returns false.
func isAllowed(
staticConfig *config.StaticConfig, // TODO: maybe just use the denied resource slice
gvk *schema.GroupVersionKind,
) bool {
if staticConfig == nil {
return true
}

for _, val := range staticConfig.DeniedResources {
// If kind is empty, that means Group/Version pair is denied entirely
if val.Kind == "" {
if gvk.Group == val.Group && gvk.Version == val.Version {
return false
}
}
if gvk.Group == val.Group &&
gvk.Version == val.Version &&
gvk.Kind == val.Kind {
return false
}
}

return true
}

func isNotAllowedError(gvk *schema.GroupVersionKind) error {
return fmt.Errorf("resource not allowed: %s", gvk.String())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(not a blocker for this PR):
We have to return 403: Forbidden HTTP Status. But I'm not really sure that mcp library we use support this. If it doesn't support returning http code we have, we'll have major issue in oauth side as well.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I don't think the library (or even the protocol) is prepared for fine-grained operation status codes yet.
As I understand it, the protocol negotiates authentication and authorization first, and then enables

image

}
74 changes: 74 additions & 0 deletions pkg/kubernetes/accesscontrol_clientset.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
package kubernetes

import (
authorizationv1api "k8s.io/api/authorization/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/client-go/discovery"
"k8s.io/client-go/kubernetes"
authorizationv1 "k8s.io/client-go/kubernetes/typed/authorization/v1"
corev1 "k8s.io/client-go/kubernetes/typed/core/v1"
"k8s.io/client-go/rest"

"github.com/manusa/kubernetes-mcp-server/pkg/config"
)

// AccessControlClientset is a limited clientset delegating interface to the standard kubernetes.Clientset
// Only a limited set of functions are implemented with a single point of access to the kubernetes API where
// apiVersion and kinds are checked for allowed access
type AccessControlClientset struct {
delegate kubernetes.Interface
discoveryClient discovery.DiscoveryInterface
staticConfig *config.StaticConfig // TODO: maybe just store the denied resource slice
}

func (a *AccessControlClientset) DiscoveryClient() discovery.DiscoveryInterface {
return a.discoveryClient
}

func (a *AccessControlClientset) Pods(namespace string) (corev1.PodInterface, error) {
gvk := &schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Pod"}
if !isAllowed(a.staticConfig, gvk) {
return nil, isNotAllowedError(gvk)
}
return a.delegate.CoreV1().Pods(namespace), nil
}

func (a *AccessControlClientset) PodsExec(namespace, name string) (*rest.Request, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about moving createExecutor also in here and this function returns remotecommand.Executor?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be done now, Schema and ParameterCodec are now reused from client-go globals

gvk := &schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Pod"}
if !isAllowed(a.staticConfig, gvk) {
return nil, isNotAllowedError(gvk)
}
// https://github.com/kubernetes/kubectl/blob/5366de04e168bcbc11f5e340d131a9ca8b7d0df4/pkg/cmd/exec/exec.go#L382-L397
return a.delegate.CoreV1().RESTClient().
Post().
Resource("pods").
Namespace(namespace).
Name(name).
SubResource("exec"), nil
}

func (a *AccessControlClientset) Services(namespace string) (corev1.ServiceInterface, error) {
gvk := &schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Service"}
if !isAllowed(a.staticConfig, gvk) {
return nil, isNotAllowedError(gvk)
}
return a.delegate.CoreV1().Services(namespace), nil
}

func (a *AccessControlClientset) SelfSubjectAccessReviews() (authorizationv1.SelfSubjectAccessReviewInterface, error) {
gvk := &schema.GroupVersionKind{Group: authorizationv1api.GroupName, Version: authorizationv1api.SchemeGroupVersion.Version, Kind: "SelfSubjectAccessReview"}
if !isAllowed(a.staticConfig, gvk) {
return nil, isNotAllowedError(gvk)
}
return a.delegate.AuthorizationV1().SelfSubjectAccessReviews(), nil
}

func NewAccessControlClientset(cfg *rest.Config, staticConfig *config.StaticConfig) (*AccessControlClientset, error) {
clientSet, err := kubernetes.NewForConfig(cfg)
if err != nil {
return nil, err
}
return &AccessControlClientset{
delegate: clientSet, discoveryClient: clientSet.DiscoveryClient, staticConfig: staticConfig,
}, nil
}
42 changes: 8 additions & 34 deletions pkg/kubernetes/accesscontrol_restmapper.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package kubernetes

import (
"fmt"

"k8s.io/apimachinery/pkg/api/meta"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/client-go/restmapper"
Expand All @@ -17,37 +15,13 @@ type AccessControlRESTMapper struct {

var _ meta.RESTMapper = &AccessControlRESTMapper{}

// isAllowed checks the resource is in denied list or not.
// If it is in denied list, this function returns false.
func (a AccessControlRESTMapper) isAllowed(gvk *schema.GroupVersionKind) bool {
if a.staticConfig == nil {
return true
}

for _, val := range a.staticConfig.DeniedResources {
// If kind is empty, that means Group/Version pair is denied entirely
if val.Kind == "" {
if gvk.Group == val.Group && gvk.Version == val.Version {
return false
}
}
if gvk.Group == val.Group &&
gvk.Version == val.Version &&
gvk.Kind == val.Kind {
return false
}
}

return true
}

func (a AccessControlRESTMapper) KindFor(resource schema.GroupVersionResource) (schema.GroupVersionKind, error) {
gvk, err := a.delegate.KindFor(resource)
if err != nil {
return schema.GroupVersionKind{}, err
}
if !a.isAllowed(&gvk) {
return schema.GroupVersionKind{}, fmt.Errorf("resource not allowed: %s", gvk.String())
if !isAllowed(a.staticConfig, &gvk) {
return schema.GroupVersionKind{}, isNotAllowedError(&gvk)
}
return gvk, nil
}
Expand All @@ -58,8 +32,8 @@ func (a AccessControlRESTMapper) KindsFor(resource schema.GroupVersionResource)
return nil, err
}
for i := range gvks {
if !a.isAllowed(&gvks[i]) {
return nil, fmt.Errorf("resource not allowed: %s", gvks[i].String())
if !isAllowed(a.staticConfig, &gvks[i]) {
return nil, isNotAllowedError(&gvks[i])
}
}
return gvks, nil
Expand All @@ -76,8 +50,8 @@ func (a AccessControlRESTMapper) ResourcesFor(input schema.GroupVersionResource)
func (a AccessControlRESTMapper) RESTMapping(gk schema.GroupKind, versions ...string) (*meta.RESTMapping, error) {
for _, version := range versions {
gvk := &schema.GroupVersionKind{Group: gk.Group, Version: version, Kind: gk.Kind}
if !a.isAllowed(gvk) {
return nil, fmt.Errorf("resource not allowed: %s", gvk.String())
if !isAllowed(a.staticConfig, gvk) {
return nil, isNotAllowedError(gvk)
}
}
return a.delegate.RESTMapping(gk, versions...)
Expand All @@ -86,8 +60,8 @@ func (a AccessControlRESTMapper) RESTMapping(gk schema.GroupKind, versions ...st
func (a AccessControlRESTMapper) RESTMappings(gk schema.GroupKind, versions ...string) ([]*meta.RESTMapping, error) {
for _, version := range versions {
gvk := &schema.GroupVersionKind{Group: gk.Group, Version: version, Kind: gk.Kind}
if !a.isAllowed(gvk) {
return nil, fmt.Errorf("resource not allowed: %s", gvk.String())
if !isAllowed(a.staticConfig, gvk) {
return nil, isNotAllowedError(gvk)
}
}
return a.delegate.RESTMappings(gk, versions...)
Expand Down
11 changes: 5 additions & 6 deletions pkg/kubernetes/kubernetes.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"k8s.io/client-go/discovery"
"k8s.io/client-go/discovery/cached/memory"
"k8s.io/client-go/dynamic"
"k8s.io/client-go/kubernetes"
_ "k8s.io/client-go/plugin/pkg/client/auth/oidc"
"k8s.io/client-go/rest"
"k8s.io/client-go/restmapper"
Expand Down Expand Up @@ -41,8 +40,8 @@ type Manager struct {
clientCmdConfig clientcmd.ClientConfig
scheme *runtime.Scheme
parameterCodec runtime.ParameterCodec
clientSet kubernetes.Interface
discoveryClient discovery.CachedDiscoveryInterface
accessControlClientSet *AccessControlClientset
accessControlRESTMapper *AccessControlRESTMapper
dynamicClient *dynamic.DynamicClient

Expand All @@ -65,11 +64,11 @@ func NewManager(kubeconfig string, config *config.StaticConfig) (*Manager, error
// return &impersonateRoundTripper{original}
//})
var err error
k8s.clientSet, err = kubernetes.NewForConfig(k8s.cfg)
k8s.accessControlClientSet, err = NewAccessControlClientset(k8s.cfg, k8s.staticConfig)
if err != nil {
return nil, err
}
k8s.discoveryClient = memory.NewMemCacheClient(discovery.NewDiscoveryClient(k8s.clientSet.CoreV1().RESTClient()))
k8s.discoveryClient = memory.NewMemCacheClient(k8s.accessControlClientSet.DiscoveryClient())
k8s.accessControlRESTMapper = NewAccessControlRESTMapper(
restmapper.NewDeferredDiscoveryRESTMapper(k8s.discoveryClient),
k8s.staticConfig,
Expand Down Expand Up @@ -163,11 +162,11 @@ func (m *Manager) Derived(ctx context.Context) *Kubernetes {
scheme: m.scheme,
parameterCodec: m.parameterCodec,
}}
derived.manager.clientSet, err = kubernetes.NewForConfig(derived.manager.cfg)
derived.manager.accessControlClientSet, err = NewAccessControlClientset(derived.manager.cfg, derived.manager.staticConfig)
if err != nil {
return &Kubernetes{manager: m}
}
derived.manager.discoveryClient = memory.NewMemCacheClient(discovery.NewDiscoveryClient(derived.manager.clientSet.CoreV1().RESTClient()))
derived.manager.discoveryClient = memory.NewMemCacheClient(derived.manager.accessControlClientSet.DiscoveryClient())
derived.manager.accessControlRESTMapper = NewAccessControlRESTMapper(
restmapper.NewDeferredDiscoveryRESTMapper(derived.manager.discoveryClient),
derived.manager.staticConfig,
Expand Down
34 changes: 22 additions & 12 deletions pkg/kubernetes/pods.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func (k *Kubernetes) PodsGet(ctx context.Context, namespace, name string) (*unst

func (k *Kubernetes) PodsDelete(ctx context.Context, namespace, name string) (string, error) {
namespace = k.NamespaceOrDefault(namespace)
pod, err := k.manager.clientSet.CoreV1().Pods(namespace).Get(ctx, name, metav1.GetOptions{})
pod, err := k.ResourcesGet(ctx, &schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Pod"}, namespace, name)
if err != nil {
return "", err
}
Expand All @@ -62,11 +62,15 @@ func (k *Kubernetes) PodsDelete(ctx context.Context, namespace, name string) (st

// Delete managed service
if isManaged {
if sl, _ := k.manager.clientSet.CoreV1().Services(namespace).List(ctx, metav1.ListOptions{
services, err := k.manager.accessControlClientSet.Services(namespace)
if err != nil {
return "", err
}
if sl, _ := services.List(ctx, metav1.ListOptions{
LabelSelector: managedLabelSelector.String(),
}); sl != nil {
for _, svc := range sl.Items {
_ = k.manager.clientSet.CoreV1().Services(namespace).Delete(ctx, svc.Name, metav1.DeleteOptions{})
_ = services.Delete(ctx, svc.Name, metav1.DeleteOptions{})
}
}
}
Expand All @@ -86,12 +90,16 @@ func (k *Kubernetes) PodsDelete(ctx context.Context, namespace, name string) (st

}
return "Pod deleted successfully",
k.manager.clientSet.CoreV1().Pods(namespace).Delete(ctx, name, metav1.DeleteOptions{})
k.ResourcesDelete(ctx, &schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Pod"}, namespace, name)
}

func (k *Kubernetes) PodsLog(ctx context.Context, namespace, name, container string) (string, error) {
tailLines := int64(256)
req := k.manager.clientSet.CoreV1().Pods(k.NamespaceOrDefault(namespace)).GetLogs(name, &v1.PodLogOptions{
pods, err := k.manager.accessControlClientSet.Pods(k.NamespaceOrDefault(namespace))
if err != nil {
return "", err
}
req := pods.GetLogs(name, &v1.PodLogOptions{
TailLines: &tailLines,
Container: container,
})
Expand Down Expand Up @@ -220,7 +228,11 @@ func (k *Kubernetes) PodsTop(ctx context.Context, options PodsTopOptions) (*metr

func (k *Kubernetes) PodsExec(ctx context.Context, namespace, name, container string, command []string) (string, error) {
namespace = k.NamespaceOrDefault(namespace)
pod, err := k.manager.clientSet.CoreV1().Pods(namespace).Get(ctx, name, metav1.GetOptions{})
pods, err := k.manager.accessControlClientSet.Pods(namespace)
if err != nil {
return "", err
}
pod, err := pods.Get(ctx, name, metav1.GetOptions{})
if err != nil {
return "", err
}
Expand Down Expand Up @@ -260,12 +272,10 @@ func (k *Kubernetes) PodsExec(ctx context.Context, namespace, name, container st
func (k *Kubernetes) createExecutor(namespace, name string, podExecOptions *v1.PodExecOptions) (remotecommand.Executor, error) {
// Compute URL
// https://github.com/kubernetes/kubectl/blob/5366de04e168bcbc11f5e340d131a9ca8b7d0df4/pkg/cmd/exec/exec.go#L382-L397
req := k.manager.clientSet.CoreV1().RESTClient().
Post().
Resource("pods").
Namespace(namespace).
Name(name).
SubResource("exec")
req, err := k.manager.accessControlClientSet.PodsExec(namespace, name)
if err != nil {
return nil, err
}
req.VersionedParams(podExecOptions, k.manager.parameterCodec)
spdyExec, err := remotecommand.NewSPDYExecutor(k.manager.cfg, "POST", req.URL())
if err != nil {
Expand Down
8 changes: 6 additions & 2 deletions pkg/kubernetes/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ func (k *Kubernetes) resourcesListAsTable(ctx context.Context, gvk *schema.Group
}
url = append(url, gvr.Resource)
var table metav1.Table
err := k.manager.clientSet.CoreV1().RESTClient().
err := k.manager.discoveryClient.RESTClient().
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, now we support all the resources

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what you mean with this.
AFAIU, before all resources were supported too, this was/is just a lazy way to pass share the RESTClient.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NVM

Get().
SetHeader("Accept", strings.Join([]string{
fmt.Sprintf("application/json;as=Table;v=%s;g=%s", metav1.SchemeGroupVersion.Version, metav1.GroupName),
Expand Down Expand Up @@ -188,7 +188,11 @@ func (k *Kubernetes) supportsGroupVersion(groupVersion string) bool {
}

func (k *Kubernetes) canIUse(ctx context.Context, gvr *schema.GroupVersionResource, namespace, verb string) bool {
response, err := k.manager.clientSet.AuthorizationV1().SelfSubjectAccessReviews().Create(ctx, &authv1.SelfSubjectAccessReview{
accessReviews, err := k.manager.accessControlClientSet.SelfSubjectAccessReviews()
if err != nil {
return false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very nice!

}
response, err := accessReviews.Create(ctx, &authv1.SelfSubjectAccessReview{
Spec: authv1.SelfSubjectAccessReviewSpec{ResourceAttributes: &authv1.ResourceAttributes{
Namespace: namespace,
Verb: verb,
Expand Down
28 changes: 27 additions & 1 deletion pkg/mcp/helm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,33 @@ func TestHelmUninstall(t *testing.T) {
}

func TestHelmUninstallDenied(t *testing.T) {
t.Skip("To be implemented") // TODO: helm_uninstall is not checking for denied resources
deniedResourcesServer := &config.StaticConfig{DeniedResources: []config.GroupVersionKind{{Version: "v1", Kind: "Secret"}}}
testCaseWithContext(t, &mcpContext{staticConfig: deniedResourcesServer}, func(c *mcpContext) {
c.withEnvTest()
kc := c.newKubernetesClient()
clearHelmReleases(c.ctx, kc)
_, _ = kc.CoreV1().Secrets("default").Create(c.ctx, &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "sh.helm.release.v1.existent-release-to-uninstall.v0",
Labels: map[string]string{"owner": "helm", "name": "existent-release-to-uninstall"},
},
Data: map[string][]byte{
"release": []byte(base64.StdEncoding.EncodeToString([]byte("{" +
"\"name\":\"existent-release-to-uninstall\"," +
"\"info\":{\"status\":\"deployed\"}," +
"\"manifest\":\"apiVersion: v1\\nkind: Secret\\nmetadata:\\n name: secret-to-deny\\n namespace: default\\n\"" +
"}"))),
},
}, metav1.CreateOptions{})
helmUninstall, _ := c.callTool("helm_uninstall", map[string]interface{}{
"name": "existent-release-to-uninstall",
})
t.Run("helm_uninstall has error", func(t *testing.T) {
if !helmUninstall.IsError {
t.Fatalf("call tool should fail")
}
})
})
}

func clearHelmReleases(ctx context.Context, kc *kubernetes.Clientset) {
Expand Down
Loading