-
Notifications
You must be signed in to change notification settings - Fork 8.5k
[Response Ops][Task Manager] Wait to invalidate API keys until no tasks are using it #244843
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?
Conversation
🔍 Preview links for changed docs |
a15bd8d to
d3f0ceb
Compare
|
@elasticmachine run docs-build |
…tion_tests/ci_checks
| const PAGE_SIZE = 100; | ||
| export const TASK_ID = `Alerts-${TASK_TYPE}`; | ||
|
|
||
| const invalidateAPIKeys = async ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved all of these functions to the task manager plugin so they can be reused by both tasks.
| includedHiddenTypes: ['task'], | ||
| }) | ||
| ); | ||
| plugin.taskManager.registerApiKeyInvalidateFn( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Due to circular dependency issues, we cannot have the task manager plugin depend on the security plugin, so we have to use this sidecar plugin in order to use security functions within task manager.
| event_loop_delay: eventLoopDelaySchema, | ||
| invalidate_api_key_task: schema.object({ | ||
| interval: schema.string({ validate: validateDuration, defaultValue: '5m' }), | ||
| removalDelay: schema.string({ validate: validateDuration, defaultValue: '1h' }), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as alerting, we introduce a removal delay to remove the possibility of race conditions between the task that's being deleted and any other child tasks that may be scheduled using the same API key.
| '--xpack.eventLog.logEntries=true', | ||
| '--xpack.eventLog.indexEntries=true', | ||
| '--xpack.task_manager.monitored_aggregated_stats_refresh_rate=5000', | ||
| '--xpack.task_manager.invalidate_api_key_task.removalDelay="1s"', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reducing the removal delay for testing
| return apiKey.id === result.userScope?.apiKeyId; | ||
| }).length | ||
| ).eql(0); | ||
| ).eql(1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated this test that previously checked whether the API key was deleted after a task was run. Now it checks that the API key is still there an an api_key_to_invalidate SO is created. Then it manually triggers the invalidate API key task and checks that the API key is deleted after that.
|
Pinging @elastic/response-ops (Team:ResponseOps) |
jbudz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker LGTM
|
@elasticmachine merge upstream |
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Public APIs missing comments
Public APIs missing exports
Unknown metric groupsAPI count
References to deprecated APIs
History
cc @ymao1 |
Resolves #243719
Summary
This PR switches to marking API keys used by tasks as ready for invalidation instead of immediately invalidating them. This is done by:
api_key_to_invalidatethat contains theapiKeyIdof the API key to invalidate (does not contain the API key itself)api_key_to_invalidatesaved objects. When it finds an API key ID, it queries for any existing tasks that may still be using it and does not invalidate if there are any tasks that need it.There is an alerting task that does the same things for API keys created by alerting rules. In the future, we will move alerting rule API key management to task manager and we can remove the duplicate task.
Verify