From c788769129e23261d7be091cac9aa036c2c04b09 Mon Sep 17 00:00:00 2001 From: Matthew Howard <78384492+matthowardcohere@users.noreply.github.com> Date: Mon, 14 Sep 2026 15:18:14 -0700 Subject: [PATCH] fix(terraform): recognize a credential name conflict and adopt the existing credential The proxy answered a duplicate credential_name with a 500 carrying Prisma's unique-constraint message, so terraform apply against a credential the state had lost died with an opaque database error. Classify that response as a conflict, adopt the existing credential with a PATCH that carries model_id, and set the resource ID only once the adopt succeeds so a failed PATCH does not taint a credential this run never owned Squashed from the commits on #39745 with authorship preserved Fixes https://github.com/BerriAI/terraform-provider-litellm/issues/8 --- terraform/provider/CHANGELOG.md | 1 + .../litellm/resource_credential_crud.go | 26 +++ .../litellm/resource_credential_crud_test.go | 157 ++++++++++++++++++ terraform/provider/litellm/utils.go | 32 ++++ 4 files changed, 216 insertions(+) diff --git a/terraform/provider/CHANGELOG.md b/terraform/provider/CHANGELOG.md index 8c0ef5a8b15..3daee3250b6 100644 --- a/terraform/provider/CHANGELOG.md +++ b/terraform/provider/CHANGELOG.md @@ -38,6 +38,7 @@ longer signal it. ### Fixed - **key**: An update that changes `team_id` and fails because the key was already cascade-deleted along with its previous team now recovers by recreating the key under the new team, instead of aborting the apply. The key's absence is confirmed against the proxy first, so an unrelated failure still errors out, and a `team_id` change between two teams that both still exist stays a plain in-place update +- **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 diff --git a/terraform/provider/litellm/resource_credential_crud.go b/terraform/provider/litellm/resource_credential_crud.go index dd9aef64f76..cb5031ee04e 100644 --- a/terraform/provider/litellm/resource_credential_crud.go +++ b/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, } diff --git a/terraform/provider/litellm/resource_credential_crud_test.go b/terraform/provider/litellm/resource_credential_crud_test.go index 3398e58dd13..6e0b818fe33 100644 --- a/terraform/provider/litellm/resource_credential_crud_test.go +++ b/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()) + } +} diff --git a/terraform/provider/litellm/utils.go b/terraform/provider/litellm/utils.go index 5e81766d3f3..a123f5d350a 100644 --- a/terraform/provider/litellm/utils.go +++ b/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)))