Refactor API and update documentation with validation tests by aliok · Pull Request #40 · kubernetes-sigs/mcp-lifecycle-operator

aliok · 2026-03-16T08:41:24Z

Fixes #32

Copilot

Pull request overview

This PR refactors the MCPServer CRD/API to use logical groupings (spec.source, spec.config, spec.runtime) and updates the controller, examples, and docs accordingly, with added CRD validation coverage (Issue #32).

Changes:

Refactor MCPServerSpec to introduce source/config/runtime groupings and a new config.storage mount model.
Update controller reconciliation logic and existing controller tests to the new spec shape.
Update CRD/schema, samples/examples, and README documentation; add envtest-based validation tests for new CRD rules.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
internal/controller/mcpserver_controller.go	Switch controller to `spec.source` + `spec.config` + `spec.runtime`, implement `config.storage` → volumes/mounts
internal/controller/mcpserver_controller_test.go	Update controller tests to new spec shape; remove old SecretRef mount test block
api/v1alpha1/mcpserver_types.go	Introduce new API types (`Source`, `ServerConfig`, `StorageMount`, `RuntimeConfig`) and move fields into groups
api/v1alpha1/zz_generated.deepcopy.go	Regenerate deepcopy implementations for new API types
api/v1alpha1/mcpserver_validation_test.go	Add envtest suite to verify CRD validation rules (source/storage/port/ref)
config/crd/bases/mcp.x-k8s.io_mcpservers.yaml	Update CRD schema, printcolumns, and validations for new grouped spec
config/samples/mcp_v1alpha1_mcpserver.yaml	Update sample manifest to new spec structure
examples/kubernetes-mcp-server/*.yaml	Update example manifests to `source/config/runtime` and `config.storage`
examples/kubernetes-mcp-server/README.md	Update example documentation to match new spec fields
examples/everything-mcp-server/mcpserver.yaml	Update example manifest to new spec structure
README.md	Update top-level documentation examples to new spec structure

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

ArangoGutierrez

Looks good, Only one comment on my side, but I think some of the GitHub Copilot comments are worth addressing

Copilot

Pull request overview

Refactors the MCPServer CRD/API to logically group fields (source/config/runtime) and updates the controller, examples, and tests accordingly (Issue #32).

Changes:

Restructures MCPServer spec to use spec.source, spec.config, and spec.runtime.security.
Reworks controller reconciliation to consume the new spec and implements spec.config.storage volume/mount generation with existence checks.
Updates examples/docs and adds CRD schema validation tests plus new controller tests for storage mounts.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
internal/controller/mcpserver_controller.go	Updates Deployment/Service creation to use new `source/config/runtime` API and new storage mount processing.
internal/controller/mcpserver_controller_test.go	Migrates controller tests to new spec and adds reconciliation assertions for storage mounts.
api/v1alpha1/mcpserver_types.go	Introduces new API types (Source/Config/Runtime/StorageMount) and validation annotations.
api/v1alpha1/zz_generated.deepcopy.go	Regenerates deepcopy implementations for the new API types.
api/v1alpha1/mcpserver_validation_test.go	Adds envtest-backed tests validating CRD schema rules for the new spec structure.
config/crd/bases/mcp.x-k8s.io_mcpservers.yaml	Updates CRD schema, validations, and printcolumns to match the new grouped fields.
config/samples/mcp_v1alpha1_mcpserver.yaml	Updates the sample MCPServer manifest to the new API shape.
examples/kubernetes-mcp-server/mcpserver.yaml	Updates example manifest to use `spec.source` + `spec.config`.
examples/kubernetes-mcp-server/mcpserver-with-config.yaml	Updates ConfigMap-mount example to use `spec.config.storage` + `spec.config.arguments`.
examples/kubernetes-mcp-server/mcpserver-with-rbac.yaml	Updates RBAC example to use `spec.runtime.security.serviceAccountName` and storage mounts.
examples/kubernetes-mcp-server/README.md	Updates example documentation to reflect the new storage/arguments/runtime fields.
examples/everything-mcp-server/mcpserver.yaml	Updates example manifest to the new API shape.
README.md	Updates top-level README examples to the new API shape.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

mrunalp · 2026-03-16T18:00:58Z

/ok-to-test

aliok · 2026-03-16T18:01:00Z

Addressed the GitHub copilot comments.
Can you take a new look please?

everettraven

I wasn't able to get through the entire PR as I had something come up in the middle of reviewing, but wanted to leave the comments I did have.

I'll pick this review back up first thing in the morning.

aliok · 2026-03-17T09:40:25Z

@everettraven Thanks a lot for the review. I think I've addressed all of them!

ArangoGutierrez

LGTM - but let's squash the commits to tell a nice git history and then we can merge

aliok · 2026-03-17T11:13:10Z

Gonna rebase and squash

aliok · 2026-03-17T11:44:53Z

LGTM - but let's squash the commits to tell a nice git history and then we can merge

@ArangoGutierrez #16

I think the PR should still have individual commits - as many as the author likes - and when we merge, we should squash them. Curious people can go to the PR and check individual commits that way.

Signed-off-by: Ali Ok <aliok@redhat.com>

…onfig and SecurityConfig Signed-off-by: Ali Ok <aliok@redhat.com>

Signed-off-by: Ali Ok <aliok@redhat.com>

matzew · 2026-03-19T07:33:44Z

/ok-to-test

…llow empty array Signed-off-by: Ali Ok <aliok@redhat.com>

aliok · 2026-03-19T08:25:14Z

presubmit check fails due to this:

go: no such tool "covdata"

But, please do not block this PR because of that. That's irrelevant with this PR and we can fix that separately.

matzew · 2026-03-19T08:34:17Z

go: no such tool "covdata"

I guess we should fix/remove that?

jaideepr97 · 2026-03-19T13:19:31Z

go: no such tool "covdata"

I guess we should fix/remove that?

#48 just merged
@matzew could you re-trigger the CI? I think it should be green now

matzew · 2026-03-19T13:27:21Z

/test presubmit-mcp-lifecycle-operator-unit-test

Signed-off-by: Matthias Wessendorf <mwessend@redhat.com>

Fix samples and RBAC

matzew

Thanks for the work and refactorings! This looks good to me now!

/lgtm
/approve

/hold
feel free to unhold

k8s-ci-robot · 2026-03-23T06:49:47Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aliok, matzew

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [matzew]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

aliok · 2026-03-23T08:01:31Z

I will wait for a few hours for @ArangoGutierrez :) I suspect he might be at KubeCon

aliok · 2026-03-23T11:02:01Z

/unhold

Let's create separate issues if we find anything

k8s-ci-robot requested review from ArangoGutierrez and mrunalp March 16, 2026 08:41

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Mar 16, 2026

aliok mentioned this pull request Mar 16, 2026

Discussion: CRD refactoring #36

Closed

ArangoGutierrez requested a review from Copilot March 16, 2026 10:03

Copilot started reviewing on behalf of ArangoGutierrez March 16, 2026 10:04 View session

Copilot AI reviewed Mar 16, 2026

View reviewed changes

ArangoGutierrez requested changes Mar 16, 2026

View reviewed changes

Comment thread internal/controller/mcpserver_controller.go

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 16, 2026

aliok force-pushed the 2026-03-10-crd-logical-grouping-impl branch from e2449fe to 35ae53a Compare March 16, 2026 11:32

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 16, 2026

aliok requested review from ArangoGutierrez and Copilot March 16, 2026 11:49

Copilot started reviewing on behalf of aliok March 16, 2026 11:50 View session

Copilot AI reviewed Mar 16, 2026

View reviewed changes

Comment thread api/v1alpha1/mcpserver_types.go Outdated

Comment thread internal/controller/mcpserver_controller.go Outdated

Comment thread internal/controller/mcpserver_controller.go Outdated

This was referenced Mar 16, 2026

Add emptyDir volume support for writable scratch space #43

Closed

Add configurable replicas to MCPServer spec #44

Merged

k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Mar 16, 2026

everettraven reviewed Mar 16, 2026

View reviewed changes

ArangoGutierrez requested changes Mar 17, 2026

View reviewed changes

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 17, 2026

aliok force-pushed the 2026-03-10-crd-logical-grouping-impl branch from a85daf0 to 3fd569b Compare March 17, 2026 11:38

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 17, 2026

everettraven reviewed Mar 17, 2026

View reviewed changes

Comment thread api/v1alpha1/mcpserver_types.go

Improve documentation for mount permissions and MCPServer conditions

c091b63

Signed-off-by: Ali Ok <aliok@redhat.com>

matzew reviewed Mar 18, 2026

View reviewed changes

Comment thread examples/everything-mcp-server/mcpserver.yaml

matzew reviewed Mar 18, 2026

View reviewed changes

Comment thread api/v1alpha1/mcpserver_types.go

aliok added 2 commits March 18, 2026 23:39

Refactor MCPServer configuration to remove pointer usage for RuntimeC…

5d4a1a3

…onfig and SecurityConfig Signed-off-by: Ali Ok <aliok@redhat.com>

Add path configuration to MCPServer for improved routing

fa194f2

Signed-off-by: Ali Ok <aliok@redhat.com>

aliok mentioned this pull request Mar 18, 2026

runtime.replicas can be a non-pointer #47

Closed

Allow scaling to zero by updating RuntimeConfig replicas validation

1efbff5

Signed-off-by: Ali Ok <aliok@redhat.com>

aliok commented Mar 18, 2026

View reviewed changes

Comment thread api/v1alpha1/mcpserver_types.go Outdated

aliok added 2 commits March 19, 2026 10:21

Move path field under config

742a4b8

Signed-off-by: Ali Ok <aliok@redhat.com>

Update storage mount validation to require at least one item

4a0cec3

Signed-off-by: Ali Ok <aliok@redhat.com>

Enhance MCPServer argument validation to disallow empty strings and a…

f8b1bc2

…llow empty array Signed-off-by: Ali Ok <aliok@redhat.com>

matzew and others added 3 commits March 19, 2026 16:47

fix(rbac): Fix rbac issue for configmap

f621ada

Signed-off-by: Matthias Wessendorf <mwessend@redhat.com>

fix(sample): Make Kube example actually work

9e9586d

Signed-off-by: Matthias Wessendorf <mwessend@redhat.com>

Merge pull request #1 from matzew/fix_api

3713812

Fix samples and RBAC

matzew approved these changes Mar 23, 2026

View reviewed changes

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 23, 2026

k8s-ci-robot assigned matzew Mar 23, 2026

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 23, 2026

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 23, 2026

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 23, 2026

k8s-ci-robot merged commit 84db7ee into kubernetes-sigs:main Mar 23, 2026
6 checks passed

aliok deleted the 2026-03-10-crd-logical-grouping-impl branch March 23, 2026 11:04

Conversation

aliok commented Mar 16, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ArangoGutierrez left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mrunalp commented Mar 16, 2026

Uh oh!

aliok commented Mar 16, 2026

Uh oh!

everettraven left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aliok commented Mar 17, 2026

Uh oh!

ArangoGutierrez left a comment

Choose a reason for hiding this comment

Uh oh!

aliok commented Mar 17, 2026

Uh oh!

aliok commented Mar 17, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

matzew commented Mar 19, 2026

Uh oh!

aliok commented Mar 19, 2026

Uh oh!

matzew commented Mar 19, 2026

Uh oh!

jaideepr97 commented Mar 19, 2026

Uh oh!

matzew commented Mar 19, 2026

Uh oh!

matzew left a comment

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Mar 23, 2026

Uh oh!

aliok commented Mar 23, 2026

Uh oh!

aliok commented Mar 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!