-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
[Fix] Separate key expiration from budget reset logic #15136
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
[Fix] Separate key expiration from budget reset logic #15136
Conversation
|
@JVenberg is attempting to deploy a commit to the CLERKIEAI Team on Vercel. A member of the Team first needs to authorize it. |
|
@emerzon would you consider this a bug or expected behaviour? asking us i recall a lot of feedback around reset budgets needing to be at month start instead of 1 month from budget creation, wondering if something similar would apply here |
|
@krrishdholakia I can say this can be a bit confusing. maybe would make sense to have specific periods for relative times, like |
|
Thanks for the feedback, @krrishdholakia and @emerzon My understanding was that key expiration (for security) should be relative to the creation date, while budget resets (for finance) should be standardized to the calendar month Could you help me understand the use case for standardizing the key expiration as well? I'm trying to understand the scenario where a key with a "1 month" duration should expire in just a day if it's created at the end of a month |
|
I would concur with the current implementation not working as my team expected. I would not expect an virtual key set with an expiration date 30days in the future to expire 1 day from now. Which is currently happening in our environment. |
|
Thanks for working on this! I've run into the same problem - any updates on when this might be merged? With the budget reset in November approaching, a solution would be really appreciated! |
Title
Fix: Separate key expiration from budget reset logic
Relevant issues
Fixes key expiration bug where API keys expire at standardized intervals instead of actual duration from creation time.
Fixes: #15142
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
I have Added testing in the
tests/litellm/directory, Adding at least 1 test is a hard requirement - see detailstests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.pyI have added a screenshot of my new test passing locally
My PR passes all unit tests on
make test-unitMy PR's scope is as isolated as possible, it only solves 1 specific problem
Type
🐛 Bug Fix
Changes
Problem
Key expiration was incorrectly using budget reset logic (
get_budget_reset_time()), causing keys to expire at standardized intervals (e.g., 1st of next month) instead of the actual duration from creation time.Example of the bug:
duration="1mo"Why This Is Wrong
Key expiration and budget reset serve fundamentally different purposes and must use different calculation methods:
1. Budget Reset (Financial/Accounting)
get_budget_reset_time()for standardized intervals2. Key Expiration (Security/Access Control)
duration_in_seconds()with relative time calculationSolution
File:
litellm/proxy/management_endpoints/key_management_endpoints.py(lines 1678-1682)Changed key expiration calculation from:
To:
Note: Budget reset logic remains unchanged and continues to use
get_budget_reset_time()for standardized intervals.Testing
New Tests Added
test_key_expiration_calculated_from_current_timeexpiresis calculated ascurrent_time + duration(not standardized)budget_reset_atis standardized to 1st of next monthtest_key_expiration_with_various_durationstest_key_budget_reset_uses_standardized_timeUpdated Test
test_budget_reset_and_expires_at_first_of_monthexpiresto be on 1st of monthTest Results
$ poetry run pytest tests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.py::test_key_expiration_calculated_from_current_time tests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.py::test_key_expiration_with_various_durations tests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.py::test_key_budget_reset_uses_standardized_time tests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.py::test_budget_reset_and_expires_at_first_of_month -v ✅ test_budget_reset_and_expires_at_first_of_month PASSED ✅ test_key_budget_reset_uses_standardized_time PASSED ✅ test_key_expiration_calculated_from_current_time PASSED ✅ test_key_expiration_with_various_durations PASSED 4 passed in 2.19sFiles Changed
litellm/proxy/management_endpoints/key_management_endpoints.pytests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.pyBackward Compatibility
This change only affects NEW keys created after the fix. Existing keys with already-set expiration times are not modified.
The fix changes how expiration is calculated for new keys:
While this may affect users relying on the buggy behavior for new key creation: