-
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][Trusted Apps] Trusted Apps post port cleanup #130959
[Security Solution][Endpoint][Trusted Apps] Trusted Apps post port cleanup #130959
Conversation
f47c78c
to
3c2c73e
Compare
Pinging @elastic/security-onboarding-and-lifecycle-mgt (Team:Onboarding and Lifecycle Mgt) |
@@ -22,7 +22,7 @@ export type ExceptionListTypeOrUndefined = t.TypeOf<typeof exceptionListTypeOrUn | |||
export enum ExceptionListTypeEnum { | |||
DETECTION = 'detection', | |||
ENDPOINT = 'endpoint', | |||
ENDPOINT_TRUSTED_APPS = 'endpoint', | |||
ENDPOINT_TRUSTED_APPS = 'endpoint_trusted_apps', |
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 remember exactly if we filter by the list type somewhere. I know we use most of the times the list_id but just want to make sure we don't break the existing exception list container for trusted apps when upgrading.
Also wondering if we should include a migration script to update the existing trusted apps container.
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.
@yctercero @marshallmain What else needs to be changed in order for this to work for older TA containers? This change is for the sake of consistency with other artifact names. I think when TAs was the first and only artifact just endpoint
made sense but not so much now with more artifacts. Actually endpoint_events
should also be renamed to endpoint_event_filters
, but we can do that in a new PR if you all agree. Thanks for your help on this.
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.
@madirey Not sure if this is the file in question, that needs to be updated? https://github.com/elastic/kibana/blob/main/x-pack/plugins/lists/server/saved_objects/migrations.ts
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.
@ashokaditya we'd chatted about updating migrations in that file you linked. How's that going?
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.
@yctercero I've not had a chance to get to it. I'll get to it this week. Thanks for following up.
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.
good, assuming @dasansol92 's comment on that list type enum gets addressed
💛 Build succeeded, but was flakyFailed CI StepsTest Failures
Metrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: cc @ashokaditya |
Hi @ashokaditya is this work still in progress? If not can we close the PR? We are still receiving reminders from slack's github bot for this PR. |
Sorry I've not gotten around to work on it layely. I'll close this for now and reopen it when it's ready for review again. |
refs /pull/129208
Summary
ExceptionListTypeEnum
forENDPOINT_TRUSTED_APPS