Horton

Re: Key management with tests

Bruce Momjian <bruce@momjian.us>

View thread
From: Bruce Momjian <bruce@momjian.us>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Tom Kincaid <tomjohnkincaid@gmail.com>, Andres Freund <andres@anarazel.de>, Amit Kapila <amit.kapila16@gmail.com>, Robert Haas <robertmhaas@gmail.com>, Thomas Munro <thomas.munro@gmail.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>, Stephen Frost <sfrost@snowman.net>, Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
Date: 2021-01-26T00:09:44Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Rethink method for assigning OIDs to the template0 and postgres DBs.

  2. pg_upgrade: Preserve database OIDs.

  3. pg_upgrade: Preserve relfilenodes and tablespace OIDs.

  4. Fix for new Boolean node

  5. Improve error handling of HMAC computations

  6. Add macro RelationIsPermanent() to report relation permanence

  7. Enhance nbtree index tuple deletion.

On Mon, Jan 25, 2021 at 08:12:01PM -0300, Álvaro Herrera wrote:
> In patch 1,
> 
> * The docs are not clear on what happens if --auth-prompt is not given
> but an auth prompt is required for the program to work.  Should it exit
> with a status other than 0?

Uh, I think the docs talk about this:

	It can prompt from the terminal if
	option>--authprompt</option> is used.  In the parameter
	value, <literal>%R</literal> is replaced by a file descriptor
	number opened to the terminal that started the server.  A file
	descriptor is only available if enabled at server start via
	<option>-R</option>.  If <literal>%R</literal> is specified and
	no file descriptor is available, the server will not start.

The code is:

	case 'R':
	{
		char fd_str[20];

		if (terminal_fd == -1)
		{
			ereport(ERROR,
					(errcode(ERRCODE_INTERNAL_ERROR),
					 errmsg("cluster key command referenced %%R, but --authprompt not specified")));
		}

Does that help?

> * BootStrapKmgr claims it is called by initdb, but that doesn't seem to
> be the case.

Well, initdb starts the postmaster in --boot mode, and that calls
BootStrapKmgr().  Does that help?

> * Also, BootStrapKmgr is the only one that checks USE_OPENSSL; what if a
> with-openssl build inits the datadir, and then a non-openssl runs it?
> What if it's the other way around?  I think you'd get a failure in
> stat() ...

Wow, I never considered that.  I have added a check to InitializeKmgr().
Thanks.

> * ... oh, KMGR_DIR_PID is used but not defined anywhere.  Is it defined
> in some later commit?  If so, then I think you've chosen to split the
> patch series wrong.

OK, fixed.  It is in include/common/kmgr_utils.c, which was in #3.

> May I suggest to use "git format-patch" to produce the patch files?  When
> working with a series like this, trying to do patch handling manually
> like you seem to be doing, is much more time-consuming and error prone.
> For example, with a branch containing individual commits, you could use 
>   git rebase -i origin/master -x "make install check-world"
> or similar, so that each commit is built and tested individually.

I used "git format-patch".  Are you asking for seven commits that then
generate seven files via one format-patch run?  Or is the primary issue
that you want compile testing for each patch?

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  The usefulness of a cup is in its emptiness, Bruce Lee