-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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] Improve Endpoint Host data generator to also integrate with Ingest #74305
[SECURITY_SOLUTION][ENDPOINT] Improve Endpoint Host data generator to also integrate with Ingest #74305
Conversation
@elasticmachine merge upstream |
ead9c3a
to
dfb9627
Compare
…t-data-generator-create-policy
Pinging @elastic/endpoint-app-team (Feature:Endpoint) |
Pinging @elastic/endpoint-management (Team:Endpoint Management) |
@@ -880,6 +883,14 @@ export interface PolicyConfig { | |||
}; | |||
} | |||
|
|||
export interface AdvancedFields { |
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.
is this in for the changes that @jmiller263 is making for Advanced policy?
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.
she's using this updated generator code for her changes, but its not explicitly in her pr
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.
I'd prefer that we make changes to the Policy types when we add those new features. @jmiller263 could use the type changes locally until it's time to merge those features.
…t-data-generator-create-policy
…t-data-generator-create-policy
…t-data-generator-create-policy
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.
KbnClient changes LGTM
…t-data-generator-create-policy
…t-data-generator-create-policy
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.
I added a few more comments, but I think they are ok to be worked on after this goes in. :)
Thanks for picking this up.
// Enroll an agent for the Host | ||
const body: PostAgentEnrollRequest['body'] = { | ||
type: 'PERMANENT', | ||
metadata: { |
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.
Here is a better metadata
message, which will no longer cause the Fleet agent list to show -
for host:
{
//...
metadata: {
local: {
"elastic": {
"agent": {
"version": versionNumber
}
},
"host": {
"architecture": "x86_64",
"hostname": `artifact-downloader.${Date.now()}.elastic.co`,
"name": "artifact-downloader",
"id": "1c032ec0-3a94-4d54-9ad2-c5610c0eaba4",
"ip": [
"fe80::703b:b9e6:887d:7f5/64",
"10.0.2.15/24",
"::1/128",
"127.0.0.1/8"
],
"mac": [
"08:00:27:d8:c5:c0"
]
},
"os": {
"family": "windows",
"kernel": "10.0.19041.388 (WinBuild.160101.0800)",
"platform": "windows",
"version": "10.0",
"name": "Windows 10 Pro",
"full": "Windows 10 Pro(10.0)"
}
},
}
change local.elastic.agent.version
to 8.0.0
(although, we should address that as well by using the kbnClient to query Kibana to get its version number, but we can do that another time (can you open an issue to track? :) )
Also - you will want to change the host.hostname
++ host.name
to be endpointHost.host
import { Client, ClientOptions } from '@elastic/elasticsearch'; | ||
import { ResponseError } from '@elastic/elasticsearch/lib/errors'; | ||
import { KbnClient, ToolingLog } from '@kbn/dev-utils'; | ||
import { AxiosResponse } from 'axios'; | ||
import { KibanaConfig } from '@kbn/dev-utils/target/kbn_client/kbn_client_requester'; |
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.
Can you check if this can be imported directly from @kbn/dev-utils
? if so, then just add it to the import above.
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.
i don't think it can be
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
@@ -98,3 +198,262 @@ async function indexAlerts( | |||
await client.bulk({ body, refresh: true }); | |||
} | |||
} | |||
|
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.
@paul-tavares this is a significant code change to the core class. I think it may be good to separate this code out. And from a cursory look it may be difficult to maintain and change. Perhaps a follow up PR with that view in mind. Because if there is any changes in the system we have to change this code also.
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.
@nnamdifrankie Agreed - some of this needs to be broken out into smaller set of classes/namespaces. but: could you clarify what class you are referring to? (or did you mean the entire module index_data.ts
?)
Also agree that if the system changes, this will have to be updated, but I think it valuable to have the ability to create a fake set of data across the entire system (minus the actual endpoint). Lets disucss if perhaps you feel strongly that we should not be creating this type of "full feature" tooling 😄
also - just FYI: my idea is to ultimately create a FakeAgent
class that will allows us to enroll and control agents against fleet without actually having an agent running (just sending messages to fleet - some of that will land in this external tool). The ability to do this will significantly improve our ability to do CI end-to-end testing within Kibana (Functional testing) - especially for our Endpoint (hosts) List page which is driven by a combination of both endpoint metadata and Fleet/Agent data (maybe even create a FakeEndpoint class to mimic Endpoints sending data to elastic).
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.
Looks great overall, just a few questions and type nits
); | ||
// eslint-disable-next-line no-process-exit | ||
process.exit(1); | ||
const setupResponse = (await kbnClient.request({ |
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.
👍 kbnClient looks like a much cleaner way to do this
x-pack/plugins/security_solution/scripts/endpoint/resolver_generator_script.ts
Outdated
Show resolved
Hide resolved
agentPolicyId: string | ||
): Promise<undefined | PostAgentEnrollResponse['item']> => { | ||
// Get Enrollement key for host's applied policy | ||
const enrollmentApiKey = await kbnClient |
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 consider making this request.then.then.catch
its own function called getEnrollmentApiKey
or similar
…t-data-generator-create-policy
@@ -32,12 +32,11 @@ import { | |||
} from '../../../ingest_manager/common'; | |||
import { factory as policyConfigFactory } from './models/policy_config'; | |||
import { HostMetadata } from './types'; | |||
import { KbnClientWithApiKeySupport } from '../../scripts/endpoint/resolver_generator_script'; |
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.
This import from resolver_generator_script.ts
is causing a cyclic dependency since that file also imports from here: https://github.com/elastic/kibana/blob/master/x-pack/plugins/security_solution/scripts/endpoint/resolver_generator_script.ts#L13
PostIngestSetupResponse, | ||
} from '../../../ingest_manager/common/types/rest_spec'; | ||
|
||
export class KbnClientWithApiKeySupport extends KbnClient { |
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.
if you move this to another file and import it from the files you need it, you should get rid of the cyclic dependency
💚 Build SucceededMetrics [docs]distributable file count
History
To update your PR or re-run it, just comment with: |
…aly-detection-partition-field * 'master' of github.com:elastic/kibana: (37 commits) Fixes for the Ticket 78375 (elastic#79004) [Security] Alert Telemetry for the Security app (elastic#77200) [Search bar] Remove duplicate `popoverProps` (elastic#79025) [Security Solution][Detections] Add rule overrides for single event EQL rules (elastic#78876) [SECURITY_SOLUTION][ENDPOINT] Improve Endpoint Host data generator to also integrate with Ingest (elastic#74305) remove file accidentally checked in (elastic#79005) [ML] DF Analytics creation wizard: replace select input with job type cards with icons (elastic#78872) [Design] A couple fixes for 7.10 (elastic#78801) Fix KQL autocomplete value suggestions (elastic#78676) [Security Solution][Resolver] New mock with cursor (elastic#78863) Embeddables: basic documentation (elastic#78900) [security solution] only import beat_schema when needed (elastic#78708) [Reporting] API Integration tests: fix flaky tests for Spaces CSV formatting (elastic#78849) [Actions] Adds a "Test Connector" button on the Connectors List to make discovery of the Test tab easier (elastic#78746) [Discover] Fix functional time picker test permissions (elastic#78564) [ML] Fixing module datafeed overrides (elastic#78925) Adds some missing licenses to the CSV export (elastic#78719) [dev/cli] ensure plugins/ and all watch source dirs exist (elastic#78973) [Lens] Stop using scripted metric to collect telemetry (elastic#78687) [Lens] fix wrong message in fields accordion (elastic#78924) ...
Summary
When the Endpoint generator is run, in addtion to indexing endpoint host metadata, it should also create the integrations needed for the endpoints in Ingest, which are:
enrolling
)ack
for a configuration change (tells fleet that the agent configuration was applied and lists the Agent in Fleet as a user ofendpoint
package)TODO
--delete
option be supported for the Ingest/Fleet created docs?Fleet showing an Agent that was created via the Endpoint data generator:
Checklist
Delete any items that are not applicable to this PR.
For maintainers