fix(terraform): stop the perpetual plan diff on litellm_mcp_server (#46781)

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
Yassin Kortam 2026-10-15 16:57:05 -07:00 • committed by GitHub
parent b602209ca6
commit 9d442b1300
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 181 additions and 3 deletions

View file

@ -64,6 +64,7 @@ longer signal it.
- **key**: A config-supplied `key` value (write-only) is now forwarded to `/key/generate`; previously it was silently dropped and the proxy generated a random key instead
- **security**: The `litellm_key` data source and `litellm_key_block` resource normalize raw `sk-` keys to their SHA-256 token hash before building request URLs and resource IDs, so plaintext keys no longer land in reverse-proxy access logs, Terraform plan output, or state IDs
- **unified_access_group**: create now accepts any 2xx response instead of requiring exactly HTTP 200; `POST /v1/unified_access_group` legitimately returns 201, so creation previously succeeded on the proxy but failed in the provider, leaving the group out of state and forcing a `terraform import` to recover on the next apply's 409. Same fix and shape as the one already applied to `mcp_server`/`model`/`key`/`organization_member`; `handleResponse` (shared by several other resources) was the one status-check helper that fix didn't reach. The legacy `litellm_access_group` resource calls the unrelated `/access_group/new` endpoint, which already returns 200, so it was never affected
- **mcp_server**: A plan right after a clean apply of a minimal litellm_mcp_server no longer shows changes. alias is now computed so the proxy's default (the server name) is kept, spec_version keeps the configured value when the proxy returns none, and an mcp_info block the proxy fills with fields the provider does not model is no longer written to state
### Changed

View file

@ -24,6 +24,7 @@ func resourceLiteLLMMCPServer() *schema.Resource {
"alias": {
Type: schema.TypeString,
Optional: true,
Computed: true,
Description: "Alias for the MCP server",
},
"description": {

View file

@ -130,7 +130,9 @@ func updateSchemaFromResponse(d *schema.ResourceData, resp *MCPServerResponse) e
d.Set("description", resp.Description)
d.Set("url", resp.URL)
d.Set("transport", resp.Transport)
d.Set("spec_version", resp.SpecVersion)
if resp.SpecVersion != "" {
d.Set("spec_version", resp.SpecVersion)
}
d.Set("auth_type", resp.AuthType)
d.Set("created_at", resp.CreatedAt)
d.Set("created_by", resp.CreatedBy)
@ -151,8 +153,9 @@ func updateSchemaFromResponse(d *schema.ResourceData, resp *MCPServerResponse) e
d.Set("args", resp.Args)
}
// Set mcp_info
if resp.MCPInfo != nil {
if !mcpInfoHasModeledFields(resp.MCPInfo) {
d.Set("mcp_info", []interface{}{})
} else {
mcpInfoList := make([]map[string]interface{}, 1)
mcpInfoMap := make(map[string]interface{})
@ -180,6 +183,14 @@ func updateSchemaFromResponse(d *schema.ResourceData, resp *MCPServerResponse) e
return nil
}
func mcpInfoHasModeledFields(info *MCPInfo) bool {
return info != nil &&
(info.ServerName != "" ||
info.Description != "" ||
info.LogoURL != "" ||
info.MCPServerCostInfo != nil)
}
func resourceLiteLLMMCPServerCreate(d *schema.ResourceData, m interface{}) error {
client, ok := m.(*Client)
if !ok {

View file

@ -1,6 +1,7 @@
package litellm
import (
"context"
"encoding/json"
"net/http"
"net/http/httptest"
@ -184,3 +185,167 @@ func TestMCPServerSchemaWriteOnlyCredentialRules(t *testing.T) {
})
}
}
func TestMCPServerReadProducesStablePlan(t *testing.T) {
const responseWithoutModeledMCPInfo = `{"alias":"lit9695_proof","approval_status":"active","args":[],"auth_type":"none","command":null,"created_at":"2026-10-15T22:48:21.530000Z","created_by":"default_user_id","credentials":null,"description":null,"env":{},"mcp_access_groups":[],"mcp_info":{"is_public":false,"is_public_explicit":false},"server_id":"dc577aa3-22ac-4fdf-a56b-f6c40e35c389","server_name":"lit9695_proof","status":"unknown","transport":"http","updated_at":"2026-10-15T22:48:21.530000Z","updated_by":"default_user_id","url":"https://mcp.example.com/mcp"}`
responseWithLogoURL := strings.Replace(
responseWithoutModeledMCPInfo,
`"mcp_info":{"is_public":false,"is_public_explicit":false}`,
`"mcp_info":{"is_public":false,"is_public_explicit":false,"logo_url":"https://x/logo.png"}`,
1,
)
if responseWithLogoURL == responseWithoutModeledMCPInfo {
t.Fatal("failed to add logo_url to MCP info response")
}
cases := []struct {
name string
config map[string]interface{}
response string
wantEmptyDiff bool
wantChangedKey string
wantAlias string
wantSpecVersion string
wantMCPInfoLogoURL string
}{
{
name: "minimal config",
config: map[string]interface{}{
"server_name": "lit9695_proof",
"url": "https://mcp.example.com/mcp",
"transport": "http",
},
response: responseWithoutModeledMCPInfo,
wantEmptyDiff: true,
wantAlias: "lit9695_proof",
wantSpecVersion: "2024-11-05",
},
{
name: "configured spec version",
config: map[string]interface{}{
"server_name": "lit9695_proof",
"url": "https://mcp.example.com/mcp",
"transport": "http",
"spec_version": "2025-03-26",
},
response: responseWithoutModeledMCPInfo,
wantEmptyDiff: true,
wantSpecVersion: "2025-03-26",
},
{
name: "configured alias drift remains visible",
config: map[string]interface{}{
"server_name": "lit9695_proof",
"alias": "custom",
"url": "https://mcp.example.com/mcp",
"transport": "http",
},
response: responseWithoutModeledMCPInfo,
wantChangedKey: "alias",
},
{
name: "configured MCP info drift remains visible",
config: map[string]interface{}{
"server_name": "lit9695_proof",
"url": "https://mcp.example.com/mcp",
"transport": "http",
"mcp_info": []interface{}{
map[string]interface{}{"logo_url": "https://x/logo.png"},
},
},
response: responseWithoutModeledMCPInfo,
wantChangedKey: "mcp_info",
},
{
name: "modeled MCP info is retained",
config: map[string]interface{}{
"server_name": "lit9695_proof",
"url": "https://mcp.example.com/mcp",
"transport": "http",
"mcp_info": []interface{}{
map[string]interface{}{"logo_url": "https://x/logo.png"},
},
},
response: responseWithLogoURL,
wantEmptyDiff: true,
wantMCPInfoLogoURL: "https://x/logo.png",
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodPost && r.Method != http.MethodGet {
t.Errorf("unexpected request method: %s", r.Method)
w.WriteHeader(http.StatusNotFound)
return
}
if r.URL.Path != "/v1/mcp/server" && r.URL.Path != "/v1/mcp/server/dc577aa3-22ac-4fdf-a56b-f6c40e35c389" {
t.Errorf("unexpected request path: %s", r.URL.Path)
w.WriteHeader(http.StatusNotFound)
return
}
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusOK)
_, _ = w.Write([]byte(tc.response))
}))
defer server.Close()
resource := resourceLiteLLMMCPServer()
d := schema.TestResourceDataRaw(t, resource.Schema, tc.config)
client := NewClient(server.URL, "test-key", true)
if err := resourceLiteLLMMCPServerCreate(d, client); err != nil {
t.Fatalf("create failed: %v", err)
}
if err := resourceLiteLLMMCPServerRead(d, client); err != nil {
t.Fatalf("read failed: %v", err)
}
diff, err := resource.Diff(context.Background(), d.State(), terraform.NewResourceConfigRaw(tc.config), nil)
if err != nil {
t.Fatalf("plan diff failed: %v", err)
}
if tc.wantEmptyDiff {
if diff != nil && !diff.Empty() {
t.Fatalf("expected empty plan diff, got %v", diff.Attributes)
}
if diff != nil {
for key := range diff.Attributes {
if key == "alias" || key == "spec_version" || strings.HasPrefix(key, "mcp_info.") {
t.Fatalf("unexpected diff on %s", key)
}
}
}
} else {
if diff == nil || diff.Empty() {
t.Fatal("expected a plan diff")
}
hasExpectedDiff := false
for key := range diff.Attributes {
hasExpectedDiff = hasExpectedDiff || key == tc.wantChangedKey ||
(tc.wantChangedKey == "mcp_info" && strings.HasPrefix(key, "mcp_info."))
}
if !hasExpectedDiff {
t.Fatalf("expected a plan diff on %s, got %v", tc.wantChangedKey, diff.Attributes)
}
}
if tc.wantAlias != "" && d.Get("alias") != tc.wantAlias {
t.Fatalf("alias = %v, want %s", d.Get("alias"), tc.wantAlias)
}
if tc.wantSpecVersion != "" && d.Get("spec_version") != tc.wantSpecVersion {
t.Fatalf("spec_version = %v, want %s", d.Get("spec_version"), tc.wantSpecVersion)
}
if tc.wantMCPInfoLogoURL != "" {
mcpInfo := d.Get("mcp_info").([]interface{})
if len(mcpInfo) != 1 {
t.Fatalf("mcp_info has %d blocks, want 1", len(mcpInfo))
}
gotLogoURL := mcpInfo[0].(map[string]interface{})["logo_url"]
if gotLogoURL != tc.wantMCPInfoLogoURL {
t.Fatalf("mcp_info logo_url = %v, want %s", gotLogoURL, tc.wantMCPInfoLogoURL)
}
}
})
}
}