[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Request for review: nginx Debconf template and control files



On 05/08/2012 09:13 PM, Justin B Rye wrote:
> Kartik Mistry wrote:
>> We recently introduced Debconf template into package and we're
>> requesting to review it by list. Our beloved Christian Perrier also
>> suggested to get debian/control reviewed, so I'm attaching both files
>> for review.
> Okay... unsurprisingly I've spent a lot more time looking at the
> control file than the debconf template!
Hello Justin.

>From my point of view, I'm not surprised because I spent more time
working on the strictly technical part than working on this part.
>> Template: nginx-naxsi-ui/dbhost
>> Type: string
>> Default: localhost
>> _Description: Host where the database is located:
>>  If you leave this field empty, the default host ('localhost') will be used.
> DevRef 6.4.2.4 recommends not explicitly mentioning string defaults,
> because it's redundant and can be overruled by pre-seeding.
> Instead, give the reader some clue what the question means - maybe: 
>
>   _Description: Database host for naxsi:
>    Please specify the hostname of the server where the naxsi Web Application
>    Firewall's database should be located.
Ok, it's way more clear than before and I understand why we don't have
to mention string defaults.

Thanks.
> Onward to the control file...
>
>> Source: nginx
> [...]
>> Package: nginx
> [...]
>> Description: small, but very powerful and efficient web server and mail proxy
> That's rather an awkwardly long synopsis, considering you're going to
> want to use it as the basis of longer blurbs.  Looking at server
> comparison pages I'd suggest:
>
>   Description: small, powerful, scalable web/proxy server
>
>>  Nginx (engine x) is a web server created by Igor Sysoev and kindly provided to
>>  the open-source community. This server can be used as standalone HTTP server
>>  and as a reverse proxy server before some Apache or another big server to
>>  reduce load to backend servers by many concurrent HTTP-sessions.
> The grammar's a bit wonky here - I don't know what the "by many
> concurrent HTTP-sessions" part means exactly.  And *everything* in 
> Debian main has been kindly provided to the open-source community, so
> that's not a distinctive feature of nginx (the upstream website treats
> the name as intrinsically-lowercase, but wiki.nginx.org allows "Nginx"
> at the start of a sentence).  I'd suggest the following boilerplate:
>
>    Nginx ("engine X") is a high-performance web and reverse proxy server
>    created by Igor Sysoev. It can be used both as a standalone web server
>    and as a proxy to reduce the load on back-end HTTP or mail servers.
>
>> .
>> This is a dummy package that selects nginx-full by default, but also can be
>> installed with nginx-light for upgrading to nginx-light directly.
> "Upgrading" to nginx-light from what?  Surely not nginx-full?
> Dropping the part I don't understand, I'm left with:
I think it's when we switched to the multiple flavours, then upgrading
the "nginx" package would install nginx-full unless nginx-light is
specified.

But I think we can drop this part, it's not relevant anymore.
>   This is a dependency package to install either nginx-full (by default) or
>   nginx-light.
>
>> Package: nginx-doc
> [...]
>> Description: small, but very powerful and efficient web server (documentation)
> My shorter version can absorb the extra word without being trimmed
> first:
>
>   Description: small, powerful, scalable web/proxy server - documentation
>
> [...boilerplate...]
>> This package provides extra documentation to help unleash the power of Nginx.
> Fair enough.
>
>> Package: nginx-common
> [...]
>> Description: small, but very powerful and efficient web server (common files)
>   Description: small, powerful, scalable web/proxy server - common files
>
> [...boilerplate...]
>> This package contains base configuration files used across multiple packaged
>> versions of Nginx (nginx-full, nginx-light, nginx-extras, nginx-naxsi).
> "Across"?  "Packaged"?  Why bother listing them anyway?
>
>   This package contains base configuration files used by all versions of
>   nginx.
Hmm, good idea. It also lighten the work when adding a new flavour.
>> Package: nginx-full
> [...]
>> Description: nginx web server with full set of core modules
> It would be nice if we could give these nginx-$FLAVOR packages short
> descriptions more similar to the rest:
>
>   Description: small, powerful, scalable web/proxy server (standard)
>
> But no, that just causes more problems later.  Try:
>
>   Description: nginx web/proxy server (standard version)
>
> (Stressing the normalness, not the fullness.)
>
> [...boilerplate...]
>>  It can also act as a POP3/IMAP mail proxy with SSL and TLS SNI support. This
>>  package has the standard set of modules enabled.
> The first sentence of that should really either be included in the
> boilerplate or left out; and since it explicitly mentions POP3/IMAP
> and not SMTP, I was assuming until I read the details that it couldn't
> function as an SMTP proxy.
>
> Then in the second sentence, "enabled" implies they're "compiled in",
> but that can't be right - at least for the ones listed as "Optional"!
>
> Wait a minute, though... do I correctly understand that nginx-extra
> contains everything that's in -full plus a bit more besides?  That
> really ought to be made clearer here.
You understood well.

