Merge 1b201cf0d4 into eddfb5fb20

terraform/provider/CHANGELOG.md

@@ -37,6 +37,7 @@ longer signal it.
 
 ### Fixed
 
+- **credential**: `litellm_credential` create now adopts an existing credential on a `credential_name` conflict instead of failing with a 500; `apply` is idempotent again once state loses track of a credential that still exists on the proxy
 - **team**: Read now decodes the `team_info` envelope `/team/info` actually returns, so team attributes refresh from the proxy instead of always falling back to the prior state
 - **key**: Read now unwraps the `info` envelope `/key/info` actually returns; previously reads mapped nothing back into state, so drift on a key was never detected
 - **key**: Read now picks up `model_rpm_limit`, `model_tpm_limit`, `guardrails`, `tags`, `enforced_params`, `allowed_passthrough_routes`, `rpm_limit_type`, `tpm_limit_type` and `prompts` from `info.metadata`, where the proxy actually stores them; previously they stayed empty in state, so a matching config showed a permanent phantom diff on them and out-of-band changes to them were never detected

terraform/provider/litellm/resource_credential_crud.go

@@ -88,6 +88,26 @@ func resourceLiteLLMCredentialCreate(d *schema.ResourceData, m interface{}) erro
 
 	err = handleCredentialAPIResponse(resp, nil, client)
 	if err != nil {
+		// If a credential with this name already exists, adopt it instead of
+		// failing: take ownership and update the existing credential's
+		// values (merged onto whatever it already had - not a full replace)
+		// rather than erroring on the unique-constraint conflict. See
+		// https://github.com/BerriAI/terraform-provider-litellm/issues/8.
+		if err.Error() == "credential_conflict" {
+			log.Printf("[WARN] Credential %q already exists; adopting it and updating to match configuration.", credentialName)
+			d.SetId(credentialName)
+			if updateErr := resourceLiteLLMCredentialUpdate(d, m); updateErr != nil {
+				// Adoption failed before this run took ownership of
+				// anything real. Clear the ID so create is reported as
+				// failed outright (matching pre-adoption behavior) instead
+				// of tainting state for a credential this run doesn't own -
+				// state that would otherwise get destroyed on the next
+				// apply.
+				d.SetId("")
+				return fmt.Errorf("failed to adopt existing credential %q: %w", credentialName, updateErr)
+			}
+			return nil
+		}
 		return fmt.Errorf("failed to create credential: %w", err)
 	}
 
@@ -142,6 +162,7 @@ func resourceLiteLLMCredentialUpdate(d *schema.ResourceData, m interface{}) erro
 	client := m.(*Client)
 	credentialName := d.Id()
 
+	modelID := d.Get("model_id").(string)
 	credentialInfo := d.Get("credential_info").(map[string]interface{})
 	credentialValues := d.Get("credential_values").(map[string]interface{})
 
@@ -157,8 +178,13 @@ func resourceLiteLLMCredentialUpdate(d *schema.ResourceData, m interface{}) erro
 		credValuesMap[k] = v
 	}
 
+	// model_id must travel with the update the same way it does on create,
+	// so the proxy's model-based credential resolution still applies. Without
+	// it, updating (or adopting) a model_id-scoped credential silently loses
+	// that association.
 	credentialRequest := CredentialRequest{
 		CredentialName:   credentialName,
+		ModelID:          modelID,
 		CredentialInfo:   credInfoMap,
 		CredentialValues: credValuesMap,
 	}

terraform/provider/litellm/resource_credential_crud_test.go

@@ -3,6 +3,7 @@ package litellm
 import (
 	"encoding/json"
 	"fmt"
+	"io"
 	"net/http"
 	"net/http/httptest"
 	"sync/atomic"
@@ -199,3 +200,159 @@ func TestRetryCredentialRead_ConnectionError(t *testing.T) {
 	// Connection error should not be retried (not a "credential_not_found")
 	fmt.Printf("connection error (expected): %v\n", err)
 }
+
+// conflictServer builds the shared conflict-then-recover mock used by the
+// adoption tests below. patchStatus/patchBody control the PATCH response, so
+// callers can exercise both the success and failure paths.
+func conflictServer(t *testing.T, patchStatus int, patchBody string) (*httptest.Server, *int32, *int32, *[]byte) {
+	t.Helper()
+	var createCalls, patchCalls int32
+	var capturedPatchBody []byte
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		switch {
+		case r.Method == http.MethodPost && r.URL.Path == "/credentials":
+			atomic.AddInt32(&createCalls, 1)
+			w.Header().Set("Content-Type", "application/json")
+			w.WriteHeader(http.StatusInternalServerError)
+			w.Write([]byte(`{"error":{"message":"Unique constraint failed on the fields: (` + "`credential_name`" + `)","type":"internal_server_error","code":"500"}}`))
+		case r.Method == http.MethodPatch:
+			atomic.AddInt32(&patchCalls, 1)
+			if r.URL.Path != "/credentials/conflict-test" {
+				t.Errorf("PATCH went to %q, want /credentials/conflict-test", r.URL.Path)
+			}
+			body, _ := io.ReadAll(r.Body)
+			capturedPatchBody = body
+			w.Header().Set("Content-Type", "application/json")
+			w.WriteHeader(patchStatus)
+			w.Write([]byte(patchBody))
+		case r.Method == http.MethodGet:
+			if r.URL.Path != "/credentials/by_name/conflict-test" || r.URL.Query().Get("model_id") != "model-1" {
+				t.Errorf("GET went to %q (query %q), want /credentials/by_name/conflict-test?model_id=model-1", r.URL.Path, r.URL.RawQuery)
+			}
+			resp := CredentialResponse{CredentialName: "conflict-test", CredentialInfo: map[string]interface{}{}}
+			body, _ := json.Marshal(resp)
+			w.Header().Set("Content-Type", "application/json")
+			w.WriteHeader(http.StatusOK)
+			w.Write(body)
+		default:
+			http.NotFound(w, r)
+		}
+	}))
+	return srv, &createCalls, &patchCalls, &capturedPatchBody
+}
+
+// A credential that already exists in LiteLLM (created out of band, or left
+// behind by a prior apply that dropped state) must be adopted on create
+// instead of failing on the credential_name unique-constraint conflict, and
+// the adopt PATCH must carry model_id so model-based credential resolution
+// still applies (previously dropped - see
+// https://github.com/BerriAI/litellm/pull/39745).
+func TestResourceLiteLLMCredentialCreate_AdoptsOnConflict(t *testing.T) {
+	srv, createCalls, patchCalls, patchBody := conflictServer(t, http.StatusOK, `{}`)
+	defer srv.Close()
+
+	client := NewClient(srv.URL, "test-key", true)
+	d := schema.TestResourceDataRaw(t, resourceLiteLLMCredential().Schema, map[string]interface{}{
+		"credential_name":   "conflict-test",
+		"model_id":          "model-1",
+		"credential_info":   map[string]interface{}{"custom_llm_provider": "bedrock"},
+		"credential_values": map[string]interface{}{"aws_access_key_id": "val"},
+	})
+
+	if err := resourceLiteLLMCredentialCreate(d, client); err != nil {
+		t.Fatalf("expected create to adopt the existing credential, got error: %v", err)
+	}
+	if d.Id() != "conflict-test" {
+		t.Fatalf("expected ID %q, got %q", "conflict-test", d.Id())
+	}
+	if got := atomic.LoadInt32(createCalls); got != 1 {
+		t.Fatalf("expected exactly 1 POST /credentials call, got %d", got)
+	}
+	if got := atomic.LoadInt32(patchCalls); got != 1 {
+		t.Fatalf("expected the conflict to trigger exactly 1 PATCH (adopt-and-update), got %d", got)
+	}
+
+	var sent map[string]interface{}
+	if err := json.Unmarshal(*patchBody, &sent); err != nil {
+		t.Fatalf("PATCH body was not valid JSON: %v (%s)", err, *patchBody)
+	}
+	if sent["credential_name"] != "conflict-test" {
+		t.Errorf("PATCH body credential_name = %v, want conflict-test", sent["credential_name"])
+	}
+	if sent["model_id"] != "model-1" {
+		t.Errorf("PATCH body model_id = %v, want model-1 (adoption must not drop model-based credential resolution)", sent["model_id"])
+	}
+	credInfo, _ := sent["credential_info"].(map[string]interface{})
+	if credInfo["custom_llm_provider"] != "bedrock" {
+		t.Errorf("PATCH body credential_info = %v, want custom_llm_provider=bedrock", sent["credential_info"])
+	}
+}
+
+// If the adopt PATCH itself fails, create must not have set the resource ID
+// for a credential this run doesn't own - otherwise Terraform taints the
+// entry and the *next* apply destroys a credential nobody here created.
+func TestResourceLiteLLMCredentialCreate_FailedAdoptDoesNotTaint(t *testing.T) {
+	srv, createCalls, patchCalls, _ := conflictServer(t, http.StatusInternalServerError, `{"error":{"message":"Internal Server Error"}}`)
+	defer srv.Close()
+
+	client := NewClient(srv.URL, "test-key", true)
+	d := schema.TestResourceDataRaw(t, resourceLiteLLMCredential().Schema, map[string]interface{}{
+		"credential_name":   "conflict-test",
+		"credential_info":   map[string]interface{}{},
+		"credential_values": map[string]interface{}{"key": "val"},
+	})
+
+	err := resourceLiteLLMCredentialCreate(d, client)
+	if err == nil {
+		t.Fatal("expected an error when the adopt PATCH fails, got nil")
+	}
+	if got := atomic.LoadInt32(createCalls); got != 1 {
+		t.Fatalf("expected exactly 1 POST /credentials call, got %d", got)
+	}
+	if got := atomic.LoadInt32(patchCalls); got != 1 {
+		t.Fatalf("expected exactly 1 PATCH attempt, got %d", got)
+	}
+	if d.Id() != "" {
+		t.Fatalf("resource ID must stay empty after a failed adopt, got %q (a tainted entry would be destroyed on the next apply)", d.Id())
+	}
+}
+
+// A non-conflict failure (a plain 500, for example) must return the original
+// error and never attempt to adopt anything.
+func TestResourceLiteLLMCredentialCreate_NonConflictErrorDoesNotAdopt(t *testing.T) {
+	var createCalls, patchCalls int32
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		switch {
+		case r.Method == http.MethodPost && r.URL.Path == "/credentials":
+			atomic.AddInt32(&createCalls, 1)
+			w.Header().Set("Content-Type", "application/json")
+			w.WriteHeader(http.StatusInternalServerError)
+			w.Write([]byte(`{"error":{"message":"Internal Server Error","type":"internal_server_error"}}`))
+		case r.Method == http.MethodPatch:
+			atomic.AddInt32(&patchCalls, 1)
+			w.WriteHeader(http.StatusOK)
+			w.Write([]byte(`{}`))
+		default:
+			http.NotFound(w, r)
+		}
+	}))
+	defer srv.Close()
+
+	client := NewClient(srv.URL, "test-key", true)
+	d := schema.TestResourceDataRaw(t, resourceLiteLLMCredential().Schema, map[string]interface{}{
+		"credential_name":   "some-cred",
+		"credential_info":   map[string]interface{}{},
+		"credential_values": map[string]interface{}{"key": "val"},
+	})
+
+	err := resourceLiteLLMCredentialCreate(d, client)
+	if err == nil {
+		t.Fatal("expected an error for a non-conflict failure, got nil")
+	}
+	if got := atomic.LoadInt32(&patchCalls); got != 0 {
+		t.Fatalf("expected no PATCH attempt for a non-conflict error, got %d", got)
+	}
+	if d.Id() != "" {
+		t.Fatalf("resource ID must stay empty on a non-conflict failure, got %q", d.Id())
+	}
+}

