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

Use zq's Zeek tag as the downloaded dependency #1215

Merged
merged 2 commits into from Nov 14, 2020
Merged

Conversation

philrz
Copy link
Contributor

@philrz philrz commented Nov 13, 2020

(This is a companion PR to brimdata/zed#1610, which would merge first once both are approved.)

Currently Brim's scripts/download-zdeps pulls down a specific Zeek artifact (and soon a Suricata one as well) to run tests and allows for the bundling of these artifacts with the app such that users can generate such logs on the desktop. zq similarly depends on the same artifacts during system tests to ensure pcaps posted to zqd produce expected log outputs.

Up until now, this all has meant updating the Zeek & Suricata tags in each repo separately, ideally keeping them always in sync. However, we already have linkage between the repos in the form of the advance-zq.yml Action that gets notified of a new zq commit hash whenever there's a merge to master on the zq side, ultimately committing the advanced zq pointer in Brim if the tests pass. By piggybacking on that same approach, we can maintain the Zeek/Suricata tags on just the zq side and ensure they start being used on the Brim side as long as triggered Brim-side tests pass.

The implementation of the zq build on the Brim side uses npm. Therefore in brimdata/zed#1610, the Zeek/Suricata tags are moved out of the Makefile and into zq's package.json, which is already used by this cross-repo build process. Now whenever an npm install is done on the Brim side, the contents of node_modules/zq/package.json will be updated with the Zeek/Suricata tags that were present on the zq side as of the zq commit hash referenced in Brim's package.json.

Here's some proof of it working by having done an npm install of the commit hash associated with the head commit from the branch in brimdata/zed#1610:

$ rm -rf zdeps
$ npm install https://github.com/brimsec/zq#a6eed836254f66ae551b0f7ca2f1af4942e04668

> zq@0.23.0-dev prepack /Users/phil/.npm/_cacache/tmp/git-clone-0555eaaa
> node npm/build

go build -ldflags="-s -X github.com/brimsec/zq/cli.Version=a6eed83" -o dist ./cmd/... ./ppl/cmd/...
+ zq@0.23.0-dev
updated 1 package and audited 2288 packages in 25.76s

80 packages are looking for funding
  run `npm fund` for details

found 345 vulnerabilities (328 low, 1 moderate, 16 high)
  run `npm audit fix` to fix them, or `npm audit` for details

$ cat node_modules/zq/package.json 
{
{
  "_from": "git+https://github.com/brimsec/zq.git#a6eed836254f66ae551b0f7ca2f1af4942e04668",
...
  "brimDependencies": {
    "suricataTag": "v5.0.3-brim8",
    "zeekTag": "v3.2.1-brim4"
  },
...
}

$ npm install

> Brim@0.19.0 postinstall /Users/phil/work/brim
> npm-run-all -s install:*


> Brim@0.19.0 install:zdeps /Users/phil/work/brim
> node scripts/download-zdeps

zeek v3.2.1-brim4 downloaded to /Users/phil/work/brim/zdeps/zeek
copied zq artifacts Version: a6eed83
...

$ ls -l zdeps
total 206128
-rwxr-xr-x  1 phil  staff  16150596 Nov 13 15:04 pcap
-rwxr-xr-x  1 phil  staff  18560572 Nov 13 15:04 zapi
-rwxr-xr-x  1 phil  staff  20997844 Nov 13 15:04 zar
drwxr-xr-x  6 phil  staff       192 Nov 13 15:04 zeek
-rwxr-xr-x  1 phil  staff  19844652 Nov 13 15:04 zq
-rwxr-xr-x  1 phil  staff  29973740 Nov 13 15:04 zqd

This same approach needs to work with tagged versions as well. Here we can see it failing because the tags aren't in the last GA tagged release. But we can see the package.json for that tagged release was still where it needs to be, so this should work fine the next time we tag a release.

$ npm install https://github.com/brimsec/zq#v0.23.0
+ zq@0.23.0-dev
updated 1 package and audited 2288 packages in 12.95s

80 packages are looking for funding
  run `npm fund` for details

found 345 vulnerabilities (328 low, 1 moderate, 16 high)
  run `npm audit fix` to fix them, or `npm audit` for details

$ npm install

> Brim@0.19.0 postinstall /Users/phil/work/brim
> npm-run-all -s install:*


> Brim@0.19.0 install:zdeps /Users/phil/work/brim
> node scripts/download-zdeps

zdeps setup:  TypeError: Cannot read property 'zeekTag' of undefined
    at main (/Users/phil/work/brim/scripts/download-zdeps/index.js:168:52)
    at Object.<anonymous> (/Users/phil/work/brim/scripts/download-zdeps/index.js:196:1)
    at Module._compile (internal/modules/cjs/loader.js:1015:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1035:10)
    at Module.load (internal/modules/cjs/loader.js:879:32)
    at Function.Module._load (internal/modules/cjs/loader.js:724:14)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:60:12)
    at internal/main/run_main_module.js:17:47
...

$ cat node_modules/zq/package.json 
{
  "_from": "git+https://github.com/brimsec/zq.git#v0.23.0",
  "_id": "zq@0.23.0-dev",
  "_inBundle": false,
  "_integrity": "",
  "_location": "/zq",
  "_phantomChildren": {},
  "_requested": {
    "type": "git",
    "raw": "https://github.com/brimsec/zq#v0.23.0",
    "rawSpec": "https://github.com/brimsec/zq#v0.23.0",
    "saveSpec": "git+https://github.com/brimsec/zq.git#v0.23.0",
    "fetchSpec": "https://github.com/brimsec/zq.git",
    "gitCommittish": "v0.23.0"
  },
  "_requiredBy": [
    "#USER",
    "/"
  ],
  "_resolved": "git+https://github.com/brimsec/zq.git#78762bb069a1662eb4b2cf9a0c57740dfc2c59d4",
  "_spec": "https://github.com/brimsec/zq#v0.23.0",
  "_where": "/Users/phil/work/brim",
  "bin": {
    "ast": "dist/ast",
    "microindex": "dist/microindex",
    "mockbrim": "dist/mockbrim",
    "pcap": "dist/pcap",
    "zapi": "dist/zapi",
    "zar": "dist/zar",
    "zq": "dist/zq",
    "zqd": "dist/zqd",
    "zst": "dist/zst"
  },
  "bundleDependencies": false,
  "deprecated": false,
  "description": "The `zq` repository contains tools and components used to search, analyze, and store structured log data, including:",
  "directories": {
    "bin": "dist"
  },
  "files": [
    "dist/**",
    "**/*.js"
  ],
  "name": "zq",
  "scripts": {
    "prepack": "node npm/build"
  },
  "version": "0.23.0-dev"
}

We can expect the CI will fail on this branch until something on the zq side is populating the package.json. Therefore if/when both PRs are approved, I'll plan to merge the zq one first such that Brim's zq pointer in package.json gets advanced in Brim's master, then catch this branch up with master so it'll run green before merging.

brim-bot pushed a commit that referenced this pull request Nov 13, 2020
…son" by philrz

This is an auto-generated commit with a zq dependency update. The zq PR
brimdata/zed#1610, authored by @philrz,
has been merged.

Move Zeek and Suricata version tags into package.json

(This is a companion PR to #1215.)

Currently zq's `make test-system` depends on specific Zeek and Suricata artifacts to ensure pcaps posted to `zqd` produce expected log outputs. Similar artifacts are also needed by Brim since Zeek and Suricata are bundled with the app such that users can generate similar logs on the desktop.

Up until now, this all has meant updating the Zeek & Suricata tags in each repo separately, ideally keeping them always in sync. However, we already have linkage between the repos in the form of the [notify-brim-merge.yml Action](https://github.com/brimsec/zq/blob/master/.github/workflows/notify-brim-merge.yml) that triggers a run of Brim's test automation against a new zq commit hash as soon as it's merged to master, ultimately committing the advanced zq pointer in Brim if the tests pass. By piggybacking on that same approach, we can maintain the Zeek/Suricata tags on just the zq side and ensure they start being used as long as triggered Brim-side tests pass.

The implementation of the zq build on the Brim side uses `npm`. Therefore, here I've taken the approach of moving the Zeek/Suricata tags out of the Makefile and into `package.json`, which is already used by this cross-repo build process. I'd done some poking around online and found a [StackOverflow thread](https://stackoverflow.com/questions/10065564/add-custom-metadata-or-config-to-package-json-is-it-valid/27232456#27232456) indicating that it's kosher to add custom metadata to `package.json` like I'm doing here, so I've opted for that route rather than creating some new separate config file.
@philrz philrz merged commit 571ef8d into master Nov 14, 2020
@philrz philrz deleted the tagfile-zeek-suricata branch November 14, 2020 01:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants