Repository navigation
fix(mcp): Serve alb-mcp and network-mcp over TLS - #901
Merged
Merged
Conversation
alb-mcp and network-mcp answered plain HTTP only, and the assistant now refuses to send a caller's bearer token to an http endpoint, so it drops both tool sets. The servers take --tls-cert-file and --tls-key-file (or ALB_MCP_TLS_* and NETWORK_MCP_TLS_*) and serve HTTPS with a watched certificate, so a renewal is picked up without a restart. With neither set they serve plain HTTP as before, and with only one they refuse to start.
The two MCP deployments served plain HTTP and their capability documents advertised http URLs, which the assistant no longer forwards a caller's token to. Each deployment now mounts a datum-control-plane certificate through the cert-manager CSI driver, passes it to the server and probes over HTTPS, and every endpoint, knowledge and runbook URL is https.
ecv
approved these changes
Nov 11, 2026
ecv
left a comment
ecv
left a comment
Contributor
There was a problem hiding this comment.
Approve.
alb-mcp and network-mcp both serve https through one shared mcpserver.TLSOptions, and the cert names match the prefixed Service hosts the capability configs now use.
- SANs: the CSI
dns-namesarenetwork-services-operator-{alb,network}-mcp[.datum-system.svc[.cluster.local]], matching the https hosts in both capability configs and the prefixed names infra's identity-forwarding patch already lists. The README correctly warns that a differentnamePrefixor namespace must patch the volume attributes, since kustomize does not rewrite them. - Trust: the assistant uses the system root pool plus
SSL_CERT_FILEfromdatum-control-plane-system-bundle(infra#8826, merged), which holds thedatum-control-planeCA. The NSO manager inapps/network-services-operator/control-plane/basealready uses the same CSI driver and issuer, so both exist where these run. - Probes are HTTPS on the unchanged port 8080, so
networkpolicy-alb-mcp.yamlandnetworkpolicy-network-mcp.yamlin infra are unaffected. Pods run as 65532, matchingfs-group. - Reload and shutdown:
certwatchersuppliesGetCertificate, a failed re-read keeps the previous cert, and the errgroup cancels the watcher and shuts the server down (20s drain, thenClose).tls_test.gocovers rotation, plain-HTTP fallback, half-config refusal and unreadable-cert failure.
Follow-ups:
- Nothing checks that the CSI
dns-namesagree with the URLs in the assistant configs; add a render-time test so a rename cannot drift. - During rollout the assistant config and deployment ship in one bundle, so expect a brief connect failure until the pods mount their certs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Since milo-os/assistant#169 the assistant refuses to send a user's token over plain http. alb-mcp and network-mcp only served http on port 8080, so on staging the assistant drops the ALB and network tools and logs
credential_plaintext_refusedfor both servers.Solution
internal/mcpservergains one TLS option that both binaries use:--tls-cert-fileand--tls-key-fileturn on https, and the certificate reloads when cert-manager rotates it. Without the flags the servers still serve http for local dev and e2e, and setting only one fails at startup. Both deployments mount a certificate from thedatum-control-planeClusterIssuer through the cert-manager CSI driver and probe over HTTPS, and all 21 URLs in the two capability documents now use https. The assistant already trusts that CA (datum-cloud/infra#8826).Refs milo-os/assistant#169