For the 3 main flavours, it's something like that :
full = light + some modules
extras = full + other modules

The only flavour which is not concerned by that is the naxsi flavour
which is specially designed to fit the need of the naxsi project.
> How about:
>
>    This package provides a version of nginx with the complete set of
>    standard modules included (but omitting some of those included in
>    nginx-extra).
>
>>  .
>>  MODULES INCLUDED:
> That's redundant (or contradictory) with the previous sentence, and
> if you leave it out it becomes possible to capitalise the subheaders
> and economise on newlines.
>
>>  Standard HTTP Modules:
>>    Core, Access, Auth Basic, Auto Index, Browser, Charset, Empty GIF, FastCGI,
>>    Geo, Gzip, Headers, Index, Limit Requests, Limit Zone, Log, Map, Memcached,
>>    Proxy, Referer, Rewrite, SCGI, Split Clients, SSI, Upstream, User ID, UWSGI
> (There's no need for these to be indented more than one space.  And do
> they really need to be verbatim-formatted at all?)
>
>>  Optional HTTP Modules:
>>    Addition, Debug, GeoIP, Gzip Precompression, HTTP Sub, Image Filter, IPv6,
>>    RealIP, Stub Status, WebDAV, XSLT
>>  Mail Modules:
>>    Mail Core, IMAP, POP3, SMTP, SSL
>>  Third Party Modules:
>>    Echo, Upstream Fair Queue, DAV Ext
> Would it be okay to reformat it like this?
>
>    .
>    STANDARD HTTP MODULES: Core, Access, Auth Basic, Auto Index, Browser,
>    Charset, Empty GIF, FastCGI, Geo, Gzip, Headers, Index, Limit
>    Requests, Limit Zone, Log, Map, Memcached, Proxy, Referer, Rewrite,
>    SCGI, Split Clients, SSI, Upstream, User ID, UWSGI.
>    .
>    OPTIONAL HTTP MODULES: Addition, Debug, GeoIP, Gzip Precompression,
>    HTTP Sub, Image Filter, IPv6, RealIP, Stub Status, WebDAV, XSLT.
>    .
>    MAIL MODULES: Mail Core, IMAP, POP3, SMTP, SSL.
>    .
>    THIRD PARTY MODULES: Echo, Upstream Fair Queue, DAV Ext.
>  
Pretty good actually, and the caps used for every category has more
effect than indentation.
>> Package: nginx-full-dbg
> [...]
>> Description: Debugging symbols for nginx (full)
>   Description: nginx web/proxy server (standard version) - debugging symbols
>
> [...boilerplate...]
>>  This package provides debugging symbols for nginx-full, to assist in debugging
>>  issues that you may find. It should not be required for normal operation.
> That's good enough.
>  
>> Package: nginx-light
> [...]
>> Description: nginx web server with minimal set of core modules
>   Description: nginx web/proxy server (basic version)
>
> [...boilerplate...]
>>  This package provides a very light version of Nginx which lacks many of the
>>  features and modules of it's full counterpart.
> Surplus apostrophe, and inconsistent capitalisation policy.  To keep
> it parallel with nginx-full I'd suggest:
>
>    This package provides a very light version of nginx with only the
>    minimal set of features and modules.
>
>>  .
>>  MODULES INCLUDED:
>>  Standard HTTP Modules:
>>    Core, Access, Auth Basic, Auto Index, Charset, Empty GIF, FastCGI, Gzip,
>>    Headers, Index, Log, Map, Proxy, Rewrite, Split Clients, Upstream, User ID
>>  Optional HTTP Modules:
>>    Gzip Precompression, IPv6, Stub Status, SSL
>>  Mail Modules:
>>    None
>>  Third Party Modules:
>>    Echo
>    STANDARD HTTP MODULES: Core, Access, Auth Basic, Auto Index, Charset,
>    Empty GIF, FastCGI, Gzip, Headers, Index, Log, Map, Proxy, Rewrite,
>    Split Clients, Upstream, User ID.
>    .
>    OPTIONAL HTTP MODULES: Gzip Precompression, IPv6, Stub Status, SSL.
>    .
>    THIRD PARTY MODULES: Echo.
>  
>> Package: nginx-light-dbg
> [...]
>> Description: Debugging symbols for nginx (light)
>   Description: nginx web/proxy server (basic version) - debugging symbols
>
> [...as above...]
>  
>> Package: nginx-extras
> [...]
>> Description: nginx web server with full set of core modules and extras
> Including the word "full" here just makes the distinction between 
> -full and -extras more confusing.  (I would have guessed that
> "nginx-extras" was a collection of optional modules you could
> install "on top of" an nginx server package.)
>
>   Description: nginx web/proxy server (extended version)
Good idea the "extended version", I never came up with a good
description for this package.
> [...boilerplate...]
>>  This package provides the standard version of Nginx. It also includes extra
>>  features and modules such as the perl module which allows the addition of
>>  perl in configuration files.
> The first sentence implies that this version of nginx is the
> default flavour.  I would suggest:
>
>    This package provides a version of nginx with the standard modules, plus
>    extra features and modules such as the Perl module, which allows the
>    addition of Perl in configuration files.
>
> (Perl the language, not perl the executable or package.)
Yes.
>>  .
>>  MODULES INCLUDED:
>>  Standard HTTP Modules:
>>    Core, Access, Auth Basic, Auto Index, Browser, Charset, Empty GIF, FastCGI,
>>    Geo, Gzip, Headers, Index, Limit Requests, Limit Zone, Log, Map, Memcached,
>>    Proxy, Referer, Rewrite, SCGI, Split Clients, SSI, Upstream, User ID, UWSGI
>>  Optional HTTP Modules:
>>    Addition, Debug, WebDAV, Flash Streaming Video, GeoIP, Gzip Precompression,
>>    HTTP Sub, Image Filter, MP4, RealIP, Stub Status, SSL, XSLT, IPv6, Embedded
>>    Perl, Secure Link, Random Index
>>  Mail Modules:
>>    Mail Core, POP3, IMAP, SMTP, SSL
>>  Third Party Modules:
>>    Echo, Embedded Lua, http push, Nginx Development Kit, Upload module,
>>    Upload Progress, HttpHeadersMore, Upstream Fair Queue, Chunkin, Auth PAM,
>>    DAV Ext
> Again:
>
>    STANDARD HTTP MODULES: Core, Access, Auth Basic, Auto Index, Browser,
>    Charset, Empty GIF, FastCGI, Geo, Gzip, Headers, Index, Limit Requests,
>    Limit Zone, Log, Map, Memcached, Proxy, Referer, Rewrite, SCGI, Split
>    Clients, SSI, Upstream, User ID, UWSGI.
>    .
>    OPTIONAL HTTP MODULES: Addition, Debug, WebDAV, Flash Streaming Video,
>    GeoIP, Gzip Precompression, HTTP Sub, Image Filter, MP4, RealIP, Stub
>    Status, SSL, XSLT, IPv6, Embedded Perl, Secure Link, Random Index.
>    .
>    MAIL MODULES: Mail Core, POP3, IMAP, SMTP, SSL.
>    .
>    THIRD PARTY MODULES: Echo, Embedded Lua, http push, Nginx Development
>    Kit, Upload module, Upload Progress, HttpHeadersMore, Upstream Fair
>    Queue, Chunkin, Auth PAM, DAV Ext.
>
> Should "http push" perhaps be "HTTP Push"?  Should "Upload module"
> be plain "Upload"? 
>
> (...It also makes we wonder whether it would make more sense to say
> "everything in nginx-full, q.v., plus...")
I think for a first re-factoring, we could keep the full list, then
change the module names like you suggested. To finish, we could replace
it by

  Everything in nginx-full plus these following modules:
  [ The list goes here ]
