Add mcpserver tls by EItanya · Pull Request #1424 · kagent-dev/kagent

EItanya · 2026-03-04T18:54:33Z

I tried to deploy an MCPServer. During deployment, the kagent controller attempts to list tools from the MCP backend over HTTPS, but it fails with the following error:

"reconcileID":"c77f3b32-f68a-443d-8a1f-b56a1fe7d3a9","error":"failed to upsert tool server for remote mcp server default/dahu-mcp: failed to fetch tools for toolServer default/dahu-mcp: failed to initialize client for toolServer default/dahu-mcp: transport error: request failed with status 400: Client sent an HTTP request to an HTTPS server.\n"

At the moment, MCPServer seems to only support calling the backend server over plain HTTP.

Therefore, I think we should allow users to configure a Secret reference in the MCPServer CRD for the certificates used to connect to the MCP server (I have already opened PR #114 in the kmcp project). The controller reconciler could then use the client certificate specified in the CRD when list tools. An example is shown below:

apiVersion: kagent.dev/v1alpha1
kind: MCPServer
metadata:
  name: test-mcp
spec:
  transportType: http
  httpTransport:
    targetPort: 8443
    path: /
    tls:
      secretRef: test-tls-client   # Custom client certificate used to access the MCP server

  deployment:
    image: ghcr.io/test/test-mcp:v0.1.0
    port: 8443
    args:
      - --http-addr
      - :8443
      - --tls-cert
      - /etc/tls/tls.crt
      - --tls-key
      - /etc/tls/tls.key

    # Explicitly create the volume
    volumes:
      - name: tls-certs
        secret:
          secretName: test-tls-server
    volumeMounts:
      - name: tls-certs
        mountPath: /etc/tls
        readOnly: true

These initial changes using a local go.mod replace of the kmcp package from #114. The final implementation will depend on the finalized definition in kmcp’s mcpserver_types.

Signed-off-by: dahu.kdh <dahu.kdh@alibaba-inc.com>

- Add missing crypto imports for TLS test functions - Initialize scheme properly in TestCreateTLSHTTPClient - Remove incorrect timeout assertion Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>

- Remove leftover merge conflict markers from types.py - Fix whitespace formatting in cli.py and _openai.py Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>

Copilot

Pull request overview

Adds TLS/mTLS support for MCP server connections (and OpenAI client mTLS), spanning Python runtime TLS plumbing plus Go controller translation/mounting so agent configs can carry TLS settings end-to-end.

Changes:

Add Python TLS utilities for loading client certificates and wiring them into httpx/OpenAI and MCP connections.
Extend Go API/types and translator logic to include MCP server TLS fields and mount client cert Secrets.
Add/adjust unit + integration tests for TLS behaviors across Python and Go.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
python/packages/kagent-adk/src/kagent/adk/types.py	Adds TLS extraction, MCP TLS application via httpx client factory, and MCP session manager patching.
python/packages/kagent-adk/src/kagent/adk/models/_ssl.py	Adds `load_client_certificate()` for mTLS and adjusts SSL context logging.
python/packages/kagent-adk/src/kagent/adk/models/_openai.py	Adds `tls_client_cert_path` support and passes mTLS certs into OpenAI http client.
python/packages/kagent-adk/src/kagent/adk/cli.py	Minor whitespace-only change near config loading.
python/packages/kagent-adk/tests/unittests/types/test_agent_config_tls.py	New tests for TLS field extraction from AgentConfig JSON.
python/packages/kagent-adk/tests/unittests/test_agent_config_tls.py	Additional (largely duplicate) tests for TLS field extraction + zip edge case.
python/packages/kagent-adk/tests/unittests/models/test_tls_integration.py	Extends TLS integration tests to cover client certificate loading and OpenAI mTLS wiring.
python/packages/kagent-adk/tests/unittests/models/test_ssl.py	Adds unit tests for `load_client_certificate()` behavior.
go/pkg/adk/types.go	Adds TLS fields to MCP connection param structs.
go/internal/controller/translator/agent/utils.go	Adds metadata helper functions (HTTP fetch).
go/internal/controller/translator/agent/adk_api_translator.go	Adds MCP server TLS secret mounting and populates TLS fields into generated agent config.
go/internal/controller/translator/agent/proxy_test.go	Adds URL conversion + TLS population tests for MCP servers.
go/internal/controller/translator/agent/convert_mcpserver_test.go	Adds tests for converting MCPServer → RemoteMCPServer spec including TLS.
go/internal/controller/reconciler/reconciler.go	Adds TLS-aware MCP transport creation and Kubernetes Secret-based TLS HTTP client.
go/internal/controller/reconciler/reconciler_test.go	Adds tests for `createTLSHTTPClient`.
go/api/v1alpha2/remotemcpserver_types.go	Adds `spec.tls` to RemoteMCPServerSpec.
go/api/v1alpha2/modelconfig_types.go	Extends TLSConfig with `clientSecretRef`.
go/api/v1alpha2/zz_generated.deepcopy.go	Updates deep-copy generation for added TLS field.

💡 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.

Copilot · 2026-03-04T19:02:02Z

python/packages/kagent-adk/src/kagent/adk/types.py

+            instance.params.tls_client_cert_path = tls_client_cert_path
+            instance.params.insecure_tls_verify = insecure_tls_verify


_extract_and_set_tls_fields() assigns instance.params.tls_client_cert_path / instance.params.insecure_tls_verify directly. For StreamableHTTPConnectionParams/SseConnectionParams these are intentionally not model fields, so direct assignment can raise (and elsewhere in this file you use object.__setattr__ for the same reason). Use object.__setattr__ here too (or remove these overrides if they’re never invoked).

Suggested change

instance.params.tls_client_cert_path = tls_client_cert_path

instance.params.insecure_tls_verify = insecure_tls_verify

object.__setattr__(instance.params, "tls_client_cert_path", tls_client_cert_path)

object.__setattr__(instance.params, "insecure_tls_verify", insecure_tls_verify)

Copilot · 2026-03-04T19:02:02Z

go/internal/controller/reconciler/reconciler.go

+	client := newHTTPClient(headers)
+	serverURL := s.Spec.URL
+	if s.Spec.TLS != nil {
+		// Convert URL to https if TLS is configured and scheme is not explicitly set
+		parsedURL, err := url.Parse(serverURL)
+		if err != nil {
+			return nil, fmt.Errorf("invalid URL: %w", err)
+		}
+		if parsedURL.Scheme == "" || parsedURL.Scheme == "http" {
+			parsedURL.Scheme = "https"
+			serverURL = parsedURL.String()
+		}
+
+		// Create HTTP client with TLS configuration
+		client, err = a.createTLSHTTPClient(ctx, s.Spec.TLS, s.Namespace)
+		if err != nil {
+			return nil, fmt.Errorf("failed to create TLS HTTP client: %w", err)
+		}
+	}


When TLS is configured, createMcpTransport replaces the header-injecting client from newHTTPClient(headers) with the TLS client from createTLSHTTPClient(...), which drops the resolved headers entirely. Consider composing the transports (e.g., wrap the TLS transport with headerTransport, or accept headers in createTLSHTTPClient and install a headerTransport{base: tlsTransport}), so TLS-enabled MCP calls still include the configured headers.

Copilot · 2026-03-04T19:02:03Z

go/internal/controller/translator/agent/adk_api_translator.go

+			certPath := fmt.Sprintf("%s/%s", mountPath, "tls.crt")
+			p.TLSClientCertPath = &certPath
+		}
+	case *adk.SseConnectionParams:
+		if mountPath != "" {
+			certPath := fmt.Sprintf("%s/%s", mountPath, "tls.crt")
+			p.TLSClientCertPath = &certPath
+		}


