-
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] Add host isolation exception IPs UI #113762
[Security Solution] Add host isolation exception IPs UI #113762
Conversation
…ation-exception-add
…ation-exception-add
Pinging @elastic/security-onboarding-and-lifecycle-mgt (Team:Onboarding and Lifecycle Mgt) |
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.
Left some minor comments but this is looking ⭐ 🔥
...y_solution/public/management/pages/host_isolation_exceptions/view/components/form_flyout.tsx
Outdated
Show resolved
Hide resolved
...gins/security_solution/public/management/pages/host_isolation_exceptions/store/middleware.ts
Show resolved
Hide resolved
...gins/security_solution/public/management/pages/host_isolation_exceptions/store/middleware.ts
Outdated
Show resolved
Hide resolved
...y_solution/public/management/pages/host_isolation_exceptions/view/components/form_flyout.tsx
Outdated
Show resolved
Hide resolved
...on/public/management/pages/host_isolation_exceptions/view/host_isolation_exceptions_list.tsx
Outdated
Show resolved
Hide resolved
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 the changes 👍
@elasticmachine merge upstream |
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! I checked out the branch locally and took it for a spin. Works as expected.
🚀 🐑
note:
The error toasts for creation and deletion may need some more work. They are not at all similar in the message content. Also, I notice some notate the sentence end with a .
and most others do not. I would suggest we keep the toast contents consistent.
We should either have a useful reason in the error toast or not have it at all.
.../plugins/security_solution/public/management/pages/host_isolation_exceptions/store/action.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/security_solution/public/management/pages/host_isolation_exceptions/types.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/security_solution/public/management/pages/host_isolation_exceptions/utils.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/security_solution/public/management/pages/host_isolation_exceptions/utils.ts
Show resolved
Hide resolved
...ution/public/management/pages/host_isolation_exceptions/view/components/form_flyout.test.tsx
Outdated
Show resolved
Hide resolved
@ashokaditya thanks for pointing the message inconsistency on the toasts. I'll fix them in the upcoming edit PR and consult about the copy. |
💚 Build SucceededMetrics [docs]Module Count
Async chunks
History
To update your PR or re-run it, just comment with: |
Co-authored-by: Kibana Machine <[email protected]>
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
…14490) Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Esteban Beltran <[email protected]>
Summary
Related to #111253 and #113541
This follow up PR adds the capability of adding host isolation exceptions IPs.
Checklist
Delete any items that are not applicable to this PR.