Skip to content
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

Add Docs for Bulk Update Schedule and Rule Actions #2506

Merged

Conversation

jpdjere
Copy link
Contributor

@jpdjere jpdjere commented Sep 27, 2022

Relates to:

#2441
#2453

Changes

  • Updated Bulk Update API request to include new possible payloads for:
    • Bulk Update Schedule
    • Bulk Update Rule Actions

Images

image

@mergify
Copy link
Contributor

mergify bot commented Sep 27, 2022

This pull request does not have a backport label. Could you fix it @jpdjere? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • v7.x is the label to automatically backport to the 7.x branch.
  • v7./d./d is the label to automatically backport to the 7./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

@github-actions
Copy link

Documentation previews:

@jpdjere jpdjere added the v8.5.0 label Sep 27, 2022
@mergify mergify bot removed the backport-skip label Sep 27, 2022
@jpdjere jpdjere self-assigned this Sep 27, 2022
Comment on lines 723 to 773
[[actions-object-schema-update]]
===== `actions` schema

These fields are required when calling `PUT` to modify the `actions` object:

[width="100%",options="header"]
|==============================================
|Name |Type |Description

|action_type_id |String a|The action type used for sending notifications, can
be:

* `.slack`
* `.email`
* `.pagerduty`
* `.webhook`

|group |String |Optionally groups actions by use cases. Use `default` for alert
notifications.

|id |String |The connector ID.

|params |Object a|Object containing the allowed connector fields, which varies according to the connector type:

* For Slack:
** `message` (string, required): The notification message.
* For email:
** `to`, `cc`, `bcc` (string): Email addresses to which the notifications are
sent. At least one field must have a value.
** `subject` (string, optional): Email subject line.
** `message` (string, required): Email body text.
* For Webhook:
** `body` (string, required): JSON payload.
* For PagerDuty:
** `severity` (string, required): Severity of on the alert notification, can
be: `Critical`, `Error`, `Warning` or `Info`.
** `eventAction` (string, required): Event https://v2.developer.pagerduty.com/docs/events-api-v2#event-action[action type], which can be `trigger`,
`resolve`, or `acknowledge`.
** `dedupKey` (string, optional): Groups alert notifications with the same
PagerDuty alert.
** `timestamp` (DateTime, optional): https://v2.developer.pagerduty.com/v2/docs/types#datetime[ISO-8601 format timestamp].
** `component` (string, optional): Source machine component responsible for the
event, for example `security-solution`.
** `group` (string, optional): Enables logical grouping of service components.
** `source` (string, optional): The affected system. Defaults to the {kib}
saved object ID of the action.
** `summary` (string, options): Summary of the event. Defaults to
`No summary provided`. Maximum length is 1024 characters.
** `class` (string, optional): Value indicating the class/type of the event.

|==============================================
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy pasted this actions schema from https://github.com/jpdjere/security-docs/blob/main/docs/detections/api/rules/rules-api-bulk-actions.asciidoc#L334

Does it make sense to extract this definition to its own page and link from both pages to that?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to extract this definition to its own page and link from both pages to that?

I think we'd like to do something like that eventually, but we'll be migrating to a new docs system soon and are trying to limit restructuring ahead of that migration.

@jpdjere
Copy link
Contributor Author

jpdjere commented Sep 27, 2022

@joepeeples Is the build failing because of the wrong format in the asciidoc file?

@joepeeples
Copy link
Contributor

@joepeeples Is the build failing because of the wrong format in the asciidoc file?

I think so, it looks like there are two errors in the console output:

12:09:55 INFO:build_docs:asciidoctor: WARNING: detections/api/rules/rules-api-bulk-actions.asciidoc: line 724: section title out of sequence: expected level 3, got level 4
12:09:55 INFO:build_docs:asciidoctor: WARNING: detections/api/rules/rules-api-bulk-actions.asciidoc: line 724: id assigned to section already in use: actions-object-schema-update

Let me try to push a fix, it'll take just a few minutes

- Add discrete tag to include section on same page of HTML output
- Rename section anchor so it's a unique value
Comment on lines 321 to 325
`actions`: <<actions-object-schema, actions[]>> ,
`throttle`: String
} | Add actions to rules
| `set_rule_actions` | {
`actions`: <<actions-object-schema, actions[]>> ,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If these links are supposed to point to the new "actions schema" section on this page, the link anchors need to be revised (below); currently they're pointing to the section on the Create rule page.

Suggested change
`actions`: <<actions-object-schema, actions[]>> ,
`throttle`: String
} | Add actions to rules
| `set_rule_actions` | {
`actions`: <<actions-object-schema, actions[]>> ,
`actions`: <<actions-object-schema-bulk, actions[]>> ,
`throttle`: String
} | Add actions to rules
| `set_rule_actions` | {
`actions`: <<actions-object-schema-bulk, actions[]>> ,

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks 👍 fixed following your suggestion

docs/detections/api/rules/rules-api-bulk-actions.asciidoc Outdated Show resolved Hide resolved

| `add_rule_actions` | {
`actions`: <<actions-object-schema, actions[]>> ,
`throttle`: String
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Throttle is not documented for add_rule_actions while it is for set_rule_actions. How do we usually document the same request parameters or response objects used in multiple contexts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I extracted out the throttle schema to the bottom of the page and linked to it from both add_rule_actions and set_rule_actions. What do you think? Also not exactly sure if to call a single property "schema" (I mean "throttle schema")

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd call it just throttle 🤷‍♂️

Comment on lines 722 to 724
[discrete]
[[actions-object-schema-bulk]]
===== `actions` schema
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this section going to be deleted?

Copy link
Contributor Author

@jpdjere jpdjere Sep 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it shouldn't. See Joe's comment above: I would have liked to extract the actions schema to its own page and link to it from both this page and from this page, but he mentioned that he'd rather not do these refactoring before the docs are migrated, soon.

And I think the actions schema should be in this page, otherwise it's confusing to understand what it means in the context of this API request.

Copy link
Contributor

@banderror banderror Sep 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. The reason I asked is that when I was reviewing this PR links to the actions schema were pointing to the rule creation endpoint. Now when they send the user to this section it LGTM 👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I'd probably do is I'd lift "actions schema" and "throttle schema" up, because right now the page structure is:

  1. "BulkEditAction object" - its schema
  2. Examples
  3. "actions schema"
  4. "throttle schema"

I'd keep examples after all schemas

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, reorganized page

@jpdjere
Copy link
Contributor Author

jpdjere commented Sep 28, 2022

@joepeeples

This is ready for review. Thanks for your comments!

The preview link: https://security-docs_2506.docs-preview.app.elstc.co/guide/en/security/master/bulk-actions-rules-api.html

Copy link
Contributor

@banderror banderror left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thank you! 🚀

docs/detections/api/rules/rules-api-bulk-actions.asciidoc Outdated Show resolved Hide resolved
docs/detections/api/rules/rules-api-bulk-actions.asciidoc Outdated Show resolved Hide resolved
Comment on lines 722 to 724
[discrete]
[[actions-object-schema-bulk]]
===== `actions` schema
Copy link
Contributor

@banderror banderror Sep 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. The reason I asked is that when I was reviewing this PR links to the actions schema were pointing to the rule creation endpoint. Now when they send the user to this section it LGTM 👍

Comment on lines 722 to 724
[discrete]
[[actions-object-schema-bulk]]
===== `actions` schema
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I'd probably do is I'd lift "actions schema" and "throttle schema" up, because right now the page structure is:

  1. "BulkEditAction object" - its schema
  2. Examples
  3. "actions schema"
  4. "throttle schema"

I'd keep examples after all schemas

Copy link
Contributor

@joepeeples joepeeples left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a few line edits for consideration, and got the build preview working again. Thanks for including me in the review!


`lookback`: Additional look-back time that the rule analyzes. For example, `"10m"` means the rule analyzes the last 10 minutes of data in addition to the frequency interval.

If `interval` is set to `"5m"` and `lookback` to `"1m"`, then the rule runs every 5 minutes but analyzes the documents added to indices during the last 6 minutes.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modified this example to be consistent with the previous two examples for interval and lookback:

Suggested change
If `interval` is set to `"5m"` and `lookback` to `"1m"`, then the rule runs every 5 minutes but analyzes the documents added to indices during the last 6 minutes.
If `interval` is set to `"10m"` and `lookback` to `"1m"`, then the rule runs every 5 minutes but analyzes the documents added to indices during the last 11 minutes.

Comment on lines 320 to 323
Both `interval` and `lookback` have a format of {integer}{time_unit}, with the accepted time units being `"s"` for
seconds, `"m"` for minutes and `"h"` for hours. The integer must be positive and larger than 0.

Examples: `"45s"`, `"30m"`, `"6h"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Both `interval` and `lookback` have a format of {integer}{time_unit}, with the accepted time units being `"s"` for
seconds, `"m"` for minutes and `"h"` for hours. The integer must be positive and larger than 0.
Examples: `"45s"`, `"30m"`, `"6h"`
Both `interval` and `lookback` have a format of `"{integer}{time_unit}"`, where accepted time units are `s` for seconds, `m` for minutes, and `h` for hours. The integer must be positive and larger than 0. Examples: `"45s"`, `"30m"`, `"6h"`

Comment on lines 392 to 394

---

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd omit the horiz rule line. We don't use them much in general, and here in particular it makes the throttle section look like it might be part of the preceding table.

Suggested change
---

===== `throttle` schema


`throttle` defines the maximum interval in which a rules `actions` are executed. It accepts the following values:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
`throttle` defines the maximum interval in which a rules `actions` are executed. It accepts the following values:
`throttle` defines the maximum interval in which a rule's actions are executed. It accepts the following values:

@jpdjere
Copy link
Contributor Author

jpdjere commented Sep 30, 2022

@joepeeples Thanks Joe! Addressed all your comments 👍

Please et me know if I should go ahead and merge this once the tests pass.

@joepeeples
Copy link
Contributor

Please et me know if I should go ahead and merge this once the tests pass.

Yes, LGTM! 👍 Thanks again for this update

@jpdjere jpdjere merged commit c76fac6 into elastic:main Sep 30, 2022
@jpdjere jpdjere deleted the add-docs-for-bulk-edit-schedule-and-actions branch September 30, 2022 18:02
mergify bot pushed a commit that referenced this pull request Sep 30, 2022
## Relates to:
#2441
#2453

## Changes

- Updated Bulk Update API request to include new possible payloads for:
    - Bulk Update Schedule
    - Bulk Update Rule Actions

## Images
![image](https://user-images.githubusercontent.com/5354282/192609873-9dc20d53-0beb-4489-9987-df611a735801.png)

(cherry picked from commit c76fac6)
joepeeples pushed a commit that referenced this pull request Sep 30, 2022
## Relates to:
#2441
#2453

## Changes

- Updated Bulk Update API request to include new possible payloads for:
    - Bulk Update Schedule
    - Bulk Update Rule Actions

## Images
![image](https://user-images.githubusercontent.com/5354282/192609873-9dc20d53-0beb-4489-9987-df611a735801.png)

(cherry picked from commit c76fac6)

Co-authored-by: Juan Pablo Djeredjian <jpdjeredjian@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants