mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-09 03:18:44 +00:00
fix(ui): let the internal-issuer flow save before a federation rule id exists
The Add Provider wizard required anthropic_federation_rule_id up front, but with LiteLLM as the issuer that id does not exist yet: the operator gets it from Anthropic after registering the JWKS URL the credential itself publishes. So the step that needed a rule id could not be reached without one. Mark the field optional for wif_internal_issuer and let the JWKS step PATCH it in once it is known. The Keycloak flow is unchanged, where the rule id is known in advance.
This commit is contained in:
parent
d832b80403
commit
576ada8a70
8 changed files with 194 additions and 8 deletions
|
|
@ -372,7 +372,7 @@
|
|||
"key": "anthropic_federation_rule_id",
|
||||
"label": "Federation Rule ID",
|
||||
"placeholder": null,
|
||||
"tooltip": "The workload identity federation rule id, created in the Anthropic Console.",
|
||||
"tooltip": "The workload identity federation rule id, created in the Anthropic Console. With the LiteLLM-signed identity source you can leave this blank and fill it in on the next step, once the generated JWKS has been registered.",
|
||||
"required": true,
|
||||
"field_type": "text",
|
||||
"options": null,
|
||||
|
|
@ -577,6 +577,9 @@
|
|||
"anthropic_issuer_ttl_seconds",
|
||||
"anthropic_issuer_signing_key_ref"
|
||||
],
|
||||
"optional_field_keys": [
|
||||
"anthropic_federation_rule_id"
|
||||
],
|
||||
"fixed_values": {
|
||||
"anthropic_identity_source": "internal_issuer"
|
||||
}
|
||||
|
|
|
|||
|
|
@ -31,23 +31,31 @@ class ProviderCredentialVariant(BaseModel):
|
|||
federation, Keycloak'. ``field_keys`` names entries in the parent
|
||||
``ProviderCredentialVariants.field_definitions`` to mount when this variant is active;
|
||||
``fixed_values`` are litellm_params values the variant implies (e.g. a discriminator like
|
||||
``anthropic_identity_source: keycloak``) and are submitted without a form field for them."""
|
||||
``anthropic_identity_source: keycloak``) and are submitted without a form field for them.
|
||||
``optional_field_keys`` relaxes a globally-required field for this variant alone, for a value
|
||||
only obtainable after the credential exists (the federation rule id an operator can only read
|
||||
off the Anthropic Console once the generated JWKS is registered)."""
|
||||
|
||||
id: str
|
||||
label: str
|
||||
field_keys: tuple[str, ...]
|
||||
optional_field_keys: tuple[str, ...] = ()
|
||||
fixed_values: Mapping[str, str] = Field(default_factory=dict)
|
||||
|
||||
|
||||
def _validate_variant(variant: "ProviderCredentialVariant", defined_keys: frozenset[str]) -> None:
|
||||
"""Each variant may only reference declared fields, and a fixed value may not also be a field the
|
||||
form would mount, or the form and the payload would disagree about who owns that key."""
|
||||
"""Each variant may only reference declared fields, may only relax fields it actually mounts, and a
|
||||
fixed value may not also be a field the form would mount, or the form and the payload would
|
||||
disagree about who owns that key."""
|
||||
unresolved: Final = tuple(key for key in variant.field_keys if key not in defined_keys)
|
||||
if unresolved:
|
||||
raise ValueError(f"variant {variant.id!r} references undefined field_keys: {unresolved}")
|
||||
overlap: Final = sorted(frozenset(variant.field_keys) & frozenset(variant.fixed_values))
|
||||
if overlap:
|
||||
raise ValueError(f"variant {variant.id!r} has fixed_values overlapping field_keys: {overlap}")
|
||||
unmounted: Final = tuple(key for key in variant.optional_field_keys if key not in frozenset(variant.field_keys))
|
||||
if unmounted:
|
||||
raise ValueError(f"variant {variant.id!r} relaxes optional_field_keys it does not mount: {unmounted}")
|
||||
|
||||
|
||||
class ProviderCredentialVariants(BaseModel):
|
||||
|
|
|
|||
|
|
@ -1171,3 +1171,36 @@ def test_provider_fields_without_credential_variants_still_parse():
|
|||
openai = next((p for p in providers if p["provider"] == "OpenAI"), None)
|
||||
assert openai is not None
|
||||
assert openai.get("credential_variants") is None
|
||||
|
||||
|
||||
def test_credential_variants_rejects_optional_field_keys_the_variant_does_not_mount():
|
||||
with pytest.raises(ValidationError, match="optional_field_keys it does not mount"):
|
||||
ProviderCredentialVariants(
|
||||
selector_label="Auth method",
|
||||
default_variant="a",
|
||||
field_definitions=[_field("x"), _field("y")],
|
||||
variants=[ProviderCredentialVariant(id="a", label="A", field_keys=["x"], optional_field_keys=["y"])],
|
||||
)
|
||||
|
||||
|
||||
def test_anthropic_internal_issuer_variant_relaxes_only_the_federation_rule_id():
|
||||
"""The federation rule id is read off the Anthropic Console only after the generated JWKS is
|
||||
registered, and the JWKS only exists once the credential is saved, so demanding it up front
|
||||
would deadlock first-time setup of the LiteLLM-signed variant. Every other variant collects it
|
||||
on the one and only form it has, so there it stays required."""
|
||||
app_instance = FastAPI()
|
||||
app_instance.include_router(router)
|
||||
test_client = TestClient(app_instance)
|
||||
|
||||
response = test_client.get("/public/providers/fields")
|
||||
assert response.status_code == 200
|
||||
|
||||
anthropic = next(p for p in response.json() if p["provider"] == "Anthropic")
|
||||
variants_block = anthropic["credential_variants"]
|
||||
field_defs_by_key = {f["key"]: f for f in variants_block["field_definitions"]}
|
||||
variants_by_id = {v["id"]: v for v in variants_block["variants"]}
|
||||
|
||||
assert field_defs_by_key["anthropic_federation_rule_id"]["required"] is True
|
||||
assert variants_by_id["wif_internal_issuer"]["optional_field_keys"] == ["anthropic_federation_rule_id"]
|
||||
for variant_id in ("api_key", "wif_token", "wif_token_file", "wif_keycloak"):
|
||||
assert variants_by_id[variant_id]["optional_field_keys"] == []
|
||||
|
|
|
|||
|
|
@ -1,4 +1,11 @@
|
|||
import { renderWithProviders, screen, waitFor, within } from "../../../../../../tests/test-utils";
|
||||
import {
|
||||
chooseSelectOption,
|
||||
fireEvent,
|
||||
renderWithProviders,
|
||||
screen,
|
||||
waitFor,
|
||||
within,
|
||||
} from "../../../../../../tests/test-utils";
|
||||
import userEvent, { PointerEventsCheckLevel } from "@testing-library/user-event";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import AddProviderPanel from "./AddProviderPanel";
|
||||
|
|
@ -10,6 +17,7 @@ const createProviderModelCall = vi.fn();
|
|||
const listAllModelsCall = vi.fn();
|
||||
const getCallbacksCall = vi.fn();
|
||||
const setCallbacksCall = vi.fn();
|
||||
const getCredentialJwksCall = vi.fn();
|
||||
const mockAuthorized = vi.fn();
|
||||
|
||||
vi.mock("@/components/networking", async (importOriginal) => {
|
||||
|
|
@ -23,6 +31,7 @@ vi.mock("@/components/networking", async (importOriginal) => {
|
|||
listAllModelsCall: (...args: unknown[]) => listAllModelsCall(...args),
|
||||
getCallbacksCall: (...args: unknown[]) => getCallbacksCall(...args),
|
||||
setCallbacksCall: (...args: unknown[]) => setCallbacksCall(...args),
|
||||
getCredentialJwksCall: (...args: unknown[]) => getCredentialJwksCall(...args),
|
||||
};
|
||||
});
|
||||
|
||||
|
|
@ -50,8 +59,39 @@ vi.mock("@/app/(dashboard)/hooks/providers/useProviderFields", () => ({
|
|||
field_definitions: [
|
||||
{ key: "api_base", label: "Upstream API Base", field_type: "text" },
|
||||
{ key: "api_key", label: "API Key", field_type: "password" },
|
||||
{
|
||||
key: "anthropic_federation_rule_id",
|
||||
label: "Federation Rule ID",
|
||||
field_type: "text",
|
||||
required: true,
|
||||
tooltip: "Can be left blank and filled in once the JWKS is registered.",
|
||||
},
|
||||
{ key: "anthropic_organization_id", label: "Organization ID", field_type: "text", required: true },
|
||||
{ key: "anthropic_issuer_url", label: "Issuer URL", field_type: "text", required: true },
|
||||
{ key: "anthropic_issuer_subject", label: "Issuer Subject", field_type: "text", required: true },
|
||||
{
|
||||
key: "anthropic_issuer_signing_key_ref",
|
||||
label: "Signing Key Reference",
|
||||
field_type: "text",
|
||||
required: true,
|
||||
},
|
||||
],
|
||||
variants: [
|
||||
{ id: "api_key", label: "API Key", field_keys: ["api_base", "api_key"], fixed_values: {} },
|
||||
{
|
||||
id: "wif_internal_issuer",
|
||||
label: "Workload Identity Federation (LiteLLM-signed)",
|
||||
field_keys: [
|
||||
"anthropic_federation_rule_id",
|
||||
"anthropic_organization_id",
|
||||
"anthropic_issuer_url",
|
||||
"anthropic_issuer_subject",
|
||||
"anthropic_issuer_signing_key_ref",
|
||||
],
|
||||
optional_field_keys: ["anthropic_federation_rule_id"],
|
||||
fixed_values: { anthropic_identity_source: "internal_issuer" },
|
||||
},
|
||||
],
|
||||
variants: [{ id: "api_key", label: "API Key", field_keys: ["api_base", "api_key"], fixed_values: {} }],
|
||||
},
|
||||
},
|
||||
],
|
||||
|
|
@ -86,6 +126,7 @@ describe("AddProviderPanel", () => {
|
|||
createProviderModelCall.mockResolvedValue({ model_id: "new-id" });
|
||||
getCallbacksCall.mockResolvedValue({ router_settings: {} });
|
||||
setCallbacksCall.mockResolvedValue({});
|
||||
getCredentialJwksCall.mockResolvedValue({ keys: [{ kid: "kid-1", kty: "RSA", n: "n", e: "AQAB" }] });
|
||||
});
|
||||
|
||||
it("walks provider -> credential -> discover -> review -> create, with blocked and aliases wired correctly", async () => {
|
||||
|
|
@ -238,4 +279,58 @@ describe("AddProviderPanel", () => {
|
|||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("saves a LiteLLM-signed credential with a blank federation rule id, then PATCHes it from the JWKS step", async () => {
|
||||
discoverProviderModelsCall.mockResolvedValue({ models: ["claude-3-opus"] });
|
||||
const { user } = await setup();
|
||||
|
||||
await chooseProvider(user, "Anthropic");
|
||||
await user.type(screen.getByLabelText("Credential name"), "anthropic-wif");
|
||||
await user.click(screen.getByRole("button", { name: /Next/ }));
|
||||
|
||||
await chooseSelectOption(
|
||||
user,
|
||||
await screen.findByRole("combobox", { name: "Authentication method" }),
|
||||
"Workload Identity Federation (LiteLLM-signed)",
|
||||
);
|
||||
|
||||
fireEvent.change(await screen.findByLabelText("Organization ID"), { target: { value: "org-1" } });
|
||||
fireEvent.change(screen.getByLabelText("Issuer URL"), { target: { value: "https://proxy.example.com" } });
|
||||
fireEvent.change(screen.getByLabelText("Issuer Subject"), { target: { value: "litellm-proxy" } });
|
||||
fireEvent.change(screen.getByLabelText("Signing Key Reference"), { target: { value: "os.environ/SIGNING_KEY" } });
|
||||
expect(screen.getByLabelText("Federation Rule ID")).toHaveValue("");
|
||||
|
||||
await user.click(screen.getByRole("button", { name: "Save credential" }));
|
||||
|
||||
// The rule id is only readable off the Anthropic Console once the JWKS below is registered,
|
||||
// and the JWKS only exists once the credential is saved, so saving must not demand it first.
|
||||
await waitFor(() =>
|
||||
expect(credentialCreateCall).toHaveBeenCalledWith("test-access-token", {
|
||||
credential_name: "anthropic-wif",
|
||||
credential_values: {
|
||||
anthropic_organization_id: "org-1",
|
||||
anthropic_issuer_url: "https://proxy.example.com",
|
||||
anthropic_issuer_subject: "litellm-proxy",
|
||||
anthropic_issuer_signing_key_ref: "os.environ/SIGNING_KEY",
|
||||
anthropic_identity_source: "internal_issuer",
|
||||
},
|
||||
credential_info: { custom_llm_provider: "anthropic" },
|
||||
}),
|
||||
);
|
||||
|
||||
expect(await screen.findByText("Register this JWKS with Anthropic")).toBeInTheDocument();
|
||||
expect(getCredentialJwksCall).toHaveBeenCalledWith("test-access-token", "anthropic-wif");
|
||||
|
||||
fireEvent.change(screen.getByLabelText("Federation Rule ID"), { target: { value: "rule-abc" } });
|
||||
await user.click(screen.getByRole("button", { name: /Next/ }));
|
||||
|
||||
await waitFor(() =>
|
||||
expect(credentialUpdateCall).toHaveBeenCalledWith("test-access-token", "anthropic-wif", {
|
||||
credential_name: "anthropic-wif",
|
||||
credential_values: { anthropic_federation_rule_id: "rule-abc" },
|
||||
credential_info: { custom_llm_provider: "anthropic" },
|
||||
}),
|
||||
);
|
||||
expect(await screen.findByText("claude-3-opus")).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -48,6 +48,7 @@ const anthropicVariants: ProviderCredentialVariants = {
|
|||
"anthropic_issuer_url",
|
||||
"anthropic_issuer_signing_key_ref",
|
||||
],
|
||||
optional_field_keys: ["anthropic_federation_rule_id"],
|
||||
fixed_values: { anthropic_identity_source: "internal_issuer" },
|
||||
},
|
||||
{
|
||||
|
|
@ -96,6 +97,23 @@ describe("resolveVariantFieldDefs", () => {
|
|||
};
|
||||
expect(resolveVariantFieldDefs(variants, "broken").map((f) => f.key)).toEqual(["api_key"]);
|
||||
});
|
||||
|
||||
it("relaxes a globally-required field the variant lists in optional_field_keys", () => {
|
||||
const fields = resolveVariantFieldDefs(anthropicVariants, "wif_internal_issuer");
|
||||
expect(fields.find((f) => f.key === "anthropic_federation_rule_id")?.required).toBe(false);
|
||||
expect(fields.find((f) => f.key === "anthropic_organization_id")?.required).toBe(true);
|
||||
});
|
||||
|
||||
it("keeps the same field required on a variant that does not relax it", () => {
|
||||
const fields = resolveVariantFieldDefs(anthropicVariants, "wif_keycloak");
|
||||
expect(fields.find((f) => f.key === "anthropic_federation_rule_id")?.required).toBe(true);
|
||||
});
|
||||
|
||||
it("relaxes a copy, leaving the shared field_definitions entry untouched", () => {
|
||||
resolveVariantFieldDefs(anthropicVariants, "wif_internal_issuer");
|
||||
const shared = anthropicVariants.field_definitions.find((f) => f.key === "anthropic_federation_rule_id");
|
||||
expect(shared?.required).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("inferActiveVariant", () => {
|
||||
|
|
@ -154,4 +172,14 @@ describe("inferActiveVariant", () => {
|
|||
const values = { api_base: "https://api.anthropic.com" };
|
||||
expect(inferActiveVariant(anthropicVariants, values)).toBe("api_key");
|
||||
});
|
||||
|
||||
it("still matches wif_internal_issuer while its relaxed federation rule id is unset", () => {
|
||||
const values = {
|
||||
anthropic_identity_source: "internal_issuer",
|
||||
anthropic_organization_id: "org-1",
|
||||
anthropic_issuer_url: "https://issuer.example.com",
|
||||
anthropic_issuer_signing_key_ref: "os.environ/SIGNING_KEY",
|
||||
};
|
||||
expect(inferActiveVariant(anthropicVariants, values)).toBe("wif_internal_issuer");
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -9,6 +9,11 @@ export const getVariant = (
|
|||
variantId: string,
|
||||
): ProviderCredentialVariant | undefined => variants.variants.find((variant) => variant.id === variantId);
|
||||
|
||||
/**
|
||||
* A variant may relax a field_definition's global `required` for itself alone, when the value is
|
||||
* only obtainable after the credential exists (see `optional_field_keys` on the API model), so the
|
||||
* relaxation is applied here rather than by every caller that reads a field's `required`.
|
||||
*/
|
||||
export const resolveVariantFieldDefs = (
|
||||
variants: ProviderCredentialVariants,
|
||||
variantId: string,
|
||||
|
|
@ -18,9 +23,11 @@ export const resolveVariantFieldDefs = (
|
|||
return [];
|
||||
}
|
||||
const byKey = new Map(variants.field_definitions.map((field) => [field.key, field]));
|
||||
const optionalKeys = new Set(variant.optional_field_keys ?? []);
|
||||
return variant.field_keys
|
||||
.map((key) => byKey.get(key))
|
||||
.filter((field): field is ProviderCredentialFieldMetadata => field !== undefined);
|
||||
.filter((field): field is ProviderCredentialFieldMetadata => field !== undefined)
|
||||
.map((field) => (optionalKeys.has(field.key) ? { ...field, required: false } : field));
|
||||
};
|
||||
|
||||
const hasValue = (value: unknown): boolean => (typeof value === "string" ? value.trim() !== "" : value != null);
|
||||
|
|
@ -34,7 +41,10 @@ const isFullySatisfied = (
|
|||
if (!fixedValuesMatch) {
|
||||
return false;
|
||||
}
|
||||
return variant.field_keys.every((key) => !fieldsByKey.get(key)?.required || hasValue(values[key]));
|
||||
const optionalKeys = new Set(variant.optional_field_keys ?? []);
|
||||
return variant.field_keys.every(
|
||||
(key) => optionalKeys.has(key) || !fieldsByKey.get(key)?.required || hasValue(values[key]),
|
||||
);
|
||||
};
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -279,6 +279,7 @@ export interface ProviderCredentialVariant {
|
|||
id: string;
|
||||
label: string;
|
||||
field_keys: string[];
|
||||
optional_field_keys?: string[];
|
||||
fixed_values: Record<string, string>;
|
||||
}
|
||||
|
||||
|
|
|
|||
8
ui/litellm-dashboard/src/lib/http/schema.d.ts
generated
vendored
8
ui/litellm-dashboard/src/lib/http/schema.d.ts
generated
vendored
|
|
@ -32022,6 +32022,9 @@ export interface components {
|
|||
* ``ProviderCredentialVariants.field_definitions`` to mount when this variant is active;
|
||||
* ``fixed_values`` are litellm_params values the variant implies (e.g. a discriminator like
|
||||
* ``anthropic_identity_source: keycloak``) and are submitted without a form field for them.
|
||||
* ``optional_field_keys`` relaxes a globally-required field for this variant alone, for a value
|
||||
* only obtainable after the credential exists (the federation rule id an operator can only read
|
||||
* off the Anthropic Console once the generated JWKS is registered).
|
||||
*/
|
||||
ProviderCredentialVariant: {
|
||||
/** Field Keys */
|
||||
|
|
@ -32034,6 +32037,11 @@ export interface components {
|
|||
id: string;
|
||||
/** Label */
|
||||
label: string;
|
||||
/**
|
||||
* Optional Field Keys
|
||||
* @default []
|
||||
*/
|
||||
optional_field_keys: string[];
|
||||
};
|
||||
/**
|
||||
* ProviderCredentialVariants
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue