Remove Teleport authentication support#687
Conversation
Teleport Application Access integration has moved out of muster into a separate operator. This change drops the entire surface: - internal/teleport package (cert watcher, client, adapter, types) - internal/api: TeleportClientHandler, TeleportAuth, AuthTypeTeleport, TeleportClientConfig, RegisterTeleportClient/GetTeleportClient - v1alpha1 MCPServer CRD: TeleportAuthConfig type and MCPServerAuth.Teleport field; auth.type enum narrowed to oauth|none - OAuthHandler.ExchangeTokenForRemoteClusterWithClient and TokenExchanger.ExchangeWithClient (only consumer was teleport) - mcpserver.MCPClientConfig.HTTPClient and the *WithHTTPClient stream client constructors (only consumer was teleport) - aggregator helper getTeleportHTTPClientIfConfigured and the dual-path branches in connection_helper.go and server.go - BDD scenarios under internal/testing/scenarios/mcpserver-teleport-*.yaml - docs/how-to/teleport-authentication.md and every reference in README, reference, getting-started, and explanation docs - Helm RBAC/values comments referencing teleport identity secrets Existing MCPServer CRs with auth.type: teleport or an auth.teleport block will fail CRD validation after upgrade and must be migrated to the new operator.
ce67066 to
98dd310
Compare
QuentinBisson
left a comment
There was a problem hiding this comment.
Light review (DDD / SOLID / YAGNI / DRY / don't-reinvent / tests).
Removal is clean. Spot-checked: schema.json enum narrowed and teleport map removed; mockTeleportClientHandler gone; mockOAuthHandler.ExchangeTokenForRemoteClusterWithClient stub gone; helm RBAC comments scrubbed; TestConfigurationChanged case rewritten to Type: "none" rather than just deleted; whole BDD scenarios deleted as a batch (right call — they were Teleport-specific). Diff +-side grep for teleport|tsh|tbot returns only the CHANGELOG line.
Non-blocking — YAGNI
validateExchangeRequestandgetExchangeDefaults(internal/oauth/token_exchange.go) were the DRY seam betweenExchangeandExchangeWithClient. WithExchangeWithClientgone they each have a single caller. Inline back intoExchangeor keep — defensible either way; flagging only because it was the seam's only reason to exist.
paurosello
left a comment
There was a problem hiding this comment.
Verified hard removal is clean:
internal/teleport/deleted as a unit (package + API surface)auth.typeCRD enum narrowed tooauth;none(helm/muster/crds/muster.giantswarm.io_mcpservers.yaml)schema.jsonhas zeroteleportmatchespkg/apis/muster/v1alpha1/mcpserver_types.go— noTeleportAuthConfigtype- BDD scenarios scrubbed;
TestConfigurationChangedrewritten sensibly toType: "none" go build ./...andgo vet ./...clean
External-consumer check (giantswarm org-wide GH code search):
- 0 hits for
auth.type: teleportin any MCPServer manifest - 0 hits for
TeleportAuthConfig,AuthTypeTeleport,muster/internal/teleportoutside this PR
CHANGELOG entry is migration-aware. No backwards-compat shim — clean break, as intended.
Conflicts: - internal/oauth/manager.go: main added filepath.Clean to createHTTPClientWithCA (gosec fix in #706); this branch deleted that helper entirely. Resolved by keeping the deletion and dropping the now-unused os / path/filepath imports. - CHANGELOG.md: kept both Added entries (--extra-ca-file flag and MCPServer.spec.family). Removed entry for the per-config CAFile knobs is unchanged on this side. Picked up the Teleport-removal commit (#687) and the OTEL / instrument refactors from main. Verified build, vet, full unit suite, and helm lint all clean. extra-ca-file render still emits the expected --extra-ca-file arg and the SPIFFE-bundle volume mount.
Summary
Teleport Application Access integration is moving out of muster into a dedicated operator, so this PR rips the whole thing out of the main codebase.
What's gone
internal/teleportpackage — cert watcher, mTLS HTTP client, security helpers, adapter, tests (about 3.5k lines).internal/api) —TeleportClientHandlerinterface,RegisterTeleportClient/GetTeleportClient,TeleportClientConfig,TeleportAuth,AuthTypeTeleport, and the entireinternal/api/teleport.gofile.muster.giantswarm.io/v1alpha1) —TeleportAuthConfigtype and theMCPServerAuth.Teleportfield are deleted. Theauth.typeenum is narrowed fromoauth;teleport;nonetooauth;none. Helm CRD and deepcopy are regenerated.OAuthHandler.ExchangeTokenForRemoteClusterWithClient,TokenExchanger.ExchangeWithClient, and their adapters. The only consumer was the Teleport mTLS path; the standardExchangeTokenForRemoteClustercovers everything else.MCPClientConfig.HTTPClient,NewStreamableHTTPClientWithHTTPClient,NewStreamableHTTPClientWithHeaderFuncAndHTTPClient. Same story: only fed by the Teleport path.getTeleportHTTPClientIfConfiguredand theTeleportClientResultdual-path branches inconnection_helper.goandserver.go. Token-exchange and MCP-client construction are now single-path.mcpserver-teleport-*.yamlscenarios; the proxied token-exchange scenario keeps its proxy semantics but no longer name-drops Teleport.docs/how-to/teleport-authentication.mddeleted; README, architecture, capabilities, problem-statement, configuration-examples, getting-started, mcp-tools, and CRD reference scrubbed of every Teleport reference. Example tool names that usedx_teleport_*now usex_kubernetes_*/x_prometheus_*.Breaking change
Existing MCPServer CRs with
auth.type: teleportor anyauth.teleportblock will be rejected by CRD validation after upgrade. Migrate those workloads to the new operator before rolling this out.Verification
go build ./...,go vet ./...,go test ./...— cleanmake helm-lint,make helm-test— cleanmuster test --generate-schemaregeneratesschema.jsonwith no teleport entriesgrep -ri "teleport\|tbot"returns only the CHANGELOG entry that documents the removalNet diff: 57 files, 59 insertions, 6091 deletions.