[Response Ops][Task Manager] Skip API Key Tasks If Security is Disabled #221732
Conversation
|
Can we resolve the conflicts and ensure CI is green before marking the PR as ready for review please? Thank you! |
| "requiredPlugins": [ | ||
| "licensing" | ||
| ], |
There was a problem hiding this comment.
For this check, could licensing be an optional dependency?
There was a problem hiding this comment.
I'm following a similar pattern as the alerting plugin to check for if security is enabled: https://github.com/elastic/kibana/blob/main/x-pack/platform/plugins/shared/alerting/kibana.jsonc
Here we have added the licensing plugin a a require plugin, I feel like it's important we determine if security is enabled, is there a downside to adding the licensing plugin to requiredPlugins? Thanks
There was a problem hiding this comment.
I don't think so per se, but it does keep the task manager plugin a bit less coupled should licensing ever be disabled. Making it an optionalPlugin also means task manager can work without it which I think is a nice property to maintain.
There was a problem hiding this comment.
Usage would just look more like:
jloleysens
left a comment
jloleysens
left a comment
There was a problem hiding this comment.
Approving to unblock progress, but recommend making licensing optional.
Ok sounds good, I will make it optional in a future PR, thanks! |
JiaweiWu
added
release_note:skip
|
Pinging @elastic/response-ops (Team:ResponseOps) |
ymao1
left a comment
ymao1
left a comment
There was a problem hiding this comment.
LGTM. Works as expected. Left one comment about leaving a log message.
|
|
||
| private getSoClientForCreate(options: ApiKeyOptions) { | ||
| if (options.request) { | ||
| if (options.request && this.getIsSecurityEnabled()) { |
There was a problem hiding this comment.
can we leave an info log when we're trying to schedule a task with a request but security is disabled? something like Trying to schedule task ${task.id} with user scope but security is disabled. Task will run without user scope.
ymao1
added
backport:version
|
Updated the labels to backport this to 8.19 |
💚 Build Succeeded
Metrics [docs]
History
|
|
Starting backport for target branches: 8.19 https://github.com/elastic/kibana/actions/runs/15864879753 |
💔 All backports failed
Manual backportTo create the backport manually run: Questions ?Please refer to the Backport tool documentation |
…ed (elastic#221732) ## Summary Resolves: elastic#216810 This PR uses the licensing plugin to check is security is enabled. If not, we skip adding API keys to tasks that pass in a `request` object, but still scheduling them. ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
…ed (elastic#221732) ## Summary Resolves: elastic#216810 This PR uses the licensing plugin to check is security is enabled. If not, we skip adding API keys to tasks that pass in a `request` object, but still scheduling them. ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios (cherry picked from commit bf0003f) # Conflicts: # x-pack/platform/plugins/shared/task_manager/server/plugin.test.ts # x-pack/platform/plugins/shared/task_manager/server/plugin.ts
kibanamachine
added
the
backport missing
|
Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync. |
4 similar comments
|
Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync. |
|
Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync. |
|
Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync. |
|
Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync. |
… Disabled (#221732) (#225402) # Backport This will backport the following commits from `main` to `8.19`: - [[Response Ops][Task Manager] Skip API Key Tasks If Security is Disabled (#221732)](#221732) <!--- Backport version: 10.0.1 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Jiawei Wu","email":"74562234+JiaweiWu@users.noreply.github.com"},"sourceCommit":{"committedDate":"2025-06-25T01:06:19Z","message":"[Response Ops][Task Manager] Skip API Key Tasks If Security is Disabled (#221732)\n\n## Summary\n\nResolves: https://github.com/elastic/kibana/issues/216810\n\nThis PR uses the licensing plugin to check is security is enabled. If\nnot, we skip adding API keys to tasks that pass in a `request` object,\nbut still scheduling them.\n\n### Checklist\n- [x] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios","sha":"bf0003f3be6711935ead682f79edec550588f5c1","branchLabelMapping":{"^v9.1.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:ResponseOps","backport:version","v9.1.0","v8.19.0"],"title":"[Response Ops][Task Manager] Skip API Key Tasks If Security is Disabled ","number":221732,"url":"https://github.com/elastic/kibana/pull/221732","mergeCommit":{"message":"[Response Ops][Task Manager] Skip API Key Tasks If Security is Disabled (#221732)\n\n## Summary\n\nResolves: https://github.com/elastic/kibana/issues/216810\n\nThis PR uses the licensing plugin to check is security is enabled. If\nnot, we skip adding API keys to tasks that pass in a `request` object,\nbut still scheduling them.\n\n### Checklist\n- [x] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios","sha":"bf0003f3be6711935ead682f79edec550588f5c1"}},"sourceBranch":"main","suggestedTargetBranches":["8.19"],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/221732","number":221732,"mergeCommit":{"message":"[Response Ops][Task Manager] Skip API Key Tasks If Security is Disabled (#221732)\n\n## Summary\n\nResolves: https://github.com/elastic/kibana/issues/216810\n\nThis PR uses the licensing plugin to check is security is enabled. If\nnot, we skip adding API keys to tasks that pass in a `request` object,\nbut still scheduling them.\n\n### Checklist\n- [x] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios","sha":"bf0003f3be6711935ead682f79edec550588f5c1"}},{"branch":"8.19","label":"v8.19.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT-->
kibanamachine
removed
the
backport missing
5 participants
Summary
Resolves: #216810
This PR uses the licensing plugin to check is security is enabled. If not, we skip adding API keys to tasks that pass in a
requestobject, but still scheduling them.Checklist