Call for inncheck testing

Florian Schlichting fschlich at CIS.FU-Berlin.DE
Thu Jul 7 23:37:53 UTC 2011


Hi Julien, everyone,

thanks for extended testing!
I see there are a range of issues, let me offer remedies for some of
them...

> >>/news/etc/innfeed.conf: required key initial-sleep not defined in global scope.
> >>/news/etc/innfeed.conf: required key force-ipv4 not defined in global scope.
> >>
> >>This looks suspicious - not sure if these are really required.
> >
> >I have no idea if they are actually required or what will happen when
> >they're not there. The manpage says they *must* be specified at global
> >scope, so I thought it prudent for inncheck to complain. In principle,
> >though, it would be good if innfeed has safe defaults for everything...
> 
> These two keys are not required, and have default values.

I've done a little digging in the innfeed sources and have come to the
conclusion that the whole required-key-thing is bogus: *all* innfeed
configuration options have a default value! Most of them are set in
innfeed/host.c next to where params->forceIPv4 is set, and then there's
initialSleep which gets set in innfeed/main.c

So what I propose is to delete the notion of required keys from the
manpage (leaving only the differentiation between global-only options
and those that can appear everywhere) as well as from inncheck. And then
I think it would make things easier for admins of small sites if we ship
a sample innfeed.conf with everything commented out, in order to
symbolize that nothing needs to be touched unless there's a special
need. Even the ip-name of a peer defaults to the name set in newsfeeds,
so many people don't really need to touch innfeed.conf at all - or am I
missing something here?

Mind I haven't had a chance to test any of this yet - I'm on beach
vacation until the end of the month, with only a netbook and a mobile
internet connection...

So this removes the required-keys check from inncheck (please check if
it has all the current options):