populateMCPServerTLSFields sets TLSClientCertPath to .../tls.crt, but the Python side load_client_certificate() expects tls_client_cert_path to be a directory containing tls.crt and tls.key (it appends /tls.crt internally). Passing a file path will make it look for .../tls.crt/tls.crt and fail at runtime. Set the param to the directory mount path instead (and consider also mapping tlsConfig.DisableVerify into TLSDisableVerify/insecure_tls_verify).

Suggested change

certPath := fmt.Sprintf("%s/%s", mountPath, "tls.crt")

p.TLSClientCertPath = &certPath

}

case *adk.SseConnectionParams:

if mountPath != "" {

certPath := fmt.Sprintf("%s/%s", mountPath, "tls.crt")

p.TLSClientCertPath = &certPath

}

// The Python side expects this to be a directory containing tls.crt/tls.key

p.TLSClientCertPath = &mountPath

}

// Propagate TLS disable-verify flag if available

disableVerify := tlsConfig.DisableVerify

p.TLSDisableVerify = &disableVerify

case *adk.SseConnectionParams:

if mountPath != "" {

// The Python side expects this to be a directory containing tls.crt/tls.key

p.TLSClientCertPath = &mountPath

}

// Propagate TLS disable-verify flag if available

disableVerify := tlsConfig.DisableVerify

