Skip to content
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

[Security Solution][Endpoint] Update to Endpoint List and associated supporting API to be space aware #194312

Conversation

paul-tavares
Copy link
Contributor

@paul-tavares paul-tavares commented Sep 27, 2024

Summary

PR makes the following changes in support of Endpoint management support for Spaces:

Fleet

  • Adds new method - getByIds() - to the server-side Agent service
  • Updates some mocks

Security Solution

  • Updates the Endpoint host metadata API for both the list and single endpoint to be space aware
  • Updates the agent status API for Endpoint to be space aware
  • Updates endpoint policy response API to be space aware
  • New FTR API integration test suite was created
    • included changes to some service utilities to enable loading test data per-space

Testing

  1. Enable the following feature keys:
    • xpack.securitySolution.enableExperimental.endpointManagementSpaceAwarenessEnabled
    • xpack.fleet.enableExperimental.useSpaceAwareness
  2. Trigger migration in fleet to be space aware: API POST /internal/fleet/enable_space_awareness
  3. Create data in multiple spaces and validate that Endpoint lIst/details/policy response display only appropriate data

Tip

As an alternative to step 2 and 3 above, you can also just run the node x-pack/plugins/security_solution/scripts/endpoint/run_endpoint_agent.js script with the --spaceId value and it will take care of adding a running endpoint to the given space

Checklist

@paul-tavares paul-tavares added release_note:skip Skip the PR/issue when compiling release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) v8.16.0 labels Sep 27, 2024
@paul-tavares paul-tavares self-assigned this Sep 27, 2024
@paul-tavares
Copy link
Contributor Author

/ci

@paul-tavares
Copy link
Contributor Author

/ci

@paul-tavares
Copy link
Contributor Author

/ci

@paul-tavares
Copy link
Contributor Author

Copy link
Contributor

@juliaElastic juliaElastic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fleet change LGTM

@kibanamachine
Copy link
Contributor

Flaky Test Runner Stats

🟠 Some tests failed. - kibana-flaky-test-suite-runner#7112

[❌] x-pack/test/security_solution_api_integration/test_suites/edr_workflows/spaces/trial_license_complete_tier/configs/ess.config.ts: 3/25 tests passed.

see run history

@paul-tavares
Copy link
Contributor Author

@kibanamachine
Copy link
Contributor

Flaky Test Runner Stats

🎉 All tests passed! - kibana-flaky-test-suite-runner#7113

[✅] x-pack/test/security_solution_api_integration/test_suites/edr_workflows/artifacts/trial_license_complete_tier/configs/ess.config.ts: 25/25 tests passed.

see run history

Copy link
Member

@ashokaditya ashokaditya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the changes. I did a code review only. I've a few nits but it looks good to ship. I'll test locally and report back if I find anything odd.

@@ -128,6 +139,14 @@ class AgentClientImpl implements AgentClient {
return getAgentById(this.internalEsClient, this.soClient, agentId);
}

