[Uptime] TLS alerting #63913

justinkambic · 2020-04-17T21:57:52Z

Summary

The goal here is to create simple alerts that allow users to monitor their TLS certificate status, and alert them when any of the certs they maintain are either close to expiring or have lived beyond a certain age threshold.

Resolves elastic/uptime#162.

~~WIP, creating TLS alerts for Uptime~~

Testing this PR

You'll need to configure your local Kibana/ES/Beats to use SSL connection, or else disable security.
Define an alert.
Adjust the app's dynamic settings cert thresholds to observe that the alert triggers appropriately.
Test the various state fields to ensure that they correspond to criteria you define when creating your alert.

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser
This was checked for cross-browser compatibility, including a check against IE11

For maintainers

This was checked for breaking API changes and was labeled appropriately

elasticmachine · 2020-04-17T21:57:55Z

Pinging @elastic/uptime (Team:uptime)

andrewvc · 2020-05-04T15:43:50Z

One functional thing I immediately noticed. The design spec calls for a new alerts drop down menu on the certificates page. In this PR the alert creation action is added to the overview page.

andrewvc · 2020-05-04T15:55:51Z

Running with the demo monitor list and a server log action I get the following error:

server    log   [10:52:55.039] [error][alerting][alerting][plugins][plugins] Executing Alert "939e80fa-5a85-46f4-87c7-2cc56ad3c9d0" has resulted in Error: [query_shard_exception] No mapping found for [undefined] in order to sort on, with { index_uuid="22ddL3ohQ8id-tr7N1r_YA" & index="heartbeat-8.0.0-2020.05.04-000001" }

justinkambic · 2020-05-04T17:33:00Z

Running with the demo monitor list and a server log action I get the following error:

This was a defect I injected while implementing other feedback, will notify when I push the fix.

andrewvc · 2020-05-04T18:02:09Z

x-pack/plugins/uptime/server/lib/alerts/__tests__/tls.test.ts

+      `);
+    });
+
+    it('handles negative diff values appropriately for aging certs', () => {


nice test for completeness

andrewvc

LGTM

justinkambic · 2020-05-04T20:36:48Z

@andrewvc might want to have another look to verify you're alright with the changes in d1b414a and 86add5b.

Essentially these allow the alert flyout toggle dropdown to handle a list of alert types. If there is > 1 item, it will nest them like on the Overview page, otherwise it'll display a single context panel. And I added the toggle to the certs page.

x-pack/plugins/uptime/public/components/overview/alerts/translations.ts

shahzad31 · 2020-05-04T21:15:18Z

x-pack/plugins/uptime/public/components/overview/alerts/settings_message_expression_popover.tsx

+    >
+      <FormattedMessage
+        id="xpack.uptime.alerts.tls.settingsPageNav.text"
+        defaultMessage="You can edit these thresholds on the {settingsPageLink}."


Should it be less verbose?
instead of saying 'You can edit these thresholds on the settings page.'
You can edit this threshold in Settings. though i am not a native english speaker
so you probably know better.

The copy comes from the spec, so I think we can just leave it for now.

shahzad31

LGTM, left few suggestions.
As far user experience goes, i know there is some technical limitation as to check if user is creating duplicate alerts.
Even if we can't enforce it, we should somehow let user know to make sure, to check if they have already created an alert or something.
It probably requires design. Otherwise Looks good.

andrewvc · 2020-05-04T21:32:21Z

@shahzad31 yes, that's probably in the second iteration WRT ensuring there aren't dupe alerts. Currently this is a limitation of alerting, so it's better to give people the option.

kibanamachine · 2020-05-04T23:49:32Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 8566b00

History

💚 Build #45277 succeeded f3f18c1
💚 Build #45242 succeeded d1b414a
💚 Build #45152 succeeded 190610c
💚 Build #45106 succeeded 5902515
💔 Build #45043 failed 6fb9d3b

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

* Refactor settings form event handling and modify certs fields. * Fix/improve broken types/unit/integration/api tests. * Modify default expiration threshold. * Rename test vars. * Implement PR feedback. * Refresh snapshots, fix broken tests/types. * Remove unnecessary state spreading. * Add type for settings field errors. * Refresh test snapshots. * Improve punctuation. * Add TLS alert type. * Add cert API request and runtime type checking. * Add api test for cert api. * Add unload command to certs test. * Extract API params interface to io-ts type. * Add TLS alert type on server. * WIP - add state for changing selected alert type. * Finish adding alert type for client, add server alert summary. * Add some state variables. * Update certs summary function to create required values. * Refresh test snapshots. * Clean up message generator function. * Add a comment. * Update formatting for alert messages, add flags denoting presence of age/expiration data. * Add relative date information to tls alert messages. * Clean up more logic in certs request function. * Fix broken unit tests. * Move tests for common function to new file. * Fix logic error in test and add common state fields to tls alerts. * Extract common state field translations from status check alert. * Add a comment. * Add nested context navigation for uptime alert selection. * Clean up types. * Fix translation key typo. * Extract translations from tls alert factory. * Extract summary messages to translation file. * Change default tls alert time window from 1w to 1d. * Remove unnecessary import. * Simplify page linking. * Extract a non-trivial component to a dedicated file. * Simplify create alert copy. * Fix broken functional test. * Fix busted types. * Fix tls query error. * Allow for alerts toggle button to receive a set of types to display. * Add alerts toggle button to certs page. * Fix copy. * Fixup punctuation in default message to avoid double-period symbols. * Refresh snapshots.

justinkambic · 2020-05-05T01:55:55Z

Backported to:
7.x/7.8.0 0bc8e3e
#65211

justinkambic added release_note:enhancement WIP Work in progress v8.0.0 Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v7.8.0 labels Apr 17, 2020

justinkambic self-assigned this Apr 17, 2020

justinkambic added 2 commits April 21, 2020 18:10

Refactor settings form event handling and modify certs fields.

674857e

Fix/improve broken types/unit/integration/api tests.

7d2bf4c

justinkambic mentioned this pull request Apr 22, 2020

[Alerting] AddAlert flyout does not immediately update state to reflect new props #64219

Closed

justinkambic force-pushed the uptime_tls-alert branch from a01a84e to 0729fbe Compare April 22, 2020 23:45

justinkambic added 17 commits April 23, 2020 10:23

Merge branch 'master' into uptime_tls-settings

caf3ef2

Modify default expiration threshold.

d0246a7

Rename test vars.

f7678b6

Implement PR feedback.

0ddd97b

Refresh snapshots, fix broken tests/types.

ef21286

Remove unnecessary state spreading.

8412951

Add type for settings field errors.

59e60e8

Refresh test snapshots.

d47d429

Improve punctuation.

a1fdf46

Add TLS alert type.

929e38a

Add cert API request and runtime type checking.

07055cb

Add api test for cert api.

9e017cb

Add unload command to certs test.

b7ab73b

Extract API params interface to io-ts type.

6f007bc

Add TLS alert type on server.

a18ee01

WIP - add state for changing selected alert type.

ff773bf

Finish adding alert type for client, add server alert summary.

5870a59

justinkambic force-pushed the uptime_tls-alert branch from 0729fbe to 5870a59 Compare April 23, 2020 17:53

Add some state variables.

4c94906

justinkambic added 6 commits May 4, 2020 09:43

Merge branch 'master' into uptime_tls-alert

e34742e

Remove unnecessary import.

7bc051e

Simplify page linking.

d693a2a

Extract a non-trivial component to a dedicated file.

5d57988

Simplify create alert copy.

2c07456

Fix broken functional test.

6fb9d3b

Fix busted types.

5902515

Fix tls query error.

190610c

andrewvc reviewed May 4, 2020

View reviewed changes

andrewvc approved these changes May 4, 2020

View reviewed changes

justinkambic added 2 commits May 4, 2020 16:17

Allow for alerts toggle button to receive a set of types to display.

86add5b

Add alerts toggle button to certs page.

d1b414a

shahzad31 reviewed May 4, 2020

View reviewed changes

x-pack/plugins/uptime/public/components/overview/alerts/translations.ts Outdated Show resolved Hide resolved

shahzad31 reviewed May 4, 2020

View reviewed changes

shahzad31 approved these changes May 4, 2020

View reviewed changes

Fix copy.

f3f18c1

justinkambic added 2 commits May 4, 2020 17:41

Fixup punctuation in default message to avoid double-period symbols.

98418d2

Refresh snapshots.

8566b00

justinkambic merged commit 6d78489 into elastic:master May 5, 2020

justinkambic mentioned this pull request May 5, 2020

[7.x] [Uptime] TLS alerting (#63913) #65211

Merged

justinkambic deleted the uptime_tls-alert branch May 5, 2020 01:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Uptime] TLS alerting #63913

[Uptime] TLS alerting #63913

justinkambic commented Apr 17, 2020 •

edited

Loading

elasticmachine commented Apr 17, 2020

andrewvc commented May 4, 2020

andrewvc commented May 4, 2020

justinkambic commented May 4, 2020

andrewvc May 4, 2020

andrewvc left a comment

justinkambic commented May 4, 2020

shahzad31 May 4, 2020

justinkambic May 4, 2020

shahzad31 left a comment

andrewvc commented May 4, 2020

kibanamachine commented May 4, 2020

justinkambic commented May 5, 2020

[Uptime] TLS alerting #63913

[Uptime] TLS alerting #63913

Conversation

justinkambic commented Apr 17, 2020 • edited Loading

Summary

Testing this PR

Checklist

For maintainers

elasticmachine commented Apr 17, 2020

andrewvc commented May 4, 2020

andrewvc commented May 4, 2020

justinkambic commented May 4, 2020

andrewvc May 4, 2020

Choose a reason for hiding this comment

andrewvc left a comment

Choose a reason for hiding this comment

justinkambic commented May 4, 2020

shahzad31 May 4, 2020

Choose a reason for hiding this comment

justinkambic May 4, 2020

Choose a reason for hiding this comment

shahzad31 left a comment

Choose a reason for hiding this comment

andrewvc commented May 4, 2020

kibanamachine commented May 4, 2020

💚 Build Succeeded

History

justinkambic commented May 5, 2020

justinkambic commented Apr 17, 2020 •

edited

Loading