-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Heartbeat: add support for pushed monitor source #31428
Heartbeat: add support for pushed monitor source #31428
Conversation
85f0c4d
to
a54114f
Compare
Pinging @elastic/uptime (Team:Uptime) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall it LGTM but I would prefer to deal with the zipslip
thing before merging this one in. Excellent work, I really like the tests (nice setup/teardown with the subtests too).
I have also E2E tested this PR by:
- Creating the following journey:
import { journey, step, monitor } from "@elastic/synthetics"; journey("latency", ({ page }) => { monitor.use({}); step("load homepage", async () => { await page.goto("https://google.com"); }); step("wait for test", async () => { await page.waitForTimeout(2e3); }); });
- I used this PR to bundle it into a zip
- Got the base64 encoded content
base64 ~/bla.zip | pbcopy
- Created this monitor in Heartbeat pointing to my local Kibana
- type: browser id: my-monitor name: My Monitor schedule: '@every 1m' source: pushed: content: UEsDBBQACAAIAGVllFQAAAAAAAAAAAAAAAAdAAAAc3ludGgtZXhhbXBsZS90ZXN0LmpvdXJuZXkuanOFz8FKxDAQgOF7n2LIKQtrA3rbRfHkA4g+wBCnbZamEzJTtJS+u0ldF28eAsPA/yVxDlCE9OTehbK4cfYonWdRdK+UWIJyDiROlkmHO/rCmEZySqLthec80dJepAkxcVZY4bo7giilI0SeKgAbdJkjmGcaUTT4H47KJObcXCNrRlSa/GKOYFdI2BNsB3h8grWBX6qdhey6Hc5lVe8oEeMHDBypBiXFYnuwtxAAPzHo7rU9K1szqCY5Odcz9yO1nqPZwb/s3nTl7fWv/7F1euH8FiLxrPaeHm5ePd9QSwcIGFh8rdsAAABmAQAAUEsBAi0DFAAIAAgAZWWUVBhYfK3bAAAAZgEAAB0AAAAAAAAAAAAgAKSBAAAAAHN5bnRoLWV4YW1wbGUvdGVzdC5qb3VybmV5LmpzUEsFBgAAAAABAAEASwAAACYBAAAAAA==
- Checked Kibana and saw results there 🎉
PS: Ideally one must either use a new locally built version of the agent or not use monitor.use
to avoid the monitor failing.
return err | ||
} | ||
|
||
tf, err := ioutil.TempFile("/tmp", "elastic-synthetics-zip-") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we also defer
the close
of this file? Although we're already removing it the docs don't mention Remove
closes the open handle.
PS: I found it interesting to learn how these are made to be globally unique
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its explicity handled after the monitor run to completion, we control the close using p.Close()
on any error and remove these unzipped directories.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the zipslip
update!
* Heartbeat: add support for pushed monitors * link to global synthetics pkg * add tests and link global path * fix lint issues * fix tests * add changelog * use install instead of link * handle zipslip vul
base64
decoded and unzipped in to the directory structure that was used to upload the monitors from the Synthetics command. HB basically does the reverse of what Synthetics CLI has done for uploading the monitors.TODO