fix(mcp): Serve compute-mcp over TLS so the assistant can call it

cmd/compute-mcp/docs.go

@@ -34,7 +34,7 @@ type document struct {
 	contentType string
 }
 
-// knowledgeHandler serves compute's knowledge and skills over plain HTTP.
+// knowledgeHandler serves compute's knowledge and skills to plain GET requests.
 //
 // Routing is an exact-match lookup in a table enumerated at startup, never a
 // path join against a directory, so "..", an absolute path, or an escaped

cmd/compute-mcp/main.go

@@ -8,6 +8,11 @@
 //	GET  /runbooks/<name>.md    Skills: triage procedures
 //	GET  /healthz               liveness
 //
+// Served over https when given a certificate (--tls-cert-file, --tls-key-file),
+// which is how it is deployed: a caller sends a bearer token to /mcp and the
+// assistant will not send one over plain http. Without a certificate it serves
+// plain http, for local development.
+//
 // One process, because compute's capability document names all of those URLs.
 // Only /mcp takes a credential; see docs.go for why the documents do not.
 //
@@ -25,7 +30,6 @@ package main
 
 import (
 	"context"
-	"errors"
 	"flag"
 	"fmt"
 	"net"
@@ -88,17 +92,29 @@ func init() {
 }
 
 func main() {
-	var addr string
+	var (
+		addr  string
+		files tlsFiles
+	)
 
 	flag.StringVar(&addr, "addr", envOr("COMPUTE_MCP_ADDR", ":8080"),
 		"address to serve MCP on")
+	flag.StringVar(&files.certFile, "tls-cert-file", os.Getenv("COMPUTE_MCP_TLS_CERT_FILE"),
+		"serving certificate; set with --tls-key-file to serve https instead of http")
+	flag.StringVar(&files.keyFile, "tls-key-file", os.Getenv("COMPUTE_MCP_TLS_KEY_FILE"),
+		"key for --tls-cert-file")
 
 	opts := zap.Options{Development: true}
 	opts.BindFlags(flag.CommandLine)
 	flag.Parse()
 
 	ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts)))
 
+	if err := files.validate(); err != nil {
+		setupLog.Error(err, "refusing to start")
+		os.Exit(1)
+	}
+
 	// GetConfig resolves the --kubeconfig flag that controller-runtime
 	// registers, then KUBECONFIG, then in-cluster config, then ~/.kube/config.
 	// Only the endpoint and CA are used; see clientForToken.
@@ -113,7 +129,7 @@ func main() {
 		os.Exit(1)
 	}
 
-	if err := run(addr, baseConfig); err != nil {
+	if err := run(ctrl.SetupSignalHandler(), addr, files, baseConfig); err != nil {
 		setupLog.Error(err, "server failed")
 		os.Exit(1)
 	}
@@ -161,7 +177,7 @@ func sameEndpoint(a, b string) bool {
 	return strings.TrimSuffix(a, "/") == strings.TrimSuffix(b, "/")
 }
 
-func run(addr string, baseConfig *rest.Config) error {
+func run(ctx context.Context, addr string, files tlsFiles, baseConfig *rest.Config) error {
 	exec, err := newExecRuntime()
 	if err != nil {
 		return err
@@ -217,17 +233,13 @@ func run(addr string, baseConfig *rest.Config) error {
 		_, _ = fmt.Fprintln(w, "ok")
 	})
 
-	server := &http.Server{
-		Addr:              addr,
-		Handler:           mux,
-		ReadHeaderTimeout: readHeaderTimeout,
+	ln, err := net.Listen("tcp", addr)
+	if err != nil {
+		return fmt.Errorf("listening on %s: %w", addr, err)
 	}
 
-	setupLog.Info("listening", "addr", addr, "mcp", "/mcp", "docs", docs.paths())
-	if err := server.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) {
-		return fmt.Errorf("serving on %s: %w", addr, err)
-	}
-	return nil
+	setupLog.Info("listening", "scheme", files.scheme(), "addr", addr, "mcp", "/mcp", "docs", docs.paths())
+	return serve(ctx, ln, mux, files)
 }
 
 // depsFromRequest returns a DepsFor that resolves the caller's identity and

cmd/compute-mcp/serve.go

@@ -0,0 +1,127 @@
+// SPDX-License-Identifier: AGPL-3.0-only
+
+package main
+
+import (
+	"context"
+	"crypto/tls"
+	"errors"
+	"fmt"
+	"net"
+	"net/http"
+	"time"
+
+	"sigs.k8s.io/controller-runtime/pkg/certwatcher"
+)
+
+const (
+	schemeHTTP  = "http"
+	schemeHTTPS = "https"
+
+	// shutdownTimeout bounds how long in-flight requests get to finish once the
+	// process is asked to stop. It sits inside the pod's termination grace
+	// period, so the kubelet never has to kill a server that is still draining.
+	shutdownTimeout = 20 * time.Second
+)
+
+// tlsFiles names the serving certificate and its key. Both empty means plain
+// http, which is what local development and the e2e environment run.
+//
+// The assistant refuses to send a caller's bearer token over anything but
+// https, so a deployment the assistant calls must set both.
+type tlsFiles struct {
+	certFile string
+	keyFile  string
+}
+
+// validate rejects a half-configured pair. One file without the other is a
+// deployment mistake, and serving plain http in answer to it would put bearer
+// tokens on the wire in the clear while looking healthy.
+func (f tlsFiles) validate() error {
+	switch {
+	case f.certFile != "" && f.keyFile == "":
+		return errors.New("--tls-cert-file is set but --tls-key-file is not: " +
+			"set both to serve https, or neither to serve http")
+	case f.certFile == "" && f.keyFile != "":
+		return errors.New("--tls-key-file is set but --tls-cert-file is not: " +
+			"set both to serve https, or neither to serve http")
+	}
+	return nil
+}
+
+func (f tlsFiles) enabled() bool {
+	return f.certFile != "" && f.keyFile != ""
+}
+
+// scheme is the URL scheme the server answers on.
+func (f tlsFiles) scheme() string {
+	if f.enabled() {
+		return schemeHTTPS
+	}
+	return schemeHTTP
+}
+
+// serve answers on ln until ctx is cancelled, then drains in-flight requests.
+// A clean shutdown returns nil.
+//
+// With a certificate it serves https and nothing else. The certificate is read
+// through a watcher rather than once at startup: the cert-manager CSI driver
+// rewrites the files in place on renewal, and a server that loaded them once
+// would present an expired certificate until its next restart.
+func serve(ctx context.Context, ln net.Listener, handler http.Handler, files tlsFiles) error {
+	if err := files.validate(); err != nil {
+		return err
+	}
+
+	server := &http.Server{
+		Handler:           handler,
+		ReadHeaderTimeout: readHeaderTimeout,
+	}
+
+	// Cancelled on every return path, so the watcher never outlives the server.
+	ctx, cancel := context.WithCancel(ctx)
+	defer cancel()
+
+	// Buffered for both goroutines: whichever loses the race to report must
+	// not block forever on a channel nobody reads any more.
+	failed := make(chan error, 2)
+
+	if files.enabled() {
+		watcher, err := certwatcher.New(files.certFile, files.keyFile)
+		if err != nil {
+			return fmt.Errorf("loading TLS certificate %s and key %s: %w",
+				files.certFile, files.keyFile, err)
+		}
+		server.TLSConfig = &tls.Config{
+			MinVersion:     tls.VersionTLS12,
+			GetCertificate: watcher.GetCertificate,
+		}
+		go func() {
+			if err := watcher.Start(ctx); err != nil {
+				failed <- fmt.Errorf("watching TLS certificate: %w", err)
+			}
+		}()
+		// The certificate comes from GetCertificate, so no files are named here.
+		go func() { failed <- server.ServeTLS(ln, "", "") }()
+	} else {
+		go func() { failed <- server.Serve(ln) }()
+	}
+
+	select {
+	case err := <-failed:
+		_ = server.Close()
+		if errors.Is(err, http.ErrServerClosed) {
+			return nil
+		}
+		return fmt.Errorf("serving %s on %s: %w", files.scheme(), ln.Addr(), err)
+	case <-ctx.Done():
+	}
+
+	// The parent context is already cancelled, so the drain needs its own.
+	shutdownCtx, stop := context.WithTimeout(context.Background(), shutdownTimeout)
+	defer stop()
+	if err := server.Shutdown(shutdownCtx); err != nil {
+		return fmt.Errorf("shutting down: %w", err)
+	}
+	return nil
+}