From 79d7f81ce2278fcdc70ca58e0615fb1bf5d2317c Mon Sep 17 00:00:00 2001 From: Ashish Ranjan <155017344+ashish6123@users.noreply.github.com> Date: Fri, 6 Nov 2026 01:01:27 +0530 Subject: [PATCH] fix(terraform): track email-only org members by resolved user_id --- terraform/provider/CHANGELOG.md | 2 +- .../resource_organization_member_add.go | 42 +++- ...resource_organization_member_drift_test.go | 237 +++++++++++++++++- .../resource_organization_member_utils.go | 15 ++ 4 files changed, 277 insertions(+), 19 deletions(-) diff --git a/terraform/provider/CHANGELOG.md b/terraform/provider/CHANGELOG.md index 8ebc413667a..835dca0b9f6 100644 --- a/terraform/provider/CHANGELOG.md +++ b/terraform/provider/CHANGELOG.md @@ -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 diff --git a/terraform/provider/litellm/resource_organization_member_add.go b/terraform/provider/litellm/resource_organization_member_add.go index e95166a18cf..0bb50736142 100644 --- a/terraform/provider/litellm/resource_organization_member_add.go +++ b/terraform/provider/litellm/resource_organization_member_add.go @@ -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 } diff --git a/terraform/provider/litellm/resource_organization_member_drift_test.go b/terraform/provider/litellm/resource_organization_member_drift_test.go index 231abc24a3a..664b57a3c05 100644 --- a/terraform/provider/litellm/resource_organization_member_drift_test.go +++ b/terraform/provider/litellm/resource_organization_member_drift_test.go @@ -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) + } +} diff --git a/terraform/provider/litellm/resource_organization_member_utils.go b/terraform/provider/litellm/resource_organization_member_utils.go index a7de8981b3c..87c77845ace 100644 --- a/terraform/provider/litellm/resource_organization_member_utils.go +++ b/terraform/provider/litellm/resource_organization_member_utils.go @@ -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