Skip to content

Commit 0f263f6

Browse files
iplay88keysiplay88keys
committed
Fix ci issues
Signed-off-by: Jeremy Alvis <jeremy.alvis@solo.io>
1 parent 0a5f8bf commit 0f263f6

10 files changed

Lines changed: 172 additions & 55 deletions

File tree

‎go/api/config/crd/bases/kagent.dev_modelconfigs.yaml‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -652,17 +652,18 @@ spec:
652652
properties:
653653
caCertSecretKey:
654654
description: |-
655-
CACertSecretKey is the key within the Secret that contains the CA certificate data.
656-
This field follows the same pattern as APIKeySecretKey.
657-
Required when CACertSecretRef is set (unless DisableVerify is true).
655+
CACertSecretKey is the key within the Secret that contains the
656+
CA certificate data (PEM-encoded). Required when CACertSecretRef
657+
is set (unless DisableVerify is true).
658658
type: string
659659
caCertSecretRef:
660660
description: |-
661661
CACertSecretRef is a reference to a Kubernetes Secret containing
662662
CA certificate(s) in PEM format. The Secret must be in the same
663-
namespace as the ModelConfig.
664-
When set, the certificate will be used to verify the provider's SSL certificate.
665-
This field follows the same pattern as APIKeySecret.
663+
namespace as the resource referencing it (ModelConfig,
664+
RemoteMCPServer, or any future consumer of TLSConfig).
665+
When set, the certificate will be used to verify the upstream's
666+
SSL certificate.
666667
type: string
667668
disableSystemCAs:
668669
default: false

‎go/api/config/crd/bases/kagent.dev_remotemcpservers.yaml‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -192,17 +192,18 @@ spec:
192192
properties:
193193
caCertSecretKey:
194194
description: |-
195-
CACertSecretKey is the key within the Secret that contains the CA certificate data.
196-
This field follows the same pattern as APIKeySecretKey.
197-
Required when CACertSecretRef is set (unless DisableVerify is true).
195+
CACertSecretKey is the key within the Secret that contains the
196+
CA certificate data (PEM-encoded). Required when CACertSecretRef
197+
is set (unless DisableVerify is true).
198198
type: string
199199
caCertSecretRef:
200200
description: |-
201201
CACertSecretRef is a reference to a Kubernetes Secret containing
202202
CA certificate(s) in PEM format. The Secret must be in the same
203-
namespace as the ModelConfig.
204-
When set, the certificate will be used to verify the provider's SSL certificate.
205-
This field follows the same pattern as APIKeySecret.
203+
namespace as the resource referencing it (ModelConfig,
204+
RemoteMCPServer, or any future consumer of TLSConfig).
205+
When set, the certificate will be used to verify the upstream's
206+
SSL certificate.
206207
type: string
207208
disableSystemCAs:
208209
default: false

‎go/api/v1alpha2/modelconfig_types.go‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -294,15 +294,16 @@ type TLSConfig struct {
294294

295295
// CACertSecretRef is a reference to a Kubernetes Secret containing
296296
// CA certificate(s) in PEM format. The Secret must be in the same
297-
// namespace as the ModelConfig.
298-
// When set, the certificate will be used to verify the provider's SSL certificate.
299-
// This field follows the same pattern as APIKeySecret.
297+
// namespace as the resource referencing it (ModelConfig,
298+
// RemoteMCPServer, or any future consumer of TLSConfig).
299+
// When set, the certificate will be used to verify the upstream's
300+
// SSL certificate.
300301
// +optional
301302
CACertSecretRef string `json:"caCertSecretRef,omitempty"`
302303

303-
// CACertSecretKey is the key within the Secret that contains the CA certificate data.
304-
// This field follows the same pattern as APIKeySecretKey.
305-
// Required when CACertSecretRef is set (unless DisableVerify is true).
304+
// CACertSecretKey is the key within the Secret that contains the
305+
// CA certificate data (PEM-encoded). Required when CACertSecretRef
306+
// is set (unless DisableVerify is true).
306307
// +optional
307308
CACertSecretKey string `json:"caCertSecretKey,omitempty"`
308309

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1130,7 +1130,7 @@ func (a *kagentReconciler) buildRemoteMCPServerTLSConfig(ctx context.Context, s
11301130
// requests. When tlsConfig is non-nil it's installed on a cloned
11311131
// transport so tool discovery honors RemoteMCPServer.spec.tls.
11321132
func newHTTPClient(headers map[string]string, timeout time.Duration, tlsConfig *tls.Config) *http.Client {
1133-
var base http.RoundTripper = http.DefaultTransport
1133+
var base = http.DefaultTransport
11341134
if tlsConfig != nil {
11351135
// Clone the default transport to preserve its dial/keepalive
11361136
// settings (proxies, dual-stack, HTTP/2) and override only the

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

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -415,7 +415,7 @@ func (a *adkApiTranslator) translateModel(ctx context.Context, namespace, modelC
415415
},
416416
}
417417
// Populate TLS fields in BaseModel
418-
openai.BaseModel.TLSInsecureSkipVerify, openai.BaseModel.TLSCACertPath, openai.BaseModel.TLSDisableSystemCAs = deriveTLSFields(model.Spec.TLS)
418+
openai.TLSInsecureSkipVerify, openai.TLSCACertPath, openai.TLSDisableSystemCAs = deriveTLSFields(model.Spec.TLS)
419419
// Populate TokenExchange fields (OpenAI-specific)
420420
addTokenExchangeConfiguration(openai, modelDeploymentData, &model.Spec)
421421
openai.APIKeyPassthrough = model.Spec.APIKeyPassthrough
@@ -473,7 +473,7 @@ func (a *adkApiTranslator) translateModel(ctx context.Context, namespace, modelC
473473
},
474474
}
475475
// Populate TLS fields in BaseModel
476-
anthropic.BaseModel.TLSInsecureSkipVerify, anthropic.BaseModel.TLSCACertPath, anthropic.BaseModel.TLSDisableSystemCAs = deriveTLSFields(model.Spec.TLS)
476+
anthropic.TLSInsecureSkipVerify, anthropic.TLSCACertPath, anthropic.TLSDisableSystemCAs = deriveTLSFields(model.Spec.TLS)
477477
anthropic.APIKeyPassthrough = model.Spec.APIKeyPassthrough
478478

479479
if model.Spec.Anthropic != nil {
@@ -522,7 +522,7 @@ func (a *adkApiTranslator) translateModel(ctx context.Context, namespace, modelC
522522
},
523523
}
524524
// Populate TLS fields in BaseModel
525-
azureOpenAI.BaseModel.TLSInsecureSkipVerify, azureOpenAI.BaseModel.TLSCACertPath, azureOpenAI.BaseModel.TLSDisableSystemCAs = deriveTLSFields(model.Spec.TLS)
525+
azureOpenAI.TLSInsecureSkipVerify, azureOpenAI.TLSCACertPath, azureOpenAI.TLSDisableSystemCAs = deriveTLSFields(model.Spec.TLS)
526526
azureOpenAI.APIKeyPassthrough = model.Spec.APIKeyPassthrough
527527

528528
return azureOpenAI, modelDeploymentData, secretHashBytes, nil
@@ -567,7 +567,7 @@ func (a *adkApiTranslator) translateModel(ctx context.Context, namespace, modelC
567567
},
568568
}
569569
// Populate TLS fields in BaseModel
570-
gemini.BaseModel.TLSInsecureSkipVerify, gemini.BaseModel.TLSCACertPath, gemini.BaseModel.TLSDisableSystemCAs = deriveTLSFields(model.Spec.TLS)
570+
gemini.TLSInsecureSkipVerify, gemini.TLSCACertPath, gemini.TLSDisableSystemCAs = deriveTLSFields(model.Spec.TLS)
571571
gemini.APIKeyPassthrough = model.Spec.APIKeyPassthrough
572572

573573
return gemini, modelDeploymentData, secretHashBytes, nil
@@ -608,7 +608,7 @@ func (a *adkApiTranslator) translateModel(ctx context.Context, namespace, modelC
608608
},
609609
}
610610
// Populate TLS fields in BaseModel
611-
anthropic.BaseModel.TLSInsecureSkipVerify, anthropic.BaseModel.TLSCACertPath, anthropic.BaseModel.TLSDisableSystemCAs = deriveTLSFields(model.Spec.TLS)
611+
anthropic.TLSInsecureSkipVerify, anthropic.TLSCACertPath, anthropic.TLSDisableSystemCAs = deriveTLSFields(model.Spec.TLS)
612612
anthropic.APIKeyPassthrough = model.Spec.APIKeyPassthrough
613613

