So, here's my argument against the sub-module approach: - If we add a sub-module into apache/metron then the way you clone from github changes (--recursive). This could break any instructions to spin up Metron full-dev, when they're using the bro plugin (forums, blog posts, metron READMEs, etc.). - This also requires that we commit a change to apache/metron in order to make any changes to apache/metron-bro-plugin-kafka effective in the project. Not that big of a deal, but notable. - The overhead of these two changes at once is very minimal, since packaging is effectively moving the same commands we run in ansible to be run by bro-pkg.
If we follow the approach I lined up before we can version the package in bro-pkg.meta, and when our scripts are used spin up bro we can just pin a specific version of the package, and bump it up in apache/metron as we want to use updates of apache/metron-bro-plugin-kafka. That said, this is implying that we're okay making two changes at once with my method - use an external repo for the sensor, and move to a package from a plugin. Since moving from a package to a plugin is literally just the following commands, I'm not too concerned: - Clone the package repo and checkout to the right branch, if applicable - Run the built-in unit tests. If they fail, alert the user and get confirmation if they want to move forward - Run the configure and make commands You can see an aggregate of all bro package configs here <https://github.com/bro/packages/blob/master/aggregate.meta>, which show different scenarios/examples of what needs to be added to make something into a bro package - it is rather straightfoward. Happy to continue discussing. Jon On Wed, Nov 8, 2017 at 9:34 AM Nick Allen <n...@nickallen.org> wrote: > I would suggest that we shift our existing deployment routines to use the > new code repository, prior to making changes to "packag-ify" it. That way > we know that our code reorganization is definitely working and has been > successful. > > Prior to step 2, we would... > * Add a sub-module pointing to the repo and ensure that the Ansible > deployment to Full Dev can deploy Bro with the Kafka plugin > > > > > > On Tue, Nov 7, 2017 at 9:19 AM, zeo...@gmail.com <zeo...@gmail.com> wrote: > > > So here's an update on this, and I'm looking for any suggestions > regarding > > the roadmap. If anybody isn't clear about the implementation specifics > or > > history here, I'm happy to reiterate, but it has also been discussed on > the > > mailing list in the past. > > > > Right now, we have a direct migration of metron-sensors/bro-plugin- > > kafka[1] > > to its own repository[2], which is required in order to turn it into a > bro > > package (the preferred mechanism of managing and installing bro plugins > as > > of 2.5). Bro 2.5 also has an improvement[3] which I think we should > > consider important to take advantage of in our testing environments and > > instructions. Here is a more well-defined roadmap suggestion: > > > > 1. *DONE* - Move the bro-plugin-kafka to its own repository (Prereq > > to METRON-813). > > 2. *PR OPEN* - Reorganize the plugin naming to be more appropriate for a > > package (METRON-1303, Prereq to METRON-813). > > 3. Transform the plugin to be a package containing a plugin > (METRON-813). > > 4. Add the package to bro/packages[4]. > > During 2-4. Upgrade full-dev to run on CentOS 7 (METRON-559, soft prereq > > to METRON-1088). > > 5. Upgrade bro to 2.5.2 (METRON-1088). > > 6. Update metron-deployment to install the package instead of using the > > plugin in apache/metron. > > 7. Remove the plugin[1] from apache/metron entirely. > > 8. Add features/improvements (METRON-1304, METRON-1305, etc.) to > > apache/metron-bro-plugin-kafka. > > - This is why my METRON-1304 PR has "DO NOT MERGE" - it was simply an > > easy win to bust out this morning while I was thinking of it. > > > > Some alternatives we may want to consider: > > - In the interim, we could change our ansible scripts to pull in the code > > from the external repository, but build it the same as we currently do > (as > > a plugin but not a package). > > - We could replace bro-plugin-kafka folder with a sub-module pointing to > > the external repo, but this may be more trouble than it's worth (new > > cloning instructions, new folder name, etc.). > > - We can try to work around some of the issues with running Bro 2.5 on > > CentOS 6, but it would be less preferred. > > > > Any comments are welcome. > > > > 1: > > https://github.com/apache/metron/tree/master/metron- > > sensors/bro-plugin-kafka > > 2: https://github.com/apache/metron-bro-plugin-kafka > > 3: https://www.bro.org/sphinx/cluster/index.html#logger > > 4: https://github.com/bro/packages > > > > On Tue, Nov 7, 2017 at 8:52 AM Nick Allen <n...@nickallen.org> wrote: > > > > > > As of bro-pkg 1.1, I need to redo my `bro-pkg.meta` work to support > the > > > newly-added > > > `external_depends`, and also upgrade to bro 2.5.2 > > > > > > Isn't upgrading to 2.5.2 an enhancement that we need to wait on before > we > > > finish some clean-up tasks? > > > > > > What all do you think we need to do before we start accepting > > enhancements? > > > > > > Thanks for the update and all the hard work, Jon. > > > > > > On Mon, Nov 6, 2017 at 10:02 PM, zeo...@gmail.com <zeo...@gmail.com> > > > wrote: > > > > > > > Sorry for the delay here - I pushed this out tonight (link > > > > <https://github.com/apache/metron-bro-plugin-kafka/commits/master>, > > link > > > > < > https://git-wip-us.apache.org/repos/asf?p=metron-bro-plugin-kafka.git > > > >). > > > > As of bro-pkg 1.1, I need to redo my `bro-pkg.meta` work to support > the > > > > newly-added `external_depends`, and also upgrade to bro 2.5.2 > (somewhat > > > > non-trivial due to the C++11 requirement, and new bro log > files/fields) > > > so > > > > we can use the bro package manager to install the plugin. Hopefully > I > > > can > > > > get this wrapped up soon so we can accept external PRs like this one > > > > <https://github.com/JonZeolla/metron-bro-plugin-kafka/pull/1>. > > > > > > > > Jon > > > > > > > > On Mon, Sep 18, 2017 at 11:52 AM Nick Allen <n...@nickallen.org> > > wrote: > > > > > > > > > Nice! Looks good to me. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Sep 18, 2017 at 11:35 AM zeo...@gmail.com < > zeo...@gmail.com> > > > > > wrote: > > > > > > > > > > > Okay, I took a stab at it this morning, can I get a double check > > > before > > > > > > pushing it out? The latest commit would be opened as a PR. > > > > > > > > > > > > https://github.com/JonZeolla/metron-bro-plugin-kafka/tree/dev > > > > > > > > > > > > Jon > > > > > > > > > > > > On Fri, Sep 15, 2017 at 12:54 PM zeo...@gmail.com < > > zeo...@gmail.com> > > > > > > wrote: > > > > > > > > > > > > > Good point, I can take that task re migrating the revision > > history > > > of > > > > > the > > > > > > > folder. > > > > > > > > > > > > > > Jon > > > > > > > > > > > > > > On Fri, Sep 15, 2017, 12:14 Nick Allen <n...@nickallen.org> > > wrote: > > > > > > > > > > > > > >> Hi Jon - > > > > > > >> > > > > > > >> I agree with you on the approach. We should first copy > > everything > > > > as > > > > > it > > > > > > >> is > > > > > > >> to the new repo. We should maintain the revision history too. > > > I'm > > > > > sure > > > > > > >> there is a way to do it, but would have to research a bit. > Then > > > we > > > > > > apply > > > > > > >> your changes on top of that. > > > > > > >> > > > > > > >> Thanks > > > > > > >> > > > > > > >> On Thu, Sep 14, 2017 at 1:36 AM, zeo...@gmail.com < > > > zeo...@gmail.com > > > > > > > > > > > >> wrote: > > > > > > >> > > > > > > >> > So, I've been working on METRON-813 > > > > > > >> > <https://issues.apache.org/jira/browse/METRON-813> lately > > and I > > > > > have > > > > > > an > > > > > > >> > initial run at it ready to go here > > > > > > >> > <https://github.com/JonZeolla/metron-bro-plugin-kafka> > > > (squashed > > > > > > >> history, > > > > > > >> > see a better history there > > > > > > >> > < > > > > > > https://github.com/JonZeolla/metron-bro-plugin-kafka/commits/bro-pkg > > > > > > >> >). > > > > > > >> > Since the metron-bro-plugin-kafka repo is empty, I can't > open > > a > > > PR > > > > > > >> against > > > > > > >> > it on GitHub for review. Does anybody have a suggestion > > > regarding > > > > > how > > > > > > >> to > > > > > > >> > move forward? I see two options: > > > > > > >> > 1. I make the initial commit a direct copy of the > > > bro-plugin-kafka > > > > > > >> folder > > > > > > >> > <https://github.com/apache/metron/tree/master/metron- > > > > > > >> > sensors/bro-plugin-kafka> > > > > > > >> > (I believe this would require a new JIRA for a direct copy), > > and > > > > > then > > > > > > >> open > > > > > > >> > a PR for the METRON-813 changes to get reviewed via the > normal > > > > > > process. > > > > > > >> > 2. I make the initial commit the result of METRON-813, but > > > review > > > > > > occurs > > > > > > >> > via the mailing list and using my fork. > > > > > > >> > > > > > > > >> > I prefer 1, but wanted to put it up for discussion. Once we > > > > decide > > > > > on > > > > > > >> the > > > > > > >> > correct approach then I would be happy to put together a > > testing > > > > > plan > > > > > > >> for > > > > > > >> > the PR as well. > > > > > > >> > > > > > > > >> > Just to clarify, the general roadmap for getting this used > in > > > > > > >> apache/metron > > > > > > >> > is: > > > > > > >> > 1. Create a bro package in apache/metron-bro-plugin-kafka > > > > > > >> > 2. Update the ansible bro setup > > > > > > >> > <https://github.com/apache/metron/tree/master/metron- > > > > > > >> > deployment/roles/bro/tasks> > > > > > > >> > to install/configure bro-pkg (`pip install bro-pkg && > bro-pkg > > > > > > >> autoconfig`) > > > > > > >> > and use it to install the apache/metron-bro-plugin-kafka > > > package. > > > > > > >> > > > > > > > >> > I will also be adding this to the official bro package > manager > > > > > > >> > <https://github.com/bro/packages>, but out of an abundance > of > > > > > > caution I > > > > > > >> > plan to setup ansible to pull the package directly from the > > > > > > >> > apache/metron-bro-plugin-kafka using bro-pkg instead of > going > > > > > through > > > > > > >> the > > > > > > >> > bro/packages package source (which removes the bro/packages > > > > > > dependency). > > > > > > >> > > > > > > > >> > Feedback on all of the above is welcome. > > > > > > >> > > > > > > > >> > Jon > > > > > > >> > -- > > > > > > >> > > > > > > > >> > Jon > > > > > > >> > > > > > > > >> > > > > > > > -- > > > > > > > > > > > > > > Jon > > > > > > > > > > > > > -- > > > > > > > > > > > > Jon > > > > > > > > > > > > > > > -- > > > > > > > > Jon > > > > > > > > > -- > > > > Jon > > > -- Jon
