Re: PATCH: Add GSSAPI ccache_name option to libpq - Mailing list pgsql-hackers

Hi Daniel,

> It's my first go at submitting a patch -- it works as far as I can tell,
> but I suspect there will probably still be stuff to fix before it's
> ready to use!

You are doing great :)

> As far as I'm concerned this is working (the code compiles successfully
> following "./configure --with-gssapi --enable-cassert", and seems to
> work for specifying the ccache location without any noticeable errors).

There are several other things worth checking:
0. Always run `make distclean` before following steps
1. Make sure `make -j4 world && make -j4 check-world` passes
2. Make sure `make install-world` and `make installcheck-world` passes
3. Since you are changing the documentation it's worth checking that
it displays properly. The documentation is in the
$(PGINSTALL)/share/doc/postgresql/html directory

Several years ago I published some scripts that simplify all this a
little: https://github.com/afiskon/pgscripts, especially step 3. They
may require some modifications for your OS of choice. Please read
https://wiki.postgresql.org/wiki/Submitting_a_Patch for more
information.

Generally speaking, it also a good idea to add some test cases for
your code, although I understand why it might be a little complicated
in this particular case. Maybe you could at least tell us how it can
be checked manually that this code actually does what is supposed to?

On Tue, Apr 20, 2021 at 12:37 PM Daniel Carter
<danielchriscarter+postgres@gmail.com> wrote:
>
> Hi,
>
> This is a small patch (against master) to allow an application using
> libpq with GSSAPI authentication to specify where to fetch the
> credential cache from -- it effectively consists of a new field in
> PQconninfoOptions to store this data and (where the user has specified a
> ccache location) a call into the gss_krb5_ccache_name function in the
> GSSAPI library.
>
> It's my first go at submitting a patch -- it works as far as I can tell,
> but I suspect there will probably still be stuff to fix before it's
> ready to use!
>
> As far as I'm concerned this is working (the code compiles successfully
> following "./configure --with-gssapi --enable-cassert", and seems to
> work for specifying the ccache location without any noticeable errors).
>
> I hope there shouldn't be anything platform-specific here (I've been
> working on Ubuntu Linux but the only interactions with external
> applications are via the GSSAPI library, which was already in use).
>
> The dispsize value for ccache_name is 64 in this code (which seems to be
> what's used with other file-path-like parameters in the existing code)
> but I'm happy to have this corrected if it needs a different value -- as
> far as I can tell this is just for display purposes rather than anything
> critical in terms of actually storing the value?
>
> If no ccache_name is specified in the connection string then it defaults
> to NULL, which means the gss_krb5_ccache_name call is not made and the
> current behaviour (of letting the GSSAPI library work out the location
> of the ccache) is not changed.
>
> Many thanks,
> Daniel
>


-- 
Best regards,
Aleksander Alekseev