>> Package: nginx-extras-dbg
>> Description: Debugging symbols for nginx (extras)
>   Description: nginx web/proxy server (extended version) - debugging symbols
>
> [...and so on...]
>
>> Package: nginx-naxsi
> [...]
>> Description: nginx web server with naxsi 0.44 included
> Oh hurrah, asdfghjkl now comes with added zxcvbnm!  No, package
> descriptions are supposed to tell me what it is and why I might want
> it if I *don't* already know it by name.
>
> Google tells me it's "Nginx Anti Xss Sql Injection", which at least
> has intelligible elements, even if in a seemingly random order.
>
>>  Nginx (engine x) is a web server created by Igor Sysoev and kindly provided to
>>  the open-source community. This server can be used as standalone HTTP server
>>  and as a reverse proxy server before some Apache or another big server to
>>  reduce load to backend servers by many concurrent HTTP-sessions.
>>  .
>>  This package provides the standard version of Nginx, including the naxsi Web
>>  Application Firewall.
> Another one that claims to provide the standard version; this time
> it's claiming that the standard version of Nginx is one that
> includes naxsi.
Actually, I didn't perform a big amount of work on the nginx-naxsi
description, only copy/paste from the nginx-full and adding/removing few
words. I scheduled to rewrite this part, and I procrastinated :(.
> I've never liked the expression "Web Application Firewall" (it
> sounds as if it means a PHP UI for iptables, rather than an
> application-layer firewall for HTTP servers), but I suppose that
> argument was lost ten years ago.
>
>>  .
>>  MODULES INCLUDED:
>>  Standard HTTP Modules:
>>    Core, Access, Auth Basic, Auto Index, Browser, Charset, Empty GIF, FastCGI,
>>    Geo, Gzip, Headers, Index, Limit Requests, Limit Zone, Log, Map, Memcached,
>>    Proxy, Referer, Rewrite, SCGI, Split Clients, SSI, Upstream, User ID, UWSGI
>>  Optional HTTP Modules:
>>    Gzip Precompression, IPv6, Stub Status, SSL
>>  Mail Modules:
>>    None
>>  Third Party Modules:
>>    Naxsi, Cache Purge, Upstream Fair
>   Description: nginx web/proxy server (version with naxsi)
>   [...]
>    This package provides a version of nginx with the basic modules, plus
>    the naxsi Web Application Firewall module.
>    .
>    STANDARD HTTP MODULES: Core, Access, Auth Basic, Auto Index, Browser,
>    Charset, Empty GIF, FastCGI, Geo, Gzip, Headers, Index, Limit Requests,
>    Limit Zone, Log, Map, Memcached, Proxy, Referer, Rewrite, SCGI, Split
>    Clients, SSI, Upstream, User ID, UWSGI.
>    .
>    OPTIONAL HTTP MODULES: Gzip Precompression, IPv6, Stub Status, SSL.
>    .
>    THIRD PARTY MODULES: Naxsi, Cache Purge, Upstream Fair.
>
> Wait, isn't that "Upstream Fair Queue"?
Yes, by the way, I just double checked, and the module is called
"Upstream Fair". I don't know why we used "Upstream Fair Queue" on other
flavours.

I think we could uniform this appellation using "Upstream Fair".
>  
>> Package: nginx-naxsi-dbg
> [...]
>> Description: Debugging symbols for nginx (naxsi)
>   Description: nginx web/proxy server (version with naxsi) - debugging symbols
>   [...]
>
>> Package: nginx-naxsi-ui
> You're quite sure these packagenames aren't insulting Apache in Navajo or
> something?
I don't know, I choosed "nginx-naxsi-ui" because it's the UI for the
nginx-naxsi package.
> [...]
>> Provides: httpd, naxsi, nginx
> (The UI provides all three, and -naxsi doesn't provide naxsi?  Oh,
> because despite the name -naxsi-ui contains the main daemon too?)
I forgot replacing "naxsi" with "naxsi-ui". The UI is optional and
provides tools for reporting and auto-learn.
>
> [...]
>> Description: naxsi UI for nginx-naxsi
>>  naxsi-ui is the autolearn daemon and the webUI for the package nginx-naxsi.
>>  .
>>  The intercepter receives requests from naxsi, it listens on the tcp port 8080
>>  .
>>  The extractor reads the database and prints reports about blocked requests.
>>  It runs on the tcp port 8081.
> This is a terrible description, offering more or less no real clue
> about what the package does or why I might want to install it.  What
> sort of thing are the "interceptor" and "extractor"?  Apparently
> daemons written in Python, but do they count as components of the
> "autolearn daemon", or what?
Ok. I have to seriously learn about writing good descriptions in the
near future.
> Guessing wildly:
>
>   Description: nginx web/proxy server - naxsi configuration front-end
>    Nginx ("engine X") is a high-performance web and reverse proxy server
>    created by Igor Sysoev. It can be used both as a standalone web server
>    and as a proxy to reduce the load on back-end HTTP or mail servers.
>    .
>    This package provides the autolearning daemon and web user interface for
>    nginx's naxsi module.
>    .
>    It includes an interceptor (listening on TCP port 8080), which monitors
>    HTTP requests from naxsi, and an extractor (running on TCP port 8081),
>    which reads the database and prints reports about blocked requests.
>
>
You guessed well.

Thank you for this enormous work.

We will see on including your work on the GIT repo asap.

Kartik, Michael, what are your thoughts on this review ?

Thanks.

-- 
Cyril "Davromaniak" Lavier
KeyID 59E9A881
http://www.davromaniak.eu


Reply to: