Skip to content

OLS-2575 Add denied_resources TOML config for openshift-mcp-server sidecar#1418

Merged
openshift-merge-bot[bot] merged 3 commits intoopenshift:mainfrom
onmete:add-ocp-mcp-config
Mar 17, 2026
Merged

OLS-2575 Add denied_resources TOML config for openshift-mcp-server sidecar#1418
openshift-merge-bot[bot] merged 3 commits intoopenshift:mainfrom
onmete:add-ocp-mcp-config

Conversation

@onmete
Copy link
Contributor

@onmete onmete commented Mar 17, 2026

Description

Configure the shipped openshift-mcp-server sidecar to deny access to Kubernetes Secret and RBAC resources at the server level, so sensitive data never reaches the LLM.

The operator now:

  • Generates a TOML config ConfigMap with denied_resources entries for v1/Secret and rbac.authorization.k8s.io/v1
  • Mounts the config into the sidecar container at /etc/mcp-server/config.toml
  • Passes --config flag to the openshift-mcp-server command (preserving --read-only and --port)
  • Manages the ConfigMap lifecycle (create/update/delete based on introspectionEnabled)

This applies only to the shipped openshift-mcp-server sidecar. User-configured MCP servers (spec.mcpServers) are the user's responsibility to secure.

When a denied resource is accessed, the MCP server returns an error resource not allowed: <GVK> — the data never reaches the LLM.

Type of change

  • New feature

Related Tickets & Documents

Checklist before requesting a review

  • I have performed a self-review of my code.
  • PR has passed all pre-merge test jobs.
  • If it is a core feature, I have added thorough tests.

Testing

  1. Unit tests added/updated in:
    • internal/controller/utils/mcp_server_config_test.go — tests TOML content, ConfigMap generation, volume/mount helpers, config path
    • internal/controller/appserver/assets_test.go — verifies --config flag in sidecar command, config volume and mount on deployment
    • internal/controller/lcore/deployment_test.go — verifies --config flag, config volume/mount presence when introspection enabled, absence when disabled
  2. All existing unit tests pass (make test)
  3. Deploy the operator on a cluster with introspectionEnabled: true and verify:
    • The openshift-mcp-server-config ConfigMap exists with correct TOML content
    • The sidecar container has --config /etc/mcp-server/config.toml in its command
    • Requesting Secret resources through the MCP server returns resource not allowed error
    • Requesting non-denied resources (e.g., Pods, Deployments) works normally
    • Toggling introspectionEnabled: false deletes the ConfigMap and removes the sidecar

Made with Cursor

@openshift-ci openshift-ci bot requested review from bparees and joshuawilson March 17, 2026 09:09
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 17, 2026
@onmete onmete changed the title Add denied_resources TOML config for openshift-mcp-server sidecar OLS-2575 Add denied_resources TOML config for openshift-mcp-server sidecar Mar 17, 2026
Configure the shipped openshift-mcp-server sidecar to deny access to
Kubernetes Secret and RBAC resources at the server level, so sensitive
data never reaches the LLM. This replaces the aggressive substring
matching in lightspeed-service that blocked legitimate operations
(e.g., deployments referencing /var/run/secrets/ or secretRef).

The operator now:
- Generates a TOML config ConfigMap with denied_resources for v1/Secret
  and rbac.authorization.k8s.io/v1
- Mounts the config into the sidecar container
- Passes --config flag to the sidecar (preserving --read-only and --port)
- Manages the ConfigMap lifecycle (create/update/delete based on
  introspection enabled/disabled)

This applies only to the shipped openshift-mcp-server sidecar.
User-configured MCP servers (spec.mcpServers) are the user's
responsibility to secure.

Made-with: Cursor
@onmete onmete force-pushed the add-ocp-mcp-config branch from 42ee842 to 191e79a Compare March 17, 2026 09:13
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 17, 2026
if cmExists {
r.GetLogger().Info("deleting MCP server config configmap", "configmap", OpenShiftMCPServerConfigCmName)
if err := r.Delete(ctx, foundCm); err != nil {
return fmt.Errorf("%s: %w", ErrGetMCPServerConfigMap, err)
Copy link
Contributor

Choose a reason for hiding this comment

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

ErrGetMCPServerConfigMap is a wrong error - you are deleting.
Maybe introduce a new constant
ErrDeleteMCPServerConfigMap = "failed to delete MCP server config configmap"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed

@@ -108,14 +112,20 @@ func addOpenShiftMCPServerSidecar(r reconciler.Reconciler, cr *olsv1alpha1.OLSCo
AllowPrivilegeEscalation: &[]bool{false}[0],
ReadOnlyRootFilesystem: &[]bool{true}[0],
},
Copy link
Contributor

Choose a reason for hiding this comment

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

This does not match the changes introduced by app server

Copy link
Contributor

Choose a reason for hiding this comment

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

For this, maybe move a function from the app server to utils and use it in app server, lcore, postgres and console. Can be done in a separate PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed

onmete added 2 commits March 17, 2026 13:12
- Use correct ErrDeleteMCPServerConfigMap constant for delete failures
  (was incorrectly using ErrGetMCPServerConfigMap)
- Properly distinguish NotFound vs other errors from Get, following the
  established codebase pattern (errors.IsNotFound check)
- Remove unused CleanupOpenShiftMCPServerConfigMap (dead code — the
  ConfigMap has an owner reference and is cleaned up by the finalizer's
  generic owner-based deletion)

Made-with: Cursor
Extract the restricted Pod Security profile SecurityContext helper from
appserver into utils so all components use the same full restricted
profile (AllowPrivilegeEscalation, ReadOnlyRootFilesystem, RunAsNonRoot,
SeccompProfile, Capabilities).

Previously lcore, postgres, and console only set two of five fields.
This aligns all operator-managed containers to the same security posture.

Made-with: Cursor
@blublinsky
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 17, 2026
@blublinsky
Copy link
Contributor

/approve

@openshift-ci
Copy link

openshift-ci bot commented Mar 17, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: blublinsky

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:

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 17, 2026
@openshift-ci
Copy link

openshift-ci bot commented Mar 17, 2026

@onmete: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@openshift-merge-bot openshift-merge-bot bot merged commit 25ce8f2 into openshift:main Mar 17, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants