mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-15 23:31:29 +00:00
feat(terraform): make credential adoption opt-in, escape names in request URLs
Terraform's convention is that create does not seize a resource the configuration never made, and credential_values holds secrets that are never read back into state, so a silent takeover overwrites values no plan showed. A name collision now fails with the terraform import command that adopts the existing credential explicitly, and adopt_existing = true opts into taking it over during create. The provider detects the conflict by the proxy's 409 and keeps the Prisma string match as a fallback for older proxies Credential names and model_id went into URLs raw, so a name with a slash or a question mark hit the wrong route. Every credential URL is now built from a package const through fmt.Sprintf with url.PathEscape or url.QueryEscape, which the endpoint audit can resolve. Toggling adopt_existing alone no longer sends a PATCH, so it does not rewrite the stored secret
This commit is contained in:
parent
81806f33cf
commit
a7f180fdd8
6 changed files with 390 additions and 160 deletions
|
|
@ -38,7 +38,9 @@ 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
|
||||
- **credential**: create now reports a `credential_name` collision as a clear error naming the `terraform import` command that adopts the existing credential, instead of surfacing the proxy's raw 500 with a Prisma `Unique constraint failed` message. New `adopt_existing` argument (default `false`) opts into taking the existing credential over during create, which makes `apply` idempotent again once state loses track of a credential that still exists on the proxy. Requires a proxy that answers 409 on the collision; older proxies are still detected by their 500 message
|
||||
- **credential**: credential names and `model_id` are now percent-encoded in request URLs, so a name containing `/`, `?`, `#` or spaces reaches the proxy intact instead of being cut at the first reserved character and read, updated or deleted as a different credential
|
||||
- **credential**: update now sends `model_id`, so a `model_id`-scoped credential keeps resolving its values from that deployment on update and on adoption instead of being overwritten with the literal `credential_values`; needs a proxy from 1.102.0, older proxies ignore the field
|
||||
- **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
|
||||
|
|
|
|||
|
|
@ -130,6 +130,7 @@ The following arguments are supported:
|
|||
* `credential_values` - (Required, Sensitive) Map of sensitive credential values such as API keys, tokens, etc.
|
||||
* `model_id` - (Optional) Model ID associated with this credential.
|
||||
* `credential_info` - (Optional) Map of additional non-sensitive information about the credential.
|
||||
* `adopt_existing` - (Optional, default `false`) Take over a credential of this name that already exists on the proxy instead of failing. Turning this on overwrites the existing credential's values with the ones in this configuration.
|
||||
|
||||
## Attributes Reference
|
||||
|
||||
|
|
|
|||
|
|
@ -39,6 +39,15 @@ func resourceLiteLLMCredential() *schema.Resource {
|
|||
Elem: &schema.Schema{Type: schema.TypeString},
|
||||
Description: "Sensitive credential values (API keys, tokens, etc.)",
|
||||
},
|
||||
"adopt_existing": {
|
||||
Type: schema.TypeBool,
|
||||
Optional: true,
|
||||
Default: false,
|
||||
Description: "Take over a credential of this name that already exists on the proxy instead of failing. " +
|
||||
"Off by default: create reports the conflict and points at `terraform import`, so an apply never " +
|
||||
"silently overwrites a credential it does not manage. Turning this on overwrites the existing " +
|
||||
"credential's values with the ones in this configuration.",
|
||||
},
|
||||
},
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,15 +1,23 @@
|
|||
package litellm
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"log"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
|
||||
)
|
||||
|
||||
const (
|
||||
endpointCredential = "/credentials/%s"
|
||||
endpointCredentialByName = "/credentials/by_name/%s"
|
||||
endpointCredentialByNameForModel = "/credentials/by_name/%s?model_id=%s"
|
||||
)
|
||||
|
||||
// retryCredentialRead attempts to read a credential with exponential backoff.
|
||||
// If the read path clears the ID (e.g., transient 404 right after create),
|
||||
// we treat it as retryable instead of accepting an empty state.
|
||||
|
|
@ -53,34 +61,28 @@ func retryCredentialRead(d *schema.ResourceData, m interface{}, maxRetries int)
|
|||
return err
|
||||
}
|
||||
|
||||
func resourceLiteLLMCredentialCreate(d *schema.ResourceData, m interface{}) error {
|
||||
client := m.(*Client)
|
||||
|
||||
credentialName := d.Get("credential_name").(string)
|
||||
modelID := d.Get("model_id").(string)
|
||||
credentialInfo := d.Get("credential_info").(map[string]interface{})
|
||||
credentialValues := d.Get("credential_values").(map[string]interface{})
|
||||
|
||||
// Convert credential_info to map[string]interface{} for JSON
|
||||
func credentialRequestFromResource(d *schema.ResourceData, credentialName string) CredentialRequest {
|
||||
credInfoMap := make(map[string]interface{})
|
||||
for k, v := range credentialInfo {
|
||||
for k, v := range d.Get("credential_info").(map[string]interface{}) {
|
||||
credInfoMap[k] = v
|
||||
}
|
||||
|
||||
// Convert credential_values to map[string]interface{} for JSON
|
||||
credValuesMap := make(map[string]interface{})
|
||||
for k, v := range credentialValues {
|
||||
for k, v := range d.Get("credential_values").(map[string]interface{}) {
|
||||
credValuesMap[k] = v
|
||||
}
|
||||
|
||||
credentialRequest := CredentialRequest{
|
||||
return CredentialRequest{
|
||||
CredentialName: credentialName,
|
||||
ModelID: modelID,
|
||||
ModelID: d.Get("model_id").(string),
|
||||
CredentialInfo: credInfoMap,
|
||||
CredentialValues: credValuesMap,
|
||||
}
|
||||
}
|
||||
|
||||
resp, err := MakeRequest(client, "POST", "/credentials", credentialRequest)
|
||||
func resourceLiteLLMCredentialCreate(d *schema.ResourceData, m interface{}) error {
|
||||
client := m.(*Client)
|
||||
credentialName := d.Get("credential_name").(string)
|
||||
|
||||
resp, err := MakeRequest(client, "POST", "/credentials", credentialRequestFromResource(d, credentialName))
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to create credential: %w", err)
|
||||
}
|
||||
|
|
@ -88,45 +90,51 @@ 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
|
||||
if errors.Is(err, errCredentialConflict) {
|
||||
return handleCredentialNameConflict(d, m, credentialName)
|
||||
}
|
||||
return fmt.Errorf("failed to create credential: %w", err)
|
||||
}
|
||||
|
||||
// Set the resource ID to the credential name
|
||||
d.SetId(credentialName)
|
||||
|
||||
log.Printf("[INFO] Credential created with name %s. Starting retry mechanism to read the credential...", credentialName)
|
||||
return retryCredentialRead(d, m, 5)
|
||||
}
|
||||
|
||||
func handleCredentialNameConflict(d *schema.ResourceData, m interface{}, credentialName string) error {
|
||||
if !d.Get("adopt_existing").(bool) {
|
||||
return fmt.Errorf(
|
||||
"credential %q already exists on the proxy but is not in Terraform state. "+
|
||||
"Import it to manage it here:\n\n"+
|
||||
" terraform import litellm_credential.<this resource's name in your config> %s\n\n"+
|
||||
"The next apply then updates it to match this configuration. To take it over during "+
|
||||
"create instead, set adopt_existing = true on this resource, which overwrites the "+
|
||||
"existing credential's values with the ones configured here",
|
||||
credentialName, shellSingleQuote(credentialName),
|
||||
)
|
||||
}
|
||||
|
||||
log.Printf("[WARN] Credential %q already exists; adopt_existing is set, so taking it over and updating it to match configuration.", credentialName)
|
||||
d.SetId(credentialName)
|
||||
if err := patchCredential(m.(*Client), d, credentialName); err != nil {
|
||||
d.SetId("")
|
||||
return fmt.Errorf("failed to adopt existing credential %q: %w", credentialName, err)
|
||||
}
|
||||
return retryCredentialRead(d, m, 5)
|
||||
}
|
||||
|
||||
func shellSingleQuote(s string) string {
|
||||
return "'" + strings.ReplaceAll(s, "'", `'\''`) + "'"
|
||||
}
|
||||
|
||||
func resourceLiteLLMCredentialRead(d *schema.ResourceData, m interface{}) error {
|
||||
client := m.(*Client)
|
||||
credentialName := d.Id()
|
||||
|
||||
// Try to get credential by name first
|
||||
modelID := d.Get("model_id").(string)
|
||||
endpoint := fmt.Sprintf("/credentials/by_name/%s", credentialName)
|
||||
if modelID != "" {
|
||||
endpoint += fmt.Sprintf("?model_id=%s", modelID)
|
||||
endpoint := fmt.Sprintf(endpointCredentialByName, url.PathEscape(credentialName))
|
||||
if modelID := d.Get("model_id").(string); modelID != "" {
|
||||
endpoint = fmt.Sprintf(endpointCredentialByNameForModel, url.PathEscape(credentialName), url.QueryEscape(modelID))
|
||||
}
|
||||
|
||||
resp, err := MakeRequest(client, "GET", endpoint, nil)
|
||||
|
|
@ -158,48 +166,28 @@ func resourceLiteLLMCredentialRead(d *schema.ResourceData, m interface{}) error
|
|||
return nil
|
||||
}
|
||||
|
||||
func resourceLiteLLMCredentialUpdate(d *schema.ResourceData, m interface{}) error {
|
||||
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{})
|
||||
|
||||
// Convert credential_info to map[string]interface{} for JSON
|
||||
credInfoMap := make(map[string]interface{})
|
||||
for k, v := range credentialInfo {
|
||||
credInfoMap[k] = v
|
||||
}
|
||||
|
||||
// Convert credential_values to map[string]interface{} for JSON
|
||||
credValuesMap := make(map[string]interface{})
|
||||
for k, v := range credentialValues {
|
||||
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,
|
||||
}
|
||||
|
||||
endpoint := fmt.Sprintf("/credentials/%s", credentialName)
|
||||
resp, err := MakeRequest(client, "PATCH", endpoint, credentialRequest)
|
||||
func patchCredential(client *Client, d *schema.ResourceData, credentialName string) error {
|
||||
resp, err := MakeRequest(client, "PATCH", fmt.Sprintf(endpointCredential, url.PathEscape(credentialName)), credentialRequestFromResource(d, credentialName))
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to update credential: %w", err)
|
||||
}
|
||||
defer resp.Body.Close()
|
||||
|
||||
err = handleCredentialAPIResponse(resp, nil, client)
|
||||
if err != nil {
|
||||
if err := handleCredentialAPIResponse(resp, nil, client); err != nil {
|
||||
return fmt.Errorf("failed to update credential: %w", err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func resourceLiteLLMCredentialUpdate(d *schema.ResourceData, m interface{}) error {
|
||||
if !d.HasChangesExcept("adopt_existing") {
|
||||
return nil
|
||||
}
|
||||
|
||||
credentialName := d.Id()
|
||||
if err := patchCredential(m.(*Client), d, credentialName); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
log.Printf("[INFO] Credential updated with name %s. Starting retry mechanism to read the credential...", credentialName)
|
||||
return retryCredentialRead(d, m, 5)
|
||||
|
|
@ -209,8 +197,7 @@ func resourceLiteLLMCredentialDelete(d *schema.ResourceData, m interface{}) erro
|
|||
client := m.(*Client)
|
||||
credentialName := d.Id()
|
||||
|
||||
endpoint := fmt.Sprintf("/credentials/%s", credentialName)
|
||||
resp, err := MakeRequest(client, "DELETE", endpoint, nil)
|
||||
resp, err := MakeRequest(client, "DELETE", fmt.Sprintf(endpointCredential, url.PathEscape(credentialName)), nil)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to delete credential: %w", err)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,15 +1,18 @@
|
|||
package litellm
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
|
||||
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
|
||||
"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
|
||||
)
|
||||
|
||||
// newTestResourceData creates a *schema.ResourceData with the credential schema,
|
||||
|
|
@ -201,10 +204,30 @@ func TestRetryCredentialRead_ConnectionError(t *testing.T) {
|
|||
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) {
|
||||
type conflictBody struct {
|
||||
status int
|
||||
body string
|
||||
}
|
||||
|
||||
var (
|
||||
modernConflictBody = conflictBody{
|
||||
status: http.StatusConflict,
|
||||
body: `{"error":{"message":"Credential 'conflict-test' already exists. Update it with PATCH /credentials/conflict-test, or delete it first.","type":"internal_server_error","param":"None","code":"409"}}`,
|
||||
}
|
||||
legacyConflictBody = conflictBody{
|
||||
status: http.StatusInternalServerError,
|
||||
body: `{"error":{"message":"Unique constraint failed on the fields: (` + "`credential_name`" + `)","type":"internal_server_error","code":"500"}}`,
|
||||
}
|
||||
)
|
||||
|
||||
type conflictServerOptions struct {
|
||||
conflict conflictBody
|
||||
patchStatus int
|
||||
patchBody string
|
||||
getStatus int
|
||||
}
|
||||
|
||||
func conflictServer(t *testing.T, opts conflictServerOptions) (*httptest.Server, *int32, *int32, *[]byte) {
|
||||
t.Helper()
|
||||
var createCalls, patchCalls int32
|
||||
var capturedPatchBody []byte
|
||||
|
|
@ -213,8 +236,8 @@ func conflictServer(t *testing.T, patchStatus int, patchBody string) (*httptest.
|
|||
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"}}`))
|
||||
w.WriteHeader(opts.conflict.status)
|
||||
w.Write([]byte(opts.conflict.body))
|
||||
case r.Method == http.MethodPatch:
|
||||
atomic.AddInt32(&patchCalls, 1)
|
||||
if r.URL.Path != "/credentials/conflict-test" {
|
||||
|
|
@ -223,12 +246,17 @@ func conflictServer(t *testing.T, patchStatus int, patchBody string) (*httptest.
|
|||
body, _ := io.ReadAll(r.Body)
|
||||
capturedPatchBody = body
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
w.WriteHeader(patchStatus)
|
||||
w.Write([]byte(patchBody))
|
||||
w.WriteHeader(opts.patchStatus)
|
||||
w.Write([]byte(opts.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)
|
||||
}
|
||||
if opts.getStatus != 0 && opts.getStatus != http.StatusOK {
|
||||
w.WriteHeader(opts.getStatus)
|
||||
w.Write([]byte(`{"error":{"message":"Internal Server Error"}}`))
|
||||
return
|
||||
}
|
||||
resp := CredentialResponse{CredentialName: "conflict-test", CredentialInfo: map[string]interface{}{}}
|
||||
body, _ := json.Marshal(resp)
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
|
|
@ -241,66 +269,114 @@ func conflictServer(t *testing.T, patchStatus int, patchBody string) (*httptest.
|
|||
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{}{
|
||||
func adoptTestData(t *testing.T, adoptExisting bool) *schema.ResourceData {
|
||||
t.Helper()
|
||||
return 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"},
|
||||
"adopt_existing": adoptExisting,
|
||||
})
|
||||
}
|
||||
|
||||
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)
|
||||
}
|
||||
func TestResourceLiteLLMCredentialCreate_AdoptsOnConflictWhenOptedIn(t *testing.T) {
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
conflict conflictBody
|
||||
}{
|
||||
{"typed 409", modernConflictBody},
|
||||
{"legacy 500 with unique-constraint message", legacyConflictBody},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
srv, createCalls, patchCalls, patchBody := conflictServer(t, conflictServerOptions{conflict: tc.conflict, patchStatus: http.StatusOK, patchBody: `{}`})
|
||||
defer srv.Close()
|
||||
|
||||
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"])
|
||||
client := NewClient(srv.URL, "test-key", true)
|
||||
d := adoptTestData(t, true)
|
||||
|
||||
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"])
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestResourceLiteLLMCredentialCreate_ConflictWithoutOptInFailsWithImportHint(t *testing.T) {
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
conflict conflictBody
|
||||
}{
|
||||
{"typed 409", modernConflictBody},
|
||||
{"legacy 500 with unique-constraint message", legacyConflictBody},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
srv, createCalls, patchCalls, _ := conflictServer(t, conflictServerOptions{conflict: tc.conflict, patchStatus: http.StatusOK, patchBody: `{}`})
|
||||
defer srv.Close()
|
||||
|
||||
client := NewClient(srv.URL, "test-key", true)
|
||||
d := adoptTestData(t, false)
|
||||
|
||||
err := resourceLiteLLMCredentialCreate(d, client)
|
||||
if err == nil {
|
||||
t.Fatal("expected create to fail on the conflict when adopt_existing is unset, 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 != 0 {
|
||||
t.Fatalf("expected no PATCH without adopt_existing - create must not overwrite an unmanaged credential - got %d", got)
|
||||
}
|
||||
if d.Id() != "" {
|
||||
t.Fatalf("resource ID must stay empty when create refuses the conflict, got %q", d.Id())
|
||||
}
|
||||
for _, want := range []string{
|
||||
"already exists",
|
||||
`terraform import litellm_credential.<this resource's name in your config> 'conflict-test'`,
|
||||
"adopt_existing = true",
|
||||
} {
|
||||
if !strings.Contains(err.Error(), want) {
|
||||
t.Errorf("error must tell the operator how to proceed; missing %q in: %v", want, err)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// 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"}}`)
|
||||
srv, createCalls, patchCalls, _ := conflictServer(t, conflictServerOptions{
|
||||
conflict: modernConflictBody,
|
||||
patchStatus: http.StatusInternalServerError,
|
||||
patchBody: `{"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"},
|
||||
})
|
||||
d := adoptTestData(t, true)
|
||||
|
||||
err := resourceLiteLLMCredentialCreate(d, client)
|
||||
if err == nil {
|
||||
|
|
@ -317,8 +393,6 @@ func TestResourceLiteLLMCredentialCreate_FailedAdoptDoesNotTaint(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
// 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) {
|
||||
|
|
@ -343,6 +417,7 @@ func TestResourceLiteLLMCredentialCreate_NonConflictErrorDoesNotAdopt(t *testing
|
|||
"credential_name": "some-cred",
|
||||
"credential_info": map[string]interface{}{},
|
||||
"credential_values": map[string]interface{}{"key": "val"},
|
||||
"adopt_existing": true,
|
||||
})
|
||||
|
||||
err := resourceLiteLLMCredentialCreate(d, client)
|
||||
|
|
@ -356,3 +431,166 @@ func TestResourceLiteLLMCredentialCreate_NonConflictErrorDoesNotAdopt(t *testing
|
|||
t.Fatalf("resource ID must stay empty on a non-conflict failure, got %q", d.Id())
|
||||
}
|
||||
}
|
||||
|
||||
func TestResourceLiteLLMCredentialCreate_AdoptKeepsIDWhenPostPatchReadFails(t *testing.T) {
|
||||
srv, _, patchCalls, _ := conflictServer(t, conflictServerOptions{
|
||||
conflict: modernConflictBody,
|
||||
patchStatus: http.StatusOK,
|
||||
patchBody: `{}`,
|
||||
getStatus: http.StatusInternalServerError,
|
||||
})
|
||||
defer srv.Close()
|
||||
|
||||
client := NewClient(srv.URL, "test-key", true)
|
||||
d := adoptTestData(t, true)
|
||||
|
||||
err := resourceLiteLLMCredentialCreate(d, client)
|
||||
if err == nil {
|
||||
t.Fatal("expected the failed post-adopt read to surface as an error, got nil")
|
||||
}
|
||||
if got := atomic.LoadInt32(patchCalls); got != 1 {
|
||||
t.Fatalf("expected exactly 1 PATCH, got %d", got)
|
||||
}
|
||||
if d.Id() != "conflict-test" {
|
||||
t.Fatalf("the PATCH already overwrote the remote credential, so the ID must stay set for Terraform to track it; got %q", d.Id())
|
||||
}
|
||||
}
|
||||
|
||||
func TestResourceLiteLLMCredentialImportHintQuotesTheNameForTheShell(t *testing.T) {
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
want string
|
||||
}{
|
||||
{"my cred", `'my cred'`},
|
||||
{"it's $HOME `id` \"x\"", `'it'\''s $HOME ` + "`id`" + ` "x"'`},
|
||||
} {
|
||||
t.Run(tc.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(http.StatusConflict)
|
||||
w.Write([]byte(`{"error":{"message":"already exists","code":"409"}}`))
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
d := schema.TestResourceDataRaw(t, resourceLiteLLMCredential().Schema, map[string]interface{}{
|
||||
"credential_name": tc.name,
|
||||
"credential_info": map[string]interface{}{},
|
||||
"credential_values": map[string]interface{}{"key": "val"},
|
||||
})
|
||||
|
||||
err := resourceLiteLLMCredentialCreate(d, NewClient(srv.URL, "test-key", true))
|
||||
if err == nil {
|
||||
t.Fatal("expected the conflict to fail create, got nil")
|
||||
}
|
||||
want := "terraform import litellm_credential.<this resource's name in your config> " + tc.want
|
||||
if !strings.Contains(err.Error(), want) {
|
||||
t.Fatalf("import hint must single-quote the name for the shell; missing %q in: %v", want, err)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestCredentialRequestsEscapeReservedCharactersInTheName(t *testing.T) {
|
||||
const name = "team/a?b c"
|
||||
var paths []string
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
paths = append(paths, r.Method+" "+r.URL.EscapedPath()+"?"+r.URL.RawQuery)
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
w.WriteHeader(http.StatusOK)
|
||||
w.Write([]byte(`{"credential_name":"` + name + `","credential_info":{}}`))
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
client := NewClient(srv.URL, "test-key", true)
|
||||
d := schema.TestResourceDataRaw(t, resourceLiteLLMCredential().Schema, map[string]interface{}{
|
||||
"credential_name": name,
|
||||
"model_id": "m&1",
|
||||
"credential_info": map[string]interface{}{},
|
||||
"credential_values": map[string]interface{}{"key": "val"},
|
||||
})
|
||||
d.SetId(name)
|
||||
|
||||
if err := resourceLiteLLMCredentialRead(d, client); err != nil {
|
||||
t.Fatalf("read failed: %v", err)
|
||||
}
|
||||
if err := patchCredential(client, d, name); err != nil {
|
||||
t.Fatalf("patch failed: %v", err)
|
||||
}
|
||||
if err := resourceLiteLLMCredentialDelete(d, client); err != nil {
|
||||
t.Fatalf("delete failed: %v", err)
|
||||
}
|
||||
|
||||
want := []string{
|
||||
"GET /credentials/by_name/team%2Fa%3Fb%20c?model_id=m%261",
|
||||
"PATCH /credentials/team%2Fa%3Fb%20c?",
|
||||
"DELETE /credentials/team%2Fa%3Fb%20c?",
|
||||
}
|
||||
if strings.Join(paths, "\n") != strings.Join(want, "\n") {
|
||||
t.Fatalf("request paths:\n%s\nwant:\n%s", strings.Join(paths, "\n"), strings.Join(want, "\n"))
|
||||
}
|
||||
}
|
||||
|
||||
func TestResourceLiteLLMCredentialUpdate_TogglingAdoptExistingSendsNoPatch(t *testing.T) {
|
||||
var patchCalls int32
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.Method == http.MethodPatch {
|
||||
atomic.AddInt32(&patchCalls, 1)
|
||||
}
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
w.WriteHeader(http.StatusOK)
|
||||
w.Write([]byte(`{"credential_name":"cred-1","credential_info":{}}`))
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
res := resourceLiteLLMCredential()
|
||||
priorData := schema.TestResourceDataRaw(t, res.Schema, map[string]interface{}{
|
||||
"credential_name": "cred-1",
|
||||
"credential_info": map[string]interface{}{},
|
||||
"credential_values": map[string]interface{}{"api_key": "sk-secret"},
|
||||
"adopt_existing": false,
|
||||
})
|
||||
priorData.SetId("cred-1")
|
||||
prior := priorData.State()
|
||||
|
||||
toggled := terraform.NewResourceConfigRaw(map[string]interface{}{
|
||||
"credential_name": "cred-1",
|
||||
"credential_info": map[string]interface{}{},
|
||||
"credential_values": map[string]interface{}{"api_key": "sk-secret"},
|
||||
"adopt_existing": true,
|
||||
})
|
||||
diff, err := res.Diff(context.Background(), prior, toggled, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("diff failed: %v", err)
|
||||
}
|
||||
d, err := schema.InternalMap(res.Schema).Data(prior, diff)
|
||||
if err != nil {
|
||||
t.Fatalf("data failed: %v", err)
|
||||
}
|
||||
if err := resourceLiteLLMCredentialUpdate(d, NewClient(srv.URL, "test-key", true)); err != nil {
|
||||
t.Fatalf("update failed: %v", err)
|
||||
}
|
||||
if got := atomic.LoadInt32(&patchCalls); got != 0 {
|
||||
t.Fatalf("flipping adopt_existing alone must not rewrite the credential's secrets; got %d PATCH calls", got)
|
||||
}
|
||||
|
||||
rotated := terraform.NewResourceConfigRaw(map[string]interface{}{
|
||||
"credential_name": "cred-1",
|
||||
"credential_info": map[string]interface{}{},
|
||||
"credential_values": map[string]interface{}{"api_key": "sk-rotated"},
|
||||
"adopt_existing": true,
|
||||
})
|
||||
diff, err = res.Diff(context.Background(), prior, rotated, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("diff failed: %v", err)
|
||||
}
|
||||
d, err = schema.InternalMap(res.Schema).Data(prior, diff)
|
||||
if err != nil {
|
||||
t.Fatalf("data failed: %v", err)
|
||||
}
|
||||
if err := resourceLiteLLMCredentialUpdate(d, NewClient(srv.URL, "test-key", true)); err != nil {
|
||||
t.Fatalf("update failed: %v", err)
|
||||
}
|
||||
if got := atomic.LoadInt32(&patchCalls); got != 1 {
|
||||
t.Fatalf("a real value change must still PATCH; got %d PATCH calls", got)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ import (
|
|||
"crypto/sha256"
|
||||
"encoding/hex"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
|
|
@ -202,33 +203,21 @@ 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
|
||||
}
|
||||
}
|
||||
var errCredentialConflict = errors.New("credential_conflict")
|
||||
|
||||
func isLegacyCredentialConflictError(errResp ErrorResponse) bool {
|
||||
isConflict := func(msg string) bool {
|
||||
return strings.Contains(msg, "Unique constraint failed") && strings.Contains(msg, "credential_name")
|
||||
}
|
||||
if msg, ok := errResp.Error.Message.(string); ok && isConflict(msg) {
|
||||
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") {
|
||||
if errStr, ok := msgMap["error"].(string); ok && isConflict(errStr) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
|
||||
return false
|
||||
return isConflict(errResp.Detail.Error)
|
||||
}
|
||||
|
||||
// handleCredentialAPIResponse handles API responses specifically for credential operations
|
||||
|
|
@ -242,14 +231,18 @@ func handleCredentialAPIResponse(resp *http.Response, result interface{}, client
|
|||
return fmt.Errorf("credential_not_found")
|
||||
}
|
||||
|
||||
if resp.StatusCode == http.StatusConflict {
|
||||
return errCredentialConflict
|
||||
}
|
||||
|
||||
if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated {
|
||||
var errResp ErrorResponse
|
||||
if err := json.Unmarshal(bodyBytes, &errResp); err == nil {
|
||||
if isCredentialNotFoundError(errResp) {
|
||||
return fmt.Errorf("credential_not_found")
|
||||
}
|
||||
if isCredentialConflictError(errResp) {
|
||||
return fmt.Errorf("credential_conflict")
|
||||
if isLegacyCredentialConflictError(errResp) {
|
||||
return errCredentialConflict
|
||||
}
|
||||
}
|
||||
return fmt.Errorf("API request failed: Status: %s, Response: %s",
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue