Improve the auto-creation of the "bucardo" role.
authorDavid E. Wheeler <david@justatheory.com>
Tue, 15 Oct 2013 23:35:57 +0000 (16:35 -0700)
committerDavid E. Wheeler <david@justatheory.com>
Tue, 15 Oct 2013 23:35:57 +0000 (16:35 -0700)
When adding a database, that is. The old code led to confusing errors, where
the user would be told that Bucardo failed to connect as user "postgres" when
you had not, in fact, asked it to. So log more informat as it goes along,
noting failure to connect as "bucardo", trying to connect as "postgres", and
showing success or failure at doing so. This makes things much clearer to the
user what's going on -- and is less fussy, to boot.

Resolves #63.

bucardo

diff --git a/bucardo b/bucardo
index 5cba8232e29449ea5750f7fa7024ac2cbc2de170..5ef59628b458110a763adaf45eca8ff34e1a0e07 100755 (executable)
--- a/bucardo
+++ b/bucardo
@@ -1469,12 +1469,9 @@ sub add_database {
         ## We put it here as we may move around with the Postgres bucardo user trick
         my ($type,$dsn,$user,$pass) = split /\n/ => $dbconn;
 
-        ## Sometimes we need to adjust the username on the fly and retry
-        my $realuser = $user;
 
         ## Handle all of the ones that do not use standard DBI first
 
-        my @users;
         if ('mongo' eq $dbtype) {
 
             my $dsn = {};
@@ -1517,64 +1514,59 @@ sub add_database {
 
         ## Anything else must be something with a standard DBI driver
         else {
-          DBICONNTEST: {
-              push @users => $user;
-                eval {
-                    $testdbh = DBI->connect($dsn, $user, $pass, {AutoCommit=>0,RaiseError=>1,PrintError=>0});
-                    $evalok = 1;
-                };
-            }
+            eval {
+                $testdbh = DBI->connect($dsn, $user, $pass, {AutoCommit=>0,RaiseError=>1,PrintError=>0});
+                $evalok = 1;
+            };
         }
 
         ## At this point, we have eval'd a connection
-
-        if (! $evalok) {
+        if ($evalok) {
+            ## Disconnect from DBI.
+            $testdbh->disconnect if $module =~ /DBD/;
+        } else {
+            my $err = $DBI::errstr || $@;
+            my $msg;
 
             ## For Postgres, we get a little fancy and try to account for instances
             ## where the bucardo user may not exist yet, by reconnecting and
             ## creating said user if needed.
-            if ('postgres' eq $dbtype and $user eq 'bucardo' and $@ =~ /FATAL.*"bucardo"/) {
-                $user = 'postgres';
-                goto DBICONNTEST;
-            }
+            if ($DBI::errstr && 'postgres' eq $dbtype && $user eq 'bucardo' && eval { require Digest::MD5; 1 }) {
 
-            my $as = join ' or ', @users;
-            if ($bcargs->{force}) {
-                print "Connection to $fullname as $as failed, but will add anyway.\nError was: $@\n\n";
-            }
-            else {
-                die "Connection to $fullname as $as failed. You may force add it with the --force argument. Error was: $@\n\n";
-            }
-        }
-        else {
-
-            ## If we connected twice to Postgres using a different user,
-            ## create the bucardo user now
-            if ('postgres' eq $dbtype and $realuser ne $user) {
-                if (eval { require Digest::MD5; 1 }) {
-                    ## We need a password. If one was not supplied, create one
-                    $evalok = 0;
+                # Try connecting as postgres instead.
+                print "Connection to $fullname as bucardo failed.\nError was: $DBI::errstr\n\n";
+                print qq{Will try to connect as postgres and create superuser $user...\n\n};
+                my $dbh = eval {
+                    DBI->connect($dsn, 'postgres', $pass, {AutoCommit=>1,RaiseError=>1,PrintError=>0});
+                };
+                if ($dbh) {
+                    ## Create the bucardo user now. We'll need a password;
+                    ## create one if we don't have one.
+                    my $connok = 0;
                     eval {
-                        my $newpass = $pass;
-                        if (! length $pass) {
-                            $newpass = generate_password();
-                        }
+                        my $newpass = $pass || generate_password();
                         my $encpass = Digest::MD5::md5_hex($newpass);
-                        $testdbh->do(qq{CREATE USER $realuser SUPERUSER ENCRYPTED PASSWORD '$encpass'});
-                        $testdbh->commit();
-                        my $extrauser = length $pass ? '' : " with password $newpass";
-                        warn "Created superuser '$realuser'$extrauser\n";
-                        $evalok = 1;
+                        $dbh->do(qq{CREATE USER $user SUPERUSER ENCRYPTED PASSWORD '$encpass'});
+                        $dbh->disconnect;
+                        my $extrauser = $pass ? '' : qq{ with password "$newpass"};
+                        warn "Created superuser '$user'$extrauser\n\n";
+                        $pass = $newpass;
+                        $connok = 1;
                     };
-                    if (! $evalok) {
-                        warn "Unable to create superuser $realuser: $@\n";
-                        $bcargs->{force} or exit 1;
-                    }
+                    goto TESTCONN if $connok;
+                    $err = $DBI::errstr || $@;
+                    $msg = "Unable to create superuser $user";
+                } else {
+                    $err = $DBI::errstr || $@;
+                    $msg = 'Connection as postgres failed, too';
                 }
+            } else {
+                $msg = "Connection to $fullname as $user failed";
             }
 
-            ## Disconnect
-            $testdbh->disconnect() if $module =~ /DBD/;
+            die "$msg. You may force add it with the --force argument.\nError was: $err\n\n"
+                unless $bcargs->{force};
+            warn "$msg, but will add anyway.\nError was: $err\n";
         }
 
     } ## end of TESTCONN
@@ -9402,6 +9394,13 @@ The service name to use for a Postgres database. Optional.
 
 =back
 
+B<Note:> As a convenience, if the C<dbuser> value is its default value,
+"bucardo", in the event that Bucardo cannot connect to the database, it will
+try connecting as "postgres" and create a superuser named "bucardo". This is
+to make things easier for folks getting started with Bucardo, but will not
+work if it cannot connect as "postgres", or if it the connection failed due to
+an authentication failure.
+
 =head3 add dbgroup
 
   bucardo add dbgroup name db1:source db2:source db3:target ...