terraform/provider/litellm/utils.go

@@ -202,6 +202,35 @@ func isCredentialNotFoundError(errResp ErrorResponse) bool {
 	return false
 }
 
+// isCredentialConflictError checks if the error response indicates a credential
+// name collision. LiteLLM surfaces this as a 500 carrying the underlying Prisma
+// unique-constraint message on credential_name. See
+// https://github.com/BerriAI/terraform-provider-litellm/issues/8.
+func isCredentialConflictError(errResp ErrorResponse) bool {
+	if msg, ok := errResp.Error.Message.(string); ok {
+		if strings.Contains(msg, "Unique constraint failed") && strings.Contains(msg, "credential_name") {
+			return true
+		}
+	}
+
+	if msgMap, ok := errResp.Error.Message.(map[string]interface{}); ok {
+		if errStr, ok := msgMap["error"].(string); ok {
+			if strings.Contains(errStr, "Unique constraint failed") && strings.Contains(errStr, "credential_name") {
+				return true
+			}
+		}
+	}
+
+	// Check Detail.Error field for LiteLLM proxy error format
+	if errResp.Detail.Error != "" {
+		if strings.Contains(errResp.Detail.Error, "Unique constraint failed") && strings.Contains(errResp.Detail.Error, "credential_name") {
+			return true
+		}
+	}
+
+	return false
+}
+
 // handleCredentialAPIResponse handles API responses specifically for credential operations
 func handleCredentialAPIResponse(resp *http.Response, result interface{}, client *Client) error {
 	bodyBytes, err := io.ReadAll(resp.Body)
@@ -219,6 +248,9 @@ func handleCredentialAPIResponse(resp *http.Response, result interface{}, client
 			if isCredentialNotFoundError(errResp) {
 				return fmt.Errorf("credential_not_found")
 			}
+			if isCredentialConflictError(errResp) {
+				return fmt.Errorf("credential_conflict")
+			}
 		}
 		return fmt.Errorf("API request failed: Status: %s, Response: %s",
 			resp.Status, client.redactSensitiveData(string(bodyBytes)))