Merge 825b092015 into 0c98afa780

terraform/provider/CHANGELOG.md

@@ -43,6 +43,7 @@ longer signal it.
 - **key**: Updates no longer send an empty `budget_duration`, which the proxy rejects with a 400; any update to a key without a configured `budget_duration` previously failed outright
 - **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
+- **mcp_server, model, key, organization_member**: Requests now accept any 2xx response instead of requiring exactly HTTP 200; the proxy legitimately returns 201 from `POST /v1/mcp/server` (and other create endpoints), so `litellm_mcp_server` create previously succeeded on the proxy but failed in the provider, leaving the resource out of state and risking a duplicate server on retry
 
 ### Changed
 

terraform/provider/litellm/client.go

@@ -422,7 +422,7 @@ func (c *Client) sendRequest(method, path string, body interface{}) (map[string]
 	log.Printf("Response status: %d", resp.StatusCode)
 	log.Printf("Response body: %s", c.redactSensitiveData(string(bodyBytes)))
 
-	if resp.StatusCode != http.StatusOK {
+	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
 		return nil, &apiError{StatusCode: resp.StatusCode, Body: string(bodyBytes)}
 	}
 

terraform/provider/litellm/client_test.go

@@ -1,10 +1,67 @@
 package litellm
 
 import (
+	"errors"
+	"net/http"
+	"net/http/httptest"
 	"strings"
 	"testing"
 )
 
+func TestSendRequestAcceptsFullSuccessRange(t *testing.T) {
+	tests := []struct {
+		name       string
+		statusCode int
+		wantErr    bool
+		wantValue  string
+	}{
+		{name: "200 OK", statusCode: http.StatusOK, wantErr: false, wantValue: "ok"},
+		{name: "201 Created", statusCode: http.StatusCreated, wantErr: false, wantValue: "created"},
+		{name: "202 Accepted", statusCode: http.StatusAccepted, wantErr: false, wantValue: "accepted"},
+		{name: "400 Bad Request", statusCode: http.StatusBadRequest, wantErr: true},
+		{name: "404 Not Found", statusCode: http.StatusNotFound, wantErr: true},
+		{name: "500 Internal Server Error", statusCode: http.StatusInternalServerError, wantErr: true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				w.Header().Set("Content-Type", "application/json")
+				w.WriteHeader(tt.statusCode)
+				w.Write([]byte(`{"value":"` + tt.wantValue + `"}`))
+			}))
+			defer srv.Close()
+
+			client := NewClient(srv.URL, "test-key", true)
+			result, err := client.sendRequest("POST", "/whatever", map[string]string{"foo": "bar"})
+
+			if tt.wantErr {
+				if err == nil {
+					t.Fatalf("sendRequest returned no error for status %d", tt.statusCode)
+				}
+				var apiErr *apiError
+				if !errors.As(err, &apiErr) {
+					t.Fatalf("error is not *apiError: %v", err)
+				}
+				if apiErr.StatusCode != tt.statusCode {
+					t.Errorf("apiErr.StatusCode = %d, want %d", apiErr.StatusCode, tt.statusCode)
+				}
+				if tt.statusCode == http.StatusNotFound && !isNotFound(err) {
+					t.Errorf("isNotFound(err) = false for 404, want true")
+				}
+				return
+			}
+
+			if err != nil {
+				t.Fatalf("sendRequest returned unexpected error: %v", err)
+			}
+			if result["value"] != tt.wantValue {
+				t.Errorf("result[value] = %v, want %q", result["value"], tt.wantValue)
+			}
+		})
+	}
+}
+
 func TestRedactSensitiveDataNestedCredentialValues(t *testing.T) {
 	c := NewClient("http://localhost:4000", "sk-test", false)
 

terraform/provider/litellm/resource_mcp_server_crud.go

@@ -270,7 +270,7 @@ func resourceLiteLLMMCPServerDelete(d *schema.ResourceData, m interface{}) error
 	defer resp.Body.Close()
 
 	// For delete operations, we expect a simple string response
-	if resp.StatusCode != 200 {
+	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
 		return fmt.Errorf("failed to delete MCP server: unexpected status code %d", resp.StatusCode)
 	}
 

terraform/provider/litellm/resource_mcp_server_crud_test.go

@@ -1,11 +1,59 @@
 package litellm
 
 import (
+	"net/http"
+	"net/http/httptest"
 	"testing"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 )
 
+func TestMCPServerDeleteAcceptsFullSuccessRange(t *testing.T) {
+	tests := []struct {
+		name       string
+		statusCode int
+		wantErr    bool
+	}{
+		{name: "200 OK", statusCode: http.StatusOK, wantErr: false},
+		{name: "202 Accepted", statusCode: http.StatusAccepted, wantErr: false},
+		{name: "404 Not Found", statusCode: http.StatusNotFound, wantErr: true},
+		{name: "500 Internal Server Error", statusCode: http.StatusInternalServerError, wantErr: true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				w.WriteHeader(tt.statusCode)
+			}))
+			defer srv.Close()
+
+			d := schema.TestResourceDataRaw(t, resourceLiteLLMMCPServer().Schema, map[string]interface{}{
+				"server_name": "gh",
+				"transport":   "stdio",
+				"command":     "npx",
+			})
+			d.SetId("srv-1")
+
+			client := NewClient(srv.URL, "test-key", true)
+			err := resourceLiteLLMMCPServerDelete(d, client)
+
+			if tt.wantErr {
+				if err == nil {
+					t.Fatalf("resourceLiteLLMMCPServerDelete returned no error for status %d", tt.statusCode)
+				}
+				return
+			}
+
+			if err != nil {
+				t.Fatalf("resourceLiteLLMMCPServerDelete returned unexpected error: %v", err)
+			}
+			if d.Id() != "" {
+				t.Errorf("resource ID not cleared after successful delete, got %q", d.Id())
+			}
+		})
+	}
+}
+
 func TestMCPServerReadDoesNotPersistServerEnv(t *testing.T) {
 	d := schema.TestResourceDataRaw(t, resourceLiteLLMMCPServer().Schema, map[string]interface{}{
 		"server_name": "gh",

terraform/provider/litellm/utils.go

@@ -42,7 +42,7 @@ func handleAPIResponse(resp *http.Response, reqBody interface{}, client *Client)
 		return nil, fmt.Errorf("failed to read response body: %v", err)
 	}
 
-	if resp.StatusCode != http.StatusOK {
+	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
 		var errResp ErrorResponse
 		if err := json.Unmarshal(bodyBytes, &errResp); err == nil {
 			if isModelNotFoundError(errResp) {
@@ -54,6 +54,10 @@ func handleAPIResponse(resp *http.Response, reqBody interface{}, client *Client)
 			resp.Status, client.redactSensitiveData(string(bodyBytes)), client.redactSensitiveData(string(reqBodyBytes)))
 	}
 
+	if len(bodyBytes) == 0 || string(bodyBytes) == "null" {
+		return &ModelResponse{}, nil
+	}
+
 	var modelResp ModelResponse
 	if err := json.Unmarshal(bodyBytes, &modelResp); err != nil {
 		return nil, fmt.Errorf("failed to parse response: %v", err)
@@ -132,7 +136,7 @@ func handleMCPAPIResponse(resp *http.Response, result interface{}, client *Clien
 		return fmt.Errorf("failed to read response body: %v", err)
 	}
 
-	if resp.StatusCode != http.StatusOK {
+	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
 		var errResp ErrorResponse
 		if err := json.Unmarshal(bodyBytes, &errResp); err == nil {
 			if isMCPServerNotFoundError(errResp) {

terraform/provider/litellm/utils_test.go

@@ -0,0 +1,128 @@
+package litellm
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"strconv"
+	"strings"
+	"testing"
+)
+
+func TestHandleAPIResponseAcceptsFullSuccessRange(t *testing.T) {
+	tests := []struct {
+		name       string
+		statusCode int
+		wantErr    bool
+	}{
+		{name: "200 OK", statusCode: http.StatusOK, wantErr: false},
+		{name: "201 Created", statusCode: http.StatusCreated, wantErr: false},
+		{name: "202 Accepted", statusCode: http.StatusAccepted, wantErr: false},
+		{name: "400 Bad Request", statusCode: http.StatusBadRequest, wantErr: true},
+		{name: "404 Not Found", statusCode: http.StatusNotFound, wantErr: true},
+		{name: "500 Internal Server Error", statusCode: http.StatusInternalServerError, wantErr: true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			rec := httptest.NewRecorder()
+			rec.WriteHeader(tt.statusCode)
+			rec.WriteString(`{"model_name":"gpt-4o"}`)
+			resp := rec.Result()
+
+			client := NewClient("http://localhost:4000", "test-key", true)
+			got, err := handleAPIResponse(resp, map[string]interface{}{"model_name": "gpt-4o"}, client)
+
+			if tt.wantErr {
+				if err == nil {
+					t.Fatalf("handleAPIResponse returned no error for status %d", tt.statusCode)
+				}
+				if !strings.Contains(err.Error(), strconv.Itoa(tt.statusCode)) {
+					t.Errorf("error %q does not mention status %d", err.Error(), tt.statusCode)
+				}
+				return
+			}
+
+			if err != nil {
+				t.Fatalf("handleAPIResponse returned unexpected error: %v", err)
+			}
+			if got.ModelName != "gpt-4o" {
+				t.Errorf("got.ModelName = %q, want gpt-4o", got.ModelName)
+			}
+		})
+	}
+}
+
+func TestHandleAPIResponseAcceptsEmptyBodyOn2xx(t *testing.T) {
+	rec := httptest.NewRecorder()
+	rec.WriteHeader(http.StatusNoContent)
+	resp := rec.Result()
+
+	client := NewClient("http://localhost:4000", "test-key", true)
+	got, err := handleAPIResponse(resp, map[string]interface{}{"model_name": "gpt-4o"}, client)
+
+	if err != nil {
+		t.Fatalf("handleAPIResponse returned unexpected error for empty-body 204: %v", err)
+	}
+	if got == nil {
+		t.Fatal("handleAPIResponse returned nil ModelResponse for empty-body 204")
+	}
+}
+
+func TestHandleMCPAPIResponseAcceptsFullSuccessRange(t *testing.T) {
+	tests := []struct {
+		name       string
+		statusCode int
+		wantErr    bool
+	}{
+		{name: "200 OK", statusCode: http.StatusOK, wantErr: false},
+		{name: "201 Created", statusCode: http.StatusCreated, wantErr: false},
+		{name: "202 Accepted", statusCode: http.StatusAccepted, wantErr: false},
+		{name: "400 Bad Request", statusCode: http.StatusBadRequest, wantErr: true},
+		{name: "404 Not Found", statusCode: http.StatusNotFound, wantErr: true},
+		{name: "500 Internal Server Error", statusCode: http.StatusInternalServerError, wantErr: true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			rec := httptest.NewRecorder()
+			rec.WriteHeader(tt.statusCode)
+			rec.WriteString(`{"server_id":"srv-1","server_name":"gh"}`)
+			resp := rec.Result()
+
+			client := NewClient("http://localhost:4000", "test-key", true)
+			var mcpResp MCPServerResponse
+			err := handleMCPAPIResponse(resp, &mcpResp, client)
+
+			if tt.wantErr {
+				if err == nil {
+					t.Fatalf("handleMCPAPIResponse returned no error for status %d", tt.statusCode)
+				}
+				if !strings.Contains(err.Error(), strconv.Itoa(tt.statusCode)) {
+					t.Errorf("error %q does not mention status %d", err.Error(), tt.statusCode)
+				}
+				return
+			}
+
+			if err != nil {
+				t.Fatalf("handleMCPAPIResponse returned unexpected error: %v", err)
+			}
+			if mcpResp.ServerID != "srv-1" {
+				t.Errorf("mcpResp.ServerID = %q, want srv-1", mcpResp.ServerID)
+			}
+		})
+	}
+}
+
+func TestHandleMCPAPIResponseRejectsEmptyBodyOn2xx(t *testing.T) {
+	rec := httptest.NewRecorder()
+	rec.WriteHeader(http.StatusNoContent)
+	resp := rec.Result()
+
+	client := NewClient("http://localhost:4000", "test-key", true)
+	var mcpResp MCPServerResponse
+	err := handleMCPAPIResponse(resp, &mcpResp, client)
+
+	if err == nil {
+		t.Fatal("handleMCPAPIResponse returned no error for empty-body 204; every MCP caller (create/read/update) writes the parsed result straight into Terraform state with no fallback, so a silently-accepted empty body would blank out or empty-ID the resource")
+	}
+}