fix(terraform): track email-only org members by resolved user_id

This commit is contained in:
Ashish Ranjan 2026-11-06 01:01:27 +05:30
parent 67738a20aa
commit 79d7f81ce2
4 changed files with 277 additions and 19 deletions

View file

@ -51,7 +51,7 @@ longer signal it.
### Fixed
- **organization_member, organization_member_add**: Refresh now reads `GET /organization/info` instead of returning the state as-is, so a member removed outside Terraform (UI or `/organization/member_delete`) leaves state and the next apply adds it back, and a role changed out of band shows as drift. Members are matched by `user_id`, or by `user_email` (case-insensitive) for email-only members, whose `user_id` the proxy mints. A deleted organization removes its members from state. Deleting a member that is already gone, or whose organization was deleted, now succeeds instead of failing every destroy
- **organization_member, organization_member_add**: Refresh now reads `GET /organization/info` instead of returning the state as-is, so a member removed outside Terraform (UI or `/organization/member_delete`) leaves state and the next apply adds it back, and a role changed out of band shows as drift. Members are matched by `user_id`, or by `user_email` (case-insensitive) for email-only members, whose `user_id` the proxy mints. A deleted organization removes its members from state. Deleting a member that is already gone, or whose organization was deleted, now succeeds instead of failing every destroy. An email-only member is tracked by the `user_id` the proxy resolved, kept in the new computed `member_user_ids` map on `litellm_organization_member_add`, so changing their email outside Terraform no longer drops them from state and removal or destroy still revokes their access
- **model**: `additional_drop_params` in `litellm_model.additional_litellm_params` is now sent to the proxy as the deployment's `additional_drop_params` list, the same list a `model_list` entry takes. Before, the provider never sent the list and instead deleted every key it named from `litellm_params`, so a `store` or `vertex_location` listed there was lost and requests still carried the parameters (#38349). The value must be a JSON list of parameter names such as `jsonencode(["store"])`, which `terraform plan` now checks, and `null` entries are skipped as before. When the configuration sets the list, refresh reads the stored list back from the proxy, so a list changed outside Terraform shows up as drift. On a model an older provider created, the proxy has no stored list, so the first plan after upgrading shows the list and every listed key the configuration sets as changes, since that apply writes them. A write-only secret such as `model_api_key_wo` is never shown in a plan
- **user**: `litellm_user` updates no longer send `blocked`, which `/user/update` writes into the user table where no such column exists, so every update failed with a 400 (`Could not find field at upsertOneLiteLLM_UserTable.create.blocked`). Create still sends `blocked`, including `false`, so a `default_internal_user_params.blocked` on the proxy cannot block the auto-created key. Changing `blocked` after creation has no effect, since the proxy has no user-level block; use `litellm_key_block` instead
- **model**: `litellm_model` refresh now reads the `{"data": [...]}` envelope `/model/info` returns, so `model_info` fields changed outside Terraform show up as drift instead of silently keeping the previous state

View file

@ -3,6 +3,7 @@ package litellm
import (
"fmt"
"log"
"strings"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
@ -46,6 +47,11 @@ func resourceLiteLLMOrganizationMemberAdd() *schema.Resource {
},
},
},
"member_user_ids": {
Type: schema.TypeMap,
Computed: true,
Elem: &schema.Schema{Type: schema.TypeString},
},
},
}
}
@ -109,27 +115,39 @@ func resourceLiteLLMOrganizationMemberAddRead(d *schema.ResourceData, m interfac
// Keep only the configured members that are still in the organization,
// each with the role the proxy reports. Members removed out of band drop
// out of state, so the next apply adds them back.
resolvedUserIDs := d.Get("member_user_ids").(map[string]interface{})
current := make([]interface{}, 0)
currentUserIDs := make(map[string]interface{})
for _, member := range d.Get("member").(*schema.Set).List() {
configured := member.(map[string]interface{})
userID, _ := configured["user_id"].(string)
userEmail, _ := configured["user_email"].(string)
configuredID, _ := configured["user_id"].(string)
configuredEmail, _ := configured["user_email"].(string)
userID, userEmail := orgMemberIdentity(configured, resolvedUserIDs)
existing, ok := findOrgMember(members, userID, userEmail)
if !ok && configuredID == "" {
existing, ok = findOrgMember(members, "", configuredEmail)
}
if !ok {
log.Printf("[WARN] Organization member (user_id=%q, user_email=%q) not found in organization %s", userID, userEmail, orgID)
log.Printf("[WARN] Organization member (user_id=%q, user_email=%q) not found in organization %s", configuredID, configuredEmail, orgID)
continue
}
current = append(current, map[string]interface{}{
"user_id": userID,
"user_email": userEmail,
"user_id": configuredID,
"user_email": configuredEmail,
"role": existing.Role,
})
if configuredID == "" {
currentUserIDs[strings.ToLower(configuredEmail)] = existing.UserID
}
}
if err := d.Set("member", current); err != nil {
return fmt.Errorf("error setting member: %w", err)
}
if err := d.Set("member_user_ids", currentUserIDs); err != nil {
return fmt.Errorf("error setting member_user_ids: %w", err)
}
return nil
}
@ -140,6 +158,7 @@ func resourceLiteLLMOrganizationMemberAddUpdate(d *schema.ResourceData, m interf
o, n := d.GetChange("member")
oldMembers := o.(*schema.Set)
newMembers := n.(*schema.Set)
resolvedUserIDs := d.Get("member_user_ids").(map[string]interface{})
// Create maps for easier lookup by user identifier
oldMemberMap := make(map[string]map[string]interface{})
@ -166,8 +185,7 @@ func resourceLiteLLMOrganizationMemberAddUpdate(d *schema.ResourceData, m interf
// Find members to delete (in old but not in new)
for key, oldMember := range oldMemberMap {
if _, exists := newMemberMap[key]; !exists {
userID, _ := oldMember["user_id"].(string)
userEmail, _ := oldMember["user_email"].(string)
userID, userEmail := orgMemberIdentity(oldMember, resolvedUserIDs)
if err := deleteOrganizationMember(client, orgID, userID, userEmail); err != nil {
return err
}
@ -183,10 +201,11 @@ func resourceLiteLLMOrganizationMemberAddUpdate(d *schema.ResourceData, m interf
"organization_id": orgID,
"role": newMember["role"].(string),
}
if userID, ok := newMember["user_id"].(string); ok && userID != "" {
userID, userEmail := orgMemberIdentity(newMember, resolvedUserIDs)
if userID != "" {
updateData["user_id"] = userID
}
if userEmail, ok := newMember["user_email"].(string); ok && userEmail != "" {
if userEmail != "" {
updateData["user_email"] = userEmail
}
@ -259,12 +278,11 @@ func resourceLiteLLMOrganizationMemberAddDelete(d *schema.ResourceData, m interf
client := m.(*Client)
orgID := d.Get("organization_id").(string)
members := d.Get("member").(*schema.Set)
resolvedUserIDs := d.Get("member_user_ids").(map[string]interface{})
// Delete each member
for _, member := range members.List() {
m := member.(map[string]interface{})
userID, _ := m["user_id"].(string)
userEmail, _ := m["user_email"].(string)
userID, userEmail := orgMemberIdentity(member.(map[string]interface{}), resolvedUserIDs)
if err := deleteOrganizationMember(client, orgID, userID, userEmail); err != nil {
return err
}

View file

@ -1,23 +1,35 @@
package litellm
import (
"context"
"encoding/json"
"net/http"
"net/http/httptest"
"testing"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
)
// fakeOrgProxy serves GET /organization/info and DELETE /organization/member_delete
// for a single organization, so Read and Delete can be checked against drift.
type fakeOrgProxy struct {
t *testing.T
orgID string
orgExists bool
members []map[string]interface{}
deleteStatus int
deleteCalls int
t *testing.T
orgID string
orgExists bool
members []map[string]interface{}
deleteStatus int
deleteCalls int
deleteBodies []map[string]interface{}
updatedBodies []map[string]interface{}
}
func decodeBody(t *testing.T, r *http.Request) map[string]interface{} {
var body map[string]interface{}
if err := json.NewDecoder(r.Body).Decode(&body); err != nil {
t.Errorf("decoding %s body: %v", r.URL.Path, err)
}
return body
}
func (p *fakeOrgProxy) serve() *httptest.Server {
@ -39,12 +51,16 @@ func (p *fakeOrgProxy) serve() *httptest.Server {
})
case r.Method == http.MethodDelete && r.URL.Path == "/organization/member_delete":
p.deleteCalls++
p.deleteBodies = append(p.deleteBodies, decodeBody(p.t, r))
status := p.deleteStatus
if status == 0 {
status = http.StatusOK
}
w.WriteHeader(status)
w.Write([]byte(`{}`))
case r.Method == http.MethodPatch && r.URL.Path == "/organization/member_update":
p.updatedBodies = append(p.updatedBodies, decodeBody(p.t, r))
w.Write([]byte(`{}`))
default:
p.t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path)
w.WriteHeader(http.StatusNotFound)
@ -246,3 +262,212 @@ func TestOrganizationMemberAddDeleteSkipsMembersAlreadyRemoved(t *testing.T) {
t.Fatalf("expected 2 member_delete calls, got %d", proxy.deleteCalls)
}
}
func emailOnlyMember(email, role string) map[string]interface{} {
return map[string]interface{}{"user_email": email, "role": role}
}
// refreshedEmailOnlyState applies the config, reads it back from the proxy, and
// returns the state Terraform would persist.
func refreshedEmailOnlyState(t *testing.T, client *Client, members ...interface{}) *terraform.InstanceState {
d := schema.TestResourceDataRaw(t, resourceLiteLLMOrganizationMemberAdd().Schema, map[string]interface{}{
"organization_id": "org-1",
"member": members,
})
d.SetId("org-1")
if err := resourceLiteLLMOrganizationMemberAddRead(d, client); err != nil {
t.Fatalf("read failed: %v", err)
}
return d.State()
}
func orgMemberAddDataFromState(t *testing.T, state *terraform.InstanceState) *schema.ResourceData {
d, err := schema.InternalMap(resourceLiteLLMOrganizationMemberAdd().Schema).Data(state, nil)
if err != nil {
t.Fatalf("loading state: %v", err)
}
return d
}
func orgMemberAddDataWithConfig(t *testing.T, state *terraform.InstanceState, members ...interface{}) *schema.ResourceData {
internal := schema.InternalMap(resourceLiteLLMOrganizationMemberAdd().Schema)
config := terraform.NewResourceConfigRaw(map[string]interface{}{"organization_id": "org-1", "member": members})
diff, err := internal.Diff(context.Background(), state, config, nil, nil, true)
if err != nil {
t.Fatalf("diffing config: %v", err)
}
d, err := internal.Data(state, diff)
if err != nil {
t.Fatalf("loading state with diff: %v", err)
}
return d
}
func TestOrganizationMemberAddReadKeepsEmailOnlyMemberAfterEmailChange(t *testing.T) {
proxy := &fakeOrgProxy{t: t, orgID: "org-1", orgExists: true, members: []map[string]interface{}{
orgMemberRow("generated-id", "bob@example.com", "internal_user"),
}}
srv := proxy.serve()
defer srv.Close()
client := NewClient(srv.URL, "test-key", true)
state := refreshedEmailOnlyState(t, client, emailOnlyMember("Bob@Example.com", "internal_user"))
proxy.members = []map[string]interface{}{orgMemberRow("generated-id", "bob-new@example.com", "internal_user_viewer")}
d := orgMemberAddDataFromState(t, state)
if err := resourceLiteLLMOrganizationMemberAddRead(d, client); err != nil {
t.Fatalf("read failed: %v", err)
}
list := d.Get("member").(*schema.Set).List()
if len(list) != 1 {
t.Fatalf("member whose email changed should stay in state, got %v", list)
}
member := list[0].(map[string]interface{})
if member["user_email"] != "Bob@Example.com" || member["role"] != "internal_user_viewer" {
t.Fatalf("member = %v, want the configured email with the proxy's role", member)
}
}
func TestOrganizationMemberAddReadFollowsEmailWhenResolvedIDIsGone(t *testing.T) {
proxy := &fakeOrgProxy{t: t, orgID: "org-1", orgExists: true, members: []map[string]interface{}{
orgMemberRow("old-id", "bob@example.com", "internal_user"),
}}
srv := proxy.serve()
defer srv.Close()
client := NewClient(srv.URL, "test-key", true)
state := refreshedEmailOnlyState(t, client, emailOnlyMember("bob@example.com", "internal_user"))
proxy.members = []map[string]interface{}{orgMemberRow("new-id", "bob@example.com", "internal_user")}
d := orgMemberAddDataFromState(t, state)
if err := resourceLiteLLMOrganizationMemberAddRead(d, client); err != nil {
t.Fatalf("read failed: %v", err)
}
if got := d.Get("member_user_ids").(map[string]interface{})["bob@example.com"]; got != "new-id" {
t.Fatalf("resolved user_id = %v, want new-id", got)
}
}
func TestOrganizationMemberAddReadDropsEmailOnlyMemberRemovedOutOfBand(t *testing.T) {
proxy := &fakeOrgProxy{t: t, orgID: "org-1", orgExists: true, members: []map[string]interface{}{
orgMemberRow("generated-id", "bob@example.com", "internal_user"),
}}
srv := proxy.serve()
defer srv.Close()
client := NewClient(srv.URL, "test-key", true)
state := refreshedEmailOnlyState(t, client, emailOnlyMember("bob@example.com", "internal_user"))
proxy.members = nil
d := orgMemberAddDataFromState(t, state)
if err := resourceLiteLLMOrganizationMemberAddRead(d, client); err != nil {
t.Fatalf("read failed: %v", err)
}
if got := d.Get("member").(*schema.Set).Len(); got != 0 {
t.Fatalf("member removed out of band should leave state, %d left", got)
}
if got := d.Get("member_user_ids").(map[string]interface{}); len(got) != 0 {
t.Fatalf("resolved user_ids of a removed member should be dropped, got %v", got)
}
}
func TestOrganizationMemberAddDeleteRevokesByResolvedIDAfterEmailChange(t *testing.T) {
proxy := &fakeOrgProxy{t: t, orgID: "org-1", orgExists: true, members: []map[string]interface{}{
orgMemberRow("generated-id", "bob@example.com", "internal_user"),
}}
srv := proxy.serve()
defer srv.Close()
client := NewClient(srv.URL, "test-key", true)
state := refreshedEmailOnlyState(t, client, emailOnlyMember("bob@example.com", "internal_user"))
proxy.members = []map[string]interface{}{orgMemberRow("generated-id", "bob-new@example.com", "internal_user")}
if err := resourceLiteLLMOrganizationMemberAddDelete(orgMemberAddDataFromState(t, state), client); err != nil {
t.Fatalf("delete failed: %v", err)
}
if len(proxy.deleteBodies) != 1 || proxy.deleteBodies[0]["user_id"] != "generated-id" {
t.Fatalf("delete bodies = %v, want one delete by generated-id", proxy.deleteBodies)
}
if _, sent := proxy.deleteBodies[0]["user_email"]; sent {
t.Fatalf("delete should not send the stale email, got %v", proxy.deleteBodies[0])
}
}
func TestOrganizationMemberAddDeleteKeepsErrorWhenEmailChangedMemberStillPresent(t *testing.T) {
proxy := &fakeOrgProxy{t: t, orgID: "org-1", orgExists: true, members: []map[string]interface{}{
orgMemberRow("generated-id", "bob@example.com", "internal_user"),
}}
srv := proxy.serve()
defer srv.Close()
client := NewClient(srv.URL, "test-key", true)
state := refreshedEmailOnlyState(t, client, emailOnlyMember("bob@example.com", "internal_user"))
proxy.members = []map[string]interface{}{orgMemberRow("generated-id", "bob-new@example.com", "internal_user")}
proxy.deleteStatus = http.StatusInternalServerError
if err := resourceLiteLLMOrganizationMemberAddDelete(orgMemberAddDataFromState(t, state), client); err == nil {
t.Fatalf("a failed delete of a member still in the organization must return an error")
}
}
func TestOrganizationMemberAddUpdateUsesResolvedIDAfterEmailChange(t *testing.T) {
proxy := &fakeOrgProxy{t: t, orgID: "org-1", orgExists: true, members: []map[string]interface{}{
orgMemberRow("generated-id", "bob@example.com", "internal_user"),
orgMemberRow("generated-id-2", "carol@example.com", "internal_user"),
}}
srv := proxy.serve()
defer srv.Close()
client := NewClient(srv.URL, "test-key", true)
state := refreshedEmailOnlyState(t, client,
emailOnlyMember("bob@example.com", "internal_user"),
emailOnlyMember("carol@example.com", "internal_user"),
)
proxy.members = []map[string]interface{}{
orgMemberRow("generated-id", "bob-new@example.com", "internal_user"),
orgMemberRow("generated-id-2", "carol-new@example.com", "internal_user"),
}
d := orgMemberAddDataWithConfig(t, state, emailOnlyMember("carol@example.com", "org_admin"))
if err := resourceLiteLLMOrganizationMemberAddUpdate(d, client); err != nil {
t.Fatalf("update failed: %v", err)
}
if len(proxy.deleteBodies) != 1 || proxy.deleteBodies[0]["user_id"] != "generated-id" {
t.Fatalf("delete bodies = %v, want one delete by generated-id", proxy.deleteBodies)
}
if len(proxy.updatedBodies) != 1 || proxy.updatedBodies[0]["user_id"] != "generated-id-2" {
t.Fatalf("update bodies = %v, want one update by generated-id-2", proxy.updatedBodies)
}
}
func TestOrganizationMemberAddPlansNoChangesAfterEmailChange(t *testing.T) {
proxy := &fakeOrgProxy{t: t, orgID: "org-1", orgExists: true, members: []map[string]interface{}{
orgMemberRow("generated-id", "bob@example.com", "internal_user"),
}}
srv := proxy.serve()
defer srv.Close()
client := NewClient(srv.URL, "test-key", true)
state := refreshedEmailOnlyState(t, client, emailOnlyMember("bob@example.com", "internal_user"))
proxy.members = []map[string]interface{}{orgMemberRow("generated-id", "bob-new@example.com", "internal_user")}
d := orgMemberAddDataFromState(t, state)
if err := resourceLiteLLMOrganizationMemberAddRead(d, client); err != nil {
t.Fatalf("read failed: %v", err)
}
config := terraform.NewResourceConfigRaw(map[string]interface{}{
"organization_id": "org-1",
"member": []interface{}{emailOnlyMember("bob@example.com", "internal_user")},
})
internal := schema.InternalMap(resourceLiteLLMOrganizationMemberAdd().Schema)
diff, err := internal.Diff(context.Background(), d.State(), config, nil, nil, true)
if err != nil {
t.Fatalf("diffing config: %v", err)
}
if diff != nil && !diff.Empty() {
t.Fatalf("plan after an email change should be empty, got %v", diff)
}
}

View file

@ -59,6 +59,21 @@ func findOrgMember(members []orgMembership, userID, userEmail string) (orgMember
return orgMembership{}, false
}
// orgMemberIdentity returns how to address a configured member on the proxy.
// An email-only member is addressed by the user_id the proxy resolved, because
// the email can change while the user_id and the membership stay the same.
func orgMemberIdentity(member, resolvedUserIDs map[string]interface{}) (userID, userEmail string) {
userID, _ = member["user_id"].(string)
userEmail, _ = member["user_email"].(string)
if userID != "" {
return userID, userEmail
}
if resolved, ok := resolvedUserIDs[strings.ToLower(userEmail)].(string); ok && resolved != "" {
return resolved, ""
}
return "", userEmail
}
// deleteOrganizationMember removes a member and treats a member (or
// organization) that is already gone as deleted. The proxy fails the delete
// in that case, which would otherwise block every destroy until the resource