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.
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
[Response Ops][Connectors] Add unsecured actions client to allow system to schedule email action #143282
[Response Ops][Connectors] Add unsecured actions client to allow system to schedule email action #143282
Changes from 1 commit
2a93a37
2fe5ce5
dbed0a8
00c611d
b0fba11
baf051a
96b696f
6cf7a3a
fa4a356
8865a80
7c884ae
f4c1851
9a58040
47f1ca9
c87baee
28c6632
db2dc2a
d5bf4ff
026646a
802dfa0
af826cb
557d5f0
943ac49
7bf6884
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Since this SO type is
namespaceType: 'multiple-isolated'
, do we need to also be specifying theinitialNamespaces
property when we create the saved object? Since we're using the bare repository, this won't be populated for you automatically.kibana/packages/core/saved-objects/core-saved-objects-api-server/src/apis/bulk_create.ts
Lines 38 to 48 in 2a93a37
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.
Looking at the
action_task_param
SO that's created by this, the namespaces field is set to['default']
which seems ok? I can pass it in explicitly if it's necessaryThere 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 know enough about the implications of these params existing in one space or another. By default, everything will live in the
default
space unless we explicitly say otherwise. For our immediate needs, this is might be ok? If a notification is triggered from a Case Assignment within themarketing
space, would it be confusing that the corresponding notification was "sent" from thedefault
space?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.
@gsoldevila WDYT of 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.
@legrego it sounds like notifications are fairly space agnostic so there will not be an indication that the notification came from any particular space, even if the case assignment occurred in a specific space @gsoldevila is that a fair statement? Given that, I think we can default to the
default
space.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. I wasn't sure if this would be surfaced in the Event Log or not, and whether or not we cared about that discrepancy within the Event Log.
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 point. I am not sure either 🙂. Maybe @ymao1 can help with 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.
The event log does not make any references to the
action_task_param
SO. It does contain information about the preconfigured connector but since the preconfigured connectors have no namespace, it does not write out a namespace for it.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.
For the event log, we already have a different
event.action
for when connectors are executed via http -execute-via-http
, so I think having a separate one for this scenario would be good.execute-via-notification
?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 suspect at some point the notification system will care about spaces, and we'll support non-pre-configured connectors that live across spaces, and we can defer figuring this out till then.
A non-pre-configured connector's space will show up in the event log, and presumably always will. Pre-configured connectors likely appear to be in the "default" space today. What happens when we have connectors in multiple spaces, not sure. But that's all about the connector, and it sort of feels like the "space of the notification" is going to be a different thing. If everything was supported I could use connector X that's defined in spaces A and B to perform a notification in space A, but the event log doesn't have a notion of "what space something is executed in". It would probably need to be some new field, if we want to capture it.