Add system and pipeline tests for Zoom by andrewkroh · Pull Request #598 · elastic/integrations

andrewkroh · 2021-02-01T02:32:58Z

What does this PR do?

Add system and pipeline tests for Zoom.

Changes:

Remove logfile input. It was only needed for testing purposes, but we are now testing via pipeline and webhook callbacks.
Remove prefix config option. Changing the value would break the pipeline.
Add url config option.
Fix basic auth parameters in handlebar template (missing password option, and username was set to password).
Add missing field definition for input.type.
Convert zoom.phone.duration to a long from a string.
Drop null zoom.chat_message.message values.
Add ignore_missing to foreach loop dealing with zoom.chat_channel.members.
Add display name to related.user for zoom.chat_channel events (otherwise that info was lost).
Add configuration textbox for SSL options.
Hide options that are advanced/optional.
Remove options that should not be configurable.
Add note about HTTPS requirement from Zoom.
Link to TLS documentation.
Add additional test for HTTPS.

These were the original failures I got when I setup the tests.

FAILURE DETAILS:

zoom/webhook test-chat-message.json:
[0] parsing field value failed: field "zoom.chat_message.message"'s Go type, <nil>, does not match the expected field type: keyword (field value: <nil>)
zoom/webhook test-phone.json:
[0] parsing field value failed: field "zoom.phone.duration"'s Go type, string, does not match the expected field type: long (field value: 1235)

ERROR: simulating pipeline processing failed: unexpected response status for Simulate (400): 400 Bad Request: elasticsearch error (type=illegal_argument_exception): field [members] not present as part of path [zoom.chat_channel.members]

Checklist

I have reviewed tips for building integrations and this pull request is aligned with them.
I have verified that all data streams collect metrics or logs.

Related issues

Relates Add automated tests for security packages #377

Screenshots

--- Test results for package: zoom - START ---
╭─────────┬─────────────┬───────────┬───────────────────────────────────────────────────┬────────┬──────────────╮
│ PACKAGE │ DATA STREAM │ TEST TYPE │ TEST NAME                                         │ RESULT │ TIME ELAPSED │
├─────────┼─────────────┼───────────┼───────────────────────────────────────────────────┼────────┼──────────────┤
│ zoom    │ webhook     │ asset     │ index_template logs-zoom.webhook is loaded        │ PASS   │ 7.159733108s │
│ zoom    │ webhook     │ asset     │ ingest_pipeline logs-zoom.webhook-0.1.2 is loaded │ PASS   │        913ns │
╰─────────┴─────────────┴───────────┴───────────────────────────────────────────────────┴────────┴──────────────╯
--- Test results for package: zoom - END   ---
Done
Run pipeline tests for the package
--- Test results for package: zoom - START ---
╭─────────┬─────────────┬───────────┬────────────────────────┬────────┬──────────────╮
│ PACKAGE │ DATA STREAM │ TEST TYPE │ TEST NAME              │ RESULT │ TIME ELAPSED │
├─────────┼─────────────┼───────────┼────────────────────────┼────────┼──────────────┤
│ zoom    │ webhook     │ pipeline  │ test-account.json      │ PASS   │   21.66429ms │
│ zoom    │ webhook     │ pipeline  │ test-chat-channel.json │ PASS   │  11.084654ms │
│ zoom    │ webhook     │ pipeline  │ test-chat-message.json │ PASS   │  10.591402ms │
│ zoom    │ webhook     │ pipeline  │ test-meeting.json      │ PASS   │  22.871165ms │
│ zoom    │ webhook     │ pipeline  │ test-phone.json        │ PASS   │  27.290518ms │
│ zoom    │ webhook     │ pipeline  │ test-recording.json    │ PASS   │  29.058689ms │
│ zoom    │ webhook     │ pipeline  │ test-user.json         │ PASS   │  10.901462ms │
│ zoom    │ webhook     │ pipeline  │ test-webinar.json      │ PASS   │  12.288897ms │
│ zoom    │ webhook     │ pipeline  │ test-zoomroom.json     │ PASS   │  10.334777ms │
╰─────────┴─────────────┴───────────┴────────────────────────┴────────┴──────────────╯
--- Test results for package: zoom - END   ---
Done
Run system tests for the package
--- Test results for package: zoom - START ---
╭─────────┬─────────────┬───────────┬───────────┬────────┬───────────────╮
│ PACKAGE │ DATA STREAM │ TEST TYPE │ TEST NAME │ RESULT │  TIME ELAPSED │
├─────────┼─────────────┼───────────┼───────────┼────────┼───────────────┤
│ zoom    │ webhook     │ system    │ webhook   │ PASS   │ 35.119313157s │
╰─────────┴─────────────┴───────────┴───────────┴────────┴───────────────╯
--- Test results for package: zoom - END   ---

Changes: - Remove logfile input. It was only needed for testing purposes, but we are now testing via pipeline and webhook callbacks. - Remove prefix config option. Changing the value would break the pipeline. - Add url config option. - Fix basic auth parameters in handlebar template (missing password option, and username was set to password). - Add missing field definition for input.type. Remaining issues: ``` FAILURE DETAILS: zoom/webhook test-chat-message.json: [0] parsing field value failed: field "zoom.chat_message.message"'s Go type, <nil>, does not match the expected field type: keyword (field value: <nil>) zoom/webhook test-phone.json: [0] parsing field value failed: field "zoom.phone.duration"'s Go type, string, does not match the expected field type: long (field value: 1235) ERROR: simulating pipeline processing failed: unexpected response status for Simulate (400): 400 Bad Request: elasticsearch error (type=illegal_argument_exception): field [members] not present as part of path [zoom.chat_channel.members] ```

elasticmachine · 2021-02-01T02:54:57Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Build Cause: Pull request #598 updated
- Start Time: 2021-02-02T18:37:14.777+0000
Duration: 33 min 30 sec
Commit: d15a81e

Test stats 🧪

Test	Results
Failed	0
Passed	1587
Skipped	0
Total	1587

- Convert `zoom.phone.duration` to a long from a string. - Drop null `zoom.chat_message.message` values. - Add ignore_missing to foreach loop dealing with zoom.chat_channel.members. - Add display name to `related.user` for zoom.chat_channel events (otherwise that info was lost).

andrewkroh · 2021-02-01T19:25:43Z

packages/zoom/data_stream/webhook/elasticsearch/ingest_pipeline/phone.yml

 - script:
    lang: painless
-    if: ctx?.zoom?.duration != null
+    if: ctx?.zoom?.phone?.duration != null


@P1llus This was referencing zoom.duration. Should it be referencing zoom.phone.duration? I change a few references in this file for that issue.

Second question -- how do we know it's in minutes? I didn't see any units in the zoom docs. For a voicemail to be in minutes duration would be surprising.

@andrewkroh
Its been a while, but both of those are correct assumptions. This one does not seem to be in minutes, though it is a bit hard to know for sure 100%. In most cases I tested myself with the API, though voicemail I would have to depend on the schemas.

Usually there is a 90-100% similarity with the webhook objects and the REST API Objects, and looking at that as a reference, since those API's actually has example documents:
Example: https://marketplace.zoom.us/docs/api-reference/zoom-api/phone/phoneuservoicemails

voice_mails": [ { "id": "Excepteur", "date_time": "2019-05-19T20:00:00Z", "download_url": "ani_maborumu7labojgde.com", "duration": "18677", "caller_number": "12345678", "caller_number_type": "1", "caller_name": "ullamconame", "callee_number": "34567889", "callee_number_type": "2", "callee_name": "somename", "status": "read" } ]

It would seem to commonly be either in seconds or milliseconds, though it's hard to say with 100% certainty what the number would represent if it was received from an actual webhook.

Ok, then I'm not going to touch the calculation at this time since I don't know what to change it to.

