Обсуждение: Where is SSPI auth username determined for TAP tests?
bowerbird is failing the pg_dump regression tests with a lot of FATAL: SSPI authentication failed for user "regress_postgres" I think this is likely a consequence of ca129e58c0 having modified 010_dump_connstr.pl to use "regress_postgres" not "postgres" as the bootstrap superuser name in the source cluster. I suppose I overlooked some dependency on the user name that only affects SSPI ... but what? I don't see anything about the destination cluster configuration (which already used a nondefault superuser name) that I didn't replicate in the source cluster configuration. regards, tom lane
On Sat, Jun 29, 2019 at 04:36:51PM -0400, Tom Lane wrote: > I think this is likely a consequence of ca129e58c0 having modified > 010_dump_connstr.pl to use "regress_postgres" not "postgres" as the > bootstrap superuser name in the source cluster. I suppose I overlooked > some dependency on the user name that only affects SSPI ... but what? > I don't see anything about the destination cluster configuration (which > already used a nondefault superuser name) that I didn't replicate > in the source cluster configuration. Didn't you get trapped with something similar to what has been fixed in d9f543e? If you want pg_hba.conf to be correctly set up for SSPI on Windows, you should pass "auth_extra => ['--create-role', 'regress_postgres']" to the init() method initializing the node. Looking at the commit... my $node = get_new_node('main'); -$node->init(extra => [ '--locale=C', '--encoding=LATIN1' ]); +$node->init(extra => + [ '-U', $src_bootstrap_super, '--locale=C', '--encoding=LATIN1' ]); [...] $node->run_log( [ $ENV{PG_REGRESS}, '--config-auth', $node->data_dir, '--create-role', - "$dbname1,$dbname2,$dbname3,$dbname4" + "$username1,$username2,$username3,$username4" ]); This part is wrong and just needs to be updated to as $src_bootstrap_super also gets its role added in --create-role, which would set up pg_hba.conf as you would like. -- Michael
Вложения
Michael Paquier <michael@paquier.xyz> writes: > On Sat, Jun 29, 2019 at 04:36:51PM -0400, Tom Lane wrote: >> I think this is likely a consequence of ca129e58c0 having modified >> 010_dump_connstr.pl to use "regress_postgres" not "postgres" as the >> bootstrap superuser name in the source cluster. I suppose I overlooked >> some dependency on the user name that only affects SSPI ... but what? > Didn't you get trapped with something similar to what has been fixed > in d9f543e? If you want pg_hba.conf to be correctly set up for SSPI > on Windows, you should pass "auth_extra => ['--create-role', > 'regress_postgres']" to the init() method initializing the node. After further study, I think the root issue here is that pg_regress.c's config_sspi_auth() has no provision for non-default bootstrap superuser names --- it makes a mapping entry for (what should be) the default superuser name whether the cluster is using that or not. I now see that 010_dump_connstr.pl is hacking around that by doing my $envar_node = get_new_node('destination_envar'); $envar_node->init(extra => [ '-U', $dst_bootstrap_super, '--locale=C', '--encoding=LATIN1' ]); $envar_node->run_log( [ $ENV{PG_REGRESS}, '--config-auth', $envar_node->data_dir, '--create-role', "$dst_bootstrap_super,$restore_super" ]); that is, it's explicitly listing the non-default bootstrap superuser among the roles to be "created". This is all pretty weird and undocumented ... We could apply the same hack on the source node, but I wonder if it wouldn't be better to make this less of a kluge. I'm tempted to propose that "pg_regress --config-auth --user XXX" should understand XXX as the bootstrap superuser name, and then we could clean up 010_dump_connstr.pl by using that. regards, tom lane
On Sun, Jun 30, 2019 at 12:09:18PM -0400, Tom Lane wrote: > We could apply the same hack on the source node, but I wonder if it > wouldn't be better to make this less of a kluge. I'm tempted to > propose that "pg_regress --config-auth --user XXX" should understand > XXX as the bootstrap superuser name, and then we could clean up > 010_dump_connstr.pl by using that. I have been reviewing that part, and the part to split the bootstrap user from the set of extra roles created looks fine to me. Now, it seems to me that you can simplify 010_dump_connstr.pl as per the attached because PostgresNode::Init can take care of --auth-config part with the correct options using auth_extra. What do you think about the cleanup attached? -- Michael
Вложения
Michael Paquier <michael@paquier.xyz> writes: > I have been reviewing that part, and the part to split the bootstrap > user from the set of extra roles created looks fine to me. Now, it > seems to me that you can simplify 010_dump_connstr.pl as per the > attached because PostgresNode::Init can take care of --auth-config > part with the correct options using auth_extra. What do you think > about the cleanup attached? I haven't checked that this actually works, but it looks plausible, and I agree it's simpler/easier. regards, tom lane
On Wed, Jul 03, 2019 at 09:53:14AM -0400, Tom Lane wrote: > I haven't checked that this actually works, but it looks plausible, > and I agree it's simpler/easier. Thanks, committed. While testing on Windows, I have been trapped by the fact that IPC::Run mishandles double quotes, causing the tests to fail for the environment variable part because of a mismatching pg_hba.conf entry. The difference is that with we run pg_regress --config-auth using IPC::Run::run on HEAD but the patch switches to system(). So I have finished by removing the double-quote handling from the restore user name which makes the whole test suite more consistent. The patch has at the end the advantage of removing in pg_ident.conf the entry related to the OS user running the scripts, which makes the environment more restricted by default. -- Michael