-
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][Platform] - Add connectors to import/export API #148703
Merged
WafaaNasr
merged 54 commits into
elastic:main
from
WafaaNasr:118774-import-export-connectors-with-rules
Feb 6, 2023
Merged
Changes from all commits
Commits
Show all changes
54 commits
Select commit
Hold shift + click to select a range
ea2319e
use getImporter and getExporter from Saved Object
WafaaNasr dddba87
[CI] Auto-commit changed files from 'node scripts/ts_project_linter -…
kibanamachine 1d5af44
Merge branch 'main' of https://github.com/elastic/kibana into 118774-…
WafaaNasr 2f5dcb4
resolve conflicts
WafaaNasr 605ba89
fix tests
WafaaNasr c9a2656
fix get export by id
WafaaNasr 9b2cb0e
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine f5a199b
comment warnings until decide if needed
WafaaNasr eb98ea3
Merge branch '118774-import-export-connectors-with-rules' of https://…
WafaaNasr 0a00326
fix tests
WafaaNasr eca3cf3
enable Rule importing even if connectors are missing secrets
WafaaNasr 3f56c94
merge with latest
WafaaNasr 912b6c9
test: Import rules response schema
WafaaNasr d898d3b
rename validateRuleActionConnectors to skipActionConnectorsValidation…
WafaaNasr f69890b
remove unused var
WafaaNasr b9f4fb3
optional skipActionConnectorsValidations
WafaaNasr 3b6106b
fix /rules/import_rules/route.test.ts
WafaaNasr 2d7c903
fix condition
WafaaNasr 67e8006
refactor closing Modal
WafaaNasr 75c3c7f
fixing export e2e test
WafaaNasr a354bab
fix api integrations
WafaaNasr 16795bd
fix import rules
WafaaNasr 5e20363
implement overwriting of connectors
WafaaNasr f0b24d1
Merge branch 'main' of https://github.com/elastic/kibana into 118774-…
WafaaNasr 0d0b61e
fix tests
WafaaNasr f3e6161
adding error handling
WafaaNasr c177d1b
Merge branch 'main' of https://github.com/elastic/kibana into 118774-…
WafaaNasr 42e9ae8
apply ux feedback
WafaaNasr c2c9ef7
add tests for overwrite checkbox
WafaaNasr 503aeb0
add export test
WafaaNasr e58dfe3
Merge branch 'main' of https://github.com/elastic/kibana into 118774-…
WafaaNasr fb1a282
fixing read only access
WafaaNasr 9d2aa0b
handle migration errors, overwrite rules, handle not importing connec…
WafaaNasr 490ffb0
Merge branch 'main' into 118774-import-export-connectors-with-rules
kibanamachine b299057
fix test
WafaaNasr efbda93
add tests to import connectors and utils
WafaaNasr 87b604d
add test for skipActionConnectorsValidations in update rules
WafaaNasr 5d04232
refactor importing connectors
WafaaNasr 2adf31d
apply comment and add tests
WafaaNasr 816cfe5
[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…
kibanamachine 31f5963
using warning message from the warning array and update the tests
WafaaNasr 2f4d171
Merge branch '118774-import-export-connectors-with-rules' of https://…
WafaaNasr 41bfb58
update tests description
WafaaNasr a417457
update overwrite message and snapshots
WafaaNasr 67b4b2f
show success messages when importing successfully
WafaaNasr f192914
apply team repos
WafaaNasr 7b2c13e
add tests for export by id
WafaaNasr 680530f
apply docs team changes
WafaaNasr 31bcd73
apply comments
WafaaNasr 879ea94
Merge branch 'main' into 118774-import-export-connectors-with-rules
kibanamachine 5098552
apply same ux as in the connectors page
WafaaNasr 2da1121
merge with latest
WafaaNasr eb6dfe5
remove todos already addressed
WafaaNasr 6dac771
Merge branch 'main' into 118774-import-export-connectors-with-rules
kibanamachine File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 were some changes merged in #145637 that I believe makes this
skipActionConnectorsValidations
unnecessary. We are now checking whether the rule type to create is a SIEM rule and stripping action level frequencies. Can you verify that you shouldn't need this skip param?cc @Zacqary
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.
skipActionConnectorsValidations
is used only when the user import rule(s), and is used because it skips the validation of theactions
as it has been done already in the import API before calling the create fn.create
method as well because we deal differently with the missing secrets warning message, we don't throw an exception, instead we continue the importing process by just showing a warning message in the UIIf you have another suggestion please let me know :)
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.
Thanks for the explanation @WafaaNasr! Instead of bypassing the action validation altogether what do you think about a flag called
allowMissingConnectorSecrets
that is passed into thevalidateActions
function. Inside that function, there is a check for connectors with missing secrets that will throw the validation error. PassingallowMissingConnectorSecrets=true
tovalidateActions
will log a warning but not throw the error? That way the rest of the action body can be correctly validated.cc @XavierM to see if he has any suggestions for 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.
Thanks, @ymao1! Sure I see your point!
Maybe as you mentioned we can pass the
allowMissingConnectorSecrets
to thevalidateActions
functionthen add an if condition around line 31=> 50, check if
allowMissingConnectorSecrets
was trueto avoid getting the actions again, and loop over them to check for missing secrets.
https://github.com/elastic/kibana/blob/main/x-pack/plugins/alerting/server/rules_client/lib/validate_actions.ts#L32
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.
@ymao1 could you please validate if this f192914 will be a valid solution?
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 agree with @ymao1 and also like better the name
allowMissingConnectorSecrets
. We should log these errors because if we have SDH, it will be easy to understand why it happens and give some guidance to our users.I will do something like below, really similar to what you did ;)
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.
Awesome, thanks @XavierM thanks for the explanation and the suggested code,
Could you please validate