Horton

Re: Allow file inclusion in pg_hba and pg_ident files

Julien Rouhaud <rjuju123@gmail.com>

View thread View on postgr.es
From: Julien Rouhaud <rjuju123@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Nathan Bossart <nathandbossart@gmail.com>
Date: 2022-10-24T08:03:03Z
Lists: pgsql-hackers 
Hi,

On Mon, Oct 24, 2022 at 04:13:51PM +0900, Michael Paquier wrote:
> On Mon, Oct 24, 2022 at 01:33:12PM +0800, Julien Rouhaud wrote:
> > v12 attached, fixing multiple conflicts with recent activity.
>
>  typedef struct TokenizedAuthLine
>  {
>     List       *fields;         /* List of lists of AuthTokens */
> +   char       *file_name;      /* File name *
>
> Hmm.  While putting my eyes on this patch set for the first time in a
> few months (sorry!), the addition of file_name to TokenizedAuthLine in
> 0002 stands out.  This impacts all the macros used for the validation
> of the ident and HBA lines, leading as well to a lot of bloat in the
> patch with patterns like that in 20~25 places:
>                  errcontext("line %d of configuration file \"%s\"", \
> -                           line_num, IdentFileName))); \
> +                           line_num, file_name))); \
> [...]
>                          errcontext("line %d of configuration file \"%s\"",
> -                                   line_num, HbaFileName)));
> +                                   line_num, file_name)));
>
> Do you think that it would make sense to split that into its own
> patch?  That would move the code toward less references to HbaFileName
> and IdentFileName, which is one step toward what we want for this
> thread.  This tokenization of HBA and ident files is already shared,
> so moving this knowledge into TokenizedAuthLine looks helpful
> independently of the rest.

It would also require to bring HbaLine->sourcefile.  I'm afraid it would be
weird to introduce such a refactoring in a separate commit just to pass a
constant down multiple level of indirection, as all the macro will remain
specific to either hba or ident anyway.

I agree that there are quite a lot of s/XXXFileName/file_name/, but those
aren't complicated, and keeping them in the same commit makes it easy to
validate that none has been forgotten since the regression tests covering those
messages are in that commit too.

And of course while double checking that none was forgotten I realize that I
missed the new regcomp_auth_token() which introduced a couple new usage of
HbaFileName.

Commits

  1. Add TAP tests for include directives in HBA end ident files

  2. Introduce variables for initial and max nesting depth on configuration files

  3. Add support for file inclusions in HBA and ident configuration files

  4. Add missing initialization in tokenize_expand_file() for output list

  5. Rework memory contexts in charge of HBA/ident tokenization

  6. Add error context callback when tokenizing authentication files

  7. Invent open_auth_file() in hba.c to refactor authentication file opening

  8. Use AbsoluteConfigLocation() when building an included path in hba.c

  9. Move code related to configuration files in directories to new file

  10. doc: Fix some descriptions related to pg_ident_file_mappings

  11. Add rule_number to pg_hba_file_rules and map_number to pg_ident_file_mappings

  12. Refactor code handling the names of files loaded in hba.c

  13. Make consistent a couple of log messages when parsing HBA files

  14. Use hba_file/ident_file GUCs rather than pg_hba.conf/pg_ident.conf in logs

  15. Fix path reference when parsing pg_ident.conf for pg_ident_file_mappings

  16. Add system view pg_ident_file_mappings

  17. Modify query on pg_hba_file_rules to check for errors in regression tests

  18. Refactor code related to pg_hba_file_rules() into new file