public async getByIds(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps rename this to getAgentsByIds?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback @ashokaditya . I named getByIds() to mirror other APIs available from fleet - thus to remain consistent with their naming conventions.

.request<Space>({
method: 'GET',
path: `/internal/spaces/_active_space`,
headers: { 'elastic-api-version': '2023-10-31' },
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we use 1 for version headers for internal APIs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll remove the header - looks like this API has not been moved to the versioned router:

router.get(
{
path: '/internal/spaces/_active_space',
validate: false,
},

const lastCheckin = new Date();
let agentVersion = version;

if (isServerless) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isServerless would always be false here as it's wrapped in a block that checks for !sServerless.

Comment on lines +54 to +56
// FIXME:PT need to remove the need for this mock. It appears in several test files on our side.
// Its currently needed due to the direct use of Fleet's `buildAgentStatusRuntimeField()` in
// `x-pack/plugins/security_solution/server/endpoint/routes/metadata/query_builders.ts:239`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥

@@ -16,8 +16,11 @@ export class EndpointAgentStatusClient extends AgentStatusClient {
protected readonly agentType: ResponseActionAgentType = 'endpoint';

async getAgentStatuses(agentIds: string[]): Promise<AgentStatusRecords> {
const metadataService = this.options.endpointService.getEndpointMetadataService();
const soClient = this.options.soClient;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: const { soClient, esClient } = this.options;

Comment on lines +76 to +78
const agentIds = (data?.hits?.hits || [])
.map((hit) => hit._source?.agent.id ?? '')
.filter((id) => !!id);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: You can use reduce to iterate over only once on the array.

*
* @throws AgentNotFoundError
*/
export const getByIds = async (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also perhaps rename this to getAgentsByIds

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comment as before - just following Fleet conventions as other getByIds() already exist

@paul-tavares paul-tavares enabled auto-merge (squash) October 17, 2024 12:43
@paul-tavares paul-tavares merged commit 2a786b8 into elastic:main Oct 17, 2024
45 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11386984275

@paul-tavares paul-tavares deleted the task/olm-8538-endpoint-list-api-support-spaces-2 branch October 17, 2024 14:25
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
fleet 1292 1296 +4
Unknown metric groups

API count

id before after diff
fleet 1415 1421 +6

History

cc @paul-tavares

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 17, 2024
…supporting API to be space aware (elastic#194312)

## Summary

PR makes the following changes in support of Endpoint management support
for Spaces:

### Fleet

- Adds new method - `getByIds()` - to the server-side Agent service
- Updates some mocks

### Security Solution

- Updates the Endpoint host metadata API for both the list and single
endpoint to be space aware
- Updates the agent status API for Endpoint to be space aware
- Updates endpoint policy response API to be space aware
- New FTR API integration test suite was created
- included changes to some service utilities to enable loading test data
per-space

(cherry picked from commit 2a786b8)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Oct 17, 2024
…iated supporting API to be space aware (#194312) (#196717)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Security Solution][Endpoint] Update to Endpoint List and associated
supporting API to be space aware
(#194312)](#194312)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Paul
Tavares","email":"56442535+paul-tavares@users.noreply.github.com"},"sourceCommit":{"committedDate":"2024-10-17T14:24:38Z","message":"[Security
Solution][Endpoint] Update to Endpoint List and associated supporting
API to be space aware (#194312)\n\n## Summary\r\n\r\nPR makes the
following changes in support of Endpoint management support\r\nfor
Spaces:\r\n\r\n\r\n### Fleet\r\n\r\n- Adds new method - `getByIds()` -
to the server-side Agent service\r\n- Updates some mocks\r\n\r\n\r\n###
Security Solution\r\n\r\n- Updates the Endpoint host metadata API for
both the list and single\r\nendpoint to be space aware\r\n- Updates the
agent status API for Endpoint to be space aware\r\n- Updates endpoint
policy response API to be space aware\r\n- New FTR API integration test
suite was created\r\n- included changes to some service utilities to
enable loading test
data\r\nper-space","sha":"2a786b88a131472c349f0bda431efe3ed6fbeaf6","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:Fleet","v9.0.0","Team:Defend
Workflows","backport:prev-minor","v8.17.0"],"title":"[Security
Solution][Endpoint] Update to Endpoint List and associated supporting
API to be space
aware","number":194312,"url":"https://github.com/elastic/kibana/pull/194312","mergeCommit":{"message":"[Security
Solution][Endpoint] Update to Endpoint List and associated supporting
API to be space aware (#194312)\n\n## Summary\r\n\r\nPR makes the
following changes in support of Endpoint management support\r\nfor
Spaces:\r\n\r\n\r\n### Fleet\r\n\r\n- Adds new method - `getByIds()` -
to the server-side Agent service\r\n- Updates some mocks\r\n\r\n\r\n###
Security Solution\r\n\r\n- Updates the Endpoint host metadata API for
both the list and single\r\nendpoint to be space aware\r\n- Updates the
agent status API for Endpoint to be space aware\r\n- Updates endpoint
policy response API to be space aware\r\n- New FTR API integration test
suite was created\r\n- included changes to some service utilities to
enable loading test
data\r\nper-space","sha":"2a786b88a131472c349f0bda431efe3ed6fbeaf6"}},"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/194312","number":194312,"mergeCommit":{"message":"[Security
Solution][Endpoint] Update to Endpoint List and associated supporting
API to be space aware (#194312)\n\n## Summary\r\n\r\nPR makes the
following changes in support of Endpoint management support\r\nfor
Spaces:\r\n\r\n\r\n### Fleet\r\n\r\n- Adds new method - `getByIds()` -
to the server-side Agent service\r\n- Updates some mocks\r\n\r\n\r\n###
Security Solution\r\n\r\n- Updates the Endpoint host metadata API for
both the list and single\r\nendpoint to be space aware\r\n- Updates the
agent status API for Endpoint to be space aware\r\n- Updates endpoint
policy response API to be space aware\r\n- New FTR API integration test
suite was created\r\n- included changes to some service utilities to
enable loading test
data\r\nper-space","sha":"2a786b88a131472c349f0bda431efe3ed6fbeaf6"}},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Paul Tavares <56442535+paul-tavares@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) release_note:skip Skip the PR/issue when compiling release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution Team:Fleet Team label for Observability Data Collection Fleet team v8.17.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants