Re: Add a new BGWORKER_BYPASS_ROLELOGINCHECK flag

On Thu, Oct 5, 2023 at 12:22 PM Drouvot, Bertrand
<bertranddrouvot(dot)pg(at)gmail(dot)com> wrote:
>
> >
> > @@ -719,6 +719,7 @@ InitPostgres(const char *in_dbname, Oid dboid,
> > const char *username, Oid useroid,
> > bool load_session_libraries,
> > bool override_allow_connections,
> > + bool bypass_login_check,
> > char *out_dbname)
> >
> > I was not paying much attention here, but load_session_libraries gives
> > a good argument in favor of switching all these booleans to a single
> > bits32 argument as that would make three of them now, with a different
> > set of flags than the bgworker ones. This can be refactored on its
> > own.
>
> Yeah good point, will work on it once the current one is committed.
>
> >
> > -#define BGWORKER_BYPASS_ALLOWCONN 1
> > +#define BGWORKER_BYPASS_ALLOWCONN 0x0001
> >
> > Can be a change of its own as well.
>
> Yeah, but I think it's simple enough to just keep this change here
> (and I don't think it's "really" needed without introducing 0x0002)

I think changing BGWORKER_BYPASS_ALLOWCONN to 0x0001 and having bits32
for InitPostgres inputs load_session_libraries and
override_allow_connections can be 0001 in this patch series so that it
can go first, no? This avoids the new code being in the old format and
changing things right after it commits.

v6-0001 LGTM.

A comment on v6-0002:
1.
+ CREATE ROLE nologrole with nologin;
+ ALTER ROLE nologrole with superuser;
+]);
We don't need superuser privileges here, do we? Or do we need it for
the worker_spi to access pg_catalog and stuff in worker_spi_main? If
not, can we remove it to showcase non-superusers requesting bg
workers?

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com