elasticmachine · 2021-02-01T19:29:09Z

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)

P1llus

Some really nice changes here, thanks for fixing up some of the broken references. LGTM for me, though the question around duration is still a bit uncertain.

There was testing done with this manually, but it's been quite some time, and there was a reason why I initially went with thinking minutes.

P1llus · 2021-02-01T21:28:03Z

packages/zoom/data_stream/webhook/agent/stream/http_endpoint.yml.hbs

+prefix: zoom
 basic_auth: {{basic_auth}}
-username: {{password}}
+username: {{username}}


I think we can remove username and password, as the Zoom Webhook does not support Basic Auth, though if they want to have a proxy between then users could always have the proxy authenticate? WDYT?

The secret.header and secret.value should at least be taking care of any filtering of unwanted data.

If basic auth is not supported then I'll remove it from here to de-clutter the UI.

P1llus · 2021-02-01T21:29:58Z

packages/zoom/data_stream/webhook/_dev/test/system/test-webhook-config.yml

+    listen_address: 0.0.0.0
+    listen_port: 9080
+    url: /zoom
+    secret_value: abc123


Might not be relevant, but its secret.value. The secret.header used by zoom is also authorization if we want to simulate that somehow

The test is relying on the default being Authorization. The webhook test client is sending Authorization: abc123.

P1llus · 2021-02-01T21:35:09Z

Might be others that want to review as well, from my side I looked at the input and output data structure, the pipelines, default configs etc, but might be someone that also want to look at the package/integration parts as well.

- Add configuration textbox for SSL options. - Hide options that are advanced/optional. - Remove options that should not be configurable. - Add note about HTTPS requirement from Zoom. - Link to TLS documentation. - Add additional test for HTTPS.

andrewkroh · 2021-02-02T18:44:30Z

This shows the UI change:

And these are the advanced options:

andrewkroh added the Team:Security-External Integrations label Feb 1, 2021

andrewkroh mentioned this pull request Feb 1, 2021

Add automated tests for security packages #377

Closed

andrewkroh commented Feb 1, 2021

View reviewed changes

andrewkroh marked this pull request as ready for review February 1, 2021 19:29

P1llus approved these changes Feb 1, 2021

View reviewed changes

Refactor Zoom config layout

d15a81e

- Add configuration textbox for SSL options. - Hide options that are advanced/optional. - Remove options that should not be configurable. - Add note about HTTPS requirement from Zoom. - Link to TLS documentation. - Add additional test for HTTPS.

andrewkroh merged commit 7aea6dd into elastic:master Feb 2, 2021

andrewkroh mentioned this pull request Feb 18, 2021

Sync zoom package with beats #719

Merged

2 tasks

andrewkroh added the Integration:zoom Zoom label Mar 27, 2025

Conversation

andrewkroh commented Feb 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Checklist

Related issues

Screenshots

Uh oh!

elasticmachine commented Feb 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💚 Build Succeeded

Build stats

Test stats 🧪

Uh oh!

andrewkroh Feb 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

P1llus Feb 1, 2021

Choose a reason for hiding this comment

Uh oh!

andrewkroh Feb 2, 2021

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Feb 1, 2021

Uh oh!

P1llus left a comment

Choose a reason for hiding this comment

Uh oh!

P1llus Feb 1, 2021

Choose a reason for hiding this comment

Uh oh!

andrewkroh Feb 2, 2021

Choose a reason for hiding this comment

Uh oh!

P1llus Feb 1, 2021

Choose a reason for hiding this comment

Uh oh!

andrewkroh Feb 2, 2021

Choose a reason for hiding this comment

Uh oh!

P1llus commented Feb 1, 2021

Uh oh!

andrewkroh commented Feb 2, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

andrewkroh commented Feb 1, 2021 •

edited

Loading

elasticmachine commented Feb 1, 2021 •

edited

Loading

andrewkroh Feb 1, 2021 •

edited

Loading