Skip to content

Commit

Permalink
Donn't fallback for Postgres user and password values.
Browse files Browse the repository at this point in the history
We were not comprehensive enough, failing to find data in the service file.
Best just to let libpq do the work, and not try to replicate it. The downside
is that users of the Docker image may be surprised to find that their default
username is `root` (or whatever the docker image username is(), so we'll just
have to document that behavior. Closes #410.
  • Loading branch information
theory committed Oct 30, 2018
1 parent d2d68ef commit 90d3246
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 13 deletions.
5 changes: 5 additions & 0 deletions Changes
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ Revision history for Perl extension App::Sqitch
`connections.dbname` variables in the SnowSQL configuration file
(`~/.snowsql/config`) before falling back on the hard-coded warehouse
name "sqitch" and using the system username as the database name.
- Removed fallback in the PostgreSQL engine on the `$PGUSER` and
`$PGPASSWORD` environnment variables, as well as the system username,
since libpq does all that automatically, and collects data from other
sources that we did not (e.g., the password and connection service
files). Thanks to Tom Bloor for the report (issue #410).

0.9998 2018-10-03T20:53:58Z
- Fixed an issue where Sqitch would sometimes truncate the registry
Expand Down
16 changes: 13 additions & 3 deletions lib/App/Sqitch/Engine/pg.pm
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,22 @@ sub destination {
# Use the URI sans password, and with the database name added.
my $uri = $self->target->uri->clone;
$uri->password(undef) if $uri->password;
$uri->dbname( $ENV{PGDATABASE} || $self->username );
$uri->dbname(
$ENV{PGDATABASE}
|| $self->username
|| $ENV{PGUSER}
|| $self->sqitch->sysuser
);
return $uri->as_string;
}

sub _def_user { $ENV{PGUSER} || shift->sqitch->sysuser }
sub _def_pass { $ENV{PGPASSWORD} }
# DBD::pg and psql use fallbacks consistently, thanks to libpq. These include
# environment variables, system info (username), the password file, and the
# connection service file. Best for us not to second-guess these values,
# though we admittedly try when setting the database name in the destination
# URI for unnamed targets a few lines up from here.
sub _def_user { }
sub _def_pass { }

has _psql => (
is => 'ro',
Expand Down
13 changes: 3 additions & 10 deletions t/pg.t
Original file line number Diff line number Diff line change
Expand Up @@ -64,15 +64,14 @@ my @std_opts = (
'--set' => 'registry=sqitch',
);
my $sysuser = $sqitch->sysuser;
is_deeply [$pg->psql], [$client, '--dbname', "user=$sysuser", @std_opts],
is_deeply [$pg->psql], [$client, @std_opts],
'psql command should be conninfo, and std opts-only';

isa_ok $pg = $CLASS->new(sqitch => $sqitch, target => $target), $CLASS;
ok $pg->set_variables(foo => 'baz', whu => 'hi there', yo => 'stellar'),
'Set some variables';
is_deeply [$pg->psql], [
$client,
'--dbname', "user=$sysuser",
'--set' => 'foo=baz',
'--set' => 'whu=hi there',
'--set' => 'yo=stellar',
Expand All @@ -84,18 +83,12 @@ is_deeply [$pg->psql], [
ENV: {
# Make sure we override system-set vars.
local $ENV{PGDATABASE};
local $ENV{PGUSER};
local $ENV{PGPASSWORD};
for my $env (qw(PGDATABASE PGUSER PGPASSWORD)) {
my $pg = $CLASS->new(sqitch => $sqitch, target => $target);
local $ENV{$env} = "\$ENV=whatever";
is $pg->target->uri, "db:pg:", "Target should not read \$$env";
is $pg->registry_destination, $pg->destination,
'Registry target should be the same as destination';
is $pg->username, $ENV{PGUSER} || $sysuser,
"Should have username when $env set";
is $pg->password, $ENV{PGPASSWORD},
"Should have password when $env set";
}

my $mocker = Test::MockModule->new('App::Sqitch');
Expand Down Expand Up @@ -132,7 +125,7 @@ is $pg->registry, 'meta', 'registry should be as configured';
is_deeply [$pg->psql], [
'/path/to/psql',
'--dbname',
"user=$sysuser dbname=try host=localhost connect_timeout=5 sslmode=disable",
"dbname=try host=localhost connect_timeout=5 sslmode=disable",
@std_opts], 'psql command should be configured from URI config';

##############################################################################
Expand All @@ -155,7 +148,7 @@ is $pg->registry, 'meta', 'registry should still be as configured';
is_deeply [$pg->psql], [
'/some/other/psql',
'--dbname',
"user=$sysuser dbname=try host=localhost connect_timeout=5 sslmode=disable",
"dbname=try host=localhost connect_timeout=5 sslmode=disable",
@std_opts], 'psql command should be as optioned';

##############################################################################
Expand Down

0 comments on commit 90d3246

Please sign in to comment.