Re: [PATCH] pg_hba.conf : new auth option : clientcert=verify-full

Поиск
Список
Период
Сортировка
От Magnus Hagander
Тема Re: [PATCH] pg_hba.conf : new auth option : clientcert=verify-full
Дата
Msg-id CABUevEyY+gON=2WQhM4s2UnUvpWjg=DyriBxUAtjM+J3YOuE9g@mail.gmail.com
обсуждение исходный текст
Ответ на Re: [PATCH] pg_hba.conf : new auth option : clientcert=verify-full  (Magnus Hagander <magnus@hagander.net>)
Ответы Re: [PATCH] pg_hba.conf : new auth option : clientcert=verify-full  (Julian Markwort <julian.markwort@uni-muenster.de>)
Список pgsql-hackers
On Sun, Apr 1, 2018 at 6:07 PM, Magnus Hagander <magnus@hagander.net> wrote:
On Sun, Apr 1, 2018 at 6:01 PM, Julian Markwort <julian.markwort@uni-muenster.de> wrote:
On 1. of April 2018 17:46:38 MESZ wrote Magnus Hagander <magnus@hagander.net>:

>I assume this is a patch that's intended to be applied on top of the
>previous patch? If so, please submit the complete pach to make sure the
>correct combination ends up actually being reviewed.

The v02.patch attached to my last mail contains both source and documentation changes.

Hmm. I think I may have been looking at the wrong file. Sorry!


>Keeping patches as short as possible is not a good thing itself. The
>important part is that the resulting code and documentation is the best
>possible. Sometimes you might want to turn it into two patches
>submitted at
>the same time if one is clearly just reorganisation, but avoiding code
>restructure just to keep the lines of patch down is not helpful.

I think I've made the right compromises regarding readability of the documentation in my patch then.

A happy Easter, passover, or Sunday to you

You, too!

(I shall return to reviewing it after the holidays are over) 


I've been through this one again.

There is one big omission from it -- it fails to work with the view pg_hba_file_rules. When fixing that, things started to look kind of ugly with the "two booleans to indicate one thing", so I went ahead and changed it to instead be an enum of 3 values.

Also, now when using verify-full or verify-ca, I figured its a lot easier to misspell the value, and we don't want that to mean "off". Instead, I made it give an error in this case instead of silently treating it as 0.

The option "2" for clientcert was never actually documented, and I think we should get rid of that. We need to keep "1" for backwards compatibility (which arguably was a bad choice originally, but that was many years ago), but let's not add another one. 

I also made a couple of very small changes to the docs.

Attached is an updated patch with these changes. I'd appreciate it if you can run it through your tests to confirm that it didn't break any of those usecases.

--
Вложения

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Tom Lane
Дата:
Сообщение: Documentation for bootstrap data conversion
Следующее
От: Tomas Vondra
Дата:
Сообщение: Re: Online enabling of checksums