Skip to content

Commit bfaed32

Browse files
authored
Merge branch 'main' into fix/azure-openai-config-params
2 parents 1a1cfed + 7fb3aa6 commit bfaed32

14 files changed

Lines changed: 753 additions & 14 deletions

go/.dockerignore

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
# Build artifacts
22
./bin/
33
bin/
4+
.cache/
5+
**/.cache/
46

57
# Test files
68
*_test.go
@@ -45,4 +47,4 @@ PROJECT
4547
hack/
4648

4749
# Makefiles
48-
Makefile
50+
Makefile

go/core/internal/controller/reconciler/reconciler.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1127,8 +1127,9 @@ func (a *kagentReconciler) createMcpTransport(ctx context.Context, s *v1alpha2.R
11271127
}, nil
11281128
default:
11291129
return &mcp.StreamableClientTransport{
1130-
Endpoint: endpoint,
1131-
HTTPClient: httpClient,
1130+
Endpoint: endpoint,
1131+
HTTPClient: httpClient,
1132+
DisableStandaloneSSE: true,
11321133
}, nil
11331134
}
11341135
}
@@ -1187,7 +1188,11 @@ func (a *kagentReconciler) buildRemoteMCPServerTLSConfig(ctx context.Context, s
11871188
// so we need to create a custom HTTP client that adds headers to all
11881189
// requests. When tlsConfig is non-nil it's installed on a cloned
11891190
// transport so tool discovery honors RemoteMCPServer.spec.tls.
1190-
func newHTTPClient(headers map[string]string, timeout time.Duration, tlsConfig *tls.Config) *http.Client {
1191+
func newHTTPClient(
1192+
headers map[string]string,
1193+
timeout time.Duration,
1194+
tlsConfig *tls.Config,
1195+
) *http.Client {
11911196
var base = http.DefaultTransport
11921197
if tlsConfig != nil {
11931198
// Clone the default transport to preserve its dial/keepalive

go/core/test/e2e/remotemcpserver_tls_test.go

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"encoding/json"
2727
"encoding/pem"
2828
"fmt"
29+
"io"
2930
"math/big"
3031
"net"
3132
"net/http"
@@ -544,6 +545,76 @@ func TestE2E_API_ToolServerCompanionSecrets(t *testing.T) {
544545
assert.Equal(t, v1alpha2.GroupVersion.Identifier(), or.APIVersion)
545546
}
546547

548+
// TestE2E_RemoteMCPServer_STREAMABLE_HTTP_WITH_STANDALONE_SSE reproduces
549+
// github.com/kagent-dev/kagent/issues/1955: a Streamable HTTP server that
550+
// holds the GET (standalone SSE) channel open without responding causes the
551+
// reconciler's tool discovery to fail when http.Client.Timeout fires and
552+
// marks the connection failed before tools/list can complete.
553+
func TestE2E_RemoteMCPServer_STREAMABLE_HTTP_WITH_STANDALONE_SSE(t *testing.T) {
554+
cli := setupK8sClient(t, false)
555+
mcp := setupMockMCP(t, false, mockmcp.Options{})
556+
557+
// mockmcp.Start() returns the bind address (e.g. http://[::]:PORT) which
558+
// is not dialable from the proxy process itself. Extract the port and
559+
// forward to 127.0.0.1 so the proxy can reach mockmcp locally.
560+
_, mcpPort, err := net.SplitHostPort(mcp.server.Addr().String())
561+
require.NoError(t, err)
562+
localMCPURL := "http://127.0.0.1:" + mcpPort + mockmcp.MCPPath
563+
564+
// Start a server that accepts GET but never responds — simulating a
565+
// spec-compliant MCP server that holds the standalone SSE channel open
566+
// without sending events. POST requests are forwarded to mockmcp so
567+
// initialize/tools-list work normally.
568+
ln, err := net.Listen("tcp", ":0")
569+
require.NoError(t, err)
570+
hangingServer := &http.Server{
571+
Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
572+
if r.Method == http.MethodGet {
573+
// Block without writing anything back. client.Do in connectSSE
574+
// blocks here until http.Client.Timeout fires, marking the
575+
// connection failed and causing tools/list to fail.
576+
<-r.Context().Done()
577+
return
578+
}
579+
proxyReq, err := http.NewRequestWithContext(r.Context(), r.Method, localMCPURL, r.Body)
580+
if err != nil {
581+
http.Error(w, err.Error(), http.StatusBadGateway)
582+
return
583+
}
584+
proxyReq.Header = r.Header.Clone()
585+
resp, err := http.DefaultTransport.RoundTrip(proxyReq)
586+
if err != nil {
587+
http.Error(w, err.Error(), http.StatusBadGateway)
588+
return
589+
}
590+
defer resp.Body.Close()
591+
for k, vs := range resp.Header {
592+
for _, v := range vs {
593+
w.Header().Add(k, v)
594+
}
595+
}
596+
w.WriteHeader(resp.StatusCode)
597+
io.Copy(w, resp.Body)
598+
}),
599+
}
600+
go hangingServer.Serve(ln)
601+
t.Cleanup(func() {
602+
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
603+
defer cancel()
604+
_ = hangingServer.Shutdown(ctx)
605+
})
606+
607+
timeout := metav1.Duration{Duration: 5 * time.Second}
608+
rms := createRMS(t, cli, v1alpha2.RemoteMCPServerSpec{
609+
URL: buildK8sURL("http://" + ln.Addr().String()),
610+
Protocol: v1alpha2.RemoteMCPServerProtocolStreamableHttp,
611+
Timeout: &timeout,
612+
})
613+
614+
assert.NotEmpty(t, rms.Status.DiscoveredTools,
615+
"tool discovery must succeed even when the server holds the GET SSE channel open")
616+
}
617+
547618
// recordedPaths is a small helper for assertion failure messages — turns
548619
// the request recorder's snapshot into a human-readable list of paths
549620
// when an expected request doesn't show up.

helm/kagent/templates/_helpers.tpl

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,33 @@ Check if leader election should be enabled (more than 1 replica)
143143
{{- gt (.Values.controller.replicas | int) 1 -}}
144144
{{- end -}}
145145

146+
{{/*
147+
Extract the TCP port from controller.metrics.bindAddress.
148+
149+
Anchors the digit run to the end of the string so every Go-style
150+
address form the controller binary accepts is handled correctly: bare
151+
":port", host-qualified "host:port", and bracketed IPv6 "[::1]:port"
152+
all yield the trailing port. Returns "0" or "" when the binary's
153+
disable sentinel is in use; callers must consult
154+
`kagent.controller.metricsEnabled` before rendering manifests.
155+
*/}}
156+
{{- define "kagent.controller.metricsPort" -}}
157+
{{- regexFind "[0-9]+$" (.Values.controller.metrics.bindAddress | toString) -}}
158+
{{- end -}}
159+
160+
{{/*
161+
Returns "1" when the controller metrics resources (Service, RBAC,
162+
container port, env vars) should render, empty otherwise. Honours both
163+
disable signals: `controller.metrics.enabled=false` and the binary's
164+
own `--metrics-bind-address=0` sentinel reached through `bindAddress`.
165+
The two are equivalent so the field name keeps faith with the binary's
166+
documented contract (see go/core/pkg/app/app.go).
167+
*/}}
168+
{{- define "kagent.controller.metricsEnabled" -}}
169+
{{- $port := include "kagent.controller.metricsPort" . -}}
170+
{{- if and .Values.controller.metrics.enabled $port (ne $port "0") -}}1{{- end -}}
171+
{{- end -}}
172+
146173
{{/*
147174
PostgreSQL service name for the bundled postgres instance
148175
*/}}

helm/kagent/templates/controller-deployment.yaml

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,12 @@ spec:
8484
{{- else }}
8585
{{ fail "No database connection configured. Set database.postgres.url, database.postgres.urlFile, or enable database.postgres.bundled." }}
8686
{{- end }}
87+
{{- if include "kagent.controller.metricsEnabled" . }}
88+
- name: METRICS_BIND_ADDRESS
89+
value: {{ .Values.controller.metrics.bindAddress | quote }}
90+
- name: METRICS_SECURE
91+
value: {{ .Values.controller.metrics.secureServing | quote }}
92+
{{- end }}
8793
{{- with .Values.controller.env }}
8894
{{- toYaml . | nindent 12 }}
8995
{{- end }}
@@ -133,6 +139,11 @@ spec:
133139
- name: http
134140
containerPort: {{ .Values.controller.service.ports.targetPort }}
135141
protocol: TCP
142+
{{- if .Values.controller.metrics.enabled }}
143+
- name: metrics
144+
containerPort: {{ include "kagent.controller.metricsPort" . | int }}
145+
protocol: TCP
146+
{{- end }}
136147
resources:
137148
{{- toYaml .Values.controller.resources | nindent 12 }}
138149
{{- with (.Values.controller.securityContext | default .Values.securityContext) }}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
{{- if include "kagent.controller.metricsEnabled" . }}
2+
apiVersion: v1
3+
kind: Service
4+
metadata:
5+
name: {{ include "kagent.fullname" . }}-controller-metrics
6+
namespace: {{ include "kagent.namespace" . }}
7+
labels:
8+
{{- include "kagent.controller.labels" . | nindent 4 }}
9+
spec:
10+
type: {{ .Values.controller.metrics.service.type }}
11+
ports:
12+
- name: {{ ternary "https" "http-metrics" .Values.controller.metrics.secureServing }}
13+
port: {{ .Values.controller.metrics.service.port }}
14+
targetPort: {{ include "kagent.controller.metricsPort" . | int }}
15+
protocol: TCP
16+
selector:
17+
{{- include "kagent.controller.selectorLabels" . | nindent 4 }}
18+
{{- end }}
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
{{- if and (include "kagent.controller.metricsEnabled" .) .Values.controller.metrics.secureServing }}
2+
apiVersion: rbac.authorization.k8s.io/v1
3+
kind: ClusterRole
4+
metadata:
5+
name: {{ include "kagent.fullname" . }}-metrics-auth-role
6+
labels:
7+
{{- include "kagent.controller.labels" . | nindent 4 }}
8+
rules:
9+
- apiGroups:
10+
- authentication.k8s.io
11+
resources:
12+
- tokenreviews
13+
verbs:
14+
- create
15+
- apiGroups:
16+
- authorization.k8s.io
17+
resources:
18+
- subjectaccessreviews
19+
verbs:
20+
- create
21+
{{- end }}
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
{{- if and (include "kagent.controller.metricsEnabled" .) .Values.controller.metrics.secureServing }}
2+
apiVersion: rbac.authorization.k8s.io/v1
3+
kind: ClusterRoleBinding
4+
metadata:
5+
name: {{ include "kagent.fullname" . }}-metrics-auth-rolebinding
6+
labels:
7+
{{- include "kagent.controller.labels" . | nindent 4 }}
8+
roleRef:
9+
apiGroup: rbac.authorization.k8s.io
10+
kind: ClusterRole
11+
name: {{ include "kagent.fullname" . }}-metrics-auth-role
12+
subjects:
13+
- kind: ServiceAccount
14+
name: {{ include "kagent.fullname" . }}-controller
15+
namespace: {{ include "kagent.namespace" . }}
16+
{{- end }}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
{{- if include "kagent.controller.metricsEnabled" . }}
2+
apiVersion: rbac.authorization.k8s.io/v1
3+
kind: ClusterRole
4+
metadata:
5+
name: {{ include "kagent.fullname" . }}-metrics-reader
6+
labels:
7+
{{- include "kagent.controller.labels" . | nindent 4 }}
8+
rules:
9+
- nonResourceURLs:
10+
- "/metrics"
11+
verbs:
12+
- get
13+
{{- end }}

0 commit comments

Comments
 (0)