p.TLSDisableVerify = &disableVerify

Copilot · 2026-03-04T19:02:03Z

go/internal/controller/translator/agent/adk_api_translator.go

+	// Use the secret name to create a unique but deterministic volume name
+	// This ensures the same Secret always gets the same volume name, avoiding duplicates
+	volumeName := fmt.Sprintf("%s-%s", mcpServerTLSVolumeName, secretName)
+	mountPath := fmt.Sprintf("%s/%s", mcpServerTLSMountPath, secretName)


addMCPServerTLSConfiguration builds a volume name by concatenating a prefix with the Secret name. Kubernetes volume names are limited to 63 chars (DNS label). A long Secret name can make this exceed the limit and fail Pod creation. Consider truncating + hashing the Secret name portion while keeping it deterministic.

Copilot · 2026-03-04T19:02:03Z

python/packages/kagent-adk/tests/unittests/test_agent_config_tls.py

+"""Unit tests for AgentConfig.model_validate TLS field extraction.
+
+These tests verify that TLS fields (tls_client_cert_path, insecure_tls_verify) are
+correctly extracted from JSON and set on StreamableHTTPConnectionParams objects
+during AgentConfig.model_validate, even though these fields are not part of the
+Pydantic model definition.
+"""


This test module substantially duplicates tests/unittests/types/test_agent_config_tls.py (same scenarios/assertions). Keeping both will run near-identical tests twice and increases maintenance cost. Consider consolidating into a single location/file (and keep only the additional edge-case tests that aren’t already covered).

Copilot · 2026-03-04T19:02:04Z

python/packages/kagent-adk/src/kagent/adk/types.py



+# Flag to track if we've monkey-patched MCPSessionManager
+_MCP_SESSION_MANAGER_PATCHED = False


The file contains unresolved Git merge conflict markers (e.g., =======, >>>>>>> ...). This will make the module invalid Python and break imports/tests. Resolve the conflict and remove the markers before merging.

Copilot · 2026-03-04T19:02:04Z

go/internal/controller/reconciler/reconciler.go

