Make NRPE lens less strict

[hillu]

See Debian bug #749919

[lutter]

I am wondering whether it wouldn't be better to have the lens parse the comma-separated list of hosts so that the test

  test Nrpe.item get "allowed_hosts=127.0.0.1, 127.0.0.2\n" =
    { "allowed_hosts" = "127.0.0.1, 127.0.0.2" }

would become

  test Nrpe.item get "allowed_hosts=127.0.0.1, 127.0.0.2\n" =
    { "allowed_hosts" = "127.0.0.1" }
    { "allowed_hosts" =  127.0.0.2" }

The reason for that is so that users of the lens do not have to parse the list of hosts themselves. This will require a little more work on the Nrpe lens, but if we are ever going to do that, this is the right time for that.

[lutter]

Since there is a little bit of trickery involved, The main changes would be to disallow , in word, and then redefining item to

let item = [ key item_re . eq . store word .
             [ label "more" . del /, */ ", " . store word]* . eol ]

It's actually not possible to get the structure I mentioned in my last post (at least not with quite a bit more headache in the lens). What the above would result in is

test Nrpe.item get "allowed_hosts=127.0.0.1, 127.0.0.2\n" =
{ 
  { "allowed_hosts" = "127.0.0.1"
    { "more" = "127.0.0.2" }
  }
}

It's a little annoying that there is no way to get at the label of the parent node in the lens - otherwise we could just use that, instead of the somewhat goofy more.

[raphink]

Or even

{ "allowed_hosts"
   { "1" = "127.0.0.1" }
   { "2" =  127.0.0.2" }
}

which breaks compatibility but makes more sense.

[hillu]

Oh, apparently I never noticed your answers, sorry. I'd be happy to make changes to my patch if somebody tells me what the structure produced by the lens should look like. I don't really mind, but I suppose that breaking compatibility is not necessarily the best idea...

[lutter]

I would strongly prefer a solution that does not break backwards compatibility. If allowed_hosts is the only kind of item that allows lists of things in it, I'd do the following:

• disallow allowed_hosts in item_re
• add a new allowed_hosts lens that generates a tree structure like { "allowed_hosts" = "ip1" { "ip" = "ip2" } { "ip" = "ip3" } .. }
• add that new lens to the definition of lns

The allowed_hosts lens would look something like:

let allowed_hosts = [ key "allowed_hosts" . eq . store Rx.ip .
                            [ label "ip" . del /, */ ", " . store word]* . eol ]

[raphink]

@hillu do you still intend to get this PR merged? If so, could you address @lutter's comments please?

[hillu]

I currently don't have the time to do that.
The Debian package has contained my patch as proposed for almost two years.

[raphink]

@lutter what do we do about this? Not merging it will break compatibility in Debian because they merged the patch 3 years ago…

[lutter]

Yeah, that's unfortunate; we should just merge this as is, and live with the fact that we could have offered a better breakdown of the file.