Uploaded image for project: 'ZABBIX BUGS AND ISSUES'
  1. ZABBIX BUGS AND ISSUES
  2. ZBX-19148

Agent2 Postgresql plugin-connector "database" name problems

XMLWordPrintable

    • Icon: Incident report Incident report
    • Resolution: Unresolved
    • Icon: Trivial Trivial
    • None
    • 5.0.8, 5.4.0alpha2
    • Agent2 plugin (N)
    • None
    • Unix, Postgresql

      This is a complaint about an external bug, a minor bug, a major bug, a design flaw, and a flaw in the zabbix-agent2 architecture. Let us start small.

      The external bug is that if the "database" field is set, it is passed onto the third-party library as a parameter, instead of handling it and updating the connection string with `dbname=` (see t line 211 of the plugin code). The DBNAME field is usually only set when using autodiscovery on per-database instance metrics. However, if set as a parameter in the ZA2 conf file such as with a session, it could cause a problem. I say 'could' because (1) this is actually a bug that underlies the Postgresql plugin, ie, a 3rd party PG library. In that code, if the "database" config parameter is set, it appends "database=<value>" to the connection string, and (2) Postgresql (12) server allows it anyway, despite this behavior being disallowed by `libpq` agents and by `pgsql` (see below) and as stated in the documentation to[ the contrary.|https://www.postgresql.org/docs/12/libpq-connect.html]  It is also very unclear what should be the behavior if the session is configured with a database name in the URI but the database name conflicts with that.  I propose that as a workaround, ZA2 ignores with a warning the use of a parameter such as 
      ```
      Plugins.Postgres.Sessions.main.Database=...
      ```
      unless the URI is a unix socket.  This gets to the (IMO) a relatively minor bug.

      The reason the above became an issue is because I wanted to (1) use UNIX sockets to connect to the database and (2) select a database to connect to. Now, it turns out, that #2 was the wrong thing to want in my case. But If I did want that, it wouldn't work because of the above bug AND the following:  The ZA2 does not properly parse `unix:` URIs. The code referenced above attempts to treat the UNIX-type URI as a URI, but then complains when there is a query portion. The query portion is supposed to be parsed ala Go's net/url `QueryParse` routine. Instead, we see: 
      ```
      failed to execute direct exporter task for key 'pgsql.archive["unix:/var/run/postgresql/.s.PGSQL.5432?user=zabbix&dbname=zabbix","",""]' error: 'Invalid first parameter "URI": socket file must satisfy the format: "/path/.s.PGSQL.nnnn" where nnnn is the server's port number.'
      ```

      The code in `conn.c` treats the string as a URI, so this should work, but the code in metrics.go#129 ensures such a query is invalid.  As such, for the unix:// socket, the only way to specify a database name is using the database parameter, which as stated above, has this underlying flaw which exploits an undocumented feature of the postgresql server.

      So, fix the regexp in metrics.go, add a warning for use of `database`.


      Now we get to the major bug. 

      It turns out, if we use the "Session" configuration to specify the database path, username, password, the ZA2 will refuse to use auto-discovered rules that are specific to database-instances on the Postgresql cluster.  Here is the error:
      ```
      failed to execute direct exporter task for key 'pgsql.db.bloating_tables["main","","","zabbix"]' error: 'Invalid parameters: fourth parameter "Database" cannot be passed along with session.'
      ````

      The configuration  in ZA2 is:
      ```
      Plugins.Postgres.Sessions.main.Uri=unix:/var/run/postgresql/.s.PGSQL.5432
      ```
      and all template macros for are left empty. The other keys work correctly, so long as they do not try to connect to a specific database (such as `zabbix` or `postgres`). 
      This pretty makes the session configuration useless (at least for autodiscovery) and makes the system more vulnerable (if the password is non-null).  
      Note: I did not track down the source of this particular bug. Sorry getting tired.


      This brings us to the specific design flaw in the Postgresql Plugin. One questions the wisdom of using '`tcp` and `unix` URI specifications in the first place, when for database servers, there exists already a very standard RFC for database connections. Why aren't you using the standard thing instead of reinventing your own?

      There is also the question of why rely on non-standard third-party libraries, especially which are (1) not audited, (2) not under  your control, (3) not from a major 3rd party vendor (such as MySQL/Oracle/Postgresql/Google/IBM).  It would be far saner to use a C-lib-to-Go wrapper utility for libraries such as libpq.  Again, why aren't you using the standard thing instead of relying on someone else's reinvention?

      But please, don't let my curmudgeonry get in the way of a solid bug report.

            asebiskveradze Aleksandre Sebiskveradze
            otheus Otheus
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

              Created:
              Updated: