feat(validation): add pre-execution validation layer by nader-ziada · Pull Request #764 · containers/kubernetes-mcp-server

nader-ziada · 2026-02-10T21:47:18Z

Add validation middleware that catches errors before they reach the Kubernetes API.

Changes:

Resource validation: catches typos in resource types (GVK)
Schema validation: validates manifests against OpenAPI schema
RBAC validation: pre-checks permissions using SelfSubjectAccessReview
All validators are enabled by default and can be configured via environment variables (MCP_VALIDATION_*) or TOML config.

Closes #775

pkg/validation/middleware.go

matzew · 2026-02-11T08:20:01Z

pkg/validation/rbac_validator.go

@Cali0707 just wondering (un releated) some sort of validation of "larger cluster" access would be nice for multi-cluster/acm use-case. I think we currently list all "managed clusters", w/o some sort of "validation"

Note that we already have a canIUser method defined here:

kubernetes-mcp-server/pkg/kubernetes/resources.go

Lines 232 to 248 in dbc1bd0

func (c *Core) canIUse(ctx context.Context, gvr *schema.GroupVersionResource, namespace, verb string) bool {

accessReviews := c.AuthorizationV1().SelfSubjectAccessReviews()

response, err := accessReviews.Create(ctx, &authv1.SelfSubjectAccessReview{

Spec: authv1.SelfSubjectAccessReviewSpec{ResourceAttributes: &authv1.ResourceAttributes{

Namespace: namespace,

Verb: verb,

Group: gvr.Group,

Version: gvr.Version,

Resource: gvr.Resource,

}},

}, metav1.CreateOptions{})

if err != nil {

// TODO: maybe return the error too

return false

}

return response.Status.Allowed

}

both methods are unexposed (private methods) and serve different purposes in different contexts
I prefer to have them in different packages with clear separation of concerns.

but let me know if you prefer to combine them somehow and I can do that

Just noting in the context of the downstream issue that relates to this fix.
I'm not sure that if in that context, we might just want to put this in the AccessControlRoundTripper.
Here, we're already checking for denied resources in the configuration (for example)

my understanding is that AccessControlRoundTripper checks configured denials (checks against a static deny list that the server operator configures. This is separate from Kubernetes RBAC) while the RBACValidator checks actual RBAC (what the cluster says about the current user's permissions)

Are you saying we don't need the RBACValidator since we will get the AccessControlRoundTripper check anyways? but the error message and logging would be different?

No, I was saying that maybe (if we're just tackling the downstream issue -it's unclear to me the scope of it since it's not properly described-) we can just add RBAC validation to the roundrtipper (simple solution: KISS, YAGNI)

In any case, I'll review everything else in the issue.

What I'd also suggest is to make this opt-in for a couple of vesions at least upstream, since it might have side-effects we haven't accounted for.

pkg/validation/middleware.go

manusa

Thx for putting this together.
I gave it an initial look and added a few comments.

pkg/mcp/mcp.go

Add validation middleware that catches errors before they reach the Kubernetes API. Signed-off-by: Nader Ziada <nziada@redhat.com>

nader-ziada · 2026-02-12T18:44:31Z

made a refactor of how this works, sorry to the reviewers, to make work from the kubernetes pkg instead of the mcp pkg, now more dynamic and flexible. also disabled by default.

manusa

I think that the decoupling from the mcp layer is going great, thx.
I added some more comments regarding the new approach.

manusa · 2026-02-16T13:50:35Z

pkg/config/validation_config.go

I don't recall we support config overrides through environment variables at the moment, we do support those through CLI flags.

Is there any reason why we need to be able to override this through an environment variable?

manusa · 2026-02-16T13:51:50Z

pkg/config/validation_config.go

Are we planning to add more config options to validation?
If so, I understand the nesting of the config into a different struct.
If not, why is the nesting needed?

i originally had different flags for each validator, but decided its too much complexity, will remove struct

manusa · 2026-02-16T13:55:15Z

pkg/kubernetes/validation_round_tripper.go

This seems to be a substitute to the AccessControlRoundTripper, but the former is still retained (which is confusing).

I'm not sure if we prefer to make the composable and compose the when wiring together in kubernetes.go.
Or merging both into this one.
Whatever works better, but we should trim and clean up the redundant bits.

merged the validation login into the AccessControlRoundTripper

pkg/mcp/events_test.go

pkg/mcp/pods_test.go

README.md

Signed-off-by: Nader Ziada <nziada@redhat.com>

matzew reviewed Feb 11, 2026

View reviewed changes

pkg/validation/middleware.go Outdated Show resolved Hide resolved

matzew reviewed Feb 11, 2026

View reviewed changes

pkg/validation/middleware.go Outdated Show resolved Hide resolved

manusa reviewed Feb 11, 2026

View reviewed changes

pkg/mcp/mcp.go Outdated Show resolved Hide resolved

pkg/mcp/mcp.go Outdated Show resolved Hide resolved

manusa mentioned this pull request Feb 12, 2026

refactor(kubernetes): export k.CanIUse func #775

Open

nader-ziada force-pushed the validation branch from f08a31e to 108f733 Compare February 12, 2026 18:38

feat(validation): add pre-execution validation layer

b07324e

Add validation middleware that catches errors before they reach the Kubernetes API. Signed-off-by: Nader Ziada <nziada@redhat.com>

nader-ziada force-pushed the validation branch from 108f733 to b07324e Compare February 12, 2026 18:42

manusa self-requested a review February 16, 2026 13:34

swghosh approved these changes Feb 16, 2026

View reviewed changes

manusa requested changes Feb 16, 2026

View reviewed changes

simplify config and merge into AccessControlRoundTripper

252fbf0

Signed-off-by: Nader Ziada <nziada@redhat.com>

nader-ziada requested a review from manusa February 18, 2026 13:44

	func (c Core) canIUse(ctx context.Context, gvr schema.GroupVersionResource, namespace, verb string) bool {
	accessReviews := c.AuthorizationV1().SelfSubjectAccessReviews()
	response, err := accessReviews.Create(ctx, &authv1.SelfSubjectAccessReview{
	Spec: authv1.SelfSubjectAccessReviewSpec{ResourceAttributes: &authv1.ResourceAttributes{
	Namespace: namespace,
	Verb: verb,
	Group: gvr.Group,
	Version: gvr.Version,
	Resource: gvr.Resource,
	}},
	}, metav1.CreateOptions{})
	if err != nil {
	// TODO: maybe return the error too
	return false
	}
	return response.Status.Allowed
	}

Conversation

nader-ziada commented Feb 10, 2026 • edited by manusa Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

manusa Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

manusa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

nader-ziada commented Feb 12, 2026

Uh oh!

manusa left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Comments

nader-ziada commented Feb 10, 2026 •

edited by manusa

Loading

manusa Feb 11, 2026 •

edited

Loading