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

[Synthetics] enable/disable - prevent incorrect keys from being added to the monitor saved object #140553

Conversation

dominiqueclarke
Copy link
Contributor

@dominiqueclarke dominiqueclarke commented Sep 12, 2022

Summary

Fixes a bug where incorrect keys were being added to montiors on the enable/disable flow in Synthetics UI.

This was due to an oversight where types were changed to accept either a Synthetics Monitor saved object or an Overview Monitor Item. In reality, Overview Monitor Items are not a type of Synthetics monitor, but is a unique object for displaying information about a monitor on the overview page in general. This only impacted 8.5.0.

The enable/disable hook was updated to adjust the types.

Additionally, extra runtime type decoding was added to ensure that unknown keys cannot be added to a monitor-saved object. The t.exact type returns a valid monitor, stripped of any additional unknown keys that are not part of the codec.

Testing

  1. Create a monitor.
  2. Navigate to Overview. Disable the monitor
  3. Inspect the monitor saved object. You can do this by viewing the monitor get request when editing the monitor.
  4. Inspect the attributes key of the monitor. Ensure that no keys from the MonitorOverviewItem are present, such as location, isEnabled or id.

dispatch(
fetchUpsertMonitorAction({
id,
monitor: { ...monitor, [ConfigKey.ENABLED]: enabled },
monitor: { [ConfigKey.ENABLED]: enabled },
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since edits are applied on top of the existing object, we can just pass the enabled key to be safe here.

@dominiqueclarke dominiqueclarke added bug Fixes for quality problems that affect the customer experience Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability release_note:skip Skip the PR/issue when compiling release notes v8.5.0 v8.6.0 labels Sep 25, 2022
@dominiqueclarke dominiqueclarke changed the title synthetics - enable/disable - prevent incorrect keys from being added to the monitor saved object [Synthetics] enable/disable - prevent incorrect keys from being added to the monitor saved object Sep 25, 2022
@dominiqueclarke dominiqueclarke marked this pull request as ready for review September 25, 2022 19:32
@dominiqueclarke dominiqueclarke requested a review from a team as a code owner September 25, 2022 19:32
@elasticmachine
Copy link
Contributor

Pinging @elastic/uptime (Team:uptime)

@dominiqueclarke
Copy link
Contributor Author

@elasticmachine merge upstream

@dominiqueclarke dominiqueclarke force-pushed the fix/synthetics-enable-disable-incorrect-keys branch from f9fe54b to 3a7d244 Compare September 29, 2022 12:34
Copy link
Contributor

@shahzad31 shahzad31 left a comment

Choose a reason for hiding this comment

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

LGTM, Works as expected, doesn't add extra fields now !!

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
synthetics 1023.3KB 1023.6KB +297.0B

History

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

@dominiqueclarke dominiqueclarke merged commit 27916d3 into elastic:main Sep 29, 2022
@dominiqueclarke dominiqueclarke deleted the fix/synthetics-enable-disable-incorrect-keys branch September 29, 2022 15:23
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Sep 29, 2022
… to the monitor saved object (elastic#140553)

* synthetics - enable/disable - prevent incorrect keys from being added to the monitor saved object

* [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix'

* adjust types

* add exact typing

* [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix'

* adjust test

* use exact types

* use exact types for editing

* adjust types

* adjust tests

* adjust types

* Update x-pack/test/api_integration/apis/uptime/rest/add_monitor.ts

* adjust normalizers

* Update x-pack/plugins/synthetics/common/runtime_types/monitor_management/monitor_types.ts

* Update x-pack/plugins/synthetics/server/routes/monitor_cruds/add_monitor.ts

* adjust types

* [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix'

* adjust jest tests

* adjust types

* adjust api_integration tests

* update tests

Co-authored-by: kibanamachine <[email protected]>
(cherry picked from commit 27916d3)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.5

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Sep 29, 2022
… to the monitor saved object (#140553) (#142238)

* synthetics - enable/disable - prevent incorrect keys from being added to the monitor saved object

* [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix'

* adjust types

* add exact typing

* [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix'

* adjust test

* use exact types

* use exact types for editing

* adjust types

* adjust tests

* adjust types

* Update x-pack/test/api_integration/apis/uptime/rest/add_monitor.ts

* adjust normalizers

* Update x-pack/plugins/synthetics/common/runtime_types/monitor_management/monitor_types.ts

* Update x-pack/plugins/synthetics/server/routes/monitor_cruds/add_monitor.ts

* adjust types

* [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix'

* adjust jest tests

* adjust types

* adjust api_integration tests

* update tests

Co-authored-by: kibanamachine <[email protected]>
(cherry picked from commit 27916d3)

Co-authored-by: Dominique Clarke <[email protected]>
WafaaNasr pushed a commit to WafaaNasr/kibana that referenced this pull request Oct 2, 2022
… to the monitor saved object (elastic#140553)

* synthetics - enable/disable - prevent incorrect keys from being added to the monitor saved object

* [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix'

* adjust types

* add exact typing

* [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix'

* adjust test

* use exact types

* use exact types for editing

* adjust types

* adjust tests

* adjust types

* Update x-pack/test/api_integration/apis/uptime/rest/add_monitor.ts

* adjust normalizers

* Update x-pack/plugins/synthetics/common/runtime_types/monitor_management/monitor_types.ts

* Update x-pack/plugins/synthetics/server/routes/monitor_cruds/add_monitor.ts

* adjust types

* [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix'

* adjust jest tests

* adjust types

* adjust api_integration tests

* update tests

Co-authored-by: kibanamachine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v8.5.0 v8.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants