test(credential): assert PATCH path/body, cover model_id, taint, and non-conflict cases

Replaces the method-only PATCH/GET stubs (which stayed green even if
adoption hit the wrong endpoint or dropped a field) with assertions
on the actual request. Adds coverage for the two bugs fixed in this
branch, plus the case where a non-conflict error must not adopt.
This commit is contained in:
Matthew Howard 2026-09-04 11:00:21 -04:00
parent 84dd8cb39d
commit d0fb841c84

View file

@ -3,6 +3,7 @@ package litellm
import (
"encoding/json"
"fmt"
"io"
"net/http"
"net/http/httptest"
"sync/atomic"
@ -200,11 +201,13 @@ func TestRetryCredentialRead_ConnectionError(t *testing.T) {
fmt.Printf("connection error (expected): %v\n", err)
}
// 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.
func TestResourceLiteLLMCredentialCreate_AdoptsOnConflict(t *testing.T) {
var createCalls, updateCalls, readCalls int32
// 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":
@ -213,12 +216,16 @@ func TestResourceLiteLLMCredentialCreate_AdoptsOnConflict(t *testing.T) {
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(&updateCalls, 1)
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(http.StatusOK)
w.Write([]byte(`{}`))
w.WriteHeader(patchStatus)
w.Write([]byte(patchBody))
case r.Method == http.MethodGet:
atomic.AddInt32(&readCalls, 1)
resp := CredentialResponse{CredentialName: "conflict-test", CredentialInfo: map[string]interface{}{}}
body, _ := json.Marshal(resp)
w.Header().Set("Content-Type", "application/json")
@ -228,6 +235,61 @@ func TestResourceLiteLLMCredentialCreate_AdoptsOnConflict(t *testing.T) {
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)
@ -237,19 +299,57 @@ func TestResourceLiteLLMCredentialCreate_AdoptsOnConflict(t *testing.T) {
"credential_values": map[string]interface{}{"key": "val"},
})
if err := resourceLiteLLMCredentialCreate(d, client); err != nil {
t.Fatalf("expected create to adopt the existing credential, got error: %v", err)
err := resourceLiteLLMCredentialCreate(d, client)
if err == nil {
t.Fatal("expected an error when the adopt PATCH fails, got nil")
}
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 atomic.LoadInt32(&createCalls) != 1 {
t.Fatalf("expected exactly 1 POST /credentials call, got %d", createCalls)
if got := atomic.LoadInt32(patchCalls); got != 1 {
t.Fatalf("expected exactly 1 PATCH attempt, got %d", got)
}
if atomic.LoadInt32(&updateCalls) != 1 {
t.Fatalf("expected the conflict to trigger exactly 1 PATCH (adopt-and-update), got %d", updateCalls)
}
if atomic.LoadInt32(&readCalls) < 1 {
t.Fatalf("expected the post-adopt retry read to run at least once, got %d", readCalls)
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())
}
}