Let me put the sub-module discussion aside for a second. I don't really have much of an opinion on that quite yet. I need to research it a bit more, but thanks for sharing the pros/cons.
My main point is that I would like to see us update our deployment code to deploy the Bro plugin from the new code repository BEFORE we make any changes to the code. So the next bit of work would do the following: * Remove the legacy code from `metron/metron-sensors/bro-kafka-plugin` * Update `metron/metron-deployment/roles/bro` so that it builds and deploys the plugin to Full Dev from the new repository What do you think? On Wed, Nov 8, 2017 at 11:00 AM zeo...@gmail.com <zeo...@gmail.com> wrote: > 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 >
