fix(provider): accept 2xx status codes in unified_access_group create (#42461)

* fix(provider): accept 2xx status in handleResponse for access group create

handleResponse (resource_team.go) only accepted exactly HTTP 200, but
POST /v1/access_group (and its /v1/unified_access_group alias) legitimately
answers 201 Created. litellm_access_group and litellm_unified_access_group
create both succeeded on the proxy and failed in the provider, leaving the
group out of state and forcing an import to recover on the next apply's
409 for the now-duplicate name.

Same fix and shape as #40723, which widened this exact check in
sendRequest/handleAPIResponse/handleMCPAPIResponse for mcp_server, model,
key and organization_member. handleResponse is the one shared status-check
helper that fix didn't reach -- it's a different function in a different
file (resource_team.go, not client.go/utils.go), so this is fully
independent of that PR and can land before, after, or alongside it with
no conflict.

handleResponse is also used by agent, budget, guardrail, organization,
prompt, search_tool, tag, team, team_block, key_block, team_member(_add)
and user -- all unaffected in practice, since every one of their own
endpoints already answers exactly 200. Widening the check costs them
nothing and only changes behavior for the two resources that were
actually broken.

Verified: go test ./... passes, including a new
TestHandleResponseAcceptsFullSuccessRange table test covering
200/201/202/204 (accepted) and 400/404/409/500 (still rejected), mirroring
#40723's own TestHandleAPIResponseAcceptsFullSuccessRange.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix: address Greptile review on PR 42461

- CHANGELOG: narrowed the fix's scope to unified_access_group only.
  litellm_access_group (legacy) calls /access_group/new, a completely
  different, unrelated endpoint (model_access_group_management_endpoints.py)
  that already returns 200 -- it was never affected. I'd wrongly assumed
  both resources shared the same /v1/access_group route; they don't.
- resource_team_test.go: removed the preamble comment above the new test,
  which restated what the table test already shows -- against repository
  guidance (AGENTS.md) that reserves comments for complex logic, tool
  inputs, or TODOs.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* chore: retrigger CI

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Louis Vauterin 2026-09-28 22:54:37 +02:00 • committed by GitHub
parent 6d4ccf7e97
commit b9ba36c231
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 40 additions and 1 deletions

View file

@ -60,6 +60,7 @@ longer signal it.
- **key**: Updates no longer send an empty `budget_duration`, which the proxy rejects with a 400; any update to a key without a configured `budget_duration` previously failed outright
- **key**: A config-supplied `key` value (write-only) is now forwarded to `/key/generate`; previously it was silently dropped and the proxy generated a random key instead
- **security**: The `litellm_key` data source and `litellm_key_block` resource normalize raw `sk-` keys to their SHA-256 token hash before building request URLs and resource IDs, so plaintext keys no longer land in reverse-proxy access logs, Terraform plan output, or state IDs
- **unified_access_group**: create now accepts any 2xx response instead of requiring exactly HTTP 200; `POST /v1/unified_access_group` legitimately returns 201, so creation previously succeeded on the proxy but failed in the provider, leaving the group out of state and forcing a `terraform import` to recover on the next apply's 409. Same fix and shape as the one already applied to `mcp_server`/`model`/`key`/`organization_member`; `handleResponse` (shared by several other resources) was the one status-check helper that fix didn't reach. The legacy `litellm_access_group` resource calls the unrelated `/access_group/new` endpoint, which already returns 200, so it was never affected
### Changed

View file

@ -466,7 +466,7 @@ func toStringSlice(v interface{}) []string {
}
func handleResponse(resp *http.Response, action string) error {
if resp.StatusCode != http.StatusOK {
if resp.StatusCode < 200 || resp.StatusCode >= 300 {
body, _ := io.ReadAll(resp.Body)
return fmt.Errorf("error %s: %s - %s", action, resp.Status, string(body))
}

View file

@ -434,3 +434,41 @@ func TestTeamLimitTypesSentOnCreateOnly(t *testing.T) {
}
}
}
func TestHandleResponseAcceptsFullSuccessRange(t *testing.T) {
tests := []struct {
name string
statusCode int
wantErr bool
}{
{name: "200 OK", statusCode: http.StatusOK, wantErr: false},
{name: "201 Created", statusCode: http.StatusCreated, wantErr: false},
{name: "202 Accepted", statusCode: http.StatusAccepted, wantErr: false},
{name: "204 No Content", statusCode: http.StatusNoContent, wantErr: false},
{name: "400 Bad Request", statusCode: http.StatusBadRequest, wantErr: true},
{name: "404 Not Found", statusCode: http.StatusNotFound, wantErr: true},
{name: "409 Conflict", statusCode: http.StatusConflict, wantErr: true},
{name: "500 Internal Server Error", statusCode: http.StatusInternalServerError, wantErr: true},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
rec := httptest.NewRecorder()
rec.WriteHeader(tt.statusCode)
rec.WriteString(`{"access_group_id":"ag-1","access_group_name":"uag-baseline"}`)
resp := rec.Result()
err := handleResponse(resp, "creating unified access group")
if tt.wantErr {
if err == nil {
t.Fatalf("handleResponse returned no error for status %d", tt.statusCode)
}
return
}
if err != nil {
t.Fatalf("handleResponse returned unexpected error for status %d: %v", tt.statusCode, err)
}
})
}
}