Bro Plugin cmake can break plugins in a cluster environment.

Description

cmake/BroPluginDynamic.cmake can cause Bro plugins (via bro-pkg) to create a link of their scripts/ directory that points to the .bro-pkg environment. In a cluster setup, the workers do not have .bro-pkg.

In particular, this causes Seth's sethhall/bro-myricom to fail in a cluster if you don't jump through some additional hoops and it's really hard to debug for anyone that makes that switch. There is potential for this to break other plugins people write as well. See https://github.com/sethhall/bro-myricom/issues/1 for more details.

We believe the fix should be to simply change 'ln -s' to cp -R' as such:
157 #COMMAND test -d ${BRO_PLUGIN_SCRIPTS_SRC} && rm -f ${BRO_PLUGIN_SCRIPTS} && ln -s ${BRO_PLUGIN_SCRIPTS_SRC} ${BRO_PLUGIN_SCRIPTS} || true)
158 COMMAND test -d ${BRO_PLUGIN_SCRIPTS_SRC} && rm -f ${BRO_PLUGIN_SCRIPTS} && cp -R ${BRO_PLUGIN_SCRIPTS_SRC} ${BRO_PLUGIN_SCRIPTS} || true)

I've set this priority to "High" because the current state will break anyone's installation using the plugin when they try to upgrade.

Environment

CentOS 7 cluster.

Activity

Show:
Jon Siwek
September 30, 2017, 3:25 AM

The one
tricky issue was getting bro-pkg to respect the new version and I wasn't
familiar with git version tags, but I think we're good.

Sorry that didn't cross my mind to mention.

It still feels like those scripts should be in site/packages/bro-myricom
instead of the plugin directory, but I'll leave that to the maintainer.

I think it's more a general problem with how Bro plugins were originally intended to be organized. E.g. everything, including scripts, live under a single directory in lib/ so that when the plugin's shared object gets loaded, it's easy to also automatically load the Bro scripts that contain the low-level builtins and type definitions since they live in pre-defined locations relative to the plugin's root dir.

With bro-pkg, it's easier to go one step further and separate the Bro scripts into 2 categories: (1) the low-level builtins and type definitions that need to get loaded along with the shared lib and (2) the higher-level Bro scripts that build new functionality on top of that low-level stuff. So generally, it's pretty easy to send (2) into site/ and keep (1) in lib/ so bro-pkg load/unload commands will operate just in relation to (2).

But yeah, from just a filesystem organization perspective, I agree it's a bit unintuitive for any Bro scripts to be living under lib/, but not sure how much it would take to improve that.

Just, FYI, if you have a package that require a .tgz and you forget to
upgrade bro-pkg, you'll get this error:
...
OSError: [Errno 20] Not a directory: '/home/bro/.bro-pkg/testing/
bro-myricom/clones/bro-myricom/build/Bro_Myricom.tgz'

Don't think I can do anything to improve that since it's in already-released code.

I'll leave this ticket open in case Robin wants to accept the original idea of changing BroPluginDynamic.cmake to copy instead of symlink: it may make package configuration marginally simpler, but maybe at expense of making plugin development marginally more tedious.

MichaelD
September 30, 2017, 3:28 AM

This also made me think that it might be good for bro-pkg dependencies to
be able to specific the bro-pkg version as well. I think right now it's
either just 'bro' or package versions.

-Dop

On Fri, Sep 29, 2017 at 12:25 PM, Jon Siwek (JIRA) <

Jon Siwek
September 30, 2017, 3:34 AM

This also made me think that it might be good for bro-pkg dependencies to
be able to specific the bro-pkg version as well. I think right now it's
either just 'bro' or package versions.

Ah, yeah, I'll see about adding that, thanks.

Jon Siwek
September 30, 2017, 4:21 AM

bro-pkg v1.2 now allows 'bro-pkg' to be entered in the 'depends' metadata field.

Jon Siwek
August 16, 2018, 3:10 AM

Seems original issue is solved and nothing more that will be done here.

Fixed

Assignee

Unassigned

Reporter

MichaelD

Labels

External issue ID

None

Components

Affects versions

Priority

Normal