BIND 10 #3080: New DB backend: Postgres (patch)

[BIND 10 Development]

#3080: New DB backend: Postgres (patch)
-------------------------------------+-------------------------------------
            Reporter:  dclink        |                        Owner:  tmark
                Type:  enhancement   |                       Status:
            Priority:  medium        |  reviewing
           Component:  dhcpdb        |                    Milestone:  DHCP-
            Keywords:                |  Kea0.9-alpha
           Sensitive:  0             |                   Resolution:
         Sub-Project:  DHCP          |                 CVSS Scoring:
Estimated Difficulty:  3             |              Defect Severity:  N/A
         Total Hours:  50            |  Feature Depending on Ticket:  2592
                                     |          Add Hours to Ticket:  8
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------

Comment (by marcin):

 I ran some manual tests to make ure the documentation is correct and that
 the setup works.

 A couple of observations:

 ----
 When compiling with the configure option --with-dhcp-mysql the compilation
 failed because mysql_lease_mgr.cc was not modified after splitting it into
 schema_mysql_copy.h and schema_pgsql_copy.h. I fixed it on the branch.

 ----
 It is not explained anywhere in the bind10-guide that kea requires special
 setup for unit tests when using Postgres (compiling with --with-dhcp-
 pgsql). Actually the same is the case for MySQL, I think. If this is
 explained in the developer's guide there could be a reference to the
 developer's guide or at least it should be mentioned that this is in
 developer's guide (if not a direct reference).

 ----

 bind10-guide There should be no apostrophe here "postgres=#> GRANT ALL
 PRIVILEGES ON DATABASE database-name TO 'user-name;" - before username

 ----

 The following text in bind10-guide is a little unclear:

 {{{
 If instead of seeing keatest=> prompt, your login will be refused with
 error code about failed peer or indent authentication, it means that
 PostgreSQL is configured to check unix username and reject login attepts
 if PostgreSQL names are different. To alter that, PostgreSQL configuration
 must be changed. That file is located at
 /etc/postgresql/9.1/main/pg_hba.conf on Ubuntu 13.10. Its location may be
 different on your system. Please consult your PostgreSQL user manual
 before applying those changes as those changes may expose your other
 databases that you run on the same system.
 }}}

 It would be better to copy-paste exact error messages.

 Also, it would be good to explain what exactly has to be changed in the
 pg_hba.conf file, e.g. replace !''peer!'' with !''trust!''.

 BTW, these are the steps that I used to setup db in postgres:

 {{{
 CREATE DATABASE keatest;
 \i /opt/bind10/share/bind10/dhcpdb_create.pgsql
 CREATE USER keatest WITH PASSWORD 'keatest';
 GRANT ALL PRIVILEGES ON ALL TABLES IN SCHEMA public
 \c keatest
 SELECT 'ALTER TABLE '|| tablename ||' OWNER TO keatest;' FROM pg_tables
 WHERE SCHEMANAME = 'public'
 edit /etc/postgresql/9.1/main/pg_hba.conf and replace peer with trust
 service postgresql restart

 \c keatest
 ALTER TABLE lease6 OWNER TO keatest;
 postgres=# \c keatest
 You are now connected to database "keatest" as user "postgres".
 keatest=# ALTER TABLE lease6 OWNER TO keatest;
 ALTER TABLE
 keatest=# ALTER TABLE lease6_types OWNER TO keatest;
 ALTER TABLE
 keatest=# ALTER TABLE schema_version OWNER TO keatest

 }}}

 According to our jabber discussion, some of them are redundant, but I
 paste it here just for clarity what I did, versus what is in the doc.

 Maybe it should be put as a note or warning in the bind10-guide that the
 keatest or kea or any other user MUST be an owner of tables!

 ----

 Postgres backend has invalid logging statement in addLeaseCommon:
 {{{
     LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
 DHCPSRV_PGSQL_ADD_ADDR4).arg(statements_[stindex].stmt_name);

 }}}

 It should be logged in the caller function, not here. Also stmt_name is
 not the address :)

 ----

 Postgres backend is using MySQL logging messages in several places.

 ----

 Not strictly related to this work but... DHCPSRV_ADDRESS6_ALLOC_ERROR  -
 this error message in alloc_engine is wrong because it talks about IPv6
 addresses only. If you fail to allocate the prefix (not a 128 address) it
 is quite misleading. And it was misleading when I spotted it for the first
 time in the kea log file, when running system tests.

 ----

 I had valgrind failing on Ubuntu 13.10 (gcc 4.7 and 4.8) but I was never
 able to reproduce it elsewhere. It worked fine on Debian7. So, maybe my
 Ubuntu setup is foo-bared. I attached the log from the valgrind run.

 ----

 There is one thing that I saw initially under valgrind, but I couldn't
 reproduce later:

 convertToQuery function initializes values three vectors passed by
 reference. The ultimate sizes of vectors are equal to size of !''params!''
 vector, so given that we never pass empty params vector it should be fine
 to do this:

 {{{
     convertToQuery(params, out_values, out_lengths, out_formats);

     PGresult * r = PQexecPrepared(conn_, statements_[stindex].stmt_name,
                                   statements_[stindex].stmt_nbparams,
                                   &out_values[0], &out_lengths[0],
                                   &out_formats[0], 0);

 }}}

 Trying to access element with index 0 of out_values and out_formats may
 result in segfault if params size is 0. So, in other words, the code could
 check for empty vectors returned. But, given that we will remove these
 bits of code soon, it is not a MUST.

 ----

 I found some local variables terminated with underscore (see
 addLeaseCommon):

 {{{
     vector<const char *> params_;
     vector<int> lengths_;
     vector<int> formats_;
     convertToQuery(params, params_, lengths_, formats_);
 }}}

-- 
Ticket URL: <http://bind10.isc.org/ticket/3080#comment:15>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development