[Bro-Dev] [JIRA] (BIT-1185) topic/dnthayer/broctld-work

[Daniel Thayer (JIRA)]

    [ https://bro-tracker.atlassian.net/browse/BIT-1185?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16209#comment-16209 ] 

Daniel Thayer commented on BIT-1185:
------------------------------------

{quote}
> Added warning to do a "broctl install" if broctl or node config changes.

The code here looks a bit fragile to me as it depends on the right format being written out so that it can then later be reparsed. I'm wondering if we could just compute a hash of the two files instead and suggest a reinstall once that changes?
That would trigger on insignificant changes, too, like comments, but I don't think that would hurt much.
{quote}

Just doing a hash of broctl.cfg itself wouldn't work very well, because there are various ways that the 
broctl config can change:
1) someone edits broctl.cfg (although some changes here wouldn't actually cause anything in the config to change,
 such as comments, whitespace, option ordering, or when someone adds an option that wasn't previously in the
 file but uses the default value),
2) someone changes a default option value in the broctl source code,
3) a dynamically-generated broctl option value changes (such as "standalone", or "time"),
4) someone changes a custom plugin, which defines its own broctl options

Admittedly, #2 and #3 are probably quite unlikely to happen in real usage.

So, we'd need to do the hash over the entire config (not just broctl.cfg) in
order to make sure that we notice all possible config changes.
But, this would sometimes warn users to "install" even when they don't need to
because there are a significant number of broctl options that do not require
an "install" when their value changes (but not sure how often users would encounter
this situation).

As a future enhancement, we could add a new attribute to each broctl option
which would be a boolean that determines whether or not the option's value
change should trigger an "install" or not.  However, this wouldn't work
if we just do a hash of the configuration.

{quote}
Also, the current code can miss some cases: like when a node gets removed, I don't think it will catch that (because it's not in the list of nodes iterated through in warnBroctlInstall().
{quote}

Yes, you're right.  I've fixed that now.


> topic/dnthayer/broctld-work
> ---------------------------
>
>                 Key: BIT-1185
>                 URL: https://bro-tracker.atlassian.net/browse/BIT-1185
>             Project: Bro Issue Tracker
>          Issue Type: Problem
>          Components: BroControl
>            Reporter: Daniel Thayer
>             Fix For: 2.3
>
>
> This branch contains some code cleanup and also fixes or improves the
> following issues:
> The df, exec, and top commands now run only once per host.
> Avoid reporting same disk check error msg multiple times for same host.
> Improve output column formatting.
> Added warning to do a "broctl install" if broctl or node config changes.
> Don't email about "$total" pseudo-node not receiving any packets.
> Remove unused "home" broctl option.
> Changed plugin API hosts() function to be more useful.



--
This message was sent by Atlassian JIRA
(v6.3-OD-02-026#6318)