-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[EDR Workflows] Fix agent count for single agent policies #194294
Conversation
Pinging @elastic/fleet (Team:Fleet) |
Pinging @elastic/security-defend-workflows (Team:Defend Workflows) |
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.
lgtm 🚀 , thank you!
@@ -96,7 +96,7 @@ export const policySettingsMiddlewareRunner: MiddlewareRunner = async ( | |||
|
|||
// Agent summary is secondary data, so its ok for it to come after the details | |||
// page is populated with the main content | |||
if (policyItem.policy_id) { | |||
if (policyItem.policy_ids.length) { |
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.
does policy_ids
always exist?
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.
Although we expect policy_ids to always be defined, I noticed some parts of the code where this check is still performed. I’ve added a safe .length
check here: f9d85f6. Thanks for bringing this to my attention!
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.
Makes sense 👌. I haven't dug into it but is it possible to update the places where we're sending policyIds
as a string instead of allowing policyIds to be string[] | string?
We are always sending |
@@ -323,13 +323,22 @@ export const getAgentStatusForAgentPolicyHandler: FleetRequestHandler< | |||
const [coreContext, fleetContext] = await Promise.all([context.core, context.fleet]); | |||
const esClient = coreContext.elasticsearch.client.asInternalUser; | |||
const soClient = fleetContext.internalSoClient; | |||
|
|||
const parsePolicyIds = (policyIds: string | string[] | undefined): string[] | undefined => { |
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.
Could you add some tests to cover these changes? It would be good especially to test the behaviour of getAgentStatusForAgentPolicy
in the different cases. Thanks!
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.
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.
LGTM - thank you for adding test coverage.
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.
LGTM 👍 Thanks for adding tests!
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Async chunks
Unknown metric groupsReferences to deprecated APIs
History
To update your PR or re-run it, just comment with: |
Starting backport for target branches: 8.x |
…4294) https://github.com/user-attachments/assets/2b64c1e0-0e6d-4ef5-952d-e4364b4403c4 The PR elastic#193705 introduced an issue when counting active agents for integration policies with only one agent policy assigned. In such cases, `query.policyIds` was treated as a single string instead of an array of strings (as expected with multiple agent policy ids like `/?policyIds=x&policyIds=y`). This PR resolves the issue by ensuring consistent handling of policyIds, regardless of the number of associated agent policies. (cherry picked from commit 847285b)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
) (#194351) # Backport This will backport the following commits from `main` to `8.x`: - [[EDR Workflows] Fix agent count for single agent policies (#194294)](#194294) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Konrad Szwarc","email":"konrad.szwarc@elastic.co"},"sourceCommit":{"committedDate":"2024-09-27T21:22:28Z","message":"[EDR Workflows] Fix agent count for single agent policies (#194294)\n\nhttps://github.com/user-attachments/assets/2b64c1e0-0e6d-4ef5-952d-e4364b4403c4\r\n\r\n\r\n\r\nThe PR #193705 introduced an issue when counting active agents for\r\nintegration policies with only one agent policy assigned. In such cases,\r\n`query.policyIds` was treated as a single string instead of an array of\r\nstrings (as expected with multiple agent policy ids like\r\n`/?policyIds=x&policyIds=y`). This PR resolves the issue by ensuring\r\nconsistent handling of policyIds, regardless of the number of associated\r\nagent policies.","sha":"847285ba7191aa6d26fb3dccc06748e1c4a202b1","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:Fleet","v9.0.0","Team:Defend Workflows","v8.16.0","backport:version"],"title":"[EDR Workflows] Fix agent count for single agent policies","number":194294,"url":"https://github.com/elastic/kibana/pull/194294","mergeCommit":{"message":"[EDR Workflows] Fix agent count for single agent policies (#194294)\n\nhttps://github.com/user-attachments/assets/2b64c1e0-0e6d-4ef5-952d-e4364b4403c4\r\n\r\n\r\n\r\nThe PR #193705 introduced an issue when counting active agents for\r\nintegration policies with only one agent policy assigned. In such cases,\r\n`query.policyIds` was treated as a single string instead of an array of\r\nstrings (as expected with multiple agent policy ids like\r\n`/?policyIds=x&policyIds=y`). This PR resolves the issue by ensuring\r\nconsistent handling of policyIds, regardless of the number of associated\r\nagent policies.","sha":"847285ba7191aa6d26fb3dccc06748e1c4a202b1"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/194294","number":194294,"mergeCommit":{"message":"[EDR Workflows] Fix agent count for single agent policies (#194294)\n\nhttps://github.com/user-attachments/assets/2b64c1e0-0e6d-4ef5-952d-e4364b4403c4\r\n\r\n\r\n\r\nThe PR #193705 introduced an issue when counting active agents for\r\nintegration policies with only one agent policy assigned. In such cases,\r\n`query.policyIds` was treated as a single string instead of an array of\r\nstrings (as expected with multiple agent policy ids like\r\n`/?policyIds=x&policyIds=y`). This PR resolves the issue by ensuring\r\nconsistent handling of policyIds, regardless of the number of associated\r\nagent policies.","sha":"847285ba7191aa6d26fb3dccc06748e1c4a202b1"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Konrad Szwarc <konrad.szwarc@elastic.co>
Screen.Recording.2024-09-27.at.22.28.05.mov
The PR #193705 introduced an issue when counting active agents for integration policies with only one agent policy assigned. In such cases,
query.policyIds
was treated as a single string instead of an array of strings (as expected with multiple agent policy ids like/?policyIds=x&policyIds=y
). This PR resolves the issue by ensuring consistent handling of policyIds, regardless of the number of associated agent policies.