+		// Convert URL to https if TLS is configured and scheme is not explicitly set
+		parsedURL, err := url.Parse(serverURL)
+		if err != nil {
+			return nil, fmt.Errorf("invalid URL: %w", err)
+		}
+		if parsedURL.Scheme == "" || parsedURL.Scheme == "http" {


The url.Parse + parsedURL.Scheme == "" branch can produce invalid URLs when the input is missing a scheme (e.g., parsing kyverno-mcp.default:8000/mcp treats it as a path, and setting Scheme="https" yields https:kyverno-mcp...). If you want to support scheme-less URLs, prepend https:// (or // + host) rather than mutating a parsed path-only URL.

Suggested change

// Convert URL to https if TLS is configured and scheme is not explicitly set

parsedURL, err := url.Parse(serverURL)

if err != nil {

return nil, fmt.Errorf("invalid URL: %w", err)

}

if parsedURL.Scheme == "" || parsedURL.Scheme == "http" {

// Normalize URL for TLS: if no scheme is provided, assume https.

// This avoids parsing a host:port/path value as a path-only URL.

if !strings.Contains(serverURL, "://") {

serverURL = "https://" + serverURL

}

parsedURL, err := url.Parse(serverURL)

if err != nil {

return nil, fmt.Errorf("invalid URL: %w", err)

}

if parsedURL.Scheme == "http" {

Copilot · 2026-03-04T19:02:05Z

go/internal/controller/translator/agent/utils.go

+func GetMetaData(resource string) (string, error) {
+	resp, err := http.Get(METADATA_URL + resource)
+	if err != nil {
+		return "", err
+	}
+	defer resp.Body.Close()
+	body, err := ioutil.ReadAll(resp.Body)
+	if err != nil {
+		return "", err
+	}
+	if resp.StatusCode >= http.StatusBadRequest {
+		return "", fmt.Errorf("get metadata via %s failed (%s): %s",
+			METADATA_URL+resource, resp.Status, string(body))
+	}
+	return string(body), nil


GetMetaData uses http.Get without any timeout/cancellation and reads the entire response body with ioutil.ReadAll (deprecated). This can hang the controller if the metadata endpoint is slow/unreachable. Prefer an http.Client{Timeout: ...} (or a request with context.Context) and io.ReadAll.

Copilot · 2026-03-04T19:02:05Z

python/packages/kagent-adk/src/kagent/adk/cli.py


    with open(os.path.join(filepath, "config.json"), "r") as f:
        config = json.load(f)
+


There is trailing whitespace on this blank line, which will be flagged by Ruff/formatters. Remove the spaces so the line is truly empty.

Copilot · 2026-03-04T19:02:05Z

python/packages/kagent-adk/tests/unittests/types/test_agent_config_tls.py

+"""Unit tests for AgentConfig.model_validate TLS field extraction.
+
+These tests verify that TLS fields (tls_client_cert_path, insecure_tls_verify) are
+correctly extracted from JSON and set on StreamableHTTPConnectionParams objects
+during AgentConfig.model_validate, even though these fields are not part of the
+Pydantic model definition.
+"""


This file appears to duplicate the same TLS extraction tests that also exist at tests/unittests/test_agent_config_tls.py. To avoid duplicated coverage and future drift, consider keeping just one canonical test module and removing the duplicate.

DahuK and others added 2 commits March 4, 2026 18:42

feat: support customized TLS configuration to mcpserver

690b8b6

Signed-off-by: dahu.kdh <dahu.kdh@alibaba-inc.com>

EItanya requested a review from ilackarms as a code owner March 4, 2026 18:54

Copilot AI review requested due to automatic review settings March 4, 2026 18:54

EItanya requested review from peterj and yuval-k as code owners March 4, 2026 18:54

Copilot started reviewing on behalf of EItanya March 4, 2026 18:54 View session

EItanya mentioned this pull request Mar 4, 2026

feat: support customized TLS configuration to mcpserver #1215

Closed

fix: remove merge conflict markers and fix Python formatting

da97686

- Remove leftover merge conflict markers from types.py - Fix whitespace formatting in cli.py and _openai.py Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>

Copilot AI reviewed Mar 4, 2026

View reviewed changes

EItanya closed this Mar 4, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add mcpserver tls#1424

Add mcpserver tls#1424
EItanya wants to merge 3 commits intomainfrom
add-mcpserver-tls

EItanya commented Mar 4, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		instance.params.tls_client_cert_path = tls_client_cert_path
		instance.params.insecure_tls_verify = insecure_tls_verify

-			certPath := fmt.Sprintf("%s/%s", mountPath, "tls.crt")
-			p.TLSClientCertPath = &certPath
-		}
-	case *adk.SseConnectionParams:
-		if mountPath != "" {
-			certPath := fmt.Sprintf("%s/%s", mountPath, "tls.crt")
-			p.TLSClientCertPath = &certPath
-		}
+			// The Python side expects this to be a directory containing tls.crt/tls.key
+			p.TLSClientCertPath = &mountPath
+		}
+		// Propagate TLS disable-verify flag if available
+		disableVerify := tlsConfig.DisableVerify
+		p.TLSDisableVerify = &disableVerify
+	case *adk.SseConnectionParams:
+		if mountPath != "" {
+			// The Python side expects this to be a directory containing tls.crt/tls.key
+			p.TLSClientCertPath = &mountPath
+		}
+		// Propagate TLS disable-verify flag if available
+		disableVerify := tlsConfig.DisableVerify
+		p.TLSDisableVerify = &disableVerify



		# Flag to track if we've monkey-patched MCPSessionManager
		_MCP_SESSION_MANAGER_PATCHED = False


		with open(os.path.join(filepath, "config.json"), "r") as f:
		config = json.load(f)

Conversation

EItanya commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

EItanya commented Mar 4, 2026 •

edited

Loading