zoom: clean-up

[efd6]

Proposed commit message

* remove redundant ctx null-safe field accesses
* use non-escaping mustache templates
* remove redundant quotes
* make long contains lists render with a reasonable width
* use direct comparison when lists have only a single element
* ensure that null-safe field access is uniformly applied
* ensure that null-safe method calls are correctly applied

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.
• I have added an entry to my package's changelog.yml file.
• I have verified that Kibana version constraints are current according to guidelines.

Author's Checklist

• [ ]

How to test this PR locally

Related issues

• For Incorrect null handling in if conditions and elsewhere #8646

Screenshots

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: 63b7509

cc @efd6

[elastic-sonarqube]

Quality Gate failed

Failed conditions

9.6% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube

[elasticmachine]

Pinging @elastic/security-service-integrations (Team:Security-Service Integrations)

[efd6]

SonarQube is missing coverage. For example, with this

diff --git a/packages/zoom/data_stream/webhook/elasticsearch/ingest_pipeline/chat_channel.yml b/packages/zoom/data_stream/webhook/elasticsearch/ingest_pipeline/chat_channel.yml
index 2e54d61c7..0f4cebd47 100644
--- a/packages/zoom/data_stream/webhook/elasticsearch/ingest_pipeline/chat_channel.yml
+++ b/packages/zoom/data_stream/webhook/elasticsearch/ingest_pipeline/chat_channel.yml
@@ -1,6 +1,8 @@
 ---
 description: Pipeline for parsing Zoom chat_channel webhooks
 processors:
+- fail:
+    message: sonarqube says we don't get here
 - append:
     field: event.type
     value: user

I see this

╭─────────┬─────────────┬───────────┬────────────────────────┬─────────────────────────────────────────────────────────────────────────┬──────────────╮
│ PACKAGE │ DATA STREAM │ TEST TYPE │ TEST NAME              │ RESULT                                                                  │ TIME ELAPSED │
├─────────┼─────────────┼───────────┼────────────────────────┼─────────────────────────────────────────────────────────────────────────┼──────────────┤
│ zoom    │ webhook     │ pipeline  │ test-account.json      │ PASS                                                                    │   2.114431ms │
│ zoom    │ webhook     │ pipeline  │ test-chat-channel.json │ FAIL: test case failed: Expected results are different from actual ones │    5.36548ms │
│ zoom    │ webhook     │ pipeline  │ test-chat-message.json │ PASS                                                                    │   3.733451ms │
│ zoom    │ webhook     │ pipeline  │ test-meeting.json      │ PASS                                                                    │  10.498784ms │
│ zoom    │ webhook     │ pipeline  │ test-phone.json        │ PASS                                                                    │   8.330816ms │
│ zoom    │ webhook     │ pipeline  │ test-recording.json    │ PASS                                                                    │    5.23241ms │
│ zoom    │ webhook     │ pipeline  │ test-user.json         │ PASS                                                                    │  11.563933ms │
│ zoom    │ webhook     │ pipeline  │ test-webinar.json      │ PASS                                                                    │   9.307289ms │
│ zoom    │ webhook     │ pipeline  │ test-zoomroom.json     │ PASS                                                                    │   4.913983ms │
╰─────────┴─────────────┴───────────┴────────────────────────┴─────────────────────────────────────────────────────────────────────────┴──────────────╯

But according to the report, this is not possible; it claims zero coverage on that pipeline by inference.

[chrisberkhout]

Proposed commit message is good.
Everything in the diff is good.
SonarQube results don't look meaningful.

[chrisberkhout]

I looked into the coverage issue.

First I repeated something like the "fail at the start of the pipeline" example above, but generating coverage reports locally with elastic-package test pipeline --test-coverage -v. That worked as expected.

---

In this case I added:

--- a/packages/zoom/data_stream/webhook/elasticsearch/ingest_pipeline/chat_channel.yml
+++ b/packages/zoom/data_stream/webhook/elasticsearch/ingest_pipeline/chat_channel.yml
@@ -1,6 +1,9 @@
 ---
 description: Pipeline for parsing Zoom chat_channel webhooks
 processors:
+- fail:
+    message: do we get here?
+    if: "false"
 - append:
     field: event.type
     value: user

With that in place I ran:

elastic-package test pipeline --test-coverage -v

Then I changed the "false" to "true" and re-ran it.

Here are the results side-by-side, and the relevant pipeline:

When the condition on the fail step is met, that gets all the hits and the other steps get none, as expected.

---

Then I went through the .buildkite/scripts directory a bit more. In Buildkite I found where the various coverage reports are merged, downloaded the coverage_merged.xml artifact, and the unmerged artifacts from the "Check integrations zoom" (.buildkite/scripts/test_one_package.sh) job.

The merge was okay, but for pipeline tests the coverage files showed only account.yml and default.yml as having coverage. It looks like one pipeline test case is overwriting the results from the others, or at least only one is being written out correctly.

The CI job uses the generic coverage format, not the default cobertura format. When I re-ran locally with:

elastic-package test pipeline -v --test-coverage --coverage-format=generic

It produced the same results as CI: coverage was only registered for default.yml and account.yml.

I filed the issue elastic/elastic-package#1666.

[elasticmachine]

Package zoom - 1.18.1 containing this change is available at https://epr.elastic.co/search?package=zoom