Client GSSENC Request Fix

[AndrewJackson2020]

This PR fixes an issue where a client that attempts to connect to pgcat with GSS encoding is rejected with a confusing error message. By default libpq will attempt to connect with GSS encoding if there is a valid kerberos ticket available on the system which can be checked with the klist command. The server should reject this request at which point libpq will attempt to connect again without gss encoding. This behavior is documented in the postgres documentation.

Do not have any automated tests right now, will need to set up a kerberos environment in the container. Can do this a bit later.

Fixes #792

[AndrewJackson2020]

Just implemented the automated test with the most recent commit. The test simply tries to connect to the database after a valid kerberos ticket has been made available. Can confirm that this test fails without this patch and passes with it.

[drdrsh]

Can you please run

cargo fmt
# and
cargo clippy --all --all-targets -- -Dwarnings

and commit any required changes.

[AndrewJackson2020]

Can you please run

cargo fmt
# and
cargo clippy --all --all-targets -- -Dwarnings

and commit any required changes.

Done

[AndrewJackson2020]

I believe that these tests ran locally because the environment created by start_test_env.sh runs on root. It looks like this is not the case with the circlci environment and thus it fails when it tries to write some of the kerberos config files into root space. Further I had to add a few packages to the test docker container but it looks like the circlecli config.yaml does not build the container from scratch but uses the image ghcr.io/postgresml/pgcat-ci:latest.

Given this I am not sure how to make this new test pass CI on my end. Maybe this would best be a local only test? Let me know how you want to proceed.

[drdrsh]

Maybe you can create a PR that updates the pgcat-ci docker image with the changes you want (https://github.com/postgresml/pgcat/blob/main/Dockerfile.ci) and merge that PR

Perhaps you can make start_test_env.sh run as non-root and see if you can reproduce the failure and work around it?

Sorry, for the hassle. The workflow can be improved and I am trying to reduce the friction for future contributors so your help is appreciated!

[AndrewJackson2020]

I think what you are saying makes a lot of sense. Will see what I can do.

[AndrewJackson2020]

Looking through the repo there are currently 5 different docker files and 3 different docker compose files.

In particular it seems like there is a lot of overlap between the docker assets in ./tests/docker and ./dev as its seems like they have the same objective of creating a dev/local test environment where tests can be run, pgcat can be built, etc. Does it makes sense to have both of these or consolidate them in some way?

Also its seem like ./dev/Dockerfile and ./tests/docker/Dockerfile both use rust:bullseye while ./Dockerfile.ci uses cimg/rust:1.79.0 as their base images. Does it make sense to have different base images between CI and dev environments? Would be nice if the CI environment and the dev/test environments can be as similar as possible.

Also not seeing any reference to ./Dockerfile.dev in ./README.md, comments, etc. Would be nice to get some sort of mention in the ./README.md or as a comment on ./Dockerfile.dev as to what it is used for and how it is different from ./dev/Dockerfile and ./tests/docker/Dockerfile.

Also any reason why go is not installed in ./dev/Dockerfile?

$ find . | grep docker-compose.yml
dev/docker-compose.yaml
docker-compose.yml
tests/docker/docker-compose.yml

 find . | grep Docker
Dockerfile
Dockerfile.ci
Dockerfile.dev
dev/Dockerfile
tests/docker/Dockerfile

[drdrsh]

Yes, this is actually one of the things that I really wanted to fix for the longest time.

For Dockerfile, I think we only really need an official one that is used to build the official image. I think we created the Dockerfile.ci to make running tests faster/cleaner. That's the only files we really need.

As for docker-compose.yml files, we probably only need one for running integration tests and for local dev.

So the grand total should be, two Dockerfiles and one docker-compose.yml

I was planning to add another docker-compose file but that would be for benchmarking so I am thinking I could add a benchmarking directory and puts this setup under it. But for now, yea we only need 2 docker files and one docker compose

[AndrewJackson2020]

Agree, would definitely help DRY out the repo and improve developer experience.