External Secret Integration for MCP Servers

[calvinmclean]

No description provided.

[entelligence-ai-pr-reviews]

EntelligenceAI PR Summary

Fixes a nil-dereference bug in SecretChanged and adds test coverage for secret delete fan-out behavior when annotations are absent.

• Added secret != nil guard in handler.go before the hash comparison to prevent a deleted (nil) secret from incorrectly matching a stored rotation annotation
• Removed stale comment in handler.go that described the now-corrected empty-string behavior for deleted secrets
• Added new test TestSecretChanged_DeleteFansOutEvenWhenNeverAnnotated in handler_test.go asserting that Update is called and rotationAnnotation is set even when the server's Annotations map is uninitialized

---

Confidence Score: 5/5 - Safe to Merge

Safe to merge — this PR correctly addresses a nil-dereference bug in SecretChanged within handler.go by adding a secret != nil guard before the hash comparison, preventing a panic when a deleted secret is passed as nil. The accompanying test TestSecretChanged_DeleteFansOutEvenWhenNeverAnnotated in handler_test.go directly validates the fan-out behavior for deleted secrets that lack rotation annotations, providing meaningful regression coverage. No review comments were generated, all changed files were reviewed, and no pre-existing unresolved concerns are associated with this PR.

Key Findings:

• The secret != nil guard in handler.go is a minimal, targeted fix that closes a real nil-dereference path without introducing side effects or altering non-delete code paths.
• The new test TestSecretChanged_DeleteFansOutEvenWhenNeverAnnotated covers the previously untested edge case where a secret is deleted but was never annotated, directly exercising the corrected guard and confirming fan-out still occurs.
• Removal of the stale comment describing the now-incorrect empty-string behavior for deleted secrets improves code clarity and prevents future misunderstanding of the logic.
• 1 previous unresolved comment(s) likely resolved in latest diff (score-only signal; thread status unchanged)

Files requiring special attention

• handler.go
• handler_test.go

[entelligence-ai-pr-reviews]

Adds end-to-end Kubernetes Secret binding support for MCP server environment variables and headers, enabling GitOps-managed credential injection with optional dynamic file-mount rotation without pod restarts.

• New MCPSecretBinding type (name, key, file, dynamic) added to MCPHeader and propagated through OpenAPI, deep copy, and UI type definitions
• pkg/mcp/secretbindings.go provides MergeBoundCreds, HashReferencedKeys, ManifestReferencesSecret, and ManifestHasOnlyDynamicFileBindingsForSecret utilities
• pkg/controller/handlers/mcpsecretbinding/handler.go watches Secrets in the obot namespace and fans out annotation-hash bumps to referencing MCPServer CRDs, with selective session shutdown skipped for dynamic file bindings
• mcp.MergeBoundCreds integrated into all API handler paths (mcp, mcpcatalogs, mcpgateway, systemmcpserver, poweruserworkspace) before server config construction
• ValidateSecretBindings and ValidateTemplateReferences enforce GitOps-only usage, mutual exclusivity with static values, and dynamic-requires-file rules in both server and catalog entry handlers/controllers
• LocalK8sClient and ObotNamespace threaded from Services → Server → api.Context to support Secret lookups; nil-safe on Docker backend
• Drift detection in mcpserver.go migrated from SlicesEqualIgnoreOrder to sort-then-hash for order-independent comparison
• UI components updated to render read-only Secret binding metadata, surface missing-Secret warnings, and disable save on non-Kubernetes engines with binding errors

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.