Skip to content

Adding noop-with-token to allow k8 authentication for MCP servers#1043

Merged
tisnik merged 1 commit intolightspeed-core:mainfrom
blublinsky:noop-with-token
Jan 23, 2026
Merged

Adding noop-with-token to allow k8 authentication for MCP servers#1043
tisnik merged 1 commit intolightspeed-core:mainfrom
blublinsky:noop-with-token

Conversation

@blublinsky
Copy link
Contributor

@blublinsky blublinsky commented Jan 22, 2026 •
edited by coderabbitai bot
Loading

Description

Type of change

  • Refactor
  • New feature
  • Bug fix
  • CVE fix
  • Optimization
  • Documentation Update
  • Configuration Update
  • Bump-up service version
  • Bump-up dependent library
  • Bump-up library or tool used for development (does not change the final image)
  • CI configuration change
  • Konflux configuration change
  • Unit tests improvement
  • Integration tests improvement
  • End to end tests improvement

Tools used to create PR

Identify any AI code assistants used in this PR (for transparency and review context)

  • Assisted-by: (e.g., Claude, CodeRabbit, Ollama, etc., N/A if not used)
  • Generated by: (e.g., tool name and version; N/A if not used)

Related Tickets & Documents

  • Related Issue #
  • Closes #

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

  • Please provide detailed steps to perform tests related to this code change.
  • How were the fix/results from this change verified? Please provide relevant screenshots or results.

Summary by CodeRabbit

  • Bug Fixes

    • Expanded MCP authorization header validation to support "noop-with-token" authentication module for Kubernetes-specific headers, in addition to "k8s" module.
    • Improved validation messages for clarity on accepted authentication module and header value pairings.

  • Documentation

    • Added clarification that "noop-with-token" is intended for testing and development; Kubernetes tokens must be provided in headers when using Kubernetes authorization headers.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 22, 2026 •
edited
Loading

Walkthrough

This PR refactors MCP authentication-related code by extracting hard-coded string literals ("kubernetes" and "client") into constants, and updates validation logic to recognize the "noop-with-token" authentication module alongside "k8s" for Kubernetes headers.

Changes

Cohort / File(s) Summary
Constants Definition
src/constants.py		 Added two new public constants: MCP_AUTH_KUBERNETES = "kubernetes" and MCP_AUTH_CLIENT = "client"
MCP Auth Header Utilities
src/utils/mcp_auth_headers.py		 Replaced hard-coded string checks for "kubernetes" and "client" with constant references
Endpoint Logic
src/app/endpoints/mcp_auth.py, src/app/endpoints/query_v2.py		 Updated MCP header-value checks to use constants.MCP_AUTH_KUBERNETES and constants.MCP_AUTH_CLIENT instead of string literals
MCP Authorization Validation
src/models/config.py		 Expanded validation to accept "noop-with-token" as an allowed authentication module for Kubernetes-header authorization, alongside existing "k8s" support; updated error/warning messages and added explanatory note for development/testing context
Type Annotation
tests/e2e/mock_jwks_server/server.py		 Added Any type hint to *args parameter in log_message method signature

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

enhancement, ok-to-test

Suggested reviewers

  • tisnik
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately describes the main change: adding noop-with-token support to enable Kubernetes authentication for MCP servers, which is reflected in the code changes to constants, config validation, and auth headers.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

jrobertboos
jrobertboos reviewed Jan 22, 2026
View reviewed changes
Copy link
Contributor

@jrobertboos jrobertboos left a comment

Choose a reason for hiding this comment

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

LGTM. I like replacing everything with constants :)

tisnik
tisnik approved these changes Jan 23, 2026
View reviewed changes
Copy link
Contributor

@tisnik tisnik left a comment

Choose a reason for hiding this comment

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

LGTM

@tisnik tisnik merged commit a743270 into lightspeed-core:main Jan 23, 2026
20 of 21 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Jan 30, 2026
Open
18 tasks
@coderabbitai coderabbitai bot mentioned this pull request Feb 12, 2026
Open
19 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tisnik tisnik tisnik approved these changes

+1 more reviewer

@jrobertboos jrobertboos jrobertboos left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@blublinsky @tisnik @jrobertboos