--- a/scripts/inncheck.in
+++ b/scripts/inncheck.in
@@ -529,45 +529,44 @@ inn_conf
 sub
 innfeed_conf
 {
-    my %required_globals = ( # allowed anywhere, but have to exist in global scope
-        'article-timeout'       => 'number',
-        'response-timeout'      => 'number',
-        'initial-sleep'         => 'number',
-        'initial-connections'   => 'number',
-        'max-connections'       => 'number',
-        'dynamic-method'        => 'number',
-        'dynamic-backlog-low'   => 'floating-point number (0.0-100.0)',
-        'dynamic-backlog-high'  => 'floating-point number (0.0-100.0)',
-        'dynamic-backlog-filter'=> 'floating-point number',
-        'max-queue-size'        => 'number',
-        'streaming'             => 'boolean',
-        'no-check-high'         => 'floating-point number (0.0-100.0)',
-        'no-check-low'          => 'floating-point number (0.0-100.0)',
-        'no-check-filter'       => 'floating-point number',
-        'bindaddress'           => 'IPv4 address / "any / "none"',
-        'bindaddress6'          => 'IPv6 address / "any / "none"',
-        'port-number'           => 'number',
-        'force-ipv4'            => 'boolean',
-        'drop-deferred'         => 'boolean',
-        'min-queue-connection'  => 'boolean',
-        'no-backlog'            => 'boolean',
-        'backlog-limit'         => 'number',
-        'backlog-factor'        => 'floating-point number',
-        'backlog-limit-highwater' => 'number',
-        'backlog-feed-first'    => 'boolean',
-        'username'              => 'string',
-        'password'              => 'string',
-        'deliver-authname'      => 'string',
-        'deliver-password'      => 'string',
-        'deliver-username'      => 'string',
-        'deliver-realm'         => 'string',
-        'deliver-rcpt-to'       => 'string',
-        'deliver-to-header'     => 'string',
-    );
     my %groups = parse_config(
         {
-            '<anywhere>'  => \%required_globals,
-            '<global scope>'    => {    # not required, defaults exist
+            '<anywhere>'        => {
+                'article-timeout'       => 'number',
+                'response-timeout'      => 'number',
+                'initial-sleep'         => 'number',
+                'initial-connections'   => 'number',
+                'max-connections'       => 'number',
+                'dynamic-method'        => 'number',
+                'dynamic-backlog-low'   => 'floating-point number (0.0-100.0)',
+                'dynamic-backlog-high'  => 'floating-point number (0.0-100.0)',
+                'dynamic-backlog-filter'=> 'floating-point number',
+                'max-queue-size'        => 'number',
+                'streaming'             => 'boolean',
+                'no-check-high'         => 'floating-point number (0.0-100.0)',
+                'no-check-low'          => 'floating-point number (0.0-100.0)',
+                'no-check-filter'       => 'floating-point number',
+                'bindaddress'           => 'IPv4 address / "any / "none"',
+                'bindaddress6'          => 'IPv6 address / "any / "none"',
+                'port-number'           => 'number',
+                'force-ipv4'            => 'boolean',
+                'drop-deferred'         => 'boolean',
+                'min-queue-connection'  => 'boolean',
+                'no-backlog'            => 'boolean',
+                'backlog-limit'         => 'number',
+                'backlog-factor'        => 'floating-point number',
+                'backlog-limit-highwater' => 'number',
+                'backlog-feed-first'    => 'boolean',
+                'username'              => 'string',
+                'password'              => 'string',
+                'deliver-authname'      => 'string',
+                'deliver-password'      => 'string',
+                'deliver-username'      => 'string',
+                'deliver-realm'         => 'string',
+                'deliver-rcpt-to'       => 'string',
+                'deliver-to-header'     => 'string',
+            },
+            '<global scope>'    => {
                 'news-spool'            => 'path',
                 'input-file'            => 'path',
                 'pid-file'              => 'path',
@@ -601,12 +600,6 @@ innfeed_conf
             },
         }
     );
-    # check presence of required keys in global scope
-    foreach (keys %required_globals) {
-        next if /bindaddress|bindaddress6|username|password|deliver/; # not required
-        eprint "$file: required key $_ not defined in global scope.\n"
-            unless defined $groups{'<global scope>'}->{$_};
-    }
     # check some numeric values
     foreach my $group (keys %groups) {
         eprint "$file:$groups{$group}->{'type'} $groups{$group}->{'name'}: dynamic-method must be between 0 and 3\n"




> I can check that if you want and reorganize the man page.
> I am waiting for your confirmation or refusal, so that we do not do
> the work twice!

yes please re-check the "everything has defaults, actually" thing;
and I'd be happy to leave the manpage to you or do it when I'm back, as
I don't feel on top of things currently.

Some useful notes from my digging around:
    - what the manpage says are "common" values are usually the actual
      defaults
    - dynamic-method defaults to 0, not 3 as the manpage suggests
    - no-check-low defaults to 90.0
    - backlog-limit and backlog-factor default to 0,
      backlog-limit-highwater to 1.10 (the combination of which results
      in... what behaviour?)



> >And please keep testing...
> 
> I think you will like this one :)

you bet!

> In readers.conf:
> 
> include readers2.conf
> 
> The "include" directive is not understood.
> (Be careful that both readers.conf and included files can be
> partially formatted files.  It means that readers2.conf can for
> instance consist only of a closing bracket "}"???)

No. Admins are free to break and obfuscate their configuration in all
the ways they want, but that's for the "new" parser to sort out then.
To keep things manageable, I'm not going to hunt for included files, and
require that what gets included is a "complete" set of blocks and
options. That is to say, inncheck is going to ignore the include
statement, and if the admin wants to check the included file he has to
run inncheck again with readers.conf=includefile. Same for
innfeed.conf's $INCLUDE. Does that sound sensible?

--- a/scripts/inncheck.in
+++ b/scripts/inncheck.in
@@ -237,6 +237,12 @@ parse_config
             next;
         }
 
+        # include-file hack: ignore, user needs to check it separately
+        if (defined $options->{'<include>'}->{$word}) {
+            my $includefile = get_config_word();
+            next;
+        }
+
         # $word must be an option key by now
         eprint "$file:$line: option $word must be immediately followed by a colon\n"
             unless $word =~ s/:$//;
@@ -598,6 +604,7 @@ innfeed_conf
             'peer'              => {
                 'ip-name'               => 'hostname',
             },
+            '<include>'         => { '$INCLUDE' => 'ignore' },
         }
     );
     # check some numeric values
@@ -958,7 +965,8 @@ readers_conf
                 'auth'          => 1,
                 'perl_auth'     => 1,
                 'python_auth'   => 1,
-            }
+            },
+            '<include>' => { 'include' => 'ignore' },
         }
     );
     return;


...more to come...
Florian



More information about the inn-workers mailing list