Show inline comments from pg_hba lines in the pg_hba_file_rules view
Hi,
Often we make changes in the pg_hba.conf and leave a #comment there,
just in case we forget why the change was done. To avoid having to open
the configuration file every time just to check the comments, it would
be quite nice to have the option to read these comments in the
pg_hba_file_rules view. Something like adding it in the end of the line
and wrapping it with characters like "", '', {}, [], etc
For instance, this pg_hba.conf ...
# TYPE DATABASE USER ADDRESS METHOD
local all all trust [foo]
host all all 127.0.0.1/32 trust
host all all ::1/128 trust [bar]
local replication all trust
host replication all 127.0.0.1/32 trust
hostssl replication all ::1/128 cert map=abc [this will
fail :)]
... could be displayed like this
postgres=# SELECT type, database, user_name, address, comment, error
FROM pg_hba_file_rules ;
type | database | user_name | address | comment | error
---------+---------------+-----------+-----------+-------------------+-----------------------------------------------------
local | {all} | {all} | | foo |
host | {all} | {all} | 127.0.0.1 | |
host | {all} | {all} | ::1 | bar |
local | {replication} | {all} | | |
host | {replication} | {all} | 127.0.0.1 | |
hostssl | {replication} | {all} | ::1 | this will fail :) |
hostssl record cannot match because SSL is disabled
(6 rows)
I wrote a very quick&dirty PoC (attached) but before going any further I
would like to ask if there is a better way to read these comments using
SQL - or if it makes sense at all ;-)
Any feedback is much appreciated. Thanks!
Jim
Attachments:
hba-comments.difftext/x-patch; charset=UTF-8; name=hba-comments.diffDownload
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index ac602bfc37..b15c913e14 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -211,6 +211,15 @@ next_token(char **lineptr, StringInfo buf,
while (c != '\0' &&
(!pg_isblank(c) || in_quote))
{
+
+ if (c == '[' && !in_quote)
+ {
+ appendStringInfoChar(buf, c);
+ while ((c = (*(*lineptr)++)) != '\0')
+ appendStringInfoChar(buf, c);
+ break;
+ }
+
/* skip comments to EOL */
if (c == '#' && !in_quote)
{
@@ -1861,6 +1870,15 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel)
str = pstrdup(token->string);
val = strchr(str, '=');
+
+ if(str[0]=='[' && str[strlen(str)-1]==']')
+ {
+ str = str + 1 ;
+ str[strlen(str)-1]='\0';
+ parsedline->comments = str;
+ continue;
+ }
+
if (val == NULL)
{
/*
diff --git a/src/backend/utils/adt/hbafuncs.c b/src/backend/utils/adt/hbafuncs.c
index 73d3ad1dad..389c14bc2e 100644
--- a/src/backend/utils/adt/hbafuncs.c
+++ b/src/backend/utils/adt/hbafuncs.c
@@ -159,7 +159,7 @@ get_hba_options(HbaLine *hba)
}
/* Number of columns in pg_hba_file_rules view */
-#define NUM_PG_HBA_FILE_RULES_ATTS 11
+#define NUM_PG_HBA_FILE_RULES_ATTS 12
/*
* fill_hba_line
@@ -346,6 +346,12 @@ fill_hba_line(Tuplestorestate *tuple_store, TupleDesc tupdesc,
values[index++] = PointerGetDatum(options);
else
nulls[index++] = true;
+
+ /* comments */
+ if(hba->comments)
+ values[index++] = CStringGetTextDatum(hba->comments);
+ else
+ nulls[index++] = true;
}
else
{
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 9805bc6118..360f71e8ef 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -6244,9 +6244,9 @@
{ oid => '3401', descr => 'show pg_hba.conf rules',
proname => 'pg_hba_file_rules', prorows => '1000', proretset => 't',
provolatile => 'v', prorettype => 'record', proargtypes => '',
- proallargtypes => '{int4,text,int4,text,_text,_text,text,text,text,_text,text}',
- proargmodes => '{o,o,o,o,o,o,o,o,o,o,o}',
- proargnames => '{rule_number,file_name,line_number,type,database,user_name,address,netmask,auth_method,options,error}',
+ proallargtypes => '{int4,text,int4,text,_text,_text,text,text,text,_text,text,text}',
+ proargmodes => '{o,o,o,o,o,o,o,o,o,o,o,o}',
+ proargnames => '{rule_number,file_name,line_number,type,database,user_name,address,netmask,auth_method,options,comment,error}',
prosrc => 'pg_hba_file_rules' },
{ oid => '6250', descr => 'show pg_ident.conf mappings',
proname => 'pg_ident_file_mappings', prorows => '1000', proretset => 't',
diff --git a/src/include/libpq/hba.h b/src/include/libpq/hba.h
index 189f6d0df2..5cb7224712 100644
--- a/src/include/libpq/hba.h
+++ b/src/include/libpq/hba.h
@@ -135,6 +135,7 @@ typedef struct HbaLine
char *radiusidentifiers_s;
List *radiusports;
char *radiusports_s;
+ char *comments;
} HbaLine;
typedef struct IdentLine
On Fri, Sep 01, 2023 at 12:01:37AM +0200, Jim Jones wrote:
Often we make changes in the pg_hba.conf and leave a #comment there, just in
case we forget why the change was done. To avoid having to open the
configuration file every time just to check the comments, it would be quite
nice to have the option to read these comments in the pg_hba_file_rules
view. Something like adding it in the end of the line and wrapping it with
characters like "", '', {}, [], etcFor instance, this pg_hba.conf ...
# TYPE DATABASE USER ADDRESS METHOD
local all all trust [foo]
host all all 127.0.0.1/32 trust
host all all ::1/128 trust [bar]
local replication all trust
host replication all 127.0.0.1/32 trust
hostssl replication all ::1/128 cert map=abc [this will fail
:)]... could be displayed like this
hba.c is complex enough these days (inclusion logic, tokenization of
the items) that I am not in favor of touching its code paths for
anything like that. This is not something that can apply only to
pg_hba.conf, but to all configuration files. And this touches in
adding support for a second type of comment format. This is one of
these areas where we may want a smarter version of pg_read_file that
returns a SRF for (line_number, line_contents) of a file read? Note
that it is possible to add comments at the end of a HBA entry already,
like:
local all all trust # My comment, and this is a correct HBA entry.
--
Michael
Hi Michael
On 01.09.23 03:18, Michael Paquier wrote:
hba.c is complex enough these days (inclusion logic, tokenization of
the items) that I am not in favor of touching its code paths for
anything like that. This is not something that can apply only to
pg_hba.conf, but to all configuration files.
It is indeed possible to extrapolate it to any configuration file, but
my point was rather to visualize comments purposefully left by the DBA
regarding user access (pg_hba and pg_ident).
And this touches in
adding support for a second type of comment format. This is one of
these areas where we may want a smarter version of pg_read_file that
returns a SRF for (line_number, line_contents) of a file read? Note
that it is possible to add comments at the end of a HBA entry already,
like:
local all all trust # My comment, and this is a correct HBA entry.
I also considered parsing the inline #comments - actually it was my
first idea - but I thought it would leave no option to make an inline
comment without populating pg_hba_file_rules. But I guess in this case
one could always write the comment in the line above :)
Would you be in favor of parsing #comments instead? Given that # is
currently already being parsed (ignored), it shouldn't add too much
complexity to the code.
Thanks for the feedback.
Jim
On Fri, Sep 01, 2023 at 11:32:35AM +0200, Jim Jones wrote:
Would you be in favor of parsing #comments instead? Given that # is
currently already being parsed (ignored), it shouldn't add too much
complexity to the code.
I am not sure what you have in mind, but IMO any solution would live
better as long as a solution is:
- not linked to hba.c, handled in a separate code path.
- linked to all configuration files where comments are supported, if
need be.
Perhaps others have more opinions.
--
Michael
On 01.09.23 12:44, Michael Paquier wrote:
I am not sure what you have in mind, but IMO any solution would live
better as long as a solution is:
- not linked to hba.c, handled in a separate code path.
- linked to all configuration files where comments are supported, if
need be.
If I understood you correctly: You mean an independent feature that i.e.
gets raw lines and parses the inline #comments.
Doing so we could indeed avoid the trouble of messing around with the
hba.c logic, and it would be accessible to other config files. Very
interesting thought! It sounds like a much more elegant solution.
Perhaps others have more opinions.
--
Michael
If I hear no objections, I'll try to sketch it as you suggested.
Thanks again for the feedback
Jim