614614
return anthropic, modelDeploymentData, secretHashBytes, nil
@@ -632,7 +632,7 @@ func (a *adkApiTranslator) translateModel(ctx context.Context, namespace, modelC
632632
Options: model.Spec.Ollama.Options,
633633
}
634634
// Populate TLS fields in BaseModel
635-
ollama.BaseModel.TLSInsecureSkipVerify, ollama.BaseModel.TLSCACertPath, ollama.BaseModel.TLSDisableSystemCAs = deriveTLSFields(model.Spec.TLS)
635+
ollama.TLSInsecureSkipVerify, ollama.TLSCACertPath, ollama.TLSDisableSystemCAs = deriveTLSFields(model.Spec.TLS)
636636
ollama.APIKeyPassthrough = model.Spec.APIKeyPassthrough
637637

638638
return ollama, modelDeploymentData, secretHashBytes, nil
@@ -655,7 +655,7 @@ func (a *adkApiTranslator) translateModel(ctx context.Context, namespace, modelC
655655
},
656656
}
657657
// Populate TLS fields in BaseModel
658-
gemini.BaseModel.TLSInsecureSkipVerify, gemini.BaseModel.TLSCACertPath, gemini.BaseModel.TLSDisableSystemCAs = deriveTLSFields(model.Spec.TLS)
658+
gemini.TLSInsecureSkipVerify, gemini.TLSCACertPath, gemini.TLSDisableSystemCAs = deriveTLSFields(model.Spec.TLS)
659659
return gemini, modelDeploymentData, secretHashBytes, nil
660660
case v1alpha2.ModelProviderBedrock:
661661
if model.Spec.Bedrock == nil {
@@ -743,7 +743,7 @@ func (a *adkApiTranslator) translateModel(ctx context.Context, namespace, modelC
743743
}
744744

745745
// Populate TLS fields in BaseModel
746-
bedrock.BaseModel.TLSInsecureSkipVerify, bedrock.BaseModel.TLSCACertPath, bedrock.BaseModel.TLSDisableSystemCAs = deriveTLSFields(model.Spec.TLS)
746+
bedrock.TLSInsecureSkipVerify, bedrock.TLSCACertPath, bedrock.TLSDisableSystemCAs = deriveTLSFields(model.Spec.TLS)
747747
bedrock.APIKeyPassthrough = model.Spec.APIKeyPassthrough
748748

749749
return bedrock, modelDeploymentData, secretHashBytes, nil
@@ -792,7 +792,7 @@ func (a *adkApiTranslator) translateModel(ctx context.Context, namespace, modelC
792792
AuthUrl: model.Spec.SAPAICore.AuthURL,
793793
}
794794

795-
sapAICore.BaseModel.TLSInsecureSkipVerify, sapAICore.BaseModel.TLSCACertPath, sapAICore.BaseModel.TLSDisableSystemCAs = deriveTLSFields(model.Spec.TLS)
795+
sapAICore.TLSInsecureSkipVerify, sapAICore.TLSCACertPath, sapAICore.TLSDisableSystemCAs = deriveTLSFields(model.Spec.TLS)
796796
sapAICore.APIKeyPassthrough = model.Spec.APIKeyPassthrough
797797

798798
return sapAICore, modelDeploymentData, secretHashBytes, nil

‎go/core/internal/httpserver/handlers/toolservers.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,10 @@ func (h *ToolServersHandler) handleCreateRemoteMCPServer(w ErrorResponseWriter,
170170
"toolServerName", toolRef.Name,
171171
"toolServerNamespace", toolRef.Namespace,
172172
)
173+
if err := Check(h.Authorizer, r, auth.Resource{Type: "ToolServer", Name: toolRef.String()}); err != nil {
174+
w.RespondWithError(err)
175+
return
176+
}
173177

174178
if err := h.KubeClient.Create(r.Context(), toolServerRequest); err != nil {
175179
w.RespondWithError(errors.NewInternalServerError("Failed to create RemoteMCPServer in Kubernetes", err))

‎go/core/internal/httpserver/handlers/toolservers_test.go‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,22 @@ import (
2727
"github.com/kagent-dev/kagent/go/core/internal/httpserver/auth"
2828
"github.com/kagent-dev/kagent/go/core/internal/httpserver/handlers"
2929
common "github.com/kagent-dev/kagent/go/core/internal/utils"
30+
pkgauth "github.com/kagent-dev/kagent/go/core/pkg/auth"
3031
"github.com/kagent-dev/kmcp/api/v1alpha1"
3132
)
3233

34+
// denyAuthorizer satisfies pkgauth.Authorizer by refusing every Check.
35+
// Used to pin the authorization gate on the create endpoints: a request
36+
// from an unauthorized caller must surface a 403 BEFORE the handler
37+
// reaches KubeClient.Create or createOrUpdateCompanionSecrets.
38+
type denyAuthorizer struct{}
39+
40+
func (denyAuthorizer) Check(_ context.Context, _ pkgauth.Principal, _ pkgauth.Verb, _ pkgauth.Resource) error {
41+
return assert.AnError
42+
}
43+
44+
var _ pkgauth.Authorizer = denyAuthorizer{}
45+
3346
func TestToolServersHandler(t *testing.T) {
3447
scheme := runtime.NewScheme()
3548

@@ -564,6 +577,83 @@ func TestToolServersHandler(t *testing.T) {
564577
assert.Equal(t, []byte("OLD"), fresh.Data["ca.crt"])
565578
})
566579

580+
// AuthorizationRequired_RemoteMCPServer pins the authz gate on the
581+
// RMS create path. A caller the authorizer rejects must get a 403
582+
// AND no RMS, no companion Secret should land in the cluster.
583+
t.Run("AuthorizationRequired_RemoteMCPServer", func(t *testing.T) {
584+
handler, kubeClient, _, responseRecorder := setupHandler(t)
585+
// Swap the Noop authorizer for one that denies every Check.
586+
handler.Authorizer = denyAuthorizer{}
587+
588+
reqBody := &handlers.ToolServerCreateRequest{
589+
Type: "RemoteMCPServer",
590+
RemoteMCPServer: &v1alpha2.RemoteMCPServer{
591+
ObjectMeta: metav1.ObjectMeta{Name: "denied-rms", Namespace: "default"},
592+
Spec: v1alpha2.RemoteMCPServerSpec{
593+
Description: "should not be created",
594+
URL: "https://x/y",
595+
},
596+
},
597+
Secrets: []api.SecretMaterial{
598+
{Name: "denied-rms-ca", Key: "ca.crt", Value: "PEM"},
599+
},
600+
}
601+
jsonBody, _ := json.Marshal(reqBody)
602+
req := httptest.NewRequest("POST", "/api/toolservers/", bytes.NewBuffer(jsonBody))
603+
req.Header.Set("Content-Type", "application/json")
604+
req = setUser(req, "unauthorized-user")
605+
606+
handler.HandleCreateToolServer(responseRecorder, req)
607+
608+
assert.Equal(t, http.StatusForbidden, responseRecorder.Code,
609+
"unauthorized RMS create must surface 403")
610+
// Neither the RMS nor the companion Secret should have been
611+
// created — the authz gate fires before any KubeClient.Create.
612+
rms := &v1alpha2.RemoteMCPServer{}
613+
err := kubeClient.Get(context.Background(),
614+
ctrl_client.ObjectKey{Namespace: "default", Name: "denied-rms"}, rms)
615+
assert.Error(t, err, "denied request must not create the RemoteMCPServer")
616+
secret := &corev1.Secret{}
617+
err = kubeClient.Get(context.Background(),
618+
ctrl_client.ObjectKey{Namespace: "default", Name: "denied-rms-ca"}, secret)
619+
assert.Error(t, err, "denied request must not create the companion Secret")
620+
})
621+
622+
// AuthorizationRequired_MCPServer pins the symmetric authz gate
623+
// on the kmcp MCPServer create path, so a regression on either
624+
// branch surfaces in the test suite.
625+
t.Run("AuthorizationRequired_MCPServer", func(t *testing.T) {
626+
handler, kubeClient, _, responseRecorder := setupHandler(t)
627+
handler.Authorizer = denyAuthorizer{}
628+
629+
reqBody := &handlers.ToolServerCreateRequest{
630+
Type: "MCPServer",
631+
MCPServer: &v1alpha1.MCPServer{
632+
ObjectMeta: metav1.ObjectMeta{Name: "denied-mcp", Namespace: "default"},
633+
Spec: v1alpha1.MCPServerSpec{
634+
Deployment: v1alpha1.MCPServerDeployment{
635+
Image: "example/kmcp:latest",
636+
Port: 8080,
637+
Cmd: "/bin/serve",
638+
},
639+
},
640+
},
641+
}
642+
jsonBody, _ := json.Marshal(reqBody)
643+
req := httptest.NewRequest("POST", "/api/toolservers/", bytes.NewBuffer(jsonBody))
644+
req.Header.Set("Content-Type", "application/json")
645+
req = setUser(req, "unauthorized-user")
646+
647+
handler.HandleCreateToolServer(responseRecorder, req)
648+
649+
assert.Equal(t, http.StatusForbidden, responseRecorder.Code,
650+
"unauthorized MCPServer create must surface 403")
651+
mcp := &v1alpha1.MCPServer{}
652+
err := kubeClient.Get(context.Background(),
653+
ctrl_client.ObjectKey{Namespace: "default", Name: "denied-mcp"}, mcp)
654+
assert.Error(t, err, "denied request must not create the MCPServer")
655+
})
656+
567657
t.Run("ToolServerAlreadyExists", func(t *testing.T) {
568658
handler, kubeClient, _, responseRecorder := setupHandler(t)
569659

‎go/core/test/e2e/remotemcpserver_tls_test.go‎

Lines changed: 33 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import (
2929
"math/big"
3030
"net"
3131
"net/http"
32+
"strings"
3233
"testing"
3334
"time"
3435

@@ -52,11 +53,11 @@ import (
5253
// mock LLM.
5354
type mockmcpFixture struct {
5455
server *mockmcp.Server
55-
baseURL string // dial URL from inside the cluster
56-
mcpURL string // dial URL with the /mcp path appended
57-
sseURL string // dial URL with the /sse path appended
58-
caCertPEM []byte // when TLS, the PEM bundle to install as a Secret (nil for plaintext fixtures)
59-
hostBindURL string // local 127.0.0.1 URL for debug logging — NOT the cluster-facing URL
56+
baseURL string // dial URL from inside the cluster
57+
mcpURL string // dial URL with the /mcp path appended
58+
sseURL string // dial URL with the /sse path appended
59+
caCertPEM []byte // when TLS, the PEM bundle to install as a Secret (nil for plaintext fixtures)
60+
hostBindURL string // local 127.0.0.1 URL for debug logging — NOT the cluster-facing URL
6061
}
6162

6263
// setupMockMCP starts a mockmcp fixture. When withTLS is true a fresh
@@ -102,7 +103,12 @@ func setupMockMCP(t *testing.T, withTLS bool, opts mockmcp.Options) *mockmcpFixt
102103
_ = server.Stop(ctx)
103104
})
104105

105-
clusterURL := buildK8sURL(hostURL)
106+
// buildK8sURL hard-codes "http://" — fine for the mockllm path it
107+
// was written for, but mockmcp's TLS scenarios serve HTTPS and the
108+
// scheme must round-trip to the controller's dial config. Reuse
109+
// buildK8sURL to pick the right host (kind gateway IP / docker
110+
// alias), then restore the actual scheme mockmcp listened on.
111+
clusterURL := strings.Replace(buildK8sURL(hostURL), "http://", schemeOf(hostURL)+"://", 1)
106112

107113
return &mockmcpFixture{
108114
server: server,
@@ -114,6 +120,18 @@ func setupMockMCP(t *testing.T, withTLS bool, opts mockmcp.Options) *mockmcpFixt
114120
}
115121
}
116122

123+
// schemeOf returns "http" or "https" depending on the URL's prefix.
124+
// Used by setupMockMCP to preserve mockmcp's actual listening scheme
125+
// when buildK8sURL rewrites the host (which it does by string-edit,
126+
// not by url.Parse, so it can't be asked to preserve the scheme
127+
// itself without changing its signature for one caller).
128+
func schemeOf(rawURL string) string {
129+
if strings.HasPrefix(rawURL, "https://") {
130+
return "https"
131+
}
132+
return "http"
133+
}
134+
117135
// createCASecret writes the CA PEM produced by setupMockMCP into a
118136
// Kubernetes Secret in the kagent namespace under the given name and
119137
// key. Registers for cleanup. Returns the Secret so the caller can
@@ -223,15 +241,15 @@ func generateSelfSignedCert(t *testing.T, dnsNames []string, ips []net.IP) (cert
223241
require.NoError(t, err)
224242

225243
tmpl := &x509.Certificate{
226-
SerialNumber: big.NewInt(time.Now().UnixNano()),
227-
Subject: pkix.Name{CommonName: "kagent-e2e-mockmcp"},
228-
NotBefore: time.Now().Add(-time.Hour),
229-
NotAfter: time.Now().Add(24 * time.Hour),
230-
KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment | x509.KeyUsageCertSign,
231-
ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
232-
DNSNames: dnsNames,
233-
IPAddresses: ips,
234-
IsCA: true,
244+
SerialNumber: big.NewInt(time.Now().UnixNano()),
245+
Subject: pkix.Name{CommonName: "kagent-e2e-mockmcp"},
246+
NotBefore: time.Now().Add(-time.Hour),
247+
NotAfter: time.Now().Add(24 * time.Hour),
248+
KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment | x509.KeyUsageCertSign,
249+
ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
250+
DNSNames: dnsNames,
251+
IPAddresses: ips,
252+
IsCA: true,
235253
BasicConstraintsValid: true,
236254
}
237255
der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key)

‎helm/kagent-crds/templates/kagent.dev_modelconfigs.yaml‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -652,17 +652,18 @@ spec:
652652
properties:
653653
caCertSecretKey:
654654
description: |-
655-
CACertSecretKey is the key within the Secret that contains the CA certificate data.
656-
This field follows the same pattern as APIKeySecretKey.
657-
Required when CACertSecretRef is set (unless DisableVerify is true).
655+
CACertSecretKey is the key within the Secret that contains the
656+
CA certificate data (PEM-encoded). Required when CACertSecretRef
657+
is set (unless DisableVerify is true).
658658
type: string
659659
caCertSecretRef:
660660
description: |-
661661
CACertSecretRef is a reference to a Kubernetes Secret containing
662662
CA certificate(s) in PEM format. The Secret must be in the same
663-
namespace as the ModelConfig.
664-
When set, the certificate will be used to verify the provider's SSL certificate.
665-
This field follows the same pattern as APIKeySecret.
663+
namespace as the resource referencing it (ModelConfig,
664+
RemoteMCPServer, or any future consumer of TLSConfig).
665+
When set, the certificate will be used to verify the upstream's
666+
SSL certificate.
666667
type: string
667668
disableSystemCAs:
668669
default: false

0 commit comments

Comments
 (0)